Merge lp:~cmiller/desktopcouch/preconditionfailed-lp707321 into lp:desktopcouch

[Chad Miller]

Change creation of the Database object to EAFP. LBYL is impossible with HTTP and no locking. If create-flag is set, then go do it and swallow an exception about it already existing. (LP: #707321)

In addition, simplify that code so it makes fewer round-trips to the server.

[Eric Casteleijn (thisfred)]

Looks good, let's land it and run it past one of the bug reporters to see if it fixes it for them.

review: Approve

[lp:~cmiller/desktopcouch/preconditionfailed-lp707321]

269. By Chad Miller on 2011-04-04

Attach bug.

[Roberto Alsina (ralsina)]

Looks good to me :-)

review: Approve

=== modified file 'desktopcouch/records/database.py'
--- desktopcouch/records/database.py	2011-01-27 20:00:45 +0000
+++ desktopcouch/records/database.py	2011-04-04 19:13:27 +0000
@@ -43,7 +43,7 @@
 # pylint: enable=F0401
 
 from couchdb import Server
-from couchdb.http import  ResourceNotFound, ResourceConflict
+from couchdb.http import ResourceNotFound, ResourceConflict, PreconditionFailed
 from couchdb.design import ViewDefinition
 
 from desktopcouch.records import Record
@@ -134,17 +134,21 @@
         """Reconnect after losing connection."""
         self._server = self._server_class(uri or self.server_uri,
                 **self._server_class_extras)
-        if self._database_name not in self._server:
-            if self._create:
+        if self._create:
+            try:
                 self._server.create(self._database_name)
+            except PreconditionFailed:
+                pass  # Not an error to want it created and DB exists.
+
+        try:
+            if self.db is None:
+                self.db = self._server[self._database_name]
             else:
-                raise NoSuchDatabase(self._database_name)
-        if self.db is None:
-            self.db = self._server[self._database_name]
-        else:
-            # Monkey-patch the object the user already uses.  Oook!
-            new_db = self._server[self._database_name]
-            self.db.resource = new_db.resource
+                # Monkey-patch the object the user already uses.  Oook!
+                new_db = self._server[self._database_name]
+                self.db.resource = new_db.resource
+        except ResourceNotFound:
+            raise NoSuchDatabase(self._database_name)
 
     def _temporary_query(self, map_fun, reduce_fun=None, language='javascript',
             wrapper=None, **options):

=== modified file 'desktopcouch/records/tests/test_mocked_server.py'
--- desktopcouch/records/tests/test_mocked_server.py	2011-01-25 17:58:49 +0000
+++ desktopcouch/records/tests/test_mocked_server.py	2011-04-04 19:13:27 +0000
@@ -17,7 +17,7 @@
 # Authors: Manuel de la Pena<[e-mail]>
 """Test the database code mocking the db access."""
 
-from couchdb.http import  ResourceNotFound
+from couchdb.http import ResourceNotFound, PreconditionFailed
 from mocker import MockerTestCase, ANY, KWARGS
 
 from desktopcouch.application.server import DesktopDatabase
@@ -31,8 +31,7 @@
     testcase.server_class(testcase.uri, ctx=testcase.ctx,
         oauth_tokens=testcase.oauth_tokens)
     testcase.mocker.result(testcase.server_class)
-    testcase.dbname in testcase.server_class
-    testcase.mocker.result(True)
+    testcase.server_class.create(testcase.dbname)
     testcase.server_class[testcase.dbname]
     testcase.mocker.result(testcase.server_class)
     testcase.server_class.info()
@@ -61,6 +60,92 @@
         views_factory=testcase.views_factory)
 
 
