Merge lp:~thisfred/u1db/check-replica-trans-id into lp:u1db

[Eric Casteleijn]

Use the validate_source method in _put_doc_if_newer to make sure the other replica isn't deluded.

Use the validate_source method in _put_doc_if_newer to make sure the other replica isn't deluded.

[John A Meinel (jameinel)]

Don't you need to free the stored_vc? Or is that code already in place? Is the value getting parsed 2 times?

Things seem to be ok, with the caveat that I think the signature of validate should change a little.

review: Approve

[Eric Casteleijn (thisfred)]

stored_vc and new_vc were already freed: That code was moved around rather than added, because we needed the vector clocks at an earlier point.

[Eric Casteleijn (thisfred)]

merged changes from validate branch

[Samuele Pedroni (pedronis)]

review: Approve

[Ubuntu One Auto Pilot (otto-pilot)]

The prerequisite https://code.launchpad.net/~thisfred/u1db/validate_transaction_id_of_source has not yet been merged into https://code.launchpad.net/~ubuntuone-hackers/u1db/trunk.

=== modified file 'src/u1db.c'
--- src/u1db.c	2012-06-14 11:25:22 +0000
+++ src/u1db.c	2012-06-14 11:25:22 +0000
@@ -783,6 +783,31 @@
     status = lookup_doc(db, doc->doc_id, &stored_doc_rev, &stored_content,
                         &stored_content_len, &statement);
     if (status != SQLITE_OK) { goto finish; }
