Admin incompatible with USE_ETAGS = True

[Chris Lamb]

In SVN, the admin does not work with USE_ETAGS = True:

Environment:

Traceback:
File "/srv/www.example.com/example/packages/django/core/handlers/base.py" in get_response
  111.                         response = callback(request, *callback_args, **callback_kwargs)
File "/srv/www.example.com/example/packages/django/contrib/admin/sites.py" in wrapper
  213.                 return self.admin_view(view, cacheable)(*args, **kwargs)
File "/srv/www.example.com/example/packages/django/utils/decorators.py" in _wrapped_view
  91.                     response = view_func(request, *args, **kwargs)
File "/srv/www.example.com/example/packages/django/views/decorators/cache.py" in _wrapped_view_func
  77.         response = view_func(request, *args, **kwargs)
File "/srv/www.example.com/example/packages/django/contrib/admin/sites.py" in inner
  196.             return view(request, *args, **kwargs)
File "/srv/www.example.com/example/packages/django/views/decorators/cache.py" in _wrapped_view_func
  78.         add_never_cache_headers(response)
File "/srv/www.example.com/example/packages/django/utils/cache.py" in add_never_cache_headers
  116.     patch_response_headers(response, cache_timeout=-1)
File "/srv/www.example.com/example/packages/django/utils/cache.py" in patch_response_headers
  105.         response['ETag'] = '"%s"' % hashlib.md5(response.content).hexdigest()
File "/srv/www.example.com/example/packages/django/template/response.py" in _get_content
  110.             raise ContentNotRenderedError('The response content must be rendered before it can be accessed.')

Exception Type: ContentNotRenderedError at /a/admin/
Exception Value: The response content must be rendered before it can be accessed.

[Karen Tracey]

Fixed formatting, please use preview. I suspect this is due to r16087. It does not happen with 1.3 release. There's also a likely-related thread on django-developers: ​http://groups.google.com/group/django-developers/browse_thread/thread/f96e982254fbe5c3

[Jannis Leidel]

Yeah, this seems like a rather major issue with the TemplateResponse feature, which I think can only be fixed by actually rendering the content if needed instead of raising a ContentNotRenderedError exception.

With proper documentation of which middlewares/decorator modify the content we should be able to keep the usefulness of TemplateResponse up.

[Peter van Kampen]

I've got a preliminary fix for this. I'll be at the djangocon sprint in A'dam tomorrow from ~10.00 on to continue work on this.

[Peter van Kampen]

A preliminary patch that fixes the issue in the admin. Needs more work for an alround solution. Also regression tests

[Peter van Kampen]

An updated patch /w tests

[Peter van Kampen]

I think I fixed this for all places patch_response_header is called. I've added a test that fails when USE_ETAGS is set to True and an admin site is called. I've added a few more tests to check various headers with TemplateResponse but I think only the etag is relevant because it requires the content is complete when the etag value is calculated.

The solution, IMO, is using the add_post_render_callback if/when the response is a TemplateResponse (using ducktype checking now rather than using isinstance).

[Tai Lee]

The patch here seems to go against the advice from core committers in the past regarding middleware having to perform repetitive behaviour such as interrogate a response to find out whether or not it can access or replace the content (e.g. checking if the content is a string or a generator, for streaming responses).

If we start heading down this path of middleware being required to ask each response if it has rendered content (or generator content) and conditionally performing an action, where does it end?

If middleware can't be certain that response.content will be a string, then ALL bundled and 3rd party middleware will always need to perform this (and possibly other) checks before taking conditional or no action.

There was some discussion in the past about the best way to conditionally disable middleware for a particular response or class of responses (e.g. streaming) without having to make every middleware perform these checks. But no consensus was ever reached. And with this issue now breaking the admin when used with etags, it seems we need to at least work-around it or roll back the change that introduced this breakage while the larger issues are examined.

See ​http://groups.google.com/group/django-developers/browse_thread/thread/3e0d78b98498c159/f08036fb1e7fe6b9 and also #7581 for past discussions.

I think that the solution should be for TemplateResponse to render it's content when .content is accessed, in combination with a mechanism for people to specify that certain middleware should not run for certain responses. This could lead to unexpected behaviour though, if a user is attempting to replace a template before rendering in another middleware.

Alternatively, there could be a generic way to defer processing of a certain middleware in HttpResponse subclasses, such as TemplateResponse (and maybe a new StreamingResponse). But it is not practical for an HttpResponse subclass to know all the middleware that it may be exposed to. There are also complex issues with general capabilities checking for responses and middleware.

[Jannis Leidel]

In [16729]:

Fixed #16003 -- Restored compatibility of the admin when using USE_ETAGS. Thanks for the initial patch, pterk.