+class TestMockedCouchDatabaseCreateStates(MockerTestCase):
+    def setUp(self):
+        """Set up tests."""
+        super(TestMockedCouchDatabaseCreateStates, self).setUp()
+        self.record_factory = self.mocker.mock()
+        self.server_class = self.mocker.mock()
+        self.oauth_tokens = self.mocker.mock()
+        self.ctx = self.mocker.mock()
+        self.uri = 'uri'
+        self.dbname = self._testMethodName
+
+    def test_create_already_exists(self):
+        """Access DB with create=True, when it already exists."""
+        self.server_class(self.uri, ctx=self.ctx,
+            oauth_tokens=self.oauth_tokens)
+        self.mocker.result(self.server_class)
+        self.server_class.create(self.dbname)
+        self.mocker.throw(PreconditionFailed)
+
+        self.server_class[self.dbname]
+        self.mocker.result(self.server_class)
+        self.server_class.info()
+        self.mocker.result({'update_seq': []})
+
+        self.mocker.replay()
+        database = DesktopDatabase(self.dbname, uri=self.uri,
+            record_factory=self.record_factory, create=True,
+            server_class=self.server_class,
+            oauth_tokens=self.oauth_tokens, ctx=self.ctx)
+
+    def test_create_new(self):
+        """Access DB with create=True, when it doesn't exist."""
+        self.server_class(self.uri, ctx=self.ctx,
+            oauth_tokens=self.oauth_tokens)
+        self.mocker.result(self.server_class)
+        self.server_class.create(self.dbname)
+        self.mocker.result(self.server_class)
+        self.server_class[self.dbname]
+        self.mocker.result(self.server_class)
+        info = self.mocker.mock()
+        self.server_class.info()
+        self.mocker.result(info)
+        info["update_seq"]
+        self.mocker.result({})
+        self.mocker.replay()
+        database = DesktopDatabase(self.dbname, uri=self.uri,
+            record_factory=self.record_factory, create=True,
+            server_class=self.server_class,
+            oauth_tokens=self.oauth_tokens, ctx=self.ctx)
+
+    def test_no_create_not_exists(self):
+        """Access DB with create=False, when it already exists."""
+        self.server_class(self.uri, ctx=self.ctx,
+            oauth_tokens=self.oauth_tokens)
+        self.mocker.result(self.server_class)
+
+        self.server_class[self.dbname]
+        self.mocker.throw(ResourceNotFound)
+
+        self.mocker.replay()
+        self.assertRaises(NoSuchDatabase,
+            DesktopDatabase, self.dbname, uri=self.uri,
+            record_factory=self.record_factory, create=False,
+            server_class=self.server_class,
+            oauth_tokens=self.oauth_tokens, ctx=self.ctx)
+
+    def test_no_create_already_exists(self):
+        """Access DB with create=False, when it doesn't exist."""
+        self.server_class(self.uri, ctx=self.ctx,
+            oauth_tokens=self.oauth_tokens)
+        self.mocker.result(self.server_class)
+        self.server_class[self.dbname]
+        self.mocker.result(self.server_class)
+        info = self.mocker.mock()
+        self.server_class.info()
+        self.mocker.result(info)
+        info["update_seq"]
+        self.mocker.result({})
+
+        self.mocker.replay()
+        database = DesktopDatabase(self.dbname, uri=self.uri,
+            record_factory=self.record_factory, create=False,
+            server_class=self.server_class,
+            oauth_tokens=self.oauth_tokens, ctx=self.ctx)
+
+
 class TestMockedCouchDatabaseDeprecated(MockerTestCase):
     """Test the deprecated API."""
 
@@ -199,8 +284,8 @@
         self.server_class(self.uri, ctx=self.ctx,
             oauth_tokens=self.oauth_tokens)
         self.mocker.result(self.server_class)
-        self.dbname in self.server_class
-        self.mocker.result(False)
+        self.server_class[self.dbname]
+        self.mocker.throw(ResourceNotFound)
         self.mocker.replay()
         self.assertRaises(NoSuchDatabase, DesktopDatabase, self.dbname,
             uri=self.uri, record_factory=self.record_factory, create=False,