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

[John A Meinel]

Implement Database.delete_doc(Document)

I also updated this test:
    def test_delete_doc(self):
        doc = self.db.create_doc(simple_doc)
        self.assertGetDoc(self.db, doc.doc_id, doc.rev, simple_doc, False)
        orig_rev = doc.rev
        self.db.delete_doc(doc)
        self.assertNotEqual(orig_rev, doc.rev)
        self.assertGetDoc(self.db, doc.doc_id, doc.rev, None, False)
        self.assertIsNot(None, self.db.get_doc(doc.doc_id))

Adding that last line is to be clear that get_doc() should return a document with content = None.
assertGetDoc was already doing so, but it may be clearer if we do an explicit check.

[Samuele Pedroni (pedronis)]

looks good, interestingly if I do this change:

=== modified file 'u1db/backends/inmemory.py'
--- u1db/backends/inmemory.py 2011-11-25 15:19:55 +0000
+++ u1db/backends/inmemory.py 2011-11-25 18:30:55 +0000
@@ -81,7 +81,7 @@
         for index in self._indexes.itervalues():
             if old_content is not None:
                 index.remove_json(doc_id, old_content)
- if content not in (None, 'null'):
+ if content not in (None,):
                 index.add_json(doc_id, content)
         self._docs[doc_id] = (new_rev, content)
         self._transaction_log.append(doc_id)
@@ -133,7 +133,7 @@
     def delete_doc(self, doc):
         if doc.doc_id not in self._docs:
             raise KeyError
- if self._docs[doc.doc_id][1] in ('null', None):
+ if self._docs[doc.doc_id][1] in (None,):
             raise KeyError
         doc.content = None
         self.put_doc(doc)

no tests fail as well

review: Approve

[Samuele Pedroni (pedronis)]

you'll get conflicts on RevisionConflict vs the old InvalidDocRev!

=== modified file 'u1db/__init__.py'
--- u1db/__init__.py	2011-11-23 13:46:56 +0000
+++ u1db/__init__.py	2011-11-25 15:23:27 +0000
@@ -119,10 +119,9 @@
         """
         raise NotImplementedError(self.force_doc_sync_conflict)
 
-    def delete_doc(self, doc_id, old_doc_rev):
+    def delete_doc(self, doc):
         """Mark a document as deleted.
