From 7b3f3c2868ab68922eb4f487f76570594714a00a Mon Sep 17 00:00:00 2001 From: Louis Chatriot Date: Sat, 7 Nov 2015 09:29:11 +0100 Subject: [PATCH] Fixed side effect bug --- lib/datastore.js | 29 +++++++++--------- test/db.test.js | 78 +++++++++++++++++++++++++++++++++++------------- 2 files changed, 73 insertions(+), 34 deletions(-) diff --git a/lib/datastore.js b/lib/datastore.js index 003e1f7..1ad0863 100644 --- a/lib/datastore.js +++ b/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 { diff --git a/test/db.test.js b/test/db.test.js index 9eba510..76567c9 100644 --- a/test/db.test.js +++ b/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) {