diff --git a/changelog.d/1082.changed.md b/changelog.d/1082.changed.md new file mode 100644 index 000000000..3a05bdf05 --- /dev/null +++ b/changelog.d/1082.changed.md @@ -0,0 +1 @@ +argus.htmx: reduce number of queries to preferences db table diff --git a/src/argus/auth/models.py b/src/argus/auth/models.py index cc2064011..15068b191 100644 --- a/src/argus/auth/models.py +++ b/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" diff --git a/src/argus/auth/utils.py b/src/argus/auth/utils.py index ad383aaa2..5fcad5891 100644 --- a/src/argus/auth/utils.py +++ b/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,10 +58,11 @@ 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) @@ -68,21 +70,21 @@ def save_preference(request, data, namespace, preference): 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 diff --git a/src/argus/htmx/dateformat/views.py b/src/argus/htmx/dateformat/views.py index 3983095c7..2bfb3078b 100644 --- a/src/argus/htmx/dateformat/views.py +++ b/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() diff --git a/src/argus/htmx/incidents/views.py b/src/argus/htmx/incidents/views.py index 26708b920..c59a172f4 100644 --- a/src/argus/htmx/incidents/views.py +++ b/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) diff --git a/src/argus/htmx/themes/views.py b/src/argus/htmx/themes/views.py index 15df291bf..091ae1e3a 100644 --- a/src/argus/htmx/themes/views.py +++ b/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() diff --git a/src/argus/htmx/user/views.py b/src/argus/htmx/user/views.py index 4eb0832be..fb54d2145 100644 --- a/src/argus/htmx/user/views.py +++ b/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() diff --git a/tests/auth/test_models.py b/tests/auth/test_models.py index 9fc501f03..cf4318607 100644 --- a/tests/auth/test_models.py +++ b/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)