-        (might be equivalent to PUT(nil)). Will abort if the document is now
-        'newer' than old_doc_rev.
+        Will abort if the current revision doesn't match doc.rev.
         """
         raise NotImplementedError(self.delete_doc)
 

=== modified file 'u1db/backends/inmemory.py'
--- u1db/backends/inmemory.py	2011-11-23 12:39:32 +0000
+++ u1db/backends/inmemory.py	2011-11-25 15:23:27 +0000
@@ -98,7 +98,7 @@
 
     def get_doc(self, doc_id):
         doc = self._get_doc(doc_id)
-        if doc is None or doc.content == 'null':
+        if doc is None:
             return None
         doc.has_conflicts = (doc.doc_id in self._conflicts)
         return doc
@@ -130,13 +130,13 @@
             self._conflicts[doc_id] = remaining_conflicts
         return new_rev, bool(remaining_conflicts)
 
-    def delete_doc(self, doc_id, doc_rev):
-        if doc_id not in self._docs:
-            raise KeyError
-        if self._docs[doc_id][1] in ('null', None):
-            raise KeyError
-        doc = Document(doc_id, doc_rev, None)
-        return self.put_doc(doc)
+    def delete_doc(self, doc):
+        if doc.doc_id not in self._docs:
+            raise KeyError
+        if self._docs[doc.doc_id][1] in ('null', None):
+            raise KeyError
+        doc.content = None
+        self.put_doc(doc)
 
     def create_index(self, index_name, index_expression):
         index = InMemoryIndex(index_name, index_expression)

=== modified file 'u1db/backends/sqlite_backend.py'
--- u1db/backends/sqlite_backend.py	2011-11-23 13:46:56 +0000
+++ u1db/backends/sqlite_backend.py	2011-11-25 15:23:27 +0000
@@ -240,7 +240,7 @@
 
     def get_doc(self, doc_id):
         doc = self._get_doc(doc_id)
-        if doc is None or doc.content == 'null':
+        if doc is None:
             return None
         # TODO: A doc which appears deleted could still have conflicts...
         doc.has_conflicts = self._has_conflicts(doc.doc_id)
@@ -317,19 +317,22 @@
             doc_ids.add(doc_id)
         return cur_gen, doc_ids
 
-    def delete_doc(self, doc_id, doc_rev):
+    def delete_doc(self, doc):
         with self._db_handle:
-            old_doc = self._get_doc(doc_id)
+            old_doc = self._get_doc(doc.doc_id)
             if old_doc is None:
                 raise KeyError
-            if old_doc.rev != doc_rev:
+            if old_doc.rev != doc.rev:
                 raise errors.InvalidDocRev()
             if old_doc.content is None:
                 raise KeyError
-            if self._has_conflicts(doc_id):
+            if self._has_conflicts(doc.doc_id):
                 raise errors.ConflictedDoc()
-            new_rev = self._allocate_doc_rev(doc_rev)
-            self._put_and_update_indexes(doc_id, old_doc, new_rev, None)
+            new_rev = self._allocate_doc_rev(doc.rev)
+            self._put_and_update_indexes(doc.doc_id,
+                old_doc.content, new_rev, None)
+            doc.rev = new_rev
+            doc.content = None
         return new_rev
 
     def _get_conflicts(self, doc_id):

=== modified file 'u1db/tests/test_backends.py'
--- u1db/tests/test_backends.py	2011-11-23 13:46:56 +0000
+++ u1db/tests/test_backends.py	2011-11-25 15:23:27 +0000
@@ -182,26 +182,29 @@
     def test_delete_doc(self):
         doc = self.db.create_doc(simple_doc)
         self.assertGetDoc(self.db, doc.doc_id, doc.rev, simple_doc, False)
-        deleted_rev = self.db.delete_doc(doc.doc_id, doc.rev)
-        self.assertNotEqual(None, deleted_rev)
-        self.assertGetDoc(self.db, doc.doc_id, deleted_rev, None, False)
+        orig_rev = doc.rev
+        self.db.delete_doc(doc)
+        self.assertNotEqual(orig_rev, doc.rev)
+        self.assertGetDoc(self.db, doc.doc_id, doc.rev, None, False)
+        self.assertIsNot(None, self.db.get_doc(doc.doc_id))
 
     def test_delete_doc_non_existant(self):
+        doc = Document('non-existing', 'other:1', simple_doc)
         self.assertRaises(KeyError,
-            self.db.delete_doc, 'non-existing', 'other:1')
+            self.db.delete_doc, doc)
 
     def test_delete_doc_already_deleted(self):
         doc = self.db.create_doc(simple_doc)
-        new_rev = self.db.delete_doc(doc.doc_id, doc.rev)
-        self.assertRaises(KeyError, self.db.delete_doc, doc.doc_id, new_rev)
-        self.assertGetDoc(self.db, doc.doc_id, new_rev, None, False)
+        self.db.delete_doc(doc)
+        self.assertRaises(KeyError, self.db.delete_doc, doc)
+        self.assertGetDoc(self.db, doc.doc_id, doc.rev, None, False)
 
     def test_delete_doc_bad_rev(self):
-        doc = self.db.create_doc(simple_doc)
-        self.assertGetDoc(self.db, doc.doc_id, doc.rev, simple_doc, False)
-        self.assertRaises(errors.InvalidDocRev,
-            self.db.delete_doc, doc.doc_id, 'other:1')
-        self.assertGetDoc(self.db, doc.doc_id, doc.rev, simple_doc, False)
+        doc1 = self.db.create_doc(simple_doc)
+        self.assertGetDoc(self.db, doc1.doc_id, doc1.rev, simple_doc, False)
+        doc2 = Document(doc1.doc_id, 'other:1', simple_doc)
+        self.assertRaises(errors.InvalidDocRev, self.db.delete_doc, doc2)
+        self.assertGetDoc(self.db, doc1.doc_id, doc1.rev, simple_doc, False)
 
     def test_put_updates_transaction_log(self):
         doc = self.db.create_doc(simple_doc)
@@ -215,7 +218,7 @@
     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.doc_id, doc.rev)
+        self.db.delete_doc(doc)
         self.assertEqual((2, set([doc.doc_id])), self.db.whats_changed(db_gen))
 
     def test_whats_changed_initial_database(self):
@@ -422,7 +425,7 @@
         self.db.create_index('test-idx', ['key'])
         self.assertEqual(sorted([doc, doc2]),
             sorted(self.db.get_from_index('test-idx', [('value',)])))
-        self.db.delete_doc(doc.doc_id, doc.rev)
+        self.db.delete_doc(doc)
         self.assertEqual([doc2],
             self.db.get_from_index('test-idx', [('value',)]))
 

=== modified file 'u1db/tests/test_sync.py'
--- u1db/tests/test_sync.py	2011-11-22 11:09:00 +0000
+++ u1db/tests/test_sync.py	2011-11-25 15:23:27 +0000
@@ -309,24 +309,24 @@
         doc_id = doc1.doc_id
         self.db1.create_index('test-idx', ['key'])
         self.sync(self.db1, self.db2)
-        doc2_rev = doc1.rev
+        doc2 = Document(doc1.doc_id, doc1.rev, doc1.content)
         new_doc = '{"key": "altval"}'
         doc1.content = new_doc
         self.db1.put_doc(doc1)
-        doc2_rev = self.db2.delete_doc(doc_id, doc2_rev)
+        self.db2.delete_doc(doc2)
         self.assertEqual([doc_id, doc_id], self.db1._get_transaction_log())
         self.sync(self.db1, self.db2)
         self.assertEqual({'receive': {'docs': [(doc_id, doc1.rev)],
                                       'from_id': 'test1',
                                       'from_gen': 2, 'last_known_gen': 1},
                           'return': {'new_docs': [],
-                                     'conf_docs': [(doc_id, doc2_rev)],
+                                     'conf_docs': [(doc_id, doc2.rev)],
                                      'last_gen': 2}},
                          self.db2._last_exchange_log)
         self.assertEqual([doc_id, doc_id, doc_id],
                          self.db1._get_transaction_log())
-        self.assertGetDoc(self.db1, doc_id, doc2_rev, None, True)
-        self.assertGetDoc(self.db2, doc_id, doc2_rev, None, False)
+        self.assertGetDoc(self.db1, doc_id, doc2.rev, None, True)
+        self.assertGetDoc(self.db2, doc_id, doc2.rev, None, False)
         self.assertEqual([], self.db1.get_from_index('test-idx', [('value',)]))
 
     def test_sync_local_race_conflicted(self):
@@ -364,7 +364,8 @@
         self.db2.create_index('test-idx', ['key'])
         self.db3 = self.create_database('test3')
         self.sync(self.db1, self.db3)
-        deleted_rev = self.db1.delete_doc(doc_id, doc1_rev)
+        self.db1.delete_doc(doc1)
+        deleted_rev = doc1.rev
         self.sync(self.db1, self.db2)
         self.assertEqual({'receive': {'docs': [(doc_id, deleted_rev)],
                                       'from_id': 'test1',
@@ -398,13 +399,10 @@
 
     def test_delete_refuses_for_conflicted(self):
         doc1 = self.db1.create_doc(simple_doc)
-        doc_id = doc1.doc_id
-        content1 = '{"key": "altval"}'
-        doc2 = self.db2.create_doc(content1, doc_id=doc_id)
+        doc2 = self.db2.create_doc(nested_doc, doc_id=doc1.doc_id)
         self.sync(self.db1, self.db2)
-        self.assertGetDoc(self.db1, doc_id, doc2.rev, content1, True)
-        self.assertRaises(errors.ConflictedDoc,
-            self.db1.delete_doc, doc_id, doc2.rev)
+        self.assertGetDoc(self.db1, doc2.doc_id, doc2.rev, nested_doc, True)
+        self.assertRaises(errors.ConflictedDoc, self.db1.delete_doc, doc2)
 
     def test_get_doc_conflicts_unconflicted(self):
         doc = self.db1.create_doc(simple_doc)