mirror of
https://github.com/paperless-ngx/paperless-ngx.git
synced 2026-10-26 22:35:13 +00:00
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.
This commit is contained in:
1 parent
ce5d5c33b0
commit
74abdf8ff8
2 files changed
+28
-28
No files matched your search
@@ -1,5 +1,6 @@
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import contextlib
|
||||||
import logging
|
import logging
|
||||||
import math
|
import math
|
||||||
import re
|
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
|
# A CustomField lookup cache scoped to a single field/serializer instance
|
||||||
# per request: once as the ordinary nested list, then again per-item while
|
# only helps within that one instance's own validation pass. Several call
|
||||||
# matching existing vs. new CustomFieldInstance rows during save() -- and
|
# sites, though, build more than one CustomFieldInstanceSerializer (or its
|
||||||
# that second pass builds a brand new serializer (and field) instance per
|
# CustomFieldInstanceListSerializer/field) for the same request and pass
|
||||||
# item (see its update_or_create_reverse_relations / _get_serializer_for_field),
|
# each of them `context=self.context` -- the *same* dict object, not a
|
||||||
# so a cache on the field instance alone only helps the first pass. It does,
|
# copy -- e.g. bulk-edit's _validate_custom_field_values() constructing a
|
||||||
# however, explicitly pass `context=self.context` to every one of those
|
# fresh CustomFieldInstanceSerializer per submitted field. That context
|
||||||
# fresh serializers -- the *same* dict object the outer DocumentSerializer
|
# dict is already request-scoped (DRF builds it fresh per request via
|
||||||
# is using, not a copy. That context dict is already request-scoped (DRF
|
# get_serializer_context()), so stashing the resolved CustomField objects
|
||||||
# builds it fresh per request via get_serializer_context()), so stashing the
|
# there -- rather than in some new global/thread-local cache -- lets every
|
||||||
# resolved CustomField objects there -- rather than in some new global/
|
# one of those separately-instantiated serializers reuse them for free
|
||||||
# thread-local cache -- lets every later pass reuse them for free while
|
# while staying entirely within DRF's existing, already-request-scoped
|
||||||
# staying entirely within DRF's existing, already-request-scoped machinery.
|
# machinery.
|
||||||
_CUSTOM_FIELD_CONTEXT_CACHE_KEY = "_custom_field_lookup_cache"
|
_CUSTOM_FIELD_CONTEXT_CACHE_KEY = "_custom_field_lookup_cache"
|
||||||
|
|
||||||
|
|
||||||
@@ -1438,12 +1439,12 @@ class DocumentSerializer(
|
|||||||
|
|
||||||
custom_fields_data = validated_data.pop("custom_fields", None)
|
custom_fields_data = validated_data.pop("custom_fields", None)
|
||||||
|
|
||||||
if settings.AUDIT_LOG_ENABLED:
|
actor_context = (
|
||||||
with set_actor(self.user):
|
set_actor(self.user)
|
||||||
super().update(instance, validated_data)
|
if settings.AUDIT_LOG_ENABLED
|
||||||
if custom_fields_data is not None:
|
else contextlib.nullcontext()
|
||||||
self._sync_custom_fields(instance, custom_fields_data)
|
)
|
||||||
else:
|
with actor_context:
|
||||||
super().update(instance, validated_data)
|
super().update(instance, validated_data)
|
||||||
if custom_fields_data is not None:
|
if custom_fields_data is not None:
|
||||||
self._sync_custom_fields(instance, custom_fields_data)
|
self._sync_custom_fields(instance, custom_fields_data)
|
||||||
|
|||||||
@@ -592,9 +592,10 @@ class TestCustomFieldsAPI(DirectoriesMixin, APITestCase):
|
|||||||
WHEN:
|
WHEN:
|
||||||
- A second, separately-instantiated CustomFieldInstanceSerializer
|
- A second, separately-instantiated CustomFieldInstanceSerializer
|
||||||
validates the same field id, sharing that same context
|
validates the same field id, sharing that same context
|
||||||
(this is what drf-writable-nested does: it rebuilds a fresh
|
(this mirrors what real call sites do, e.g. bulk-edit's
|
||||||
serializer -- and fresh field instances -- per item while
|
validate_custom_fields()/_validate_custom_field_values()
|
||||||
matching existing vs. new instances during save())
|
constructing a fresh CustomFieldInstanceSerializer per
|
||||||
|
submitted field, all sharing the outer serializer's context)
|
||||||
THEN:
|
THEN:
|
||||||
- No additional query is issued to resolve the CustomField
|
- 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
|
- A PATCH request updates 2 of them, adds 1 new one, and omits
|
||||||
the 3rd (which should be deleted)
|
the 3rd (which should be deleted)
|
||||||
THEN:
|
THEN:
|
||||||
- The number of queries used to create/update/delete the
|
- The omitted custom field is removed via exactly one
|
||||||
CustomFieldInstance rows is bounded and does not scale with
|
delete-shaped query, not drf-writable-nested's old two-step
|
||||||
drf-writable-nested's per-item serializer-rebuild + separate
|
soft-delete-then-hard-delete-after pattern (two delete
|
||||||
soft-delete-then-hard-delete-after pattern (one query per
|
passes)
|
||||||
CustomFieldInstance write via update_or_create, plus exactly
|
|
||||||
one final hard-delete query -- not two delete passes)
|
|
||||||
"""
|
"""
|
||||||
doc = Document.objects.create(
|
doc = Document.objects.create(
|
||||||
title="WOW",
|
title="WOW",
|
||||||
|
|||||||
Reference in new issue
Block a user