chibisov / drf-extensions

DRF-extensions is a collection of custom extensions for Django REST Framework
http://chibisov.github.io/drf-extensions/docs
MIT License
1.48k stars 208 forks source link

cache_response should cache content rather than rendered_content #250

Open ivoscc opened 5 years ago

ivoscc commented 5 years ago

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.

tspecht commented 5 years ago

+1!

SerhiyRomanov commented 5 years ago

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.