diff --git a/crates/oa-desktop/src/app/hosted.rs b/crates/oa-desktop/src/app/hosted.rs index a51081c8..37352744 100644 --- a/crates/oa-desktop/src/app/hosted.rs +++ b/crates/oa-desktop/src/app/hosted.rs @@ -255,19 +255,26 @@ impl App { self.hosted.base = base; self.hosted.connection = None; self.hosted.catalog.clear(); - self.hosted.next = None; + self.hosted.invalidate_catalog(); self.hosted.selected = None; } Event::Token(token) => { self.hosted.token = token; self.hosted.connection = None; + self.hosted.catalog.clear(); + self.hosted.selected = None; + self.hosted.invalidate_catalog(); } Event::Query(query) => { - self.hosted.query = query; - self.hosted.next = None; + if self.hosted.query != query { + self.hosted.query = query; + self.hosted.selected = None; + self.hosted.invalidate_catalog(); + } } Event::Connect => match Connection::new(&self.hosted.base, self.hosted.token.clone()) { Ok(connection) => { + self.hosted.invalidate_catalog(); return self.hosted_start(Operation::Catalog(false), connection, None); } Err(error) => { @@ -276,7 +283,8 @@ impl App { } }, Event::More => { - if self.hosted.next.is_some() + if !self.hosted.catalog_stale + && self.hosted.next.is_some() && let Some(connection) = &self.hosted.connection { return self.hosted_start(Operation::Catalog(true), connection.clone(), None); @@ -382,6 +390,32 @@ impl App { } let request = self.hosted.request.take().expect("matching request"); self.busy = false; + // A catalog is a read snapshot, never a live mirror. Known or + // uncertain writes and observed newer revisions invalidate only + // the service whose results are currently displayed. + let catalog_changed = match &result { + Ok(Outcome::Remote(remote)) => { + matches!(request.operation, Operation::Publish | Operation::Save) + || self.hosted.catalog.iter().any(|item| { + item.resource_id == remote.resource_id + && item.revision != remote.revision + }) + } + Err(error) => { + matches!(request.operation, Operation::Publish | Operation::Save) + && (error.uncertain || error.status == Some(412)) + } + Ok(Outcome::Catalog(_)) => false, + }; + if catalog_changed + && self + .hosted + .connection + .as_ref() + .is_some_and(|connection| connection.base() == request.connection.base()) + { + self.hosted.invalidate_catalog(); + } match result { Err(error) => { let conflict = @@ -435,6 +469,7 @@ impl App { } } self.hosted.next = catalog.next_after; + self.hosted.catalog_stale = false; if !self .hosted .catalog diff --git a/crates/oa-desktop/src/app/hosted/tests.rs b/crates/oa-desktop/src/app/hosted/tests.rs index 22a8633f..440e81b6 100644 --- a/crates/oa-desktop/src/app/hosted/tests.rs +++ b/crates/oa-desktop/src/app/hosted/tests.rs @@ -329,7 +329,35 @@ async fn hosted_actual_app_tasks_publish_edit_conflict_correct_credentials_reloa app.sync(); app.hosted.base = base.clone(); app.hosted.token = Secret(token.into()); + dispatch(&mut app, Message::Hosted(Event::Connect)).await; + assert!(!app.hosted.catalog_stale); + assert!(app.hosted.catalog.is_empty()); + assert!( + app.hosted + .catalog_message() + .unwrap() + .starts_with("No models found") + ); dispatch(&mut app, Message::Hosted(Event::Publish)).await; + assert!(app.hosted.catalog_stale); + assert!( + !app.hosted + .catalog_message() + .unwrap() + .starts_with("No models found") + ); + dispatch(&mut app, Message::Hosted(Event::Close)).await; + dispatch(&mut app, Message::Hosted(Event::Show)).await; + assert!(app.hosted.catalog_stale); + assert!( + app.hosted.request.is_none(), + "Showing the dialog does not poll" + ); + dispatch(&mut app, Message::Hosted(Event::Connect)).await; + assert!(!app.hosted.catalog_stale); + assert_eq!(app.hosted.catalog.len(), 1); + assert_eq!(app.hosted.catalog[0].revision, 1); + dispatch(&mut app, Message::Hosted(Event::Close)).await; let session = app .hosted .session @@ -347,6 +375,15 @@ async fn hosted_actual_app_tasks_publish_edit_conflict_correct_credentials_reloa ) .await; dispatch(&mut app, Message::Save(false)).await; + assert!(app.hosted.catalog_stale); + assert_eq!( + app.hosted.catalog[0].revision, 1, + "Old results stay identified as a snapshot" + ); + dispatch(&mut app, Message::Hosted(Event::Connect)).await; + assert!(!app.hosted.catalog_stale); + assert_eq!(app.hosted.catalog[0].revision, 2); + assert_eq!(app.hosted.catalog[0].name, "Graphical update"); let external = Connection::new(&base, Secret(token.into())).unwrap(); let remote = external.open(session.resource_id).await.unwrap(); assert_eq!(remote.compiled.decoded.project.name, "Graphical update"); @@ -558,3 +595,137 @@ fn hosted_failed_or_cancelled_local_open_keeps_the_existing_document_and_destina assert_eq!(app.hosted.session.as_ref().unwrap().resource_id, resource); } } + +fn catalog_result(app: &App, resource: Id, revision: u64) -> Outcome { + Outcome::Catalog(crate::hosted::client::Catalog { + items: vec![crate::hosted::client::Summary { + resource_id: resource, + revision, + model_id: app.project().id, + name: format!("Catalog snapshot {revision}"), + elements: app.project().elements.len(), + relations: app.project().relations.len(), + }], + next_after: Some(resource), + }) +} + +#[test] +fn hosted_catalog_search_credentials_and_failed_refresh_never_mix_result_scopes() { + let mut app = App::with_project(Project::empty()); + let first = Id::new_v4(); + send(&mut app, Event::Connect); + let catalog = catalog_result(&app, first, 1); + complete(&mut app, Ok(catalog)); + assert!(!app.hosted.catalog_stale); + send(&mut app, Event::More); + assert_eq!( + app.hosted.request.as_ref().unwrap().operation, + Operation::Catalog(true) + ); + let next_page = catalog_result(&app, Id::new_v4(), 1); + complete(&mut app, Ok(next_page)); + assert_eq!(app.hosted.catalog.len(), 2); + send(&mut app, Event::Select(first)); + send(&mut app, Event::Query("different search".into())); + assert!(app.hosted.catalog_stale); + assert!(app.hosted.next.is_none()); + assert!(app.hosted.selected.is_none()); + assert_eq!(app.hosted.catalog.len(), 2); + assert!( + app.hosted + .catalog_message() + .unwrap() + .contains("Connect / refresh") + ); + send(&mut app, Event::More); + assert!(app.hosted.request.is_none()); + send(&mut app, Event::Connect); + complete(&mut app, Err(failure(None, false))); + assert!(app.hosted.catalog_stale); + assert_eq!(app.hosted.catalog[0].resource_id, first); + send(&mut app, Event::Connect); + let second = Id::new_v4(); + let catalog = catalog_result(&app, second, 1); + complete(&mut app, Ok(catalog)); + assert!(!app.hosted.catalog_stale); + assert_eq!(app.hosted.catalog.len(), 1); + assert_eq!(app.hosted.catalog[0].resource_id, second); + send(&mut app, Event::Query("different search".into())); + assert!( + !app.hosted.catalog_stale, + "Unchanged query preserves the snapshot" + ); + send( + &mut app, + Event::Token(Secret("different-credential".into())), + ); + assert!(app.hosted.catalog_stale); + assert!(app.hosted.catalog.is_empty()); + assert!(app.hosted.connection.is_none()); + assert!(app.hosted.next.is_none()); + assert!(app.hosted.selected.is_none()); + assert!( + !app.hosted + .catalog_message() + .unwrap() + .starts_with("No models found") + ); + send(&mut app, Event::Connect); + let catalog = catalog_result(&app, second, 2); + complete(&mut app, Ok(catalog)); + send(&mut app, Event::Base("http://127.0.0.1:9876".into())); + assert!(app.hosted.catalog.is_empty()); + assert!(app.hosted.connection.is_none()); + assert!(app.hosted.catalog_stale); + assert!(app.hosted.request.is_none()); +} + +#[test] +fn hosted_catalog_invalidation_follows_service_writes_and_observed_revision_changes() { + let mut app = App::with_project(Project::empty()); + let resource = bind(&mut app); + let other = Id::new_v4(); + send(&mut app, Event::Base("http://127.0.0.1:9876".into())); + send(&mut app, Event::Connect); + let catalog = catalog_result(&app, other, 7); + complete(&mut app, Ok(catalog)); + rename(&mut app, "Saved to original service"); + send(&mut app, Event::Save); + let saved = remote(app.document.snapshot(), resource, 2); + complete(&mut app, Ok(Outcome::Remote(saved))); + assert!( + !app.hosted.catalog_stale, + "A write to A cannot invalidate B's catalog" + ); + assert_eq!(app.hosted.next, Some(other)); + assert_eq!(app.hosted.catalog[0].resource_id, other); + + send(&mut app, Event::Base("http://127.0.0.1:8787".into())); + send(&mut app, Event::Connect); + let catalog = catalog_result(&app, resource, 2); + complete(&mut app, Ok(catalog)); + send(&mut app, Event::Check); + let changed = remote(app.document.snapshot(), resource, 3); + complete(&mut app, Ok(Outcome::Remote(changed))); + assert!( + app.hosted.catalog_stale, + "A known newer server revision invalidates old metadata" + ); + assert!(app.hosted.next.is_none()); + send(&mut app, Event::Connect); + let catalog = catalog_result(&app, resource, 3); + complete(&mut app, Ok(catalog)); + send(&mut app, Event::Reload); + rename(&mut app, "Write with uncertain outcome"); + send(&mut app, Event::Save); + complete(&mut app, Err(failure(None, true))); + assert!(app.hosted.catalog_stale); + assert!(app.hosted.next.is_none()); + assert_eq!(app.hosted.catalog[0].revision, 3); + app.hosted.leave_document(); + assert!( + app.hosted.catalog_stale, + "Leaving the hosted destination cannot make old results fresh" + ); +} diff --git a/crates/oa-desktop/src/hosted/mod.rs b/crates/oa-desktop/src/hosted/mod.rs index f957c38b..ab7f572d 100644 --- a/crates/oa-desktop/src/hosted/mod.rs +++ b/crates/oa-desktop/src/hosted/mod.rs @@ -76,6 +76,7 @@ pub struct State { pub token: Secret, pub query: String, pub catalog: Vec, + pub catalog_stale: bool, pub next: Option, pub selected: Option, pub connection: Option, @@ -97,6 +98,7 @@ impl Default for State { token: Secret::default(), query: String::new(), catalog: vec![], + catalog_stale: true, next: None, selected: None, connection: None, @@ -113,6 +115,23 @@ impl Default for State { } impl State { + pub fn invalidate_catalog(&mut self) { + self.catalog_stale = true; + self.next = None; + } + + pub fn catalog_message(&self) -> Option<&'static str> { + if self.connection.is_none() { + Some("Connect / refresh to load models for this service.") + } else if self.catalog_stale { + Some("Catalog may be out of date. Connect / refresh to update these results.") + } else if self.catalog.is_empty() { + Some("No models found. Publish the current model or change the search.") + } else { + None + } + } + /// Keep explicit connection settings in memory, but never carry a resource /// binding or in-flight result into another local document. pub fn leave_document(&mut self) { diff --git a/crates/oa-desktop/src/hosted/view.rs b/crates/oa-desktop/src/hosted/view.rs index aea1cc22..232ec831 100644 --- a/crates/oa-desktop/src/hosted/view.rs +++ b/crates/oa-desktop/src/hosted/view.rs @@ -180,10 +180,8 @@ fn dialog(app: &App) -> Element<'_, Message> { ] .spacing(8), ); - if state.connection.is_some() && state.catalog.is_empty() { - content = content.push(label( - "No models found. Publish the current model or change the search.", - )); + if let Some(message) = state.catalog_message() { + content = content.push(label(message)); } for item in &state.catalog { let selected = state.selected == Some(item.resource_id); @@ -208,7 +206,7 @@ fn dialog(app: &App) -> Element<'_, Message> { .on_press_maybe(enabled.then_some(Message::Hosted(Event::Select(item.resource_id)))), ); } - if state.next.is_some() { + if !state.catalog_stale && state.next.is_some() { content = content.push(control("More models", Event::More, enabled)); } let publish_label = if state