diff --git a/lib/indexes.js b/lib/indexes.js index 565bf7a..7bbff91 100644 --- a/lib/indexes.js +++ b/lib/indexes.js @@ -121,29 +121,19 @@ Index.prototype.remove = function (doc) { /** * Update a document in the index + * If a constraint is violated, changes are rolled back and an error thrown * Naive implementation, still in O(log(n)) */ Index.prototype.update = function (oldDoc, newDoc) { if (util.isArray(oldDoc)) { this.updateMultipleDocs(oldDoc); } this.remove(oldDoc); - this.insert(newDoc); -}; - - -/** - * Revert an update - */ -Index.prototype.revertUpdate = function (oldDoc, newDoc) { - var revert = []; - if (!util.isArray(oldDoc)) { - this.update(newDoc, oldDoc); - } else { - oldDoc.forEach(function (pair) { - revert.push({ oldDoc: pair.newDoc, newDoc: pair.oldDoc }); - }); - this.update(revert); + try { + this.insert(newDoc); + } catch (e) { + this.insert(oldDoc); + throw e; } }; @@ -186,6 +176,23 @@ Index.prototype.updateMultipleDocs = function (pairs) { }; +/** + * Revert an update + */ +Index.prototype.revertUpdate = function (oldDoc, newDoc) { + var revert = []; + + if (!util.isArray(oldDoc)) { + this.update(newDoc, oldDoc); + } else { + oldDoc.forEach(function (pair) { + revert.push({ oldDoc: pair.newDoc, newDoc: pair.oldDoc }); + }); + this.update(revert); + } +}; + + /** * Get all documents in index that match the query on fieldName * For now only works with field equality (i.e. can't use the index for $lt query for example) diff --git a/test/db.test.js b/test/db.test.js index 34fac25..47bd009 100644 --- a/test/db.test.js +++ b/test/db.test.js @@ -1491,7 +1491,7 @@ describe('Database', function () { }); // ==== End of 'Indexing newly inserted documents' ==== // - describe.only('Updating indexes upon document update', function () { + describe('Updating indexes upon document update', function () { it('Updating docs still works as before with indexing', function (done) { d.ensureIndex({ fieldName: 'a' }); @@ -1589,7 +1589,51 @@ describe('Database', function () { }); }); - it.skip('If an update violates a contraint, all changes are rolled back and an error is thrown', function (done) { + it('If a simple update violates a contraint, all changes are rolled back and an error is thrown', function (done) { + d.ensureIndex({ fieldName: 'a', unique: true }); + d.ensureIndex({ fieldName: 'b', unique: true }); + d.ensureIndex({ fieldName: 'c', unique: true }); + + d.insert({ a: 1, b: 10, c: 100 }, function (err, _doc1) { + d.insert({ a: 2, b: 20, c: 200 }, function (err, _doc2) { + d.insert({ a: 3, b: 30, c: 300 }, function (err, _doc3) { + // Will conflict with doc3 + d.update({ a: 2 }, { $inc: { a: 10, c: 1000 }, $set: { b: 30 } }, {}, function (err) { + var data = d.getAllData() + , doc1 = _.find(data, function (doc) { return doc._id === _doc1._id; }) + , doc2 = _.find(data, function (doc) { return doc._id === _doc2._id; }) + , doc3 = _.find(data, function (doc) { return doc._id === _doc3._id; }) + ; + + err.errorType.should.equal('uniqueViolated'); + + // Data left unchanged + data.length.should.equal(3); + assert.deepEqual(doc1, { a: 1, b: 10, c: 100, _id: _doc1._id }); + assert.deepEqual(doc2, { a: 2, b: 20, c: 200, _id: _doc2._id }); + assert.deepEqual(doc3, { a: 3, b: 30, c: 300, _id: _doc3._id }); + + // All indexes left unchanged and pointing to the same docs + d.indexes.a.tree.getNumberOfKeys().should.equal(3); + d.indexes.a.getMatching(1)[0].should.equal(doc1); + d.indexes.a.getMatching(2)[0].should.equal(doc2); + d.indexes.a.getMatching(3)[0].should.equal(doc3); + + d.indexes.b.tree.getNumberOfKeys().should.equal(3); + d.indexes.b.getMatching(10)[0].should.equal(doc1); + d.indexes.b.getMatching(20)[0].should.equal(doc2); + d.indexes.b.getMatching(30)[0].should.equal(doc3); + + d.indexes.c.tree.getNumberOfKeys().should.equal(3); + d.indexes.c.getMatching(100)[0].should.equal(doc1); + d.indexes.c.getMatching(200)[0].should.equal(doc2); + d.indexes.c.getMatching(300)[0].should.equal(doc3); + + done(); + }); + }); + }); + }); }); }); // ==== End of 'Updating indexes upon document update' ==== // diff --git a/test/indexes.test.js b/test/indexes.test.js index f7ee8cf..68be062 100644 --- a/test/indexes.test.js +++ b/test/indexes.test.js @@ -239,6 +239,36 @@ describe('Indexes', function () { assert.deepEqual(idx.tree.search('changed'), [doc5]); }); + it('If a simple update violates a unique constraint, changes are rolled back and an error thrown', function () { + var idx = new Index({ fieldName: 'tf', unique: true }) + , doc1 = { a: 5, tf: 'hello' } + , doc2 = { a: 8, tf: 'world' } + , doc3 = { a: 2, tf: 'bloup' } + , bad = { a: 23, tf: 'world' } + ; + + idx.insert(doc1); + idx.insert(doc2); + idx.insert(doc3); + + idx.tree.getNumberOfKeys().should.equal(3); + assert.deepEqual(idx.tree.search('hello'), [doc1]); + assert.deepEqual(idx.tree.search('world'), [doc2]); + assert.deepEqual(idx.tree.search('bloup'), [doc3]); + + try { + idx.update(doc3, bad); + } catch (e) { + e.errorType.should.equal('uniqueViolated'); + } + + // No change + idx.tree.getNumberOfKeys().should.equal(3); + assert.deepEqual(idx.tree.search('hello'), [doc1]); + assert.deepEqual(idx.tree.search('world'), [doc2]); + assert.deepEqual(idx.tree.search('bloup'), [doc3]); + }); + it('Can update an array of documents', function () { var idx = new Index({ fieldName: 'tf' }) , doc1 = { a: 5, tf: 'hello' }