Merge lp:~pedronis/u1db/http-create-doc into lp:u1db

[Samuele Pedroni]

support create_doc for HTTPDatabase, the case when doc_id is not specified is implemented by locally generating a uuid

add support for transmitting errors back, and raising the corresponding exceptions again

rename InvalidDocRev to RevisionConflict as discussed with Stuart, it is transmitted back as status 409

add a generic HTTPError to deal with general protocol errors, server errors

[John A Meinel (jameinel)]

84 +# mapping wire (transimission) descriptions/tags for errors to the exceptions
85 +wire_description_to_exc = dict((x.wire_description, x)
86 + for x in globals().values()
87 + if isinstance(x, type) and
88 + issubclass(x, U1DBError) and
89 + x.wire_description)

Why not just do:

wire_description_to_exc = dict((x.wire_description, x) for x in globals().values()
                               if getattr(x, 'wire_description', None) is not None)

Seems to do the same thing, but simpler.

review: Approve

[lp:~pedronis/u1db/http-create-doc]

132. By Samuele Pedroni on 2011-11-25

xxx comment

133. By Samuele Pedroni on 2011-11-25

simplify, thanks jam

=== 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:07:25 +0000
@@ -67,10 +67,10 @@
                 raise errors.ConflictedDoc()
             old_rev, old_content = self._docs[doc.doc_id]
             if old_rev != doc.rev:
-                raise errors.InvalidDocRev()
+                raise errors.RevisionConflict()
         else:
             if doc.rev is not None:
-                raise errors.InvalidDocRev()
+                raise errors.RevisionConflict()
         new_rev = self._allocate_doc_rev(doc.rev)
         self._put_and_update_indexes(doc.doc_id, old_content, new_rev,
                                      doc.content)

=== 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:07:25 +0000
@@ -257,10 +257,10 @@
             if old_doc is not None:
                 old_content = old_doc.content
                 if old_doc.rev != doc.rev:
-                    raise errors.InvalidDocRev()
+                    raise errors.RevisionConflict()
             else:
                 if doc.rev is not None:
-                    raise errors.InvalidDocRev()
+                    raise errors.RevisionConflict()
             new_rev = self._allocate_doc_rev(doc.rev)
             self._put_and_update_indexes(doc.doc_id, old_content, new_rev,
                                          doc.content)
@@ -323,7 +323,7 @@
             if old_doc is None:
                 raise KeyError
             if old_doc.rev != doc_rev:
-                raise errors.InvalidDocRev()
+                raise errors.RevisionConflict()
             if old_doc.content is None:
                 raise KeyError
             if self._has_conflicts(doc_id):

=== modified file 'u1db/errors.py'
--- u1db/errors.py	2011-11-23 10:12:18 +0000
+++ u1db/errors.py	2011-11-25 15:07:25 +0000
@@ -18,10 +18,18 @@
 class U1DBError(Exception):
     """Generic base class for U1DB errors."""
 
-
-class InvalidDocRev(U1DBError):
+    # description/tag for identifying the error during transmission (http,...)
+    wire_description = "error"
+
+    def __init__(self, message=None):
+        self.message = message
+
+
+class RevisionConflict(U1DBError):
     """The document revisions supplied does not match the current version."""
 
+    wire_description = "revision conflict"
+
 
 class InvalidDocId(U1DBError):
     """A document was tried with an invalid document identifier."""
@@ -41,3 +49,21 @@
 
 class DatabaseDoesNotExist(U1DBError):
     """The database does not exist."""
+
+
+class HTTPError(U1DBError):
+    """Unspecific HTTP errror."""
+
+    wire_description = None
+
+    def __init__(self, status, message=None):
+        self.status = status
+        self.message = message
+
+
+# mapping wire (transimission) descriptions/tags for errors to the exceptions
+wire_description_to_exc = dict((x.wire_description, x)
+                                   for x in globals().values()
+                                   if isinstance(x, type) and
+                                      issubclass(x, U1DBError) and
+                                      x.wire_description)

=== modified file 'u1db/remote/http_app.py'
--- u1db/remote/http_app.py	2011-11-24 13:38:48 +0000
+++ u1db/remote/http_app.py	2011-11-25 15:07:25 +0000
@@ -26,6 +26,7 @@
 from u1db import (
     __version__ as _u1db_version,
     Document,
+    errors,
     )
 
 
