From 6cd4315e08eecd3a547cbd3851961507f924c5cd Mon Sep 17 00:00:00 2001 From: Oliver Gassner Date: Sat, 26 Sep 2026 15:09:38 +0200 Subject: [PATCH 1/5] Add durable bidirectional EPUB annotation synchronization --- apps/readest-sync/CMakeLists.txt | 2 +- apps/readest-sync/CONTEXT.md | 9 +- .../research/readest-annotation-protocol.md | 195 ++++++++++ apps/readest-sync/src/annotations.cpp | 340 ++++++++++++++++++ apps/readest-sync/src/annotations.h | 12 + apps/readest-sync/src/application.cpp | 10 + apps/readest-sync/src/application.h | 2 +- apps/readest-sync/src/device_adapter.cpp | 1 + apps/readest-sync/src/integrity.cpp | 119 ++++++ apps/readest-sync/src/integrity.h | 3 + apps/readest-sync/src/position.cpp | 25 +- apps/readest-sync/src/probe.h | 4 + apps/readest-sync/src/state.cpp | 15 + apps/readest-sync/src/state.h | 3 + apps/readest-sync/tests/CMakeLists.txt | 1 + apps/readest-sync/tests/annotations_test.cpp | 115 ++++++ apps/readest-sync/tests/sync_test.cpp | 18 +- 17 files changed, 860 insertions(+), 14 deletions(-) create mode 100644 apps/readest-sync/docs/research/readest-annotation-protocol.md create mode 100644 apps/readest-sync/src/annotations.cpp create mode 100644 apps/readest-sync/src/annotations.h create mode 100644 apps/readest-sync/tests/annotations_test.cpp diff --git a/apps/readest-sync/CMakeLists.txt b/apps/readest-sync/CMakeLists.txt index bb30111..3eff058 100644 --- a/apps/readest-sync/CMakeLists.txt +++ b/apps/readest-sync/CMakeLists.txt @@ -54,7 +54,7 @@ else() endif() add_library(readest-core STATIC src/application.cpp src/upload.cpp src/cloud.cpp src/download.cpp src/http.cpp src/integrity.cpp src/library.cpp - src/position.cpp src/probe.cpp src/progress.cpp src/state.cpp src/sync.cpp + src/annotations.cpp src/position.cpp src/probe.cpp src/progress.cpp src/state.cpp src/sync.cpp src/vendor/miniz/miniz.c) target_compile_definitions(readest-core PUBLIC MINIZ_NO_DEFLATE_APIS MINIZ_NO_ZLIB_APIS) target_include_directories(readest-core PUBLIC src) diff --git a/apps/readest-sync/CONTEXT.md b/apps/readest-sync/CONTEXT.md index 3ff06b2..b29799f 100644 --- a/apps/readest-sync/CONTEXT.md +++ b/apps/readest-sync/CONTEXT.md @@ -1,6 +1,6 @@ # Readest Sync -Readest Sync connects a PocketBook device's books and reading progress with a +Readest Sync connects a PocketBook device's books, reading progress, and annotations with a Readest library. ## Language @@ -31,3 +31,10 @@ the same linked book while having different saved reading positions. **PocketBook library**: The native device library that identifies the EPUBs available for local browsing in Readest Sync. + +**Annotation**: +A highlighted passage in a linked EPUB, optionally carrying a written note. + +**Annotation conflict**: +Different changes to the same annotation on PocketBook and Readest since their +last successful synchronization. Both versions are retained until resolved. diff --git a/apps/readest-sync/docs/research/readest-annotation-protocol.md b/apps/readest-sync/docs/research/readest-annotation-protocol.md new file mode 100644 index 0000000..ec582e8 --- /dev/null +++ b/apps/readest-sync/docs/research/readest-annotation-protocol.md @@ -0,0 +1,195 @@ +# Bidirectional annotation sync: upstream protocol and native prerequisite + +Research date: 2026-09-26. Readest source pinned to +[`8d76d3fc1f0e26f8c5dbe1fc343f9d69256eefc2`](https://github.com/readest/readest/tree/8d76d3fc1f0e26f8c5dbe1fc343f9d69256eefc2). +This is source research, not a live cloud or device round-trip test. + +## Findings + +The KOReader plugin uses the normal Readest `/api/sync` endpoint. It pushes +`{books: [], notes: [...], configs: []}` and pulls with `type=notes`, `since` +(epoch milliseconds), `book` and `meta_hash`. There is no separate annotation +endpoint to reproduce. [Endpoint specification](https://github.com/readest/readest/blob/8d76d3fc1f0e26f8c5dbe1fc343f9d69256eefc2/apps/readest.koplugin/readest-sync-api.json#L1-L19) + +Push fields are camelCase: `bookHash`, `metaHash`, `id`, `type`, `xpointer0`, +optional `xpointer1`, selected `text`, optional user `note`, `style`, `color`, +`page`, `createdAt`, `updatedAt`, and optional `deletedAt`. Highlights and their +notes share one `annotation` record; bookmarks use `bookmark`. The plugin maps +underline and strikeout styles and several colors. PDF-style table positions +are not exported by this descriptor, and PDF pull is explicitly unsupported. +[Plugin implementation](https://github.com/readest/readest/blob/8d76d3fc1f0e26f8c5dbe1fc343f9d69256eefc2/apps/readest.koplugin/readest_syncannotations.lua) + +Readest additionally accepts `cfi` and `global`; the server stores supplied +location fields, rather than converting them in the payload transformer. Pull +returns snake_case database fields and ISO timestamps. Readest's client-side +XCFI converter supports ranges and needs the EPUB document; the KOReader plugin +itself consumes XPointer endpoints. A native PocketBook implementation must +preserve both endpoints, not approximate a highlight with a page or reading +position. [Payload conversion](https://github.com/readest/readest/blob/8d76d3fc1f0e26f8c5dbe1fc343f9d69256eefc2/apps/readest-app/src/utils/transform.ts), +[range conversion](https://github.com/readest/readest/blob/8d76d3fc1f0e26f8c5dbe1fc343f9d69256eefc2/apps/readest-app/src/utils/xcfi.ts#L62-L118) + +## Identity, edits and deletions + +- The plugin preserves a pulled note's ID. For native KOReader items it derives + seven hexadecimal characters from MD5 of `ko:book_hash:type:pos0:pos1`. + Pull deduplicates by ID and then by range endpoints (bookmark: one endpoint). + This is useful precedent for persistent identity, not a reason to copy its + short hash or its KOReader-specific location encoding. [Identity and pull](https://github.com/readest/readest/blob/8d76d3fc1f0e26f8c5dbe1fc343f9d69256eefc2/apps/readest.koplugin/readest_syncannotations.lua) +- **Remote edits are a limitation of this plugin:** pull skips any live record + whose ID or position already exists. It does not replace the existing note, + selected text, color, style or range. Thus “both ways” in this implementation + covers additions and deletions, but does not establish correct propagation of + edits to existing records. PocketBook should not inherit this behavior. + [Pull implementation](https://github.com/readest/readest/blob/8d76d3fc1f0e26f8c5dbe1fc343f9d69256eefc2/apps/readest.koplugin/readest_syncannotations.lua) +- Locally deleted records become persisted tombstones in the book sidecar. + They are sent with a deletion timestamp and retained on failed push. Only + the exact tombstones acknowledged by that request are cleared. Pull remembers + pending deletions from both request start and completion, preventing a stale + response from resurrecting a deleted highlight. Remote tombstones remove + matching local records before additions are considered. [Implementation](https://github.com/readest/readest/blob/8d76d3fc1f0e26f8c5dbe1fc343f9d69256eefc2/apps/readest.koplugin/readest_syncannotations.lua), + [regression specifications](https://github.com/readest/readest/blob/8d76d3fc1f0e26f8c5dbe1fc343f9d69256eefc2/apps/readest.koplugin/spec/syncannotations_spec.lua) +- Server writes use `(user_id, book_hash, id)` identity. Existing note rows are + replaced when the incoming update timestamp **or** deletion timestamp is + newer; this is row replacement, not field-level conflict reconciliation. + Inserts get a server update timestamp. POST returns authoritative records, + including the server winner, but the plugin push callback ignores that body. + [Server push](https://github.com/readest/readest/blob/8d76d3fc1f0e26f8c5dbe1fc343f9d69256eefc2/apps/readest-app/src/pages/api/sync.ts#L659-L879) + +## Cursors, pagination and retries + +Notes pull filters `updated_at > since OR deleted_at > since`. When both book +and metadata hashes are present it matches either hash. The server internally +accumulates pages of 1,000 rows, then deduplicates by ID, choosing the largest +update/deletion timestamp and preferring a tombstone on ties. Notes do not use +the newer server-assigned `synced_at` cursor used for books, or the explicit +limited books pagination contract. [Server pull](https://github.com/readest/readest/blob/8d76d3fc1f0e26f8c5dbe1fc343f9d69256eefc2/apps/readest-app/src/pages/api/sync.ts#L217-L440) + +The plugin uses `settings.last_notes_sync_at` for push selection and pull. +Successful push advances it to request-start seconds minus one millisecond, +preserving edits in that second. Nonempty successful pull advances it to the +client's current wall clock. Full sync uses zero. **Inference:** copying this +cursor scheme would risk missing late-arriving or clock-skewed records; it is +not proof of a lossless incremental protocol. Native sync needs durable +per-linked-book reconciliation state and retry-safe application before any +watermark advances. [Plugin cursors](https://github.com/readest/readest/blob/8d76d3fc1f0e26f8c5dbe1fc343f9d69256eefc2/apps/readest.koplugin/readest_syncannotations.lua) + +The client retries certain read-only transport timeouts once with longer +timeouts. It does not automatically retry ambiguous writes or HTTP errors. +Pending tombstones survive failed pushes for subsequent attempts. +[Transport](https://github.com/readest/readest/blob/8d76d3fc1f0e26f8c5dbe1fc343f9d69256eefc2/apps/readest.koplugin/readest_syncclient.lua#L164-L208) + +## Native PocketBook prerequisite + +The examined public SDK header exposes `OpenBookmarks` and `SwitchBookmark` +using caller-provided page/position arrays. It does not establish a complete +native EPUB highlight-and-note write contract. This is a finding about this +header, not an exhaustive claim about all firmware internals. +[PocketBook SDK, pinned commit](https://github.com/pocketbook/SDK_6.3.0/blob/dd8de463733a2bc0d53d1214b64734586fac427e/SDK-B288/usr/arm-obreey-linux-gnueabi/sysroot/usr/local/include/inkview.h#L2214-L2217) + +Local firmware strings for U743g.6.11.1683 contain `getBookmarks`, +`saveBookmark`, `dbReadAllBookmarks`, `newBookmarkWithExternalUuid` and +`delBookmark` names under `PocketBook::ReaderLib::DocumentObj`. These are leads, +not a supported ABI or sufficient evidence for direct database writes. +Evidence: `build/readest-sync/firmware-6.11.1683/reader-strings.txt`. +The existing native reading-position probe is not an annotation decoder; +its point conversion must not be reused in a way that drops a range endpoint. +[Local probe](../../src/probe.cpp), [position conversion](../../src/position.cpp), +[probe tests](../../tests/probe_test.cpp). + +No native annotation fixtures were inspected in this investigation; the device +was unmounted. The next concrete experiment is a controlled EPUB containing +a native highlight plus note, followed by edit and deletion snapshots. Inspect +the associated database and sidecar changes read-only, establish stable IDs, +range encoding, timestamps and deletion behavior, and verify a round trip on +that same book before enabling native writes. Do not assume an older community +database tag/type layout matches this firmware. + +## Implementation implications + +Reuse Readest's existing notes transport, preserve remote IDs, persist local +identity mappings and an outbox, and acknowledge only applied/accepted changes. +Test repeated sync, edits on both readers, deletion while offline, stale-pull +resurrection, interrupted writes and exact highlighted-text/range round trips. +These are proposed requirements derived from the observed protocol; annotation +sync has not been implemented or validated on the device by this research. + +## First native device snapshot + +On 2026-09-26, a read-only capture from the connected PB743G found two +highlights with attached notes in Beren und Lúthien (book item 48). Both are +`obj.book_mark` items with `bm.type=note`, UUIDs, `State=0`, and `TimeAlt`. +`bm.note` contains JSON text; `bm.quotation` contains JSON `begin`, `end`, +and selected `text`. Both endpoints are `pbr:/page?...#epubcfi(...)` +locations, including text offsets. `bm.book_mark` contains an anchor and +creation timestamp; color is the literal `cian`. Tag-level edit timestamps +are also present. UUID stability across edits and deletion behavior remain +unverified. This establishes the observed representation, not write support. + +Evidence is a local private snapshot (not committed): +`/private/tmp/pocketbook-annotation-baseline-s0bpzmow/annotations.json`, +with the source database/WAL/SHM hashes recorded in `manifest.json`. +The source device files remained byte-for-byte unchanged after inspection. +Next comparison: edit the note “codex note” to “codex note edited” and delete +the complete highlight carrying “test note”, then capture again. + +## Edit and deletion snapshot + +The second capture is `/private/tmp/pocketbook-annotation-edited-j82_h0jt/`. +Both original items (49 and 50) now have `State=2` and retain their UUIDs +and range data. Item 50 changed from `note` to `highlight` with an empty note +object before its final timestamp. This is consistent with clearing the note +then deleting the highlight; the UI action sequence is not yet confirmed. + +The edited note is a new item (51), `State=0`, with a new UUID and creation +timestamp. It contains “codex note edited”. Its quotation JSON matches the +original item 49 exactly, while item 49 retains the old note and has `State=2`. +Thus this observed edit workflow replaces identity rather than preserving it. +The user confirmed using the native Edit action, not deleting and recreating +the annotation. For this observed note edit, native Edit therefore replaces +the item and UUID while marking the previous item deleted. State 2 is consistent with a deletion marker +in this controlled comparison; broader state semantics remain unverified. +Sync must account for replacement plus deletion, not assume immutable native +UUIDs across note edits. All source DB/WAL/SHM hashes remained unchanged. + +## First Readest-to-native import trial + +Fetched the single live “readest import test” note through Readest's notes GET +endpoint using the existing device session (no cloud mutation). The Readest +book hash matched the local validated EPUB, whose SHA-256 was +`bcf0d4757fda435a1253efaccd323b13eb8518948c8889129837fc9b27bf7549`. +Its CFI range was `epubcfi(/6/20!/4/18,/1:0,/1:355)`. Both endpoints resolve +to the expected plain-text paragraph; the first 355 UTF-16 units equal the +selected text exactly. The remaining seven units are trailing whitespace. +The existing XPointer converter independently resolved the remote end +XPointer to the same CFI text offset 355. + +A fixed-fixture diagnostic inserted one new item (52) and five annotation +tags, first into a local snapshot and then transactionally into the device +DB. Native IDs were resolved through type/tag names. Exact pre/post logical +database fingerprints, foreign keys, integrity, duplicate rejection and +rollback on missing-tag failure were checked. The prepared database and +backup are in `/private/tmp/pocketbook-annotation-import-45e25a3q/`; the +diagnostic script is `/private/tmp/pocketbook-annotation-import-trial.py`. +It uses `pbr:/webkit?##` point-CFI anchors, retains the note and yellow color, +and uses a deterministic diagnostic UUID. There were no changes to existing +annotation rows. The diagnostic is not part of the application or an enabled +sync feature. Native reader display and persistence remain to be tested; +successful SQLite insertion alone does not prove native import compatibility. + +## Native display verification + +Inspected device screenshots `screens/scr0011.bmp` and `screens/scr0012.bmp` +(copies in `/private/tmp/pocketbook-annotation-screens/`). The first shows +the exact intended paragraph highlighted, ending at its closing punctuation +and excluding the following paragraph. The second shows the native Comments +dialog with “readest import test” and Edit/Delete controls. The user also +confirmed correct display. This validates the single import fixture; it is +not an end-to-end bidirectional sync test. + +Post-open database capture: `/private/tmp/pocketbook-annotation-after-open-ozyy9jta`. The imported UUID +is preserved and State remains 0. Changed tag names after opening: bm.book_mark, bm.quotation. +The reader normalized `pbr:/webkit?##` anchors to `pbr:/page?...#` +anchors with page/offset metadata, and removed the explicit zero text offset +from the start CFI. The end CFI offset 355, selected text, note, color and UUID +were preserved. Reconciliation must compare semantic ranges rather than raw +location strings, so this normalization does not create a false user edit. diff --git a/apps/readest-sync/src/annotations.cpp b/apps/readest-sync/src/annotations.cpp new file mode 100644 index 0000000..090b919 --- /dev/null +++ b/apps/readest-sync/src/annotations.cpp @@ -0,0 +1,340 @@ +#include "annotations.h" +#include "json_util.h" +#include +#include +#include +#include +#include +#include + +namespace readest { +namespace { +void require(bool ok,const char* message) { if(!ok) throw std::runtime_error(message); } +void check(const std::atomic& cancel) { require(!cancel.load(),"Annotation sync cancelled"); } +Json object() { return Json(json_object_new_object(),json_object_put); } +void put(json_object* o,const char* k,const std::string& s) { json_object_object_add(o,k,json_object_new_string_len(s.data(),s.size())); } +void put(json_object* o,const char* k,long long n) { json_object_object_add(o,k,json_object_new_int64(n)); } +std::string optional(json_object* o,const char* k) { return member(o,k)?string_member(o,k):""; } +std::string hash(const std::string& s) { + unsigned char bytes[EVP_MAX_MD_SIZE]; unsigned size=0; + require(EVP_Digest(s.data(),s.size(),bytes,&size,EVP_sha256(),nullptr)==1,"Cannot hash annotation identity"); + static const char hex[]="0123456789abcdef"; std::string out; + for(unsigned i=0;i>4];out+=hex[bytes[i]&15];} return out; +} +std::string normalized_text(const std::string& s) { + std::string out; + for(unsigned char c:s) { + if(c==' ' || c=='\n' || c=='\r' || c=='\t' || c=='\f') {if(!out.empty() && out.back()!=' ') out+=' ';} + else out+=static_cast(c); + } + if(!out.empty() && out.back()==' ') out.pop_back(); return out; +} +std::string point(std::string s) { + const auto start=s.find("epubcfi("); + if(start!=std::string::npos) s=s.substr(start); + require(!point_cfi(s).empty(),"Unsupported native annotation location"); + // Assertions and explicit zero offsets are not changes to a resolved range. + std::string out; + for(size_t i=0;i3 && out.substr(out.size()-3)==":0)") out.erase(out.size()-3,2); + return out; +} +struct Note { + std::string id,begin,end,text,note,color="yellow",style="highlight",meta; + long long created=0,updated=0,deleted=0; +}; +std::string fingerprint(const Note& n) { + if(n.deleted) return "deleted"; + auto o=object(); put(o.get(),"begin",point(n.begin)); put(o.get(),"end",point(n.end)); + put(o.get(),"text",normalized_text(n.text)); put(o.get(),"note",n.note); + put(o.get(),"color",n.color); put(o.get(),"style",n.style); return hash(json_text(o.get())); +} +std::string fingerprint(const std::map& notes,const std::string& id) { + auto it=notes.find(id); return it==notes.end()?"deleted":fingerprint(it->second); +} +std::string range(const Note& n) { + auto a=point(n.begin), b=point(n.end); const auto bang=a.find('!'); + require(a.substr(0,bang+1)==b.substr(0,b.find('!')+1),"Annotation spans different chapters"); + auto slash=a.find('/',bang+2); + require(slash!=std::string::npos && b.compare(0,slash,a.substr(0,slash))==0,"Unsupported annotation common path"); + auto out=a.substr(0,slash)+","+a.substr(slash,a.size()-slash-1)+","+b.substr(slash); + require(!readest_range_cfi(out).first.empty(),"Cannot encode annotation range"); return out; +} +void validate(const Note& n,const std::string& epub) { + if(n.deleted) return; + require(n.style=="highlight","Unsupported annotation style; nothing overwritten"); + require(n.color=="yellow" || n.color=="#00bcd4","Unsupported annotation color; nothing overwritten"); + require(n.text.size()<=65536 && n.note.size()<=65536 && n.note.find('\0')==std::string::npos,"Invalid annotation text"); + validate_annotation_range(epub,n.begin,n.end,n.text); +} +struct DB { + sqlite3* db=nullptr; + DB(const std::string& path,bool write) { + if(sqlite3_open_v2(path.c_str(),&db,write?SQLITE_OPEN_READWRITE:SQLITE_OPEN_READONLY,nullptr)!=SQLITE_OK) { + sqlite3_close(db); db=nullptr; throw std::runtime_error("Cannot open native annotations database"); + } + sqlite3_busy_timeout(db,3000); + } + ~DB(){sqlite3_close(db);} + void exec(const char* sql) {require(sqlite3_exec(db,sql,nullptr,nullptr,nullptr)==SQLITE_OK,"Native annotation transaction failed");} +}; +struct Query { + sqlite3_stmt* s=nullptr; + Query(DB& db,const char* sql,const std::vector& args={}) { + require(sqlite3_prepare_v2(db.db,sql,-1,&s,nullptr)==SQLITE_OK,"Unsupported native annotation schema"); + for(size_t i=0;i(p),n):"";} +}; +struct Snapshot { std::string book,raw;std::map notes; }; +Snapshot snapshot(DB& db,const std::string& fast_hash) { + require(!fast_hash.empty(),"Native annotation book identity missing"); + Snapshot out; + Query book(db,"SELECT OID FROM Items WHERE TypeID=(SELECT OID FROM TypeNames WHERE TypeName='type.book') AND upper(HashUUID)=upper(?)",{fast_hash}); + require(book.row(),"Open this book once in the native reader before syncing annotations");out.book=book.text(0); + require(!book.row(),"Ambiguous native annotation book identity"); + Query rows(db,"SELECT i.OID,i.HashUUID,i.State,i.TimeAlt,n.TagName,t.Val,t.TimeEdt FROM Items i " + "LEFT JOIN Tags t ON t.ItemID=i.OID LEFT JOIN TagNames n ON n.OID=t.TagID " + "WHERE i.ParentID=? ORDER BY i.OID,n.TagName",{out.book}); + std::map> tags; + std::map all;std::map native_ids; + Json raw(json_object_new_array(),json_object_put); + size_t count=0; + while(rows.row()) { + require(++count<=50000,"Too many native annotations"); + Json row(json_object_new_array(),json_object_put); + for(int i=0;i<7;++i) json_object_array_add(row.get(),json_object_new_string(rows.text(i).c_str())); + json_object_array_add(raw.get(),row.release()); + const auto id=rows.text(1);require(!id.empty(),"Native annotation has no UUID"); + auto [identity,inserted]=native_ids.emplace(id,rows.text(0)); + require(inserted || identity->second==rows.text(0),"Duplicate native annotation UUID"); + auto& n=all[id];n.id=id; + require(rows.text(2)=="0" || rows.text(2)=="2","Unknown native annotation state"); + auto seconds=rows.text(3); + require(!seconds.empty() && seconds.size()<=12 && seconds.find_first_not_of("0123456789")==std::string::npos,"Invalid native annotation timestamp"); + n.updated=std::stoll(seconds)*1000; n.deleted=rows.text(2)=="2"?std::max(1LL,n.updated):0; + tags[id][rows.text(4)]=rows.text(5); + } + out.raw=hash(out.book+json_text(raw.get())); + for(auto& [id,n]:all) { + const auto& t=tags.at(id); auto type=t.find("bm.type"); + if(type==t.end() || (type->second!="note" && type->second!="highlight")) { + require(!t.count("bm.quotation"),"Unsupported native annotation type; nothing overwritten");continue; + } + if(n.deleted) {out.notes[id]=n;continue;} + auto quote=parse_json(t.at("bm.quotation")); auto bookmark=parse_json(t.at("bm.book_mark")); + auto location=[](std::string s) {auto at=s.find("epubcfi(");require(at!=std::string::npos,"Unsupported native annotation anchor");return s.substr(at);}; + n.begin=location(string_member(quote.get(),"begin"));n.end=location(string_member(quote.get(),"end")); + n.text=string_member(quote.get(),"text"); + n.created=integer_member(bookmark.get(),"created"); + require(n.created>0 && n.created<=999999999999LL,"Invalid native annotation creation time");n.created*=1000; + auto note=t.find("bm.note");if(note!=t.end()) {auto o=parse_json(note->second);n.note=optional(o.get(),"text");} + auto color=t.find("bm.color");require(color!=t.end(),"Native annotation color missing");n.color=color->second=="cian"?"#00bcd4":color->second; + auto subtype=t.find("bm.subtype");require(subtype==t.end() || subtype->second.empty(),"Unsupported native annotation subtype"); + out.notes[id]=n; + } + return out; +} +Snapshot read_native(const std::string& database,const std::string& fast_hash) { + DB db(database,false); db.exec("BEGIN");auto result=snapshot(db,fast_hash);db.exec("COMMIT");return result; +} +std::map remote_notes(const std::string& body,const std::string& user,const std::string& book,const std::string& epub) { + auto json=parse_json(body);auto* rows=member(json.get(),"notes"); + require(rows && json_object_get_type(rows)==json_type_array && json_object_array_length(rows)<=5000,"Invalid Readest annotations response"); + std::map notes; + for(size_t i=0;i0,"Missing Readest annotation timestamp"); + if(!n.deleted) { + const auto cfi=optional(row,"cfi");auto endpoints=readest_range_cfi(cfi); + if(endpoints.first.empty() && cfi.empty()) endpoints={xpointer_cfi(epub,string_member(row,"xpointer0")),xpointer_cfi(epub,string_member(row,"xpointer1"))}; + require(!endpoints.first.empty(),"Unsupported Readest annotation range");n.begin=endpoints.first;n.end=endpoints.second; + n.text=string_member(row,"text");n.note=optional(row,"note");n.color=string_member(row,"color");n.style=string_member(row,"style"); + n.meta=optional(row,"meta_hash"); + validate(n,epub); + } + require(notes.emplace(n.id,n).second,"Duplicate Readest annotation ID"); + } + return notes; +} +std::string payload(const Note& n,const std::string& book) { + auto o=object();put(o.get(),"id",n.id);put(o.get(),"type",std::string("annotation"));put(o.get(),"bookHash",book); + put(o.get(),"createdAt",n.created);put(o.get(),"updatedAt",n.updated); + if(!n.meta.empty()) put(o.get(),"metaHash",n.meta); + if(n.deleted) put(o.get(),"deletedAt",n.deleted); + else { + put(o.get(),"cfi",range(n));put(o.get(),"text",n.text);put(o.get(),"note",n.note); + put(o.get(),"style",n.style);put(o.get(),"color",n.color); + } + return json_text(o.get()); +} +void apply_native(const std::string& database,const std::string& fast_hash,const Snapshot& expected, + const std::string& uuid,const Note& note,const std::atomic& cancel) { + check(cancel);DB db(database,true); db.exec("PRAGMA foreign_keys=ON; PRAGMA synchronous=FULL; BEGIN IMMEDIATE"); + try { + require(snapshot(db,fast_hash).raw==expected.raw,"Annotations changed on PocketBook during sync; retry"); + Query triggers(db,"SELECT 1 FROM sqlite_master WHERE type='trigger' AND tbl_name IN ('Items','Tags','TagNames','TypeNames')"); + require(!triggers.row(),"Unrecognized native annotation triggers"); + std::string item; + Query existing(db,"SELECT OID FROM Items WHERE ParentID=? AND HashUUID=?",{expected.book,uuid}); + if(existing.row()) {item=existing.text(0);require(!existing.row(),"Ambiguous native annotation UUID");} + check(cancel); + auto stamp=std::to_string(std::max(note.updated,note.deleted)/1000); + if(note.deleted) { + if(!item.empty()) Query(db,"UPDATE Items SET State=2,TimeAlt=? WHERE OID=?",{stamp,item}).row(); + } else { + if(item.empty()) { + Query type(db,"SELECT OID FROM TypeNames WHERE TypeName='obj.book_mark'");require(type.row(),"Missing native annotation type"); + Query(db,"INSERT INTO Items(ParentID,TypeID,State,TimeAlt,HashUUID) VALUES(?,?,0,?,?)",{expected.book,type.text(0),stamp,uuid}).row(); + item=std::to_string(sqlite3_last_insert_rowid(db.db)); + } else Query(db,"UPDATE Items SET State=0,TimeAlt=? WHERE OID=?",{stamp,item}).row(); + auto quote=object(),anchor=object(),comment=object(); + put(quote.get(),"begin","pbr:/webkit?##"+note.begin);put(quote.get(),"end","pbr:/webkit?##"+note.end);put(quote.get(),"text",note.text); + put(anchor.get(),"anchor","pbr:/webkit?##"+note.begin);put(anchor.get(),"created",note.created/1000);put(comment.get(),"text",note.note); + for(const auto& [name,value]:std::map{{"bm.type",note.note.empty()?"highlight":"note"}, + {"bm.color",note.color=="#00bcd4"?"cian":note.color},{"bm.note",json_text(comment.get())}, + {"bm.book_mark",json_text(anchor.get())},{"bm.quotation",json_text(quote.get())}}) { + Query tag(db,"SELECT OID FROM TagNames WHERE TagName=?",{name});require(tag.row(),"Missing native annotation tag"); + Query(db,"INSERT OR REPLACE INTO Tags(ItemID,TagID,Val,TimeEdt) VALUES(?,?,?,?)",{item,tag.text(0),value,stamp}).row(); + } + } + // Do not report success or advance the baseline until COMMIT succeeds. + db.exec("COMMIT"); + } catch(...) {sqlite3_exec(db.db,"ROLLBACK",nullptr,nullptr,nullptr);throw;} +} +} // namespace + +void sync_annotations(Cloud& cloud,State& state,const VerifiedManagedBook& verified, + const NativePosition& native,const std::string& database,long long now, + const std::string& model,const std::string& firmware,const std::atomic& cancel) { + require(model=="PB743G" && firmware=="U743g.6.11.1683","Annotation sync is not verified on this firmware"); + require(now>0 && now<0x7fffffffffffffffLL/1000-1,"Invalid annotation sync clock"); + const auto& book=verified.book();const auto user=cloud.session().user_id; + require(!user.empty() && !book.local_only && !book.book.deleted && native.book_path==book.path,"Invalid annotation sync book"); + check(cancel); + auto local=read_native(database,native.fast_hash); + for(const auto& [id,n]:local.notes) { (void)id;validate(n,book.path); } + const auto response=cloud.get("/api/sync?type=notes&since=0&book="+book.book.hash,now); + require(response.status==200,"Cannot fetch Readest annotations"); + auto remote=remote_notes(response.body,user,book.book.hash,book.path); + struct Entry {std::string local,base_local="deleted",base_remote="deleted",pending,pending_local;long long remote_stamp=0;}; + std::map entries;std::set used; + auto saved=parse_json(state.annotations(user,book.book.hash,book.path,book.sha256));auto* records=member(saved.get(),"entries"); + require(records && json_object_get_type(records)==json_type_array && json_object_array_length(records)<=5000,"Invalid saved annotation journal"); + for(size_t i=0;isecond.updated,observed->second.deleted); + require(remote_stamp>=e.remote_stamp,"Stale Readest annotation response; retry"); + auto lf=fingerprint(local.notes,e.local),rf=fingerprint(remote,id); + if(lf==rf) { + e.base_local=lf;e.base_remote=rf;e.remote_stamp=remote_stamp;e.pending.clear();e.pending_local.clear();save();continue; + } + if(!e.pending.empty()) { + // An ambiguous HTTP failure may already have committed remotely. + if(rf==e.pending_local) { + e.base_local=e.pending_local;e.base_remote=rf;e.remote_stamp=remote_stamp;e.pending.clear();e.pending_local.clear();save(); + } else require(rf==e.base_remote,"Annotation changed in Readest during a pending upload; resolve the conflicting edits"); + } + if(e.pending.empty()) { + const bool lc=lf!=e.base_local,rc=rf!=e.base_remote; + require(!(lc && rc),"Annotation changed on both readers; resolve the conflicting notes before syncing"); + if(!lc && !rc) continue; + if(rc) { + auto it=remote.find(id);require(it!=remote.end(),"Readest annotation disappeared without a deletion marker"); + apply_native(database,native.fast_hash,local,e.local,it->second,cancel); + local=read_native(database,native.fast_hash); + require(fingerprint(local.notes,e.local)==rf,"Native annotation write could not be verified"); + e.base_local=rf;e.base_remote=rf;e.remote_stamp=remote_stamp;save();continue; + } + Note n; + auto found=local.notes.find(e.local); + if(found!=local.notes.end()) n=found->second; + else n.deleted=now*1000; + n.id=id; + const auto old=remote.find(id); + if(old!=remote.end()) {n.meta=old->second.meta;n.created=old->second.created;} + const auto stamp=old==remote.end()?0:std::max(old->second.updated,old->second.deleted); + require(stamp<0x7fffffffffffffffLL-1,"Invalid remote annotation timestamp"); + n.updated=std::max(now*1000,stamp+1); + if(n.created<=0) n.created=n.updated; + if(lf=="deleted") n.deleted=n.updated; + e.pending=payload(n,book.book.hash);e.pending_local=lf;save(); + } + check(cancel); + require(read_native(database,native.fast_hash).raw==local.raw,"Annotations changed on PocketBook during sync; retry"); + // Recheck the remote record before a write; the API has no conditional update. + auto fresh=cloud.get("/api/sync?type=notes&since=0&book="+book.book.hash,now); + require(fresh.status==200,"Cannot recheck Readest annotations; saved for retry"); + auto latest=remote_notes(fresh.body,user,book.book.hash,book.path); + require(fingerprint(latest,id)==rf,"Annotation changed in Readest before upload; retry"); + check(cancel); + // Retry the same body and ID. Server-authoritative data must acknowledge it. + auto posted=cloud.post("/api/sync","{\"books\":[],\"configs\":[],\"notes\":["+e.pending+"]}",now); + require(posted.status==200,"Cannot upload annotations; saved for retry"); + auto accepted=remote_notes(posted.body,user,book.book.hash,book.path); + auto winner=accepted.find(id); + require(winner!=accepted.end() && fingerprint(winner->second)==e.pending_local,"Readest did not accept the annotation; saved for reconciliation"); + remote[id]=winner->second;e.base_local=e.pending_local;e.base_remote=e.pending_local; + e.remote_stamp=std::max(winner->second.updated,winner->second.deleted); + e.pending.clear();e.pending_local.clear();save(); + local=read_native(database,native.fast_hash); + require(fingerprint(local.notes,e.local)==e.base_local,"Newer PocketBook annotation edits remain; sync again"); + } +} +} // namespace readest diff --git a/apps/readest-sync/src/annotations.h b/apps/readest-sync/src/annotations.h new file mode 100644 index 0000000..0849451 --- /dev/null +++ b/apps/readest-sync/src/annotations.h @@ -0,0 +1,12 @@ +#pragma once +#include "progress.h" + +namespace readest { +// Synchronize EPUB text highlights and attached notes for one verified local +// copy. Full per-book pulls avoid client-clock cursors. Retry state is durable +// before writes; conflicting edits fail without overwriting either side. +// Native writes require the validated firmware and an unchanged snapshot. +void sync_annotations(Cloud& cloud,State& state,const VerifiedManagedBook& verified, + const NativePosition& native,const std::string& database,long long now, + const std::string& model,const std::string& firmware,const std::atomic& cancel); +} diff --git a/apps/readest-sync/src/application.cpp b/apps/readest-sync/src/application.cpp index b321a47..a04d668 100644 --- a/apps/readest-sync/src/application.cpp +++ b/apps/readest-sync/src/application.cpp @@ -1,4 +1,5 @@ #include "application.h" +#include "annotations.h" #include "upload.h" #include #include @@ -129,6 +130,15 @@ void ApplicationService::synchronize(const Request& request, OperationResult& re transition.outcome==ResumeOutcome::Applied || (request.choice==ProgressChoice::Readest && result.sync_action==SyncAction::ApplyRemote))) state_->save_upload(request.book.account,request.book.hash,{}); + if(!config_.annotations_database.empty() && transition.outcome!=ResumeOutcome::SyncUnavailable && + transition.outcome!=ResumeOutcome::CommitUncertain && transition.outcome!=ResumeOutcome::AppliedUnrecorded) { + try { sync_annotations(*cloud_,*state_,verified,native,config_.annotations_database,time(nullptr),config_.model,config_.firmware,cancel); } + catch(const std::exception& e) { + check_cancel(cancel); + if(!result.progress_warning.empty()) result.progress_warning+=" "; + result.progress_warning+="Annotations: "+std::string(e.what()); + } + } switch(transition.outcome) { case ResumeOutcome::NotRequested: case ResumeOutcome::Blocked: return; case ResumeOutcome::NoPending: break; diff --git a/apps/readest-sync/src/application.h b/apps/readest-sync/src/application.h index 99d88c1..c52266c 100644 --- a/apps/readest-sync/src/application.h +++ b/apps/readest-sync/src/application.h @@ -6,7 +6,7 @@ namespace readest { struct ApplicationConfig { - std::string root, books_root, database, ca, model, firmware; + std::string root, books_root, database, annotations_database, ca, model, firmware; std::vector book_roots; std::string public_key, auth_origin="https://readest.supabase.co", api_origin="https://web.readest.com"; HttpTransport transport; diff --git a/apps/readest-sync/src/device_adapter.cpp b/apps/readest-sync/src/device_adapter.cpp index 346678c..fc9f352 100644 --- a/apps/readest-sync/src/device_adapter.cpp +++ b/apps/readest-sync/src/device_adapter.cpp @@ -27,6 +27,7 @@ readest::ApplicationConfig deviceApplicationConfig() { config.root=platform::dataRoot().toStdString(); config.books_root=QDir::cleanPath(QString::fromStdString(config.root)+"/../../Books/Readest").toStdString(); config.database=platform::nativeDatabase().toStdString(); config.ca=config.root+"/ca-certificates.crt"; + config.annotations_database=QDir::cleanPath(QString::fromStdString(config.root)+"/../config/books.db").toStdString(); config.model=platform::model().toStdString(); config.firmware=platform::firmware().toStdString(); config.book_roots=platform::bookRoots(); config.public_key=readest::public_anon_key; return config; diff --git a/apps/readest-sync/src/integrity.cpp b/apps/readest-sync/src/integrity.cpp index d3ef7c8..c727077 100644 --- a/apps/readest-sync/src/integrity.cpp +++ b/apps/readest-sync/src/integrity.cpp @@ -1,4 +1,6 @@ #include "integrity.h" +#include "probe.h" +#include #include "vendor/miniz/miniz.h" #include #include @@ -389,4 +391,121 @@ std::string xpointer_cfi(const std::string& path,const std::string& pointer) { } return cfi+")"; } + +void validate_annotation_range(const std::string& path,const std::string& begin, + const std::string& end,const std::string& selected) { + // Resolve both endpoints against the exact EPUB, including UTF-16 offsets. + // Unsupported assertion forms fail closed instead of moving a highlight. + struct Step { unsigned n; std::string id; }; + struct Point { std::vector package,content; unsigned offset=0; }; + auto parse=[](const std::string& cfi) { + if(point_cfi(cfi).empty()) throw std::runtime_error("Unsupported annotation CFI"); + Point p; auto bang=cfi.find('!'); + auto steps=[](const std::string& s,std::vector& out,unsigned* offset) { + size_t at=0; + while(at(s[at]))) ++at; + if(at-start>7) throw std::runtime_error("Annotation CFI index is too large"); + Step step{static_cast(std::stoul(s.substr(start,at-start))),{}}; + if(at7 || value.find_first_not_of("0123456789")!=std::string::npos) + throw std::runtime_error("Unsupported annotation text assertion"); + *offset=static_cast(std::stoul(value)); at=s.size(); + } + if(at!=s.size()) throw std::runtime_error("Unsupported annotation path"); + }; + steps(cfi.substr(8,bang-8),p.package,nullptr); + steps(cfi.substr(bang+1,cfi.size()-bang-2),p.content,&p.offset); + if(p.package.size()!=2 || p.content.empty() || !(p.content.back().n%2)) + throw std::runtime_error("Annotation must end in a text node"); + return p; + }; + const auto first=parse(begin), last=parse(end); + if(first.package[0].n!=last.package[0].n || first.package[1].n!=last.package[1].n) + throw std::runtime_error("Annotations spanning EPUB chapters are unsupported"); + int fd=open(path.c_str(),O_RDONLY|O_NOFOLLOW); + if(fd<0) throw std::runtime_error("Cannot read annotation EPUB"); + struct stat st; + if(fstat(fd,&st) || !S_ISREG(st.st_mode) || st.st_size<=0 || st.st_size>256LL*1024*1024) { + close(fd); throw std::runtime_error("Invalid annotation EPUB"); + } + FILE* raw=fdopen(fd,"rb"); if(!raw) {close(fd);throw std::runtime_error("Cannot read annotation EPUB");} + std::unique_ptr file(raw,fclose); Zip zip(raw,st.st_size); + auto container=xml(zip.read("META-INF/container.xml",1024*1024)); + auto package_path=property(child(child(xmlDocGetRootElement(container.get()),"rootfiles"),"rootfile"),"full-path"); + if(!safe_path(package_path)) throw std::runtime_error("Invalid EPUB package path"); + auto package=xml(zip.read(package_path,4*1024*1024)); auto* root=xmlDocGetRootElement(package.get()); + auto element=[](xmlNode* parent,const Step& step) { + if(step.n%2 || !step.n) throw std::runtime_error("Unsupported annotation element"); + auto index=step.n/2; xmlNode* result=nullptr; + for(auto* n=parent->children;n;n=n->next) if(n->type==XML_ELEMENT_NODE && --index==0) {result=n;break;} + if(!result || (!step.id.empty() && property(result,"id")!=step.id)) + throw std::runtime_error("Annotation element does not match EPUB"); + return result; + }; + auto* spine=element(root,first.package[0]); auto* itemref=element(spine,first.package[1]); + if(!named(spine,"spine") || !named(itemref,"itemref")) throw std::runtime_error("Invalid annotation spine"); + element(element(root,last.package[0]),last.package[1]); + std::string href; + for(auto* n=child(root,"manifest")->children;n;n=n->next) if(named(n,"item") && property(n,"id")==property(itemref,"idref")) { + if(!href.empty() || property(n,"media-type")!="application/xhtml+xml") throw std::runtime_error("Unsupported annotation resource"); + href=property(n,"href"); + } + auto slash=package_path.rfind('/'); + auto doc=chapter_xml(zip.read(zip_relative(slash==std::string::npos?"":package_path.substr(0,slash+1),href),16*1024*1024)); + auto* html=xmlDocGetRootElement(doc.get()); + if(!named(html,"html")) throw std::runtime_error("Invalid annotation document"); + std::vector text; + std::map starts; + const std::set blocks={"p","div","section","li","h1","h2","h3","h4","h5","h6","br","blockquote"}; + std::function flatten=[&](xmlNode* node) { + for(auto* n=node;n;n=n->next) { + bool block=n->type==XML_ELEMENT_NODE && blocks.count(reinterpret_cast(n->name)); + if(block) text.push_back('\n'); + if(n->type==XML_TEXT_NODE || n->type==XML_CDATA_SECTION_NODE) { + starts[n]=text.size(); auto units=utf16(n->content); text.insert(text.end(),units.begin(),units.end()); + } else if(n->type==XML_ELEMENT_NODE) flatten(n->children); + if(block) text.push_back('\n'); + } + }; + flatten(html); + auto resolve=[&](const Point& p) { + auto* parent=html; + for(size_t i=0;i+1children;n;n=n->next) { + if(n->type==XML_ELEMENT_NODE) {slot+=2;continue;} + if(slot!=step.n || (n->type!=XML_TEXT_NODE && n->type!=XML_CDATA_SECTION_NODE)) continue; + found=true; auto units=utf16(n->content); + if(remaining<=units.size()) { + if(remaining && remaining=0xdc00 && units[remaining]<=0xdfff) + throw std::runtime_error("Annotation splits a Unicode character"); + return starts.at(n)+remaining; + } + remaining-=units.size(); + } + throw std::runtime_error(found?"Annotation text offset exceeds EPUB text":"Annotation text node is missing"); + }; + auto a=resolve(first), b=resolve(last); + if(a>=b) throw std::runtime_error("Empty or reversed annotation range"); + auto normalized=[](const std::vector& input) { + std::vector result; + for(auto c:input) { if(blank(c)) {if(!result.empty() && result.back()!=32) result.push_back(32);} else result.push_back(c); } + if(!result.empty() && result.back()==32) result.pop_back(); return result; + }; + const auto expected=utf16(reinterpret_cast(selected.c_str())); + if(selected.find('\0')!=std::string::npos || normalized(std::vector(text.begin()+a,text.begin()+b))!=normalized(expected)) + throw std::runtime_error("Annotation text does not match its EPUB range"); +} } // namespace readest diff --git a/apps/readest-sync/src/integrity.h b/apps/readest-sync/src/integrity.h index cd203ff..8673b05 100644 --- a/apps/readest-sync/src/integrity.h +++ b/apps/readest-sync/src/integrity.h @@ -15,4 +15,7 @@ BookIntegrity inspect_epub(const std::string& path); // Resolve a CREngine element/text XPointer in the original EPUB. Throws when // structure or offset cannot be resolved exactly; never uses percentages. std::string xpointer_cfi(const std::string& path, const std::string& xpointer); +// Verify exact text-node range against original EPUB bytes (UTF-16 offsets). +void validate_annotation_range(const std::string& path,const std::string& begin, + const std::string& end,const std::string& selected); } // namespace readest diff --git a/apps/readest-sync/src/position.cpp b/apps/readest-sync/src/position.cpp index 3177271..9d06d47 100644 --- a/apps/readest-sync/src/position.cpp +++ b/apps/readest-sync/src/position.cpp @@ -64,27 +64,25 @@ std::string point_cfi(const std::string& position) { return at + 1 == s.size() && s[at] == ')' ? s : ""; } -std::string readest_start_cfi(const std::string& s) { +std::pair readest_range_cfi(const std::string& s) { if (s.size() > 8192 || s.compare(0, 8, "epubcfi(") != 0 || s.back() != ')') - return ""; - auto point = point_cfi(s); - if (!point.empty()) return point; + return {}; // Commas inside assertions are not range separators. Use the same // assertion grammar as point_cfi, including escaped brackets/commas. std::vector commas; for (size_t at = 8; at + 1 < s.size();) { if (s[at] == '[') { - if (!assertion(s, at)) return ""; + if (!assertion(s, at)) return {}; } else { if (s[at] == ',') commas.push_back(at); ++at; } } - if (commas.size() != 2) return ""; + if (commas.size() != 2) return {}; const auto parent = s.substr(0, commas[0]); size_t at = 8; if (!path(parent, at) || at >= parent.size() || parent[at++] != '!' || - !path(parent, at) || at != parent.size()) return ""; + !path(parent, at) || at != parent.size()) return {}; const auto start = s.substr(commas[0] + 1, commas[1] - commas[0] - 1); const auto end = s.substr(commas[1] + 1, s.size() - commas[1] - 2); // A relative endpoint may extend the path or supply a text offset. @@ -92,11 +90,18 @@ std::string readest_start_cfi(const std::string& s) { // Readest itself rejects empty-start ranges as malformed saved locations. if (start.empty() || (start[0] != '/' && start[0] != ':') || end.empty() || (end[0] != '/' && end[0] != ':') || - s.find(';') != std::string::npos) return ""; + s.find(';') != std::string::npos) return {}; const auto first = point_cfi(parent + start + ")"); const auto last = point_cfi(parent + end + ")"); - if (first.empty() || last.empty() || compare_cfi(first, last) > 0) return ""; - return first; + if (first.empty() || last.empty() || compare_cfi(first, last) > 0) return {}; + return {first, last}; +} + +std::string readest_start_cfi(const std::string& s) { + if (s.compare(0, 8, "epubcfi(") != 0) return ""; + const auto point = point_cfi(s); + if (!point.empty()) return point; + return readest_range_cfi(s).first; } int compare_cfi(const std::string& left, const std::string& right) { diff --git a/apps/readest-sync/src/probe.h b/apps/readest-sync/src/probe.h index b36f87d..1c46d4c 100644 --- a/apps/readest-sync/src/probe.h +++ b/apps/readest-sync/src/probe.h @@ -1,6 +1,7 @@ #pragma once #include +#include #include #include #include @@ -11,6 +12,9 @@ namespace readest { // A deliberately limited point-CFI syntax check, not an EPUB resolver. // Unsupported locations fail closed; no page/percentage fallback. std::string point_cfi(const std::string& position); +// Both endpoints of a supported Readest range, retaining assertions. +// Empty pair for point CFIs or unsupported syntax; does not resolve the EPUB. +std::pair readest_range_cfi(const std::string& position); // Readest stores visible ranges. Return the start, retaining assertions. // Syntax-only: callers must still validate against the original EPUB. std::string readest_start_cfi(const std::string& position); diff --git a/apps/readest-sync/src/state.cpp b/apps/readest-sync/src/state.cpp index 4843a7c..d8ce6a3 100644 --- a/apps/readest-sync/src/state.cpp +++ b/apps/readest-sync/src/state.cpp @@ -163,6 +163,7 @@ State::State(const std::string& path) { "CREATE TABLE IF NOT EXISTS copy_choices(user TEXT,hash TEXT,path TEXT,PRIMARY KEY(user,hash));" "CREATE TABLE IF NOT EXISTS local_integrity(path TEXT PRIMARY KEY,stamp TEXT,sha256 TEXT);" "CREATE TABLE IF NOT EXISTS uploads(user TEXT,hash TEXT,stage INTEGER,sha256 TEXT,PRIMARY KEY(user,hash));" + "CREATE TABLE IF NOT EXISTS annotation_sync(user TEXT,hash TEXT,path TEXT,sha256 TEXT,data TEXT NOT NULL,PRIMARY KEY(user,hash,path,sha256));" "PRAGMA user_version=1; COMMIT;"); } catch (...) { sqlite3_close(db_); db_ = nullptr; throw; } } @@ -429,3 +430,17 @@ std::vector recover_downloads(State& state, const std::string& user return warnings; } } // namespace readest + +namespace readest { +std::string State::annotations(const std::string& user,const std::string& hash,const std::string& path,const std::string& sha) { + identity(user,hash); + Query q(db_,"SELECT data FROM annotation_sync WHERE user=? AND hash=? AND path=? AND sha256=?"); + q.bind(1,user); q.bind(2,hash); q.bind(3,path); q.bind(4,sha); + return q.row()?q.text(0):"{\"entries\":[]}"; +} +void State::save_annotations(const std::string& user,const std::string& hash,const std::string& path,const std::string& sha,const std::string& data) { + identity(user,hash); parse_json(data); + Query q(db_,"INSERT OR REPLACE INTO annotation_sync(user,hash,path,sha256,data) VALUES(?,?,?,?,?)"); + q.bind(1,user); q.bind(2,hash); q.bind(3,path); q.bind(4,sha); q.bind(5,data); q.row(); +} +} diff --git a/apps/readest-sync/src/state.h b/apps/readest-sync/src/state.h index e92426b..445c7c2 100644 --- a/apps/readest-sync/src/state.h +++ b/apps/readest-sync/src/state.h @@ -58,6 +58,9 @@ class State { SavedSync sync(const std::string& user, const std::string& hash); // Returns the revision committed by this write. long long save_sync(const std::string& user, const std::string& hash, const SavedSync& next); + // Durable per-account, exact-copy annotation baselines and retry journal. + std::string annotations(const std::string& user,const std::string& hash,const std::string& path,const std::string& sha); + void save_annotations(const std::string& user,const std::string& hash,const std::string& path,const std::string& sha,const std::string& data); private: sqlite3* db_ = nullptr; }; diff --git a/apps/readest-sync/tests/CMakeLists.txt b/apps/readest-sync/tests/CMakeLists.txt index ed03e50..a48b328 100644 --- a/apps/readest-sync/tests/CMakeLists.txt +++ b/apps/readest-sync/tests/CMakeLists.txt @@ -10,6 +10,7 @@ function(readest_native_test name script) ${Python3_EXECUTABLE} "${CMAKE_CURRENT_SOURCE_DIR}/${script}") set_tests_properties(${name} PROPERTIES LABELS core) endfunction() +readest_native_test(annotations test_epub_command.py annotations_test.cpp) readest_native_test(probe test_probe.py probe_test.cpp sync_test.cpp) readest_native_test(cloud test_cloud.py cloud_test.cpp) readest_native_test(http test_http.py http_test.cpp) diff --git a/apps/readest-sync/tests/annotations_test.cpp b/apps/readest-sync/tests/annotations_test.cpp new file mode 100644 index 0000000..d82bf8c --- /dev/null +++ b/apps/readest-sync/tests/annotations_test.cpp @@ -0,0 +1,115 @@ +#include "annotations.h" +#include "json_util.h" +#include +#include +#include + +using namespace readest; +namespace { +void sql(sqlite3* db,const std::string& query) {assert(sqlite3_exec(db,query.c_str(),nullptr,nullptr,nullptr)==SQLITE_OK);} +std::string scalar(sqlite3* db,const char* query) { + sqlite3_stmt* s=nullptr;assert(sqlite3_prepare_v2(db,query,-1,&s,nullptr)==SQLITE_OK); + assert(sqlite3_step(s)==SQLITE_ROW);auto* p=sqlite3_column_text(s,0);std::string result=p?reinterpret_cast(p):"";sqlite3_finalize(s);return result; +} +void set(json_object* o,const char* key,const std::string& value) {json_object_object_add(o,key,json_object_new_string(value.c_str()));} +void set(json_object* o,const char* key,long long value) {json_object_object_add(o,key,json_object_new_int64(value));} +void fails(const std::function& f,const std::string& message) { + try {f();} catch(const std::exception& e) {if(std::string(e.what()).find(message)==std::string::npos) {std::cerr<<"Expected "< server; + int posts=0,gets=0;bool fail_post=false,lose_response=false,server_wins=false,race=false;std::string last_post; + auto wire=[&] {std::string body="{\"notes\":[";for(const auto& [id,row]:server){(void)id;if(body.back()!= '[')body+=',';body+=row;}return body+"]}";}; + auto api=[&](const std::string& url,const std::string& method,const std::vector&,const std::string& body,const std::string&,size_t) { + if(url.find("grant_type=password")!=std::string::npos) return HttpResponse{200,R"({"access_token":"test","refresh_token":"test-refresh","expires_at":999999,"user":{"id":"user"}})",0}; + if(method=="GET") {assert(url=="https://api.test/api/sync?type=notes&since=0&book="+integrity.readest_hash);++gets; + if(race) sql(db,"UPDATE Items SET TimeAlt=TimeAlt+1 WHERE OID=1+1"); + return HttpResponse{200,wire(),0};} + assert(url=="https://api.test/api/sync");++posts;last_post=body; + if(fail_post) return HttpResponse{503,"{}",0}; + auto json=parse_json(body);auto* list=member(json.get(),"notes");assert(json_object_array_length(list)==1); + auto* in=json_object_array_get_idx(list,0);const auto id=string_member(in,"id"); + auto out=parse_json(server.count(id)?server.at(id):"{}"); + set(out.get(),"user_id",std::string("user"));set(out.get(),"book_hash",integrity.readest_hash); + if(!server_wins) { + for(const auto& [a,b]:std::map{{"id","id"},{"type","type"},{"cfi","cfi"},{"text","text"},{"note","note"},{"color","color"},{"style","style"}, + {"createdAt","created_at"},{"updatedAt","updated_at"},{"deletedAt","deleted_at"}}) { + auto* v=member(in,a.c_str());json_object_object_add(out.get(),b.c_str(),v?json_object_get(v):nullptr); + } + server[id]=json_text(out.get()); + } + if(lose_response) throw std::runtime_error("lost response"); + return HttpResponse{200,"{\"notes\":["+server.at(id)+"]}",0}; + }; + Cloud cloud(root+"/annotations-session.json","ca","public","https://auth.test","https://api.test",api);cloud.sign_in("test","test",1000); + State state(root+"/annotations-state.db");std::atomic cancel{false};long long now=2000; + auto sync=[&] {sync_annotations(cloud,state,verified,native,database,++now,"PB743G","U743g.6.11.1683",cancel);}; + const std::string cfi="epubcfi(/6/2!/4/4/1,:0,:16)",text="Marker ALPHA-01."; + auto remote=[&](std::string id,std::string note) { + auto row=parse_json("{}");set(row.get(),"id",id);set(row.get(),"user_id",std::string("user"));set(row.get(),"book_hash",integrity.readest_hash); + set(row.get(),"type",std::string("annotation"));set(row.get(),"cfi",cfi);set(row.get(),"text",text);set(row.get(),"note",note); + set(row.get(),"style",std::string("highlight"));set(row.get(),"color",std::string("yellow"));set(row.get(),"created_at",1000000LL);set(row.get(),"updated_at",now*1000);return json_text(row.get()); + }; + auto mutate=[&](const std::string& id,const char* key,const std::string& value) {auto row=parse_json(server.at(id));set(row.get(),key,value);set(row.get(),"updated_at",(++now)*1000);server[id]=json_text(row.get());}; + auto native_note=[&](const std::string& value) {sql(db,"UPDATE Tags SET Val='"+value+"' WHERE TagID=105 AND ItemID=2");}; + validate_annotation_range(epub,"epubcfi(/6/2[alpha]!/4/4[ALPHA-01]/1:0)","epubcfi(/6/2!/4/4/1:16)",text); + fails([&]{validate_annotation_range(epub,"epubcfi(/6/2!/4/4/1:0)","epubcfi(/6/2!/4/4/1:17)",text+"X");},"does not match"); + fails([&]{validate_annotation_range(epub,"epubcfi(/6/2!/4/4/1:0)","epubcfi(/6/2!/4/4/1:999999)",text);},"exceeds"); + // First import, exact repeat, and reader normalization do not upload or duplicate. + server["remote1"]=remote("remote1","remote note");sync();assert(posts==0 && scalar(db,"SELECT count(*) FROM Items")=="2"); + sync();assert(posts==0 && scalar(db,"SELECT count(*) FROM Items")=="2"); + sql(db,"UPDATE Tags SET Val=replace(replace(Val,'pbr:/webkit?##','pbr:/page?page=2&offs=4#'),'/1:0)','/1)') WHERE TagID IN (104,107)"); + sync();assert(posts==0); + mutate("remote1","note","remote edited");sync();assert(scalar(db,"SELECT Val FROM Tags WHERE ItemID=2 AND TagID=105")=="{\"text\":\"remote edited\"}"); + // Native in-place edits upload to the known ID. + native_note("{\"text\":\"local edited\"}");sync();assert(posts==1 && server.size()==1); + sync();assert(posts==1); + // Both sides changed: neither is overwritten. + native_note("{\"text\":\"local conflict\"}");mutate("remote1","note","remote conflict"); + fails(sync,"both readers");assert(posts==1 && scalar(db,"SELECT Val FROM Tags WHERE ItemID=2 AND TagID=105").find("local conflict")!=std::string::npos); + native_note("{\"text\":\"remote conflict\"}");sync(); + // Durable failed push survives reopening State, and reuses exactly the payload. + native_note("{\"text\":\"retry me\"}");fail_post=true;fails(sync,"saved for retry");const auto pending=last_post; + fail_post=false; + {State reopened(root+"/annotations-state.db");sync_annotations(cloud,reopened,verified,native,database,++now,"PB743G","U743g.6.11.1683",cancel);} + assert(last_post==pending);int successful=posts;sync();assert(posts==successful); + // Ambiguous commit: the next pull acknowledges it without a second POST. + native_note("{\"text\":\"already committed\"}");lose_response=true;fails(sync,"lost response");successful=posts;lose_response=false;sync();assert(posts==successful); + // Server winner is not treated as an acknowledgement of our edit. + native_note("{\"text\":\"rejected edit\"}");server_wins=true;fails(sync,"did not accept");server_wins=false;sync(); + // Native Edit creates a new UUID and tombstones the old UUID. + sql(db,"UPDATE Items SET State=2 WHERE OID=2; INSERT INTO Items VALUES(3,1,4,0,2001,'NEW-NATIVE-UUID');" + "INSERT INTO Tags(ItemID,TagID,Val,TimeEdt) SELECT 3,TagID,Val,TimeEdt FROM Tags WHERE ItemID=2;" + "UPDATE Tags SET Val='{\"text\":\"replacement edit\"}' WHERE ItemID=3 AND TagID=105;"); + sync();assert(server.size()==2);assert(record_timestamp(parse_json(server.at("remote1")).get(),"deleted_at")>0); + successful=posts;sync();assert(posts==successful); + std::string replacement;for(const auto& [id,row]:server) if(id!="remote1") replacement=id; + const auto before_delete=server.at(replacement); + auto deleted=parse_json(server.at(replacement));set(deleted.get(),"deleted_at",(++now)*1000);set(deleted.get(),"updated_at",now*1000);server[replacement]=json_text(deleted.get()); + sync();assert(scalar(db,"SELECT State FROM Items WHERE OID=3")=="2");successful=posts;sync();assert(posts==successful); + // A stale pull after a successful deletion must never resurrect the highlight. + const auto tombstone=server.at(replacement);server[replacement]=before_delete; + fails(sync,"Stale Readest");assert(scalar(db,"SELECT State FROM Items WHERE OID=3")=="2");server[replacement]=tombstone; + // Wrong account, bad ranges and concurrent native changes fail before import. + server["new"]=remote("new","new note");mutate("new","user_id","wrong");fails(sync,"account/book mismatch");mutate("new","user_id","user"); + mutate("new","cfi","epubcfi(/6/2!/4/4/1,:0,:999999)");fails(sync,"exceeds");mutate("new","cfi",cfi); + race=true;fails(sync,"changed on PocketBook");race=false;sync(); + assert(scalar(db,"SELECT count(*) FROM Items WHERE State=0 AND ParentID=1")=="1"); + cancel=true;fails(sync,"cancelled");cancel=false; + fails([&]{sync_annotations(cloud,state,verified,native,database,now,"other","other",cancel);},"firmware"); + assert(scalar(db,"PRAGMA integrity_check")=="ok");sqlite3_close(db); + std::cout<<"Annotation create/edit/delete, durable retry, conflicts and exact ranges passed\n"; +} diff --git a/apps/readest-sync/tests/sync_test.cpp b/apps/readest-sync/tests/sync_test.cpp index 84988d3..b0d78ff 100644 --- a/apps/readest-sync/tests/sync_test.cpp +++ b/apps/readest-sync/tests/sync_test.cpp @@ -11,7 +11,19 @@ void sync_checks() { const std::string charlie = "epubcfi(/6/6!/4/2/1)"; const std::string range = "epubcfi(/6/4[bravo]!/4,/2,/12[BRAVO-05]/1:179)"; assert(readest_start_cfi(range) == bravo); + assert(readest_range_cfi(range).first == bravo); + assert(readest_range_cfi(range).second == "epubcfi(/6/4[bravo]!/4/12[BRAVO-05]/1:179)"); + assert(readest_range_cfi(charlie).first.empty()); + const auto same_node = readest_range_cfi("epubcfi(/6/4[a^,b]!/4/2/1,:3,:9)"); + assert(same_node.first == "epubcfi(/6/4[a^,b]!/4/2/1:3)"); + assert(same_node.second == "epubcfi(/6/4[a^,b]!/4/2/1:9)"); + // Exact endpoints observed on the PocketBook, including an implicit zero offset. + const auto native = readest_range_cfi("epubcfi(/6/16!/4,/60/1,/62/1:131)"); + assert(native.first == "epubcfi(/6/16!/4/60/1)"); + assert(native.second == "epubcfi(/6/16!/4/62/1:131)"); assert(readest_start_cfi(charlie) == charlie); + assert(readest_start_cfi("#" + charlie).empty()); + assert(readest_start_cfi("pbr:/webkit?##" + charlie).empty()); assert(compare_cfi(alpha, bravo) < 0); assert(compare_cfi(charlie, bravo) > 0); assert(compare_cfi("epubcfi(/6/6[x]!/4/2/1)", charlie) == 0); @@ -22,7 +34,11 @@ void sync_checks() { "epubcfi(/6/4!/4,/2,/4,/6)", "epubcfi(/6/4!/4,/2,/4:-1)", "epubcfi(/6/4!/4,/2,/4)junk", "epubcfi(/6/4!/4,/2/1:0[;s=b],/4)", "epubcfi(/6/4!/4,/2,/4~1)", "epubcfi(/6/4!/4,/2,/4[broken)", - "epubcfi(/6/4!/4:1,/2,/4)", "epubcfi(/6/4!/4,/4,/2)"}) assert(readest_start_cfi(bad).empty()); + "epubcfi(/6/4!/4:1,/2,/4)", "epubcfi(/6/4!/4,/4,/2)"}) { + assert(readest_start_cfi(bad).empty()); + const auto endpoints = readest_range_cfi(bad); + assert(endpoints.first.empty() && endpoints.second.empty()); + } SyncPositions p; p.local = alpha; p.remote = range; From 567140151cd9eb88b5fc9fc3b76ff0a05bb69f01 Mon Sep 17 00:00:00 2001 From: Oliver Gassner Date: Sat, 26 Sep 2026 15:23:57 +0200 Subject: [PATCH 2/5] Handle unsupported annotation replacements and cloud retry versions --- .../research/readest-annotation-protocol.md | 27 +++++ apps/readest-sync/src/annotations.cpp | 100 ++++++++++++++---- apps/readest-sync/src/annotations.h | 3 +- apps/readest-sync/src/application.cpp | 4 +- apps/readest-sync/tests/annotations_test.cpp | 47 +++++++- apps/readest-sync/tests/application_test.cpp | 21 +++- .../apps/readest-sync/mock_cloud.cpp | 22 +++- .../apps/readest-sync/mock_cloud.h | 2 +- .../apps/readest-sync/mock_cloud_test.cpp | 8 ++ .../apps/readest-sync/simulator.cpp | 18 +++- 10 files changed, 220 insertions(+), 32 deletions(-) diff --git a/apps/readest-sync/docs/research/readest-annotation-protocol.md b/apps/readest-sync/docs/research/readest-annotation-protocol.md index ec582e8..d154e98 100644 --- a/apps/readest-sync/docs/research/readest-annotation-protocol.md +++ b/apps/readest-sync/docs/research/readest-annotation-protocol.md @@ -193,3 +193,30 @@ anchors with page/offset metadata, and removed the explicit zero text offset from the start CFI. The end CFI offset 355, selected text, note, color and UUID were preserved. Reconciliation must compare semantic ranges rather than raw location strings, so this normalization does not create a false user edit. + +## Application implementation + +The annotation module is now called by the existing per-book Sync and Open +operations. Durable state is scoped to account, Readest book identity, exact +local copy and EPUB SHA-256. It stores both content baselines, accepted remote +versions, native/remote identities and pending uploads before network writes. +Full per-book pulls avoid relying on a clock-based incremental cursor. + +Supported text highlights retain notes and the native palette mappings for +yellow, blue, green, cyan (`cian`), lime and magenta. Unsupported styles, colors +or ranges remain untouched and produce a warning. Native-to-cloud deletion +is deferred when an unsupported live native annotation may be a replacement +for an edited UUID; the same protection applies in the other direction. +Conflicting edits are reported without choosing a winner. The server does not +provide conditional writes, so a preflight read narrows but cannot eliminate +the concurrent-write window on the cloud side. + +Regression coverage includes both directions of creation/edit/deletion, native +UUID replacement, reader anchor normalization, exact EPUB text/UTF-16 range +validation, durable retries and lost responses, server-authoritative rejection, +newer timestamps with unchanged remote content, stale deletion responses, +mixed supported/unsupported records, replacement deletion deferral, identity +checks, cancellation, native races and firmware gating. Full Linux host tests +and the Qt6 ARM ABI/package check passed during development. End-to-end app +validation on the reader is still required; the earlier screenshots validate +the separate single-note import trial. diff --git a/apps/readest-sync/src/annotations.cpp b/apps/readest-sync/src/annotations.cpp index 090b919..5d7132f 100644 --- a/apps/readest-sync/src/annotations.cpp +++ b/apps/readest-sync/src/annotations.cpp @@ -43,11 +43,12 @@ std::string point(std::string s) { return out; } struct Note { - std::string id,begin,end,text,note,color="yellow",style="highlight",meta; - long long created=0,updated=0,deleted=0; + std::string id,begin,end,text,note,color="yellow",style="highlight",meta,unsupported; + long long created=0,updated=0,deleted=0;bool annotation=true; }; std::string fingerprint(const Note& n) { if(n.deleted) return "deleted"; + if(!n.unsupported.empty()) return "unsupported:"+n.id; auto o=object(); put(o.get(),"begin",point(n.begin)); put(o.get(),"end",point(n.end)); put(o.get(),"text",normalized_text(n.text)); put(o.get(),"note",n.note); put(o.get(),"color",n.color); put(o.get(),"style",n.style); return hash(json_text(o.get())); @@ -63,10 +64,22 @@ std::string range(const Note& n) { auto out=a.substr(0,slash)+","+a.substr(slash,a.size()-slash-1)+","+b.substr(slash); require(!readest_range_cfi(out).first.empty(),"Cannot encode annotation range"); return out; } +std::string native_color(const std::string& color) { + const std::map colors={{"yellow","yellow"},{"blue","blue"},{"green","green"}, + {"#00bcd4","cian"},{"#00ff00","lime"},{"#ff00ff","magenta"}}; + auto found=colors.find(color); + require(found!=colors.end(),"Unsupported annotation color; nothing overwritten");return found->second; +} +std::string remote_color(const std::string& color) { + const std::map colors={{"yellow","yellow"},{"blue","blue"},{"green","green"}, + {"cian","#00bcd4"},{"cyan","#00bcd4"},{"lime","#00ff00"},{"magenta","#ff00ff"}}; + auto found=colors.find(color); + require(found!=colors.end(),"Unsupported native annotation color; nothing overwritten");return found->second; +} void validate(const Note& n,const std::string& epub) { if(n.deleted) return; require(n.style=="highlight","Unsupported annotation style; nothing overwritten"); - require(n.color=="yellow" || n.color=="#00bcd4","Unsupported annotation color; nothing overwritten"); + native_color(n.color); require(n.text.size()<=65536 && n.note.size()<=65536 && n.note.find('\0')==std::string::npos,"Invalid annotation text"); validate_annotation_range(epub,n.begin,n.end,n.text); } @@ -106,11 +119,14 @@ Snapshot snapshot(DB& db,const std::string& fast_hash) { std::map> tags; std::map all;std::map native_ids; Json raw(json_object_new_array(),json_object_put); - size_t count=0; + size_t count=0,bytes=0; while(rows.row()) { require(++count<=50000,"Too many native annotations"); Json row(json_object_new_array(),json_object_put); - for(int i=0;i<7;++i) json_object_array_add(row.get(),json_object_new_string(rows.text(i).c_str())); + for(int i=0;i<7;++i) { + const auto value=rows.text(i);bytes+=value.size();require(bytes<=4*1024*1024,"Native annotation data is too large"); + json_object_array_add(row.get(),json_object_new_string(value.c_str())); + } json_object_array_add(raw.get(),row.release()); const auto id=rows.text(1);require(!id.empty(),"Native annotation has no UUID"); auto [identity,inserted]=native_ids.emplace(id,rows.text(0)); @@ -126,9 +142,10 @@ Snapshot snapshot(DB& db,const std::string& fast_hash) { for(auto& [id,n]:all) { const auto& t=tags.at(id); auto type=t.find("bm.type"); if(type==t.end() || (type->second!="note" && type->second!="highlight")) { - require(!t.count("bm.quotation"),"Unsupported native annotation type; nothing overwritten");continue; + n.unsupported="Unsupported native annotation type";n.annotation=t.count("bm.quotation");out.notes[id]=n;continue; } if(n.deleted) {out.notes[id]=n;continue;} + try { auto quote=parse_json(t.at("bm.quotation")); auto bookmark=parse_json(t.at("bm.book_mark")); auto location=[](std::string s) {auto at=s.find("epubcfi(");require(at!=std::string::npos,"Unsupported native annotation anchor");return s.substr(at);}; n.begin=location(string_member(quote.get(),"begin"));n.end=location(string_member(quote.get(),"end")); @@ -136,14 +153,20 @@ Snapshot snapshot(DB& db,const std::string& fast_hash) { n.created=integer_member(bookmark.get(),"created"); require(n.created>0 && n.created<=999999999999LL,"Invalid native annotation creation time");n.created*=1000; auto note=t.find("bm.note");if(note!=t.end()) {auto o=parse_json(note->second);n.note=optional(o.get(),"text");} - auto color=t.find("bm.color");require(color!=t.end(),"Native annotation color missing");n.color=color->second=="cian"?"#00bcd4":color->second; + auto color=t.find("bm.color");require(color!=t.end(),"Native annotation color missing");n.color=remote_color(color->second); auto subtype=t.find("bm.subtype");require(subtype==t.end() || subtype->second.empty(),"Unsupported native annotation subtype"); + } catch(const std::exception& e) {n.unsupported=e.what();} out.notes[id]=n; } return out; } -Snapshot read_native(const std::string& database,const std::string& fast_hash) { - DB db(database,false); db.exec("BEGIN");auto result=snapshot(db,fast_hash);db.exec("COMMIT");return result; +Snapshot read_native(const std::string& database,const std::string& fast_hash,const std::string& epub) { + DB db(database,false);db.exec("BEGIN");auto result=snapshot(db,fast_hash);db.exec("COMMIT"); + for(auto& [id,n]:result.notes) { + (void)id;if(!n.unsupported.empty()) continue; + try {validate(n,epub);} catch(const std::exception& e) {n.unsupported=e.what();} + } + return result; } std::map remote_notes(const std::string& body,const std::string& user,const std::string& book,const std::string& epub) { auto json=parse_json(body);auto* rows=member(json.get(),"notes"); @@ -158,12 +181,14 @@ std::map remote_notes(const std::string& body,const std::strin n.created=record_timestamp(row,"created_at");n.updated=record_timestamp(row,"updated_at");n.deleted=record_timestamp(row,"deleted_at"); require(n.updated>0,"Missing Readest annotation timestamp"); if(!n.deleted) { + try { const auto cfi=optional(row,"cfi");auto endpoints=readest_range_cfi(cfi); if(endpoints.first.empty() && cfi.empty()) endpoints={xpointer_cfi(epub,string_member(row,"xpointer0")),xpointer_cfi(epub,string_member(row,"xpointer1"))}; require(!endpoints.first.empty(),"Unsupported Readest annotation range");n.begin=endpoints.first;n.end=endpoints.second; n.text=string_member(row,"text");n.note=optional(row,"note");n.color=string_member(row,"color");n.style=string_member(row,"style"); n.meta=optional(row,"meta_hash"); validate(n,epub); + } catch(const std::exception& e) {n.unsupported=e.what();} } require(notes.emplace(n.id,n).second,"Duplicate Readest annotation ID"); } @@ -204,7 +229,7 @@ void apply_native(const std::string& database,const std::string& fast_hash,const put(quote.get(),"begin","pbr:/webkit?##"+note.begin);put(quote.get(),"end","pbr:/webkit?##"+note.end);put(quote.get(),"text",note.text); put(anchor.get(),"anchor","pbr:/webkit?##"+note.begin);put(anchor.get(),"created",note.created/1000);put(comment.get(),"text",note.note); for(const auto& [name,value]:std::map{{"bm.type",note.note.empty()?"highlight":"note"}, - {"bm.color",note.color=="#00bcd4"?"cian":note.color},{"bm.note",json_text(comment.get())}, + {"bm.color",native_color(note.color)},{"bm.note",json_text(comment.get())}, {"bm.book_mark",json_text(anchor.get())},{"bm.quotation",json_text(quote.get())}}) { Query tag(db,"SELECT OID FROM TagNames WHERE TagName=?",{name});require(tag.row(),"Missing native annotation tag"); Query(db,"INSERT OR REPLACE INTO Tags(ItemID,TagID,Val,TimeEdt) VALUES(?,?,?,?)",{item,tag.text(0),value,stamp}).row(); @@ -216,7 +241,7 @@ void apply_native(const std::string& database,const std::string& fast_hash,const } } // namespace -void sync_annotations(Cloud& cloud,State& state,const VerifiedManagedBook& verified, +std::string sync_annotations(Cloud& cloud,State& state,const VerifiedManagedBook& verified, const NativePosition& native,const std::string& database,long long now, const std::string& model,const std::string& firmware,const std::atomic& cancel) { require(model=="PB743G" && firmware=="U743g.6.11.1683","Annotation sync is not verified on this firmware"); @@ -224,11 +249,23 @@ void sync_annotations(Cloud& cloud,State& state,const VerifiedManagedBook& verif const auto& book=verified.book();const auto user=cloud.session().user_id; require(!user.empty() && !book.local_only && !book.book.deleted && native.book_path==book.path,"Invalid annotation sync book"); check(cancel); - auto local=read_native(database,native.fast_hash); - for(const auto& [id,n]:local.notes) { (void)id;validate(n,book.path); } + auto local=read_native(database,native.fast_hash,book.path); const auto response=cloud.get("/api/sync?type=notes&since=0&book="+book.book.hash,now); require(response.status==200,"Cannot fetch Readest annotations"); auto remote=remote_notes(response.body,user,book.book.hash,book.path); + std::string warnings; + for(const auto& source:{std::make_pair("PocketBook",&local.notes),std::make_pair("Readest",&remote)}) + for(const auto& [id,n]:*source.second) if(n.annotation && !n.unsupported.empty()) { + if(!warnings.empty()) warnings+=" "; + warnings+="Skipped "+std::string(source.first)+" annotation "+id+": "+n.unsupported+"."; + } + auto unsupported_live=[](const auto& notes) { + return std::any_of(notes.begin(),notes.end(),[](const auto& item) { + const auto& n=item.second;return n.annotation && !n.deleted && !n.unsupported.empty(); + }); + }; + const bool local_unsupported=unsupported_live(local.notes),remote_unsupported=unsupported_live(remote); + bool deferred=false; struct Entry {std::string local,base_local="deleted",base_remote="deleted",pending,pending_local;long long remote_stamp=0;}; std::map entries;std::set used; auto saved=parse_json(state.annotations(user,book.book.hash,book.path,book.sha256));auto* records=member(saved.get(),"entries"); @@ -253,10 +290,10 @@ void sync_annotations(Cloud& cloud,State& state,const VerifiedManagedBook& verif state.save_annotations(user,book.book.hash,book.path,book.sha256,json_text(data.get())); }; for(const auto& [id,n]:remote) { - if(entries.count(id) || n.deleted) continue; + if(entries.count(id) || n.deleted || !n.unsupported.empty()) continue; Entry e; // Adopt a previously imported identical record, including the device trial. - for(const auto& [uuid,l]:local.notes) if(!l.deleted && !used.count(uuid) && fingerprint(l)==fingerprint(n)) { + for(const auto& [uuid,l]:local.notes) if(!l.deleted && l.unsupported.empty() && !used.count(uuid) && fingerprint(l)==fingerprint(n)) { require(e.local.empty(),"Ambiguous identical local annotations");e.local=uuid; } if(e.local.empty()) { @@ -268,7 +305,7 @@ void sync_annotations(Cloud& cloud,State& state,const VerifiedManagedBook& verif require(used.insert(e.local).second,"Annotation mapping collision");entries.emplace(id,e); } for(const auto& [uuid,n]:local.notes) { - if(n.deleted || used.count(uuid)) continue; + if(n.deleted || !n.unsupported.empty() || used.count(uuid)) continue; const auto id=hash(user+":"+book.book.hash+":"+book.path+":"+uuid).substr(0,32); require(!entries.count(id) && !remote.count(id),"Readest annotation identity collision"); Entry e;e.local=uuid;entries[id]=e;used.insert(uuid); @@ -277,11 +314,23 @@ void sync_annotations(Cloud& cloud,State& state,const VerifiedManagedBook& verif save(); for(auto& [id,e]:entries) { check(cancel); + auto native_note=local.notes.find(e.local);auto remote_note=remote.find(id); + if((native_note!=local.notes.end() && !native_note->second.unsupported.empty()) || + (remote_note!=remote.end() && !remote_note->second.unsupported.empty())) continue; auto observed=remote.find(id); require(!e.remote_stamp || observed!=remote.end(),"Readest annotation disappeared without a deletion marker"); const auto remote_stamp=observed==remote.end()?0:std::max(observed->second.updated,observed->second.deleted); require(remote_stamp>=e.remote_stamp,"Stale Readest annotation response; retry"); auto lf=fingerprint(local.notes,e.local),rf=fingerprint(remote,id); + if(e.base_local=="deleted" && e.base_remote=="deleted" && lf!=rf) { + const bool native_deleted=native_note!=local.notes.end() && native_note->second.deleted; + const bool remote_deleted=remote_note!=remote.end() && remote_note->second.deleted; + require(!native_deleted && !remote_deleted,"Annotation deleted during interrupted initial sync; resolve the conflicting copies"); + } + // Native Edit may tombstone an old UUID and create an unsupported + // replacement. Keep the other reader's copy until that replacement can sync. + if((lf=="deleted" && rf!="deleted" && e.base_local!="deleted" && local_unsupported) || + (rf=="deleted" && lf!="deleted" && e.base_remote!="deleted" && remote_unsupported)) {deferred=true;continue;} if(lf==rf) { e.base_local=lf;e.base_remote=rf;e.remote_stamp=remote_stamp;e.pending.clear();e.pending_local.clear();save();continue; } @@ -298,7 +347,7 @@ void sync_annotations(Cloud& cloud,State& state,const VerifiedManagedBook& verif if(rc) { auto it=remote.find(id);require(it!=remote.end(),"Readest annotation disappeared without a deletion marker"); apply_native(database,native.fast_hash,local,e.local,it->second,cancel); - local=read_native(database,native.fast_hash); + local=read_native(database,native.fast_hash,book.path); require(fingerprint(local.notes,e.local)==rf,"Native annotation write could not be verified"); e.base_local=rf;e.base_remote=rf;e.remote_stamp=remote_stamp;save();continue; } @@ -317,12 +366,23 @@ void sync_annotations(Cloud& cloud,State& state,const VerifiedManagedBook& verif e.pending=payload(n,book.book.hash);e.pending_local=lf;save(); } check(cancel); - require(read_native(database,native.fast_hash).raw==local.raw,"Annotations changed on PocketBook during sync; retry"); + require(read_native(database,native.fast_hash,book.path).raw==local.raw,"Annotations changed on PocketBook during sync; retry"); // Recheck the remote record before a write; the API has no conditional update. auto fresh=cloud.get("/api/sync?type=notes&since=0&book="+book.book.hash,now); require(fresh.status==200,"Cannot recheck Readest annotations; saved for retry"); auto latest=remote_notes(fresh.body,user,book.book.hash,book.path); require(fingerprint(latest,id)==rf,"Annotation changed in Readest before upload; retry"); + auto pending=parse_json(e.pending);const auto current=latest.find(id); + if(current!=latest.end()) { + const auto version=std::max(current->second.updated,current->second.deleted); + require(version>=e.remote_stamp,"Stale Readest annotation response; retry"); + if(version>=integer_member(pending.get(),"updatedAt")) { + require(version<0x7fffffffffffffffLL-1,"Invalid remote annotation timestamp"); + put(pending.get(),"updatedAt",version+1); + if(member(pending.get(),"deletedAt")) put(pending.get(),"deletedAt",version+1); + e.pending=json_text(pending.get());save(); + } + } check(cancel); // Retry the same body and ID. Server-authoritative data must acknowledge it. auto posted=cloud.post("/api/sync","{\"books\":[],\"configs\":[],\"notes\":["+e.pending+"]}",now); @@ -333,8 +393,10 @@ void sync_annotations(Cloud& cloud,State& state,const VerifiedManagedBook& verif remote[id]=winner->second;e.base_local=e.pending_local;e.base_remote=e.pending_local; e.remote_stamp=std::max(winner->second.updated,winner->second.deleted); e.pending.clear();e.pending_local.clear();save(); - local=read_native(database,native.fast_hash); + local=read_native(database,native.fast_hash,book.path); require(fingerprint(local.notes,e.local)==e.base_local,"Newer PocketBook annotation edits remain; sync again"); } + if(deferred) warnings+=" Deletions were deferred because unsupported annotations may be replacement edits."; + return warnings; } } // namespace readest diff --git a/apps/readest-sync/src/annotations.h b/apps/readest-sync/src/annotations.h index 0849451..5bb9bae 100644 --- a/apps/readest-sync/src/annotations.h +++ b/apps/readest-sync/src/annotations.h @@ -6,7 +6,8 @@ namespace readest { // copy. Full per-book pulls avoid client-clock cursors. Retry state is durable // before writes; conflicting edits fail without overwriting either side. // Native writes require the validated firmware and an unchanged snapshot. -void sync_annotations(Cloud& cloud,State& state,const VerifiedManagedBook& verified, +// Returns a warning describing annotations skipped without modifying them. +std::string sync_annotations(Cloud& cloud,State& state,const VerifiedManagedBook& verified, const NativePosition& native,const std::string& database,long long now, const std::string& model,const std::string& firmware,const std::atomic& cancel); } diff --git a/apps/readest-sync/src/application.cpp b/apps/readest-sync/src/application.cpp index a04d668..cd6dd2f 100644 --- a/apps/readest-sync/src/application.cpp +++ b/apps/readest-sync/src/application.cpp @@ -132,7 +132,9 @@ void ApplicationService::synchronize(const Request& request, OperationResult& re state_->save_upload(request.book.account,request.book.hash,{}); if(!config_.annotations_database.empty() && transition.outcome!=ResumeOutcome::SyncUnavailable && transition.outcome!=ResumeOutcome::CommitUncertain && transition.outcome!=ResumeOutcome::AppliedUnrecorded) { - try { sync_annotations(*cloud_,*state_,verified,native,config_.annotations_database,time(nullptr),config_.model,config_.firmware,cancel); } + try { const auto warning=sync_annotations(*cloud_,*state_,verified,native,config_.annotations_database,time(nullptr),config_.model,config_.firmware,cancel); + if(!warning.empty()) {if(!result.progress_warning.empty()) result.progress_warning+=" ";result.progress_warning+=warning;} + } catch(const std::exception& e) { check_cancel(cancel); if(!result.progress_warning.empty()) result.progress_warning+=" "; diff --git a/apps/readest-sync/tests/annotations_test.cpp b/apps/readest-sync/tests/annotations_test.cpp index d82bf8c..577da87 100644 --- a/apps/readest-sync/tests/annotations_test.cpp +++ b/apps/readest-sync/tests/annotations_test.cpp @@ -44,7 +44,9 @@ int main(int argc,char** argv) { auto* in=json_object_array_get_idx(list,0);const auto id=string_member(in,"id"); auto out=parse_json(server.count(id)?server.at(id):"{}"); set(out.get(),"user_id",std::string("user"));set(out.get(),"book_hash",integrity.readest_hash); - if(!server_wins) { + const bool newer=!server.count(id) || integer_member(in,"updatedAt")>record_timestamp(out.get(),"updated_at") || + record_timestamp(in,"deletedAt")>record_timestamp(out.get(),"deleted_at"); + if(!server_wins && newer) { for(const auto& [a,b]:std::map{{"id","id"},{"type","type"},{"cfi","cfi"},{"text","text"},{"note","note"},{"color","color"},{"style","style"}, {"createdAt","created_at"},{"updatedAt","updated_at"},{"deletedAt","deleted_at"}}) { auto* v=member(in,a.c_str());json_object_object_add(out.get(),b.c_str(),v?json_object_get(v):nullptr); @@ -56,7 +58,7 @@ int main(int argc,char** argv) { }; Cloud cloud(root+"/annotations-session.json","ca","public","https://auth.test","https://api.test",api);cloud.sign_in("test","test",1000); State state(root+"/annotations-state.db");std::atomic cancel{false};long long now=2000; - auto sync=[&] {sync_annotations(cloud,state,verified,native,database,++now,"PB743G","U743g.6.11.1683",cancel);}; + auto sync=[&] {return sync_annotations(cloud,state,verified,native,database,++now,"PB743G","U743g.6.11.1683",cancel);}; const std::string cfi="epubcfi(/6/2!/4/4/1,:0,:16)",text="Marker ALPHA-01."; auto remote=[&](std::string id,std::string note) { auto row=parse_json("{}");set(row.get(),"id",id);set(row.get(),"user_id",std::string("user"));set(row.get(),"book_hash",integrity.readest_hash); @@ -74,6 +76,10 @@ int main(int argc,char** argv) { sql(db,"UPDATE Tags SET Val=replace(replace(Val,'pbr:/webkit?##','pbr:/page?page=2&offs=4#'),'/1:0)','/1)') WHERE TagID IN (104,107)"); sync();assert(posts==0); mutate("remote1","note","remote edited");sync();assert(scalar(db,"SELECT Val FROM Tags WHERE ItemID=2 AND TagID=105")=="{\"text\":\"remote edited\"}"); + for(const auto& color:{"blue","green","#00bcd4","#00ff00","#ff00ff","yellow"}) { + mutate("remote1","color",color);assert(sync().empty()); + assert(sync().empty());assert(posts==0); + } // Native in-place edits upload to the known ID. native_note("{\"text\":\"local edited\"}");sync();assert(posts==1 && server.size()==1); sync();assert(posts==1); @@ -88,6 +94,11 @@ int main(int argc,char** argv) { assert(last_post==pending);int successful=posts;sync();assert(posts==successful); // Ambiguous commit: the next pull acknowledges it without a second POST. native_note("{\"text\":\"already committed\"}");lose_response=true;fails(sync,"lost response");successful=posts;lose_response=false;sync();assert(posts==successful); + // Rebase a pending upload when the server saved unchanged content later. + native_note("{\"text\":\"newer pending\"}");fail_post=true;fails(sync,"saved for retry");fail_post=false; + auto advanced=parse_json(server.at("remote1"));set(advanced.get(),"updated_at",(now+100)*1000);server["remote1"]=json_text(advanced.get()); + sync();assert(string_member(parse_json(server.at("remote1")).get(),"note")=="newer pending"); + now+=101; // Server winner is not treated as an acknowledgement of our edit. native_note("{\"text\":\"rejected edit\"}");server_wins=true;fails(sync,"did not accept");server_wins=false;sync(); // Native Edit creates a new UUID and tombstones the old UUID. @@ -105,9 +116,37 @@ int main(int argc,char** argv) { fails(sync,"Stale Readest");assert(scalar(db,"SELECT State FROM Items WHERE OID=3")=="2");server[replacement]=tombstone; // Wrong account, bad ranges and concurrent native changes fail before import. server["new"]=remote("new","new note");mutate("new","user_id","wrong");fails(sync,"account/book mismatch");mutate("new","user_id","user"); - mutate("new","cfi","epubcfi(/6/2!/4/4/1,:0,:999999)");fails(sync,"exceeds");mutate("new","cfi",cfi); - race=true;fails(sync,"changed on PocketBook");race=false;sync(); + mutate("new","cfi","epubcfi(/6/2!/4/4/1,:0,:999999)");assert(sync().find("exceeds")!=std::string::npos);mutate("new","cfi",cfi); + race=true;fails(sync,"changed on PocketBook");race=false; + server["unsupported"]=remote("unsupported","unsupported color");mutate("unsupported","color","red"); + assert(sync().find("unsupported")!=std::string::npos); + assert(server.count("unsupported")); // Unrelated supported import still completed. + server.erase("unsupported"); assert(scalar(db,"SELECT count(*) FROM Items WHERE State=0 AND ParentID=1")=="1"); + // An unsupported replacement must not delete the previously synced cloud copy. + sql(db,"UPDATE Items SET State=2 WHERE OID=4; INSERT INTO Items VALUES(5,1,4,0,2001,'UNSUPPORTED-REPLACEMENT');" + "INSERT INTO Tags(ItemID,TagID,Val,TimeEdt) SELECT 5,TagID,Val,TimeEdt FROM Tags WHERE ItemID=4;" + "UPDATE Tags SET Val='unknown-color' WHERE ItemID=5 AND TagID=106;"); + server["another"]=remote("another","unrelated incoming"); + assert(sync().find("deferred")!=std::string::npos); + assert(scalar(db,"SELECT count(*) FROM Items WHERE State=0 AND ParentID=1")=="2"); // Unsupported replacement plus new import. + assert(record_timestamp(parse_json(server.at("new")).get(),"deleted_at")==0); + sql(db,"UPDATE Tags SET Val='yellow' WHERE ItemID=5 AND TagID=106;"); + server["unsupported2"]=remote("unsupported2","unrelated unsupported remote");mutate("unsupported2","color","red"); + assert(!sync().empty());assert(record_timestamp(parse_json(server.at("new")).get(),"deleted_at")>0); + bool replacement_uploaded=false; + for(const auto& [id,row]:server) if(id!="new" && id!="another" && id!="unsupported2" && !record_timestamp(parse_json(row).get(),"deleted_at")) replacement_uploaded=true; + assert(replacement_uploaded);server.erase("unsupported2"); + // A deletion after an interrupted first import is not a missing-new-record case. + auto journal=parse_json(state.annotations("user",integrity.readest_hash,epub,integrity.sha256)); + auto* entries=member(journal.get(),"entries"); + for(size_t i=0;i* expected_cancel=nullptr; config.transport.request=[&](const std::string& url,const std::string&,const std::vector&, const std::string&,const std::string&,size_t,const std::atomic& token) { assert(&token==expected_cancel); ++requests; + if(url.find("type=notes")!=std::string::npos) {++annotation_gets;return HttpResponse{200,R"({"notes":[]})",0};} if(url.find("type=configs")!=std::string::npos) { HttpResponse r; r.status=fail_progress?503:200; r.body=R"({"configs":[]})"; return r; } @@ -73,6 +74,24 @@ int main(int argc,char** argv) { assert(full_inspections==1); assert(result.open_path==(command==Command::Sync?std::string():local)); } + { + auto annotated=config;annotated.annotations_database=base+"/annotations.db"; + annotated.model="PB743G";annotated.firmware="U743g.6.11.1683"; + sqlite3* notes=nullptr;assert(sqlite3_open(annotated.annotations_database.c_str(),¬es)==SQLITE_OK); + assert(sqlite3_exec(notes,"CREATE TABLE TypeNames(OID,TypeName);CREATE TABLE TagNames(OID,TagName);" + "CREATE TABLE Items(OID,ParentID,TypeID,State,TimeAlt,HashUUID);CREATE TABLE Tags(ItemID,TagID,Val,TimeEdt);" + "INSERT INTO TypeNames VALUES(0,'type.book');INSERT INTO Items VALUES(1,NULL,0,0,0,'00112233445566778899AABBCCDDEEFF');", + nullptr,nullptr,nullptr)==SQLITE_OK);sqlite3_close(notes); + ApplicationService with_annotations(annotated);assert(with_annotations.execute(Request{},cancel).outcome==Outcome::Ready); + request.command=Command::Sync;auto annotated_result=with_annotations.execute(request,cancel); + assert(annotated_result.outcome==Outcome::Synced && annotated_result.progress_warning.empty() && annotation_gets==1); + request.command=Command::Open;annotated_result=with_annotations.execute(request,cancel); + assert(annotated_result.open_path==local && annotated_result.progress_warning.empty() && annotation_gets==2); + request.command=Command::ReadOffline;with_annotations.execute(request,cancel);assert(annotation_gets==2); + assert(unlink(annotated.annotations_database.c_str())==0); + request.command=Command::Open;annotated_result=with_annotations.execute(request,cancel); + assert(annotated_result.open_path==local && annotated_result.progress_warning.find("Annotations:")!=std::string::npos); + } fail_progress=true; request.command=Command::Open; full_inspections=0; result=service.execute(request,cancel); assert(result.outcome==Outcome::SyncUnavailable && result.open_path==local && full_inspections==1); diff --git a/tools/pocketbook-simulator/apps/readest-sync/mock_cloud.cpp b/tools/pocketbook-simulator/apps/readest-sync/mock_cloud.cpp index 1296cbb..ed319d3 100644 --- a/tools/pocketbook-simulator/apps/readest-sync/mock_cloud.cpp +++ b/tools/pocketbook-simulator/apps/readest-sync/mock_cloud.cpp @@ -42,6 +42,7 @@ void pause(int milliseconds,const std::atomic& cancellation) { } void MockCloud::saveRemote() { writeFile(root+"/remote.json",QJsonDocument(configs).toJson()); + writeFile(root+"/notes.json",QJsonDocument(notes).toJson()); writeFile(root+"/library.json",QJsonDocument(books).toJson()); writeFile(root+"/files.json",QJsonDocument(files).toJson()); // Keep the library's display metadata consistent with saved remote configs. @@ -101,6 +102,7 @@ MockCloud::MockCloud(QString storage,const QString& fixtures):root(std::move(sto const auto rows=saved.object(); for(auto it=rows.begin();it!=rows.end();++it) configs[it.key()]=it.value(); } + if(QFile::exists(root+"/notes.json")) notes=QJsonDocument::fromJson(readFile(root+"/notes.json")).object(); saveRemote(); } HttpResponse MockCloud::request(const std::string& address,const std::string& method, @@ -125,6 +127,10 @@ HttpResponse MockCloud::request(const std::string& address,const std::string& me } else if(query.queryItemValue("type")=="configs") { const auto row=configs.value(query.queryItemValue("book")); result={{"configs",row.isObject()?QJsonArray{row}:QJsonArray{}}}; + } else if(query.queryItemValue("type")=="notes") { + QJsonArray rows; + for(const auto& value:notes) if(value.toObject()["book_hash"]==query.queryItemValue("book")) rows.append(value); + result={{"notes",rows}}; } else throw std::runtime_error("Unknown simulator sync request"); } else if(url.path()=="/api/sync" && method=="POST") { const auto payload=QJsonDocument::fromJson(QByteArray::fromStdString(body)).object(); @@ -145,7 +151,21 @@ HttpResponse MockCloud::request(const std::string& address,const std::string& me row["updated_at"]=incoming["updatedAt"]; configs[hash]=row; } - saveRemote(); result={{"success",true}}; + QJsonArray accepted; + for(const auto& entry:payload["notes"].toArray()) { + const auto incoming=entry.toObject();const auto key=incoming["bookHash"].toString()+"|"+incoming["id"].toString(); + auto row=notes[key].toObject(); + if(row.isEmpty() || incoming["updatedAt"].toInteger()>row["updated_at"].toInteger() || + incoming["deletedAt"].toInteger()>row["deleted_at"].toInteger()) { + row={{"user_id",user},{"book_hash",incoming["bookHash"]}}; + const QMap fields={{"id","id"},{"type","type"},{"cfi","cfi"},{"text","text"},{"note","note"}, + {"color","color"},{"style","style"},{"createdAt","created_at"},{"updatedAt","updated_at"},{"deletedAt","deleted_at"}}; + for(auto field=fields.cbegin();field!=fields.cend();++field) row[field.value()]=incoming.value(field.key()); + notes[key]=row; + } + accepted.append(row); + } + saveRemote(); result={{"success",true},{"notes",accepted}}; } else if(url.path()=="/api/storage/upload" && method=="POST") { const auto payload=QJsonDocument::fromJson(QByteArray::fromStdString(body)).object(); const auto key=user+"/"+payload["fileName"].toString(); diff --git a/tools/pocketbook-simulator/apps/readest-sync/mock_cloud.h b/tools/pocketbook-simulator/apps/readest-sync/mock_cloud.h index c136a4d..1eb02b1 100644 --- a/tools/pocketbook-simulator/apps/readest-sync/mock_cloud.h +++ b/tools/pocketbook-simulator/apps/readest-sync/mock_cloud.h @@ -23,7 +23,7 @@ class MockCloud : public std::enable_shared_from_this { void saveRemote(); // Caller holds remoteMutex once published to other threads. QString root; QJsonArray books,files; - QJsonObject configs; + QJsonObject configs,notes; QMap objects; mutable QMutex remoteMutex; std::atomic transferMode{0}; diff --git a/tools/pocketbook-simulator/apps/readest-sync/mock_cloud_test.cpp b/tools/pocketbook-simulator/apps/readest-sync/mock_cloud_test.cpp index ca51bd2..a09ae16 100644 --- a/tools/pocketbook-simulator/apps/readest-sync/mock_cloud_test.cpp +++ b/tools/pocketbook-simulator/apps/readest-sync/mock_cloud_test.cpp @@ -1,5 +1,6 @@ #include "mock_cloud.h" #include "application.h" +#include "json_util.h" #include #include #include @@ -27,6 +28,13 @@ int main() { "https://readest.supabase.co","https://web.readest.com",second->transport().bind_request(cancel)); a.sign_in("test","test",time(nullptr)); b.sign_in("test","test",time(nullptr)); const auto hash=first->firstHash(); + const auto annotation="{\"books\":[],\"configs\":[],\"notes\":[{\"id\":\"test-note\",\"type\":\"annotation\",\"bookHash\":\""+hash.toStdString()+ + "\",\"cfi\":\"epubcfi(/6/2!/4/4/1,:0,:16)\",\"text\":\"Marker ALPHA-01.\",\"note\":\"mock note\",\"style\":\"highlight\",\"color\":\"yellow\",\"createdAt\":1,\"updatedAt\":10}]}"; + assert(a.post("/api/sync",annotation,time(nullptr)).body.find("mock note")!=std::string::npos); + auto older=annotation;older.replace(older.find("mock note"),9,"old note");older.replace(older.find("\"updatedAt\":10"),14,"\"updatedAt\":9"); + assert(a.post("/api/sync",older,time(nullptr)).body.find("mock note")!=std::string::npos); + assert(a.get("/api/sync?type=notes&book="+hash.toStdString(),time(nullptr)).body.find("mock note")!=std::string::npos); + assert(b.get("/api/sync?type=notes&book="+hash.toStdString(),time(nullptr)).body.find("mock note")==std::string::npos); first->remoteChapter(hash,3); assert(a.get("/api/sync?type=configs&book="+hash.toStdString(),time(nullptr)).body.find("/6/6!")!=std::string::npos); assert(b.get("/api/sync?type=configs&book="+hash.toStdString(),time(nullptr)).body.find("/6/2!")!=std::string::npos); diff --git a/tools/pocketbook-simulator/apps/readest-sync/simulator.cpp b/tools/pocketbook-simulator/apps/readest-sync/simulator.cpp index f9f651a..6164b81 100644 --- a/tools/pocketbook-simulator/apps/readest-sync/simulator.cpp +++ b/tools/pocketbook-simulator/apps/readest-sync/simulator.cpp @@ -38,9 +38,10 @@ void writeFile(const QString& path,const QByteArray& bytes) { throw std::runtime_error("Cannot save simulator data"); } QString cfi(int chapter) { return QString("epubcfi(/6/%1!/4/2)").arg(chapter*2); } -void sql(const char* statement,const QStringList& values={}) { +void sql(const char* statement,const QStringList& values={},bool annotations=false) { sqlite3* raw=nullptr; - if(sqlite3_open(platform::nativeDatabase().toUtf8().constData(),&raw)!=SQLITE_OK) { + const auto database=annotations?storage()+"/system/config/books.db":platform::nativeDatabase(); + if(sqlite3_open(database.toUtf8().constData(),&raw)!=SQLITE_OK) { if(raw) sqlite3_close(raw); throw std::runtime_error("Cannot open simulated reading database"); } @@ -94,11 +95,18 @@ void Simulator::prepare() { prepareStorage(); const auto base=storage(); QDir data(storage()); - if(!data.mkpath("system/readest-sync") || !data.mkpath("system/explorer-3") || !data.mkpath("Books/Readest")) + if(!data.mkpath("system/readest-sync") || !data.mkpath("system/explorer-3") || !data.mkpath("system/config") || !data.mkpath("Books/Readest")) throw std::runtime_error("Cannot create simulator storage"); sql("CREATE TABLE IF NOT EXISTS folders(id INTEGER PRIMARY KEY, storageid INTEGER, name TEXT)"); sql("CREATE TABLE IF NOT EXISTS files(book_id INTEGER PRIMARY KEY, folder_id INTEGER, storageid INTEGER, filename TEXT, fast_hash BLOB)"); sql("CREATE TABLE IF NOT EXISTS books_settings(bookid INTEGER PRIMARY KEY, profileid INTEGER, position TEXT, position_ts INTEGER, cpage INTEGER, npage INTEGER, completed INTEGER)"); + for(const auto* statement:{ + "CREATE TABLE IF NOT EXISTS TypeNames(OID INTEGER PRIMARY KEY,TypeName TEXT UNIQUE)", + "CREATE TABLE IF NOT EXISTS TagNames(OID INTEGER PRIMARY KEY,TagName TEXT UNIQUE)", + "CREATE TABLE IF NOT EXISTS Items(OID INTEGER PRIMARY KEY,ParentID INTEGER,TypeID INTEGER,State INTEGER DEFAULT 0,TimeAlt INTEGER,HashUUID TEXT)", + "CREATE TABLE IF NOT EXISTS Tags(OID INTEGER PRIMARY KEY,ItemID INTEGER REFERENCES Items(OID),TagID INTEGER REFERENCES TagNames(OID),Val TEXT,TimeEdt INTEGER,UNIQUE(ItemID,TagID))", + "INSERT OR IGNORE INTO TypeNames VALUES(0,'type.book'),(4,'obj.book_mark')", + "INSERT OR IGNORE INTO TagNames VALUES(101,'bm.book_mark'),(102,'bm.type'),(104,'bm.quotation'),(105,'bm.note'),(106,'bm.color')"}) sql(statement,{},true); if(realCloud()) { // Reuse the system trust store; never disable TLS verification. const auto certificates=qEnvironmentVariable("READEST_SIM_CA","/etc/ssl/certs/ca-certificates.crt"); @@ -151,7 +159,9 @@ bool Simulator::open(const QString& path) { for(int i=2;i<=3;++i) if(!position.cfi.empty() && readest::compare_cfi(position.cfi,cfi(i).toStdString())>=0) chapter_=i; const auto number=QString::number(id); sql("INSERT OR REPLACE INTO folders VALUES(?,1,?)",{number,QFileInfo(path).absolutePath()}); - sql("INSERT OR REPLACE INTO files VALUES(?,?,1,?,zeroblob(16))",{number,number,QFileInfo(path).fileName()}); + sql(("INSERT OR REPLACE INTO files VALUES(?,?,1,?,X'"+hash+"')").toUtf8().constData(),{number,number,QFileInfo(path).fileName()}); + sql("INSERT INTO Items(ParentID,TypeID,State,TimeAlt,HashUUID) SELECT NULL,0,0,0,? WHERE NOT EXISTS(SELECT 1 FROM Items WHERE HashUUID=?)", + {hash.toUpper(),hash.toUpper()},true); path_=path; lastHash_=hash; saveChapter(); emit changed(); return true; } catch(const std::exception& error) { path_.clear(); report(QString::fromUtf8(error.what())); return false; } From eb0e76512a735947c09857898d82e82cfd9c196c Mon Sep 17 00:00:00 2001 From: Oliver Gassner Date: Sat, 26 Sep 2026 15:56:10 +0200 Subject: [PATCH 3/5] Persist only changed local library rows during scans --- apps/readest-sync/src/state.cpp | 18 ++++++- apps/readest-sync/tests/application_test.cpp | 49 ++++++++++++++++++++ 2 files changed, 65 insertions(+), 2 deletions(-) diff --git a/apps/readest-sync/src/state.cpp b/apps/readest-sync/src/state.cpp index d8ce6a3..17145a8 100644 --- a/apps/readest-sync/src/state.cpp +++ b/apps/readest-sync/src/state.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include @@ -295,11 +296,24 @@ std::vector State::local_copies() { void State::replace_local_copies(const std::vector& copies) { sql(db_,"BEGIN IMMEDIATE"); try { - sql(db_,"DELETE FROM local_copies"); + std::map previous; + for(const auto& copy:local_copies()) previous[copy.path]=copy; + std::set present; + const auto fields=[](const LocalCopy& copy) { + return std::tie(copy.stamp,copy.hash,copy.title,copy.author,copy.position,copy.size); + }; for(const auto& copy:copies) { - Query q(db_,"INSERT OR REPLACE INTO local_copies VALUES(?,?,?,?,?,?,?)"); + present.insert(copy.path); + const auto found=previous.find(copy.path); + if(found!=previous.end() && fields(found->second)==fields(copy)) continue; + Query q(db_,found==previous.end() ? "INSERT INTO local_copies VALUES(?,?,?,?,?,?,?)" : + "UPDATE local_copies SET stamp=?2,hash=?3,title=?4,author=?5,position=?6,size=?7 WHERE path=?1"); q.bind(1,copy.path);q.bind(2,copy.stamp);q.bind(3,copy.hash);q.bind(4,copy.title); q.bind(5,copy.author);q.bind(6,copy.position);q.bind(7,copy.size);q.row(); + previous[copy.path]=copy; + } + for(const auto& entry:previous) if(!present.count(entry.first)) { + Query q(db_,"DELETE FROM local_copies WHERE path=?");q.bind(1,entry.first);q.row(); } sql(db_,"COMMIT"); } catch(...) {sqlite3_exec(db_,"ROLLBACK",nullptr,nullptr,nullptr);throw;} diff --git a/apps/readest-sync/tests/application_test.cpp b/apps/readest-sync/tests/application_test.cpp index f3f201d..1ba2524 100644 --- a/apps/readest-sync/tests/application_test.cpp +++ b/apps/readest-sync/tests/application_test.cpp @@ -12,6 +12,39 @@ extern unsigned full_inspections; int main(int argc,char** argv) { assert(argc==3); const auto base=std::string(argv[1])+"/application"; make_directory(base); + { + State state(base+"/delta.db"); sqlite3* audit=nullptr; + assert(sqlite3_open((base+"/delta.db").c_str(),&audit)==SQLITE_OK); + assert(sqlite3_exec(audit,"CREATE TABLE events(kind);" + "CREATE TRIGGER inserted AFTER INSERT ON local_copies BEGIN INSERT INTO events VALUES('insert'); END;" + "CREATE TRIGGER updated AFTER UPDATE ON local_copies BEGIN INSERT INTO events VALUES('update'); END;" + "CREATE TRIGGER deleted AFTER DELETE ON local_copies BEGIN INSERT INTO events VALUES('delete'); END;", + nullptr,nullptr,nullptr)==SQLITE_OK); + auto counts=[&](int inserts,int updates,int deletes) { + sqlite3_stmt* q=nullptr; + assert(sqlite3_prepare_v2(audit,"SELECT COUNT(*) FROM events WHERE kind=?",-1,&q,nullptr)==SQLITE_OK); + for(const auto& entry:std::vector>{{"insert",inserts},{"update",updates},{"delete",deletes}}) { + sqlite3_bind_text(q,1,entry.first,-1,SQLITE_STATIC); + assert(sqlite3_step(q)==SQLITE_ROW && sqlite3_column_int(q,0)==entry.second); + sqlite3_reset(q); + } + sqlite3_finalize(q); + assert(sqlite3_exec(audit,"DELETE FROM events",nullptr,nullptr,nullptr)==SQLITE_OK); + }; + LocalCopy a,b,c; a.path="a"; b.path="b"; c.path="c"; + state.replace_local_copies({a,b}); counts(2,0,0); + state.replace_local_copies({b,a}); counts(0,0,0); + a.position="new position"; + state.replace_local_copies({a,b,c}); counts(1,1,0); + state.replace_local_copies({a,c}); counts(0,0,1); + a.stamp="changed"; a.hash="new hash"; a.title="new title"; a.author="new author"; a.size=42; + state.replace_local_copies({a,c}); counts(0,1,0); + const auto saved=state.local_copies().at(0); + assert(saved.stamp==a.stamp && saved.hash==a.hash && saved.title==a.title && saved.author==a.author && saved.size==a.size); + state.replace_local_copies({}); counts(0,0,2); + state.replace_local_copies({}); counts(0,0,0); + assert(sqlite3_close(audit)==SQLITE_OK); + } ApplicationConfig config; config.root=base+"/state"; config.books_root=base+"/Books/Readest"; config.database=base+"/absent-native.db"; config.ca="fixture-ca"; config.public_key="public"; config.book_roots={base}; make_directory(config.root); @@ -65,6 +98,22 @@ int main(int argc,char** argv) { result=service.execute(request,cancel); assert(result.outcome==Outcome::Reused && requests==1); assert(result.library.books[0].book.path==local && result.library.books[0].availability==Availability::OnDevice); + // A warm scan must not rewrite an unchanged library on flash storage. + sqlite3* scan_state=nullptr; + assert(sqlite3_open((config.root+"/state.db").c_str(),&scan_state)==SQLITE_OK); + assert(sqlite3_exec(scan_state,"CREATE TRIGGER unchanged_scan BEFORE DELETE ON local_copies " + "BEGIN SELECT RAISE(ABORT,'Unchanged scan rewrote the library'); END;",nullptr,nullptr,nullptr)==SQLITE_OK); + request.command=Command::Scan; result=service.execute(request,cancel); + assert(result.outcome==Outcome::Scanned); + assert(sqlite3_exec(scan_state,"DROP TRIGGER unchanged_scan",nullptr,nullptr,nullptr)==SQLITE_OK); + assert(sqlite3_close(scan_state)==SQLITE_OK); + // The fast path must still persist a changed native reading position. + assert(sqlite3_open(config.database.c_str(),&native)==SQLITE_OK); + assert(sqlite3_exec(native,"INSERT INTO books_settings(bookid,profileid,position) VALUES(1,0,'changed-position')",nullptr,nullptr,nullptr)==SQLITE_OK); + result=service.execute(request,cancel); assert(result.outcome==Outcome::Scanned); + { State saved(config.root+"/state.db"); assert(saved.local_copies().at(0).position=="changed-position"); } + assert(sqlite3_exec(native,"DELETE FROM books_settings",nullptr,nullptr,nullptr)==SQLITE_OK); + assert(sqlite3_close(native)==SQLITE_OK); request.command=Command::ReadOffline; result=service.execute(request,cancel); assert(result.outcome==Outcome::LocalOpen && result.open_path==local && requests==1); for(const auto command:{Command::Sync,Command::Open,Command::ReadOffline}) { From 988c148435b46dead22dca3b17ee40ba5269a6c5 Mon Sep 17 00:00:00 2001 From: Oliver Gassner Date: Sat, 26 Sep 2026 16:19:25 +0200 Subject: [PATCH 4/5] Keep offline operations and UI result delivery awake --- apps/readest-sync/src/operation_runner.cpp | 14 ++++++----- apps/readest-sync/src/operation_runner.h | 2 +- apps/readest-sync/src/platform.cpp | 7 +++++- apps/readest-sync/tests/qt_ui_test.cpp | 29 +++++++++++++++++++++- 4 files changed, 43 insertions(+), 9 deletions(-) diff --git a/apps/readest-sync/src/operation_runner.cpp b/apps/readest-sync/src/operation_runner.cpp index 3f5ac55..17e9096 100644 --- a/apps/readest-sync/src/operation_runner.cpp +++ b/apps/readest-sync/src/operation_runner.cpp @@ -10,18 +10,18 @@ OperationRunner::~OperationRunner() { releaseAwake(); } void OperationRunner::releaseAwake() { - if(online_ && device_.keepAwake) device_.keepAwake(false); - online_=false; + if(awake_ && device_.keepAwake) device_.keepAwake(false); + awake_=false; online_=false; } void OperationRunner::cancel() { cancelled_=true; } bool OperationRunner::start(Task task,bool online,Completion complete,std::function phase) { if(busy_) return false; task_=std::move(task); complete_=std::move(complete); phase_=std::move(phase); result_={}; cancelled_=false; done_=false; busy_=true; online_=online; connecting_=online; + // Standby pauses offline workers too, including startup and library scans. + if(!awake_ && device_.keepAwake) device_.keepAwake(true); + awake_=true; if(online) { - // PocketBook standby pauses both background work and the monotonic - // timeout clock. Hold it off until this online operation finishes. - if(device_.keepAwake) device_.keepAwake(true); networkTrace("runner.online"); if(connection_ && connection_->done) connection_.reset(); next_ping_=Clock::now()+std::chrono::milliseconds(device_.keepaliveMs); @@ -98,7 +98,9 @@ void OperationRunner::poll() { } if(!done_) return; if(worker_.joinable()) worker_.join(); - timer_.stop(); busy_=false; releaseAwake(); phase_=nullptr; + timer_.stop(); busy_=false; phase_=nullptr; auto complete=std::move(complete_); complete_=nullptr; complete(std::move(result_)); + // Completion updates the UI and may start the next operation immediately. + if(!busy_) releaseAwake(); } diff --git a/apps/readest-sync/src/operation_runner.h b/apps/readest-sync/src/operation_runner.h index 263b8b5..05ef96d 100644 --- a/apps/readest-sync/src/operation_runner.h +++ b/apps/readest-sync/src/operation_runner.h @@ -22,7 +22,7 @@ class OperationRunner : public QObject { QTimer timer_; std::thread worker_; std::atomic cancelled_{false},done_{false}; - bool busy_=false,connecting_=false,online_=false; + bool busy_=false,connecting_=false,online_=false,awake_=false; std::shared_ptr connection_; Clock::time_point deadline_,next_ping_; Task task_; diff --git a/apps/readest-sync/src/platform.cpp b/apps/readest-sync/src/platform.cpp index d7f5f01..261a3a8 100644 --- a/apps/readest-sync/src/platform.cpp +++ b/apps/readest-sync/src/platform.cpp @@ -16,7 +16,12 @@ std::vector bookRoots() { return {"/mnt/ext1", "/mnt/ext2"}; } int wakeNetwork() { return WiFiPower(1); } int connectNetwork(int (*callback)(int)) { return NetConnectAsync(callback); } void pingNetwork() { NetMgrPing(); } -void keepAwake(bool active) { iv_sleepmode(active?0:1); } +void keepAwake(bool active) { + // Qt renders queued changes after the operation's completion callback. + // Allow that frame and native controls' 200 ms release timers to finish. + if(!active) BanSleep(1); + iv_sleepmode(active?0:1); +} bool networkReady() { static int previous_flags=-1,previous_route=-1; if(previous_flags==-1) networkTrace("ready.first-probe"); diff --git a/apps/readest-sync/tests/qt_ui_test.cpp b/apps/readest-sync/tests/qt_ui_test.cpp index c8a4d35..2c67250 100644 --- a/apps/readest-sync/tests/qt_ui_test.cpp +++ b/apps/readest-sync/tests/qt_ui_test.cpp @@ -169,6 +169,33 @@ static void bookActionChecks() { check(plan(BookChoice::None,BookBlock::Acknowledged),"refresh,back","refresh","","",""); } static void runnerChecks() { + { + // Startup and other offline workers must not be suspended mid-task. + bool awake=false,completed=false; + DeviceAccess device; + device.keepAwake=[&](bool value) { assert(awake!=value);awake=value; }; + device.connect=[](std::function) { assert(false);return false; }; + for(bool fail:{false,true}) { + completed=false; OperationRunner runner(device); + assert(runner.start([&](const std::atomic&) -> OperationResult { + assert(awake); + if(fail) throw std::runtime_error("Offline failure"); + return OperationResult{}; + },false,[&](OperationResult r) { + assert(awake);assert(r.outcome==(fail?Outcome::Failed:Outcome::Ready));completed=true; + },[](bool online) { assert(!online); })); + settle(250);assert(completed && !awake); + } + OperationRunner runner(device); int completions=0; + auto work=[&](const std::atomic&) { assert(awake);return OperationResult{}; }; + assert(runner.start(work,false,[&](OperationResult) { + assert(awake);++completions; + assert(runner.start(work,false,[&](OperationResult) { + assert(awake);++completions; + },[](bool) {})); + },[](bool) {})); + settle(500);assert(completions==2 && !awake); + } { // An online operation must hold standby off before waking Wi-Fi, // through the worker, and release it on every completion path. @@ -179,7 +206,7 @@ static void runnerChecks() { device.connectionTimeoutMs=1000; device.connect=[&](std::function callback) { assert(awake);callback(0);return true; }; auto work=[&](const std::atomic&) { assert(awake);++executions;return OperationResult{}; }; - auto complete=[&](OperationResult) { assert(!awake);completed=true; }; + auto complete=[&](OperationResult) { assert(awake);completed=true; }; { OperationRunner runner(device); assert(runner.start(work,true,complete,[](bool) {})); From da9b3e7e053f72fae866a293b8b5bce565eb05d7 Mon Sep 17 00:00:00 2001 From: Oliver Gassner Date: Sat, 26 Sep 2026 16:22:21 +0200 Subject: [PATCH 5/5] Record monotonic milliseconds in operation traces --- apps/readest-sync/src/application.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/apps/readest-sync/src/application.cpp b/apps/readest-sync/src/application.cpp index cd6dd2f..308967d 100644 --- a/apps/readest-sync/src/application.cpp +++ b/apps/readest-sync/src/application.cpp @@ -9,6 +9,7 @@ #include #include #include +#include namespace readest { ApplicationService::ApplicationService(ApplicationConfig config):config_(std::move(config)) { @@ -29,8 +30,10 @@ void ApplicationService::trace(const char* phase) const { peak/=1024; #endif char line[192]; - const int size=snprintf(line,sizeof(line),"%lld pid=%ld peak_kib=%ld %s\n", - static_cast(time(nullptr)),static_cast(getpid()),peak,phase); + const auto ms=std::chrono::duration_cast( + std::chrono::steady_clock::now().time_since_epoch()).count(); + const int size=snprintf(line,sizeof(line),"%lld pid=%ld peak_kib=%ld mono_ms=%lld %s\n", + static_cast(time(nullptr)),static_cast(getpid()),peak,static_cast(ms),phase); if(size>0 && static_cast(size)