Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cache_response should cache content rather than rendered_content #250

Open
ivoscc opened this issue Jan 17, 2019 · 2 comments
Open

cache_response should cache content rather than rendered_content #250

ivoscc opened this issue Jan 17, 2019 · 2 comments

Comments

@ivoscc
Copy link
Contributor

ivoscc commented Jan 17, 2019

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
Copy link

tspecht commented Jan 17, 2019

+1!

@SerhiyRomanov
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants