authorclover caruso <git@paperclover.net> 2026-10-15 05:55:27-07:00
committerclover caruso <git@paperclover.net> 2026-10-15 06:17:43-07:00
logc830e2971fbbcfeb96cfb5286d43d88e0edff1d5
tree1629ab84ffd4ebf8e5e7b41359a66a83cb8f3d52
parent0c15feff0434402e533f3d088940143f3bb4cfa5
signature Signed by SSH key SHA256:52mNGHRsVFBDED9IAX5pe+LRWUefqTbxEReunq21QvU (~clover)

fix: a move that only copies a page says so and keeps the history whole

A move reads the page together with its conflict pages and versions, which no copy carries, and retires it only while all three are as read, so a conflict page kept or a version made during the move is no longer deleted with the page. The API is named for moves: Replica::moving reads the page, and Replica::retire deletes it, refusing with "The page changed while it moved." otherwise. Once the copy has landed, any failure to retire the original, whether a change or a cache that cannot write, leaves the page in both sections. The desktop then shows "Page kept in both sections" with the reason and records no step that cannot be taken back, and an Undo of a move that ends this way shows the copy it brought back. iOS reports the same through the copied field of sb_library_move_page and its own alert. A desktop move no longer fails after its copy when the target section still has edits waiting for the cache: the section is kept open, retrying them, rather than closed. Refs #138 Assisted-by: claude-opus-5.5

13 files changed, 395 insertions(+), 126 deletions(-)

