Fixed bugs #1, #3, and undocumented bugs. Added getClient, and added …

…fromData() functionality to create()
scottwrobinson · Jun 26, 2015 · 0a5c7e3 · 0a5c7e3
1 parent dc7282e
commit 0a5c7e3
Show file tree

Hide file tree

Showing 15 changed files with 407 additions and 68 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,18 @@
+## 0.5.0 (2015-06-26)
+
+Features:
+ - Exposed `getClient()` method for retrieving the active Camo client.
+ - Added `options` parameter to `connect()` so options can be passed to backend DB client.
+ - Static method `Document.fromData()` is now a private helper method. Static method `.create()` should be used instead.
+
+Bugfixes:
+ - In `Document._fromData()`, added check to see if ID exists before assigning
+ - Changed `BaseDocument._fromData()` so it returns data in same form as it was passed.
+   + i.e. Array of data returned as array, single object returned as single object.
+ - Fixed bug where assigning an array of Documents in `.create()` lost the references.
+ - Stopped using the depracated `_.extend()` alias. Now using `_.assign()` instead. ([#1](https://github.com/scottwrobinson/camo/issues/1)).
+ - Fixed `get` and `set` issue with Proxy ([#3](https://github.com/scottwrobinson/camo/issues/3)).
+
 ## 0.4.0 (2015-06-22)
 
 Features:

diff --git a/README.md b/README.md
@@ -205,9 +205,10 @@ wallet.save().then(function() {
 To create a new instance of our document, we need to use the `.create()` method, which handles all of the construction for us.
 
 ```javascript
-var lassie = Dog.create();
-lassie.name = 'Lassie';
-lassie.breed = 'Collie';
+var lassie = Dog.create({
+    name: 'Lassie',
+    breed: 'Collie'
+});
 
 lassie.save().then(function(l) {
 	console.log(l.id);

diff --git a/index.js b/index.js
@@ -3,5 +3,7 @@
 require('./lib/proxyShim');
 
 exports.connect = require('./lib/db').connect;
+exports.getClient = require('./lib/clients').getClient;
+
 exports.Document = require('./lib/document');
 exports.EmbeddedDocument = require('./lib/embedded-document');
diff --git a/lib/base-document.js b/lib/base-document.js
@@ -30,7 +30,7 @@ var normalizeType = function(property) {
 // https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Proxy
 
 let schemaProxyHandler = {
-    get: function(target, propKey) {
+    get: function(target, propKey, receiver) {
         // Return current value, if set
         if (propKey in target._values) {
             return target._values[propKey];
@@ -41,10 +41,10 @@ let schemaProxyHandler = {
             return target._values._id;
         }
 
-        return Reflect.get(target, propKey);
+        return Reflect.get(target, propKey, receiver);
     },
 
-    set: function(target, propKey, value) {
+    set: function(target, propKey, value, receiver) {
         if (propKey in target._schema) {
             target._values[propKey] = value;
             return true;
@@ -56,7 +56,7 @@ let schemaProxyHandler = {
             return true;
         }
 
-        return Reflect.set(target, propKey, value);
+        return Reflect.set(target, propKey, value, receiver);
     },
 
     deleteProperty: function(target, propKey) {
@@ -72,10 +72,10 @@ let schemaProxyHandler = {
 
 class BaseDocument {
 	constructor() {
-        this._schema = {                // Defines document structure/properties
-            _id: { type: String },      // Native ID to backend database
+        this._schema = {                            // Defines document structure/properties
+            _id: { type: DB().nativeIdType() },     // Native ID to backend database
         };
-        this._values = {};              // Contains values for properties defined in schema
+        this._values = {};                          // Contains values for properties defined in schema
     }
 
     // TODO: Is there a way to tell if a class is
@@ -99,7 +99,7 @@ class BaseDocument {
 
     schema(extension) {
         if (!extension) return;
-        _.extend(this._schema, extension);
+        _.assign(this._schema, extension);
     }
 
     /*
@@ -155,7 +155,8 @@ class BaseDocument {
         _.keys(that._values).forEach(function(key) {
             var value = that._values[key];
 
-            if (value !== null && value.documentClass && value.documentClass() === 'embedded') {
+            if (value !== null && value !== undefined && 
+                value.documentClass && value.documentClass() === 'embedded') {
                 value.validate();
                 return;
             }
@@ -197,7 +198,14 @@ class BaseDocument {
         });
     }
 
-    static create() {
+    static create(data) {
+        if (typeof(data) !== 'undefined') {
+            return this._fromData(data);
+        }
+        return this._instantiate();
+    }
+
+    static _instantiate() {
         var instance = new this();
         instance.generateSchema();
         return new Proxy(instance, schemaProxyHandler);
@@ -206,21 +214,17 @@ class BaseDocument {
     // TODO: Should probably move some of this to 
     // Embedded and Document classes since Base shouldn't
     // need to know about child classes
-    static fromData(datas, populate) {
+    static _fromData(datas) {
         var that = this;
 
         if (!isArray(datas)) {
             datas = [datas];
         }
 
-        if (typeof(populate) === 'undefined') {
-            populate = true;
-        }
-
         var documents = [];
         var embeddedPromises = [];
         datas.forEach(function(d) {
-            var instance = that.create();
+            var instance = that._instantiate();
             _.keys(d).forEach(function(key) {
 
                 var value = null;
@@ -234,18 +238,8 @@ class BaseDocument {
                 if (key in instance._schema) {
                     var type = instance._schema[key].type;
 
-                    // TODO: We should dereference in 'fromData'. This way
-                    // we only need to check if its a document (not even a
-                    // Base/Embedded doc) and call 'fromData' on that.
                     if (type.documentClass && type.documentClass() === 'embedded') {
-                        // For each EmbeddedDocument, save a promise that
-                        // will later populate its data (done right before
-                        // we return from this function)
-                        var p = Promise.resolve(type.fromData(value, populate))
-                        .then(function(embedded) {
-                            instance._values[key] = embedded[0];
-                        });
-                        embeddedPromises.push(p);
+                        instance._values[key] = type._fromData(value);
                     } else {
                         instance._values[key] = value;
                     }
@@ -254,21 +248,20 @@ class BaseDocument {
 
             documents.push(instance);
         });
-
-        // Populate references or just return docs
-        return Promise.all(embeddedPromises).then(function() {
-            if (populate) {
-                return that.dereferenceDocuments(documents).then(function(docs) {
-                    return docs;
-                });
-            }
-            return Promise.all(documents);
-        });
+
+        if (documents.length === 1) {
+            return documents[0];
+        }
+        return documents;
+    }
+
+    populate() {
+        return BaseDocument.populate(this);
     }
 
     // TODO : EMBEDDED
     // 
-    static dereferenceDocuments(docs) {
+    static populate(docs) {
         if (!docs) return Promise.all([]);
 
         var documents = null;

diff --git a/lib/clients/client.js b/lib/clients/client.js
@@ -33,7 +33,7 @@ class DatabaseClient {
         throw new TypeError('You must override count.');
     }
 
-    static connect(url) {
+    static connect(url, options) {
 		throw new TypeError('You must override connect (static).');
 	}
 
@@ -57,6 +57,10 @@ class DatabaseClient {
         throw new TypeError('You must override isNativeId.');
     }
 
+    nativeIdType() {
+        throw new TypeError('You must override nativeIdType.');
+    }
+
     driver() {
         throw new TypeError('You must override driver.');
     }

diff --git a/lib/clients/mongoclient.js b/lib/clients/mongoclient.js
@@ -1,8 +1,8 @@
 "use strict";
 
+var _ = require('lodash');
 var path = require('path');
 var fs = require('fs');
-var _ = require('lodash');
 var MDBClient = require('mongodb').MongoClient;
 var ObjectId = require('mongodb').ObjectId;
 var DatabaseClient = require('./client');
@@ -108,9 +108,12 @@ class MongoClient extends DatabaseClient {
         });
     }
 
-    static connect(url) {
+    static connect(url, options) {
+        if (typeof(options) === 'undefined') {
+            options = { };
+        }
         return new Promise(function(resolve, reject) {
-            MDBClient.connect(url, function(error, client) {
+            MDBClient.connect(url, options, function(error, client) {
                 if (error) return reject(error);
                 return resolve(new MongoClient(url, client));
             });
@@ -155,6 +158,10 @@ class MongoClient extends DatabaseClient {
         return value instanceof ObjectId;
     }
 
+    nativeIdType() {
+        return ObjectId;
+    }
+
     driver() {
         return this._mongo;
     }

diff --git a/lib/clients/nedbclient.js b/lib/clients/nedbclient.js
@@ -1,8 +1,8 @@
 "use strict";
 
+var _ = require('lodash');
 var path = require('path');
 var fs = require('fs');
-var _ = require('lodash');
 var Datastore = require('nedb');
 var DatabaseClient = require('./client');
 
@@ -135,7 +135,7 @@ class NeDbClient extends DatabaseClient {
     	});
     }
 
-    static connect(url) {
+    static connect(url, options) {
 		var dbFolderPath = urlToPath(url);
 
 		return new Promise(function(resolve, reject) {
@@ -205,6 +205,10 @@ class NeDbClient extends DatabaseClient {
         return true;
     }
 
+    nativeIdType() {
+        return String;
+    }
+
     driver() {
         return this._collections;
     }

diff --git a/lib/db.js b/lib/db.js
@@ -1,16 +1,16 @@
 var NeDbClient = require('./clients/nedbclient');
 var MongoClient = require('./clients/mongoclient');
 
-exports.connect = function(url) {
+exports.connect = function(url, options) {
 	if (url.indexOf('nedb://') > -1) {
 		// url example: nedb://path/to/file/folder
-		return NeDbClient.connect(url).then(function(db) {
+		return NeDbClient.connect(url, options).then(function(db) {
 			global.CLIENT = db;
 			return db;
 		});
 	} else if(url.indexOf('mongodb://') > -1) {
 		// url example: 'mongodb://localhost:27017/myproject'
-		return MongoClient.connect(url).then(function(db) {
+		return MongoClient.connect(url, options).then(function(db) {
 			global.CLIENT = db;
 			return db;
 		});

diff --git a/lib/document.js b/lib/document.js
@@ -216,10 +216,15 @@ class Document extends BaseDocument {
                 return null;
             }
 
-            return that.fromData(data, populate);
+            var doc = that._fromData(data);
+            if (populate) {
+                return that.populate(doc);
+            }
+
+            return doc;
         }).then(function(docs) {
-            if (docs && docs.length > 0) {
-                return docs[0];
+            if (docs) {
+                return docs;
             }
             return null;
         });
@@ -236,9 +241,14 @@ class Document extends BaseDocument {
 
         return DB().loadMany(this.collectionName(), query)
         .then(function(datas) {
-            return that.fromData(datas, populate);
-        }).then(function(docs) {
+            var docs = that._fromData(datas);
+            if (populate) {
+                return that.populate(docs);
+            }
             return docs;
+        }).then(function(docs) {
+            // Ensure we always return an array
+            return [].concat(docs);
         });
     }
 
@@ -247,17 +257,23 @@ class Document extends BaseDocument {
         return DB().count(this.collectionName(), query);
     }
 
-    static fromData(datas, populate) {
-        if (!isArray(datas)) {
-            datas = [datas];
-        }
-        return super.fromData(datas, populate).then(function(instances) {
-            for (var i = 0; i < instances.length; i++) {
-                instances[i].id = datas[i]._id;
+    static _fromData(datas) {
+        var instances = super._fromData(datas);
+        // This way we preserve the original structure of the data. Data
+        // that was passed as an array is returned as an array, and data
+        // passes as a single object is returned as single object
+        var datasArray = [].concat(datas);
+        var instancesArray = [].concat(instances);
+
+        for (var i = 0; i < instancesArray.length; i++) {
+            if (datasArray[i].hasOwnProperty('_id')) {
+                instancesArray[i].id = datasArray[i]._id;
+            } else {
+                instancesArray[i].id = null;
             }
-            
-            return instances;
-        });
+        }
+
+        return instances;
     }
 
     static clearCollection() {