Split the site job into smaller functions

A new story, the chapter list of the site and the choice of chapters to
download each have a function. One helper downloads a chapter for both
loops.

bindery/orchestrator.py

@@ -1116,17 +1116,9 @@ def _recent_chapters(story_id: int) -> int:
     return _setting_recent_chapters()
 
 
-def _run_site_job(job: Job) -> str:
+def _new_story(job: Job, snap: fff.SiteSnapshot) -> dict[str, Any]:
-    story = store.get_story(job.story_id) if job.story_id else None
+    """Create the story of a site job. If the library already has it (for example from the FanFicFare calibre
-    url = story["url"] if story else job.url
+    plugin), link it and keep its chapters."""
-    adapter = fff.get_adapter(url)
-    _progress(job, "Reading story page")
-    snap = fff.fetch_story(adapter)
-    if story is None:
-        story = store.get_story_by_url(snap.url)
-    if story is None:
-        # A new story. If the library already has it (for example from the
-        # FanFicFare calibre plugin), link it and keep its chapters.
     found = _find_book(snap.url, [snap.title]) if _link_books() and library_available() else None
     book_id, book_uuid = (found[0].id, found[0].uuid) if found else (None, None)
     contents = None
@@ -1146,18 +1138,17 @@ def _run_site_job(job: Job) -> str:
         store.update_story(story_id, ingested_at=now())
     if contents:
         _import_epub(story_id, contents)
-        story = _require_story(story_id)
+    return _require_story(story_id)
-    job.story_id = story["id"]
-    job_queue.update(job, title=snap.title)
-    story_id = story["id"]
 
-    store.update_story(story_id, metahtml=snap.metahtml, site=snap.site, author=snap.author)
-    story["metahtml"] = snap.metahtml
-    if snap.cover:
-        store.put_image(
-            story_id, snap.cover["url"], snap.cover["ext"], snap.cover["mime"], snap.cover["data"], is_cover=True
-        )
 
+def _sync_chapters(
+    story_id: int, adapter: Any, site_titles: dict[str, str], full: bool
+) -> tuple[list[dict[str, Any]], dict[int, str], int]:
+    """Mark each stored chapter as the site lists it now.
+
+    Returns the chapters, the new site title of each renamed chapter by its id, and how many chapters a full
+    download took out of the book.
+    """
     # Chapter URLs can change (RoyalRoad changes the slug when a title
     # changes). FanFicFare maps old forms to the current one.
     chapters = store.list_chapters(story_id)
@@ -1170,11 +1161,8 @@ def _run_site_job(job: Job) -> str:
             known.add(normal)
             c["url"] = normal
 
-    site_titles = {c["url"]: c["title"] for c in snap.chapters}
-    site_index = {url: i for i, url in enumerate(site_titles)}
-    by_url = {c["url"]: c for c in chapters}
     dropped = 0
-    renamed: dict[int, str] = {}  # chapter id: new title on the site
+    renamed: dict[int, str] = {}
     for c in chapters:
         on_site = int(c["url"] in site_titles)
         changes = {}
@@ -1182,46 +1170,73 @@ def _run_site_job(job: Job) -> str:
             changes["on_site"] = on_site
         if on_site and site_titles[c["url"]] != c["title"]:
             renamed[c["id"]] = site_titles[c["url"]]
-        if job.mode == DownloadMode.FULL and c["included"] != on_site:
+        if full and c["included"] != on_site:
             changes["included"] = on_site
             dropped += 1 - on_site
         if changes:
             store.update_chapter(c["id"], **changes)
             c.update(changes)
+    return chapters, renamed, dropped
 
+
+def _chapters_to_fetch(
+    job: Job, story_id: int, chapters: list[dict[str, Any]], site_titles: dict[str, str], renamed: dict[int, str]
+) -> tuple[list[str], list[dict[str, Any]], list[str]]:
+    """What a site job downloads: the URLs of the new chapters, and the stored chapters to check again.
+
+    Also returns the titles of the selected chapters that the site no longer lists.
+    """
     mode = job.mode
-    new_urls = [u for u in site_titles if u not in by_url]
+    stored = {c["url"] for c in chapters}
-    if mode == DownloadMode.METADATA:
+    new_urls = [u for u in site_titles if u not in stored]
+    if mode in (DownloadMode.METADATA, DownloadMode.RECHECK):
         new_urls = []
-    refetch: list[dict[str, Any]] = []
+    selected = [c for c in chapters if c["id"] in job.chapter_ids] if mode == DownloadMode.RECHECK else []
-    skipped: list[str] = []
+    skipped = [fff.plain_title(c["title"]) for c in selected if not c["on_site"]]
     if mode in (DownloadMode.RECHECK_ALL, DownloadMode.FULL):
         refetch = [c for c in chapters if c["on_site"]]
