cache_response should cache content rather than rendered_content
ivoscc opened this issue · 2 comments
Hi, I ran into an issue while using the cache_response
decorator.
Specifically when trying to cache a compressed response using both a custom gzip
decorator and the cache_response
one, similar to:
class SomeView(GenericAPIView):
@cache_response()
@gzip
def get(self, request, *args, **kwargs):
return Response(...)
My gzip decorator does something similar to what django's gzip_page
decorator does.
def gzip():
# ...
response.content = compressed_content
response['Content-Length'] = str(len(response.content))
response['Content-Encoding'] = 'gzip'
# ...
I noticed the problem stems from https://github.com/chibisov/drf-extensions/blob/0.4.0/rest_framework_extensions/cache/decorators.py#L74 caching the Response.rendered_content
rather than the Response.content
.
The latter is the gzipped version while the former is already uncompressed. This causes issues when creating a new response from the cached data, because the headers indicate the response is gzipped/has a certain length, but the actual content is already rendered/uncompressed.
This was working as expected in drf-extensions==0.3.1 where the whole response object was cached. I've fixed this in drf-extensions==0.4.0 by subclassing cache_response
and actually caching the Response.content
.
I can send a PR for this but wanted to check I wasn't missing anything before doing so. Please let me know if I can provide any more information.
EDIT: Sorry, pressed "Submit" too soon.
EDIT 2: Formatting.
+1!
Hi! Is any problems if use @gzip_page from Django? Why do you use custom gzip-decorator?
That were worked for you with 0.3.1 because it cached whole Response object rather than only content and headers.
IMO, it better to enable gzip compression on nginx or Apache etc.