diff --git a/src/documents/file_handling.py b/src/documents/file_handling.py index 3a0ffd9fb..48cd57311 100644 --- a/src/documents/file_handling.py +++ b/src/documents/file_handling.py @@ -99,6 +99,29 @@ def generate_unique_filename(doc, *, archive_filename=False) -> Path: return new_filename +def format_filename(document: Document, template_str: str) -> str | None: + rendered_filename = validate_filepath_template_and_render( + template_str, + document, + ) + if rendered_filename is None: + return None + + # Apply this setting. It could become a filter in the future (or users could use |default) + if settings.FILENAME_FORMAT_REMOVE_NONE: + rendered_filename = rendered_filename.replace("/-none-/", "/") + rendered_filename = rendered_filename.replace(" -none-", "") + rendered_filename = rendered_filename.replace("-none-", "") + rendered_filename = rendered_filename.strip(os.sep) + + rendered_filename = rendered_filename.replace( + "-none-", + "none", + ) # backward compatibility + + return rendered_filename + + def generate_filename( doc: Document, *, @@ -108,28 +131,6 @@ def generate_filename( ) -> Path: base_path: Path | None = None - def format_filename(document: Document, template_str: str) -> str | None: - rendered_filename = validate_filepath_template_and_render( - template_str, - document, - ) - if rendered_filename is None: - return None - - # Apply this setting. It could become a filter in the future (or users could use |default) - if settings.FILENAME_FORMAT_REMOVE_NONE: - rendered_filename = rendered_filename.replace("/-none-/", "/") - rendered_filename = rendered_filename.replace(" -none-", "") - rendered_filename = rendered_filename.replace("-none-", "") - rendered_filename = rendered_filename.strip(os.sep) - - rendered_filename = rendered_filename.replace( - "-none-", - "none", - ) # backward compatibility - - return rendered_filename - # Determine the source of the format string if doc.storage_path is not None: filename_format = doc.storage_path.path diff --git a/src/documents/templating/filepath.py b/src/documents/templating/filepath.py index 00de8de2c..7d76e7f31 100644 --- a/src/documents/templating/filepath.py +++ b/src/documents/templating/filepath.py @@ -52,6 +52,33 @@ class FilePathTemplate(Template): return clean_filepath(original_render) +class PlaceholderString(str): + """ + String subclass used as a sentinel for empty metadata values inside templates. + + - Renders as \"-none-\" to preserve existing filename cleaning logic. + - Compares equal to either \"-none-\" or \"none\" so templates can check for either. + - Evaluates to False so {% if correspondent %} behaves intuitively. + """ + + def __new__(cls, value: str = "-none-"): + return super().__new__(cls, value) + + def __bool__(self) -> bool: + return False + + def __eq__(self, other) -> bool: + if isinstance(other, str) and other == "none": + other = "-none-" + return super().__eq__(other) + + def __ne__(self, other) -> bool: + return not self.__eq__(other) + + +NO_VALUE_PLACEHOLDER = PlaceholderString("-none-") + + _template_environment.undefined = _LogStrictUndefined _template_environment.filters["get_cf_value"] = get_cf_value @@ -128,7 +155,7 @@ def get_added_date_context(document: Document) -> dict[str, str]: def get_basic_metadata_context( document: Document, *, - no_value_default: str, + no_value_default: str = NO_VALUE_PLACEHOLDER, ) -> dict[str, str]: """ Given a Document, constructs some basic information about it. If certain values are not set, @@ -266,7 +293,7 @@ def validate_filepath_template_and_render( # Build the context dictionary context = ( {"document": document} - | get_basic_metadata_context(document, no_value_default="-none-") + | get_basic_metadata_context(document, no_value_default=NO_VALUE_PLACEHOLDER) | get_creation_date_context(document) | get_added_date_context(document) | get_tags_context(tags_list) diff --git a/src/documents/tests/test_api_objects.py b/src/documents/tests/test_api_objects.py index bba9031db..014dd3c2a 100644 --- a/src/documents/tests/test_api_objects.py +++ b/src/documents/tests/test_api_objects.py @@ -4,6 +4,7 @@ from unittest import mock from django.contrib.auth.models import Permission from django.contrib.auth.models import User +from django.test import override_settings from rest_framework import status from rest_framework.test import APITestCase @@ -334,6 +335,45 @@ class TestApiStoragePaths(DirectoriesMixin, APITestCase): self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response.data, "path/Something") + def test_test_storage_path_respects_none_placeholder_setting(self): + """ + GIVEN: + - A storage path template referencing an empty field + WHEN: + - Testing the template before and after enabling remove-none + THEN: + - The preview shows "none" by default and drops the placeholder when configured + """ + document = Document.objects.create( + mime_type="application/pdf", + storage_path=self.sp1, + title="Something", + checksum="123", + ) + payload = json.dumps( + { + "document": document.id, + "path": "folder/{{ correspondent }}/{{ title }}", + }, + ) + + response = self.client.post( + f"{self.ENDPOINT}test/", + payload, + content_type="application/json", + ) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data, "folder/none/Something") + + with override_settings(FILENAME_FORMAT_REMOVE_NONE=True): + response = self.client.post( + f"{self.ENDPOINT}test/", + payload, + content_type="application/json", + ) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data, "folder/Something") + class TestBulkEditObjects(APITestCase): # See test_api_permissions.py for bulk tests on permissions diff --git a/src/documents/tests/test_file_handling.py b/src/documents/tests/test_file_handling.py index 62ca52d71..c0070aa81 100644 --- a/src/documents/tests/test_file_handling.py +++ b/src/documents/tests/test_file_handling.py @@ -1078,6 +1078,47 @@ class TestFilenameGeneration(DirectoriesMixin, TestCase): Path("SomeImportantNone/2020-07-25.pdf"), ) + @override_settings( + FILENAME_FORMAT=( + "{% if correspondent == 'none' %}none/{% endif %}" + "{% if correspondent == '-none-' %}dash/{% endif %}" + "{% if not correspondent %}false/{% endif %}" + "{% if correspondent != 'none' %}notnoneyes/{% else %}notnoneno/{% endif %}" + "{{ correspondent or 'missing' }}/{{ title }}" + ), + ) + def test_placeholder_matches_none_variants_and_false(self): + """ + GIVEN: + - Templates that compare against 'none', '-none-' and rely on truthiness + WHEN: + - A document has or lacks a correspondent + THEN: + - Empty placeholders behave like both strings and evaluate False + """ + doc_without_correspondent = Document.objects.create( + title="does not matter", + mime_type="application/pdf", + checksum="abc", + ) + doc_with_correspondent = Document.objects.create( + title="does not matter", + mime_type="application/pdf", + checksum="def", + correspondent=Correspondent.objects.create(name="Acme"), + ) + + self.assertEqual( + generate_filename(doc_without_correspondent), + Path( + "none/dash/false/notnoneno/missing/does not matter.pdf", + ), + ) + self.assertEqual( + generate_filename(doc_with_correspondent), + Path("notnoneyes/Acme/does not matter.pdf"), + ) + @override_settings( FILENAME_FORMAT="{created_year_short}/{created_month_name_short}/{created_month_name}/{title}", ) diff --git a/src/documents/views.py b/src/documents/views.py index ec347a553..c007a4cee 100644 --- a/src/documents/views.py +++ b/src/documents/views.py @@ -108,6 +108,7 @@ from documents.conditionals import thumbnail_last_modified from documents.data_models import ConsumableDocument from documents.data_models import DocumentMetadataOverrides from documents.data_models import DocumentSource +from documents.file_handling import format_filename from documents.filters import CorrespondentFilterSet from documents.filters import CustomFieldFilterSet from documents.filters import DocumentFilterSet @@ -183,7 +184,6 @@ from documents.tasks import index_optimize from documents.tasks import sanity_check from documents.tasks import train_classifier from documents.tasks import update_document_parent_tags -from documents.templating.filepath import validate_filepath_template_and_render from documents.utils import get_boolean from paperless import version from paperless.celery import app as celery_app @@ -2336,7 +2336,7 @@ class StoragePathViewSet(ModelViewSet, PermissionsAwareDocumentCountMixin): document = serializer.validated_data.get("document") path = serializer.validated_data.get("path") - result = validate_filepath_template_and_render(path, document) + result = format_filename(document, path) return Response(result)