Merge lp:~thisfred/u1db/validate_transaction_id_of_target into lp:u1db

[Eric Casteleijn]

Added a validation function that checks that the generation + txid the sync target knows about us is correct.

Added a validation function that checks that the generation + txid the sync target knows about us is correct.

[lp:~thisfred/u1db/validate_transaction_id_of_target]

338. By Eric Casteleijn on 2012-06-19

merged trunk resolved conflicts

339. By Eric Casteleijn on 2012-06-19

merged txid branch

[Samuele Pedroni (pedronis)]

looks good, as we discussed we need integration tests showing that the validate cases trigger in appropriate sync situations

review: Approve

=== modified file 'include/u1db/u1db_internal.h'
--- include/u1db/u1db_internal.h	2012-06-19 19:41:19 +0000
+++ include/u1db/u1db_internal.h	2012-06-19 19:41:19 +0000
@@ -233,6 +233,17 @@
 int u1db__get_generation(u1database *db, int *generation);
 
 /**
+ * Internal API, Get the global database rev and transaction id.
+ */
+int u1db__get_generation_info(u1database *db, int *generation,
+                              char **trans_id);
+
+/**
+ * Internal API, Validate generation and transaction id.
+ */
+int u1db_validate_gen_and_trans_id(u1database *db, int generation,
+                                   const char *trans_id);
+/**
  * Internal API, Allocate a new document id, for cases when callers do not
  * supply their own. Callers of this API are expected to free the result.
  */

=== modified file 'src/u1db.c'
--- src/u1db.c	2012-06-19 19:16:35 +0000
+++ src/u1db.c	2012-06-19 19:41:19 +0000
@@ -900,8 +900,8 @@
                            (stored_doc_rev != NULL));
     }
     if (status == U1DB_OK && replica_uid != NULL) {
-        status = u1db__set_sync_info(db, replica_uid, replica_gen,
-                                     replica_trans_id);
+        status = u1db__set_sync_info(
+            db, replica_uid, replica_gen, replica_trans_id);
     }
     if (status == U1DB_OK && at_gen != NULL) {
         status = u1db__get_generation(db, at_gen);
@@ -1264,14 +1264,26 @@
         status = U1DB_OK;
         *gen = 0;
         *trans_id = strdup("");
+        if (*trans_id == NULL) {
+            status = U1DB_NOMEM;
+            goto finish;
+        }
     } else if (status == SQLITE_ROW) {
         status = U1DB_OK;
         *gen = sqlite3_column_int(statement, 0);
         tmp = (const char *)sqlite3_column_text(statement, 1);
         if (tmp == NULL) {
             *trans_id = strdup("");
+            if (*trans_id == NULL) {
+                status = U1DB_NOMEM;
+                goto finish;
+            }
         } else {
             *trans_id = strdup(tmp);
+            if (*trans_id == NULL) {
+                status = U1DB_NOMEM;
+                goto finish;
+            }
         }
     }
 finish:
@@ -1411,6 +1423,80 @@
     return status;
 }
 
