Merge pull request #14 from sprockets/move-negotiation-into-settings

Add functions to install & retrieve ContentSettings from application

README.rst

@@ -12,6 +12,25 @@ This mix-in adds two methods to a ``tornado.web.RequestHandler`` instance:
 - ``send_response(object)``: serializes the response into the content type
   requested by the ``Accept`` header.
 
+Before adding support for specific content types, you SHOULD install the
+content settings into your ``tornado.web.Application`` instance.  If you
+don't install the content settings, then an instance will be created for
+you by the mix-in; however, the created instance will be empty.  You
+should already have a function that creates the ``Application`` instance.
+If you don't, now is a good time to add one.
+
+.. code-block:: python
+
+   from sprockets.mixins.mediatype import content
+   from tornado import web
+
+   def make_application():
+       application = web.Application([
+           # insert your handlers here
+       ])
+       content.install(application, 'application/json', 'utf-8')
+       return application
+
 Support for a content types is enabled by calling ``add_binary_content_type``,
 ``add_text_content_type`` or the ``add_transcoder`` functions with the
 ``tornado.web.Application`` instance, the content type, encoding and decoding
@@ -29,6 +48,7 @@ functions as parameters:
            # insert your handlers here
        ])
 
+       content.install(application, 'application/json', 'utf-8')
        content.add_text_content_type(application,
                                      'application/json', 'utf-8',
                                      json.dumps, json.loads)
@@ -52,6 +72,7 @@ types:
            # insert your handlers here
        ])
 
+       content.install(application, 'application/json', 'utf-8')
        content.add_transcoder(application, transcoders.JSONTranscoder())
 
        return application

docs/api.rst

@@ -9,6 +9,10 @@ Content Type Handling
 
 Content Type Registration
 -------------------------
+.. autofunction:: install
+
+.. autofunction:: get_settings
+
 .. autofunction:: set_default_content_type
 
 .. autofunction:: add_binary_content_type

docs/history.rst

@@ -1,5 +1,10 @@
 Version History
 ===============
+`Next Release`_
+---------------
+- Add :func:`sprockets.mixins.mediatype.content.install`.
+- Add :func:`sprockets.mixins.mediatype.content.get_settings`.
+- Deprecate :meth:`sprockets.mixins.mediatype.content.ContentSettings.from_application`.
 
 `2.1.0`_ (16 Mar 2016)
 ----------------------
@@ -42,14 +47,14 @@ Version History
 ---------------------
 - Initial Release
 
-.. _Next Release: https://github.com/sprockets/sprockets.http/compare/2.1.0...HEAD
-.. _2.1.0: https://github.com/sprockets/sprockets.http/compare/2.0.1...2.1.0
-.. _2.0.1: https://github.com/sprockets/sprockets.http/compare/2.0.0...2.0.1
-.. _2.0.0: https://github.com/sprockets/sprockets.http/compare/1.0.4...2.0.0
-.. _1.0.4: https://github.com/sprockets/sprockets.http/compare/1.0.3...1.0.4
-.. _1.0.3: https://github.com/sprockets/sprockets.http/compare/1.0.2...1.0.3
-.. _1.0.2: https://github.com/sprockets/sprockets.http/compare/1.0.1...1.0.2
-.. _1.0.1: https://github.com/sprockets/sprockets.http/compare/1.0.0...1.0.1
-.. _1.0.0: https://github.com/sprockets/sprockets.http/compare/0.0.0...1.0.0
+.. _Next Release: https://github.com/sprockets/sprockets.mixins.media_type/compare/2.1.0...HEAD
+.. _2.1.0: https://github.com/sprockets/sprockets.mixins.media_type/compare/2.0.1...2.1.0
+.. _2.0.1: https://github.com/sprockets/sprockets.mixins.media_type/compare/2.0.0...2.0.1
+.. _2.0.0: https://github.com/sprockets/sprockets.mixins.media_type/compare/1.0.4...2.0.0
+.. _1.0.4: https://github.com/sprockets/sprockets.mixins.media_type/compare/1.0.3...1.0.4
+.. _1.0.3: https://github.com/sprockets/sprockets.mixins.media_type/compare/1.0.2...1.0.3
+.. _1.0.2: https://github.com/sprockets/sprockets.mixins.media_type/compare/1.0.1...1.0.2
+.. _1.0.1: https://github.com/sprockets/sprockets.mixins.media_type/compare/1.0.0...1.0.1
+.. _1.0.0: https://github.com/sprockets/sprockets.mixins.media_type/compare/0.0.0...1.0.0
 
 .. _Vary: http://tools.ietf.org/html/rfc7234#section-4.1

docs/static/custom.css

