diff --git a/taskchampion/taskchampion/src/server/cloud/server.rs b/taskchampion/taskchampion/src/server/cloud/server.rs index d06c18958..7314e47f0 100644 --- a/taskchampion/taskchampion/src/server/cloud/server.rs +++ b/taskchampion/taskchampion/src/server/cloud/server.rs @@ -45,7 +45,7 @@ use uuid::Uuid; /// /// Snapshots are stored as objects with name `s-VERSION` where `VERSION` is the version at which /// the snapshot was made. These objects are created with simple `put` requests, as any snapshot -/// for a given version is interchangeable with any other. +/// for a given version is functionally equivalent to any other. /// /// ## Cleanup /// @@ -103,7 +103,8 @@ impl CloudServer { .into_bytes() } - /// Parse a version name as generated by `version_name`. + /// Parse a version name as generated by `version_name`, returning None if the name does not + /// have a valid format. fn parse_version_name(name: &[u8]) -> Option<(VersionId, VersionId)> { if name.len() != 2 + 32 + 1 + 32 || !name.starts_with(b"v-") || name[2 + 32] != b'-' { return None; @@ -122,7 +123,8 @@ impl CloudServer { format!("s-{}", version_id.as_simple()).into_bytes() } - /// Parse a snapshot name as generated by `snapshot_name`. + /// Parse a snapshot name as generated by `snapshot_name`, returning None if the name does not + /// have a valid format. fn parse_snapshot_name(name: &[u8]) -> Option { if name.len() != 2 + 32 || !name.starts_with(b"s-") { return None; @@ -172,9 +174,9 @@ impl CloudServer { .collect::>>() } - /// Determine the snapshot urgency. This is done probabalistically, so that approximately one - /// in 25 sync's results in a new snapshot on a client that is not trying to avoid snapshots, - /// and one in 100 on a client that is trying to avoid snapshots. + /// Determine the snapshot urgency. This is done probabalistically: + /// - High urgency approximately 1% of the time. + /// - Low urgecny approximately 10% of the time. fn snapshot_urgency(&self) -> Result { let r = self.randint()?; if r < 2 { @@ -469,7 +471,7 @@ mod tests { /// A simple in-memory service for testing. All insertions via Service methods occur at time /// `INSERTION_TIME`. All versions older that 1000 are considered "old". - #[derive(Default)] + #[derive(Default, Clone)] struct MockService(HashMap, (u64, Vec)>); const INSERTION_TIME: u64 = 9999999999; @@ -575,7 +577,7 @@ mod tests { /// Create a copy of this server without any data; used for creating a MockService /// to compare to with `assert_eq!` - fn empty_copy(&self) -> Self { + fn empty_clone(&self) -> Self { Self { cryptor: self.cryptor.clone(), cleanup_probability: 0, @@ -617,6 +619,16 @@ mod tests { .collect() } } + impl Clone for CloudServer { + fn clone(&self) -> Self { + Self { + cryptor: self.cryptor.clone(), + cleanup_probability: self.cleanup_probability, + service: self.service.clone(), + add_version_intercept: None, + } + } + } const SECRET: &[u8] = b"testing"; @@ -703,7 +715,7 @@ mod tests { #[test] fn parse_snapshot_name_invalid() { assert_eq!( - CloudServer::::parse_snapshot_name(b"s-ggggggggggggggggggd2d3d4d5d6d7d8"), + CloudServer::::parse_snapshot_name(b"s-xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"), None ); } @@ -799,28 +811,32 @@ mod tests { let (v1, v2) = (Uuid::new_v4(), Uuid::new_v4()); server.mock_add_version(v1, v2, 1000, b"first"); server.mock_set_latest(v2); + let (res, _) = server.add_version(v2, b"history".to_vec()).unwrap(); let AddVersionResult::Ok(new_version) = res else { panic!("expected OK"); }; - let mut expected = server.empty_copy(); + + let mut expected = server.empty_clone(); expected.mock_add_version(v1, v2, 1000, b"first"); expected.mock_add_version(v2, new_version, INSERTION_TIME, b"history"); expected.mock_set_latest(new_version); + assert_eq!(server.unencrypted(), expected.unencrypted()); } #[test] fn add_version_not_latest() { + // The `add_version` method does nothing if the version is not latest. let mut server = make_server(); let (v1, v2) = (Uuid::new_v4(), Uuid::new_v4()); server.mock_add_version(v1, v2, 1000, b"first"); server.mock_set_latest(v2); + + let expected = server.clone(); + let (res, _) = server.add_version(v1, b"history".to_vec()).unwrap(); assert_eq!(res, AddVersionResult::ExpectedParentVersion(v2)); - let mut expected = server.empty_copy(); - expected.mock_add_version(v1, v2, 1000, b"first"); - expected.mock_set_latest(v2); assert_eq!(server.unencrypted(), expected.unencrypted()); } @@ -839,12 +855,15 @@ mod tests { server.add_version_intercept = Some(|service| { service.put(LATEST, &version_to_bytes(V3)).unwrap(); }); - let (res, _) = server.add_version(v2, b"history".to_vec()).unwrap(); - assert_eq!(res, AddVersionResult::ExpectedParentVersion(V3)); - let mut expected = server.empty_copy(); + + let mut expected = server.empty_clone(); expected.mock_add_version(v1, v2, 1000, b"first"); expected.mock_add_version(v2, V3, 1000, b"second"); - expected.mock_set_latest(V3); + expected.mock_set_latest(V3); // updated by the intercept + + assert_ne!(server.unencrypted(), expected.unencrypted()); + let (res, _) = server.add_version(v2, b"history".to_vec()).unwrap(); + assert_eq!(res, AddVersionResult::ExpectedParentVersion(V3)); assert_eq!(server.unencrypted(), expected.unencrypted()); } @@ -854,13 +873,13 @@ mod tests { let (v1, v2) = (Uuid::new_v4(), Uuid::new_v4()); server.mock_add_version(v1, v2, 1000, b"first"); server.mock_set_latest(v2); + + let expected = server.clone(); + let (res, _) = server .add_version(Uuid::new_v4(), b"history".to_vec()) .unwrap(); assert_eq!(res, AddVersionResult::ExpectedParentVersion(v2)); - let mut expected = server.empty_copy(); - expected.mock_add_version(v1, v2, 1000, b"first"); - expected.mock_set_latest(v2); assert_eq!(server.unencrypted(), expected.unencrypted()); } @@ -933,6 +952,8 @@ mod tests { #[test] fn cleanup_linear() { + // Test that cleanup does nothing for a linear version history with a snapshot at the + // oldest version. let mut server = make_server(); let (v1, v2, v3) = (Uuid::new_v4(), Uuid::new_v4(), Uuid::new_v4()); server.mock_add_version(NIL_VERSION_ID, v1, 1000, b"first"); @@ -941,19 +962,15 @@ mod tests { server.mock_add_snapshot(v1, 1000, b"snap 1"); server.mock_set_latest(v3); - server.cleanup().unwrap(); + let expected = server.clone(); - let mut expected = server.empty_copy(); - expected.mock_add_version(NIL_VERSION_ID, v1, 1000, b"first"); - expected.mock_add_version(v1, v2, 1000, b"second"); - expected.mock_add_version(v2, v3, 1000, b"third"); - expected.mock_add_snapshot(v1, 1000, b"snap 1"); - expected.mock_set_latest(v3); + server.cleanup().unwrap(); assert_eq!(server.unencrypted(), expected.unencrypted()); } #[test] fn cleanup_cycle() { + // When a cycle is present, cleanup succeeds and makes no changes. let mut server = make_server(); let (v1, v2, v3) = (Uuid::new_v4(), Uuid::new_v4(), Uuid::new_v4()); server.mock_add_version(v3, v1, 1000, b"first"); @@ -961,18 +978,15 @@ mod tests { server.mock_add_version(v2, v3, 1000, b"third"); server.mock_set_latest(v3); - assert!(server.cleanup().is_err()); + let expected = server.clone(); - let mut expected = server.empty_copy(); - expected.mock_add_version(v3, v1, 1000, b"first"); - expected.mock_add_version(v1, v2, 1000, b"second"); - expected.mock_add_version(v2, v3, 1000, b"third"); - expected.mock_set_latest(v3); + assert!(server.cleanup().is_err()); assert_eq!(server.unencrypted(), expected.unencrypted()); } #[test] fn cleanup_extra_branches() { + // Cleanup deletes extra branches in the versions. let mut server = make_server(); let (v1, v2, v3) = (Uuid::new_v4(), Uuid::new_v4(), Uuid::new_v4()); let (vx, vy) = (Uuid::new_v4(), Uuid::new_v4()); @@ -982,12 +996,13 @@ mod tests { server.mock_add_version(v2, vy, 1000, b"false start y"); server.mock_set_latest(v3); - server.cleanup().unwrap(); - - let mut expected = server.empty_copy(); + let mut expected = server.empty_clone(); expected.mock_add_version(v1, v2, 1000, b"second"); expected.mock_add_version(v2, v3, 1000, b"third"); expected.mock_set_latest(v3); + + assert_ne!(server.unencrypted(), expected.unencrypted()); + server.cleanup().unwrap(); assert_eq!(server.unencrypted(), expected.unencrypted()); } @@ -1004,36 +1019,36 @@ mod tests { server.mock_add_snapshot(vy, 1000, b"snap y"); server.mock_set_latest(v3); - server.cleanup().unwrap(); - - let mut expected = server.empty_copy(); + let mut expected = server.empty_clone(); expected.mock_add_version(v1, v2, 1000, b"second"); expected.mock_add_version(v2, v3, 1000, b"third"); expected.mock_add_snapshot(v2, 1000, b"snap 2"); expected.mock_set_latest(v3); + + assert_ne!(server.unencrypted(), expected.unencrypted()); + server.cleanup().unwrap(); assert_eq!(server.unencrypted(), expected.unencrypted()); } #[test] fn cleanup_old_versions_no_snapshot() { + // If there are old versions ,but no snapshot, nothing is cleaned up. let mut server = make_server(); let (v1, v2, v3) = (Uuid::new_v4(), Uuid::new_v4(), Uuid::new_v4()); server.mock_add_version(v1, v2, 200, b"second"); server.mock_add_version(v2, v3, 300, b"third"); server.mock_set_latest(v3); - server.cleanup().unwrap(); + let expected = server.clone(); - // Nothing is deleted. - let mut expected = server.empty_copy(); - expected.mock_add_version(v1, v2, 200, b"second"); - expected.mock_add_version(v2, v3, 300, b"third"); - expected.mock_set_latest(v3); + server.cleanup().unwrap(); assert_eq!(server.unencrypted(), expected.unencrypted()); } #[test] fn cleanup_old_versions_with_snapshot() { + // If there are old versions that are also older than a snapshot, they are + // cleaned up. let mut server = make_server(); let (v1, v2, v3) = (Uuid::new_v4(), Uuid::new_v4(), Uuid::new_v4()); let (v4, v5, v6) = (Uuid::new_v4(), Uuid::new_v4(), Uuid::new_v4()); @@ -1045,19 +1060,21 @@ mod tests { server.mock_add_version(v5, v6, 1600, b"sixth"); server.mock_set_latest(v6); - server.cleanup().unwrap(); - - let mut expected = server.empty_copy(); + let mut expected = server.empty_clone(); expected.mock_add_version(v3, v4, 1400, b"fourth"); // Not old enough to be deleted. expected.mock_add_version(v4, v5, 1500, b"fifth"); expected.mock_add_snapshot(v5, 1501, b"snap 1"); expected.mock_add_version(v5, v6, 1600, b"sixth"); expected.mock_set_latest(v6); + + assert_ne!(server.unencrypted(), expected.unencrypted()); + server.cleanup().unwrap(); assert_eq!(server.unencrypted(), expected.unencrypted()); } #[test] fn cleanup_old_versions_newer_than_snapshot() { + // Old versions that are newer than the latest snapshot are not cleaned up. let mut server = make_server(); let (v1, v2, v3) = (Uuid::new_v4(), Uuid::new_v4(), Uuid::new_v4()); let (v4, v5, v6) = (Uuid::new_v4(), Uuid::new_v4(), Uuid::new_v4()); @@ -1069,19 +1086,21 @@ mod tests { server.mock_add_version(v5, v6, 600, b"sixth"); server.mock_set_latest(v6); - server.cleanup().unwrap(); - - let mut expected = server.empty_copy(); + let mut expected = server.empty_clone(); expected.mock_add_snapshot(v3, 301, b"snap 1"); expected.mock_add_version(v3, v4, 400, b"fourth"); expected.mock_add_version(v4, v5, 500, b"fifth"); expected.mock_add_version(v5, v6, 600, b"sixth"); expected.mock_set_latest(v6); + + assert_ne!(server.unencrypted(), expected.unencrypted()); + server.cleanup().unwrap(); assert_eq!(server.unencrypted(), expected.unencrypted()); } #[test] fn cleanup_children_of_latest() { + // New versions that are children of the latest version are not cleaned up. let mut server = make_server(); let (v1, v2, v3) = (Uuid::new_v4(), Uuid::new_v4(), Uuid::new_v4()); let (vnew1, vnew2) = (Uuid::new_v4(), Uuid::new_v4()); @@ -1092,15 +1111,9 @@ mod tests { // Two replicas are adding new versions, but v3 is still latest. server.mock_set_latest(v3); - server.cleanup().unwrap(); + let expected = server.clone(); - let mut expected = server.empty_copy(); - expected.mock_add_version(v1, v2, 1000, b"second"); - expected.mock_add_version(v2, v3, 1000, b"third"); - // New versions that are children of latest are not deleted. - expected.mock_add_version(v3, vnew1, 1000, b"new 1"); - expected.mock_add_version(v3, vnew2, 1000, b"new 2"); - expected.mock_set_latest(v3); + server.cleanup().unwrap(); assert_eq!(server.unencrypted(), expected.unencrypted()); } @@ -1108,9 +1121,12 @@ mod tests { fn add_snapshot() { let mut server = make_server(); let v = Uuid::new_v4(); - server.add_snapshot(v, b"SNAP".to_vec()).unwrap(); - let mut expected = server.empty_copy(); + + let mut expected = server.empty_clone(); expected.mock_add_snapshot(v, INSERTION_TIME, b"SNAP"); + + assert_ne!(server.unencrypted(), expected.unencrypted()); + server.add_snapshot(v, b"SNAP".to_vec()).unwrap(); assert_eq!(server.unencrypted(), expected.unencrypted()); }