diff --git a/shelfmark/config/env.py b/shelfmark/config/env.py index b01a31d4..df88e55d 100644 --- a/shelfmark/config/env.py +++ b/shelfmark/config/env.py @@ -4,6 +4,7 @@ import json import os import shutil import tempfile +import time from pathlib import Path LOG_LEVELS = ("DEBUG", "INFO", "WARNING", "ERROR", "CRITICAL") @@ -118,14 +119,36 @@ def _is_config_dir_writable() -> bool: return True +# The probe above creates and deletes a file, and cover URLs are rewritten per result, +# per queue task and per history row, so it is remembered for a short time. +_WRITABLE_PROBE_TTL_SECONDS = 60.0 +_writable_probe: tuple[float, bool] | None = None + + +def _config_dir_writable_cached() -> bool: + global _writable_probe + now = time.monotonic() + if _writable_probe is not None and now - _writable_probe[0] < _WRITABLE_PROBE_TTL_SECONDS: + return _writable_probe[1] + writable = _is_config_dir_writable() + _writable_probe = (now, writable) + return writable + + +def reset_config_dir_writable_cache() -> None: + """Forget the remembered probe (tests change the directory under it).""" + global _writable_probe + _writable_probe = None + + def is_covers_cache_enabled() -> bool: """Check if cover caching is enabled (requires setting + writable config dir).""" from shelfmark.core.config import config setting_enabled = config.get("COVERS_CACHE_ENABLED", True) if isinstance(setting_enabled, str): - return string_to_bool(setting_enabled) and _is_config_dir_writable() - return bool(setting_enabled) and _is_config_dir_writable() + return string_to_bool(setting_enabled) and _config_dir_writable_cached() + return bool(setting_enabled) and _config_dir_writable_cached() # ============================================================================= diff --git a/shelfmark/core/config.py b/shelfmark/core/config.py index 7b4c4104..5aa960df 100644 --- a/shelfmark/core/config.py +++ b/shelfmark/core/config.py @@ -124,9 +124,10 @@ class Config: self._field_map.clear() self._cache.clear() - for key, (field, tab_name) in registry.get_settings_field_map().items(): - self._field_map[key] = (field, tab_name) - self._cache[key] = registry.get_setting_value(field, tab_name) + with registry.config_files_read_once(): + for key, (field, tab_name) in registry.get_settings_field_map().items(): + self._field_map[key] = (field, tab_name) + self._cache[key] = registry.get_setting_value(field, tab_name) self._loaded = True diff --git a/shelfmark/core/image_cache.py b/shelfmark/core/image_cache.py index 77af1a66..1245e1ee 100644 --- a/shelfmark/core/image_cache.py +++ b/shelfmark/core/image_cache.py @@ -47,6 +47,11 @@ NEGATIVE_CACHE_TTL = 3600 # Short enough to retry soon, long enough to prevent spam during one page view TRANSIENT_CACHE_TTL = 60 +# A hit only moves the entry's last-access time, which orders LRU eviction. The whole +# index is rewritten to disk, so that is done at most this often instead of per hit. +# A crash inside the window loses some access times, nothing else. +ACCESS_FLUSH_SECONDS = 30 + _MIN_WEBP_HEADER_LENGTH = 12 HTTP_NOT_FOUND = HTTPStatus.NOT_FOUND @@ -102,6 +107,7 @@ class ImageCacheService: self.index_path = cache_dir / "cache_index.json" self._lock = threading.RLock() self._index: dict[str, dict[str, Any]] = {} + self._last_saved = time.monotonic() # Stats tracking self._hits = 0 @@ -195,6 +201,7 @@ class ImageCacheService: with temp_path.open("w") as f: json.dump(self._index, f) temp_path.rename(self.index_path) + self._last_saved = time.monotonic() except OSError: pass @@ -325,9 +332,11 @@ class ImageCacheService: with image_path.open("rb") as f: data = f.read() - # Update accessed time + # Update accessed time. Written out with the next save, or here once + # enough time has passed, not once per hit. entry["accessed_at"] = time.time() - self._save_index() + if time.monotonic() - self._last_saved >= ACCESS_FLUSH_SECONDS: + self._save_index() result = data, content_type except OSError: diff --git a/shelfmark/core/library_index.py b/shelfmark/core/library_index.py index 3e91a980..970ee9ab 100644 --- a/shelfmark/core/library_index.py +++ b/shelfmark/core/library_index.py @@ -25,7 +25,8 @@ from shelfmark.core.text_match import ( author_surname, extra_work_tokens, isbn_variants, - title_tokens_match, + significant_tokens, + significant_tokens_match, ) if TYPE_CHECKING: @@ -128,8 +129,9 @@ def match_entries(book: BookMetadata, entries: list[LibraryEntry]) -> str | None title = book.search_title or book.title surname = author_surname(book.search_author or (book.authors[0] if book.authors else "")) collection: str | None = None + title_toks = significant_tokens(title) for entry in entries: - if not title_tokens_match(title, set(entry.tokens)): + if not significant_tokens_match(title_toks, entry.tokens): continue if surname is not None and surname not in entry.tokens: continue diff --git a/shelfmark/core/settings_registry.py b/shelfmark/core/settings_registry.py index 2081d809..6ea6d605 100644 --- a/shelfmark/core/settings_registry.py +++ b/shelfmark/core/settings_registry.py @@ -2,6 +2,8 @@ import json import os +from contextlib import contextmanager +from contextvars import ContextVar from dataclasses import dataclass, field from pathlib import Path from threading import Lock @@ -419,8 +421,40 @@ def _ensure_config_dir(tab_name: str) -> None: config_path.parent.mkdir(parents=True, exist_ok=True) +# Set while the whole settings cache is being rebuilt. Every field of a tab used to +# re-read and re-parse that tab's file, so a rebuild parsed each file once per field. +_config_file_memo: ContextVar[dict[str, dict[str, Any]] | None] = ContextVar( + "config_file_memo", default=None +) + + +@contextmanager +def config_files_read_once() -> Iterator[None]: + """Read each tab's config file at most once inside the block. + + Only for a read-only pass such as rebuilding the settings cache; a save inside the + block would not be seen by reads after it. + """ + token = _config_file_memo.set({}) + try: + yield + finally: + _config_file_memo.reset(token) + + def load_config_file(tab_name: str) -> dict[str, Any]: """Load a settings tab config file, returning an empty dict on failure.""" + memo = _config_file_memo.get() + if memo is None: + return _read_config_file(tab_name) + config_path = _get_config_file_path(tab_name) + key = str(config_path) # several tabs share settings.json + if key not in memo: + memo[key] = _read_config_file(tab_name) + return memo[key] + + +def _read_config_file(tab_name: str) -> dict[str, Any]: config_path = _get_config_file_path(tab_name) if not config_path.exists(): diff --git a/shelfmark/core/text_match.py b/shelfmark/core/text_match.py index 4c66fa07..375c36bb 100644 --- a/shelfmark/core/text_match.py +++ b/shelfmark/core/text_match.py @@ -7,6 +7,10 @@ library matches titles, authors and ISBNs the same way. from __future__ import annotations import re +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from collections.abc import Collection DEFAULT_TITLE_MATCH_THRESHOLD = 0.85 @@ -57,7 +61,18 @@ def title_tokens_match( threshold: float = DEFAULT_TITLE_MATCH_THRESHOLD, ) -> bool: """True when enough significant title tokens appear in ``haystack_tokens``.""" - title_toks = significant_tokens(title) + return significant_tokens_match(significant_tokens(title), haystack_tokens, threshold) + + +def significant_tokens_match( + title_toks: list[str], + haystack_tokens: Collection[str], + threshold: float = DEFAULT_TITLE_MATCH_THRESHOLD, +) -> bool: + """``title_tokens_match`` for a title already run through ``significant_tokens``. + + Matching one title against many entries should tokenise it once, not per entry. + """ if not title_toks: return False present = sum(1 for tok in title_toks if tok in haystack_tokens) diff --git a/tests/config/test_docker_volumes.py b/tests/config/test_docker_volumes.py index dd96148a..31154fd8 100644 --- a/tests/config/test_docker_volumes.py +++ b/tests/config/test_docker_volumes.py @@ -104,3 +104,20 @@ def test_save_config_file_returns_false_when_config_dir_is_not_writable(tmp_path assert save_config_file("downloads", {"key": "value"}) is False finally: config_dir.chmod(stat.S_IRWXU) + + +def test_covers_cache_probe_is_remembered_for_a_short_time(monkeypatch): + from shelfmark.config import env + + probes: list[int] = [] + monkeypatch.setattr(env, "_is_config_dir_writable", lambda: (probes.append(1), True)[1]) + clock = [1000.0] + monkeypatch.setattr(env.time, "monotonic", lambda: clock[0]) + + for _ in range(5): + assert env._config_dir_writable_cached() is True + assert probes == [1] + + clock[0] += env._WRITABLE_PROBE_TTL_SECONDS + 1 + assert env._config_dir_writable_cached() is True + assert probes == [1, 1] diff --git a/tests/conftest.py b/tests/conftest.py index de13e702..60cc8952 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -29,6 +29,16 @@ sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) import pytest +@pytest.fixture(autouse=True) +def _reset_config_dir_writable_cache(): + """The writable-config-dir probe is remembered; each test starts without it.""" + from shelfmark.config.env import reset_config_dir_writable_cache + + reset_config_dir_writable_cache() + yield + reset_config_dir_writable_cache() + + @pytest.fixture(autouse=True) def _clear_torrent_fetch_cache(): """Keep the shared torrent fetch cache from leaking between tests.""" diff --git a/tests/core/test_image_cache.py b/tests/core/test_image_cache.py index e0a3fd98..7bd4e087 100644 --- a/tests/core/test_image_cache.py +++ b/tests/core/test_image_cache.py @@ -84,3 +84,40 @@ def test_cover_cache_key_depends_on_the_url_only() -> None: assert key != cover_cache_key("https://example.com/cover.jpg?size=large") assert key.isalnum() and key == key.lower() assert "/" not in key and "." not in key + + +def test_cache_hits_do_not_rewrite_the_index_each_time(tmp_path, monkeypatch) -> None: + """A hit only moves the last-access time, so the index is flushed on a timer.""" + cache = ImageCacheService(tmp_path) + assert cache.put("cover-a", b"\xff\xd8\xff" + b"0" * 64, "image/jpeg") + saves: list[int] = [] + real_save = cache._save_index + monkeypatch.setattr(cache, "_save_index", lambda: (saves.append(1), real_save())[1]) + + before = cache._index["cover-a"]["accessed_at"] + for _ in range(5): + assert cache.get("cover-a") is not None + + assert saves == [] + assert cache._index["cover-a"]["accessed_at"] >= before + + +def test_cache_hit_flushes_the_index_once_the_window_has_passed(tmp_path, monkeypatch) -> None: + from shelfmark.core import image_cache + + cache = ImageCacheService(tmp_path) + assert cache.put("cover-a", b"\xff\xd8\xff" + b"0" * 64, "image/jpeg") + saves: list[int] = [] + real_save = cache._save_index + monkeypatch.setattr(cache, "_save_index", lambda: (saves.append(1), real_save())[1]) + + now = cache._last_saved + monkeypatch.setattr( + image_cache.time, "monotonic", lambda: now + image_cache.ACCESS_FLUSH_SECONDS + ) + assert cache.get("cover-a") is not None + assert saves == [1] + + # The flush restarts the window. + assert cache.get("cover-a") is not None + assert saves == [1] diff --git a/tests/core/test_library_index.py b/tests/core/test_library_index.py index 08d55d91..1197a484 100644 --- a/tests/core/test_library_index.py +++ b/tests/core/test_library_index.py @@ -466,3 +466,21 @@ def test_an_exact_shelf_title_outranks_a_collection_holding_the_same_book() -> N book = _book(title="Dungeon Crawler Carl", authors=["Matt Dinniman"]) assert library_index.match_entries(book, [collection, single]) == "owned" + + +def test_matching_tokenises_the_title_once_for_all_entries(monkeypatch: pytest.MonkeyPatch) -> None: + """The title does not change between entries, so it is split into words once.""" + from shelfmark.core import text_match + + calls: list[str | None] = [] + real = text_match.significant_tokens + + def counting(text: str | None) -> list[str]: + calls.append(text) + return real(text) + + monkeypatch.setattr(text_match, "significant_tokens", counting) + entries = [_entry("unrelated", f"word{i}", "someone") for i in range(50)] + + assert library_index.match_entries(_book(), entries) is None + assert len(calls) <= 2 diff --git a/tests/core/test_settings_reload.py b/tests/core/test_settings_reload.py new file mode 100644 index 00000000..89b8d4cb --- /dev/null +++ b/tests/core/test_settings_reload.py @@ -0,0 +1,46 @@ +"""Rebuilding the settings cache parses each tab's config file once.""" + +from __future__ import annotations + +import json +from pathlib import Path + +from shelfmark.core import settings_registry +from shelfmark.core.config import config as app_config + + +def test_refresh_reads_each_config_file_once(tmp_path, monkeypatch): + plugins = tmp_path / "plugins" + plugins.mkdir() + (tmp_path / "settings.json").write_text(json.dumps({"SEARCH_MODE": "universal"})) + (plugins / "downloads.json").write_text(json.dumps({"MAX_CONCURRENT_DOWNLOADS": 3})) + (plugins / "hardcover.json").write_text(json.dumps({"HARDCOVER_ENABLED": False})) + + opened: dict[str, int] = {} + real_open = Path.open + + def counting_open(self, *args, **kwargs): + if self.suffix == ".json" and self.parent in (tmp_path, plugins): + opened[self.name] = opened.get(self.name, 0) + 1 + return real_open(self, *args, **kwargs) + + with monkeypatch.context() as patched: + patched.setattr(settings_registry, "_get_config_dir", lambda: tmp_path) + patched.setattr(Path, "open", counting_open) + app_config.refresh(force=True) + + app_config.refresh(force=True) + assert opened, "no config file was read" + assert max(opened.values()) == 1, opened + + +def test_config_files_are_read_fresh_outside_the_block(tmp_path, monkeypatch): + monkeypatch.setattr(settings_registry, "_get_config_dir", lambda: tmp_path) + (tmp_path / "settings.json").write_text(json.dumps({"A": 1})) + + with settings_registry.config_files_read_once(): + assert settings_registry.load_config_file("general") == {"A": 1} + (tmp_path / "settings.json").write_text(json.dumps({"A": 2})) + assert settings_registry.load_config_file("general") == {"A": 1} + + assert settings_registry.load_config_file("general") == {"A": 2} diff --git a/tests/core/test_text_match.py b/tests/core/test_text_match.py index cf2fcd34..559b218d 100644 --- a/tests/core/test_text_match.py +++ b/tests/core/test_text_match.py @@ -75,3 +75,11 @@ def test_title_tokens_match_without_significant_tokens_is_false(title): ) def test_normalize_isbn(value, expected): assert text_match.normalize_isbn(value) == expected + + +def test_significant_tokens_match_agrees_with_title_tokens_match(): + haystack = {"alpha", "beta", "gamma"} + for title in ("Alpha Beta", "alpha delta", "the of", ""): + assert text_match.significant_tokens_match( + text_match.significant_tokens(title), haystack + ) is text_match.title_tokens_match(title, haystack)