I may have seen this idea in IRC but instead of a new middleware_decorator function did you consider varying decorator_from_middleware's behavior based on "if settings.MIDDLEWARE is None" similar to what BaseHandler does? We could add a check in decorator_from_middleware to raise an error if MIDDLEWARE is None and the middleware doesn't seem to be the new-style.
On Monday, June 20, 2016 at 10:52:06 AM UTC-4, Carl Meyer wrote: > > Hi all, > > In porting decorator_from_middleware to support DEP 5 middleware, I ran > into one stumbling block: TemplateResponse. Normally with old-style > middleware, process_response runs after TemplateResponses have been > rendered. But a view decorator runs immediately wrapping the view, when > TemplateResponses will not have been rendered yet. So > decorator_from_middleware has special support for TemplateResponse -- if > it detects one, rather than calling process_response immediately, it > sets it as an on_render_callback. > > But in DEP 5 middleware, request and response processing are not > separable -- they are part of the same function. This is the source of > the benefits of DEP 5: strict in/out layering, the ability to use > context managers in middleware, etc. But it also means that there is no > way to delay just the response-processing portion of the middleware > until after render. > > My initial strategy [1] was to simply make middleware authors > responsible for this: a middleware that needed to access > response.content and wanted to be compatible with > decorator_from_middleware would be responsible to detect and handle > unrendered TemplateResponse appropriately. > > In discussion in IRC, Florian Apolloner suggested an alternative: change > the implementation of decorator_from_middleware so that instead of > immediately applying all middleware methods as an actual view decorator, > make the decorator simply annotate the view callable with an > `_extra_middleware` list, and then add support for this annotation to > the base handler (effectively making it support per-view middleware). > The handler runs per-view middleware as if it were listed last in > MIDDLEWARE (that is, "closest" to the view). I've implemented this > suggestion in [2]. It has several advantages: > > 1. It consolidates middleware-handling logic in one place, the handler. > Currently this logic is effectively duplicated in the handler and in > decorator_from_middleware, and I've noticed several bugs in the current > decorator_from_middleware resulting from inconsistencies between the > duplicate implementations (e.g. if process_view or process_exception > returns a response, normally that response passes through > process_response, but in decorator_from_middleware it does not.) > Consolidation of this logic will improve consistency of middleware > behavior when listed in settings vs when applied via decorator. > > 2. It allows us to apply middleware-decorators in closer to the same way > that normal middleware is applied, e.g. response-handling can happen > after render() is called on TemplateResponses, solving the initial > problem. > > Possible disadvantages of this approach: > > 1. In order to avoid immediate breakage of decorator_from_middleware > when used with third-party middleware that don't yet support DEP 5, I've > implemented the new approach as a new function, `middleware_decorator`, > that will replace and deprecate `decorator_from_middleware`. This is > unfortunate churn, but I don't see another good way to be > backwards-compatible. > > 2. The decorators internal to Django that rely on > decorator_from_middleware (the csrf decorators, cache_page) will be > immediately converted to middleware_decorator, resulting in subtle > behavior differences from the current ones. > > 3. The new behavior may surprise some people accustomed to the old > behavior, since it means the effect of the middleware decorator won't > occur immediately where the decorator is applied. E.g. if you apply a > middleware decorator to a view, and then another custom view decorator > outside that, the custom view decorator won't see the effects of the > middleware decorator in the actual response (it would only see a new > entry in view_func._extra_middleware). > > Thoughts welcome. > > Carl > > [1] https://github.com/django/django/pull/6765 > [2] https://github.com/django/django/pull/6805 > > -- You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. Visit this group at https://groups.google.com/group/django-developers. To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/d84381c2-5e2f-425f-b257-50eeae9a3aa6%40googlegroups.com. For more options, visit https://groups.google.com/d/optout.
