Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

[fix] Properly handle options when using an engine #98

Closed
wants to merge 1 commit into from

3 participants

@siddMahen

Added a test to back this up, but npm test doesn't seem to work. For what it's worth, the same number of tests are failing before and after this patch.

@pksunkara

Will review soon but there seems to be some unnecessary changes.

@Marak

It's a good idea to fix this. +1

@siddMahen - Can you clarify about broken tests? According to our continuous integration, everything should be passing now as per master.

see: http://travis-ci.org/#!/flatiron/resourceful

@siddMahen

Hmm very strange... I get a whole bunch of errors. npm test results in the following on my machine (OSX Lion):

$ node -v
v0.8.3
$ npm test

> resourceful@0.3.0 test /Users/siddharth_mahen/GitProjects/resourceful
> vows --spec


  ♢ resourceful/resource/cache 

  When creating an instance, and saving it and then loading it back up with get()
    ✓ it should return the previous instance
  When creating an instance, and saving it and then loading it back up with find()
    ✓ it should return the previous instance
  When creating an instance, and saving it and then clearing the cache and loading it back up with get()
    ✓ it should return a new instance

  ♢ resourceful/resource/view 

  A database containing articles and other resources
    ✓ is created
  A database containing articles and other resources A Resource definition with filters
    ✓ should respond to the filters
  A database containing articles and other resources A Resource definition with filters can be used to query the database: <by> 'yoda'
    ✗ should return an array of Article records by 'yoda' 
        » expected undefined to be an Array // couchdb-filter-test.js:93
  A database containing articles and other resources A Resource definition with filters can be used to query the database: <by> 'cloudhead'
    ✗ should return an array of Article records by 'cloudhead' 
        » expected undefined to be an Array // couchdb-filter-test.js:79
  A database containing articles and other resources A Resource definition with filters can be used to query the database: <all>
    ✗ should return an array of all Article records 
        » expected undefined to be an Array // couchdb-filter-test.js:70
  A database containing articles and other resources A Resource definition with filters can be used to query the database: <published>
    ✗ should return an array of all published Articles 
        » expected undefined to be an Array // couchdb-filter-test.js:54
  A second Resource definition with filters
    ✓ should have no side effects on the first resource views

  ♢ resourceful/deferredRelationship 

  Defining a child resource of non-defined parent
    ✓ should be successful
    ✓ should have no parent
  Defining a child resource of non-defined parent and defining the parent resource
    ✓ should be successful
    ✓ should have child set
    ✓ and child should have parent set

  ♢ resourceful/engines/couchdb 

  In database "test" with defined resources "book"
    ✓ will be successful
  In database "test" with defined resources "author"
    ✓ will be successful
  In database "test" with defined resources "creature"
    ✓ will be successful
  In database "test" an Resource.all() request
    ✗ should respond with an array of all records 
        » expected null, got { 
      stack: 'Error: connect ECONNREFUSED\n    at errnoException (net.js:776:11)\n    at Object.afterConnect [as oncomplete] (net.js:767:19)', 
      arguments: undefined, 
      message: 'connect ECONNREFUSED', 
      errno: 'ECONNREFUSED', 
      type: undefined, 
      syscall: 'connect', 
      code: 'ECONNREFUSED' 
  } // net.js:776
  In database "test" a Resource.get() request when unsuccessful
    ✗ should respond with an error 
        » expected 404, 
    got  undefined (==) // engines-test.js:79
  In database "test" a Resource.get() request when successful
    ✗ should respond with a Resource instance 
        » expected null, got { 
      stack: 'Error: connect ECONNREFUSED\n    at errnoException (net.js:776:11)\n    at Object.afterConnect [as oncomplete] (net.js:767:19)', 
      arguments: undefined, 
      message: 'connect ECONNREFUSED', 
      errno: 'ECONNREFUSED', 
      type: undefined, 
      syscall: 'connect', 
      code: 'ECONNREFUSED' 
  } // net.js:776
    ✗ should respond with the right object 
        » expected null, got { 
      stack: 'Error: connect ECONNREFUSED\n    at errnoException (net.js:776:11)\n    at Object.afterConnect [as oncomplete] (net.js:767:19)', 
      arguments: undefined, 
      message: 'connect ECONNREFUSED', 
      errno: 'ECONNREFUSED', 
      type: undefined, 
      syscall: 'connect', 
      code: 'ECONNREFUSED' 
  } // net.js:776
    ✗ should not be a new record 
        » expected null, got { 
      stack: 'Error: connect ECONNREFUSED\n    at errnoException (net.js:776:11)\n    at Object.afterConnect [as oncomplete] (net.js:767:19)', 
      arguments: undefined, 
      message: 'connect ECONNREFUSED', 
      errno: 'ECONNREFUSED', 
      type: undefined, 
      syscall: 'connect', 
      code: 'ECONNREFUSED' 
  } // net.js:776
  In database "test" a Resource.create() request
    ✗ should return the newly created object 
        » expected null, got { 
      stack: 'Error: connect ECONNREFUSED\n    at errnoException (net.js:776:11)\n    at Object.afterConnect [as oncomplete] (net.js:767:19)', 
      arguments: undefined, 
      message: 'connect ECONNREFUSED', 
      errno: 'ECONNREFUSED', 
      type: undefined, 
      syscall: 'connect', 
      code: 'ECONNREFUSED' 
  } // net.js:776
    ✗ should not be a new record 
        » expected null, got { 
      stack: 'Error: connect ECONNREFUSED\n    at errnoException (net.js:776:11)\n    at Object.afterConnect [as oncomplete] (net.js:767:19)', 
      arguments: undefined, 
      message: 'connect ECONNREFUSED', 
      errno: 'ECONNREFUSED', 
      type: undefined, 
      syscall: 'connect', 
      code: 'ECONNREFUSED' 
  } // net.js:776


