request: move status code check into swift3.Request

No longer needs to check the returned status code from Swift in the
controllers.

Change-Id: I5c6f6a0b594ea59a8678f42e00e976c8487f286a

swift3/exception.py

@@ -20,3 +20,7 @@ class S3Exception(Exception):
 
 class NotS3Request(S3Exception):
     pass
+
+
+class BadSwiftRequest(S3Exception):
+    pass

swift3/middleware.py

@@ -52,26 +52,19 @@ following for an SAIO setup::
         calling_format=boto.s3.connection.OrdinaryCallingFormat())
 """
 
-from urllib import quote
 from simplejson import loads
 import re
 
 from swift.common.utils import get_logger
-from swift.common.http import HTTP_OK, HTTP_CREATED, HTTP_ACCEPTED, \
+from swift.common.http import HTTP_OK
-    HTTP_NO_CONTENT, HTTP_UNAUTHORIZED, HTTP_FORBIDDEN, HTTP_NOT_FOUND, \
-    HTTP_CONFLICT, HTTP_UNPROCESSABLE_ENTITY, is_success, \
-    HTTP_REQUEST_ENTITY_TOO_LARGE
 from swift.common.middleware.acl import parse_acl, referrer_allowed
 
 from swift3.etree import fromstring, tostring, Element, SubElement
 from swift3.exception import NotS3Request
 from swift3.request import Request
-from swift3.response import HTTPNoContent, HTTPOk, ErrorResponse, \
+from swift3.response import HTTPOk, ErrorResponse, AccessDenied, \
-    AccessDenied, BucketAlreadyExists, BucketNotEmpty, EntityTooLarge, \
+    InternalError, InvalidArgument, MalformedACLError, MethodNotAllowed, \
-    InternalError, InvalidArgument, InvalidDigest, MalformedACLError, \
+    NoSuchKey, S3NotImplemented
-    MethodNotAllowed, NoSuchBucket, NoSuchKey, S3NotImplemented, \
-    SignatureDoesNotMatch
-
 
 XMLNS_XSI = 'http://www.w3.org/2001/XMLSchema-instance'
 
@@ -227,17 +220,7 @@ class ServiceController(Controller):
         """
         Handle GET Service request
         """
-        req.query_string = 'format=json'
+        resp = req.get_response(self.app, query={'format': 'json'})
-        resp = req.get_response(self.app)
-        status = resp.status_int
-
-        if status != HTTP_OK:
-            if status == HTTP_UNAUTHORIZED:
-                raise SignatureDoesNotMatch()
-            if status == HTTP_FORBIDDEN:
-                raise AccessDenied()
-            else:
-                raise InternalError()
 
         containers = loads(resp.body)
         # we don't keep the creation time of a backet (s3cmd doesn't
@@ -263,14 +246,7 @@ class BucketController(Controller):
         """
         Handle HEAD Bucket (Get Metadata) request
         """
-        if req.query_string:
+        return req.get_response(self.app)
-                req.query_string = ''
-
-        resp = req.get_response(self.app)
-        if resp.status_int == HTTP_NO_CONTENT:
-                resp.status_int = HTTP_OK
-
-        return resp
 
     def GET(self, req):
         """
@@ -283,26 +259,18 @@ class BucketController(Controller):
         max_keys = min(int(req.params.get('max-keys', MAX_BUCKET_LISTING)),
                        MAX_BUCKET_LISTING)
 
-        req.query_string = 'format=json&limit=%s' % (max_keys + 1)
+        query = {
+            'format': 'json',
+            'limit': max_keys + 1,
+        }
         if 'marker' in req.params:
-            req.query_string += '&marker=%s' % quote(req.params['marker'])
+            query.update({'marker': req.params['marker']})
         if 'prefix' in req.params:
-            req.query_string += '&prefix=%s' % quote(req.params['prefix'])
+            query.update({'prefix': req.params['prefix']})
         if 'delimiter' in req.params:
-            req.query_string += '&delimiter=%s' % \
+            query.update({'delimiter': req.params['delimiter']})
-                quote(req.params['delimiter'])
-        resp = req.get_response(self.app)
-        status = resp.status_int
 
-        if status != HTTP_OK:
+        resp = req.get_response(self.app, query=query)
-            if status == HTTP_UNAUTHORIZED:
-                raise SignatureDoesNotMatch()
-            if status == HTTP_FORBIDDEN:
-                raise AccessDenied()
-            elif status == HTTP_NOT_FOUND:
-                raise NoSuchBucket(req.container_name)
-            else:
-                raise InternalError()
 
         objects = loads(resp.body)
 
@@ -360,40 +328,16 @@ class BucketController(Controller):
                 req.headers[header] = acl
 
         resp = req.get_response(self.app)
-        status = resp.status_int
+        resp.status = HTTP_OK
+        resp.headers.update({'Location': req.container_name})
 
-        if status != HTTP_CREATED and status != HTTP_NO_CONTENT:
+        return resp
-            if status == HTTP_UNAUTHORIZED:
-                raise SignatureDoesNotMatch()
-            if status == HTTP_FORBIDDEN:
-                raise AccessDenied()
-            elif status == HTTP_ACCEPTED:
-                raise BucketAlreadyExists(req.container_name)
-            else:
-                raise InternalError()
-
-        return HTTPOk(headers={'Location': req.container_name})
 
     def DELETE(self, req):
         """
         Handle DELETE Bucket request
         """
-        resp = req.get_response(self.app)
+        return req.get_response(self.app)
-        status = resp.status_int
-
-        if status != HTTP_NO_CONTENT:
-            if status == HTTP_UNAUTHORIZED:
-                raise SignatureDoesNotMatch()
-            if status == HTTP_FORBIDDEN:
-                raise AccessDenied()
-            elif status == HTTP_NOT_FOUND:
-                raise NoSuchBucket(req.container_name)
-            elif status == HTTP_CONFLICT:
-                raise BucketNotEmpty()
-            else:
-                raise InternalError()
-
-        return HTTPNoContent()
 
     def POST(self, req):
         """
@@ -408,21 +352,10 @@ class ObjectController(Controller):
     """
     def GETorHEAD(self, req):
         resp = req.get_response(self.app)
-        status = resp.status_int
-
         if req.method == 'HEAD':
             resp.app_iter = None
 
-        if is_success(status):
         return resp
-        elif status == HTTP_UNAUTHORIZED:
-            raise SignatureDoesNotMatch()
-        elif status == HTTP_FORBIDDEN:
-            raise AccessDenied()
-        elif status == HTTP_NOT_FOUND:
-            raise NoSuchKey(req.object_name)
-        else:
-            raise InternalError()
 
     def HEAD(self, req):
         """
@@ -441,21 +374,6 @@ class ObjectController(Controller):
         Handle PUT Object and PUT Object (Copy) request
         """
         resp = req.get_response(self.app)
-        status = resp.status_int
-
-        if status != HTTP_CREATED:
-            if status == HTTP_UNAUTHORIZED:
-                raise SignatureDoesNotMatch()
-            elif status == HTTP_FORBIDDEN:
-                raise AccessDenied()
-            elif status == HTTP_NOT_FOUND:
-                raise NoSuchBucket(req.container_name)
-            elif status == HTTP_UNPROCESSABLE_ENTITY:
-                raise InvalidDigest()
-            elif status == HTTP_REQUEST_ENTITY_TOO_LARGE:
-                raise EntityTooLarge()
-            else:
-                raise InternalError()
 
         if 'HTTP_X_COPY_FROM' in req.environ:
             elem = Element('CopyObjectResult')
@@ -463,7 +381,9 @@ class ObjectController(Controller):
             body = tostring(elem, use_s3ns=False)
             return HTTPOk(body=body)
 
-        return HTTPOk(etag=resp.etag)
+        resp.status = HTTP_OK
+
+        return resp
 
     def POST(self, req):
         raise AccessDenied()
@@ -472,24 +392,7 @@ class ObjectController(Controller):
         """
         Handle DELETE Object request
         """
-        try:
+        return req.get_response(self.app)
-            resp = req.get_response(self.app)
-        except Exception:
-            raise InternalError()
-
-        status = resp.status_int
-
-        if status != HTTP_NO_CONTENT:
-            if status == HTTP_UNAUTHORIZED:
-                raise SignatureDoesNotMatch()
-            elif status == HTTP_FORBIDDEN:
-                raise AccessDenied()
-            elif status == HTTP_NOT_FOUND:
-                raise NoSuchKey(req.object_name)
-            else:
-                raise InternalError()
-
-        return HTTPNoContent()
 
 
 class AclController(Controller):
@@ -507,48 +410,9 @@ class AclController(Controller):
         """
         Handles GET Bucket acl and GET Object acl.
         """
-        if req.object_name:
+        resp = req.get_response(self.app, method='HEAD')
-            # Handle Object ACL
 
