amorfo77/paperless-ngx
1
0
Fork 0
mirror of https://github.com/paperless-ngx/paperless-ngx.git synced 2026-08-27 21:23:20 +00:00
Code Issues Projects Releases Wiki Activity

Fix: cache document/field on updated CustomFieldInstance rows, not just created ones

Browse Source 
update_or_create() fetches an existing row via plain .get() before saving
it, so passing already-resolved document/field objects as lookup kwargs
never actually cached them on that row.  Replaced with an explicit
get-or-build + assign + save so both paths get the cache.

Also: only build docs_by_id when there's something to add (a remove-only
call has no use for it), and resolve the removal pass's source documents
via select_related instead, so it doesn't force-load irrelevant documents.

Added tests for the update-path caching and the removal-path batching.
This commit is contained in:
stumpylog
2026-08-27 09:42:14 -07:00
parent 9321d8772f
commit 02c547e856
2 changed files with 142 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/documents/bulk_edit.py
+38 -24
View File
@@ -311,17 +311,19 @@ def modify_custom_fields(
id__in=[int(field) for field, _ in add_custom_fields],
)
}
# Deferred, not `.only()`: these objects get cached onto the FK
# descriptor of newly-created CustomFieldInstance rows below, and
# downstream post_save receivers (e.g. the filename-generation signal)
# touch other Document fields -- `.only("pk")` would just turn that into
# a deferred-field reload per document, trading one N+1 for another.
# `content` is the one field guaranteed to be both large (full OCR text)
# and unused by anything this function or its receivers touch.
docs_by_id: dict[int, Document] = {
doc.id: doc
for doc in Document.objects.filter(id__in=affected_docs).defer("content")
}
# Deferred, not `.only()`: signal receivers touch other Document fields,
# and `.only("pk")` would just turn that into a per-document reload.
# `content` is the one field both large and unused here. Skipped
# entirely for a remove-only call -- the removal pass below resolves
# its own documents.
docs_by_id: dict[int, Document] = (
{
doc.id: doc
for doc in Document.objects.filter(id__in=affected_docs).defer("content")
}
if add_custom_fields
else {}
)
for field_id, value in add_custom_fields:
custom_field = custom_fields_by_id[field_id]
value_field = CustomFieldInstance.TYPE_TO_DATA_STORE_NAME_MAP[
@@ -336,29 +338,41 @@ def modify_custom_fields(
):
# Prevent self-linking
continue
# Pass the already-resolved objects, not bare ids: this caches
# them on the FK descriptor of any newly-created instance, so a
# later `.field`/`.document` access (e.g. auditlog's post_save
# receiver calling `str(instance)`, which touches `.field.name`)
# doesn't trigger its own per-instance re-fetch.
CustomFieldInstance.objects.update_or_create(
document=docs_by_id[doc_id],
field=custom_field,
defaults=defaults,
)
# Not update_or_create(): it fetches an existing row via plain
# `.get()` before calling .save(), so a signal receiver touching
# `.field`/`.document` (e.g. auditlog) on that save re-fetches
# per instance regardless of what's passed in as lookup kwargs.
# Assigning the cached objects ourselves before .save() avoids
# that for both the create and update case.
try:
instance = CustomFieldInstance.objects.get(
document=docs_by_id[doc_id],
field=custom_field,
)
except CustomFieldInstance.DoesNotExist:
instance = CustomFieldInstance(
document=docs_by_id[doc_id],
field=custom_field,
)
instance.document = docs_by_id[doc_id]
instance.field = custom_field
for attr, val in defaults.items():
setattr(instance, attr, val)
instance.save()
if custom_field.data_type == CustomField.FieldDataType.DOCUMENTLINK:
reflect_doclinks(docs_by_id[doc_id], custom_field, value)
# For doc link fields that are being removed, remove symmetrical links
# For doc link fields being removed, remove symmetrical links.
# select_related here avoids resolving every affected document up front.
for doclink_being_removed_instance in CustomFieldInstance.objects.filter(
document_id__in=affected_docs,
field__id__in=remove_custom_fields,
field__data_type=CustomField.FieldDataType.DOCUMENTLINK,
value_document_ids__isnull=False,
).select_related("field"):
).select_related("field", "document"):
for target_doc_id in doclink_being_removed_instance.value:
remove_doclink(
document=docs_by_id[doclink_being_removed_instance.document_id],
document=doclink_being_removed_instance.document,
field=doclink_being_removed_instance.field,
target_doc_id=target_doc_id,
)
src/documents/tests/test_bulk_edit.py
+104
View File
@@ -440,6 +440,110 @@ class TestBulkEdit(DirectoriesMixin, TestCase):
[target.id],
)
def test_modify_custom_fields_update_caches_document_and_field(self) -> None:
"""
GIVEN:
- Several documents already have an instance of a custom field
WHEN:
- modify_custom_fields runs again for the same field, updating
the existing instances rather than creating new ones
THEN:
- No per-instance `.document`/`.field` reload query is issued
(e.g. by auditlog's post_save receiver touching them)
"""
docs = [
Document.objects.create(checksum=f"update-{i}", title=f"update-{i}")
for i in range(6)
]
field = CustomField.objects.create(
name="Update Field",
data_type=CustomField.FieldDataType.STRING,
)
bulk_edit.modify_custom_fields(
[doc.id for doc in docs],
add_custom_fields=[field.id],
remove_custom_fields=[],
)
with CaptureQueriesContext(connection) as ctx:
bulk_edit.modify_custom_fields(
[doc.id for doc in docs],
add_custom_fields={field.id: "updated value"},
remove_custom_fields=[],
)
single_row_reloads = [
q
for q in ctx.captured_queries
if ('FROM "documents_document"' in q["sql"] and '."id" = ' in q["sql"])
or ('FROM "documents_customfield"' in q["sql"] and '."id" = ' in q["sql"])
]
self.assertEqual(
single_row_reloads,
[],
"Expected no per-instance document/field reload queries when "
f"updating existing custom field instances, got: {single_row_reloads}",
)
for doc in docs:
self.assertEqual(
doc.custom_fields.get(field=field).value,
"updated value",
)
def test_modify_custom_fields_removes_symmetrical_doclinks_batched(self) -> None:
"""
GIVEN:
- Several source documents link to a shared target via a doc
link field
WHEN:
- The field is removed from all of them in one call
THEN:
- The symmetrical links are removed from the target
- The source documents come from one batched query, not one
lookup per source document (the target side, fetched inside
remove_doclink(), is untouched by this PR and out of scope)
"""
target = Document.objects.create(checksum="rm-target", title="rm-target")
docs = [
Document.objects.create(checksum=f"rm-{i}", title=f"rm-{i}")
for i in range(6)
]
field = CustomField.objects.create(
name="Related",
data_type=CustomField.FieldDataType.DOCUMENTLINK,
)
bulk_edit.modify_custom_fields(
[doc.id for doc in docs],
add_custom_fields={field.id: [target.id]},
remove_custom_fields=[],
)
self.assertEqual(
target.custom_fields.get(field=field).value,
[d.id for d in docs],
)
with CaptureQueriesContext(connection) as ctx:
bulk_edit.modify_custom_fields(
[doc.id for doc in docs],
add_custom_fields=[],
remove_custom_fields=[field.id],
)
source_doc_lookups = [
q
for q in ctx.captured_queries
if 'FROM "documents_document"' in q["sql"]
and any(f'."id" = {doc.id} ' in q["sql"] for doc in docs)
]
self.assertEqual(
source_doc_lookups,
[],
"Expected source documents to come from a batched query, not "
f"per-document lookups, got: {source_doc_lookups}",
)
self.assertEqual(target.custom_fields.get(field=field).value, [])
def test_modify_custom_fields_doclink_self_link(self) -> None:
"""
GIVEN: