Merge lp:~vds/desktopcouch/remove_delete_flag into lp:desktopcouch

[Vincenzo Di Somma]

Instead of deleting records we are marking as deleted in private application annotation, but we don't want to maintain "deleted" records in the database for ever, it's bad for couchdb both for views performance and coding the views.
What we want to do is move the records to a "trash" database that can be replicated and can offer undo functionalities.

Instead of deleting records we are marking as deleted in private application annotation, but we don't want to maintain "deleted" records in the database for ever, it's bad for couchdb both for views performance and coding the views.
What we want to do is move the records to a "trash" database that can be replicated and can offer undo functionalities.

[Eric Casteleijn (thisfred)]

I LOVE it! Also, the tests pass... +1000

review: Approve

[John Lenton (chipaca)]

A migration path would be nice.

review: Approve

=== modified file 'desktopcouch/records/server.py'
--- desktopcouch/records/server.py	2010-10-31 21:37:35 +0000
+++ desktopcouch/records/server.py	2010-11-04 00:03:52 +0000
@@ -20,7 +20,9 @@
 #          Chad Miller <[e-mail]>
  
 """The Desktop Couch Records API."""
- 
+
+import copy, uuid
+
 from couchdb import Server
 from couchdb.client import Resource
 import desktopcouch
@@ -28,7 +30,12 @@
 from desktopcouch.platform import find_port
 import urlparse
 
+DCTRASH = 'dctrash'
+
 class OAuthCapableServer(Server):
+    """Subclass Server to provide oauth magic"""
+    # pylint: disable-msg=W0231
+    # __init__ method from base class is not called
     def __init__(self, uri, oauth_tokens=None, ctx=None):
         """Subclass of couchdb.client.Server which creates a custom
            httplib2.Http subclass which understands OAuth"""
@@ -41,9 +48,12 @@
         (consumer_key, consumer_secret, token, token_secret) = (
             oauth_tokens["consumer_key"], oauth_tokens["consumer_secret"], 
             oauth_tokens["token"], oauth_tokens["token_secret"])
-        http.add_oauth_tokens(consumer_key, consumer_secret, token, token_secret)
+        http.add_oauth_tokens(
+            consumer_key, consumer_secret, token, token_secret)
         self.resource = Resource(http, uri)
- 
+    # pylint: enable-msg=W0231
+
+    
 class CouchDatabase(server_base.CouchDatabaseBase):
     """An small records specific abstraction over a couch db database."""
  
@@ -56,6 +66,33 @@
                 database, uri, record_factory=record_factory, create=create,
                 server_class=server_class, oauth_tokens=oauth_tokens, ctx=ctx)
 
+    # pylint: disable-msg=W0212
+    #Access to a protected member
+    def delete_record(self, record_id):
+        """Delete record with given id"""
+        record = self.get_record(record_id)
+        new_record = copy.deepcopy(record)
+        dctrash = CouchDatabase(
+            database=DCTRASH,
+            uri=self.server_uri,
+            create=True,
+            **self._server_class_extras)
+        new_record.record_id = str(uuid.uuid4())
+        del new_record._data['_rev'] 
+        try:
+            del new_record._data['_attachments'] 
+        except KeyError:
+            pass
+        new_record.application_annotations['desktopcouch'] = {
+            'private_application_annotations':
+            {'original_database_name': self._database_name,
+             'original_id': record_id}}
+        del self.db[record_id]
+        return dctrash.put_record(new_record)
+    # pylint: enable-msg=W0212
+
+    # pylint: disable-msg=W0221
+    # Arguments number differs from overridden method
     def _reconnect(self):
         if not self.server_uri:
             port = find_port(ctx=self.ctx)
@@ -63,4 +100,4 @@
         else:
             uri = self.server_uri
         super(CouchDatabase, self)._reconnect(uri=uri)
-
+    # pylint: enable-msg=W0221

=== modified file 'desktopcouch/records/server_base.py'
--- desktopcouch/records/server_base.py	2010-11-02 21:20:35 +0000
+++ desktopcouch/records/server_base.py	2010-11-04 00:03:52 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.
+# Copyright 2009-2010 Canonical Ltd.
 #
 # This file is part of desktopcouch.
 #
@@ -18,6 +18,7 @@
 #          Mark G. Saye <[e-mail]>
 #          Stuart Langridge <[e-mail]>
 #          Chad Miller <[e-mail]>
+#          Vincenzo Di Somma <[e-mail]>
 
 """The Desktop Couch Records API."""
 
@@ -44,10 +45,8 @@
 from record import Record
 import logging
 
-#DEFAULT_DESIGN_DOCUMENT = "design"
 DEFAULT_DESIGN_DOCUMENT = None  # each view in its own eponymous design doc.
 
-
 def get_changes(self, changes_since):
     """This method is used to monkey patch the database to provide a
     get_changes method"""
@@ -161,12 +160,12 @@
 
     def __init__(self, database, uri, record_factory=None, create=False,
                  server_class=Server, **server_class_extras):
+        self.server_uri = uri
         self._database_name = database
+        self.record_factory = record_factory or Record
         self._create = create
         self._server_class = server_class
         self._server_class_extras = server_class_extras
-        self.record_factory = record_factory or Record
-        self.server_uri = uri
         self._server = None
         self.db = None
         self._reconnect()
@@ -378,6 +377,14 @@
             # take a well deserved break.
             break
 
+    def record_exists(self, record_id):
+        """Check if record with given id exists."""
+        try:
+            record = self.db[record_id]
+        except ResourceNotFound:
+            return False
+        return not row_is_deleted(record)
+
     def delete_record(self, record_id):
         """Delete record with given id"""
         record = self.db[record_id]
@@ -386,14 +393,6 @@
             'deleted'] = True
         self.db[record_id] = record
 
-    def record_exists(self, record_id):
-        """Check if record with given id exists."""
-        try:
-            record = self.db[record_id]
-        except ResourceNotFound:
-            return False
-        return not row_is_deleted(record)
-
     def delete_view(self, view_name, design_doc=DEFAULT_DESIGN_DOCUMENT):
         """Remove a view, given its name.  Raises a KeyError on a unknown
         view.  Returns a dict of functions the deleted view defined."""

=== modified file 'desktopcouch/records/tests/test_server.py'
--- desktopcouch/records/tests/test_server.py	2010-11-03 02:47:32 +0000
+++ desktopcouch/records/tests/test_server.py	2010-11-04 00:03:52 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.
+# Copyright 2009-2010 Canonical Ltd.
 #
 # This file is part of desktopcouch.
 #
@@ -16,6 +16,7 @@
 #
 # Authors: Eric Casteleijn <[e-mail]>
 #          Chad Miller <[e-mail]>
+#          Vincenzo Di Somma <[e-mail]>
 
 """testing database/contact.py module"""
 import testtools
@@ -38,6 +39,8 @@
     from cStringIO import StringIO as StringIO
 # pylint: enable-msg=F0401
 
+DCTRASH = 'dctrash'
+
 FAKE_RECORD_TYPE = "http://example.org/test"
 
 js = """
@@ -48,8 +51,11 @@
 }""" % FAKE_RECORD_TYPE
 
 
+def get_test_context():
+        return test_environment.test_context
+
 class TestCouchDatabaseDeprecated(testtools.TestCase):
-    """tests specific for CouchDatabase"""
+    """Test specific for CouchDatabase"""
 
     def setUp(self):
         """setup each test"""