-            # ACL requests need to make a HEAD call rather than GET
+        return get_acl(req.access_key, resp.headers)
-            req.method = 'HEAD'
-            req.script_name = ''
-            req.query_string = ''
-
-            resp = req.get_response(self.app)
-            status = resp.status_int
-            headers = resp.headers
-
-            if is_success(status):
-                # Method must be GET or the body wont be returned to the caller
-                req.environ['REQUEST_METHOD'] = 'GET'
-                return get_acl(req.access_key, headers)
-            elif status == HTTP_UNAUTHORIZED:
-                raise SignatureDoesNotMatch()
-            elif status == HTTP_FORBIDDEN:
-                raise AccessDenied()
-            elif status == HTTP_NOT_FOUND:
-                raise NoSuchKey(req.object_name)
-            else:
-                raise InternalError()
-
-        else:
-            # Handle Bucket ACL
-            resp = req.get_response(self.app)
-            status = resp.status_int
-            headers = resp.headers
-
-            if is_success(status):
-                return get_acl(req.access_key, headers)
-
-            if status == HTTP_UNAUTHORIZED:
-                raise SignatureDoesNotMatch()
-            elif status == HTTP_FORBIDDEN:
-                raise AccessDenied()
-            elif status == HTTP_NOT_FOUND:
-                raise NoSuchBucket(req.container_name)
-            else:
-                raise InternalError()
 
     def PUT(self, req):
         """
@@ -568,20 +432,12 @@ class AclController(Controller):
                 raise MalformedACLError()
             for header, acl in translated_acl:
                 req.headers[header] = acl
-            req.method = 'POST'
 
             resp = req.get_response(self.app)
-            status = resp.status_int
+            resp.status = HTTP_OK
+            resp.headers.update({'Location': req.container_name})
 
-            if status != HTTP_ACCEPTED:
+            return resp
-                if status == HTTP_UNAUTHORIZED:
-                    raise SignatureDoesNotMatch()
-                elif status == HTTP_FORBIDDEN:
-                    raise AccessDenied()
-                else:
-                    raise InternalError()
-
-            return HTTPOk(headers={'Location': req.container_name})
 
 
 class LocationController(Controller):
@@ -593,18 +449,7 @@ class LocationController(Controller):
         """
         Handles GET Bucket location.
         """
-        resp = req.get_response(self.app)
+        req.get_response(self.app, method='HEAD')
-        status = resp.status_int
-
-        if status != HTTP_OK:
-            if status == HTTP_UNAUTHORIZED:
-                raise SignatureDoesNotMatch()
-            elif status == HTTP_FORBIDDEN:
-                raise AccessDenied()
-            elif status == HTTP_NOT_FOUND:
-                raise NoSuchBucket(req.container_name)
-            else:
-                raise InternalError()
 
         elem = Element('LocationConstraint')
         if self.conf['location'] != 'US':
@@ -627,18 +472,7 @@ class LoggingStatusController(Controller):
         """
         Handles GET Bucket logging.
         """
-        resp = req.get_response(self.app)
+        req.get_response(self.app, method='HEAD')
-        status = resp.status_int
-
-        if status != HTTP_OK:
-            if status == HTTP_UNAUTHORIZED:
-                raise SignatureDoesNotMatch()
-            elif status == HTTP_FORBIDDEN:
-                raise AccessDenied()
-            elif status == HTTP_NOT_FOUND:
-                raise NoSuchBucket(req.container_name)
-            else:
-                raise InternalError()
 
         # logging disabled
         elem = Element('BucketLoggingStatus')
@@ -679,15 +513,10 @@ class MultiObjectDeleteController(Controller):
                 # TODO: delete the specific version of the object
                 raise S3NotImplemented()
 
-            sub_req = Request(req.environ.copy())
+            req.object_name = key
-            sub_req.query_string = ''
-            sub_req.content_length = 0
-            sub_req.method = 'DELETE'
-            sub_req.object_name = key
 
-            controller = ObjectController(self.app, self.conf)
             try:
-                controller.DELETE(sub_req)
+                req.get_response(self.app, method='DELETE')
             except NoSuchKey:
                 pass
             except ErrorResponse as e:
@@ -791,18 +620,7 @@ class VersioningController(Controller):
         """
         Handles GET Bucket versioning.
         """
-        resp = req.get_response(self.app)
+        req.get_response(self.app, method='HEAD')
-        status = resp.status_int
-
-        if status != HTTP_OK:
-            if status == HTTP_UNAUTHORIZED:
-                raise SignatureDoesNotMatch()
-            elif status == HTTP_FORBIDDEN:
-                raise AccessDenied()
-            elif status == HTTP_NOT_FOUND:
-                raise NoSuchBucket(req.container_name)
-            else:
-                raise InternalError()
 
         # Just report there is no versioning configured here.
         elem = Element('VersioningConfiguration')

swift3/request.py

@@ -19,10 +19,20 @@ import email.utils
 import datetime
 
 from swift.common import swob
+from swift.common.http import HTTP_OK, HTTP_CREATED, HTTP_ACCEPTED, \
+    HTTP_NO_CONTENT, HTTP_UNAUTHORIZED, HTTP_FORBIDDEN, HTTP_NOT_FOUND, \
+    HTTP_CONFLICT, HTTP_UNPROCESSABLE_ENTITY, HTTP_REQUEST_ENTITY_TOO_LARGE, \
+    HTTP_PARTIAL_CONTENT, HTTP_NOT_MODIFIED, HTTP_PRECONDITION_FAILED, \
+    HTTP_REQUESTED_RANGE_NOT_SATISFIABLE, HTTP_LENGTH_REQUIRED, \
+    HTTP_BAD_REQUEST, HTTP_SERVICE_UNAVAILABLE
 
 from swift3.response import AccessDenied, InvalidArgument, InvalidDigest, \
-    RequestTimeTooSkewed, Response, SignatureDoesNotMatch
+    RequestTimeTooSkewed, Response, SignatureDoesNotMatch, \
-from swift3.exception import NotS3Request
+    ServiceUnavailable, BucketAlreadyExists, BucketNotEmpty, EntityTooLarge, \
+    InternalError, NoSuchBucket, NoSuchKey, PreconditionFailed, InvalidRange, \
+    MissingContentLength
+
+from swift3.exception import NotS3Request, BadSwiftRequest
 
 # List of sub-resources that must be maintained as part of the HMAC
 # signature string.
@@ -192,7 +202,7 @@ class Request(swob.Request):
 
         return ServiceController
 
-    def to_swift_req(self):
+    def to_swift_req(self, method, query=None):
         """
         Create a Swift request based on this request's environment.
         """
@@ -208,6 +218,8 @@ class Request(swob.Request):
                 del env[key]
 
         env['swift.source'] = 'S3'
+        if method is not None:
+            env['REQUEST_METHOD'] = method
         env['HTTP_X_AUTH_TOKEN'] = self.token
 
         if self.object_name:
@@ -219,16 +231,160 @@ class Request(swob.Request):
             path = '/v1/%s' % (self.access_key)
         env['PATH_INFO'] = path
 
-        env['QUERY_STRING'] = self.query_string
+        query_string = ''
+        if query is not None:
+            params = []
+            for key, value in sorted(query.items()):
+                if value is not None:
+                    params.append('%s=%s' % (key, quote(str(value))))
+                else:
+                    params.append(key)
+            query_string = '&'.join(params)
+        env['QUERY_STRING'] = query_string
 
         return swob.Request.blank(quote(path), environ=env)
 
-    def get_response(self, app):
+    def _swift_success_codes(self, method):
+        """
+        Returns a list of expected success codes from Swift.
+        """
+        if self.container_name is None:
+            # Swift account access.
+            code_map = {
+                'GET': [
+                    HTTP_OK,
+                ],
+            }
+        elif self.object_name is None:
+            # Swift container access.
+            code_map = {
+                'HEAD': [
+                    HTTP_NO_CONTENT,
+                ],
+                'GET': [
+                    HTTP_OK,
+                    HTTP_NO_CONTENT,
+                ],
+                'PUT': [
+                    HTTP_CREATED,
+                ],
+                'POST': [
+                    HTTP_NO_CONTENT,
+                ],
+                'DELETE': [
+                    HTTP_NO_CONTENT,
+                ],
+            }
+        else:
+            # Swift object access.
+            code_map = {
+                'HEAD': [
+                    HTTP_OK,
+                    HTTP_PARTIAL_CONTENT,
+                    HTTP_NOT_MODIFIED,
+                ],
+                'GET': [
+                    HTTP_OK,
+                    HTTP_PARTIAL_CONTENT,
+                    HTTP_NOT_MODIFIED,
+                ],
+                'PUT': [
+                    HTTP_CREATED,
+                ],
+                'DELETE': [
+                    HTTP_NO_CONTENT,
+                ],
+            }
+
+        return code_map[method]
+
+    def _swift_error_codes(self, method):
+        """
+        Returns a dict from expected Swift error codes to the corresponding S3
+        error responses.
+        """
+        if self.container_name is None:
+            # Swift account access.
+            code_map = {
+                'GET': {
+                },
+            }
+        elif self.object_name is None:
+            # Swift container access.
+            code_map = {
+                'HEAD': {
+                    HTTP_NOT_FOUND: (NoSuchBucket, self.container_name),
+                },
+                'GET': {
+                    HTTP_NOT_FOUND: (NoSuchBucket, self.container_name),
+                },
+                'PUT': {
+                    HTTP_ACCEPTED: (BucketAlreadyExists, self.container_name),
+                },
+                'POST': {
+                    HTTP_NOT_FOUND: (NoSuchBucket, self.container_name),
+                },
+                'DELETE': {
+                    HTTP_NOT_FOUND: (NoSuchBucket, self.container_name),
+                    HTTP_CONFLICT: BucketNotEmpty,
+                },
+            }
+        else:
+            # Swift object access.
+            code_map = {
+                'HEAD': {
+                    HTTP_NOT_FOUND: (NoSuchKey, self.object_name),
+                    HTTP_PRECONDITION_FAILED: PreconditionFailed,
+                },
+                'GET': {
+                    HTTP_NOT_FOUND: (NoSuchKey, self.object_name),
+                    HTTP_PRECONDITION_FAILED: PreconditionFailed,
+                    HTTP_REQUESTED_RANGE_NOT_SATISFIABLE: InvalidRange,
+                },
+                'PUT': {
+                    HTTP_NOT_FOUND: (NoSuchBucket, self.container_name),
+                    HTTP_UNPROCESSABLE_ENTITY: InvalidDigest,
+                    HTTP_REQUEST_ENTITY_TOO_LARGE: EntityTooLarge,
+                    HTTP_LENGTH_REQUIRED: MissingContentLength,
+                },
+                'DELETE': {
+                    HTTP_NOT_FOUND: (NoSuchKey, self.object_name),
+                },
+            }
+
+        return code_map[method]
+
+    def get_response(self, app, method=None, query=None):
         """
         Calls the application with this request's environment.  Returns a
         Response object that wraps up the application's result.
         """
-        sw_req = self.to_swift_req()
+        method = method or self.environ['REQUEST_METHOD']
+        sw_req = self.to_swift_req(method=method, query=query)
         sw_resp = sw_req.get_response(app)
+        resp = Response.from_swift_resp(sw_resp)
+        status = resp.status_int  # pylint: disable-msg=E1101
 
-        return Response.from_swift_resp(sw_resp)
+        success_codes = self._swift_success_codes(method)
+        error_codes = self._swift_error_codes(method)
+
+        if status in success_codes:
+            return resp
+
+        if status in error_codes:
+            err_resp = error_codes[sw_resp.status_int]
+            if isinstance(err_resp, tuple):
+                raise err_resp[0](*err_resp[1:])
+            else:
+                raise err_resp()
+
+        if status == HTTP_BAD_REQUEST:
+            raise BadSwiftRequest(resp.body)
+        if status == HTTP_UNAUTHORIZED:
+            raise SignatureDoesNotMatch()
+        if status == HTTP_FORBIDDEN:
+            raise AccessDenied()
+        if status == HTTP_SERVICE_UNAVAILABLE:
+            raise ServiceUnavailable()
+
+        raise InternalError('unexpteted status code %d' % status)

swift3/test/unit/test_swift3.py

@@ -88,6 +88,8 @@ class TestSwift3(unittest.TestCase):
         for b in self.objects:
             json_out.append(json_pattern % b)
         object_list = '[' + ','.join(json_out) + ']'
+        self.swift.register('HEAD', '/v1/AUTH_test/junk', swob.HTTPNoContent,
+                            {}, object_list)
         self.swift.register('GET', '/v1/AUTH_test/junk', swob.HTTPOk, {},
                             object_list)
 
@@ -111,7 +113,7 @@ class TestSwift3(unittest.TestCase):
         self.swift.register('PUT', '/v1/AUTH_test/bucket',
                             swob.HTTPCreated, {}, None)
         self.swift.register('POST', '/v1/AUTH_test/bucket',
-                            swob.HTTPAccepted, {}, None)
+                            swob.HTTPNoContent, {}, None)
         self.swift.register('DELETE', '/v1/AUTH_test/bucket',
                             swob.HTTPNoContent, {}, None)
 
@@ -543,6 +545,11 @@ class TestSwift3(unittest.TestCase):
         self.assertEquals(status.split()[0], '204')
 
     def test_object_multi_DELETE(self):
+        self.swift.register('DELETE', '/v1/AUTH_test/bucket/Key1',
+                            swob.HTTPNoContent, {}, None)
+        self.swift.register('DELETE', '/v1/AUTH_test/bucket/Key2',
+                            swob.HTTPNotFound, {}, None)
+
         elem = Element('Delete')
         for key in ['Key1', 'Key2']:
             obj = SubElement(elem, 'Object')