From a1367f431d9e2ffc72ee97a03e14c35819144fb7 Mon Sep 17 00:00:00 2001 From: CaliBrain Date: Tue, 28 Jul 2026 01:23:31 -0400 Subject: [PATCH] Ship Prowlarr per-entry rows as opt-in, not the default (#1145) #1140 fixed the guid-only dedup that hid results from filter-specific indexer entries, but shipped the new behaviour on by default: PROWLARR_COLLAPSE_DUPLICATES defaulted off, so every existing Prowlarr user got extra rows for any release that two indexer entries both returned, and the setting only let them opt back into what they already had. Default it on. The dedup key stays indexer-qualified, so the entries are still distinct internally; collapse then merges them back to one row, resolved by the Prowlarr priority rather than by query order as before. The visible result set matches what users had prior to #1140, and anyone who wants the per-entry rows (freeleech and the like) turns the setting off. Beyond the noisier list, the default mattered because split rows differ only by indexer name while sharing a title, size and peer count. Two of them have distinct source_ids, so the queue's duplicate guard does not fire, and the second grab's find_existing() matches the first by infohash and runs post-processing over the same download again, delivering the book twice. The search-side fallback in config.get(..., True) is flipped to agree with the field default. In production the field default governs, since the config cache is seeded from the registry and the fallback only applies to an unregistered key. The source tests monkeypatch config.get with a plain dict lookup, though, so the fallback is what they exercise: leaving it False would have kept every default-behaviour test asserting the opposite of what ships. A new test pins the two together. The deduplication tests now opt out explicitly, since they assert the split itself. test_collapse_off_by_default_keeps_both_rows becomes a pair, one for the untouched setting collapsing to a single row and one for opting out. docs/environment-variables.md is regenerated rather than hand-edited. It was already stale on main, so it also picks up RTORRENT_AUDIOBOOK_LABEL, DIRECT_DOWNLOAD_LANGUAGE_FROM_PATH, and reworded IRC_SEARCH_BOT and RTORRENT_LABEL text from earlier merges. The rest is fallout from the ruff 0.16.0 bump in #1139, which enabled a much larger default rule set and started formatting Python code blocks in Markdown: _find_existing_alias_user() uses min() instead of sorted()[0] (FURB192, currently failing Python Quality on main), and the two READMEs get their code blocks reformatted. --- docs/environment-variables.md | 34 +++++++++++---- shelfmark/core/external_user_linking.py | 2 +- shelfmark/metadata_providers/README.md | 31 +++++++------- .../release_sources/prowlarr/settings.py | 9 ++-- shelfmark/release_sources/prowlarr/source.py | 2 +- tests/README.md | 2 + tests/prowlarr/test_source.py | 41 +++++++++++++++++-- 7 files changed, 90 insertions(+), 31 deletions(-) diff --git a/docs/environment-variables.md b/docs/environment-variables.md index bff0bd7..7e845b5 100644 --- a/docs/environment-variables.md +++ b/docs/environment-variables.md @@ -1211,7 +1211,7 @@ How long to cache individual book details. Default: 600 (10 minutes). Max: 60480 | `PROWLARR_API_KEY` | Found in Prowlarr: Settings > General > API Key | string (secret) | _none_ | | `PROWLARR_INDEXERS` | Select which indexers to search. 📚 = has book categories. Leave empty to search all. | string (comma-separated) | _empty list_ | | `PROWLARR_AUTO_EXPAND` | Automatically retry search without category filtering if no results are found | boolean | `false` | -| `PROWLARR_COLLAPSE_DUPLICATES` | Collapse a release that several indexer entries returned down to a single row. Leave this off to see every entry that carried it, which is what makes results from filter-specific entries (freeleech and the like) visible. | boolean | `false` | +| `PROWLARR_COLLAPSE_DUPLICATES` | Collapse a release that several indexer entries returned down to a single row, keeping the entry with the best Prowlarr priority. Turn this off to see every entry that carried it, which is what makes results from filter-specific entries (freeleech and the like) visible. | boolean | `true` | | `PROWLARR_USE_SEED_PREFERENCES` | Apply per-indexer seed time and ratio preferences from Prowlarr when sending torrents to the download client | boolean | `false` |
@@ -1268,10 +1268,10 @@ Automatically retry search without category filtering if no results are found **Show one row per release** -Collapse a release that several indexer entries returned down to a single row. Leave this off to see every entry that carried it, which is what makes results from filter-specific entries (freeleech and the like) visible. +Collapse a release that several indexer entries returned down to a single row, keeping the entry with the best Prowlarr priority. Turn this off to see every entry that carried it, which is what makes results from filter-specific entries (freeleech and the like) visible. - **Type:** boolean -- **Default:** `false` +- **Default:** `true` #### `PROWLARR_USE_SEED_PREFERENCES` @@ -1407,7 +1407,7 @@ Delay between requests in seconds to avoid rate limiting (0-10). | `IRC_USE_TLS` | Enable TLS/SSL encryption for the IRC connection. Disable for servers that don't support TLS. | boolean | `true` | | `IRC_CHANNEL` | Channel name without the # prefix. Used for all searches unless a separate audiobook channel is configured below. | string | _none_ | | `IRC_NICK` | Your IRC nickname (required). Must be unique on the IRC network. | string | _none_ | -| `IRC_SEARCH_BOT` | The search bot to address queries to (required). | string | _none_ | +| `IRC_SEARCH_BOT` | The search bot to address queries to (required). Searches are sent as "@ ". | string | _none_ | | `IRC_AUDIOBOOK_CHANNEL` | Optional. Channel name (without the # prefix) to use for audiobook searches. Leave blank to use the main channel above for audiobooks too. | string | _none_ | | `IRC_AUDIOBOOK_SEARCH_BOT` | Optional. Search bot for the audiobook channel. Leave blank to reuse the main search bot above. Only used when an audiobook channel is set. | string | _none_ | | `IRC_CACHE_TTL` | How long to keep cached search results before they expire. | string (choice) | `2592000` | @@ -1467,7 +1467,7 @@ Your IRC nickname (required). Must be unique on the IRC network. **Search bot** -The search bot to address queries to (required). Searches are sent as "@ ". Without it, queries would be posted unaddressed to the channel. +The search bot to address queries to (required). Searches are sent as "@ ". - **Type:** string - **Default:** _none_ @@ -1531,7 +1531,8 @@ How long to keep cached search results before they expire. | `RTORRENT_URL` | XML-RPC URL of your rTorrent instance | string | _none_ | | `RTORRENT_USERNAME` | HTTP Basic auth username (if authentication enabled) | string | _none_ | | `RTORRENT_PASSWORD` | HTTP Basic auth password | string (secret) | _none_ | -| `RTORRENT_LABEL` | Label to assign to book downloads in rTorrent | string | `cwabd` | +| `RTORRENT_LABEL` | Label to assign to ebook downloads in rTorrent | string | `cwabd` | +| `RTORRENT_AUDIOBOOK_LABEL` | Label to assign to audiobook downloads in rTorrent (falls back to Book Label if not set) | string | _none_ | | `RTORRENT_DOWNLOAD_DIR` | Server-side directory where torrents are downloaded (optional, uses rTorrent default if not specified) | string | _none_ | | `PROWLARR_TORRENT_ACTION` | Remove deletes the torrent from your client immediately after import (stops seeding, files are kept); Keep leaves it in the client to continue seeding | string (choice) | `keep` | | `PROWLARR_USENET_CLIENT` | Choose which usenet client to use | string (choice) | _empty string_ | @@ -1770,11 +1771,20 @@ HTTP Basic auth password **Book Label** -Label to assign to book downloads in rTorrent +Label to assign to ebook downloads in rTorrent - **Type:** string - **Default:** `cwabd` +#### `RTORRENT_AUDIOBOOK_LABEL` + +**Audiobook Label** + +Label to assign to audiobook downloads in rTorrent (falls back to Book Label if not set) + +- **Type:** string +- **Default:** _none_ + #### `RTORRENT_DOWNLOAD_DIR` **Download Directory** @@ -2051,6 +2061,7 @@ Default sort order for Google Books search results. | Variable | Description | Type | Default | |----------|-------------|------|---------| | `DIRECT_DOWNLOAD_ENABLED` | Show Direct Download in release-source lists and allow Direct mode searches. Add your own mirror URLs in the Mirrors tab before using it. | boolean | `false` | +| `DIRECT_DOWNLOAD_LANGUAGE_FROM_PATH` | When language metadata is missing or unknown, parse the distant path (file path shown in search results) for language tags like [BD FR] or [En]. Also enables local language filtering so lgli files without AA language metadata are not excluded before the distant path can be checked. | boolean | `false` | | `AA_DONATOR_KEY` | Enables fast download access on AA. Get this from your donator account page. | string (secret) | _none_ | | `FAST_SOURCES_DISPLAY` | Always tried first, no waiting or bypass required. | JSON array | _see UI for defaults_ | | `SOURCE_PRIORITY` | Fallback sources, may have waiting. Requires bypasser. Drag to reorder. | JSON array | _see UI for defaults_ | @@ -2078,6 +2089,15 @@ Show Direct Download in release-source lists and allow Direct mode searches. Add - **Type:** boolean - **Default:** `false` +#### `DIRECT_DOWNLOAD_LANGUAGE_FROM_PATH` + +**Detect Language From Distant Path** + +When language metadata is missing or unknown, parse the distant path (file path shown in search results) for language tags like [BD FR] or [En]. Also enables local language filtering so lgli files without AA language metadata are not excluded before the distant path can be checked. + +- **Type:** boolean +- **Default:** `false` + #### `AA_DONATOR_KEY` **Account Donator Key** diff --git a/shelfmark/core/external_user_linking.py b/shelfmark/core/external_user_linking.py index 5f23fde..75cb3f8 100644 --- a/shelfmark/core/external_user_linking.py +++ b/shelfmark/core/external_user_linking.py @@ -149,7 +149,7 @@ def _find_existing_alias_user( ] if not candidates: return None - return sorted(candidates, key=lambda user: int(user.get("id") or 0))[0] + return min(candidates, key=lambda user: int(user.get("id") or 0), default=None) def _resolve_create_username( diff --git a/shelfmark/metadata_providers/README.md b/shelfmark/metadata_providers/README.md index 8124d67..5a78944 100644 --- a/shelfmark/metadata_providers/README.md +++ b/shelfmark/metadata_providers/README.md @@ -24,12 +24,12 @@ Dataclass representing a book from a metadata provider: ```python @dataclass class BookMetadata: - provider: str # Internal provider name (e.g., "hardcover") - provider_id: str # ID in that provider's system + provider: str # Internal provider name (e.g., "hardcover") + provider_id: str # ID in that provider's system title: str # Optional fields - provider_display_name: str # Human-readable name (e.g., "Hardcover") + provider_display_name: str # Human-readable name (e.g., "Hardcover") authors: List[str] isbn_10: str isbn_13: str @@ -39,7 +39,7 @@ class BookMetadata: publish_year: int language: str genres: List[str] - source_url: str # Link to book on provider's site + source_url: str # Link to book on provider's site display_fields: List[DisplayField] # Provider-specific display data ``` @@ -50,9 +50,9 @@ Provider-specific metadata for UI cards (ratings, page counts, reader counts, et ```python @dataclass class DisplayField: - label: str # e.g., "Rating", "Pages", "Readers" - value: str # e.g., "4.5", "496", "8,041" - icon: str # Icon name: "star", "book", "users", "editions" + label: str # e.g., "Rating", "Pages", "Readers" + value: str # e.g., "4.5", "496", "8,041" + icon: str # Icon name: "star", "book", "users", "editions" ``` ### MetadataSearchOptions @@ -64,7 +64,7 @@ Unified search options that work across all providers: class MetadataSearchOptions: query: str search_type: SearchType = SearchType.GENERAL # GENERAL, TITLE, AUTHOR, ISBN - language: str = None # ISO 639-1 code (e.g., "en") + language: str = None # ISO 639-1 code (e.g., "en") sort: SortOrder = SortOrder.RELEVANCE limit: int = 40 page: int = 1 @@ -88,10 +88,10 @@ All providers must implement this interface: ```python class MetadataProvider(ABC): - name: str # Internal identifier - display_name: str # Human-readable name - requires_auth: bool # True if API key required - supported_sorts: List[SortOrder] # Supported sort options + name: str # Internal identifier + display_name: str # Human-readable name + requires_auth: bool # True if API key required + supported_sorts: List[SortOrder] # Supported sort options @abstractmethod def search(self, options: MetadataSearchOptions) -> List[BookMetadata]: @@ -121,9 +121,9 @@ class MetadataProvider(ABC): ```python from shelfmark.metadata_providers import register_provider + @register_provider("my_provider") -class MyProvider(MetadataProvider): - ... +class MyProvider(MetadataProvider): ... ``` ### Getting Providers @@ -281,11 +281,13 @@ from shelfmark.config.env import ( METADATA_CACHE_BOOK_TTL, ) + @cacheable(ttl=METADATA_CACHE_SEARCH_TTL, key_prefix="myprovider:search") def _search_cached(self, cache_key: str, options: MetadataSearchOptions): # Cached search implementation pass + @cacheable(ttl=METADATA_CACHE_BOOK_TTL, key_prefix="myprovider:book") def get_book(self, book_id: str): # Cached book lookup @@ -302,6 +304,7 @@ from shelfmark.metadata_providers.openlibrary import RateLimiter # 90 requests per 60 seconds rate_limiter = RateLimiter(max_requests=90, window_seconds=60) + def make_request(self): rate_limiter.wait_if_needed() # Blocks if rate limited # ... make request diff --git a/shelfmark/release_sources/prowlarr/settings.py b/shelfmark/release_sources/prowlarr/settings.py index 900f1c1..e3c4c30 100644 --- a/shelfmark/release_sources/prowlarr/settings.py +++ b/shelfmark/release_sources/prowlarr/settings.py @@ -193,11 +193,12 @@ def prowlarr_config_settings() -> list[SettingsField]: CheckboxField( key="PROWLARR_COLLAPSE_DUPLICATES", label="Show one row per release", - default=False, + default=True, description=( - "Collapse a release that several indexer entries returned down to a single row. " - "Leave this off to see every entry that carried it, which is what makes results " - "from filter-specific entries (freeleech and the like) visible." + "Collapse a release that several indexer entries returned down to a single row, " + "keeping the entry with the best Prowlarr priority. Turn this off to see every " + "entry that carried it, which is what makes results from filter-specific entries " + "(freeleech and the like) visible." ), show_when={"field": "PROWLARR_ENABLED", "value": True}, ), diff --git a/shelfmark/release_sources/prowlarr/source.py b/shelfmark/release_sources/prowlarr/source.py index b406646..904c05e 100644 --- a/shelfmark/release_sources/prowlarr/source.py +++ b/shelfmark/release_sources/prowlarr/source.py @@ -1018,7 +1018,7 @@ class ProwlarrSource(ReleaseSource): seen_keys.add(key) all_results.append(r) - if config.get("PROWLARR_COLLAPSE_DUPLICATES", False): + if config.get("PROWLARR_COLLAPSE_DUPLICATES", True): before_collapse = len(all_results) all_results = _collapse_duplicate_indexer_results(all_results, indexer_priority) if len(all_results) != before_collapse: diff --git a/tests/README.md b/tests/README.md index 7f1e22e..d4abfdf 100644 --- a/tests/README.md +++ b/tests/README.md @@ -169,6 +169,7 @@ uv run pytest tests/ --cov=shelfmark -m "not integration" ```python from unittest.mock import MagicMock, patch + class TestMyFeature: def test_something(self, monkeypatch): # Mock config values @@ -188,6 +189,7 @@ class TestMyFeature: import pytest from .conftest import APIClient, DownloadTracker + @pytest.mark.e2e class TestMyEndpoint: def test_endpoint_works(self, protected_api_client: APIClient): diff --git a/tests/prowlarr/test_source.py b/tests/prowlarr/test_source.py index 4bf330c..d3e6a0d 100644 --- a/tests/prowlarr/test_source.py +++ b/tests/prowlarr/test_source.py @@ -796,7 +796,14 @@ def _mam_result(indexer_id: int, indexer: str, guid: str, *, freeleech: bool = F class TestIndexerAwareDeduplication: - """One tracker as several Prowlarr entries must not collapse to one row (#1137).""" + """One tracker as several Prowlarr entries must not collapse to one row (#1137). + + These assert the split itself, so they turn PROWLARR_COLLAPSE_DUPLICATES off: + it ships on, which keeps the release list as it was before #1137 for everyone + who has not asked for the per-entry rows. + """ + + SPLIT = {"PROWLARR_COLLAPSE_DUPLICATES": False} ONLY_ACTIVE = 10 FREELEECH = 25 @@ -834,6 +841,7 @@ class TestIndexerAwareDeduplication: ) ], }, + config_values=self.SPLIT, ) assert len(releases) == 2 @@ -850,6 +858,7 @@ class TestIndexerAwareDeduplication: _mam_result(self.FREELEECH, "MyAnonamouse - Freeleech", shared_guid) ], }, + config_values=self.SPLIT, ) source_ids = [r.source_id for r in releases] @@ -868,6 +877,7 @@ class TestIndexerAwareDeduplication: _mam_result(self.FREELEECH, "MyAnonamouse - Freeleech", shared_guid) ], }, + config_values=self.SPLIT, ) assert len(releases) == 2 @@ -914,7 +924,7 @@ class TestBuildSourceId: class TestCollapseDuplicatesSetting: - """Opt-in one-row-per-release collapse, resolved by Prowlarr's priority.""" + """One-row-per-release collapse, on by default, resolved by Prowlarr's priority.""" ONLY_ACTIVE = 10 FREELEECH = 25 @@ -951,8 +961,8 @@ class TestCollapseDuplicatesSetting: assert len(releases) == 1 assert releases[0].indexer == "MyAnonamouse" - def test_collapse_off_by_default_keeps_both_rows(self, monkeypatch): - releases = TestIndexerAwareDeduplication()._search( + def _search_shared_guid(self, monkeypatch, config_values): + return TestIndexerAwareDeduplication()._search( monkeypatch, { self.ONLY_ACTIVE: [ @@ -962,11 +972,34 @@ class TestCollapseDuplicatesSetting: _mam_result(self.FREELEECH, "MAM - Freeleech", "https://tracker.example/t/9") ], }, + config_values=config_values, priorities={self.FREELEECH: 20, self.ONLY_ACTIVE: 24}, ) + def test_collapse_is_on_when_the_setting_is_untouched(self, monkeypatch): + """An upgrading user who sets nothing keeps the single row they had before #1137.""" + releases = self._search_shared_guid(monkeypatch, None) + + assert len(releases) == 1 + assert releases[0].indexer == "MAM - Freeleech" + + def test_opting_out_keeps_every_indexer_entry(self, monkeypatch): + releases = self._search_shared_guid(monkeypatch, {"PROWLARR_COLLAPSE_DUPLICATES": False}) + assert len(releases) == 2 + def test_the_settings_field_and_the_search_fallback_agree(self): + """The field default is what governs in production; the search fallback only + applies to an unregistered key. They have to say the same thing. + """ + from shelfmark.release_sources.prowlarr.settings import prowlarr_config_settings + + field = next( + f for f in prowlarr_config_settings() if f.key == "PROWLARR_COLLAPSE_DUPLICATES" + ) + + assert field.default is True + class TestBuildIndexerPriority: """The priority NUMBER from Prowlarr is the rank; the id is only the key."""