@@ -57,7 +63,7 @@
         # Connect to CouchDB server
         self.dbname = self._testMethodName
         self.database = CouchDatabase(self.dbname, create=True,
-                ctx=self.get_test_context())
+                ctx=get_test_context())
         #create some records to pull out and test
         self.database.put_record(Record({
             "key1_1": "val1_1", "key1_2": "val1_2", "key1_3": "val1_3",
@@ -72,21 +78,18 @@
     def tearDown(self):
         """tear down each test"""
         del self.database._server[self.database._database_name]
-        this_context = self.get_test_context()
+        this_context = get_test_context()
         if this_context != test_environment.test_context:
             stop_couchdb(ctx=this_context)
         super(TestCouchDatabaseDeprecated, self).tearDown()
 
-    def get_test_context(self):
-        return test_environment.test_context
-
     def maybe_die(self):
         pass
 
     def wait_until_server_dead(self, pid=None):
         if pid is not None:
             pid = find_pid(
-                start_if_not_running=False, ctx=self.get_test_context())
+                start_if_not_running=False, ctx=get_test_context())
         if pid is None:
             return
         while True:
@@ -163,7 +166,7 @@
         # Connect to CouchDB server
         self.dbname = self._testMethodName
         self.database = CouchDatabase(self.dbname, create=True,
-                ctx=self.get_test_context())
+                ctx=get_test_context())
         #create some records to pull out and test
         self.database.put_record(Record({
             "key1_1": "val1_1", "key1_2": "val1_2", "key1_3": "val1_3",
@@ -178,21 +181,18 @@
     def tearDown(self):
         """tear down each test"""
         del self.database._server[self.database._database_name]
-        this_context = self.get_test_context()
+        this_context = get_test_context()
         if this_context != test_environment.test_context:
             stop_couchdb(ctx=this_context)
         super(TestCouchDatabase, self).tearDown()
 
-    def get_test_context(self):
-        return test_environment.test_context
-
     def maybe_die(self):
         pass
 
     def wait_until_server_dead(self, pid=None):
         if pid is not None:
             pid = find_pid(
-                start_if_not_running=False, ctx=self.get_test_context())
+                start_if_not_running=False, ctx=get_test_context())
         if pid is None:
             return
         while True:
@@ -221,23 +221,13 @@
         self.assertEqual(0, retrieved_record['record_number'])
 
     def test_get_deleted_record(self):
-        """Test getting a record."""
+        """Test getting a deleted record."""
         record = Record({'record_number': 0}, record_type="http://example.com/")
         record_id = self.database.put_record(record)
         self.database.delete_record(record_id)
         retrieved_record = self.database.get_record(record_id)
-        #FIXME the record has to be None
         self.assertEqual(retrieved_record, None)
 
-    def test_get_deleted_record_not_hidden(self):
-        """Test getting a record."""
-        record = Record({'record_number': 0}, record_type="http://example.com/")
-        record_id = self.database.put_record(record)
-        self.database.delete_record(record_id)
-        retrieved_record = self.database.get_record(record_id=record_id,
-                                                    hide_deleted=False)
-        self.assertEqual(0, retrieved_record['record_number'])
-
     def test_put_record(self):
         """Test putting a record."""
         record = Record({'record_number': 0}, record_type="http://example.com/")
@@ -269,15 +259,21 @@
                 self.assertFalse(self.database._server[self.dbname][docid])
 
     def test_delete_record(self):
-        """Test deletion of records."""
+        """Test getting a record."""
         record = Record({'record_number': 0}, record_type="http://example.com/")
         record_id = self.database.put_record(record)
-        self.database.delete_record(record_id)
-        self.maybe_die()  # should be able to survive couchdb death
-        deleted_record = self.database.get_record(record_id=record_id,
-                                                  hide_deleted=False)
-        self.assert_(deleted_record.application_annotations['Ubuntu One'][
-            'private_application_annotations']['deleted'])
+        trash_record_id = self.database.delete_record(record_id)
+        self.assertIn(DCTRASH, self.database._server)
+        dctrash = CouchDatabase(DCTRASH, create=False,
+                                         ctx=get_test_context())
+        deleted_record = dctrash.get_record(trash_record_id)
+        self.assertNotEqual(record_id, deleted_record.record_id)
+        ann = deleted_record.application_annotations['desktopcouch']
+        priv_ann = ann['private_application_annotations']
+        original_database_name = priv_ann['original_database_name']
+        original_id = priv_ann['original_id']
+        self.assertEqual(record_id, original_id)
+        self.assertEqual(self.dbname, original_database_name)
 
     def test_delete_and_recreate_record(self):
         """Test deletion of records."""