From 1dcb0343511f2ace03aa4f29c9c7a1b39716096b Mon Sep 17 00:00:00 2001 From: clover caruso Date: Thu, 15 Oct 2026 12:27:46 -0700 Subject: [PATCH] fix(notebook): a provider that lists but cannot open still opens the notebook A cloud provider offline still lists a notebook's folder from its cache but cannot open its files, and opening such a notebook failed outright. A section listed but unreachable now stands as it was last read, so the notebook opens and its sections open from their offline copies, showing they are not connected and keeping new edits until the provider can be reached again. A section never read before still fails the notebook as before. Refs #24 Assisted-by: claude-opus-5.5 --- crates/mobile/src/tests.rs | 63 +++++++++++++++++++++++++++ crates/notebook/README.md | 2 + crates/notebook/src/discover.rs | 35 +++++++++++++-- crates/notebook/src/provider/tests.rs | 40 +++++++++++++++++ 4 files changed, 136 insertions(+), 4 deletions(-) diff --git a/crates/mobile/src/tests.rs b/crates/mobile/src/tests.rs index d33418b1f7c53b22879e0bc17c09cbf1296e0d31..ecdf5934b2ec5d01830a33f73f1c6a999537b671 100644 --- a/crates/mobile/src/tests.rs +++ b/crates/mobile/src/tests.rs @@ -3610,6 +3610,14 @@ mod provider_fake { }; static FAILING: Mutex> = Mutex::new(Vec::new()); + /// Folders whose documents list but don't open, as a cloud provider's offline. + static UNOPENABLE: Mutex> = Mutex::new(Vec::new()); + + pub(super) fn unopenable(root: &Path, unopenable: bool) { + let mut roots = UNOPENABLE.lock().unwrap(); + roots.retain(|held| held != root); + roots.extend(unopenable.then(|| root.to_owned())); + } /// Has every call on documents in `root` fail with `code`, or none. pub(super) fn fail(root: &Path, code: Option) { @@ -3676,6 +3684,14 @@ mod provider_fake { Ok(path) => path, Err(code) => return code, }; + if UNOPENABLE + .lock() + .unwrap() + .iter() + .any(|root| path.starts_with(root)) + { + return 1; + } let write = string(mode) == "rw"; match std::fs::File::options().read(true).write(write).open(path) { Ok(file) => { @@ -3894,3 +3910,50 @@ fn a_lone_provider_document_opens_and_publishes() { assert!(stored_text(&file, space).contains("Alone ")); closed(section); } + +#[test] +fn a_provider_that_lists_but_cannot_open_opens_from_the_offline_copies() { + let (directory, root) = copy("m6/native-features-01/notebook"); + let cache = directory.path().join("cache"); + let library = provided(&root, true, &cache).unwrap(); + let section = open(&library, "Features.one"); + let space = typed(§ion, "Before "); + assert!(flushed(&library, 20.0)); + closed(section); + drop(library); + // Listed otherwise than when last read, so discovery would read it again. + let file = root.join("Features.one"); + let touched = std::fs::metadata(&file).unwrap().modified().unwrap() + Duration::from_secs(5); + std::fs::File::options() + .write(true) + .open(&file) + .unwrap() + .set_modified(touched) + .unwrap(); + provider_fake::unopenable(&root, true); + + let library = provided(&root, true, &cache).unwrap(); + assert!( + library + .tabs() + .unwrap() + .iter() + .any(|tab| tab.path == "Features.one" && tab.readable) + ); + let section = open(&library, "Features.one"); + let (page, _) = section.shared.page(space).unwrap(); + assert!(canvas::search::page_text(&page).contains("Before ")); + typed(§ion, "Offline"); + let (page, _) = section.shared.page(space).unwrap(); + assert!(canvas::search::page_text(&page).contains("Offline")); + assert!(eventually(|| library.sync_status().iter().any(|sync| { + sync.path == "Features.one" + && sync.state == SyncState::NotConnected as u8 + && sync.queued == 1 + }))); + provider_fake::unopenable(&root, false); + sb_library_sync_now(&library); + assert!(flushed(&library, 20.0)); + assert!(stored_text(&file, space).contains("Offline")); + closed(section); +} diff --git a/crates/notebook/README.md b/crates/notebook/README.md index b297517f1f9eda97cb4a3729f3be3b0956f9f027..f2d818bdf89554c99e58ed718c4702c102c05092 100644 --- a/crates/notebook/README.md +++ b/crates/notebook/README.md @@ -312,6 +312,8 @@ with one TOC identity, the one its parent's TOC names, else the newest, lists; t list in `unavailable` as `Reason::Copy`. iOS's `.Name.one.icloud` placeholders, and macOS's dataless files, list there under their real names as `Reason::Evicted`, unless listed unchanged since discovery last read them. +A file listed but not reachable, as a cloud provider offline lists from its cache, stands as +it was last read, and is read again next time; one never read fails the discovery. Retain the last accepted catalog if discovery fails; a connection failure does not mean files were deleted. diff --git a/crates/notebook/src/discover.rs b/crates/notebook/src/discover.rs index a91d3e77255ce241605660966b06a7bd2dbf50aa..73f2359760a8010ff967d36a2fd35ff0fed45b6a 100644 --- a/crates/notebook/src/discover.rs +++ b/crates/notebook/src/discover.rs @@ -369,7 +369,7 @@ fn scan( order: Vec::new(), unavailable: Vec::new(), }; - for entry in &listing { + 'entries: for entry in &listing { let child = join(path, &entry.name); if entry.kind == EntryKind::Directory { if entry.name.to_ascii_lowercase().ends_with("_onefiles") { @@ -432,7 +432,7 @@ fn scan( found.read.insert(child.clone(), known.clone()); (file_id, Ok(known.held.clone())) } - None => { + None => 'read: { let copied = cached .get(&child) .and_then(Read::file_id) @@ -474,11 +474,21 @@ fn scan( let bytes = match read { Ok(bytes) => bytes, Err(error) => { + // Listed but unreachable, as a cloud provider lists from its cache: the + // file as last read, read again next time. + if unreachable(&error) + && let Some((file_id, known)) = cached + .get(&child) + .and_then(|known| Some((known.file_id()?, known))) + { + found.read.insert(child.clone(), known.clone()); + break 'read (file_id, Ok(known.held.clone())); + } list_unavailable(Error::Io { path: child.clone(), error, })?; - continue; + continue 'entries; } }; if bytes.len() > limits.bytes_per_file { @@ -500,7 +510,7 @@ fn scan( path: child.clone(), error, })?; - continue; + continue 'entries; } }; let held = held(&store, expected); @@ -831,6 +841,23 @@ fn unavailable(error: &Error) -> Option { } } +/// Whether a file listed could not be read for want of a connection. +fn unreachable(error: &io::Error) -> bool { + use io::ErrorKind::*; + matches!( + error.kind(), + NotConnected + | TimedOut + | ConnectionRefused + | ConnectionReset + | ConnectionAborted + | HostUnreachable + | NetworkUnreachable + | NetworkDown + | BrokenPipe + ) +} + /// Not the notebook's: dot files, among them macOS's `.DS_Store` and AppleDouble `._` /// companions and Snowbound's `.snowbound` folder, and Office's `~$` owner files. fn foreign(name: &str) -> bool { diff --git a/crates/notebook/src/provider/tests.rs b/crates/notebook/src/provider/tests.rs index bcfa2b464f2bf40eb5832a90b3878e296299ce4c..e5a1320f95362768c1f1bdbf8efe6cff99c11141 100644 --- a/crates/notebook/src/provider/tests.rs +++ b/crates/notebook/src/provider/tests.rs @@ -41,6 +41,8 @@ struct State { next: u32, writable: bool, offline: bool, + /// Lists, from its cache, but opens nothing, as a cloud provider offline does. + unopenable: bool, revoked: bool, /// One device's writes reach the others only at `publish_staged`, as the lab's /// upload-on-close provider showed. @@ -80,6 +82,7 @@ impl Fake { next: 0, writable: true, offline: false, + unopenable: false, revoked: false, delayed: false, swap: None, @@ -246,6 +249,9 @@ impl Provider for Fake { fn open(&self, id: &str, mode: &str) -> io::Result { let mut state = self.reachable()?; + if state.unopenable { + return Err(io::Error::new(io::ErrorKind::NotConnected, "Offline")); + } if !state.nodes.contains_key(id) { return Err(io::ErrorKind::NotFound.into()); } @@ -806,3 +812,37 @@ fn a_lone_document_opens_offline_from_its_replica() { assert_eq!(stored(fake.bytes("Lone.one")), "Not Alone now"); section.close().unwrap(); } + +#[test] +fn a_provider_that_lists_but_cannot_open_opens_sections_from_their_replicas() { + let fake = Fake::new(Kind::Seekable); + notebook(&fake); + let cache = tempfile::tempdir().unwrap(); + let documents = fake.documents("root"); + let notebook = Notebook::open_provider(Arc::clone(&documents), cache.path()).unwrap(); + open(¬ebook, &documents, "Notes.one").close().unwrap(); + drop(notebook); + // Another device changes the section, so it lists otherwise than when last read. + let elsewhere = fake.device(1).documents("root"); + let other_cache = tempfile::tempdir().unwrap(); + let other = Notebook::open_provider(Arc::clone(&elsewhere), other_cache.path()).unwrap(); + let other = open(&other, &elsewhere, "Notes.one"); + let id = typed(&other, 0..0, "Theirs "); + published(&other, id); + other.close().unwrap(); + fake.set(|state| state.unopenable = true); + + let documents = fake.documents("root"); + let notebook = Notebook::open_provider(Arc::clone(&documents), cache.path()).unwrap(); + assert_eq!(notebook.catalog().order, ["Notes.one", "Ideas.one"]); + let section = open(¬ebook, &documents, "Notes.one"); + assert_eq!(text(§ion), "Original text"); + let id = typed(§ion, 13..13, " mine"); + status(§ion, |status| { + status.state() == SyncState::NotConnected && status.queued == 1 + }); + fake.set(|state| state.unopenable = false); + published(§ion, id); + assert_eq!(stored(fake.bytes("Notes.one")), "Theirs Original text mine"); + section.close().unwrap(); +} -- 2.54.0