| author | |
| committer | |
| log | 9c8254dbc5a7af05da2ef7a0b1835916f3c697ba |
| tree | 899f3fb09ca619fc4ddc62e16b3220c1d317426a |
| parent | 43477347100b0d4076ef55ad09b7e6866a33b1d0 |
| signature | Signed by SSH key SHA256:52mNGHRsVFBDED9IAX5pe+LRWUefqTbxEReunq21QvU (~clover) |
Wait for the shown section, kept sections, and background synchronization before renaming a notebook. Prove every root and copied-section replica is exclusively available before moving source files, including browser receipts retained after eviction. Browser rename retries while those owners finish; no cache connection or mutex spans that wait.
Extend the actual WASM receipt probe with a dropped-owner rename refusal and asynchronous recovery preserving the queued edit and replica UUID. Add a native refusal-before-source-move regression.
Fixes #98
Assisted-by: gpt-6.1-sol5 files changed, 179 insertions(+), 23 deletions(-)
crates/notebook/examples/browser_receipts.rs+69| ... | @@ -177,5 +177,74 @@ mod web { | ... | @@ -177,5 +177,74 @@ mod web { |
| 177 | }; | 177 | }; |
| 178 | assert_eq!(reopened.pending().unwrap().len(), 1); | 178 | assert_eq!(reopened.pending().unwrap().len(), 1); |
| 179 | assert_eq!(text(&reopened.snapshot().unwrap()).2, "once original"); | 179 | assert_eq!(text(&reopened.snapshot().unwrap()).2, "once original"); |
| 180 | drop(reopened); | ||
| 181 | |||
| 182 | phase("renaming past an evicted pending receipt"); | ||
| 183 | let cache = std::path::Path::new("/receipts/rename-cache"); | ||
| 184 | let page = onestore::PageCreation::new(None, Some("original"), "Fixture").unwrap(); | ||
| 185 | let notebook = notebook::session::Notebook::create( | ||
| 186 | "/receipts/Before", | ||
| 187 | cache, | ||
| 188 | notebook::session::Notebook::NEW_COLOR, | ||
| 189 | &page, | ||
| 190 | ) | ||
| 191 | .unwrap(); | ||
| 192 | let original_cache = notebook.replica_path("New Section 1.one").unwrap(); | ||
| 193 | let section = notebook.section("New Section 1.one", || {}).unwrap(); | ||
| 194 | section.set_offline(true); | ||
| 195 | let (space, text_id, _) = text(&section.replica().snapshot().unwrap()); | ||
| 196 | let replica = Arc::clone(section.replica()); | ||
| 197 | fail_storage(1); | ||
| 198 | { | ||
| 199 | let mut applying = std::pin::pin!(replica.apply_async( | ||
| 200 | "Fixture", | ||
| 201 | Edit { | ||
| 202 | at, | ||
| 203 | ops: vec![Op::Page { | ||
| 204 | space, | ||
| 205 | op: PageOp::Text { | ||
| 206 | text: text_id, | ||
| 207 | range: 0..0, | ||
| 208 | with: "once ".into(), | ||
| 209 | }, | ||
| 210 | }], | ||
| 211 | } | ||
| 212 | )); | ||
| 213 | assert!(matches!( | ||
| 214 | applying | ||
| 215 | .as_mut() | ||
| 216 | .poll(&mut Context::from_waker(Waker::noop())), | ||
| 217 | Poll::Pending | ||
| 218 | )); | ||
| 219 | } | ||
| 220 | section.close().unwrap(); | ||
| 221 | drop(replica); | ||
| 222 | next_turn().await; | ||
| 223 | match notebook.rename_folder("Refused") { | ||
| 224 | Err(error) => assert!(error.busy()), | ||
| 225 | Ok(_) => panic!("A notebook moved before its replica closed"), | ||
| 226 | } | ||
| 227 | assert!(notebook::fs::metadata("/receipts/Before").is_ok()); | ||
| 228 | assert!(notebook::fs::metadata("/receipts/Refused").is_err()); | ||
| 229 | let notebook = notebook::session::Notebook::open("/receipts/Before", cache).unwrap(); | ||
| 230 | let mut renaming = std::pin::pin!(notebook.rename_folder_async("After")); | ||
| 231 | assert!(matches!( | ||
| 232 | renaming | ||
| 233 | .as_mut() | ||
| 234 | .poll(&mut Context::from_waker(Waker::noop())), | ||
| 235 | Poll::Pending | ||
| 236 | )); | ||
| 237 | next_turn().await; | ||
| 238 | assert!(notebook::fs::metadata("/receipts/Before").is_ok()); | ||
| 239 | assert!(notebook::fs::metadata("/receipts/After").is_err()); | ||
| 240 | fail_storage(0); | ||
| 241 | renaming.await.unwrap(); | ||
| 242 | let notebook = notebook::session::Notebook::open("/receipts/After", cache).unwrap(); | ||
| 243 | let moved_cache = notebook.replica_path("New Section 1.one").unwrap(); | ||
| 244 | assert_eq!(original_cache.file_name(), moved_cache.file_name()); | ||
| 245 | assert!(notebook::fs::metadata(original_cache).is_err()); | ||
| 246 | let replica = Replica::open(moved_cache).unwrap(); | ||
| 247 | assert_eq!(replica.pending().unwrap().len(), 1); | ||
| 248 | assert_eq!(text(&replica.snapshot().unwrap()).2, "once original"); | ||
| 180 | } | 249 | } |
| 181 | } | 250 | } |
crates/notebook/src/background.rs+15| ... | @@ -521,6 +521,21 @@ impl Background { | ... | @@ -521,6 +521,21 @@ impl Background { |
| 521 | drop(thread); | 521 | drop(thread); |
| 522 | } | 522 | } |
| 523 | 523 | ||
| 524 | /// Stops future checks and awaits the current step before files can move. | ||
| 525 | pub async fn stop_async(&self) { | ||
| 526 | #[cfg(not(target_arch = "wasm32"))] | ||
| 527 | self.stop(); | ||
| 528 | #[cfg(target_arch = "wasm32")] | ||
| 529 | { | ||
| 530 | self.0.signal.stopped.store(true, Ordering::Release); | ||
| 531 | self.0.signal.wake(); | ||
| 532 | let thread = self.1.lock().ok().and_then(|mut thread| thread.take()); | ||
| 533 | if let Some(thread) = thread { | ||
| 534 | let _ = thread.join().await; | ||
| 535 | } | ||
| 536 | } | ||
| 537 | } | ||
| 538 | |||
| 524 | /// Working offline, nothing is checked until `wake`, or until working online again, which | 539 | /// Working offline, nothing is checked until `wake`, or until working online again, which |
| 525 | /// lists every folder again. | 540 | /// lists every folder again. |
| 526 | pub fn set_offline(&self, offline: bool) { | 541 | pub fn set_offline(&self, offline: bool) { |
crates/notebook/src/session.rs+46| ... | @@ -839,6 +839,7 @@ impl Notebook { | ... | @@ -839,6 +839,7 @@ impl Notebook { |
| 839 | if !folder_name(name) { | 839 | if !folder_name(name) { |
| 840 | return Err(io::Error::from(io::ErrorKind::InvalidInput).into()); | 840 | return Err(io::Error::from(io::ErrorKind::InvalidInput).into()); |
| 841 | } | 841 | } |
| 842 | self.replicas_closed()?; | ||
| 842 | let from = self.storage.location(); | 843 | let from = self.storage.location(); |
| 843 | let files: Vec<String> = (self.catalog.folders()) | 844 | let files: Vec<String> = (self.catalog.folders()) |
| 844 | .flat_map(|folder| { | 845 | .flat_map(|folder| { |
| ... | @@ -868,6 +869,51 @@ impl Notebook { | ... | @@ -868,6 +869,51 @@ impl Notebook { |
| 868 | Ok(to) | 869 | Ok(to) |
| 869 | } | 870 | } |
| 870 | 871 | ||
| 872 | /// Waits for browser cache receipts before moving the notebook and its replicas. | ||
| 873 | pub async fn rename_folder_async(self, name: &str) -> Result<String> { | ||
| 874 | #[cfg(target_arch = "wasm32")] | ||
| 875 | { | ||
| 876 | let deadline = web_time::Instant::now() + Duration::from_secs(30); | ||
| 877 | let (_, retry) = crate::task::channel(); | ||
| 878 | loop { | ||
| 879 | match self.replicas_closed() { | ||
| 880 | Err(error) if error.busy() && web_time::Instant::now() < deadline => { | ||
| 881 | crate::task::wait(&retry, Some(Duration::from_millis(50))).await; | ||
| 882 | } | ||
| 883 | result => { | ||
| 884 | result?; | ||
| 885 | break; | ||
| 886 | } | ||
| 887 | } | ||
| 888 | } | ||
| 889 | } | ||
| 890 | self.rename_folder(name) | ||
| 891 | } | ||
| 892 | |||
| 893 | fn replicas_closed(&self) -> Result<()> { | ||
| 894 | let from = self.storage.location(); | ||
| 895 | let locations = std::iter::once(from.clone()).chain( | ||
| 896 | self.catalog | ||
| 897 | .sections() | ||
| 898 | .filter(|section| section.copy) | ||
| 899 | .map(|section| replica_location(&from, section)), | ||
| 900 | ); | ||
| 901 | for location in locations { | ||
| 902 | let entries = match fs::read_dir(crate::location::folder(&self.cache, &location)) { | ||
| 903 | Ok(entries) => entries, | ||
| 904 | Err(error) if error.kind() == io::ErrorKind::NotFound => continue, | ||
| 905 | Err(error) => return Err(error.into()), | ||
| 906 | }; | ||
| 907 | for entry in entries { | ||
| 908 | let path = entry?.path(); | ||
| 909 | if path.extension() == Some("sqlite".as_ref()) { | ||
| 910 | drop(crate::closed(&path)?); | ||
| 911 | } | ||
| 912 | } | ||
| 913 | } | ||
| 914 | Ok(()) | ||
| 915 | } | ||
| 916 | |||
| 871 | /// Sets a section's colour (COLORREF) in its own metadata, where OneNote keeps it. | 917 | /// Sets a section's colour (COLORREF) in its own metadata, where OneNote keeps it. |
| 872 | pub fn set_section_color(&mut self, path: &str, color: Option<u32>) -> Result<()> { | 918 | pub fn set_section_color(&mut self, path: &str, color: Option<u32>) -> Result<()> { |
| 873 | let path = self.section_path(path)?.path.clone(); | 919 | let path = self.section_path(path)?.path.clone(); |
crates/snowbound/src/library.rs+25-10| ... | @@ -548,17 +548,23 @@ impl Library { | ... | @@ -548,17 +548,23 @@ impl Library { |
| 548 | /// place of a display name. Refused, with nothing changed, while another writer holds a file | 548 | /// place of a display name. Refused, with nothing changed, while another writer holds a file |
| 549 | /// of it. Returns the notebook's new location, where it opens again, or why not, as its | 549 | /// of it. Returns the notebook's new location, where it opens again, or why not, as its |
| 550 | /// reader is told. | 550 | /// reader is told. |
| 551 | pub fn rename_folder(&self, name: &str) -> Result<String, String> { | 551 | pub async fn rename_folder(&self, name: &str) -> Result<String, String> { |
| 552 | let to = self | 552 | let to = self |
| 553 | .renamed_location(name) | 553 | .renamed_location(name) |
| 554 | .ok_or("This notebook’s folder can’t be renamed.")?; | 554 | .ok_or("This notebook’s folder can’t be renamed.")?; |
| 555 | if let Some(background) = &self.background { | 555 | if let Some(background) = &self.background { |
| 556 | background.stop(); | 556 | background.stop_async().await; |
| 557 | } | 557 | } |
| 558 | self.close_kept(); | 558 | self.close_kept_async() |
| 559 | let renamed = self | 559 | .await |
| 560 | .reopen() | 560 | .map_err(|error| crate::plain(&*error, "section"))?; |
| 561 | .and_then(|notebook| Ok(notebook.rename_folder(name)?)); | 561 | let renamed = match self.reopen() { |
| 562 | Ok(notebook) => notebook | ||
| 563 | .rename_folder_async(name) | ||
| 564 | .await | ||
| 565 | .map_err(|error| Box::new(error) as Box<dyn Error>), | ||
| 566 | Err(error) => Err(error), | ||
| 567 | }; | ||
| 562 | if let Err(error) = renamed { | 568 | if let Err(error) = renamed { |
| 563 | return Err(match crate::io_kind(&*error) { | 569 | return Err(match crate::io_kind(&*error) { |
| 564 | Some(io::ErrorKind::WouldBlock | io::ErrorKind::ResourceBusy) => { | 570 | Some(io::ErrorKind::WouldBlock | io::ErrorKind::ResourceBusy) => { |
| ... | @@ -2108,7 +2114,7 @@ mod tests { | ... | @@ -2108,7 +2114,7 @@ mod tests { |
| 2108 | section.apply("Rust Author", typed).unwrap(); | 2114 | section.apply("Rust Author", typed).unwrap(); |
| 2109 | section.close().unwrap(); | 2115 | section.close().unwrap(); |
| 2110 | assert_eq!( | 2116 | assert_eq!( |
| 2111 | library.rename_folder("After").unwrap(), | 2117 | pollster::block_on(library.rename_folder("After")).unwrap(), |
| 2112 | mount("After").url() | 2118 | mount("After").url() |
| 2113 | ); | 2119 | ); |
| 2114 | assert!(client.read_dir(&mount("Before").root, 10).is_err()); | 2120 | assert!(client.read_dir(&mount("Before").root, 10).is_err()); |
| ... | @@ -2154,11 +2160,20 @@ mod tests { | ... | @@ -2154,11 +2160,20 @@ mod tests { |
| 2154 | let location = |name: &str| directory.join(name).to_string_lossy().into_owned(); | 2160 | let location = |name: &str| directory.join(name).to_string_lossy().into_owned(); |
| 2155 | let library = Library::notebook(&location("Mine"), &cache); | 2161 | let library = Library::notebook(&location("Mine"), &cache); |
| 2156 | library.set_display_name("Shown").unwrap(); | 2162 | library.set_display_name("Shown").unwrap(); |
| 2157 | let refused = library.rename_folder("Taken").unwrap_err(); | 2163 | let held = library.open("New Section 1.one", || {}).unwrap(); |
| 2164 | held.set_offline(true); | ||
| 2165 | assert!(pollster::block_on(library.rename_folder("Busy")).is_err()); | ||
| 2166 | assert!(notebook::fs::metadata(location("Mine")).is_ok()); | ||
| 2167 | assert!(notebook::fs::metadata(location("Busy")).is_err()); | ||
| 2168 | held.close().unwrap(); | ||
| 2169 | let refused = pollster::block_on(library.rename_folder("Taken")).unwrap_err(); | ||
| 2158 | assert!(refused.starts_with("A folder named “Taken”"), "{refused}"); | 2170 | assert!(refused.starts_with("A folder named “Taken”"), "{refused}"); |
| 2159 | let library = Library::notebook(&location("Mine"), &cache); | 2171 | let library = Library::notebook(&location("Mine"), &cache); |
| 2160 | assert_eq!(library.name, "Shown"); | 2172 | assert_eq!(library.name, "Shown"); |
| 2161 | assert_eq!(library.rename_folder("Ours").unwrap(), location("Ours")); | 2173 | assert_eq!( |
| 2174 | pollster::block_on(library.rename_folder("Ours")).unwrap(), | ||
| 2175 | location("Ours") | ||
| 2176 | ); | ||
| 2162 | assert!(notebook::fs::metadata(location("Mine")).is_err()); | 2177 | assert!(notebook::fs::metadata(location("Mine")).is_err()); |
| 2163 | let renamed = Library::notebook(&location("Ours"), &cache); | 2178 | let renamed = Library::notebook(&location("Ours"), &cache); |
| 2164 | assert_eq!(renamed.name, "Ours"); | 2179 | assert_eq!(renamed.name, "Ours"); |
crates/snowbound/src/manage.rs+24-13| ... | @@ -718,18 +718,18 @@ impl State { | ... | @@ -718,18 +718,18 @@ impl State { |
| 718 | .as_ref() | 718 | .as_ref() |
| 719 | .filter(|session| ours(&session.library)) | 719 | .filter(|session| ours(&session.library)) |
| 720 | .map(|session| session.tabs[session.tab].path.clone()); | 720 | .map(|session| session.tabs[session.tab].path.clone()); |
| 721 | if shown.is_some() { | 721 | if shown.is_some() |
| 722 | let closed = self.persist().and_then(|()| match self.session.take() { | 722 | && let Err(error) = self.persist() |
| 723 | Some(session) => Ok(session.section.close()?), | 723 | { |
| 724 | None => Ok(()), | 724 | return platform::alert( |
| 725 | }); | 725 | "Couldn't rename the folder", |
| 726 | if let Err(error) = closed { | 726 | &crate::plain(&*error, "section"), |
| 727 | return platform::alert( | 727 | ); |
| 728 | "Couldn't rename the folder", | ||
| 729 | &crate::plain(&*error, "section"), | ||
| 730 | ); | ||
| 731 | } | ||
| 732 | } | 728 | } |
| 729 | let closing = shown | ||
| 730 | .as_ref() | ||
| 731 | .and_then(|_| self.session.take()) | ||
| 732 | .map(|session| session.section); | ||
| 733 | let showing = shown.is_some() || self.sectionless.as_ref().is_some_and(ours); | 733 | let showing = shown.is_some() || self.sectionless.as_ref().is_some_and(ours); |
| 734 | if showing { | 734 | if showing { |
| 735 | self.sectionless = Some(Arc::clone(&library)); | 735 | self.sectionless = Some(Arc::clone(&library)); |
| ... | @@ -737,8 +737,19 @@ impl State { | ... | @@ -737,8 +737,19 @@ impl State { |
| 737 | } | 737 | } |
| 738 | self.folder_rename = Some((library.location.clone(), to.clone())); | 738 | self.folder_rename = Some((library.location.clone(), to.clone())); |
| 739 | let (cache, proxy) = (self.cache.clone(), self.proxy.clone()); | 739 | let (cache, proxy) = (self.cache.clone(), self.proxy.clone()); |
| 740 | crate::spawn(move || { | 740 | crate::spawn_async(move || async move { |
| 741 | let renamed = library.rename_folder(&name); | 741 | let structure = Arc::clone(&library.structure); |
| 742 | let _structure = structure.lock().await; | ||
| 743 | let renamed = async { | ||
| 744 | if let Some(section) = closing { | ||
| 745 | section | ||
| 746 | .close_async() | ||
| 747 | .await | ||
| 748 | .map_err(|error| crate::plain(&error, "section"))?; | ||
| 749 | } | ||
| 750 | library.rename_folder(&name).await | ||
| 751 | } | ||
| 752 | .await; | ||
| 742 | // A failure after the folder moved, as its replicas followed, leaves it there. | 753 | // A failure after the folder moved, as its replicas followed, leaves it there. |
| 743 | let moved = library | 754 | let moved = library |
| 744 | .folder() | 755 | .folder() |