-    elif mode == DownloadMode.RECHECK:
-        new_urls = []
-        for c in chapters:
-            if c["id"] not in job.chapter_ids:
-                continue
-            if c["on_site"]:
-                refetch.append(c)
     else:
-                skipped.append(fff.plain_title(c["title"]))
+        refetch = [c for c in selected if c["on_site"]]
     # A new title can come with new text. On a site that numbers its chapter
     # URLs, it can also mean that the chapters moved up. So a renamed chapter is
     # checked again, and it takes the new title only when that works. A metadata
     # update downloads no chapters, so it only takes the new titles.
-    renamed_count = job.result.get("renamed", 0)
+    if mode != DownloadMode.METADATA:
-    if mode == DownloadMode.METADATA:
-        for chapter_id, title in renamed.items():
-            store.update_chapter(chapter_id, title=title)
-        renamed_count += len(renamed)
-    else:
         refetch += [c for c in chapters if c["id"] in renamed and c not in refetch]
     # Authors most often fix their latest chapters, so a new-chapter download can check those again.
     recent = _recent_chapters(story_id) if mode == DownloadMode.NEW else 0
     if recent:
         latest = [c for c in chapters if c["on_site"] and c["included"]][-recent:]
         refetch += [c for c in latest if c not in refetch]
+    return new_urls, refetch, skipped
+
+
+def _run_site_job(job: Job) -> str:
+    story = store.get_story(job.story_id) if job.story_id else None
+    adapter = fff.get_adapter(story["url"] if story else job.url)
+    _progress(job, "Reading story page")
+    snap = fff.fetch_story(adapter)
+    story = story or store.get_story_by_url(snap.url) or _new_story(job, snap)
+    story_id = job.story_id = story["id"]
+    job_queue.update(job, title=snap.title)
+
+    store.update_story(story_id, metahtml=snap.metahtml, site=snap.site, author=snap.author)
+    story["metahtml"] = snap.metahtml
+    if snap.cover:
+        store.put_image(
+            story_id, snap.cover["url"], snap.cover["ext"], snap.cover["mime"], snap.cover["data"], is_cover=True
+        )
+
+    mode = job.mode
+    site_titles = {c["url"]: c["title"] for c in snap.chapters}
+    site_index = {url: i for i, url in enumerate(site_titles)}
+    chapters, renamed, dropped = _sync_chapters(story_id, adapter, site_titles, mode == DownloadMode.FULL)
+    new_urls, refetch, skipped = _chapters_to_fetch(job, story_id, chapters, site_titles, renamed)
+    renamed_count = job.result.get("renamed", 0)
+    if mode == DownloadMode.METADATA:
+        for chapter_id, title in renamed.items():
+            store.update_chapter(chapter_id, title=title)
+        renamed_count += len(renamed)
 
     # Each chapter is stored as soon as it arrives, after its images. The job
     # result keeps what this job did, so a job that resumes after a restart
@@ -1236,11 +1251,23 @@ def _run_site_job(job: Job) -> str:
     failed: list[str] = []
     stored_images = store.image_urls(story_id)
 
-    def store_images() -> None:
+    def fetch(url: str, title: str, action: str) -> str | None:
+        """Download a chapter and store its images. A chapter that fails goes in `failed`, and gives None."""
+        nonlocal done
+        name = fff.plain_title(title)
+        _progress(job, f"{action} {done + 1} of {total}: {name}", done / max(total, 1))
+        done += 1
+        try:
+            html = fff.fetch_chapter(adapter, url, site_index[url])
+        except Exception as e:
+            logger.warning(f"Chapter {url} failed: {e}")
+            failed.append(f"{name}: {e}")
+            return None
         for img in fff.collected_images(adapter):
             if img["url"] not in stored_images:
                 store.put_image(story_id, img["url"], img["ext"], img["mime"], img["data"])
                 stored_images.add(img["url"])
+        return html
 
     def save_progress() -> None:
         job_queue.update(
@@ -1248,15 +1275,9 @@ def _run_site_job(job: Job) -> str:
         )
 
     for url in new_urls:
-        _progress(job, f"New chapter {done + 1} of {total}: {fff.plain_title(site_titles[url])}", done / max(total, 1))
+        html = fetch(url, site_titles[url], "New chapter")
-        done += 1
+        if html is None:
-        try:
-            html = fff.fetch_chapter(adapter, url, site_index[url])
-        except Exception as e:
-            logger.warning(f"Chapter {url} failed: {e}")
-            failed.append(f"{fff.plain_title(site_titles[url])}: {e}")
             continue
