diff --git a/Cargo.lock b/Cargo.lock index 690adfc8c..86be4733f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1528,6 +1528,7 @@ dependencies = [ "actix-web", "anyhow", "cc", + "chrono", "env_logger 0.8.4", "lazy_static", "log", diff --git a/cli/src/argparse/subcommand.rs b/cli/src/argparse/subcommand.rs index 22c270ba3..553350740 100644 --- a/cli/src/argparse/subcommand.rs +++ b/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: " 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: " 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 { diff --git a/cli/src/invocation/cmd/gc.rs b/cli/src/invocation/cmd/gc.rs index f35eaed76..e9b8a2828 100644 --- a/cli/src/invocation/cmd/gc.rs +++ b/cli/src/invocation/cmd/gc.rs @@ -4,6 +4,8 @@ use termcolor::WriteColor; pub(crate) fn execute(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(()) } diff --git a/docs/src/taskdb.md b/docs/src/taskdb.md index 40ee74b9c..b70e9bf3c 100644 --- a/docs/src/taskdb.md +++ b/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. diff --git a/integration-tests/Cargo.toml b/integration-tests/Cargo.toml index 1d57b1a7d..89e3f0fbc 100644 --- a/integration-tests/Cargo.toml +++ b/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" } diff --git a/integration-tests/tests/update-and-delete-sync.rs b/integration-tests/tests/update-and-delete-sync.rs new file mode 100644 index 000000000..e75dceae0 --- /dev/null +++ b/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(()) +} diff --git a/taskchampion/src/replica.rs b/taskchampion/src/replica.rs index 79938423f..686b34842 100644 --- a/taskchampion/src/replica.rs +++ b/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")); + } + } } diff --git a/taskchampion/src/task/task.rs b/taskchampion/src/task/task.rs index d5517f5b4..cd8a12f85 100644 --- a/taskchampion/src/task/task.rs +++ b/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, ) -> 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);