Easily add/validate etags.

[schinckel]

Django has an awesome etag/condition/last_modified decorator. It doesn't work with the d-r-f class-based views, as you cannot decorate 'get' with them. Because get returns an object that is not an http response, there is no way to add the etag header to the response.

I would like to see a way to do this from within d-r-f. I'm thinking something along the lines of an overridable method on a resource, or a view (or a mixin) that can be used to generate the etag.

The other way of doing it in django is to use the middleware, but it cannot shortcut running the body of the view altogether like the decorator can.

[schinckel]

Okay, I have an preliminary patch: schinckel@cc3a88e

[schinckel]

Very happy for feedback on this, though.

[tomchristie]

Ok, so, initially what I wrote was this....

Cool, yeah I'd really love to see this in.

Coupla thoughts - you should be able to just use View.add_header, rather than the View._ETAG that you currently have.
(And it looks like .add_header probably ought to be moved into the ResponseMixin class.)

Secondly, I'd like to see the @condition, @etag and @last_modified decorates can probably be pretty much a straight-up clone of https://github.com/django/django/blob/master/django/views/decorators/http.py, just replacing a couple of add_header and ErrorResponses

[tomchristie]

But was looking into things a bit more...

[tomchristie]

And perhaps this isn't quite the right way to go afterall...

[tomchristie]

You can actually return HttpResponses from REST framework views, they just don't get all the usual content negotiation/serialization stuff applied. The @last_modified, @etag and @condition decorators only ever return empty HttpResponses, so that's not really a problem.

So, what I'm thinking is, if we simply added __setitem__ __getitem__ and has_header to the Response class, then I think that Django's existing @last_modified, @etag and @condition decorators should work just fine on a REST framework view so long as the view is using the return Response(status, data) style rather than the return data style.

Obv it'd be helpful if we documented that, but it might make more sense than having to replicate something that Django already does.

What do you think?

[schinckel]

There may be one problem with using the django decorators: I'm not sure they'll work with methods, only bare functions. The decorator I wrote was heavily based on one I found via StackOverflow just for this case.

That may not be the case, in which case this solution sounds superior.

Having said that, I have been returning the return data style, as it is less boilerplate, and I'm usually only returning objects I want to serialize as json. We may be able to make it work with both ways.

Another option may be a mixin that adds these to the View class.

It also occurs to me that the django decorators may not do the right thing with respect to conditional PUT, POST and DELETE requests. Just submitted a patch to sinatra to fix that issue.

[schinckel]

Ignore the last bit: clearly I hadn't read the code properly.

[tomchristie]

Actually there's a point there: those decorators might not work ATM on methods, since they've got the additional 'self' argument. I'll prob look into that and poss submit a ticket to Django, since they ought to work with CBVs too...

[tomchristie]

Ah, okay - I see the @method_decorator now... https://docs.djangoproject.com/en/dev/topics/class-based-views/#decorating-class-based-views
So I guess this for this to be closed we need:

1. A bit of tweaking to Response
2. Some light documentation

[schinckel]

I'm sure I looked in that document, but I didn't see that decorator!

[vitormazzi]

I tried to use the django decorator and the initial result was really strange, with information which should belong only in the views appearing in unrelated etag functions.

After a few hours trying to make it a little better, I came up a solution which resolved my problems and seems general enough. What are your thoughts on this:

https://bitbucket.org/vitormazzi/django-rest-framework/changeset/6f8de4500c6f

[ghickman]

Hoping to breath some new life into this issue now that we're on the 2.x releases.

My thoughts here are mostly cobbled together from trying to add the functionality in a project and from reading this post so definitely still rough.

I see two areas where DRF needs to consider ETags - usage in views and how to get the unique representation of an instance's version.

Views

GET
GET requests simply need to serve the objects ETag in the appropriate header. A one line change to RetrieveModelMixin can easily add this:

def retrieve(self, request, *args, **kwargs):
    self.object = self.get_object()
    serializer = self.get_serializer(self.object)
    headers = {'ETag': self.object.etag}
    return Response(serializer.data, headers=headers)

PUT, PATCH, DELETE
A general check for updating HTTP verbs could be done in the view's dispatch or possibly pulled out into another method as it'll need to check if ETags are turned on (see options section below):

    header_etag = request.META.get('HTTP_IF_MATCH')
    if header_etag is None:
        return Response({'error': 'IF_MATCH header is required'}, status=400)

