account: improve exception handling

oioswift/proxy/controllers/account.py

@@ -31,7 +31,7 @@
 
 from oio.common import exceptions
 
-from oioswift.utils import handle_service_busy
+from oioswift.utils import handle_oio_timeout, handle_service_busy
 
 
 def get_response_headers(info):
@@ -105,8 +105,29 @@ def account_listing_response(account, req, response_content_type,
     return ret
 
 
+def handle_account_not_found_autocreate(fnc):
+    """
+    Catch NoSuchAccount and NotFound errors.
+    If account_autocreate is enabled, return a dummy listing.
+    Otherwise, return a proper '404 Not Found' response.
+    """
+    def _account_not_found_wrapper(self, req, *args, **kwargs):
+        try:
+            resp = fnc(self, req, *args, **kwargs)
+        except (exceptions.NotFound, exceptions.NoSuchAccount):
+            if self.app.account_autocreate:
+                resp = account_listing_response(self.account_name, req,
+                                                get_listing_content_type(req))
+            else:
+                resp = HTTPNotFound(request=req)
+        return resp
+    return _account_not_found_wrapper
+
+
 class AccountController(SwiftAccountController):
     @public
+    @handle_account_not_found_autocreate
+    @handle_oio_timeout
     @handle_service_busy
     def GET(self, req):
         """Handler for HTTP GET requests."""
@@ -145,35 +166,29 @@ def get_account_listing_resp(self, req):
         end_marker = get_param(req, 'end_marker')
 
         oio_headers = {'X-oio-req-id': self.trans_id}
-        try:
-            info = None
-            if hasattr(self.app.storage, 'account'):
-                # Call directly AccountClient.container_list()
-                # because storage.container_list() does not return
-                # account metadata
-                info = self.app.storage.account.container_list(
-                    self.account_name, limit=limit, marker=marker,
-                    end_marker=end_marker, prefix=prefix,
-                    delimiter=delimiter, headers=oio_headers)
-                listing = info.pop('listing')
-            else:
-                # Legacy call to account service
-                listing, info = self.app.storage.container_list(
-                    self.account_name, limit=limit, marker=marker,
-                    end_marker=end_marker, prefix=prefix,
-                    delimiter=delimiter, headers=oio_headers)
-            resp = account_listing_response(
-                self.account_name, req, get_listing_content_type(req),
-                info=info, listing=listing)
-        except (exceptions.NotFound, exceptions.NoSuchAccount):
-            if self.app.account_autocreate:
-                resp = account_listing_response(self.account_name, req,
-                                                get_listing_content_type(req))
-            else:
-                resp = HTTPNotFound(request=req)
-        return resp
+        info = None
+        if hasattr(self.app.storage, 'account'):
+            # Call directly AccountClient.container_list()
+            # because storage.container_list() does not return
+            # account metadata
+            info = self.app.storage.account.container_list(
+                self.account_name, limit=limit, marker=marker,
+                end_marker=end_marker, prefix=prefix,
+                delimiter=delimiter, headers=oio_headers)
+            listing = info.pop('listing')
+        else:
+            # Legacy call to account service
+            listing, info = self.app.storage.container_list(
+                self.account_name, limit=limit, marker=marker,
+                end_marker=end_marker, prefix=prefix,
+                delimiter=delimiter, headers=oio_headers)
+        return account_listing_response(
+            self.account_name, req, get_listing_content_type(req),
+            info=info, listing=listing)
 
     @public
+    @handle_account_not_found_autocreate
+    @handle_oio_timeout
     @handle_service_busy
     def HEAD(self, req):
         """HTTP HEAD request handler."""
@@ -197,22 +212,14 @@ def HEAD(self, req):
 
     def get_account_head_resp(self, req):
         oio_headers = {'X-oio-req-id': self.trans_id}
-        try:
-            info = self.app.storage.account_show(
-                self.account_name, headers=oio_headers)
-            resp = account_listing_response(self.account_name, req,
-                                            get_listing_content_type(req),
-                                            info=info)
-        except (exceptions.NotFound, exceptions.NoSuchAccount):
-            if self.app.account_autocreate:
-                resp = account_listing_response(self.account_name, req,
-                                                get_listing_content_type(req))
-            else:
-                resp = HTTPNotFound(request=req)
-
-        return resp
+        info = self.app.storage.account_show(
+            self.account_name, headers=oio_headers)
+        return account_listing_response(self.account_name, req,
+                                        get_listing_content_type(req),
+                                        info=info)
 
     @public
+    @handle_oio_timeout
     @handle_service_busy
     def PUT(self, req):
         """HTTP PUT request handler."""
@@ -256,6 +263,7 @@ def get_account_put_resp(self, req, headers):
         return resp
 
     @public
+    @handle_oio_timeout
     @handle_service_busy
     def POST(self, req):
         """HTTP POST request handler."""
@@ -298,6 +306,7 @@ def get_account_post_resp(self, req, headers):
         return resp
 
     @public
+    @handle_oio_timeout
     @handle_service_busy
     def DELETE(self, req):
         """HTTP DELETE request handler."""

tests/unit/controllers/test_account.py

@@ -7,6 +7,7 @@
 from swift.proxy.controllers.base import headers_to_account_info
 from oioswift.common.ring import FakeRing
 from oioswift import server as proxy_server
+from oio.common import exceptions as oioexc
 from tests.unit import FakeStorageAPI, FakeMemcache, debug_logger
 
 
@@ -45,6 +46,41 @@ def test_account_info(self):
             headers_to_account_info(resp.headers, resp.status_int),
             resp.environ['swift.infocache']['account/AUTH_openio'])
 
+    def test_account_info_not_found(self):
+        req = Request.blank(
+            '/v1/AUTH_openio', {'PATH_INFO': '/v1/AUTH_openio'}, method='HEAD')
+
+        self.storage.account_show = Mock(side_effect=oioexc.NoSuchAccount)
+        resp = req.get_response(self.app)
+        self.assertEqual(404, resp.status_int)
+
+    def test_account_info_not_found_autocreate(self):
+        req = Request.blank(
+            '/v1/AUTH_openio', {'PATH_INFO': '/v1/AUTH_openio'}, method='HEAD')
+
+        self.storage.account_show = Mock(side_effect=oioexc.NoSuchAccount)
+        self.app.account_autocreate = True
+        resp = req.get_response(self.app)
+        self.assertEqual(2, resp.status_int // 100)
+        # We got a dummy response, it should not be cached
+        self.assertNotIn('swift.infocache', resp.environ)
+
+    def test_account_info_service_busy(self):
+        req = Request.blank(
+            '/v1/AUTH_openio', {'PATH_INFO': '/v1/AUTH_openio'}, method='HEAD')
+
+        self.storage.account_show = Mock(side_effect=oioexc.ServiceBusy)
+        resp = req.get_response(self.app)
+        self.assertEqual(503, resp.status_int)
+
+    def test_account_info_timeout(self):
+        req = Request.blank(
+            '/v1/AUTH_openio', {'PATH_INFO': '/v1/AUTH_openio'}, method='HEAD')
+
+        self.storage.account_show = Mock(side_effect=oioexc.OioTimeout)
+        resp = req.get_response(self.app)
+        self.assertEqual(503, resp.status_int)
+
     def test_swift_owner(self):
         owner_headers = {
             'x-account-meta-temp-url-key': 'value',