Merge pull request #67 from xsnippet/werkzeug-best-match

Use werkzeug.MIMEAccept() to find best MIME match

tests/test_resource.py

@@ -9,6 +9,7 @@
 """
 
 import json
+import collections
 
 import aiohttp.web as web
 import pytest
@@ -29,24 +30,29 @@ async def post(self):
 @pytest.fixture(scope='function')
 async def testapp(test_client):
     app = web.Application()
-    app['db'] = None
     app.router.add_route('*', '/test', _TestResource)
     return await test_client(app)
 
 
-async def test_get_json(testapp):
-    resp = await testapp.get('/test', headers={
-        'Accept': 'application/json',
-    })
+@pytest.mark.parametrize('headers,', [
+    {'Accept': 'application/json'},
+    {'Accept': 'application/*'},
+    {'Accept': '*/*'},
+    {},
+])
+async def test_get_json(testapp, headers):
+    resp = await testapp.get('/test', headers=headers)
 
     assert resp.status == 299
     assert await resp.json() == {'who': 'batman'}
 
 
-async def test_get_unsupported_media_type(testapp):
-    resp = await testapp.get('/test', headers={
-        'Accept': 'application/mytype',
-    })
+@pytest.mark.parametrize('headers', [
+    {'Accept': 'application/mytype'},
+    {'Accept': 'foobar/json'},
+])
+async def test_get_unsupported_media_type(testapp, headers):
+    resp = await testapp.get('/test', headers=headers)
 
     # NOTE: Do not check response context, since it's not clear
     # whether should we respond with JSON or plain/text or something
@@ -55,6 +61,49 @@ async def test_get_unsupported_media_type(testapp):
         assert resp.status == 406
 
 
+@pytest.mark.parametrize('accept, best_match', [
+    ('text/csv; q=0.9, application/json',
+     'application/json'),
+    ('application/json; q=0.9, text/csv',
+     'text/csv'),
+    ('application/json; q=0.9, image/png, text/csv',
+     'text/csv'),
+    ('application/json; q=0.9, image/png; q=0.8, text/csv',
+     'text/csv'),
+    ('application/json; q=0.9, image/png; q=0.8, text/csv; q=1',
+     'text/csv'),
+    ('application/json; q=1, image/png; q=0.8, text/csv',
+     'application/json'),
+    ('application/json; q=0.4, image/png; q=0.3, text/csv; q=0.45',
+     'text/csv'),
+    ('text/plain, application/json; q=0.8',
+     'application/json'),
+    ('application/*, text/csv',
+     'text/csv'),
+    ('application/*, text/csv; q=0.9',
+     'application/json'),
+    ('text/*, application/yaml',
+     'text/csv')
+])
+async def test_get_best_mimetype(test_client, accept, best_match):
+    class _TestResource(resource.Resource):
+        _encoders = collections.OrderedDict([
+            ('application/json', lambda _: 'application/json'),
+            ('text/csv', lambda _: 'text/csv'),
+            ('image/png', lambda _: 'image/png'),
+        ])
+
+        async def get(self):
+            return {}
+
+    app = web.Application()
+    app.router.add_route('*', '/test', _TestResource)
+    testapp = await test_client(app)
+
+    resp = await testapp.get('/test', headers={'Accept': accept})
+    assert await resp.text() == best_match
+
+
 async def test_post_json(testapp):
     resp = await testapp.post(
         '/test',

xsnippet/api/resource.py

@@ -13,11 +13,10 @@
 import asyncio
 import json
 import datetime
-import fnmatch
 import functools
 
 from aiohttp import web, hdrs
-from werkzeug import http
+import werkzeug
 
 from . import exceptions
 
@@ -105,34 +104,28 @@ def _make_response(self, data, headers=None, status=200):
 
         :raise: :class:`aiohttp.web.HTTPNotAcceptable`
         """
-        # By an HTTP standard the 'Accept' header might have multiple
-        # media ranges (i.e. types) specified with priority. Moreover,
-        # we also may have several 'Accept' headers in one HTTP request.
-        #
-        # This is going to parse them and prioritize, so we respond
-        # with most preferable content type.
-        #
-        # No header means any type.
-        accepts = http.parse_accept_header(
-
-            # combine few headers into one in order to simplify parsing,
-            # or use any type pattern if no headers are passed
-            ','.join(self.request.headers.getall(hdrs.ACCEPT, ['*/*']))
+        mimeaccept = werkzeug.parse_accept_header(
+            # According to HTTP standard, 'Accept' header may show up multiple
+            # times in the request. So let's join them before passing to parser
+            # so we call parser only once.
+            ','.join(self.request.headers.getall(hdrs.ACCEPT, ['*/*'])),
+
+            # A handful wrapper to choose a best match with one method.
+            werkzeug.MIMEAccept,
         )
 
-        for accept, _ in accepts:
-            for supported_accept, encode in self._encoders.items():
-
-                # since 'Accept' may contain glob patterns (e.g. */*) we
-                # can't use dict lookup and should search for suitable
-                # encoder manually
-                if fnmatch.fnmatch(supported_accept, accept):
-                    return web.Response(
-                        status=status,
-                        headers=headers,
-                        content_type=supported_accept,
-                        text=encode(data),
-                    )
+        # According to HTTP standard, 'Accept' header may have multiple media
+        # ranges (i.e. MIME types), each specified with a priority. So choose
+        # one that matches best for further usage.
+        accept = mimeaccept.best_match(self._encoders)
+
+        if accept in self._encoders:
+            return web.Response(
+                status=status,
+                headers=headers,
+                content_type=accept,
+                text=self._encoders[accept](data),
+            )
 
         raise web.HTTPNotAcceptable()