Then a more detailed check after retrieving the object to see if the request thinks it's looking at the right one:

    if self.object.etag != header_etag:
        return Response({'error': 'object has been updated since you last saw it'}, status=412)

Unique Representation of an Instance Version

I don't think the actual generation of an object's ETag should be DRF's problem. I've been testing using the epoch time of my object's updated field but I could easily see that needing to be more complex.

I propose that DRF looks up obj.etag by default but it's configurable using the normal CBV flow, e.g. get_etag() and etag_var = 'get_my_objects_etag'.

We'll also need to enforce ETags are retrieved from objects as a string since we're comparing against a header and trying to interpret the type would be painful at best.

Options

• Global setting (as with serializers, etc) to turn the use of ETags on or off.
• Two settings on Views:
  • use_etags (or something similar) - a boolean
  • etag_var - string of a function name that we can getattr on the object in question

[tomchristie]

@ghickman - I'd like to see the behavior for determining ETags and LastModified look similar to the other pluggable classes. Ie. Have a something like:

class MyView(views.APIView):
    cache_lookup_classes = []

Caching signatures ought to deal with both ETags and LastModified, and there's a two different things we want to provide for:

• Determine an etag and/or last modified given an object instance.
• Preemptively determine an etag and/or last modified given the incoming request.

There'd be a BaseCacheLookup, with two method signatures which might look something like:

.object_etag_and_last_modified(self, view, obj)
.preemptive_etag_and_last_modified(self, view, request, *view_kwargs, **view_kwargs)

Given a object return a two-tuple of (etag, last modified), either of which may simply be none.
If the incoming request contains a matching If-Modified-Since or If-None-Match header, then a 304 Not Modified response will be returned. If the incoming response contains a matching If-Match or If-Unmodified-Since then a 412 Precondition Failed response will be returned.

This'd allow a CacheLookupClass matching the implementation you've described, but also other variants.

You could also apply multiple cache lookup classes, at different last modified granularity, for example,
include a GlobalLastModifiedLookup in addition to an ObjectETagLookup. That'd allow the view to preemptively return prior to making any database calls if no writes had been made since the cached copy. (Even really basic policies like that could make a massive difference if you're using server side caching with Varnish)

Does the pluggable class side of this sound reasonable to you?

[ghickman]

I hadn't thought about LastModified as I'm not using it in my current implementation but it definitely makes sense to include given it's purpose.

The pluggable classes sounds like a great idea, especially if we include LastModified and ETag implementations as basic examples. I like the idea that the GET caching would be super easy to turn on by with minimal changes to a project.

I'd prefer to split the etag and last_modified generation into two methods (of those names) that, as you suggested, return None when not implemented. CacheLookup backends could then choose to implement one and/or the other. We could always provide a utility method for convenience (cachable_obj_repr or unique_obj_repr maybe?) that combined the two if you felt it would be useful.

tl;dr yes the pluggable class side sounds reasonable and should give much greater flexibility. I'm happy to start writing the patch for this.

[chibisov]

Hello everyone. If you interested i've implemented different approach for etag support in my extensions library http://chibisov.github.io/drf-extensions/docs/

[tomchristie]

@chibisov Neato. We really should finish off #1019 so we've got somewhere in the docs to link to packages like this.

[xordoquy]

Closing this as #1019 has been closed and @chibisov 's package is listed.

[tomchristie]

This one was milestoned as 3.3 on purpose as I would kind of like us to give some formal direction on this at some point. Not super-concerned if we do choose to leave this closed, but it has been on my internal roadmap.

[mbox]

Unfortunately the default implementation and documentation on the Etag functionality in drf-extensions is simply wrong and dangerously buggy. It changes the Etag if the request changes, not if the response changes. Which is exactly what you want for server-side caching, and exactly what you don't want for an Etag.

[xordoquy]

@mbox the best thing would be to open an issue about this on drf-extensions or if you think it's a DRF "core" issue open on here. Please note that a failing test would be a good start for us to look at the issue.

[pkainz]

@mbox @xordoquy
I just submitted a PR to drf-extensions (chibisov/drf-extensions#171) that allows optimistic concurrency control for manipulating resources via the DRF API. It's using a semantic hash of all object fields and I included a test app for demonstration purposes. It has been tested against DRF>=3.3.1 and django>=1.8 with Python 2.7, 3.4, 3.5.

[auvipy]

thanks

[jozo]

Just a note for the future readers - I've created small package to use conditional decorators from Django together with DRF. So if you are interested:
https://github.com/jozo/django-rest-framework-condition