diff --git a/shelfmark/core/saved_autoget.py b/shelfmark/core/saved_autoget.py index b8564fa0d..d02b604ed 100644 --- a/shelfmark/core/saved_autoget.py +++ b/shelfmark/core/saved_autoget.py @@ -75,11 +75,16 @@ class AutoGetHooks: @dataclass class Decision: - """Whether an item can be got now; if not, why it waits.""" + """Which of an item's picks can be got now, and why the rest wait. - ready: bool - status: str = "" + `ready` holds the indexes of the picks that can go (into the item's releases and + payloads), with `payloads` their payloads to queue. `status` says why the others + wait; it is empty when none do. + """ + + ready: list[int] = field(default_factory=list) payloads: list[dict[str, Any]] = field(default_factory=list) + status: str = "" def enabled() -> bool: @@ -172,51 +177,76 @@ def _unsat_wait(check: mam_account.BufferCheck) -> str: return f"Waiting for {needed} unsatisfied slot{'s' if needed != 1 else ''}" +def _waiting(status: str) -> Decision: + return Decision(status=status) + + def evaluate(item: dict[str, Any], owner: Owner, hooks: AutoGetHooks, run: _RunContext) -> Decision: - """Decide whether one saved item can be downloaded now.""" + """Decide which of a saved item's picks can be downloaded now. + + Each pick is weighed on its own, so a freeleech torrent or a small ebook isn't held + back by a large audiobook picked with it that the ratio can't take yet: the cheapest + go first (nothing on MAM, then freeleech, then by size), each one that still fits + alongside those already going. + """ releases = item.get("releases") or [] payloads = [p for p in item.get("payloads") or [] if isinstance(p, dict)] if not payloads or len(payloads) != len(releases): - return Decision(False, "Save the pick again to download it automatically") + return _waiting("Save the pick again to download it automatically") if not all(hooks.may_download(owner, payload) for payload in payloads): - return Decision(False, "Needs approval: use Get to request it") + return _waiting("Needs approval: use Get to request it") + # Nothing counts against MyAnonamouse here, or nothing can be checked: all go. mam_indexes = [i for i, p in enumerate(payloads) if mam_torrent_id_for_release(p)] - if not mam_indexes: - # Nothing here counts against MyAnonamouse, so nothing to wait for. - return Decision(True, payloads=payloads) - if not mam_account.is_configured(): - return Decision(True, payloads=payloads) + if not mam_indexes or not mam_account.is_configured(): + return Decision(ready=list(range(len(payloads))), payloads=payloads) + free_indexes = [i for i in range(len(payloads)) if i not in mam_indexes] stats, error = run.stats() if stats is None: - return Decision(False, f"Couldn't read the MyAnonamouse account: {error}") + return Decision( + ready=free_indexes, + payloads=[payloads[i] for i in free_indexes], + status=f"Couldn't read the MyAnonamouse account: {error}", + ) # Freeleech can start or end at any time, so ask MAM rather than trusting the save. vip = mam_account.is_vip_class(stats.classname) + checked = list(payloads) for index in mam_indexes: - torrent_id = mam_torrent_id_for_release(payloads[index]) + torrent_id = mam_torrent_id_for_release(checked[index]) if torrent_id is None: continue try: freeleech = mam_account.torrent_is_freeleech(torrent_id, vip=vip) except mam_account.MAM_ERRORS: continue # Keep what the search said; the ratio check covers a wrong guess - payloads[index] = _with_freeleech(payloads[index], freeleech) + checked[index] = _with_freeleech(checked[index], freeleech) + + going: list[int] = [] + reason = "" + for index in sorted(mam_indexes, key=lambda i: mam_charge_bytes_for_release(checked[i])): + trial = [checked[i] for i in [*going, index]] + wait = _pick_wait(trial, stats) + if wait: + reason = reason or wait + continue + going.append(index) + + ready = sorted([*free_indexes, *going]) + return Decision(ready=ready, payloads=[checked[i] for i in ready], status=reason) - check = mam_account.check_buffer(payloads) + +def _pick_wait(trial: list[dict[str, Any]], stats: mam_account.MamStats) -> str | None: + """Why the last of `trial` must wait, with the rest of it going too; None if it may go.""" + check = mam_account.check_buffer(trial) if not check.ok: if not check.unsat_ok: - return Decision(False, _unsat_wait(check)) - return Decision(False, f"Waiting for buffer: {_gib(check.missing_bytes)} short") + return _unsat_wait(check) + return f"Waiting for buffer: {_gib(check.missing_bytes)} short" if not check.checked and check.error: - return Decision(False, f"Couldn't check MyAnonamouse: {check.error}") - - wait = _ratio_wait(stats, sum(mam_charge_bytes_for_release(p) for p in payloads)) - if wait: - return Decision(False, wait) - - return Decision(True, payloads=payloads) + return f"Couldn't check MyAnonamouse: {check.error}" + return _ratio_wait(stats, sum(mam_charge_bytes_for_release(p) for p in trial)) @dataclass @@ -275,28 +305,35 @@ def _check_item(self, item: dict[str, Any], run: _RunContext, report: RunReport) self._service.update(owner_key, item["id"], auto_status=decision.status) return - failed: list[tuple[dict[str, Any], dict[str, Any], str]] = [] releases = item.get("releases") or [] - for pick, payload in zip(releases, decision.payloads, strict=True): - ok, error = self._hooks.queue_release(payload, owner) + payloads = item.get("payloads") or [] + failed: list[int] = [] + error = "" + for index, payload in zip(decision.ready, decision.payloads, strict=True): + ok, queue_error = self._hooks.queue_release(payload, owner) if not ok: - failed.append((pick, payload, error or "Could not queue the download")) - if len(failed) == len(decision.payloads): - error = failed[0][2] + failed.append(index) + error = error or queue_error or "Could not queue the download" + if len(failed) == len(decision.ready): report.waiting[item["id"]] = error self._service.update(owner_key, item["id"], auto_status=f"Couldn't queue: {error}") return + report.queued.append(item["id"]) - if failed: + left = [i for i in range(len(releases)) if i not in decision.ready or i in failed] + if not left: + self._service.delete(owner_key, item["id"]) + else: + if decision.status: + report.waiting[item["id"]] = decision.status self._service.keep_picks( owner_key, item["id"], - releases=[pick for pick, _, _ in failed], - payloads=[payload for _, payload, _ in failed], - last_error=f"Some picks couldn't be queued: {failed[0][2]}", + releases=[releases[i] for i in left], + payloads=[payloads[i] for i in left], + last_error=f"Some picks couldn't be queued: {error}" if failed else None, + auto_status=decision.status, ) - else: - self._service.delete(owner_key, item["id"]) try: self._hooks.notify(item, owner) except Exception: # a notification failure must not undo the download diff --git a/shelfmark/core/saved_items.py b/shelfmark/core/saved_items.py index 96ffc3e3c..d1623c739 100644 --- a/shelfmark/core/saved_items.py +++ b/shelfmark/core/saved_items.py @@ -387,25 +387,32 @@ def keep_picks( *, releases: list[dict[str, Any]], payloads: list[dict[str, Any]], - last_error: str, + last_error: str | None = None, + auto_status: str | None = None, ) -> None: - """After a partly failed automatic download: keep only the picks still to get. + """After an automatic download got some of an item's picks: keep only the rest. - Automatic downloading is turned off so the error is seen before anything retries. + With `last_error` (a pick couldn't be queued), automatic downloading is turned + off so the error is seen before anything retries. Without it, the rest are + simply still waiting for room: they stay queued, with `auto_status` saying why. """ kind = "release" if len(releases) == 1 else "combined" + failed = last_error is not None with self._lock: conn = self._connect() try: conn.execute( """UPDATE saved_items SET kind = CASE WHEN content_type = 'combined' - THEN 'combined' ELSE ? END, releases = ?, payloads = ?, auto_get = 0, - auto_status = NULL, last_error = ?, updated_at = ? + THEN 'combined' ELSE ? END, releases = ?, payloads = ?, auto_get = ?, + auto_status = ?, auto_checked_at = ?, last_error = ?, updated_at = ? WHERE owner = ? AND id = ?""", ( kind, json.dumps(releases), json.dumps(payloads), + 0 if failed else 1, + None if failed else (auto_status or None), + None if failed else now_utc_iso(), last_error, now_utc_iso(), owner, diff --git a/src/frontend/src/components/activity/SavedPanel.tsx b/src/frontend/src/components/activity/SavedPanel.tsx index f50ac1ef6..dc7c72d80 100644 --- a/src/frontend/src/components/activity/SavedPanel.tsx +++ b/src/frontend/src/components/activity/SavedPanel.tsx @@ -368,7 +368,7 @@ const SavedTable = ({

