Use atomic increment for nr_times_visited

Author: benkonrath

regex_redirects/middleware.py

@@ -1,9 +1,14 @@
 import re
+import logging
 
 from django import http
 from django.conf import settings
 from django.core.cache import cache
 from django.core.exceptions import ImproperlyConfigured
+from django.db import transaction
+from django.db.models.expressions import F
+
+logger = logging.getLogger(__name__)
 
 from .models import Redirect
 
@@ -34,9 +39,19 @@ def __init__(self, *args, **kwargs):
         super(RedirectFallbackMiddleware, self).__init__(*args, **kwargs)
 
     def increment_redirect(self, pk):
-        redirect = Redirect.objects.get(pk=pk)
-        redirect.nr_times_visited += 1
-        redirect.save()
+        def increment():
+            try:
+                Redirect.objects.filter(pk=pk).update(
+                    nr_times_visited=F("nr_times_visited") + 1
+                )
+            except Exception:
+                logger.warning("Increment nr_times_visited failed.")
+                raise
+
+        # https://docs.djangoproject.com/en/5.1/topics/db/transactions/#performing-actions-after-commit
+        # All errors derived from Python’s Exception class are caught and logged to the
+        # django.db.backends.base logger.
+        transaction.on_commit(increment)
 
     def process_response(self, request, response):
         if response.status_code != 404:

regex_redirects/tests.py

@@ -22,7 +22,9 @@ def test_model(self):
     def test_redirect(self):
         redirect = Redirect.objects.create(old_path="/initial", new_path="/new_target")
         self.assertEqual(redirect.nr_times_visited, 0)
-        response = self.client.get("/initial")
+        with self.captureOnCommitCallbacks(execute=True) as callbacks:
+            response = self.client.get("/initial")
+        self.assertEqual(len(callbacks), 1)
         self.assertRedirects(
             response, "/new_target", status_code=301, target_status_code=404
         )
@@ -35,7 +37,9 @@ def test_redirect_with_append_slash(self):
             old_path="/initial/", new_path="/new_target/"
         )
         self.assertEqual(redirect.nr_times_visited, 0)
-        response = self.client.get("/initial")
+        with self.captureOnCommitCallbacks(execute=True) as callbacks:
+            response = self.client.get("/initial")
+        self.assertEqual(len(callbacks), 1)
         self.assertRedirects(
             response, "/new_target/", status_code=301, target_status_code=404
         )
@@ -51,12 +55,14 @@ def test_redirect_with_append_slash_and_query_string(self):
         )
 
     def test_regular_expression(self):
-        redirect = Redirect.objects.create(
+        Redirect.objects.create(
             old_path=r"/news/index/(\d+)/(.*)/",
             new_path="/my/news/$2/",
             regular_expression=True,
         )
-        response = self.client.get("/news/index/12345/foobar/")
+        with self.captureOnCommitCallbacks(execute=True) as callbacks:
+            response = self.client.get("/news/index/12345/foobar/")
+        self.assertEqual(len(callbacks), 1)
         self.assertRedirects(
             response, "/my/news/foobar/", status_code=301, target_status_code=404
         )

testsettings.py

@@ -22,6 +22,8 @@
 
 SITE_ID = 1
 
+DEFAULT_AUTO_FIELD = 'django.db.models.BigAutoField'
+
 # Django seems to require a ROOT_URLCONF.
 ROOT_URLCONF = __name__
 urlpatterns = []