+    // TODO: u1db__vectorclock_from_str returns NULL if there is an error
+    //       in the vector clock, or if we run out of memory... Probably
+    //       shouldn't be U1DB_NOMEM
+    stored_vc = u1db__vectorclock_from_str(stored_doc_rev);
+    if (stored_vc == NULL) {
+        status = U1DB_NOMEM;
+        goto finish;
+    }
+    new_vc = u1db__vectorclock_from_str(doc->doc_rev);
+    if (new_vc == NULL) {
+        status = U1DB_NOMEM;
+        goto finish;
+    }
+    if (replica_uid != NULL && replica_trans_id != NULL) {
+        status = u1db__validate_source(
+            db, replica_uid, replica_gen, replica_trans_id, stored_vc, new_vc,
+            state);
+        if (status != U1DB_OK) {
+            goto finish;
+        }
+        if (*state != U1DB_OK) {
+            status = u1db__get_generation(db, at_gen);
+            goto finish;
+        }
+    }
     if (stored_doc_rev == NULL) {
         status = U1DB_OK;
         *state = U1DB_INSERTED;
@@ -792,19 +817,6 @@
         *state = U1DB_CONVERGED;
         store = 0;
     } else {
-        // TODO: u1db__vectorclock_from_str returns NULL if there is an error
-        //       in the vector clock, or if we run out of memory... Probably
-        //       shouldn't be U1DB_NOMEM
-        stored_vc = u1db__vectorclock_from_str(stored_doc_rev);
-        if (stored_vc == NULL) {
-            status = U1DB_NOMEM;
-            goto finish;
-        }
-        new_vc = u1db__vectorclock_from_str(doc->doc_rev);
-        if (new_vc == NULL) {
-            status = U1DB_NOMEM;
-            goto finish;
-        }
         if (u1db__vectorclock_is_newer(new_vc, stored_vc)) {
             // Just take the newer version
             store = 1;

=== modified file 'u1db/backends/__init__.py'
--- u1db/backends/__init__.py	2012-06-14 11:25:22 +0000
+++ u1db/backends/__init__.py	2012-06-14 11:25:22 +0000
@@ -121,6 +121,12 @@
             cur_vcr = VectorClockRev(None)
         else:
             cur_vcr = VectorClockRev(cur_doc.rev)
+        if replica_uid is not None and replica_gen is not None:
+            state = self._validate_source(
+                replica_uid, replica_gen, replica_trans_id, cur_vcr,
+                doc_vcr)
+            if state != 'ok':
+                return state, self._get_generation()
         if doc_vcr.is_newer(cur_vcr):
             self._put_and_update_indexes(cur_doc, doc)
             self._prune_conflicts(doc, doc_vcr)

=== modified file 'u1db/errors.py'
--- u1db/errors.py	2012-06-14 11:25:22 +0000
+++ u1db/errors.py	2012-06-14 11:25:22 +0000
@@ -44,7 +44,7 @@
 
 
 class InvalidGeneration(U1DBError):
-    """Generation was previously synced."""
+    """Generation was previously synced with a different transaction id."""
 
 
 class ConflictedDoc(U1DBError):

=== modified file 'u1db/tests/test_backends.py'
--- u1db/tests/test_backends.py	2012-06-14 11:25:22 +0000
+++ u1db/tests/test_backends.py	2012-06-14 11:25:22 +0000
@@ -392,6 +392,51 @@
         # The database wasn't altered
         self.assertGetDoc(self.db, doc1.doc_id, doc1.rev, simple_doc, False)
 
+    def test_put_doc_if_newer_newer_generation(self):
+        self.db._set_sync_info('other', 1, 'T-sid')
+        doc = self.make_document('doc_id', 'other:2', simple_doc)
+        state, _ = self.db._put_doc_if_newer(
+            doc, save_conflict=False, replica_uid='other', replica_gen=2,
+            replica_trans_id='T-irrelevant')
+        self.assertEqual('inserted', state)
+
+    def test_put_doc_if_newer_same_generation_same_txid(self):
+        self.db._set_sync_info('other', 1, 'T-sid')
+        doc = self.make_document('doc_id', 'other:2', simple_doc)
+        state, _ = self.db._put_doc_if_newer(
+            doc, save_conflict=False, replica_uid='other', replica_gen=1,
+            replica_trans_id='T-sid')
+        self.assertEqual('superseded', state)
+
+    def test_put_doc_if_newer_wrong_transaction_id(self):
+        self.db._set_sync_info('other', 1, 'T-sid')
+        doc = self.make_document('doc_id', 'other:1', simple_doc)
+        self.assertRaises(
+            errors.InvalidTransactionId,
+            self.db._put_doc_if_newer, doc, save_conflict=False,
+            replica_uid='other', replica_gen=1, replica_trans_id='T-sad')
+
+    def test_put_doc_if_newer_old_generation_older_doc(self):
+        orig_doc = '{"new": "doc"}'
+        doc = self.db.create_doc(orig_doc)
+        doc_rev1 = doc.rev
+        doc.set_json(simple_doc)
+        self.db.put_doc(doc)
+        self.db._set_sync_info('other', 5, 'T-sid')
+        older_doc = self.make_document(doc.doc_id, doc_rev1, simple_doc)
+        state, _ = self.db._put_doc_if_newer(
+            older_doc, save_conflict=False, replica_uid='other', replica_gen=3,
+            replica_trans_id='T-irrelevant')
+        self.assertEqual('superseded', state)
+
+    def test_put_doc_if_newer_old_generation_newer_doc(self):
+        self.db._set_sync_info('other', 5, 'T-sid')
+        doc = self.make_document('doc_id', 'other:1', simple_doc)
+        self.assertRaises(
+            errors.InvalidGeneration,
+            self.db._put_doc_if_newer, doc, save_conflict=False,
+            replica_uid='other', replica_gen=1, replica_trans_id='T-sad')
+
     def test_validate_source_gen_and_trans_id_same(self):
         self.db._set_sync_info('other', 1, 'T-sid')
         v1 = vectorclock.VectorClockRev('other:1|self:1')
@@ -678,7 +723,8 @@
         doc2 = self.make_document(doc1.doc_id, 'alternate:1', nested_doc)
         self.db._put_doc_if_newer(doc2, save_conflict=True)
         self.assertTrue(doc2.has_conflicts)
-        self.assertGetDoc(self.db, doc1.doc_id, 'alternate:1', nested_doc, True)
+        self.assertGetDoc(
+            self.db, doc1.doc_id, 'alternate:1', nested_doc, True)
         self.assertGetDocConflicts(self.db, doc1.doc_id,
             [('alternate:1', nested_doc), (doc1.rev, None)])