From 3a7c82cb65b895455286e6f38540c24df68aa2fe Mon Sep 17 00:00:00 2001 From: Rosie Storey Date: Fri, 22 Mar 2019 16:24:17 -0400 Subject: [PATCH] Use one query instead of two to clean up expired asset reservations --- concordia/tests/test_view.py | 24 ++++++++++++------------ concordia/views.py | 19 +++++-------------- 2 files changed, 17 insertions(+), 26 deletions(-) diff --git a/concordia/tests/test_view.py b/concordia/tests/test_view.py index 0cc31b8d3..4384fee59 100644 --- 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 index c85968451..3ed9f7b3e 100644 --- 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]