Merge pull request #146 from ryanpetrello/next

Add support for content type detection via Accept headers.

2 files changed, 92 insertions, 20 deletions

diff --git a/pecan/core.py b/pecan/core.py
index 1ff1663..8155180 100644
--- a/pecan/core.py
+++ b/pecan/core.py
@@ -1,27 +1,29 @@
-from templating import RendererFactory
-from routing import lookup_controller, NonCanonicalPath
-from util import _cfg, encode_if_needed
-from middleware.recursive import ForwardRequestException
-
-from webob import Request, Response, exc
+import urllib
+import sys
+try:
+    from simplejson import loads
+except ImportError:             # pragma: no cover
+    from json import loads      # noqa
 from threading import local
 from itertools import chain
 from mimetypes import guess_type, add_type
 from urlparse import urlsplit, urlunsplit
 from os.path import splitext
+import logging
 
-try:
-    from simplejson import loads
-except ImportError:             # pragma: no cover
-    from json import loads      # noqa
+from webob import Request, Response, exc, acceptparse
+
+from templating import RendererFactory
+from routing import lookup_controller, NonCanonicalPath
+from util import _cfg, encode_if_needed
+from middleware.recursive import ForwardRequestException
 
-import urllib
-import sys
 
 # make sure that json is defined in mimetypes
 add_type('application/json', '.json', True)
 
 state = local()
+logger = logging.getLogger(__name__)
 
 
 def proxy(key):
@@ -367,6 +369,7 @@ class Pecan(object):
         # by the file extension on the URI
         path = request.pecan['routing_path']
 
+        # attempt to guess the content type based on the file extension
         if not request.pecan['content_type'] and '.' in path.split('/')[-1]:
             path, extension = splitext(path)
             request.pecan['extension'] = extension
@@ -392,24 +395,45 @@ class Pecan(object):
 
         # if unsure ask the controller for the default content type
         if not request.pecan['content_type']:
-            request.pecan['content_type'] = cfg.get(
-                'content_type',
-                'text/html'
-            )
+            # attempt to find a best match based on accept headers (if they
+            # exist)
+            if 'Accept' in request.headers:
+                best_default = acceptparse.MIMEAccept(
+                    request.headers['Accept']
+                ).best_match(
+                    cfg.get('content_types', {}).keys()
+                )
+
+                if best_default is None:
+                    msg = "Controller '%s' defined does not support " + \
+                          "content_type '%s'. Supported type(s): %s"
+                    logger.error(
+                        msg % (
+                            controller.__name__,
+                            request.pecan['content_type'],
+                            cfg.get('content_types', {}).keys()
+                        )
+                    )
+                    raise exc.HTTPNotAcceptable()
+
+                request.pecan['content_type'] = best_default
+            else:
+                request.pecan['content_type'] = cfg.get(
+                    'content_type',
+                    'text/html'
+                )
         elif cfg.get('content_type') is not None and \
                 request.pecan['content_type'] not in \
                 cfg.get('content_types', {}):
 
-            import warnings
             msg = "Controller '%s' defined does not support content_type " + \
                   "'%s'. Supported type(s): %s"
-            warnings.warn(
+            logger.error(
                 msg % (
                     controller.__name__,
                     request.pecan['content_type'],
                     cfg.get('content_types', {}).keys()
-                ),
-                RuntimeWarning
+                )
             )
             raise exc.HTTPNotFound
 
diff --git a/pecan/tests/test_base.py b/pecan/tests/test_base.py
index 0b81bd1..13d1c94 100644
--- a/pecan/tests/test_base.py
+++ b/pecan/tests/test_base.py
@@ -908,6 +908,54 @@ class TestFileTypeExtensions(unittest.TestCase):
             assert r.status_int == 404
 
 
+class TestContentTypeByAcceptHeaders(unittest.TestCase):
+
+    @property
+    def app_(self):
+        """
+        Test that content type is set appropriately based on Accept headers.
+        """
+        class RootController(object):
+
+            @expose(content_type='text/html')
+            @expose(content_type='application/json')
+            def index(self, *args):
+                return 'Foo'
+
+        return TestApp(Pecan(RootController()))
+
+    def test_quality(self):
+        r = self.app_.get('/', headers={
+            'Accept': 'text/html,application/json;q=0.9,*/*;q=0.8'
+        })
+        assert r.status_int == 200
+        assert r.content_type == 'text/html'
+
+        r = self.app_.get('/', headers={
+            'Accept': 'application/json,text/html;q=0.9,*/*;q=0.8'
+        })
+        assert r.status_int == 200
+        assert r.content_type == 'application/json'
+
+    def test_file_extension_has_higher_precedence(self):
+        r = self.app_.get('/index.html', headers={
+            'Accept': 'application/json,text/html;q=0.9,*/*;q=0.8'
+        })
+        assert r.status_int == 200
+        assert r.content_type == 'text/html'
+
+    def test_not_acceptable(self):
+        r = self.app_.get('/', headers={
+            'Accept': 'application/xml',
+        }, status=406)
+        assert r.status_int == 406
+
+    def test_accept_header_missing(self):
+        r = self.app_.get('/')
+        assert r.status_int == 200
+        assert r.content_type == 'text/html'
+
+
 class TestCanonicalRouting(unittest.TestCase):
 
     @property