fix(desktop): identify stale hosted catalog results
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -76,6 +76,7 @@ pub struct State {
|
||||
pub token: Secret,
|
||||
pub query: String,
|
||||
pub catalog: Vec<Summary>,
|
||||
pub catalog_stale: bool,
|
||||
pub next: Option<Id>,
|
||||
pub selected: Option<Id>,
|
||||
pub connection: Option<Connection>,
|
||||
@@ -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) {
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user