From ebb47bb38f9acd3a60eb96baf69ff12f567794b3 Mon Sep 17 00:00:00 2001 From: George Hickman Date: Tue, 12 Mar 2013 20:35:36 -0700 Subject: [PATCH 01/16] Basic implementation of etags Settings and options aren't sorted at all, but the functionality exists in the rest verb methods. --- rest_framework/mixins.py | 14 +++++++++++++- rest_framework/views.py | 9 +++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/rest_framework/mixins.py b/rest_framework/mixins.py index 8e4012049..f2812b7bc 100644 --- a/rest_framework/mixins.py +++ b/rest_framework/mixins.py @@ -100,8 +100,12 @@ class RetrieveModelMixin(object): queryset = self.get_queryset() filtered_queryset = self.filter_queryset(queryset) self.object = self.get_object(filtered_queryset) + if self.use_etags: + if self.get_etag(self.object) == request.META.get('HTTP_IF_NONE_MATCH'): + return Response(status=304) + headers = {'ETag': self.get_etag(self.object)} serializer = self.get_serializer(self.object) - return Response(serializer.data) + return Response(serializer.data, headers=headers) class UpdateModelMixin(object): @@ -110,6 +114,10 @@ class UpdateModelMixin(object): Should be mixed in with `SingleObjectAPIView`. """ def update(self, request, *args, **kwargs): + header_etag = request.META.get('HTTP_IF_MATCH') + if header_etag is None: + return Response({'error': 'IF_MATCH header is required'}, status=400) + partial = kwargs.pop('partial', False) self.object = None try: @@ -121,6 +129,8 @@ class UpdateModelMixin(object): created = True success_status_code = status.HTTP_201_CREATED else: + if self.object.etag != header_etag: + return Response({'error': 'object has been updated since you last saw it'}, status=412) created = False success_status_code = status.HTTP_200_OK @@ -164,5 +174,7 @@ class DestroyModelMixin(object): """ def destroy(self, request, *args, **kwargs): obj = self.get_object() + if self.get_etag(obj) != self.header_etag: + return Response({'error': 'object has been updated since you last saw it'}, status=412) obj.delete() return Response(status=status.HTTP_204_NO_CONTENT) diff --git a/rest_framework/views.py b/rest_framework/views.py index 81cbdcbb2..9e2a0208a 100644 --- a/rest_framework/views.py +++ b/rest_framework/views.py @@ -398,6 +398,13 @@ class APIView(View): else: handler = self.http_method_not_allowed + if self.use_etags and request.method.lower() in ('put', 'delete'): + etag_header = request.META.get('HTTP_IF_MATCH') + if etag_header is None: + return Response({'error': 'IF_MATCH header is required'}, status=400) + else: + self.etag_header = etag_header + response = handler(request, *args, **kwargs) except Exception as exc: @@ -413,3 +420,5 @@ class APIView(View): a less useful default implementation. """ return Response(self.metadata(request), status=status.HTTP_200_OK) + def get_etag(self, obj): + return getattr(obj, self.etag_var) From 53ca0c02b0c399f4cf17a6e0f8a8504e4dde4e91 Mon Sep 17 00:00:00 2001 From: George Hickman Date: Wed, 20 Mar 2013 23:10:24 -0700 Subject: [PATCH 02/16] Add PreconditionFailed exception --- rest_framework/exceptions.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/rest_framework/exceptions.py b/rest_framework/exceptions.py index 0c96ecdd5..9ceb64fe1 100644 --- a/rest_framework/exceptions.py +++ b/rest_framework/exceptions.py @@ -88,6 +88,14 @@ class Throttled(APIException): self.detail = detail or self.default_detail +class PreconditionFailed(APIException): + status_code = status.HTTP_412_PRECONDITION_FAILED + default_detail = 'Object has been updated since you retrieved it.' + + def __init__(self, detail=None): + self.detail = detail or self.default_detail + + class ConfigurationError(Exception): """ Indicates an internal server error. From bceca2364a9fc5e5c11a62e769c28335a322d309 Mon Sep 17 00:00:00 2001 From: George Hickman Date: Wed, 20 Mar 2013 23:14:27 -0700 Subject: [PATCH 03/16] Switch to a view level setting for using etags * Check for existance of `use_etags` as a precondition. * Set etag_header irregardless of the HTTP_IF_MATCH header state. --- rest_framework/views.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/rest_framework/views.py b/rest_framework/views.py index 9e2a0208a..98c7dcc64 100644 --- a/rest_framework/views.py +++ b/rest_framework/views.py @@ -398,12 +398,10 @@ class APIView(View): else: handler = self.http_method_not_allowed - if self.use_etags and request.method.lower() in ('put', 'delete'): - etag_header = request.META.get('HTTP_IF_MATCH') - if etag_header is None: - return Response({'error': 'IF_MATCH header is required'}, status=400) - else: - self.etag_header = etag_header + if getattr(self, 'use_etags', False) and request.method.lower() in ('put', 'delete'): + self.etag_header = request.META.get('HTTP_IF_MATCH') + if self.etag_header is None: + return Response({'detail': 'IF_MATCH header is required'}, status=400, exception=True) response = handler(request, *args, **kwargs) From cfd083d7595bf92b02e4d9cce3ab1a1614abddd6 Mon Sep 17 00:00:00 2001 From: George Hickman Date: Wed, 20 Mar 2013 23:17:30 -0700 Subject: [PATCH 04/16] Tidy up etag precondition checks --- rest_framework/mixins.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/rest_framework/mixins.py b/rest_framework/mixins.py index f2812b7bc..1290dffc5 100644 --- a/rest_framework/mixins.py +++ b/rest_framework/mixins.py @@ -8,6 +8,7 @@ from __future__ import unicode_literals from django.http import Http404 from rest_framework import status +from rest_framework.exceptions import PreconditionFailed from rest_framework.response import Response from rest_framework.request import clone_request @@ -114,10 +115,6 @@ class UpdateModelMixin(object): Should be mixed in with `SingleObjectAPIView`. """ def update(self, request, *args, **kwargs): - header_etag = request.META.get('HTTP_IF_MATCH') - if header_etag is None: - return Response({'error': 'IF_MATCH header is required'}, status=400) - partial = kwargs.pop('partial', False) self.object = None try: @@ -129,8 +126,8 @@ class UpdateModelMixin(object): created = True success_status_code = status.HTTP_201_CREATED else: - if self.object.etag != header_etag: - return Response({'error': 'object has been updated since you last saw it'}, status=412) + if getattr(self, 'use_etags', False) and self.object.etag != self.etag_header: + raise PreconditionFailed created = False success_status_code = status.HTTP_200_OK @@ -174,7 +171,7 @@ class DestroyModelMixin(object): """ def destroy(self, request, *args, **kwargs): obj = self.get_object() - if self.get_etag(obj) != self.header_etag: - return Response({'error': 'object has been updated since you last saw it'}, status=412) + if self.get_etag(obj) != self.etag_header: + raise PreconditionFailed obj.delete() return Response(status=status.HTTP_204_NO_CONTENT) From a337dc3b5f3816987515ccb3ce31a6801aa46520 Mon Sep 17 00:00:00 2001 From: George Hickman Date: Wed, 20 Mar 2013 23:55:13 -0700 Subject: [PATCH 05/16] Add IfMatchMissing exception Needed to make testing easier. --- rest_framework/exceptions.py | 8 ++++++++ rest_framework/views.py | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/rest_framework/exceptions.py b/rest_framework/exceptions.py index 9ceb64fe1..229c8df82 100644 --- a/rest_framework/exceptions.py +++ b/rest_framework/exceptions.py @@ -96,6 +96,14 @@ class PreconditionFailed(APIException): self.detail = detail or self.default_detail +class IfMatchMissing(APIException): + default_detail = 'IF_MATCH header is required' + status_code = 400 + + def __init__(self, detail=None): + self.detail = detail or self.default_detail + + class ConfigurationError(Exception): """ Indicates an internal server error. diff --git a/rest_framework/views.py b/rest_framework/views.py index 98c7dcc64..36f5a92fa 100644 --- a/rest_framework/views.py +++ b/rest_framework/views.py @@ -401,7 +401,7 @@ class APIView(View): if getattr(self, 'use_etags', False) and request.method.lower() in ('put', 'delete'): self.etag_header = request.META.get('HTTP_IF_MATCH') if self.etag_header is None: - return Response({'detail': 'IF_MATCH header is required'}, status=400, exception=True) + raise exceptions.IfMatchMissing response = handler(request, *args, **kwargs) From 61f6fe2b8158c955000277915d2af2acffae0646 Mon Sep 17 00:00:00 2001 From: George Hickman Date: Thu, 21 Mar 2013 00:03:41 -0700 Subject: [PATCH 06/16] Check `use_etags` before trying to set ETag header --- rest_framework/mixins.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/rest_framework/mixins.py b/rest_framework/mixins.py index 1290dffc5..ada821754 100644 --- a/rest_framework/mixins.py +++ b/rest_framework/mixins.py @@ -101,10 +101,11 @@ class RetrieveModelMixin(object): queryset = self.get_queryset() filtered_queryset = self.filter_queryset(queryset) self.object = self.get_object(filtered_queryset) - if self.use_etags: + headers = {} + if getattr(self, 'use_etags', False): if self.get_etag(self.object) == request.META.get('HTTP_IF_NONE_MATCH'): return Response(status=304) - headers = {'ETag': self.get_etag(self.object)} + headers.update({'ETag': self.get_etag(self.object)}) serializer = self.get_serializer(self.object) return Response(serializer.data, headers=headers) From 00778e3d2425bb6b84fcab6bc36676feafcfb01c Mon Sep 17 00:00:00 2001 From: George Hickman Date: Thu, 21 Mar 2013 00:23:31 -0700 Subject: [PATCH 07/16] Use built-in status code variables --- rest_framework/exceptions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rest_framework/exceptions.py b/rest_framework/exceptions.py index 229c8df82..89dc88fe3 100644 --- a/rest_framework/exceptions.py +++ b/rest_framework/exceptions.py @@ -98,7 +98,7 @@ class PreconditionFailed(APIException): class IfMatchMissing(APIException): default_detail = 'IF_MATCH header is required' - status_code = 400 + status_code = status.HTTP_400_BAD_REQUEST def __init__(self, detail=None): self.detail = detail or self.default_detail From 4f43804d42d521753ce0dee55941dd1e7f22f755 Mon Sep 17 00:00:00 2001 From: George Hickman Date: Mon, 1 Apr 2013 16:15:05 +0100 Subject: [PATCH 08/16] Move cache lookup code to a pluggable backend ETagCacheLookup is the first example implementation. --- rest_framework/cache_lookups.py | 42 +++++++++++++++++++++++++++++++ rest_framework/mixins.py | 15 ++++------- rest_framework/settings.py | 2 ++ rest_framework/views.py | 44 ++++++++++++++++++++++++++++----- 4 files changed, 87 insertions(+), 16 deletions(-) create mode 100644 rest_framework/cache_lookups.py diff --git a/rest_framework/cache_lookups.py b/rest_framework/cache_lookups.py new file mode 100644 index 000000000..4d51c426e --- /dev/null +++ b/rest_framework/cache_lookups.py @@ -0,0 +1,42 @@ +""" +Provides a set of pluggable cache policies. +""" +from django.core.cache import cache + + +class BaseCacheLookup(object): + def get_header(self, obj): + return {} + + def resource_unchanged(self, request): + """ + Return `False` if resource has changed, `True` otherwise. + """ + return False + + +class ETagCacheLookup(BaseCacheLookup): + """ + """ + etag_variable = 'etag' + + @staticmethod + def get_cache_key(cls, pk): + class_name = cls.__class__.__name__ + return 'etag-{}-{}'.format(class_name, pk) + + def get_etag(self, obj): + return getattr(obj, self.etag_variable) + + def get_header(self, obj): + key = self.get_cache_key(obj, 'pk') + etag = self.get_etag(obj) + cache.set(key, etag) + return {'ETag': etag} + + def resource_unchanged(self, request, key): + etag = cache.get(key) + header = request.META.get('HTTP_IF_NONE_MATCH') + if etag is not None and etag == header: + return True + return False diff --git a/rest_framework/mixins.py b/rest_framework/mixins.py index ada821754..a0fc51e35 100644 --- a/rest_framework/mixins.py +++ b/rest_framework/mixins.py @@ -8,7 +8,6 @@ from __future__ import unicode_literals from django.http import Http404 from rest_framework import status -from rest_framework.exceptions import PreconditionFailed from rest_framework.response import Response from rest_framework.request import clone_request @@ -101,11 +100,9 @@ class RetrieveModelMixin(object): queryset = self.get_queryset() filtered_queryset = self.filter_queryset(queryset) self.object = self.get_object(filtered_queryset) - headers = {} - if getattr(self, 'use_etags', False): - if self.get_etag(self.object) == request.META.get('HTTP_IF_NONE_MATCH'): - return Response(status=304) - headers.update({'ETag': self.get_etag(self.object)}) + + headers = self.get_cache_lookup_headers(self.object) + serializer = self.get_serializer(self.object) return Response(serializer.data, headers=headers) @@ -127,8 +124,7 @@ class UpdateModelMixin(object): created = True success_status_code = status.HTTP_201_CREATED else: - if getattr(self, 'use_etags', False) and self.object.etag != self.etag_header: - raise PreconditionFailed + self.cache_precondition_check(self.object, request) created = False success_status_code = status.HTTP_200_OK @@ -172,7 +168,6 @@ class DestroyModelMixin(object): """ def destroy(self, request, *args, **kwargs): obj = self.get_object() - if self.get_etag(obj) != self.etag_header: - raise PreconditionFailed + self.cache_precondition_check(obj, request) obj.delete() return Response(status=status.HTTP_204_NO_CONTENT) diff --git a/rest_framework/settings.py b/rest_framework/settings.py index eede0c5a0..be687b86a 100644 --- a/rest_framework/settings.py +++ b/rest_framework/settings.py @@ -47,6 +47,8 @@ DEFAULTS = { ), 'DEFAULT_THROTTLE_CLASSES': ( ), + 'DEFAULT_CACHE_LOOKUP_CLASSES': ( + ), 'DEFAULT_CONTENT_NEGOTIATION_CLASS': 'rest_framework.negotiation.DefaultContentNegotiation', diff --git a/rest_framework/views.py b/rest_framework/views.py index 36f5a92fa..8e38d7e6a 100644 --- a/rest_framework/views.py +++ b/rest_framework/views.py @@ -60,6 +60,7 @@ class APIView(View): throttle_classes = api_settings.DEFAULT_THROTTLE_CLASSES permission_classes = api_settings.DEFAULT_PERMISSION_CLASSES content_negotiation_class = api_settings.DEFAULT_CONTENT_NEGOTIATION_CLASS + cache_lookup_classes = api_settings.DEFAULT_CACHE_LOOKUP_CLASSES @classmethod def as_view(cls, **initkwargs): @@ -241,6 +242,12 @@ class APIView(View): self._negotiator = self.content_negotiation_class() return self._negotiator + def get_cache_lookups(self): + """ + Instantiates and returns the list of cache lookups that this view requires. + """ + return [cache_lookup() for cache_lookup in self.cache_lookup_classes] + # API policy implementation methods def perform_content_negotiation(self, request, force=False): @@ -294,6 +301,33 @@ class APIView(View): if not throttle.allow_request(request, self): self.throttled(request, throttle.wait()) + def check_preemptive_cache(self, request): + for cache_lookup in self.get_cache_lookups(): + cache_key = cache_lookup.get_cache_key(self.model, self.kwargs['pk']) + if cache_lookup.resource_unchanged(request, cache_key): + return Response(status=304) + + def get_cache_lookup_headers(self, obj): + headers = {} + for cache_lookup in self.get_cache_lookups(): + headers.update(cache_lookup.get_header(obj)) + return headers + + def check_update_validity(self, request): + """ + """ + # TODO add setting to cover + # * raise IfMatchMissing when it's missing (if it's there, carry on) + # * continue regardless + if request.META.get('HTTP_IF_MATCH') is None: + raise exceptions.IfMatchMissing + + def cache_precondition_check(self, obj, request): + header = request.META.get('HTTP_IF_MATCH') + for cache_lookup in self.get_cache_lookups(): + if cache_lookup.get_etag(obj) != header: + raise exceptions.PreconditionFailed + # Dispatch methods def initialize_request(self, request, *args, **kargs): @@ -318,6 +352,7 @@ class APIView(View): self.perform_authentication(request) self.check_permissions(request) self.check_throttles(request) + self.check_preemptive_cache(request) # Perform content negotiation and store the accepted info on the request neg = self.perform_content_negotiation(request) @@ -398,10 +433,9 @@ class APIView(View): else: handler = self.http_method_not_allowed - if getattr(self, 'use_etags', False) and request.method.lower() in ('put', 'delete'): - self.etag_header = request.META.get('HTTP_IF_MATCH') - if self.etag_header is None: - raise exceptions.IfMatchMissing + if request.method.lower() in ('put', 'delete'): + # FIXME this method name isn't obvious + self.check_update_validity(request) response = handler(request, *args, **kwargs) @@ -418,5 +452,3 @@ class APIView(View): a less useful default implementation. """ return Response(self.metadata(request), status=status.HTTP_200_OK) - def get_etag(self, obj): - return getattr(obj, self.etag_var) From 65b16071c480ffd5761ce9e0c1656428c339940e Mon Sep 17 00:00:00 2001 From: George Hickman Date: Tue, 2 Apr 2013 10:43:44 +0100 Subject: [PATCH 09/16] Get cache keys from a model or instance --- rest_framework/cache_lookups.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/rest_framework/cache_lookups.py b/rest_framework/cache_lookups.py index 4d51c426e..628e3c748 100644 --- a/rest_framework/cache_lookups.py +++ b/rest_framework/cache_lookups.py @@ -22,7 +22,10 @@ class ETagCacheLookup(BaseCacheLookup): @staticmethod def get_cache_key(cls, pk): - class_name = cls.__class__.__name__ + try: + class_name = cls.__name__ # class + except AttributeError: + class_name = cls.__class__.__name__ # instance return 'etag-{}-{}'.format(class_name, pk) def get_etag(self, obj): From cede7d86a3db9cf45c29c2f5a8a4d1f8e22e7bbd Mon Sep 17 00:00:00 2001 From: George Hickman Date: Tue, 2 Apr 2013 10:45:47 +0100 Subject: [PATCH 10/16] Refactor precondition check into the cache lookup backends --- rest_framework/cache_lookups.py | 5 +++++ rest_framework/views.py | 4 +--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/rest_framework/cache_lookups.py b/rest_framework/cache_lookups.py index 628e3c748..2d9271b0c 100644 --- a/rest_framework/cache_lookups.py +++ b/rest_framework/cache_lookups.py @@ -2,6 +2,7 @@ Provides a set of pluggable cache policies. """ from django.core.cache import cache +from rest_framework.exceptions import PreconditionFailed class BaseCacheLookup(object): @@ -37,6 +38,10 @@ class ETagCacheLookup(BaseCacheLookup): cache.set(key, etag) return {'ETag': etag} + def precondition_check(self, obj, request): + if self.get_etag(obj) != request.META.get('HTTP_IF_NONE_MATCH'): + raise PreconditionFailed + def resource_unchanged(self, request, key): etag = cache.get(key) header = request.META.get('HTTP_IF_NONE_MATCH') diff --git a/rest_framework/views.py b/rest_framework/views.py index 8e38d7e6a..0c780be74 100644 --- a/rest_framework/views.py +++ b/rest_framework/views.py @@ -323,10 +323,8 @@ class APIView(View): raise exceptions.IfMatchMissing def cache_precondition_check(self, obj, request): - header = request.META.get('HTTP_IF_MATCH') for cache_lookup in self.get_cache_lookups(): - if cache_lookup.get_etag(obj) != header: - raise exceptions.PreconditionFailed + cache_lookup.precondition_check(obj, request) # Dispatch methods From 5e5de9c499b95fdfd32ec20853bb4732c4c71d31 Mon Sep 17 00:00:00 2001 From: George Hickman Date: Tue, 2 Apr 2013 10:46:48 +0100 Subject: [PATCH 11/16] Only preemptively check the cache on GET requests --- rest_framework/mixins.py | 4 ++++ rest_framework/views.py | 1 - 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/rest_framework/mixins.py b/rest_framework/mixins.py index a0fc51e35..e5de46abb 100644 --- a/rest_framework/mixins.py +++ b/rest_framework/mixins.py @@ -97,6 +97,10 @@ class RetrieveModelMixin(object): Should be mixed in with `SingleObjectAPIView`. """ def retrieve(self, request, *args, **kwargs): + cached_object = self.check_preemptive_cache(request) + if cached_object: + return cached_object + queryset = self.get_queryset() filtered_queryset = self.filter_queryset(queryset) self.object = self.get_object(filtered_queryset) diff --git a/rest_framework/views.py b/rest_framework/views.py index 0c780be74..4caec27c0 100644 --- a/rest_framework/views.py +++ b/rest_framework/views.py @@ -350,7 +350,6 @@ class APIView(View): self.perform_authentication(request) self.check_permissions(request) self.check_throttles(request) - self.check_preemptive_cache(request) # Perform content negotiation and store the accepted info on the request neg = self.perform_content_negotiation(request) From 70199d9755223a95c7d68adf8f943309f2ee7e58 Mon Sep 17 00:00:00 2001 From: George Hickman Date: Tue, 2 Apr 2013 10:50:49 +0100 Subject: [PATCH 12/16] Tidy up header usage Explicity define request and response headers. --- rest_framework/cache_lookups.py | 10 +++++++--- rest_framework/mixins.py | 2 +- rest_framework/views.py | 4 ++-- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/rest_framework/cache_lookups.py b/rest_framework/cache_lookups.py index 2d9271b0c..2973d5029 100644 --- a/rest_framework/cache_lookups.py +++ b/rest_framework/cache_lookups.py @@ -20,6 +20,7 @@ class ETagCacheLookup(BaseCacheLookup): """ """ etag_variable = 'etag' + request_header = 'HTTP_IF_NONE_MATCH' @staticmethod def get_cache_key(cls, pk): @@ -32,19 +33,22 @@ class ETagCacheLookup(BaseCacheLookup): def get_etag(self, obj): return getattr(obj, self.etag_variable) - def get_header(self, obj): + def get_request_header(self): + return self.request_header + + def get_response_header(self, obj): key = self.get_cache_key(obj, 'pk') etag = self.get_etag(obj) cache.set(key, etag) return {'ETag': etag} def precondition_check(self, obj, request): - if self.get_etag(obj) != request.META.get('HTTP_IF_NONE_MATCH'): + if self.get_etag(obj) != request.META.get(self.get_request_header()): raise PreconditionFailed def resource_unchanged(self, request, key): etag = cache.get(key) - header = request.META.get('HTTP_IF_NONE_MATCH') + header = request.META.get(self.get_request_header()) if etag is not None and etag == header: return True return False diff --git a/rest_framework/mixins.py b/rest_framework/mixins.py index e5de46abb..29d0f03dc 100644 --- a/rest_framework/mixins.py +++ b/rest_framework/mixins.py @@ -105,7 +105,7 @@ class RetrieveModelMixin(object): filtered_queryset = self.filter_queryset(queryset) self.object = self.get_object(filtered_queryset) - headers = self.get_cache_lookup_headers(self.object) + headers = self.get_cache_lookup_response_headers(self.object) serializer = self.get_serializer(self.object) return Response(serializer.data, headers=headers) diff --git a/rest_framework/views.py b/rest_framework/views.py index 4caec27c0..837d20912 100644 --- a/rest_framework/views.py +++ b/rest_framework/views.py @@ -307,10 +307,10 @@ class APIView(View): if cache_lookup.resource_unchanged(request, cache_key): return Response(status=304) - def get_cache_lookup_headers(self, obj): + def get_cache_lookup_response_headers(self, obj): headers = {} for cache_lookup in self.get_cache_lookups(): - headers.update(cache_lookup.get_header(obj)) + headers.update(cache_lookup.get_response_header(obj)) return headers def check_update_validity(self, request): From 879302c815b22a9aa6fbe7b6263daf1c1e66d4ab Mon Sep 17 00:00:00 2001 From: George Hickman Date: Tue, 2 Apr 2013 12:02:26 +0100 Subject: [PATCH 13/16] Add update header to ETagCacheLookup Need to use the IF_MATCH header to check the validity of updating a model in PUT and DELETE methods. --- rest_framework/cache_lookups.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/rest_framework/cache_lookups.py b/rest_framework/cache_lookups.py index 2973d5029..eb314ca42 100644 --- a/rest_framework/cache_lookups.py +++ b/rest_framework/cache_lookups.py @@ -21,6 +21,7 @@ class ETagCacheLookup(BaseCacheLookup): """ etag_variable = 'etag' request_header = 'HTTP_IF_NONE_MATCH' + update_header = 'HTTP_IF_MATCH' @staticmethod def get_cache_key(cls, pk): @@ -36,6 +37,9 @@ class ETagCacheLookup(BaseCacheLookup): def get_request_header(self): return self.request_header + def get_update_header(self): + return self.update_header + def get_response_header(self, obj): key = self.get_cache_key(obj, 'pk') etag = self.get_etag(obj) @@ -43,7 +47,7 @@ class ETagCacheLookup(BaseCacheLookup): return {'ETag': etag} def precondition_check(self, obj, request): - if self.get_etag(obj) != request.META.get(self.get_request_header()): + if self.get_etag(obj) != request.META.get(self.get_update_header()): raise PreconditionFailed def resource_unchanged(self, request, key): From 2139c109abd327d68bef4823128b4ca093fe6623 Mon Sep 17 00:00:00 2001 From: George Hickman Date: Tue, 2 Apr 2013 12:28:58 +0100 Subject: [PATCH 14/16] Dont' require IF_MATCH header The HTTP RFC doesn't require the IF_MATCH header and it's language on what to do when it's not included isn't absolute so removing this check. The implementing codebase can easily add this by overriding precondition_check in ETagCacheLookup if necessary. --- rest_framework/exceptions.py | 8 -------- rest_framework/views.py | 13 ------------- 2 files changed, 21 deletions(-) diff --git a/rest_framework/exceptions.py b/rest_framework/exceptions.py index 89dc88fe3..9ceb64fe1 100644 --- a/rest_framework/exceptions.py +++ b/rest_framework/exceptions.py @@ -96,14 +96,6 @@ class PreconditionFailed(APIException): self.detail = detail or self.default_detail -class IfMatchMissing(APIException): - default_detail = 'IF_MATCH header is required' - status_code = status.HTTP_400_BAD_REQUEST - - def __init__(self, detail=None): - self.detail = detail or self.default_detail - - class ConfigurationError(Exception): """ Indicates an internal server error. diff --git a/rest_framework/views.py b/rest_framework/views.py index 837d20912..59b15e998 100644 --- a/rest_framework/views.py +++ b/rest_framework/views.py @@ -313,15 +313,6 @@ class APIView(View): headers.update(cache_lookup.get_response_header(obj)) return headers - def check_update_validity(self, request): - """ - """ - # TODO add setting to cover - # * raise IfMatchMissing when it's missing (if it's there, carry on) - # * continue regardless - if request.META.get('HTTP_IF_MATCH') is None: - raise exceptions.IfMatchMissing - def cache_precondition_check(self, obj, request): for cache_lookup in self.get_cache_lookups(): cache_lookup.precondition_check(obj, request) @@ -430,10 +421,6 @@ class APIView(View): else: handler = self.http_method_not_allowed - if request.method.lower() in ('put', 'delete'): - # FIXME this method name isn't obvious - self.check_update_validity(request) - response = handler(request, *args, **kwargs) except Exception as exc: From e5d66d3ef89b354d37e5efe68281fd4d35f61969 Mon Sep 17 00:00:00 2001 From: George Hickman Date: Tue, 2 Apr 2013 14:21:19 +0100 Subject: [PATCH 15/16] Add public facing methods to BaseCacheLookup There seems little point in using BaseCacheLookup by default so it can act as an example to implementing classes. --- rest_framework/cache_lookups.py | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/rest_framework/cache_lookups.py b/rest_framework/cache_lookups.py index eb314ca42..8ee96e197 100644 --- a/rest_framework/cache_lookups.py +++ b/rest_framework/cache_lookups.py @@ -2,18 +2,25 @@ Provides a set of pluggable cache policies. """ from django.core.cache import cache +from django.core.exceptions import ImproperlyConfigured from rest_framework.exceptions import PreconditionFailed class BaseCacheLookup(object): - def get_header(self, obj): - return {} + def get_request_header(self): + raise ImproperlyConfigured('Subclass must implement `get_request_header`.') + + def get_update_header(self): + raise ImproperlyConfigured('Subclass must implement `get_update_header`.') + + def get_response_header(self, obj): + raise ImproperlyConfigured('Subclass must impelement `get_response_header`.') + + def precondition_check(self, obj, request): + raise ImproperlyConfigured('Subclass must implement `precondition_check`.') def resource_unchanged(self, request): - """ - Return `False` if resource has changed, `True` otherwise. - """ - return False + raise ImproperlyConfigured('Subclass must implement `resource_unchanged`.') class ETagCacheLookup(BaseCacheLookup): From 077c4a69acbec6aef0e1597a58a38aee87c93e04 Mon Sep 17 00:00:00 2001 From: George Hickman Date: Tue, 2 Apr 2013 14:30:11 +0100 Subject: [PATCH 16/16] Follow naming convention more closely --- rest_framework/{cache_lookups.py => caches.py} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename rest_framework/{cache_lookups.py => caches.py} (100%) diff --git a/rest_framework/cache_lookups.py b/rest_framework/caches.py similarity index 100% rename from rest_framework/cache_lookups.py rename to rest_framework/caches.py