Merge pull request #341 from djmitche/issue26

Add support for expiration

Cargo.lock

@@ -1528,6 +1528,7 @@ dependencies = [
  "actix-web",
  "anyhow",
  "cc",
+ "chrono",
  "env_logger 0.8.4",
  "lazy_static",
  "log",

cli/src/argparse/subcommand.rs

@@ -218,6 +218,7 @@ impl Modify {
                 "start" => modification.active = Some(true),
                 "stop" => modification.active = Some(false),
                 "done" => modification.status = Some(Status::Completed),
+                "delete" => modification.status = Some(Status::Deleted),
                 "annotate" => {
                     // what would be parsed as a description is, here, used as the annotation
                     if let DescriptionMod::Set(s) = modification.description {
@@ -243,6 +244,7 @@ impl Modify {
                     arg_matching(literal("start")),
                     arg_matching(literal("stop")),
                     arg_matching(literal("done")),
+                    arg_matching(literal("delete")),
                     arg_matching(literal("annotate")),
                 )),
                 Modification::parse,
@@ -297,10 +299,19 @@ impl Modify {
                 Mark all tasks matching the required filter as completed, additionally applying any given
                 modifications.",
         });
+        u.subcommands.push(usage::Subcommand {
+            name: "delete",
+            syntax: "<filter> delete [modification]",
+            summary: "Mark tasks as deleted",
+            description: "
+                Mark all tasks matching the required filter as deleted, additionally applying any given
+                modifications.  Deleted tasks remain until they are expired in a 'ta gc' operation at
+                least six months after their last modification.",
+        });
         u.subcommands.push(usage::Subcommand {
             name: "annotate",
             syntax: "<filter> annotate [modification]",
-            summary: "Mark tasks as completed",
+            summary: "Annotate a task",
             description: "
                 Add an annotation to all tasks matching the required filter.",
         });
@@ -761,6 +772,23 @@ mod test {
         );
     }
 
+    #[test]
+    fn test_delete() {
+        let subcommand = Subcommand::Modify {
+            filter: Filter {
+                conditions: vec![Condition::IdList(vec![TaskId::WorkingSetId(123)])],
+            },
+            modification: Modification {
+                status: Some(Status::Deleted),
+                ..Default::default()
+            },
+        };
+        assert_eq!(
+            Subcommand::parse(argv!["123", "delete"]).unwrap(),
+            (&EMPTY[..], subcommand)
+        );
+    }
+
     #[test]
     fn test_annotate() {
         let subcommand = Subcommand::Modify {

cli/src/invocation/cmd/gc.rs

@@ -4,6 +4,8 @@ use termcolor::WriteColor;
 pub(crate) fn execute<W: WriteColor>(w: &mut W, replica: &mut Replica) -> Result<(), crate::Error> {
     log::debug!("rebuilding working set");
     replica.rebuild_working_set(true)?;
+    log::debug!("expiring old tasks");
+    replica.expire_tasks()?;
     writeln!(w, "garbage collected.")?;
     Ok(())
 }

docs/src/taskdb.md

@@ -25,4 +25,8 @@ Operations are described in [Replica Storage](./storage.md).
 Each operation is added to the list of operations in the storage, and simultaneously applied to the tasks in that storage.
 Operations are checked for validity as they are applied.
 
+## Deletion and Expiration
 
+Deletion of a task merely changes the task's status to "deleted", leaving it in the Task database.
+Actual removal of tasks from the task database takes place as part of _expiration_, triggered by the user as part of a garbage-collection process.
+Expiration removes tasks with a `modified` property more than 180 days in the past, by creating a `Delete(uuid)` operation.

integration-tests/Cargo.toml

@@ -7,6 +7,7 @@ publish = false
 build = "build.rs"
 
 [dependencies]
+chrono = { version = "^0.4.10", features = ["serde"] }
 taskchampion = { path = "../taskchampion" }
 taskchampion-sync-server = { path = "../sync-server" }
 

integration-tests/tests/update-and-delete-sync.rs

@@ -0,0 +1,72 @@
+use chrono::{TimeZone, Utc};
+use taskchampion::{Replica, ServerConfig, Status, StorageConfig};
+use tempfile::TempDir;
+
+#[test]
+fn update_and_delete_sync_delete_first() -> anyhow::Result<()> {
+    update_and_delete_sync(true)
+}
+
+#[test]
+fn update_and_delete_sync_update_first() -> anyhow::Result<()> {
+    update_and_delete_sync(false)
+}
+
+/// Test what happens when an update is sync'd into a repo after a task is deleted.
+/// If delete_first, then the deletion is sync'd to the server first; otherwise
+/// the update is sync'd first.  Either way, the task is gone.
+fn update_and_delete_sync(delete_first: bool) -> anyhow::Result<()> {
+    // set up two replicas, and demonstrate replication between them
+    let mut rep1 = Replica::new(StorageConfig::InMemory.into_storage()?);
+    let mut rep2 = Replica::new(StorageConfig::InMemory.into_storage()?);
+
+    let tmp_dir = TempDir::new().expect("TempDir failed");
+    let mut server = ServerConfig::Local {
+        server_dir: tmp_dir.path().to_path_buf(),
+    }
+    .into_server()?;
+
+    // add a task on rep1, and sync it to rep2
+    let t = rep1.new_task(Status::Pending, "test task".into())?;
+    let u = t.get_uuid();
+
+    rep1.sync(&mut server, false)?;
+    rep2.sync(&mut server, false)?;
+
+    // mark the task as deleted, long in the past, on rep2
+    {
+        let mut t = rep2.get_task(u)?.unwrap().into_mut(&mut rep2);
+        t.delete()?;
+        t.set_modified(Utc.ymd(1980, 1, 1).and_hms(0, 0, 0))?;
+    }
+
+    // sync it back to rep1
+    rep2.sync(&mut server, false)?;
+    rep1.sync(&mut server, false)?;
+
+    // expire the task on rep1 and check that it is gone locally
+    rep1.expire_tasks()?;
+    assert!(rep1.get_task(u)?.is_none());
+
+    // modify the task on rep2
+    {
+        let mut t = rep2.get_task(u)?.unwrap().into_mut(&mut rep2);
+        t.set_description("modified".to_string())?;
+    }
+
+    // sync back and forth
+    if delete_first {
+        rep1.sync(&mut server, false)?;
+    }
+    rep2.sync(&mut server, false)?;
+    rep1.sync(&mut server, false)?;
+    if !delete_first {
+        rep2.sync(&mut server, false)?;
+    }
+
+    // check that the task is gone on both replicas
+    assert!(rep1.get_task(u)?.is_none());
+    assert!(rep2.get_task(u)?.is_none());
+
+    Ok(())
+}

taskchampion/src/replica.rs

@@ -4,7 +4,7 @@ use crate::task::{Status, Task};
 use crate::taskdb::TaskDb;
 use crate::workingset::WorkingSet;
 use anyhow::Context;
-use chrono::Utc;
+use chrono::{Duration, Utc};
 use log::trace;
 use std::collections::HashMap;
 use uuid::Uuid;
@@ -127,7 +127,6 @@ impl Replica {
     /// Delete a task.  The task must exist.  Note that this is different from setting status to
     /// Deleted; this is the final purge of the task.  This is not a public method as deletion
     /// should only occur through expiration.
-    #[allow(dead_code)]
     fn delete_task(&mut self, uuid: Uuid) -> anyhow::Result<()> {
         self.add_undo_point(false)?;
         self.taskdb.apply(SyncOp::Delete { uuid })?;
@@ -175,6 +174,29 @@ impl Replica {
         Ok(())
     }
 
+    /// Expire old, deleted tasks.
+    ///
+    /// Expiration entails removal of tasks from the replica. Any modifications that occur after
+    /// the deletion (such as operations synchronized from other replicas) will do nothing.
+    ///
+    /// Tasks are eligible for expiration when they have status Deleted and have not been modified
+    /// for 180 days (about six months). Note that completed tasks are not eligible.
+    pub fn expire_tasks(&mut self) -> anyhow::Result<()> {
+        let six_mos_ago = Utc::now() - Duration::days(180);
+        self.all_tasks()?
+            .iter()
+            .filter(|(_, t)| t.get_status() == Status::Deleted)
+            .filter(|(_, t)| {
+                if let Some(m) = t.get_modified() {
+                    m < six_mos_ago
+                } else {
+                    false
+                }
+            })
+            .try_for_each(|(u, _)| self.delete_task(*u))?;
+        Ok(())
+    }
+
     /// Add an UndoPoint, if one has not already been added by this Replica.  This occurs
     /// automatically when a change is made.  The `force` flag allows forcing a new UndoPoint
     /// even if one has already been created by this Replica, and may be useful when a Replica
@@ -193,6 +215,7 @@ mod tests {
     use super::*;
     use crate::storage::ReplicaOp;
     use crate::task::Status;
+    use chrono::TimeZone;
     use pretty_assertions::assert_eq;
     use uuid::Uuid;
 
@@ -392,4 +415,34 @@ mod tests {
         let uuid = Uuid::new_v4();
         assert_eq!(rep.get_task(uuid).unwrap(), None);
     }
+
+    #[test]
+    fn expire() {
+        let mut rep = Replica::new_inmemory();
+        let mut t;
+
+        rep.new_task(Status::Pending, "keeper 1".into()).unwrap();
+        rep.new_task(Status::Completed, "keeper 2".into()).unwrap();
+
+        t = rep.new_task(Status::Deleted, "keeper 3".into()).unwrap();
+        {
+            let mut t = t.into_mut(&mut rep);
+            // set entry, with modification set as a side-effect
+            t.set_entry(Some(Utc::now())).unwrap();
+        }
+
+        t = rep.new_task(Status::Deleted, "goner".into()).unwrap();
+        {
+            let mut t = t.into_mut(&mut rep);
+            t.set_modified(Utc.ymd(1980, 1, 1).and_hms(0, 0, 0))
+                .unwrap();
+        }
+
+        rep.expire_tasks().unwrap();
+
+        for (_, t) in rep.all_tasks().unwrap() {
+            println!("got task {}", t.get_description());
+            assert!(t.get_description().starts_with("keeper"));
+        }
+    }
 }

taskchampion/src/task/task.rs

@@ -425,7 +425,7 @@ impl<'r> TaskMut<'r> {
 
     // -- utility functions
 
-    fn lastmod(&mut self) -> anyhow::Result<()> {
+    fn update_modified(&mut self) -> anyhow::Result<()> {
         if !self.updated_modified {
             let now = format!("{}", Utc::now().timestamp());
             trace!("task {}: set property modified={:?}", self.task.uuid, now);
@@ -443,7 +443,10 @@ impl<'r> TaskMut<'r> {
         value: Option<String>,
     ) -> anyhow::Result<()> {
         let property = property.into();
-        self.lastmod()?;
+        // updated the modified timestamp unless we are setting it explicitly
+        if &property != "modified" {
+            self.update_modified()?;
+        }
 
         if let Some(ref v) = value {
             trace!("task {}: set property {}={:?}", self.task.uuid, property, v);