From 480a406e8a7b24a970126f13517fc7cef1c95542 Mon Sep 17 00:00:00 2001 From: Trenton H <797416+stumpylog@users.noreply.github.com> Date: Mon, 23 Feb 2026 12:58:06 -0800 Subject: [PATCH] Makes the system checks return multiples, not mashed together --- src/paperless/checks.py | 66 +++++------- src/paperless/tests/test_checks.py | 163 +++++++++++++++++++++++------ 2 files changed, 161 insertions(+), 68 deletions(-) diff --git a/src/paperless/checks.py b/src/paperless/checks.py index b26cb5098..c0c7f49e1 100644 --- a/src/paperless/checks.py +++ b/src/paperless/checks.py @@ -205,48 +205,40 @@ def audit_log_check(app_configs, **kwargs): @register() -def check_deprecated_db_settings(app_configs, **kwargs) -> list[Warning]: +def check_deprecated_db_settings( + app_configs: object, + **kwargs: object, +) -> list[Warning]: """Check for deprecated database environment variables. Detects legacy advanced options that should be migrated to - PAPERLESS_DB_OPTIONS. - - Returns: - List of Django Warning instances for any deprecated vars found. + PAPERLESS_DB_OPTIONS. Returns one Warning per deprecated variable found. """ - deprecated_vars = { - "PAPERLESS_DB_TIMEOUT": "timeout (or connect_timeout for Postgres/MariaDB)", - "PAPERLESS_DB_POOLSIZE": "pool.min_size,pool.max_size", - "PAPERLESS_DBSSLMODE": "sslmode (or ssl_mode for MariaDB)", - "PAPERLESS_DBSSLROOTCERT": "sslrootcert (or ssl.ca for MariaDB)", - "PAPERLESS_DBSSLCERT": "sslcert (or ssl.cert for MariaDB)", - "PAPERLESS_DBSSLKEY": "sslkey (or ssl.key for MariaDB)", + deprecated_vars: dict[str, str] = { + "PAPERLESS_DB_TIMEOUT": "timeout", + "PAPERLESS_DB_POOLSIZE": "pool.min_size / pool.max_size", + "PAPERLESS_DBSSLMODE": "sslmode", + "PAPERLESS_DBSSLROOTCERT": "sslrootcert", + "PAPERLESS_DBSSLCERT": "sslcert", + "PAPERLESS_DBSSLKEY": "sslkey", } - found_vars = [] - for var_name in deprecated_vars: - if os.getenv(var_name): - found_vars.append(var_name) + warnings: list[Warning] = [] - if not found_vars: - return [] - - # Build migration example - examples = [] - for var in found_vars: - examples.append(f"{var} -> PAPERLESS_DB_OPTIONS={deprecated_vars[var]}=") - - return [ - Warning( - "Deprecated database environment variables detected", - # TODO: Need to check this URL - hint=( - f"Found: {', '.join(found_vars)}. " - "These will be removed in v3.2. " - "Migrate to PAPERLESS_DB_OPTIONS instead. " - f"Examples: {'; '.join(examples[:3])}. " - "See https://docs.paperless-ngx.com/migration/" + for var_name, db_option_key in deprecated_vars.items(): + if not os.getenv(var_name): + continue + warnings.append( + Warning( + f"Deprecated environment variable: {var_name}", + hint=( + f"{var_name} is no longer supported and will be removed in v3.2. " + f"Set the equivalent option via PAPERLESS_DB_OPTIONS instead. " + f'Example: PAPERLESS_DB_OPTIONS=\'{{"{db_option_key}": ""}}\'. ' + "See https://docs.paperless-ngx.com/migration/ for the full reference." + ), + id="paperless.W001", ), - id="paperless.W001", - ), - ] + ) + + return warnings diff --git a/src/paperless/tests/test_checks.py b/src/paperless/tests/test_checks.py index 0e70cf792..50f0ad3f7 100644 --- a/src/paperless/tests/test_checks.py +++ b/src/paperless/tests/test_checks.py @@ -3,6 +3,7 @@ from pathlib import Path from unittest import mock import pytest +from django.core.checks import Warning from django.test import TestCase from django.test import override_settings from pytest_mock import MockerFixture @@ -242,65 +243,165 @@ class TestAuditLogChecks(TestCase): ) -class TestDeprecatedDbSettings: - """Test suite for deprecated database settings system check.""" +DEPRECATED_VARS: dict[str, str] = { + "PAPERLESS_DB_TIMEOUT": "timeout", + "PAPERLESS_DB_POOLSIZE": "pool.min_size / pool.max_size", + "PAPERLESS_DBSSLMODE": "sslmode", + "PAPERLESS_DBSSLROOTCERT": "sslrootcert", + "PAPERLESS_DBSSLCERT": "sslcert", + "PAPERLESS_DBSSLKEY": "sslkey", +} - def test_no_deprecated_vars_no_warning( + +class TestDeprecatedDbSettings: + """Test suite for the check_deprecated_db_settings system check.""" + + def test_no_deprecated_vars_returns_empty( self, mocker: MockerFixture, ) -> None: - """Test that no warning is raised when no deprecated vars are set.""" + """No warnings when none of the deprecated vars are present.""" + # clear=True ensures vars from the outer test environment do not leak in mocker.patch.dict(os.environ, {}, clear=True) - - warnings = check_deprecated_db_settings(None) - assert warnings == [] + result = check_deprecated_db_settings(None) + assert result == [] @pytest.mark.parametrize( - ("env_var", "expected_hint_fragment"), + ("env_var", "db_option_key"), [ ("PAPERLESS_DB_TIMEOUT", "timeout"), - ("PAPERLESS_DB_POOLSIZE", "pool.min_size,pool.max_size"), + ("PAPERLESS_DB_POOLSIZE", "pool.min_size / pool.max_size"), ("PAPERLESS_DBSSLMODE", "sslmode"), ("PAPERLESS_DBSSLROOTCERT", "sslrootcert"), ("PAPERLESS_DBSSLCERT", "sslcert"), ("PAPERLESS_DBSSLKEY", "sslkey"), ], + ids=[ + "db-timeout", + "db-poolsize", + "ssl-mode", + "ssl-rootcert", + "ssl-cert", + "ssl-key", + ], ) - def test_deprecated_var_triggers_warning( + def test_single_deprecated_var_produces_one_warning( self, mocker: MockerFixture, env_var: str, - expected_hint_fragment: str, + db_option_key: str, ) -> None: - """Test that each deprecated var triggers appropriate warning.""" + """Each deprecated var in isolation produces exactly one warning.""" mocker.patch.dict(os.environ, {env_var: "some_value"}, clear=True) + result = check_deprecated_db_settings(None) - warnings = check_deprecated_db_settings(None) + assert len(result) == 1 + warning = result[0] + assert isinstance(warning, Warning) + assert warning.id == "paperless.W001" + assert env_var in warning.hint + assert db_option_key in warning.hint - assert len(warnings) == 1 - assert warnings[0].id == "paperless.W001" - assert env_var in warnings[0].hint - assert expected_hint_fragment in warnings[0].hint - assert "v3.2" in warnings[0].hint - - def test_multiple_deprecated_vars( + def test_multiple_deprecated_vars_produce_one_warning_each( self, mocker: MockerFixture, ) -> None: - """Test that multiple deprecated vars are all listed in warning.""" + """Each deprecated var present in the environment gets its own warning.""" + set_vars = { + "PAPERLESS_DB_TIMEOUT": "30", + "PAPERLESS_DB_POOLSIZE": "10", + "PAPERLESS_DBSSLMODE": "require", + } + mocker.patch.dict(os.environ, set_vars, clear=True) + result = check_deprecated_db_settings(None) + + assert len(result) == len(set_vars) + assert all(isinstance(w, Warning) for w in result) + assert all(w.id == "paperless.W001" for w in result) + all_hints = " ".join(w.hint for w in result) + for var_name in set_vars: + assert var_name in all_hints + + def test_all_deprecated_vars_produces_one_warning_each( + self, + mocker: MockerFixture, + ) -> None: + """All deprecated vars set simultaneously produces one warning per var.""" + all_vars = {var: "some_value" for var in DEPRECATED_VARS} + mocker.patch.dict(os.environ, all_vars, clear=True) + result = check_deprecated_db_settings(None) + + assert len(result) == len(DEPRECATED_VARS) + assert all(isinstance(w, Warning) for w in result) + assert all(w.id == "paperless.W001" for w in result) + + def test_unset_vars_not_mentioned_in_warnings( + self, + mocker: MockerFixture, + ) -> None: + """Vars absent from the environment do not appear in any warning.""" mocker.patch.dict( os.environ, - { - "PAPERLESS_DB_TIMEOUT": "30", - "PAPERLESS_DB_POOLSIZE": "10", - "PAPERLESS_DBSSLMODE": "require", - }, + {"PAPERLESS_DB_TIMEOUT": "30"}, clear=True, ) + result = check_deprecated_db_settings(None) - warnings = check_deprecated_db_settings(None) + assert len(result) == 1 + assert "PAPERLESS_DB_TIMEOUT" in result[0].hint + unset_vars = [v for v in DEPRECATED_VARS if v != "PAPERLESS_DB_TIMEOUT"] + for var_name in unset_vars: + assert var_name not in result[0].hint - assert len(warnings) == 1 - assert "PAPERLESS_DB_TIMEOUT" in warnings[0].hint - assert "PAPERLESS_DB_POOLSIZE" in warnings[0].hint - assert "PAPERLESS_DBSSLMODE" in warnings[0].hint + def test_empty_string_var_not_treated_as_set( + self, + mocker: MockerFixture, + ) -> None: + """A var set to an empty string is not flagged as a deprecated setting.""" + mocker.patch.dict( + os.environ, + {"PAPERLESS_DB_TIMEOUT": ""}, + clear=True, + ) + result = check_deprecated_db_settings(None) + assert result == [] + + def test_warning_mentions_migration_target( + self, + mocker: MockerFixture, + ) -> None: + """Each warning hints at PAPERLESS_DB_OPTIONS as the migration target.""" + mocker.patch.dict( + os.environ, + {"PAPERLESS_DBSSLMODE": "require"}, + clear=True, + ) + result = check_deprecated_db_settings(None) + + assert len(result) == 1 + assert "PAPERLESS_DB_OPTIONS" in result[0].hint + + def test_warning_message_identifies_var( + self, + mocker: MockerFixture, + ) -> None: + """The warning message (not just the hint) identifies the offending var.""" + mocker.patch.dict( + os.environ, + {"PAPERLESS_DBSSLCERT": "/path/to/cert.pem"}, + clear=True, + ) + result = check_deprecated_db_settings(None) + + assert len(result) == 1 + assert "PAPERLESS_DBSSLCERT" in result[0].msg + + def test_check_accepts_app_configs_argument( + self, + mocker: MockerFixture, + ) -> None: + """The check function accepts the app_configs argument Django passes.""" + mocker.patch.dict(os.environ, {}, clear=True) + # Django passes app_configs as a list or None depending on invocation + assert check_deprecated_db_settings(None) == [] + assert check_deprecated_db_settings([]) == []