From f8a60162454588d933fbe346c28ae6beca9fcebd Mon Sep 17 00:00:00 2001 From: bndct-devops Date: Sun, 5 Jul 2026 11:14:52 +0200 Subject: [PATCH] fix(tomesync): strict filename resolve + position-row uniqueness + unread reset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Server-side correctness fixes from the 2026-07-03 audit (no plugin change): - resolve (C1-C4): the filename→book fallback that fires for never-served files no longer guesses. Whole-phrase title matching with a minimum length (a bare "It" can't match "The Italian Job"), a volume number in the name must match the book's actual series_index (a "Foo v2" no longer lands on a standalone "Foo"), and the volume regex is bounded so a 4-digit year isn't read as a volume and a half-volume (2.5) parses. Ambiguous → 404, never a silent mis-map onto another book's progress/annotations. - position uniqueness (B2): add UNIQUE(user_id, book_id) to tome_sync_positions + a dedupe migration (keep freshest). Both writers (device PUT, web set_book_status) go through a shared upsert helper that absorbs the insert race via a SAVEPOINT, so a device+web first-write can't fork duplicate rows that flap on read and double-count stats. - unread reset (B3): marking a book unread on the web now clears its TomeSyncPosition, so the device can't re-pull the stale position and undo the reset on next open. Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 24 +++ backend/api/tome_sync.py | 114 ++++++++----- backend/api/users.py | 34 ++-- backend/main.py | 24 +++ backend/models/tome_sync.py | 6 + backend/services/book_progress.py | 66 ++++++++ tests/test_tomesync_correctness.py | 249 +++++++++++++++++++++++++++++ 7 files changed, 464 insertions(+), 53 deletions(-) create mode 100644 tests/test_tomesync_correctness.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 71be719..80fb204 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -76,6 +76,30 @@ All notable changes to Tome are documented here. Format loosely follows folder is kept. The same whitelist governs the folder cleanup after a reorganise, which also no longer errors out after the moves have already succeeded. +- **A sideloaded book can no longer hijack another book's reading progress.** + When KOReader opens a file Tome never served (a sideload, or a copy renamed + on the device), Tome matches it to a library book by filename as a last + resort. That matcher was too eager: a short title like *"It"* matched + *The Italian Job*, a `Foo v2` filename landed on a standalone *"Foo"*, and a + year in the name (`… - 1984 - …`) was misread as "volume 1984". Any of these + silently wrote one book's position, sessions and highlights onto another. + Matching is now strict — whole-word title matches, a minimum length before a + substring counts, and a volume number in the filename must line up with the + book's actual volume — and when it can't be sure it declines (the device just + keeps tracking locally) rather than guessing. Books downloaded from Tome are + unaffected: they match by content hash, not by name. +- **Reading position no longer flaps between two values.** The position table + had no uniqueness guard, so a device sync and a web-reader save landing at + the same moment could each create a row for the same book — after which reads + picked an arbitrary one and the progress appeared to jump back and forth (and + stats double-counted it). There is now one position per book, both writers + update it in place, and a one-time cleanup collapses any existing duplicates + to the most recent. +- **Marking a book "unread" on the web now sticks on your device too.** The + reset cleared the web side but left the synced position untouched, so the + next time the book opened on KOReader it jumped back to where you'd been and + flipped itself to "reading" again. Marking a book unread now also clears the + synced position. - **Hardcover sync could pin a translation's or the audiobook's edition.** Publishers reuse one catalogue across translations and audio, so a stored ISBN sometimes names the German edition (or the audiobook) of the right diff --git a/backend/api/tome_sync.py b/backend/api/tome_sync.py index 604af31..d5d7cfd 100644 --- a/backend/api/tome_sync.py +++ b/backend/api/tome_sync.py @@ -29,7 +29,7 @@ from backend.models.tome_sync import Annotation, AnnotationTombstone, ApiKey, ReadingSession, TomeSyncPosition from backend.models.ko_stats import StatsImport from backend.models.send_queue import SendQueueItem -from backend.services.book_progress import apply_progress_to_status +from backend.services.book_progress import apply_progress_to_status, upsert_position from backend.services.hardcover_sync import nudge as hardcover_nudge router = APIRouter(tags=["tome-sync"]) @@ -83,6 +83,57 @@ def _get_position(db: Session, user_id: int, book_id: int) -> Optional[TomeSyncP ) +# ── Filename → book resolution helpers ──────────────────────────────────────── +# These back the heuristic fallbacks in resolve_book, which only fire for files +# that never passed through Tome (no content-hash, no exact path). A wrong guess +# there silently writes one book's progress and annotations onto another, so the +# matching is deliberately strict: it would rather return 404 (device keeps +# local-only tracking) than clobber the wrong book. + +# Titles/stems shorter than this may match only by exact equality — a bare +# "It"/"Us"/"Go" substring otherwise swallows unrelated sideloaded filenames. +_MIN_HEURISTIC_LEN = 4 + + +def _phrase_index(haystack: str, needle: str) -> int: + """Index of needle in haystack where it is not glued to an alphanumeric on + either side (a whole-phrase occurrence), or -1. Tolerant of punctuation in + needle, unlike a naive ``\\b`` regex.""" + start = 0 + n = len(needle) + while True: + pos = haystack.find(needle, start) + if pos == -1: + return -1 + before = haystack[pos - 1] if pos > 0 else "" + after = haystack[pos + n] if pos + n < len(haystack) else "" + if not before.isalnum() and not after.isalnum(): + return pos + start = pos + 1 + + +def _title_in_stem(title_l: str, stem_l: str) -> bool: + """Book title occurs as a whole phrase inside the filename stem. Short + titles must equal the stem exactly, so "It" can't match "The Italian Job".""" + if len(title_l) < _MIN_HEURISTIC_LEN: + return title_l == stem_l + return _phrase_index(stem_l, title_l) != -1 + + +def _stem_in_title(stem_l: str, title_l: str) -> bool: + """Filename stem is a word-boundaried fragment of the title (handles + truncated names). Minimum length guards against "1" matching "1984".""" + if len(stem_l) < _MIN_HEURISTIC_LEN: + return False + pos = title_l.find(stem_l) + while pos != -1: + before = title_l[pos - 1] if pos > 0 else "" + if not before.isalnum(): # stem begins at a word boundary + return True + pos = title_l.find(stem_l, pos + 1) + return False + + # ── Resolve endpoint ───────────────────────────────────────────────────────── @router.get("/tome-sync/resolve") @@ -132,34 +183,36 @@ def resolve_book( return {"book_id": book.id} # Volume number, in any of the shapes the download paths emit: - # "Vol. 12", "v01", or a separator-delimited token "Series - 02 - Title". + # "Vol. 12", "Vol. 2.5", "v01", or a separator-delimited token + # "Series - 02 - Title". The separator token is bounded to 1-3 digits so a + # 4-digit year ("Foo - 1984 - Bar") is not misread as volume 1984, and the + # "Vol." form accepts a half-volume (2.5) instead of truncating to 2. vol_num = None vol_match = ( - re.search(r'[Vv]ol\.?\s*(\d+)', stem) + re.search(r'[Vv]ol\.?\s*(\d+(?:\.\d+)?)', stem) or re.search(r'\bv(\d{1,3})\b', stem) - or re.search(r'[-—]\s*0*(\d+)\s*[-—]', stem) + or re.search(r'[-—]\s*(\d{1,3})\s*[-—]', stem) ) if vol_match: vol_num = float(vol_match.group(1)) all_active = db.query(Book).filter(Book.status == "active").all() - # 2. Forward match: the book's title appears inside the filename. This is the - # strong signal — the download paths all put the title into the name. - candidates = [b for b in all_active if b.title and b.title.lower() in stem_l] + # 2. Forward match: the book's title appears as a whole phrase in the + # filename (short titles must match exactly — see _title_in_stem). + candidates = [b for b in all_active if b.title and _title_in_stem(b.title.lower(), stem_l)] - # When the filename carries a volume number it is authoritative: it selects - # the right volume AND rejects any candidate whose series_index disagrees. - # This is what stops vol-2's filename (which contains vol-1's title when the - # series shares its name with its first book) from resolving to vol-1. + # When the filename carries a volume number it is authoritative: only a book + # at that exact index may resolve. An index-less (standalone) candidate must + # NOT win by substring — a numbered file does not belong to an unnumbered + # book — and two books at the same index are genuinely ambiguous. if vol_num is not None: exact = [b for b in candidates if b.series_index == vol_num] if len(exact) == 1: return {"book_id": exact[0].id} - candidates = [ - b for b in candidates - if b.series_index is None or b.series_index == vol_num - ] + raise HTTPException( + status_code=404, detail="Ambiguous filename; could not resolve uniquely" + ) if len(candidates) == 1: return {"book_id": candidates[0].id} @@ -175,14 +228,12 @@ def resolve_book( status_code=404, detail="Ambiguous filename; could not resolve uniquely" ) - # 3. Reverse fallback: the filename is a substring of exactly one title - # (handles truncated/shortened names). Honour the volume here too. - reverse = [b for b in all_active if b.title and stem_l in b.title.lower()] + # 3. Reverse fallback: the stem is a word-boundaried fragment of exactly one + # title (truncated names). Min length guards against "1" → "1984"; a + # volume number, if present, still requires an exact index match. + reverse = [b for b in all_active if b.title and _stem_in_title(stem_l, b.title.lower())] if vol_num is not None: - reverse = [ - b for b in reverse - if b.series_index is None or b.series_index == vol_num - ] + reverse = [b for b in reverse if b.series_index == vol_num] if len(reverse) == 1: return {"book_id": reverse[0].id} @@ -234,21 +285,10 @@ def put_position( # Clamp percentage to 0-1 range pct = max(0.0, min(1.0, body.percentage)) - pos = _get_position(db, user.id, book_id) - if pos: - pos.progress = body.progress - pos.percentage = pct - pos.device = body.device - pos.updated_at = datetime.utcnow() - else: - pos = TomeSyncPosition( - user_id=user.id, - book_id=book_id, - progress=body.progress, - percentage=pct, - device=body.device, - ) - db.add(pos) + upsert_position( + db, user_id=user.id, book_id=book_id, + percentage=pct, progress=body.progress, device=body.device, + ) # Keep UserBookStatus in sync via the shared sticky-completion rule. # Position sync tracks the device last-write-wins (monotonic=False) — a diff --git a/backend/api/users.py b/backend/api/users.py index baf8439..a40c83d 100644 --- a/backend/api/users.py +++ b/backend/api/users.py @@ -12,6 +12,7 @@ from backend.core.security import get_current_user from backend.core.permissions import require_role from backend.services.hardcover_sync import nudge as hardcover_nudge +from backend.services.book_progress import upsert_position, clear_position from backend.models.user import User, UserPermission from backend.models.user_book_status import UserBookStatus from backend.models.tome_sync import ReadingSession, TomeSyncPosition @@ -134,6 +135,9 @@ def set_book_status( if body.status == 'unread': row.progress_pct = None row.cfi = None + # Drop the synced device position too, or the device re-pulls it on + # open and the "unread" reset silently un-does itself. + clear_position(db, user_id=current_user.id, book_id=book_id) else: if 'progress_pct' in raw: row.progress_pct = body.progress_pct @@ -158,25 +162,23 @@ def set_book_status( # progress_pct is a 0–1 fraction end-to-end (API field, UserBookStatus.progress_pct, # and TomeSyncPosition.percentage), so no scaling is needed. if body.progress_pct is not None or body.cfi is not None: - pos = db.query(TomeSyncPosition).filter( + # Upsert against the UNIQUE(user, book) constraint so a device heartbeat + # racing this write can't fork a duplicate position row. Preserve the + # partial-update semantics: only overwrite the fields actually sent. + existing = db.query(TomeSyncPosition).filter( TomeSyncPosition.user_id == current_user.id, TomeSyncPosition.book_id == book_id, ).first() - if pos: - if body.progress_pct is not None: - pos.percentage = body.progress_pct - if body.cfi is not None: - pos.progress = body.cfi - pos.device = "web" - pos.updated_at = datetime.utcnow() - else: - db.add(TomeSyncPosition( - user_id=current_user.id, - book_id=book_id, - percentage=(body.progress_pct or 0), - progress=body.cfi, - device="web", - )) + pct = body.progress_pct if body.progress_pct is not None else ( + existing.percentage if existing else 0.0 + ) + cfi = body.cfi if body.cfi is not None else ( + existing.progress if existing else None + ) + upsert_position( + db, user_id=current_user.id, book_id=book_id, + percentage=pct, progress=cfi, device="web", + ) db.commit() # ── Web reader session tracking ────────────────────────────────────────── diff --git a/backend/main.py b/backend/main.py index 58ffc1d..5aaf2f6 100644 --- a/backend/main.py +++ b/backend/main.py @@ -145,6 +145,30 @@ async def lifespan(app: FastAPI): if qc_cols and "poll_token" not in qc_cols: conn.execute(text("ALTER TABLE quick_connect_codes ADD COLUMN poll_token VARCHAR(64)")) conn.commit() + # Enforce one reading-position row per (user, book). Concurrent device + # and web first-writes could previously both INSERT, leaving duplicate + # rows that made position reads flap and double-counted stats joins. + # Dedupe (keep the freshest) before adding the constraint so it can't + # fail on legacy data. + tsp_cols = {r[1] for r in conn.execute(text("PRAGMA table_info(tome_sync_positions)")).fetchall()} + if tsp_cols: + conn.execute(text(""" + DELETE FROM tome_sync_positions + WHERE id NOT IN ( + SELECT keep_id FROM ( + SELECT id AS keep_id, ROW_NUMBER() OVER ( + PARTITION BY user_id, book_id + ORDER BY updated_at DESC, id DESC + ) AS rn + FROM tome_sync_positions + ) WHERE rn = 1 + ) + """)) + conn.execute(text( + "CREATE UNIQUE INDEX IF NOT EXISTS uq_tspos_user_book " + "ON tome_sync_positions (user_id, book_id)" + )) + conn.commit() # Per-user ratings/reviews on user_book_status (nullable — existing rows stay unrated). ubs_cols = {r[1] for r in conn.execute(text("PRAGMA table_info(user_book_status)")).fetchall()} if ubs_cols and "rating" not in ubs_cols: diff --git a/backend/models/tome_sync.py b/backend/models/tome_sync.py index a18d52c..c45e49f 100644 --- a/backend/models/tome_sync.py +++ b/backend/models/tome_sync.py @@ -65,6 +65,12 @@ class ReadingSession(Base): class TomeSyncPosition(Base): """Latest reading position per user+book, updated on every push.""" __tablename__ = "tome_sync_positions" + __table_args__ = ( + # One position row per user+book. Two writers (device heartbeat, web + # autosave) both do read-modify-write; without this they could each + # INSERT a first-ever row and fork into duplicates that flap on read. + UniqueConstraint("user_id", "book_id", name="uq_tspos_user_book"), + ) id: Mapped[int] = mapped_column(Integer, primary_key=True) user_id: Mapped[int] = mapped_column( diff --git a/backend/services/book_progress.py b/backend/services/book_progress.py index 2f8e090..d3cedab 100644 --- a/backend/services/book_progress.py +++ b/backend/services/book_progress.py @@ -19,11 +19,77 @@ from datetime import datetime from typing import Optional +from sqlalchemy.exc import IntegrityError from sqlalchemy.orm import Session +from backend.models.tome_sync import TomeSyncPosition from backend.models.user_book_status import UserBookStatus +def upsert_position( + db: Session, + *, + user_id: int, + book_id: int, + percentage: float, + progress: Optional[str], + device: Optional[str], +) -> TomeSyncPosition: + """Insert-or-update the single (user, book) reading-position row. + + Both the device heartbeat and the web reader's autosave write positions; + with a UNIQUE(user_id, book_id) constraint in place, a losing concurrent + INSERT raises IntegrityError, which we absorb (via a SAVEPOINT so the + caller's other pending changes survive) and retry as an UPDATE. The two + writers then converge on one row instead of silently forking into two. + Does not commit — the caller owns the transaction. + """ + row = ( + db.query(TomeSyncPosition) + .filter(TomeSyncPosition.user_id == user_id, TomeSyncPosition.book_id == book_id) + .first() + ) + if row is None: + row = TomeSyncPosition( + user_id=user_id, book_id=book_id, + percentage=percentage, progress=progress, device=device, + ) + try: + with db.begin_nested(): + db.add(row) + db.flush() + return row + except IntegrityError: + # Another writer created the row between our SELECT and INSERT. + row = ( + db.query(TomeSyncPosition) + .filter(TomeSyncPosition.user_id == user_id, + TomeSyncPosition.book_id == book_id) + .first() + ) + if row is None: + raise + + row.percentage = percentage + row.progress = progress + row.device = device + row.updated_at = datetime.utcnow() + return row + + +def clear_position(db: Session, *, user_id: int, book_id: int) -> None: + """Drop the synced reading position for a user+book. + + Called when the web explicitly resets a book to "unread": otherwise the + stale position row survives, the device re-pulls it on open, and the reset + un-does itself. Does not commit — the caller owns the transaction. + """ + db.query(TomeSyncPosition).filter( + TomeSyncPosition.user_id == user_id, + TomeSyncPosition.book_id == book_id, + ).delete(synchronize_session=False) + + def apply_progress_to_status( db: Session, *, diff --git a/tests/test_tomesync_correctness.py b/tests/test_tomesync_correctness.py new file mode 100644 index 0000000..4d89576 --- /dev/null +++ b/tests/test_tomesync_correctness.py @@ -0,0 +1,249 @@ +"""Server-side TomeSync correctness fixes (2026-07-03 audit). + +- C1-C4: filename→book resolution must not silently map a file onto the wrong + book. Word-boundary/min-length matching, index-aware volume handling, and a + bounded volume regex. +- B2: one reading-position row per (user, book) — the two writers (device + heartbeat, web autosave) converge instead of forking duplicates. +- B3: resetting a book to "unread" on the web clears the synced position, so + the device can't re-pull it and undo the reset. +""" +import pytest +from sqlalchemy.exc import IntegrityError +from starlette.testclient import TestClient + +from backend.models.tome_sync import ApiKey, TomeSyncPosition +from backend.models.user_book_status import UserBookStatus + + +@pytest.fixture() +def api_key(db, admin_user) -> str: + user, _ = admin_user + plaintext = ApiKey.generate() + db.add(ApiKey(user_id=user.id, key_hash=ApiKey.hash_key(plaintext), + key_prefix=plaintext[:11], label="test")) + db.flush() + return plaintext + + +def _resolve(client: TestClient, api_key: str, filename: str): + return client.get( + "/api/tome-sync/resolve", + params={"filename": filename}, + headers={"Authorization": f"Bearer {api_key}"}, + ) + + +# --------------------------------------------------------------------------- +# C1 — short titles / word boundaries +# --------------------------------------------------------------------------- + +class TestResolveC1: + def test_short_title_does_not_swallow_unrelated_file(self, client, db, make_book, api_key): + make_book(title="It") # Stephen King, 2 chars + db.commit() + # Never served by Tome → no md5/path hit → heuristic fallback. + r = _resolve(client, api_key, "The Italian Job.epub") + assert r.status_code == 404 + + def test_short_title_still_matches_exact_stem(self, client, db, make_book, api_key): + book = make_book(title="It") + db.commit() + r = _resolve(client, api_key, "It.epub") + assert r.status_code == 200 + assert r.json()["book_id"] == book.id + + def test_title_must_be_whole_phrase(self, client, db, make_book, api_key): + # "Room" as a substring of "Mushroom Cookbook" must not match. + make_book(title="Room") + db.commit() + r = _resolve(client, api_key, "Mushroom Cookbook.epub") + assert r.status_code == 404 + + def test_most_specific_title_wins(self, client, db, make_book, api_key): + make_book(title="Dune") + messiah = make_book(title="Dune Messiah") + db.commit() + r = _resolve(client, api_key, "Dune Messiah.epub") + assert r.status_code == 200 + assert r.json()["book_id"] == messiah.id + + def test_plain_title_still_resolves(self, client, db, make_book, api_key): + # Regression guard: the emulator-verified Frankenstein path. + book = make_book(title="Frankenstein") + db.commit() + r = _resolve(client, api_key, "Frankenstein; or, the modern prometheus.epub") + assert r.status_code == 200 + assert r.json()["book_id"] == book.id + + +# --------------------------------------------------------------------------- +# C2 — volume number vs index-less book +# --------------------------------------------------------------------------- + +class TestResolveC2: + def test_numbered_file_does_not_land_on_standalone(self, client, db, make_book, api_key): + make_book(title="Foo", series=None, series_index=None) # standalone + db.commit() + # "Foo v2" — vol 2 of a Foo series that isn't in Tome. + r = _resolve(client, api_key, "Foo v2.epub") + assert r.status_code == 404 + + def test_numbered_file_resolves_to_matching_index(self, client, db, make_book, api_key): + v2 = make_book(title="Gantz", series="Gantz", series_index=2) + db.commit() + r = _resolve(client, api_key, "Gantz v2.epub") + assert r.status_code == 200 + assert r.json()["book_id"] == v2.id + + def test_series_browser_filename_resolves(self, client, db, make_book, api_key): + v1 = make_book(title="Berserk, Vol. 1", series="Berserk", series_index=1) + db.commit() + r = _resolve(client, api_key, "Vol. 1 — Berserk, Vol. 1.cbz") + assert r.status_code == 200 + assert r.json()["book_id"] == v1.id + + def test_wrong_volume_number_refuses(self, client, db, make_book, api_key): + make_book(title="Gantz", series="Gantz", series_index=1) + db.commit() + r = _resolve(client, api_key, "Gantz v2.epub") # only vol 1 exists + assert r.status_code == 404 + + +# --------------------------------------------------------------------------- +# C3 — reverse fallback minimum length +# --------------------------------------------------------------------------- + +class TestResolveC3: + def test_tiny_stem_does_not_match_longer_title(self, client, db, make_book, api_key): + make_book(title="1984") + db.commit() + r = _resolve(client, api_key, "1.epub") # split-chapter stem "1" + assert r.status_code == 404 + + def test_truncated_stem_still_resolves(self, client, db, make_book, api_key): + book = make_book(title="Frankenstein") + db.commit() + # A shortened filename that is a leading fragment of exactly one title. + r = _resolve(client, api_key, "Franken.epub") + assert r.status_code == 200 + assert r.json()["book_id"] == book.id + + def test_midword_fragment_does_not_match(self, client, db, make_book, api_key): + make_book(title="Starship Troopers") + db.commit() + r = _resolve(client, api_key, "ship.epub") # not at a word boundary + assert r.status_code == 404 + + +# --------------------------------------------------------------------------- +# C4 — volume regex bounds (year token, half-volume) +# --------------------------------------------------------------------------- + +class TestResolveC4: + def test_year_token_not_read_as_volume(self, client, db, make_book, api_key): + # "- 1984 -" must not parse as volume 1984 and then exclude the book. + book = make_book(title="Collected Orwell", series="Collected Orwell", series_index=3) + db.commit() + r = _resolve(client, api_key, "Collected Orwell - 1984 - Essays.epub") + assert r.status_code == 200 + assert r.json()["book_id"] == book.id + + def test_half_volume_resolves(self, client, db, make_book, api_key): + book = make_book(title="Chainsaw Man", series="Chainsaw Man", series_index=2.5) + db.commit() + r = _resolve(client, api_key, "Chainsaw Man Vol. 2.5.epub") + assert r.status_code == 200 + assert r.json()["book_id"] == book.id + + def test_separator_volume_still_parses(self, client, db, make_book, api_key): + book = make_book(title="Some Title", series="Series", series_index=2) + db.commit() + r = _resolve(client, api_key, "Series - 02 - Some Title.epub") + assert r.status_code == 200 + assert r.json()["book_id"] == book.id + + +# --------------------------------------------------------------------------- +# B2 — one position row per (user, book) +# --------------------------------------------------------------------------- + +class TestPositionUniqueness: + def test_duplicate_insert_rejected_by_constraint(self, db, admin_user, make_book): + user, _ = admin_user + book = make_book(title="Dup") + db.flush() + db.add(TomeSyncPosition(user_id=user.id, book_id=book.id, percentage=0.1)) + db.flush() + db.add(TomeSyncPosition(user_id=user.id, book_id=book.id, percentage=0.2)) + with pytest.raises(IntegrityError): + db.flush() + db.rollback() + + def test_device_then_web_converge_to_one_row(self, client, db, make_book, admin_user, api_key): + user, jwt = admin_user + book = make_book(title="Converge") + db.commit() + + # Device heartbeat (api-key auth) + r1 = client.put( + f"/api/tome-sync/position/{book.id}", + json={"percentage": 0.30, "progress": "cfi-device", "device": "kindle"}, + headers={"Authorization": f"Bearer {api_key}"}, + ) + assert r1.status_code == 200 + # Web autosave (JWT auth — client default header) + r2 = client.put( + f"/api/books/{book.id}/status", + json={"status": "reading", "progress_pct": 0.55, "cfi": "cfi-web"}, + ) + assert r2.status_code == 200 + + rows = db.query(TomeSyncPosition).filter( + TomeSyncPosition.user_id == user.id, TomeSyncPosition.book_id == book.id + ).all() + assert len(rows) == 1 + assert rows[0].percentage == 0.55 # last writer wins on the single row + + def test_repeated_device_push_updates_in_place(self, client, db, make_book, admin_user, api_key): + user, _ = admin_user + book = make_book(title="Repeat") + db.commit() + for pct in (0.1, 0.2, 0.3): + client.put( + f"/api/tome-sync/position/{book.id}", + json={"percentage": pct, "progress": f"cfi-{pct}", "device": "kindle"}, + headers={"Authorization": f"Bearer {api_key}"}, + ) + rows = db.query(TomeSyncPosition).filter( + TomeSyncPosition.user_id == user.id, TomeSyncPosition.book_id == book.id + ).all() + assert len(rows) == 1 + assert rows[0].percentage == 0.3 + + +# --------------------------------------------------------------------------- +# B3 — mark unread clears the synced position +# --------------------------------------------------------------------------- + +class TestMarkUnreadClearsPosition: + def test_unread_deletes_position(self, client, db, make_book, admin_user): + user, _ = admin_user + book = make_book(title="ReReadMe") + db.commit() + + # Read to 74% via the web writer → creates a position row. + client.put( + f"/api/books/{book.id}/status", + json={"status": "reading", "progress_pct": 0.74, "cfi": "cfi-74"}, + ) + assert db.query(TomeSyncPosition).filter_by(user_id=user.id, book_id=book.id).count() == 1 + + # Reset to unread → the synced position must be gone. + r = client.put(f"/api/books/{book.id}/status", json={"status": "unread"}) + assert r.status_code == 200 + assert db.query(TomeSyncPosition).filter_by(user_id=user.id, book_id=book.id).count() == 0 + + status = db.query(UserBookStatus).filter_by(user_id=user.id, book_id=book.id).first() + assert status.status == "unread" + assert status.progress_pct is None