Use one query instead of two to clean up expired asset reservations

LibraryOfCongress · Mar 22, 2019 · 3a7c82c · 3a7c82c
1 parent 9a92099
commit 3a7c82c
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 26 deletions.
diff --git a/concordia/tests/test_view.py b/concordia/tests/test_view.py
@@ -416,8 +416,8 @@ def _asset_reservation_test_payload(self, user_id):
         asset = create_asset()
 
         # Acquire the reservation:
-        # 1 auth query + 2 expiry + 1 acquire + 1 feature flag check
-        with self.assertNumQueries(5):
+        # 1 auth query + 1 expiry + 1 acquire + 1 feature flag check
+        with self.assertNumQueries(4):
             resp = self.client.post(reverse("reserve-asset", args=(asset.pk,)))
         self.assertEqual(204, resp.status_code)
 
@@ -427,8 +427,8 @@ def _asset_reservation_test_payload(self, user_id):
 
         # Confirm that an update did not change the pk when it updated the timestamp:
 
-        # 1 auth query + 2 expiry + 1 acquire + 1 feature flag check
-        with self.assertNumQueries(5):
+        # 1 auth query + 1 expiry + 1 acquire + 1 feature flag check
+        with self.assertNumQueries(4):
             resp = self.client.post(reverse("reserve-asset", args=(asset.pk,)))
         self.assertEqual(204, resp.status_code)
 
@@ -441,8 +441,8 @@ def _asset_reservation_test_payload(self, user_id):
 
         # Release the reservation now that we're done:
 
-        # 4 = 1 auth query + 2 expiry + 1 delete + 1 feature flag check
-        with self.assertNumQueries(5):
+        # 4 = 1 auth query + 1 expiry + 1 delete + 1 feature flag check
+        with self.assertNumQueries(4):
             resp = self.client.post(
                 reverse("reserve-asset", args=(asset.pk,)), data={"release": True}
             )
@@ -462,16 +462,16 @@ def test_asset_reservation_competition(self):
 
         # 5 queries =
         # 1 auth query + 1 anonymous user creation
-        # + 2 expiry + 1 acquire + 1 feature flag
-        with self.assertNumQueries(6):
+        # + 1 expiry + 1 acquire + 1 feature flag
+        with self.assertNumQueries(5):
             resp = self.client.post(reverse("reserve-asset", args=(asset.pk,)))
         self.assertEqual(204, resp.status_code)
         self.assertEqual(1, AssetTranscriptionReservation.objects.count())
 
         self.login_user()
 
-        # 1 auth query + 2 expiry + 1 acquire + 1 feature flag check
-        with self.assertNumQueries(5):
+        # 1 auth query + 1 expiry + 1 acquire + 1 feature flag check
+        with self.assertNumQueries(4):
             resp = self.client.post(reverse("reserve-asset", args=(asset.pk,)))
         self.assertEqual(409, resp.status_code)
         self.assertEqual(1, AssetTranscriptionReservation.objects.count())
@@ -495,8 +495,8 @@ def test_asset_reservation_expiration(self):
 
         self.login_user()
 
-        # 1 auth query + 2 expiry + 1 acquire + 1 feature flag check
-        with self.assertNumQueries(5):
+        # 1 auth query + 1 expiry + 1 acquire + 1 feature flag check
+        with self.assertNumQueries(4):
             resp = self.client.post(reverse("reserve-asset", args=(asset.pk,)))
         self.assertEqual(204, resp.status_code)
 

diff --git a/concordia/views.py b/concordia/views.py
@@ -1134,31 +1134,22 @@ def reserve_asset(request, *, asset_pk):
         timedelta(seconds=2 * settings.TRANSCRIPTION_RESERVATION_SECONDS)
     )
 
+    reservations = []
+
     with connection.cursor() as cursor:
         rows_to_release = cursor.execute(
             (
-                "SELECT asset_id as asset_pk, user_id as user_pk "
-                "FROM concordia_assettranscriptionreservation "
-                "WHERE updated_on < %s"
+                "DELETE FROM concordia_assettranscriptionreservation "
+                "WHERE updated_on < %s "
+                "RETURNING asset_id as asset_pk, user_id as user_pk"
             ),
             [cutoff],
         )
 
-    reservations = []
-
     if rows_to_release:
         for row in rows_to_release.fetchone():
             reservations += (row["asset_pk"], row["user_pk"])
 
-    with connection.cursor() as cursor:
-        cursor.execute(
-            (
-                "DELETE FROM concordia_assettranscriptionreservation "
-                "WHERE updated_on < %s"
-            ),
-            [cutoff],
-        )
-
     for reservation in reservations:
         reservation_released.send(
             sender="reserve_asset", asset_pk=reservation[0], user_pk=reservation[1]