From adc409404a2a9294a0d38a2eaea362ecacc8a3ff Mon Sep 17 00:00:00 2001 From: Dariusz L Date: Fri, 7 Aug 2026 17:24:53 +0200 Subject: [PATCH] refactor: unify optional JSON object payload parsing --- core/request_utils.py | 13 +++++++- core/routes/helpers.py | 13 ++++---- core/services/download_service.py | 10 ++---- core/services/metadata_service.py | 14 ++------ tests/test_request_utils.py | 55 +++++++++++++++++++++++++++++++ 5 files changed, 79 insertions(+), 26 deletions(-) create mode 100644 tests/test_request_utils.py diff --git a/core/request_utils.py b/core/request_utils.py index 0eebed9..49691ea 100644 --- a/core/request_utils.py +++ b/core/request_utils.py @@ -1,8 +1,19 @@ -"""Helpers for validating common request payload fields.""" +"""Helpers for validating and reading common request payload fields.""" from typing import Any, Optional, Tuple +async def read_optional_object_payload(request: Any) -> dict[str, Any]: + """Read an optional JSON object, returning an empty dict on invalid input.""" + if not getattr(request, "can_read_body", True): + return {} + try: + payload = await request.json() + except Exception: + return {} + return payload if isinstance(payload, dict) else {} + + def validate_workflow_payload( workflow: Any, *, diff --git a/core/routes/helpers.py b/core/routes/helpers.py index cd7ec04..c716214 100644 --- a/core/routes/helpers.py +++ b/core/routes/helpers.py @@ -4,6 +4,8 @@ import asyncio import threading from functools import wraps +from ..request_utils import read_optional_object_payload + def register_service_route( context, @@ -165,13 +167,10 @@ def create_route_helpers(web, logger, load_settings, hash_calculation_cancelled) async def get_override_settings_from_request(request): settings = await asyncio.to_thread(load_settings) if request.method == "POST": - try: - payload = await request.json() - if isinstance(payload, dict) and "aria2c_path" in payload: - settings = dict(settings) - settings["aria2c_path"] = payload.get("aria2c_path", "") - except Exception: - pass + payload = await read_optional_object_payload(request) + if "aria2c_path" in payload: + settings = dict(settings) + settings["aria2c_path"] = payload.get("aria2c_path", "") return settings return ( diff --git a/core/services/download_service.py b/core/services/download_service.py index c9ff1bb..506a7dc 100644 --- a/core/services/download_service.py +++ b/core/services/download_service.py @@ -3,6 +3,7 @@ import os from urllib.parse import unquote, urlparse +from ..request_utils import read_optional_object_payload from ..routes.context import RouteContext @@ -307,13 +308,8 @@ class DownloadService: async def aria2_install(self, request): """Install aria2 and persist the selected download backend.""" - try: - payload = await request.json() - except Exception: - payload = {} - force = False - if isinstance(payload, dict): - force = self.to_bool(payload.get("force"), False) + payload = await read_optional_object_payload(request) + force = self.to_bool(payload.get("force"), False) try: install_result = await self.asyncio.to_thread( self.install_aria2_engine, diff --git a/core/services/metadata_service.py b/core/services/metadata_service.py index a792e10..68d889b 100644 --- a/core/services/metadata_service.py +++ b/core/services/metadata_service.py @@ -2,6 +2,7 @@ import uuid +from ..request_utils import read_optional_object_payload from ..routes.context import RouteContext @@ -34,18 +35,9 @@ class MetadataService: def metadata_builder_progress(self): return self.extension.metadata_builder_progress - async def _read_optional_object_payload(self, request): - if not request.can_read_body: - return {} - try: - payload = await request.json() - except Exception: - return {} - return payload if isinstance(payload, dict) else {} - async def metadata_size_audit(self, request): """Check local metadata sidecars for stale file-size values.""" - payload = await self._read_optional_object_payload(request) + payload = await read_optional_object_payload(request) force_rescan = self.to_bool(payload.get("force_rescan"), True) result = await self.asyncio.to_thread( @@ -64,7 +56,7 @@ class MetadataService: async def metadata_build_start(self, request): """Start building missing local metadata sidecars in the background.""" - payload = await self._read_optional_object_payload(request) + payload = await read_optional_object_payload(request) force_rescan = self.to_bool(payload.get("force_rescan"), True) worker_count = self.to_int(payload.get("worker_count"), 0) diff --git a/tests/test_request_utils.py b/tests/test_request_utils.py new file mode 100644 index 0000000..ce46ebb --- /dev/null +++ b/tests/test_request_utils.py @@ -0,0 +1,55 @@ +from types import SimpleNamespace +from unittest.mock import AsyncMock + +import pytest + +from core.request_utils import read_optional_object_payload + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + ("payload", "expected"), + [ + ({"force": True}, {"force": True}), + ([], {}), + ("invalid", {}), + (None, {}), + ], +) +async def test_read_optional_object_payload_normalizes_json_values( + payload, expected +): + request = SimpleNamespace( + can_read_body=True, + json=AsyncMock(return_value=payload), + ) + + assert await read_optional_object_payload(request) == expected + + +@pytest.mark.asyncio +async def test_read_optional_object_payload_ignores_json_parse_errors(): + request = SimpleNamespace( + can_read_body=True, + json=AsyncMock(side_effect=ValueError("invalid json")), + ) + + assert await read_optional_object_payload(request) == {} + + +@pytest.mark.asyncio +async def test_read_optional_object_payload_skips_unreadable_body(): + request = SimpleNamespace( + can_read_body=False, + json=AsyncMock(), + ) + + assert await read_optional_object_payload(request) == {} + request.json.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_read_optional_object_payload_supports_requests_without_body_flag(): + request = SimpleNamespace(json=AsyncMock(return_value={"value": 1})) + + assert await read_optional_object_payload(request) == {"value": 1}