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.47k stars 208 forks source link

etag_func evaluation should happen after resource is computed #117

Closed nangia closed 8 years ago

nangia commented 8 years ago

Consider an API request GET /nowtime/ implemented as following:

from django.utils import timezone

class CurrentTimeView(APIView):
    permission_classes = (AllowAny,)

    @etag()
    def get(self, request):
        nowtime = timezone.now()
        return Response({"curtime": str(nowtime)})

Every time I call this API, I would expect a new value of the resource irrespective of whether I pass If-None-Match or not.

However, this is not the behaviour I see.

When, I call /nowtime/ without sending in If-None-Match, I get the following (showing the results using httpie.org).

http -v GET 'http://localhost:8000/api/v1/nowtime/' GET /api/v1/nowtime/ HTTP/1.1 Accept: / Accept-Encoding: gzip, deflate Connection: keep-alive Host: localhost:8000 User-Agent: HTTPie/0.9.2

Output is:

HTTP/1.0 200 OK Allow: GET, HEAD, OPTIONS Content-Type: application/json Date: Wed, 02 Dec 2015 13:21:54 GMT ETag: "be4b3d44ccb6e47d19aad5712ea1d2ce299e1f30742f44d7beda7f6bdcfb3417" Server: WSGIServer/0.1 Python/2.7.10 Vary: Accept, Cookie X-Frame-Options: SAMEORIGIN

{ "curtime": "2015-12-02 13:21:54.399502+00:00" }

Now, when I call the API with a header value of ETAG received above, I get the following:

http -v GET 'http://localhost:8000/api/v1/nowtime/' If-None-Match:be4b3d44ccb6e47d19aad5712ea1d2ce299e1f30742f44d7beda7f6bdcfb3417 GET /api/v1/nowtime/ HTTP/1.1 Accept: / Accept-Encoding: gzip, deflate Connection: keep-alive Host: localhost:8000 If-None-Match: be4b3d44ccb6e47d19aad5712ea1d2ce299e1f30742f44d7beda7f6bdcfb3417 User-Agent: HTTPie/0.9.2

Output is:

HTTP/1.0 304 NOT MODIFIED Allow: GET, HEAD, OPTIONS Content-Length: 0 Date: Wed, 02 Dec 2015 13:23:25 GMT ETag: "be4b3d44ccb6e47d19aad5712ea1d2ce299e1f30742f44d7beda7f6bdcfb3417" Server: WSGIServer/0.1 Python/2.7.10 Vary: Accept, Cookie X-Frame-Options: SAMEORIGIN

My expected behaviour was that overtime I should get a different value of `/nowtime/ and never get 304 outputs.

When I tried to understand what is happening, I see that my function is not even called when I call with the same etag as was received last time. I expected instead that my function would be called, a hash value (etag) computed afresh and then a comparison be made with the etag value that was passed to it via header If-None-Match. If it matches, a 304 response should have been returned. Instead, I see that a comparison is made with the last etag sent and then evaluation of the resource (GET) is simply ignored.

nangia commented 8 years ago

I noticed another thing:

Whenever I call the API GET /nowtime/ I always see the same ETAG. I am using the default etag implementation. Aren't we supposed to get a different ETAG based on differing output?

BTW, I am using drf-extensions==0.2.8.

http -v GET 'http://localhost:8000/api/v1/nowtime/'
GET /api/v1/nowtime/ HTTP/1.1 Accept: / Accept-Encoding: gzip, deflate Connection: keep-alive Host: localhost:8000 User-Agent: HTTPie/0.9.2

Output

HTTP/1.0 200 OK Allow: GET, HEAD, OPTIONS Content-Type: application/json Date: Wed, 02 Dec 2015 13:40:55 GMT ETag: "be4b3d44ccb6e47d19aad5712ea1d2ce299e1f30742f44d7beda7f6bdcfb3417" Server: WSGIServer/0.1 Python/2.7.10 Vary: Accept, Cookie X-Frame-Options: SAMEORIGIN

{ "curtime": "2015-12-02 13:40:55.113443+00:00" }

chibisov commented 8 years ago

That's correct because Default key constructor doesn't know how to invalidate your cache. Please, read how to create custom key bit

auvipy commented 8 years ago

@chibisov this issue should be closed as invalid?

nangia commented 8 years ago

@chibisov, thank for you response. I tried going through the code as well. To me it appers that, the etag is computed based on various possible inputs like current languge, URL, sql query used etc before the actual evaluation of response (after a sql query is fired on database). Is this understanding correct?

If so, this leads to a possible case, where the URl, sql query, page for a GET query is same. However, the query result is different because some other database query changed the tables. Or perhaps, to handle the case, you expect that some sort of cache invalidation should happen? Do I get this right?

chibisov commented 8 years ago

Or perhaps, to handle the case, you expect that some sort of cache invalidation should happen?

Absolutely. drf-extensions only does simple key construction. You have to invalidate cache data by this key by custom logic. No magic in this place.