Reduce number of queries to preferences db table

changelog.d/1082.changed.md

@@ -0,0 +1 @@
+argus.htmx: reduce number of queries to preferences db table

src/argus/auth/models.py

@@ -1,5 +1,7 @@
+from __future__ import annotations
+
 import functools
-from typing import Any, Optional, Union, Protocol
+from typing import Any, List, Optional, Type, Union, Protocol
 
 from django.contrib.auth.models import AbstractUser, Group
 from django.db import models
@@ -92,19 +94,19 @@ def is_used(self):
         # user is not considered in use
         return False
 
-    def get_or_create_preferences(self):
-        Preferences.ensure_for_user(self)
-        return self.preferences.filter(namespace__in=Preferences.NAMESPACES)
-
     def get_preferences_context(self):
-        pref_sets = self.get_or_create_preferences()
+        pref_sets = Preferences.ensure_for_user(self)
         prefdict = {}
         for pref_set in pref_sets:
-            prefdict[pref_set._namespace] = pref_set.get_context()
+            prefdict[pref_set.namespace] = pref_set.get_context()
         return prefdict
 
     def get_namespaced_preferences(self, namespace):
-        return self.get_or_create_preferences().get(namespace=namespace)
+        obj, _ = Preferences.objects.get_or_create(user=self, namespace=namespace)
+        if not obj.preferences and (defaults := obj.get_defaults()):
+            obj.preferences = defaults
+            obj.save()
+        return obj
 
 
 class PreferencesManager(models.Manager):
@@ -114,13 +116,6 @@ def get_queryset(self):
     def get_by_natural_key(self, user, namespace):
         return self.get(user=user, namespace=namespace)
 
-    def create_missing_preferences(self):
-        precount = Preferences.objects.count()
-        for namespace, subclass in Preferences.NAMESPACES.items():
-            for user in User.objects.all():
-                Preferences.ensure_for_user(user)
-        return (precount, Preferences.objects.count())
-
     def get_all_defaults(self):
         prefdict = {}
         for namespace, subclass in Preferences.NAMESPACES.items():
@@ -206,7 +201,7 @@ class Meta:
             models.UniqueConstraint(name="unique_preference", fields=["user", "namespace"]),
         ]
 
-    NAMESPACES = {}
+    NAMESPACES: dict[str, Type[Preferences]] = {}
 
     user = models.ForeignKey(User, on_delete=models.CASCADE, related_name="preferences")
     namespace = models.CharField(blank=False, max_length=255)
@@ -215,15 +210,19 @@ class Meta:
     objects = PreferencesManager()
     unregistered = UnregisteredPreferencesManager()
 
-    # storage for field forms in preference
-    FORMS = None
-    # storage for field defaults in preference
-    _FIELD_DEFAULTS = None
+    # must be set by the subclasses
+    FORMS: dict[str, forms.Form]
+    _FIELD_DEFAULTS: dict[str, Any]
 
     # django methods
 
     # called when subclass is constructing itself
     def __init_subclass__(cls, **kwargs):
+        assert isinstance(getattr(cls, "FORMS", None), dict), f"{cls.__name__}.FORMS must be a dictionary"
+        assert isinstance(
+            getattr(cls, "_FIELD_DEFAULTS", None), dict
+        ), f"{cls.__name__}._FIELD_DEFAULTS must be a dictionary"
+
         super().__init_subclass__(**kwargs)
         cls.NAMESPACES[cls._namespace] = cls
 
@@ -250,12 +249,20 @@ def get_defaults(cls):
         return cls._FIELD_DEFAULTS.copy() if cls._FIELD_DEFAULTS else {}
 
     @classmethod
-    def ensure_for_user(cls, user):
+    def ensure_for_user(cls, user) -> List[Preferences]:
+        all_preferences = {p.namespace: p for p in user.preferences.all()}
+        valid_preferences = []
+
         for namespace, subclass in cls.NAMESPACES.items():
-            obj, _ = subclass.objects.get_or_create(user=user, namespace=namespace)
-            if not obj.preferences and (defaults := subclass.get_defaults()):
-                obj.preferences = defaults
-                obj.save()
+            if namespace in all_preferences:
+                valid_preferences.append(all_preferences[namespace])
+                continue
+            obj = subclass.objects.create(user=user, namespace=namespace)
+            obj.preferences = subclass.get_defaults()
+            obj.save()
+            valid_preferences.append(obj)
+
+        return valid_preferences
 
     def update_context(self, context):
         "Override this to change what is put in context"

src/argus/auth/utils.py

@@ -1,5 +1,6 @@
 from copy import deepcopy
 import logging
+from typing import Any, Tuple
 
 from django.conf import settings
 from django.contrib.auth.backends import ModelBackend, RemoteUserBackend
@@ -57,32 +58,33 @@ def get_preference(request, namespace, preference):
     return prefs.get_preference(preference)
 
 
-def save_preference(request, data, namespace, preference):
+def get_or_update_preference(request, data, namespace, preference) -> Tuple[Any, bool]:
     """Save the single preference given in data to the given namespace
 
-    Returns True on success, otherwise False
+    Returns a tuple (value, success). Value is the value of the preference, and success a boolean
+    indicating whether the preference was successfully updated
     """
     prefs = get_preference_obj(request, namespace)
     value = prefs.get_preference(preference)
     LOG.debug("Changing %s: currently %s", preference, value)
 
     if not data.get(preference, None):
         LOG.debug("Failed to change %s, not in input: %s", preference, data)
-        return False
+        return value, False
 
     form = prefs.FORMS[preference](data)
     if not form.is_valid():
         messages.warning(request, f"Failed to change {preference}, invalid input")
         LOG.warning("Failed to change %s, invalid input: %s", preference, data)
