Merge lp:~jameinel/u1db/whats_changed_999574 into lp:u1db

[John A Meinel]

This is hopefully a small patch that is easier to review.

It just updates the whats_changed api so that it returns the transaction_id along with the database generation. Hopefully I updated enough of the docs to match it.

It does mean that we have to expose the transaction ids a bit in the test suite, for now I just went with '_get_transaction_log()' and pull out the bit we care about. I don't really want to add another not-for-use-in-production-code api for grabbing it. (You could argue that put_doc could return it.)

(resubmitting because I forgot to set the prereq branch.)

[Eric Casteleijn (thisfred)]

+1

review: Approve

=== modified file 'include/u1db/u1db.h'
--- include/u1db/u1db.h	2012-05-17 15:13:42 +0000
+++ include/u1db/u1db.h	2012-05-18 09:33:19 +0000
@@ -198,15 +198,16 @@
  *               get all changes in the database. The integer will be updated
  *               to point at the current generation.
  * @param cb     A callback function. This will be called passing in 'context',
- *               and a document identifier for each document that has been modified.
- *               The doc_id string is transient, so callers must copy it to
- *               their own memory if they want to keep it.
- *               If a document is changed more than once, it is currently
- *               undefined whether this will call cb() once per change, or just
- *               once per doc_id.
+ *               and a document identifier for each document that has been
+ *               modified. This includes the generation and associated
+ *               transaction id for each change. If a document is modified more
+ *               than once, only the most recent change will be given.
+ *               Note that the strings passed are transient, so must be copied
+ *               if callers want to use them after they return.
  * @param context Opaque context, passed back to the caller.
  */