-        store_images()
         chapter = {"url": url, "title": site_titles[url], "html": html, "text_hash": text_hash(html)}
         # The chapter goes in the order that the story has now. A move or a delete by hand during this job stays.
         store.add_chapters(story_id, [chapter], mode.value, lambda have: merge_chapter_lists(have, list(site_titles)))
@@ -1264,18 +1285,9 @@ def _run_site_job(job: Job) -> str:
         save_progress()
 
     for c in refetch:
-        _progress(job, f"Rechecking chapter {done + 1} of {total}: {fff.plain_title(c['title'])}", done / max(total, 1))
+        html = fetch(c["url"], c["title"], "Rechecking chapter")
-        try:
+        revisions = store.list_revisions(c["id"]) if html is not None else []
-            html = fff.fetch_chapter(adapter, c["url"], site_index[c["url"]])
+        if not revisions:  # the download failed, or the user deleted the chapter during this job
-        except Exception as e:
-            logger.warning(f"Chapter {c['url']} failed: {e}")
-            failed.append(f"{fff.plain_title(c['title'])}: {e}")
-            done += 1
-            continue
-        store_images()
-        revisions = store.list_revisions(c["id"])
-        if not revisions:  # the user deleted the chapter during this job
-            done += 1
             continue
         if c["id"] in renamed:
             c["title"] = renamed[c["id"]]
@@ -1293,22 +1305,18 @@ def _run_site_job(job: Job) -> str:
                 changed.append({"chapter_id": c["id"], "title": c["title"], "rev": rev})
         checked.append(c["id"])
         save_progress()
-        done += 1
 
     removed = sum(1 for c in chapters if not c["on_site"] and c["included"])
-    summary = [f"{new_count} new chapter{'s' if new_count != 1 else ''}"]
+    parts = [
-    if renamed_count:
+        (True, f"{new_count} new chapter{'s' if new_count != 1 else ''}"),
-        summary.append(f"{renamed_count} renamed")
+        (renamed_count, f"{renamed_count} renamed"),
-    if rechecked:
+        (rechecked, f"{len(changed)} of {rechecked} rechecked chapters changed"),
-        summary.append(f"{len(changed)} of {rechecked} rechecked chapters changed")
+        (removed, f"{removed} kept after removal from site"),
-    if removed:
+        (dropped, f"{dropped} taken out because site removed them"),
-        summary.append(f"{removed} kept after removal from site")
+        (skipped, f"{len(skipped)} skipped, no longer on site"),
-    if dropped:
+        (failed, f"{len(failed)} failed"),
-        summary.append(f"{dropped} taken out because site removed them")
+    ]
-    if skipped:
+    summary = ", ".join(text for show, text in parts if show)
-        summary.append(f"{len(skipped)} skipped, no longer on site")
-    if failed:
-        summary.append(f"{len(failed)} failed")
     job_queue.update(
         job, result={"new": new_count, "changed": changed, "failed": failed, "skipped": skipped, "removed": removed}
     )
@@ -1316,7 +1324,7 @@ def _run_site_job(job: Job) -> str:
     if mode in (DownloadMode.RECHECK_ALL, DownloadMode.FULL):
         store.set_setting(story_id, "last_full_recheck", now())
     if not store.chapters_for_build(story_id):
-        raise StoryError(", ".join(summary) + ". No chapters downloaded.")
+        raise StoryError(f"{summary}. No chapters downloaded.")
 
     # The book is built and sent only when its chapters, rules or metadata changed. A job that resumes, or that
     # follows a failed job, finds the changes of the earlier run through the key.
@@ -1324,17 +1332,17 @@ def _run_site_job(job: Job) -> str:
     title = _display_title(pinned, snap.title)
     overrides = _epub_overrides(pinned)
     key = _build_key(story, overrides, snap.calibre)
-    checked: dict[str, Any] = {"pinned": pinned, "site_values": snap.calibre, "title": title, "last_checked": now()}
+    changes: dict[str, Any] = {"pinned": pinned, "site_values": snap.calibre, "title": title, "last_checked": now()}
     if key == _done_key(story_id):
-        store.update_story(story_id, **checked, status="ok", message="No changes")
+        store.update_story(story_id, **changes, status="ok", message="No changes")
-        return "No changes. " + ", ".join(summary)
+        return f"No changes. {summary}"
 
     _progress(job, "Building epub", 1.0)
     message, book_changed = _send(story, overrides, key, title, snap.author)
     if book_changed:
-        checked["last_changed"] = now()
+        changes["last_changed"] = now()
-    store.update_story(story_id, **checked, message=message, status="error" if failed else "ok")
+    store.update_story(story_id, **changes, message=message, status="error" if failed else "ok")
-    return ", ".join(summary) + ". " + message
+    return f"{summary}. {message}"
 
 
 def _run_job(job: Job) -> str: