From 3c2055b50e4d0f2e7f757ccf17613a5b3cffee40 Mon Sep 17 00:00:00 2001 From: vcoppe Date: Wed, 14 Oct 2026 08:07:58 +0200 Subject: [PATCH] improve flow --- gpx-rs/engine/src/engine/command/apply.rs | 3 - gpx-rs/engine/src/engine/mod.rs | 17 +---- gpx-rs/engine/src/engine/tests.rs | 33 ---------- gpx-rs/engine/src/storage/persister.rs | 10 ++- gpx-rs/wasm/src/persistence.rs | 80 ++++++++++++++++------- website/src/lib/engine/engine.ts | 11 +--- website/src/lib/engine/storage.ts | 16 ++++- 7 files changed, 80 insertions(+), 90 deletions(-) diff --git a/gpx-rs/engine/src/engine/command/apply.rs b/gpx-rs/engine/src/engine/command/apply.rs index 60381092b..108a0b9e9 100644 --- a/gpx-rs/engine/src/engine/command/apply.rs +++ b/gpx-rs/engine/src/engine/command/apply.rs @@ -16,8 +16,6 @@ pub enum CommandError { NothingToDo, /// The input data could not be parsed. InvalidData(String), - /// The engine is in the outdated state. - Outdated, } impl fmt::Display for CommandError { @@ -25,7 +23,6 @@ impl fmt::Display for CommandError { match self { Self::NothingToDo => write!(f, "nothing to do"), Self::InvalidData(err) => write!(f, "invalid data: {err}"), - Self::Outdated => write!(f, "saved by a newer version"), } } } diff --git a/gpx-rs/engine/src/engine/mod.rs b/gpx-rs/engine/src/engine/mod.rs index f23ddb869..bd75da891 100644 --- a/gpx-rs/engine/src/engine/mod.rs +++ b/gpx-rs/engine/src/engine/mod.rs @@ -27,8 +27,6 @@ pub struct Engine { routing_buffer: RoutingBuffer, categories: TrackpointCategories, clipboard: Option, - /// Whether what is stored was written by a newer version (see [`Engine::mark_outdated`]). - outdated: bool, } impl Engine { @@ -137,7 +135,6 @@ impl Engine { /// from the ones that were here are reported as added, removed or modified (all the files /// that were here, as they are not the same objects), and the order as changed if it is. pub fn restore(&mut self, restored: Restored) -> Outcome { - self.outdated = false; let selection_before = self.selection.clone(); let order_before = self.order.0.clone(); let before = self.stack.current().cloned(); @@ -166,14 +163,6 @@ impl Engine { } } - /// Tells that what is stored was written by a newer version of the app, which cannot be read - /// (see [`crate::StorageError::Newer`]). The files are left as they are, but cannot be changed - /// from now on, as the changes could not be kept: [`Engine::execute`] refuses with - /// [`CommandError::Outdated`], until something that is not newer is restored. - pub fn mark_outdated(&mut self) { - self.outdated = true; - } - /// A file as GPX (UTF-8), `None` if it does not exist. pub fn export(&self, id: &FileId, options: ExportOptions) -> Option> { let file = self.stack.current()?.get(id)?; @@ -218,12 +207,8 @@ impl Engine { } /// Single entry point for every action coming from the frontend. Says what it changed, or why - /// it did nothing: an edit that fails or has nothing to do, or any change of the files while - /// they are outdated (see [`Engine::is_outdated`]). + /// it did nothing: an edit that fails or has nothing to do. pub fn execute(&mut self, action: Action) -> Result { - if self.outdated { - return Err(CommandError::Outdated); - } let selection_before = self.selection.clone(); let order_before = self.order.0.clone(); let clipboard_before = self.clipboard.clone(); diff --git a/gpx-rs/engine/src/engine/tests.rs b/gpx-rs/engine/src/engine/tests.rs index 48d99e490..3e6cc26dc 100644 --- a/gpx-rs/engine/src/engine/tests.rs +++ b/gpx-rs/engine/src/engine/tests.rs @@ -1434,36 +1434,3 @@ fn test_overwriting_forgets_the_history() { engine.restore(restored_from(&engine, &ids, vec![])); assert!(!engine.can_undo()); } - -#[test] -fn test_files_cannot_be_changed_when_what_is_stored_is_from_a_newer_version() { - let mut engine = TestEngine::default(); - new(&mut engine, "file"); - let id = engine.order()[0]; - - engine.mark_outdated(); - - for action in [ - Action::Edit(Command::Reverse(crate::Reverse)), - Action::Undo, - Action::Redo, - Action::Reorder { - file_ids: vec![id], - index: 0, - }, - ] { - assert_eq!(engine.execute(action), Err(CommandError::Outdated)); - } - assert_eq!(engine.order(), [id]); - - // so is anything else, the selection included - let result = engine.execute(Action::Select { - selection: Selection::Empty, - mode: SelectMode::Replace, - }); - assert_eq!(result, Err(CommandError::Outdated)); - - // reading a storage that is not newer lifts it - engine.restore(restored_from(&engine, &[id], vec![])); - assert_ne!(engine.execute(Action::Undo), Err(CommandError::Outdated)); -} diff --git a/gpx-rs/engine/src/storage/persister.rs b/gpx-rs/engine/src/storage/persister.rs index 4992cc0a8..6177b5570 100644 --- a/gpx-rs/engine/src/storage/persister.rs +++ b/gpx-rs/engine/src/storage/persister.rs @@ -171,6 +171,9 @@ impl Persister { /// Writes what changed since the last save. Nothing is written if nothing changed. /// + /// Nothing is written while what is stored comes from a newer version (`Failed(Newer)`, see + /// [`Persister::restore`]). + /// /// The write only happens if nothing else was committed since the persister read or wrote /// last: otherwise it is refused (`Stale`), nothing is written, and the persister has to read /// the storage again ([`Persister::restore`]) before it can write. @@ -180,7 +183,7 @@ impl Persister { snapshot: Snapshot, ) -> Result<(), CommitError> { if self.read_only { - return Ok(()); + return Err(StorageError::Newer.into()); } let mut plan = self.plan(snapshot); if plan.batch.is_empty() { @@ -525,11 +528,12 @@ mod tests { let result = block_on(persister.restore(&storage)); assert_eq!(result.unwrap_err(), StorageError::Newer, "{what}"); - // whatever is saved afterwards, nothing is written: the newer version needs its data + // whatever is saved afterwards is refused: the newer version needs its data let mut changed = snapshot.clone(); changed.order.clear(); changed.files.clear(); - block_on(persister.save(&storage, changed)).unwrap(); + let result = block_on(persister.save(&storage, changed)); + assert_eq!(result, Err(CommitError::Failed(StorageError::Newer)), "{what}"); assert!(storage.batches.borrow().is_empty(), "{what}"); assert_eq!(storage.inner.counts(), before, "{what}"); } diff --git a/gpx-rs/wasm/src/persistence.rs b/gpx-rs/wasm/src/persistence.rs index 53f996a33..41708b8cf 100644 --- a/gpx-rs/wasm/src/persistence.rs +++ b/gpx-rs/wasm/src/persistence.rs @@ -18,7 +18,8 @@ use crate::ts::{Outcome, StorageOpened}; // wait: `schedule_save` starts a loop that writes what changed since the last write, until // nothing has. When another tab wrote in between, the write is refused (the storage has a newer // head than this tab last saw): the tab reads the storage again and overwrites what it holds, see -// `sync_storage`. The settings are opaque to the engine, the frontend keeps its own copy of them. +// `sync_storage`. When what is stored was written by a newer version of the app, the write is +// refused too, and the frontend is told, so that it makes the user reload the page. The settings are opaque to the engine, the frontend keeps its own copy of them. thread_local! { static STORAGE: RefCell>> = const { RefCell::new(None) }; @@ -28,10 +29,18 @@ thread_local! { /// Resolve functions of the promises of `flush_storage`. /// Called after the files were written, and when a write was refused because they were /// written by someone else since: see `open_storage`. - static LISTENERS: RefCell> = const { RefCell::new(None) }; + static LISTENERS: RefCell> = const { RefCell::new(None) }; static WAITERS: RefCell> = const { RefCell::new(Vec::new()) }; } +/// What the frontend is told: the files were written, a write was refused because another tab +/// wrote since, a write was refused because what is stored comes from a newer version. +struct Listeners { + saved: js_sys::Function, + behind: js_sys::Function, + newer: js_sys::Function, +} + #[wasm_bindgen] extern "C" { #[wasm_bindgen(js_namespace = console, js_name = error)] @@ -52,7 +61,7 @@ pub(crate) fn schedule_save() { return; } wasm_bindgen_futures::spawn_local(async { - let (mut saved, mut behind) = (false, false); + let (mut saved, mut behind, mut newer) = (false, false, false); while DIRTY.replace(false) { let Some(storage) = storage() else { break }; let Some(snapshot) = with_engine(Engine::snapshot) else { @@ -71,16 +80,24 @@ pub(crate) fn schedule_save() { DIRTY.set(false); behind = true; } + // nothing is written, so that the newer version finds its data again + Err(engine::CommitError::Failed(engine::StorageError::Newer)) => { + DIRTY.set(false); + newer = true; + } // what failed is written with the next change Err(error) => console_error(&error.to_string()), } } SAVING.set(false); if saved { - notify(|(saved, _)| saved); + notify(|listeners| &listeners.saved); } if behind { - notify(|(_, behind)| behind); + notify(|listeners| &listeners.behind); + } + if newer { + notify(|listeners| &listeners.newer); } for resolve in WAITERS.with(|waiters| std::mem::take(&mut *waiters.borrow_mut())) { let _ = resolve.call0(&JsValue::UNDEFINED); @@ -88,7 +105,7 @@ pub(crate) fn schedule_save() { }); } -fn notify(pick: impl FnOnce(&(js_sys::Function, js_sys::Function)) -> &js_sys::Function) { +fn notify(pick: impl FnOnce(&Listeners) -> &js_sys::Function) { let callback = LISTENERS.with(|l| l.borrow().as_ref().map(|l| pick(l).clone())); if let Some(callback) = callback { let _ = callback.call0(&JsValue::UNDEFINED); @@ -98,17 +115,19 @@ fn notify(pick: impl FnOnce(&(js_sys::Function, js_sys::Function)) -> &js_sys::F /// Opens the database `name` (it is created if it does not exist), puts the files that it holds /// in the engine, which has to be empty, and returns the settings (JSON strings by key) and the /// outcome of putting the files in. The changes of the files that follow are saved, unless some of -/// what is stored was written by a newer version of the app: then nothing is read, and the files -/// cannot be changed (the outcome has the `Outdated` error), so that the newer version finds its -/// data again. +/// what is stored was written by a newer version of the app: then nothing is read and nothing is +/// ever written, so that the newer version finds its data again. /// -/// `on_saved` is called after the files were written (to tell the other tabs), and `on_behind` when -/// a write was refused because another tab wrote since (call `sync_storage` then). +/// `on_saved` is called after the files were written (to tell the other tabs), `on_behind` when +/// a write was refused because another tab wrote since (call `sync_storage` then), and `on_newer` +/// when what is stored comes from a newer version (now, or when it is read again): the page has to +/// be reloaded, the changes are not kept. #[wasm_bindgen] pub async fn open_storage( name: &str, on_saved: js_sys::Function, on_behind: js_sys::Function, + on_newer: js_sys::Function, ) -> Result { let empty = with_engine(|e| e.snapshot().files.is_empty()).unwrap_or(false); if !empty { @@ -117,11 +136,22 @@ pub async fn open_storage( let storage = IdbStorage::open(name).await.map_err(to_js_error)?; let mut persister = Persister::default(); let outcome = restore_engine(persister.restore(&storage).await).map_err(to_js_error)?; + let newer = outcome.is_none(); + let outcome = outcome.unwrap_or_else(|| outcome_object(&engine::Outcome::default(), None)); let settings = storage.load_settings().await.map_err(to_js_error)?; PERSISTER.with(|slot| *slot.borrow_mut() = Some(persister)); STORAGE.with(|slot| *slot.borrow_mut() = Some(Rc::new(storage))); - LISTENERS.with(|slot| *slot.borrow_mut() = Some((on_saved, on_behind))); + LISTENERS.with(|slot| { + *slot.borrow_mut() = Some(Listeners { + saved: on_saved, + behind: on_behind, + newer: on_newer, + }) + }); + if newer { + notify(|listeners| &listeners.newer); + } let settings_object = Object::new(); for (key, value) in settings { @@ -134,20 +164,19 @@ pub async fn open_storage( } /// Overwrites the files of the engine with what was read, and tells what that did. If what is -/// stored comes from a newer version of the app, the files are left as they are and cannot be -/// changed anymore: the error of the outcome says so, as for the edits that are refused afterwards. +/// stored comes from a newer version of the app, the files are left as they are and `None` is +/// returned: the persister does not write anymore (see `Persister::save`). fn restore_engine( read: Result, -) -> Result { +) -> Result, engine::StorageError> { match read { - Ok(restored) => Ok(match with_engine_mut(|engine| engine.restore(restored)) { - Some(outcome) => outcome_object(&outcome, None), - None => outcome_object(&engine::Outcome::default(), None), - }), - Err(engine::StorageError::Newer) => { - with_engine_mut(Engine::mark_outdated); - Ok(fail(engine::CommandError::Outdated.to_string())) - } + Ok(restored) => Ok(Some( + match with_engine_mut(|engine| engine.restore(restored)) { + Some(outcome) => outcome_object(&outcome, None), + None => outcome_object(&engine::Outcome::default(), None), + }, + )), + Err(engine::StorageError::Newer) => Ok(None), Err(error) => Err(error), } } @@ -191,7 +220,10 @@ pub async fn sync_storage() -> Result { } let outcome = restore_engine(persister.restore(&*storage).await)?; DIRTY.set(false); - Ok(outcome) + if outcome.is_none() { + notify(|listeners| &listeners.newer); + } + Ok(outcome.unwrap_or_else(|| outcome_object(&engine::Outcome::default(), None))) } .await; PERSISTER.with(|slot| *slot.borrow_mut() = Some(persister)); diff --git a/website/src/lib/engine/engine.ts b/website/src/lib/engine/engine.ts index 0b6a99294..ad2032ad4 100644 --- a/website/src/lib/engine/engine.ts +++ b/website/src/lib/engine/engine.ts @@ -33,9 +33,6 @@ import { type WaypointDetails, } from './types'; -/** The error of the outcomes while the stored files come from a newer version of the app. */ -const OUTDATED = 'saved by a newer version'; - /** * Thin wrapper around the WASM engine. * @@ -70,7 +67,7 @@ class Engine { readonly order: Readable = { subscribe: this._order.subscribe }; /** Statistics of what is currently selected. */ readonly statistics: Readable = this._statistics.store; - /** Whether the stored files are from a newer version of the app: they cannot be changed. */ + /** Whether the stored files are from a newer version of the app: changes are not kept, the page has to be reloaded. */ readonly outdated: Readable = { subscribe: this._outdated.subscribe }; readonly canUndo: Readable = { subscribe: this._canUndo.subscribe }; readonly canRedo: Readable = { subscribe: this._canRedo.subscribe }; @@ -116,6 +113,7 @@ class Engine { wasm: this.wasm!, visibility: this._visibility, sync: (update) => this.sync(this.wasm!, update), + outdated: () => this._outdated.set(true), hasFiles: () => get(this._order).length > 0, loadFiles: (files) => this.loadFiles(files), deselect: () => this.select([]), @@ -702,16 +700,13 @@ class Engine { this._lastError.set(outcome.error); if (outcome.error === 'nothing to do') { console.debug('engine:', outcome.error); - } else if (outcome.error !== undefined && outcome.error !== OUTDATED) { + } else if (outcome.error !== undefined) { console.warn('engine:', outcome.error); } } /** Updates what is kept of the engine with what a call changed. */ private sync(wasm: Wasm, update: Outcome) { - if (update.error === OUTDATED) { - this._outdated.set(true); - } this._canUndo.set(wasm.can_undo()); this._canRedo.set(wasm.can_redo()); this._canPaste.set(wasm.can_paste()); diff --git a/website/src/lib/engine/storage.ts b/website/src/lib/engine/storage.ts index 1f0dcc8bf..aab5acf8f 100644 --- a/website/src/lib/engine/storage.ts +++ b/website/src/lib/engine/storage.ts @@ -16,6 +16,8 @@ export type StorageHost = { visibility: Map; /** Applies what a call changed to what the facade keeps of the files. */ sync(update: Outcome): void; + /** Tells that what is stored comes from a newer version of the app: nothing can be saved. */ + outdated(): void; hasFiles(): boolean; loadFiles(files: { data: Uint8Array; name: string }[]): Promise; deselect(): Promise; @@ -27,7 +29,11 @@ export type StorageHost = { * what they hold (see `sync_storage`). The same happens when a write of this tab is refused * because of that, and when the tab is shown again (a hidden tab can miss messages). */ -function followOtherTabs(host: StorageHost): { saved: () => void; behind: () => void } { +function followOtherTabs(host: StorageHost): { + saved: () => void; + behind: () => void; + newer: () => void; +} { const { wasm } = host; let reading = Promise.resolve(); const refresh = () => { @@ -47,7 +53,11 @@ function followOtherTabs(host: StorageHost): { saved: () => void; behind: () => } }); } - return { saved: () => channel?.postMessage('saved'), behind: refresh }; + return { + saved: () => channel?.postMessage('saved'), + behind: refresh, + newer: () => host.outdated(), + }; } /** @@ -61,7 +71,7 @@ export async function openStoredFiles(host: StorageHost): Promise