From df07ed3e6f70ce78d73c198e3736c9d182bd8f3d Mon Sep 17 00:00:00 2001 From: splitsec2 <35583321+splitsec2@users.noreply.github.com> Date: Fri, 13 Nov 2026 16:55:16 -0600 Subject: [PATCH] fix(auto-download): leave a failed request alone when no source can search it (#1693) Fixes #1678. The retry after the cooldown (**Retry failed downloads after (days)**) reopened a failed request owned by an admin, or any request on an install without login, even when no release source was usable for its content type. Reopening clears the release and delivery state, and with no source the auto-download pass never searches the request, so it sat pending with nothing to act on it. A regular user's request has only been reopened when the pass could fulfil it since #1676. `OwnerPolicy.may_fulfil` returned True for those owners before looking at sources. It now asks `download_sources` for every owner. That already lets an exempt owner use every configured source, so an admin's request is reopened exactly as before whenever a source exists, and is left as it is when none does. Tests: a request with no usable source is left alone for an admin-owned request and on an install without login (both fail on main), and an admin-owned request with a source is still reopened. `TestRetryAfterCooldown` now configures a source, since every case there expects one to search. Full Python suite, ruff and basedpyright pass. ## Summary by CodeRabbit * **Bug Fixes** * Automatic download requests are reopened only when an available source supports the requested content type. This applies to requests owned by administrators and requests made without logging in, preventing requests from being marked as fulfillable when no usable source is available. Requests with a compatible source can still be reopened after a cooldown or when a stalled download has settled. --- shelfmark/core/auto_download.py | 15 +++++------- tests/core/test_auto_download.py | 39 +++++++++++++++++++++++++++++++- 2 files changed, 44 insertions(+), 10 deletions(-) diff --git a/shelfmark/core/auto_download.py b/shelfmark/core/auto_download.py index 73cf8ee3..1f7f920e 100644 --- a/shelfmark/core/auto_download.py +++ b/shelfmark/core/auto_download.py @@ -1397,15 +1397,12 @@ class OwnerPolicy: ] def may_fulfil(self, row: dict[str, Any]) -> bool: - """True when auto-download could fulfil ``row`` from at least one configured source.""" - if self._bypass: - return True - user_id = coerce_int(row.get("user_id"), 0) - owner = self._owner(user_id) if user_id > 0 else None - if owner is None: - return False - if owner[0]: - return True + """True when auto-download could fulfil ``row`` from at least one configured source. + + An exempt owner (an admin, a user with requests off, or any owner on an install + without login) may use every configured source, but still needs one: with none + usable for the content type, nothing would ever search the request. + """ book_data = row.get("book_data") content_type = normalize_content_type( row.get("content_type") diff --git a/tests/core/test_auto_download.py b/tests/core/test_auto_download.py index 73083e09..0a268346 100644 --- a/tests/core/test_auto_download.py +++ b/tests/core/test_auto_download.py @@ -2606,6 +2606,13 @@ class TestRetryAfterCooldown: ) return row + @pytest.fixture(autouse=True) + def _a_source_is_configured(self, monkeypatch): + """A reopen needs a source to search, so every case here has one unless it removes it.""" + monkeypatch.setattr( + auto_download, "_configured_source_priority", lambda _ct: ["audiobookbay"] + ) + def _reopen(self, user_db, **kwargs): kwargs.setdefault("cooldown_days", 7) kwargs.setdefault("db_path", user_db._db_path) @@ -2615,6 +2622,32 @@ class TestRetryAfterCooldown: def _user(user_db): return user_db.create_user(username="reader", role="user") + @pytest.mark.parametrize( + ("role", "auth_mode"), + [("admin", "builtin"), ("user", "none")], + ids=["admin-owned", "no-login"], + ) + def test_a_request_with_no_usable_source_is_left_as_it_is( + self, user_db, monkeypatch, role, auth_mode + ): + """An exempt owner still needs a source, or the reopened request would sit unsearched.""" + monkeypatch.setattr(auto_download, "_configured_source_priority", lambda _ct: []) + owner = user_db.create_user(username="owner", role=role) + row = self._request(user_db, owner["id"], provider_id="1", history="error") + policy = auto_download.OwnerPolicy(user_db, auth_mode=auth_mode) + + assert self._reopen(user_db, owner_policy=policy) == 0 + + stored = user_db.get_request(row["id"]) + assert stored["status"] == "fulfilled" + assert stored["delivery_state"] == "error" + + def test_an_admin_owned_request_with_a_source_is_still_reopened(self, user_db): + admin = user_db.create_user(username="ops", role="admin") + self._request(user_db, admin["id"], provider_id="1", history="active") + + assert self._reopen(user_db) == 1 + def test_an_interrupted_download_is_reopened_and_may_use_the_same_release(self, user_db): row = self._request(user_db, self._user(user_db)["id"], provider_id="1", history="active") @@ -3627,7 +3660,11 @@ class TestSettleDeliveryStatesFromHistory: ) assert self._state(user_db, rid) == "queued" - def test_the_stale_sweep_then_reopens_a_stalled_download(self, user_db): + def test_the_stale_sweep_then_reopens_a_stalled_download(self, user_db, monkeypatch): + """Once settled, a stalled download is reopened when a source can search it again.""" + monkeypatch.setattr( + auto_download, "_configured_source_priority", lambda _ct: ["audiobookbay"] + ) rid = self._fulfilled( user_db, state="downloading", delivery_updated_at="2026-10-16T20:56:38+00:00" )