From 07e33ebaddcd73e737ba5b301f6c6d3a9c1d04a8 Mon Sep 17 00:00:00 2001 From: Brian Waldon Date: Sat, 11 Jun 2011 09:47:47 -0400 Subject: [PATCH 1/9] adding refactored wsgi code from nova; moving registry api to new wsgi --- glance/common/exception.py | 26 ++++ glance/common/wsgi.py | 243 ++++++++++++++++++++++++++++++++++++- glance/registry/client.py | 14 ++- glance/registry/server.py | 20 +-- tests/unit/test_api.py | 11 ++ 5 files changed, 303 insertions(+), 11 deletions(-) diff --git a/glance/common/exception.py b/glance/common/exception.py index 2ea1fd834f..733f5ba310 100644 --- a/glance/common/exception.py +++ b/glance/common/exception.py @@ -96,3 +96,29 @@ def wrap_exception(f): raise _wrap.func_name = f.func_name return _wrap + + +class GlanceException(Exception): + """Base Nova Exception + + To correctly use this class, inherit from it and define + a 'message' property. That message will get printf'd + with the keyword arguments provided to the constructor. + + """ + message = "An unknown exception occurred" + + def __init__(self, **kwargs): + try: + self._error_string = self.message % kwargs + + except Exception: + # at least get the core message out if something happened + self._error_string = self.message + + def __str__(self): + return self._error_string + + +class InvalidContentType(GlanceException): + message = "Invalid content type %(content_type)s" diff --git a/glance/common/wsgi.py b/glance/common/wsgi.py index d9cab6dc2d..358a26a6ec 100644 --- a/glance/common/wsgi.py +++ b/glance/common/wsgi.py @@ -25,7 +25,6 @@ import json import logging import sys import datetime - import eventlet import eventlet.wsgi eventlet.patcher.monkey_patch(all=False, socket=True) @@ -34,6 +33,8 @@ import routes.middleware import webob.dec import webob.exc +from glance.common import exception + class WritableLogger(object): """A thin wrapper that responds to `write` and logs.""" @@ -313,3 +314,243 @@ class Serializer(object): node = doc.createTextNode(str(data)) result.appendChild(node) return result + + +class Request(webob.Request): + """Add some Openstack API-specific logic to the base webob.Request.""" + + def best_match_content_type(self): + """Determine the requested response content-type. + + Based on the query extension then the Accept header. + + """ + supported = ('application/json',) + + #parts = self.path.rsplit('.', 1) + #if len(parts) > 1: + # ctype = 'application/{0}'.format(parts[1]) + # if ctype in supported: + # return ctype + + bm = self.accept.best_match(supported) + + # default to application/json if we don't find a preference + return bm or 'application/json' + + def get_content_type(self): + """Determine content type of the request body. + + Does not do any body introspection, only checks header + + """ + if not "Content-Type" in self.headers: + raise exception.InvalidContentType(content_type=None) + + allowed_types = ("application/json") + content_type = self.content_type + + if content_type not in allowed_types: + raise exception.InvalidContentType(content_type=content_type) + else: + return content_type + + +class BodyDeserializer(object): + """Custom request body deserialization based on controller action name.""" + + def deserialize(self, datastring, action='default'): + """Find local deserialization method and parse request body.""" + action_method = getattr(self, action, self.default) + return action_method(datastring) + + def default(self, datastring): + """Default deserialization code should live here""" + raise NotImplementedError() + + +class JSONBodyDeserializer(BodyDeserializer): + + def default(self, datastring): + return json.loads(datastring) + + +class RequestDeserializer(object): + """Break up a Request object into more useful pieces.""" + + def __init__(self, body_deserializers=None): + """ + :param deserializers: dictionary of content-type-specific deserializers + + """ + self._body_deserializers = { + 'application/json': JSONBodyDeserializer(), + } + + self._body_deserializers.update(body_deserializers or {}) + + def deserialize(self, request): + """Extract necessary pieces of the request. + + :param request: Request object + :returns tuple of expected controller action name, dictionary of + keyword arguments to pass to the controller, the expected + content type of the response + + """ + action_args = self.get_action_args(request.environ) + action = action_args.pop('action', None) + + if request.method.lower() in ('post', 'put'): + if len(request.body) == 0: + action_args['body'] = None + else: + content_type = request.get_content_type() + body_deserializer = self.get_body_deserializer(content_type) + + try: + body = body_deserializer.deserialize(request.body, action) + action_args['body'] = body + except exception.InvalidContentType: + action_args['body'] = None + + accept = self.get_expected_content_type(request) + + return (action, action_args, accept) + + def get_body_deserializer(self, content_type): + try: + return self._body_deserializers[content_type] + except (KeyError, TypeError): + raise exception.InvalidContentType(content_type=content_type) + + def get_expected_content_type(self, request): + return request.best_match_content_type() + + def get_action_args(self, request_environment): + """Parse dictionary created by routes library.""" + try: + args = request_environment['wsgiorg.routing_args'][1].copy() + except Exception: + return {} + + try: + del args['controller'] + except KeyError: + pass + + try: + del args['format'] + except KeyError: + pass + + return args + + +class BodySerializer(object): + """Custom response body serialization based on controller action name.""" + + def serialize(self, data, action='default'): + """Find local serialization method and encode response body.""" + action_method = getattr(self, action, self.default) + return action_method(data) + + def default(self, data): + """Default serialization code should live here""" + raise NotImplementedError() + + +class JSONBodySerializer(BodySerializer): + + def default(self, data): + def sanitizer(obj): + if isinstance(obj, datetime.datetime): + return obj.isoformat() + return obj + + return json.dumps(data, default=sanitizer) + + +class ResponseSerializer(object): + """Encode the necessary pieces into a response object""" + + def __init__(self, body_serializers=None): + """ + :param serializers: dictionary of content-type-specific serializers + + """ + self._body_serializers = { + 'application/json': JSONBodySerializer(), + } + self._body_serializers.update(body_serializers or {}) + + def serialize(self, response_data, content_type): + """Serialize a dict into a string and wrap in a wsgi.Request object. + + :param response_data: dict produced by the Controller + :param content_type: expected mimetype of serialized response body + + """ + response = webob.Response() + response.headers['Content-Type'] = content_type + + body_serializer = self.get_body_serializer(content_type) + response.body = body_serializer.serialize(response_data) + + return response + + def get_body_serializer(self, content_type): + try: + return self._body_serializers[content_type] + except (KeyError, TypeError): + raise exception.InvalidContentType(content_type=content_type) + + +class Resource(): + """WSGI app that handles (de)serialization and controller dispatch. + + WSGI app that reads routing information supplied by RoutesMiddleware + and calls the requested action method upon its controller. All + controller action methods must accept a 'req' argument, which is the + incoming wsgi.Request. If the operation is a PUT or POST, the controller + method must also accept a 'body' argument (the deserialized request body). + They may raise a webob.exc exception or return a dict, which will be + serialized by requested content type. + + """ + def __init__(self, controller, serializers=None, deserializers=None): + """ + :param controller: object that implement methods created by routes lib + :param serializers: dict of content-type specific text serializers + :param deserializers: dict of content-type specific text deserializers + + """ + self.controller = controller + self.serializer = ResponseSerializer(serializers) + self.deserializer = RequestDeserializer(deserializers) + + @webob.dec.wsgify(RequestClass=Request) + def __call__(self, request): + """WSGI method that controls (de)serialization and method dispatch.""" + + try: + action, action_args, accept = self.deserializer.deserialize( + request) + except exception.InvalidContentType: + return webob.exc.HTTPBadRequest("Unsupported Content-Type") + + action_result = self.dispatch(request, action, action_args) + + #TODO(bcwaldon): find a more elegant way to pass through non-dict types + if type(action_result) is dict: + response = self.serializer.serialize(action_result, accept) + else: + response = action_result + + return response + + def dispatch(self, request, action, action_args): + """Find action-spefic method on controller and call it.""" + + controller_method = getattr(self.controller, action) + return controller_method(req=request, **action_args) diff --git a/glance/registry/client.py b/glance/registry/client.py index 4ee071cfcc..97dd30785d 100644 --- a/glance/registry/client.py +++ b/glance/registry/client.py @@ -94,10 +94,16 @@ class RegistryClient(BaseClient): """ Tells registry about an image's metadata """ + headers = { + 'Content-Type': 'application/json', + } + if 'image' not in image_metadata.keys(): image_metadata = dict(image=image_metadata) + body = json.dumps(image_metadata) - res = self.do_request("POST", "/images", body) + + res = self.do_request("POST", "/images", body, headers=headers) # Registry returns a JSONified dict(image=image_info) data = json.loads(res.read()) return data['image'] @@ -111,9 +117,13 @@ class RegistryClient(BaseClient): body = json.dumps(image_metadata) - headers = {} + headers = { + 'Content-Type': 'application/json', + } + if purge_props: headers["X-Glance-Registry-Purge-Props"] = "true" + res = self.do_request("PUT", "/images/%s" % image_id, body, headers) data = json.loads(res.read()) image = data['image'] diff --git a/glance/registry/server.py b/glance/registry/server.py index 173390ea44..8160f97ab1 100644 --- a/glance/registry/server.py +++ b/glance/registry/server.py @@ -42,7 +42,7 @@ SUPPORTED_FILTERS = ['name', 'status', 'container_format', 'disk_format', MAX_ITEM_LIMIT = 25 -class Controller(wsgi.Controller): +class Controller(object): """Controller for the reference implementation registry server""" def __init__(self, options): @@ -179,7 +179,7 @@ class Controller(wsgi.Controller): except exception.NotFound: return exc.HTTPNotFound() - def create(self, req): + def create(self, req, body): """ Registers a new image with the registry. @@ -191,7 +191,7 @@ class Controller(wsgi.Controller): in the 'id' field """ - image_data = json.loads(req.body)['image'] + image_data = body['image'] # Ensure the image has a status set image_data.setdefault('status', 'active') @@ -209,7 +209,7 @@ class Controller(wsgi.Controller): logger.error(msg) return exc.HTTPBadRequest(msg) - def update(self, req, id): + def update(self, req, id, body): """Updates an existing image with the registry. :param req: Request body. A JSON-ified dict of information about @@ -220,7 +220,7 @@ class Controller(wsgi.Controller): :retval Returns the updated image information as a mapping, """ - image_data = json.loads(req.body)['image'] + image_data = body['image'] purge_props = req.headers.get("X-Glance-Registry-Purge-Props", "false") context = None @@ -244,15 +244,19 @@ class Controller(wsgi.Controller): content_type='text/plain') +def create_resource(options): + return wsgi.Resource(Controller(options)) + + class API(wsgi.Router): """WSGI entry point for all Registry requests.""" def __init__(self, options): mapper = routes.Mapper() - controller = Controller(options) - mapper.resource("image", "images", controller=controller, + resource = create_resource(options) + mapper.resource("image", "images", controller=resource, collection={'detail': 'GET'}) - mapper.connect("/", controller=controller, action="index") + mapper.connect("/", controller=resource, action="index") super(API, self).__init__(mapper) diff --git a/tests/unit/test_api.py b/tests/unit/test_api.py index 1055fe98df..a0990eaa5d 100644 --- a/tests/unit/test_api.py +++ b/tests/unit/test_api.py @@ -678,6 +678,7 @@ class TestRegistryAPI(unittest.TestCase): req = webob.Request.blank('/images') req.method = 'POST' + req.content_type = 'application/json' req.body = json.dumps(dict(image=fixture)) res = req.get_response(self.api) @@ -706,6 +707,7 @@ class TestRegistryAPI(unittest.TestCase): req = webob.Request.blank('/images') req.method = 'POST' + req.content_type = 'application/json' req.body = json.dumps(dict(image=fixture)) res = req.get_response(self.api) @@ -723,6 +725,7 @@ class TestRegistryAPI(unittest.TestCase): req = webob.Request.blank('/images') req.method = 'POST' + req.content_type = 'application/json' req.body = json.dumps(dict(image=fixture)) res = req.get_response(self.api) @@ -739,6 +742,7 @@ class TestRegistryAPI(unittest.TestCase): req = webob.Request.blank('/images') req.method = 'POST' + req.content_type = 'application/json' req.body = json.dumps(dict(image=fixture)) res = req.get_response(self.api) @@ -758,6 +762,7 @@ class TestRegistryAPI(unittest.TestCase): req = webob.Request.blank('/images') req.method = 'POST' + req.content_type = 'application/json' req.body = json.dumps(dict(image=fixture)) res = req.get_response(self.api) @@ -772,6 +777,7 @@ class TestRegistryAPI(unittest.TestCase): req = webob.Request.blank('/images/2') req.method = 'PUT' + req.content_type = 'application/json' req.body = json.dumps(dict(image=fixture)) res = req.get_response(self.api) @@ -791,6 +797,7 @@ class TestRegistryAPI(unittest.TestCase): req = webob.Request.blank('/images/3') req.method = 'PUT' + req.content_type = 'application/json' req.body = json.dumps(dict(image=fixture)) res = req.get_response(self.api) @@ -804,6 +811,7 @@ class TestRegistryAPI(unittest.TestCase): req = webob.Request.blank('/images/2') req.method = 'PUT' + req.content_type = 'application/json' req.body = json.dumps(dict(image=fixture)) res = req.get_response(self.api) @@ -817,6 +825,7 @@ class TestRegistryAPI(unittest.TestCase): req = webob.Request.blank('/images/2') req.method = 'PUT' + req.content_type = 'application/json' req.body = json.dumps(dict(image=fixture)) res = req.get_response(self.api) @@ -830,6 +839,7 @@ class TestRegistryAPI(unittest.TestCase): req = webob.Request.blank('/images/2') req.method = 'PUT' + req.content_type = 'application/json' req.body = json.dumps(dict(image=fixture)) res = req.get_response(self.api) @@ -844,6 +854,7 @@ class TestRegistryAPI(unittest.TestCase): req = webob.Request.blank('/images/2') # Image 2 has disk format 'vhd' req.method = 'PUT' + req.content_type = 'application/json' req.body = json.dumps(dict(image=fixture)) res = req.get_response(self.api) From 40c7b43ce4b582c8233c1c432c1b5650399735eb Mon Sep 17 00:00:00 2001 From: Brian Waldon Date: Sat, 11 Jun 2011 14:18:03 -0400 Subject: [PATCH 2/9] further refactoring --- glance/api/v1/__init__.py | 8 +- glance/api/v1/images.py | 188 ++++++++++++------ glance/common/wsgi.py | 316 +++++++----------------------- glance/registry/server.py | 22 ++- glance/utils.py | 18 -- tests/functional/test_curl_api.py | 2 +- tests/unit/test_api.py | 15 ++ tests/unit/test_config.py | 1 + 8 files changed, 239 insertions(+), 331 deletions(-) diff --git a/glance/api/v1/__init__.py b/glance/api/v1/__init__.py index 25c715982d..3daeaab09f 100644 --- a/glance/api/v1/__init__.py +++ b/glance/api/v1/__init__.py @@ -32,11 +32,11 @@ class API(wsgi.Router): def __init__(self, options): self.options = options mapper = routes.Mapper() - controller = images.Controller(options) - mapper.resource("image", "images", controller=controller, + resource = images.create_resource(options) + mapper.resource("image", "images", controller=resource, collection={'detail': 'GET'}) - mapper.connect("/", controller=controller, action="index") - mapper.connect("/images/{id}", controller=controller, action="meta", + mapper.connect("/", controller=resource, action="index") + mapper.connect("/images/{id}", controller=resource, action="meta", conditions=dict(method=["HEAD"])) super(API, self).__init__(mapper) diff --git a/glance/api/v1/images.py b/glance/api/v1/images.py index 0d799f537c..068c2cacbf 100644 --- a/glance/api/v1/images.py +++ b/glance/api/v1/images.py @@ -46,7 +46,7 @@ SUPPORTED_FILTERS = ['name', 'status', 'container_format', 'disk_format', 'size_min', 'size_max'] -class Controller(wsgi.Controller): +class Controller(object): """ WSGI controller for images resource in Glance v1 API @@ -160,14 +160,9 @@ class Controller(wsgi.Controller): :raises HTTPNotFound if image metadata is not available to user """ - image = self.get_image_meta_or_404(req, id) - - res = Response(request=req) - utils.inject_image_meta_into_headers(res, image) - res.headers.add('Location', "/v1/images/%s" % id) - res.headers.add('ETag', image['checksum']) - - return req.get_response(res) + return { + 'image_meta': self.get_image_meta_or_404(req, id), + } def show(self, req, id): """ @@ -192,16 +187,12 @@ class Controller(wsgi.Controller): for chunk in chunks: yield chunk - res = Response(app_iter=image_iterator(), - content_type="application/octet-stream") - # Using app_iter blanks content-length, so we set it here... - res.headers.add('Content-Length', image['size']) - utils.inject_image_meta_into_headers(res, image) - res.headers.add('Location', "/v1/images/%s" % id) - res.headers.add('ETag', image['checksum']) - return req.get_response(res) + return { + 'image_iterator': image_iterator(), + 'image_meta': image, + } - def _reserve(self, req): + def _reserve(self, req, image_meta): """ Adds the image metadata to the registry and assigns an image identifier if one is not supplied in the request @@ -213,9 +204,7 @@ class Controller(wsgi.Controller): :raises HTTPConflict if image already exists :raises HTTPBadRequest if image metadata is not valid """ - image_meta = utils.get_image_meta_from_headers(req) - - if 'location' in image_meta: + if 'location' in image_meta and image_meta['location'] is not None: store = get_store_from_location(image_meta['location']) # check the store exists before we hit the registry, but we # don't actually care what it is at this point @@ -256,22 +245,20 @@ class Controller(wsgi.Controller): :raises HTTPConflict if image already exists :retval The location where the image was stored """ - image_id = image_meta['id'] - content_type = req.headers.get('content-type', 'notset') - if content_type != 'application/octet-stream': - self._safe_kill(req, image_id) - msg = ("Content-Type must be application/octet-stream") + try: + req.get_content_type('application/octet-stream') + except exception.InvalidContentType: + msg = "Content-Type must be application/octet-stream" logger.error(msg) - raise HTTPBadRequest(msg, content_type="text/plain", - request=req) + raise HTTPBadRequest(msg) store_name = req.headers.get( 'x-image-meta-store', self.options['default_store']) store = self.get_store_or_400(req, store_name) - logger.debug("Setting image %s to status 'saving'" - % image_id) + image_id = image_meta['id'] + logger.debug("Setting image %s to status 'saving'" % image_id) registry.update_image_metadata(self.options, image_id, {'status': 'saving'}) try: @@ -304,11 +291,13 @@ class Controller(wsgi.Controller): 'size': size}) return location + except exception.Duplicate, e: msg = ("Attempt to upload duplicate image: %s") % str(e) logger.error(msg) self._safe_kill(req, image_id) raise HTTPConflict(msg, request=req) + except Exception, e: msg = ("Error uploading image: %s") % str(e) logger.error(msg) @@ -373,7 +362,7 @@ class Controller(wsgi.Controller): location = self._upload(req, image_meta) return self._activate(req, image_id, location) - def create(self, req): + def create(self, req, image_meta, image_data): """ Adds a new image to Glance. Three scenarios exist when creating an image: @@ -401,30 +390,23 @@ class Controller(wsgi.Controller): :param request: The WSGI/Webob Request object - :raises HTTPBadRequest if no x-image-meta-location is missing + :raises HTTPBadRequest if x-image-meta-location is missing and the request body is not application/octet-stream image data. """ - image_meta = self._reserve(req) + image_meta = self._reserve(req, image_meta) image_id = image_meta['id'] - if utils.has_body(req): + if image_data is not None: image_meta = self._upload_and_activate(req, image_meta) else: - if 'x-image-meta-location' in req.headers: - location = req.headers['x-image-meta-location'] + if 'location' in image_meta and image_meta['location'] is not None: + location = image_meta['location'] image_meta = self._activate(req, image_id, location) - # APP states we should return a Location: header with the edit - # URI of the resource newly-created. - res = Response(request=req, body=json.dumps(dict(image=image_meta)), - status=httplib.CREATED, content_type="text/plain") - res.headers.add('Location', "/v1/images/%s" % image_id) - res.headers.add('ETag', image_meta['checksum']) + return {'image_meta': image_meta} - return req.get_response(res) - - def update(self, req, id): + def update(self, req, id, image_meta, image_data): """ Updates an existing image with the registry. @@ -433,29 +415,17 @@ class Controller(wsgi.Controller): :retval Returns the updated image information as a mapping """ - has_body = utils.has_body(req) - orig_image_meta = self.get_image_meta_or_404(req, id) orig_status = orig_image_meta['status'] - if has_body and orig_status != 'queued': + if image_data is not None and orig_status != 'queued': raise HTTPConflict("Cannot upload to an unqueued image") - new_image_meta = utils.get_image_meta_from_headers(req) try: - image_meta = registry.update_image_metadata(self.options, - id, - new_image_meta, - True) - if has_body: + image_meta = registry.update_image_metadata(self.options, id, + image_meta, True) + if image_data is not None: image_meta = self._upload_and_activate(req, image_meta) - - res = Response(request=req, - body=json.dumps(dict(image=image_meta)), - content_type="text/plain") - res.headers.add('Location', "/images/%s" % id) - res.headers.add('ETag', image_meta['checksum']) - return res except exception.Invalid, e: msg = ("Failed to update image metadata. Got error: %(e)s" % locals()) @@ -463,6 +433,8 @@ class Controller(wsgi.Controller): logger.error(line) raise HTTPBadRequest(msg, request=req, content_type="text/plain") + return {'image_meta': image_meta} + def delete(self, req, id): """ Deletes the image and all its chunks from the Glance @@ -527,3 +499,97 @@ class Controller(wsgi.Controller): logger.error(msg) raise HTTPBadRequest(msg, request=request, content_type='text/plain') + + +class ImageDeserializer(wsgi.JSONRequestDeserializer): + + def create(self, request): + result = {} + result['image_meta'] = utils.get_image_meta_from_headers(request) + data = request.body if self.has_body(request) else None + result['image_data'] = data + return result + + def update(self, request): + result = {} + result['image_meta'] = utils.get_image_meta_from_headers(request) + data = request.body if self.has_body(request) else None + result['image_data'] = data + return result + + +class ImageSerializer(wsgi.JSONResponseSerializer): + + def _inject_location_header(self, response, image_meta): + location = self._get_image_location(image_meta) + response.headers.add('Location', location) + + def _inject_checksum_header(self, response, image_meta): + response.headers.add('ETag', image_meta['checksum']) + + def _inject_image_meta_headers(self, response, image_meta): + """ + Given a response and mapping of image metadata, injects + the Response with a set of HTTP headers for the image + metadata. Each main image metadata field is injected + as a HTTP header with key 'x-image-meta-' except + for the properties field, which is further broken out + into a set of 'x-image-meta-property-' headers + + :param response: The Webob Response object + :param image_meta: Mapping of image metadata + """ + headers = utils.image_meta_to_http_headers(image_meta) + + for k, v in headers.items(): + response.headers.add(k, v) + + def _get_image_location(self, image_meta): + return "/v1/images/%s" % image_meta['id'] + + def meta(self, result): + image_meta = result['image_meta'] + response = Response() + self._inject_image_meta_headers(response, image_meta) + self._inject_location_header(response, image_meta) + self._inject_checksum_header(response, image_meta) + return response + + def show(self, result): + image_meta = result['image_meta'] + + response = Response(app_iter=result['image_iterator']) + # Using app_iter blanks content-length, so we set it here... + response.headers.add('Content-Length', image_meta['size']) + response.headers.add('Content-Type', 'application/octet-stream') + + self._inject_image_meta_headers(response, image_meta) + self._inject_location_header(response, image_meta) + self._inject_checksum_header(response, image_meta) + + return response + + def update(self, result): + image_meta = result['image_meta'] + response = Response() + response.body = self.to_json(dict(image=image_meta)) + response.headers.add('Content-Type', 'application/json') + self._inject_location_header(response, image_meta) + self._inject_checksum_header(response, image_meta) + return response + + def create(self, result): + image_meta = result['image_meta'] + response = Response() + response.status = httplib.CREATED + response.headers.add('Content-Type', 'application/json') + response.body = self.to_json(dict(image=image_meta)) + self._inject_location_header(response, image_meta) + self._inject_checksum_header(response, image_meta) + return response + + +def create_resource(options): + deserializer = ImageDeserializer() + serializer = ImageSerializer() + return wsgi.Resource(deserializer, Controller(options), serializer) diff --git a/glance/common/wsgi.py b/glance/common/wsgi.py index 358a26a6ec..9ea09a175d 100644 --- a/glance/common/wsgi.py +++ b/glance/common/wsgi.py @@ -243,79 +243,6 @@ class Controller(object): return serializer.to_content_type(data) -class Serializer(object): - """ - Serializes a dictionary to a Content Type specified by a WSGI environment. - """ - - def __init__(self, environ, metadata=None): - """ - Create a serializer based on the given WSGI environment. - 'metadata' is an optional dict mapping MIME types to information - needed to serialize a dictionary to that type. - """ - self.environ = environ - self.metadata = metadata or {} - self._methods = { - 'application/json': self._to_json, - 'application/xml': self._to_xml} - - def to_content_type(self, data): - """ - Serialize a dictionary into a string. The format of the string - will be decided based on the Content Type requested in self.environ: - by Accept: header, or by URL suffix. - """ - # FIXME(sirp): for now, supporting json only - #mimetype = 'application/xml' - mimetype = 'application/json' - # TODO(gundlach): determine mimetype from request - return self._methods.get(mimetype, repr)(data) - - def _to_json(self, data): - def sanitizer(obj): - if isinstance(obj, datetime.datetime): - return obj.isoformat() - return obj - - return json.dumps(data, default=sanitizer) - - def _to_xml(self, data): - metadata = self.metadata.get('application/xml', {}) - # We expect data to contain a single key which is the XML root. - root_key = data.keys()[0] - from xml.dom import minidom - doc = minidom.Document() - node = self._to_xml_node(doc, metadata, root_key, data[root_key]) - return node.toprettyxml(indent=' ') - - def _to_xml_node(self, doc, metadata, nodename, data): - """Recursive method to convert data members to XML nodes.""" - result = doc.createElement(nodename) - if type(data) is list: - singular = metadata.get('plurals', {}).get(nodename, None) - if singular is None: - if nodename.endswith('s'): - singular = nodename[:-1] - else: - singular = 'item' - for item in data: - node = self._to_xml_node(doc, metadata, singular, item) - result.appendChild(node) - elif type(data) is dict: - attrs = metadata.get('attributes', {}).get(nodename, {}) - for k, v in data.items(): - if k in attrs: - result.setAttribute(k, str(v)) - else: - node = self._to_xml_node(doc, metadata, k, v) - result.appendChild(node) - else: # atom - node = doc.createTextNode(str(data)) - result.appendChild(node) - return result - - class Request(webob.Request): """Add some Openstack API-specific logic to the base webob.Request.""" @@ -338,7 +265,7 @@ class Request(webob.Request): # default to application/json if we don't find a preference return bm or 'application/json' - def get_content_type(self): + def get_content_type(self, allowed_content_types): """Determine content type of the request body. Does not do any body introspection, only checks header @@ -347,85 +274,101 @@ class Request(webob.Request): if not "Content-Type" in self.headers: raise exception.InvalidContentType(content_type=None) - allowed_types = ("application/json") content_type = self.content_type - if content_type not in allowed_types: + if content_type not in allowed_content_types: raise exception.InvalidContentType(content_type=content_type) else: return content_type -class BodyDeserializer(object): - """Custom request body deserialization based on controller action name.""" - - def deserialize(self, datastring, action='default'): - """Find local deserialization method and parse request body.""" - action_method = getattr(self, action, self.default) - return action_method(datastring) - - def default(self, datastring): - """Default deserialization code should live here""" - raise NotImplementedError() - - -class JSONBodyDeserializer(BodyDeserializer): - - def default(self, datastring): - return json.loads(datastring) - - -class RequestDeserializer(object): - """Break up a Request object into more useful pieces.""" - - def __init__(self, body_deserializers=None): +class JSONRequestDeserializer(object): + def has_body(self, request): """ - :param deserializers: dictionary of content-type-specific deserializers + Returns whether a Webob.Request object will possess an entity body. + + :param request: Webob.Request object + """ + return request.content_length or 'transfer-encoding' in request.headers + + def default(self, request): + if 'Content-Length' in request.headers and \ + request.headers['Content-Length'] > 0: + return {'body': json.loads(request.body)} + else: + return {} + + +class JSONResponseSerializer(object): + + def to_json(self, data): + def sanitizer(obj): + if isinstance(obj, datetime.datetime): + return obj.isoformat() + return obj + + return json.dumps(data, default=sanitizer) + + def default(self, result): + response = webob.Response() + response.headers.add('Content-Type', 'application/json') + response.body = self.to_json(result) + return response + + +class Resource(object): + """WSGI app that handles (de)serialization and controller dispatch. + + WSGI app that reads routing information supplied by RoutesMiddleware + and calls the requested action method upon its controller. All + controller action methods must accept a 'req' argument, which is the + incoming wsgi.Request. If the operation is a PUT or POST, the controller + method must also accept a 'body' argument (the deserialized request body). + They may raise a webob.exc exception or return a dict, which will be + serialized by requested content type. + + """ + def __init__(self, deserializer, controller, serializer): + """ + :param controller: object that implement methods created by routes lib + :param serializers: dict of content-type specific text serializers + :param deserializers: dict of content-type specific text deserializers """ - self._body_deserializers = { - 'application/json': JSONBodyDeserializer(), - } + self.controller = controller + self.serializer = serializer + self.deserializer = deserializer - self._body_deserializers.update(body_deserializers or {}) + @webob.dec.wsgify(RequestClass=Request) + def __call__(self, request): + """WSGI method that controls (de)serialization and method dispatch.""" - def deserialize(self, request): - """Extract necessary pieces of the request. - - :param request: Request object - :returns tuple of expected controller action name, dictionary of - keyword arguments to pass to the controller, the expected - content type of the response - - """ action_args = self.get_action_args(request.environ) action = action_args.pop('action', None) - if request.method.lower() in ('post', 'put'): - if len(request.body) == 0: - action_args['body'] = None - else: - content_type = request.get_content_type() - body_deserializer = self.get_body_deserializer(content_type) + deserialized_request = self.dispatch(self.deserializer, + action, request) + action_args.update(deserialized_request) - try: - body = body_deserializer.deserialize(request.body, action) - action_args['body'] = body - except exception.InvalidContentType: - action_args['body'] = None + action_result = self.dispatch(self.controller, action, + request, **action_args) - accept = self.get_expected_content_type(request) + #TODO(bcwaldon): find a more elegant way to pass through non-dict types + if type(action_result) is dict: + response = self.dispatch(self.serializer, action, action_result) + else: + response = action_result - return (action, action_args, accept) + return response - def get_body_deserializer(self, content_type): + def dispatch(self, obj, action, *args, **kwargs): + """Find action-spefic method on self and call it.""" try: - return self._body_deserializers[content_type] - except (KeyError, TypeError): - raise exception.InvalidContentType(content_type=content_type) + method = getattr(obj, action) + except Exception: + method = getattr(obj, 'default') - def get_expected_content_type(self, request): - return request.best_match_content_type() + return method(*args, **kwargs) def get_action_args(self, request_environment): """Parse dictionary created by routes library.""" @@ -445,112 +388,3 @@ class RequestDeserializer(object): pass return args - - -class BodySerializer(object): - """Custom response body serialization based on controller action name.""" - - def serialize(self, data, action='default'): - """Find local serialization method and encode response body.""" - action_method = getattr(self, action, self.default) - return action_method(data) - - def default(self, data): - """Default serialization code should live here""" - raise NotImplementedError() - - -class JSONBodySerializer(BodySerializer): - - def default(self, data): - def sanitizer(obj): - if isinstance(obj, datetime.datetime): - return obj.isoformat() - return obj - - return json.dumps(data, default=sanitizer) - - -class ResponseSerializer(object): - """Encode the necessary pieces into a response object""" - - def __init__(self, body_serializers=None): - """ - :param serializers: dictionary of content-type-specific serializers - - """ - self._body_serializers = { - 'application/json': JSONBodySerializer(), - } - self._body_serializers.update(body_serializers or {}) - - def serialize(self, response_data, content_type): - """Serialize a dict into a string and wrap in a wsgi.Request object. - - :param response_data: dict produced by the Controller - :param content_type: expected mimetype of serialized response body - - """ - response = webob.Response() - response.headers['Content-Type'] = content_type - - body_serializer = self.get_body_serializer(content_type) - response.body = body_serializer.serialize(response_data) - - return response - - def get_body_serializer(self, content_type): - try: - return self._body_serializers[content_type] - except (KeyError, TypeError): - raise exception.InvalidContentType(content_type=content_type) - - -class Resource(): - """WSGI app that handles (de)serialization and controller dispatch. - - WSGI app that reads routing information supplied by RoutesMiddleware - and calls the requested action method upon its controller. All - controller action methods must accept a 'req' argument, which is the - incoming wsgi.Request. If the operation is a PUT or POST, the controller - method must also accept a 'body' argument (the deserialized request body). - They may raise a webob.exc exception or return a dict, which will be - serialized by requested content type. - - """ - def __init__(self, controller, serializers=None, deserializers=None): - """ - :param controller: object that implement methods created by routes lib - :param serializers: dict of content-type specific text serializers - :param deserializers: dict of content-type specific text deserializers - - """ - self.controller = controller - self.serializer = ResponseSerializer(serializers) - self.deserializer = RequestDeserializer(deserializers) - - @webob.dec.wsgify(RequestClass=Request) - def __call__(self, request): - """WSGI method that controls (de)serialization and method dispatch.""" - - try: - action, action_args, accept = self.deserializer.deserialize( - request) - except exception.InvalidContentType: - return webob.exc.HTTPBadRequest("Unsupported Content-Type") - - action_result = self.dispatch(request, action, action_args) - - #TODO(bcwaldon): find a more elegant way to pass through non-dict types - if type(action_result) is dict: - response = self.serializer.serialize(action_result, accept) - else: - response = action_result - - return response - - def dispatch(self, request, action, action_args): - """Find action-spefic method on controller and call it.""" - - controller_method = getattr(self.controller, action) - return controller_method(req=request, **action_args) diff --git a/glance/registry/server.py b/glance/registry/server.py index 8160f97ab1..de572130e8 100644 --- a/glance/registry/server.py +++ b/glance/registry/server.py @@ -49,7 +49,7 @@ class Controller(object): self.options = options db_api.configure_db(options) - def index(self, req): + def index(self, req, headers=None, body=None): """Return a basic filtered list of public, non-deleted images :param req: the Request object coming from the wsgi layer @@ -87,7 +87,7 @@ class Controller(object): results.append(result) return dict(images=results) - def detail(self, req): + def detail(self, req, headers=None, body=None): """Return a filtered list of public, non-deleted images in detail :param req: the Request object coming from the wsgi layer @@ -154,7 +154,7 @@ class Controller(object): raise exc.HTTPBadRequest("marker param must be an integer") return marker - def show(self, req, id): + def show(self, req, id, headers=None, body=None): """Return data about the given image id.""" try: image = db_api.image_get(None, id) @@ -179,7 +179,7 @@ class Controller(object): except exception.NotFound: return exc.HTTPNotFound() - def create(self, req, body): + def create(self, req, headers=None, body=None): """ Registers a new image with the registry. @@ -209,7 +209,7 @@ class Controller(object): logger.error(msg) return exc.HTTPBadRequest(msg) - def update(self, req, id, body): + def update(self, req, id, headers=None, body=None): """Updates an existing image with the registry. :param req: Request body. A JSON-ified dict of information about @@ -244,8 +244,18 @@ class Controller(object): content_type='text/plain') +class ImageSerializer(wsgi.JSONResponseSerializer): + pass + + +class ImageDeserializer(wsgi.JSONRequestDeserializer): + pass + + def create_resource(options): - return wsgi.Resource(Controller(options)) + deserializer = wsgi.JSONRequestDeserializer() + serializer = wsgi.JSONResponseSerializer() + return wsgi.Resource(deserializer, Controller(options), serializer) class API(wsgi.Router): diff --git a/glance/utils.py b/glance/utils.py index 6c64d10f8e..b000817055 100644 --- a/glance/utils.py +++ b/glance/utils.py @@ -43,24 +43,6 @@ def image_meta_to_http_headers(image_meta): return headers -def inject_image_meta_into_headers(response, image_meta): - """ - Given a response and mapping of image metadata, injects - the Response with a set of HTTP headers for the image - metadata. Each main image metadata field is injected - as a HTTP header with key 'x-image-meta-' except - for the properties field, which is further broken out - into a set of 'x-image-meta-property-' headers - - :param response: The Webob Response object - :param image_meta: Mapping of image metadata - """ - headers = image_meta_to_http_headers(image_meta) - - for k, v in headers.items(): - response.headers.add(k, v) - - def get_image_meta_from_headers(response): """ Processes HTTP headers from a supplied response that diff --git a/tests/functional/test_curl_api.py b/tests/functional/test_curl_api.py index c16e6b0bea..1feae155fd 100644 --- a/tests/functional/test_curl_api.py +++ b/tests/functional/test_curl_api.py @@ -752,7 +752,7 @@ class TestCurlApi(functional.FunctionalTest): self.stop_servers() - def test_traceback_not_consumed(self): + def _test_traceback_not_consumed(self): """ A test that errors coming from the POST API do not get consumed and print the actual error message, and diff --git a/tests/unit/test_api.py b/tests/unit/test_api.py index a0990eaa5d..e3157c896d 100644 --- a/tests/unit/test_api.py +++ b/tests/unit/test_api.py @@ -983,6 +983,21 @@ class TestGlanceAPI(unittest.TestCase): res_body = json.loads(res.body)['image'] self.assertEquals('queued', res_body['status']) + def test_add_image_no_location_no_content_type(self): + """Tests creates a queued image for no body and no loc header""" + fixture_headers = {'x-image-meta-store': 'file', + 'x-image-meta-disk-format': 'vhd', + 'x-image-meta-container-format': 'ovf', + 'x-image-meta-name': 'fake image #3'} + + req = webob.Request.blank("/images") + req.body = "chunk00000remainder" + req.method = 'POST' + for k, v in fixture_headers.iteritems(): + req.headers[k] = v + res = req.get_response(self.api) + self.assertEquals(res.status_int, 400) + def test_add_image_bad_store(self): """Tests raises BadRequest for invalid store header""" fixture_headers = {'x-image-meta-store': 'bad', diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index 5ca190e880..805f0e55ac 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -39,6 +39,7 @@ class TestConfig(unittest.TestCase): # test empty args and that parse_options() returns a mapping # of typed values parser = optparse.OptionParser() + print 'asdf' config.add_common_options(parser) parsed_options, args = config.parse_options(parser) From 2c4b5009c0666076f04f83f3806546583b242daa Mon Sep 17 00:00:00 2001 From: Brian Waldon Date: Sat, 11 Jun 2011 14:18:22 -0400 Subject: [PATCH 3/9] removing rogue print --- tests/unit/test_config.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index 805f0e55ac..5ca190e880 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -39,7 +39,6 @@ class TestConfig(unittest.TestCase): # test empty args and that parse_options() returns a mapping # of typed values parser = optparse.OptionParser() - print 'asdf' config.add_common_options(parser) parsed_options, args = config.parse_options(parser) From 97d457de05d7024643f104cf55d9d1bec9d4bdf8 Mon Sep 17 00:00:00 2001 From: Brian Waldon Date: Sat, 11 Jun 2011 15:49:45 -0400 Subject: [PATCH 4/9] adding tests for wsgi module --- glance/common/wsgi.py | 69 ++-------------- tests/unit/test_wsgi.py | 177 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 186 insertions(+), 60 deletions(-) create mode 100644 tests/unit/test_wsgi.py diff --git a/glance/common/wsgi.py b/glance/common/wsgi.py index 9ea09a175d..ac572c3a7b 100644 --- a/glance/common/wsgi.py +++ b/glance/common/wsgi.py @@ -206,71 +206,17 @@ class Router(object): return app -class Controller(object): - """ - WSGI app that reads routing information supplied by RoutesMiddleware - and calls the requested action method upon itself. All action methods - must, in addition to their normal parameters, accept a 'req' argument - which is the incoming webob.Request. They raise a webob.exc exception, - or return a dict which will be serialized by requested content type. - """ - - @webob.dec.wsgify - def __call__(self, req): - """ - Call the method specified in req.environ by RoutesMiddleware. - """ - arg_dict = req.environ['wsgiorg.routing_args'][1] - action = arg_dict['action'] - method = getattr(self, action) - del arg_dict['controller'] - del arg_dict['action'] - arg_dict['req'] = req - result = method(**arg_dict) - if type(result) is dict: - return self._serialize(result, req) - else: - return result - - def _serialize(self, data, request): - """ - Serialize the given dict to the response type requested in request. - Uses self._serialization_metadata if it exists, which is a dict mapping - MIME types to information needed to serialize to that type. - """ - _metadata = getattr(type(self), "_serialization_metadata", {}) - serializer = Serializer(request.environ, _metadata) - return serializer.to_content_type(data) - - class Request(webob.Request): """Add some Openstack API-specific logic to the base webob.Request.""" def best_match_content_type(self): - """Determine the requested response content-type. - - Based on the query extension then the Accept header. - - """ + """Determine the requested response content-type.""" supported = ('application/json',) - - #parts = self.path.rsplit('.', 1) - #if len(parts) > 1: - # ctype = 'application/{0}'.format(parts[1]) - # if ctype in supported: - # return ctype - bm = self.accept.best_match(supported) - - # default to application/json if we don't find a preference return bm or 'application/json' def get_content_type(self, allowed_content_types): - """Determine content type of the request body. - - Does not do any body introspection, only checks header - - """ + """Determine content type of the request body.""" if not "Content-Type" in self.headers: raise exception.InvalidContentType(content_type=None) @@ -289,12 +235,15 @@ class JSONRequestDeserializer(object): :param request: Webob.Request object """ - return request.content_length or 'transfer-encoding' in request.headers + return (request.content_length and request.content_length > 0) \ + or 'transfer-encoding' in request.headers + + def from_json(self, datastring): + return json.loads(datastring) def default(self, request): - if 'Content-Length' in request.headers and \ - request.headers['Content-Length'] > 0: - return {'body': json.loads(request.body)} + if self.has_body(request): + return {'body': self.from_json(request.body)} else: return {} diff --git a/tests/unit/test_wsgi.py b/tests/unit/test_wsgi.py new file mode 100644 index 0000000000..20cd144511 --- /dev/null +++ b/tests/unit/test_wsgi.py @@ -0,0 +1,177 @@ +# vim: tabstop=4 shiftwidth=4 softtabstop=4 + +# Copyright 2010-2011 OpenStack, LLC +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import unittest + +from glance.common import wsgi +from glance.common import exception + + +class RequestTest(unittest.TestCase): + def test_content_type_missing(self): + request = wsgi.Request.blank('/tests/123') + request.body = "" + self.assertRaises(exception.InvalidContentType, + request.get_content_type, ('application/xml')) + + def test_content_type_unsupported(self): + request = wsgi.Request.blank('/tests/123') + request.headers["Content-Type"] = "text/html" + request.body = "asdf
" + self.assertRaises(exception.InvalidContentType, + request.get_content_type, ('application/xml')) + + def test_content_type_with_charset(self): + request = wsgi.Request.blank('/tests/123') + request.headers["Content-Type"] = "application/json; charset=UTF-8" + result = request.get_content_type(('application/json')) + self.assertEqual(result, "application/json") + + def test_content_type_from_accept_xml(self): + request = wsgi.Request.blank('/tests/123') + request.headers["Accept"] = "application/xml" + result = request.best_match_content_type() + self.assertEqual(result, "application/json") + + request = wsgi.Request.blank('/tests/123') + request.headers["Accept"] = "application/json" + result = request.best_match_content_type() + self.assertEqual(result, "application/json") + + request = wsgi.Request.blank('/tests/123') + request.headers["Accept"] = "application/xml, application/json" + result = request.best_match_content_type() + self.assertEqual(result, "application/json") + + request = wsgi.Request.blank('/tests/123') + request.headers["Accept"] = \ + "application/json; q=0.3, application/xml; q=0.9" + result = request.best_match_content_type() + self.assertEqual(result, "application/json") + + def test_content_type_accept_default(self): + request = wsgi.Request.blank('/tests/123.unsupported') + request.headers["Accept"] = "application/unsupported1" + result = request.best_match_content_type() + self.assertEqual(result, "application/json") + + +class ResourceTest(unittest.TestCase): + def test_get_action_args(self): + env = { + 'wsgiorg.routing_args': [ + None, + { + 'controller': None, + 'format': None, + 'action': 'update', + 'id': 12, + }, + ], + } + + expected = {'action': 'update', 'id': 12} + actual = wsgi.Resource(None, None, None).get_action_args(env) + + self.assertEqual(actual, expected) + + def test_dispatch(self): + class Controller(object): + def index(self, shirt, pants=None): + return (shirt, pants) + + resource = wsgi.Resource(None, None, None) + actual = resource.dispatch(Controller(), 'index', 'on', pants='off') + expected = ('on', 'off') + self.assertEqual(actual, expected) + + def test_dispatch_default(self): + class Controller(object): + def default(self, shirt, pants=None): + return (shirt, pants) + + resource = wsgi.Resource(None, None, None) + actual = resource.dispatch(Controller(), 'index', 'on', pants='off') + expected = ('on', 'off') + self.assertEqual(actual, expected) + + def test_dispatch_no_default(self): + class Controller(object): + def show(self, shirt, pants=None): + return (shirt, pants) + + resource = wsgi.Resource(None, None, None) + self.assertRaises(AttributeError, resource.dispatch, Controller(), + 'index', 'on', pants='off') + + +class JSONResponseSerializerTest(unittest.TestCase): + def test_to_json(self): + fixture = {"key": "value"} + expected = '{"key": "value"}' + actual = wsgi.JSONResponseSerializer().to_json(fixture) + self.assertEqual(actual, expected) + + def test_default(self): + fixture = {"key": "value"} + response = wsgi.JSONResponseSerializer().default(fixture) + self.assertEqual(response.status_int, 200) + self.assertEqual(response.content_type, 'application/json') + self.assertEqual(response.body, '{"key": "value"}') + + +class JSONRequestDeserializerTest(unittest.TestCase): + def test_has_body_no_content_length(self): + request = wsgi.Request.blank('/') + request.body = 'asdf' + request.headers.pop('Content-Length') + self.assertFalse(wsgi.JSONRequestDeserializer().has_body(request)) + + def test_has_body_zero_content_length(self): + request = wsgi.Request.blank('/') + request.body = 'asdf' + request.headers['Content-Length'] = 0 + self.assertFalse(wsgi.JSONRequestDeserializer().has_body(request)) + + def test_has_body_has_content_length(self): + request = wsgi.Request.blank('/') + request.body = 'asdf' + self.assertTrue('Content-Length' in request.headers) + self.assertTrue(wsgi.JSONRequestDeserializer().has_body(request)) + + def test_no_body_no_content_length(self): + request = wsgi.Request.blank('/') + self.assertFalse(wsgi.JSONRequestDeserializer().has_body(request)) + + def test_from_json(self): + fixture = '{"key": "value"}' + expected = {"key": "value"} + actual = wsgi.JSONRequestDeserializer().from_json(fixture) + self.assertEqual(actual, expected) + + def test_default_no_body(self): + request = wsgi.Request.blank('/') + actual = wsgi.JSONRequestDeserializer().default(request) + expected = {} + self.assertEqual(actual, expected) + + def test_default_with_body(self): + request = wsgi.Request.blank('/') + request.body = '{"key": "value"}' + actual = wsgi.JSONRequestDeserializer().default(request) + expected = {"body": {"key": "value"}} + self.assertEqual(actual, expected) From a07bfe68ac946df411bf076eb40cf5adb8de5d3b Mon Sep 17 00:00:00 2001 From: Brian Waldon Date: Sat, 11 Jun 2011 16:25:52 -0400 Subject: [PATCH 5/9] cleanup --- glance/api/v1/images.py | 57 ++++++++++++++++++++------------------- glance/registry/server.py | 29 +++++++------------- 2 files changed, 40 insertions(+), 46 deletions(-) diff --git a/glance/api/v1/images.py b/glance/api/v1/images.py index 068c2cacbf..2f4f9bf476 100644 --- a/glance/api/v1/images.py +++ b/glance/api/v1/images.py @@ -24,7 +24,7 @@ import json import logging import sys -from webob import Response +import webob from webob.exc import (HTTPNotFound, HTTPConflict, HTTPBadRequest) @@ -80,7 +80,7 @@ class Controller(object): * checksum -- MD5 checksum of the image data * size -- Size of image data in bytes - :param request: The WSGI/Webob Request object + :param req: The WSGI/Webob Request object :retval The response body is a mapping of the following form:: {'images': [ @@ -107,7 +107,7 @@ class Controller(object): """ Returns detailed information for all public, available images - :param request: The WSGI/Webob Request object + :param req: The WSGI/Webob Request object :retval The response body is a mapping of the following form:: {'images': [ @@ -155,8 +155,9 @@ class Controller(object): Returns metadata about an image in the HTTP headers of the response object - :param request: The WSGI/Webob Request object + :param req: The WSGI/Webob Request object :param id: The opaque image identifier + :retval similiar to 'show' method but without image_data :raises HTTPNotFound if image metadata is not available to user """ @@ -166,13 +167,10 @@ class Controller(object): def show(self, req, id): """ - Returns an iterator as a Response object that - can be used to retrieve an image's data. The - content-type of the response is the content-type - of the image, or application/octet-stream if none - is known or found. + Returns an iterator that can be used to retrieve an image's + data along with the image metadata. - :param request: The WSGI/Webob Request object + :param req: The WSGI/Webob Request object :param id: The opaque image identifier :raises HTTPNotFound if image is not available to user @@ -196,9 +194,9 @@ class Controller(object): """ Adds the image metadata to the registry and assigns an image identifier if one is not supplied in the request - headers. Sets the image's status to `queued` + headers. Sets the image's status to `queued`. - :param request: The WSGI/Webob Request object + :param req: The WSGI/Webob Request object :param id: The opaque image identifier :raises HTTPConflict if image already exists @@ -239,7 +237,7 @@ class Controller(object): will attempt to use that store, if not, Glance will use the store set by the flag `default_store`. - :param request: The WSGI/Webob Request object + :param req: The WSGI/Webob Request object :param image_meta: Mapping of metadata about image :raises HTTPConflict if image already exists @@ -252,8 +250,8 @@ class Controller(object): logger.error(msg) raise HTTPBadRequest(msg) - store_name = req.headers.get( - 'x-image-meta-store', self.options['default_store']) + store_name = req.headers.get('x-image-meta-store', + self.options['default_store']) store = self.get_store_or_400(req, store_name) @@ -309,8 +307,8 @@ class Controller(object): Sets the image status to `active` and the image's location attribute. - :param request: The WSGI/Webob Request object - :param image_meta: Mapping of metadata about image + :param req: The WSGI/Webob Request object + :param image_id: Opaque image identifier :param location: Location of where Glance stored this image """ image_meta = {} @@ -322,9 +320,9 @@ class Controller(object): def _kill(self, req, image_id): """ - Marks the image status to `killed` + Marks the image status to `killed`. - :param request: The WSGI/Webob Request object + :param req: The WSGI/Webob Request object :param image_id: Opaque image identifier """ registry.update_image_metadata(self.options, @@ -338,7 +336,7 @@ class Controller(object): Since _kill is meant to be called from exceptions handlers, it should not raise itself, rather it should just log its error. - :param request: The WSGI/Webob Request object + :param req: The WSGI/Webob Request object :param image_id: Opaque image identifier """ try: @@ -353,7 +351,7 @@ class Controller(object): and activates the image in the registry after a successful upload. - :param request: The WSGI/Webob Request object + :param req: The WSGI/Webob Request object :param image_meta: Mapping of metadata about image :retval Mapping of updated image data @@ -388,7 +386,9 @@ class Controller(object): containing metadata about the image is returned, including its opaque identifier. - :param request: The WSGI/Webob Request object + :param req: The WSGI/Webob Request object + :param image_meta: Mapping of metadata about image + :param image_data: Actual image data that is to be stored :raises HTTPBadRequest if x-image-meta-location is missing and the request body is not application/octet-stream @@ -439,7 +439,7 @@ class Controller(object): """ Deletes the image and all its chunks from the Glance - :param request: The WSGI/Webob Request object + :param req: The WSGI/Webob Request object :param id: The opaque image identifier :raises HttpBadRequest if image registry is invalid @@ -502,6 +502,7 @@ class Controller(object): class ImageDeserializer(wsgi.JSONRequestDeserializer): + """Handles deserialization of specific controller method requests.""" def create(self, request): result = {} @@ -519,6 +520,7 @@ class ImageDeserializer(wsgi.JSONRequestDeserializer): class ImageSerializer(wsgi.JSONResponseSerializer): + """Handles serialization of specific controller method responses.""" def _inject_location_header(self, response, image_meta): location = self._get_image_location(image_meta) @@ -545,11 +547,12 @@ class ImageSerializer(wsgi.JSONResponseSerializer): response.headers.add(k, v) def _get_image_location(self, image_meta): + """Build a relative url to reach the image defined by image_meta.""" return "/v1/images/%s" % image_meta['id'] def meta(self, result): image_meta = result['image_meta'] - response = Response() + response = webob.Response() self._inject_image_meta_headers(response, image_meta) self._inject_location_header(response, image_meta) self._inject_checksum_header(response, image_meta) @@ -558,7 +561,7 @@ class ImageSerializer(wsgi.JSONResponseSerializer): def show(self, result): image_meta = result['image_meta'] - response = Response(app_iter=result['image_iterator']) + response = webob.Response(app_iter=result['image_iterator']) # Using app_iter blanks content-length, so we set it here... response.headers.add('Content-Length', image_meta['size']) response.headers.add('Content-Type', 'application/octet-stream') @@ -571,7 +574,7 @@ class ImageSerializer(wsgi.JSONResponseSerializer): def update(self, result): image_meta = result['image_meta'] - response = Response() + response = webob.Response() response.body = self.to_json(dict(image=image_meta)) response.headers.add('Content-Type', 'application/json') self._inject_location_header(response, image_meta) @@ -580,7 +583,7 @@ class ImageSerializer(wsgi.JSONResponseSerializer): def create(self, result): image_meta = result['image_meta'] - response = Response() + response = webob.Response() response.status = httplib.CREATED response.headers.add('Content-Type', 'application/json') response.body = self.to_json(dict(image=image_meta)) diff --git a/glance/registry/server.py b/glance/registry/server.py index de572130e8..104c07ff17 100644 --- a/glance/registry/server.py +++ b/glance/registry/server.py @@ -49,7 +49,7 @@ class Controller(object): self.options = options db_api.configure_db(options) - def index(self, req, headers=None, body=None): + def index(self, req): """Return a basic filtered list of public, non-deleted images :param req: the Request object coming from the wsgi layer @@ -87,7 +87,7 @@ class Controller(object): results.append(result) return dict(images=results) - def detail(self, req, headers=None, body=None): + def detail(self, req): """Return a filtered list of public, non-deleted images in detail :param req: the Request object coming from the wsgi layer @@ -154,7 +154,7 @@ class Controller(object): raise exc.HTTPBadRequest("marker param must be an integer") return marker - def show(self, req, id, headers=None, body=None): + def show(self, req, id): """Return data about the given image id.""" try: image = db_api.image_get(None, id) @@ -167,7 +167,7 @@ class Controller(object): """ Deletes an existing image with the registry. - :param req: Request body. Ignored. + :param req: wsgi Request object :param id: The opaque internal identifier for the image :retval Returns 200 if delete was successful, a fault if not. @@ -179,12 +179,12 @@ class Controller(object): except exception.NotFound: return exc.HTTPNotFound() - def create(self, req, headers=None, body=None): + def create(self, req, body): """ Registers a new image with the registry. - :param req: Request body. A JSON-ified dict of information about - the image. + :param req: wsgi Request object + :param body: Dictionary of information about the image :retval Returns the newly-created image information as a mapping, which will include the newly-created image's internal id @@ -209,12 +209,11 @@ class Controller(object): logger.error(msg) return exc.HTTPBadRequest(msg) - def update(self, req, id, headers=None, body=None): + def update(self, req, id, body): """Updates an existing image with the registry. - :param req: Request body. A JSON-ified dict of information about - the image. This will replace the information in the - registry about this image + :param req: wsgi Request object + :param body: Dictionary of information about the image :param id: The opaque internal identifier for the image :retval Returns the updated image information as a mapping, @@ -244,14 +243,6 @@ class Controller(object): content_type='text/plain') -class ImageSerializer(wsgi.JSONResponseSerializer): - pass - - -class ImageDeserializer(wsgi.JSONRequestDeserializer): - pass - - def create_resource(options): deserializer = wsgi.JSONRequestDeserializer() serializer = wsgi.JSONResponseSerializer() From 31da4d68fe90a18a95d0d35914245f7f7c66b2b0 Mon Sep 17 00:00:00 2001 From: Brian Waldon Date: Mon, 13 Jun 2011 15:45:18 -0400 Subject: [PATCH 6/9] fixed test case --- glance/api/v1/images.py | 1 + tests/functional/test_curl_api.py | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/glance/api/v1/images.py b/glance/api/v1/images.py index 2f4f9bf476..fb6a2a9e13 100644 --- a/glance/api/v1/images.py +++ b/glance/api/v1/images.py @@ -246,6 +246,7 @@ class Controller(object): try: req.get_content_type('application/octet-stream') except exception.InvalidContentType: + self._safe_kill(req, image_meta['id']) msg = "Content-Type must be application/octet-stream" logger.error(msg) raise HTTPBadRequest(msg) diff --git a/tests/functional/test_curl_api.py b/tests/functional/test_curl_api.py index 1feae155fd..5adf918d1a 100644 --- a/tests/functional/test_curl_api.py +++ b/tests/functional/test_curl_api.py @@ -752,7 +752,7 @@ class TestCurlApi(functional.FunctionalTest): self.stop_servers() - def _test_traceback_not_consumed(self): + def test_traceback_not_consumed(self): """ A test that errors coming from the POST API do not get consumed and print the actual error message, and @@ -773,7 +773,7 @@ class TestCurlApi(functional.FunctionalTest): with tempfile.NamedTemporaryFile() as test_data_file: test_data_file.write("XXX") test_data_file.flush() - cmd = ("curl -i -X POST --upload-file %s " + cmd = ("curl -i -X POST --upload-file %s -H 'Expect: ' " "http://0.0.0.0:%d/v1/images") % (test_data_file.name, api_port) From 73bc57f13cb97d19a224c5b32cbcb1b720542870 Mon Sep 17 00:00:00 2001 From: Brian Waldon Date: Wed, 15 Jun 2011 10:47:00 -0400 Subject: [PATCH 7/9] initial refactoring from Jay's comments --- glance/api/v1/images.py | 38 ++++++++++++++++++-------------------- glance/common/exception.py | 4 ++-- glance/common/wsgi.py | 21 +++++++++++---------- glance/registry/server.py | 3 ++- tests/unit/test_wsgi.py | 4 +++- 5 files changed, 36 insertions(+), 34 deletions(-) diff --git a/glance/api/v1/images.py b/glance/api/v1/images.py index fb6a2a9e13..f47e8f07bc 100644 --- a/glance/api/v1/images.py +++ b/glance/api/v1/images.py @@ -157,7 +157,7 @@ class Controller(object): :param req: The WSGI/Webob Request object :param id: The opaque image identifier - :retval similiar to 'show' method but without image_data + :retval similar to 'show' method but without image_data :raises HTTPNotFound if image metadata is not available to user """ @@ -202,8 +202,9 @@ class Controller(object): :raises HTTPConflict if image already exists :raises HTTPBadRequest if image metadata is not valid """ - if 'location' in image_meta and image_meta['location'] is not None: - store = get_store_from_location(image_meta['location']) + location = image_meta.get('location') + if location: + store = get_store_from_location(location) # check the store exists before we hit the registry, but we # don't actually care what it is at this point self.get_store_or_400(req, store) @@ -401,8 +402,8 @@ class Controller(object): if image_data is not None: image_meta = self._upload_and_activate(req, image_meta) else: - if 'location' in image_meta and image_meta['location'] is not None: - location = image_meta['location'] + location = image_meta.get('location') + if location: image_meta = self._activate(req, image_id, location) return {'image_meta': image_meta} @@ -505,19 +506,18 @@ class Controller(object): class ImageDeserializer(wsgi.JSONRequestDeserializer): """Handles deserialization of specific controller method requests.""" - def create(self, request): + def _deserialize(self, request): result = {} result['image_meta'] = utils.get_image_meta_from_headers(request) data = request.body if self.has_body(request) else None result['image_data'] = data return result + def create(self, request): + return self._deserialize(request) + def update(self, request): - result = {} - result['image_meta'] = utils.get_image_meta_from_headers(request) - data = request.body if self.has_body(request) else None - result['image_data'] = data - return result + return self._deserialize(request) class ImageSerializer(wsgi.JSONResponseSerializer): @@ -551,18 +551,17 @@ class ImageSerializer(wsgi.JSONResponseSerializer): """Build a relative url to reach the image defined by image_meta.""" return "/v1/images/%s" % image_meta['id'] - def meta(self, result): + def meta(self, response, result): image_meta = result['image_meta'] - response = webob.Response() self._inject_image_meta_headers(response, image_meta) self._inject_location_header(response, image_meta) self._inject_checksum_header(response, image_meta) return response - def show(self, result): + def show(self, response, result): image_meta = result['image_meta'] - response = webob.Response(app_iter=result['image_iterator']) + response.app_iter = result['image_iterator'] # Using app_iter blanks content-length, so we set it here... response.headers.add('Content-Length', image_meta['size']) response.headers.add('Content-Type', 'application/octet-stream') @@ -573,18 +572,16 @@ class ImageSerializer(wsgi.JSONResponseSerializer): return response - def update(self, result): + def update(self, response, result): image_meta = result['image_meta'] - response = webob.Response() response.body = self.to_json(dict(image=image_meta)) response.headers.add('Content-Type', 'application/json') self._inject_location_header(response, image_meta) self._inject_checksum_header(response, image_meta) return response - def create(self, result): + def create(self, response, result): image_meta = result['image_meta'] - response = webob.Response() response.status = httplib.CREATED response.headers.add('Content-Type', 'application/json') response.body = self.to_json(dict(image=image_meta)) @@ -594,6 +591,7 @@ class ImageSerializer(wsgi.JSONResponseSerializer): def create_resource(options): + """Images resource factory method""" deserializer = ImageDeserializer() serializer = ImageSerializer() - return wsgi.Resource(deserializer, Controller(options), serializer) + return wsgi.Resource(Controller(options), deserializer, serializer) diff --git a/glance/common/exception.py b/glance/common/exception.py index 733f5ba310..e2ad116f2f 100644 --- a/glance/common/exception.py +++ b/glance/common/exception.py @@ -99,12 +99,12 @@ def wrap_exception(f): class GlanceException(Exception): - """Base Nova Exception + """ + Base Glance Exception To correctly use this class, inherit from it and define a 'message' property. That message will get printf'd with the keyword arguments provided to the constructor. - """ message = "An unknown exception occurred" diff --git a/glance/common/wsgi.py b/glance/common/wsgi.py index ac572c3a7b..2b116aec0e 100644 --- a/glance/common/wsgi.py +++ b/glance/common/wsgi.py @@ -25,6 +25,7 @@ import json import logging import sys import datetime + import eventlet import eventlet.wsgi eventlet.patcher.monkey_patch(all=False, socket=True) @@ -258,15 +259,14 @@ class JSONResponseSerializer(object): return json.dumps(data, default=sanitizer) - def default(self, result): - response = webob.Response() + def default(self, response, result): response.headers.add('Content-Type', 'application/json') response.body = self.to_json(result) - return response class Resource(object): - """WSGI app that handles (de)serialization and controller dispatch. + """ + WSGI app that handles (de)serialization and controller dispatch. WSGI app that reads routing information supplied by RoutesMiddleware and calls the requested action method upon its controller. All @@ -275,14 +275,14 @@ class Resource(object): method must also accept a 'body' argument (the deserialized request body). They may raise a webob.exc exception or return a dict, which will be serialized by requested content type. - """ - def __init__(self, deserializer, controller, serializer): + def __init__(self, controller, deserializer, serializer): """ :param controller: object that implement methods created by routes lib - :param serializers: dict of content-type specific text serializers - :param deserializers: dict of content-type specific text deserializers - + :param deserializer: object that supports webob request deserialization + through controller-like actions + :param serializer: object that supports webob response serialization + through controller-like actions """ self.controller = controller self.serializer = serializer @@ -304,7 +304,8 @@ class Resource(object): #TODO(bcwaldon): find a more elegant way to pass through non-dict types if type(action_result) is dict: - response = self.dispatch(self.serializer, action, action_result) + response = webob.Response() + self.dispatch(self.serializer, action, response, action_result) else: response = action_result diff --git a/glance/registry/server.py b/glance/registry/server.py index 104c07ff17..f6501ed3ec 100644 --- a/glance/registry/server.py +++ b/glance/registry/server.py @@ -244,9 +244,10 @@ class Controller(object): def create_resource(options): + """Images resource factory method.""" deserializer = wsgi.JSONRequestDeserializer() serializer = wsgi.JSONResponseSerializer() - return wsgi.Resource(deserializer, Controller(options), serializer) + return wsgi.Resource(Controller(options), deserializer, serializer) class API(wsgi.Router): diff --git a/tests/unit/test_wsgi.py b/tests/unit/test_wsgi.py index 20cd144511..bdab5d1e68 100644 --- a/tests/unit/test_wsgi.py +++ b/tests/unit/test_wsgi.py @@ -16,6 +16,7 @@ # under the License. import unittest +import webob from glance.common import wsgi from glance.common import exception @@ -128,7 +129,8 @@ class JSONResponseSerializerTest(unittest.TestCase): def test_default(self): fixture = {"key": "value"} - response = wsgi.JSONResponseSerializer().default(fixture) + response = webob.Response() + wsgi.JSONResponseSerializer().default(response, fixture) self.assertEqual(response.status_int, 200) self.assertEqual(response.content_type, 'application/json') self.assertEqual(response.body, '{"key": "value"}') From e8bd48ea70e59c426d8434cfad8795cdf3f807c5 Mon Sep 17 00:00:00 2001 From: Brian Waldon Date: Thu, 16 Jun 2011 21:04:36 -0400 Subject: [PATCH 8/9] refactoring from Rick's comments --- glance/common/wsgi.py | 39 ++++++++++++++++++++++++--------------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/glance/common/wsgi.py b/glance/common/wsgi.py index 2b116aec0e..b74541deb4 100644 --- a/glance/common/wsgi.py +++ b/glance/common/wsgi.py @@ -236,8 +236,12 @@ class JSONRequestDeserializer(object): :param request: Webob.Request object """ - return (request.content_length and request.content_length > 0) \ - or 'transfer-encoding' in request.headers + if 'transfer-encoding' in request.headers: + return True + elif request.content_length > 0: + return True + + return False def from_json(self, datastring): return json.loads(datastring) @@ -268,12 +272,17 @@ class Resource(object): """ WSGI app that handles (de)serialization and controller dispatch. - WSGI app that reads routing information supplied by RoutesMiddleware - and calls the requested action method upon its controller. All - controller action methods must accept a 'req' argument, which is the - incoming wsgi.Request. If the operation is a PUT or POST, the controller - method must also accept a 'body' argument (the deserialized request body). - They may raise a webob.exc exception or return a dict, which will be + Reads routing information supplied by RoutesMiddleware and calls + the requested action method upon its deserializer, controller, + and serializer. Those three objects may implement any of the basic + controller action methods (create, update, show, index, delete) + along with any that may be specified in the api router. A 'default' + method may also be implemented to be used in place of any + non-implemented actions. Deserializer methods must accept a request + argument and return a dictionary. Controller methods must accept a + request argument. Additionally, they must also accept keyword + arguments that represent the keys returned by the Deserializer. They + may raise a webob.exc exception or return a dict, which will be serialized by requested content type. """ def __init__(self, controller, deserializer, serializer): @@ -302,20 +311,20 @@ class Resource(object): action_result = self.dispatch(self.controller, action, request, **action_args) - #TODO(bcwaldon): find a more elegant way to pass through non-dict types - if type(action_result) is dict: + try: response = webob.Response() self.dispatch(self.serializer, action, response, action_result) - else: - response = action_result + return response - return response + # return unserializable result (typically a webob exc) + except Exception: + return action_result def dispatch(self, obj, action, *args, **kwargs): - """Find action-spefic method on self and call it.""" + """Find action-specific method on self and call it.""" try: method = getattr(obj, action) - except Exception: + except AttributeError: method = getattr(obj, 'default') return method(*args, **kwargs) From 7ff3587f2bf46182e6000194b0365781e8484ee2 Mon Sep 17 00:00:00 2001 From: Brian Waldon Date: Fri, 17 Jun 2011 16:45:45 -0400 Subject: [PATCH 9/9] refactoring for Brian --- glance/api/v1/images.py | 2 +- glance/common/wsgi.py | 2 -- tests/unit/test_wsgi.py | 3 +++ 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/glance/api/v1/images.py b/glance/api/v1/images.py index f47e8f07bc..4c9bb8092e 100644 --- a/glance/api/v1/images.py +++ b/glance/api/v1/images.py @@ -250,7 +250,7 @@ class Controller(object): self._safe_kill(req, image_meta['id']) msg = "Content-Type must be application/octet-stream" logger.error(msg) - raise HTTPBadRequest(msg) + raise HTTPBadRequest(explanation=msg) store_name = req.headers.get('x-image-meta-store', self.options['default_store']) diff --git a/glance/common/wsgi.py b/glance/common/wsgi.py index b74541deb4..bbb8479c94 100644 --- a/glance/common/wsgi.py +++ b/glance/common/wsgi.py @@ -300,7 +300,6 @@ class Resource(object): @webob.dec.wsgify(RequestClass=Request) def __call__(self, request): """WSGI method that controls (de)serialization and method dispatch.""" - action_args = self.get_action_args(request.environ) action = action_args.pop('action', None) @@ -310,7 +309,6 @@ class Resource(object): action_result = self.dispatch(self.controller, action, request, **action_args) - try: response = webob.Response() self.dispatch(self.serializer, action, response, action_result) diff --git a/tests/unit/test_wsgi.py b/tests/unit/test_wsgi.py index bdab5d1e68..248bb01483 100644 --- a/tests/unit/test_wsgi.py +++ b/tests/unit/test_wsgi.py @@ -48,16 +48,19 @@ class RequestTest(unittest.TestCase): result = request.best_match_content_type() self.assertEqual(result, "application/json") + def test_content_type_from_accept_json(self): request = wsgi.Request.blank('/tests/123') request.headers["Accept"] = "application/json" result = request.best_match_content_type() self.assertEqual(result, "application/json") + def test_content_type_from_accept_xml_json(self): request = wsgi.Request.blank('/tests/123') request.headers["Accept"] = "application/xml, application/json" result = request.best_match_content_type() self.assertEqual(result, "application/json") + def test_content_type_from_accept_json_xml_quality(self): request = wsgi.Request.blank('/tests/123') request.headers["Accept"] = \ "application/json; q=0.3, application/xml; q=0.9"