diff --git a/apps/ios/Snowbound/Notebooks.swift b/apps/ios/Snowbound/Notebooks.swift index 0474147c78e2e58412ea1fe8ec14217bfdffc191..2d468a4463f41c3d9826762dd5bc93abd9942798 100644 --- a/apps/ios/Snowbound/Notebooks.swift +++ b/apps/ios/Snowbound/Notebooks.swift @@ -523,9 +523,10 @@ enum Notebooks { list = [] return [] } + guard let names = notebooks(in: folder) else { return [] } var added: [Notebook] = [] list = ordered( - notebooks(in: folder).map { [list] name in + names.map { [list] name in if let kept = list.first(where: { $0.id == prefix + name }) { return kept } let notebook = Notebook( id: prefix + name, name: (name as NSString).deletingPathExtension, source: source(name)) @@ -564,15 +565,16 @@ enum Notebooks { /// Whether Snowbound's folder in iCloud Drive holds other notebooks than the list shows, as /// when another device added or removed one. static var inCloudChanged: Bool { - guard let folder = ICloud.documents, scripted == nil else { return false } - return notebooks(in: folder).map { "iCloud/" + $0 } != inCloud.map(\.id) + guard let folder = ICloud.documents, scripted == nil, let names = notebooks(in: folder) else { return false } + return names.map { "iCloud/" + $0 } != inCloud.map(\.id) } /// The notebook folders and section files in `folder`, by name, as Files orders them. - private static func notebooks(in folder: URL) -> [String] { - let listed = try? FileManager.default.contentsOfDirectory( + private static func notebooks(in folder: URL) -> [String]? { + guard let listed = try? FileManager.default.contentsOfDirectory( at: folder, includingPropertiesForKeys: [.isDirectoryKey], options: .skipsHiddenFiles) - return (listed ?? []).filter { url in + else { return nil } + return listed.filter { url in (try? url.resourceValues(forKeys: [.isDirectoryKey]).isDirectory) == true || url.pathExtension.lowercased() == "one" } diff --git a/crates/mobile/src/tests.rs b/crates/mobile/src/tests.rs index b84331cb5d08c06eba79cefa53c39812cf384353..a7c91fb2b9fb50020232d8b3c7594e07b654fcf4 100644 --- a/crates/mobile/src/tests.rs +++ b/crates/mobile/src/tests.rs @@ -317,6 +317,65 @@ fn typing_is_stored_in_the_cache_and_published_to_the_file() { assert!(canvas::search::page_text(&page).contains("Saved Collapsed parent")); } +#[test] +fn cached_cloud_sections_reopen_with_offline_edits_after_eviction() { + let directory = tempfile::tempdir().unwrap(); + let root = directory.path().join("notebook"); + let cache = directory.path().join("cache"); + let creation = onestore::PageCreation::new(None, Some("Kept offline"), "Fixture").unwrap(); + let notebook = + notebook::session::Notebook::create(&root, &cache, 0x00f0_a0c0, &creation).unwrap(); + let path = notebook.catalog().sections[0].path.clone(); + drop(notebook); + let library = Arc::new(Library::open(&root, &cache, false).unwrap()); + sb_library_set_offline(&library, true); + let section = open(&library, &path); + let space = space(§ion, "Kept offline"); + section + .shared + .apply(Edit { + at: library::filetime(), + ops: vec![Op::Page { + space, + op: PageOp::Color(Some(0x00ef_cdab)), + }], + }) + .unwrap(); + let shared = Arc::clone(§ion.shared); + drop((section, library)); + Arc::into_inner(shared).unwrap().section.close().unwrap(); + let file = root.join(&path); + let held = directory.path().join("held.one"); + std::fs::rename(&file, &held).unwrap(); + let placeholder = root.join(format!(".{path}.icloud")); + std::fs::write(&placeholder, b"placeholder").unwrap(); + + let library = Arc::new(Library::open(&root, &cache, false).unwrap()); + sb_library_set_offline(&library, true); + for _ in 0..2 { + let tabs = library.tabs().unwrap(); + assert_eq!(tabs[0].path, path); + assert!(tabs[0].readable && !tabs[0].downloading); + } + let section = open(&library, &path); + assert_eq!(section.rows().unwrap()[0].title, "Kept offline"); + assert_eq!( + section.shared.page(space).unwrap().0.color, + Some(0x00ef_cdab) + ); + assert_eq!(section.shared.section.pending().unwrap().len(), 1); + assert!(!file.exists()); + + std::fs::rename(held, &file).unwrap(); + std::fs::remove_file(placeholder).unwrap(); + assert!(library.tabs().unwrap()[0].readable); + sb_library_set_offline(&library, false); + assert!(section.flush(Duration::from_secs(20))); + let arena = onestore::Arena::default(); + let stored = onestore::Section::open(&arena, onestore::read_file(file).unwrap()).unwrap(); + assert_eq!(stored.page(space).unwrap().color, Some(0x00ef_cdab)); +} + #[test] fn a_new_page_and_subpage_list_after_their_parent_and_delete_to_the_recycle_bin() { let (directory, root) = copy("m6/native-features-01/notebook"); diff --git a/crates/notebook/src/discover.rs b/crates/notebook/src/discover.rs index 7b194d230e5c223a053fc49c3e07e1a9ee6a929c..ceb22da656f51ee3113258e728d1a49e887dbcf9 100644 --- a/crates/notebook/src/discover.rs +++ b/crates/notebook/src/discover.rs @@ -408,11 +408,9 @@ fn scan( }; let reused = cached .get(&child) - .filter(|known| known.listed == entry.listed) + .filter(|known| entry.kind == EntryKind::Evicted || known.listed == entry.listed) .and_then(|known| Some((known.file_id()?, known))); - // An evicted file listed as it was last read lists as then. Without an evicted TOC the - // folder lists in path order; nothing writes a second one, as its placeholder, or the - // dataless file itself, blocks creating it. + // A placeholder's metadata describes the placeholder, not the cached section. if entry.kind == EntryKind::Evicted && reused.is_none() { if expected == FileType::Section { result.unavailable.push(Unavailable { diff --git a/crates/notebook/src/session.rs b/crates/notebook/src/session.rs index 84d642abec9b0bc0423898bbd8f7b65671f3b4a7..107153511fef8e69ffd93154e814538dcec39f8c 100644 --- a/crates/notebook/src/session.rs +++ b/crates/notebook/src/session.rs @@ -1651,8 +1651,19 @@ impl Notebook { ) .into()); }; - // A section replaced by a link out of the notebook is not the catalog's section. - let file = fs::canonicalize(root.join(path))?; + let file = root.join(path); + // Evicted files resume their replicas; existing links still resolve within the root. + let file = match fs::symlink_metadata(&file) { + Err(error) if error.kind() == io::ErrorKind::NotFound => fs::canonicalize( + file.parent() + .ok_or(io::Error::from(io::ErrorKind::InvalidInput))?, + )? + .join( + file.file_name() + .ok_or(io::Error::from(io::ErrorKind::InvalidInput))?, + ), + _ => fs::canonicalize(&file)?, + }; if !file.starts_with(root) { return Err(io::Error::from(io::ErrorKind::PermissionDenied).into()); } diff --git a/crates/notebook/src/session/tests.rs b/crates/notebook/src/session/tests.rs index 06490e7945375ff2795823c74af7df3559751e40..4cda501114d64f6b1e83eda52cc8bdc1700e3525 100644 --- a/crates/notebook/src/session/tests.rs +++ b/crates/notebook/src/session/tests.rs @@ -1,5 +1,24 @@ use super::*; +#[cfg(unix)] +#[test] +fn cached_sections_refuse_links_outside_the_notebook_and_dangling_links() { + let temporary = tempfile::tempdir().unwrap(); + let folder = temporary.path().join("notebook"); + let cache = temporary.path().join("cache"); + let creation = PageCreation::new(None, Some("Mine"), "Fixture").unwrap(); + let notebook = Notebook::create(&folder, &cache, 0x00f0_a0c0, &creation).unwrap(); + let path = ¬ebook.catalog().sections[0].path; + notebook.section(path, || {}).unwrap().close().unwrap(); + let file = folder.join(path); + let outside = temporary.path().join("outside.one"); + fs::rename(&file, &outside).unwrap(); + std::os::unix::fs::symlink(&outside, &file).unwrap(); + assert!(notebook.section(path, || {}).is_err()); + fs::remove_file(outside).unwrap(); + assert!(notebook.section(path, || {}).is_err()); +} + /// A mounted notebook where another writer commits `revision` to the section file just /// before Snowbound's rewritten file takes its place. struct Racing { diff --git a/crates/notebook/tests/discovery.rs b/crates/notebook/tests/discovery.rs index 5d9689260efbb2f07c9c59bd4581399c2120baf3..76d14d80a44707492fa77f6f56f9e6e2668b9d48 100644 --- a/crates/notebook/tests/discovery.rs +++ b/crates/notebook/tests/discovery.rs @@ -197,6 +197,33 @@ fn evicted_files_list_under_their_names_until_downloaded() { assert!(!root.join("Open Notebook.onetoc2").exists()); } +#[test] +fn cached_sections_and_tocs_survive_eviction_to_different_placeholder_metadata() { + let root = tempfile::tempdir().unwrap(); + fixture(root.path()); + let mut cache = Cache::default(); + let mut source = Local::open(root.path()).unwrap(); + let before = cache.discover(&mut source, limits()).unwrap(); + for name in ["one.one", "Open Notebook.onetoc2"] { + fs::remove_file(root.path().join(name)).unwrap(); + fs::write(root.path().join(format!(".{name}.icloud")), b"placeholder").unwrap(); + } + for _ in 0..2 { + let evicted = cache.discover(&mut source, limits()).unwrap(); + assert_eq!(evicted.sections[0].file_id, before.sections[0].file_id); + assert_eq!(evicted.order, before.order); + assert!(evicted.toc.is_some() && evicted.unavailable.is_empty()); + } + fs::write( + root.path().join("one.one"), + onestore::create_section("one.one", "Downloaded change", "Fixture").unwrap(), + ) + .unwrap(); + touch(&root.path().join("one.one"), 1_000_000_000); + let downloaded = cache.discover(&mut source, limits()).unwrap(); + assert_ne!(downloaded.sections[0].file_id, before.sections[0].file_id); +} + /// A source whose files list as macOS lists those iCloud Drive evicted: dataless, under their /// names and listings, and never read. struct Dataless(Local); @@ -215,8 +242,7 @@ impl Source for Dataless { } } -/// A dataless file lists as its last read found it, where it lists unchanged since, and as not -/// downloaded otherwise. +/// A dataless file lists as its last read found it, and waits for download without a cache. #[test] fn dataless_files_list_as_last_read_or_wait_for_their_download() { let root = tempfile::tempdir().unwrap();