-int u1db_whats_changed(u1database *db, int *gen, void *context, u1db_doc_id_gen_callback cb);
+int u1db_whats_changed(u1database *db, int *gen, void *context,
+                       u1db_trans_info_callback cb);
 
 
 /**

=== modified file 'src/u1db.c'
--- src/u1db.c	2012-05-18 09:33:19 +0000
+++ src/u1db.c	2012-05-18 09:33:19 +0000
@@ -1108,7 +1108,7 @@
 
 int
 u1db_whats_changed(u1database *db, int *gen, void *context,
-                   int (*cb)(void *, const char *doc_id, int gen))
+                   u1db_trans_info_callback cb)
 {
     int status;
     sqlite3_stmt *statement;
@@ -1116,8 +1116,8 @@
         return -1; // Bad parameters
     }
     status = sqlite3_prepare_v2(db->sql_handle,
-        "SELECT max(generation) as g, doc_id FROM transaction_log"
-        " WHERE generation > ?"
+        "SELECT max(generation) as g, doc_id, transaction_id"
+        " FROM transaction_log WHERE generation > ?"
         " GROUP BY doc_id ORDER BY g",
         -1, &statement, NULL);
     if (status != SQLITE_OK) {
@@ -1131,13 +1131,15 @@
     status = sqlite3_step(statement);
     while (status == SQLITE_ROW) {
         int local_gen;
-        char *doc_id;
+        const char *doc_id;
+        const char *trans_id;
         local_gen = sqlite3_column_int(statement, 0);
         if (local_gen > *gen) {
             *gen = local_gen;
         }
-        doc_id = (char *)sqlite3_column_text(statement, 1);
-        cb(context, doc_id, local_gen);
+        doc_id = (const char *)sqlite3_column_text(statement, 1);
+        trans_id = (const char *)sqlite3_column_text(statement, 2);
+        cb(context, doc_id, local_gen, trans_id);
         status = sqlite3_step(statement);
     }
     if (status == SQLITE_DONE) {

=== modified file 'src/u1db_sync_target.c'
--- src/u1db_sync_target.c	2012-05-18 09:33:19 +0000
+++ src/u1db_sync_target.c	2012-05-18 09:33:19 +0000
@@ -304,7 +304,8 @@
 // Callback for whats_changed to map the callback into the sync_exchange
 // doc_ids_to_return array.
 static int
-whats_changed_to_doc_ids(void *context, const char *doc_id, int gen)
+whats_changed_to_doc_ids(void *context, const char *doc_id, int gen,
+                         const char *trans_id)
 {
     struct lh_entry *e;
     struct _whats_changed_doc_ids_state *state;

=== modified file 'u1db/__init__.py'
--- u1db/__init__.py	2012-05-18 09:33:19 +0000
+++ u1db/__init__.py	2012-05-18 09:33:19 +0000
@@ -57,11 +57,12 @@
 
         :param old_generation: The generation of the database in the old
             state.
-        :return: (cur_generation, [(doc_id, generation),...])
+        :return: (cur_generation, [(doc_id, generation, trans_id),...])
             The current generation of the database, and a list of of
             changed documents since old_generation, represented by tuples
-            with for each document its doc_id and the generation corresponding
-            to the last intervening change and sorted by generation
+            with for each document its doc_id and the generation and
+            transaction id corresponding to the last intervening change and
+            sorted by generation (old changes first)
         """
         raise NotImplementedError(self.whats_changed)
 

=== modified file 'u1db/backends/inmemory.py'
--- u1db/backends/inmemory.py	2012-05-18 09:33:19 +0000
+++ u1db/backends/inmemory.py	2012-05-18 09:33:19 +0000
@@ -207,7 +207,7 @@
         generation = cur_generation
         for doc_id, trans_id in reversed(relevant_tail):
             if doc_id not in seen:
-                changes.append((doc_id, generation))
+                changes.append((doc_id, generation, trans_id))
                 seen.add(doc_id)
             generation -= 1
         changes.reverse()

=== modified file 'u1db/backends/sqlite_backend.py'
--- u1db/backends/sqlite_backend.py	2012-05-18 09:33:19 +0000
+++ u1db/backends/sqlite_backend.py	2012-05-18 09:33:19 +0000
@@ -355,16 +355,17 @@
 
     def whats_changed(self, old_generation=0):
         c = self._db_handle.cursor()
-        c.execute("SELECT generation, doc_id FROM transaction_log"
+        c.execute("SELECT generation, doc_id, transaction_id"
+                  " FROM transaction_log"
                   " WHERE generation > ? ORDER BY generation DESC",
                   (old_generation,))
         results = c.fetchall()
         cur_gen = old_generation
         seen = set()
         changes = []
-        for generation, doc_id in results:
+        for generation, doc_id, trans_id in results:
             if doc_id not in seen:
-                changes.append((doc_id, generation))
+                changes.append((doc_id, generation, trans_id))
                 seen.add(doc_id)
         if changes:
             cur_gen = changes[0][1]  # max generation

=== modified file 'u1db/sync.py'
--- u1db/sync.py	2012-05-18 09:33:19 +0000
+++ u1db/sync.py	2012-05-18 09:33:19 +0000
@@ -103,11 +103,11 @@
             self.target_replica_uid)
         if not changes and target_last_known_gen == target_gen:
             return my_gen
-        changed_doc_ids = [doc_id for doc_id, _ in changes]
+        changed_doc_ids = [doc_id for doc_id, _, _ in changes]
         # prepare to send all the changed docs
         docs_to_send = self.source.get_docs(changed_doc_ids,
             check_for_conflicts=False)
-        docs_by_generation = zip(docs_to_send, (gen for _, gen in changes))
+        docs_by_generation = zip(docs_to_send, (gen for _, gen, _ in changes))
 
         # exchange documents and try to insert the returned ones with
         # the target, return target synced-up-to gen
@@ -206,10 +206,10 @@
         self.new_gen = gen
         seen_ids = self.seen_ids
         # changed docs that weren't superseded by or converged with
-        self.changes_to_return = [(doc_id, gen) for (doc_id, gen)  in changes
-                                         if doc_id not in seen_ids or
-                                         # there was a subsequent update
-                                            seen_ids.get(doc_id) < gen]
+        self.changes_to_return = [(doc_id, gen) for (doc_id, gen, _) in changes
+                                  # there was a subsequent update
+                                  if doc_id not in seen_ids or
+                                     seen_ids.get(doc_id) < gen]
         return self.new_gen
 
     def return_docs(self, return_doc_cb):

=== modified file 'u1db/tests/__init__.py'
--- u1db/tests/__init__.py	2012-05-17 15:13:42 +0000
+++ u1db/tests/__init__.py	2012-05-18 09:33:19 +0000
@@ -177,6 +177,10 @@
             seen_transactions.add(transaction_id)
         self.assertEqual(doc_ids, just_ids)
 
+    def getLastTransId(self, db):
+        """Return the transaction id for the last database update."""
+        return self.db._get_transaction_log()[-1][-1]
+
 
 class ServerStateForTests(server_state.ServerState):
     """Used in the test suite, so we don't have to touch disk, etc."""

=== modified file 'u1db/tests/c_backend_wrapper.pyx'
--- u1db/tests/c_backend_wrapper.pyx	2012-05-18 09:33:19 +0000
+++ u1db/tests/c_backend_wrapper.pyx	2012-05-18 09:33:19 +0000
@@ -61,8 +61,6 @@
     ctypedef int (*u1db_key_callback)(void *context, char *key)
     ctypedef int (*u1db_doc_gen_callback)(void *context,
         u1db_document *doc, int gen)
-    ctypedef int (*u1db_doc_id_gen_callback)(void *context,
-        const_char_ptr doc_id, int gen)
     ctypedef int (*u1db_trans_info_callback)(void *context,
         const_char_ptr doc_id, int gen, const_char_ptr trans_id)
 
@@ -85,7 +83,7 @@
                          int n_revs, const_char_ptr *revs)
     int u1db_delete_doc(u1database *db, u1db_document *doc)
     int u1db_whats_changed(u1database *db, int *gen, void *context,
-                           u1db_doc_id_gen_callback cb)
+                           u1db_trans_info_callback cb)
     int u1db__get_transaction_log(u1database *db, void *context,
                                   u1db_trans_info_callback cb)
     int u1db_get_doc_conflicts(u1database *db, char *doc_id, void *context,
@@ -254,14 +252,6 @@
 from sqlite3 import dbapi2
 
 
-cdef int _append_doc_gen_to_list(void *context, const_char_ptr doc_id,
-                                 int generation) with gil:
-    a_list = <object>(context)
-    doc = doc_id
-    a_list.append((doc, generation))
-    return 0
-
-
 cdef int _append_trans_info_to_list(void *context, const_char_ptr doc_id,
                                     int generation,
                                     const_char_ptr trans_id) with gil:
@@ -958,7 +948,7 @@
         c_generation = generation
         handle_status("whats_changed",
             u1db_whats_changed(self._db, &c_generation, <void*>a_list,
-                               _append_doc_gen_to_list))
+                               _append_trans_info_to_list))
         return c_generation, a_list
 
     def _get_transaction_log(self):

=== modified file 'u1db/tests/test_backends.py'
--- u1db/tests/test_backends.py	2012-05-18 09:33:19 +0000
+++ u1db/tests/test_backends.py	2012-05-18 09:33:19 +0000
@@ -330,13 +330,17 @@
         doc.content = '{"something": "else"}'
         self.db.put_doc(doc)
         self.assertTransactionLog([doc.doc_id, doc.doc_id], self.db)
-        self.assertEqual((2, [(doc.doc_id, 2)]), self.db.whats_changed())
+        last_trans_id = self.getLastTransId(self.db)
+        self.assertEqual((2, [(doc.doc_id, 2, last_trans_id)]),
+                         self.db.whats_changed())
 
     def test_delete_updates_transaction_log(self):
         doc = self.db.create_doc(simple_doc)
         db_gen, _ = self.db.whats_changed()
         self.db.delete_doc(doc)
-        self.assertEqual((2, [(doc.doc_id, 2)]), self.db.whats_changed(db_gen))
+        last_trans_id = self.getLastTransId(self.db)
+        self.assertEqual((2, [(doc.doc_id, 2, last_trans_id)]),
+                         self.db.whats_changed(db_gen))
 
     def test_whats_changed_initial_database(self):
         self.assertEqual((0, []), self.db.whats_changed())
@@ -345,7 +349,9 @@
         doc = self.db.create_doc(simple_doc)
         doc.content = '{"new": "contents"}'
         self.db.put_doc(doc)
-        self.assertEqual((2, [(doc.doc_id, 2)]), self.db.whats_changed())
+        last_trans_id = self.getLastTransId(self.db)
+        self.assertEqual((2, [(doc.doc_id, 2, last_trans_id)]),
+                         self.db.whats_changed())
         self.assertEqual((2, []), self.db.whats_changed(2))
 
     def test_whats_changed_returns_last_edits_ascending(self):
@@ -353,15 +359,19 @@
         doc1 = self.db.create_doc(simple_doc)
         doc.content = '{"new": "contents"}'
         self.db.delete_doc(doc1)
+        delete_trans_id = self.getLastTransId(self.db)
         self.db.put_doc(doc)
-        self.assertEqual((4, [(doc1.doc_id, 3), (doc.doc_id, 4)]),
+        put_trans_id = self.getLastTransId(self.db)
+        self.assertEqual((4, [(doc1.doc_id, 3, delete_trans_id),
+                              (doc.doc_id, 4, put_trans_id)]),
                          self.db.whats_changed())
 
-    def test_whats_changed_doesnt_includ_old_gen(self):
+    def test_whats_changed_doesnt_include_old_gen(self):
         self.db.create_doc(simple_doc)
         self.db.create_doc(simple_doc)
         doc2 = self.db.create_doc(simple_doc)
-        self.assertEqual((3, [(doc2.doc_id, 3)]),
+        last_trans_id = self.getLastTransId(self.db)
+        self.assertEqual((3, [(doc2.doc_id, 3, last_trans_id)]),
                          self.db.whats_changed(2))