✗ Errored » Asynchronous Error 
      in resourceful/engines/couchdb 
      in test/engines-test.js
✗ Errored » 14 honored ∙ 11 broken ∙ 1 errored

npm ERR! Test failed.  See above for more details.
npm ERR! not ok code 0
@Marak

Can you confirm that tests have access and are connecting to a running couchdb?

All those ECONNREFUSED make me think the couch can't be found.

@siddMahen

I looked into the test/engines/couch.js and made the following changes:

[siddharth_mahen:resourceful] $ git diff
diff --git a/test/engines/couchdb.js b/test/engines/couchdb.js
index 247445c..c2d408c 100644
--- a/test/engines/couchdb.js
+++ b/test/engines/couchdb.js
@@ -3,7 +3,7 @@ var cradle = require('cradle');
 var engine = exports;

 engine.name = 'couchdb';
-engine.options = { database: 'test' };
+engine.options = { uri: "couchdb://nodejitsudb918533120327.iriscouch.com:5984/test", database: 'test' };

 engine.load = function (resourceful, data, callback) {
   var db = new(cradle.Connection)(engine.options).database(engine.options.database);
[siddharth_mahen:resourceful] $ git branch
  fix-engine-ambiguity
* master
[siddharth_mahen:resourceful] $ 

Which resulted in the following test result. Still failing, except this time, with other errors. Note that this is on master, and not this PR branch.



> resourceful@0.3.0 test /Users/siddharth_mahen/GitProjects/resourceful
> vows --spec


  ♢ resourceful/resource/cache 

  When creating an instance, and saving it and then loading it back up with get()
    ✓ it should return the previous instance
  When creating an instance, and saving it and then loading it back up with find()
    ✓ it should return the previous instance
  When creating an instance, and saving it and then clearing the cache and loading it back up with get()
    ✓ it should return a new instance

  ♢ resourceful/resource/view 

  A database containing articles and other resources
    ✓ is created
  A database containing articles and other resources A Resource definition with filters
    ✓ should respond to the filters
  A database containing articles and other resources A Resource definition with filters can be used to query the database: <by> 'cloudhead'
    ✗ should return an array of Article records by 'cloudhead' 
        » expected 3, 
    got  0 (==) // couchdb-filter-test.js:80
  A database containing articles and other resources A Resource definition with filters can be used to query the database: <published>
    ✗ should return an array of all published Articles 
        » expected 3, 
    got  0 (==) // couchdb-filter-test.js:55
  A database containing articles and other resources A Resource definition with filters can be used to query the database: <by> 'yoda'
    ✗ should return an array of Article records by 'yoda' 
        » expected 1, 
    got  0 (==) // couchdb-filter-test.js:94
  A database containing articles and other resources A Resource definition with filters can be used to query the database: <all>
    ✗ should return an array of all Article records 
        » expected 5, 
    got  0 (==) // couchdb-filter-test.js:71
  A second Resource definition with filters
    ✓ should have no side effects on the first resource views

  ♢ resourceful/deferredRelationship 

  Defining a child resource of non-defined parent
    ✓ should be successful
    ✓ should have no parent
  Defining a child resource of non-defined parent and defining the parent resource
    ✓ should be successful
    ✓ should have child set
    ✓ and child should have parent set

  ♢ resourceful/engines/couchdb 

  In database "test" with defined resources "book"
    ✓ will be successful
  In database "test" with defined resources "author"
    ✓ will be successful
  In database "test" with defined resources "creature"
    ✓ will be successful
  In database "test" an Resource.all() request
    ✗ should respond with an array of all records 
        » expected 3, 
    got  0 (==) // engines-test.js:39
  In database "test" a Resource.get() request when unsuccessful
    ✓ should respond with an error
  In database "test" a Resource.get() request when successful
    ✗ should respond with a Resource instance 
        » expected null, got { 
      reason: 'missing', 
      status: 404, 
      error: 'not_found', 
      headers: { 
          content-type: 'text/plain; charset=utf-8', 
          server: 'CouchDB/1.2.0 (Erlang OTP/R15B)', 
          cache-control: 'must-revalidate', 
          status: 404, 
          date: 'Mon, 06 Aug 2012 16:04:06 GMT', 
          content-length: '41' 
      } 
  } // engines-test.js:57
    ✗ should respond with the right object 
        » expected null, got { 
      reason: 'missing', 
      status: 404, 
      error: 'not_found', 
      headers: { 
          content-type: 'text/plain; charset=utf-8', 
          server: 'CouchDB/1.2.0 (Erlang OTP/R15B)', 
          cache-control: 'must-revalidate', 
          status: 404, 
          date: 'Mon, 06 Aug 2012 16:04:06 GMT', 
          content-length: '41' 
      } 
  } // engines-test.js:63
    ✗ should not be a new record 
        » expected null, got { 
      reason: 'missing', 
      status: 404, 
      error: 'not_found', 
      headers: { 
          content-type: 'text/plain; charset=utf-8', 
          server: 'CouchDB/1.2.0 (Erlang OTP/R15B)', 
          cache-control: 'must-revalidate', 
          status: 404, 
          date: 'Mon, 06 Aug 2012 16:04:06 GMT', 
          content-length: '41' 
      } 
  } // engines-test.js:70
  In database "test" a Resource.create() request
    ✓ should return the newly created object
    ✓ should not be a new record
  In database "test" a Resource.create() request should create the record in the db
    ✓ should respond with a Resource instance
    ✓ should respond with the right object
    ✓ should not be a new record
  In database "test" a diffirent Resource.create() request with the same id
    ✓ should return the newly created object
    ✓ should not be a new record
  In database "test" a diffirent Resource.create() request with the same id should create the record in the db
    ✓ should respond with a Resource instance
    ✓ should respond with the right object
    ✓ should not be a new record
  In database "test" and a Resource.destroy() request
    ✓ should be successful
  In database "test" and a Resource.destroy() request and Resource.get() the destroyed object
    ✓ should respond with an error
  Instantiating a new instance
    ✓ should be a new record
  Instantiating a new instance should not be in the db
    ✓ should respond with an error
  Instantiating a new instance
    ✓ should be a new record
  Instantiating a new instance a Resource.prototype.save() request
    ✓ should respond with a Resource instance
    ✓ should respond with the right object
    ✓ should not be a new record
  Instantiating a new instance a Resource.prototype.save() request should create the object in db
    ✓ should respond with a Resource instance
    ✓ should respond with the right object
    ✓ should not be a new record
  In database "test" a Resource.find() request when unsuccessful
    ✓ should respond with an empty array
  In database "test" a Resource.find() request when successful
    ✗ should respond with an array of length 2 
        » expected 2, 
    got  0 (==) // engines-test.js:279
    ✗ should respond with an array of Resource instances 
        » expected undefined to be an instance of  // engines-test.js:284
    ✗ should not be a new record 
      TypeError: Cannot read property 'isNewRecord' of undefined 
      at Object.engines.forEach.vows.describe.addBatch.addBatch.addBatch.addBatch.addBatch.addBatch.addBatch.addBatch.addBatch.In database "test".a Resource.find() request.when successful.should not be a new record (/Users/siddharth_mahen/GitProjects/resourceful/test/engines-test.js:297:34) 
      at runTest (/Users/siddharth_mahen/GitProjects/resourceful/node_modules/vows/lib/vows.js:132:26) 
      at EventEmitter.<anonymous> (/Users/siddharth_mahen/GitProjects/resourceful/node_modules/vows/lib/vows.js:78:9) 
      at EventEmitter.emit (events.js:115:20) 
      at EventEmitter.vows.describe.options.Emitter.emit (/Users/siddharth_mahen/GitProjects/resourceful/node_modules/vows/lib/vows.js:237:24) 
      at env.callback.that.emitter.ctx (/Users/siddharth_mahen/GitProjects/resourceful/node_modules/vows/lib/vows/context.js:32:52) 
      at env.callback (/Users/siddharth_mahen/GitProjects/resourceful/node_modules/vows/lib/vows/context.js:46:29) 
      at Resource._request (/Users/siddharth_mahen/GitProjects/resourceful/lib/resourceful/resource.js:166:13) 
      at Function.Resource.runAfterHooks (/Users/siddharth_mahen/GitProjects/resourceful/lib/resourceful/resource.js:93:12) 
      at Resource._request (/Users/siddharth_mahen/GitProjects/resourceful/lib/resourceful/resource.js:162:14)
  In database "test"
    ✗ it should have 'bob' object 
        » expected null, got { 
      reason: 'missing', 
      status: 404, 
      error: 'not_found', 
      headers: { 
          content-type: 'text/plain; charset=utf-8', 
          server: 'CouchDB/1.2.0 (Erlang OTP/R15B)', 
          cache-control: 'must-revalidate', 
          status: 404, 
          date: 'Mon, 06 Aug 2012 16:04:11 GMT', 
          content-length: '41' 
      } 
  } // engines-test.js:319
    ✗ should not be a new record 
        » expected null, got { 
      reason: 'missing', 
      status: 404, 
      error: 'not_found', 
      headers: { 
          content-type: 'text/plain; charset=utf-8', 
          server: 'CouchDB/1.2.0 (Erlang OTP/R15B)', 
          cache-control: 'must-revalidate', 
          status: 404, 
          date: 'Mon, 06 Aug 2012 16:04:11 GMT', 
          content-length: '41' 
      } 
  } // engines-test.js:326


✗ Errored » Asynchronous Error 
      in resourceful/engines/couchdb 
      in test/engines-test.js
✗ Errored » 37 honored ∙ 12 broken ∙ 2 errored

npm ERR! Test failed.  See above for more details.
npm ERR! not ok code 0
@siddMahen

@marak would it be possible to merge this and handle the testing bugs in another ticket?

@pksunkara

I will do this tomorrow, Thanks for reminding

@pksunkara

Cherry-picked in 71ca310

@pksunkara pksunkara closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Aug 5, 2012
  1. @siddMahen
This page is out of date. Refresh to see the latest.
View
55 lib/resourceful/core.js
@@ -15,25 +15,28 @@ resourceful.deferredRelationships = {};
// Select a storage engine
//
resourceful.use = function (engine, options) {
- if (typeof(engine) === "string") {
- engine = common.capitalize(engine);
-
- if (resourceful.engines[engine]) {
- this.engine = resourceful.engines[engine];
- }
- else {
- throw new Error("unrecognised engine: " + engine);
- }
- }
- else if (typeof engine === 'function') {
- this.engine = engine;
- }
- else {
- throw new Error("invalid engine ");
+ switch (typeof(engine)) {
+ case "string":
+ engine = common.capitalize(engine);
+
+ if (resourceful.engines[engine]) {
+ this.engine = resourceful.engines[engine];
+ }
+ else {
+ throw new Error("unrecognised engine: " + engine);
+ }
+
+ break;
+ case "function":
+ this.engine = engine;
+ break;
+ default:
+ throw new Error("invalid engine");
}
this.key = this.engine.key || 'id';
this.connect(options || {});
+
return this;
};
//
@@ -53,15 +56,21 @@ resourceful.connect = function (/* [uri], [port], [options] */) {
case 'object': options = a; break;
}
});
- // Extract the optional 'protocol'
- // ex: "couchdb://127.0.0.1" would have "database" as protocol.
- if (m = options.uri && options.uri.match(/^([a-z]+):\/\//)) {
- protocol = m[1];
- options.uri = options.uri.replace(protocol + '://', '');
- }
- if (protocol) {
- engine = resourceful.engines[common.capitalize(protocol)];
+ // Extract the optional 'protocol' if we haven't already selected an engine
+ // ex: "couchdb://127.0.0.1" would have "couchdb" as it's protocol.
+
+ if (!this.engine) {
+ if (m = options.uri && options.uri.match(/^([a-z]+):\/\//)) {
+ protocol = common.capitalize(m[1]);
+
+ if (resourceful.engines[protocol]) {
+ engine = resourceful.engines[protocol];
+ }
+ else {
+ throw new Error("unrecognised engine: " + engine);
+ }
+ }
}
else {
engine = resourceful.engine || this.engine;
View
6 lib/resourceful/engines/couchdb/index.js
@@ -14,10 +14,10 @@ var url = require('url'),
var Couchdb = exports.Couchdb = function Couchdb(config) {
if (config.uri) {
- var parsed = url.parse('couchdb://' + config.uri);
+ var parsed = url.parse(config.uri);
config.uri = parsed.hostname;
- config.port = parseInt(parsed.port, 10);
- config.database = (parsed.pathname || '').replace(/^\//, '');
+ config.port = parseInt(config.port || parsed.port, 10);
+ config.database = config.database || ((parsed.pathname || '').replace(/\//g, ''));
}
this.connection = new(cradle.Connection)({
View
1  lib/resourceful/engines/memory.js
@@ -4,7 +4,6 @@ var resourceful = require('../../resourceful'),
exports.stores = {};
exports.caches = {};
-
var Memory = exports.Memory = function (options) {
var counter = 0;
options = options || {};
View
86 test/resourceful-test.js
@@ -342,15 +342,85 @@ vows.describe('resourceful').addVows({
}
},
"Engine can be initialised":{
- "by name": function () {
- resourceful.use('memory');
- assert.isFunction(resourceful.engine);
- assert.equal(resourceful.connection.protocol, 'memory');
+ "by name": {
+ "with options": {
+ topic: function() {
+ return function(r) {
+ assert.equal(r.connection.host, "test");
+ assert.equal(r.connection.port, 123);
+ assert.equal(r.name, "db");
+ }
+ },
+ "uri": function(f) {
+ var r = resourceful.define();
+ r.use("couchdb", { uri: "http://test:123/db" });
+ f(r);
+ },
+ "uri + port": function(f) {
+ var r = resourceful.define();
+ r.use("couchdb", { uri: "http://test/db", port: 123 });
+ f(r);
+ },
+ "uri + port + database": function(f) {
+ var r = resourceful.define();
+ r.use("couchdb", {
+ uri: "http://test",
+ database: "db",
+ port: 123
+ });
+
+ f(r);
+ }
+ },
+ "or without": function() {
+ resourceful.use('couchdb');
+ assert.isFunction(resourceful.engine);
+ assert.equal(resourceful.connection.protocol, 'couchdb');
+
+ resourceful.use('memory');
+ assert.isFunction(resourceful.engine);
+ assert.equal(resourceful.connection.protocol, 'memory');
+ },
},
- "by reference": function () {
- resourceful.use(resourceful.engines.Memory);
- assert.isFunction(resourceful.engine);
- assert.equal(resourceful.connection.protocol, 'memory');
+ "by reference": {
+ "with options": {
+ topic: function() {
+ return function(r) {
+ assert.equal(r.connection.host, "test");
+ assert.equal(r.connection.port, 123);
+ assert.equal(r.name, "db");
+ }
+ },
+ "uri": function(f) {
+ var r = resourceful.define();
+ r.use(resourceful.engines.Couchdb, { uri: "http://test:123/db" });
+ f(r);
+ },
+ "uri + port": function(f) {
+ var r = resourceful.define();
+ r.use(resourceful.engines.Couchdb, { uri: "http://test/db", port: 123 });
+ f(r);
+ },
+ "uri + port + database": function(f) {
+ var r = resourceful.define();
+ r.use(resourceful.engines.Couchdb, {
+ uri: "http://test",
+ database: "db",
+ port: 123
+ });
+
+ f(r);
+ }
+ },
+ "or without": function () {
+ resourceful.use(resourceful.engines.Couchdb);
+ assert.isFunction(resourceful.engine);
+ assert.equal(resourceful.connection.protocol, 'couchdb');
+
+ resourceful.use(resourceful.engines.Memory);
+ assert.isFunction(resourceful.engine);
+ assert.equal(resourceful.connection.protocol, 'memory');
+ }
}
}
}).addVows({
Something went wrong with that request. Please try again.