Chore: New helper for JSON parsing during settings, to provide a better error message to users (#14419)

src/paperless/settings/__init__.py

@@ -1,5 +1,4 @@
 import datetime
-import json
 import logging
 import logging.config
 import math
@@ -7,7 +6,6 @@ import multiprocessing
 import os
 import tempfile
 from pathlib import Path
-from typing import Any
 from typing import Final
 from urllib.parse import urlparse
 
@@ -25,6 +23,7 @@ from paperless.settings.parsers import get_bool_from_env
 from paperless.settings.parsers import get_choice_from_env
 from paperless.settings.parsers import get_float_from_env
 from paperless.settings.parsers import get_int_from_env
+from paperless.settings.parsers import get_json_from_env
 from paperless.settings.parsers import get_list_from_env
 from paperless.settings.parsers import get_path_from_env
 
@@ -331,8 +330,10 @@ SOCIALACCOUNT_ALLOW_SIGNUPS = get_bool_from_env(
     "yes",
 )
 SOCIALACCOUNT_AUTO_SIGNUP = get_bool_from_env("PAPERLESS_SOCIAL_AUTO_SIGNUP")
-SOCIALACCOUNT_PROVIDERS = json.loads(
-    os.getenv("PAPERLESS_SOCIALACCOUNT_PROVIDERS", "{}"),
+SOCIALACCOUNT_PROVIDERS = get_json_from_env(
+    "PAPERLESS_SOCIALACCOUNT_PROVIDERS",
+    {},
+    dict,
 )
 SOCIAL_ACCOUNT_DEFAULT_GROUPS = get_list_from_env(
     "PAPERLESS_SOCIAL_ACCOUNT_DEFAULT_GROUPS",
@@ -488,7 +489,7 @@ ALLAUTH_TRUSTED_CLIENT_IP_HEADER = os.getenv(
 USE_X_FORWARDED_HOST = get_bool_from_env("PAPERLESS_USE_X_FORWARD_HOST", "false")
 USE_X_FORWARDED_PORT = get_bool_from_env("PAPERLESS_USE_X_FORWARD_PORT", "false")
 SECURE_PROXY_SSL_HEADER = (
-    tuple(json.loads(os.environ["PAPERLESS_PROXY_SSL_HEADER"]))
+    tuple(get_json_from_env("PAPERLESS_PROXY_SSL_HEADER", [], list))
     if "PAPERLESS_PROXY_SSL_HEADER" in os.environ
     else None
 )
@@ -828,23 +829,17 @@ CONSUMER_DELETE_DUPLICATES = get_bool_from_env("PAPERLESS_CONSUMER_DELETE_DUPLIC
 CONSUMER_RECURSIVE = get_bool_from_env("PAPERLESS_CONSUMER_RECURSIVE")
 
 # Ignore regex patterns, matched against filename only
-CONSUMER_IGNORE_PATTERNS = list(
-    json.loads(
-        os.getenv(
-            "PAPERLESS_CONSUMER_IGNORE_PATTERNS",
-            json.dumps([]),
-        ),
-    ),
+CONSUMER_IGNORE_PATTERNS = get_json_from_env(
+    "PAPERLESS_CONSUMER_IGNORE_PATTERNS",
+    [],
+    list,
 )
 
 # Directories to always ignore.  These are matched by directory name, not full path
-CONSUMER_IGNORE_DIRS = list(
-    json.loads(
-        os.getenv(
-            "PAPERLESS_CONSUMER_IGNORE_DIRS",
-            json.dumps([]),
-        ),
-    ),
+CONSUMER_IGNORE_DIRS = get_json_from_env(
+    "PAPERLESS_CONSUMER_IGNORE_DIRS",
+    [],
+    list,
 )
 
 CONSUMER_SUBDIRS_AS_TAGS = get_bool_from_env("PAPERLESS_CONSUMER_SUBDIRS_AS_TAGS")
@@ -894,13 +889,10 @@ CONSUMER_ENABLE_TAG_BARCODE: Final[bool] = get_bool_from_env(
     "PAPERLESS_CONSUMER_ENABLE_TAG_BARCODE",
 )
 
-CONSUMER_TAG_BARCODE_MAPPING = dict(
-    json.loads(
-        os.getenv(
-            "PAPERLESS_CONSUMER_TAG_BARCODE_MAPPING",
-            '{"TAG:(.*)": "\\\\g<1>"}',
-        ),
-    ),
+CONSUMER_TAG_BARCODE_MAPPING = get_json_from_env(
+    "PAPERLESS_CONSUMER_TAG_BARCODE_MAPPING",
+    {"TAG:(.*)": "\\g<1>"},
+    dict,
 )
 
 CONSUMER_TAG_BARCODE_SPLIT: Final[bool] = get_bool_from_env(
@@ -1088,25 +1080,6 @@ CLASSIFIER_LANGUAGES: Final[dict[str, str]] = {
 }
 
 
-def _get_llm_extra_params() -> dict[str, Any]:
-    """
-    Parse PAPERLESS_AI_LLM_EXTRA_PARAMS, a JSON object passed straight through
-    to the LLM backend's request body.
-    """
-    raw = os.getenv("PAPERLESS_AI_LLM_EXTRA_PARAMS", "{}")
-    try:
-        parsed = json.loads(raw)
-    except json.JSONDecodeError as e:
-        raise ImproperlyConfigured(
-            "PAPERLESS_AI_LLM_EXTRA_PARAMS must be valid JSON",
-        ) from e
-    if not isinstance(parsed, dict):
-        raise ImproperlyConfigured(
-            "PAPERLESS_AI_LLM_EXTRA_PARAMS must be a JSON object",
-        )
-    return parsed
-
-
 def _get_classifier_language_setting(ocr_lang: str) -> str | None:
     """
     Maps the primary Tesseract language to the classifier's stemming
@@ -1268,4 +1241,5 @@ LLM_ALLOW_INTERNAL_ENDPOINTS = get_bool_from_env(
     "PAPERLESS_AI_LLM_ALLOW_INTERNAL_ENDPOINTS",
     "true",
 )
-LLM_EXTRA_PARAMS = _get_llm_extra_params()
+# JSON object passed straight through to the LLM backend's request body
+LLM_EXTRA_PARAMS = get_json_from_env("PAPERLESS_AI_LLM_EXTRA_PARAMS", {}, dict)

src/paperless/settings/parsers.py

@@ -312,3 +312,58 @@ def get_choice_from_env(
         )
 
     return value
+
+
+@overload
+def get_json_from_env(key: str, default: T, expected_type: type[T]) -> T: ...
+
+
+@overload
+def get_json_from_env(
+    key: str,
+    default: Any,
+    expected_type: None = None,
+) -> Any: ...
+
+
+def get_json_from_env(
+    key: str,
+    default: Any,
+    expected_type: type | None = None,
+) -> Any:
+    """
+    Gets an environment variable and parses it as JSON.
+
+    Args:
+        key: The environment variable key
+        default: Python value returned when the variable is not set
+        expected_type: If given, the parsed value must be an instance of it
+
+    Returns:
+        The parsed JSON value, or the default if the variable is not set
+
+    Raises:
+        ValueError: If the value is not valid JSON or is not the expected type.
+                    The message names the offending variable.
+    """
+    raw = os.environ.get(key)
+    if raw is None:
+        return default
+
+    import json
+
+    try:
+        parsed = json.loads(raw)
+    except json.JSONDecodeError as e:
+        raise ValueError(
+            f"Environment variable '{key}' is not valid JSON: {e.msg} "
+            f"(line {e.lineno}, column {e.colno}). Value: {raw!r}",
+        ) from e
+
+    if expected_type is not None and not isinstance(parsed, expected_type):
+        raise ValueError(
+            f"Environment variable '{key}' must be a JSON {expected_type.__name__}, "
+            f"got {type(parsed).__name__}",
+        )
+
+    return parsed

src/paperless/tests/settings/test_environment_parsers.py

@@ -8,6 +8,7 @@ from paperless.settings.parsers import get_bool_from_env
 from paperless.settings.parsers import get_choice_from_env
 from paperless.settings.parsers import get_float_from_env
 from paperless.settings.parsers import get_int_from_env
+from paperless.settings.parsers import get_json_from_env
 from paperless.settings.parsers import get_list_from_env
 from paperless.settings.parsers import get_path_from_env
 from paperless.settings.parsers import parse_dict_from_str
@@ -611,3 +612,43 @@ class TestGetEnvChoice:
         result = get_choice_from_env("TEST_ENV", large_choices)
 
         assert result == "option_50"
+
+
+class TestGetJsonFromEnv:
+    @pytest.mark.parametrize(
+        ("env_value", "expected_type", "expected"),
+        [
+            pytest.param(None, list, ["default"], id="unset-uses-default"),
+            pytest.param('["a", "b"]', list, ["a", "b"], id="list"),
+            pytest.param('["a"]', None, ["a"], id="no-type-check"),
+        ],
+    )
+    def test_returns_value(
+        self,
+        mocker: MockerFixture,
+        env_value: str | None,
+        expected_type: type | None,
+        expected: list[str],
+    ) -> None:
+        env = {} if env_value is None else {"TEST_ENV": env_value}
+        mocker.patch.dict("os.environ", env, clear=True)
+
+        assert get_json_from_env("TEST_ENV", ["default"], expected_type) == expected
+
+    @pytest.mark.parametrize(
+        ("env_value", "match"),
+        [
+            pytest.param('["a", , "b"]', "'TEST_ENV' is not valid JSON", id="invalid"),
+            pytest.param('{"a": 1}', "must be a JSON list, got dict", id="wrong-type"),
+        ],
+    )
+    def test_raises(
+        self,
+        mocker: MockerFixture,
+        env_value: str,
+        match: str,
+    ) -> None:
+        mocker.patch.dict("os.environ", {"TEST_ENV": env_value})
+
+        with pytest.raises(ValueError, match=match):
+            get_json_from_env("TEST_ENV", [], list)

src/paperless/tests/settings/test_settings.py

@@ -7,7 +7,6 @@ from django.core.exceptions import ImproperlyConfigured
 
 from paperless.settings import _get_allauth_trusted_proxy_count
 from paperless.settings import _get_classifier_language_setting
-from paperless.settings import _get_llm_extra_params
 from paperless.settings import _get_search_language_setting
 from paperless.settings import _parse_paperless_url
 from paperless.settings import default_threads_per_worker
@@ -167,45 +166,3 @@ class TestPaperlessURLSettings(TestCase):
 
             self.assertIn(url, settings.CSRF_TRUSTED_ORIGINS)
             self.assertIn(url, settings.CORS_ALLOWED_ORIGINS)
-
-
-class TestLlmExtraParams:
-    @pytest.mark.parametrize(
-        ("env_value", "expected"),
-        [
-            pytest.param(None, {}, id="unset"),
-            pytest.param(
-                '{"reasoning_effort": "none"}',
-                {"reasoning_effort": "none"},
-                id="json-object",
-            ),
-        ],
-    )
-    def test_parses(
-        self,
-        monkeypatch,
-        env_value,
-        expected,
-    ):
-        if env_value is None:
-            monkeypatch.delenv("PAPERLESS_AI_LLM_EXTRA_PARAMS", raising=False)
-        else:
-            monkeypatch.setenv("PAPERLESS_AI_LLM_EXTRA_PARAMS", env_value)
-        assert _get_llm_extra_params() == expected
-
-    @pytest.mark.parametrize(
-        ("env_value", "match"),
-        [
-            pytest.param("reasoning_effort=none", "valid JSON", id="invalid-json"),
-            pytest.param('["none"]', "JSON object", id="not-an-object"),
-        ],
-    )
-    def test_invalid_raises(
-        self,
-        monkeypatch,
-        env_value,
-        match,
-    ):
-        monkeypatch.setenv("PAPERLESS_AI_LLM_EXTRA_PARAMS", env_value)
-        with pytest.raises(ImproperlyConfigured, match=match):
-            _get_llm_extra_params()