From 643d0205bc68713d4e4864618dd163f5fe6abc66 Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Tue, 18 Aug 2026 07:19:51 -0700 Subject: [PATCH] Fix: dont re-render path template when checking collisions (#13718) --- src/documents/file_handling.py | 22 +++++++++------- src/documents/tests/test_file_handling.py | 32 +++++++++++++++++++++++ 2 files changed, 44 insertions(+), 10 deletions(-) diff --git a/src/documents/file_handling.py b/src/documents/file_handling.py index 7f59d4557..6bc202733 100644 --- a/src/documents/file_handling.py +++ b/src/documents/file_handling.py @@ -62,18 +62,17 @@ def generate_unique_filename(doc, *, archive_filename=False) -> Path: old_filename = Path(doc.filename) if doc.filename else None root = settings.ORIGINALS_DIR + base_filename = generate_filename(doc, archive_filename=archive_filename) + # If generating archive filenames, try to make a name that is similar to # the original filename first. if archive_filename and doc.filename: - # Generate the full path using the same logic as generate_filename - base_generated = generate_filename(doc, archive_filename=archive_filename) - # Try to create a simple PDF version based on the original filename # but preserve any directory structure from the template - if str(base_generated.parent) != ".": + if str(base_filename.parent) != ".": # Has directory structure, preserve it - simple_pdf_name = base_generated.parent / (Path(doc.filename).stem + ".pdf") + simple_pdf_name = base_filename.parent / (Path(doc.filename).stem + ".pdf") else: # No directory structure simple_pdf_name = Path(Path(doc.filename).stem + ".pdf") @@ -81,14 +80,17 @@ def generate_unique_filename(doc, *, archive_filename=False) -> Path: if simple_pdf_name == old_filename or not (root / simple_pdf_name).exists(): return simple_pdf_name + file_extension = ".pdf" if archive_filename else doc.file_type + filename_stem = base_filename.name.removesuffix(file_extension) counter = 0 while True: - new_filename = generate_filename( - doc, - counter=counter, - archive_filename=archive_filename, - ) + new_filename = base_filename + if counter: + new_filename = base_filename.with_name( + f"{filename_stem}_{counter:02}{file_extension}", + ) + if new_filename == old_filename: # still the same as before. return new_filename diff --git a/src/documents/tests/test_file_handling.py b/src/documents/tests/test_file_handling.py index 399d4ba7d..0a4c13cd2 100644 --- a/src/documents/tests/test_file_handling.py +++ b/src/documents/tests/test_file_handling.py @@ -10,8 +10,10 @@ from auditlog.context import disable_auditlog from django.conf import settings from django.contrib.auth.models import User from django.db import DatabaseError +from django.db import connection from django.test import TestCase from django.test import override_settings +from django.test.utils import CaptureQueriesContext from django.utils import timezone from documents.file_handling import create_source_path_directory @@ -32,6 +34,36 @@ from documents.tests.utils import FileSystemAssertsMixin class TestFileHandling(DirectoriesMixin, FileSystemAssertsMixin, TestCase): + @override_settings(FILENAME_FORMAT="{title}") + def test_generate_unique_filename_renders_template_once(self) -> None: + document = Document.objects.create( + title="collision", + mime_type="application/pdf", + ) + Document.objects.filter(pk=document.pk).update(filename="collision_03.pdf") + document.refresh_from_db() + + for filename in ( + "collision.pdf", + "collision_01.pdf", + "collision_02.pdf", + "collision_03.pdf", + ): + (settings.ORIGINALS_DIR / filename).touch() + + with CaptureQueriesContext(connection) as queries: + generated = generate_unique_filename(document) + + relation_queries = [ + query["sql"] + for query in queries.captured_queries + if "documents_tag" in query["sql"] + or "documents_customfieldinstance" in query["sql"] + ] + + self.assertEqual(generated, Path("collision_03.pdf")) + self.assertEqual(len(relation_queries), 2) + @override_settings(FILENAME_FORMAT="") def test_generate_source_filename(self) -> None: document = Document()