Merge lp:~johannes.erdfelt/nova/lp844905 into lp:~hudson-openstack/nova/trunk

[Johannes Erdfelt]

The 1.1 API specifies that the API version can be determined by URL path (eg /v1.1/tenant/servers/detail), Content-Type header (eg application/json;version=1.1) or Accept header (eg application/json;q=0.8;version=1.1, application/xml;q=0.2;version=1.1).

This branch adds support for looking at the Content-Type and Accept headers for the API version to use.

The 1.1 API specifies that the API version can be determined by URL path (eg /v1.1/tenant/servers/detail), Content-Type header (eg application/json;version=1.1) or Accept header (eg application/json;q=0.8;version=1.1, application/xml;q=0.2;version=1.1).

This branch adds support for looking at the Content-Type and Accept headers for the API version to use.

This merge proposal is more of a code review than a claim than the branch is ready. I'd like to hear opinions on the direction of the branch.

In particular:
* The approach I took using a custom URLMap class to integrate into paste deploy. It has the benefit of still allowing the paste config to enable/disable API versions. It subclasses a paste class, so it may end up being fragile in the long term.
* To support this correctly, the accepted content type needs to be determined much earlier than before (so we get the right version specified by the MIME type in the Accept header). Unfortunately, there are a couple of resources that define an additional supported content type of application/atom+xml. Since we determine the accepted content type before we call the resource, this leaves us in a difficult position. The branch semi-hard codes it. If anyone has a better idea, I'm open for discussion.
* Much of the current code tests like to skip the paste deploy part of the WSGI stack. Also the direct API doesn't use paste deploy. As a result, I left in code to determine the accepted content type at a later point. I originally went down the path of changing the rest of the code, but that resulted in a set of huge changes, but also some corner cases where only part of the WSGI stack is intentionally tested.

[lp:~johannes.erdfelt/nova/lp844905]

1588. By Johannes Erdfelt on 2011-09-21

Remove hardcoded content type now that the code handles direct API again

1589. By Johannes Erdfelt on 2011-09-21

Remove FIXME now that it has a workaround

1590. By Johannes Erdfelt on 2011-09-21

Cleanup docstrings
Remove is_filename option, it's not necessary for us and is a hack anyway

[Sandy Walsh (sandy-walsh)]

Finding it hard to see that the new code is being properly covered by the tests.

Ideally I'd like to see explicit unit tests on each of the new methods added (and ensuring they hit each of the conditional branches).

At the very least, some description in the unit tests as to what they are actually testing and which methods are being touched. But ideally, simple single-method unit tests.

[Johannes Erdfelt (johannes.erdfelt)]

I'm not sure I understand what needs to be changed.

The unit tests check the two different methods to determine the best content type and the three different methods to determine the API version.

Each unit test checks a single piece of functionality, so I'm not sure I understand what you mean by "single-method unit tests".

Are you saying some code isn't being tested? The existing code being tested isn't tested well enough?

[lp:~johannes.erdfelt/nova/lp844905]

1591. By Johannes Erdfelt on 2011-09-21

Add docstrings to tests to better document the intent

[Paul Voccio (pvo)]

Wanted to echo removing comments and some of the formatting the others mentioned here.

I got some 3 failures with the S3APITestCase. Not sure if this is related or not yet.

Please also add yourself to the Authors file. This failed on the unit tests for me.

======================================================================
FAIL: test_authors_up_to_date (nova.tests.test_misc.ProjectTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/root/bzr/TrustedComputingPools/nova/tests/test_misc.py", line 93, in test_authors_up_to_date
    '%r not listed in Authors' % missing)
AssertionError: set([u'<email address hidden>']) not listed in Authors

review: Needs Fixing

[Paul Voccio (pvo)]

meh, too many windows open.

review: Abstain

[Brian Waldon (bcwaldon)]

I'm very curious why nova/api/openstack/urlmap.py is necessary. I feel like we can get most of this functionality out of webob, is there something I'm missing?

Since we have to do content-type checking up front now, can we guarantee it *always* gets done up front? It seems like best_match_content_type is now designed to allow late checking of that header.

review: Needs Information

[Johannes Erdfelt (johannes.erdfelt)]

paste deploy determines which versions of the API are available by using the configuration in etc/nova/api-paste.ini. This all happens before webob gets into the picture.

My original stab at this task modified nova.api.openstack.versions (which is the handler for / and thusly the default for any URL paths not prefixed with a version) instead. However, I had to duplicate the configuration of API versions, which I wanted to avoid.

The reason for best_match_content_type is covered in the description for the merge prop. As a summary, it's because the direct API uses some of the same WSGI stack but not paste deploy, and there are tests that deliberately operate on only part of the WSGI stack (for instance nova/tests/api/openstack/test_faults.py) and partially test the same functionality of urlmap (suffix content type tests).

I can't say I'm pleased with having two different methods of determining content type either. It might be possible to fix the direct API to use a different request object or maybe even use paste deploy. It might also be possible to mock up a different request object for the tests. I think there might be other problems too, I ran into a lot when I tried developing a patch that left best_match_content_type out.

It's guaranteed to get done up front if the paste configuration is correct. I'm not a fan of requiring configuration to get correct behavior, but there's already precedent for that (see the auth middleware) and I didn't want the scope to grow beyond the original bug.

[Brian Waldon (bcwaldon)]

Okay, I can definitely understand many of your concerns here. Maybe one day we will get away from paste and our problems will be solved ;)

As for the accept header parsing: I feel like the code in urlmap.py is duplicating functionality provided in webob.acceptparse (http://docs.webob.org/en/latest/modules/webob.html). Can you take a look at that and see if you agree with me?

[Johannes Erdfelt (johannes.erdfelt)]

It *does* duplicate the code in webob, but only because webob ignores all parameters except for 'q'. We need the 'version' parameter as well (which is kind of the purpose of the branch).

I should also mention that the Accept parsing code in this branch is based off of the code in Werkzeug. It's BSD licensed and appears to be compatible with the Apache license Openstack uses. I'm thinking I might need to move that code to a separate file to keep the license separate.

[Brian Waldon (bcwaldon)]

Okay, I didn't look close enough at acceptparse to notice it didn't return values other than quality. I'm going to approve with the understanding that some of this will get simplified in the future.

review: Approve

[Brian Waldon (bcwaldon)]

Moved to gerrit.

Unmerged revisions

1591. By Johannes Erdfelt on 2011-09-21

Add docstrings to tests to better document the intent

1590. By Johannes Erdfelt on 2011-09-21

Cleanup docstrings
Remove is_filename option, it's not necessary for us and is a hack anyway

1589. By Johannes Erdfelt on 2011-09-21

Remove FIXME now that it has a workaround

1588. By Johannes Erdfelt on 2011-09-21

Remove hardcoded content type now that the code handles direct API again

1587. By Johannes Erdfelt on 2011-09-21

Accept application/atom+xml for a few exceptions as well

1586. By Johannes Erdfelt on 2011-09-21

Add content type tests

1585. By Johannes Erdfelt on 2011-09-21

PEP8 cleanup

1584. By Johannes Erdfelt on 2011-09-21

Merge with trunk

1583. By Johannes Erdfelt on 2011-09-21

Start adding some unit tests

1582. By Johannes Erdfelt on 2011-09-21

Rename to urlmap to better reflect the purpose

=== modified file 'etc/nova/api-paste.ini'
--- etc/nova/api-paste.ini	2011-09-20 22:17:03 +0000
+++ etc/nova/api-paste.ini	2011-09-21 20:54:24 +0000
@@ -71,7 +71,7 @@
 #############
 
 [composite:osapi]
-use = egg:Paste#urlmap
+use = call:nova.api.openstack.urlmap:urlmap_factory
 /: osversions
 /v1.0: openstackapi10
 /v1.1: openstackapi11

=== added file 'nova/api/openstack/urlmap.py'
--- nova/api/openstack/urlmap.py	1970-01-01 00:00:00 +0000
+++ nova/api/openstack/urlmap.py	2011-09-21 20:54:24 +0000
@@ -0,0 +1,297 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 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 paste.urlmap
+import re
+import urllib2
+
+from nova import log as logging
+from nova.api.openstack import wsgi
+
+
+_quoted_string_re = r'"[^"\\]*(?:\\.[^"\\]*)*"'
+_option_header_piece_re = re.compile(r';\s*([^\s;=]+|%s)\s*'
+                                     r'(?:=\s*([^;]+|%s))?\s*' %
+    (_quoted_string_re, _quoted_string_re))
+
+LOG = logging.getLogger('nova.api.openstack.map')
+
+
+def unquote_header_value(value):
+    """Unquotes a header value.
+    This does not use the real unquoting but what browsers are actually
+    using for quoting.
+
+    :param value: the header value to unquote.
+    """
+    if value and value[0] == value[-1] == '"':
+        # this is not the real unquoting, but fixing this so that the
+        # RFC is met will result in bugs with internet explorer and
+        # probably some other browsers as well.  IE for example is
+        # uploading files with "C:\foo\bar.txt" as filename
+        value = value[1:-1]
+    return value
+
+
+def parse_list_header(value):
+    """Parse lists as described by RFC 2068 Section 2.
+
+    In particular, parse comma-separated lists where the elements of
+    the list may include quoted-strings.  A quoted-string could
+    contain a comma.  A non-quoted string could have quotes in the
+    middle.  Quotes are removed automatically after parsing.
+
+    The return value is a standard :class:`list`:
+
+    >>> parse_list_header('token, "quoted value"')
+    ['token', 'quoted value']
+
+    :param value: a string with a list header.
+    :return: :class:`list`
+    """
+    result = []
+    for item in urllib2.parse_http_list(value):
+        if item[:1] == item[-1:] == '"':
+            item = unquote_header_value(item[1:-1])
+        result.append(item)
+    return result
+
+
+def parse_options_header(value):
+    """Parse a ``Content-Type`` like header into a tuple with the content
+    type and the options:
+
+    >>> parse_options_header('Content-Type: text/html; mimetype=text/html')
+    ('Content-Type:', {'mimetype': 'text/html'})
+
+    :param value: the header to parse.
+    :return: (str, options)
+    """
+    def _tokenize(string):
+        for match in _option_header_piece_re.finditer(string):
+            key, value = match.groups()
+            key = unquote_header_value(key)
+            if value is not None:
+                value = unquote_header_value(value)
+            yield key, value
+
+    if not value:
+        return '', {}
+
+    parts = _tokenize(';' + value)
+    name = parts.next()[0]
+    extra = dict(parts)
+    return name, extra
+
+
+class Accept(object):
+    def __init__(self, value):
+        self._content_types = [parse_options_header(v) for v in
+                               parse_list_header(value)]
+
+    def best_match(self, supported_content_types):
+        # FIXME: Should we have a more sophisticated matching algorithm that
+        # takes into account the version as well?
+        best_quality = -1
+        best_content_type = None
+        best_params = {}
+        best_match = '*/*'
+
+        for content_type in supported_content_types:
+            for content_mask, params in self._content_types:
+                try:
+                    quality = float(params.get('q', 1))
+                except ValueError:
+                    continue
+
+                if quality < best_quality:
+                    continue
+                elif best_quality == quality:
+                    if best_match.count('*') <= content_mask.count('*'):
+                        continue
+
+                if self._match_mask(content_mask, content_type):
+                    best_quality = quality
+                    best_content_type = content_type
+                    best_params = params
+                    best_match = content_mask
+
+        return best_content_type, best_params
+
+    def content_type_params(self, best_content_type):
+        """Find parameters in Accept header for given content type."""
+        for content_type, params in self._content_types:
+            if best_content_type == content_type:
+                return params
+
+        return {}
+
+    def _match_mask(self, mask, content_type):
+        if '*' not in mask:
+            return content_type == mask
+        if mask == '*/*':
+            return True
+        mask_major = mask[:-2]
+        content_type_major = content_type.split('/', 1)[0]
+        return content_type_major == mask_major
+
+
+def urlmap_factory(loader, global_conf, **local_conf):
+    if 'not_found_app' in local_conf:
+        not_found_app = local_conf.pop('not_found_app')
+    else:
+        not_found_app = global_conf.get('not_found_app')
+    if not_found_app:
+        not_found_app = loader.get_app(not_found_app, global_conf=global_conf)
+    urlmap = URLMap(not_found_app=not_found_app)
+    for path, app_name in local_conf.items():
+        path = paste.urlmap.parse_path_expression(path)
+        app = loader.get_app(app_name, global_conf=global_conf)
+        urlmap[path] = app
+    return urlmap
+
+
+class URLMap(paste.urlmap.URLMap):
+    def _match(self, host, port, path_info):
+        """Find longest match for a given URL path."""
+        for (domain, app_url), app in self.applications:
+            if domain and domain != host and domain != host + ':' + port:
+                continue
+            if (path_info == app_url
+                or path_info.startswith(app_url + '/')):
+                return app, app_url
+
+        return None, None
+
+    def _set_script_name(self, app, app_url):
+        def wrap(environ, start_response):
+            environ['SCRIPT_NAME'] += app_url
+            return app(environ, start_response)
+
+        return wrap
+
+    def _munge_path(self, app, path_info, app_url):
+        def wrap(environ, start_response):
+            environ['SCRIPT_NAME'] += app_url
+            environ['PATH_INFO'] = path_info[len(app_url):]
+            return app(environ, start_response)
+
+        return wrap
+
+    def _path_strategy(self, host, port, path_info):
+        """Check path suffix for MIME type and path prefix for API version."""
+        mime_type = app = app_url = None
+
+        parts = path_info.rsplit('.', 1)
+        if len(parts) > 1:
+            possible_type = 'application/' + parts[1]
+            if possible_type in wsgi.SUPPORTED_CONTENT_TYPES:
+                mime_type = possible_type
+
+        parts = path_info.split('/')
+        if len(parts) > 1:
+            possible_app, possible_app_url = self._match(host, port, path_info)
+            # Don't use prefix if it ends up matching default
+            if possible_app and possible_app_url:
+                app_url = possible_app_url
+                app = self._munge_path(possible_app, path_info, app_url)
+
+        return mime_type, app, app_url
+
+    def _content_type_strategy(self, host, port, environ):
+        """Check Content-Type header for API version."""
+        app = None
+        params = parse_options_header(environ.get('CONTENT_TYPE', ''))[1]
+        if 'version' in params:
+            app, app_url = self._match(host, port, '/v' + params['version'])
+            if app:
+                app = self._set_script_name(app, app_url)
+
+        return app
+
+    def _accept_strategy(self, host, port, environ, supported_content_types):
+        """Check Accept header for best matching MIME type and API version."""
+        accept = Accept(environ.get('HTTP_ACCEPT', ''))
+
+        app = None
+
+        # Find the best match in the Accept header
+        mime_type, params = accept.best_match(supported_content_types)
+        if 'version' in params:
+            app, app_url = self._match(host, port, '/v' + params['version'])
+            if app:
+                app = self._set_script_name(app, app_url)
+
+        return mime_type, app
+
+    def __call__(self, environ, start_response):
+        host = environ.get('HTTP_HOST', environ.get('SERVER_NAME')).lower()
+        if ':' in host:
+            host, port = host.split(':', 1)
+        else:
+            if environ['wsgi.url_scheme'] == 'http':
+                port = '80'
+            else:
+                port = '443'
+
+        path_info = environ['PATH_INFO']
+        path_info = self.normalize_url(path_info, False)[1]
+
+        # The MIME type for the response is determined in one of two ways:
+        # 1) URL path suffix (eg /servers/detail.json)
+        # 2) Accept header (eg application/json;q=0.8, application/xml;q=0.2)
+
+        # The API version is determined in one of three ways:
+        # 1) URL path prefix (eg /v1.1/tenant/servers/detail)
+        # 2) Content-Type header (eg application/json;version=1.1)
+        # 3) Accept header (eg application/json;q=0.8;version=1.1)
+
+        supported_content_types = list(wsgi.SUPPORTED_CONTENT_TYPES)
+
+        mime_type, app, app_url = self._path_strategy(host, port, path_info)
+
+        # Accept application/atom+xml for the index query of each API
+        # version mount point as well as the root index
+        if (app_url and app_url + '/' == path_info) or path_info == '/':
+            supported_content_types.append('application/atom+xml')
+
+        if not app:
+            app = self._content_type_strategy(host, port, environ)
+
+        if not mime_type or not app:
+            possible_mime_type, possible_app = self._accept_strategy(
+                    host, port, environ, supported_content_types)
+            if possible_mime_type and not mime_type:
+                mime_type = possible_mime_type
+            if possible_app and not app:
+                app = possible_app
+
+        if not mime_type:
+            mime_type = 'application/json'
+
+        if not app:
+            # Didn't match a particular version, probably matches default
+            app, app_url = self._match(host, port, path_info)
+            if app:
+                app = self._munge_path(app, path_info, app_url)
+
+        if app:
+            environ['nova.best_content_type'] = mime_type
+            return app(environ, start_response)
+
+        environ['paste.urlmap_object'] = self
+        return self.not_found_application(environ, start_response)

=== modified file 'nova/api/openstack/versions.py'
--- nova/api/openstack/versions.py	2011-09-14 17:32:33 +0000
+++ nova/api/openstack/versions.py	2011-09-21 20:54:24 +0000
@@ -47,11 +47,11 @@
         "media-types": [
             {
                 "base": "application/xml",
-                "type": "application/vnd.openstack.compute-v1.0+xml",
+                "type": "application/vnd.openstack.compute+xml;version=1.0",
             },
             {
                 "base": "application/json",
-                "type": "application/vnd.openstack.compute-v1.0+json",
+                "type": "application/vnd.openstack.compute+json;version=1.0",
             }
         ],
     },
@@ -76,11 +76,11 @@
         "media-types": [
             {
                 "base": "application/xml",
-                "type": "application/vnd.openstack.compute-v1.1+xml",
+                "type": "application/vnd.openstack.compute+xml;version=1.1",
             },
             {
                 "base": "application/json",
-                "type": "application/vnd.openstack.compute-v1.1+json",
+                "type": "application/vnd.openstack.compute+json;version=1.1",
             }
         ],
     },
@@ -106,13 +106,7 @@
             body_serializers=body_serializers,
             headers_serializer=headers_serializer)
 
-        supported_content_types = ('application/json',
-                                   'application/vnd.openstack.compute+json',
-                                   'application/xml',
-                                   'application/vnd.openstack.compute+xml',
-                                   'application/atom+xml')
-        deserializer = VersionsRequestDeserializer(
-            supported_content_types=supported_content_types)
+        deserializer = VersionsRequestDeserializer()
 
         wsgi.Resource.__init__(self, None, serializer=serializer,
                                deserializer=deserializer)
@@ -141,15 +135,6 @@
 
 
 class VersionsRequestDeserializer(wsgi.RequestDeserializer):
-    def get_expected_content_type(self, request):
-        supported_content_types = list(self.supported_content_types)
-        if request.path != '/':
-            # Remove atom+xml accept type for 300 responses
-            if 'application/atom+xml' in supported_content_types:
-                supported_content_types.remove('application/atom+xml')
-
-        return request.best_match_content_type(supported_content_types)
-
     def get_action_args(self, request_environment):
         """Parse dictionary created by routes library."""
         args = {}
@@ -309,13 +294,7 @@
     }
     serializer = wsgi.ResponseSerializer(body_serializers)
 
-    supported_content_types = ('application/json',
-                               'application/vnd.openstack.compute+json',
-                               'application/xml',
-                               'application/vnd.openstack.compute+xml',
-                               'application/atom+xml')
-    deserializer = wsgi.RequestDeserializer(
-        supported_content_types=supported_content_types)
+    deserializer = wsgi.RequestDeserializer()
 
     return wsgi.Resource(controller, serializer=serializer,
                          deserializer=deserializer)

=== modified file 'nova/api/openstack/wsgi.py'
--- nova/api/openstack/wsgi.py	2011-09-15 18:32:10 +0000
+++ nova/api/openstack/wsgi.py	2011-09-21 20:54:24 +0000
@@ -43,7 +43,7 @@
     'application/vnd.openstack.compute+xml': 'application/xml',
 }
 
-_SUPPORTED_CONTENT_TYPES = (
+SUPPORTED_CONTENT_TYPES = (
     'application/json',
     'application/vnd.openstack.compute+json',
     'application/xml',
@@ -54,25 +54,26 @@
 class Request(webob.Request):
     """Add some Openstack API-specific logic to the base webob.Request."""
 
-    def best_match_content_type(self, supported_content_types=None):
-        """Determine the requested response content-type.
-
-        Based on the query extension then the Accept header.
-
-        """
-        supported_content_types = supported_content_types or \
-            _SUPPORTED_CONTENT_TYPES
-
-        parts = self.path.rsplit('.', 1)
-        if len(parts) > 1:
-            ctype = 'application/{0}'.format(parts[1])
-            if ctype in supported_content_types:
-                return ctype
-
-        bm = self.accept.best_match(supported_content_types)
-
-        # default to application/json if we don't find a preference
-        return bm or 'application/json'
+    def best_match_content_type(self):
+        """Determine the requested response content-type."""
+        if 'nova.best_content_type' not in self.environ:
+            # Calculate the best MIME type
+            content_type = None
+
+            # Check URL path suffix
+            parts = self.path.rsplit('.', 1)
+            if len(parts) > 1:
+                possible_type = 'application/' + parts[1]
+                if possible_type in SUPPORTED_CONTENT_TYPES:
+                    content_type = possible_type
+
+            if not content_type:
+                content_type = self.accept.best_match(SUPPORTED_CONTENT_TYPES)
+
+            self.environ['nova.best_content_type'] = content_type or \
+                'application/json'
+
+        return self.environ['nova.best_content_type']
 
     def get_content_type(self):
         """Determine content type of the request body.
@@ -83,7 +84,7 @@
         if not "Content-Type" in self.headers:
             return None
 
-        allowed_types = _SUPPORTED_CONTENT_TYPES
+        allowed_types = SUPPORTED_CONTENT_TYPES
         content_type = self.content_type
 
         if content_type not in allowed_types:
@@ -219,12 +220,7 @@
 class RequestDeserializer(object):
     """Break up a Request object into more useful pieces."""
 
-    def __init__(self, body_deserializers=None, headers_deserializer=None,
-                 supported_content_types=None):
-
-        self.supported_content_types = supported_content_types or \
-                _SUPPORTED_CONTENT_TYPES
-
+    def __init__(self, body_deserializers=None, headers_deserializer=None):
         self.body_deserializers = {
             'application/xml': XMLDeserializer(),
             'application/json': JSONDeserializer(),
@@ -287,7 +283,7 @@
             raise exception.InvalidContentType(content_type=content_type)
 
     def get_expected_content_type(self, request):
-        return request.best_match_content_type(self.supported_content_types)
+        return request.best_match_content_type()
 
     def get_action_args(self, request_environment):
         """Parse dictionary created by routes library."""

=== modified file 'nova/tests/api/openstack/fakes.py'
--- nova/tests/api/openstack/fakes.py	2011-09-12 21:52:29 +0000
+++ nova/tests/api/openstack/fakes.py	2011-09-21 20:54:24 +0000
@@ -21,7 +21,6 @@
 
 import webob
 import webob.dec
-from paste import urlmap
 
 from glance import client as glance_client
 from glance.common import exception as glance_exc
@@ -37,6 +36,7 @@
 from nova.api.openstack import extensions
 from nova.api.openstack import versions
 from nova.api.openstack import limits
+from nova.api.openstack import urlmap
 from nova.auth.manager import User, Project
 import nova.image.fake
 from nova.image import glance

=== added file 'nova/tests/api/openstack/test_urlmap.py'
--- nova/tests/api/openstack/test_urlmap.py	1970-01-01 00:00:00 +0000
+++ nova/tests/api/openstack/test_urlmap.py	2011-09-21 20:54:24 +0000
@@ -0,0 +1,111 @@
+# Copyright 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 json
+import webob
+
+from nova import test
+from nova import log as logging
+from nova.tests.api.openstack import fakes
+
+LOG = logging.getLogger('nova.tests.api.openstack.test_urlmap')
+
+
+class UrlmapTest(test.TestCase):
+    def setUp(self):
+        super(UrlmapTest, self).setUp()
+        fakes.stub_out_rate_limiting(self.stubs)
+
+    def test_path_version_v1_0(self):
+        """Test URL path specifying v1.0 returns v1.0 content."""
+        req = webob.Request.blank('/v1.0/')
+        req.accept = "application/json"
+        res = req.get_response(fakes.wsgi_app())
+        self.assertEqual(res.status_int, 200)
+        self.assertEqual(res.content_type, "application/json")
+        body = json.loads(res.body)
+        self.assertEqual(body['version']['id'], 'v1.0')
+
+    def test_path_version_v1_1(self):
+        """Test URL path specifying v1.1 returns v1.1 content."""
+        req = webob.Request.blank('/v1.1/')
+        req.accept = "application/json"
+        res = req.get_response(fakes.wsgi_app())
+        self.assertEqual(res.status_int, 200)
+        self.assertEqual(res.content_type, "application/json")
+        body = json.loads(res.body)
+        self.assertEqual(body['version']['id'], 'v1.1')
+
+    def test_content_type_version_v1_0(self):
+        """Test Content-Type specifying v1.0 returns v1.0 content."""
+        req = webob.Request.blank('/')
+        req.content_type = "application/json;version=1.0"
+        req.accept = "application/json"
+        res = req.get_response(fakes.wsgi_app())
+        self.assertEqual(res.status_int, 200)
+        self.assertEqual(res.content_type, "application/json")
+        body = json.loads(res.body)
+        self.assertEqual(body['version']['id'], 'v1.0')
+
+    def test_content_type_version_v1_1(self):
+        """Test Content-Type specifying v1.1 returns v1.1 content."""
+        req = webob.Request.blank('/')
+        req.content_type = "application/json;version=1.1"
+        req.accept = "application/json"
+        res = req.get_response(fakes.wsgi_app())
+        self.assertEqual(res.status_int, 200)
+        self.assertEqual(res.content_type, "application/json")
+        body = json.loads(res.body)
+        self.assertEqual(body['version']['id'], 'v1.1')
+
+    def test_accept_version_v1_0(self):
+        """Test Accept header specifying v1.0 returns v1.0 content."""
+        req = webob.Request.blank('/')
+        req.accept = "application/json;version=1.0"
+        res = req.get_response(fakes.wsgi_app())
+        self.assertEqual(res.status_int, 200)
+        self.assertEqual(res.content_type, "application/json")
+        body = json.loads(res.body)
+        self.assertEqual(body['version']['id'], 'v1.0')
+
+    def test_accept_version_v1_1(self):
+        """Test Accept header specifying v1.1 returns v1.1 content."""
+        req = webob.Request.blank('/')
+        req.accept = "application/json;version=1.1"
+        res = req.get_response(fakes.wsgi_app())
+        self.assertEqual(res.status_int, 200)
+        self.assertEqual(res.content_type, "application/json")
+        body = json.loads(res.body)
+        self.assertEqual(body['version']['id'], 'v1.1')
+
+    def test_path_content_type(self):
+        """Test URL path specifying JSON returns JSON content."""
+        req = webob.Request.blank('/v1.1/foobar/images/1.json')
+        req.accept = "application/xml"
+        res = req.get_response(fakes.wsgi_app())
+        self.assertEqual(res.status_int, 200)
+        self.assertEqual(res.content_type, "application/json")
+        body = json.loads(res.body)
+        self.assertEqual(body['image']['id'], '1')
+
+    def test_accept_content_type(self):
+        """Test Accept header specifying JSON returns JSON content."""
+        req = webob.Request.blank('/v1.1/foobar/images/1')
+        req.accept = "application/xml;q=0.8, application/json"
+        res = req.get_response(fakes.wsgi_app())
+        self.assertEqual(res.status_int, 200)
+        self.assertEqual(res.content_type, "application/json")
+        body = json.loads(res.body)
+        self.assertEqual(body['image']['id'], '1')

=== modified file 'nova/tests/api/openstack/test_versions.py'
--- nova/tests/api/openstack/test_versions.py	2011-09-13 21:09:25 +0000
+++ nova/tests/api/openstack/test_versions.py	2011-09-21 20:54:24 +0000
@@ -55,11 +55,11 @@
         "media-types": [
             {
                 "base": "application/xml",
-                "type": "application/vnd.openstack.compute-v1.0+xml",
+                "type": "application/vnd.openstack.compute+xml;version=1.0",
             },
             {
                 "base": "application/json",
-                "type": "application/vnd.openstack.compute-v1.0+json",
+                "type": "application/vnd.openstack.compute+json;version=1.0",
             },
         ],
     },
@@ -84,11 +84,11 @@
         "media-types": [
             {
                 "base": "application/xml",
-                "type": "application/vnd.openstack.compute-v1.1+xml",
+                "type": "application/vnd.openstack.compute+xml;version=1.1",
             },
             {
                 "base": "application/json",
-                "type": "application/vnd.openstack.compute-v1.1+json",
+                "type": "application/vnd.openstack.compute+json;version=1.1",
             },
         ],
     },
@@ -174,12 +174,12 @@
                     {
                         "base": "application/xml",
                         "type": "application/"
-                                "vnd.openstack.compute-v1.0+xml",
+                                "vnd.openstack.compute+xml;version=1.0",
                     },
                     {
                         "base": "application/json",
                         "type": "application/"
-                                "vnd.openstack.compute-v1.0+json",
+                                "vnd.openstack.compute+json;version=1.0",
                     },
                 ],
             },
@@ -220,12 +220,58 @@
                     {
                         "base": "application/xml",
                         "type": "application/"
-                                "vnd.openstack.compute-v1.1+xml",
-                    },
-                    {
-                        "base": "application/json",
-                        "type": "application/"
-                                "vnd.openstack.compute-v1.1+json",
+                                "vnd.openstack.compute+xml;version=1.1",
+                    },
+                    {
+                        "base": "application/json",
+                        "type": "application/"
+                                "vnd.openstack.compute+json;version=1.1",
+                    },
+                ],
+            },
+        }
+        self.assertEqual(expected, version)
+
+    def test_get_version_1_1_detail_content_type(self):
+        req = webob.Request.blank('/')
+        req.accept = "application/json;version=1.1"
+        res = req.get_response(fakes.wsgi_app())
+        self.assertEqual(res.status_int, 200)
+        self.assertEqual(res.content_type, "application/json")
+        version = json.loads(res.body)
+        expected = {
+            "version": {
+                "id": "v1.1",
+                "status": "CURRENT",
+                "updated": "2011-01-21T11:33:21Z",
+                "links": [
+                    {
+                        "rel": "self",
+                        "href": "http://localhost/v1.1/",
+                    },
+                    {
+                        "rel": "describedby",
+                        "type": "application/pdf",
+                        "href": "http://docs.rackspacecloud.com/"
+                                "servers/api/v1.1/cs-devguide-20110125.pdf",
+                    },
+                    {
+                        "rel": "describedby",
+                        "type": "application/vnd.sun.wadl+xml",
+                        "href": "http://docs.rackspacecloud.com/"
+                                "servers/api/v1.1/application.wadl",
+                    },
+                ],
+                "media-types": [
+                    {
+                        "base": "application/xml",
+                        "type": "application/"
+                                "vnd.openstack.compute+xml;version=1.1",
+                    },
+                    {
+                        "base": "application/json",
+                        "type": "application/"
+                                "vnd.openstack.compute+json;version=1.1",
                     },
                 ],
             },
@@ -433,11 +479,13 @@
                 "media-types": [
                     {
                         "base": "application/xml",
-                        "type": "application/vnd.openstack.compute-v1.1+xml"
+                        "type": "application/vnd.openstack.compute+xml"
+                                ";version=1.1"
                     },
                     {
                         "base": "application/json",
-                        "type": "application/vnd.openstack.compute-v1.1+json"
+                        "type": "application/vnd.openstack.compute+json"
+                                ";version=1.1"
                     },
                 ],
             },
@@ -453,11 +501,13 @@
                 "media-types": [
                     {
                         "base": "application/xml",
-                        "type": "application/vnd.openstack.compute-v1.0+xml"
+                        "type": "application/vnd.openstack.compute+xml"
+                                ";version=1.0"
                     },
                     {
                         "base": "application/json",
-                        "type": "application/vnd.openstack.compute-v1.0+json"
+                        "type": "application/vnd.openstack.compute+json"
+                                ";version=1.0"
                     },
                 ],
             },
@@ -531,11 +581,13 @@
                 "media-types": [
                     {
                         "base": "application/xml",
-                        "type": "application/vnd.openstack.compute-v1.1+xml"
+                        "type": "application/vnd.openstack.compute+xml"
+                                ";version=1.1"
                     },
                     {
                         "base": "application/json",
-                        "type": "application/vnd.openstack.compute-v1.1+json"
+                        "type": "application/vnd.openstack.compute+json"
+                                ";version=1.1"
                     },
                 ],
             },
@@ -551,11 +603,13 @@
                 "media-types": [
                     {
                         "base": "application/xml",
-                        "type": "application/vnd.openstack.compute-v1.0+xml"
+                        "type": "application/vnd.openstack.compute+xml"
+                                ";version=1.0"
                     },
                     {
                         "base": "application/json",
-                        "type": "application/vnd.openstack.compute-v1.0+json"
+                        "type": "application/vnd.openstack.compute+json"
+                                ";version=1.0"
                     },
                 ],
             },
@@ -713,11 +767,13 @@
                 "media-types": [
                     {
                         "base": "application/xml",
-                        "type": "application/vnd.openstack.compute-v1.0+xml",
+                        "type": "application/vnd.openstack.compute+xml"
+                                ";version=1.0",
                     },
                     {
                         "base": "application/json",
-                        "type": "application/vnd.openstack.compute-v1.0+json",
+                        "type": "application/vnd.openstack.compute+json"
+                                ";version=1.0",
                     },
                 ],
             },
@@ -817,11 +873,13 @@
                 "media-types": [
                     {
                         "base": "application/xml",
-                        "type": "application/vnd.openstack.compute-v1.1+xml",
+                        "type": "application/vnd.openstack.compute+xml"
+                                ";version=1.1",
                     },
                     {
                         "base": "application/json",
-                        "type": "application/vnd.openstack.compute-v1.1+json",
+                        "type": "application/vnd.openstack.compute+json"
+                                ";version=1.1",
                     }
                 ],
             },