diff --git a/src/documents/serialisers.py b/src/documents/serialisers.py index 635f6b1cb..4c58767ee 100644 --- a/src/documents/serialisers.py +++ b/src/documents/serialisers.py @@ -877,32 +877,60 @@ def validate_documentlink_targets(user, doc_ids): ) +# drf-writable-nested revalidates a document's custom_fields more than once +# per request: once as the ordinary nested list, then again per-item while +# matching existing vs. new CustomFieldInstance rows during save() -- and +# that second pass builds a brand new serializer (and field) instance per +# item (see its update_or_create_reverse_relations / _get_serializer_for_field), +# so a cache on the field instance alone only helps the first pass. It does, +# however, explicitly pass `context=self.context` to every one of those +# fresh serializers -- the *same* dict object the outer DocumentSerializer +# is using, not a copy. That context dict is already request-scoped (DRF +# builds it fresh per request via get_serializer_context()), so stashing the +# resolved CustomField objects there -- rather than in some new global/ +# thread-local cache -- lets every later pass reuse them for free while +# staying entirely within DRF's existing, already-request-scoped machinery. +_CUSTOM_FIELD_CONTEXT_CACHE_KEY = "_custom_field_lookup_cache" + + class _CachingCustomFieldPrimaryKeyField(serializers.PrimaryKeyRelatedField): """ - A document's custom_fields are validated as a list; the default - PrimaryKeyRelatedField issues one SELECT per item. CustomFieldInstanceListSerializer - below resolves all of an incoming list's field ids in a single query up - front and caches them here, keyed by id, so per-item validation is free - instead of re-querying. The cache lives on this field instance, which - DRF constructs fresh for each request -- no state persists between - requests. + Resolves CustomField ids with as few queries as possible: a per-instance + cache for repeat lookups on this exact field instance, backed by a + shared cache on the serializer context (see _CUSTOM_FIELD_CONTEXT_CACHE_KEY + above) so later, separately-instantiated fields for the same request + reuse what was already resolved instead of re-querying. """ def __init__(self, **kwargs: Any) -> None: super().__init__(**kwargs) self._cache: dict[int, CustomField] = {} + def _shared_cache(self) -> dict[int, CustomField]: + return self.context.setdefault(_CUSTOM_FIELD_CONTEXT_CACHE_KEY, {}) + def prefetch(self, ids: Iterable[int]) -> None: - missing = {i for i in ids if i not in self._cache} + shared_cache = self._shared_cache() + missing = {i for i in ids if i not in self._cache and i not in shared_cache} if missing: for obj in self.get_queryset().filter(pk__in=missing): - self._cache[obj.pk] = obj + shared_cache[obj.pk] = obj + for i in ids: + obj = shared_cache.get(i) + if obj is not None: + self._cache[i] = obj def to_internal_value(self, data: int) -> CustomField: if data in self._cache: return self._cache[data] + shared_cache = self._shared_cache() + if data in shared_cache: + obj = shared_cache[data] + self._cache[data] = obj + return obj obj: CustomField = super().to_internal_value(data) self._cache[obj.pk] = obj + shared_cache[obj.pk] = obj return obj diff --git a/src/documents/tests/test_api_custom_fields.py b/src/documents/tests/test_api_custom_fields.py index 862e78563..3b81c4593 100644 --- a/src/documents/tests/test_api_custom_fields.py +++ b/src/documents/tests/test_api_custom_fields.py @@ -15,6 +15,7 @@ from rest_framework.test import APITestCase from documents.models import CustomField from documents.models import CustomFieldInstance from documents.models import Document +from documents.serialisers import CustomFieldInstanceSerializer from documents.serialisers import DocumentSerializer from documents.tests.factories import DocumentFactory from documents.tests.utils import DirectoriesMixin @@ -583,6 +584,52 @@ class TestCustomFieldsAPI(DirectoriesMixin, APITestCase): f"got {len(custom_field_lookups)}: {custom_field_lookups}", ) + def test_custom_field_lookup_reuses_shared_context_cache(self) -> None: + """ + GIVEN: + - A CustomField has already been resolved once, by a serializer + sharing a given `context` dict + WHEN: + - A second, separately-instantiated CustomFieldInstanceSerializer + validates the same field id, sharing that same context + (this is what drf-writable-nested does: it rebuilds a fresh + serializer -- and fresh field instances -- per item while + matching existing vs. new instances during save()) + THEN: + - No additional query is issued to resolve the CustomField + """ + custom_field = CustomField.objects.create( + name="Test Custom Field", + data_type=CustomField.FieldDataType.STRING, + ) + + context: dict = {} + first_pass = CustomFieldInstanceSerializer( + data={"field": custom_field.id, "value": "a"}, + context=context, + ) + self.assertTrue(first_pass.is_valid(), first_pass.errors) + + second_pass = CustomFieldInstanceSerializer( + data={"field": custom_field.id, "value": "b"}, + context=context, + ) + with CaptureQueriesContext(connection) as ctx: + self.assertTrue(second_pass.is_valid(), second_pass.errors) + + custom_field_lookups = [ + query + for query in ctx.captured_queries + if 'FROM "documents_customfield" WHERE "documents_customfield"."id"' + in query["sql"] + ] + self.assertEqual( + len(custom_field_lookups), + 0, + "Expected the second, separately-instantiated serializer to reuse " + f"the already-resolved CustomField, got: {custom_field_lookups}", + ) + def test_change_custom_field_instance_value(self) -> None: """ GIVEN: