From 74abdf8ff8d197178254bee029bd9840d5d2a8ad Mon Sep 17 00:00:00 2001 From: stumpylog <797416+stumpylog@users.noreply.github.com> Date: Tue, 25 Aug 2026 11:53:42 -0700 Subject: [PATCH] Docs: remove stale drf-writable-nested references, DRY up audit-actor update() Final-review cleanup on refactor/remove-drf-writable-nested: several comments/docstrings still described drf-writable-nested's removed NestedUpdateMixin behavior in the present tense. Rewrote them to explain the shared context-cache and delete-query-count guarantees in terms of the current code (bulk-edit's per-field CustomFieldInstanceSerializer construction, and _sync_custom_fields' single hard-delete query). Also collapsed the duplicated `if custom_fields_data is not None: self._sync_custom_fields(...)` line in DocumentSerializer.update()'s audit-log branches into a single code path using contextlib.nullcontext(), removing the drift risk that caused the earlier audit-actor bug. No behavior change. --- src/documents/serialisers.py | 39 ++++++++++--------- src/documents/tests/test_api_custom_fields.py | 17 ++++---- 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/src/documents/serialisers.py b/src/documents/serialisers.py index a653ca880..dc3a7711c 100644 --- a/src/documents/serialisers.py +++ b/src/documents/serialisers.py @@ -1,5 +1,6 @@ from __future__ import annotations +import contextlib import logging import math import re @@ -956,19 +957,19 @@ 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. +# A CustomField lookup cache scoped to a single field/serializer instance +# only helps within that one instance's own validation pass. Several call +# sites, though, build more than one CustomFieldInstanceSerializer (or its +# CustomFieldInstanceListSerializer/field) for the same request and pass +# each of them `context=self.context` -- the *same* dict object, not a +# copy -- e.g. bulk-edit's _validate_custom_field_values() constructing a +# fresh CustomFieldInstanceSerializer per submitted field. 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 +# one of those separately-instantiated serializers reuse them for free +# while staying entirely within DRF's existing, already-request-scoped +# machinery. _CUSTOM_FIELD_CONTEXT_CACHE_KEY = "_custom_field_lookup_cache" @@ -1438,12 +1439,12 @@ class DocumentSerializer( custom_fields_data = validated_data.pop("custom_fields", None) - if settings.AUDIT_LOG_ENABLED: - with set_actor(self.user): - super().update(instance, validated_data) - if custom_fields_data is not None: - self._sync_custom_fields(instance, custom_fields_data) - else: + actor_context = ( + set_actor(self.user) + if settings.AUDIT_LOG_ENABLED + else contextlib.nullcontext() + ) + with actor_context: super().update(instance, validated_data) if custom_fields_data is not None: self._sync_custom_fields(instance, custom_fields_data) diff --git a/src/documents/tests/test_api_custom_fields.py b/src/documents/tests/test_api_custom_fields.py index feeb3905d..2ca8c2fef 100644 --- a/src/documents/tests/test_api_custom_fields.py +++ b/src/documents/tests/test_api_custom_fields.py @@ -592,9 +592,10 @@ class TestCustomFieldsAPI(DirectoriesMixin, APITestCase): 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()) + (this mirrors what real call sites do, e.g. bulk-edit's + validate_custom_fields()/_validate_custom_field_values() + constructing a fresh CustomFieldInstanceSerializer per + submitted field, all sharing the outer serializer's context) THEN: - No additional query is issued to resolve the CustomField """ @@ -1560,12 +1561,10 @@ class TestCustomFieldsAPI(DirectoriesMixin, APITestCase): - A PATCH request updates 2 of them, adds 1 new one, and omits the 3rd (which should be deleted) THEN: - - The number of queries used to create/update/delete the - CustomFieldInstance rows is bounded and does not scale with - drf-writable-nested's per-item serializer-rebuild + separate - soft-delete-then-hard-delete-after pattern (one query per - CustomFieldInstance write via update_or_create, plus exactly - one final hard-delete query -- not two delete passes) + - The omitted custom field is removed via exactly one + delete-shaped query, not drf-writable-nested's old two-step + soft-delete-then-hard-delete-after pattern (two delete + passes) """ doc = Document.objects.create( title="WOW",