From 35b89b0d78c9eba6f45b5f19969021f6f17c4109 Mon Sep 17 00:00:00 2001 From: CaliBrain Date: Mon, 14 Sep 2026 01:30:05 -0400 Subject: [PATCH] fix(sources): restore Direct Download search errors and language matches (#1339) Fixes two regressions from the provider-driven refactor (#1337). First, the composite search caught RuntimeError, TypeError, ValueError and request errors from each provider and returned an empty list, so a failed search looked like one with no hits. It now raises the first provider failure when no provider returned releases. Second, the shared parser re-matched every row's language locally, dropping rows Anna's Archive had already matched with &lang= (free-text cells like 'English, French' or 'unknown'). parse_search_items gains a filter_languages option, which AA turns off, so AA's own language-from-path filter is again the only local one. --- .../direct_download/annas_archive.py | 3 ++ .../release_sources/direct_download/common.py | 10 +++++- .../release_sources/direct_download/source.py | 11 +++--- tests/direct_download/test_search_queries.py | 35 +++++++++++++++++++ tests/direct_download/test_web_providers.py | 25 +++++++++++++ 5 files changed, 79 insertions(+), 5 deletions(-) diff --git a/shelfmark/release_sources/direct_download/annas_archive.py b/shelfmark/release_sources/direct_download/annas_archive.py index c3855c2..6978663 100644 --- a/shelfmark/release_sources/direct_download/annas_archive.py +++ b/shelfmark/release_sources/direct_download/annas_archive.py @@ -768,6 +768,9 @@ def search_books(query: str, filters: SearchFilters) -> list[BrowseRecord]: provider_id="annas_archive", item_selector="tr", extract_item=_extract_aa_search_result, + # AA already applied &lang= server-side; only the path-language pass below + # (which skips &lang=) needs a local language filter. + filter_languages=False, ) if path_language_enabled and requested_langs: diff --git a/shelfmark/release_sources/direct_download/common.py b/shelfmark/release_sources/direct_download/common.py index e134894..f68d77e 100644 --- a/shelfmark/release_sources/direct_download/common.py +++ b/shelfmark/release_sources/direct_download/common.py @@ -188,13 +188,19 @@ def parse_search_items( *, provider_id: str, extract_item: Callable[[Tag], ParsedSearchResult | None], + filter_languages: bool = True, ) -> list[BrowseRecord]: """Normalize provider-specific HTML elements into Direct Download records. Providers only describe how fields are extracted from their DOM. Language and format filtering, stable IDs, and BrowseRecord construction stay shared. + + Pass ``filter_languages=False`` when the site already filtered by language: its + language cells are free text, and re-matching them locally drops rows it matched. """ - requested_languages = normalize_requested_languages(filters.lang) if filters else set() + requested_languages = ( + normalize_requested_languages(filters.lang) if filters and filter_languages else set() + ) requested_formats = ( {value.casefold() for value in (filters.format or get_supported_formats())} if filters @@ -253,6 +259,7 @@ def parse_search_page( provider_id: str, item_selector: str, extract_item: Callable[[Tag], ParsedSearchResult | None], + filter_languages: bool = True, ) -> list[BrowseRecord]: """Parse a result page using provider-specific selectors and extraction.""" root = BeautifulSoup(page, "html.parser") if isinstance(page, str) else page @@ -261,4 +268,5 @@ def parse_search_page( filters, provider_id=provider_id, extract_item=extract_item, + filter_languages=filter_languages, ) diff --git a/shelfmark/release_sources/direct_download/source.py b/shelfmark/release_sources/direct_download/source.py index 9a23bfd..022cd14 100644 --- a/shelfmark/release_sources/direct_download/source.py +++ b/shelfmark/release_sources/direct_download/source.py @@ -173,7 +173,7 @@ class DirectDownloadSource(ReleaseSource): raise DirectDownloadUnavailableError(unavailable_reason) releases: list[Release] = [] - unavailable_errors: list[SourceUnavailableError] = [] + failures: list[Exception] = [] for provider in registry.enabled_providers(self._providers): try: records = provider.search( @@ -183,7 +183,7 @@ class DirectDownloadSource(ReleaseSource): content_type=content_type, ) except SourceUnavailableError as exc: - unavailable_errors.append(exc) + failures.append(exc) continue except ( RuntimeError, @@ -192,11 +192,14 @@ class DirectDownloadSource(ReleaseSource): requests.exceptions.RequestException, ) as exc: logger.warning("%s search failed: %s", provider.display_name, exc) + failures.append(exc) continue releases.extend(_browse_record_to_release(record) for record in records) - if unavailable_errors and not releases: - raise unavailable_errors[0] + # A provider failure is only quiet when another provider answered. Otherwise the + # caller has to see it, or a failed search reads as a search with no hits. + if failures and not releases: + raise failures[0] return releases def is_available(self) -> bool: diff --git a/tests/direct_download/test_search_queries.py b/tests/direct_download/test_search_queries.py index fef3fcd..0a7ad3c 100644 --- a/tests/direct_download/test_search_queries.py +++ b/tests/direct_download/test_search_queries.py @@ -350,3 +350,38 @@ def test_book_matches_requested_languages_logic(): assert aa._book_matches_requested_languages(None, set()) is True assert aa._book_matches_requested_languages("en", {"fr"}) is False assert aa._book_matches_requested_languages("fr", {"fr"}) is True + + +def test_search_books_keeps_server_language_matches_when_path_language_disabled(monkeypatch): + monkeypatch.setattr(aa, "_is_language_from_path_enabled", lambda: False) + monkeypatch.setattr(aa.network, "get_aa_base_url", lambda: "https://mirror.example") + monkeypatch.setattr(aa.network, "AAMirrorSelector", lambda: object()) + + captured_url: dict[str, str] = {} + + def _row(record_id: str, language: str) -> str: + return f""" + + + Book {record_id}Author + Publisher2025 + -- + {language} + fictionpdf + 2 mb + + """ + + def _fake_html_get_page(url: str, selector, **_kwargs): + del selector + captured_url["url"] = url + rows = _row("rec-multi", "English, French") + _row("rec-unknown", "unknown") + return f"{rows}
" + + monkeypatch.setattr(aa.downloader, "html_get_page", _fake_html_get_page) + + records = aa.search_books("demo", SearchFilters(lang=["en"], format=["pdf"])) + + # AA already narrowed by &lang=; its free-text language cells must not be re-matched. + assert "&lang=en" in captured_url["url"] + assert [record.id for record in records] == ["rec-multi", "rec-unknown"] diff --git a/tests/direct_download/test_web_providers.py b/tests/direct_download/test_web_providers.py index 93a7754..c2e16af 100644 --- a/tests/direct_download/test_web_providers.py +++ b/tests/direct_download/test_web_providers.py @@ -183,3 +183,28 @@ def test_provider_failure_is_suppressed_when_another_provider_succeeds(monkeypat releases = source.search(SimpleNamespace(title="Example"), SimpleNamespace()) assert [release.source_id for release in releases] == ["working:1"] + + +def test_provider_search_error_surfaces_when_no_provider_answers(monkeypatch): + from shelfmark.release_sources.direct_download.source import DirectDownloadSource + + class FailingProvider: + id = "failing" + display_name = "Failing" + + def is_enabled(self): + return True + + def search(self, *_args, **_kwargs): + raise RuntimeError("No books found. Please try another query.") + + monkeypatch.setattr( + registry.config, + "get", + lambda key, default=None: True if key == "DIRECT_DOWNLOAD_ENABLED" else default, + ) + source = DirectDownloadSource() + source._providers = (FailingProvider(),) + + with pytest.raises(RuntimeError, match="No books found"): + source.search(SimpleNamespace(title="Example"), SimpleNamespace())