+int
+u1db__get_generation_info(u1database *db, int *generation, char **trans_id)
+{
+    int status;
+    const char *tmp;
+
+    sqlite3_stmt *statement;
+    if (db == NULL || generation == NULL) {
+        return U1DB_INVALID_PARAMETER;
+    }
+    status = sqlite3_prepare_v2(db->sql_handle,
+        "SELECT max(generation), transaction_id FROM transaction_log", -1,
+        &statement, NULL);
+    if (status != SQLITE_OK) {
+        return status;
+    }
+    status = sqlite3_step(statement);
+    if (status == SQLITE_DONE) {
+        // No records, we are at rev 0
+        status = SQLITE_OK;
+        *generation = 0;
+    } else if (status == SQLITE_ROW) {
+        status = SQLITE_OK;
+        *generation = sqlite3_column_int(statement, 0);
+        tmp = (const char *)sqlite3_column_text(statement, 1);
+        if (tmp == NULL) {
+            *trans_id = NULL;
+        } else {
+            *trans_id = strdup(tmp);
+            if (*trans_id == NULL) {
+                status = U1DB_NOMEM;
+            }
+        }
+    }
+    sqlite3_finalize(statement);
+    return status;
+}
+
+int
+u1db_validate_gen_and_trans_id(u1database *db, int generation,
+                               const char *trans_id)
+{
+    int status = U1DB_OK;
+    sqlite3_stmt *statement;
+
+    if (generation == 0)
+        return status;
+    if (db == NULL) {
+        return U1DB_INVALID_PARAMETER;
+    }
+    status = sqlite3_prepare_v2(db->sql_handle,
+        "SELECT transaction_id FROM transaction_log WHERE generation = ?", -1,
+        &statement, NULL);
+    if (status != SQLITE_OK) { goto finish; }
+    status = sqlite3_bind_int(statement, 1, generation);
+    if (status != SQLITE_OK) { goto finish; }
+    status = sqlite3_step(statement);
+    if (status == SQLITE_DONE) {
+        status = U1DB_INVALID_GENERATION;
+        goto finish;
+    } else if (status == SQLITE_ROW) {
+        // Note: We may want to handle the column containing NULL
+        if (strcmp(trans_id,
+                   (const char *)sqlite3_column_text(statement, 0)) == 0) {
+            status = U1DB_OK;
+            goto finish;
+        }
+        status = U1DB_INVALID_TRANSACTION_ID;
+    }
+finish:
+    sqlite3_finalize(statement);
+    return status;
+}
+
 char *
 u1db__allocate_doc_id(u1database *db)
 {
@@ -1544,7 +1630,10 @@
         // Note: We may want to handle the column containing NULL
         tmp = (const char *)sqlite3_column_text(statement, 1);
         if (tmp == NULL) {
-            *trans_id = NULL;
+            *trans_id = strdup("");
+            if (*trans_id == NULL) {
+                status = U1DB_NOMEM;
+            }
         } else {
             *trans_id = strdup(tmp);
             if (*trans_id == NULL) {

=== modified file 'src/u1db_sync_target.c'
--- src/u1db_sync_target.c	2012-06-19 19:41:19 +0000
+++ src/u1db_sync_target.c	2012-06-19 19:41:19 +0000
@@ -24,7 +24,7 @@
 static int st_get_sync_info(u1db_sync_target *st,
         const char *source_replica_uid,
         const char **st_replica_uid, int *st_gen, int *source_gen,
-        char **trans_id);
+        char **source_trans_id);
 
 static int st_record_sync_info(u1db_sync_target *st,
         const char *source_replica_uid, int source_gen, const char *trans_id);
@@ -106,7 +106,7 @@
 static int
 st_get_sync_info(u1db_sync_target *st, const char *source_replica_uid,
         const char **st_replica_uid, int *st_gen, int *source_gen,
-        char **trans_id)
+        char **source_trans_id)
 {
     int status = U1DB_OK;
     u1database *db;
@@ -125,8 +125,8 @@
     db = (u1database *)st->implementation;
     status = u1db_get_replica_uid(db, st_replica_uid);
     if (status != U1DB_OK) { goto finish; }
-    status = u1db__get_sync_gen_info(db, source_replica_uid, source_gen,
-                                     trans_id);
+    status = u1db__get_sync_gen_info(
+        db, source_replica_uid, source_gen, source_trans_id);
     if (status != U1DB_OK) { goto finish; }
     status = u1db__get_generation(db, st_gen);
 finish:
@@ -614,6 +614,9 @@
     status = target->get_sync_info(target, local_uid, &target_uid, &target_gen,
                    &local_gen_known_by_target, &local_trans_id_known_by_target);
     if (status != U1DB_OK) { goto finish; }
+    status = u1db_validate_gen_and_trans_id(
+            db, local_gen_known_by_target, local_trans_id_known_by_target);
+    if (status != U1DB_OK) { goto finish; }
     status = u1db__get_sync_gen_info(db, target_uid,
         &target_gen_known_by_local, &target_trans_id_known_by_local);
     if (status != U1DB_OK) { goto finish; }
@@ -623,8 +626,9 @@
     // Before we start the sync exchange, get the list of doc_ids that we want
     // to send. We have to do this first, so that local_gen_before_sync will
     // match exactly the list of doc_ids we send
-    status = u1db_whats_changed(db, &local_gen, &local_trans_id,
-                            (void*)&to_send_state, whats_changed_to_doc_ids);
+    status = u1db_whats_changed(
+        db, &local_gen, &local_trans_id, (void*)&to_send_state,
+        whats_changed_to_doc_ids);
     if (status != U1DB_OK) { goto finish; }
     if (local_gen == local_gen_known_by_target
         && target_gen == target_gen_known_by_local)
@@ -644,19 +648,23 @@
         &target_gen_known_by_local, &target_trans_id_known_by_local,
         &return_doc_state, return_doc_to_insert_from_target);
     if (status != U1DB_OK) { goto finish; }
-    status = u1db__get_generation(db, &local_gen);
+    if (local_trans_id != NULL) {
+        free(local_trans_id);
+    }
+    status = u1db__get_generation_info(db, &local_gen, &local_trans_id);
     if (status != U1DB_OK) { goto finish; }
     // Now we successfully sent and received docs, make sure we record the
     // current remote generation
-    status = u1db__set_sync_info(db, target_uid, target_gen_known_by_local,
-                                 "T-sid");
+    status = u1db__set_sync_info(
+        db, target_uid, target_gen_known_by_local,
+        target_trans_id_known_by_local);
     if (status != U1DB_OK) { goto finish; }
     if (return_doc_state.num_inserted > 0 &&
-            ((*local_gen_before_sync + return_doc_state.num_inserted)
-              == local_gen))
+        ((*local_gen_before_sync + return_doc_state.num_inserted)
+         == local_gen))
     {
-        status = target->record_sync_info(target, local_uid, local_gen,
-                                          "T-sid");
+        status = target->record_sync_info(
+            target, local_uid, local_gen, local_trans_id);
         if (status != U1DB_OK) { goto finish; }
     }
 finish:

