Skip to content

Commit 1038f97

Browse files
committed
Harden reload completion guards
1 parent 8269c63 commit 1038f97

1 file changed

Lines changed: 124 additions & 12 deletions

File tree

src/app.rs

Lines changed: 124 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,15 @@ impl App {
160160
pub fn complete_reload_buckets(&mut self, result: Result<CompleteReloadBucketsResult>) {
161161
let view_state = match self.page_stack.current_page() {
162162
Page::BucketList(page) => page.view_state_snapshot(),
163-
page => panic!("Invalid page: {page:?}"),
163+
page => {
164+
tracing::warn!(
165+
current_page = ?page,
166+
"ignoring CompleteReloadBuckets because current page is not BucketList"
167+
);
168+
self.pending_single_bucket_reload = None;
169+
self.is_loading = false;
170+
return;
171+
}
164172
};
165173

166174
match result {
@@ -313,21 +321,40 @@ impl App {
313321
pub fn complete_load_objects(&mut self, result: Result<CompleteLoadObjectsResult>) {
314322
match result {
315323
Ok(CompleteLoadObjectsResult { items, object_key }) => {
316-
self.app_objects
317-
.set_object_items(object_key.clone(), items.clone());
318-
319324
let bucket_name = object_key.bucket_name.clone();
320-
let object_list_page =
321-
Page::of_object_list(items, object_key, Rc::clone(&self.ctx), self.tx.clone());
322325
if let Some(bucket) = self.pending_single_bucket_reload.take() {
323-
debug_assert_eq!(bucket.name, bucket_name);
324-
325-
self.app_objects.set_bucket_items(vec![bucket]);
326+
if bucket.name != bucket_name {
327+
tracing::warn!(
328+
pending_bucket = %bucket.name,
329+
loaded_bucket = %bucket_name,
330+
"ignoring CompleteLoadObjects because pending single-bucket reload targets a different bucket"
331+
);
332+
self.is_loading = false;
333+
return;
334+
}
326335

327-
if matches!(self.page_stack.current_page(), Page::BucketList(_)) {
328-
self.page_stack.pop();
336+
match self.page_stack.current_page() {
337+
Page::BucketList(_) => {
338+
self.app_objects.set_bucket_items(vec![bucket]);
339+
self.page_stack.pop();
340+
}
341+
page => {
342+
tracing::warn!(
343+
current_page = ?page,
344+
bucket = %bucket_name,
345+
"ignoring CompleteLoadObjects because pending single-bucket reload is no longer on BucketList"
346+
);
347+
self.is_loading = false;
348+
return;
349+
}
329350
}
330351
}
352+
353+
self.app_objects
354+
.set_object_items(object_key.clone(), items.clone());
355+
356+
let object_list_page =
357+
Page::of_object_list(items, object_key, Rc::clone(&self.ctx), self.tx.clone());
331358
self.page_stack.push(object_list_page);
332359
}
333360
Err(e) => {
@@ -356,7 +383,14 @@ impl App {
356383
pub fn complete_reload_objects(&mut self, result: Result<CompleteReloadObjectsResult>) {
357384
let view_state = match self.page_stack.current_page() {
358385
Page::ObjectList(page) => page.view_state_snapshot(),
359-
page => panic!("Invalid page: {page:?}"),
386+
page => {
387+
tracing::warn!(
388+
current_page = ?page,
389+
"ignoring CompleteReloadObjects because current page is not ObjectList"
390+
);
391+
self.is_loading = false;
392+
return;
393+
}
360394
};
361395

362396
match result {
@@ -1176,6 +1210,59 @@ mod tests {
11761210
assert_notify_error(&mut rx, "Failed to load objects").await;
11771211
}
11781212

1213+
#[tokio::test]
1214+
async fn complete_reload_buckets_ignores_response_when_current_page_is_not_bucket_list() {
1215+
let (mut app, _rx) = app().await;
1216+
let object_key = ObjectKey::bucket("test-bucket");
1217+
let items = vec![object_file_item("foo.txt")];
1218+
1219+
app.page_stack.push(Page::of_object_list(
1220+
items,
1221+
object_key,
1222+
Rc::clone(&app.ctx),
1223+
app.tx.clone(),
1224+
));
1225+
app.is_loading = true;
1226+
1227+
app.complete_reload_buckets(Ok(CompleteReloadBucketsResult {
1228+
buckets: vec![bucket_item("foo-bucket")],
1229+
}));
1230+
1231+
assert!(!app.loading());
1232+
assert!(app.pending_single_bucket_reload.is_none());
1233+
assert_eq!(app.page_stack.len(), 1);
1234+
assert!(matches!(app.page_stack.current_page(), Page::ObjectList(_)));
1235+
assert!(app.app_objects.get_bucket_items().is_empty());
1236+
}
1237+
1238+
#[tokio::test]
1239+
async fn complete_load_objects_ignores_mismatched_pending_single_bucket_reload() {
1240+
let (mut app, _rx) = app().await;
1241+
let buckets = vec![bucket_item("foo-bucket"), bucket_item("bar-bucket")];
1242+
let object_key = ObjectKey::bucket("other-bucket");
1243+
let items = vec![object_file_item("foo.txt")];
1244+
1245+
app.complete_initialize(Ok(CompleteInitializeResult {
1246+
buckets,
1247+
prefix: None,
1248+
}));
1249+
app.pending_single_bucket_reload = Some(bucket_item("solo-bucket"));
1250+
app.is_loading = true;
1251+
1252+
app.complete_load_objects(Ok(CompleteLoadObjectsResult {
1253+
items,
1254+
object_key: object_key.clone(),
1255+
}));
1256+
1257+
assert!(!app.loading());
1258+
assert!(app.pending_single_bucket_reload.is_none());
1259+
assert_eq!(app.page_stack.len(), 1);
1260+
assert!(matches!(app.page_stack.current_page(), Page::BucketList(_)));
1261+
assert_bucket_selected(&app, "foo-bucket");
1262+
assert_bucket_names(&app, &["foo-bucket", "bar-bucket"]);
1263+
assert!(app.app_objects.get_object_items(&object_key).is_none());
1264+
}
1265+
11791266
#[tokio::test]
11801267
async fn object_list_refresh_keeps_view_state() {
11811268
let (mut app, _rx) = app().await;
@@ -1212,6 +1299,31 @@ mod tests {
12121299
assert_object_selected(&app, "baz.txt");
12131300
}
12141301

1302+
#[tokio::test]
1303+
async fn complete_reload_objects_ignores_response_when_current_page_is_not_object_list() {
1304+
let (mut app, _rx) = app().await;
1305+
let buckets = vec![bucket_item("foo-bucket"), bucket_item("bar-bucket")];
1306+
1307+
app.complete_initialize(Ok(CompleteInitializeResult {
1308+
buckets,
1309+
prefix: None,
1310+
}));
1311+
app.is_loading = true;
1312+
1313+
app.complete_reload_objects(Ok(CompleteReloadObjectsResult {
1314+
items: vec![object_file_item("foo.txt")],
1315+
object_key: ObjectKey::bucket("foo-bucket"),
1316+
}));
1317+
1318+
assert!(!app.loading());
1319+
assert_eq!(app.page_stack.len(), 1);
1320+
assert!(matches!(app.page_stack.current_page(), Page::BucketList(_)));
1321+
assert!(app
1322+
.app_objects
1323+
.get_object_items(&ObjectKey::bucket("foo-bucket"))
1324+
.is_none());
1325+
}
1326+
12151327
#[tokio::test]
12161328
async fn object_list_refresh_keeps_scroll_offset(
12171329
) -> std::result::Result<(), core::convert::Infallible> {

0 commit comments

Comments
 (0)