mirror of
https://github.com/calibrain/shelfmark.git
synced 2026-11-15 21:18:11 +00:00
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. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## 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. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
1 parent
5bc64df95d
commit
df07ed3e6f
2 files changed
+44
-10
No files matched your search
@@ -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")
|
||||
|
||||
@@ -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"
|
||||
)
|
||||
|
||||
Reference in new issue
Block a user