@@ -170,6 +171,7 @@
     def get(self):
         self.responder.send_response(version=_u1db_version)
 
+
 @url_to_resource.register
 class DocResource(object):
     """Document resource."""
@@ -377,10 +379,12 @@
         try:
             resource = self._lookup_resource(environ, responder)
             HTTPInvocationByMethodWithBody(resource, environ)()
+        except (errors.RevisionConflict,), e:
+            responder.send_response(409, error=e.wire_description)
         except BadRequest:
             # xxx introduce logging
             #print environ['PATH_INFO']
             #import traceback
             #traceback.print_exc()
-            responder.send_response(400)
+            responder.send_response(400, error="bad request")
         return responder.content

=== modified file 'u1db/remote/http_client.py'
--- u1db/remote/http_client.py	2011-11-21 14:24:59 +0000
+++ u1db/remote/http_client.py	2011-11-25 15:07:25 +0000
@@ -19,6 +19,10 @@
 import urlparse
 import urllib
 
+from u1db import (
+    errors,
+    )
+
 
 class HTTPClientBase(object):
     """Base class to make requests to a remote HTTP server."""
@@ -44,9 +48,13 @@
         resp = self._conn.getresponse()
         if resp.status in (200, 201):
             return resp.read(), dict(resp.getheaders())
-        else:
-            # xxx raise the proper exceptions depending on status
-            raise Exception(resp.status)
+        elif resp.status in (409,):
+            respdic = simplejson.loads(resp.read())
+            exc_cls = errors.wire_description_to_exc.get(respdic.get("error"))
+            if exc_cls is not None:
+                message = respdic.get("message")
+                raise exc_cls(message)
+        raise errors.HTTPError(resp.status, resp.read())
 
     def _request(self, method, url_parts, params=None, body=None,
                                                        content_type=None):

=== modified file 'u1db/remote/http_database.py'
--- u1db/remote/http_database.py	2011-11-18 14:10:09 +0000
+++ u1db/remote/http_database.py	2011-11-25 15:07:25 +0000
@@ -15,7 +15,7 @@
 """HTTPDabase to access a remote db over the HTTP API."""
 
 import simplejson
-
+import uuid
 
 from u1db import (
     Database,
@@ -38,7 +38,6 @@
         doc.rev = res['rev']
         return res['rev']
 
-
     def get_doc(self, doc_id):
         res, headers = self._request('GET', ['doc', doc_id])
         doc_rev = headers['x-u1db-rev']
@@ -46,3 +45,11 @@
         doc = Document(doc_id, doc_rev, res)
         doc.has_conflicts = has_conflicts
         return doc
+
+    def create_doc(self, content, doc_id=None):
+        if doc_id is None:
+            doc_id = str(uuid.uuid4())
+        res, headers = self._request_json('PUT', ['doc', doc_id], {},
+                                          content, 'application/json')
+        new_doc = Document(doc_id, res['rev'], content)
+        return new_doc

=== 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:07:25 +0000
@@ -47,17 +47,6 @@
                   'server_def': http_server_def}),
         ]
 
-    def test_put_doc_creating_initial(self):
-        doc = Document('my_doc_id', None, simple_doc)
-        new_rev = self.db.put_doc(doc)
-        self.assertGetDoc(self.db, 'my_doc_id', new_rev, simple_doc, False)
-
-
-class LocalDatabaseTests(tests.DatabaseBaseTests):
-
-    def test_close(self):
-        self.db.close()
-
     def test_create_doc_allocating_doc_id(self):
         doc = self.db.create_doc(simple_doc)
         self.assertNotEqual(None, doc.doc_id)
@@ -73,17 +62,28 @@
     def test_create_doc_existing_id(self):
         doc = self.db.create_doc(simple_doc)
         new_content = '{"something": "else"}'
