ensure concurrent PUT does not raise an IntegrityError (fixes #19384)

petale/api_views.py

@@ -21,6 +21,7 @@ import logging
 from django.db.models.query import Q, F
 from django.http import StreamingHttpResponse, HttpResponse
 from django.conf import settings
+from django.db.transaction import atomic
 
 from rest_framework import status
 from rest_framework.views import APIView
@@ -195,6 +196,7 @@ class PetalAPIView(APIView):
         return response
 
     @logit
+    @atomic
     def put(self, request, partner_name, cut_uuid, petal_name):
         if_match = request.META.get('HTTP_IF_MATCH')
         if_none_match = request.META.get('HTTP_IF_NONE_MATCH')
@@ -208,6 +210,7 @@ class PetalAPIView(APIView):
             petal = self.get_petal(partner_name, cut_uuid, petal_name,
                                    if_match=if_match,
                                    if_none_match=if_none_match)
+            created = False
         except PreconditionException:
             raise ConcurrentAccess
         except NotFound:
@@ -220,7 +223,11 @@ class PetalAPIView(APIView):
             except CUT.DoesNotExist:
                 raise CutNotFound
 
-            petal = Petal(name=petal_name, partner=partner, cut=cut, size=0)
+            petal, created = Petal.objects.get_or_create(
+                name=petal_name, partner=partner, cut=cut,
+                defaults={'size': 0})
+            if not created and if_none_match:
+                raise ConcurrentAccess
         else:
             if if_none_match:
                 raise ConcurrentAccess
@@ -234,7 +241,7 @@ class PetalAPIView(APIView):
 
         petal.check_limits(content_length)
 
-        if not petal.id:
+        if created:
             status_code = status.HTTP_201_CREATED
             old_name = None
         else:

tests/settings.py

@@ -1,2 +1,7 @@
 PETALE_AUTHENTIC_URL = 'https://example.net/idp/'
 PETALE_AUTHENTIC_AUTH = ('foo', 'bar')
+DATABASES = {
+    'default': {
+        'ENGINE': 'django.db.backends.postgresql_psycopg2',
+    }
+}

tests/test_api.py

@@ -1,6 +1,7 @@
 import os
 import json
 from xml.etree import ElementTree as etree
+from multiprocessing.pool import ThreadPool
 
 import pytest
 import mock
@@ -404,3 +405,38 @@ def test_storage_error(app, partner_southpark, cut_kevin_uuid, acl, caplog):
     os.unlink(Petal.objects.get().data.path)
 
     app.get(url, status=500)
+
+
+def test_concurrent_put(app, transactional_db):
+    '''Test concurrent PUT to the same key'''
+    from utils import create_cut, create_partner, create_service, create_acl_record
+
+    uuid = create_cut('a' * 255).uuid
+    southpark = create_partner('southpark', hg=20240, hk=19728)
+    library = create_service('library')
+    create_acl_record(1, southpark, library, 'loans', methods='GET,HEAD,PUT,DELETE')
+
+    app.authorization = ('Basic', ('library', 'library'))
+
+    payload = json.loads(get_tests_file_content('books.json'))
+    url = '/api/%s/%s/loans/' % (southpark.name, uuid)
+
+    pool_count = 40
+    put_count = 60
+
+    pool = ThreadPool(pool_count)
+
+    def f(i):
+        from django.db import connection
+        if i % 2 == 0:
+            headers = {'If-None-Match': '*'}
+        else:
+            headers = {}
+
+        response = app.put_json(url, params=payload, headers=headers, status='*')
+        assert response.status_code in (412, 201, 200)
+        connection.close()
+        return i, response.status_code
+
+    l = pool.map(f, range(put_count))
+    assert len(l) == put_count

tests/utils.py

@@ -27,6 +27,7 @@ def create_service(name, password=None):
     user = User.objects.create(username=name)
     user.set_password(password)
     user.save()
+    return user
 
 
 def get_service(name):

tox.ini

@@ -20,6 +20,7 @@ deps =
   mock
   django-webtest
   djangorestframework>=3.3,<3.4
+  psycopg2
 commands =
   py.test {env:COVERAGE:} {posargs:tests/}
   pylint: ./pylint.sh petale/