From 63abff86816eb7e12e3b3a509612c6ffa77826c3 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 8 Oct 2026 15:05:43 -0400 Subject: [PATCH] DRY pass #2, batch 8: the Rust and ffi docs (#5372) sync/mod.rs listed 5 of its 9 modules; migrate's doc sat above the v9 SQL; client.rs had items after its test module; the ffi's sync_now doc had fused into client_update's; complete_reminder (ffi and EditorAction) still said recurrence advancement was to come, though the core does it; NoteQuery.view listed views the core never matched and claimed it validated. Test scratch dirs drop the old ts-/iw- prefixes. Co-Authored-By: Claude Opus 5.5 --- .../fabledsword/inkwell/ui/EditorAction.kt | 7 +- android/ffi/src/lib.rs | 18 +-- android/ffi/src/models.rs | 3 +- core/src/local/schema.rs | 4 +- core/src/sync/blobs.rs | 4 +- core/src/sync/client.rs | 138 +++++++++--------- core/src/sync/mod.rs | 4 + desktop/src-tauri/src/crossover.rs | 3 +- desktop/src-tauri/src/update.rs | 2 +- 9 files changed, 93 insertions(+), 90 deletions(-) diff --git a/android/app/src/main/java/com/fabledsword/inkwell/ui/EditorAction.kt b/android/app/src/main/java/com/fabledsword/inkwell/ui/EditorAction.kt index 7901e60..7fce37a 100644 --- a/android/app/src/main/java/com/fabledsword/inkwell/ui/EditorAction.kt +++ b/android/app/src/main/java/com/fabledsword/inkwell/ui/EditorAction.kt @@ -80,10 +80,9 @@ sealed interface EditorAction { /** * Mark the reminder dealt with. * - * Distinct from [ClearReminder] even though the core does the same thing to - * the column today: this is where recurrence advancement lands when it is - * built, so a recurring reminder finished through the generic clear would - * silently stop recurring. + * Distinct from [ClearReminder]: a recurring reminder moves on to its next + * occurrence here, and only a one-off is cleared, so a recurring reminder + * finished through the generic clear would stop recurring. */ data object CompleteReminder : EditorAction diff --git a/android/ffi/src/lib.rs b/android/ffi/src/lib.rs index 151e6f2..f4a3feb 100644 --- a/android/ffi/src/lib.rs +++ b/android/ffi/src/lib.rs @@ -291,11 +291,9 @@ impl Inkwell { /// Clear the reminder, marking it dealt with. /// - /// Distinct from `NoteEdit::ClearRemindAt` even though today they do the same - /// thing: the core reserves this one for "the reminder fired and is finished", - /// which is where recurrence advancement lands when it is built. A UI that - /// called the generic clear instead would silently stop recurring reminders - /// from recurring the day that changes. + /// Distinct from `NoteEdit::ClearRemindAt`: a recurring reminder moves on to its + /// next occurrence here, and only a one-off is cleared. A UI that called the + /// generic clear instead would stop a recurring reminder from recurring. pub fn complete_reminder(&self, id: String) -> Result { let conn = self.db.conn().map_err(CoreError::store)?; local::store::complete_reminder(&conn, &id) @@ -539,11 +537,6 @@ impl Inkwell { Ok(revoked.into()) } - /// Run one full sync: push local changes, then pull the server's. - /// - /// The only sync entry point, on purpose. Push and pull exist separately inside - /// the core, but offering a bare "pull" would let the UI overwrite unsent local - /// edits — the ordering isn't a suggestion, it's what keeps them. /// The Android client the linked server is offering, if any. /// /// `None` covers two different-looking situations that are one answer to the @@ -592,6 +585,11 @@ impl Inkwell { .map_err(CoreError::network) } + /// Run one full sync: push local changes, then pull the server's. + /// + /// The only sync entry point, on purpose. Push and pull exist separately inside + /// the core, but offering a bare "pull" would let the UI overwrite unsent local + /// edits — the ordering isn't a suggestion, it's what keeps them. pub async fn sync_now(&self) -> Result { let (base_url, token) = self.credentials()?; engine::run_cycle(&self.db, &self.blobs, &base_url, &token) diff --git a/android/ffi/src/models.rs b/android/ffi/src/models.rs index d33aa9b..a59446a 100644 --- a/android/ffi/src/models.rs +++ b/android/ffi/src/models.rs @@ -358,7 +358,8 @@ impl From for Label { /// What the board is asking for. Mirrors the core's `ListQuery`. #[derive(Debug, Clone, uniffi::Record)] pub struct NoteQuery { - /// "notes" | "archive" | "trash" | "reminders" | "labels" — the core validates. + /// "trash" | "archived"; anything else is the board. The core doesn't reject an + /// unknown one, so a typo shows the board rather than failing. pub view: String, pub label_id: Option, pub sort: Option, diff --git a/core/src/local/schema.rs b/core/src/local/schema.rs index 176eacd..1b7a246 100644 --- a/core/src/local/schema.rs +++ b/core/src/local/schema.rs @@ -1,6 +1,6 @@ //! Local SQLite schema + migrations. The schema mirrors the note/label model so an //! offline note can later sync 1:1 with the server. Each syncable row carries local -//! `sync_revision` + `dirty` bookkeeping (consumed by the sync engine in M10.7); +//! `sync_revision` + `dirty` bookkeeping (consumed by `sync::push` and `sync::pull`); //! `#tags` are NOT stored as such (derived at query time into labels), matching //! docs/sync.md. //! @@ -250,7 +250,6 @@ fn migrate_v8(conn: &Connection) -> rusqlite::Result<()> { Ok(()) } -/// Bring the database up to the latest schema. Idempotent. // v9 (M315): `notes.color` is gone. A card is one neutral surface now and colour lives // only on a tag, so the column was written by a picker nothing read and read by nothing // at all. `labels.color` is untouched — that is the colour that survived. @@ -356,6 +355,7 @@ const STEPS: &[Step] = &[ Step::Sql(SCHEMA_V13), ]; +/// Bring the database up to the latest schema. Idempotent. pub fn migrate(conn: &Connection) -> rusqlite::Result<()> { conn.execute_batch("PRAGMA foreign_keys = ON;")?; let version: i64 = conn.query_row("PRAGMA user_version", [], |r| r.get(0))?; diff --git a/core/src/sync/blobs.rs b/core/src/sync/blobs.rs index 57675b7..4bd4197 100644 --- a/core/src/sync/blobs.rs +++ b/core/src/sync/blobs.rs @@ -223,7 +223,7 @@ pub(crate) fn scratch(tag: &str) -> BlobStore { use std::sync::atomic::{AtomicU32, Ordering}; static SEQ: AtomicU32 = AtomicU32::new(0); let n = SEQ.fetch_add(1, Ordering::Relaxed); - let dir = std::env::temp_dir().join(format!("ts-blobs-{}-{n}-{tag}", std::process::id())); + let dir = std::env::temp_dir().join(format!("inkwell-blobs-{}-{n}-{tag}", std::process::id())); let _ = fs::remove_dir_all(&dir); BlobStore::new(dir).expect("scratch blob store") } @@ -326,7 +326,7 @@ mod tests { #[test] fn serving_refuses_a_path_that_isnt_a_hash() { // Delegated to `path`, so the traversal guard is the same one `store` uses. - publish_root(std::env::temp_dir().join("ts-blobs-serve-guard")); + publish_root(std::env::temp_dir().join("inkwell-blobs-serve-guard")); let (status, _, body) = serve("/../../etc/passwd", None); assert_eq!(status, 404); assert!(body.is_empty()); diff --git a/core/src/sync/client.rs b/core/src/sync/client.rs index 949fb69..39bedf2 100644 --- a/core/src/sync/client.rs +++ b/core/src/sync/client.rs @@ -594,75 +594,6 @@ fn describe_transport_error(base_url: &str, err: &reqwest::Error) -> String { } } -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn a_share_names_a_person_or_a_group() { - assert_eq!( - ShareTarget::Member("u1".into()).body("view"), - serde_json::json!({ "user_id": "u1", "permission": "view" }) - ); - assert_eq!( - ShareTarget::Group("g1".into()).body("edit"), - serde_json::json!({ "group_id": "g1", "permission": "edit" }) - ); - let raw = r#"{"id":"s2","member":null, - "group":{"id":"g1","name":"Family","member_count":3}, - "permission":"edit"}"#; - let share: NoteShare = serde_json::from_str(raw).expect("a group share reads"); - assert!(share.member.is_none()); - assert_eq!(share.group.expect("group").member_count, 3); - // A server from before groups sends no `groups` at all. - let older: Directory = serde_json::from_str(r#"{"members":[]}"#).expect("directory"); - assert!(older.groups.is_empty()); - } - - #[test] - fn urls_join_without_doubling_slashes() { - // normalize_base_url has already stripped any trailing slash, so plain - // concatenation is correct — this pins that assumption. - assert_eq!( - config_url("https://notes.example.com"), - "https://notes.example.com/api/config" - ); - assert_eq!( - device_login_url("https://notes.example.com"), - "https://notes.example.com/api/auth/device-login" - ); - assert_eq!( - me_url("https://notes.example.com"), - "https://notes.example.com/api/auth/me" - ); - assert_eq!( - revoke_self_url("https://notes.example.com"), - "https://notes.example.com/api/auth/devices/self" - ); - } - - #[test] - fn revoke_outcome_serializes_tagged_for_the_frontend() { - // The UI decides between "signed out on the server" and "still valid, go - // revoke it" by reading this tag, so its shape is part of the contract. - let json = serde_json::to_string(&RevokeOutcome::Failed { - reason: "offline".into(), - }) - .expect("outcome serializes"); - assert!(json.contains("\"status\":\"failed\""), "got {json}"); - let json = serde_json::to_string(&RevokeOutcome::Revoked).expect("outcome serializes"); - assert!(json.contains("\"status\":\"revoked\""), "got {json}"); - } - - #[test] - fn urls_preserve_a_port_and_subpath() { - assert_eq!( - config_url("http://192.168.1.10:8000/inkwell"), - "http://192.168.1.10:8000/inkwell/api/config" - ); - } -} - /// The Android client a linked server can hand out. /// /// Mirrors `/api/client/android` (see the server's `client_dist.py`). Absent there @@ -782,3 +713,72 @@ pub async fn download_client( std::fs::rename(&partial, dest) .map_err(|e| format!("Couldn't put the downloaded update in place: {e}")) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn a_share_names_a_person_or_a_group() { + assert_eq!( + ShareTarget::Member("u1".into()).body("view"), + serde_json::json!({ "user_id": "u1", "permission": "view" }) + ); + assert_eq!( + ShareTarget::Group("g1".into()).body("edit"), + serde_json::json!({ "group_id": "g1", "permission": "edit" }) + ); + let raw = r#"{"id":"s2","member":null, + "group":{"id":"g1","name":"Family","member_count":3}, + "permission":"edit"}"#; + let share: NoteShare = serde_json::from_str(raw).expect("a group share reads"); + assert!(share.member.is_none()); + assert_eq!(share.group.expect("group").member_count, 3); + // A server from before groups sends no `groups` at all. + let older: Directory = serde_json::from_str(r#"{"members":[]}"#).expect("directory"); + assert!(older.groups.is_empty()); + } + + #[test] + fn urls_join_without_doubling_slashes() { + // normalize_base_url has already stripped any trailing slash, so plain + // concatenation is correct — this pins that assumption. + assert_eq!( + config_url("https://notes.example.com"), + "https://notes.example.com/api/config" + ); + assert_eq!( + device_login_url("https://notes.example.com"), + "https://notes.example.com/api/auth/device-login" + ); + assert_eq!( + me_url("https://notes.example.com"), + "https://notes.example.com/api/auth/me" + ); + assert_eq!( + revoke_self_url("https://notes.example.com"), + "https://notes.example.com/api/auth/devices/self" + ); + } + + #[test] + fn revoke_outcome_serializes_tagged_for_the_frontend() { + // The UI decides between "signed out on the server" and "still valid, go + // revoke it" by reading this tag, so its shape is part of the contract. + let json = serde_json::to_string(&RevokeOutcome::Failed { + reason: "offline".into(), + }) + .expect("outcome serializes"); + assert!(json.contains("\"status\":\"failed\""), "got {json}"); + let json = serde_json::to_string(&RevokeOutcome::Revoked).expect("outcome serializes"); + assert!(json.contains("\"status\":\"revoked\""), "got {json}"); + } + + #[test] + fn urls_preserve_a_port_and_subpath() { + assert_eq!( + config_url("http://192.168.1.10:8000/inkwell"), + "http://192.168.1.10:8000/inkwell/api/config" + ); + } +} diff --git a/core/src/sync/mod.rs b/core/src/sync/mod.rs index 064908d..99df420 100644 --- a/core/src/sync/mod.rs +++ b/core/src/sync/mod.rs @@ -8,6 +8,10 @@ //! - `client` — HTTP transport: the handshake call and device-token auth. //! - `state` — the persisted link record (server, token, change-feed cursor). //! - `engine` — one full cycle: push local changes, then pull the server's. +//! - `link` — linking a device to a server and unlinking it, one flow for every client. +//! - `pull` / `push` — the two halves of that cycle. +//! - `wire` — the delta-feed JSON shapes, as the server sends them. +//! - `blobs` — attachment bytes on disk, filed under their own sha256. //! - `sharing` — the Share dialog's calls, straight to the server (#5175). //! //! The UI surface that drives this lives in whichever client is wrapping the crate, diff --git a/desktop/src-tauri/src/crossover.rs b/desktop/src-tauri/src/crossover.rs index 7fc330a..c3d33c2 100644 --- a/desktop/src-tauri/src/crossover.rs +++ b/desktop/src-tauri/src/crossover.rs @@ -90,7 +90,8 @@ mod tests { fn root() -> (PathBuf, PathBuf) { static SEQ: AtomicU32 = AtomicU32::new(0); let n = SEQ.fetch_add(1, Ordering::Relaxed); - let root = std::env::temp_dir().join(format!("iw-crossover-{}-{n}", std::process::id())); + let root = + std::env::temp_dir().join(format!("inkwell-crossover-{}-{n}", std::process::id())); (root.join("old"), root.join("new")) } diff --git a/desktop/src-tauri/src/update.rs b/desktop/src-tauri/src/update.rs index 9a558ad..7274ef5 100644 --- a/desktop/src-tauri/src/update.rs +++ b/desktop/src-tauri/src/update.rs @@ -558,7 +558,7 @@ mod tests { fn scratch_path(tag: &str) -> std::path::PathBuf { static SEQ: AtomicU32 = AtomicU32::new(0); let n = SEQ.fetch_add(1, Ordering::Relaxed); - std::env::temp_dir().join(format!("ts-{tag}-{}-{n}", std::process::id())) + std::env::temp_dir().join(format!("inkwell-{tag}-{}-{n}", std::process::id())) } /// The channel `update_check` would actually use.