From 36bb77c299a9ac15d978f3b49ee4a7315a393743 Mon Sep 17 00:00:00 2001 From: Harald Date: Sat, 14 Nov 2026 16:46:06 +0100 Subject: [PATCH] fix(storage): keep a page changed by another device when saving over it A change from another device could reach the page file while a stroke was being drawn; the check that takes it over waits until everything is saved, so the next autosave overwrote it. save_page now notices that the file is not what this process last read or wrote: their version is kept as a conflict copy (which becomes a page of its own), or simply stays if there is nothing new to save here. Co-Authored-By: Claude Fable 5.2 --- README.de.md | 2 +- README.md | 2 +- docs/FILE_FORMAT.de.md | 1 + docs/FILE_FORMAT.md | 1 + src/disk_watch.rs | 96 ++++++++++++++++++++++++++++++++++++++++++ src/storage.rs | 96 +++++++++++++++++++++++++++++++++++++----- 6 files changed, 186 insertions(+), 12 deletions(-) diff --git a/README.de.md b/README.de.md index a2fa30b..d74f13e 100644 --- a/README.de.md +++ b/README.de.md @@ -73,7 +73,7 @@ Die Oberfläche gibt es auf Deutsch und Englisch; ink-notes folgt der Systemspra - Jede Datei wird erst vollständig in eine temporäre Datei geschrieben und dann ersetzt – ein Absturz beim Speichern hinterlässt keine halbe Datei. - **Beim Schließen** (Fenster oder `Strg + Q`) und beim Wechsel des Notizbuchs wird alles Ausstehende geschrieben, bevor es weitergeht. Schlägt das fehl, bleibt die App offen und bietet Wiederholen oder eine Notfall-Sicherung an. - `Strg + S` speichert sofort. Der Punkt am ☰-Knopf zeigt den Stand: grün = gespeichert, orange = Änderungen warten, gelb = speichert gerade, rot = Fehler. -- **Speicherort** (☰-Menü, „Speicherort“): Liegen die Notizbücher in einem Ordner, den **Nextcloud, Google Drive, Syncthing oder Dropbox** abgleichen, sind sie auf allen Geräten verfügbar. Die Notizbücher liegen im Unterordner `ink-notes`; ist er noch leer, werden die vorhandenen Notizbücher dorthin kopiert (die alten bleiben erhalten). Änderungen von einem anderen Gerät erscheinen nach wenigen Sekunden ohne neu zu öffnen – aber nur, solange lokal alles gespeichert ist, damit nichts überschrieben wird. Legt der Sync-Client eine Konfliktkopie einer Seite an, wird sie als eigene Seite („Konfliktkopie: …“) übernommen statt verloren zu gehen. Ist ein Notizbuch gerade auf einem anderen Gerät geöffnet, warnt die App beim Öffnen. +- **Speicherort** (☰-Menü, „Speicherort“): Liegen die Notizbücher in einem Ordner, den **Nextcloud, Google Drive, Syncthing oder Dropbox** abgleichen, sind sie auf allen Geräten verfügbar. Die Notizbücher liegen im Unterordner `ink-notes`; ist er noch leer, werden die vorhandenen Notizbücher dorthin kopiert (die alten bleiben erhalten). Änderungen von einem anderen Gerät erscheinen nach wenigen Sekunden ohne neu zu öffnen – aber nur, solange lokal alles gespeichert ist, damit nichts überschrieben wird. Legt der Sync-Client eine Konfliktkopie einer Seite an, wird sie als eigene Seite („Konfliktkopie: …“) übernommen statt verloren zu gehen. Dasselbe passiert, wenn eine Änderung genau dann eintrifft, während du auf dieser Seite schreibst: Deine Fassung wird gespeichert, die andere bleibt als Konfliktkopie erhalten. Ist ein Notizbuch gerade auf einem anderen Gerät geöffnet, warnt die App beim Öffnen. - **Nextcloud eingebaut** (☰-Menü, „Nextcloud“; auch unter Android und ohne Sync-Client): Adresse der eigenen Nextcloud eingeben – auch ein privater Server, in einem Unterordner (`example.org/nextcloud`) oder mit eigenem Port. Die Anmeldung läuft im Browser (auch mit Zwei-Faktor-Anmeldung); die App bekommt ein eigenes App-Passwort, das sich in Nextcloud unter Einstellungen → Sicherheit jederzeit widerrufen lässt, und sieht das echte Passwort nie. Die Notizbücher werden alle 30 Sekunden und kurz nach dem Speichern mit dem Ordner `ink-notes` in den Nextcloud-Dateien abgeglichen; offline arbeitet die App normal weiter. Wurde eine Seite auf zwei Geräten geändert, bleiben beide Fassungen erhalten (die andere als eigene Seite); gelöschte Notizbücher und Seiten landen im Papierkorb (lokal `.trash`, auf dem Server dessen Papierkorb). Einem selbst signierten Zertifikat vertraut die App, nachdem du seinen Fingerabdruck bestätigt hast; Adressen mit `http://` gehen nur mit Warnung, weil das App-Passwort dann im Netz lesbar wäre. „Trennen“ beendet den Abgleich und widerruft das App-Passwort; die Notizbücher bleiben erhalten. Bitte entweder den eingebauten Abgleich oder den Ordner eines Sync-Clients als Speicherort verwenden, nicht beides für dieselben Notizbücher. ### Export diff --git a/README.md b/README.md index 4f5687e..e04bb68 100644 --- a/README.md +++ b/README.md @@ -73,7 +73,7 @@ The interface is available in English and German; ink-notes follows the system l - Every file is first written completely to a temporary file and then replaced – a crash while saving never leaves a half-written file. - **When closing** (window or `Ctrl + Q`) and when switching notebooks, everything pending is written before continuing. If that fails, the app stays open and offers to retry or to make an emergency backup. - `Ctrl + S` saves immediately. The dot on the ☰ button shows the state: green = saved, orange = changes pending, yellow = saving, red = error. -- **Storage location** (☰ menu, "Storage location"): put the notebooks into a folder that **Nextcloud, Google Drive, Syncthing or Dropbox** keep in sync, and they are available on all your devices. The notebooks are kept in the subfolder `ink-notes`; if it is still empty, the existing notebooks are copied there (the old ones stay). Changes arriving from another device show up within a few seconds without reopening – but only while everything local is saved, so nothing gets overwritten. If the sync client makes a conflict copy of a page, it is added as a page of its own („Conflict copy: …“) instead of getting lost. Opening a notebook that is open on another device shows a warning. +- **Storage location** (☰ menu, "Storage location"): put the notebooks into a folder that **Nextcloud, Google Drive, Syncthing or Dropbox** keep in sync, and they are available on all your devices. The notebooks are kept in the subfolder `ink-notes`; if it is still empty, the existing notebooks are copied there (the old ones stay). Changes arriving from another device show up within a few seconds without reopening – but only while everything local is saved, so nothing gets overwritten. If the sync client makes a conflict copy of a page, it is added as a page of its own („Conflict copy: …“) instead of getting lost. The same happens when a change arrives just while you are writing on that page: your version is saved and theirs is kept as a conflict copy. Opening a notebook that is open on another device shows a warning. - **Nextcloud built in** (☰ menu, "Nextcloud"; also on Android and without a sync client): enter the address of your Nextcloud – also a private server, in a subfolder (`example.org/nextcloud`) or with its own port. Signing in happens in the browser (also with two-factor sign-in); the app gets an app password of its own, which you can revoke in Nextcloud under Settings → Security at any time, and never sees your real password. The notebooks are synced with the folder `ink-notes` in your Nextcloud files every 30 seconds and shortly after saving; the app keeps working offline. If a page was changed on two devices, both versions are kept (the other one as a page of its own); deleted notebooks and pages go into the trash (locally `.trash`, on the server its trash bin). A self-signed certificate is trusted once you have confirmed its fingerprint; addresses with `http://` only work with a warning, since the app password would be readable on the network. "Disconnect" stops syncing and revokes the app password; the notebooks stay. Use either the built-in sync or a sync client's folder as storage location, not both for the same notebooks. ### Export diff --git a/docs/FILE_FORMAT.de.md b/docs/FILE_FORMAT.de.md index 7160cb0..b04f184 100644 --- a/docs/FILE_FORMAT.de.md +++ b/docs/FILE_FORMAT.de.md @@ -177,6 +177,7 @@ Liegen die Notizbücher in einem Ordner, den Nextcloud, Google Drive, Syncthing - `.opened_by.json` (`{"device": "", "since": }`) wird beim Öffnen eines Notizbuchs geschrieben und beim Schließen entfernt. Eine Markierung eines anderen Geräts führt zu einer Warnung; die Sperrdatei `.lock` schützt nur vor einer zweiten Instanz auf demselben Rechner. - Solange alle lokalen Änderungen gespeichert sind, prüft die App den Ordner alle 3 Sekunden (und wenn das Fenster den Fokus bekommt). Geänderte Seitendateien werden neu geladen, ein geändertes `manifest.json` bringt neue, entfernte oder umsortierte Seiten und Titel mit. - Konfliktkopien der Sync-Clients (`page_ (….json`, `page__….json`, `page_.sync-conflict-….json`) werden als neue Seiten hinter dem Original eingefügt und in `.imported` umbenannt. +- Bevor die App eine Seitendatei überschreibt, die sich seit ihrem letzten Lesen oder Schreiben geändert hat, sichert sie den vorgefundenen Inhalt als `page_ (conflict ).json` – außer sie hat selbst nichts geändert (dann bleibt die Datei, wie sie ist). ### Neuere Versionen Dateien mit einer höheren `format_version` als der unterstützten werden nicht geöffnet – so kann eine ältere App-Version keine Inhalte verlieren, die sie nicht kennt. diff --git a/docs/FILE_FORMAT.md b/docs/FILE_FORMAT.md index 539b330..f83eda2 100644 --- a/docs/FILE_FORMAT.md +++ b/docs/FILE_FORMAT.md @@ -177,6 +177,7 @@ If the notebooks are kept in a folder synced by Nextcloud, Google Drive, Syncthi - `.opened_by.json` (`{"device": "", "since": }`) is written when a notebook is opened and removed when it is closed. A marker from another device leads to a warning; the lock file `.lock` only protects against a second instance on the same computer. - While all local changes are saved, the app checks the folder every 3 seconds (and when the window gets the focus). Changed page files are reloaded, a changed `manifest.json` brings in new, removed or reordered pages and titles. - Conflict copies of sync clients (`page_ (….json`, `page__….json`, `page_.sync-conflict-….json`) are added as new pages after the original and renamed to `.imported`. +- Before the app overwrites a page file that changed since it last read or wrote it, it keeps the content found there as `page_ (conflict ).json`, unless it has no changes of its own (then the file is left as it is). ### Newer versions Files with a higher `format_version` than supported are not opened – this way an older app version cannot lose content it does not know. diff --git a/src/disk_watch.rs b/src/disk_watch.rs index 9ad5a8d..8397456 100644 --- a/src/disk_watch.rs +++ b/src/disk_watch.rs @@ -461,6 +461,102 @@ mod tests { assert!(outcome.is_empty(), "{outcome:?}"); } + /// Writes the page file the way a sync client does: behind the back of `storage`. + fn write_from_another_device(dir: &Path, page: &Page) { + let bytes = serde_json::to_vec(&PageData::from(page)).unwrap(); + fs::write(dir.join(format!("page_{}.json", page.id)), bytes).unwrap(); + } + + fn conflict_copies(dir: &Path) -> Vec { + fs::read_dir(dir) + .unwrap() + .flatten() + .map(|e| e.file_name().to_string_lossy().into_owned()) + .filter(|n| conflict_copy_of(n).is_some()) + .collect() + } + + #[test] + fn test_saving_over_a_change_from_another_device_keeps_both() { + let dir = tempfile::tempdir().unwrap(); + let (mut notebook, mut manifest) = saved_notebook(dir.path()); + let now = Instant::now(); + let mut watch = DiskWatch::new(dir.path().to_path_buf(), now); + + // Their stroke arrives while a stroke is being drawn here, so the check has to wait + let mut theirs = notebook.current_page().clone(); + theirs.add_stroke(stroke(5, 300.0)); + write_from_another_device(dir.path(), &theirs); + + // The stroke is finished and saved before the check ran + notebook.current_page_mut().add_stroke(stroke(2, 200.0)); + notebook.next_stroke_id = 3; + storage::save_page(dir.path(), &PageData::from(notebook.current_page())).unwrap(); + + // Ours is in the page file, theirs next to it + let saved = storage::load_page(dir.path(), 1).unwrap(); + let ids: Vec = saved.strokes.iter().map(|s| s.id).collect(); + assert_eq!(ids, vec![1, 2]); + assert_eq!(conflict_copies(dir.path()).len(), 1); + + // The check turns their version into a page of its own + let outcome = watch.check(&mut notebook, &mut manifest, now); + assert_eq!(outcome.conflict_pages.len(), 1); + assert_eq!(notebook.pages.len(), 2); + assert_eq!(notebook.pages[0].strokes.len(), 2); + assert_eq!(notebook.pages[1].strokes.len(), 2); + assert!(conflict_copies(dir.path()).is_empty()); + + // Saving again is an ordinary save + notebook.pages[0].add_stroke(stroke(9, 500.0)); + storage::save_page(dir.path(), &PageData::from(¬ebook.pages[0])).unwrap(); + assert!(conflict_copies(dir.path()).is_empty()); + } + + #[test] + fn test_saving_an_unchanged_page_keeps_the_change_from_another_device() { + let dir = tempfile::tempdir().unwrap(); + let (mut notebook, mut manifest) = saved_notebook(dir.path()); + let now = Instant::now(); + let mut watch = DiskWatch::new(dir.path().to_path_buf(), now); + + let mut theirs = notebook.current_page().clone(); + theirs.add_stroke(stroke(5, 300.0)); + write_from_another_device(dir.path(), &theirs); + + // E.g. turning pages marks the page for saving although nothing changed on it + storage::save_page(dir.path(), &PageData::from(notebook.current_page())).unwrap(); + assert!(conflict_copies(dir.path()).is_empty()); + + let outcome = watch.check(&mut notebook, &mut manifest, now); + assert_eq!(outcome.reloaded_pages, 1); + assert_eq!(notebook.pages.len(), 1); + assert_eq!(notebook.pages[0].strokes.len(), 2); + } + + #[test] + fn test_saving_a_new_page_over_one_from_another_device_keeps_both() { + let dir = tempfile::tempdir().unwrap(); + let (mut notebook, _) = saved_notebook(dir.path()); + + // Both devices add a second page; it gets the same id on both + let index = notebook.add_page(PageBackground::Blank); + let mut theirs = notebook.pages[index].clone(); + theirs.add_stroke(stroke(7, 300.0)); + write_from_another_device(dir.path(), &theirs); + + notebook.pages[index].add_stroke(stroke(8, 400.0)); + storage::save_page(dir.path(), &PageData::from(¬ebook.pages[index])).unwrap(); + assert_eq!(conflict_copies(dir.path()).len(), 1); + + // A file never read here but with the content to be saved is no conflict + let index = notebook.add_page(PageBackground::Blank); + notebook.pages[index].add_stroke(stroke(9, 300.0)); + write_from_another_device(dir.path(), ¬ebook.pages[index]); + storage::save_page(dir.path(), &PageData::from(¬ebook.pages[index])).unwrap(); + assert_eq!(conflict_copies(dir.path()).len(), 1); + } + #[test] fn test_page_still_being_synced_is_tried_again() { let dir = tempfile::tempdir().unwrap(); diff --git a/src/storage.rs b/src/storage.rs index 951be91..5457d5d 100644 --- a/src/storage.rs +++ b/src/storage.rs @@ -721,21 +721,96 @@ pub fn quarantine_corrupted_page(notebook_dir: &Path, page_id: u64) -> Result.json`. +/// +/// The file may have been changed behind our back since this process last read or wrote it: +/// by the built-in sync or a sync client that brought a change from another device, before +/// `disk_watch` got to take it over (it waits while something is being drawn or is unsaved). +/// Overwriting it would silently drop that change. So, as the sync does for a page changed on +/// both sides, their version is kept as a conflict copy next to the page (`disk_watch` turns +/// it into a page of its own); if we have nothing new ourselves, their version simply stays. pub fn save_page(notebook_dir: &Path, page: &PageData) -> Result<(), StorageError> { let path = notebook_dir.join(format!("page_{}.json", page.id)); - if path.exists() { - let bak_path = notebook_dir.join(format!("page_{}.json.v0.bak", page.id)); - if !bak_path.exists() { - if let Ok(content) = fs::read_to_string(&path) { - if let Ok(old_p) = serde_json::from_str::(&content) { - if old_p.format_version == 0 { - let _ = fs::copy(&path, &bak_path); - } - } + let bytes = serde_json::to_vec(page)?; + let ours = file_hash(&bytes); + let key = seen_key(&path); + + if let Ok(theirs) = fs::read(&path) { + let seen = seen_files().get(&key).copied(); + let their_page = serde_json::from_slice::(&theirs).ok(); + if seen != Some(file_hash(&theirs)) && theirs != bytes { + if seen == Some(ours) { + // Nothing changed here since we last read or wrote the page + return Ok(()); + } + let differs = their_page.as_ref().is_some_and(|their_page| { + let mut their_page = their_page.clone(); + their_page.format_version = page.format_version; + their_page != *page + }); + if differs { + // Fails rather than overwrites: the caller tries again + write_conflict_copy(notebook_dir, page.id, &theirs)?; } } + let bak_path = notebook_dir.join(format!("page_{}.json.v0.bak", page.id)); + if !bak_path.exists() && their_page.is_some_and(|p| p.format_version == 0) { + let _ = fs::copy(&path, &bak_path); + } } - atomic_write_json(&path, page) + atomic_write_file(&path, &bytes)?; + seen_files().insert(key, ours); + Ok(()) +} + +type FileHash = [u8; 32]; + +fn file_hash(bytes: &[u8]) -> FileHash { + use sha2::Digest; + sha2::Sha256::digest(bytes).into() +} + +/// Content of the page files as this process last read or wrote them (see [`save_page`]). +fn seen_files() -> std::sync::MutexGuard<'static, std::collections::HashMap> { + static SEEN: std::sync::OnceLock< + std::sync::Mutex>, + > = std::sync::OnceLock::new(); + SEEN.get_or_init(Default::default) + .lock() + .unwrap_or_else(|e| e.into_inner()) +} + +/// The same file must get the same key however its folder is spelled. +fn seen_key(path: &Path) -> PathBuf { + match (path.parent().map(Path::canonicalize), path.file_name()) { + (Some(Ok(dir)), Some(name)) => dir.join(name), + _ => path.to_path_buf(), + } +} + +/// Keeps `content` as `page_ (conflict ).json`, the name the sync uses too. +fn write_conflict_copy(notebook_dir: &Path, page_id: u64, content: &[u8]) -> io::Result { + let ts = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap_or_default() + .as_secs(); + let mut n = 0; + let path = loop { + let name = match n { + 0 => format!("page_{page_id} (conflict {ts}).json"), + n => format!("page_{page_id} (conflict {ts}-{n}).json"), + }; + let path = notebook_dir.join(name); + if !path.exists() { + break path; + } + n += 1; + }; + atomic_write_file(&path, content)?; + eprintln!( + "[Storage] Seite {page_id} wurde außerhalb geändert, Konfliktkopie: {}", + path.display() + ); + Ok(path) } /// Lädt eine Seite aus `page_.json`. @@ -767,6 +842,7 @@ pub fn load_page(notebook_dir: &Path, page_id: u64) -> Result