From 8bca4fc7289262062efaf6204291fc508896e538 Mon Sep 17 00:00:00 2001 From: Albrecht Degering Date: Tue, 8 Sep 2026 12:19:37 +0200 Subject: [PATCH] perf(campaign): share linear collision-safe attachment naming Release v0.1.29. Coordinated integrity review: GovOPlaN/govoplan-core#298. --- package.json | 2 +- pyproject.toml | 4 +- .../backend/german_documentation.py | 16 +--- src/govoplan_campaign/backend/manifest.py | 16 +++- .../backend/messages/builder.py | 19 ++--- .../backend/services/filenames.py | 23 ++++++ .../backend/services/zip_service.py | 13 +--- tests/test_filename_allocator.py | 75 +++++++++++++++++++ webui/package.json | 2 +- 9 files changed, 130 insertions(+), 40 deletions(-) create mode 100755 src/govoplan_campaign/backend/services/filenames.py create mode 100755 tests/test_filename_allocator.py diff --git a/package.json b/package.json index 528c995..bf13fc1 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@govoplan/campaign-webui", - "version": "0.1.28", + "version": "0.1.29", "private": true, "type": "module", "main": "webui/src/index.ts", diff --git a/pyproject.toml b/pyproject.toml index e1b38cb..1c67b3b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,14 +4,14 @@ build-backend = "setuptools.build_meta" [project] name = "govoplan-campaign" -version = "0.1.28" +version = "0.1.29" description = "GovOPlaN campaigns module with backend and WebUI integration." readme = "README.md" requires-python = ">=3.12" license = { file = "LICENSE" } authors = [{ name = "GovOPlaN" }] dependencies = [ - "govoplan-core>=0.1.45", + "govoplan-core>=0.1.46", "jsonschema>=4,<5", "pydantic>=2,<3", "SQLAlchemy>=2,<3", diff --git a/src/govoplan_campaign/backend/german_documentation.py b/src/govoplan_campaign/backend/german_documentation.py index 5716272..d7e8d98 100644 --- a/src/govoplan_campaign/backend/german_documentation.py +++ b/src/govoplan_campaign/backend/german_documentation.py @@ -1,9 +1,8 @@ from __future__ import annotations -from dataclasses import replace from typing import Iterable -from govoplan_core.core.modules import DocumentationTopic +from govoplan_core.core.modules import DocumentationTopic, localize_documentation_topics as _localize_topics _TRANSLATIONS = { @@ -258,15 +257,4 @@ _TRANSLATIONS = { def localize_documentation_topics( topics: Iterable[DocumentationTopic], ) -> tuple[DocumentationTopic, ...]: - localized: list[DocumentationTopic] = [] - for topic in topics: - german = _TRANSLATIONS.get(topic.id) - if german is None: - localized.append(topic) - continue - translations = { - locale: dict(value) for locale, value in topic.translations.items() - } - translations["de"] = {**translations.get("de", {}), **german} - localized.append(replace(topic, translations=translations)) - return tuple(localized) + return _localize_topics(topics, locale="de", translations=_TRANSLATIONS) diff --git a/src/govoplan_campaign/backend/manifest.py b/src/govoplan_campaign/backend/manifest.py index 9047fe5..4c76ae9 100644 --- a/src/govoplan_campaign/backend/manifest.py +++ b/src/govoplan_campaign/backend/manifest.py @@ -463,7 +463,7 @@ def _campaigns_router(context: ModuleContext): manifest = ModuleManifest( id="campaigns", name="Campaigns", - version="0.1.28", + version="0.1.29", workflow_definitions=campaign_workflow_definitions(module_version="0.1.28"), required_capabilities=( CAPABILITY_AUTH_PRINCIPAL_RESOLVER, @@ -834,6 +834,20 @@ manifest = ModuleManifest( ), ), documentation=localize_documentation_topics(( + DocumentationTopic( + id="campaigns.attachment-filename-fidelity", + title="Deterministic attachment and ZIP names", + summary="Resolve repeated names efficiently without dropping or reordering attachments.", + body="Message attachments and ZIP members share a first-free suffix allocator. Repeated names retain the established case-insensitive collision rule and exact numbered suffixes, including names already containing suffixes, Unicode case folding and multiple extensions. Each message/archive has independent allocation state. Large groups of identical requested names no longer restart every suffix search from two. This changes naming work only: intended attachment bytes, recipients, order, existing duplicate-file review policy and ZIP encryption remain unchanged.", + layer="available", documentation_types=("user", "admin"), audience=("campaign_manager", "campaign_admin"), order=18, + conditions=(DocumentationCondition(required_modules=("campaigns",), any_scopes=("campaigns:campaign:read", "campaigns:campaign:write")),), + links=(DocumentationLink(label="Campaigns", href="/campaigns", kind="runtime"),), + translations={"de": { + "title": "Deterministische Namen für Anhänge und ZIP-Einträge", + "summary": "Wiederholte Namen effizient auflösen, ohne Anhänge auszulassen oder umzuordnen.", + "body": "Nachrichtenanhänge und ZIP-Einträge verwenden dieselbe Vergabe des ersten freien nummerierten Suffixes. Die bisherige groß-/kleinschreibungsunabhängige Kollisionsregel und exakte Nummerierung bleiben erhalten, auch bei vorhandenen Nummernsuffixen, Unicode-Groß-/Kleinschreibung und mehrfachen Erweiterungen. Jede Nachricht und jedes Archiv besitzt einen getrennten Vergabezustand. Große Gruppen gleicher gewünschter Namen beginnen die Suffixsuche nicht mehr jeweils bei zwei. Nur der Suchaufwand ändert sich: vorgesehene Bytes, Empfänger, Reihenfolge, bestehende Prüfung mehrfach verwendeter Dateien und ZIP-Verschlüsselung bleiben unverändert.", + }}, + ), *CAMPAIGN_USER_DOCUMENTATION, DocumentationTopic( id="campaigns.workflow.link-exact-campaign-to-case", diff --git a/src/govoplan_campaign/backend/messages/builder.py b/src/govoplan_campaign/backend/messages/builder.py index fbee40d..79dfb99 100644 --- a/src/govoplan_campaign/backend/messages/builder.py +++ b/src/govoplan_campaign/backend/messages/builder.py @@ -1,5 +1,7 @@ from __future__ import annotations +from govoplan_campaign.backend.services.filenames import FilenameAllocator + import mimetypes import re import tempfile @@ -303,15 +305,8 @@ def _archive_filename(archive: ZipArchiveConfig, values: dict[str, Any], entry_i return filename if filename.lower().endswith(".zip") else f"{filename}.zip" -def _unique_attachment_filename(filename: str, used: set[str]) -> str: - candidate = filename - path = Path(filename) - counter = 2 - while candidate.casefold() in used: - candidate = f"{path.stem} ({counter}){path.suffix}" - counter += 1 - used.add(candidate.casefold()) - return candidate +def _unique_attachment_filename(filename: str, used: FilenameAllocator) -> str: + return used.allocate(filename) def _deduplicated_archive_members(members: list[tuple[Path, str]]) -> list[tuple[Path, str]]: @@ -381,8 +376,8 @@ def _attach_files( evidence: list[dict[str, object]] = [] archive_members: dict[str, list[tuple[Path, str]]] = {} archive_attachments: dict[str, list[ResolvedAttachment]] = {} - used_message_filenames: set[str] = set() - used_zip_member_filenames: dict[str, set[str]] = {} + used_message_filenames = FilenameAllocator() + used_zip_member_filenames: dict[str, FilenameAllocator] = {} for attachment in resolution.attachments: attachment.message_filenames = [] @@ -394,7 +389,7 @@ def _attach_files( continue match_paths = [Path(match) for match in attachment.matches] if attachment.zip_enabled and attachment.zip_archive_id: - used_archive_names = used_zip_member_filenames.setdefault(attachment.zip_archive_id, set()) + used_archive_names = used_zip_member_filenames.setdefault(attachment.zip_archive_id, FilenameAllocator()) for position, path in enumerate(match_paths, start=1): requested = _render_attachment_filename( template=attachment.zip_entry_name_template, diff --git a/src/govoplan_campaign/backend/services/filenames.py b/src/govoplan_campaign/backend/services/filenames.py new file mode 100755 index 0000000..b37651f --- /dev/null +++ b/src/govoplan_campaign/backend/services/filenames.py @@ -0,0 +1,23 @@ +"""Deterministic collision naming shared by message and ZIP construction.""" + +from pathlib import Path + + +class FilenameAllocator: + """Append-only names with the original first-free, casefolded suffix rule.""" + + def __init__(self) -> None: + self.used: set[str] = set() + self._next: dict[str, int] = {} + + def allocate(self, filename: str) -> str: + key = filename.casefold() + candidate = filename + path = Path(filename) + counter = self._next.get(key, 2) + while candidate.casefold() in self.used: + candidate = f"{path.stem} ({counter}){path.suffix}" + counter += 1 + self.used.add(candidate.casefold()) + self._next[key] = counter + return candidate diff --git a/src/govoplan_campaign/backend/services/zip_service.py b/src/govoplan_campaign/backend/services/zip_service.py index 23d42f2..53b1f55 100644 --- a/src/govoplan_campaign/backend/services/zip_service.py +++ b/src/govoplan_campaign/backend/services/zip_service.py @@ -12,6 +12,8 @@ from pathlib import Path from typing import Iterable import zlib +from govoplan_campaign.backend.services.filenames import FilenameAllocator + try: import pyzipper except ImportError: # pragma: no cover @@ -24,18 +26,11 @@ ZIP_METHOD_STANDARD = "zip_standard" def _normalized_members(files: Iterable[Path | ArchiveMember]) -> list[ArchiveMember]: members: list[ArchiveMember] = [] - used_names: set[str] = set() + names = FilenameAllocator() for item in files: path, requested_name = item if isinstance(item, tuple) else (item, item.name) requested = Path(requested_name).name or path.name - stem = Path(requested).stem - suffix = Path(requested).suffix - candidate = requested - counter = 2 - while candidate.casefold() in used_names: - candidate = f"{stem} ({counter}){suffix}" - counter += 1 - used_names.add(candidate.casefold()) + candidate = names.allocate(requested) members.append((path, candidate)) return members diff --git a/tests/test_filename_allocator.py b/tests/test_filename_allocator.py new file mode 100755 index 0000000..b935f19 --- /dev/null +++ b/tests/test_filename_allocator.py @@ -0,0 +1,75 @@ +from pathlib import Path +import random +import unittest + +from govoplan_campaign.backend.services.filenames import FilenameAllocator +from govoplan_campaign.backend.services.zip_service import _normalized_members + + +def legacy_names(names): + used = set() + result = [] + for name in names: + path = Path(name) + candidate = name + counter = 2 + while candidate.casefold() in used: + candidate = f"{path.stem} ({counter}){path.suffix}" + counter += 1 + used.add(candidate.casefold()) + result.append(candidate) + return result + + +class FilenameAllocatorTests(unittest.TestCase): + def test_exact_equivalence_with_colliding_suffixes_case_unicode_and_multiple_dots( + self, + ): + choices = [ + "report.pdf", + "REPORT.PDF", + "report (2).pdf", + "report (3).pdf", + "report (2) (2).pdf", + ".hidden", + "a.tar.gz", + "A.TAR.GZ", + "Straße.txt", + "STRASSE.txt", + "readme", + ] + rng = random.Random(42) + for _ in range(30): + names = [rng.choice(choices) for _ in range(200)] + allocator = FilenameAllocator() + self.assertEqual( + legacy_names(names), [allocator.allocate(name) for name in names] + ) + + def test_zip_and_message_names_preserve_every_input_and_sequence(self): + names = ["a.pdf", "A.pdf", "a (2).pdf", "a.pdf"] + paths = [Path(f"/synthetic/{index}/source") for index in range(len(names))] + members = _normalized_members(list(zip(paths, names))) + self.assertEqual(paths, [path for path, _ in members]) + self.assertEqual(legacy_names(names), [name for _, name in members]) + self.assertEqual("a.pdf", FilenameAllocator().allocate("a.pdf")) + + def test_many_collisions_have_linear_membership_work(self): + class CountingSet(set): + probes = 0 + + def __contains__(self, item): + self.probes += 1 + return super().__contains__(item) + + allocator = FilenameAllocator() + allocator.used = CountingSet() + for _ in range(10000): + last = allocator.allocate("report.pdf") + self.assertEqual("report (10000).pdf", last) + self.assertEqual(10000, len(allocator.used)) + self.assertEqual(19999, allocator.used.probes) + + +if __name__ == "__main__": + unittest.main() diff --git a/webui/package.json b/webui/package.json index 53076af..14dacd6 100644 --- a/webui/package.json +++ b/webui/package.json @@ -1,6 +1,6 @@ { "name": "@govoplan/campaign-webui", - "version": "0.1.28", + "version": "0.1.29", "private": true, "type": "module", "main": "src/index.ts",