From 23b2bb3c0a0be5c24f3f5b1555fb733d8e161df0 Mon Sep 17 00:00:00 2001
From: Tom Hacohen <tom@stosb.com>
Date: Thu, 11 Jun 2020 11:17:01 +0300
Subject: [PATCH] Batch: refactor code and allow passing deps to check against.

---
 django_etesync/views.py | 93 +++++++++++++++--------------------------
 1 file changed, 34 insertions(+), 59 deletions(-)

diff --git a/django_etesync/views.py b/django_etesync/views.py
index f23971d..1d29ae1 100644
--- a/django_etesync/views.py
+++ b/django_etesync/views.py
@@ -316,73 +316,48 @@ class CollectionItemViewSet(BaseViewSet):
 
     @action_decorator(detail=False, methods=['POST'])
     def batch(self, request, collection_uid=None):
-        stoken = request.GET.get('stoken', None)
-        collection_object = get_object_or_404(self.get_collection_queryset(Collection.objects), uid=collection_uid)
-
-        if stoken is not None and stoken != collection_object.stoken:
-            content = {'code': 'stale_stoken', 'detail': 'Stoken is too old'}
-            return Response(content, status=status.HTTP_400_BAD_REQUEST)
-
-        items = request.data.get('items')
-        serializer = self.get_serializer(data=items, many=True)
-
-        if serializer.is_valid():
-            try:
-                with transaction.atomic():
-                    items = serializer.save(collection=collection_object)
-            except IntegrityError:
-                # FIXME: should return the items with a bad token (including deps) so we don't have to fetch them after
-                content = {'code': 'integrity_error'}
-                return Response(content, status=status.HTTP_400_BAD_REQUEST)
-
-            ret = {
-            }
-            return Response(ret, status=status.HTTP_200_OK)
-
-        return Response(
-            {
-                "items": serializer.errors,
-            },
-            status=status.HTTP_400_BAD_REQUEST)
+        return self.transaction(request, collection_uid, validate_etag=False)
 
     @action_decorator(detail=False, methods=['POST'])
-    def transaction(self, request, collection_uid=None):
+    def transaction(self, request, collection_uid=None, validate_etag=True):
         stoken = request.GET.get('stoken', None)
-        collection_object = get_object_or_404(self.get_collection_queryset(Collection.objects), uid=collection_uid)
-
-        if stoken is not None and stoken != collection_object.stoken:
-            content = {'code': 'stale_stoken', 'detail': 'Stoken is too old'}
-            return Response(content, status=status.HTTP_400_BAD_REQUEST)
+        with transaction.atomic():  # We need this for locking on the collection object
+            collection_object = get_object_or_404(
+                self.get_collection_queryset(Collection.objects).select_for_update(),  # Lock writes on the collection
+                uid=collection_uid)
 
-        items = request.data.get('items')
-        deps = request.data.get('deps', None)
-        # FIXME: It should just be one serializer
-        context = self.get_serializer_context()
-        context.update({'validate_etag': True})
-        serializer = self.get_serializer_class()(data=items, context=context, many=True)
-        deps_serializer = CollectionItemDepSerializer(data=deps, context=context, many=True)
+            if stoken is not None and stoken != collection_object.stoken:
+                content = {'code': 'stale_stoken', 'detail': 'Stoken is too old'}
+                return Response(content, status=status.HTTP_400_BAD_REQUEST)
 
-        ser_valid = serializer.is_valid()
-        deps_ser_valid = (deps is None or deps_serializer.is_valid())
-        if ser_valid and deps_ser_valid:
-            try:
-                with transaction.atomic():
+            items = request.data.get('items')
+            deps = request.data.get('deps', None)
+            # FIXME: It should just be one serializer
+            context = self.get_serializer_context()
+            context.update({'validate_etag': validate_etag})
+            serializer = self.get_serializer_class()(data=items, context=context, many=True)
+            deps_serializer = CollectionItemDepSerializer(data=deps, context=context, many=True)
+
+            ser_valid = serializer.is_valid()
+            deps_ser_valid = (deps is None or deps_serializer.is_valid())
+            if ser_valid and deps_ser_valid:
+                try:
                     items = serializer.save(collection=collection_object)
-            except IntegrityError:
-                # FIXME: should return the items with a bad token (including deps) so we don't have to fetch them after
-                content = {'code': 'integrity_error'}
-                return Response(content, status=status.HTTP_400_BAD_REQUEST)
+                except IntegrityError:
+                    # FIXME: return the items with a bad token (including deps) so we don't have to fetch them after
+                    content = {'code': 'integrity_error'}
+                    return Response(content, status=status.HTTP_400_BAD_REQUEST)
 
-            ret = {
-            }
-            return Response(ret, status=status.HTTP_200_OK)
+                ret = {
+                }
+                return Response(ret, status=status.HTTP_200_OK)
 
-        return Response(
-            {
-                "items": serializer.errors,
-                "deps": deps_serializer.errors if deps is not None else [],
-            },
-            status=status.HTTP_400_BAD_REQUEST)
+            return Response(
+                {
+                    "items": serializer.errors,
+                    "deps": deps_serializer.errors if deps is not None else [],
+                },
+                status=status.HTTP_400_BAD_REQUEST)
 
 
 class CollectionItemChunkViewSet(viewsets.ViewSet):