-        self.assertRaises(errors.InvalidDocRev, self.db.create_doc,
+        self.assertRaises(errors.RevisionConflict, self.db.create_doc,
                           new_content, doc.doc_id)
         self.assertGetDoc(self.db, doc.doc_id, doc.rev, simple_doc, False)
 
+    def test_put_doc_creating_initial(self):
+        doc = Document('my_doc_id', None, simple_doc)
+        new_rev = self.db.put_doc(doc)
+        self.assertGetDoc(self.db, 'my_doc_id', new_rev, simple_doc, False)
+
+
+class LocalDatabaseTests(tests.DatabaseBaseTests):
+
+    def test_close(self):
+        self.db.close()
+
     def test_put_doc_refuses_no_id(self):
         doc = Document(None, None, simple_doc)
         self.assertRaises(errors.InvalidDocId, self.db.put_doc, doc)
 
     def test_put_doc_refuses_non_existing_old_rev(self):
         doc = Document('doc-id', 'test:4', simple_doc)
-        self.assertRaises(errors.InvalidDocRev, self.db.put_doc, doc)
+        self.assertRaises(errors.RevisionConflict, self.db.put_doc, doc)
 
     def test_get_docs(self):
         doc1 = self.db.create_doc(simple_doc)
@@ -176,7 +176,7 @@
         old_rev = doc.rev
         doc.rev = 'other:1'
         doc.content = '{"something": "else"}'
-        self.assertRaises(errors.InvalidDocRev, self.db.put_doc, doc)
+        self.assertRaises(errors.RevisionConflict, self.db.put_doc, doc)
         self.assertGetDoc(self.db, 'my_doc_id', old_rev, simple_doc, False)
 
     def test_delete_doc(self):
@@ -199,7 +199,7 @@
     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.assertRaises(errors.RevisionConflict,
             self.db.delete_doc, doc.doc_id, 'other:1')
         self.assertGetDoc(self.db, doc.doc_id, doc.rev, simple_doc, False)
 

=== added file 'u1db/tests/test_errors.py'
--- u1db/tests/test_errors.py	1970-01-01 00:00:00 +0000
+++ u1db/tests/test_errors.py	2011-11-25 15:07:25 +0000
@@ -0,0 +1,45 @@
+# Copyright 2011 Canonical Ltd.
+#
+# This program is free software: you can redistribute it and/or modify it
+# under the terms of the GNU General Public License version 3, as published
+# by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranties of
+# MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+# PURPOSE.  See the GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License along
+# with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Tests error infrastructure."""
+
+from u1db import (
+    errors,
+    tests,
+    )
+
+
+class TestError(tests.TestCase):
+
+    def test_error_base(self):
+        err = errors.U1DBError()
+        self.assertEqual("error", err.wire_description)
+        self.assertIs(None, err.message)
+
+        err = errors.U1DBError("Message.")
+        self.assertEqual("error", err.wire_description)
+        self.assertEqual("Message.", err.message)
+
+
+    def test_HTTPError(self):
+        err = errors.HTTPError(500)
+        self.assertEqual(500, err.status)
+        self.assertIs(None, err.wire_description)
+        self.assertIs(None, err.message)
+
+        err = errors.HTTPError(500, "Crash.")
+        self.assertEqual(500, err.status)
+        self.assertIs(None, err.wire_description)
+        self.assertEqual("Crash.", err.message)
+

=== modified file 'u1db/tests/test_http_app.py'
--- u1db/tests/test_http_app.py	2011-11-21 14:24:59 +0000
+++ u1db/tests/test_http_app.py	2011-11-25 15:07:25 +0000
@@ -20,6 +20,7 @@
 
 from u1db import (
     __version__ as _u1db_version,
+    errors,
     tests,
     )
 
@@ -494,3 +495,32 @@
         self.assertEqual({'doc': '{"value": "there"}',
                           'rev': doc.rev, 'id': doc.doc_id},
                          simplejson.loads(parts[1]))
+
+
+class TestHTTPAppErrorHandling(tests.TestCase):
+
+    def setUp(self):
+        super(TestHTTPAppErrorHandling, self).setUp()
+        self.exc = None
+        self.state = tests.ServerStateForTests()
+        class ErroringResource(object):
+
+            def post(_, args, content):
+                raise self.exc
+
+        def lookup_resource(environ, responder):
+            return ErroringResource()
+
+        application = http_app.HTTPApp(self.state)
+        application._lookup_resource = lookup_resource
+        self.app = paste.fixture.TestApp(application)
+
+    def test_RevisionConflict(self):
+        self.exc = errors.RevisionConflict()
+        resp = self.app.post('/req', params='{}',
+                             headers={'content-type': 'application/json'},
+                             expect_errors=True)
+        self.assertEqual(409, resp.status)
+        self.assertEqual('application/json', resp.header('content-type'))
+        self.assertEqual({"error": "revision conflict"},
+                         simplejson.loads(resp.body))

=== modified file 'u1db/tests/test_http_client.py'
--- u1db/tests/test_http_client.py	2011-11-23 09:34:59 +0000
+++ u1db/tests/test_http_client.py	2011-11-25 15:07:25 +0000
@@ -18,10 +18,11 @@
 from wsgiref import simple_server
 
 from u1db import (
+    errors,
     tests,
     )
 from u1db.remote import (
-    http_client
+    http_client,
     )
 
 
@@ -38,6 +39,16 @@
                 content_length = int(environ['CONTENT_LENGTH'])
                 ret['body'] = environ['wsgi.input'].read(content_length)
             return [simplejson.dumps(ret)]
+        elif environ['PATH_INFO'].endswith('error'):
+            content_length = int(environ['CONTENT_LENGTH'])
+            error = simplejson.loads(environ['wsgi.input'].read(content_length))
+            start_response(error['status'],
+                           [('Content-Type', 'application/json')])
+            response = error['response']
+            if isinstance(response, basestring):
+                return [response]
+            else:
+                return [simplejson.dumps(error['response'])]
 
     def server_def(self):
         def make_server(host_port, handler, state):
@@ -115,3 +126,26 @@
                           'QUERY_STRING': 'b=2',
                           'body': '{"a": "x"}',
                           'REQUEST_METHOD': 'POST'}, res)
