Fix: dont re-render path template when checking collisions (#13718)

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

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()