Merge lp:~thisfred/u1db/document-too-big into lp:u1db

[Eric Casteleijn]

Added get_size for documents, and set_document_size_limit for dbs.

This is step 1 of the document size restriction work. This defines and implements the API on all backends. What it does not yet handle/test is size violations on remote dbs, mainly because testing this is turning out to be hard, so I thought I'd add that on a separate branch.

[Eric Casteleijn (thisfred)]

working on memory leaks

[John O'Brien (jdobrien)]

Looks good to me.

review: Approve

[Eric Casteleijn (thisfred)]

memory leaks fixed

[Ubuntu One Auto Pilot (otto-pilot)]

Attempt to merge into lp:u1db failed due to conflicts:

text conflict in u1db/tests/test_sync.py

[lp:~thisfred/u1db/document-too-big]

344. By Eric Casteleijn on 2012-07-10

merged trunk, resolved conflict

=== modified file 'include/u1db/u1db.h'
--- include/u1db/u1db.h	2012-06-27 18:56:17 +0000
+++ include/u1db/u1db.h	2012-07-10 17:17:20 +0000
@@ -44,7 +44,8 @@
                                  const char **key);
 typedef int (*u1db_doc_gen_callback)(void *context, u1db_document *doc,
                                      int gen, const char *trans_id);
-typedef int (*u1db_doc_id_gen_callback)(void *context, const char *doc_id, int gen);
+typedef int (*u1db_doc_id_gen_callback)(void *context, const char *doc_id,
+                                        int gen);
 typedef int (*u1db_trans_info_callback)(void *context, const char *doc_id,
                                         int gen, const char *trans_id);
 
@@ -73,6 +74,7 @@
 #define U1DB_INVALID_TRANSACTION_ID -20
 #define U1DB_INVALID_GENERATION -21
 #define U1DB_TARGET_UNAVAILABLE -22
+#define U1DB_DOCUMENT_TOO_BIG -23
 #define U1DB_INTERNAL_ERROR -999
 
 // Used by put_doc_if_newer
@@ -99,6 +101,11 @@
 int u1db_set_replica_uid(u1database *db, const char *replica_uid);
 
 /**
+ * Set the replica_uid defined for this database.
+ */
+int u1db_set_document_size_limit(u1database *db, int limit);
+
+/**
  * Get the replica_uid defined for this database.
  *
  * @param replica_uid (OUT) The unique identifier for this replica. The
@@ -256,14 +263,20 @@
 
 
 /**
+ * Get the size of the document in bytes.
+ */
+int u1db_doc_get_size(u1db_document *doc);
+
+
+/**
  * Create an index that you can query for matching documents.
  *
  * @param index_name    An identifier for this index.
  * @param n_expressions The number of index expressions.
  * @param exp0... The values to match in the index, all of these should be char*
  */
-int u1db_create_index(u1database *db, const char *index_name, int n_expressions,
-                      ...);
+int u1db_create_index(u1database *db, const char *index_name,
+                      int n_expressions, ...);
 
 
 /**

=== modified file 'include/u1db/u1db_internal.h'
--- include/u1db/u1db_internal.h	2012-07-06 17:34:14 +0000
+++ include/u1db/u1db_internal.h	2012-07-10 17:17:20 +0000
@@ -31,6 +31,7 @@
 {
     sqlite3 *sql_handle;
     char *replica_uid;
+    int document_size_limit;
 };
 
 struct _u1query {
@@ -231,6 +232,11 @@
 int u1db__get_generation(u1database *db, int *generation);
 
 /**
+ * Internal API, Get the document size limit.
+ */
+int u1db__get_document_size_limit(u1database *db, int *limit);
+
+/**
  * Internal API, Get the global database rev and transaction id.
  */
 int u1db__get_generation_info(u1database *db, int *generation,

=== modified file 'src/u1db.c'
--- src/u1db.c	2012-07-06 17:34:14 +0000
+++ src/u1db.c	2012-07-10 17:17:20 +0000
@@ -70,7 +70,8 @@
     }
     u1db__generate_hex_uuid(default_replica_uid);
     u1db_set_replica_uid(db, default_replica_uid);
-    return SQLITE_OK;
+    u1db_set_document_size_limit(db, 0);
+    return U1DB_OK;
 }
 
 u1database *
@@ -84,6 +85,8 @@
         free(db);
         return NULL;
     }
+    // TODO: surely this is not right? We should get the db sqlite, and only if
+    // that fails because it's not there, should we initialize?!?
     initialize(db);
     return db;
 }
@@ -154,7 +157,39 @@
     num_bytes = strlen(replica_uid);
     db->replica_uid = (char *)calloc(1, num_bytes + 1);
     memcpy(db->replica_uid, replica_uid, num_bytes + 1);
-    return 0;
+    return U1DB_OK;
+}
+
+int
+u1db_set_document_size_limit(u1database *db, int limit)
+{
+    sqlite3_stmt *statement;
+    int status, final_status;
+
+    status = sqlite3_prepare_v2(db->sql_handle,
+        "INSERT OR REPLACE INTO u1db_config VALUES ('document_size_limit', ?)",
+        -1, &statement, NULL);
+    if (status != SQLITE_OK) {
+        return status;
+    }
+    status = sqlite3_bind_int(statement, 1, limit);
+    if (status != SQLITE_OK) {
+        sqlite3_finalize(statement);
+        return status;
+    }
+    status = sqlite3_step(statement);
+    if (status != SQLITE_DONE) {
+        sqlite3_finalize(statement);
+        return status;
+    }
+    status = sqlite3_finalize(statement);
+    if (status != SQLITE_OK) {
+        return status;
+    }
+    // If we got this far, then document_size_limit has been properly set.
+    // Copy it
+    db->document_size_limit = limit;
+    return U1DB_OK;
 }
 
 int
@@ -165,7 +200,7 @@
     const unsigned char *text;
     if (db->replica_uid != NULL) {
         *replica_uid = db->replica_uid;
-        return SQLITE_OK;
+        return U1DB_OK;
     }
     status = sqlite3_prepare_v2(db->sql_handle,
         "SELECT value FROM u1db_config WHERE name = 'replica_uid'", -1,
@@ -181,7 +216,7 @@
         if (status == SQLITE_DONE) {
             // No replica_uid set yet
             *replica_uid = NULL;
-            return SQLITE_OK;
+            return U1DB_OK;
         }
         *replica_uid = "Failed to step prepared statement";
         return status;
@@ -196,7 +231,7 @@
     db->replica_uid = (char *)calloc(1, num_bytes + 1);
     memcpy(db->replica_uid, text, num_bytes+1);
     *replica_uid = db->replica_uid;
-    return SQLITE_OK;
+    return U1DB_OK;
 }
 
 static int
@@ -482,6 +517,20 @@
     return status;
 }
 
+static int
+u1db__check_doc_size(u1database *db, u1db_document *doc)
+{
+    int status = U1DB_OK;
+    int limit = 0;
+    status = u1db__get_document_size_limit(db, &limit);
+    if (status != U1DB_OK)
+        return status;
+    if (limit == 0)
+        return U1DB_OK;
+    if (u1db_doc_get_size(doc) <= limit)
+        return U1DB_OK;
+    return U1DB_DOCUMENT_TOO_BIG;
+}
 
 int
 u1db_put_doc(u1database *db, u1db_document *doc)
@@ -493,13 +542,16 @@
     sqlite3_stmt *statement = NULL;
 
     if (db == NULL || doc == NULL) {
-        // Bad parameter
-        return -1;
+        return U1DB_INVALID_PARAMETER;
     }
     status = u1db__is_doc_id_valid(doc->doc_id);
     if (status != U1DB_OK) {
         return status;
     }
+    status = u1db__check_doc_size(db, doc);
+    if (status != U1DB_OK) {
+        return status;
+    }
     status = sqlite3_exec(db->sql_handle, "BEGIN", NULL, NULL, NULL);
     if (status != SQLITE_OK) {
         return status;
@@ -1426,6 +1478,48 @@
 }
 
 int
+u1db__get_document_size_limit(u1database *db, int *limit)
+{
+    int status = U1DB_OK;
+    sqlite3_stmt *statement;
+
+    if (db == NULL || limit == NULL) {
+        return U1DB_INVALID_PARAMETER;
+    }
+    // TODO: make sure we don't do this lookup every time in case the limit
+    // really is 0.
+    if (db->document_size_limit != 0) {
+        *limit = db->document_size_limit;
+        return U1DB_OK;
+    }
+    status = sqlite3_prepare_v2(db->sql_handle,
+        "SELECT value FROM u1db_config WHERE name = 'document_size_limit'", -1,
+        &statement, NULL);
+    if(status != SQLITE_OK) {
+        return status;
+    }
+    status = sqlite3_step(statement);
+    if(status != SQLITE_ROW) {
+        // TODO: Check return for failures
+        sqlite3_finalize(statement);
+        if (status == SQLITE_DONE) {
+            // No document_size_limit set yet
+            *limit = 0;
+            db->document_size_limit = *limit;
+            return U1DB_OK;
+        }
+        return status;
+    }
+    *limit = sqlite3_column_int(statement, 0);
+    status = sqlite3_finalize(statement);
+    if (status != SQLITE_OK) {
+        return status;
+    }
+    db->document_size_limit = *limit;
+    return U1DB_OK;
+}
+
+int
 u1db__get_generation_info(u1database *db, int *generation, char **trans_id)
 {
     int status;
@@ -1914,6 +2008,24 @@
     return status;
 }
 
+
+int
+u1db_doc_get_size(u1db_document *doc)
+{
+    int size = 0;
+    if (doc->json != NULL) {
+        size += strlen(doc->json);
+    }
+    if (doc->doc_id != NULL) {
+        size += strlen(doc->doc_id);
+    }
+    if (doc->doc_rev != NULL) {
+        size += strlen(doc->doc_rev);
+    }
+    return size;
+}
+
+
 int
 u1db__is_doc_id_valid(const char *doc_id)
 {

=== modified file 'u1db/__init__.py'
--- u1db/__init__.py	2012-07-05 15:50:36 +0000
+++ u1db/__init__.py	2012-07-10 17:17:20 +0000
@@ -65,6 +65,13 @@
         """
         raise NotImplementedError(self.set_document_factory)
 
+    def set_document_size_limit(self, limit):
+        """Set the maximum allowed document size for this database.
+
+        :param limit: Maximum allowed document size in bytes.
+        """
+        raise NotImplementedError(self.set_document_size_limit)
+
     def whats_changed(self, old_generation):
         """Return a list of documents that have changed since old_generation.
         This allows APPS to only store a db generation before going
@@ -125,6 +132,9 @@
         You can optionally specify the document identifier, but the document
         must not already exist. See 'put_doc' if you want to override an
         existing document.
+        If the database specifies a maximum document size and the document
+        exceeds it, create will fail and raise a DocumentTooBig exception.
+
         :param content: The JSON document string
         :param doc_id: An optional identifier specifying the document id.
         :return: Document
@@ -134,6 +144,8 @@
     def put_doc(self, doc):
         """Update a document.
         If the document currently has conflicts, put will fail.
+        If the database specifies a maximum document size and the document
+        exceeds it, put will fail and raise a DocumentTooBig exception.
 
         :param doc: A Document with new content.
         :return: new_doc_rev - The new revision identifier for the document.
@@ -422,6 +434,18 @@
             return self._json
         return None
 
+    def get_size(self):
+        """Calculate the total size of the document."""
+        size = 0
+        json = self.get_json()
+        if json:
+            size += len(json)
+        if self.rev:
+            size += len(self.rev)
+        if self.doc_id:
+            size += len(self.doc_id)
+        return size
+
     def set_json(self, json):
         """Set the json serialization of this document."""
         if json is not None:

=== modified file 'u1db/backends/__init__.py'
--- u1db/backends/__init__.py	2012-07-06 17:34:14 +0000
+++ u1db/backends/__init__.py	2012-07-10 17:17:20 +0000
@@ -36,6 +36,8 @@
 
 class CommonBackend(u1db.Database):
 
+    document_size_limit = 0
+
     def _allocate_doc_id(self):
         """Generate a unique identifier for this document."""
         return 'D-' + uuid.uuid4().hex  # 'D-' stands for document
@@ -52,6 +54,12 @@
         if not check_doc_id_re.match(doc_id):
             raise errors.InvalidDocId()
 
+    def _check_doc_size(self, doc):
+        if not self.document_size_limit:
+            return
+        if doc.get_size() > self.document_size_limit:
+            raise errors.DocumentTooBig
+
     def _get_generation(self):
         """Return the current generation.
 
@@ -192,3 +200,6 @@
             vcr.maximize(VectorClockRev(rev))
         vcr.increment(self._replica_uid)
         return vcr.as_str()
+
+    def set_document_size_limit(self, limit):
+        self.document_size_limit = limit

=== modified file 'u1db/backends/inmemory.py'
--- u1db/backends/inmemory.py	2012-07-06 17:34:14 +0000
+++ u1db/backends/inmemory.py	2012-07-10 17:17:20 +0000
@@ -100,6 +100,7 @@
         if doc.doc_id is None:
             raise errors.InvalidDocId()
         self._check_doc_id(doc.doc_id)
+        self._check_doc_size(doc)
         if self._has_conflicts(doc.doc_id):
             raise errors.ConflictedDoc()
         old_doc = self._get_doc(doc.doc_id)

=== modified file 'u1db/backends/sqlite_backend.py'
--- u1db/backends/sqlite_backend.py	2012-07-06 17:34:14 +0000
+++ u1db/backends/sqlite_backend.py	2012-07-10 17:17:20 +0000
@@ -340,6 +340,7 @@
         if doc.doc_id is None:
             raise errors.InvalidDocId()
         self._check_doc_id(doc.doc_id)
+        self._check_doc_size(doc)
         with self._db_handle:
             if self._has_conflicts(doc.doc_id):
                 raise errors.ConflictedDoc()

=== modified file 'u1db/errors.py'
--- u1db/errors.py	2012-07-04 15:44:36 +0000
+++ u1db/errors.py	2012-07-10 17:17:20 +0000
@@ -43,6 +43,12 @@
     wire_description = "invalid document id"
 
 
+class DocumentTooBig(U1DBError):
+    """Document exceeds the maximum document size for this database."""
+
+    wire_description = "document too big"
+
+
 class InvalidTransactionId(U1DBError):
     """Invalid transaction for generation."""
 

=== modified file 'u1db/remote/http_database.py'
--- u1db/remote/http_database.py	2012-05-30 14:39:36 +0000
+++ u1db/remote/http_database.py	2012-07-10 17:17:20 +0000
@@ -41,10 +41,20 @@
     def __init__(self, url, document_factory=None):
         super(HTTPDatabase, self).__init__(url)
         self._factory = document_factory or Document
+        self.document_size_limit = 0
 
     def set_document_factory(self, factory):
         self._factory = factory
 
+    def set_document_size_limit(self, limit):
+        self.document_size_limit = limit
+
+    def _check_doc_size(self, doc):
+        if not self.document_size_limit:
+            return
+        if doc.get_size() > self.document_size_limit:
+            raise errors.DocumentTooBig
+
     @staticmethod
     def open_database(url, create):
         db = HTTPDatabase(url)
@@ -75,6 +85,7 @@
     def put_doc(self, doc):
         if doc.doc_id is None:
             raise errors.InvalidDocId()
+        self._check_doc_size(doc)
         params = {}
         if doc.rev is not None:
             params['old_rev'] = doc.rev
@@ -105,6 +116,7 @@
     def create_doc(self, content, doc_id=None):
         if doc_id is None:
             doc_id = 'D-%s' % (uuid.uuid4().hex,)
+        self._check_doc_size(self._factory(doc_id, None, content))
         res, headers = self._request_json('PUT', ['doc', doc_id], {},
                                           content, 'application/json')
         new_doc = self._factory(doc_id, res['rev'], content)

=== modified file 'u1db/remote/http_errors.py'
--- u1db/remote/http_errors.py	2012-07-04 15:43:00 +0000
+++ u1db/remote/http_errors.py	2012-07-10 17:17:20 +0000
@@ -25,6 +25,7 @@
 wire_description_to_status = dict([
     (errors.InvalidDocId.wire_description, 400),
     (errors.Unauthorized.wire_description, 401),
+    (errors.DocumentTooBig.wire_description, 403),
     (errors.DatabaseDoesNotExist.wire_description, 404),
     (errors.DocumentDoesNotExist.wire_description, 404),
     (errors.DocumentAlreadyDeleted.wire_description, 404),

=== modified file 'u1db/tests/c_backend_wrapper.pyx'
--- u1db/tests/c_backend_wrapper.pyx	2012-07-06 17:34:14 +0000
+++ u1db/tests/c_backend_wrapper.pyx	2012-07-10 17:17:20 +0000
@@ -68,6 +68,7 @@
     u1database * u1db_open(char *fname)
     void u1db_free(u1database **)
     int u1db_set_replica_uid(u1database *, char *replica_uid)
+    int u1db_set_document_size_limit(u1database *, int limit)
     int u1db_get_replica_uid(u1database *, const_char_ptr *replica_uid)
     int u1db_create_doc(u1database *db, char *json, char *doc_id,
                         u1db_document **doc)
@@ -128,6 +129,7 @@
     int U1DB_DOCUMENT_DOES_NOT_EXIST
     int U1DB_NOT_IMPLEMENTED
     int U1DB_INVALID_JSON
+    int U1DB_DOCUMENT_TOO_BIG
     int U1DB_INVALID_VALUE_FOR_INDEX
     int U1DB_INVALID_GLOBBING
     int U1DB_BROKEN_SYNC_STREAM
@@ -145,6 +147,7 @@
 
     void u1db_free_doc(u1db_document **doc)
     int u1db_doc_set_json(u1db_document *doc, char *json)
+    int u1db_doc_get_size(u1db_document *doc)
 
 
 cdef extern from "u1db/u1db_internal.h":
@@ -203,6 +206,7 @@
 
 
     int u1db__get_generation(u1database *, int *db_rev)
+    int u1db__get_document_size_limit(u1database *, int *limit)
     int u1db__get_generation_info(u1database *, int *db_rev, char **trans_id)
     int u1db__get_trans_id_for_gen(u1database *, int db_rev, char **trans_id)
     int u1db_validate_gen_and_trans_id(u1database *, int db_rev,
@@ -487,6 +491,8 @@
     def set_json(self, val):
         u1db_doc_set_json(self._doc, val)
 
+    def get_size(self):
+        return u1db_doc_get_size(self._doc)
 
     property has_conflicts:
         def __get__(self):
@@ -600,6 +606,8 @@
         raise errors.Unavailable
     if status == U1DB_INVALID_JSON:
         raise errors.InvalidJSON
+    if status == U1DB_DOCUMENT_TOO_BIG:
+        raise errors.DocumentTooBig
     raise RuntimeError('%s (status: %s)' % (context, status))
 
 
@@ -881,9 +889,24 @@
         cdef int status
         status = u1db_set_replica_uid(self._db, replica_uid)
         if status != 0:
-            raise RuntimeError('Machine_id could not be set to %s, error: %d'
+            raise RuntimeError('replica_uid could not be set to %s, error: %d'
                                % (replica_uid, status))
 
+    property document_size_limit:
+        def __get__(self):
+            cdef int limit
+            handle_status("document_size_limit",
+                u1db__get_document_size_limit(self._db, &limit))
+            return limit
+
+    def set_document_size_limit(self, limit):
+        cdef int status
+        status = u1db_set_document_size_limit(self._db, limit)
+        if status != 0:
+            raise RuntimeError(
+                "document_size_limit could not be set to %d, error: %d",
+                (limit, status))
+
     def _allocate_doc_id(self):
         cdef char *val
         val = u1db__allocate_doc_id(self._db)

=== modified file 'u1db/tests/test_backends.py'
--- u1db/tests/test_backends.py	2012-07-06 17:34:14 +0000
+++ u1db/tests/test_backends.py	2012-07-10 17:17:20 +0000
@@ -150,6 +150,17 @@
         doc = self.make_document('d\xc3\xa5c-id', None, simple_doc)
         self.assertRaises(errors.InvalidDocId, self.db.put_doc, doc)
 
+    def test_put_doc_refuses_oversized_documents(self):
+        self.db.set_document_size_limit(1)
+        doc = self.make_document('doc-id', None, simple_doc)
+        self.assertRaises(errors.DocumentTooBig, self.db.put_doc, doc)
+
+    def test_create_doc_refuses_oversized_documents(self):
+        self.db.set_document_size_limit(1)
+        self.assertRaises(
+            errors.DocumentTooBig, self.db.create_doc, simple_doc,
+            doc_id='my_doc_id')
+
     def test_put_fails_with_bad_old_rev(self):
         doc = self.db.create_doc(simple_doc, doc_id='my_doc_id')
         old_rev = doc.rev
@@ -265,6 +276,14 @@
         self.db.put_doc(doc)
         self.assertGetDoc(self.db, doc.doc_id, doc.rev, nested_doc, False)
 
+    def test_set_document_size_limit_zero(self):
+        self.db.set_document_size_limit(0)
+        self.assertEqual(0, self.db.document_size_limit)
+
+    def test_set_document_size_limit(self):
+        self.db.set_document_size_limit(1000000)
+        self.assertEqual(1000000, self.db.document_size_limit)
+
 
 class LocalDatabaseTests(tests.DatabaseBaseTests):
 

=== modified file 'u1db/tests/test_document.py'
--- u1db/tests/test_document.py	2012-06-29 16:18:14 +0000
+++ u1db/tests/test_document.py	2012-07-10 17:17:20 +0000
@@ -71,6 +71,15 @@
             errors.InvalidJSON, self.make_document, 'id', 'uid:1',
             'not a json dictionary')
 
+    def test_get_size(self):
+        doc_a = self.make_document('a', 'b', '{"some": "content"}')
+        self.assertEqual(
+            len('a' + 'b' + '{"some": "content"}'), doc_a.get_size())
+
+    def test_get_size_empty_document(self):
+        doc_a = self.make_document('a', 'b', None)
+        self.assertEqual(len('a' + 'b'), doc_a.get_size())
+
 
 class TestPyDocument(tests.TestCase):