| author | |
| committer | |
| log | 1dcb0343511f2ace03aa4f29c9c7a1b39716096b |
| tree | 7372187fcd6565a05e727718b00cce5b50e7c7c1 |
| parent | 196e41e3847eea042b716aae2225149d9790d27b |
| signature | Signed by SSH key SHA256:52mNGHRsVFBDED9IAX5pe+LRWUefqTbxEReunq21QvU (~clover) |
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.54 files changed, 136 insertions(+), 4 deletions(-)
crates/mobile/src/tests.rs+63| ... | @@ -3610,6 +3610,14 @@ mod provider_fake { | ... | @@ -3610,6 +3610,14 @@ mod provider_fake { |
| 3610 | }; | 3610 | }; |
| 3611 | 3611 | ||
| 3612 | static FAILING: Mutex<Vec<(PathBuf, i32)>> = Mutex::new(Vec::new()); | 3612 | static FAILING: Mutex<Vec<(PathBuf, i32)>> = Mutex::new(Vec::new()); |
| 3613 | /// Folders whose documents list but don't open, as a cloud provider's offline. | ||
| 3614 | static UNOPENABLE: Mutex<Vec<PathBuf>> = Mutex::new(Vec::new()); | ||
| 3615 | |||
| 3616 | pub(super) fn unopenable(root: &Path, unopenable: bool) { | ||
| 3617 | let mut roots = UNOPENABLE.lock().unwrap(); | ||
| 3618 | roots.retain(|held| held != root); | ||
| 3619 | roots.extend(unopenable.then(|| root.to_owned())); | ||
| 3620 | } | ||
| 3613 | 3621 | ||
| 3614 | /// Has every call on documents in `root` fail with `code`, or none. | 3622 | /// Has every call on documents in `root` fail with `code`, or none. |
| 3615 | pub(super) fn fail(root: &Path, code: Option<i32>) { | 3623 | pub(super) fn fail(root: &Path, code: Option<i32>) { |
| ... | @@ -3676,6 +3684,14 @@ mod provider_fake { | ... | @@ -3676,6 +3684,14 @@ mod provider_fake { |
| 3676 | Ok(path) => path, | 3684 | Ok(path) => path, |
| 3677 | Err(code) => return code, | 3685 | Err(code) => return code, |
| 3678 | }; | 3686 | }; |
| 3687 | if UNOPENABLE | ||
| 3688 | .lock() | ||
| 3689 | .unwrap() | ||
| 3690 | .iter() | ||
| 3691 | .any(|root| path.starts_with(root)) | ||
| 3692 | { | ||
| 3693 | return 1; | ||
| 3694 | } | ||
| 3679 | let write = string(mode) == "rw"; | 3695 | let write = string(mode) == "rw"; |
| 3680 | match std::fs::File::options().read(true).write(write).open(path) { | 3696 | match std::fs::File::options().read(true).write(write).open(path) { |
| 3681 | Ok(file) => { | 3697 | Ok(file) => { |
| ... | @@ -3894,3 +3910,50 @@ fn a_lone_provider_document_opens_and_publishes() { | ... | @@ -3894,3 +3910,50 @@ fn a_lone_provider_document_opens_and_publishes() { |
| 3894 | assert!(stored_text(&file, space).contains("Alone ")); | 3910 | assert!(stored_text(&file, space).contains("Alone ")); |
| 3895 | closed(section); | 3911 | closed(section); |
| 3896 | } | 3912 | } |
| 3913 | |||
| 3914 | #[test] | ||
| 3915 | fn a_provider_that_lists_but_cannot_open_opens_from_the_offline_copies() { | ||
| 3916 | let (directory, root) = copy("m6/native-features-01/notebook"); | ||
| 3917 | let cache = directory.path().join("cache"); | ||
| 3918 | let library = provided(&root, true, &cache).unwrap(); | ||
| 3919 | let section = open(&library, "Features.one"); | ||
| 3920 | let space = typed(&section, "Before "); | ||
| 3921 | assert!(flushed(&library, 20.0)); | ||
| 3922 | closed(section); | ||
| 3923 | drop(library); | ||
| 3924 | // Listed otherwise than when last read, so discovery would read it again. | ||
| 3925 | let file = root.join("Features.one"); | ||
| 3926 | let touched = std::fs::metadata(&file).unwrap().modified().unwrap() + Duration::from_secs(5); | ||
| 3927 | std::fs::File::options() | ||
| 3928 | .write(true) | ||
| 3929 | .open(&file) | ||
| 3930 | .unwrap() | ||
| 3931 | .set_modified(touched) | ||
| 3932 | .unwrap(); | ||
| 3933 | provider_fake::unopenable(&root, true); | ||
| 3934 | |||
| 3935 | let library = provided(&root, true, &cache).unwrap(); | ||
| 3936 | assert!( | ||
| 3937 | library | ||
| 3938 | .tabs() | ||
| 3939 | .unwrap() | ||
| 3940 | .iter() | ||
| 3941 | .any(|tab| tab.path == "Features.one" && tab.readable) | ||
| 3942 | ); | ||
| 3943 | let section = open(&library, "Features.one"); | ||
| 3944 | let (page, _) = section.shared.page(space).unwrap(); | ||
| 3945 | assert!(canvas::search::page_text(&page).contains("Before ")); | ||
| 3946 | typed(&section, "Offline"); | ||
| 3947 | let (page, _) = section.shared.page(space).unwrap(); | ||
| 3948 | assert!(canvas::search::page_text(&page).contains("Offline")); | ||
| 3949 | assert!(eventually(|| library.sync_status().iter().any(|sync| { | ||
| 3950 | sync.path == "Features.one" | ||
| 3951 | && sync.state == SyncState::NotConnected as u8 | ||
| 3952 | && sync.queued == 1 | ||
| 3953 | }))); | ||
| 3954 | provider_fake::unopenable(&root, false); | ||
| 3955 | sb_library_sync_now(&library); | ||
| 3956 | assert!(flushed(&library, 20.0)); | ||
| 3957 | assert!(stored_text(&file, space).contains("Offline")); | ||
| 3958 | closed(section); | ||
| 3959 | } |
crates/notebook/README.md+2| ... | @@ -312,6 +312,8 @@ with one TOC identity, the one its parent's TOC names, else the newest, lists; t | ... | @@ -312,6 +312,8 @@ with one TOC identity, the one its parent's TOC names, else the newest, lists; t |
| 312 | list in `unavailable` as `Reason::Copy`. iOS's | 312 | list in `unavailable` as `Reason::Copy`. iOS's |
| 313 | `.Name.one.icloud` placeholders, and macOS's dataless files, list there under their real names | 313 | `.Name.one.icloud` placeholders, and macOS's dataless files, list there under their real names |
| 314 | as `Reason::Evicted`, unless listed unchanged since discovery last read them. | 314 | as `Reason::Evicted`, unless listed unchanged since discovery last read them. |
| 315 | A file listed but not reachable, as a cloud provider offline lists from its cache, stands as | ||
| 316 | it was last read, and is read again next time; one never read fails the discovery. | ||
| 315 | Retain the last accepted catalog if discovery fails; a connection failure does not mean | 317 | Retain the last accepted catalog if discovery fails; a connection failure does not mean |
| 316 | files were deleted. | 318 | files were deleted. |
| 317 | 319 |
crates/notebook/src/discover.rs+31-4| ... | @@ -369,7 +369,7 @@ fn scan( | ... | @@ -369,7 +369,7 @@ fn scan( |
| 369 | order: Vec::new(), | 369 | order: Vec::new(), |
| 370 | unavailable: Vec::new(), | 370 | unavailable: Vec::new(), |
| 371 | }; | 371 | }; |
| 372 | for entry in &listing { | 372 | 'entries: for entry in &listing { |
| 373 | let child = join(path, &entry.name); | 373 | let child = join(path, &entry.name); |
| 374 | if entry.kind == EntryKind::Directory { | 374 | if entry.kind == EntryKind::Directory { |
| 375 | if entry.name.to_ascii_lowercase().ends_with("_onefiles") { | 375 | if entry.name.to_ascii_lowercase().ends_with("_onefiles") { |
| ... | @@ -432,7 +432,7 @@ fn scan( | ... | @@ -432,7 +432,7 @@ fn scan( |
| 432 | found.read.insert(child.clone(), known.clone()); | 432 | found.read.insert(child.clone(), known.clone()); |
| 433 | (file_id, Ok(known.held.clone())) | 433 | (file_id, Ok(known.held.clone())) |
| 434 | } | 434 | } |
| 435 | None => { | 435 | None => 'read: { |
| 436 | let copied = cached | 436 | let copied = cached |
| 437 | .get(&child) | 437 | .get(&child) |
| 438 | .and_then(Read::file_id) | 438 | .and_then(Read::file_id) |
| ... | @@ -474,11 +474,21 @@ fn scan( | ... | @@ -474,11 +474,21 @@ fn scan( |
| 474 | let bytes = match read { | 474 | let bytes = match read { |
| 475 | Ok(bytes) => bytes, | 475 | Ok(bytes) => bytes, |
| 476 | Err(error) => { | 476 | Err(error) => { |
| 477 | // Listed but unreachable, as a cloud provider lists from its cache: the | ||
| 478 | // file as last read, read again next time. | ||
| 479 | if unreachable(&error) | ||
| 480 | && let Some((file_id, known)) = cached | ||
| 481 | .get(&child) | ||
| 482 | .and_then(|known| Some((known.file_id()?, known))) | ||
| 483 | { | ||
| 484 | found.read.insert(child.clone(), known.clone()); | ||
| 485 | break 'read (file_id, Ok(known.held.clone())); | ||
| 486 | } | ||
| 477 | list_unavailable(Error::Io { | 487 | list_unavailable(Error::Io { |
| 478 | path: child.clone(), | 488 | path: child.clone(), |
| 479 | error, | 489 | error, |
| 480 | })?; | 490 | })?; |
| 481 | continue; | 491 | continue 'entries; |
| 482 | } | 492 | } |
| 483 | }; | 493 | }; |
| 484 | if bytes.len() > limits.bytes_per_file { | 494 | if bytes.len() > limits.bytes_per_file { |
| ... | @@ -500,7 +510,7 @@ fn scan( | ... | @@ -500,7 +510,7 @@ fn scan( |
| 500 | path: child.clone(), | 510 | path: child.clone(), |
| 501 | error, | 511 | error, |
| 502 | })?; | 512 | })?; |
| 503 | continue; | 513 | continue 'entries; |
| 504 | } | 514 | } |
| 505 | }; | 515 | }; |
| 506 | let held = held(&store, expected); | 516 | let held = held(&store, expected); |
| ... | @@ -831,6 +841,23 @@ fn unavailable(error: &Error) -> Option<Reason> { | ... | @@ -831,6 +841,23 @@ fn unavailable(error: &Error) -> Option<Reason> { |
| 831 | } | 841 | } |
| 832 | } | 842 | } |
| 833 | 843 | ||
| 844 | /// Whether a file listed could not be read for want of a connection. | ||
| 845 | fn unreachable(error: &io::Error) -> bool { | ||
| 846 | use io::ErrorKind::*; | ||
| 847 | matches!( | ||
| 848 | error.kind(), | ||
| 849 | NotConnected | ||
| 850 | | TimedOut | ||
| 851 | | ConnectionRefused | ||
| 852 | | ConnectionReset | ||
| 853 | | ConnectionAborted | ||
| 854 | | HostUnreachable | ||
| 855 | | NetworkUnreachable | ||
| 856 | | NetworkDown | ||
| 857 | | BrokenPipe | ||
| 858 | ) | ||
| 859 | } | ||
| 860 | |||
| 834 | /// Not the notebook's: dot files, among them macOS's `.DS_Store` and AppleDouble `._` | 861 | /// Not the notebook's: dot files, among them macOS's `.DS_Store` and AppleDouble `._` |
| 835 | /// companions and Snowbound's `.snowbound` folder, and Office's `~$` owner files. | 862 | /// companions and Snowbound's `.snowbound` folder, and Office's `~$` owner files. |
| 836 | fn foreign(name: &str) -> bool { | 863 | fn foreign(name: &str) -> bool { |
crates/notebook/src/provider/tests.rs+40| ... | @@ -41,6 +41,8 @@ struct State { | ... | @@ -41,6 +41,8 @@ struct State { |
| 41 | next: u32, | 41 | next: u32, |
| 42 | writable: bool, | 42 | writable: bool, |
| 43 | offline: bool, | 43 | offline: bool, |
| 44 | /// Lists, from its cache, but opens nothing, as a cloud provider offline does. | ||
| 45 | unopenable: bool, | ||
| 44 | revoked: bool, | 46 | revoked: bool, |
| 45 | /// One device's writes reach the others only at `publish_staged`, as the lab's | 47 | /// One device's writes reach the others only at `publish_staged`, as the lab's |
| 46 | /// upload-on-close provider showed. | 48 | /// upload-on-close provider showed. |
| ... | @@ -80,6 +82,7 @@ impl Fake { | ... | @@ -80,6 +82,7 @@ impl Fake { |
| 80 | next: 0, | 82 | next: 0, |
| 81 | writable: true, | 83 | writable: true, |
| 82 | offline: false, | 84 | offline: false, |
| 85 | unopenable: false, | ||
| 83 | revoked: false, | 86 | revoked: false, |
| 84 | delayed: false, | 87 | delayed: false, |
| 85 | swap: None, | 88 | swap: None, |
| ... | @@ -246,6 +249,9 @@ impl Provider for Fake { | ... | @@ -246,6 +249,9 @@ impl Provider for Fake { |
| 246 | 249 | ||
| 247 | fn open(&self, id: &str, mode: &str) -> io::Result<File> { | 250 | fn open(&self, id: &str, mode: &str) -> io::Result<File> { |
| 248 | let mut state = self.reachable()?; | 251 | let mut state = self.reachable()?; |
| 252 | if state.unopenable { | ||
| 253 | return Err(io::Error::new(io::ErrorKind::NotConnected, "Offline")); | ||
| 254 | } | ||
| 249 | if !state.nodes.contains_key(id) { | 255 | if !state.nodes.contains_key(id) { |
| 250 | return Err(io::ErrorKind::NotFound.into()); | 256 | return Err(io::ErrorKind::NotFound.into()); |
| 251 | } | 257 | } |
| ... | @@ -806,3 +812,37 @@ fn a_lone_document_opens_offline_from_its_replica() { | ... | @@ -806,3 +812,37 @@ fn a_lone_document_opens_offline_from_its_replica() { |
| 806 | assert_eq!(stored(fake.bytes("Lone.one")), "Not Alone now"); | 812 | assert_eq!(stored(fake.bytes("Lone.one")), "Not Alone now"); |
| 807 | section.close().unwrap(); | 813 | section.close().unwrap(); |
| 808 | } | 814 | } |
| 815 | |||
| 816 | #[test] | ||
| 817 | fn a_provider_that_lists_but_cannot_open_opens_sections_from_their_replicas() { | ||
| 818 | let fake = Fake::new(Kind::Seekable); | ||
| 819 | notebook(&fake); | ||
| 820 | let cache = tempfile::tempdir().unwrap(); | ||
| 821 | let documents = fake.documents("root"); | ||
| 822 | let notebook = Notebook::open_provider(Arc::clone(&documents), cache.path()).unwrap(); | ||
| 823 | open(&notebook, &documents, "Notes.one").close().unwrap(); | ||
| 824 | drop(notebook); | ||
| 825 | // Another device changes the section, so it lists otherwise than when last read. | ||
| 826 | let elsewhere = fake.device(1).documents("root"); | ||
| 827 | let other_cache = tempfile::tempdir().unwrap(); | ||
| 828 | let other = Notebook::open_provider(Arc::clone(&elsewhere), other_cache.path()).unwrap(); | ||
| 829 | let other = open(&other, &elsewhere, "Notes.one"); | ||
| 830 | let id = typed(&other, 0..0, "Theirs "); | ||
| 831 | published(&other, id); | ||
| 832 | other.close().unwrap(); | ||
| 833 | fake.set(|state| state.unopenable = true); | ||
| 834 | |||
| 835 | let documents = fake.documents("root"); | ||
| 836 | let notebook = Notebook::open_provider(Arc::clone(&documents), cache.path()).unwrap(); | ||
| 837 | assert_eq!(notebook.catalog().order, ["Notes.one", "Ideas.one"]); | ||
| 838 | let section = open(&notebook, &documents, "Notes.one"); | ||
| 839 | assert_eq!(text(&section), "Original text"); | ||
| 840 | let id = typed(&section, 13..13, " mine"); | ||
| 841 | status(&section, |status| { | ||
| 842 | status.state() == SyncState::NotConnected && status.queued == 1 | ||
| 843 | }); | ||
| 844 | fake.set(|state| state.unopenable = false); | ||
| 845 | published(&section, id); | ||
| 846 | assert_eq!(stored(fake.bytes("Notes.one")), "Theirs Original text mine"); | ||
| 847 | section.close().unwrap(); | ||
| 848 | } |