@@ -1,4 +1,32 @@
 @import url("alabaster.css");
 h1.logo {
   font-size: 12pt;
+  overflow-wrap: normal;
+  word-wrap: normal;
+  overflow: hidden;
+  margin-right: -25px; /* works together with div.body padding-left */
+}
+div.body {padding-left: 30px;}
+th.field-name {hyphens: none; -webkit-hyphens: none; -ms-hyphens: none;}
+div.document {width: 90%;}
+/* support small screens too! */
+@media screen and (max-width: 1000px) {
+  div.sphinxsidebar {display: none;}
+  div.document {width: 100%!important;}
+  div.bodywrapper {margin-left: 0;}
+  div.highlight pre {margin-right: -30px;}
+}
+@media screen and (min-width: 1000px) {
+  div.bodywrapper {margin-left: default;}
+}
+/* hanging indent for class names
+ * would use "text-indent: 2em hanging" if it were supported everywhere
+ */
+dl.class > dt, dl.function > dt {
+	text-indent: -4em;
+	padding-left: 4em;
+}
+/* add some space to wrap nicely */
+span.sig-paren::after {
+	content: " ";
 }

sprockets/mixins/mediatype/content.py

@@ -1,6 +1,10 @@
 """
 Content handling for Tornado.
 
+- :func:`.install` creates a settings object and installs it into
+  the :class:`tornado.web.Application` instance
+- :func:`.get_settings` retrieve a :class:`.ContentSettings` object
+  from a :class:`tornado.web.Application` instance
 - :func:`.set_default_content_type` sets the content type that is
   used when an ``Accept`` or ``Content-Type`` header is omitted.
 - :func:`.add_binary_content_type` register transcoders for a binary
@@ -9,6 +13,7 @@ Content handling for Tornado.
   content type
 - :func:`.add_transcoder` register a custom transcoder instance
   for a content type
+
 - :class:`.ContentSettings` an instance of this is attached to
   :class:`tornado.web.Application` to hold the content mapping
   information for the application
@@ -23,6 +28,7 @@ instances.
 
 """
 import logging
+import warnings
 
 from ietfparse import algorithms, errors, headers
 from tornado import web
@@ -31,16 +37,24 @@ from . import handlers
 
 
 logger = logging.getLogger(__name__)
+SETTINGS_KEY = 'sprockets.mixins.mediatype.ContentSettings'
+"""Key in application.settings to store the ContentSettings instance."""
+
+_warning_issued = False
 
 
 class ContentSettings(object):
     """
     Content selection settings.
 
-    An instance of this class is stashed as the ``_content_settings``
-    attribute on the application object.  It contains the list of
-    available content types and handlers associated with them.  Each
-    handler implements a simple interface:
+    An instance of this class is stashed in ``application.settings``
+    under the :data:`.SETTINGS_KEY` key.  Instead of creating an
+    instance of this class yourself, use the :func:`.install`
+    function to install it into the application.
+
+    The settings instance contains the list of available content
+    types and handlers associated with them.  Each handler implements
+    a simple interface:
 
     - ``to_bytes(dict, encoding:str) -> bytes``
     - ``from_bytes(bytes, encoding:str) -> dict``
@@ -100,12 +114,23 @@ class ContentSettings(object):
     def get(self, content_type, default=None):
         return self._handlers.get(content_type, default)
 
-    @classmethod
-    def from_application(cls, application):
-        """Retrieve the content settings from an application."""
-        if not hasattr(application, '_content_settings'):
-            setattr(application, '_content_settings', cls())
-        return application._content_settings
+    @staticmethod
+    def from_application(application):
+        """
+        Retrieve the content settings from an application.
+
+        .. deprecated:: 2.2
+           Use :func:`.install` and :func:`.get_settings` instead
+
+        """
+        global _warning_issued
+        if not _warning_issued:
+            warnings.warn('ContentSettings.from_application returns blank '
+                          'settings object. Please use content.install() '
+                          'and content.get_settings() instead',
+                          DeprecationWarning)
+            _warning_issued = True
+        return get_settings(application, force_instance=True)
 
     @property
     def available_content_types(self):
@@ -119,6 +144,48 @@ class ContentSettings(object):
         return self._available_types
 
 
+def install(application, default_content_type, encoding=None):
+    """
+    Install the media type management settings.
+
+    :param tornado.web.Application application: the application to
+        install a :class:`.ContentSettings` object into.
+    :param str|NoneType default_content_type:
+    :param str|NoneType encoding:
+
+    :returns: the content settings instance
+    :rtype: sprockets.mixins.mediatype.content.ContentSettings
+
+    """
+    try:
+        settings = application.settings[SETTINGS_KEY]
+    except KeyError:
+        settings = application.settings[SETTINGS_KEY] = ContentSettings()
+        settings.default_content_type = default_content_type
+        settings.default_encoding = encoding
+    return settings
+
+
+def get_settings(application, force_instance=False):
+    """
+    Retrieve the media type settings for a application.
+
+    :param tornado.web.Application application:
+    :keyword bool force_instance: if :data:`True` then create the
+        instance if it does not exist
+
+    :return: the content settings instance
+    :rtype: sprockets.mixins.mediatype.content.ContentSettings
+
+    """
+    try:
+        return application.settings[SETTINGS_KEY]
+    except KeyError:
+        if not force_instance:
+            return None
+    return install(application, None)
+
+
 def add_binary_content_type(application, content_type, pack, unpack):
     """
     Add handler for a binary content type.
