- refactoring wsgi code to divide deserialization, controller, serialization among different objects

- Resource object acts as coordinator

- tests are coming, this is for review purposes

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)
+        resource = images.create_resource(options)
-        mapper.resource("image", "images", controller=controller,
+        mapper.resource("image", "images", controller=resource,
                        collection={'detail': 'GET'})
-        mapper.connect("/", controller=controller, action="index")
+        mapper.connect("/", controller=resource, action="index")
-        mapper.connect("/images/{id}", controller=controller, action="meta",
+        mapper.connect("/images/{id}", controller=resource, action="meta",
                        conditions=dict(method=["HEAD"]))
         super(API, self).__init__(mapper)
 

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)
@@ -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
@@ -80,7 +80,7 @@ class Controller(wsgi.Controller):
             * 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(wsgi.Controller):
         """
         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,29 +155,22 @@ class Controller(wsgi.Controller):
         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 similar to 'show' method but without image_data
 
         :raises HTTPNotFound if image metadata is not available to user
         """
-        image = self.get_image_meta_or_404(req, id)
+        return {
-
+            'image_meta': 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)
 
     def show(self, req, id):
         """
-        Returns an iterator as a Response object that
+        Returns an iterator that can be used to retrieve an image's
-        can be used to retrieve an image's data. The
+        data along with the image metadata.
-        content-type of the response is the content-type
-        of the image, or application/octet-stream if none
-        is known or found.
 
-        :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
@@ -192,31 +185,26 @@ class Controller(wsgi.Controller):
             for chunk in chunks:
                 yield chunk
 
-        res = Response(app_iter=image_iterator(),
+        return {
-                       content_type="application/octet-stream")
+            'image_iterator': image_iterator(),
-        # Using app_iter blanks content-length, so we set it here...
+            'image_meta': image,
-        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)
 
-    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
-        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
         :raises HTTPBadRequest if image metadata is not valid
         """
-        image_meta = utils.get_image_meta_from_headers(req)
+        location = image_meta.get('location')
-
+        if location:
-        if 'location' in image_meta:
+            store = get_store_from_location(location)
-            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
             self.get_store_or_400(req, store)
@@ -250,28 +238,27 @@ class Controller(wsgi.Controller):
         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
         :retval The location where the image was stored
         """
-        image_id = image_meta['id']
+        try:
-        content_type = req.headers.get('content-type', 'notset')
+            req.get_content_type('application/octet-stream')
-        if content_type != 'application/octet-stream':
+        except exception.InvalidContentType:
-            self._safe_kill(req, image_id)
+            self._safe_kill(req, image_meta['id'])
-            msg = ("Content-Type must be application/octet-stream")
+            msg = "Content-Type must be application/octet-stream"
             logger.error(msg)
-            raise HTTPBadRequest(msg, content_type="text/plain",
+            raise HTTPBadRequest(explanation=msg)
-                                 request=req)
 
-        store_name = req.headers.get(
+        store_name = req.headers.get('x-image-meta-store',
-            'x-image-meta-store', self.options['default_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_meta['id']
-                     % image_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)
@@ -320,8 +309,8 @@ class Controller(wsgi.Controller):
         Sets the image status to `active` and the image's location
         attribute.
 
-        :param request: The WSGI/Webob Request object
+        :param req: The WSGI/Webob Request object
-        :param image_meta: Mapping of metadata about image
+        :param image_id: Opaque image identifier
         :param location: Location of where Glance stored this image
         """
         image_meta = {}
@@ -333,9 +322,9 @@ class Controller(wsgi.Controller):
 
     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,
@@ -349,7 +338,7 @@ class Controller(wsgi.Controller):
         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:
@@ -364,7 +353,7 @@ class Controller(wsgi.Controller):
         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
@@ -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:
@@ -399,32 +388,27 @@ class Controller(wsgi.Controller):
         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 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 = image_meta.get('location')
-                location = req.headers['x-image-meta-location']
+            if location:
                 image_meta = self._activate(req, image_id, location)
 
-        # APP states we should return a Location: header with the edit
+        return {'image_meta': image_meta}
-        # 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 req.get_response(res)
+    def update(self, req, id, image_meta, image_data):
-
-    def update(self, req, id):
         """
         Updates an existing image with the registry.
 
@@ -433,29 +417,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,
+            image_meta = registry.update_image_metadata(self.options, id,
-                                                        id,
+                                                         image_meta, True)
-                                                        new_image_meta,
+            if image_data is not None:
-                                                        True)
-            if has_body:
                 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,11 +435,13 @@ 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
 
-        :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
@@ -527,3 +501,97 @@ class Controller(wsgi.Controller):
             logger.error(msg)
             raise HTTPBadRequest(msg, request=request,
                                  content_type='text/plain')
+
+
+class ImageDeserializer(wsgi.JSONRequestDeserializer):
+    """Handles deserialization of specific controller method requests."""
+
+    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):
+        return self._deserialize(request)
+
+
+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)
+        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-<FIELD>' except
+        for the properties field, which is further broken out
+        into a set of 'x-image-meta-property-<KEY>' 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):
+        """Build a relative url to reach the image defined by image_meta."""
+        return "/v1/images/%s" % image_meta['id']
+
+    def meta(self, response, result):
+        image_meta = result['image_meta']
+        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, response, result):
+        image_meta = result['image_meta']
+
+        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, response, result):
+        image_meta = result['image_meta']
+        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, response, result):
+        image_meta = result['image_meta']
+        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):
+    """Images resource factory method"""
+    deserializer = ImageDeserializer()
+    serializer = ImageSerializer()
+    return wsgi.Resource(Controller(options), deserializer, serializer)

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 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"
+
+    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"

glance/common/wsgi.py

@@ -34,6 +34,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."""
@@ -205,73 +207,55 @@ class Router(object):
         return app
 
 
-class Controller(object):
+class Request(webob.Request):
-    """
+    """Add some Openstack API-specific logic to the base webob.Request."""
-    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 best_match_content_type(self):
-    def __call__(self, req):
+        """Determine the requested response content-type."""
-        """
+        supported = ('application/json',)
-        Call the method specified in req.environ by RoutesMiddleware.
+        bm = self.accept.best_match(supported)
-        """
+        return bm or 'application/json'
-        arg_dict = req.environ['wsgiorg.routing_args'][1]
+
-        action = arg_dict['action']
+    def get_content_type(self, allowed_content_types):
-        method = getattr(self, action)
+        """Determine content type of the request body."""
-        del arg_dict['controller']
+        if not "Content-Type" in self.headers:
-        del arg_dict['action']
+            raise exception.InvalidContentType(content_type=None)
-        arg_dict['req'] = req
+
-        result = method(**arg_dict)
+        content_type = self.content_type
-        if type(result) is dict:
+
-            return self._serialize(result, req)
+        if content_type not in allowed_content_types:
+            raise exception.InvalidContentType(content_type=content_type)
         else:
-            return result
+            return content_type
-
-    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 Serializer(object):
+class JSONRequestDeserializer(object):
-    """
+    def has_body(self, request):
-    Serializes a dictionary to a Content Type specified by a WSGI environment.
+        """
-    """
+        Returns whether a Webob.Request object will possess an entity body.
 
-    def __init__(self, environ, metadata=None):
+        :param request:  Webob.Request object
         """
-        Create a serializer based on the given WSGI environment.
+        if 'transfer-encoding' in request.headers:
-        'metadata' is an optional dict mapping MIME types to information
+            return True
-        needed to serialize a dictionary to that type.
+        elif request.content_length > 0:
-        """
+            return True
-        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):
+        return False
-        """
-        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 from_json(self, datastring):
+        return json.loads(datastring)
+
+    def default(self, request):
+        if self.has_body(request):
+            return {'body': self.from_json(request.body)}
+        else:
+            return {}
+
+
+class JSONResponseSerializer(object):
+
+    def to_json(self, data):
         def sanitizer(obj):
             if isinstance(obj, datetime.datetime):
                 return obj.isoformat()
@@ -279,37 +263,85 @@ class Serializer(object):
 
         return json.dumps(data, default=sanitizer)
 
-    def _to_xml(self, data):
+    def default(self, response, result):
-        metadata = self.metadata.get('application/xml', {})
+        response.headers.add('Content-Type', 'application/json')
-        # We expect data to contain a single key which is the XML root.
+        response.body = self.to_json(result)
-        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."""
+class Resource(object):
-        result = doc.createElement(nodename)
+    """
-        if type(data) is list:
+    WSGI app that handles (de)serialization and controller dispatch.
-            singular = metadata.get('plurals', {}).get(nodename, None)
+
-            if singular is None:
+    Reads routing information supplied by RoutesMiddleware and calls
-                if nodename.endswith('s'):
+    the requested action method upon its deserializer, controller,
-                    singular = nodename[:-1]
+    and serializer. Those three objects may implement any of the basic
-                else:
+    controller action methods (create, update, show, index, delete)
-                    singular = 'item'
+    along with any that may be specified in the api router. A 'default'
-            for item in data:
+    method may also be implemented to be used in place of any
-                node = self._to_xml_node(doc, metadata, singular, item)
+    non-implemented actions. Deserializer methods must accept a request
-                result.appendChild(node)
+    argument and return a dictionary. Controller methods must accept a
-        elif type(data) is dict:
+    request argument. Additionally, they must also accept keyword
-            attrs = metadata.get('attributes', {}).get(nodename, {})
+    arguments that represent the keys returned by the Deserializer. They
-            for k, v in data.items():
+    may raise a webob.exc exception or return a dict, which will be
-                if k in attrs:
+    serialized by requested content type.
-                    result.setAttribute(k, str(v))
+    """
-                else:
+    def __init__(self, controller, deserializer, serializer):
-                    node = self._to_xml_node(doc, metadata, k, v)
+        """
-                    result.appendChild(node)
+        :param controller: object that implement methods created by routes lib
-        else:  # atom
+        :param deserializer: object that supports webob request deserialization
-            node = doc.createTextNode(str(data))
+                             through controller-like actions
-            result.appendChild(node)
+        :param serializer: object that supports webob response serialization
-        return result
+                           through controller-like actions
+        """
+        self.controller = controller
+        self.serializer = serializer
+        self.deserializer = deserializer
+
+    @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)
+
+        deserialized_request = self.dispatch(self.deserializer,
+                                             action, request)
+        action_args.update(deserialized_request)
+
+        action_result = self.dispatch(self.controller, action,
+                                      request, **action_args)
+        try:
+            response = webob.Response()
+            self.dispatch(self.serializer, action, response, action_result)
+            return response
+
+        # return unserializable result (typically a webob exc)
+        except Exception:
+            return action_result
+
+    def dispatch(self, obj, action, *args, **kwargs):
+        """Find action-specific method on self and call it."""
+        try:
+            method = getattr(obj, action)
+        except AttributeError:
+            method = getattr(obj, 'default')
+
+        return method(*args, **kwargs)
+
+    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

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']

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):
@@ -167,7 +167,7 @@ class Controller(wsgi.Controller):
         """
         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,19 +179,19 @@ 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.
 
-        :param req: Request body.  A JSON-ified dict of information about
+        :param req: wsgi Request object
-                    the image.
+        :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
                 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,18 +209,17 @@ 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
+        :param req: wsgi Request object
-                    the image.  This will replace the information in the
+        :param body: Dictionary of information about the image
-                    registry about this image
         :param id:  The opaque internal identifier for the image
 
         :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 +243,22 @@ class Controller(wsgi.Controller):
                                content_type='text/plain')
 
 
+def create_resource(options):
+    """Images resource factory method."""
+    deserializer = wsgi.JSONRequestDeserializer()
+    serializer = wsgi.JSONResponseSerializer()
+    return wsgi.Resource(Controller(options), deserializer, serializer)
+
+
 class API(wsgi.Router):
     """WSGI entry point for all Registry requests."""
 
     def __init__(self, options):
         mapper = routes.Mapper()
-        controller = Controller(options)
+        resource = create_resource(options)
-        mapper.resource("image", "images", controller=controller,
+        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)
 
 

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-<FIELD>' except
-    for the properties field, which is further broken out
-    into a set of 'x-image-meta-property-<KEY>' 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

tests/functional/test_curl_api.py

@@ -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)
 

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)
@@ -972,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',

tests/unit/test_wsgi.py

@@ -0,0 +1,182 @@
+# 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
+import webob
+
+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 = "<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<br />"
+        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")
+
+    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"
+        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 = 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"}')
+
+
+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)