mirror of
https://github.com/calibrain/shelfmark.git
synced 2026-11-10 12:28:27 +00:00
fix(webshare): clear the "Resolving" status once the download starts (#1575)
## What
A Webshare.cz download shows "Resolving Webshare link" for its entire
duration, so a large download (e.g. a 1.3 GB audiobook archive) looks
stuck.
## Why
After the link was resolved, the handler called
`status_callback("downloading", None)`. `update_download_status` treats
a `None` message as "keep the current message", so the "Resolving
Webshare link" detail stayed in the activity panel. The HTTP handler
sends `""` for exactly this reason.
## How
- Report `downloading` with an empty message, which clears the resolving
text.
- When the download response has no `Content-Length`, use the release's
size (`task.size`, parsed with `parse_size_string`) as the total for
progress. Without it, progress never moved, and stall detection would
flag a healthy long download after 5 minutes.
## Testing
- New tests in `tests/webshare/test_handler.py`: the downloading status
clears the message, and progress is computed from the release size when
there is no Content-Length.
- `uv run pytest tests/webshare` is green (34 passed), and `ruff check`
and `ruff format --check` are clean.
Follow-up to #1543.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
525b094b57
commit
d50440f6c2
2 files changed
+40
-8
No files matched your search
@@ -68,8 +68,10 @@ class WebshareHandler(DownloadHandler):
|
||||
link = WebshareClient(*get_credentials()).file_link(ident)
|
||||
|
||||
book_path = _target_path(task, ident, link)
|
||||
status_callback("downloading", None)
|
||||
if not self._stream(link, book_path, cancel_flag, progress_callback):
|
||||
# An empty message clears "Resolving..."; None would leave it on screen.
|
||||
status_callback("downloading", "")
|
||||
expected_size = downloader.parse_size_string(task.size or "") or 0
|
||||
if not self._stream(link, book_path, cancel_flag, progress_callback, expected_size):
|
||||
book_path.unlink(missing_ok=True)
|
||||
status_callback("cancelled", "Cancelled")
|
||||
return None
|
||||
@@ -95,8 +97,13 @@ class WebshareHandler(DownloadHandler):
|
||||
book_path: Path,
|
||||
cancel_flag: Event,
|
||||
progress_callback: Callable[[float], None],
|
||||
expected_size: float = 0,
|
||||
) -> bool:
|
||||
"""Stream ``link`` into ``book_path``; return False if cancelled midway."""
|
||||
"""Stream ``link`` into ``book_path``; return False if cancelled midway.
|
||||
|
||||
Without a Content-Length the search result's size drives progress, so the
|
||||
orchestrator's stall detection still sees the download moving.
|
||||
"""
|
||||
with requests.get(
|
||||
link,
|
||||
headers=downloader.DOWNLOAD_HEADERS,
|
||||
@@ -106,7 +113,7 @@ class WebshareHandler(DownloadHandler):
|
||||
verify=network.get_ssl_verify(link),
|
||||
) as response:
|
||||
response.raise_for_status()
|
||||
total = int(response.headers.get("Content-Length") or 0)
|
||||
total = int(response.headers.get("Content-Length") or 0) or expected_size
|
||||
written = 0
|
||||
with book_path.open("wb") as file:
|
||||
for chunk in response.iter_content(chunk_size=_CHUNK_SIZE):
|
||||
|
||||
@@ -13,17 +13,17 @@ def _task(task_id="webshare:abc123", fmt="epub"):
|
||||
return DownloadTask(task_id=task_id, source="webshare", title="Krysar", format=fmt)
|
||||
|
||||
|
||||
def _http_response(body: bytes):
|
||||
def _http_response(body: bytes, *, content_length=True):
|
||||
response = MagicMock()
|
||||
response.__enter__.return_value = response
|
||||
response.__exit__.return_value = False
|
||||
response.headers = {"Content-Length": str(len(body))}
|
||||
response.headers = {"Content-Length": str(len(body))} if content_length else {}
|
||||
response.raise_for_status.return_value = None
|
||||
response.iter_content.return_value = [body[i : i + 4096] for i in range(0, len(body), 4096)]
|
||||
return response
|
||||
|
||||
|
||||
def _run(tmp_path, *, file_link, body=b"x" * 20000, cancel=None, task=None):
|
||||
def _run(tmp_path, *, file_link, body=b"x" * 20000, cancel=None, task=None, content_length=True):
|
||||
status = MagicMock()
|
||||
progress = MagicMock()
|
||||
cancel = cancel or threading.Event()
|
||||
@@ -36,7 +36,11 @@ def _run(tmp_path, *, file_link, body=b"x" * 20000, cancel=None, task=None):
|
||||
),
|
||||
patch.object(webshare_handler, "get_credentials", return_value=("user", "pw")),
|
||||
patch.object(webshare_handler.WebshareClient, "file_link", side_effect=file_link),
|
||||
patch.object(webshare_handler.requests, "get", return_value=_http_response(body)),
|
||||
patch.object(
|
||||
webshare_handler.requests,
|
||||
"get",
|
||||
return_value=_http_response(body, content_length=content_length),
|
||||
),
|
||||
):
|
||||
result = WebshareHandler().download(task or _task(), cancel, progress, status)
|
||||
return result, status, progress
|
||||
@@ -58,6 +62,27 @@ def test_download_streams_file_named_after_link(tmp_path):
|
||||
progress.assert_called_with(100.0)
|
||||
|
||||
|
||||
def test_downloading_status_clears_resolving_message(tmp_path):
|
||||
_, status, _ = _run(tmp_path, file_link=lambda ident: "https://dl.webshare.cz/f.epub")
|
||||
|
||||
# None would keep "Resolving Webshare link" on screen for the whole download.
|
||||
status.assert_any_call("downloading", "")
|
||||
|
||||
|
||||
def test_progress_uses_release_size_without_content_length(tmp_path):
|
||||
task = _task()
|
||||
task.size = "40 KB"
|
||||
_, _, progress = _run(
|
||||
tmp_path,
|
||||
file_link=lambda ident: "https://dl.webshare.cz/f.epub",
|
||||
body=b"x" * 20480,
|
||||
task=task,
|
||||
content_length=False,
|
||||
)
|
||||
|
||||
progress.assert_any_call(50.0)
|
||||
|
||||
|
||||
def test_too_small_file_is_rejected_and_removed(tmp_path):
|
||||
result, status, _ = _run(
|
||||
tmp_path, file_link=lambda ident: "https://dl.webshare.cz/f.epub", body=b"tiny"
|
||||
|
||||
Reference in new issue
Block a user