Merge lp:~pedronis/u1db/open-vs-ensure into lp:u1db

[Samuele Pedroni]

Try to in down the workings/semantics of open_database:

- fail with DatabasesDoesNotExist if the databases file is not there
- try to be robust in case the database is being opened while another process is setting it up

add an ensure_database to create a database, expose it on server state (for use by later the http app)

[lp:~pedronis/u1db/open-vs-ensure]

126. By Samuele Pedroni on 2011-11-23

fold SQLiteDatabase.ensure_database into open_database(create=True) which is also the default

127. By Samuele Pedroni on 2011-11-23

use open_database(create=True) in test_client

Unmerged revisions

=== modified file 'u1db/backends/sqlite_backend.py'
--- u1db/backends/sqlite_backend.py	2011-11-22 15:08:58 +0000
+++ u1db/backends/sqlite_backend.py	2011-11-23 12:42:23 +0000
@@ -14,8 +14,10 @@
 
 """A U1DB implementation that uses SQLite as its persistence layer."""
 
+import os
 import simplejson
 from sqlite3 import dbapi2
+import time
 
 from u1db.backends import CommonBackend, CommonSyncTarget
 from u1db import Document, errors
@@ -35,18 +37,46 @@
     def get_sync_target(self):
         return SQLiteSyncTarget(self)
 
-    @staticmethod
-    def open_database(sqlite_file):
-        # TODO: We really want a way to indicate that the database must already
-        # exist.
+    @classmethod
+    def _which_index_storage(cls, c):
+        try:
+            c.execute("SELECT value FROM u1db_config"
+                      " WHERE name = 'index_storage'")
+        except dbapi2.OperationalError, e:
+            # The table does not exist yet
+            return None, e
+        else:
+            return c.fetchone()[0], None
+
+    @classmethod
+    def _open_database(cls, sqlite_file):
+        if not os.path.isfile(sqlite_file):
+            raise errors.DatabaseDoesNotExist()
+        tries = 2
         db_handle = dbapi2.connect(sqlite_file)
         c = db_handle.cursor()
-        c.execute("SELECT value FROM u1db_config WHERE name = 'index_storage'")
-        v = c.fetchone()
-        # if v is None:
-        #     raise ValueError('No defined index_storage for database %s'
-        #                      % (sqlite_file,))
-        return SQLiteDatabase._sqlite_registry[v[0]](sqlite_file)
+        while tries:
+            v, err = cls._which_index_storage(c)
+            if v is not None:
+                break
+            # possibly another process is initializing it, wait for it to be
+            # done
+            tries -= 1
+            time.sleep(0.5)
+        else:
+            raise err # go for the richest error?
+        return SQLiteDatabase._sqlite_registry[v](sqlite_file)
+
+    @classmethod
+    def open_database(cls, sqlite_file, backend_cls=None, create=True):
+        try:
+            return cls._open_database(sqlite_file)
+        except errors.DatabaseDoesNotExist:
+            if not create:
+                raise
+            if backend_cls is None:
+                backend_cls = SQLitePartialExpandDatabase # default
+            return backend_cls(sqlite_file)
 
     @staticmethod
     def register_implementation(klass):

=== modified file 'u1db/commandline/client.py'
--- u1db/commandline/client.py	2011-11-21 14:31:33 +0000
+++ u1db/commandline/client.py	2011-11-23 12:42:23 +0000
@@ -48,7 +48,7 @@
     def run(self, database, infile, doc_id):
         if infile is None:
             infile = self.stdin
-        db = sqlite_backend.SQLiteDatabase.open_database(database)
+        db = sqlite_backend.SQLiteDatabase.open_database(database, create=False)
         doc = db.create_doc(infile.read(), doc_id=doc_id)
         self.stderr.write('id: %s\nrev: %s\n' % (doc.doc_id, doc.rev))
 
@@ -71,7 +71,7 @@
     def run(self, database, doc_id, outfile):
         if outfile is None:
             outfile = self.stdout
-        db = sqlite_backend.SQLiteDatabase.open_database(database)
+        db = sqlite_backend.SQLiteDatabase.open_database(database, create=False)
         doc = db.get_doc(doc_id)
         outfile.write(doc.content)
         self.stderr.write('rev: %s\n' % (doc.rev,))
@@ -95,7 +95,7 @@
             help='The unique identifier for this database')
 
     def run(self, database, replica_uid):
-        db = sqlite_backend.SQLitePartialExpandDatabase(database)
+        db = sqlite_backend.SQLiteDatabase.open_database(database)
         db._set_replica_uid(replica_uid)
 
 client_commands.register(CmdInitDB)
@@ -119,7 +119,7 @@
     def run(self, database, doc_id, doc_rev, infile):
         if infile is None:
             infile = self.stdin
-        db = sqlite_backend.SQLiteDatabase.open_database(database)
+        db = sqlite_backend.SQLiteDatabase.open_database(database, create=False)
         doc = Document(doc_id, doc_rev, infile.read())
         doc_rev = db.put_doc(doc)
         self.stderr.write('rev: %s\n' % (doc_rev,))
@@ -147,7 +147,8 @@
 
     def run(self, source, target):
         """Start a Sync request."""
