From d4aec32a33080995654f0ea065c9424a93128213 Mon Sep 17 00:00:00 2001 From: Paul Schilling Date: Thu, 25 Apr 2024 14:34:20 +0200 Subject: [PATCH 1/2] [#9] Retrieve Open Forms choices lazily --- openformsclient/models.py | 45 ++++++++++++++++++++------------------- tests/test_integration.py | 4 ++-- 2 files changed, 25 insertions(+), 24 deletions(-) diff --git a/openformsclient/models.py b/openformsclient/models.py index 88bddd3..56e4876 100644 --- a/openformsclient/models.py +++ b/openformsclient/models.py @@ -5,7 +5,7 @@ from django.db.models.fields import BLANK_CHOICE_DASH from django.forms.fields import TypedChoiceField from django.forms.widgets import Select -from django.utils.functional import cached_property +from django.utils.functional import cached_property, lazy from django.utils.text import capfirst from django.utils.translation import gettext_lazy as _ @@ -107,8 +107,7 @@ def formfield(self, **kwargs): "widget": Select, } - if self.choices is None: - defaults["choices"] = self.get_choices(include_blank=self.blank) + defaults["choices"] = self.get_choices(include_blank=self.blank) defaults["coerce"] = self.to_python return TypedChoiceField(**defaults) @@ -120,25 +119,27 @@ def get_choices( limit_choices_to=None, ordering=(), ): - cache_key = f"openformsclient.models.OpenFormsFieldMixin.get_choices__use_uuids_{self.use_uuids}" - - choices = cache.get(cache_key) - if choices is None: - try: - choices = get_form_choices(use_uuids=self.use_uuids) - except Exception as e: - logger.exception(e) - choices = [] - else: - cache.set(cache_key, choices, timeout=60) - - if choices: - if include_blank: - blank_defined = any(choice in ("", None) for choice, _ in choices) - if not blank_defined: - choices = blank_choice + choices - - return choices + def _fetch(): + cache_key = f"openformsclient.models.OpenFormsBaseField.get_choices__use_uuids_{self.use_uuids}" + + choices = cache.get(cache_key) + if choices is None: + try: + choices = get_form_choices(use_uuids=self.use_uuids) + except Exception as exc: + logger.exception(exc) + choices = [] + else: + cache.set(cache_key, choices, timeout=60) + + if choices: + if include_blank: + blank_defined = any(choice in ("", None) for choice, _ in choices) + if not blank_defined: + choices = blank_choice + choices + return choices + + return lazy(_fetch, list) class OpenFormsUUIDField(OpenFormsBaseField, models.UUIDField): diff --git a/tests/test_integration.py b/tests/test_integration.py index abcee73..b465e91 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -41,7 +41,7 @@ def test_values_in_slug_form_field(self, m): page_form = PageForm() self.assertListEqual( - page_form.fields["form_slug"].choices, + list(page_form.fields["form_slug"].choices), [ ("", "---------"), ("test-1", "Test 1"), @@ -56,7 +56,7 @@ def test_values_in_uuid_form_field(self, m): page_form = PageForm() self.assertListEqual( - page_form.fields["form_uuid"].choices, + list(page_form.fields["form_uuid"].choices), [ ("", "---------"), ("f4423c99-6341-442e-aedc-b47779579f4d", "Test 1"), From 105d753709528d694e8aaef7e84db10da4c79e98 Mon Sep 17 00:00:00 2001 From: Paul Schilling Date: Tue, 18 Jun 2024 10:31:04 +0200 Subject: [PATCH 2/2] [#9] Add test for caching forms retrieval --- setup.cfg | 2 ++ tests/test_integration.py | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/setup.cfg b/setup.cfg index fe0398e..d8754e5 100644 --- a/setup.cfg +++ b/setup.cfg @@ -41,6 +41,7 @@ tests_require = isort black flake8 + time-machine [options.packages.find] include = @@ -56,6 +57,7 @@ tests = isort black flake8 + time-machine pep8 = flake8 coverage = pytest-cov docs = diff --git a/tests/test_integration.py b/tests/test_integration.py index b465e91..5a5d5e3 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -1,9 +1,11 @@ +import datetime from uuid import UUID from django.forms import modelform_factory from django.test import TestCase import requests_mock +import time_machine from openformsclient.models import Configuration from testapp.models import Page @@ -129,3 +131,21 @@ def test_slug_form_field_blank(self, m): self.assertEqual(Page.objects.count(), 1) self.assertEqual(Page.objects.get().form_slug, "") + + def test_form_retrieval_cache(self, m): + self._prepare_mock(m) + + PageForm = modelform_factory(Page, fields=["form_slug"]) + page_form = PageForm() + + with time_machine.travel(0) as traveller: + list(page_form.fields["form_slug"].choices) + list(page_form.fields["form_slug"].choices) + + self.assertEqual(m.call_count, 1) + + traveller.shift(datetime.timedelta(seconds=60)) + + list(page_form.fields["form_slug"].choices) + + self.assertEqual(m.call_count, 2)