=== modified file 'u1db/backends/__init__.py'
--- u1db/backends/__init__.py	2012-06-19 19:16:35 +0000
+++ u1db/backends/__init__.py	2012-06-19 19:41:19 +0000
@@ -22,7 +22,6 @@
 import u1db
 from u1db import (
     errors,
-    vectorclock,
 )
 import u1db.sync
 from u1db.vectorclock import VectorClockRev
@@ -54,8 +53,17 @@
             raise errors.InvalidDocId()
 
     def _get_generation(self):
+        """Return the current generation.
+
+        """
         raise NotImplementedError(self._get_generation)
 
+    def _get_generation_info(self):
+        """Return the current generation and transaction id.
+
+        """
+        raise NotImplementedError(self._get_generation_info)
+
     def _get_doc(self, doc_id):
         """Extract the document from storage.
 
@@ -94,6 +102,15 @@
             result.append(doc)
         return result
 
+    def validate_gen_and_trans_id(self, generation, trans_id):
+        """Validate the generation and transaction id.
+
+        Raises an InvalidGeneration when the generation does not exist, and an
+        InvalidTransactionId when it does but with a different transaction id.
+
+        """
+        raise NotImplementedError(self.validate_gen_and_trans_id)
+
     def _validate_source(self, other_replica_uid, other_generation,
                          other_transaction_id, cur_vcr, other_vcr):
         """Validate the new generation and transaction id.

=== modified file 'u1db/backends/inmemory.py'
--- u1db/backends/inmemory.py	2012-06-18 12:51:14 +0000
+++ u1db/backends/inmemory.py	2012-06-19 19:41:19 +0000
@@ -79,6 +79,18 @@
     def _get_generation(self):
         return len(self._transaction_log)
 
+    def _get_generation_info(self):
+        return len(self._transaction_log), self._transaction_log[-1][1]
+
+    def validate_gen_and_trans_id(self, generation, trans_id):
+        if generation == 0:
+            return
+        if generation > len(self._transaction_log):
+            raise errors.InvalidGeneration
+        if self._transaction_log[generation - 1][1] == trans_id:
+            return
+        raise errors.InvalidTransactionId
+
     def put_doc(self, doc):
         if doc.doc_id is None:
             raise errors.InvalidDocId()

=== modified file 'u1db/backends/sqlite_backend.py'
--- u1db/backends/sqlite_backend.py	2012-06-18 12:51:14 +0000
+++ u1db/backends/sqlite_backend.py	2012-06-19 19:41:19 +0000
@@ -265,6 +265,28 @@
             return 0
         return val
 
+    def _get_generation_info(self):
+        c = self._db_handle.cursor()
+        c.execute(
+            'SELECT max(generation), transaction_id FROM transaction_log ')
+        val = c.fetchone()
+        if val[0] is None:
+            return(0, val[1])
+        return val
+
+    def validate_gen_and_trans_id(self, generation, trans_id):
+        if generation == 0:
+            return
+        c = self._db_handle.cursor()
+        c.execute(
+            'SELECT transaction_id FROM transaction_log WHERE generation = ?',
+            (generation,))
+        val = c.fetchone()
+        if val is None:
+            raise errors.InvalidGeneration
+        if val[0] != trans_id:
+            raise errors.InvalidTransactionId
+
     def _get_transaction_log(self):
         c = self._db_handle.cursor()
         c.execute("SELECT doc_id, transaction_id FROM transaction_log"

=== modified file 'u1db/sync.py'
--- u1db/sync.py	2012-06-19 19:41:19 +0000
+++ u1db/sync.py	2012-06-19 19:41:19 +0000
@@ -83,11 +83,11 @@
         record with the target that they are fully up to date with our
         new generation.
         """
-        cur_gen = self.source._get_generation()
+        cur_gen, trans_id = self.source._get_generation_info()
         if (cur_gen == start_generation + self.num_inserted
-            and self.num_inserted > 0):
-            self.sync_target.record_sync_info(self.source._replica_uid,
-                                              cur_gen, 'T-sid')
+                and self.num_inserted > 0):
+            self.sync_target.record_sync_info(
+                self.source._replica_uid, cur_gen, trans_id)
 
     def sync(self, callback=None):
         """Synchronize documents between source and target."""
@@ -98,6 +98,8 @@
          target_my_trans_id) = sync_target.get_sync_info(
              self.source._replica_uid)
         # what's changed since that generation and this current gen
+        self.source.validate_gen_and_trans_id(
+            target_my_gen, target_my_trans_id)
         my_gen, _, changes = self.source.whats_changed(target_my_gen)
 
         # this source last-seen database generation for the target
@@ -121,7 +123,8 @@
                         self.source._replica_uid, target_last_known_gen,
                         return_doc_cb=self._insert_doc_from_target)
         # record target synced-up-to generation including applying what we sent
-        self.source._set_sync_info(self.target_replica_uid, new_gen, 'T-sid')
+        self.source._set_sync_info(
+            self.target_replica_uid, new_gen, new_trans_id)
 
         # if gapless record current reached generation with target
         self._record_sync_info_with_the_target(my_gen)

=== modified file 'u1db/tests/c_backend_wrapper.pyx'
--- u1db/tests/c_backend_wrapper.pyx	2012-06-19 19:41:19 +0000
+++ u1db/tests/c_backend_wrapper.pyx	2012-06-19 19:41:19 +0000
@@ -204,6 +204,9 @@
 
 
     int u1db__get_generation(u1database *, int *db_rev)
+    int u1db__get_generation_info(u1database *, int *db_rev, char **trans_id)
+    int u1db_validate_gen_and_trans_id(u1database *, int db_rev,
+                                       const_char_ptr trans_id)
     char *u1db__allocate_doc_id(u1database *)
     int u1db__sql_close(u1database *)
     int u1db__sql_is_open(u1database *)
@@ -1064,6 +1067,22 @@
             u1db__get_generation(self._db, &generation))
         return generation
 
+    def _get_generation_info(self):
+        cdef int generation
+        cdef char *trans_id
+        handle_status("get_generation_info",
+            u1db__get_generation_info(self._db, &generation, &trans_id))
+        raw_trans_id = None
+        if trans_id != NULL:
+            raw_trans_id = trans_id
+            free(trans_id)
+        return generation, raw_trans_id
+
+    def validate_gen_and_trans_id(self, generation, trans_id):
+        handle_status(
+            "validate_gen_and_trans_id",
+            u1db_validate_gen_and_trans_id(self._db, generation, trans_id))
+
     def _get_sync_gen_info(self, replica_uid):
         cdef int generation, status
         cdef char *trans_id = NULL

=== modified file 'u1db/tests/test_backends.py'
--- u1db/tests/test_backends.py	2012-06-19 19:41:19 +0000
+++ u1db/tests/test_backends.py	2012-06-19 19:41:19 +0000
@@ -437,6 +437,25 @@
             self.db._put_doc_if_newer, doc, save_conflict=False,
             replica_uid='other', replica_gen=1, replica_trans_id='T-sad')
 
+    def test_validate_gen_and_trans_id(self):
+        self.db.create_doc(simple_doc)
+        gen, trans_id = self.db._get_generation_info()
+        self.db.validate_gen_and_trans_id(gen, trans_id)
+
+    def test_validate_gen_and_trans_id_invalid_txid(self):
+        self.db.create_doc(simple_doc)
+        gen, _ = self.db._get_generation_info()
+        self.assertRaises(
+            errors.InvalidTransactionId,
+            self.db.validate_gen_and_trans_id, gen, 'wrong')
+
+    def test_validate_gen_and_trans_id_invalid_txid(self):
+        self.db.create_doc(simple_doc)
+        gen, trans_id = self.db._get_generation_info()
+        self.assertRaises(
+            errors.InvalidGeneration,
+            self.db.validate_gen_and_trans_id, gen + 1, trans_id)
+
     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')

=== modified file 'u1db/tests/test_c_backend.py'
--- u1db/tests/test_c_backend.py	2012-06-19 19:41:19 +0000
+++ u1db/tests/test_c_backend.py	2012-06-19 19:41:19 +0000
@@ -73,6 +73,14 @@
         db.create_doc(tests.simple_doc)
         self.assertEqual(1, db._get_generation())
 
+    def test__get_generation_info(self):
+        db = c_backend_wrapper.CDatabase(':memory:')
+        self.assertEqual((0, None), db._get_generation_info())
+        db.create_doc(tests.simple_doc)
+        info = db._get_generation_info()
+        self.assertEqual(1, info[0])
+        self.assertTrue(info[1].startswith('T-'))
+
     def test__set_replica_uid(self):
         db = c_backend_wrapper.CDatabase(':memory:')
         self.assertIsNot(None, db._replica_uid)

=== modified file 'u1db/tests/test_sqlite_backend.py'
--- u1db/tests/test_sqlite_backend.py	2012-05-31 16:31:31 +0000
+++ u1db/tests/test_sqlite_backend.py	2012-06-19 19:41:19 +0000
@@ -152,6 +152,9 @@
     def test__get_generation(self):
         self.assertEqual(0, self.db._get_generation())
 
+    def test__get_generation_info(self):
+        self.assertEqual((0, None), self.db._get_generation_info())
+
     def test_create_index(self):
         self.db.create_index('test-idx', "key")
         self.assertEqual([('test-idx', ["key"])], self.db.list_indexes())
@@ -261,7 +264,7 @@
     def test__open_database_with_factory(self):
         temp_dir = self.createTempDir(prefix='u1db-test-')
         path = temp_dir + '/test.sqlite'
-        db = sqlite_backend.SQLitePartialExpandDatabase(path)
+        sqlite_backend.SQLitePartialExpandDatabase(path)
         db2 = sqlite_backend.SQLiteDatabase._open_database(
             path, document_factory=TestAlternativeDocument)
         self.assertEqual(TestAlternativeDocument, db2._factory)
@@ -322,7 +325,7 @@
     def test_open_database_with_factory(self):
         temp_dir = self.createTempDir(prefix='u1db-test-')
         path = temp_dir + '/existing.sqlite'
-        db = sqlite_backend.SQLitePartialExpandDatabase(path)
+        sqlite_backend.SQLitePartialExpandDatabase(path)
         db2 = sqlite_backend.SQLiteDatabase.open_database(
             path, create=False, document_factory=TestAlternativeDocument)
         self.assertEqual(TestAlternativeDocument, db2._factory)

=== modified file 'u1db/tests/test_sync.py'
--- u1db/tests/test_sync.py	2012-06-19 19:41:19 +0000
+++ u1db/tests/test_sync.py	2012-06-19 19:41:19 +0000
@@ -839,6 +839,12 @@
         doc1 = self.db1.create_doc('{"a": 1}', doc_id='the-doc')
         db3 = self.create_database('test3')
         self.sync(self.db2, self.db1)
+        self.assertEqual(
+            self.db1._get_generation_info(),
+            self.db2._get_sync_gen_info(self.db1._replica_uid))
+        self.assertEqual(
+            self.db2._get_generation_info(),
+            self.db1._get_sync_gen_info(self.db2._replica_uid))
         self.sync(db3, self.db1)
         # update on 2
         doc2 = self.make_document('the-doc', doc1.rev, '{"a": 2}')
@@ -872,7 +878,19 @@
         self.db2.create_doc('{"b": 1}', doc_id='the-doc')
         db3.create_doc('{"c": 1}', doc_id='the-doc')
         self.sync(db3, self.db1)
+        self.assertEqual(
+            self.db1._get_generation_info(),
+            db3._get_sync_gen_info(self.db1._replica_uid))
+        self.assertEqual(
+            db3._get_generation_info(),
+            self.db1._get_sync_gen_info(db3._replica_uid))
         self.sync(db3, self.db2)
+        self.assertEqual(
+            self.db2._get_generation_info(),
+            db3._get_sync_gen_info(self.db2._replica_uid))
+        self.assertEqual(
+            db3._get_generation_info(),
+            self.db2._get_sync_gen_info(db3._replica_uid))
         self.assertEqual(3, len(db3.get_doc_conflicts('the-doc')))
         doc1.set_json('{"a": 2}')
         self.db1.put_doc(doc1)