Hi Carl, I think your core idea is quite interesting. I haven't had time to think through all the implications.
It does make the result of what is returned from view functions rather obscure, and this could affect things like testing. For example, the new tests I wrote for decorator_from_middleware, which pass with my first patch on #16004, will not pass with this implementation. This means the semantics are quite difference. It also means that tests using RequestFactory could be broken, because they won't pass through the machinery of request handling. However, with one modification, we can get something quite a bit better. Basically, if it is a TemplateResponse, we defer the process_response handling as you say. Otherwise, we do it the way we currently do it. In fact, we *almost* have machinery in place to do this - 'TemplateResponse.add_post_render_callback' - and I've got a patch based on that which works. The only problem is that currently 'process_response' is allowed to return a completely new response object, whereas that is not supported with add_post_render_callback. That would be easy to change, however - or we could add a similar hook which does support it. Either way, it would require changing code like this: response.render() to response = response.render() This way, the only change people have to cope with in tests is adding 'response = response.render()' to things that have 'render', rather than having to invoke some other machinery to get the expected result - TemplateResponse has all the machinery itself. Since they already have to invoke 'render' for anything that has changed to TemplateResponse, this change adds no extra burden. (BTW, we do need to document the change introduced by [16087] in the 1.4 release notes as a breaking change - any tests against customized admin views that use RequestFactory will currently break if they don't add '.render()' calls.) Putting all these things together, I've uploaded a new patch to #16004: http://code.djangoproject.com/attachment/ticket/16004/16004.fix.alternative.diff It is not nearly as invasive as the other changes you suggested. However, it does fix the failing tests in my project (i.e. fixes csrf_protect), and leaves TemplateResponse objects unrendered as long as possible, without having to change one line of my own code/tests. And the implementation doesn't make me puke either. I agree that we ought to be doing something about middleware ordering and things like that, but perhaps that is something for Django 2000 - when we can sit down and work out how everything should fit together. One little note while I remember - I'm really not happy about decorator functions looking at settings to see how they should work - this is fixing fragility by adding more fragility. We want to be getting rid of this kind of thing, not adding more of it. There was also the general assumption of just one view being involved, one response object. We don't want to break code that is slightly more inventive, like this: http://docs.djangoproject.com/en/dev/ref/contrib/csrf/#view-needs-protection-for-one-path (Sorry for vague criticisms, I don't have more time to spend on this today). Regards, Luke -- "The first ten million years were the worst. And the second ten million, they were the worst too. The third ten million, I didn't enjoy at all. After that I went into a bit of a decline." (Marvin the paranoid android) Luke Plant || http://lukeplant.me.uk/ -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