-        source_db = sqlite_backend.SQLiteDatabase.open_database(source)
+        source_db = sqlite_backend.SQLiteDatabase.open_database(source,
+                                                                create=False)
         st = self._open_target(target)
         syncer = sync.Synchronizer(source_db, st)
         syncer.sync()

=== modified file 'u1db/errors.py'
--- u1db/errors.py	2011-11-23 09:32:06 +0000
+++ u1db/errors.py	2011-11-23 12:42:23 +0000
@@ -37,3 +37,7 @@
     Can also be raised if wildcard matches are not strictly at the tail of the
     request.
     """
+
+
+class DatabaseDoesNotExist(U1DBError):
+    """The database does not exist."""

=== modified file 'u1db/remote/server_state.py'
--- u1db/remote/server_state.py	2011-11-22 11:09:00 +0000
+++ u1db/remote/server_state.py	2011-11-23 12:42:23 +0000
@@ -42,4 +42,11 @@
         """Open a database at the given location."""
         from u1db.backends import sqlite_backend
         full_path = self._relpath(path)
+        return sqlite_backend.SQLiteDatabase.open_database(full_path,
+                                                           create=False)
+
+    def ensure_database(self, path):
+        """Ensure database at the given location."""
+        from u1db.backends import sqlite_backend
+        full_path = self._relpath(path)
         return sqlite_backend.SQLiteDatabase.open_database(full_path)

=== modified file 'u1db/tests/commandline/test_client.py'
--- u1db/tests/commandline/test_client.py	2011-11-23 09:34:59 +0000
+++ u1db/tests/commandline/test_client.py	2011-11-23 12:42:23 +0000
@@ -99,7 +99,7 @@
         super(TestCaseWithDB, self).setUp()
         self.working_dir = self.createTempDir()
         self.db_path = self.working_dir + '/test.db'
-        self.db = sqlite_backend.SQLitePartialExpandDatabase(self.db_path)
+        self.db = sqlite_backend.SQLiteDatabase.open_database(self.db_path)
         self.db._set_replica_uid('test')
         self.addCleanup(self.db.close)
 
@@ -146,7 +146,7 @@
         cmd = self.make_command(client.CmdInitDB)
         cmd.run(path, 'test-uid')
         self.assertTrue(os.path.exists(path))
-        db = sqlite_backend.SQLiteDatabase.open_database(path)
+        db = sqlite_backend.SQLiteDatabase.open_database(path, create=False)
         self.assertEqual('test-uid', db._replica_uid)
 
 
@@ -174,7 +174,7 @@
     def setUp(self):
         super(TestCmdSync, self).setUp()
         self.db2_path = self.working_dir + '/test2.db'
-        self.db2 = sqlite_backend.SQLitePartialExpandDatabase(self.db2_path)
+        self.db2 = sqlite_backend.SQLiteDatabase.open_database(self.db2_path)
         self.addCleanup(self.db2.close)
         self.db2._set_replica_uid('test2')
         self.doc  = self.db.create_doc(tests.simple_doc, doc_id='test-id')
@@ -262,7 +262,7 @@
     def test_init_db(self):
         path = self.working_dir + '/test2.db'
         ret, stdout, stderr = self.run_main(['init-db', path, 'uid'])
-        db2 = sqlite_backend.SQLiteDatabase.open_database(path)
+        db2 = sqlite_backend.SQLiteDatabase.open_database(path, create=False)
 
     def test_put(self):
         doc = self.db.create_doc(tests.simple_doc, doc_id='test-id')
@@ -279,7 +279,7 @@
     def test_sync(self):
         doc = self.db.create_doc(tests.simple_doc, doc_id='test-id')
         self.db2_path = self.working_dir + '/test2.db'
-        self.db2 = sqlite_backend.SQLitePartialExpandDatabase(self.db2_path)
+        self.db2 = sqlite_backend.SQLiteDatabase.open_database(self.db2_path)
         self.addCleanup(self.db2.close)
         ret, stdout, stderr = self.run_main(
             ['sync', self.db_path, self.db2_path])

=== modified file 'u1db/tests/test_server_state.py'
--- u1db/tests/test_server_state.py	2011-11-23 09:31:16 +0000
+++ u1db/tests/test_server_state.py	2011-11-23 12:42:23 +0000
@@ -46,3 +46,14 @@
         sqlite_backend.SQLitePartialExpandDatabase(path)
         db = self.state.open_database('test.db')
         self.assertIsInstance(db, sqlite_backend.SQLitePartialExpandDatabase)
+
+    def test_ensure_database(self):
+        tempdir = self.createTempDir()
+        self.state.set_workingdir(tempdir)
+        path = tempdir + '/test.db'
+        self.assertFalse(os.path.exists(path))
+        db = self.state.ensure_database('test.db')
+        self.assertIsInstance(db, sqlite_backend.SQLitePartialExpandDatabase)
+        self.assertTrue(os.path.exists(path))
+        db2 = self.state.open_database('test.db')
+        self.assertIsInstance(db2, sqlite_backend.SQLitePartialExpandDatabase)

=== modified file 'u1db/tests/test_sqlite_backend.py'
--- u1db/tests/test_sqlite_backend.py	2011-11-23 09:34:59 +0000
+++ u1db/tests/test_sqlite_backend.py	2011-11-23 12:42:23 +0000
@@ -15,14 +15,13 @@
 """Test sqlite backend internals."""
 
 import os
-import tempfile
 import time
 import threading
-import shutil
 
 from sqlite3 import dbapi2
 
 from u1db import (
+    errors,
     tests,
     )
 from u1db.backends import sqlite_backend
@@ -72,7 +71,7 @@
         db1 = SQLiteDatabaseTesting(dbname, 1)
         t2.join()
 
-        self.assertTrue(isinstance(outcome2[0], SQLiteDatabaseTesting))
+        self.assertIsInstance(outcome2[0], SQLiteDatabaseTesting)
         db2 = outcome2[0]
         self.assertTrue(db2._is_initialized(db1._get_sqlite_handle().cursor()))
 
@@ -210,13 +209,72 @@
                           (doc1.doc_id, "sub.doc", "underneath"),
                          ], c.fetchall())
 
-    def test_open_database(self):
-        temp_dir = tempfile.mkdtemp(prefix='u1db-test-')
-        self.addCleanup(shutil.rmtree, temp_dir)
+    def test__open_database(self):
+        temp_dir = self.createTempDir(prefix='u1db-test-')
         path = temp_dir + '/test.sqlite'
         db = sqlite_backend.SQLitePartialExpandDatabase(path)
-        db2 = sqlite_backend.SQLiteDatabase.open_database(path)
-        self.assertIsInstance(db2, sqlite_backend.SQLitePartialExpandDatabase)
+        db2 = sqlite_backend.SQLiteDatabase._open_database(path)
+        self.assertIsInstance(db2, sqlite_backend.SQLitePartialExpandDatabase)
+
+    def test__open_database_non_existent(self):
+        temp_dir = self.createTempDir(prefix='u1db-test-')
+        path = temp_dir + '/non-existent.sqlite'
+        self.assertRaises(errors.DatabaseDoesNotExist,
+                         sqlite_backend.SQLiteDatabase._open_database, path)
+
+    def test__open_database_during_init(self):
+        temp_dir = self.createTempDir(prefix='u1db-test-')
+        path = temp_dir + '/initialised.db'
+        db = sqlite_backend.SQLitePartialExpandDatabase.__new__(
+                                    sqlite_backend.SQLitePartialExpandDatabase)
+        db._db_handle = dbapi2.connect(path) # db is there but not yet init-ed
+        observed = []
+        class SQLiteDatabaseTesting(sqlite_backend.SQLiteDatabase):
+            @classmethod
+            def _which_index_storage(cls, c):
+                res = super(SQLiteDatabaseTesting, cls)._which_index_storage(c)
+                db._ensure_schema() # init db
+                observed.append(res[0])
+                return res
+        db2 = SQLiteDatabaseTesting._open_database(path)
+        self.assertIsInstance(db2, sqlite_backend.SQLitePartialExpandDatabase)
+        self.assertEqual([None,
+              sqlite_backend.SQLitePartialExpandDatabase._index_storage_value],
+                         observed)
+
+    def test__open_database_invalid(self):
+        temp_dir = self.createTempDir(prefix='u1db-test-')
+        path1 = temp_dir + '/invalid1.db'
+        with open(path1, 'wb') as f:
+            f.write("")
+        self.assertRaises(dbapi2.OperationalError,
+                          sqlite_backend.SQLiteDatabase._open_database, path1)
+        path2 = temp_dir + '/invalid2.db'
+        with open(path1, 'wb') as f:
+            f.write("invalid")
+        self.assertRaises(dbapi2.DatabaseError,
+                          sqlite_backend.SQLiteDatabase._open_database, path1)
+
+    def test_open_database_existing(self):
+        temp_dir = self.createTempDir(prefix='u1db-test-')
+        path = temp_dir + '/existing.sqlite'
+        db = sqlite_backend.SQLitePartialExpandDatabase(path)
+        db2 = sqlite_backend.SQLiteDatabase.open_database(path, create=False)
+        self.assertIsInstance(db2, sqlite_backend.SQLitePartialExpandDatabase)
+
+    def test_open_database_create(self):
+        temp_dir = self.createTempDir(prefix='u1db-test-')
+        path = temp_dir + '/new.sqlite'
+        db = sqlite_backend.SQLiteDatabase.open_database(path)
+        db2 = sqlite_backend.SQLiteDatabase.open_database(path, create=False)
+        self.assertIsInstance(db2, sqlite_backend.SQLitePartialExpandDatabase)
+
+    def test_open_database_non_existent(self):
+        temp_dir = self.createTempDir(prefix='u1db-test-')
+        path = temp_dir + '/non-existent.sqlite'
+        self.assertRaises(errors.DatabaseDoesNotExist,
+                          sqlite_backend.SQLiteDatabase.open_database, path,
+                          create=False)
 
     def assertTransform(self, sql_value, value):
         transformed = sqlite_backend.SQLiteDatabase._transform_glob(value)