Makes the system checks return multiples, not mashed together

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.
+    PAPERLESS_DB_OPTIONS. Returns one Warning per deprecated variable found.
-
-    Returns:
-        List of Django Warning instances for any deprecated vars found.
     """
-    deprecated_vars = {
+    deprecated_vars: dict[str, str] = {
-        "PAPERLESS_DB_TIMEOUT": "timeout (or connect_timeout for Postgres/MariaDB)",
+        "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 (or ssl_mode for MariaDB)",
+        "PAPERLESS_DBSSLMODE": "sslmode",
-        "PAPERLESS_DBSSLROOTCERT": "sslrootcert (or ssl.ca for MariaDB)",
+        "PAPERLESS_DBSSLROOTCERT": "sslrootcert",
-        "PAPERLESS_DBSSLCERT": "sslcert (or ssl.cert for MariaDB)",
+        "PAPERLESS_DBSSLCERT": "sslcert",
-        "PAPERLESS_DBSSLKEY": "sslkey (or ssl.key for MariaDB)",
+        "PAPERLESS_DBSSLKEY": "sslkey",
     }
 
-    found_vars = []
+    warnings: list[Warning] = []
-    for var_name in deprecated_vars:
-        if os.getenv(var_name):
-            found_vars.append(var_name)
 
-    if not found_vars:
+    for var_name, db_option_key in deprecated_vars.items():
-        return []
+        if not os.getenv(var_name):
-
+            continue
-    # Build migration example
+        warnings.append(
-    examples = []
+            Warning(
-    for var in found_vars:
+                f"Deprecated environment variable: {var_name}",
-        examples.append(f"{var} -> PAPERLESS_DB_OPTIONS={deprecated_vars[var]}=<value>")
+                hint=(
-
+                    f"{var_name} is no longer supported and will be removed in v3.2. "
-    return [
+                    f"Set the equivalent option via PAPERLESS_DB_OPTIONS instead. "
-        Warning(
+                    f'Example: PAPERLESS_DB_OPTIONS=\'{{"{db_option_key}": "<value>"}}\'. '
-            "Deprecated database environment variables detected",
+                    "See https://docs.paperless-ngx.com/migration/ for the full reference."
-            # TODO: Need to check this URL
+                ),
-            hint=(
+                id="paperless.W001",
-                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/"
             ),
-            id="paperless.W001",
+        )
-        ),
+
-    ]
+    return warnings

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:
+DEPRECATED_VARS: dict[str, str] = {
-    """Test suite for deprecated database settings system check."""
+    "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)
-
+        result = check_deprecated_db_settings(None)
-        warnings = check_deprecated_db_settings(None)
+        assert result == []
-        assert warnings == []
 
     @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
+    def test_multiple_deprecated_vars_produce_one_warning_each(
-        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(
         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_TIMEOUT": "30",
-                "PAPERLESS_DB_POOLSIZE": "10",
-                "PAPERLESS_DBSSLMODE": "require",
-            },
             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
+    def test_empty_string_var_not_treated_as_set(
-        assert "PAPERLESS_DB_TIMEOUT" in warnings[0].hint
+        self,
-        assert "PAPERLESS_DB_POOLSIZE" in warnings[0].hint
+        mocker: MockerFixture,
-        assert "PAPERLESS_DBSSLMODE" in warnings[0].hint
+    ) -> 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([]) == []