-        return False
+        return value, False
 
     old_value = deepcopy(value)  # Just in case value is mutable..
     value = form.cleaned_data[preference]
     if value == old_value:
         LOG.debug("Did not change %s: no change", preference)
-        return False
+        return value, False
 
     prefs.save_preference(preference, value)
     messages.success(request, f"Changed {preference}: {old_value} → {value}")
     LOG.info("Changed %s: %s → %s", preference, old_value, value)
-    return True
+    return value, True

src/argus/htmx/dateformat/views.py

@@ -6,7 +6,7 @@
 from django.http import HttpResponse
 from django_htmx.http import HttpResponseClientRefresh
 
-from argus.auth.utils import save_preference
+from argus.auth.utils import get_or_update_preference
 
 from argus.htmx.incidents.views import HtmxHttpRequest
 from .constants import DATETIME_FORMATS
@@ -22,5 +22,5 @@ def dateformat_names(request: HtmxHttpRequest) -> HttpResponse:
 
 @require_POST
 def change_dateformat(request: HtmxHttpRequest) -> HttpResponse:
-    save_preference(request, request.POST, "argus_htmx", "datetime_format_name")
+    get_or_update_preference(request, request.POST, "argus_htmx", "datetime_format_name")
     return HttpResponseClientRefresh()

src/argus/htmx/incidents/views.py

@@ -13,7 +13,7 @@
 from django_htmx.middleware import HtmxDetails
 from django_htmx.http import HttpResponseClientRefresh
 
-from argus.auth.utils import get_preference, save_preference
+from argus.auth.utils import get_or_update_preference
 from argus.incident.models import Incident
 from argus.util.datetime_utils import make_aware
 
@@ -117,10 +117,8 @@ def incident_list(request: HtmxHttpRequest) -> HttpResponse:
 
     # Standard Django pagination
 
-    page_size = get_preference(request, "argus_htmx", "page_size")
-    success = save_preference(request, request.GET, "argus_htmx", "page_size")
-    if success:
-        page_size = get_preference(request, "argus_htmx", "page_size")
+    page_size, _ = get_or_update_preference(request, request.GET, "argus_htmx", "page_size")
+
     paginator = Paginator(object_list=qs, per_page=page_size)
     page_num = params.pop("page", "1")
     page = paginator.get_page(page_num)

src/argus/htmx/themes/views.py

@@ -6,7 +6,7 @@
 from django.http import HttpResponse
 from django_htmx.http import HttpResponseClientRefresh
 
-from argus.auth.utils import save_preference
+from argus.auth.utils import get_or_update_preference
 
 from argus.htmx.constants import THEME_NAMES
 from argus.htmx.incidents.views import HtmxHttpRequest
@@ -23,7 +23,7 @@ def theme_names(request: HtmxHttpRequest) -> HttpResponse:
 
 @require_POST
 def change_theme(request: HtmxHttpRequest) -> HttpResponse:
-    success = save_preference(request, request.POST, "argus_htmx", "theme")
+    _, success = get_or_update_preference(request, request.POST, "argus_htmx", "theme")
     if success:
         return render(request, "htmx/themes/_current_theme.html")
     return HttpResponseClientRefresh()

src/argus/htmx/user/views.py

@@ -3,7 +3,7 @@
 from django.views.decorators.http import require_GET, require_POST
 from django_htmx.http import HttpResponseClientRefresh
 
-from argus.auth.utils import save_preference
+from argus.auth.utils import get_or_update_preference
 
 from argus.htmx.constants import ALLOWED_PAGE_SIZES
 from argus.htmx.incidents.views import HtmxHttpRequest
@@ -17,7 +17,7 @@ def page_size_names(request: HtmxHttpRequest) -> HttpResponse:
 
 @require_POST
 def change_page_size(request: HtmxHttpRequest) -> HttpResponse:
-    save_preference(request, request.POST, "argus_htmx", "page_size")
+    get_or_update_preference(request, request.POST, "argus_htmx", "page_size")
     return HttpResponseClientRefresh()
 
 

tests/auth/test_models.py

@@ -138,6 +138,12 @@ def test_get_instance_converts_to_subclass(self):
         instance = instances[0]
         self.assertIsInstance(instance, MyPreferences)
 
+    def test_get_namespaced_preferences_creates_preferences_with_defaults(self):
+        user = PersonUserFactory()
+        pref_set = user.get_namespaced_preferences(namespace=MyPreferences._namespace)
+        self.assertIsInstance(pref_set, MyPreferences)
+        self.assertEqual(pref_set.preferences, {"magic_number": 42})
+
     def test_vanilla_get_context_dumps_preferences_field_including_defaults(self):
         user = PersonUserFactory()
 
@@ -236,19 +242,6 @@ def test_get_by_natural_key_fetches_preference_of_correct_namespace(self):
         result = Preferences.objects.get_by_natural_key(user, MyPreferences._namespace)
         self.assertEqual(prefs, result)
 
-    def test_create_missing_preferences_creates_all_namespaces_for_all_users(self):
-        user1 = PersonUserFactory()
-        user2 = PersonUserFactory()
-
-        # no preferences yet
-        self.assertFalse(Preferences.objects.filter(user=user1).exists())
-        self.assertFalse(Preferences.objects.filter(user=user2).exists())
-
-        Preferences.objects.create_missing_preferences()
-        # three each (two from tests, one from app)
-        self.assertEqual(Preferences.objects.filter(user=user1).count(), 3)
-        self.assertEqual(Preferences.objects.filter(user=user2).count(), 3)
-
     def test_get_all_defaults_returns_all_prefs_defaults(self):
         defaults = Preferences.objects.get_all_defaults()
         self.assertIsInstance(defaults, dict)