If a simple update violates a constraint, all changes are rolled back and an error thrown

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)

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' ==== //

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' }