+
+    def test_unspecified_http_error(self):
+        cli = self.getClient()
+        self.assertRaises(errors.HTTPError,
+                          cli._request_json, 'POST', ['error'], {},
+                          {'status': "500 Internal Error",
+                           'response': "Crash."})
+        try:
+            cli._request_json('POST', ['error'], {},
+                              {'status': "500 Internal Error",
+                               'response': "Fail."})
+        except errors.HTTPError, e:
+            pass
+
+        self.assertEqual(500, e.status)
+        self.assertEqual("Fail.", e.message)
+
+    def test_revision_conflict(self):
+        cli = self.getClient()
+        self.assertRaises(errors.RevisionConflict,
+                          cli._request_json, 'POST', ['error'], {},
+                          {'status': "409 Conflict",
+                           'response': {"error": "revision conflict"}})

=== modified file 'u1db/tests/test_http_database.py'
--- u1db/tests/test_http_database.py	2011-11-23 09:34:59 +0000
+++ u1db/tests/test_http_database.py	2011-11-25 15:07:25 +0000
@@ -78,3 +78,20 @@
         self.assertGetDoc(self.db, 'doc-id', 'doc-rev', '{"v": 2}', False)
         self.assertEqual(('GET', ['doc', 'doc-id'], None, None, None),
                          self.got)
+
+    def test_create_doc_with_id(self):
+        self.response_val = {'rev': 'doc-rev'}, {}
+        new_doc = self.db.create_doc('{"v": 1}', doc_id='doc-id')
+        self.assertEqual('doc-rev', new_doc.rev)
+        self.assertEqual('doc-id', new_doc.doc_id)
+        self.assertEqual('{"v": 1}', new_doc.content)
+        self.assertEqual(('PUT', ['doc', 'doc-id'], {},
+                          '{"v": 1}', 'application/json'), self.got)
+
+    def test_create_doc_without_id(self):
+        self.response_val = {'rev': 'doc-rev-2'}, {}
+        new_doc = self.db.create_doc('{"v": 3}')
+        self.assertEqual('doc-rev-2', new_doc.rev)
+        self.assertEqual('{"v": 3}', new_doc.content)
+        self.assertEqual(('PUT', ['doc', new_doc.doc_id], {},
+                          '{"v": 3}', 'application/json'), self.got)