Store data necessary to undo ReplicaOps

taskchampion/src/storage/op.rs

@@ -1,4 +1,5 @@
 use crate::server::SyncOp;
+use crate::storage::TaskMap;
 use chrono::{DateTime, Utc};
 use serde::{Deserialize, Serialize};
 use uuid::Uuid;
@@ -9,21 +10,22 @@ use uuid::Uuid;
 pub enum ReplicaOp {
     /// Create a new task.
     ///
-    /// On application, if the task already exists, the operation does nothing.
+    /// On undo, the task is deleted.
     Create { uuid: Uuid },
 
     /// Delete an existing task.
     ///
-    /// On application, if the task does not exist, the operation does nothing.
+    /// On undo, the task's data is restored from old_task.
-    Delete { uuid: Uuid },
+    Delete { uuid: Uuid, old_task: TaskMap },
 
     /// Update an existing task, setting the given property to the given value.  If the value is
     /// None, then the corresponding property is deleted.
     ///
-    /// If the given task does not exist, the operation does nothing.  
+    /// On undo, the property is set back to its previous value.
     Update {
         uuid: Uuid,
         property: String,
+        old_value: Option<String>,
         value: Option<String>,
         timestamp: DateTime<Utc>,
     },
@@ -34,12 +36,13 @@ impl ReplicaOp {
     pub fn into_sync(self) -> SyncOp {
         match self {
             Self::Create { uuid } => SyncOp::Create { uuid },
-            Self::Delete { uuid } => SyncOp::Delete { uuid },
+            Self::Delete { uuid, .. } => SyncOp::Delete { uuid },
             Self::Update {
                 uuid,
                 property,
                 value,
                 timestamp,
+                ..
             } => SyncOp::Update {
                 uuid,
                 property,
@@ -56,6 +59,8 @@ mod test {
     use chrono::Utc;
     use pretty_assertions::assert_eq;
 
+    use ReplicaOp::*;
+
     #[test]
     fn test_json_create() -> anyhow::Result<()> {
         let uuid = Uuid::new_v4();
@@ -70,9 +75,16 @@ mod test {
     #[test]
     fn test_json_delete() -> anyhow::Result<()> {
         let uuid = Uuid::new_v4();
-        let op = Delete { uuid };
+        let old_task = vec![("foo".into(), "bar".into())].drain(..).collect();
+        let op = Delete { uuid, old_task };
         let json = serde_json::to_string(&op)?;
-        assert_eq!(json, format!(r#"{{"Delete":{{"uuid":"{}"}}}}"#, uuid));
+        assert_eq!(
+            json,
+            format!(
+                r#"{{"Delete":{{"uuid":"{}","old_task":{{"foo":"bar"}}}}}}"#,
+                uuid
+            )
+        );
         let deser: ReplicaOp = serde_json::from_str(&json)?;
         assert_eq!(deser, op);
         Ok(())
@@ -86,6 +98,7 @@ mod test {
         let op = Update {
             uuid,
             property: "abc".into(),
+            old_value: Some("true".into()),
             value: Some("false".into()),
             timestamp,
         };
@@ -94,7 +107,7 @@ mod test {
         assert_eq!(
             json,
             format!(
-                r#"{{"Update":{{"uuid":"{}","property":"abc","value":"false","timestamp":"{:?}"}}}}"#,
+                r#"{{"Update":{{"uuid":"{}","property":"abc","old_value":"true","value":"false","timestamp":"{:?}"}}}}"#,
                 uuid, timestamp,
             )
         );
@@ -111,6 +124,7 @@ mod test {
         let op = Update {
             uuid,
             property: "abc".into(),
+            old_value: None,
             value: None,
             timestamp,
         };
@@ -119,7 +133,7 @@ mod test {
         assert_eq!(
             json,
             format!(
-                r#"{{"Update":{{"uuid":"{}","property":"abc","value":null,"timestamp":"{:?}"}}}}"#,
+                r#"{{"Update":{{"uuid":"{}","property":"abc","old_value":null,"value":null,"timestamp":"{:?}"}}}}"#,
                 uuid, timestamp,
             )
         );
@@ -137,7 +151,14 @@ mod test {
     #[test]
     fn test_into_sync_delete() {
         let uuid = Uuid::new_v4();
-        assert_eq!(Delete { uuid }.into_sync(), SyncOp::Delete { uuid });
+        assert_eq!(
+            Delete {
+                uuid,
+                old_task: TaskMap::new()
+            }
+            .into_sync(),
+            SyncOp::Delete { uuid }
+        );
     }
 
     #[test]
@@ -148,6 +169,7 @@ mod test {
             Update {
                 uuid,
                 property: "prop".into(),
+                old_value: Some("foo".into()),
                 value: Some("v".into()),
                 timestamp,
             }

taskchampion/src/storage/sqlite.rs

@@ -633,8 +633,14 @@ mod test {
         {
             let mut txn = storage.txn()?;
             txn.set_operations(vec![
-                ReplicaOp::Delete { uuid: uuid2 },
+                ReplicaOp::Delete {
-                ReplicaOp::Delete { uuid: uuid1 },
+                    uuid: uuid2,
+                    old_task: TaskMap::new(),
+                },
+                ReplicaOp::Delete {
+                    uuid: uuid1,
+                    old_task: TaskMap::new(),
+                },
             ])?;
             txn.commit()?;
         }
@@ -643,7 +649,10 @@ mod test {
         {
             let mut txn = storage.txn()?;
             txn.add_operation(ReplicaOp::Create { uuid: uuid3 })?;
-            txn.add_operation(ReplicaOp::Delete { uuid: uuid3 })?;
+            txn.add_operation(ReplicaOp::Delete {
+                uuid: uuid3,
+                old_task: TaskMap::new(),
+            })?;
             txn.commit()?;
         }
 
@@ -654,10 +663,19 @@ mod test {
             assert_eq!(
                 ops,
                 vec![
-                    ReplicaOp::Delete { uuid: uuid2 },
+                    ReplicaOp::Delete {
-                    ReplicaOp::Delete { uuid: uuid1 },
+                        uuid: uuid2,
+                        old_task: TaskMap::new()
+                    },
+                    ReplicaOp::Delete {
+                        uuid: uuid1,
+                        old_task: TaskMap::new()
+                    },
                     ReplicaOp::Create { uuid: uuid3 },
-                    ReplicaOp::Delete { uuid: uuid3 },
+                    ReplicaOp::Delete {
+                        uuid: uuid3,
+                        old_task: TaskMap::new()
+                    },
                 ]
             );
         }

taskchampion/src/taskdb/apply.rs

@@ -20,10 +20,12 @@ pub(super) fn apply(txn: &mut dyn StorageTxn, op: SyncOp) -> anyhow::Result<Task
         }
         SyncOp::Delete { uuid } => {
             let task = txn.get_task(uuid)?;
-            // (we'll need _task in the next commit)
+            if let Some(task) = task {
-            if let Some(_task) = task {
                 txn.delete_task(uuid)?;
-                txn.add_operation(ReplicaOp::Delete { uuid })?;
+                txn.add_operation(ReplicaOp::Delete {
+                    uuid,
+                    old_task: task,
+                })?;
                 txn.commit()?;
                 Ok(TaskMap::new())
             } else {
@@ -38,6 +40,7 @@ pub(super) fn apply(txn: &mut dyn StorageTxn, op: SyncOp) -> anyhow::Result<Task
         } => {
             let task = txn.get_task(uuid)?;
             if let Some(mut task) = task {
+                let old_value = task.get(&property).cloned();
                 if let Some(ref v) = value {
                     task.insert(property.clone(), v.clone());
                 } else {
@@ -47,6 +50,7 @@ pub(super) fn apply(txn: &mut dyn StorageTxn, op: SyncOp) -> anyhow::Result<Task
                 txn.add_operation(ReplicaOp::Update {
                     uuid,
                     property,
+                    old_value,
                     value,
                     timestamp,
                 })?;
@@ -149,6 +153,7 @@ mod tests {
                 ReplicaOp::Update {
                     uuid,
                     property: "title".into(),
+                    old_value: None,
                     value: Some("my task".into()),
                     timestamp: now
                 }
@@ -226,18 +231,21 @@ mod tests {
                 ReplicaOp::Update {
                     uuid,
                     property: "title".into(),
+                    old_value: None,
                     value: Some("my task".into()),
                     timestamp: now,
                 },
                 ReplicaOp::Update {
                     uuid,
                     property: "priority".into(),
+                    old_value: None,
                     value: Some("H".into()),
                     timestamp: now,
                 },
                 ReplicaOp::Update {
                     uuid,
                     property: "title".into(),
+                    old_value: Some("my task".into()),
                     value: None,
                     timestamp: now,
                 }
@@ -273,22 +281,52 @@ mod tests {
     fn test_apply_create_delete() -> anyhow::Result<()> {
         let mut db = TaskDb::new_inmemory();
         let uuid = Uuid::new_v4();
-        let op1 = SyncOp::Create { uuid };
+        let now = Utc::now();
-        let op2 = SyncOp::Delete { uuid };
 
+        let op1 = SyncOp::Create { uuid };
         {
             let mut txn = db.storage.txn()?;
             let taskmap = apply(txn.as_mut(), op1)?;
             assert_eq!(taskmap.len(), 0);
+        }
+
+        let op2 = SyncOp::Update {
+            uuid,
+            property: String::from("priority"),
+            value: Some("H".into()),
+            timestamp: now,
+        };
+        {
+            let mut txn = db.storage.txn()?;
             let taskmap = apply(txn.as_mut(), op2)?;
+            assert_eq!(taskmap.get("priority"), Some(&"H".to_owned()));
+            txn.commit()?;
+        }
+
+        let op3 = SyncOp::Delete { uuid };
+        {
+            let mut txn = db.storage.txn()?;
+            let taskmap = apply(txn.as_mut(), op3)?;
             assert_eq!(taskmap.len(), 0);
             txn.commit()?;
         }
 
         assert_eq!(db.sorted_tasks(), vec![]);
+        let mut old_task = TaskMap::new();
+        old_task.insert("priority".into(), "H".into());
         assert_eq!(
             db.operations(),
-            vec![ReplicaOp::Create { uuid }, ReplicaOp::Delete { uuid },]
+            vec![
+                ReplicaOp::Create { uuid },
+                ReplicaOp::Update {
+                    uuid,
+                    property: "priority".into(),
+                    old_value: None,
+                    value: Some("H".into()),
+                    timestamp: now,
+                },
+                ReplicaOp::Delete { uuid, old_task },
+            ]
         );
 
         Ok(())