Cargo.lock+1
......@@ -3649,6 +3649,7 @@ dependencies = [
36493649 "png",
36503650 "pollster",
36513651 "ring",
3652 "rusqlite",
36523653 "rustc-demangle",
36533654 "rustls-native-certs",
36543655 "sctk-adwaita",
apps/ios/Snowbound/Arranging.swift+6-1
......@@ -12,6 +12,8 @@ private struct MovedPage: Decodable {
1212 let added: String?
1313 let before: String?
1414 let level: Int
15 /// Why the page also stayed where it was: the move only copied it.
16 let copied: String?
1517}
1618
1719/// The catalog path of the folder holding the section or group at `path`; "" for the
......@@ -155,6 +157,7 @@ enum Arranging {
155157 section.reloadRows()
156158 }
157159 if moved == nil { failed("Can’t Move Page") }
160 if let copied = moved?.copied { failed("Page Kept in Both Sections", copied) }
158161 }
159162 }
160163
......@@ -244,7 +247,9 @@ enum Arranging {
244247 release()
245248 source.reloadRows()
246249 target.reloadRows()
247 if !succeeded {
250 if let copied = reported.lazy.compactMap(\.copied).first {
251 failed("Page Kept in Both Sections", copied)
252 } else if !succeeded {
248253 failed("Can’t Move All Pages", reported.isEmpty
249254 ? "Check both sections, then try moving the remaining pages again."
250255 : "Check both sections. Use \(undoing ? "Redo" : "Undo") to reverse completed moves, or try moving the remaining pages again.")
crates/mobile/src/library.rs+16-10
......@@ -1387,9 +1387,9 @@ impl Library {
13871387 let at = (listed.iter().position(|(listed, ..)| *listed == space))
13881388 .ok_or("The page is not listed")?;
13891389 let next = listed.get(at + 1).map(|(next, ..)| *next);
1390 Ok((section.page(space)?, (next, listed[at].2)))
1390 Ok((section.replica().moving(space)?, (next, listed[at].2)))
13911391 })?;
1392 let import = session::moved(&page, author)?;
1392 let import = session::moved(page.page(), author)?;
13931393 let Op::Section(SectionOp::Import { creation, .. }) = &import else {
13941394 unreachable!("notebook::session::moved imports");
13951395 };
......@@ -1415,7 +1415,7 @@ impl Library {
14151415 };
14161416 Ok(section.replica().apply(author, edit)?)
14171417 })?;
1418 let added = self.with_section(from, |section| {
1418 let retired = self.with_section(from, |section| {
14191419 let mut ops = vec![Op::Section(SectionOp::Delete(vec![space]))];
14201420 let added = match section.pages()?.as_slice() {
14211421 [(only, ..)] if *only == space => {
......@@ -1430,17 +1430,20 @@ impl Library {
14301430 at: filetime(),
14311431 ops,
14321432 };
1433 section
1434 .replica()
1435 .apply_unchanged(author, edit, space, page)?;
1433 section.replica().retire(author, edit, page)?;
14361434 Ok(added)
1437 })?;
1435 });
14381436 let id = |space: Option<ExGuid>| space.map(|space| space.to_string());
1437 let (added, copied) = match retired {
1438 Ok(added) => (id(added), None),
1439 Err(error) => (None, Some(error.to_string())),
1440 };
14391441 Ok(MovedPage {
14401442 page: arrived.to_string(),
1441 added: id(added),
1443 added,
14421444 before: id(was.0),
14431445 level: was.1,
1446 copied,
14441447 })
14451448 }
14461449
......@@ -1481,6 +1484,8 @@ pub(crate) struct MovedPage {
14811484 pub(crate) added: Option<String>,
14821485 pub(crate) before: Option<String>,
14831486 pub(crate) level: u32,
1487 /// Why the page also stayed where it was: the move only copied it.
1488 pub(crate) copied: Option<String>,
14841489}
14851490
14861491#[derive(serde::Serialize)]
......@@ -1937,8 +1942,9 @@ pub unsafe extern "C" fn sb_library_new_section(
19371942/// before page `before` at `level`, or last with `before` null; `discard`, unless null, is
19381943/// the page `to` gained when this page left it. Returns JSON with `page`, its id there,
19391944/// `added`, the page titled `date` and `time` that `from` gained, or null, and `before` and
1940/// `level`, where it was in `from`; null when it cannot move. Writes to both sections, so
1941/// call it off the main thread.
1945/// `level`, where it was in `from`, and `copied`, why it also stayed in `from` where the move
1946/// only copied it, or null; null when it cannot move. Writes to both sections, so call it
1947/// off the main thread.
19421948///
19431949/// # Safety
19441950/// Every string is NUL-terminated UTF-8; `before` and `discard` may be null.
crates/mobile/src/tests.rs+58-3
......@@ -1813,7 +1813,7 @@ fn a_page_edited_while_it_moves_stays_with_its_edit() {
18131813 "Clover Test",
18141814 date,
18151815 )
1816 .map(|_| ())
1816 .map(|moved| moved.copied)
18171817 .map_err(|error| error.to_string())
18181818 });
18191819 holding.recv_timeout(Duration::from_secs(10)).unwrap();
......@@ -1830,8 +1830,8 @@ fn a_page_edited_while_it_moves_stays_with_its_edit() {
18301830 }
18311831 features.shared.section.written().unwrap();
18321832 drop(release);
1833 let moved = move_page.join().unwrap();
1834 assert_eq!(moved.is_ok(), !edited, "{moved:?}");
1833 let copied = move_page.join().unwrap().unwrap();
1834 assert_eq!(copied.is_some(), edited, "{copied:?}");
18351835 drop(features);
18361836 library.close(Duration::from_secs(20)).unwrap();
18371837 let arena = onestore::Arena::default();
......@@ -1853,6 +1853,58 @@ fn a_page_edited_while_it_moves_stays_with_its_edit() {
18531853 }
18541854}
18551855
1856/// A page whose section cannot write as it leaves stays there beside the copy that arrived,
1857/// and the move says so; both files hold it once the notebook closes.
1858#[test]
1859fn a_page_its_section_cannot_retire_stays_beside_its_copy() {
1860 vfs::install();
1861 let _exclusive = vfs::EXCLUSIVE
1862 .lock()
1863 .unwrap_or_else(std::sync::PoisonError::into_inner);
1864 let (directory, root, library, empty) = notebook_open("Empty.one");
1865 let features = open(&library, "Features.one");
1866 let space = space(&features, "Task controls");
1867 drop(features);
1868 let file = std::fs::read(root.join("Features.one")).unwrap();
1869 let identity = onestore::RevisionIndex::parse(&onestore::Store::parse(&file).unwrap())
1870 .unwrap()
1871 .root
1872 .guid;
1873 let location = notebook::location::local(&root).unwrap();
1874 let replica =
1875 notebook::location::folder(&directory.path().join("cache"), &location).join(format!(
1876 "{}.sqlite",
1877 (identity.iter())
1878 .map(|byte| format!("{byte:02x}"))
1879 .collect::<String>()
1880 ));
1881 *vfs::FAILING.lock().unwrap() = Some(replica.canonicalize().unwrap());
1882 let date = ["Thursday, October 1, 2026", "9:00 AM"];
1883 let moved = library.move_page(
1884 "Features.one",
1885 space,
1886 "Empty.one",
1887 None,
1888 1,
1889 None,
1890 "Clover Test",
1891 date,
1892 );
1893 *vfs::FAILING.lock().unwrap() = None;
1894 assert!(moved.unwrap().copied.is_some());
1895 drop(empty);
1896 library.close(Duration::from_secs(20)).unwrap();
1897 let titles = |name: &str| {
1898 let arena = onestore::Arena::default();
1899 let stored = onestore::read_file(root.join(name)).unwrap();
1900 let mut section = onestore::Section::open(&arena, stored).unwrap();
1901 (section.pages().unwrap().into_iter())
1902 .filter(|(_, title, _)| title == "Task controls")
1903 .count()
1904 };
1905 assert_eq!((titles("Features.one"), titles("Empty.one")), (1, 1));
1906}
1907
18561908/// A page the section it moves to refuses stays where it was, and the move fails.
18571909#[test]
18581910fn a_page_the_other_section_refuses_stays() {
......@@ -3386,6 +3438,9 @@ fn reader_dropped_and_synchronous_failed_apply_callbacks_never_release_the_reloa
33863438
33873439#[test]
33883440fn reader_deferred_reload_preserves_rich_composition_after_real_local_write_failure() {
3441 let _exclusive = vfs::EXCLUSIVE
3442 .lock()
3443 .unwrap_or_else(std::sync::PoisonError::into_inner);
33893444 for select in [false, true] {
33903445 let directory = tempfile::tempdir().unwrap();
33913446 let file = directory.path().join("Reader fault.one");
crates/notebook/README.md+5-4
......@@ -38,10 +38,11 @@ assert_eq!(reopened.pending()?.last().map(|edit| edit.id), Some(id));
3838# }
3939```
4040
41`apply_unchanged(author, edit, space, page)` applies the edit only while page `space`
42still holds `page`, checked on the section thread in order with every other edit, and
43refuses it as `Rejected` otherwise: a move across sections copies a page there, then retires
44it here this way, so an edit made since the copy keeps the page.
41A move across sections reads the page with `moving(space)`, together with its conflict pages
42and versions, which no copy carries; copies it there; then retires it here with
43`retire(author, edit, moving)`, which applies the edit only while all of them are as read,
44checked on the section thread in order with every other edit, and refuses it as `Rejected`
45otherwise, so an edit made since the copy keeps the page.
4546
4647Page reads never wait for the sync thread: a rebase or reread of the queue runs on a
4748new thread while the current one keeps answering `page` and `pages` from the section as
crates/notebook/src/lib.rs+31-21
......@@ -88,6 +88,22 @@ type Result<T> = std::result::Result<T, Error>;
8888
8989const APPLICATION_ID: u32 = 0x4f4e454f;
9090
91/// A page as a move copies it (`Replica::moving`), with its conflict pages and versions, which
92/// stay behind, read together.
93#[derive(Debug, PartialEq)]
94pub struct Moving {
95 space: ExGuid,
96 page: Page,
97 conflicts: Vec<onestore::ConflictPage>,
98 versions: Vec<onestore::PageVersion>,
99}
100
101impl Moving {
102 pub fn page(&self) -> &Page {
103 &self.page
104 }
105}
106
91107/// A locally durable edit; its ID remains stable across cache reopen.
92108#[derive(Debug, Clone, PartialEq)]
93109pub struct PendingEdit {
......@@ -308,39 +324,33 @@ impl Replica {
308324 edit,
309325 reply,
310326 waiting: None,
311 unchanged: None,
327 moving: None,
312328 })
313329 })
314330 .await
315331 }
316332
317 /// `apply`, provided page `space` still holds `page`; refused as `Rejected` otherwise.
318 pub fn apply_unchanged(
319 &self,
320 author: &str,
321 edit: Edit,
322 space: ExGuid,
323 page: Page,
324 ) -> Result<u64> {
325 task::blocking(self.apply_unchanged_async(author, edit, space, page))?
333 /// Page `space` as a move copies it, read at once with what the copy leaves behind.
334 pub fn moving(&self, space: ExGuid) -> Result<Moving> {
335 self.ask(|reply| working::Request::Moving { space, reply })
326336 }
327337
328 /// `apply_async`, provided page `space` still holds `page`, checked where edits apply in
329 /// order: a move retires the page it copied this way, keeping any edit made since.
330 pub async fn apply_unchanged_async(
331 &self,
332 author: &str,
333 edit: Edit,
334 space: ExGuid,
335 page: Page,
336 ) -> Result<u64> {
338 /// `retire_async`, blocking.
339 pub fn retire(&self, author: &str, edit: Edit, moving: Moving) -> Result<u64> {
340 task::blocking(self.retire_async(author, edit, moving))?
341 }
342
343 /// Applies `edit`, which deletes the page `moving` read once its copy has landed elsewhere,
344 /// only while the page and what it leaves behind are as read: checked in order with every
345 /// other edit, so none made since is deleted with it. One changed is refused as `Rejected`.
346 pub async fn retire_async(&self, author: &str, edit: Edit, moving: Moving) -> Result<u64> {
337347 self.ask_async(|reply| {
338348 working::Request::Apply(working::Accepted {
339349 author: author.to_owned(),
340350 edit,
341351 reply,
342352 waiting: None,
343 unchanged: Some((space, page)),
353 moving: Some(Box::new(moving)),
344354 })
345355 })
346356 .await
......@@ -360,7 +370,7 @@ impl Replica {
360370 edit,
361371 reply,
362372 waiting: Some(waiting),
363 unchanged: None,
373 moving: None,
364374 }))
365375 }
366376
crates/notebook/src/session.rs+1-2
......@@ -2565,8 +2565,7 @@ impl Section {
25652565 }
25662566
25672567 /// Removes pages or conflict pages permanently, queued like the user's own edits (a
2568 /// move across sections imports a page there, then retires it here with
2569 /// `Replica::apply_unchanged`).
2568 /// move across sections imports a page there, then retires it here with `Replica::retire`).
25702569 pub fn delete_pages(&self, pages: &[ExGuid]) -> Result<u64> {
25712570 crate::task::blocking(self.delete_pages_async(pages))?
25722571 }
crates/notebook/src/working.rs+30-7
......@@ -4,7 +4,7 @@
44//! and, when the remote changed, to rebase the queue. In the browser, which has one thread,
55//! the section is served as each request is sent, and a rebuild runs as it is asked for.
66
7use crate::{Result, base, lock, merge, queue, worker::Signal};
7use crate::{Moving, Result, base, lock, merge, queue, worker::Signal};
88use onestore::{
99 Arena, ExGuid, Section, Transaction,
1010 op::{Edit, OpError},
......@@ -42,6 +42,10 @@ pub(crate) enum Request {
4242 version: ExGuid,
4343 reply: Reply<Page>,
4444 },
45 Moving {
46 space: ExGuid,
47 reply: Reply<Moving>,
48 },
4549 /// Answers once the edits before it are written, optionally awaiting host durability.
4650 Flush {
4751 reply: Reply<()>,
......@@ -89,6 +93,7 @@ impl Request {
8993 Self::Conflicts { reply } => reply(Err(error())),
9094 Self::Versions { reply } => reply(Err(error())),
9195 Self::Version { reply, .. } => reply(Err(error())),
96 Self::Moving { reply, .. } => reply(Err(error())),
9297 Self::Flush { reply, .. } => reply(Err(error())),
9398 Self::Seal { reply } => reply(Err(error())),
9499 Self::Rebase { reply, .. } => reply(Err(error())),
......@@ -482,7 +487,7 @@ pub(crate) struct Accepted {
482487 /// Set where nobody blocks on the reply: a failed write keeps the edit and reports here.
483488 pub waiting: Option<Notice>,
484489 /// A page as a move copied it, which the edit applies only while it still holds.
485 pub unchanged: Option<(ExGuid, Page)>,
490 pub moving: Option<Box<Moving>>,
486491}
487492
488493struct Working<'a> {
......@@ -553,6 +558,7 @@ impl<'a> Working<'a> {
553558 version,
554559 reply,
555560 } => reply(self.section.version(space, version).map_err(Into::into)),
561 Request::Moving { space, reply } => reply(self.moving(space)),
556562 Request::Handover(to) => {
557563 let mut waiting = held.take().unwrap_or_default();
558564 waiting.extend(burst.drain(..).chain(later.drain(..)));
......@@ -613,6 +619,10 @@ impl<'a> Working<'a> {
613619 reply(self.section.version(space, version).map_err(Into::into));
614620 false
615621 }
622 Request::Moving { space, reply } => {
623 reply(self.moving(space));
624 false
625 }
616626 Request::Flush { reply, durable } => {
617627 #[cfg(target_arch = "wasm32")]
618628 if durable {
......@@ -699,15 +709,28 @@ impl<'a> Working<'a> {
699709 .is_ok()
700710 }
701711
712 /// Page `space` as a move copies it, with the conflict pages and versions it leaves.
713 fn moving(&mut self, space: ExGuid) -> Result<Moving> {
714 fn of<T>(space: ExGuid, listed: Vec<(ExGuid, Vec<T>)>) -> Vec<T> {
715 (listed.into_iter())
716 .find_map(|(page, of)| (page == space).then_some(of))
717 .unwrap_or_default()
718 }
719 Ok(Moving {
720 space,
721 page: self.section.page(space)?,
722 conflicts: of(space, self.section.conflicts()?),
723 versions: of(space, self.section.versions()?),
724 })
725 }
726
702727 /// Applies an edit, keeping it to be written with the burst. A refused edit is answered
703728 /// here; true when it failed part way and the section must be reread.
704729 fn accept(&mut self, accepted: Accepted) -> bool {
705 if let Some((space, page)) = &accepted.unchanged
706 && !self.section.page(*space).is_ok_and(|now| now == *page)
730 if let Some(moving) = &accepted.moving
731 && !self.moving(moving.space).is_ok_and(|now| now == **moving)
707732 {
708 let changed = OpError::StructureChanged(
709 "The page changed while it moved. Both sections now have a copy.",
710 );
733 let changed = OpError::StructureChanged("The page changed while it moved.");
711734 (accepted.reply)(Err(changed.into()));
712735 return false;
713736 }
crates/notebook/tests/support/vfs.rs+2
......@@ -34,6 +34,8 @@ static REAL: OnceLock<usize> = OnceLock::new();
3434static WAL: OnceLock<(usize, usize, usize)> = OnceLock::new();
3535/// While set, every write to this database's WAL fails, for WALs opened while it was set.
3636pub static FAILING: Mutex<Option<PathBuf>> = Mutex::new(None);
37/// Held by a test for as long as it sets `FAILING`, which one test sets at a time.
38pub static EXCLUSIVE: Mutex<()> = Mutex::new(());
3739
3840fn fire(fault: u8) -> bool {
3941 let fired = ARMED.fetch_and(!fault, Ordering::SeqCst) & fault != 0;
crates/notebook/tests/sync_pages.rs+61-5
......@@ -745,8 +745,9 @@ fn page_moves_and_a_removed_page_merge_as_onenote_merges_them() {
745745}
746746
747747/// A page a move copied leaves only as it was copied: an edit made here since, published or
748/// not, or another writer's that a rebase took, keeps it; edits to other pages, published or
749/// rebased, do not. Either way the queue reopens as it was left and publishes it.
748/// not, another writer's that a rebase took, or a conflict page kept under it keeps it; edits
749/// to other pages, published or rebased, do not. Either way the queue reopens as it was left
750/// and publishes it.
750751#[test]
751752fn a_moved_page_leaves_only_as_it_was_copied() {
752753 // The first two pages holding text.
......@@ -774,7 +775,7 @@ fn a_moved_page_leaves_only_as_it_was_copied() {
774775 server.durable.clone_from(&server.visible);
775776 }
776777 type Between = fn(&Replica, &mut Server, [(ExGuid, ExGuid); 2]);
777 let schedules: [(&str, bool, Between); 5] = [
778 let schedules: [(&str, bool, Between); 6] = [
778779 ("unrelated", true, |cache, server, [_, other]| {
779780 here(cache, other);
780781 cache.sync_once(server).unwrap();
......@@ -801,13 +802,33 @@ fn a_moved_page_leaves_only_as_it_was_copied() {
801802 cache.sync_once(server).unwrap();
802803 },
803804 ),
805 (
806 "kept as a conflict page",
807 false,
808 |cache, _, [(moving, _), _]| {
809 let page = cache.page(moving).unwrap();
810 let none = std::collections::BTreeSet::new();
811 let kept =
812 notebook::session::conflict(moving, &page, &none, "Keeper", true).unwrap();
813 let at = 133_000_000_000_000_001;
814 cache
815 .apply(
816 "Keeper",
817 onestore::op::Edit {
818 at,
819 ops: vec![kept],
820 },
821 )
822 .unwrap();
823 },
824 ),
804825 ];
805826 for (schedule, leaves, between) in schedules {
806827 let directory = tempfile::tempdir().unwrap();
807828 let path = directory.path().join("moving.sqlite");
808829 let cache = Replica::create(&path, SOURCE).unwrap();
809830 let mut server = Server::new(SOURCE);
810 let copied = cache.page(moving).unwrap();
831 let copied = cache.moving(moving).unwrap();
811832 between(
812833 &cache,
813834 &mut server,
......@@ -818,7 +839,7 @@ fn a_moved_page_leaves_only_as_it_was_copied() {
818839 at: 133_000_000_000_000_002,
819840 ops: vec![Op::Section(SectionOp::Delete(vec![moving]))],
820841 };
821 let retired = cache.apply_unchanged("Mover", delete, moving, copied);
842 let retired = cache.retire("Mover", delete, copied);
822843 assert_eq!(retired.is_ok(), leaves, "{schedule}: {retired:?}");
823844 assert!(
824845 leaves || matches!(retired, Err(notebook::Error::Rejected(_))),
......@@ -837,3 +858,38 @@ fn a_moved_page_leaves_only_as_it_was_copied() {
837858 assert!(cache.pending().unwrap().is_empty(), "{schedule}");
838859 }
839860}
861
862/// A version of a page deleted while the page moves keeps the page, which no copy carries
863/// the versions of.
864#[test]
865fn a_moved_page_leaves_only_with_the_versions_it_had() {
866 let source =
867 include_bytes!("../../../corpus/page-versions/native/step-02/notebook/History.one");
868 let directory = tempfile::tempdir().unwrap();
869 let cache = Replica::create(directory.path().join("history.sqlite"), source).unwrap();
870 let (moving, versions) = cache.versions().unwrap().remove(0);
871 let copied = cache.moving(moving).unwrap();
872 section_op(
873 &cache,
874 SectionOp::DeleteVersions {
875 page: moving,
876 versions: vec![versions[0].context],
877 },
878 );
879 let delete = onestore::op::Edit {
880 at: 133_000_000_000_000_002,
881 ops: vec![Op::Section(SectionOp::Delete(vec![moving]))],
882 };
883 let retired = cache.retire("Mover", delete, copied);
884 assert!(
885 matches!(retired, Err(notebook::Error::Rejected(_))),
886 "{retired:?}"
887 );
888 assert!(
889 cache
890 .pages()
891 .unwrap()
892 .iter()
893 .any(|(space, ..)| *space == moving)
894 );
895}
crates/snowbound/Cargo.toml+2
......@@ -126,3 +126,5 @@ x11rb = "0.13.2"
126126[dev-dependencies]
127127# Draws the pictures of the video recording the tests store.
128128image = { version = "0.25", default-features = false }
129# Fails a replica's writes, as a full disk does (`notebook/tests/support/vfs.rs`).
130rusqlite = { version = "=0.40.2", features = ["bundled", "backup"] }
crates/snowbound/src/main.rs+4
......@@ -94,6 +94,10 @@ mod undo;
9494mod unpack;
9595mod unread;
9696mod update;
97/// Fails a replica's writes on demand, for tests of what a full disk does.
98#[cfg(test)]
99#[path = "../../notebook/tests/support/vfs.rs"]
100mod vfs;
97101#[cfg_attr(target_arch = "wasm32", path = "watch_web.rs")]
98102mod watch;
99103
crates/snowbound/src/undo.rs+178-73
......@@ -351,7 +351,8 @@ struct Site {
351351
352352/// A change to pages done: the change taking it back, the page to show, and pages gone.
353353struct Applied {
354 undo: Change,
354 /// What takes it back; where a move only copied its page, why the page also stayed.
355 undo: Result<Change, String>,
355356 show: ExGuid,
356357 gone: Vec<ExGuid>,
357358}
......@@ -415,6 +416,19 @@ impl Site {
415416 entry_of(self.library.catalog()?, identity).map(|(_, path)| path)
416417 }
417418
419 /// Closes `section`, at catalog `path`, once its edits are written; while some wait to be,
420 /// keeps it open, where they are tried again until they are.
421 async fn release(&self, path: &str, section: notebook::session::Section) {
422 match section.replica().written_async().await {
423 Ok(()) => {
424 if let Err(error) = section.close_async().await {
425 eprintln!("Synchronization stopped: {error}");
426 }
427 }
428 Err(_) => self.library.keep(path, section),
429 }
430 }
431
418432 /// Does `change` to the section's pages, `shown` the page open; `None` where what it
419433 /// changes is gone or holds what it did not make, which it leaves alone.
420434 async fn change(
......@@ -468,11 +482,11 @@ impl Site {
468482 }
469483 self.apply(ops).await?;
470484 Applied {
471 undo: Change::Restore {
485 undo: Ok(Change::Restore {
472486 pages: kept,
473487 added,
474488 created,
475 },
489 }),
476490 show,
477491 gone: pages,
478492 }
......@@ -519,11 +533,11 @@ impl Site {
519533 self.library = Arc::new(self.library.with(notebook));
520534 }
521535 Applied {
522 undo: Change::Delete {
536 undo: Ok(Change::Delete {
523537 pages: spaces.clone(),
524538 show: Some(shown),
525539 created,
526 },
540 }),
527541 show: spaces[0],
528542 gone,
529543 }
......@@ -536,13 +550,13 @@ impl Site {
536550 self.apply(vec![Op::Section(SectionOp::Pages(edits))])
537551 .await?;
538552 Applied {
539 undo: Change::Arrange {
553 undo: Ok(Change::Arrange {
540554 order: listed
541555 .iter()
542556 .map(|(space, _, level)| (*space, *level))
543557 .collect(),
544558 moved,
545 },
559 }),
546560 show,
547561 gone: Vec::new(),
548562 }
......@@ -554,11 +568,11 @@ impl Site {
554568 let op = crate::rename::retitled(&self.replica.page(page)?, page, to.clone())?;
555569 self.apply(vec![op]).await?;
556570 Applied {
557 undo: Change::Retitle {
571 undo: Ok(Change::Retitle {
558572 page,
559573 from: to,
560574 to: from,
561 },
575 }),
562576 show: page,
563577 gone: Vec::new(),
564578 }
......@@ -567,8 +581,8 @@ impl Site {
567581 let (Some(from), Some(path)) = (at(page), self.section_path(target)) else {
568582 return Ok(None);
569583 };
570 let sent = self.replica.page(page)?;
571 let import = notebook::session::moved(&sent, &self.author)?;
584 let sent = self.replica.moving(page)?;
585 let import = notebook::session::moved(sent.page(), &self.author)?;
572586 let arrived = imported(&import);
573587 let before = listed[from + 1..].first().map(|(space, ..)| *space);
574588 let level = listed[from].2;
......@@ -585,24 +599,29 @@ impl Site {
585599 },
586600 )
587601 .await?;
588 section.close_async().await?;
602 self.release(&path, section).await;
589603 let edit = Edit {
590604 at: crate::filetime(),
591605 ops,
592606 };
593 self.replica
594 .apply_unchanged_async(&self.author, edit, page, sent)
595 .await?;
596 Applied {
597 undo: Change::Return {
598 page: arrived,
599 target,
600 before,
601 level,
602 added,
607 let retired = self.replica.retire_async(&self.author, edit, sent).await;
608 match retired {
609 Ok(_) => Applied {
610 undo: Ok(Change::Return {
611 page: arrived,
612 target,
613 before,
614 level,
615 added,
616 }),
617 show,
618 gone: vec![page],
619 },
620 Err(error) => Applied {
621 undo: Err(crate::plain(&error, "page")),
622 show: page,
623 gone: Vec::new(),
603624 },
604 show,
605 gone: vec![page],
606625 }
607626 }
608627 Change::Return {
......@@ -616,15 +635,12 @@ impl Site {
616635 return Ok(None);
617636 };
618637 let section = self.library.open_async(&path, || {}).await?;
619 let sent = match section.pages()?.iter().any(|(listed, ..)| *listed == page) {
620 true => Some(section.page(page)?),
621 false => None,
622 };
623 let Some(sent) = sent else {
624 section.close_async().await?;
638 if !(section.pages()?.iter()).any(|(listed, ..)| *listed == page) {
639 self.release(&path, section).await;
625640 return Ok(None);
626 };
627 let import = notebook::session::moved(&sent, &self.author)?;
641 }
642 let sent = section.replica().moving(page)?;
643 let import = notebook::session::moved(sent.page(), &self.author)?;
628644 let back = imported(&import);
629645 let mut ops = vec![
630646 import,
......@@ -640,13 +656,14 @@ impl Site {
640656 at: crate::filetime(),
641657 ops: vec![Op::Section(SectionOp::Delete(vec![page]))],
642658 };
643 section
659 let retired = section
644660 .replica()
645 .apply_unchanged_async(&self.author, delete, page, sent)
646 .await?;
647 section.close_async().await?;
661 .retire_async(&self.author, delete, sent)
662 .await;
663 self.release(&path, section).await;
648664 Applied {
649 undo: Change::Send { page: back, target },
665 undo: (retired.map(|_| Change::Send { page: back, target }))
666 .map_err(|error| crate::plain(&error, "page")),
650667 show: back,
651668 gone,
652669 }
......@@ -1152,12 +1169,17 @@ impl State {
11521169 };
11531170 };
11541171 state.undo.forget(&applied.gone);
1155 let undo = Action {
1156 notebook,
1157 section: Some(section),
1158 change: applied.undo,
1159 };
1160 state.undo.note(taken.map(|(redo, _)| redo), undo);
1172 match applied.undo {
1173 Ok(change) => {
1174 let undo = Action {
1175 notebook,
1176 section: Some(section),
1177 change,
1178 };
1179 state.undo.note(taken.map(|(redo, _)| redo), undo);
1180 }
1181 Err(copied) => crate::platform::alert("Page kept in both sections", &copied),
1182 }
11611183 state.edited(Vec::new());
11621184 Ok(())
11631185 })));
......@@ -1503,6 +1525,24 @@ mod tests {
15031525 )
15041526 }
15051527
1528 /// The titles of the section at `path`, opened again.
1529 fn stored(&self, path: &str) -> Vec<(String, u32)> {
1530 let section = self.library.open(path, || {}).unwrap();
1531 let titles = (section.pages().unwrap().into_iter())
1532 .map(|(_, title, level)| (title, level))
1533 .collect();
1534 section.close().unwrap();
1535 titles
1536 }
1537
1538 /// The replica of the section at `path`, which `vfs::FAILING` names.
1539 fn replica(&self, path: &str) -> std::path::PathBuf {
1540 let Ok(Some(notebook)) = &self.library.notebook else {
1541 panic!("a notebook")
1542 };
1543 notebook.replica_path(path).unwrap().canonicalize().unwrap()
1544 }
1545
15061546 /// Does `change`, as the app's thread does, returning what takes it back.
15071547 fn change(&self, change: Change) -> Option<Applied> {
15081548 let shown = self.pages()[0].0;
......@@ -1554,7 +1594,7 @@ mod tests {
15541594 assert_eq!(fixture.titles(), titled(&["First", "Third"]));
15551595 assert_eq!(deleted.show, fixture.space("Third"));
15561596 assert_eq!(fixture.binned(), ["Second"]);
1557 let restored = pollster::block_on(site.change(deleted.undo, deleted.show))
1597 let restored = pollster::block_on(site.change(deleted.undo.unwrap(), deleted.show))
15581598 .unwrap()
15591599 .unwrap();
15601600 assert!(!Arc::ptr_eq(&site.library, &deleted_library));
......@@ -1562,19 +1602,19 @@ mod tests {
15621602 assert_eq!(fixture.titles(), titled(&["First", "Second", "Third"]));
15631603 assert_eq!(restored.show, fixture.space("Second"));
15641604 assert!(fixture.binned().is_empty());
1565 let again = pollster::block_on(site.change(restored.undo, restored.show))
1605 let again = pollster::block_on(site.change(restored.undo.unwrap(), restored.show))
15661606 .unwrap()
15671607 .unwrap();
15681608 assert_eq!(fixture.titles(), titled(&["First", "Third"]));
15691609 assert_eq!(fixture.binned(), ["Second"]);
15701610
15711611 // Put back elsewhere first, the page stays as that device left it.
1572 let Change::Restore { pages, .. } = &again.undo else {
1612 let Ok(Change::Restore { pages, .. }) = &again.undo else {
15731613 panic!()
15741614 };
15751615 let copy = notebook::session::moved(&pages[0].page, AUTHOR).unwrap();
15761616 fixture.edit(vec![copy]);
1577 assert!(fixture.change(again.undo).is_none());
1617 assert!(fixture.change(again.undo.unwrap()).is_none());
15781618 assert_eq!(fixture.titles(), titled(&["First", "Third", "Second"]));
15791619 }
15801620
......@@ -1598,13 +1638,13 @@ mod tests {
15981638 assert_eq!(fixture.titles(), titled(&["First", "Second"]));
15991639 assert_eq!(removed.show, first);
16001640 assert!(fixture.binned().is_empty());
1601 let back = fixture.change(removed.undo).unwrap();
1641 let back = fixture.change(removed.undo.unwrap()).unwrap();
16021642 assert_eq!(fixture.titles(), titled(&["First", "Second", ""]));
1603 let Change::Delete { pages, .. } = &back.undo else {
1643 let Ok(Change::Delete { pages, .. }) = &back.undo else {
16041644 panic!()
16051645 };
16061646 fixture.retitle(pages[0], "Written elsewhere");
1607 assert!(fixture.change(back.undo).is_none());
1647 assert!(fixture.change(back.undo.unwrap()).is_none());
16081648 assert_eq!(
16091649 fixture.titles(),
16101650 titled(&["First", "Second", "Written elsewhere"])
......@@ -1637,7 +1677,7 @@ mod tests {
16371677 let arranged = fixture.change(arrange).unwrap();
16381678 assert_eq!(fixture.titles(), titled(&["A", "B", "C", "D"]));
16391679 assert_eq!(arranged.show, a);
1640 fixture.change(arranged.undo).unwrap();
1680 fixture.change(arranged.undo.unwrap()).unwrap();
16411681 fixture.edit(vec![Op::Section(SectionOp::Delete(vec![b]))]);
16421682 fixture
16431683 .change(Change::Arrange {
......@@ -1673,23 +1713,23 @@ mod tests {
16731713 .unwrap();
16741714 assert_eq!(fixture.titles(), titled(&["First", "Third"]));
16751715 assert_eq!(in_b(&fixture), ["", "Second"]);
1676 let returned = fixture.change(sent.undo).unwrap();
1716 let returned = fixture.change(sent.undo.unwrap()).unwrap();
16771717 assert_eq!(fixture.titles(), titled(&["First", "Second", "Third"]));
16781718 assert_eq!(in_b(&fixture), [""]);
1679 let sent = fixture.change(returned.undo).unwrap();
1680 let Change::Return { page, .. } = &sent.undo else {
1719 let sent = fixture.change(returned.undo.unwrap()).unwrap();
1720 let Ok(Change::Return { page, .. }) = &sent.undo else {
16811721 panic!()
16821722 };
16831723 let section = fixture.library.open("B.one", || {}).unwrap();
16841724 section.delete_pages(&[*page]).unwrap();
16851725 section.close().unwrap();
1686 assert!(fixture.change(sent.undo).is_none());
1726 assert!(fixture.change(sent.undo.unwrap()).is_none());
16871727 assert_eq!(fixture.titles(), titled(&["First", "Third"]));
16881728 }
16891729
16901730 /// A page edited while it is on its way to another section stays, with the edit, beside
1691 /// the copy that arrived there; an edit to another page meanwhile lets it go. Both hold
1692 /// once each section is opened again.
1731 /// the copy that arrived there, with nothing for Undo to take back; an edit to another page
1732 /// meanwhile lets it go. Both hold once each section is opened again.
16931733 #[test]
16941734 fn a_page_edited_while_it_is_sent_stays_with_its_edit() {
16951735 struct Unreached;
......@@ -1731,39 +1771,104 @@ mod tests {
17311771 let shown = fixture.pages()[0].0;
17321772 let sending = std::thread::spawn(move || {
17331773 pollster::block_on(site.change(Change::Send { page, target }, shown))
1734 .map(|applied| applied.is_some())
17351774 .map_err(|error| error.to_string())
17361775 });
1776 let deadline = std::time::Instant::now() + std::time::Duration::from_secs(20);
17371777 while !(arrived.pages().unwrap().iter()).any(|(_, title, _)| title == "Second") {
1778 assert!(
1779 std::time::Instant::now() < deadline,
1780 "the copy never arrived"
1781 );
17381782 std::thread::sleep(std::time::Duration::from_millis(5));
17391783 }
17401784 fixture.retitle(fixture.space(edited), &format!("{edited} edited"));
17411785 drop(release);
17421786 drop(arrived);
1743 let sent = sending.join().unwrap();
1787 let sent = sending.join().unwrap().unwrap().unwrap();
17441788
17451789 let stayed = match edited {
17461790 "Second" => {
1747 assert!(sent.is_err(), "{sent:?}");
1791 assert!(sent.undo.is_err());
17481792 titled(&["First", "Second edited", "Third"])
17491793 }
17501794 _ => {
1751 assert_eq!(sent, Ok(true));
1795 assert!(sent.undo.is_ok());
17521796 titled(&["First", "Third edited"])
17531797 }
17541798 };
17551799 fixture.section.take().unwrap().close().unwrap();
1756 let titles = |path: &str| {
1757 let section = fixture.library.open(path, || {}).unwrap();
1758 let titles: Vec<(String, u32)> = (section.pages().unwrap().into_iter())
1759 .map(|(_, title, level)| (title, level))
1760 .collect();
1761 section.close().unwrap();
1762 titles
1763 };
1764 assert_eq!(titles("New Section 1.one"), stayed);
1765 assert_eq!(titles("B.one"), titled(&["", "Second"]));
1800 assert_eq!(fixture.stored("New Section 1.one"), stayed);
1801 assert_eq!(fixture.stored("B.one"), titled(&["", "Second"]));
1802 }
1803 }
1804
1805 use crate::vfs;
1806
1807 /// A page Undo brings back stays in the section it was sent to as well where that one
1808 /// cannot write, with nothing left for Redo; both hold once each is opened again.
1809 #[test]
1810 fn a_page_brought_back_where_the_cache_fails_stays_in_both_sections() {
1811 vfs::install();
1812 let _exclusive = vfs::EXCLUSIVE
1813 .lock()
1814 .unwrap_or_else(std::sync::PoisonError::into_inner);
1815 let mut fixture = Fixture::new("return-fault", &["First", "Second", "Third"]);
1816 let target = fixture.library.section_identity("B.one").unwrap();
1817 let sent = (fixture.change(Change::Send {
1818 page: fixture.space("Second"),
1819 target,
1820 }))
1821 .unwrap();
1822 *vfs::FAILING.lock().unwrap() = Some(fixture.replica("B.one"));
1823 let back = fixture.change(sent.undo.unwrap());
1824 *vfs::FAILING.lock().unwrap() = None;
1825 assert!(back.unwrap().undo.is_err());
1826 assert_eq!(fixture.titles(), titled(&["First", "Second", "Third"]));
1827 fixture.section.take().unwrap().close().unwrap();
1828 let stayed = titled(&["First", "Second", "Third"]);
1829 assert_eq!(fixture.stored("New Section 1.one"), stayed);
1830 assert_eq!(fixture.stored("B.one"), titled(&["", "Second"]));
1831 }
1832
1833 /// A section a move leaves with edits waiting for the cache stays open, writing them once
1834 /// it can, rather than closing without them.
1835 #[test]
1836 fn a_section_left_with_edits_waiting_keeps_them() {
1837 vfs::install();
1838 let _exclusive = vfs::EXCLUSIVE
1839 .lock()
1840 .unwrap_or_else(std::sync::PoisonError::into_inner);
1841 let fixture = Fixture::new("release", &["First"]);
1842 fixture
1843 .library
1844 .open("B.one", || {})
1845 .unwrap()
1846 .close()
1847 .unwrap();
1848 *vfs::FAILING.lock().unwrap() = Some(fixture.replica("B.one"));
1849 let b = fixture.library.open("B.one", || {}).unwrap();
1850 let space = b.pages().unwrap()[0].0;
1851 let op = crate::rename::retitled(&b.page(space).unwrap(), space, "Waiting".into());
1852 let edit = Edit {
1853 at: crate::filetime(),
1854 ops: vec![op.unwrap()],
1855 };
1856 b.apply(AUTHOR, edit).unwrap();
1857 let replica = Arc::clone(b.replica());
1858 assert!(replica.written().is_err());
1859 pollster::block_on(fixture.site().release("B.one", b));
1860 *vfs::FAILING.lock().unwrap() = None;
1861 let deadline = std::time::Instant::now() + std::time::Duration::from_secs(20);
1862 while replica.written().is_err() {
1863 assert!(
1864 std::time::Instant::now() < deadline,
1865 "the edit was never written"
1866 );
1867 std::thread::sleep(std::time::Duration::from_millis(50));
17661868 }
1869 drop(replica);
1870 fixture.library.close_kept();
1871 assert_eq!(fixture.stored("B.one"), titled(&["Waiting"]));
17671872 }
17681873
17691874 /// A title given in the page list goes back while the page still has it.
......@@ -1780,7 +1885,7 @@ mod tests {
17801885 .unwrap();
17811886 assert_eq!(fixture.titles(), titled(&["First", "Second"]));
17821887 fixture.retitle(page, "Changed elsewhere");
1783 assert!(fixture.change(back.undo).is_none());
1888 assert!(fixture.change(back.undo.unwrap()).is_none());
17841889 }
17851890
17861891 /// Carries out `change` as `manage::State::restructure` does.
......@@ -1886,7 +1991,7 @@ mod tests {
18861991 let mut fixture = Fixture::new("export", &["First", "Second", "Third", "Fourth"]);
18871992 let back = |fixture: &Fixture, change| {
18881993 let done = fixture.change(change).unwrap();
1889 fixture.change(done.undo).unwrap()
1994 fixture.change(done.undo.unwrap()).unwrap()
18901995 };
18911996 back(
18921997 &fixture,