diff --git a/README.md b/README.md index ac4bb6c..40a45b1 100644 --- a/README.md +++ b/README.md @@ -321,6 +321,9 @@ The catalog has three lists, with 50 books per page: - By Author, in surname order. Each author leads to their books. A book shows under each of its authors. - By Last Changed, with the newest change first. +Last Changed records when Bindery stores a new chapter or a new revision of an existing chapter. +Rebuilds, metadata edits and rule changes do not update this date. + A book appears when it has a stored, included chapter. It shows its authors, series, description, tags, language and cover, with your kept values in place of the published values. A reader can search by title, author, series and tag. A download uses the stored chapters, the current ePub rules and the download diff --git a/bindery/orchestrator.py b/bindery/orchestrator.py index e481fef..6e0b8a5 100644 --- a/bindery/orchestrator.py +++ b/bindery/orchestrator.py @@ -1200,9 +1200,8 @@ def _run_rebuild(job: Job) -> str: force = bool(job.options.get("force")) if not force and key == _done_key(story["id"]): return "No changes" - message, changed = _send(story, overrides, key, story["title"], story["author"], force) - changes: dict[str, Any] = {"last_changed": now()} if changed or force else {} - store.update_story(story["id"], pinned=pinned, status="ok", message=message, **changes) + message, _ = _send(story, overrides, key, story["title"], story["author"], force) + store.update_story(story["id"], pinned=pinned, status="ok", message=message) return message @@ -1454,9 +1453,7 @@ def _run_site_job(job: Job) -> str: return f"No changes. {summary}" _progress(job, "Building epub", 1.0) - message, book_changed = _send(story, overrides, key, title, snap.author) - if book_changed: - changes["last_changed"] = now() + message, _ = _send(story, overrides, key, title, snap.author) store.update_story(story_id, **changes, message=message, status="error" if failed else "ok") return f"{summary}. {message}" diff --git a/bindery/store.py b/bindery/store.py index 516e2d7..f79de9c 100644 --- a/bindery/store.py +++ b/bindery/store.py @@ -273,15 +273,17 @@ class Store: conn: sqlite3.Connection, story_id: int, chapter: dict[str, str], source: str, position: int ) -> int: """Store a chapter with its first revision. The chapter needs url, title, html and text_hash.""" + changed_at = now() cur = conn.execute( "INSERT INTO chapters (story_id, position, url, title, added_at) VALUES (?, ?, ?, ?, ?)", - (story_id, position, chapter["url"], chapter["title"], now()), + (story_id, position, chapter["url"], chapter["title"], changed_at), ) chapter_id = int(cur.lastrowid) conn.execute( "INSERT INTO revisions (chapter_id, rev, html, text_hash, source, fetched_at) VALUES (?, 1, ?, ?, ?, ?)", - (chapter_id, chapter["html"], chapter["text_hash"], source, now()), + (chapter_id, chapter["html"], chapter["text_hash"], source, changed_at), ) + conn.execute("UPDATE stories SET last_changed = ? WHERE id = ?", (changed_at, story_id)) return chapter_id def add_chapter( @@ -326,6 +328,7 @@ class Store: def add_revision(self, chapter_id: int, html: str, text_hash: str, source: str) -> int: """Store a new revision and make it the current one.""" + changed_at = now() with self._connect() as conn: rev = conn.execute( "SELECT COALESCE(MAX(rev), 0) + 1 FROM revisions WHERE chapter_id = ?", (chapter_id,) @@ -333,9 +336,15 @@ class Store: conn.execute( "INSERT INTO revisions (chapter_id, rev, html, text_hash, source, fetched_at) " "VALUES (?, ?, ?, ?, ?, ?)", - (chapter_id, rev, html, text_hash, source, now()), + (chapter_id, rev, html, text_hash, source, changed_at), + ) + conn.execute( + "UPDATE chapters SET current_rev = ?, changed_at = ? WHERE id = ?", (rev, changed_at, chapter_id) + ) + conn.execute( + "UPDATE stories SET last_changed = ? WHERE id = (SELECT story_id FROM chapters WHERE id = ?)", + (changed_at, chapter_id), ) - conn.execute("UPDATE chapters SET current_rev = ?, changed_at = ? WHERE id = ?", (rev, now(), chapter_id)) return int(rev) def update_chapter(self, chapter_id: int, **fields: Any) -> None: diff --git a/tests/test_build_keys.py b/tests/test_build_keys.py index 9d5dd67..05374e4 100644 --- a/tests/test_build_keys.py +++ b/tests/test_build_keys.py @@ -12,6 +12,7 @@ from test_deliver import jobs as jobs from test_deliver import story_epub as story_epub from bindery import config, orchestrator, pages +from bindery import store as storage from bindery.jobs import DownloadMode, Job ONE = [(1, "One", "a")] @@ -44,13 +45,21 @@ def test_an_unchanged_story_is_not_built_or_sent_again(builds, monkeypatch): assert len(ingest_files()) == 2 # one epub and its sidecar -@pytest.mark.parametrize("change", ["chapter", "rule", "global rule", "paragraphs", "metadata", "included"]) +@pytest.mark.parametrize( + "change", ["chapter", "revision", "renamed", "rule", "global rule", "paragraphs", "metadata", "included"] +) def test_each_change_builds_and_sends_the_book(builds, monkeypatch, change): story_id = linked_story([(1, "One", "a"), (2, "Two", "b")]) site(monkeypatch, [(1, "One", "a"), (2, "Two", "b")]) update(story_id) + previous = "2020-01-01T00:00:00" + orchestrator.store.update_story(story_id, last_changed=previous) if change == "chapter": site(monkeypatch, [(1, "One", "a"), (2, "Two", "b"), (3, "Three", "c")]) + elif change == "revision": + site(monkeypatch, [(1, "One", "a"), (2, "Two", "new text")]) + elif change == "renamed": + site(monkeypatch, [(1, "One", "a"), (2, "Renamed", "b")]) elif change == "rule": orchestrator.save_rules(story_id, [{"pattern": "a", "replacement": "b"}]) elif change == "global rule": @@ -62,10 +71,14 @@ def test_each_change_builds_and_sends_the_book(builds, monkeypatch, change): else: chapter = orchestrator.store.list_chapters(story_id)[0] orchestrator.store.update_chapter(chapter["id"], included=0) - assert "library book 1" in update(story_id) + mode = DownloadMode.RECHECK_ALL if change == "revision" else DownloadMode.NEW + assert "library book 1" in update(story_id, mode) assert builds.call_count == 2 assert len(ingest_files()) == 4 + changed_at = story(story_id)["last_changed"] + assert (changed_at != previous) == (change in ("chapter", "revision")) assert update(story_id).startswith("No changes") + assert story(story_id)["last_changed"] == changed_at def test_a_held_book_that_did_not_change_is_not_built_again(builds, monkeypatch): @@ -110,13 +123,14 @@ def test_a_lost_file_is_built_again_only_to_send_it(builds): assert builds.call_count == 2 -def test_a_rebuild_without_a_change_does_nothing_unless_the_user_asks_for_it(builds, client, jobs): +@pytest.mark.parametrize("last_changed", [None, "2020-01-01T00:00:00"]) +def test_a_rebuild_without_a_change_does_nothing_unless_the_user_asks_for_it(builds, client, jobs, last_changed): story_id = add_story(ingested_at="2026-01-01T00:00:00", calibre_id=1, calibre_uuid="uuid-1") job = Job(mode=DownloadMode.REBUILD, story_id=story_id) assert "library book 1" in orchestrator._run_rebuild(job) - orchestrator.store.update_story(story_id, last_changed=None, message="before") + orchestrator.store.update_story(story_id, last_changed=last_changed, message="before") assert orchestrator._run_rebuild(job) == "No changes" - assert (story(story_id)["last_changed"], story(story_id)["message"]) == (None, "before") + assert (story(story_id)["last_changed"], story(story_id)["message"]) == (last_changed, "before") assert builds.call_count == 1 # The Rebuild button of the story page. queued = client.post(f"/api/stories/{story_id}/download", json={"mode": "rebuild"}, headers=H).json @@ -124,7 +138,7 @@ def test_a_rebuild_without_a_change_does_nothing_unless_the_user_asks_for_it(bui assert "library book 1" in orchestrator._run_rebuild(jobs.get_next()) assert builds.call_count == 2 assert len(ingest_files()) == 4 - assert story(story_id)["last_changed"] + assert story(story_id)["last_changed"] == last_changed # Other modes take no such option. queued = client.post(f"/api/stories/{story_id}/download", json={"mode": "new"}, headers=H).json assert queued["options"] == {} @@ -152,6 +166,31 @@ def test_a_failed_delivery_is_tried_again_without_a_build(builds, fake, monkeypa assert builds.call_count == 1 +@pytest.mark.parametrize("change", ["chapter", "revision"]) +def test_chapter_timestamp_survives_a_failed_build_and_retry(builds, monkeypatch, change): + story_id = linked_story(ONE) + site(monkeypatch, ONE) + update(story_id) + previous = "2020-01-01T00:00:00" + orchestrator.store.update_story(story_id, last_changed=previous) + clock = ["2024-01-01T00:00:00"] + monkeypatch.setattr(storage, "now", lambda: clock[0]) + site(monkeypatch, [*ONE, (2, "Two", "b")] if change == "chapter" else [(1, "One", "revised")]) + job = Job(mode=DownloadMode.RECHECK_ALL, story_id=story_id) + build = orchestrator._build + monkeypatch.setattr(orchestrator, "_build", Mock(side_effect=OSError("disk full"))) + with pytest.raises(OSError, match="disk full"): + orchestrator._run_site_job(job) + assert story(story_id)["last_changed"] == clock[0] + assert job.result["new"] or job.result["changed"] + clock[0] = "2024-02-01T00:00:00" + monkeypatch.setattr(orchestrator, "_build", build) + assert "library book 1" in orchestrator._run_site_job(job) + assert story(story_id)["last_changed"] == "2024-01-01T00:00:00" + assert update(story_id).startswith("No changes") + assert story(story_id)["last_changed"] == "2024-01-01T00:00:00" + + def test_a_new_build_removes_the_older_file(builds, monkeypatch): story_id = linked_story(ONE) site(monkeypatch, ONE) @@ -227,6 +266,7 @@ def test_a_download_uses_the_built_book(builds, client, monkeypatch): def test_a_download_builds_a_book_that_has_no_file_or_that_changed(builds, client, monkeypatch): story_id = add_story() + changed_at = story(story_id)["last_changed"] first = download(client, story_id) download(client, story_id) assert builds.call_count == 1 @@ -234,12 +274,12 @@ def test_a_download_builds_a_book_that_has_no_file_or_that_changed(builds, clien assert download(client, story_id) != first assert builds.call_count == 2 assert len(list(config.TMP_DIR.iterdir())) == 1 - # The build of a download is not the build of a job. The job sends the book, and marks the change. + # The build of a download is not the build of a job. The job sends the book. assert orchestrator.store.get_setting(story_id, "built_key") is None job = Job(mode=DownloadMode.REBUILD, story_id=story_id) assert "library book 1" in orchestrator._run_rebuild(job) assert builds.call_count == 2 - assert story(story_id)["last_changed"] + assert story(story_id)["last_changed"] == changed_at # Page counts diff --git a/tests/test_last_changed.py b/tests/test_last_changed.py new file mode 100644 index 0000000..7231d92 --- /dev/null +++ b/tests/test_last_changed.py @@ -0,0 +1,66 @@ +"""Last Changed records stored chapters and revisions, not changes to the built book.""" + +import sqlite3 + +import pytest + +from bindery import store as storage + + +@pytest.fixture +def state(tmp_path, monkeypatch): + clock = ["2024-01-01T00:00:00"] + monkeypatch.setattr(storage, "now", lambda: clock[0]) + store = storage.Store(tmp_path / "stories.db") + sid = store.create_story("story", "site", "Title", "Author", []) + return store, sid, clock + + +@pytest.mark.parametrize("source", ["new", "import"]) +def test_new_chapters_and_revisions_set_the_timestamp_in_the_store(state, source): + store, sid, clock = state + other = store.create_story("other", "site", "Other", "Author", []) + assert store.get_story(sid)["last_changed"] is None + first = store.add_chapter(sid, "one", "One", "first", "h1", source, 0) + assert store.get_story(sid)["last_changed"] == clock[0] + assert store.get_chapter(first)["added_at"] == clock[0] + clock[0] = "2024-02-01T00:00:00" + store.add_chapters(sid, [{"url": "two", "title": "Two", "html": "second", "text_hash": "h2"}], source, []) + assert store.get_story(sid)["last_changed"] == clock[0] + clock[0] = "2024-03-01T00:00:00" + store.add_revision(first, "revised", "h3", "recheck") + assert store.get_story(sid)["last_changed"] == clock[0] + assert store.get_chapter(first)["changed_at"] == clock[0] + assert store.list_revisions(first)[-1]["fetched_at"] == clock[0] + assert storage.Store(store.path).get_story(sid)["last_changed"] == clock[0] + assert store.get_story(other)["last_changed"] is None + + +def test_changes_without_new_chapters_or_revisions_keep_the_timestamp(state): + store, sid, clock = state + first = store.add_chapter(sid, "one", "One", "first", "h1", "new", 0) + second = store.add_chapter(sid, "two", "Two", "second", "h2", "new", 1) + store.add_revision(first, "revised", "h3", "recheck") + previous = clock[0] + clock[0] = "2024-02-01T00:00:00" + store.update_story(sid, title="Edited", pinned={"title": "Edited"}) + store.set_rules(sid, [{"pattern": "first", "replacement": "1st"}]) + store.update_chapter(first, title="Renamed", included=False, on_site=False, current_rev=1) + store.replace_revision(first, 1, "original site text", "h4") + store.reorder_chapters(sid, lambda order: list(reversed(order))) + store.add_chapters(sid, [], "import", ["one", "two"]) + store.delete_revision(first, 2) + store.delete_chapters(sid, [second]) + assert store.get_story(sid)["last_changed"] == previous + + +def test_a_failed_chapter_transaction_does_not_change_the_timestamp(state): + store, sid, clock = state + store.add_chapter(sid, "one", "One", "first", "h1", "new", 0) + previous = clock[0] + clock[0] = "2024-02-01T00:00:00" + chapters = [{"url": url, "title": url, "html": "text", "text_hash": "h"} for url in ("two", "one")] + with pytest.raises(sqlite3.IntegrityError): + store.add_chapters(sid, chapters, "import", []) + assert store.get_story(sid)["last_changed"] == previous + assert [chapter["url"] for chapter in store.list_chapters(sid)] == ["one"] diff --git a/tests/test_opds.py b/tests/test_opds.py index 398997a..3f89b4f 100644 --- a/tests/test_opds.py +++ b/tests/test_opds.py @@ -48,10 +48,10 @@ def add_book(title="A book", author="Writer", changed=None, url=None, included=T store = orchestrator.store url = url or f"https://example.com/books/{len(store.list_stories()) + 1}" sid = store.create_story(url, "example.com", title, author, []) - store.update_story(sid, last_changed=changed, site_values=meta or {}) if included: html = "
Stored text.
" store.add_chapter(sid, url + "/1", "One", html, diffing.text_hash(html), "site", 0) + store.update_story(sid, last_changed=changed, site_values=meta or {}) return sid