Fixed side effect bug

lib/datastore.js

@@ -34,6 +34,7 @@ function Datastore (options) {
     filename = options.filename;
     this.inMemoryOnly = options.inMemoryOnly || false;
     this.autoload = options.autoload || false;
+    this.timestampData = options.timestampData || false;
   }
 
   // Determine whether in memory or persistent
@@ -61,7 +62,7 @@ function Datastore (options) {
   // binary is always well-balanced
   this.indexes = {};
   this.indexes._id = new Index({ fieldName: '_id', unique: true });
-  
+
   // Queue a load of the database right away and call the onload handler
   // By default (no onload handler), if there is an error there, no operation will be possible so warn the user by throwing an exception
   if (this.autoload) { this.loadDatabase(options.onload || function (err) {
@@ -283,17 +284,19 @@ Datastore.prototype.getCandidates = function (query) {
  */
 Datastore.prototype._insert = function (newDoc, cb) {
   var callback = cb || function () {}
+    , preparedDoc
     ;
 
   try {
-    this._insertInCache(newDoc);
+    preparedDoc = this.prepareDocumentForInsertion(newDoc)
+    this._insertInCache(preparedDoc);
   } catch (e) {
     return callback(e);
   }
 
-  this.persistence.persistNewState(util.isArray(newDoc) ? newDoc : [newDoc], function (err) {
+  this.persistence.persistNewState(util.isArray(preparedDoc) ? preparedDoc : [preparedDoc], function (err) {
     if (err) { return callback(err); }
-    return callback(null, newDoc);
+    return callback(null, model.deepCopy(preparedDoc));
   });
 };
 
@@ -311,6 +314,7 @@ Datastore.prototype.createNewId = function () {
 
 /**
  * Prepare a document (or array of documents) to be inserted in a database
+ * Meaning adds _id and createdAt if necessary on a copy of newDoc to avoid any side effect on user input
  * @api private
  */
 Datastore.prototype.prepareDocumentForInsertion = function (newDoc) {
@@ -320,9 +324,8 @@ Datastore.prototype.prepareDocumentForInsertion = function (newDoc) {
     preparedDoc = [];
     newDoc.forEach(function (doc) { preparedDoc.push(self.prepareDocumentForInsertion(doc)); });
   } else {
-    if (newDoc._id === undefined) { newDoc._id = this.createNewId(); }
     preparedDoc = model.deepCopy(newDoc);
-    //if (preparedDoc._id === undefined) { preparedDoc._id = this.createNewId(); }
+    if (preparedDoc._id === undefined) { preparedDoc._id = this.createNewId(); }
     if (this.timestampData && preparedDoc.createdAt === undefined) { preparedDoc.createdAt = new Date(); }
     model.checkObject(preparedDoc);
   }
@@ -334,11 +337,11 @@ Datastore.prototype.prepareDocumentForInsertion = function (newDoc) {
  * If newDoc is an array of documents, this will insert all documents in the cache
  * @api private
  */
-Datastore.prototype._insertInCache = function (newDoc) {
-  if (util.isArray(newDoc)) {
-    this._insertMultipleDocsInCache(newDoc);
+Datastore.prototype._insertInCache = function (preparedDoc) {
+  if (util.isArray(preparedDoc)) {
+    this._insertMultipleDocsInCache(preparedDoc);
   } else {
-    this.addToIndexes(this.prepareDocumentForInsertion(newDoc));
+    this.addToIndexes(preparedDoc);
   }
 };
 
@@ -347,10 +350,8 @@ Datastore.prototype._insertInCache = function (newDoc) {
  * inserts and throws the error
  * @api private
  */
-Datastore.prototype._insertMultipleDocsInCache = function (newDocs) {
-  var i, failingI, error
-    , preparedDocs = this.prepareDocumentForInsertion(newDocs)
-    ;
+Datastore.prototype._insertMultipleDocsInCache = function (preparedDocs) {
+  var i, failingI, error;
 
   for (i = 0; i < preparedDocs.length; i += 1) {
     try {

test/db.test.js

@@ -54,44 +54,44 @@ describe('Database', function () {
   });
 
   describe('Autoloading', function () {
-  
+
     it('Can autoload a database and query it right away', function (done) {
       var fileStr = model.serialize({ _id: '1', a: 5, planet: 'Earth' }) + '\n' + model.serialize({ _id: '2', a: 5, planet: 'Mars' }) + '\n'
         , autoDb = 'workspace/auto.db'
         , db
         ;
-      
+
       fs.writeFileSync(autoDb, fileStr, 'utf8');
       db = new Datastore({ filename: autoDb, autoload: true })
-      
+
       db.find({}, function (err, docs) {
         assert.isNull(err);
         docs.length.should.equal(2);
         done();
       });
     });
-    
+
     it('Throws if autoload fails', function (done) {
       var fileStr = model.serialize({ _id: '1', a: 5, planet: 'Earth' }) + '\n' + model.serialize({ _id: '2', a: 5, planet: 'Mars' }) + '\n' + '{"$$indexCreated":{"fieldName":"a","unique":true}}'
         , autoDb = 'workspace/auto.db'
         , db
         ;
-      
+
       fs.writeFileSync(autoDb, fileStr, 'utf8');
-      
+
       // Check the loadDatabase generated an error
       function onload (err) {
         err.errorType.should.equal('uniqueViolated');
         done();
       }
-      
+
       db = new Datastore({ filename: autoDb, autoload: true, onload: onload })
-      
+
       db.find({}, function (err, docs) {
         done("Find should not be executed since autoload failed");
-      });    
+      });
     });
- 
+
   });
 
   describe('Insert', function () {
@@ -207,7 +207,7 @@ describe('Database', function () {
 
         d.insert({ _id: 'test', otherstuff: 42 }, function (err) {
           err.errorType.should.equal('uniqueViolated');
-        
+
           done();
         });
       });
@@ -223,18 +223,18 @@ describe('Database', function () {
         });
       });
     });
-    
+
     it('Can insert an array of documents at once', function (done) {
       var docs = [{ a: 5, b: 'hello' }, { a: 42, b: 'world' }];
-    
+
       d.insert(docs, function (err) {
         d.find({}, function (err, docs) {
           var data;
-        
+
           docs.length.should.equal(2);
           _.find(docs, function (doc) { return doc.a === 5; }).b.should.equal('hello');
           _.find(docs, function (doc) { return doc.a === 42; }).b.should.equal('world');
-          
+
           // The data has been persisted correctly
           data = _.filter(fs.readFileSync(testDb, 'utf8').split('\n'), function (line) { return line.length > 0; });
           data.length.should.equal(2);
@@ -242,19 +242,19 @@ describe('Database', function () {
           model.deserialize(data[0]).b.should.equal('hello');
           model.deserialize(data[1]).a.should.equal(42);
           model.deserialize(data[1]).b.should.equal('world');
-                    
+
           done();
         });
       });
     });
-    
+
     it('If a bulk insert violates a constraint, all changes are rolled back', function (done) {
       var docs = [{ a: 5, b: 'hello' }, { a: 42, b: 'world' }, { a: 5, b: 'bloup' }, { a: 7 }];
-    
+
       d.ensureIndex({ fieldName: 'a', unique: true }, function () {   // Important to specify callback here to make sure filesystem synced
         d.insert(docs, function (err) {
           err.errorType.should.equal('uniqueViolated');
-        
+
           d.find({}, function (err, docs) {
             // Datafile only contains index definition
             var datafileContents = model.deserialize(fs.readFileSync(testDb, 'utf8'));
@@ -265,10 +265,48 @@ describe('Database', function () {
             done();
           });
         });
+      });
+    });
+
+    it("If timestampData option is set, a createdAt field is added and persisted", function (done) {
+      var newDoc = { hello: 'world' }, beginning = Date.now();
+      d = new Datastore({ filename: testDb, timestampData: true, autoload: true });
+      d.find({}, function (err, docs) {
+        assert.isNull(err);
+        docs.length.should.equal(0);
 
+        d.insert(newDoc, function (err, insertedDoc) {
+          // No side effect on given input
+          assert.deepEqual(newDoc, { hello: 'world' });
+          // Insert doc has two new fields, _id and createdAt
+          insertedDoc.hello.should.equal('world');
+          assert.isDefined(insertedDoc.createdAt);
+          assert.isDefined(insertedDoc._id);
+          Object.keys(insertedDoc).length.should.equal(3);
+          assert.isBelow(Math.abs(insertedDoc.createdAt.getTime() - beginning), 15);   // No more than 15ms should have elapsed
 
+          // Modifying results of insert doesn't change the cache
+          insertedDoc.bloup = "another";
+          Object.keys(insertedDoc).length.should.equal(4);
+
+          d.find({}, function (err, docs) {
+            docs.length.should.equal(1);
+            assert.deepEqual(newDoc, { hello: 'world' });
+            assert.deepEqual({ hello: 'world', _id: insertedDoc._id, createdAt: insertedDoc.createdAt }, docs[0]);
+
+            // All data correctly persisted on disk
+            d.loadDatabase(function () {
+              d.find({}, function (err, docs) {
+                docs.length.should.equal(1);
+                assert.deepEqual(newDoc, { hello: 'world' });
+                assert.deepEqual({ hello: 'world', _id: insertedDoc._id, createdAt: insertedDoc.createdAt }, docs[0]);
+
+                done();
+              });
+            });
+          });
+        });
       });
-      
     });
 
     it('Can insert a doc with id 0', function (done) {