{inLine - ? "Each downloads on its own once there's room, checked in this order: one that doesn't fit yet doesn't hold up the ones after it." + ? "Each release downloads on its own once there's room, checked in this order: one that doesn't fit yet (a big audiobook waiting for your ratio, say) doesn't hold up the others, even those picked with it." : 'Choose releases for a book and it joins the queue, to download once there’s room.'}

diff --git a/tests/core/test_saved_autoget.py b/tests/core/test_saved_autoget.py index ee9e6236a..c425b1328 100644 --- a/tests/core/test_saved_autoget.py +++ b/tests/core/test_saved_autoget.py @@ -138,7 +138,7 @@ def test_gets_an_item_once_it_fits_then_removes_it(service, mam): def test_waits_for_unsatisfied_slots_and_says_how_many(service, mam): - mam.unsat_free = 1 + mam.unsat_free = 0 recorder = Recorder() item_id = _mark(service, [_payload("r1", torrent_id=1), _payload("r2", torrent_id=2)]) report = SavedAutoGetter(service, recorder.hooks()).run_check() @@ -151,6 +151,60 @@ def test_waits_for_unsatisfied_slots_and_says_how_many(service, mam): assert item["auto_get"] is True +def test_picks_that_fit_go_and_the_rest_keep_waiting(service, mam): + # One slot free, two picks: the first goes now, the second waits for the next slot. + mam.unsat_free = 1 + recorder = Recorder() + item_id = _mark(service, [_payload("r1", torrent_id=1), _payload("r2", torrent_id=2)]) + report = SavedAutoGetter(service, recorder.hooks()).run_check() + assert report.queued == [item_id] + assert [p["source_id"] for p in recorder.queued] == ["r1"] + item = service.get_item("user:1", item_id) + assert item is not None + assert [pick["release"]["source_id"] for pick in item["releases"]] == ["r2"] + assert item["auto_get"] is True # Still queued: nothing went wrong + assert item["last_error"] is None + assert item["auto_status"] == "Waiting for 1 unsatisfied slot" + assert recorder.notified == ["Book Title"] + + +def test_freeleech_and_small_picks_are_not_held_back_by_a_large_one(service, mam): + # Ratio 1.5, under the 2.0 target. A freeleech audiobook and a small ebook can go; + # the 40 GB non-freeleech audiobook picked with them waits for the ratio. + mam.stats = _stats(uploaded=150 * GIB, downloaded=100 * GIB) + mam.freeleech[2] = True + recorder = Recorder() + item_id = _mark( + service, + [ + _payload("ebook", torrent_id=1, size=5 * 1024**2), + _payload("fl-audiobook", torrent_id=2, size=30 * GIB), + _payload("big-audiobook", torrent_id=3, size=40 * GIB), + ], + ) + report = SavedAutoGetter(service, recorder.hooks()).run_check() + assert report.queued == [item_id] + assert sorted(p["source_id"] for p in recorder.queued) == ["ebook", "fl-audiobook"] + item = service.get_item("user:1", item_id) + assert item is not None + assert [pick["release"]["source_id"] for pick in item["releases"]] == ["big-audiobook"] + assert item["auto_get"] is True + assert item["auto_status"].startswith("Waiting for freeleech or ratio 2.00") + + +def test_the_cheapest_picks_get_the_free_slots(service, mam): + # One slot: the freeleech pick gets it, though it was picked after the paid one. + mam.unsat_free = 1 + mam.freeleech[2] = True + recorder = Recorder() + _mark( + service, + [_payload("paid", torrent_id=1, size=GIB), _payload("freeleech", torrent_id=2, size=GIB)], + ) + SavedAutoGetter(service, recorder.hooks()).run_check() + assert [p["source_id"] for p in recorder.queued] == ["freeleech"] + + def test_freeleech_goes_even_below_the_target_ratio(service, mam): # Ratio 1.5, under the 2.0 target: a freeleech torrent can't lower it, so it goes. mam.stats = _stats(uploaded=150 * GIB, downloaded=100 * GIB)