Merge pull request #174 from chibisov/revert-171-master

Revert "Optimistic concurrency control using ETags"

Author: chibisov

docs/index.html

@@ -8,6 +8,7 @@
 from rest_framework import status
 from rest_framework.permissions import SAFE_METHODS
 from rest_framework.response import Response
+from rest_framework_extensions.exceptions import PreconditionRequiredException
 
 from rest_framework_extensions.utils import prepare_header_name
 from rest_framework_extensions.settings import extensions_api_settings
@@ -29,7 +30,7 @@ def __call__(self, func):
         this = self
 
         @wraps(func, assigned=available_attrs(func))
-        def wrapper(self, request, *args, **kwargs):
+        def inner(self, request, *args, **kwargs):
             return this.process_conditional_request(
                 view_instance=self,
                 view_method=func,
@@ -38,7 +39,7 @@ def wrapper(self, request, *args, **kwargs):
                 kwargs=kwargs,
             )
 
-        return wrapper
+        return inner
 
     def process_conditional_request(self,
                                     view_instance,
@@ -135,21 +136,68 @@ def _get_and_log_precondition_failed_response(self, request):
                        )
         return Response(status=status.HTTP_412_PRECONDITION_FAILED)
 
-#
-# class APIETAGProcessor(ETAGProcessor):
-#     """
-#     This class is responsible for calculating the ETag value given (a list of) model instance(s).
-#
-#     It does not make sense to compute a default ETag here, because the processor would always issue a 304 response,
-#     even if the response was modified meanwhile.
-#     Therefore the `APIETAGProcessor` cannot be used without specifying an `etag_func` as keyword argument.
-#     """
-#     def __init__(self, etag_func=None, rebuild_after_method_evaluation=False):
-#         assert etag_func is not None, ('None-type functions are not allowed for processing API ETags.'
-#                                        'You must specify a proper function to calculate the API ETags '
-#                                        'using the "etag_func" keyword argument.')
-#
-#         super(APIETAGProcessor, self).__init__(etag_func=etag_func,
-#                                                rebuild_after_method_evaluation=rebuild_after_method_evaluation)
+
+class APIETAGProcessor(ETAGProcessor):
+    """
+    This class is responsible for calculating the ETag value given (a list of) model instance(s).
+
+    It does not make sense to compute a default ETag here, because the processor would always issue a 304 response,
+    even if the response was modified meanwhile.
+    Therefore the `APIETAGProcessor` cannot be used without specifying an `etag_func` as keyword argument.
+
+    According to RFC 6585, conditional headers may be enforced for certain services that support conditional
+    requests. For optimistic locking, the server should respond status code 428 including a description on how
+    to resubmit the request successfully, see https://tools.ietf.org/html/rfc6585#section-3.
+    """
+
+    # require a pre-conditional header (e.g. If-Match) for unsafe HTTP methods (RFC 6585)
+    # override this defaults, if required
+    precondition_map = {'PUT': ['If-Match'],
+                        'PATCH': ['If-Match'],
+                        'DELETE': ['If-Match']}
+
+    def __init__(self, etag_func=None, rebuild_after_method_evaluation=False, precondition_map=None):
+        assert etag_func is not None, ('None-type functions are not allowed for processing API ETags.'
+                                       'You must specify a proper function to calculate the API ETags '
+                                       'using the "etag_func" keyword argument.')
+
+        if precondition_map is not None:
+            self.precondition_map = precondition_map
+        assert isinstance(self.precondition_map, dict), ('`precondition_map` must be a dict, where '
+                                                         'the key is the HTTP verb, and the value is a list of '
+                                                         'HTTP headers that must all be present for that request.')
+
+        super(APIETAGProcessor, self).__init__(etag_func=etag_func,
+                                               rebuild_after_method_evaluation=rebuild_after_method_evaluation)
+
+    def get_etags_and_matchers(self, request):
+        """Get the etags from the header and perform a validation against the required preconditions."""
+        # evaluate the preconditions, raises 428 if condition is not met
+        self.evaluate_preconditions(request)
+        # alright, headers are present, extract the values and match the conditions
+        return super(APIETAGProcessor, self).get_etags_and_matchers(request)
+
+    def evaluate_preconditions(self, request):
+        """Evaluate whether the precondition for the request is met."""
+        if request.method.upper() in self.precondition_map.keys():
+            required_headers = self.precondition_map.get(request.method.upper(), [])
+            # check the required headers
+            for header in required_headers:
+                if not request.META.get(prepare_header_name(header)):
+                    # raise an error for each header that does not match
+                    logger.warning('Precondition required: %s', request.path,
+                                   extra={
+                                       'status_code': status.HTTP_428_PRECONDITION_REQUIRED,
+                                       'request': request
+                                   }
+                                   )
+                    # raise an RFC 6585 compliant exception
+                    raise PreconditionRequiredException(detail='Precondition required. This "%s" request '
+                                                               'is required to be conditional. '
+                                                               'Try again using "%s".' % (request.method, header)
+                                                        )
+        return True
+
 
 etag = ETAGProcessor
+api_etag = APIETAGProcessor

docs/index.md

@@ -1,5 +1,5 @@
 # -*- coding: utf-8 -*-
-from rest_framework_extensions.etag.decorators import etag
+from rest_framework_extensions.etag.decorators import etag, api_etag
 from rest_framework_extensions.settings import extensions_api_settings
 
 
@@ -8,11 +8,6 @@ class BaseETAGMixin(object):
     object_etag_func = extensions_api_settings.DEFAULT_OBJECT_ETAG_FUNC
     list_etag_func = extensions_api_settings.DEFAULT_LIST_ETAG_FUNC
 
-    # Default functions to compute the ETags from (a list of) individual API resources.
-    # The default functions *DO NOT* consider the request method or view instance, but only the queried resources.
-    api_object_etag_func = extensions_api_settings.DEFAULT_API_OBJECT_ETAG_FUNC
-    api_list_etag_func = extensions_api_settings.DEFAULT_API_LIST_ETAG_FUNC
-
 
 class ListETAGMixin(BaseETAGMixin):
     @etag(etag_func='list_etag_func')
@@ -50,36 +45,32 @@ class ETAGMixin(RetrieveETAGMixin,
     pass
 
 
-class APIListETAGMixin(BaseETAGMixin):
-    @etag(etag_func='api_list_etag_func')
+class APIBaseETAGMixin(object):
+    # todo: test me. Create generic test like test_etag(view_instance, method, should_rebuild_after_method_evaluation)
+    api_object_etag_func = extensions_api_settings.DEFAULT_API_OBJECT_ETAG_FUNC
+    api_list_etag_func = extensions_api_settings.DEFAULT_API_LIST_ETAG_FUNC
+
+
+class APIListETAGMixin(APIBaseETAGMixin):
+    @api_etag(etag_func='api_list_etag_func')
     def list(self, request, *args, **kwargs):
         return super(APIListETAGMixin, self).list(request, *args, **kwargs)
 
 
-class APIRetrieveETAGMixin(BaseETAGMixin):
-    @etag(etag_func='api_object_etag_func')
+class APIRetrieveETAGMixin(APIBaseETAGMixin):
+    @api_etag(etag_func='api_object_etag_func')
     def retrieve(self, request, *args, **kwargs):
         return super(APIRetrieveETAGMixin, self).retrieve(request, *args, **kwargs)
 
 
-class APIUpdateETAGMixin(BaseETAGMixin):
-    """
-    A mixin that principally *allows* a conditional request for an update (PUT/PATCH) operation.
-    The resource version is optionally provided as `ETag` using the `If-Match` HTTP header.
-    If the header is not present, the operation may succeed anyway.
-    """
-    @etag(etag_func='api_object_etag_func', rebuild_after_method_evaluation=True)
+class APIUpdateETAGMixin(APIBaseETAGMixin):
+    @api_etag(etag_func='api_object_etag_func', rebuild_after_method_evaluation=True)
     def update(self, request, *args, **kwargs):
         return super(APIUpdateETAGMixin, self).update(request, *args, **kwargs)
 
 
-class APIDestroyETAGMixin(BaseETAGMixin):
-    """
-    A mixin that principally *allows* a conditional request for a delete (DELETE) operation.
-    The resource version is optionally provided as `ETag` using the `If-Match` HTTP header.
-    If the header is not present, the operation may succeed anyway.
-    """
-    @etag(etag_func='api_object_etag_func')
+class APIDestroyETAGMixin(APIBaseETAGMixin):
+    @api_etag(etag_func='api_object_etag_func')
     def destroy(self, request, *args, **kwargs):
         return super(APIDestroyETAGMixin, self).destroy(request, *args, **kwargs)
 
@@ -89,14 +80,8 @@ class APIReadOnlyETAGMixin(APIRetrieveETAGMixin,
     pass
 
 
-class APIETAGMixin(APIListETAGMixin,
-                   APIRetrieveETAGMixin,
+class APIETAGMixin(APIRetrieveETAGMixin,
                    APIUpdateETAGMixin,
-                   APIDestroyETAGMixin):
-    """
-    A mixin that principally *allows* optimistic concurrency control (OCC) using `ETag`s for DRF API views and viewsets.
-
-    NB: Update and delete operations are *NOT* required to be conditional! Use `OCCAPIETAGMixin` to enforce
-    the default pre-conditional header checks for update and delete.
-    """
+                   APIDestroyETAGMixin,
+                   APIListETAGMixin):
     pass