@@ -192,7 +259,7 @@ def add_transcoder(application, transcoder, content_type=None):
        :returns: the decoded :class:`object` instance
 
     """
-    settings = ContentSettings.from_application(application)
+    settings = get_settings(application, force_instance=True)
     settings[content_type or transcoder.content_type] = transcoder
 
 
@@ -205,7 +272,7 @@ def set_default_content_type(application, content_type, encoding=None):
     :param str|None encoding: encoding to use when one is unspecified
 
     """
-    settings = ContentSettings.from_application(application)
+    settings = get_settings(application, force_instance=True)
     settings.default_content_type = content_type
     settings.default_encoding = encoding
 
@@ -241,7 +308,7 @@ class ContentMixin(object):
     def get_response_content_type(self):
         """Figure out what content type will be used in the response."""
         if self._best_response_match is None:
-            settings = ContentSettings.from_application(self.application)
+            settings = get_settings(self.application, force_instance=True)
             acceptable = headers.parse_http_accept_header(
                 self.request.headers.get(
                     'Accept',
@@ -269,7 +336,7 @@ class ContentMixin(object):
 
         """
         if self._request_body is None:
-            settings = ContentSettings.from_application(self.application)
+            settings = get_settings(self.application, force_instance=True)
             content_type_header = headers.parse_content_type(
                 self.request.headers.get('Content-Type',
                                          settings.default_content_type))
@@ -298,7 +365,7 @@ class ContentMixin(object):
             header be set?  Defaults to :data:`True`
 
         """
-        settings = ContentSettings.from_application(self.application)
+        settings = get_settings(self.application, force_instance=True)
         handler = settings[self.get_response_content_type()]
         content_type, data_bytes = handler.to_bytes(body)
         if set_content_type:

tests.py

@@ -30,7 +30,8 @@ class UTC(datetime.tzinfo):
 
 class Context(object):
     """Super simple class to call setattr on"""
-    pass
+    def __init__(self):
+        self.settings = {}
 
 
 def pack_string(obj):
@@ -186,13 +187,6 @@ class JSONTranscoderTests(unittest.TestCase):
 
 class ContentSettingsTests(unittest.TestCase):
 
-    def test_that_from_application_creates_instance(self):
-
-        context = Context()
-        settings = content.ContentSettings.from_application(context)
-        self.assertIs(content.ContentSettings.from_application(context),
-                      settings)
-
     def test_that_handler_listed_in_available_content_types(self):
         settings = content.ContentSettings()
         settings['application/json'] = object()
@@ -236,32 +230,53 @@ class ContentFunctionTests(unittest.TestCase):
         self.context = Context()
 
     def test_that_add_binary_content_type_creates_binary_handler(self):
+        settings = content.install(self.context,
+                                   'application/octet-stream')
         content.add_binary_content_type(self.context,
                                         'application/vnd.python.pickle',
                                         pickle.dumps, pickle.loads)
-        settings = content.ContentSettings.from_application(self.context)
         transcoder = settings['application/vnd.python.pickle']
         self.assertIsInstance(transcoder, handlers.BinaryContentHandler)
         self.assertIs(transcoder._pack, pickle.dumps)
         self.assertIs(transcoder._unpack, pickle.loads)
 
     def test_that_add_text_content_type_creates_text_handler(self):
+        settings = content.install(self.context, 'application/json')
         content.add_text_content_type(self.context, 'application/json', 'utf8',
                                       json.dumps, json.loads)
-        settings = content.ContentSettings.from_application(self.context)
         transcoder = settings['application/json']
         self.assertIsInstance(transcoder, handlers.TextContentHandler)
         self.assertIs(transcoder._dumps, json.dumps)
         self.assertIs(transcoder._loads, json.loads)
 
     def test_that_add_text_content_type_discards_charset_parameter(self):
+        settings = content.install(self.context, 'application/json', 'utf-8')
         content.add_text_content_type(self.context,
                                       'application/json;charset=UTF-8', 'utf8',
                                       json.dumps, json.loads)
-        settings = content.ContentSettings.from_application(self.context)
         transcoder = settings['application/json']
         self.assertIsInstance(transcoder, handlers.TextContentHandler)
 
+    def test_that_install_creates_settings(self):
+        settings = content.install(self.context, 'application/json', 'utf8')
+        self.assertIsNotNone(settings)
+        self.assertEqual(settings.default_content_type, 'application/json')
+        self.assertEqual(settings.default_encoding, 'utf8')
+
+    def test_that_get_settings_returns_none_when_no_settings(self):
+        settings = content.get_settings(self.context)
+        self.assertIsNone(settings)
+
+    def test_that_get_settings_returns_installed_settings(self):
+        settings = content.install(self.context, 'application/xml', 'utf8')
+        other_settings = content.get_settings(self.context)
+        self.assertIs(settings, other_settings)
+
+    def test_that_get_settings_will_create_instance_if_requested(self):
+        settings = content.get_settings(self.context, force_instance=True)
+        self.assertIsNotNone(settings)
+        self.assertIs(content.get_settings(self.context), settings)
+
 
 class MsgPackTranscoderTests(unittest.TestCase):