[feature] use 409 status code for existing blob id

Raising was generating 500, which is a generic status code for server
side errors. This commit adds proper status code of 409 while handling
the error on client side by translating the code into a proper exception
class.

5 files changed, 36 insertions, 14 deletions

diff --git a/client/src/leap/soledad/client/_blobs.py b/client/src/leap/soledad/client/_blobs.py
index eb09d69f..0d25702c 100644
--- a/client/src/leap/soledad/client/_blobs.py
+++ b/client/src/leap/soledad/client/_blobs.py
@@ -36,6 +36,7 @@ import treq
 
 from leap.soledad.client.sqlcipher import SQLCipherOptions
 from leap.soledad.client import pragmas
+from leap.soledad.common.errors import SoledadError
 
 from _crypto import DocInfo, BlobEncryptor, BlobDecryptor
 from _http import HTTPClient
@@ -44,6 +45,10 @@ from _http import HTTPClient
 logger = Logger()
 
 
+class BlobAlreadyExistsError(SoledadError):
+    pass
+
+
 class ConnectionPool(adbapi.ConnectionPool):
 
     def insertAndGetLastRowid(self, *args, **kwargs):
@@ -90,6 +95,13 @@ class ConnectionPool(adbapi.ConnectionPool):
         return handle
 
 
+def check_http_status(code):
+    if code == 409:
+        raise BlobAlreadyExistsError()
+    elif code != 200:
+        raise SoledadError("Server Error")
+
+
 class DecrypterBuffer(object):
 
     def __init__(self, doc_id, rev, secret, tag):
@@ -217,7 +229,7 @@ class BlobManager(object):
                                 armor=False)
         fd = yield crypter.encrypt()
         response = yield self._client.put(uri, data=fd)
-        assert response.code == 200
+        check_http_status(response.code)
         logger.info("Finished upload: %s" % (blob_id,))
 
     @defer.inlineCallbacks
diff --git a/client/src/leap/soledad/client/_http.py b/client/src/leap/soledad/client/_http.py
index ba6f9a27..2a6b9e39 100644
--- a/client/src/leap/soledad/client/_http.py
+++ b/client/src/leap/soledad/client/_http.py
@@ -67,8 +67,7 @@ class PinnedTokenAgent(Agent):
 
     def request(self, method, uri, headers=None, bodyProducer=None):
         # authenticate the request
-        if not headers:
-            headers = Headers()
+        headers = headers or Headers()
         headers.addRawHeader('Authorization', self._creds)
         # perform the authenticated request
         return Agent.request(
diff --git a/server/src/leap/soledad/server/_blobs.py b/server/src/leap/soledad/server/_blobs.py
index 4317deff..25266b18 100644
--- a/server/src/leap/soledad/server/_blobs.py
+++ b/server/src/leap/soledad/server/_blobs.py
@@ -52,10 +52,6 @@ logger = Logger()
 # [ ] chunking (should we do it on the client or on the server?)
 
 
-class BlobAlreadyExists(Exception):
-    pass
-
-
 class IBlobsBackend(Interface):
 
     """
@@ -125,8 +121,9 @@ class FilesystemBlobsBackend(object):
         except:
             pass
         if os.path.isfile(path):
-            # XXX return some 5xx code
-            raise BlobAlreadyExists()
+            # 409 - Conflict
+            request.setResponseCode(409)
+            return "Blob already exists: %s" % blob_id
         used = self.get_total_storage(user)
         if used > self.quota:
             logger.error("Error 507: Quota exceeded for user: %s" % user)
diff --git a/testing/tests/blobs/test_fs_backend.py b/testing/tests/blobs/test_fs_backend.py
index fd38336d..ce82fce0 100644
--- a/testing/tests/blobs/test_fs_backend.py
+++ b/testing/tests/blobs/test_fs_backend.py
@@ -18,6 +18,7 @@
 Tests for blobs backend on server side.
 """
 from twisted.trial import unittest
+from twisted.web.test.test_web import DummyRequest
 from leap.soledad.server import _blobs
 from io import BytesIO
 from mock import Mock
@@ -45,7 +46,7 @@ class FilesystemBackendTestCase(unittest.TestCase):
         render_mock = Mock()
         file_mock.return_value = render_mock
         backend = _blobs.FilesystemBlobsBackend()
-        request = object()
+        request = DummyRequest([''])
         backend._get_path = Mock(return_value='path')
         backend.read_blob('user', 'blob_id', request)
 
@@ -59,8 +60,10 @@ class FilesystemBackendTestCase(unittest.TestCase):
         isfile.return_value = True
         backend = _blobs.FilesystemBlobsBackend()
         backend._get_path = Mock(return_value='path')
-        with pytest.raises(_blobs.BlobAlreadyExists):
-            backend.write_blob('user', 'blob_id', 'request')
+        request = DummyRequest([''])
+        result = backend.write_blob('user', 'blob_id', request)
+        assert result == "Blob already exists: blob_id"
+        assert request.responseCode == 409
 
     @pytest.mark.usefixtures("method_tmpdir")
     @mock.patch.object(os.path, 'isfile')
diff --git a/testing/tests/server/test_blobs_server.py b/testing/tests/server/test_blobs_server.py
index 2c52ea46..1f35c285 100644
--- a/testing/tests/server/test_blobs_server.py
+++ b/testing/tests/server/test_blobs_server.py
@@ -24,7 +24,7 @@ from twisted.web.server import Site
 from twisted.internet import reactor
 from twisted.internet import defer
 from leap.soledad.server import _blobs as server_blobs
-from leap.soledad.client._blobs import BlobManager
+from leap.soledad.client._blobs import BlobManager, BlobAlreadyExistsError
 
 
 class BlobServerTestCase(unittest.TestCase):
@@ -42,7 +42,7 @@ class BlobServerTestCase(unittest.TestCase):
 
     @defer.inlineCallbacks
     @pytest.mark.usefixtures("method_tmpdir")
-    def test_upload(self):
+    def test_upload_download(self):
         manager = BlobManager('', self.uri, self.secret,
                               self.secret, 'user')
         fd = BytesIO("save me")
@@ -50,3 +50,14 @@ class BlobServerTestCase(unittest.TestCase):
         blob, size = yield manager._download_and_decrypt('blob_id',
                                                          'mydoc', '1')
         assert blob.getvalue() == "save me"
+
+    @defer.inlineCallbacks
+    @pytest.mark.usefixtures("method_tmpdir")
+    def test_upload_deny_duplicates(self):
+        manager = BlobManager('', self.uri, self.secret,
+                              self.secret, 'user')
+        fd = BytesIO("save me")
+        yield manager._encrypt_and_upload('blob_id', 'mydoc', '1', fd)
+        fd = BytesIO("save me")
+        with pytest.raises(BlobAlreadyExistsError):
+            yield manager._encrypt_and_upload('blob_id', 'mydoc', '1', fd)