From 597e43967c5b6960842e9501af00d2999acb7ea2 Mon Sep 17 00:00:00 2001 From: Roland Date: Fri, 9 Oct 2026 19:40:07 -0600 Subject: [PATCH] API JSON: encode paths without a lookup per path CustomJSONEncoder looked up the media directory and called Path.relative_to for every path it encoded. A response with hundreds of paths (an Archive's page with 51 history entries holds 416) spent most of its encoding time there: 18.8 ms of 19 ms per response, measured on a Mac. The encoder now looks up the media directory once per response and strips it as a string prefix: 1.5 ms for the same response. Paths outside the media directory, relative paths, and siblings that share the media directory's name prefix are still returned unchanged; the media directory itself and "." are still "". Co-Authored-By: Claude Opus 5.5 --- wrolpi/api_utils.py | 26 +++++++++++------ wrolpi/test/test_json_encoder.py | 49 ++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 9 deletions(-) create mode 100644 wrolpi/test/test_json_encoder.py diff --git a/wrolpi/api_utils.py b/wrolpi/api_utils.py index d6a40774..00147edd 100644 --- a/wrolpi/api_utils.py +++ b/wrolpi/api_utils.py @@ -53,6 +53,22 @@ def json_response(*a, **kwargs) -> HTTPResponse: class CustomJSONEncoder(json.JSONEncoder): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + # One encoder serializes one response; look up the media directory once, not once per path. + self._media_prefix = None + + def _relative_path(self, path: Path) -> str: + """The path relative to the media directory, or unchanged if it is outside of it.""" + if self._media_prefix is None: + self._media_prefix = str(get_media_directory()).rstrip('/') + '/' + path = str(path) + if path.startswith(self._media_prefix): + return path[len(self._media_prefix):] + if path == '.' or path + '/' == self._media_prefix: + return '' + return path + def default(self, obj): try: if hasattr(obj, '__json__'): @@ -77,15 +93,7 @@ class CustomJSONEncoder(json.JSONEncoder): if hasattr(obj, 'dict'): return obj.dict() elif isinstance(obj, Path): - media_directory = get_media_directory() - try: - path = obj.relative_to(media_directory) - except ValueError: - # Path may not be absolute. - path = obj - if str(path) == '.': - return '' - return str(path) + return self._relative_path(obj) return super(CustomJSONEncoder, self).default(obj) except Exception as e: logger.fatal(f'Failed to JSON encode {obj}', exc_info=e) diff --git a/wrolpi/test/test_json_encoder.py b/wrolpi/test/test_json_encoder.py new file mode 100644 index 00000000..0daa325d --- /dev/null +++ b/wrolpi/test/test_json_encoder.py @@ -0,0 +1,49 @@ +"""Tests for the API's JSON encoder.""" +import json +from pathlib import Path +from unittest import mock + +import pytest + +from wrolpi import api_utils +from wrolpi.api_utils import CustomJSONEncoder + + +def encode(obj): + return json.loads(json.dumps(obj, cls=CustomJSONEncoder)) + + +@pytest.mark.parametrize('path, expected', [ + ('videos/channel/video.mp4', 'videos/channel/video.mp4'), # Inside the media directory. + ('videos', 'videos'), + ('', ''), # The media directory itself. +]) +def test_paths_in_media_directory_are_relative(test_directory, path, expected): + assert encode(test_directory / path) == expected + + +@pytest.mark.parametrize('path', [ + 'videos/video.mp4', # Already relative. + '/opt/wrolpi/main.py', # Outside the media directory. +]) +def test_other_paths_are_unchanged(test_directory, path): + assert encode(Path(path)) == path + + +def test_relative_dot_is_empty(test_directory): + assert encode(Path('.')) == '' + + +def test_sibling_of_media_directory_is_unchanged(test_directory): + """A directory whose name merely starts with the media directory's name is not inside it.""" + sibling = Path(f'{test_directory}-sibling/video.mp4') + assert encode(sibling) == str(sibling) + + +def test_media_directory_is_looked_up_once_per_response(test_directory): + """A response can hold hundreds of paths; finding the media directory for each one was most of the + encoding time.""" + paths = [test_directory / f'videos/{i}.mp4' for i in range(20)] + with mock.patch.object(api_utils, 'get_media_directory', wraps=api_utils.get_media_directory) as lookup: + assert encode(paths) == [f'videos/{i}.mp4' for i in range(20)] + assert lookup.call_count == 1