From 121f45b82f2722c0d5e5c5a7b768270299793eeb Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Wed, 8 Aug 2018 00:31:24 -0500 Subject: fix deprecated usage of request.accept in AcceptPredicate --- docs/glossary.rst | 6 ++ docs/narr/extconfig.rst | 1 + docs/narr/viewconfig.rst | 107 +++++++++++++++++++++++++------- pyramid/config/__init__.py | 1 + pyramid/config/routes.py | 37 +++++++---- pyramid/config/views.py | 151 ++++++++++++++++++++++++++++++++++----------- pyramid/interfaces.py | 7 +++ pyramid/predicates.py | 6 +- pyramid/testing.py | 2 + 9 files changed, 244 insertions(+), 74 deletions(-) diff --git a/docs/glossary.rst b/docs/glossary.rst index b05344ae9..7244aeb8d 100644 --- a/docs/glossary.rst +++ b/docs/glossary.rst @@ -1209,3 +1209,9 @@ Glossary Alembic `Alembic `_ is a lightweight database migration tool for usage with the SQLAlchemy Database Toolkit for Python. + + media type + A label representing the type of some content. + A media type is a nested structure containing a top-level type and a subtype. + Optionally, a media type can also contain parameters specific to the type. + See :rfc:`6838` for more information about media types. diff --git a/docs/narr/extconfig.rst b/docs/narr/extconfig.rst index 61bd7a05f..a9ea93e60 100644 --- a/docs/narr/extconfig.rst +++ b/docs/narr/extconfig.rst @@ -255,6 +255,7 @@ Pre-defined Phases :const:`pyramid.config.PHASE1_CONFIG` +- :meth:`pyramid.config.Configurator.add_accept_view_option` - :meth:`pyramid.config.Configurator.add_renderer` - :meth:`pyramid.config.Configurator.add_route_predicate` - :meth:`pyramid.config.Configurator.add_subscriber_predicate` diff --git a/docs/narr/viewconfig.rst b/docs/narr/viewconfig.rst index c463d297e..e85338573 100644 --- a/docs/narr/viewconfig.rst +++ b/docs/narr/viewconfig.rst @@ -285,6 +285,17 @@ Non-Predicate Arguments are just developing stock Pyramid applications. Pay no attention to the man behind the curtain. +``exception_only`` + + When this value is ``True``, the ``context`` argument must be a subclass of + ``Exception``. This flag indicates that only an :term:`exception view` should + be created, and that this view should not match if the traversal + :term:`context` matches the ``context`` argument. If the ``context`` is a + subclass of ``Exception`` and this value is ``False`` (the default), then a + view will be registered to match the traversal :term:`context` as well. + + .. versionadded:: 1.8 + Predicate Arguments +++++++++++++++++++ @@ -317,17 +328,6 @@ configured view. If ``context`` is not supplied, the value ``None``, which matches any resource, is used. -``exception_only`` - - When this value is ``True``, the ``context`` argument must be a subclass of - ``Exception``. This flag indicates that only an :term:`exception view` should - be created, and that this view should not match if the traversal - :term:`context` matches the ``context`` argument. If the ``context`` is a - subclass of ``Exception`` and this value is ``False`` (the default), then a - view will be registered to match the traversal :term:`context` as well. - - .. versionadded:: 1.8 - ``route_name`` If ``route_name`` is supplied, the view callable will be invoked only when the named route has matched. @@ -344,6 +344,20 @@ configured view. request/context pair found via :term:`resource location` does not indicate it matched any configured route. +``accept`` + A :term:`media type` that will be matched against the ``Accept`` HTTP request header. + If this value is specified, it must be a specific media type, such as ``text/html``. + If the media type is acceptable by the ``Accept`` header of the request, or if the ``Accept`` header isn't set at all in the request, this predicate will match. + If this does not match the ``Accept`` header of the request, view matching continues. + + If ``accept`` is not specified, the ``HTTP_ACCEPT`` HTTP header is not taken into consideration when deciding whether or not to invoke the associated view callable. + + See :ref:`accept_content_negotation` for more information. + + .. versionchanged:: 1.10 + Media ranges such as ``text/*`` will now raise :class:`pyramid.exceptions.ConfigurationError`. + Previously these values had undefined behavior based on the version of WebOb being used and was never fully supported. + ``request_type`` This value should be an :term:`interface` that the :term:`request` must provide in order for this view to be found and called. @@ -424,19 +438,6 @@ configured view. taken into consideration when deciding whether or not to invoke the associated view callable. -``accept`` - The value of this argument represents a match query for one or more mimetypes - in the ``Accept`` HTTP request header. If this value is specified, it must - be in one of the following forms: a mimetype match token in the form - ``text/plain``, a wildcard mimetype match token in the form ``text/*``, or a - match-all wildcard mimetype match token in the form ``*/*``. If any of the - forms matches the ``Accept`` header of the request, this predicate will be - true. - - If ``accept`` is not specified, the ``HTTP_ACCEPT`` HTTP header is not taken - into consideration when deciding whether or not to invoke the associated view - callable. - ``header`` This value represents an HTTP header name or a header name/value pair. @@ -1028,6 +1029,64 @@ these values. .. index:: single: HTTP caching +.. _accept_content_negotation: + +Accept Header Content Negotiation +--------------------------------- + +The ``accept`` argument to :meth:`pyramid.config.Configurator.add_view` can be used to control :term:`view lookup` by dispatching to different views based on the HTTP ``Accept`` request header. +Consider the below example in which there are two views, sharing the same view callable. +Each view specifies uses the accept header to trigger the appropriate response renderer. + +.. code-block:: python + + from pyramid.view import view_config + + @view_config(accept='application/json', renderer='json') + @view_config(accept='text/html', renderer='templates/hello.jinja2') + def myview(request): + return { + 'name': request.GET.get('name', 'bob'), + } + +Wildcard Accept header +++++++++++++++++++++++ + +The appropriate view is selected here when the client specifies an unambiguous header such as ``Accept: text/*`` or ``Accept: application/json``. +However, by default, if a client specifies ``Accept: */*``, the ordering is undefined. +This can be fixed by telling :app:`Pyramid` what the preferred relative ordering is between various accept mimetypes by using :meth:`pyramid.config.Configurator.add_accept_view_option`. +For example: + +.. code-block:: python + + from pyramid.config import Configurator + + def main(global_config, **settings): + config = Configurator(settings=settings) + config.add_accept_view_option('text/html') + config.add_accept_view_option( + 'application/json', + weighs_more_than='text/html', + ) + config.scan() + return config.make_wsgi_app() + +Missing Accept header ++++++++++++++++++++++ + +The above example will not match any view if the ``Accept`` header is not specified by the client. +This can be solved by adding a fallback view without an ``accept`` predicate. +For example, below the html response will be returned in all cases unless ``application/json`` is requested specifically. + +.. code-block:: python + + @view_config(accept='application/json', renderer='json') + @view_config(renderer='templates/hello.jinja2') + def myview(request): + return { + 'name': request.GET.get('name', 'bob'), + } + .. _influencing_http_caching: Influencing HTTP Caching diff --git a/pyramid/config/__init__.py b/pyramid/config/__init__.py index f4fcf413e..2f4e133f0 100644 --- a/pyramid/config/__init__.py +++ b/pyramid/config/__init__.py @@ -394,6 +394,7 @@ class Configurator( self.add_default_response_adapters() self.add_default_renderers() + self.add_default_accept_view_order() self.add_default_view_predicates() self.add_default_view_derivers() self.add_default_route_predicates() diff --git a/pyramid/config/routes.py b/pyramid/config/routes.py index 904c7bd4e..051bd9edb 100644 --- a/pyramid/config/routes.py +++ b/pyramid/config/routes.py @@ -139,18 +139,6 @@ class RoutesConfiguratorMixin(object): .. versionadded:: 1.1 - accept - - This value represents a match query for one or more mimetypes in the - ``Accept`` HTTP request header. If this value is specified, it must - be in one of the following forms: a mimetype match token in the form - ``text/plain``, a wildcard mimetype match token in the form - ``text/*`` or a match-all wildcard mimetype match token in the form - ``*/*``. If any of the forms matches the ``Accept`` header of the - request, or if the ``Accept`` header isn't set at all in the request, - this will match the current route. If this does not match the - ``Accept`` header of the request, route matching continues. - Predicate Arguments pattern @@ -233,6 +221,27 @@ class RoutesConfiguratorMixin(object): case of the header name is not significant. If this predicate returns ``False``, route matching continues. + accept + + A media type that will be matched against the ``Accept`` HTTP + request header. If this value is specified, it must be a specific + media type, such as ``text/html``. If the media type is acceptable + by the ``Accept`` header of the request, or if the ``Accept`` header + isn't set at all in the request, this predicate will match. If this + does not match the ``Accept`` header of the request, route matching + continues. + + If ``accept`` is not specified, the ``HTTP_ACCEPT`` HTTP header is + not taken into consideration when deciding whether or not to select + the route. + + + .. versionchanged:: 1.10 + Media ranges such as ``text/*`` will now raise + :class:`pyramid.exceptions.ConfigurationError`. Previously, + these values had undefined behavior based on the version of + WebOb being used and was never fully supported. + effective_principals If specified, this value should be a :term:`principal` identifier or @@ -289,6 +298,10 @@ class RoutesConfiguratorMixin(object): DeprecationWarning, stacklevel=3 ) + + if accept is not None: + accept = accept.lower() + # these are route predicates; if they do not match, the next route # in the routelist will be tried if request_method is not None: diff --git a/pyramid/config/views.py b/pyramid/config/views.py index 5d46de276..4eab27542 100644 --- a/pyramid/config/views.py +++ b/pyramid/config/views.py @@ -13,6 +13,7 @@ from zope.interface import ( from zope.interface.interfaces import IInterface from pyramid.interfaces import ( + IAcceptOrder, IExceptionViewClassifier, IException, IMultiView, @@ -115,14 +116,14 @@ class MultiView(object): view = self.match(context, request) return view.__discriminator__(context, request) - def add(self, view, order, accept=None, phash=None): + def add(self, view, order, accept=None, phash=None, accept_order=None): if phash is not None: for i, (s, v, h) in enumerate(list(self.views)): if phash == h: self.views[i] = (order, view, phash) return - if accept is None or '*' in accept: + if accept is None: self.views.append((order, view, phash)) self.views.sort(key=operator.itemgetter(0)) else: @@ -134,21 +135,24 @@ class MultiView(object): else: subset.append((order, view, phash)) subset.sort(key=operator.itemgetter(0)) + # dedupe accepts and sort appropriately accepts = set(self.accepts) accepts.add(accept) - self.accepts = list(accepts) # dedupe + if accept_order is not None: + sorted_accepts = [] + for accept in accept_order.sorted(): + if accept in accepts: + sorted_accepts.append(accept) + accepts.remove(accept) + sorted_accepts.extend(accepts) + accepts = sorted_accepts + self.accepts = list(accepts) def get_views(self, request): if self.accepts and hasattr(request, 'accept'): - accepts = self.accepts[:] views = [] - while accepts: - match = request.accept.best_match(accepts) - if match is None: - break - subset = self.media_views[match] - views.extend(subset) - accepts.remove(match) + for offer, _ in request.accept.acceptable_offers(self.accepts): + views.extend(self.media_views[offer]) views.extend(self.views) return views return self.views @@ -533,17 +537,17 @@ class ViewsConfiguratorMixin(object): very useful for 'civilians' who are just developing stock Pyramid applications. Pay no attention to the man behind the curtain. - accept + exception_only - This value represents a match query for one or more mimetypes in the - ``Accept`` HTTP request header. If this value is specified, it must - be in one of the following forms: a mimetype match token in the form - ``text/plain``, a wildcard mimetype match token in the form - ``text/*`` or a match-all wildcard mimetype match token in the form - ``*/*``. If any of the forms matches the ``Accept`` header of the - request, or if the ``Accept`` header isn't set at all in the request, - this will match the current view. If this does not match the - ``Accept`` header of the request, view matching continues. + .. versionadded:: 1.8 + + When this value is ``True``, the ``context`` argument must be + a subclass of ``Exception``. This flag indicates that only an + :term:`exception view` should be created, and that this view should + not match if the traversal :term:`context` matches the ``context`` + argument. If the ``context`` is a subclass of ``Exception`` and + this value is ``False`` (the default), then a view will be + registered to match the traversal :term:`context` as well. Predicate Arguments @@ -566,18 +570,6 @@ class ViewsConfiguratorMixin(object): spelling). If the view should *only* match when handling exceptions, then set the ``exception_only`` to ``True``. - exception_only - - .. versionadded:: 1.8 - - When this value is ``True``, the ``context`` argument must be - a subclass of ``Exception``. This flag indicates that only an - :term:`exception view` should be created, and that this view should - not match if the traversal :term:`context` matches the ``context`` - argument. If the ``context`` is a subclass of ``Exception`` and - this value is ``False`` (the default), then a view will be - registered to match the traversal :term:`context` as well. - route_name This value must match the ``name`` of a :term:`route @@ -677,6 +669,28 @@ class ViewsConfiguratorMixin(object): represents a header name or a header name/value pair, the case of the header name is not significant. + accept + + A :term:`media type` that will be matched against the ``Accept`` + HTTP request header. If this value is specified, it must be a + specific media type, such as ``text/html``. If the media type is + acceptable by the ``Accept`` header of the request, or if the + ``Accept`` header isn't set at all in the request, this predicate + will match. If this does not match the ``Accept`` header of the + request, view matching continues. + + If ``accept`` is not specified, the ``HTTP_ACCEPT`` HTTP header is + not taken into consideration when deciding whether or not to invoke + the associated view callable. + + See :ref:`accept_content_negotation` for more information. + + .. versionchanged:: 1.10 + Media ranges such as ``text/*`` will now raise + :class:`pyramid.exceptions.ConfigurationError`. Previously, + these values had undefined behavior based on the version of + WebOb being used and was never fully supported. + path_info This value represents a regular expression pattern that will @@ -804,6 +818,9 @@ class ViewsConfiguratorMixin(object): stacklevel=4, ) + if accept is not None: + accept = accept.lower() + view = self.maybe_dotted(view) context = self.maybe_dotted(context) for_ = self.maybe_dotted(for_) @@ -857,9 +874,6 @@ class ViewsConfiguratorMixin(object): name=renderer, package=self.package, registry=self.registry) - if accept is not None: - accept = accept.lower() - introspectables = [] ovals = view_options.copy() ovals.update(dict( @@ -1104,8 +1118,11 @@ class ViewsConfiguratorMixin(object): multiview = MultiView(name) old_accept = getattr(old_view, '__accept__', None) old_order = getattr(old_view, '__order__', MAX_ORDER) + # don't bother passing accept_order here as we know we're + # adding another one right after which will re-sort multiview.add(old_view, old_order, old_accept, old_phash) - multiview.add(derived_view, order, accept, phash) + accept_order = self.registry.queryUtility(IAcceptOrder) + multiview.add(derived_view, order, accept, phash, accept_order) for view_type in (IView, ISecuredView): # unregister any existing views self.registry.adapters.unregister( @@ -1222,6 +1239,66 @@ class ViewsConfiguratorMixin(object): ): self.add_view_predicate(name, factory) + def add_default_accept_view_order(self): + for accept in ( + 'text/html', + 'application/xhtml+xml', + 'application/xml', + 'text/xml', + 'application/json', + ): + self.add_accept_view_order(accept) + + @action_method + def add_accept_view_order( + self, + value, + weighs_more_than=None, + weighs_less_than=None, + ): + """ + Specify an ordering preference for the ``accept`` view option used + during :term:`view lookup`. + + By default, if two views have different ``accept`` options and a + request specifies ``Accept: */*`` or omits the header entirely then + it is random which view will be selected. This method provides a way + to specify a server-side, relative ordering between accept media types. + + ``value`` should be a :term:`media type` as specified by + :rfc:`7231#section-5.3.2`. For example, ``text/plain``, + ``application/json`` or ``text/html``. + + ``weighs_more_than`` and ``weighs_less_than`` control the ordering + of media types. Each value may be a string or a list of strings. + + See :ref:`accept_content_negotation` for more information. + + .. versionadded:: 1.10 + + """ + discriminator = ('accept view order', value) + intr = self.introspectable( + 'accept view order', + value, + value, + 'accept view order') + intr['value'] = value + intr['weighs_more_than'] = weighs_more_than + intr['weighs_less_than'] = weighs_less_than + def register(): + sorter = self.registry.queryUtility(IAcceptOrder) + if sorter is None: + sorter = TopologicalSorter() + self.registry.registerUtility(sorter, IAcceptOrder) + sorter.add( + value, value, + after=weighs_more_than, + before=weighs_less_than, + ) + self.action(discriminator, register, introspectables=(intr,), + order=PHASE1_CONFIG) # must be registered before add_view + @action_method def add_view_deriver(self, deriver, name=None, under=None, over=None): """ diff --git a/pyramid/interfaces.py b/pyramid/interfaces.py index bedfb60b3..17673087d 100644 --- a/pyramid/interfaces.py +++ b/pyramid/interfaces.py @@ -586,6 +586,13 @@ class IRouteRequest(Interface): """ *internal only* interface used as in a utility lookup to find route-specific interfaces. Not an API.""" +class IAcceptOrder(Interface): + """ + Marker interface for a list of accept headers with the most important + first. + + """ + class IStaticURLInfo(Interface): """ A policy for generating URLs to static assets """ def add(config, name, spec, **extra): diff --git a/pyramid/predicates.py b/pyramid/predicates.py index 5e54badff..4f63122aa 100644 --- a/pyramid/predicates.py +++ b/pyramid/predicates.py @@ -132,6 +132,10 @@ class HeaderPredicate(object): class AcceptPredicate(object): def __init__(self, val, config): self.val = val + if '*' in self.val: + raise ConfigurationError( + '"accept" predicate only accepts specific media types', + ) def text(self): return 'accept = %s' % (self.val,) @@ -139,7 +143,7 @@ class AcceptPredicate(object): phash = text def __call__(self, context, request): - return self.val in request.accept + return bool(request.accept.acceptable_offers([self.val])) class ContainmentPredicate(object): def __init__(self, val, config): diff --git a/pyramid/testing.py b/pyramid/testing.py index 7ff4c2f73..4986c0e27 100644 --- a/pyramid/testing.py +++ b/pyramid/testing.py @@ -474,7 +474,9 @@ def setUp(registry=None, request=None, hook_zca=True, autocommit=True, # someone may be passing us an esoteric "dummy" registry, and # the below won't succeed if it doesn't have a registerUtility # method. + config.add_default_response_adapters() config.add_default_renderers() + config.add_default_accept_view_order() config.add_default_view_predicates() config.add_default_view_derivers() config.add_default_route_predicates() -- cgit v1.2.3 From ed6ddcb8c7badca425093d85e23791f942dd9a34 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Wed, 29 Aug 2018 02:38:13 -0500 Subject: update docs and changelog --- CHANGES.rst | 13 ++++++++++++ docs/narr/viewconfig.rst | 52 +++++++++++++++++++++++++++++------------------- pyramid/config/views.py | 5 +++++ 3 files changed, 50 insertions(+), 20 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index d0dbbe5c0..d3ffa19fc 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -57,6 +57,12 @@ Features - Add support for Python 3.7. Add testing on Python 3.8 with allowed failures. See https://github.com/Pylons/pyramid/pull/3333 +- Added the ``pyramid.config.Configurator.add_accept_view_order`` directive, + allowing users to specify media type preferences in ambiguous situations + such as when several views match. A default ordering is defined for media + types that prefers human-readable html/text responses over JSON. + See https://github.com/Pylons/pyramid/pull/3326 + Bug Fixes --------- @@ -107,6 +113,13 @@ Backward Incompatibilities of previous ``pyramid.httpexceptions.HTTPFound``. See https://github.com/Pylons/pyramid/pull/3328 +- Accept-handling has undergone work to get rid of undefined behaviors and + runtime exceptions. As part of this effort, it is now a hard error to pass + any media ranges to the ``accept`` predicate on routes and views. + Previously, depending on the version of WebOb, this would error on certain + requests or it would work in undefined ways. + https://github.com/Pylons/pyramid/pull/3326 + Documentation Changes --------------------- diff --git a/docs/narr/viewconfig.rst b/docs/narr/viewconfig.rst index e85338573..2060393bb 100644 --- a/docs/narr/viewconfig.rst +++ b/docs/narr/viewconfig.rst @@ -1035,11 +1035,12 @@ Accept Header Content Negotiation --------------------------------- The ``accept`` argument to :meth:`pyramid.config.Configurator.add_view` can be used to control :term:`view lookup` by dispatching to different views based on the HTTP ``Accept`` request header. -Consider the below example in which there are two views, sharing the same view callable. -Each view specifies uses the accept header to trigger the appropriate response renderer. +Consider the example below in which there are three defined views. +Each view uses the ``Accept`` header to trigger an appropriate response renderer. .. code-block:: python + from pyramid.httpexceptions import HTTPNotAcceptable from pyramid.view import view_config @view_config(accept='application/json', renderer='json') @@ -1049,12 +1050,20 @@ Each view specifies uses the accept header to trigger the appropriate response r 'name': request.GET.get('name', 'bob'), } -Wildcard Accept header -++++++++++++++++++++++ + @view_config() + def myview_unacceptable(request): + raise HTTPNotAcceptable The appropriate view is selected here when the client specifies an unambiguous header such as ``Accept: text/*`` or ``Accept: application/json``. -However, by default, if a client specifies ``Accept: */*``, the ordering is undefined. -This can be fixed by telling :app:`Pyramid` what the preferred relative ordering is between various accept mimetypes by using :meth:`pyramid.config.Configurator.add_accept_view_option`. +Similarly, if the client specifies a media type that no view is registered to handle, such as ``Accept: text/plain``, it will fall through to ``myview_unacceptable`` and raise ``406 Not Acceptable``. +There are a few cases in which the client may specify ambiguous constraints: + +- ``Accept: */*``. +- A missing ``Accept`` header. +- An invalid ``Accept`` header. + +In these cases the preferred view is not clearly defined (see :rfc:`7231#section-5.3.2`) and :app:`Pyramid` will select one randomly. +This can be controlled by telling :app:`Pyramid` what the preferred relative ordering is between various media types by using :meth:`pyramid.config.Configurator.add_accept_view_order`. For example: .. code-block:: python @@ -1063,29 +1072,32 @@ For example: def main(global_config, **settings): config = Configurator(settings=settings) - config.add_accept_view_option('text/html') - config.add_accept_view_option( + config.add_accept_view_order('text/html') + config.add_accept_view_order( 'application/json', weighs_more_than='text/html', ) config.scan() return config.make_wsgi_app() -Missing Accept header -+++++++++++++++++++++ +In this case, the ``application/json`` view should always be selected in cases where it is otherwise ambiguous. -The above example will not match any view if the ``Accept`` header is not specified by the client. -This can be solved by adding a fallback view without an ``accept`` predicate. -For example, below the html response will be returned in all cases unless ``application/json`` is requested specifically. +Default Accept Ordering ++++++++++++++++++++++++ -.. code-block:: python +By default, :app:`Pyramid` defines a very simple priority ordering for views that prefers human-readable responses over JSON: - @view_config(accept='application/json', renderer='json') - @view_config(renderer='templates/hello.jinja2') - def myview(request): - return { - 'name': request.GET.get('name', 'bob'), - } +- ``text/html`` +- ``application/xhtml+xml`` +- ``application/xml`` +- ``text/xml`` +- ``text/plain`` +- ``application/json`` + +API clients tend to be able to specify their desired headers with more control than web browsers, and can specify the correct ``Accept`` value, if necessary. +Therefore, the motivation for this ordering is to optimize for readability. +Media types that are not listed above are ordered randomly during :term:`view lookup` between otherwise-similar views. +The defaults can be overridden using :meth:`pyramid.config.Configurator.add_accept_view_order` as described above. .. _influencing_http_caching: diff --git a/pyramid/config/views.py b/pyramid/config/views.py index 4eab27542..6ea672e4d 100644 --- a/pyramid/config/views.py +++ b/pyramid/config/views.py @@ -1245,6 +1245,7 @@ class ViewsConfiguratorMixin(object): 'application/xhtml+xml', 'application/xml', 'text/xml', + 'text/plain', 'application/json', ): self.add_accept_view_order(accept) @@ -1277,6 +1278,10 @@ class ViewsConfiguratorMixin(object): .. versionadded:: 1.10 """ + value = value.lower() + if '*' in value: + raise ConfigurationError( + '"accept" ordering is done between media types, not ranges') discriminator = ('accept view order', value) intr = self.introspectable( 'accept view order', -- cgit v1.2.3 From 2497d05c17016ee5dc7cb5900399a28f1845c8cc Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Mon, 3 Sep 2018 21:20:38 -0500 Subject: support multiple types in the route predicate --- CHANGES.rst | 4 ++++ pyramid/config/views.py | 5 +++++ pyramid/predicates.py | 12 +++++------- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index d3ffa19fc..592ab8c63 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -63,6 +63,10 @@ Features types that prefers human-readable html/text responses over JSON. See https://github.com/Pylons/pyramid/pull/3326 +- Support a list of media types in the ``accept`` predicate used in + ``pyramid.config.Configurator.add_route``. + See https://github.com/Pylons/pyramid/pull/3326 + Bug Fixes --------- diff --git a/pyramid/config/views.py b/pyramid/config/views.py index 6ea672e4d..d6a80fe11 100644 --- a/pyramid/config/views.py +++ b/pyramid/config/views.py @@ -818,6 +818,11 @@ class ViewsConfiguratorMixin(object): stacklevel=4, ) + if accept is not None and is_nonstr_iter(accept): + raise ConfigurationError( + 'A list is not supported in the "accept" view predicate.', + ) + if accept is not None: accept = accept.lower() diff --git a/pyramid/predicates.py b/pyramid/predicates.py index 4f63122aa..91fb41006 100644 --- a/pyramid/predicates.py +++ b/pyramid/predicates.py @@ -131,19 +131,17 @@ class HeaderPredicate(object): class AcceptPredicate(object): def __init__(self, val, config): - self.val = val - if '*' in self.val: - raise ConfigurationError( - '"accept" predicate only accepts specific media types', - ) + if not is_nonstr_iter(val): + val = (val,) + self.values = val def text(self): - return 'accept = %s' % (self.val,) + return 'accept = %s' % (self.values,) phash = text def __call__(self, context, request): - return bool(request.accept.acceptable_offers([self.val])) + return bool(request.accept.acceptable_offers(self.values)) class ContainmentPredicate(object): def __init__(self, val, config): -- cgit v1.2.3 From dd357250217ec4b5eeba96e245c46509f231c6d8 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Mon, 3 Sep 2018 21:21:20 -0500 Subject: fallback to __contains__ if a media range is supplied --- pyramid/config/routes.py | 9 +++++++++ pyramid/config/views.py | 13 ++++++++++++- pyramid/predicates.py | 5 +++++ 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/pyramid/config/routes.py b/pyramid/config/routes.py index 051bd9edb..b701a4bef 100644 --- a/pyramid/config/routes.py +++ b/pyramid/config/routes.py @@ -299,6 +299,15 @@ class RoutesConfiguratorMixin(object): stacklevel=3 ) + if accept is not None and '*' in accept: + warnings.warn( + ('The usage of a media range in the "accept" route predicate ' + 'is deprecated as of Pyramid 1.10. Use a list of explicit ' + 'media types instead.'), + DeprecationWarning, + stacklevel=4, + ) + if accept is not None: accept = accept.lower() diff --git a/pyramid/config/views.py b/pyramid/config/views.py index d6a80fe11..87584b858 100644 --- a/pyramid/config/views.py +++ b/pyramid/config/views.py @@ -123,7 +123,7 @@ class MultiView(object): self.views[i] = (order, view, phash) return - if accept is None: + if accept is None or '*' in accept: self.views.append((order, view, phash)) self.views.sort(key=operator.itemgetter(0)) else: @@ -818,6 +818,17 @@ class ViewsConfiguratorMixin(object): stacklevel=4, ) + if accept is not None and '*' in accept: + warnings.warn( + ('The usage of a media range in the "accept" view predicate ' + 'is deprecated as of Pyramid 1.10. Register multiple views ' + 'with explicit media ranges and read ' + '"Accept Header Content Negotiation" in the ' + '"View Configuration" documentation for more information.'), + DeprecationWarning, + stacklevel=4, + ) + if accept is not None and is_nonstr_iter(accept): raise ConfigurationError( 'A list is not supported in the "accept" view predicate.', diff --git a/pyramid/predicates.py b/pyramid/predicates.py index 91fb41006..262b28d89 100644 --- a/pyramid/predicates.py +++ b/pyramid/predicates.py @@ -130,8 +130,11 @@ class HeaderPredicate(object): return self.val.match(val) is not None class AcceptPredicate(object): + _use_deprecated_range_fallback = False + def __init__(self, val, config): if not is_nonstr_iter(val): + self._use_deprecated_range_fallback = '*' in val val = (val,) self.values = val @@ -141,6 +144,8 @@ class AcceptPredicate(object): phash = text def __call__(self, context, request): + if self._use_deprecated_range_fallback: + return self.values[0] in request.accept return bool(request.accept.acceptable_offers(self.values)) class ContainmentPredicate(object): -- cgit v1.2.3 From 762bd9c5c7b8974d0927eeef8074c7ad1e248f70 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Mon, 3 Sep 2018 22:17:50 -0500 Subject: support a list of media types in the accept predicate --- pyramid/config/routes.py | 46 +++++++++++---------- pyramid/config/views.py | 102 ++++++++++++++++++++++++----------------------- 2 files changed, 78 insertions(+), 70 deletions(-) diff --git a/pyramid/config/routes.py b/pyramid/config/routes.py index b701a4bef..97561ca2b 100644 --- a/pyramid/config/routes.py +++ b/pyramid/config/routes.py @@ -223,13 +223,13 @@ class RoutesConfiguratorMixin(object): accept - A media type that will be matched against the ``Accept`` HTTP - request header. If this value is specified, it must be a specific - media type, such as ``text/html``. If the media type is acceptable - by the ``Accept`` header of the request, or if the ``Accept`` header - isn't set at all in the request, this predicate will match. If this - does not match the ``Accept`` header of the request, route matching - continues. + A :term:`media type` that will be matched against the ``Accept`` + HTTP request header. If this value is specified, it must be a + specific media type, such as ``text/html``, or a list of media types. + If the media type is acceptable by the ``Accept`` header of the + request, or if the ``Accept`` header isn't set at all in the request, + this predicate will match. If this does not match the ``Accept`` + header of the request, route matching continues. If ``accept`` is not specified, the ``HTTP_ACCEPT`` HTTP header is not taken into consideration when deciding whether or not to select @@ -237,10 +237,10 @@ class RoutesConfiguratorMixin(object): .. versionchanged:: 1.10 - Media ranges such as ``text/*`` will now raise - :class:`pyramid.exceptions.ConfigurationError`. Previously, - these values had undefined behavior based on the version of - WebOb being used and was never fully supported. + Media ranges such as ``text/*`` are deprecated. Use a list of + explicit media types instead. Media ranges are non-deterministic. + + Also, added support for a list of media types. effective_principals @@ -299,17 +299,21 @@ class RoutesConfiguratorMixin(object): stacklevel=3 ) - if accept is not None and '*' in accept: - warnings.warn( - ('The usage of a media range in the "accept" route predicate ' - 'is deprecated as of Pyramid 1.10. Use a list of explicit ' - 'media types instead.'), - DeprecationWarning, - stacklevel=4, - ) - if accept is not None: - accept = accept.lower() + if isinstance(accept, str): + accept = list(accept) + accept = [accept_option.lower() for accept_option in accept] + if any('*' in accept_option for accept_option in accept): + warnings.warn( + ('The usage of a media range in the "accept" view ' + 'predicate is deprecated as of Pyramid 1.10. You may ' + 'pass multiple explicit media types, if necessary. ' + 'Please read "Accept Header Content Negotiation" in the ' + '"View Configuration" documentation for more ' + 'information.'), + DeprecationWarning, + stacklevel=4, + ) # these are route predicates; if they do not match, the next route # in the routelist will be tried diff --git a/pyramid/config/views.py b/pyramid/config/views.py index 87584b858..bd5ec6032 100644 --- a/pyramid/config/views.py +++ b/pyramid/config/views.py @@ -116,7 +116,7 @@ class MultiView(object): view = self.match(context, request) return view.__discriminator__(context, request) - def add(self, view, order, accept=None, phash=None, accept_order=None): + def add(self, view, order, phash=None, accept=None, accept_order=None): if phash is not None: for i, (s, v, h) in enumerate(list(self.views)): if phash == h: @@ -673,11 +673,11 @@ class ViewsConfiguratorMixin(object): A :term:`media type` that will be matched against the ``Accept`` HTTP request header. If this value is specified, it must be a - specific media type, such as ``text/html``. If the media type is - acceptable by the ``Accept`` header of the request, or if the - ``Accept`` header isn't set at all in the request, this predicate - will match. If this does not match the ``Accept`` header of the - request, view matching continues. + specific media type, such as ``text/html``, or a list of media types. + If the media type is acceptable by the ``Accept`` header of the + request, or if the ``Accept`` header isn't set at all in the request, + this predicate will match. If this does not match the ``Accept`` + header of the request, view matching continues. If ``accept`` is not specified, the ``HTTP_ACCEPT`` HTTP header is not taken into consideration when deciding whether or not to invoke @@ -686,10 +686,10 @@ class ViewsConfiguratorMixin(object): See :ref:`accept_content_negotation` for more information. .. versionchanged:: 1.10 - Media ranges such as ``text/*`` will now raise - :class:`pyramid.exceptions.ConfigurationError`. Previously, - these values had undefined behavior based on the version of - WebOb being used and was never fully supported. + Media ranges such as ``text/*`` are deprecated. Use a list of + explicit media types instead. Media ranges are non-deterministic. + + Also, added support for a list of media types. path_info @@ -818,24 +818,21 @@ class ViewsConfiguratorMixin(object): stacklevel=4, ) - if accept is not None and '*' in accept: - warnings.warn( - ('The usage of a media range in the "accept" view predicate ' - 'is deprecated as of Pyramid 1.10. Register multiple views ' - 'with explicit media ranges and read ' - '"Accept Header Content Negotiation" in the ' - '"View Configuration" documentation for more information.'), - DeprecationWarning, - stacklevel=4, - ) - - if accept is not None and is_nonstr_iter(accept): - raise ConfigurationError( - 'A list is not supported in the "accept" view predicate.', - ) - if accept is not None: - accept = accept.lower() + if isinstance(accept, str): + accept = list(accept) + accept = [accept_option.lower() for accept_option in accept] + if any('*' in accept_option for accept_option in accept): + warnings.warn( + ('The usage of a media range in the "accept" view ' + 'predicate is deprecated as of Pyramid 1.10. You may ' + 'pass multiple explicit media types, if necessary. ' + 'Please read "Accept Header Content Negotiation" in the ' + '"View Configuration" documentation for more ' + 'information.'), + DeprecationWarning, + stacklevel=4, + ) view = self.maybe_dotted(view) context = self.maybe_dotted(context) @@ -1092,7 +1089,19 @@ class ViewsConfiguratorMixin(object): if old_view is not None: break - def regclosure(): + old_phash = getattr(old_view, '__phash__', DEFAULT_PHASH) + is_multiview = IMultiView.providedBy(old_view) + want_multiview = ( + is_multiview + # multiple accept options requires a multiview + or (accept is not None and len(accept) > 1) + # no component was yet registered for exactly this triad + # or only one was registered but with the same hash as an + # override + or (old_view is not None and old_phash != phash) + ) + + if not want_multiview: if hasattr(derived_view, '__call_permissive__'): view_iface = ISecuredView else: @@ -1104,21 +1113,6 @@ class ViewsConfiguratorMixin(object): name ) - is_multiview = IMultiView.providedBy(old_view) - old_phash = getattr(old_view, '__phash__', DEFAULT_PHASH) - - if old_view is None: - # - No component was yet registered for any of our I*View - # interfaces exactly; this is the first view for this - # triad. - regclosure() - - elif (not is_multiview) and (old_phash == phash): - # - A single view component was previously registered with - # the same predicate hash as this view; this registration - # is therefore an override. - regclosure() - else: # - A view or multiview was already registered for this # triad, and the new view is not an override. @@ -1132,13 +1126,23 @@ class ViewsConfiguratorMixin(object): multiview = old_view else: multiview = MultiView(name) - old_accept = getattr(old_view, '__accept__', None) - old_order = getattr(old_view, '__order__', MAX_ORDER) - # don't bother passing accept_order here as we know we're - # adding another one right after which will re-sort - multiview.add(old_view, old_order, old_accept, old_phash) + if old_view is not None: + old_accept = getattr(old_view, '__accept__', None) + old_order = getattr(old_view, '__order__', MAX_ORDER) + # don't bother passing accept_order here as we know + # we're adding another one right after which will + # re-sort + multiview.add( + old_view, old_order, old_phash, old_accept) accept_order = self.registry.queryUtility(IAcceptOrder) - multiview.add(derived_view, order, accept, phash, accept_order) + for accept_option in accept: + multiview.add( + derived_view, + order, + phash, + accept_option, + accept_order, + ) for view_type in (IView, ISecuredView): # unregister any existing views self.registry.adapters.unregister( -- cgit v1.2.3 From 13b74d11e53f491ab20cadc05cb4290ab5c02601 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Thu, 6 Sep 2018 20:08:15 -0500 Subject: Revert "support a list of media types in the accept predicate" This reverts commit 762bd9c5c7b8974d0927eeef8074c7ad1e248f70. --- pyramid/config/routes.py | 46 ++++++++++----------- pyramid/config/views.py | 102 +++++++++++++++++++++++------------------------ 2 files changed, 70 insertions(+), 78 deletions(-) diff --git a/pyramid/config/routes.py b/pyramid/config/routes.py index 97561ca2b..b701a4bef 100644 --- a/pyramid/config/routes.py +++ b/pyramid/config/routes.py @@ -223,13 +223,13 @@ class RoutesConfiguratorMixin(object): accept - A :term:`media type` that will be matched against the ``Accept`` - HTTP request header. If this value is specified, it must be a - specific media type, such as ``text/html``, or a list of media types. - If the media type is acceptable by the ``Accept`` header of the - request, or if the ``Accept`` header isn't set at all in the request, - this predicate will match. If this does not match the ``Accept`` - header of the request, route matching continues. + A media type that will be matched against the ``Accept`` HTTP + request header. If this value is specified, it must be a specific + media type, such as ``text/html``. If the media type is acceptable + by the ``Accept`` header of the request, or if the ``Accept`` header + isn't set at all in the request, this predicate will match. If this + does not match the ``Accept`` header of the request, route matching + continues. If ``accept`` is not specified, the ``HTTP_ACCEPT`` HTTP header is not taken into consideration when deciding whether or not to select @@ -237,10 +237,10 @@ class RoutesConfiguratorMixin(object): .. versionchanged:: 1.10 - Media ranges such as ``text/*`` are deprecated. Use a list of - explicit media types instead. Media ranges are non-deterministic. - - Also, added support for a list of media types. + Media ranges such as ``text/*`` will now raise + :class:`pyramid.exceptions.ConfigurationError`. Previously, + these values had undefined behavior based on the version of + WebOb being used and was never fully supported. effective_principals @@ -299,21 +299,17 @@ class RoutesConfiguratorMixin(object): stacklevel=3 ) + if accept is not None and '*' in accept: + warnings.warn( + ('The usage of a media range in the "accept" route predicate ' + 'is deprecated as of Pyramid 1.10. Use a list of explicit ' + 'media types instead.'), + DeprecationWarning, + stacklevel=4, + ) + if accept is not None: - if isinstance(accept, str): - accept = list(accept) - accept = [accept_option.lower() for accept_option in accept] - if any('*' in accept_option for accept_option in accept): - warnings.warn( - ('The usage of a media range in the "accept" view ' - 'predicate is deprecated as of Pyramid 1.10. You may ' - 'pass multiple explicit media types, if necessary. ' - 'Please read "Accept Header Content Negotiation" in the ' - '"View Configuration" documentation for more ' - 'information.'), - DeprecationWarning, - stacklevel=4, - ) + accept = accept.lower() # these are route predicates; if they do not match, the next route # in the routelist will be tried diff --git a/pyramid/config/views.py b/pyramid/config/views.py index bd5ec6032..87584b858 100644 --- a/pyramid/config/views.py +++ b/pyramid/config/views.py @@ -116,7 +116,7 @@ class MultiView(object): view = self.match(context, request) return view.__discriminator__(context, request) - def add(self, view, order, phash=None, accept=None, accept_order=None): + def add(self, view, order, accept=None, phash=None, accept_order=None): if phash is not None: for i, (s, v, h) in enumerate(list(self.views)): if phash == h: @@ -673,11 +673,11 @@ class ViewsConfiguratorMixin(object): A :term:`media type` that will be matched against the ``Accept`` HTTP request header. If this value is specified, it must be a - specific media type, such as ``text/html``, or a list of media types. - If the media type is acceptable by the ``Accept`` header of the - request, or if the ``Accept`` header isn't set at all in the request, - this predicate will match. If this does not match the ``Accept`` - header of the request, view matching continues. + specific media type, such as ``text/html``. If the media type is + acceptable by the ``Accept`` header of the request, or if the + ``Accept`` header isn't set at all in the request, this predicate + will match. If this does not match the ``Accept`` header of the + request, view matching continues. If ``accept`` is not specified, the ``HTTP_ACCEPT`` HTTP header is not taken into consideration when deciding whether or not to invoke @@ -686,10 +686,10 @@ class ViewsConfiguratorMixin(object): See :ref:`accept_content_negotation` for more information. .. versionchanged:: 1.10 - Media ranges such as ``text/*`` are deprecated. Use a list of - explicit media types instead. Media ranges are non-deterministic. - - Also, added support for a list of media types. + Media ranges such as ``text/*`` will now raise + :class:`pyramid.exceptions.ConfigurationError`. Previously, + these values had undefined behavior based on the version of + WebOb being used and was never fully supported. path_info @@ -818,21 +818,24 @@ class ViewsConfiguratorMixin(object): stacklevel=4, ) + if accept is not None and '*' in accept: + warnings.warn( + ('The usage of a media range in the "accept" view predicate ' + 'is deprecated as of Pyramid 1.10. Register multiple views ' + 'with explicit media ranges and read ' + '"Accept Header Content Negotiation" in the ' + '"View Configuration" documentation for more information.'), + DeprecationWarning, + stacklevel=4, + ) + + if accept is not None and is_nonstr_iter(accept): + raise ConfigurationError( + 'A list is not supported in the "accept" view predicate.', + ) + if accept is not None: - if isinstance(accept, str): - accept = list(accept) - accept = [accept_option.lower() for accept_option in accept] - if any('*' in accept_option for accept_option in accept): - warnings.warn( - ('The usage of a media range in the "accept" view ' - 'predicate is deprecated as of Pyramid 1.10. You may ' - 'pass multiple explicit media types, if necessary. ' - 'Please read "Accept Header Content Negotiation" in the ' - '"View Configuration" documentation for more ' - 'information.'), - DeprecationWarning, - stacklevel=4, - ) + accept = accept.lower() view = self.maybe_dotted(view) context = self.maybe_dotted(context) @@ -1089,19 +1092,7 @@ class ViewsConfiguratorMixin(object): if old_view is not None: break - old_phash = getattr(old_view, '__phash__', DEFAULT_PHASH) - is_multiview = IMultiView.providedBy(old_view) - want_multiview = ( - is_multiview - # multiple accept options requires a multiview - or (accept is not None and len(accept) > 1) - # no component was yet registered for exactly this triad - # or only one was registered but with the same hash as an - # override - or (old_view is not None and old_phash != phash) - ) - - if not want_multiview: + def regclosure(): if hasattr(derived_view, '__call_permissive__'): view_iface = ISecuredView else: @@ -1113,6 +1104,21 @@ class ViewsConfiguratorMixin(object): name ) + is_multiview = IMultiView.providedBy(old_view) + old_phash = getattr(old_view, '__phash__', DEFAULT_PHASH) + + if old_view is None: + # - No component was yet registered for any of our I*View + # interfaces exactly; this is the first view for this + # triad. + regclosure() + + elif (not is_multiview) and (old_phash == phash): + # - A single view component was previously registered with + # the same predicate hash as this view; this registration + # is therefore an override. + regclosure() + else: # - A view or multiview was already registered for this # triad, and the new view is not an override. @@ -1126,23 +1132,13 @@ class ViewsConfiguratorMixin(object): multiview = old_view else: multiview = MultiView(name) - if old_view is not None: - old_accept = getattr(old_view, '__accept__', None) - old_order = getattr(old_view, '__order__', MAX_ORDER) - # don't bother passing accept_order here as we know - # we're adding another one right after which will - # re-sort - multiview.add( - old_view, old_order, old_phash, old_accept) + old_accept = getattr(old_view, '__accept__', None) + old_order = getattr(old_view, '__order__', MAX_ORDER) + # don't bother passing accept_order here as we know we're + # adding another one right after which will re-sort + multiview.add(old_view, old_order, old_accept, old_phash) accept_order = self.registry.queryUtility(IAcceptOrder) - for accept_option in accept: - multiview.add( - derived_view, - order, - phash, - accept_option, - accept_order, - ) + multiview.add(derived_view, order, accept, phash, accept_order) for view_type in (IView, ISecuredView): # unregister any existing views self.registry.adapters.unregister( -- cgit v1.2.3 From 30f79dc6f3f7b6a478004ca4e41171468095c235 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Sun, 9 Sep 2018 21:18:43 -0500 Subject: enable sorting of offers --- docs/narr/extconfig.rst | 4 +- docs/narr/viewconfig.rst | 30 +++++++++- pyramid/config/routes.py | 40 +++++-------- pyramid/config/views.py | 142 ++++++++++++++++++++++++--------------------- pyramid/predicates.py | 7 +-- pyramid/tests/test_util.py | 60 +++++++++++++++++++ pyramid/util.py | 79 +++++++++++++++++++++++++ tox.ini | 3 + 8 files changed, 264 insertions(+), 101 deletions(-) diff --git a/docs/narr/extconfig.rst b/docs/narr/extconfig.rst index a9ea93e60..4c6c8b70b 100644 --- a/docs/narr/extconfig.rst +++ b/docs/narr/extconfig.rst @@ -255,12 +255,12 @@ Pre-defined Phases :const:`pyramid.config.PHASE1_CONFIG` -- :meth:`pyramid.config.Configurator.add_accept_view_option` +- :meth:`pyramid.config.Configurator.add_accept_view_order` - :meth:`pyramid.config.Configurator.add_renderer` - :meth:`pyramid.config.Configurator.add_route_predicate` - :meth:`pyramid.config.Configurator.add_subscriber_predicate` -- :meth:`pyramid.config.Configurator.add_view_predicate` - :meth:`pyramid.config.Configurator.add_view_deriver` +- :meth:`pyramid.config.Configurator.add_view_predicate` - :meth:`pyramid.config.Configurator.override_asset` - :meth:`pyramid.config.Configurator.set_authorization_policy` - :meth:`pyramid.config.Configurator.set_default_csrf_options` diff --git a/docs/narr/viewconfig.rst b/docs/narr/viewconfig.rst index 2060393bb..cd49d6e94 100644 --- a/docs/narr/viewconfig.rst +++ b/docs/narr/viewconfig.rst @@ -1027,7 +1027,8 @@ See :ref:`environment_chapter` for more information about how, and where to set these values. .. index:: - single: HTTP caching + single: Accept + single: Accept content negotation .. _accept_content_negotation: @@ -1059,10 +1060,11 @@ Similarly, if the client specifies a media type that no view is registered to ha There are a few cases in which the client may specify ambiguous constraints: - ``Accept: */*``. +- More than one acceptable media type with the same quality. - A missing ``Accept`` header. - An invalid ``Accept`` header. -In these cases the preferred view is not clearly defined (see :rfc:`7231#section-5.3.2`) and :app:`Pyramid` will select one randomly. +In these cases the preferred view is not clearly defined (see :rfc:`7231#section-5.3.2`) and :app:`Pyramid` will select one semi-randomly. This can be controlled by telling :app:`Pyramid` what the preferred relative ordering is between various media types by using :meth:`pyramid.config.Configurator.add_accept_view_order`. For example: @@ -1082,9 +1084,30 @@ For example: In this case, the ``application/json`` view should always be selected in cases where it is otherwise ambiguous. +.. _default_accept_ordering: + Default Accept Ordering +++++++++++++++++++++++ +:app:`Pyramid` will always sort multiple views with the same ``(name, context, route_name)`` first by the specificity of the ``accept`` offer. +This means that ``text/plain`` will always be offered before ``text/*``. +Similarly ``text/plain;charset=utf8`` will always be offered before ``text/plain``. +The following order is always preserved between the following offers (more preferred to less preferred): + +- ``type/subtype;params`` +- ``type/subtype`` +- ``type/*`` +- ``*/*`` + +Within each of these levels of specificity, the ordering is ambiguous and may be controlled using :meth:`pyramid.config.Configurator.add_accept_view_order`. For example, to sort ``text/plain`` higher than ``text/html`` and to prefer a ``charset=utf8`` versus a ``charset=latin-1`` within the ``text/plain`` media type: + +.. code-block:: python + + config.add_accept_view_order('text/plain', weighs_more_than='text/html') + config.add_accept_view_order('text/plain;charset=utf8', weighs_more_than='text/plain;charset=latin-1') + +It is an error to try and sort accept headers across levels of specificity. You can only sort a ``type/subtype`` against another ``type/subtype``, not against a ``type/*``. That ordering is a hard requirement. + By default, :app:`Pyramid` defines a very simple priority ordering for views that prefers human-readable responses over JSON: - ``text/html`` @@ -1099,6 +1122,9 @@ Therefore, the motivation for this ordering is to optimize for readability. Media types that are not listed above are ordered randomly during :term:`view lookup` between otherwise-similar views. The defaults can be overridden using :meth:`pyramid.config.Configurator.add_accept_view_order` as described above. +.. index:: + single: HTTP caching + .. _influencing_http_caching: Influencing HTTP Caching diff --git a/pyramid/config/routes.py b/pyramid/config/routes.py index b701a4bef..e2f2e7782 100644 --- a/pyramid/config/routes.py +++ b/pyramid/config/routes.py @@ -13,7 +13,10 @@ from pyramid.exceptions import ConfigurationError from pyramid.request import route_request_iface from pyramid.urldispatch import RoutesMapper -from pyramid.util import as_sorted_tuple +from pyramid.util import ( + as_sorted_tuple, + is_nonstr_iter, +) import pyramid.predicates @@ -223,25 +226,18 @@ class RoutesConfiguratorMixin(object): accept - A media type that will be matched against the ``Accept`` HTTP - request header. If this value is specified, it must be a specific - media type, such as ``text/html``. If the media type is acceptable - by the ``Accept`` header of the request, or if the ``Accept`` header - isn't set at all in the request, this predicate will match. If this - does not match the ``Accept`` header of the request, route matching - continues. + A :term:`media type` that will be matched against the ``Accept`` + HTTP request header. This value may be a specific media type such + as ``text/html``, or a range like ``text/*``, or a list of the same. + If the media type is acceptable by the ``Accept`` header of the + request, or if the ``Accept`` header isn't set at all in the + request, this predicate will match. If this does not match the + ``Accept`` header of the request, route matching continues. If ``accept`` is not specified, the ``HTTP_ACCEPT`` HTTP header is not taken into consideration when deciding whether or not to select the route. - - .. versionchanged:: 1.10 - Media ranges such as ``text/*`` will now raise - :class:`pyramid.exceptions.ConfigurationError`. Previously, - these values had undefined behavior based on the version of - WebOb being used and was never fully supported. - effective_principals If specified, this value should be a :term:`principal` identifier or @@ -299,17 +295,11 @@ class RoutesConfiguratorMixin(object): stacklevel=3 ) - if accept is not None and '*' in accept: - warnings.warn( - ('The usage of a media range in the "accept" route predicate ' - 'is deprecated as of Pyramid 1.10. Use a list of explicit ' - 'media types instead.'), - DeprecationWarning, - stacklevel=4, - ) - if accept is not None: - accept = accept.lower() + if is_nonstr_iter(accept): + accept = [accept] + + accept = [accept_option.lower() for accept_option in accept] # these are route predicates; if they do not match, the next route # in the routelist will be tried diff --git a/pyramid/config/views.py b/pyramid/config/views.py index 87584b858..45a89e5fb 100644 --- a/pyramid/config/views.py +++ b/pyramid/config/views.py @@ -5,6 +5,7 @@ import operator import os import warnings +from webob.acceptparse import Accept from zope.interface import ( Interface, implementedBy, @@ -67,6 +68,7 @@ from pyramid.view import AppendSlashNotFoundViewFactory from pyramid.util import ( as_sorted_tuple, + sort_accept_offers, TopologicalSorter, ) @@ -116,14 +118,14 @@ class MultiView(object): view = self.match(context, request) return view.__discriminator__(context, request) - def add(self, view, order, accept=None, phash=None, accept_order=None): + def add(self, view, order, phash=None, accept=None, accept_order=None): if phash is not None: for i, (s, v, h) in enumerate(list(self.views)): if phash == h: self.views[i] = (order, view, phash) return - if accept is None or '*' in accept: + if accept is None: self.views.append((order, view, phash)) self.views.sort(key=operator.itemgetter(0)) else: @@ -138,15 +140,9 @@ class MultiView(object): # dedupe accepts and sort appropriately accepts = set(self.accepts) accepts.add(accept) - if accept_order is not None: - sorted_accepts = [] - for accept in accept_order.sorted(): - if accept in accepts: - sorted_accepts.append(accept) - accepts.remove(accept) - sorted_accepts.extend(accepts) - accepts = sorted_accepts - self.accepts = list(accepts) + if accept_order: + accept_order = accept_order.sorted() + self.accepts = sort_accept_offers(accepts, accept_order) def get_views(self, request): if self.accepts and hasattr(request, 'accept'): @@ -246,6 +242,14 @@ def viewdefaults(wrapped): return wrapped(self, *arg, **defaults) return functools.wraps(wrapped)(wrapper) +def combine_decorators(*decorators): + def decorated(view_callable): + # reversed() allows a more natural ordering in the api + for decorator in reversed(decorators): + view_callable = decorator(view_callable) + return view_callable + return decorated + class ViewsConfiguratorMixin(object): @viewdefaults @action_method @@ -672,8 +676,8 @@ class ViewsConfiguratorMixin(object): accept A :term:`media type` that will be matched against the ``Accept`` - HTTP request header. If this value is specified, it must be a - specific media type, such as ``text/html``. If the media type is + HTTP request header. This value may be a specific media type such + as ``text/html``, or a range like ``text/*``. If the media type is acceptable by the ``Accept`` header of the request, or if the ``Accept`` header isn't set at all in the request, this predicate will match. If this does not match the ``Accept`` header of the @@ -685,12 +689,6 @@ class ViewsConfiguratorMixin(object): See :ref:`accept_content_negotation` for more information. - .. versionchanged:: 1.10 - Media ranges such as ``text/*`` will now raise - :class:`pyramid.exceptions.ConfigurationError`. Previously, - these values had undefined behavior based on the version of - WebOb being used and was never fully supported. - path_info This value represents a regular expression pattern that will @@ -818,23 +816,11 @@ class ViewsConfiguratorMixin(object): stacklevel=4, ) - if accept is not None and '*' in accept: - warnings.warn( - ('The usage of a media range in the "accept" view predicate ' - 'is deprecated as of Pyramid 1.10. Register multiple views ' - 'with explicit media ranges and read ' - '"Accept Header Content Negotiation" in the ' - '"View Configuration" documentation for more information.'), - DeprecationWarning, - stacklevel=4, - ) - - if accept is not None and is_nonstr_iter(accept): - raise ConfigurationError( - 'A list is not supported in the "accept" view predicate.', - ) - if accept is not None: + if is_nonstr_iter(accept): + raise ConfigurationError( + 'A list is not supported in the "accept" view predicate.', + ) accept = accept.lower() view = self.maybe_dotted(view) @@ -843,16 +829,8 @@ class ViewsConfiguratorMixin(object): containment = self.maybe_dotted(containment) mapper = self.maybe_dotted(mapper) - def combine(*decorators): - def decorated(view_callable): - # reversed() allows a more natural ordering in the api - for decorator in reversed(decorators): - view_callable = decorator(view_callable) - return view_callable - return decorated - if is_nonstr_iter(decorator): - decorator = combine(*map(self.maybe_dotted, decorator)) + decorator = combine_decorators(*map(self.maybe_dotted, decorator)) else: decorator = self.maybe_dotted(decorator) @@ -1092,7 +1070,17 @@ class ViewsConfiguratorMixin(object): if old_view is not None: break - def regclosure(): + old_phash = getattr(old_view, '__phash__', DEFAULT_PHASH) + is_multiview = IMultiView.providedBy(old_view) + want_multiview = ( + is_multiview + # no component was yet registered for exactly this triad + # or only one was registered but with the same phash, meaning + # that this view is an override + or (old_view is not None and old_phash != phash) + ) + + if not want_multiview: if hasattr(derived_view, '__call_permissive__'): view_iface = ISecuredView else: @@ -1104,21 +1092,6 @@ class ViewsConfiguratorMixin(object): name ) - is_multiview = IMultiView.providedBy(old_view) - old_phash = getattr(old_view, '__phash__', DEFAULT_PHASH) - - if old_view is None: - # - No component was yet registered for any of our I*View - # interfaces exactly; this is the first view for this - # triad. - regclosure() - - elif (not is_multiview) and (old_phash == phash): - # - A single view component was previously registered with - # the same predicate hash as this view; this registration - # is therefore an override. - regclosure() - else: # - A view or multiview was already registered for this # triad, and the new view is not an override. @@ -1136,9 +1109,9 @@ class ViewsConfiguratorMixin(object): old_order = getattr(old_view, '__order__', MAX_ORDER) # don't bother passing accept_order here as we know we're # adding another one right after which will re-sort - multiview.add(old_view, old_order, old_accept, old_phash) + multiview.add(old_view, old_order, old_phash, old_accept) accept_order = self.registry.queryUtility(IAcceptOrder) - multiview.add(derived_view, order, accept, phash, accept_order) + multiview.add(derived_view, order, phash, accept, accept_order) for view_type in (IView, ISecuredView): # unregister any existing views self.registry.adapters.unregister( @@ -1283,7 +1256,7 @@ class ViewsConfiguratorMixin(object): to specify a server-side, relative ordering between accept media types. ``value`` should be a :term:`media type` as specified by - :rfc:`7231#section-5.3.2`. For example, ``text/plain``, + :rfc:`7231#section-5.3.2`. For example, ``text/plain;charset=utf8``, ``application/json`` or ``text/html``. ``weighs_more_than`` and ``weighs_less_than`` control the ordering @@ -1294,10 +1267,47 @@ class ViewsConfiguratorMixin(object): .. versionadded:: 1.10 """ - value = value.lower() - if '*' in value: + if value == '*/*': raise ConfigurationError( - '"accept" ordering is done between media types, not ranges') + 'cannot specify an ordering for an offer of */*') + + def normalize_type(type): + return type.lower() + + def check_type(than): + than_type, than_subtype, than_params = Accept.parse_offer(than) + if ( + # text/* vs text/plain + (offer_subtype == '*') ^ (than_subtype == '*') + # text/plain vs text/html;charset=utf8 + or (bool(offer_params) ^ bool(than_params)) + ): + raise ConfigurationError( + 'cannot compare across media range specificity levels') + # text/plain;charset=utf8 vs text/html;charset=utf8 + if offer_params and ( + offer_subtype != than_subtype or offer_type != than_type + ): + raise ConfigurationError( + 'cannot compare params across media types') + + value = normalize_type(value) + offer_type, offer_subtype, offer_params = Accept.parse_offer(value) + + if weighs_more_than: + if not is_nonstr_iter(weighs_more_than): + weighs_more_than = [weighs_more_than] + weighs_more_than = [normalize_type(w) for w in weighs_more_than] + for than in weighs_more_than: + check_type(than) + + if weighs_less_than: + if not is_nonstr_iter(weighs_less_than): + weighs_less_than = [weighs_less_than] + weighs_less_than = [normalize_type(w) for w in weighs_less_than] + for than in weighs_less_than: + check_type(than) + discriminator = ('accept view order', value) intr = self.introspectable( 'accept view order', diff --git a/pyramid/predicates.py b/pyramid/predicates.py index 262b28d89..5bd98fdf2 100644 --- a/pyramid/predicates.py +++ b/pyramid/predicates.py @@ -130,22 +130,17 @@ class HeaderPredicate(object): return self.val.match(val) is not None class AcceptPredicate(object): - _use_deprecated_range_fallback = False - def __init__(self, val, config): if not is_nonstr_iter(val): - self._use_deprecated_range_fallback = '*' in val val = (val,) self.values = val def text(self): - return 'accept = %s' % (self.values,) + return 'accept = %s' % (', '.join(self.values),) phash = text def __call__(self, context, request): - if self._use_deprecated_range_fallback: - return self.values[0] in request.accept return bool(request.accept.acceptable_offers(self.values)) class ContainmentPredicate(object): diff --git a/pyramid/tests/test_util.py b/pyramid/tests/test_util.py index a76cd2017..907ca7351 100644 --- a/pyramid/tests/test_util.py +++ b/pyramid/tests/test_util.py @@ -1109,3 +1109,63 @@ class TestSimpleSerializer(unittest.TestCase): def test_dumps(self): inst = self._makeOne() self.assertEqual(inst.dumps('abc'), bytes_('abc')) + + +class Test_sort_accept_offers(unittest.TestCase): + def _callFUT(self, offers, order=None): + from pyramid.util import sort_accept_offers + return sort_accept_offers(offers, order) + + def test_default_specificities(self): + result = self._callFUT(['*/*', 'text/*', 'text/html', 'text/html;charset=utf8']) + self.assertEqual(result, [ + 'text/html;charset=utf8', 'text/html', 'text/*', '*/*', + ]) + + def test_wildcard_type_order(self): + result = self._callFUT( + ['*/*', 'text/*', 'image/*'], + ['image/*', 'text/*'], + ) + self.assertEqual(result, ['image/*', 'text/*', '*/*']) + + def test_specific_type_order(self): + result = self._callFUT( + ['text/html', 'application/json', 'text/html;charset=utf8', 'text/plain'], + ['application/json', 'text/html'], + ) + self.assertEqual(result, [ + 'application/json', 'text/html;charset=utf8', 'text/html', 'text/plain', + ]) + + def test_params_order(self): + result = self._callFUT( + ['text/html;charset=utf8', 'text/html;charset=latin1', 'text/html;foo=bar'], + ['text/html;charset=latin1', 'text/html;charset=utf8'], + ) + self.assertEqual(result, [ + 'text/html;charset=latin1', 'text/html;charset=utf8', 'text/html;foo=bar', + ]) + + def test_params_inherit_type_prefs(self): + result = self._callFUT( + ['text/html;charset=utf8', 'text/plain;charset=latin1'], + ['text/plain', 'text/html'], + ) + self.assertEqual(result, ['text/plain;charset=latin1', 'text/html;charset=utf8']) + + def test_params_inherit_wildcard_prefs(self): + result = self._callFUT( + ['image/png;progressive=1', 'text/html;charset=utf8'], + ['text/*', 'image/*'], + ) + self.assertEqual(result, ['text/html;charset=utf8', 'image/png;progressive=1']) + + def test_type_overrides_wildcard_prefs(self): + result = self._callFUT( + ['text/html;charset=utf8', 'image/png', 'foo/bar', 'text/bar'], + ['foo/*', 'text/*', 'image/*', 'image/png', 'text/html'], + ) + self.assertEqual(result, [ + 'image/png', 'text/html;charset=utf8', 'foo/bar', 'text/bar', + ]) diff --git a/pyramid/util.py b/pyramid/util.py index 6655455bf..677880794 100644 --- a/pyramid/util.py +++ b/pyramid/util.py @@ -1,3 +1,5 @@ +from collections import defaultdict +from collections import namedtuple from contextlib import contextmanager import functools try: @@ -7,6 +9,7 @@ except ImportError: # pragma: no cover compare_digest = None import inspect import weakref +from webob.acceptparse import Accept from pyramid.exceptions import ( ConfigurationError, @@ -649,3 +652,79 @@ class SimpleSerializer(object): def dumps(self, appstruct): return bytes_(appstruct) + +def sort_accept_offers(offers, order=None): + """ + Sort a list of offers by specificity and preference. + + Supported offers are of the following forms, ordered by specificity + (higher to lower): + + - ``type/subtype;params`` and ``type/subtype`` + - ``type/*`` + - ``*/*`` + + :param offers: A list of offers to be sorted. + :param order: A weighted list of offer where items closer to the start of + the list will be a preferred over items closer to the end. + :param is_order_parsed: If the list of orders is pre-parsed then do not + parse it again. + :return: A list of offers sorted first by specificity (higher to lower) + then by ``order``. + + """ + if order is None: + order = [] + + max_weight = len(offers) + + def find_order_index(value, default=None): + return next((i for i, x in enumerate(order) if x == value), default) + + def offer_sort_key(value): + """ + (category, type_weight, params_weight) + + category: + 1 - foo/bar and foo/bar;params + 2 - foo/* + 3 - */* + + type_weight: + if category 1 & 2: + - index of type/* in order list + - ``max_weight`` if no match is found + + - index of type/subtype in order list + - index of type/* in order list + ``max_weight`` + - ``max_weight * 2`` if no match is found + + params_weight: + - index of specific ``type/subtype;params`` in order list + - ``max_weight`` if not found + - ``max_weight + 1`` if no params at all + + """ + parsed = Accept.parse_offer(value) + + if value == '*/*': + return (3, 0, 0) + + elif parsed.subtype == '*': + type_w = find_order_index(value, max_weight) + return (2, type_w, 0) + + type_w = find_order_index(parsed.type + '/' + parsed.subtype, None) + if type_w is None: + type_w = max_weight + find_order_index( + parsed.type + '/*', max_weight) + + if parsed.params: + param_w = find_order_index(value, max_weight) + + else: + param_w = max_weight + 1 + + return (1, type_w, param_w) + + return sorted(offers, key=offer_sort_key) diff --git a/tox.ini b/tox.ini index 5a73bc426..76428846f 100644 --- a/tox.ini +++ b/tox.ini @@ -25,6 +25,9 @@ commands = extras = testing +deps = + -egit+https://github.com/mmerickel/webob.git@accept-parse-offer#egg=webob + [testenv:py27-scaffolds] basepython = python2.7 commands = -- cgit v1.2.3 From c3c83eb71ff3352cee754b404ad4d65ab02fa464 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Thu, 27 Sep 2018 01:22:02 -0500 Subject: fix old tests --- pyramid/config/routes.py | 2 +- pyramid/tests/test_config/test_routes.py | 30 +++++++++++++++++++++-- pyramid/tests/test_config/test_views.py | 42 +++++++++++++++----------------- 3 files changed, 48 insertions(+), 26 deletions(-) diff --git a/pyramid/config/routes.py b/pyramid/config/routes.py index e2f2e7782..01917537d 100644 --- a/pyramid/config/routes.py +++ b/pyramid/config/routes.py @@ -296,7 +296,7 @@ class RoutesConfiguratorMixin(object): ) if accept is not None: - if is_nonstr_iter(accept): + if not is_nonstr_iter(accept): accept = [accept] accept = [accept_option.lower() for accept_option in accept] diff --git a/pyramid/tests/test_config/test_routes.py b/pyramid/tests/test_config/test_routes.py index 1d2530c02..afae4db33 100644 --- a/pyramid/tests/test_config/test_routes.py +++ b/pyramid/tests/test_config/test_routes.py @@ -182,10 +182,25 @@ class RoutesConfiguratorMixinTests(unittest.TestCase): route = self._assertRoute(config, 'name', 'path', 1) predicate = route.predicates[0] request = self._makeRequest(config) - request.accept = ['text/xml'] + request.accept = DummyAccept('text/xml') self.assertEqual(predicate(None, request), True) request = self._makeRequest(config) - request.accept = ['text/html'] + request.accept = DummyAccept('text/html') + self.assertEqual(predicate(None, request), False) + + def test_add_route_with_accept_list(self): + config = self._makeOne(autocommit=True) + config.add_route('name', 'path', accept=['text/xml', 'text/plain']) + route = self._assertRoute(config, 'name', 'path', 1) + predicate = route.predicates[0] + request = self._makeRequest(config) + request.accept = DummyAccept('text/xml') + self.assertEqual(predicate(None, request), True) + request = self._makeRequest(config) + request.accept = DummyAccept('text/plain') + self.assertEqual(predicate(None, request), True) + request = self._makeRequest(config) + request.accept = DummyAccept('text/html') self.assertEqual(predicate(None, request), False) def test_add_route_no_pattern_with_path(self): @@ -253,3 +268,14 @@ class DummyRequest: self.environ = environ self.params = {} self.cookies = {} + +class DummyAccept(object): + def __init__(self, *matches): + self.matches = list(matches) + + def acceptable_offers(self, offers): + results = [] + for match in self.matches: + if match in offers: + results.append((match, 1.0)) + return results diff --git a/pyramid/tests/test_config/test_views.py b/pyramid/tests/test_config/test_views.py index 1c99d2ac5..db15a39fb 100644 --- a/pyramid/tests/test_config/test_views.py +++ b/pyramid/tests/test_config/test_views.py @@ -842,7 +842,7 @@ class TestViewsConfigurationMixin(unittest.TestCase): config.add_view(view=view2, renderer=null_renderer) wrapper = self._getViewCallable(config) self.assertTrue(IMultiView.providedBy(wrapper)) - self.assertEqual([x[:2] for x in wrapper.views], [(view2, None)]) + self.assertEqual([(x[0], x[2]) for x in wrapper.views], [(view2, None)]) self.assertEqual(wrapper(None, None), 'OK1') def test_add_view_exc_multiview_replaces_multiviews(self): @@ -869,13 +869,13 @@ class TestViewsConfigurationMixin(unittest.TestCase): hot_wrapper = self._getViewCallable( config, ctx_iface=implementedBy(RuntimeError)) self.assertTrue(IMultiView.providedBy(hot_wrapper)) - self.assertEqual([x[:2] for x in hot_wrapper.views], [(view2, None)]) + self.assertEqual([(x[0], x[2]) for x in hot_wrapper.views], [(view2, None)]) self.assertEqual(hot_wrapper(None, None), 'OK1') exc_wrapper = self._getViewCallable( config, exc_iface=implementedBy(RuntimeError)) self.assertTrue(IMultiView.providedBy(exc_wrapper)) - self.assertEqual([x[:2] for x in exc_wrapper.views], [(view2, None)]) + self.assertEqual([(x[0], x[2]) for x in exc_wrapper.views], [(view2, None)]) self.assertEqual(exc_wrapper(None, None), 'OK1') def test_add_view_exc_multiview_replaces_only_exc_multiview(self): @@ -908,7 +908,7 @@ class TestViewsConfigurationMixin(unittest.TestCase): exc_wrapper = self._getViewCallable( config, exc_iface=implementedBy(RuntimeError)) self.assertTrue(IMultiView.providedBy(exc_wrapper)) - self.assertEqual([x[:2] for x in exc_wrapper.views], [(view2, None)]) + self.assertEqual([(x[0], x[2]) for x in exc_wrapper.views], [(view2, None)]) self.assertEqual(exc_wrapper(None, None), 'OK1') def test_add_view_multiview_context_superclass_then_subclass(self): @@ -1465,7 +1465,7 @@ class TestViewsConfigurationMixin(unittest.TestCase): config.add_view(view=view, accept='text/xml', renderer=null_renderer) wrapper = self._getViewCallable(config) request = self._makeRequest(config) - request.accept = ['text/xml'] + request.accept = DummyAccept('text/xml') self.assertEqual(wrapper(None, request), 'OK') def test_add_view_with_accept_nomatch(self): @@ -1474,7 +1474,7 @@ class TestViewsConfigurationMixin(unittest.TestCase): config.add_view(view=view, accept='text/xml') wrapper = self._getViewCallable(config) request = self._makeRequest(config) - request.accept = ['text/html'] + request.accept = DummyAccept('text/html') self._assertNotFound(wrapper, None, request) def test_add_view_with_containment_true(self): @@ -2499,19 +2499,17 @@ class TestMultiView(unittest.TestCase): self.assertEqual(mv.views, [(100, 'view', None)]) mv.add('view2', 99) self.assertEqual(mv.views, [(99, 'view2', None), (100, 'view', None)]) - mv.add('view3', 100, 'text/html') + mv.add('view3', 100, accept='text/html') self.assertEqual(mv.media_views['text/html'], [(100, 'view3', None)]) - mv.add('view4', 99, 'text/html', 'abc') + mv.add('view4', 99, 'abc', accept='text/html') self.assertEqual(mv.media_views['text/html'], [(99, 'view4', 'abc'), (100, 'view3', None)]) - mv.add('view5', 100, 'text/xml') + mv.add('view5', 100, accept='text/xml') self.assertEqual(mv.media_views['text/xml'], [(100, 'view5', None)]) self.assertEqual(set(mv.accepts), set(['text/xml', 'text/html'])) self.assertEqual(mv.views, [(99, 'view2', None), (100, 'view', None)]) - mv.add('view6', 98, 'text/*') - self.assertEqual(mv.views, [(98, 'view6', None), - (99, 'view2', None), - (100, 'view', None)]) + mv.add('view6', 98, accept='text/*') + self.assertEqual(mv.media_views['text/*'], [(98, 'view6', None)]) def test_add_with_phash(self): mv = self._makeOne() @@ -3440,14 +3438,12 @@ class DummyAccept(object): def __init__(self, *matches): self.matches = list(matches) - def best_match(self, offered): - if self.matches: - for match in self.matches: - if match in offered: - self.matches.remove(match) - return match - def __contains__(self, val): - return val in self.matches + def acceptable_offers(self, offers): + results = [] + for match in self.matches: + if match in offers: + results.append((match, 1.0)) + return results class DummyConfig: def __init__(self): @@ -3475,8 +3471,8 @@ class DummyMultiView: def __init__(self): self.views = [] self.name = 'name' - def add(self, view, order, accept=None, phash=None): - self.views.append((view, accept, phash)) + def add(self, view, order, phash=None, accept=None, accept_order=None): + self.views.append((view, phash, accept, accept_order)) def __call__(self, context, request): return 'OK1' def __permitted__(self, context, request): -- cgit v1.2.3 From f081ae991a9107363fceeeeccd361c2f85bdd046 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Thu, 27 Sep 2018 21:53:03 -0500 Subject: fix docs --- docs/narr/viewconfig.rst | 9 ++++++--- pyramid/config/views.py | 4 ++-- pyramid/util.py | 4 +--- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/docs/narr/viewconfig.rst b/docs/narr/viewconfig.rst index cd49d6e94..7e6617b78 100644 --- a/docs/narr/viewconfig.rst +++ b/docs/narr/viewconfig.rst @@ -352,7 +352,7 @@ configured view. If ``accept`` is not specified, the ``HTTP_ACCEPT`` HTTP header is not taken into consideration when deciding whether or not to invoke the associated view callable. - See :ref:`accept_content_negotation` for more information. + See :ref:`accept_content_negotiation` for more information. .. versionchanged:: 1.10 Media ranges such as ``text/*`` will now raise :class:`pyramid.exceptions.ConfigurationError`. @@ -1028,9 +1028,9 @@ these values. .. index:: single: Accept - single: Accept content negotation + single: Accept content negotiation -.. _accept_content_negotation: +.. _accept_content_negotiation: Accept Header Content Negotiation --------------------------------- @@ -1084,6 +1084,9 @@ For example: In this case, the ``application/json`` view should always be selected in cases where it is otherwise ambiguous. +.. index:: + single: default accept ordering + .. _default_accept_ordering: Default Accept Ordering diff --git a/pyramid/config/views.py b/pyramid/config/views.py index 45a89e5fb..e40a851ff 100644 --- a/pyramid/config/views.py +++ b/pyramid/config/views.py @@ -687,7 +687,7 @@ class ViewsConfiguratorMixin(object): not taken into consideration when deciding whether or not to invoke the associated view callable. - See :ref:`accept_content_negotation` for more information. + See :ref:`accept_content_negotiation` for more information. path_info @@ -1262,7 +1262,7 @@ class ViewsConfiguratorMixin(object): ``weighs_more_than`` and ``weighs_less_than`` control the ordering of media types. Each value may be a string or a list of strings. - See :ref:`accept_content_negotation` for more information. + See :ref:`accept_content_negotiation` for more information. .. versionadded:: 1.10 diff --git a/pyramid/util.py b/pyramid/util.py index 677880794..708931eea 100644 --- a/pyramid/util.py +++ b/pyramid/util.py @@ -665,10 +665,8 @@ def sort_accept_offers(offers, order=None): - ``*/*`` :param offers: A list of offers to be sorted. - :param order: A weighted list of offer where items closer to the start of + :param order: A weighted list of offers where items closer to the start of the list will be a preferred over items closer to the end. - :param is_order_parsed: If the list of orders is pre-parsed then do not - parse it again. :return: A list of offers sorted first by specificity (higher to lower) then by ``order``. -- cgit v1.2.3 From f2294fb6969a7bc04642f2f987dec5ee131ad98c Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Thu, 27 Sep 2018 22:18:56 -0500 Subject: move sort_accept_offers to pyramid.config.util keeping this code isolated for now as it's kind of crazy --- pyramid/config/util.py | 76 +++++++++++++++++++++++++++++++++ pyramid/config/views.py | 2 +- pyramid/tests/test_config/test_util.py | 59 ++++++++++++++++++++++++++ pyramid/tests/test_util.py | 60 -------------------------- pyramid/util.py | 77 ---------------------------------- 5 files changed, 136 insertions(+), 138 deletions(-) diff --git a/pyramid/config/util.py b/pyramid/config/util.py index aedebd9e2..7024fa862 100644 --- a/pyramid/config/util.py +++ b/pyramid/config/util.py @@ -1,6 +1,7 @@ import functools from hashlib import md5 import traceback +from webob.acceptparse import Accept from zope.interface import implementer from pyramid.compat import ( @@ -218,3 +219,78 @@ class PredicateList(object): score = score | bit order = (MAX_ORDER - score) / (len(preds) + 1) return order, preds, phash.hexdigest() + + +def sort_accept_offers(offers, order=None): + """ + Sort a list of offers by specificity and preference. + + Supported offers are of the following forms, ordered by specificity + (higher to lower): + + - ``type/subtype;params`` and ``type/subtype`` + - ``type/*`` + - ``*/*`` + + :param offers: A list of offers to be sorted. + :param order: A weighted list of offers where items closer to the start of + the list will be a preferred over items closer to the end. + :return: A list of offers sorted first by specificity (higher to lower) + then by ``order``. + + """ + if order is None: + order = [] + + max_weight = len(offers) + + def find_order_index(value, default=None): + return next((i for i, x in enumerate(order) if x == value), default) + + def offer_sort_key(value): + """ + (category, type_weight, params_weight) + + category: + 1 - foo/bar and foo/bar;params + 2 - foo/* + 3 - */* + + type_weight: + if category 1 & 2: + - index of type/* in order list + - ``max_weight`` if no match is found + + - index of type/subtype in order list + - index of type/* in order list + ``max_weight`` + - ``max_weight * 2`` if no match is found + + params_weight: + - index of specific ``type/subtype;params`` in order list + - ``max_weight`` if not found + - ``max_weight + 1`` if no params at all + + """ + parsed = Accept.parse_offer(value) + + if value == '*/*': + return (3, 0, 0) + + elif parsed.subtype == '*': + type_w = find_order_index(value, max_weight) + return (2, type_w, 0) + + type_w = find_order_index(parsed.type + '/' + parsed.subtype, None) + if type_w is None: + type_w = max_weight + find_order_index( + parsed.type + '/*', max_weight) + + if parsed.params: + param_w = find_order_index(value, max_weight) + + else: + param_w = max_weight + 1 + + return (1, type_w, param_w) + + return sorted(offers, key=offer_sort_key) diff --git a/pyramid/config/views.py b/pyramid/config/views.py index e40a851ff..26e86d7d3 100644 --- a/pyramid/config/views.py +++ b/pyramid/config/views.py @@ -68,7 +68,6 @@ from pyramid.view import AppendSlashNotFoundViewFactory from pyramid.util import ( as_sorted_tuple, - sort_accept_offers, TopologicalSorter, ) @@ -90,6 +89,7 @@ from pyramid.config.util import ( DEFAULT_PHASH, MAX_ORDER, predvalseq, + sort_accept_offers, ) urljoin = urlparse.urljoin diff --git a/pyramid/tests/test_config/test_util.py b/pyramid/tests/test_config/test_util.py index 99c67e8c6..bbda615c9 100644 --- a/pyramid/tests/test_config/test_util.py +++ b/pyramid/tests/test_config/test_util.py @@ -431,6 +431,65 @@ class TestDeprecatedPredicates(unittest.TestCase): from pyramid.config.predicates import XHRPredicate self.assertEqual(len(w), 1) +class Test_sort_accept_offers(unittest.TestCase): + def _callFUT(self, offers, order=None): + from pyramid.config.util import sort_accept_offers + return sort_accept_offers(offers, order) + + def test_default_specificities(self): + result = self._callFUT(['*/*', 'text/*', 'text/html', 'text/html;charset=utf8']) + self.assertEqual(result, [ + 'text/html;charset=utf8', 'text/html', 'text/*', '*/*', + ]) + + def test_wildcard_type_order(self): + result = self._callFUT( + ['*/*', 'text/*', 'image/*'], + ['image/*', 'text/*'], + ) + self.assertEqual(result, ['image/*', 'text/*', '*/*']) + + def test_specific_type_order(self): + result = self._callFUT( + ['text/html', 'application/json', 'text/html;charset=utf8', 'text/plain'], + ['application/json', 'text/html'], + ) + self.assertEqual(result, [ + 'application/json', 'text/html;charset=utf8', 'text/html', 'text/plain', + ]) + + def test_params_order(self): + result = self._callFUT( + ['text/html;charset=utf8', 'text/html;charset=latin1', 'text/html;foo=bar'], + ['text/html;charset=latin1', 'text/html;charset=utf8'], + ) + self.assertEqual(result, [ + 'text/html;charset=latin1', 'text/html;charset=utf8', 'text/html;foo=bar', + ]) + + def test_params_inherit_type_prefs(self): + result = self._callFUT( + ['text/html;charset=utf8', 'text/plain;charset=latin1'], + ['text/plain', 'text/html'], + ) + self.assertEqual(result, ['text/plain;charset=latin1', 'text/html;charset=utf8']) + + def test_params_inherit_wildcard_prefs(self): + result = self._callFUT( + ['image/png;progressive=1', 'text/html;charset=utf8'], + ['text/*', 'image/*'], + ) + self.assertEqual(result, ['text/html;charset=utf8', 'image/png;progressive=1']) + + def test_type_overrides_wildcard_prefs(self): + result = self._callFUT( + ['text/html;charset=utf8', 'image/png', 'foo/bar', 'text/bar'], + ['foo/*', 'text/*', 'image/*', 'image/png', 'text/html'], + ) + self.assertEqual(result, [ + 'image/png', 'text/html;charset=utf8', 'foo/bar', 'text/bar', + ]) + class DummyCustomPredicate(object): def __init__(self): self.__text__ = 'custom predicate' diff --git a/pyramid/tests/test_util.py b/pyramid/tests/test_util.py index 907ca7351..a76cd2017 100644 --- a/pyramid/tests/test_util.py +++ b/pyramid/tests/test_util.py @@ -1109,63 +1109,3 @@ class TestSimpleSerializer(unittest.TestCase): def test_dumps(self): inst = self._makeOne() self.assertEqual(inst.dumps('abc'), bytes_('abc')) - - -class Test_sort_accept_offers(unittest.TestCase): - def _callFUT(self, offers, order=None): - from pyramid.util import sort_accept_offers - return sort_accept_offers(offers, order) - - def test_default_specificities(self): - result = self._callFUT(['*/*', 'text/*', 'text/html', 'text/html;charset=utf8']) - self.assertEqual(result, [ - 'text/html;charset=utf8', 'text/html', 'text/*', '*/*', - ]) - - def test_wildcard_type_order(self): - result = self._callFUT( - ['*/*', 'text/*', 'image/*'], - ['image/*', 'text/*'], - ) - self.assertEqual(result, ['image/*', 'text/*', '*/*']) - - def test_specific_type_order(self): - result = self._callFUT( - ['text/html', 'application/json', 'text/html;charset=utf8', 'text/plain'], - ['application/json', 'text/html'], - ) - self.assertEqual(result, [ - 'application/json', 'text/html;charset=utf8', 'text/html', 'text/plain', - ]) - - def test_params_order(self): - result = self._callFUT( - ['text/html;charset=utf8', 'text/html;charset=latin1', 'text/html;foo=bar'], - ['text/html;charset=latin1', 'text/html;charset=utf8'], - ) - self.assertEqual(result, [ - 'text/html;charset=latin1', 'text/html;charset=utf8', 'text/html;foo=bar', - ]) - - def test_params_inherit_type_prefs(self): - result = self._callFUT( - ['text/html;charset=utf8', 'text/plain;charset=latin1'], - ['text/plain', 'text/html'], - ) - self.assertEqual(result, ['text/plain;charset=latin1', 'text/html;charset=utf8']) - - def test_params_inherit_wildcard_prefs(self): - result = self._callFUT( - ['image/png;progressive=1', 'text/html;charset=utf8'], - ['text/*', 'image/*'], - ) - self.assertEqual(result, ['text/html;charset=utf8', 'image/png;progressive=1']) - - def test_type_overrides_wildcard_prefs(self): - result = self._callFUT( - ['text/html;charset=utf8', 'image/png', 'foo/bar', 'text/bar'], - ['foo/*', 'text/*', 'image/*', 'image/png', 'text/html'], - ) - self.assertEqual(result, [ - 'image/png', 'text/html;charset=utf8', 'foo/bar', 'text/bar', - ]) diff --git a/pyramid/util.py b/pyramid/util.py index 708931eea..6655455bf 100644 --- a/pyramid/util.py +++ b/pyramid/util.py @@ -1,5 +1,3 @@ -from collections import defaultdict -from collections import namedtuple from contextlib import contextmanager import functools try: @@ -9,7 +7,6 @@ except ImportError: # pragma: no cover compare_digest = None import inspect import weakref -from webob.acceptparse import Accept from pyramid.exceptions import ( ConfigurationError, @@ -652,77 +649,3 @@ class SimpleSerializer(object): def dumps(self, appstruct): return bytes_(appstruct) - -def sort_accept_offers(offers, order=None): - """ - Sort a list of offers by specificity and preference. - - Supported offers are of the following forms, ordered by specificity - (higher to lower): - - - ``type/subtype;params`` and ``type/subtype`` - - ``type/*`` - - ``*/*`` - - :param offers: A list of offers to be sorted. - :param order: A weighted list of offers where items closer to the start of - the list will be a preferred over items closer to the end. - :return: A list of offers sorted first by specificity (higher to lower) - then by ``order``. - - """ - if order is None: - order = [] - - max_weight = len(offers) - - def find_order_index(value, default=None): - return next((i for i, x in enumerate(order) if x == value), default) - - def offer_sort_key(value): - """ - (category, type_weight, params_weight) - - category: - 1 - foo/bar and foo/bar;params - 2 - foo/* - 3 - */* - - type_weight: - if category 1 & 2: - - index of type/* in order list - - ``max_weight`` if no match is found - - - index of type/subtype in order list - - index of type/* in order list + ``max_weight`` - - ``max_weight * 2`` if no match is found - - params_weight: - - index of specific ``type/subtype;params`` in order list - - ``max_weight`` if not found - - ``max_weight + 1`` if no params at all - - """ - parsed = Accept.parse_offer(value) - - if value == '*/*': - return (3, 0, 0) - - elif parsed.subtype == '*': - type_w = find_order_index(value, max_weight) - return (2, type_w, 0) - - type_w = find_order_index(parsed.type + '/' + parsed.subtype, None) - if type_w is None: - type_w = max_weight + find_order_index( - parsed.type + '/*', max_weight) - - if parsed.params: - param_w = find_order_index(value, max_weight) - - else: - param_w = max_weight + 1 - - return (1, type_w, param_w) - - return sorted(offers, key=offer_sort_key) -- cgit v1.2.3 From bd82c8b17d2f7b0b0c6ecb0e80bc36393a7d807e Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Mon, 1 Oct 2018 23:17:35 -0500 Subject: fix coverage --- pyramid/config/views.py | 15 +++++--- pyramid/tests/test_config/test_views.py | 66 +++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 5 deletions(-) diff --git a/pyramid/config/views.py b/pyramid/config/views.py index 26e86d7d3..3e4dbea0e 100644 --- a/pyramid/config/views.py +++ b/pyramid/config/views.py @@ -141,7 +141,7 @@ class MultiView(object): accepts = set(self.accepts) accepts.add(accept) if accept_order: - accept_order = accept_order.sorted() + accept_order = [v for _, v in accept_order.sorted()] self.accepts = sort_accept_offers(accepts, accept_order) def get_views(self, request): @@ -1235,7 +1235,6 @@ class ViewsConfiguratorMixin(object): 'application/xml', 'text/xml', 'text/plain', - 'application/json', ): self.add_accept_view_order(accept) @@ -1260,7 +1259,13 @@ class ViewsConfiguratorMixin(object): ``application/json`` or ``text/html``. ``weighs_more_than`` and ``weighs_less_than`` control the ordering - of media types. Each value may be a string or a list of strings. + of media types. Each value may be a string or a list of strings. If + all options for ``weighs_more_than`` (or ``weighs_less_than``) cannot + be found, it is an error. + + Earlier calls to ``add_accept_view_order`` are given higher priority + over later calls, assuming similar constraints but standard conflict + resolution mechanisms can be used to override constraints. See :ref:`accept_content_negotiation` for more information. @@ -1324,8 +1329,8 @@ class ViewsConfiguratorMixin(object): self.registry.registerUtility(sorter, IAcceptOrder) sorter.add( value, value, - after=weighs_more_than, - before=weighs_less_than, + before=weighs_more_than, + after=weighs_less_than, ) self.action(discriminator, register, introspectables=(intr,), order=PHASE1_CONFIG) # must be registered before add_view diff --git a/pyramid/tests/test_config/test_views.py b/pyramid/tests/test_config/test_views.py index db15a39fb..730363cf9 100644 --- a/pyramid/tests/test_config/test_views.py +++ b/pyramid/tests/test_config/test_views.py @@ -2389,6 +2389,72 @@ class TestViewsConfigurationMixin(unittest.TestCase): request.exception = Exception() self.assertEqual(derived_view(None, request), 'OK') + def test_add_view_does_not_accept_iterable_accept(self): + from pyramid.exceptions import ConfigurationError + config = self._makeOne(autocommit=True) + self.assertRaises( + ConfigurationError, config.add_view, accept=['image/*', 'text/*'], + ) + + def test_default_accept_view_order(self): + from pyramid.interfaces import IAcceptOrder + config = self._makeOne(autocommit=True) + order = config.registry.getUtility(IAcceptOrder) + result = [v for _, v in order.sorted()] + self.assertEqual(result, [ + 'text/html', + 'application/xhtml+xml', + 'application/xml', + 'text/xml', + 'text/plain', + ]) + + def test_add_accept_view_order_override(self): + from pyramid.interfaces import IAcceptOrder + config = self._makeOne(autocommit=False) + config.add_accept_view_order( + 'text/html', + weighs_more_than='text/xml', + weighs_less_than='application/xml', + ) + config.commit() + order = config.registry.getUtility(IAcceptOrder) + result = [v for _, v in order.sorted()] + self.assertEqual(result, [ + 'application/xhtml+xml', + 'application/xml', + 'text/html', + 'text/xml', + 'text/plain', + ]) + + def test_add_accept_view_order_throws_on_wildcard(self): + from pyramid.exceptions import ConfigurationError + config = self._makeOne(autocommit=True) + self.assertRaises( + ConfigurationError, config.add_accept_view_order, '*/*', + ) + + def test_add_accept_view_order_throws_on_type_mismatch(self): + config = self._makeOne(autocommit=True) + self.assertRaises( + ConfigurationError, config.add_accept_view_order, + 'text/*', weighs_more_than='text/html', + ) + self.assertRaises( + ConfigurationError, config.add_accept_view_order, + 'text/html', weighs_less_than='application/*', + ) + self.assertRaises( + ConfigurationError, config.add_accept_view_order, + 'text/html', weighs_more_than='text/html;charset=utf8', + ) + self.assertRaises( + ConfigurationError, config.add_accept_view_order, + 'text/html;charset=utf8', + weighs_more_than='text/plain;charset=utf8', + ) + class Test_runtime_exc_view(unittest.TestCase): def _makeOne(self, view1, view2): from pyramid.config.views import runtime_exc_view -- cgit v1.2.3 From a3d3a289e7a201c311f9fca6abe40343e29c9696 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Mon, 1 Oct 2018 23:54:32 -0500 Subject: copy integration tests from whiteroses --- pyramid/config/views.py | 1 + pyramid/tests/test_config/test_views.py | 2 + pyramid/tests/test_integration.py | 227 ++++++++++++++++++++++++++++++++ 3 files changed, 230 insertions(+) diff --git a/pyramid/config/views.py b/pyramid/config/views.py index 3e4dbea0e..15ceadcbc 100644 --- a/pyramid/config/views.py +++ b/pyramid/config/views.py @@ -1235,6 +1235,7 @@ class ViewsConfiguratorMixin(object): 'application/xml', 'text/xml', 'text/plain', + 'application/json', ): self.add_accept_view_order(accept) diff --git a/pyramid/tests/test_config/test_views.py b/pyramid/tests/test_config/test_views.py index 730363cf9..abc10d3f5 100644 --- a/pyramid/tests/test_config/test_views.py +++ b/pyramid/tests/test_config/test_views.py @@ -2407,6 +2407,7 @@ class TestViewsConfigurationMixin(unittest.TestCase): 'application/xml', 'text/xml', 'text/plain', + 'application/json', ]) def test_add_accept_view_order_override(self): @@ -2426,6 +2427,7 @@ class TestViewsConfigurationMixin(unittest.TestCase): 'text/html', 'text/xml', 'text/plain', + 'application/json', ]) def test_add_accept_view_order_throws_on_wildcard(self): diff --git a/pyramid/tests/test_integration.py b/pyramid/tests/test_integration.py index c99e89f59..01708eb1b 100644 --- a/pyramid/tests/test_integration.py +++ b/pyramid/tests/test_integration.py @@ -712,7 +712,234 @@ class AcceptContentTypeTest(unittest.TestCase): self.assertEqual(res.content_type, 'application/json') res = self.testapp.get('/hello', headers={'Accept': 'text/*'}, status=200) self.assertEqual(res.content_type, 'text/plain') + res = self.testapp.get('/hello', headers={'Accept': '*/*'}, status=200) + self.assertEqual(res.content_type, 'text/plain') + + def test_no_accept(self): + res = self.testapp.get('/hello', status=200) + self.assertEqual(res.content_type, 'text/plain') + + def test_invalid_accept(self): + res = self.testapp.get('/hello', headers={'Accept': 'foo'}, status=200) + self.assertEqual(res.content_type, 'text/plain') + +class AddViewAcceptArgMediaRangeAllTest(unittest.TestCase): + def setUp(self): + def view(request): + return 'text/plain' + from pyramid.config import Configurator + config = Configurator() + config.add_route('root', '/') + config.add_view( + view, route_name='root', accept='*/*', renderer='string', + ) + app = config.make_wsgi_app() + from webtest import TestApp + self.testapp = TestApp(app) + + def tearDown(self): + import pyramid.config + pyramid.config.global_registries.empty() + + def test_no_header(self): + res = self.testapp.get('/', headers={}, status=200) + self.assertEqual(res.content_type, 'text/plain') + + def test_header_all(self): + res = self.testapp.get('/', headers={'Accept': '*/*'}, status=200) + self.assertEqual(res.content_type, 'text/plain') + + def test_header_all_subtypes_of_type(self): + res = self.testapp.get('/', headers={'Accept': 'text/*'}, status=200) + self.assertEqual(res.content_type, 'text/plain') + + def test_header_specific_media_type(self): + res = self.testapp.get( + '/', headers={'Accept': 'text/plain'}, status=200, + ) + self.assertEqual(res.content_type, 'text/plain') + + def test_header_ruled_out_by_specific_media_type_q0(self): + res = self.testapp.get( + '/', headers={'Accept': 'text/plain;q=0, */*'}, status=200, + ) + self.assertEqual(res.content_type, 'text/plain') + def test_header_ruled_out_by_type_range_q0(self): + res = self.testapp.get( + '/', headers={'Accept': 'text/*;q=0, text/html'}, status=200, + ) + self.assertEqual(res.content_type, 'text/plain') + + def test_header_ruled_out_by_all_range_q0(self): + res = self.testapp.get( + '/', headers={'Accept': '*/*;q=0, text/html'}, status=200, + ) + self.assertEqual(res.content_type, 'text/plain') + +class AddViewAcceptArgMediaRangeAllSubtypesOfTypeTest(unittest.TestCase): + def setUp(self): + def view(request): + return 'text/plain' + from pyramid.config import Configurator + config = Configurator() + config.add_route('root', '/') + config.add_view( + view, route_name='root', accept='text/*', renderer='string', + ) + app = config.make_wsgi_app() + from webtest import TestApp + self.testapp = TestApp(app) + + def tearDown(self): + import pyramid.config + pyramid.config.global_registries.empty() + + def test_no_header(self): + res = self.testapp.get('/', headers={}, status=200) + self.assertEqual(res.content_type, 'text/plain') + + def test_header_all(self): + res = self.testapp.get('/', headers={'Accept': '*/*'}, status=200) + self.assertEqual(res.content_type, 'text/plain') + + def test_header_all_subtypes_of_type(self): + res = self.testapp.get('/', headers={'Accept': 'text/*'}, status=200) + self.assertEqual(res.content_type, 'text/plain') + + def test_header_specific_media_type(self): + res = self.testapp.get( + '/', headers={'Accept': 'text/plain'}, status=200, + ) + self.assertEqual(res.content_type, 'text/plain') + + def test_header_none_acceptable(self): + self.testapp.get('/', headers={'Accept': 'application/*'}, status=404) + + def test_header_ruled_out_by_specific_media_type_q0(self): + res = self.testapp.get( + '/', headers={'Accept': 'text/plain;q=0, */*'}, status=200, + ) + self.assertEqual(res.content_type, 'text/plain') + + def test_header_ruled_out_by_type_range_q0(self): + res = self.testapp.get( + '/', headers={'Accept': 'text/*;q=0, text/html'}, status=200, + ) + self.assertEqual(res.content_type, 'text/plain') + + def test_header_ruled_out_by_all_range_q0(self): + res = self.testapp.get( + '/', headers={'Accept': '*/*;q=0, text/html'}, status=200, + ) + self.assertEqual(res.content_type, 'text/plain') + +class AddRouteAcceptArgMediaRangeAllTest(unittest.TestCase): + def setUp(self): + def view(request): + return 'text/plain' + from pyramid.config import Configurator + config = Configurator() + config.add_route('root', '/', accept='*/*') + config.add_view(view, route_name='root', renderer='string') + app = config.make_wsgi_app() + from webtest import TestApp + self.testapp = TestApp(app) + + def tearDown(self): + import pyramid.config + pyramid.config.global_registries.empty() + + def test_no_header(self): + res = self.testapp.get('/', headers={}, status=200) + self.assertEqual(res.content_type, 'text/plain') + + def test_header_all(self): + res = self.testapp.get('/', headers={'Accept': '*/*'}, status=200) + self.assertEqual(res.content_type, 'text/plain') + + def test_header_all_subtypes_of_type(self): + res = self.testapp.get('/', headers={'Accept': 'text/*'}, status=200) + self.assertEqual(res.content_type, 'text/plain') + + def test_header_specific_media_type(self): + res = self.testapp.get( + '/', headers={'Accept': 'text/plain'}, status=200, + ) + self.assertEqual(res.content_type, 'text/plain') + + def test_header_ruled_out_by_specific_media_type_q0(self): + res = self.testapp.get( + '/', headers={'Accept': 'text/plain;q=0, */*'}, status=200, + ) + self.assertEqual(res.content_type, 'text/plain') + + def test_header_ruled_out_by_type_range_q0(self): + res = self.testapp.get( + '/', headers={'Accept': 'text/*;q=0, text/html'}, status=200, + ) + self.assertEqual(res.content_type, 'text/plain') + + def test_header_ruled_out_by_all_range_q0(self): + res = self.testapp.get( + '/', headers={'Accept': '*/*;q=0, text/html'}, status=200, + ) + self.assertEqual(res.content_type, 'text/plain') + +class AddRouteAcceptArgMediaRangeAllSubtypesOfTypeTest(unittest.TestCase): + def setUp(self): + def view(request): + return 'text/plain' + from pyramid.config import Configurator + config = Configurator() + config.add_route('root', '/', accept='text/*') + config.add_view(view, route_name='root', renderer='string') + app = config.make_wsgi_app() + from webtest import TestApp + self.testapp = TestApp(app) + + def tearDown(self): + import pyramid.config + pyramid.config.global_registries.empty() + + def test_no_header(self): + res = self.testapp.get('/', headers={}, status=200) + self.assertEqual(res.content_type, 'text/plain') + + def test_header_all(self): + res = self.testapp.get('/', headers={'Accept': '*/*'}, status=200) + self.assertEqual(res.content_type, 'text/plain') + + def test_header_all_subtypes_of_type(self): + res = self.testapp.get('/', headers={'Accept': 'text/*'}, status=200) + self.assertEqual(res.content_type, 'text/plain') + + def test_header_specific_media_type(self): + res = self.testapp.get( + '/', headers={'Accept': 'text/plain'}, status=200, + ) + self.assertEqual(res.content_type, 'text/plain') + + def test_header_none_acceptable(self): + self.testapp.get('/', headers={'Accept': 'application/*'}, status=404) + + def test_header_ruled_out_by_specific_media_type_q0(self): + res = self.testapp.get( + '/', headers={'Accept': 'text/plain;q=0, */*'}, status=200, + ) + self.assertEqual(res.content_type, 'text/plain') + + def test_header_ruled_out_by_type_range_q0(self): + res = self.testapp.get( + '/', headers={'Accept': 'text/*;q=0, text/html'}, status=200, + ) + self.assertEqual(res.content_type, 'text/plain') + + def test_header_ruled_out_by_all_range_q0(self): + res = self.testapp.get( + '/', headers={'Accept': '*/*;q=0, text/html'}, status=200, + ) + self.assertEqual(res.content_type, 'text/plain') class DummyContext(object): pass -- cgit v1.2.3 From ae79dfab339309f150e63be92c8ec0c58e13043e Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Tue, 2 Oct 2018 02:24:04 -0500 Subject: fix lints --- docs/narr/viewconfig.rst | 2 +- setup.cfg | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/narr/viewconfig.rst b/docs/narr/viewconfig.rst index 7e6617b78..c44109661 100644 --- a/docs/narr/viewconfig.rst +++ b/docs/narr/viewconfig.rst @@ -1090,7 +1090,7 @@ In this case, the ``application/json`` view should always be selected in cases w .. _default_accept_ordering: Default Accept Ordering -+++++++++++++++++++++++ +~~~~~~~~~~~~~~~~~~~~~~~ :app:`Pyramid` will always sort multiple views with the same ``(name, context, route_name)`` first by the specificity of the ``accept`` offer. This means that ``text/plain`` will always be offered before ``text/*``. diff --git a/setup.cfg b/setup.cfg index 3bf28ee15..cb74bd24c 100644 --- a/setup.cfg +++ b/setup.cfg @@ -63,7 +63,9 @@ ignore = # W293: blank line contains whitespace W293, # W391: blank line at end of file - W391 + W391, + # W503: line break before binary operator + W503 exclude = pyramid/tests/,pyramid/compat.py,pyramid/resource.py show-source = True -- cgit v1.2.3 From 684a2cb4dc39b701e357bd0cb5c683a726cfb5f8 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Tue, 2 Oct 2018 21:13:33 -0500 Subject: exercise some accept ordering --- pyramid/tests/test_integration.py | 104 +++++++++++++++++++++++++------------- 1 file changed, 68 insertions(+), 36 deletions(-) diff --git a/pyramid/tests/test_integration.py b/pyramid/tests/test_integration.py index 01708eb1b..1bf2abb4f 100644 --- a/pyramid/tests/test_integration.py +++ b/pyramid/tests/test_integration.py @@ -16,6 +16,7 @@ from pyramid.compat import ( ) from zope.interface import Interface +from webtest import TestApp # 5 years from now (more or less) fiveyrsfuture = datetime.datetime.utcnow() + datetime.timedelta(5*365) @@ -65,7 +66,6 @@ class IntegrationBase(object): package=self.package) config.include(self.package) app = config.make_wsgi_app() - from webtest import TestApp self.testapp = TestApp(app) self.config = config @@ -482,7 +482,6 @@ class TestConflictApp(unittest.TestCase): config = self._makeConfig() config.include(self.package) app = config.make_wsgi_app() - from webtest import TestApp self.testapp = TestApp(app) res = self.testapp.get('/') self.assertTrue(b'a view' in res.body) @@ -497,7 +496,6 @@ class TestConflictApp(unittest.TestCase): return Response('this view') config.add_view(thisview) app = config.make_wsgi_app() - from webtest import TestApp self.testapp = TestApp(app) res = self.testapp.get('/') self.assertTrue(b'this view' in res.body) @@ -510,7 +508,6 @@ class TestConflictApp(unittest.TestCase): return Response('this view') config.add_view(thisview, route_name='aroute') app = config.make_wsgi_app() - from webtest import TestApp self.testapp = TestApp(app) res = self.testapp.get('/route') self.assertTrue(b'this view' in res.body) @@ -519,7 +516,6 @@ class TestConflictApp(unittest.TestCase): config = self._makeConfig() config.include(self.package) app = config.make_wsgi_app() - from webtest import TestApp self.testapp = TestApp(app) res = self.testapp.get('/protected', status=403) self.assertTrue(b'403 Forbidden' in res.body) @@ -531,7 +527,6 @@ class TestConflictApp(unittest.TestCase): config.set_authorization_policy(DummySecurityPolicy('fred')) config.set_authentication_policy(DummySecurityPolicy(permissive=True)) app = config.make_wsgi_app() - from webtest import TestApp self.testapp = TestApp(app) res = self.testapp.get('/protected', status=200) self.assertTrue('protected view' in res) @@ -543,7 +538,6 @@ class ImperativeIncludeConfigurationTest(unittest.TestCase): from pyramid.tests.pkgs.includeapp1.root import configure configure(config) app = config.make_wsgi_app() - from webtest import TestApp self.testapp = TestApp(app) self.config = config @@ -567,7 +561,6 @@ class SelfScanAppTest(unittest.TestCase): from pyramid.tests.test_config.pkgs.selfscan import main config = main() app = config.make_wsgi_app() - from webtest import TestApp self.testapp = TestApp(app) self.config = config @@ -587,7 +580,6 @@ class WSGIApp2AppTest(unittest.TestCase): from pyramid.tests.pkgs.wsgiapp2app import main config = main() app = config.make_wsgi_app() - from webtest import TestApp self.testapp = TestApp(app) self.config = config @@ -603,7 +595,6 @@ class SubrequestAppTest(unittest.TestCase): from pyramid.tests.pkgs.subrequestapp import main config = main() app = config.make_wsgi_app() - from webtest import TestApp self.testapp = TestApp(app) self.config = config @@ -635,7 +626,6 @@ class RendererScanAppTest(IntegrationBase, unittest.TestCase): def test_rescan(self): self.config.scan('pyramid.tests.pkgs.rendererscanapp') app = self.config.make_wsgi_app() - from webtest import TestApp testapp = TestApp(app) res = testapp.get('/one', status=200) self.assertTrue(b'One!' in res.body) @@ -649,7 +639,6 @@ class UnicodeInURLTest(unittest.TestCase): return config def _makeTestApp(self, config): - from webtest import TestApp app = config.make_wsgi_app() return TestApp(app) @@ -685,43 +674,90 @@ class UnicodeInURLTest(unittest.TestCase): class AcceptContentTypeTest(unittest.TestCase): - def setUp(self): + def _makeConfig(self): def hello_view(request): return {'message': 'Hello!'} from pyramid.config import Configurator config = Configurator() config.add_route('hello', '/hello') - config.add_view(hello_view, route_name='hello', accept='text/plain', renderer='string') - config.add_view(hello_view, route_name='hello', accept='application/json', renderer='json') + config.add_view(hello_view, route_name='hello', + accept='text/plain', renderer='string') + config.add_view(hello_view, route_name='hello', + accept='application/json', renderer='json') + def hello_fallback_view(request): + request.response.content_type = 'text/x-fallback' + return 'hello fallback' + config.add_view(hello_fallback_view, route_name='hello', + renderer='string') + return config + + def _makeTestApp(self, config): app = config.make_wsgi_app() - from webtest import TestApp - self.testapp = TestApp(app) + return TestApp(app) def tearDown(self): import pyramid.config - pyramid.config.global_registries.empty() + pyramid.config.global_registries.empty() - def test_ordering(self): - res = self.testapp.get('/hello', headers={'Accept': 'application/json; q=1.0, text/plain; q=0.9'}, status=200) + def test_client_side_ordering(self): + config = self._makeConfig() + app = self._makeTestApp(config) + res = app.get('/hello', headers={ + 'Accept': 'application/json; q=1.0, text/plain; q=0.9', + }, status=200) self.assertEqual(res.content_type, 'application/json') - res = self.testapp.get('/hello', headers={'Accept': 'text/plain; q=0.9, application/json; q=1.0'}, status=200) + res = app.get('/hello', headers={ + 'Accept': 'text/plain; q=0.9, application/json; q=1.0', + }, status=200) self.assertEqual(res.content_type, 'application/json') - - def test_wildcards(self): - res = self.testapp.get('/hello', headers={'Accept': 'application/*'}, status=200) + res = app.get('/hello', headers={'Accept': 'application/*'}, status=200) self.assertEqual(res.content_type, 'application/json') - res = self.testapp.get('/hello', headers={'Accept': 'text/*'}, status=200) - self.assertEqual(res.content_type, 'text/plain') - res = self.testapp.get('/hello', headers={'Accept': '*/*'}, status=200) + res = app.get('/hello', headers={'Accept': 'text/*'}, status=200) self.assertEqual(res.content_type, 'text/plain') + res = app.get('/hello', headers={'Accept': 'something/else'}, status=200) + self.assertEqual(res.content_type, 'text/x-fallback') - def test_no_accept(self): - res = self.testapp.get('/hello', status=200) + def test_default_server_side_ordering(self): + config = self._makeConfig() + app = self._makeTestApp(config) + res = app.get('/hello', headers={ + 'Accept': 'application/json, text/plain', + }, status=200) self.assertEqual(res.content_type, 'text/plain') - - def test_invalid_accept(self): - res = self.testapp.get('/hello', headers={'Accept': 'foo'}, status=200) + res = app.get('/hello', headers={ + 'Accept': 'text/plain, application/json', + }, status=200) + self.assertEqual(res.content_type, 'text/plain') + res = app.get('/hello', headers={'Accept': '*/*'}, status=200) self.assertEqual(res.content_type, 'text/plain') + res = app.get('/hello', status=200) + self.assertEqual(res.content_type, 'text/plain') + res = app.get('/hello', headers={'Accept': 'invalid'}, status=200) + self.assertEqual(res.content_type, 'text/plain') + res = app.get('/hello', headers={'Accept': 'something/else'}, status=200) + self.assertEqual(res.content_type, 'text/x-fallback') + + def test_custom_server_side_ordering(self): + config = self._makeConfig() + config.add_accept_view_order( + 'application/json', weighs_more_than='text/plain') + app = self._makeTestApp(config) + res = app.get('/hello', headers={ + 'Accept': 'application/json, text/plain', + }, status=200) + self.assertEqual(res.content_type, 'application/json') + res = app.get('/hello', headers={ + 'Accept': 'text/plain, application/json', + }, status=200) + self.assertEqual(res.content_type, 'application/json') + res = app.get('/hello', headers={'Accept': '*/*'}, status=200) + self.assertEqual(res.content_type, 'application/json') + res = app.get('/hello', status=200) + self.assertEqual(res.content_type, 'application/json') + res = app.get('/hello', headers={'Accept': 'invalid'}, status=200) + self.assertEqual(res.content_type, 'application/json') + res = app.get('/hello', headers={'Accept': 'something/else'}, status=200) + self.assertEqual(res.content_type, 'text/x-fallback') class AddViewAcceptArgMediaRangeAllTest(unittest.TestCase): def setUp(self): @@ -734,7 +770,6 @@ class AddViewAcceptArgMediaRangeAllTest(unittest.TestCase): view, route_name='root', accept='*/*', renderer='string', ) app = config.make_wsgi_app() - from webtest import TestApp self.testapp = TestApp(app) def tearDown(self): @@ -788,7 +823,6 @@ class AddViewAcceptArgMediaRangeAllSubtypesOfTypeTest(unittest.TestCase): view, route_name='root', accept='text/*', renderer='string', ) app = config.make_wsgi_app() - from webtest import TestApp self.testapp = TestApp(app) def tearDown(self): @@ -843,7 +877,6 @@ class AddRouteAcceptArgMediaRangeAllTest(unittest.TestCase): config.add_route('root', '/', accept='*/*') config.add_view(view, route_name='root', renderer='string') app = config.make_wsgi_app() - from webtest import TestApp self.testapp = TestApp(app) def tearDown(self): @@ -895,7 +928,6 @@ class AddRouteAcceptArgMediaRangeAllSubtypesOfTypeTest(unittest.TestCase): config.add_route('root', '/', accept='text/*') config.add_view(view, route_name='root', renderer='string') app = config.make_wsgi_app() - from webtest import TestApp self.testapp = TestApp(app) def tearDown(self): -- cgit v1.2.3 From 4a9f4f43684c3a754f43935b97013057340c305d Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Wed, 10 Oct 2018 00:04:43 -0500 Subject: deprecate range support --- CHANGES.rst | 11 ++ docs/narr/viewconfig.rst | 14 +- pyramid/config/routes.py | 38 +++-- pyramid/config/util.py | 47 +++--- pyramid/config/views.py | 61 ++++---- pyramid/predicates.py | 16 ++- pyramid/tests/test_config/test_routes.py | 18 ++- pyramid/tests/test_config/test_util.py | 27 +--- pyramid/tests/test_config/test_views.py | 35 ++++- pyramid/tests/test_integration.py | 236 ++++--------------------------- 10 files changed, 183 insertions(+), 320 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 3934c5aed..e1f782e60 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -113,6 +113,17 @@ Deprecations implementation if you're still using these features. See https://github.com/Pylons/pyramid/pull/3353 +- Media ranges are deprecated in the ``accept`` argument of + ``pyramid.config.Configurator.add_route``. Use a list of explicit + media types to ``add_route`` to support multiple types. + +- Media ranges are deprecated in the ``accept`` argument of + ``pyramid.config.Configurator.add_view``. There is no replacement for + ranges to ``add_view``, but after much discussion the workflow is + fundamentally ambiguous in the face of various client-supplied values for + the ``Accept`` header. + See https://github.com/Pylons/pyramid/pull/3326 + Backward Incompatibilities -------------------------- diff --git a/docs/narr/viewconfig.rst b/docs/narr/viewconfig.rst index c44109661..238599528 100644 --- a/docs/narr/viewconfig.rst +++ b/docs/narr/viewconfig.rst @@ -1093,23 +1093,17 @@ Default Accept Ordering ~~~~~~~~~~~~~~~~~~~~~~~ :app:`Pyramid` will always sort multiple views with the same ``(name, context, route_name)`` first by the specificity of the ``accept`` offer. -This means that ``text/plain`` will always be offered before ``text/*``. -Similarly ``text/plain;charset=utf8`` will always be offered before ``text/plain``. -The following order is always preserved between the following offers (more preferred to less preferred): +For any set of media type offers with the same ``type/subtype``, the offers with params will weigh more than the bare ``type/subtype`` offer. +This means that ``text/plain;charset=utf8`` will always be offered before ``text/plain``. -- ``type/subtype;params`` -- ``type/subtype`` -- ``type/*`` -- ``*/*`` - -Within each of these levels of specificity, the ordering is ambiguous and may be controlled using :meth:`pyramid.config.Configurator.add_accept_view_order`. For example, to sort ``text/plain`` higher than ``text/html`` and to prefer a ``charset=utf8`` versus a ``charset=latin-1`` within the ``text/plain`` media type: +By default, within a given ``type/subtype``, the order of offers is ambiguous. For example, ``text/plain;charset=utf8`` versus ``text/plain;charset=latin1`` are sorted in an unspecified way. Similarly, between media types the order is also unspecified other than the defaults described below. For example, ``image/jpeg`` versus ``image/png`` versus ``application/pdf``. In these cases, the ordering may be controlled using :meth:`pyramid.config.Configurator.add_accept_view_order`. For example, to sort ``text/plain`` higher than ``text/html`` and to prefer a ``charset=utf8`` versus a ``charset=latin-1`` within the ``text/plain`` media type: .. code-block:: python config.add_accept_view_order('text/plain', weighs_more_than='text/html') config.add_accept_view_order('text/plain;charset=utf8', weighs_more_than='text/plain;charset=latin-1') -It is an error to try and sort accept headers across levels of specificity. You can only sort a ``type/subtype`` against another ``type/subtype``, not against a ``type/*``. That ordering is a hard requirement. +It is an error to try and sort accept headers across levels of specificity. You can only sort a ``type/subtype`` against another ``type/subtype``, not against a ``type/subtype;params``. That ordering is a hard requirement. By default, :app:`Pyramid` defines a very simple priority ordering for views that prefers human-readable responses over JSON: diff --git a/pyramid/config/routes.py b/pyramid/config/routes.py index 01917537d..4d1f830c8 100644 --- a/pyramid/config/routes.py +++ b/pyramid/config/routes.py @@ -22,6 +22,7 @@ import pyramid.predicates from pyramid.config.util import ( action_method, + normalize_accept_offer, predvalseq, ) @@ -228,16 +229,24 @@ class RoutesConfiguratorMixin(object): A :term:`media type` that will be matched against the ``Accept`` HTTP request header. This value may be a specific media type such - as ``text/html``, or a range like ``text/*``, or a list of the same. - If the media type is acceptable by the ``Accept`` header of the - request, or if the ``Accept`` header isn't set at all in the - request, this predicate will match. If this does not match the - ``Accept`` header of the request, route matching continues. + as ``text/html``, or a list of the same. If the media type is + acceptable by the ``Accept`` header of the request, or if the + ``Accept`` header isn't set at all in the request, this predicate + will match. If this does not match the ``Accept`` header of the + request, route matching continues. If ``accept`` is not specified, the ``HTTP_ACCEPT`` HTTP header is not taken into consideration when deciding whether or not to select the route. + .. versionchanged:: 1.10 + + Specifying a media range is deprecated due to changes in WebOb + and ambiguities that occur when trying to match ranges against + ranges in the ``Accept`` header. Support will be removed in + :app:`Pyramid` 2.0. Use a list of specific media types to match + more than one type. + effective_principals If specified, this value should be a :term:`principal` identifier or @@ -297,9 +306,22 @@ class RoutesConfiguratorMixin(object): if accept is not None: if not is_nonstr_iter(accept): - accept = [accept] - - accept = [accept_option.lower() for accept_option in accept] + if '*' in accept: + warnings.warn( + ('Passing a media range to the "accept" argument of ' + 'Configurator.add_route is deprecated as of Pyramid ' + '1.10. Use a list of explicit media types.'), + DeprecationWarning, + stacklevel=3, + ) + # XXX switch this to verify=True when range support is dropped + accept = [normalize_accept_offer(accept, verify=False)] + + else: + accept = [ + normalize_accept_offer(accept_option) + for accept_option in accept + ] # these are route predicates; if they do not match, the next route # in the routelist will be tried diff --git a/pyramid/config/util.py b/pyramid/config/util.py index 7024fa862..8ebc8e45c 100644 --- a/pyramid/config/util.py +++ b/pyramid/config/util.py @@ -221,16 +221,18 @@ class PredicateList(object): return order, preds, phash.hexdigest() +def normalize_accept_offer(offer, verify=True): + if verify: + Accept.parse_offer(offer) + return offer.lower() + + def sort_accept_offers(offers, order=None): """ - Sort a list of offers by specificity and preference. + Sort a list of offers by preference. - Supported offers are of the following forms, ordered by specificity - (higher to lower): - - - ``type/subtype;params`` and ``type/subtype`` - - ``type/*`` - - ``*/*`` + For a given ``type/subtype`` category of offers, this algorithm will + always sort offers with params higher than the bare offer. :param offers: A list of offers to be sorted. :param order: A weighted list of offers where items closer to the start of @@ -249,20 +251,10 @@ def sort_accept_offers(offers, order=None): def offer_sort_key(value): """ - (category, type_weight, params_weight) - - category: - 1 - foo/bar and foo/bar;params - 2 - foo/* - 3 - */* + (type_weight, params_weight) type_weight: - if category 1 & 2: - - index of type/* in order list - - ``max_weight`` if no match is found - - - index of type/subtype in order list - - index of type/* in order list + ``max_weight`` + - index of specific ``type/subtype`` in order list - ``max_weight * 2`` if no match is found params_weight: @@ -273,17 +265,10 @@ def sort_accept_offers(offers, order=None): """ parsed = Accept.parse_offer(value) - if value == '*/*': - return (3, 0, 0) - - elif parsed.subtype == '*': - type_w = find_order_index(value, max_weight) - return (2, type_w, 0) - - type_w = find_order_index(parsed.type + '/' + parsed.subtype, None) - if type_w is None: - type_w = max_weight + find_order_index( - parsed.type + '/*', max_weight) + type_w = find_order_index( + parsed.type + '/' + parsed.subtype, + max_weight, + ) if parsed.params: param_w = find_order_index(value, max_weight) @@ -291,6 +276,6 @@ def sort_accept_offers(offers, order=None): else: param_w = max_weight + 1 - return (1, type_w, param_w) + return (type_w, param_w) return sorted(offers, key=offer_sort_key) diff --git a/pyramid/config/views.py b/pyramid/config/views.py index 15ceadcbc..ca299c4b5 100644 --- a/pyramid/config/views.py +++ b/pyramid/config/views.py @@ -88,6 +88,7 @@ from pyramid.config.util import ( action_method, DEFAULT_PHASH, MAX_ORDER, + normalize_accept_offer, predvalseq, sort_accept_offers, ) @@ -125,7 +126,7 @@ class MultiView(object): self.views[i] = (order, view, phash) return - if accept is None: + if accept is None or '*' in accept: self.views.append((order, view, phash)) self.views.sort(key=operator.itemgetter(0)) else: @@ -676,8 +677,8 @@ class ViewsConfiguratorMixin(object): accept A :term:`media type` that will be matched against the ``Accept`` - HTTP request header. This value may be a specific media type such - as ``text/html``, or a range like ``text/*``. If the media type is + HTTP request header. This value must be a specific media type such + as ``text/html`` or ``text/html;level=1``. If the media type is acceptable by the ``Accept`` header of the request, or if the ``Accept`` header isn't set at all in the request, this predicate will match. If this does not match the ``Accept`` header of the @@ -689,6 +690,12 @@ class ViewsConfiguratorMixin(object): See :ref:`accept_content_negotiation` for more information. + .. versionchanged:: 1.10 + + Specifying a media range is deprecated and will be removed in + :app:`Pyramid` 2.0. Use explicit media types to avoid any + ambiguities in content negotiation. + path_info This value represents a regular expression pattern that will @@ -821,7 +828,17 @@ class ViewsConfiguratorMixin(object): raise ConfigurationError( 'A list is not supported in the "accept" view predicate.', ) - accept = accept.lower() + if '*' in accept: + warnings.warn( + ('Passing a media range to the "accept" argument of ' + 'Configurator.add_view is deprecated as of Pyramid 1.10. ' + 'Use explicit media types to avoid ambiguities in ' + 'content negotiation that may impact your users.'), + DeprecationWarning, + stacklevel=4, + ) + # XXX when media ranges are gone, switch verify=True + accept = normalize_accept_offer(accept, verify=False) view = self.maybe_dotted(view) context = self.maybe_dotted(context) @@ -1273,46 +1290,38 @@ class ViewsConfiguratorMixin(object): .. versionadded:: 1.10 """ - if value == '*/*': - raise ConfigurationError( - 'cannot specify an ordering for an offer of */*') - - def normalize_type(type): - return type.lower() - def check_type(than): than_type, than_subtype, than_params = Accept.parse_offer(than) - if ( - # text/* vs text/plain - (offer_subtype == '*') ^ (than_subtype == '*') - # text/plain vs text/html;charset=utf8 - or (bool(offer_params) ^ bool(than_params)) - ): + # text/plain vs text/html;charset=utf8 + if bool(offer_params) ^ bool(than_params): raise ConfigurationError( - 'cannot compare across media range specificity levels') + 'cannot compare a media type with params to one without ' + 'params') # text/plain;charset=utf8 vs text/html;charset=utf8 if offer_params and ( offer_subtype != than_subtype or offer_type != than_type ): raise ConfigurationError( - 'cannot compare params across media types') + 'cannot compare params across different media types') + + def normalize_types(thans): + thans = [normalize_accept_offer(o, verify=False) for o in thans] + for o in thans: + check_type(o) + return thans - value = normalize_type(value) + value = normalize_accept_offer(value, verify=False) offer_type, offer_subtype, offer_params = Accept.parse_offer(value) if weighs_more_than: if not is_nonstr_iter(weighs_more_than): weighs_more_than = [weighs_more_than] - weighs_more_than = [normalize_type(w) for w in weighs_more_than] - for than in weighs_more_than: - check_type(than) + weighs_more_than = normalize_types(weighs_more_than) if weighs_less_than: if not is_nonstr_iter(weighs_less_than): weighs_less_than = [weighs_less_than] - weighs_less_than = [normalize_type(w) for w in weighs_less_than] - for than in weighs_less_than: - check_type(than) + weighs_less_than = normalize_types(weighs_less_than) discriminator = ('accept view order', value) intr = self.introspectable( diff --git a/pyramid/predicates.py b/pyramid/predicates.py index 5bd98fdf2..97edae8a0 100644 --- a/pyramid/predicates.py +++ b/pyramid/predicates.py @@ -130,10 +130,16 @@ class HeaderPredicate(object): return self.val.match(val) is not None class AcceptPredicate(object): - def __init__(self, val, config): - if not is_nonstr_iter(val): - val = (val,) - self.values = val + _is_using_deprecated_ranges = False + + def __init__(self, values, config): + if not is_nonstr_iter(values): + values = (values,) + # deprecated media ranges were only supported in versions of the + # predicate that didn't support lists, so check it here + if len(values) == 1 and '*' in values[0]: + self._is_using_deprecated_ranges = True + self.values = values def text(self): return 'accept = %s' % (', '.join(self.values),) @@ -141,6 +147,8 @@ class AcceptPredicate(object): phash = text def __call__(self, context, request): + if self._is_using_deprecated_ranges: + return self.values[0] in request.accept return bool(request.accept.acceptable_offers(self.values)) class ContainmentPredicate(object): diff --git a/pyramid/tests/test_config/test_routes.py b/pyramid/tests/test_config/test_routes.py index afae4db33..9f4ce9bc6 100644 --- a/pyramid/tests/test_config/test_routes.py +++ b/pyramid/tests/test_config/test_routes.py @@ -203,6 +203,18 @@ class RoutesConfiguratorMixinTests(unittest.TestCase): request.accept = DummyAccept('text/html') self.assertEqual(predicate(None, request), False) + def test_add_route_with_wildcard_accept(self): + config = self._makeOne(autocommit=True) + config.add_route('name', 'path', accept='text/*') + route = self._assertRoute(config, 'name', 'path', 1) + predicate = route.predicates[0] + request = self._makeRequest(config) + request.accept = DummyAccept('text/xml', contains=True) + self.assertEqual(predicate(None, request), True) + request = self._makeRequest(config) + request.accept = DummyAccept('application/json', contains=False) + self.assertEqual(predicate(None, request), False) + def test_add_route_no_pattern_with_path(self): config = self._makeOne(autocommit=True) config.add_route('name', path='path') @@ -270,8 +282,9 @@ class DummyRequest: self.cookies = {} class DummyAccept(object): - def __init__(self, *matches): + def __init__(self, *matches, **kw): self.matches = list(matches) + self.contains = kw.pop('contains', False) def acceptable_offers(self, offers): results = [] @@ -279,3 +292,6 @@ class DummyAccept(object): if match in offers: results.append((match, 1.0)) return results + + def __contains__(self, value): + return self.contains diff --git a/pyramid/tests/test_config/test_util.py b/pyramid/tests/test_config/test_util.py index bbda615c9..540f3d14c 100644 --- a/pyramid/tests/test_config/test_util.py +++ b/pyramid/tests/test_config/test_util.py @@ -437,18 +437,11 @@ class Test_sort_accept_offers(unittest.TestCase): return sort_accept_offers(offers, order) def test_default_specificities(self): - result = self._callFUT(['*/*', 'text/*', 'text/html', 'text/html;charset=utf8']) + result = self._callFUT(['text/html', 'text/html;charset=utf8']) self.assertEqual(result, [ - 'text/html;charset=utf8', 'text/html', 'text/*', '*/*', + 'text/html;charset=utf8', 'text/html', ]) - def test_wildcard_type_order(self): - result = self._callFUT( - ['*/*', 'text/*', 'image/*'], - ['image/*', 'text/*'], - ) - self.assertEqual(result, ['image/*', 'text/*', '*/*']) - def test_specific_type_order(self): result = self._callFUT( ['text/html', 'application/json', 'text/html;charset=utf8', 'text/plain'], @@ -474,22 +467,6 @@ class Test_sort_accept_offers(unittest.TestCase): ) self.assertEqual(result, ['text/plain;charset=latin1', 'text/html;charset=utf8']) - def test_params_inherit_wildcard_prefs(self): - result = self._callFUT( - ['image/png;progressive=1', 'text/html;charset=utf8'], - ['text/*', 'image/*'], - ) - self.assertEqual(result, ['text/html;charset=utf8', 'image/png;progressive=1']) - - def test_type_overrides_wildcard_prefs(self): - result = self._callFUT( - ['text/html;charset=utf8', 'image/png', 'foo/bar', 'text/bar'], - ['foo/*', 'text/*', 'image/*', 'image/png', 'text/html'], - ) - self.assertEqual(result, [ - 'image/png', 'text/html;charset=utf8', 'foo/bar', 'text/bar', - ]) - class DummyCustomPredicate(object): def __init__(self): self.__text__ = 'custom predicate' diff --git a/pyramid/tests/test_config/test_views.py b/pyramid/tests/test_config/test_views.py index abc10d3f5..6565a35d5 100644 --- a/pyramid/tests/test_config/test_views.py +++ b/pyramid/tests/test_config/test_views.py @@ -1477,6 +1477,25 @@ class TestViewsConfigurationMixin(unittest.TestCase): request.accept = DummyAccept('text/html') self._assertNotFound(wrapper, None, request) + def test_add_view_with_range_accept_match(self): + from pyramid.renderers import null_renderer + view = lambda *arg: 'OK' + config = self._makeOne(autocommit=True) + config.add_view(view=view, accept='text/*', renderer=null_renderer) + wrapper = self._getViewCallable(config) + request = self._makeRequest(config) + request.accept = DummyAccept('text/html', contains=True) + self.assertEqual(wrapper(None, request), 'OK') + + def test_add_view_with_range_accept_nomatch(self): + view = lambda *arg: 'OK' + config = self._makeOne(autocommit=True) + config.add_view(view=view, accept='text/*') + wrapper = self._getViewCallable(config) + request = self._makeRequest(config) + request.accept = DummyAccept('application/json', contains=False) + self._assertNotFound(wrapper, None, request) + def test_add_view_with_containment_true(self): from pyramid.renderers import null_renderer from zope.interface import directlyProvides @@ -2431,20 +2450,19 @@ class TestViewsConfigurationMixin(unittest.TestCase): ]) def test_add_accept_view_order_throws_on_wildcard(self): - from pyramid.exceptions import ConfigurationError config = self._makeOne(autocommit=True) self.assertRaises( - ConfigurationError, config.add_accept_view_order, '*/*', + ValueError, config.add_accept_view_order, '*/*', ) def test_add_accept_view_order_throws_on_type_mismatch(self): config = self._makeOne(autocommit=True) self.assertRaises( - ConfigurationError, config.add_accept_view_order, + ValueError, config.add_accept_view_order, 'text/*', weighs_more_than='text/html', ) self.assertRaises( - ConfigurationError, config.add_accept_view_order, + ValueError, config.add_accept_view_order, 'text/html', weighs_less_than='application/*', ) self.assertRaises( @@ -2577,7 +2595,8 @@ class TestMultiView(unittest.TestCase): self.assertEqual(set(mv.accepts), set(['text/xml', 'text/html'])) self.assertEqual(mv.views, [(99, 'view2', None), (100, 'view', None)]) mv.add('view6', 98, accept='text/*') - self.assertEqual(mv.media_views['text/*'], [(98, 'view6', None)]) + self.assertEqual(mv.views, [ + (98, 'view6', None), (99, 'view2', None), (100, 'view', None)]) def test_add_with_phash(self): mv = self._makeOne() @@ -3503,8 +3522,9 @@ class DummyContext: pass class DummyAccept(object): - def __init__(self, *matches): + def __init__(self, *matches, **kw): self.matches = list(matches) + self.contains = kw.pop('contains', False) def acceptable_offers(self, offers): results = [] @@ -3513,6 +3533,9 @@ class DummyAccept(object): results.append((match, 1.0)) return results + def __contains__(self, value): + return self.contains + class DummyConfig: def __init__(self): self.registry = DummyRegistry() diff --git a/pyramid/tests/test_integration.py b/pyramid/tests/test_integration.py index 1bf2abb4f..eedc145ad 100644 --- a/pyramid/tests/test_integration.py +++ b/pyramid/tests/test_integration.py @@ -759,219 +759,37 @@ class AcceptContentTypeTest(unittest.TestCase): res = app.get('/hello', headers={'Accept': 'something/else'}, status=200) self.assertEqual(res.content_type, 'text/x-fallback') -class AddViewAcceptArgMediaRangeAllTest(unittest.TestCase): - def setUp(self): - def view(request): - return 'text/plain' - from pyramid.config import Configurator - config = Configurator() - config.add_route('root', '/') - config.add_view( - view, route_name='root', accept='*/*', renderer='string', - ) - app = config.make_wsgi_app() - self.testapp = TestApp(app) - - def tearDown(self): - import pyramid.config - pyramid.config.global_registries.empty() - - def test_no_header(self): - res = self.testapp.get('/', headers={}, status=200) - self.assertEqual(res.content_type, 'text/plain') - - def test_header_all(self): - res = self.testapp.get('/', headers={'Accept': '*/*'}, status=200) - self.assertEqual(res.content_type, 'text/plain') - - def test_header_all_subtypes_of_type(self): - res = self.testapp.get('/', headers={'Accept': 'text/*'}, status=200) - self.assertEqual(res.content_type, 'text/plain') - - def test_header_specific_media_type(self): - res = self.testapp.get( - '/', headers={'Accept': 'text/plain'}, status=200, - ) - self.assertEqual(res.content_type, 'text/plain') - - def test_header_ruled_out_by_specific_media_type_q0(self): - res = self.testapp.get( - '/', headers={'Accept': 'text/plain;q=0, */*'}, status=200, - ) - self.assertEqual(res.content_type, 'text/plain') - - def test_header_ruled_out_by_type_range_q0(self): - res = self.testapp.get( - '/', headers={'Accept': 'text/*;q=0, text/html'}, status=200, - ) - self.assertEqual(res.content_type, 'text/plain') - - def test_header_ruled_out_by_all_range_q0(self): - res = self.testapp.get( - '/', headers={'Accept': '*/*;q=0, text/html'}, status=200, - ) - self.assertEqual(res.content_type, 'text/plain') - -class AddViewAcceptArgMediaRangeAllSubtypesOfTypeTest(unittest.TestCase): - def setUp(self): - def view(request): - return 'text/plain' - from pyramid.config import Configurator - config = Configurator() - config.add_route('root', '/') - config.add_view( - view, route_name='root', accept='text/*', renderer='string', - ) - app = config.make_wsgi_app() - self.testapp = TestApp(app) - - def tearDown(self): - import pyramid.config - pyramid.config.global_registries.empty() - - def test_no_header(self): - res = self.testapp.get('/', headers={}, status=200) - self.assertEqual(res.content_type, 'text/plain') - - def test_header_all(self): - res = self.testapp.get('/', headers={'Accept': '*/*'}, status=200) - self.assertEqual(res.content_type, 'text/plain') - - def test_header_all_subtypes_of_type(self): - res = self.testapp.get('/', headers={'Accept': 'text/*'}, status=200) - self.assertEqual(res.content_type, 'text/plain') - - def test_header_specific_media_type(self): - res = self.testapp.get( - '/', headers={'Accept': 'text/plain'}, status=200, - ) - self.assertEqual(res.content_type, 'text/plain') - - def test_header_none_acceptable(self): - self.testapp.get('/', headers={'Accept': 'application/*'}, status=404) - - def test_header_ruled_out_by_specific_media_type_q0(self): - res = self.testapp.get( - '/', headers={'Accept': 'text/plain;q=0, */*'}, status=200, - ) - self.assertEqual(res.content_type, 'text/plain') - - def test_header_ruled_out_by_type_range_q0(self): - res = self.testapp.get( - '/', headers={'Accept': 'text/*;q=0, text/html'}, status=200, - ) - self.assertEqual(res.content_type, 'text/plain') - - def test_header_ruled_out_by_all_range_q0(self): - res = self.testapp.get( - '/', headers={'Accept': '*/*;q=0, text/html'}, status=200, - ) - self.assertEqual(res.content_type, 'text/plain') - -class AddRouteAcceptArgMediaRangeAllTest(unittest.TestCase): - def setUp(self): - def view(request): - return 'text/plain' - from pyramid.config import Configurator - config = Configurator() - config.add_route('root', '/', accept='*/*') - config.add_view(view, route_name='root', renderer='string') - app = config.make_wsgi_app() - self.testapp = TestApp(app) - - def tearDown(self): - import pyramid.config - pyramid.config.global_registries.empty() - - def test_no_header(self): - res = self.testapp.get('/', headers={}, status=200) - self.assertEqual(res.content_type, 'text/plain') - - def test_header_all(self): - res = self.testapp.get('/', headers={'Accept': '*/*'}, status=200) - self.assertEqual(res.content_type, 'text/plain') - - def test_header_all_subtypes_of_type(self): - res = self.testapp.get('/', headers={'Accept': 'text/*'}, status=200) - self.assertEqual(res.content_type, 'text/plain') - - def test_header_specific_media_type(self): - res = self.testapp.get( - '/', headers={'Accept': 'text/plain'}, status=200, - ) - self.assertEqual(res.content_type, 'text/plain') - - def test_header_ruled_out_by_specific_media_type_q0(self): - res = self.testapp.get( - '/', headers={'Accept': 'text/plain;q=0, */*'}, status=200, - ) - self.assertEqual(res.content_type, 'text/plain') - - def test_header_ruled_out_by_type_range_q0(self): - res = self.testapp.get( - '/', headers={'Accept': 'text/*;q=0, text/html'}, status=200, - ) - self.assertEqual(res.content_type, 'text/plain') - - def test_header_ruled_out_by_all_range_q0(self): - res = self.testapp.get( - '/', headers={'Accept': '*/*;q=0, text/html'}, status=200, - ) - self.assertEqual(res.content_type, 'text/plain') - -class AddRouteAcceptArgMediaRangeAllSubtypesOfTypeTest(unittest.TestCase): - def setUp(self): - def view(request): - return 'text/plain' - from pyramid.config import Configurator - config = Configurator() - config.add_route('root', '/', accept='text/*') - config.add_view(view, route_name='root', renderer='string') - app = config.make_wsgi_app() - self.testapp = TestApp(app) - - def tearDown(self): - import pyramid.config - pyramid.config.global_registries.empty() - - def test_no_header(self): - res = self.testapp.get('/', headers={}, status=200) - self.assertEqual(res.content_type, 'text/plain') - - def test_header_all(self): - res = self.testapp.get('/', headers={'Accept': '*/*'}, status=200) - self.assertEqual(res.content_type, 'text/plain') - - def test_header_all_subtypes_of_type(self): - res = self.testapp.get('/', headers={'Accept': 'text/*'}, status=200) - self.assertEqual(res.content_type, 'text/plain') - - def test_header_specific_media_type(self): - res = self.testapp.get( - '/', headers={'Accept': 'text/plain'}, status=200, - ) - self.assertEqual(res.content_type, 'text/plain') - - def test_header_none_acceptable(self): - self.testapp.get('/', headers={'Accept': 'application/*'}, status=404) - - def test_header_ruled_out_by_specific_media_type_q0(self): - res = self.testapp.get( - '/', headers={'Accept': 'text/plain;q=0, */*'}, status=200, - ) + def test_deprecated_ranges_in_route_predicate(self): + config = self._makeConfig() + config.add_route('foo', '/foo', accept='text/*') + config.add_view(lambda r: 'OK', route_name='foo', renderer='string') + app = self._makeTestApp(config) + res = app.get('/foo', headers={ + 'Accept': 'application/json; q=1.0, text/plain; q=0.9', + }, status=200) self.assertEqual(res.content_type, 'text/plain') + self.assertEqual(res.body, b'OK') + res = app.get('/foo', headers={ + 'Accept': 'application/json', + }, status=404) + self.assertEqual(res.content_type, 'application/json') - def test_header_ruled_out_by_type_range_q0(self): - res = self.testapp.get( - '/', headers={'Accept': 'text/*;q=0, text/html'}, status=200, - ) + def test_deprecated_ranges_in_view_predicate(self): + config = self._makeConfig() + config.add_route('foo', '/foo') + config.add_view(lambda r: 'OK', route_name='foo', + accept='text/*', renderer='string') + app = self._makeTestApp(config) + res = app.get('/foo', headers={ + 'Accept': 'application/json; q=1.0, text/plain; q=0.9', + }, status=200) self.assertEqual(res.content_type, 'text/plain') + self.assertEqual(res.body, b'OK') + res = app.get('/foo', headers={ + 'Accept': 'application/json', + }, status=404) + self.assertEqual(res.content_type, 'application/json') - def test_header_ruled_out_by_all_range_q0(self): - res = self.testapp.get( - '/', headers={'Accept': '*/*;q=0, text/html'}, status=200, - ) - self.assertEqual(res.content_type, 'text/plain') class DummyContext(object): pass -- cgit v1.2.3 From 4cc3a67060f22bb11d037bd44a24112efca6d507 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Wed, 10 Oct 2018 00:18:28 -0500 Subject: fix up the docs --- docs/narr/viewconfig.rst | 31 +++++++++++++++++-------------- pyramid/config/routes.py | 16 ++++++++++------ pyramid/config/views.py | 47 +++++++++++++++++++++++++---------------------- 3 files changed, 52 insertions(+), 42 deletions(-) diff --git a/docs/narr/viewconfig.rst b/docs/narr/viewconfig.rst index 238599528..4c8427d98 100644 --- a/docs/narr/viewconfig.rst +++ b/docs/narr/viewconfig.rst @@ -285,6 +285,23 @@ Non-Predicate Arguments are just developing stock Pyramid applications. Pay no attention to the man behind the curtain. +``accept`` + A :term:`media type` that will be matched against the ``Accept`` HTTP request header. + If this value is specified, it must be a specific media type such as ``text/html`` or ``text/html;level=1``. + If the media type is acceptable by the ``Accept`` header of the request, or if the ``Accept`` header isn't set at all in the request, this predicate will match. + If this does not match the ``Accept`` header of the request, view matching continues. + + If ``accept`` is not specified, the ``HTTP_ACCEPT`` HTTP header is not taken into consideration when deciding whether or not to invoke the associated view callable. + + The ``accept`` argument is technically not a predicate and does not support wrapping with :func:`pyramid.config.not_`. + + See :ref:`accept_content_negotiation` for more information. + + .. versionchanged:: 1.10 + + Specifying a media range is deprecated and will be removed in :app:`Pyramid` 2.0. + Use explicit media types to avoid any ambiguities in content negotiation. + ``exception_only`` When this value is ``True``, the ``context`` argument must be a subclass of @@ -344,20 +361,6 @@ configured view. request/context pair found via :term:`resource location` does not indicate it matched any configured route. -``accept`` - A :term:`media type` that will be matched against the ``Accept`` HTTP request header. - If this value is specified, it must be a specific media type, such as ``text/html``. - If the media type is acceptable by the ``Accept`` header of the request, or if the ``Accept`` header isn't set at all in the request, this predicate will match. - If this does not match the ``Accept`` header of the request, view matching continues. - - If ``accept`` is not specified, the ``HTTP_ACCEPT`` HTTP header is not taken into consideration when deciding whether or not to invoke the associated view callable. - - See :ref:`accept_content_negotiation` for more information. - - .. versionchanged:: 1.10 - Media ranges such as ``text/*`` will now raise :class:`pyramid.exceptions.ConfigurationError`. - Previously these values had undefined behavior based on the version of WebOb being used and was never fully supported. - ``request_type`` This value should be an :term:`interface` that the :term:`request` must provide in order for this view to be found and called. diff --git a/pyramid/config/routes.py b/pyramid/config/routes.py index 4d1f830c8..832fd3f8b 100644 --- a/pyramid/config/routes.py +++ b/pyramid/config/routes.py @@ -228,17 +228,21 @@ class RoutesConfiguratorMixin(object): accept A :term:`media type` that will be matched against the ``Accept`` - HTTP request header. This value may be a specific media type such - as ``text/html``, or a list of the same. If the media type is - acceptable by the ``Accept`` header of the request, or if the - ``Accept`` header isn't set at all in the request, this predicate - will match. If this does not match the ``Accept`` header of the - request, route matching continues. + HTTP request header. If this value is specified, it may be a + specific media type such as ``text/html``, or a list of the same. + If the media type is acceptable by the ``Accept`` header of the + request, or if the ``Accept`` header isn't set at all in the request, + this predicate will match. If this does not match the ``Accept`` + header of the request, route matching continues. If ``accept`` is not specified, the ``HTTP_ACCEPT`` HTTP header is not taken into consideration when deciding whether or not to select the route. + Unlike the ``accept`` argument to + :meth:`pyramid.config.Configurator.add_view`, this value is + strictly a predicate and supports :func:`pyramid.config.not_`. + .. versionchanged:: 1.10 Specifying a media range is deprecated due to changes in WebOb diff --git a/pyramid/config/views.py b/pyramid/config/views.py index ca299c4b5..8d542a8a1 100644 --- a/pyramid/config/views.py +++ b/pyramid/config/views.py @@ -542,6 +542,31 @@ class ViewsConfiguratorMixin(object): very useful for 'civilians' who are just developing stock Pyramid applications. Pay no attention to the man behind the curtain. + accept + + A :term:`media type` that will be matched against the ``Accept`` + HTTP request header. If this value is specified, it must be a + specific media type such as ``text/html`` or ``text/html;level=1``. + If the media type is acceptable by the ``Accept`` header of the + request, or if the ``Accept`` header isn't set at all in the request, + this predicate will match. If this does not match the ``Accept`` + header of the request, view matching continues. + + If ``accept`` is not specified, the ``HTTP_ACCEPT`` HTTP header is + not taken into consideration when deciding whether or not to invoke + the associated view callable. + + The ``accept`` argument is technically not a predicate and does + not support wrapping with :func:`pyramid.config.not_`. + + See :ref:`accept_content_negotiation` for more information. + + .. versionchanged:: 1.10 + + Specifying a media range is deprecated and will be removed in + :app:`Pyramid` 2.0. Use explicit media types to avoid any + ambiguities in content negotiation. + exception_only .. versionadded:: 1.8 @@ -674,28 +699,6 @@ class ViewsConfiguratorMixin(object): represents a header name or a header name/value pair, the case of the header name is not significant. - accept - - A :term:`media type` that will be matched against the ``Accept`` - HTTP request header. This value must be a specific media type such - as ``text/html`` or ``text/html;level=1``. If the media type is - acceptable by the ``Accept`` header of the request, or if the - ``Accept`` header isn't set at all in the request, this predicate - will match. If this does not match the ``Accept`` header of the - request, view matching continues. - - If ``accept`` is not specified, the ``HTTP_ACCEPT`` HTTP header is - not taken into consideration when deciding whether or not to invoke - the associated view callable. - - See :ref:`accept_content_negotiation` for more information. - - .. versionchanged:: 1.10 - - Specifying a media range is deprecated and will be removed in - :app:`Pyramid` 2.0. Use explicit media types to avoid any - ambiguities in content negotiation. - path_info This value represents a regular expression pattern that will -- cgit v1.2.3 From 0ddcb55c37770f1879490d25692ee644c754e67d Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Wed, 10 Oct 2018 00:27:47 -0500 Subject: track webob master --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 76428846f..9dee1b20d 100644 --- a/tox.ini +++ b/tox.ini @@ -26,7 +26,7 @@ extras = testing deps = - -egit+https://github.com/mmerickel/webob.git@accept-parse-offer#egg=webob + -egit+https://github.com/Pylons/webob.git@master#egg=webob [testenv:py27-scaffolds] basepython = python2.7 -- cgit v1.2.3 From 19eef8916cc556dffa3f9d48498bea869ee74b88 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Wed, 10 Oct 2018 02:09:49 -0500 Subject: use webob's new offer normalization --- pyramid/config/routes.py | 4 ++-- pyramid/config/util.py | 8 ++++---- pyramid/config/views.py | 12 ++++++------ tox.ini | 2 +- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/pyramid/config/routes.py b/pyramid/config/routes.py index 832fd3f8b..5d05429a7 100644 --- a/pyramid/config/routes.py +++ b/pyramid/config/routes.py @@ -318,8 +318,8 @@ class RoutesConfiguratorMixin(object): DeprecationWarning, stacklevel=3, ) - # XXX switch this to verify=True when range support is dropped - accept = [normalize_accept_offer(accept, verify=False)] + # XXX switch this to False when range support is dropped + accept = [normalize_accept_offer(accept, allow_range=True)] else: accept = [ diff --git a/pyramid/config/util.py b/pyramid/config/util.py index 8ebc8e45c..05d810f6f 100644 --- a/pyramid/config/util.py +++ b/pyramid/config/util.py @@ -221,10 +221,10 @@ class PredicateList(object): return order, preds, phash.hexdigest() -def normalize_accept_offer(offer, verify=True): - if verify: - Accept.parse_offer(offer) - return offer.lower() +def normalize_accept_offer(offer, allow_range=False): + if allow_range and '*' in offer: + return offer.lower() + return str(Accept.parse_offer(offer)) def sort_accept_offers(offers, order=None): diff --git a/pyramid/config/views.py b/pyramid/config/views.py index 8d542a8a1..e6baa7c17 100644 --- a/pyramid/config/views.py +++ b/pyramid/config/views.py @@ -840,8 +840,8 @@ class ViewsConfiguratorMixin(object): DeprecationWarning, stacklevel=4, ) - # XXX when media ranges are gone, switch verify=True - accept = normalize_accept_offer(accept, verify=False) + # XXX when media ranges are gone, switch allow_range=False + accept = normalize_accept_offer(accept, allow_range=True) view = self.maybe_dotted(view) context = self.maybe_dotted(context) @@ -1308,12 +1308,12 @@ class ViewsConfiguratorMixin(object): 'cannot compare params across different media types') def normalize_types(thans): - thans = [normalize_accept_offer(o, verify=False) for o in thans] - for o in thans: - check_type(o) + thans = [normalize_accept_offer(than) for than in thans] + for than in thans: + check_type(than) return thans - value = normalize_accept_offer(value, verify=False) + value = normalize_accept_offer(value) offer_type, offer_subtype, offer_params = Accept.parse_offer(value) if weighs_more_than: diff --git a/tox.ini b/tox.ini index 9dee1b20d..a5bb0f691 100644 --- a/tox.ini +++ b/tox.ini @@ -26,7 +26,7 @@ extras = testing deps = - -egit+https://github.com/Pylons/webob.git@master#egg=webob + -egit+https://github.com/mmerickel/webob.git@accept-offer-object#egg=webob [testenv:py27-scaffolds] basepython = python2.7 -- cgit v1.2.3 From 2a69c7b60069d5c6de7a49436c26dd329f310fea Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Wed, 10 Oct 2018 02:13:02 -0500 Subject: sentence-per-line --- docs/narr/viewconfig.rst | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/docs/narr/viewconfig.rst b/docs/narr/viewconfig.rst index 4c8427d98..f756fa161 100644 --- a/docs/narr/viewconfig.rst +++ b/docs/narr/viewconfig.rst @@ -1099,14 +1099,21 @@ Default Accept Ordering For any set of media type offers with the same ``type/subtype``, the offers with params will weigh more than the bare ``type/subtype`` offer. This means that ``text/plain;charset=utf8`` will always be offered before ``text/plain``. -By default, within a given ``type/subtype``, the order of offers is ambiguous. For example, ``text/plain;charset=utf8`` versus ``text/plain;charset=latin1`` are sorted in an unspecified way. Similarly, between media types the order is also unspecified other than the defaults described below. For example, ``image/jpeg`` versus ``image/png`` versus ``application/pdf``. In these cases, the ordering may be controlled using :meth:`pyramid.config.Configurator.add_accept_view_order`. For example, to sort ``text/plain`` higher than ``text/html`` and to prefer a ``charset=utf8`` versus a ``charset=latin-1`` within the ``text/plain`` media type: +By default, within a given ``type/subtype``, the order of offers is ambiguous. +For example, ``text/plain;charset=utf8`` versus ``text/plain;charset=latin1`` are sorted in an unspecified way. +Similarly, between media types the order is also unspecified other than the defaults described below. +For example, ``image/jpeg`` versus ``image/png`` versus ``application/pdf``. +In these cases, the ordering may be controlled using :meth:`pyramid.config.Configurator.add_accept_view_order`. +For example, to sort ``text/plain`` higher than ``text/html`` and to prefer a ``charset=utf8`` versus a ``charset=latin-1`` within the ``text/plain`` media type: .. code-block:: python config.add_accept_view_order('text/plain', weighs_more_than='text/html') config.add_accept_view_order('text/plain;charset=utf8', weighs_more_than='text/plain;charset=latin-1') -It is an error to try and sort accept headers across levels of specificity. You can only sort a ``type/subtype`` against another ``type/subtype``, not against a ``type/subtype;params``. That ordering is a hard requirement. +It is an error to try and sort accept headers across levels of specificity. +You can only sort a ``type/subtype`` against another ``type/subtype``, not against a ``type/subtype;params``. +That ordering is a hard requirement. By default, :app:`Pyramid` defines a very simple priority ordering for views that prefers human-readable responses over JSON: -- cgit v1.2.3 From 5dd3165722ffabb1beed0072ec8164d9630e7d26 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Wed, 10 Oct 2018 02:20:56 -0500 Subject: switch back to webob master --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index a5bb0f691..9dee1b20d 100644 --- a/tox.ini +++ b/tox.ini @@ -26,7 +26,7 @@ extras = testing deps = - -egit+https://github.com/mmerickel/webob.git@accept-offer-object#egg=webob + -egit+https://github.com/Pylons/webob.git@master#egg=webob [testenv:py27-scaffolds] basepython = python2.7 -- cgit v1.2.3 From 9a3238a7c21f2af1467be1bc0b1f84ee01c167a2 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Wed, 10 Oct 2018 19:52:13 -0500 Subject: improve the language around unspecified match ordering --- docs/narr/viewconfig.rst | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/docs/narr/viewconfig.rst b/docs/narr/viewconfig.rst index f756fa161..dd767e3f1 100644 --- a/docs/narr/viewconfig.rst +++ b/docs/narr/viewconfig.rst @@ -1039,8 +1039,7 @@ Accept Header Content Negotiation --------------------------------- The ``accept`` argument to :meth:`pyramid.config.Configurator.add_view` can be used to control :term:`view lookup` by dispatching to different views based on the HTTP ``Accept`` request header. -Consider the example below in which there are three defined views. -Each view uses the ``Accept`` header to trigger an appropriate response renderer. +Consider the example below in which there are three views configured. .. code-block:: python @@ -1058,16 +1057,19 @@ Each view uses the ``Accept`` header to trigger an appropriate response renderer def myview_unacceptable(request): raise HTTPNotAcceptable -The appropriate view is selected here when the client specifies an unambiguous header such as ``Accept: text/*`` or ``Accept: application/json``. +Each view relies on the ``Accept`` header to trigger an appropriate response renderer. +The appropriate view is selected here when the client specifies headers such as ``Accept: text/*`` or ``Accept: application/json, text/html;q=0.9`` in which only one of the views matches or it's clear based on the preferences which one should win. Similarly, if the client specifies a media type that no view is registered to handle, such as ``Accept: text/plain``, it will fall through to ``myview_unacceptable`` and raise ``406 Not Acceptable``. -There are a few cases in which the client may specify ambiguous constraints: + +There are a few cases in which the client may specify an ``Accept`` header such that it's not clear which view should win. +For example: - ``Accept: */*``. - More than one acceptable media type with the same quality. - A missing ``Accept`` header. - An invalid ``Accept`` header. -In these cases the preferred view is not clearly defined (see :rfc:`7231#section-5.3.2`) and :app:`Pyramid` will select one semi-randomly. +In these cases the preferred view is not clearly defined (see :rfc:`7231#section-5.3.2`) and :app:`Pyramid` will select one randomly. This can be controlled by telling :app:`Pyramid` what the preferred relative ordering is between various media types by using :meth:`pyramid.config.Configurator.add_accept_view_order`. For example: @@ -1085,7 +1087,7 @@ For example: config.scan() return config.make_wsgi_app() -In this case, the ``application/json`` view should always be selected in cases where it is otherwise ambiguous. +Now, the ``application/json`` view should always be preferred in cases where the client wasn't clear. .. index:: single: default accept ordering @@ -1099,8 +1101,8 @@ Default Accept Ordering For any set of media type offers with the same ``type/subtype``, the offers with params will weigh more than the bare ``type/subtype`` offer. This means that ``text/plain;charset=utf8`` will always be offered before ``text/plain``. -By default, within a given ``type/subtype``, the order of offers is ambiguous. -For example, ``text/plain;charset=utf8`` versus ``text/plain;charset=latin1`` are sorted in an unspecified way. +By default, within a given ``type/subtype``, the order of offers is unspecified. +For example, ``text/plain;charset=utf8`` versus ``text/plain;charset=latin1`` are sorted randomly. Similarly, between media types the order is also unspecified other than the defaults described below. For example, ``image/jpeg`` versus ``image/png`` versus ``application/pdf``. In these cases, the ordering may be controlled using :meth:`pyramid.config.Configurator.add_accept_view_order`. @@ -1108,6 +1110,8 @@ For example, to sort ``text/plain`` higher than ``text/html`` and to prefer a `` .. code-block:: python + config.add_accept_view_order('text/html') + config.add_accept_view_order('text/plain;charset=latin-1') config.add_accept_view_order('text/plain', weighs_more_than='text/html') config.add_accept_view_order('text/plain;charset=utf8', weighs_more_than='text/plain;charset=latin-1') -- cgit v1.2.3 From 364bd7dfef2c6599b126671784b23f9fb7fa6dea Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Sun, 14 Oct 2018 20:56:50 -0500 Subject: pin to webob >= 1.8.3 --- setup.py | 2 +- tox.ini | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/setup.py b/setup.py index d3d453a0d..aa7e3ab60 100644 --- a/setup.py +++ b/setup.py @@ -29,7 +29,7 @@ install_requires = [ 'setuptools', 'translationstring >= 0.4', # py3 compat 'venusian >= 1.0', # ``ignore`` - 'webob >= 1.8.2', # cookies.make_cookie allows non-bytes samesite + 'webob >= 1.8.3', # Accept.parse_offer 'zope.deprecation >= 3.5.0', # py3 compat 'zope.interface >= 3.8.0', # has zope.interface.registry ] diff --git a/tox.ini b/tox.ini index 9dee1b20d..5a73bc426 100644 --- a/tox.ini +++ b/tox.ini @@ -25,9 +25,6 @@ commands = extras = testing -deps = - -egit+https://github.com/Pylons/webob.git@master#egg=webob - [testenv:py27-scaffolds] basepython = python2.7 commands = -- cgit v1.2.3