Delete digest nonce when blocking timeout exceeded

onadata/apps/django_digest_backends/cache.py

@@ -1,16 +1,19 @@
 from django.core.cache import caches
 from django_digest.utils import get_setting
+from redis.exceptions import LockError
 
-
-DIGEST_NONCE_CACHE_NAME = get_setting('DIGEST_NONCE_CACHE_NAME', 'default')
-NONCE_TIMEOUT = get_setting('DIGEST_NONCE_TIMEOUT_IN_SECONDS', 5 * 60)
 NONCE_NO_COUNT = ''  # Needs to be something other than None to determine not set vs set to null
 
 
 class RedisCacheNonceStorage():
+    _blocking_timeout = 30
+
     def _get_cache(self):
         # Dynamic fetching of cache is necessary to work with override_settings
-        return caches[DIGEST_NONCE_CACHE_NAME]
+        return caches[get_setting('DIGEST_NONCE_CACHE_NAME', 'default')]
+
+    def _get_timeout(self):
+        return get_setting('DIGEST_NONCE_TIMEOUT_IN_SECONDS', 5 * 60)
 
     def _generate_cache_key(self, user, nonce):
         return f'user_nonce_{user}_{nonce}'
@@ -28,19 +31,23 @@ def update_existing_nonce(self, user, nonce, nonce_count):
             existing = cache.get(cache_key)
             if existing is None:
                 return False
-            cache.set(cache_key, NONCE_NO_COUNT, NONCE_TIMEOUT)
+            cache.set(cache_key, NONCE_NO_COUNT, self._get_timeout())
         else:
-            with cache.lock(
-                f'user_nonce_lock_{user}_{nonce}',
-                timeout=NONCE_TIMEOUT,
-                blocking_timeout=30
-            ):
-                existing = cache.get(cache_key)
-                if existing is None:
-                    return False
-                if nonce_count <= existing:
-                    return False
-                cache.set(cache_key, nonce_count, NONCE_TIMEOUT)
+            try:
+                with cache.lock(
+                    f'user_nonce_lock_{user}_{nonce}',
+                    timeout=self._get_timeout(),
+                    blocking_timeout=self._blocking_timeout
+                ):
+                    existing = cache.get(cache_key)
+                    if existing is None:
+                        return False
+                    if nonce_count <= existing:
+                        return False
+                    cache.set(cache_key, nonce_count, self._get_timeout())
+            except LockError:
+                cache.delete(cache_key)
+                return False
         return True
 
     def store_nonce(self, user, nonce, nonce_count):
@@ -51,4 +58,4 @@ def store_nonce(self, user, nonce, nonce_count):
             nonce_count = NONCE_NO_COUNT
         cache = self._get_cache()
         cache_key = self._generate_cache_key(user, nonce)
-        return cache.set(cache_key, nonce_count, NONCE_TIMEOUT)
+        return cache.set(cache_key, nonce_count, self._get_timeout())

onadata/apps/django_digest_backends/tests.py

@@ -1,5 +1,6 @@
 from django.core.cache import caches
 from django.test import TestCase
+
 from .cache import RedisCacheNonceStorage
 
 
@@ -30,12 +31,13 @@ def test_update_count(self):
         self.assertFalse(self.storage.update_existing_nonce(self.test_user, 'testnonce', 2))
         self.assertTrue(self.storage.update_existing_nonce(self.test_user, 'testnonce', 3))
 
-    def xtest_nonce_lock(self):
+    def test_nonce_lock(self):
         """
-        Prove the lock halts execution, intended to be manually run
+        Lock timeout should be considered False and delete the nonce
         """
         nonce = 'testnonce'
+        self.storage._blocking_timeout = 0.1
         self.storage.store_nonce(self.test_user, nonce, 1)
         with self.cache.lock(f'user_nonce_lock_{self.test_user}_{nonce}'):
-            self.storage.update_existing_nonce(self.test_user, nonce, 2)
-        self.assertTrue()
+            self.assertFalse(self.storage.update_existing_nonce(self.test_user, nonce, 2))
+        self.assertFalse(self.cache.get(self.storage._generate_cache_key(self.test_user, nonce)))