From 24b46353a9b3963f2093a92d493d7cb5229416db Mon Sep 17 00:00:00 2001 From: clover caruso Date: Mon, 12 Oct 2026 03:35:54 -0700 Subject: [PATCH] fix(mobile): require notebook backups before page deletion A cached SMB section can reopen while its notebook catalog is unavailable. Classify the stored notebook location so backup failure returns before source Delete is submitted. Cover original cached page and queued edits, canonical notebook recycling, and standalone last-page replacement. Refs #133 Assisted-by: gpt-6.1-sol --- crates/mobile/src/library.rs | 102 ++++++++++++++++++++++++++++++++--- crates/mobile/src/tests.rs | 47 +++++++++++++++- 2 files changed, 140 insertions(+), 9 deletions(-) diff --git a/crates/mobile/src/library.rs b/crates/mobile/src/library.rs index 1bac8980c907b6d14fd41ca030c33a03fb6af662..a5e578218f4f8d258ad6d83131cc8394234e935f 100644 --- a/crates/mobile/src/library.rs +++ b/crates/mobile/src/library.rs @@ -278,7 +278,7 @@ struct Checked { } pub struct Library { - /// `None` for a lone section. + /// A share's catalog may be absent while cached sections remain available. notebook: Mutex>, place: Place, cache: PathBuf, @@ -1020,14 +1020,14 @@ impl Section { Ok(space) } - /// Deletes a page as OneNote 2010 does, keeping a copy in the notebook's recycle bin; - /// a conflict page goes for good. A section left empty gets a new page. + /// Recycles an ordinary notebook page before submitting its deletion; a lone section's + /// page or a conflict page goes for good. A section left empty gets a new page. pub(crate) fn delete_page(&self, space: ExGuid, date: &str, time: &str) -> Result<()> { let section = &self.shared.section; let listed = section.pages()?; let mut ops = vec![Op::Section(SectionOp::Delete(vec![space]))]; if listed.iter().any(|(page, ..)| *page == space) { - if self.library.notebook().is_some() { + if !matches!(self.library.place, Place::File(_)) { let page = section.page(space)?; let author = self.shared.author(); self.library.with_notebook(true, |notebook| { @@ -2351,9 +2351,9 @@ pub unsafe extern "C" fn sb_section_set_theme( report(set).is_some() } -/// Deletes page `id` to the notebook's recycle bin, or a conflict page for good; a section -/// left without pages gets one titled `date` and `time`. Writes to the notebook, so call it -/// off the main thread. +/// Recycles notebook page `id` before submitting its deletion; a lone section's page or a +/// conflict page goes for good. A section left empty gets one titled `date` and `time`. +/// Writes to the notebook, so call it off the main thread. /// /// # Safety /// Every string is NUL-terminated UTF-8. @@ -2437,3 +2437,91 @@ pub extern "C" fn sb_section_wake(section: &Section) { pub unsafe extern "C" fn sb_section_free(section: *mut Section) { drop(unsafe { Box::from_raw(section) }); } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn deleting_a_cold_offline_share_page_preserves_its_source_and_queue() { + let directory = tempfile::tempdir().unwrap(); + let root = directory.path().join("notebook"); + let cache = directory.path().join("cache"); + std::fs::create_dir(&root).unwrap(); + let bytes = + onestore::create_section("Read.one", "Offline source content", "Fixture").unwrap(); + std::fs::write(root.join("Read.one"), &bytes).unwrap(); + let notebook = Notebook::open(&root, &cache).unwrap(); + let mut listed = Listed { + tabs: Vec::new(), + files: BTreeMap::new(), + }; + tabs(notebook.catalog(), &HashMap::new(), &mut listed.tabs); + files(notebook.catalog(), &mut listed.files); + let identity = listed.files["Read.one"].clone(); + drop(notebook); + + let address = "127.0.0.1:0"; + let location = notebook::location::smb(address, "agent", "Notebook"); + let replicas = notebook::location::folder(&cache, &location); + std::fs::create_dir_all(&replicas).unwrap(); + let original = replicas.join(format!("{identity}.sqlite")); + let replica = Replica::create(&original, &bytes).unwrap(); + let space = replica.pages().unwrap()[0].0; + replica + .apply( + "Fixture", + Edit { + at: filetime(), + ops: vec![Op::Page { + space, + op: onestore::op::PageOp::Color(Some(0x00ef_cdab)), + }], + }, + ) + .unwrap(); + let page = replica.page(space).unwrap(); + let pending = replica.pending().unwrap(); + assert_eq!(pending.len(), 1); + drop(replica); + let listing = cache.join("smb"); + std::fs::create_dir_all(&listing).unwrap(); + std::fs::write( + listing.join("127_0_0_1_0_agent_Notebook.json"), + serde_json::to_vec(&listed).unwrap(), + ) + .unwrap(); + + let library = Arc::new( + Library::server( + Server { + address: address.into(), + share: "agent".into(), + user: String::new(), + password: String::new(), + domain: String::new(), + }, + "Notebook", + &cache, + ) + .unwrap(), + ); + sb_library_set_offline(&library, true); + assert!(library.notebook().is_none()); + let section = + Section::open(Arc::clone(&library), "Read.one", "Fixture".into(), || {}).unwrap(); + assert_eq!(section.shared.section.page(space).unwrap(), page); + assert_eq!(section.shared.section.pending().unwrap(), pending); + assert_eq!( + hex(§ion.shared.section.replica().identity().unwrap()), + identity + ); + + assert!(section.delete_page(space, "Date", "Time").is_err()); + section.shared.section.written().unwrap(); + assert!(library.notebook().is_none()); + assert_eq!(section.shared.section.page(space).unwrap(), page); + assert_eq!(section.shared.section.pending().unwrap(), pending); + assert_eq!(std::fs::read(root.join("Read.one")).unwrap(), bytes); + } +} diff --git a/crates/mobile/src/tests.rs b/crates/mobile/src/tests.rs index f6927a989570a21116cb0736d433679bc87e2814..6d88eff11839e4452a320081f4643c7cd4d7213f 100644 --- a/crates/mobile/src/tests.rs +++ b/crates/mobile/src/tests.rs @@ -491,6 +491,7 @@ fn a_new_page_and_subpage_list_after_their_parent_and_delete_to_the_recycle_bin( .shared .apply(canvas.edit().unwrap().unwrap()) .unwrap(); + section.shared.section.written().unwrap(); assert!( section .rows() @@ -498,9 +499,12 @@ fn a_new_page_and_subpage_list_after_their_parent_and_delete_to_the_recycle_bin( .iter() .any(|row| row.title == "Sketches") ); + let deleted = section.shared.section.page(subpage).unwrap(); + assert!(deleted.identity.is_some()); section .delete_page(subpage, "Sunday, September 27, 2026", "3:06 AM") .unwrap(); + section.shared.section.written().unwrap(); assert!( section .rows() @@ -508,12 +512,40 @@ fn a_new_page_and_subpage_list_after_their_parent_and_delete_to_the_recycle_bin( .iter() .all(|row| row.id != subpage.to_string()) ); + let bytes = std::fs::read(root.join("OneNote_RecycleBin/OneNote_DeletedPages.one")).unwrap(); assert!( - root.join("OneNote_RecycleBin/OneNote_DeletedPages.one") - .exists() + notebook::session::stored_pages(&bytes) + .unwrap() + .iter() + .any(|stored| stored.page.identity == deleted.identity + && stored.page.title == "Sketches") ); } +#[test] +fn deleting_a_lone_section_page_still_creates_its_replacement_without_a_bin() { + let directory = tempfile::tempdir().unwrap(); + let file = directory.path().join("Lone.one"); + std::fs::write( + &file, + onestore::create_section("Lone.one", "Standalone page", "Fixture").unwrap(), + ) + .unwrap(); + let library = Arc::new(Library::open(&file, &directory.path().join("cache"), true).unwrap()); + sb_library_set_offline(&library, true); + let section = open(&library, &file.to_string_lossy()); + let space = section.shared.section.pages().unwrap()[0].0; + section + .delete_page(space, "Sunday, September 27, 2026", "3:06 AM") + .unwrap(); + section.shared.section.written().unwrap(); + let pages = section.shared.section.pages().unwrap(); + assert_eq!(pages.len(), 1); + assert_ne!(pages[0].0, space); + assert!(section.shared.section.page(space).is_err()); + assert!(!directory.path().join("OneNote_RecycleBin").exists()); +} + #[test] fn conflict_pages_list_under_their_page_and_take_no_edits() { let (directory, root) = copy("conflict-page/candidate"); @@ -825,6 +857,17 @@ fn a_notebook_on_a_share_saves_and_opens_offline() { .unwrap() .0; assert!(canvas::search::page_text(&page).contains("Shared ")); + sb_library_set_offline(&offline, true); + let space = space(&reopened, "Paragraph controls"); + let pending = reopened.shared.section.pending().unwrap(); + assert!( + reopened + .delete_page(space, "Sunday, September 27, 2026", "3:06 AM") + .is_err() + ); + reopened.shared.section.written().unwrap(); + assert_eq!(reopened.shared.section.page(space).unwrap(), page); + assert_eq!(reopened.shared.section.pending().unwrap(), pending); client.delete(&format!("{root}/Features.one")).unwrap(); client.delete(&root).unwrap(); } -- 2.54.0