diff --git a/cli/src/invocation/report.rs b/cli/src/invocation/report.rs index 5c75f191e..81b4c8622 100644 --- a/cli/src/invocation/report.rs +++ b/cli/src/invocation/report.rs @@ -406,9 +406,12 @@ mod test { let task = replica.get_task(uuids[0]).unwrap().unwrap(); assert_eq!( task_column(&task, &column, &working_set), - s!("+PENDING +bar +foo") + s!("+PENDING +UNBLOCKED +bar +foo") ); let task = replica.get_task(uuids[2]).unwrap().unwrap(); - assert_eq!(task_column(&task, &column, &working_set), s!("+PENDING")); + assert_eq!( + task_column(&task, &column, &working_set), + s!("+PENDING +UNBLOCKED") + ); } } diff --git a/cli/src/macros.rs b/cli/src/macros.rs index 0f0eab6ad..f2cbe803b 100644 --- a/cli/src/macros.rs +++ b/cli/src/macros.rs @@ -15,12 +15,13 @@ macro_rules! argv { // NOTE: in Rust 1.56.0, this can be changed to HashSet::from([..]) #[cfg(test)] macro_rules! set( - { $($key:expr),+ } => { + { $($key:expr),* $(,)? } => { { + #[allow(unused_mut)] let mut s = ::std::collections::HashSet::new(); $( s.insert($key); - )+ + )* s } }; diff --git a/taskchampion/src/depmap.rs b/taskchampion/src/depmap.rs new file mode 100644 index 000000000..d2f6225bf --- /dev/null +++ b/taskchampion/src/depmap.rs @@ -0,0 +1,81 @@ +use uuid::Uuid; + +/// DependencyMap stores information on task dependencies between pending tasks. +/// +/// This information requires a scan of the working set to generate, so it is +/// typically calculated once and re-used. +#[derive(Debug, PartialEq)] +pub struct DependencyMap { + /// Edges of the dependency graph. If (a, b) is in this array, then task a depends on tsak b. + edges: Vec<(Uuid, Uuid)>, +} + +impl DependencyMap { + /// Create a new, empty DependencyMap. + pub(super) fn new() -> Self { + Self { edges: Vec::new() } + } + + /// Add a dependency of a on b. + pub(super) fn add_dependency(&mut self, a: Uuid, b: Uuid) { + self.edges.push((a, b)); + } + + /// Return an iterator of Uuids on which task `deps_of` depends. This is equivalent to + /// `task.get_dependencies()`. + pub fn dependencies(&self, dep_of: Uuid) -> impl Iterator + '_ { + self.edges + .iter() + .filter_map(move |(a, b)| if a == &dep_of { Some(*b) } else { None }) + } + + /// Return an iterator of Uuids of tasks that depend on `dep_on` + /// `task.get_dependencies()`. + pub fn dependents(&self, dep_on: Uuid) -> impl Iterator + '_ { + self.edges + .iter() + .filter_map(move |(a, b)| if b == &dep_on { Some(*a) } else { None }) + } +} + +#[cfg(test)] +mod test { + use super::*; + use pretty_assertions::assert_eq; + use std::collections::HashSet; + + #[test] + fn dependencies() { + let t = Uuid::new_v4(); + let uuid1 = Uuid::new_v4(); + let uuid2 = Uuid::new_v4(); + let mut dm = DependencyMap::new(); + + dm.add_dependency(t, uuid1); + dm.add_dependency(t, uuid2); + dm.add_dependency(Uuid::new_v4(), t); + dm.add_dependency(Uuid::new_v4(), uuid1); + dm.add_dependency(uuid2, Uuid::new_v4()); + + assert_eq!( + dm.dependencies(t).collect::>(), + set![uuid1, uuid2] + ); + } + + #[test] + fn dependents() { + let t = Uuid::new_v4(); + let uuid1 = Uuid::new_v4(); + let uuid2 = Uuid::new_v4(); + let mut dm = DependencyMap::new(); + + dm.add_dependency(uuid1, t); + dm.add_dependency(uuid2, t); + dm.add_dependency(t, Uuid::new_v4()); + dm.add_dependency(Uuid::new_v4(), uuid1); + dm.add_dependency(uuid2, Uuid::new_v4()); + + assert_eq!(dm.dependents(t).collect::>(), set![uuid1, uuid2]); + } +} diff --git a/taskchampion/src/lib.rs b/taskchampion/src/lib.rs index 24dcd852d..2e22b992e 100644 --- a/taskchampion/src/lib.rs +++ b/taskchampion/src/lib.rs @@ -45,6 +45,10 @@ This crate supports Rust version 1.47 and higher. */ +// NOTE: it's important that this 'mod' comes first so that the macros can be used in other modules +mod macros; + +mod depmap; mod errors; mod replica; pub mod server; @@ -54,6 +58,7 @@ mod taskdb; mod utils; mod workingset; +pub use depmap::DependencyMap; pub use errors::Error; pub use replica::Replica; pub use server::{Server, ServerConfig}; diff --git a/taskchampion/src/macros.rs b/taskchampion/src/macros.rs new file mode 100644 index 000000000..eb34b4640 --- /dev/null +++ b/taskchampion/src/macros.rs @@ -0,0 +1,17 @@ +#![macro_use] + +/// Create a hashset, similar to vec! +// NOTE: in Rust 1.56.0, this can be changed to HashSet::from([..]) +#[cfg(test)] +macro_rules! set( + { $($key:expr),* $(,)? } => { + { + #[allow(unused_mut)] + let mut s = ::std::collections::HashSet::new(); + $( + s.insert($key); + )* + s + } + }; +); diff --git a/taskchampion/src/replica.rs b/taskchampion/src/replica.rs index 686b34842..3787f3b25 100644 --- a/taskchampion/src/replica.rs +++ b/taskchampion/src/replica.rs @@ -1,3 +1,4 @@ +use crate::depmap::DependencyMap; use crate::server::{Server, SyncOp}; use crate::storage::{Storage, TaskMap}; use crate::task::{Status, Task}; @@ -7,6 +8,7 @@ use anyhow::Context; use chrono::{Duration, Utc}; use log::trace; use std::collections::HashMap; +use std::rc::Rc; use uuid::Uuid; /// A replica represents an instance of a user's task data, providing an easy interface @@ -28,7 +30,12 @@ use uuid::Uuid; /// during the garbage-collection process. pub struct Replica { taskdb: TaskDb, + + /// If true, this replica has already added an undo point. added_undo_point: bool, + + /// The dependency map for this replica, if it has been calculated. + depmap: Option>, } impl Replica { @@ -36,6 +43,7 @@ impl Replica { Replica { taskdb: TaskDb::new(storage), added_undo_point: false, + depmap: None, } } @@ -76,9 +84,10 @@ impl Replica { /// Get all tasks represented as a map keyed by UUID pub fn all_tasks(&mut self) -> anyhow::Result> { + let depmap = self.dependency_map(false)?; let mut res = HashMap::new(); for (uuid, tm) in self.taskdb.all_tasks()?.drain(..) { - res.insert(uuid, Task::new(uuid, tm)); + res.insert(uuid, Task::new(uuid, tm, depmap.clone())); } Ok(res) } @@ -94,12 +103,47 @@ impl Replica { Ok(WorkingSet::new(self.taskdb.working_set()?)) } + /// Get the dependency map for all pending tasks. + /// + /// The data in this map is cached when it is first requested and may not contain modifications + /// made locally in this Replica instance. The result is reference-counted and may + /// outlive the Replica. + /// + /// If `force` is true, then the result is re-calculated from the current state of the replica, + /// although previously-returned dependency maps are not updated. + pub fn dependency_map(&mut self, force: bool) -> anyhow::Result> { + if force || self.depmap.is_none() { + let mut dm = DependencyMap::new(); + let ws = self.working_set()?; + for i in 1..=ws.largest_index() { + if let Some(u) = ws.by_index(i) { + // note: we can't use self.get_task here, as that depends on a + // DependencyMap + if let Some(taskmap) = self.taskdb.get_task(u)? { + for p in taskmap.keys() { + if let Some(dep_str) = p.strip_prefix("dep_") { + if let Ok(dep) = Uuid::parse_str(dep_str) { + dm.add_dependency(u, dep); + } + } + } + } + } + } + self.depmap = Some(Rc::new(dm)); + } + + // at this point self.depmap is guaranteed to be Some(_) + Ok(self.depmap.as_ref().unwrap().clone()) + } + /// Get an existing task by its UUID pub fn get_task(&mut self, uuid: Uuid) -> anyhow::Result> { + let depmap = self.dependency_map(false)?; Ok(self .taskdb .get_task(uuid)? - .map(move |tm| Task::new(uuid, tm))) + .map(move |tm| Task::new(uuid, tm, depmap))) } /// Create a new task. @@ -107,7 +151,8 @@ impl Replica { let uuid = Uuid::new_v4(); self.add_undo_point(false)?; let taskmap = self.taskdb.apply(SyncOp::Create { uuid })?; - let mut task = Task::new(uuid, taskmap).into_mut(self); + let depmap = self.dependency_map(false)?; + let mut task = Task::new(uuid, taskmap, depmap).into_mut(self); task.set_description(description)?; task.set_status(status)?; task.set_entry(Some(Utc::now()))?; @@ -121,7 +166,8 @@ impl Replica { pub fn import_task_with_uuid(&mut self, uuid: Uuid) -> anyhow::Result { self.add_undo_point(false)?; let taskmap = self.taskdb.apply(SyncOp::Create { uuid })?; - Ok(Task::new(uuid, taskmap)) + let depmap = self.dependency_map(false)?; + Ok(Task::new(uuid, taskmap, depmap)) } /// Delete a task. The task must exist. Note that this is different from setting status to @@ -217,6 +263,7 @@ mod tests { use crate::task::Status; use chrono::TimeZone; use pretty_assertions::assert_eq; + use std::collections::HashSet; use uuid::Uuid; #[test] @@ -445,4 +492,67 @@ mod tests { assert!(t.get_description().starts_with("keeper")); } } + + #[test] + fn dependency_map() { + let mut rep = Replica::new_inmemory(); + + let mut tasks = vec![]; + for _ in 0..4 { + tasks.push(rep.new_task(Status::Pending, "t".into()).unwrap()); + } + + let uuids: Vec<_> = tasks.iter().map(|t| t.get_uuid()).collect(); + + // t[3] depends on t[2], and t[1] + { + let mut t = tasks.pop().unwrap().into_mut(&mut rep); + t.add_dependency(uuids[2]).unwrap(); + t.add_dependency(uuids[1]).unwrap(); + } + + // t[2] depends on t[0] + { + let mut t = tasks.pop().unwrap().into_mut(&mut rep); + t.add_dependency(uuids[0]).unwrap(); + } + + // t[1] depends on t[0] + { + let mut t = tasks.pop().unwrap().into_mut(&mut rep); + t.add_dependency(uuids[0]).unwrap(); + } + + // generate the dependency map, forcing an update based on the newly-added + // dependencies + let dm = rep.dependency_map(true).unwrap(); + + assert_eq!( + dm.dependencies(uuids[3]).collect::>(), + set![uuids[1], uuids[2]] + ); + assert_eq!( + dm.dependencies(uuids[2]).collect::>(), + set![uuids[0]] + ); + assert_eq!( + dm.dependencies(uuids[1]).collect::>(), + set![uuids[0]] + ); + assert_eq!(dm.dependencies(uuids[0]).collect::>(), set![]); + + assert_eq!(dm.dependents(uuids[3]).collect::>(), set![]); + assert_eq!( + dm.dependents(uuids[2]).collect::>(), + set![uuids[3]] + ); + assert_eq!( + dm.dependents(uuids[1]).collect::>(), + set![uuids[3]] + ); + assert_eq!( + dm.dependents(uuids[0]).collect::>(), + set![uuids[1], uuids[2]] + ); + } } diff --git a/taskchampion/src/task/tag.rs b/taskchampion/src/task/tag.rs index bb8361f80..a4f1e677c 100644 --- a/taskchampion/src/task/tag.rs +++ b/taskchampion/src/task/tag.rs @@ -134,6 +134,9 @@ pub(super) enum SyntheticTag { Pending, Completed, Deleted, + Blocked, + Unblocked, + Blocking, } #[cfg(test)] diff --git a/taskchampion/src/task/task.rs b/taskchampion/src/task/task.rs index fa8700a62..2a8464837 100644 --- a/taskchampion/src/task/task.rs +++ b/taskchampion/src/task/task.rs @@ -1,11 +1,13 @@ use super::tag::{SyntheticTag, TagInner}; use super::{Annotation, Priority, Status, Tag, Timestamp}; +use crate::depmap::DependencyMap; use crate::replica::Replica; use crate::storage::TaskMap; use chrono::prelude::*; use log::trace; use std::convert::AsRef; use std::convert::TryInto; +use std::rc::Rc; use std::str::FromStr; use uuid::Uuid; @@ -29,10 +31,18 @@ use uuid::Uuid; /// This struct contains only getters for various values on the task. The /// [`into_mut`](Task::into_mut) method /// returns a TaskMut which can be used to modify the task. -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone)] pub struct Task { uuid: Uuid, taskmap: TaskMap, + depmap: Rc, +} + +impl PartialEq for Task { + fn eq(&self, other: &Task) -> bool { + // compare only the taskmap and uuid; depmap is just present for reference + self.uuid == other.uuid && self.taskmap == other.taskmap + } } /// A mutable task, with setter methods. @@ -84,8 +94,12 @@ fn uda_tuple_to_string(namespace: impl AsRef, key: impl AsRef) -> Stri } impl Task { - pub(crate) fn new(uuid: Uuid, taskmap: TaskMap) -> Task { - Task { uuid, taskmap } + pub(crate) fn new(uuid: Uuid, taskmap: TaskMap, depmap: Rc) -> Task { + Task { + uuid, + taskmap, + depmap, + } } pub fn get_uuid(&self) -> Uuid { @@ -151,6 +165,16 @@ impl Task { self.taskmap.contains_key(Prop::Start.as_ref()) } + /// Determine whether this task is blocked -- that is, has at least one unresolved dependency. + pub fn is_blocked(&self) -> bool { + self.depmap.dependencies(self.uuid).next().is_some() + } + + /// Determine whether this task is blocking -- that is, has at least one unresolved dependent. + pub fn is_blocking(&self) -> bool { + self.depmap.dependents(self.uuid).next().is_some() + } + /// Determine whether a given synthetic tag is present on this task. All other /// synthetic tag calculations are based on this one. fn has_synthetic_tag(&self, synth: &SyntheticTag) -> bool { @@ -160,6 +184,9 @@ impl Task { SyntheticTag::Pending => self.get_status() == Status::Pending, SyntheticTag::Completed => self.get_status() == Status::Completed, SyntheticTag::Deleted => self.get_status() == Status::Deleted, + SyntheticTag::Blocked => self.is_blocked(), + SyntheticTag::Unblocked => !self.is_blocked(), + SyntheticTag::Blocking => self.is_blocking(), } } @@ -520,6 +547,11 @@ impl<'r> std::ops::Deref for TaskMut<'r> { mod test { use super::*; use pretty_assertions::assert_eq; + use std::collections::HashSet; + + fn dm() -> Rc { + Rc::new(DependencyMap::new()) + } fn with_mut_task(f: F) { let mut replica = Replica::new_inmemory(); @@ -540,7 +572,7 @@ mod test { #[test] fn test_is_active_never_started() { - let task = Task::new(Uuid::new_v4(), TaskMap::new()); + let task = Task::new(Uuid::new_v4(), TaskMap::new(), dm()); assert!(!task.is_active()); } @@ -551,6 +583,7 @@ mod test { vec![(String::from("start"), String::from("1234"))] .drain(..) .collect(), + dm(), ); assert!(task.is_active()); @@ -558,13 +591,13 @@ mod test { #[test] fn test_is_active_inactive() { - let task = Task::new(Uuid::new_v4(), Default::default()); + let task = Task::new(Uuid::new_v4(), Default::default(), dm()); assert!(!task.is_active()); } #[test] fn test_entry_not_set() { - let task = Task::new(Uuid::new_v4(), TaskMap::new()); + let task = Task::new(Uuid::new_v4(), TaskMap::new(), dm()); assert_eq!(task.get_entry(), None); } @@ -576,13 +609,14 @@ mod test { vec![(String::from("entry"), format!("{}", ts.timestamp()))] .drain(..) .collect(), + dm(), ); assert_eq!(task.get_entry(), Some(ts)); } #[test] fn test_wait_not_set() { - let task = Task::new(Uuid::new_v4(), TaskMap::new()); + let task = Task::new(Uuid::new_v4(), TaskMap::new(), dm()); assert!(!task.is_waiting()); assert_eq!(task.get_wait(), None); @@ -596,6 +630,7 @@ mod test { vec![(String::from("wait"), format!("{}", ts.timestamp()))] .drain(..) .collect(), + dm(), ); assert!(!task.is_waiting()); @@ -610,6 +645,7 @@ mod test { vec![(String::from("wait"), format!("{}", ts.timestamp()))] .drain(..) .collect(), + dm(), ); assert!(task.is_waiting()); @@ -626,6 +662,7 @@ mod test { ] .drain(..) .collect(), + dm(), ); assert!(task.has_tag(&utag("abc"))); @@ -647,17 +684,17 @@ mod test { ] .drain(..) .collect(), + dm(), ); - let mut tags: Vec<_> = task.get_tags().collect(); - tags.sort(); - let mut exp = vec![ + let tags: HashSet<_> = task.get_tags().collect(); + let exp = set![ utag("abc"), utag("def"), stag(SyntheticTag::Pending), stag(SyntheticTag::Waiting), + stag(SyntheticTag::Unblocked), ]; - exp.sort(); assert_eq!(tags, exp); } @@ -673,11 +710,19 @@ mod test { ] .drain(..) .collect(), + dm(), ); // only "ok" is OK - let tags: Vec<_> = task.get_tags().collect(); - assert_eq!(tags, vec![utag("ok"), stag(SyntheticTag::Pending)]); + let tags: HashSet<_> = task.get_tags().collect(); + assert_eq!( + tags, + set![ + utag("ok"), + stag(SyntheticTag::Pending), + stag(SyntheticTag::Unblocked) + ] + ); } #[test] @@ -698,6 +743,7 @@ mod test { ] .drain(..) .collect(), + dm(), ); let mut anns: Vec<_> = task.get_annotations().collect(); @@ -913,6 +959,7 @@ mod test { ] .drain(..) .collect(), + dm(), ); let mut udas: Vec<_> = task.get_udas().collect(); @@ -934,6 +981,7 @@ mod test { ] .drain(..) .collect(), + dm(), ); assert_eq!(task.get_uda("", "description"), None); // invalid UDA @@ -954,6 +1002,7 @@ mod test { ] .drain(..) .collect(), + dm(), ); assert_eq!(task.get_legacy_uda("description"), None); // invalid UDA @@ -1061,4 +1110,32 @@ mod test { assert_eq!(task.get_dependencies().collect::>(), vec![dep2]); }) } + + #[test] + fn dependencies_tags() { + let mut rep = Replica::new_inmemory(); + let uuid1; + let uuid2; + { + let t1 = rep.new_task(Status::Pending, "1".into()).unwrap(); + uuid1 = t1.get_uuid(); + let t2 = rep.new_task(Status::Pending, "2".into()).unwrap(); + uuid2 = t2.get_uuid(); + + let mut t1 = t1.into_mut(&mut rep); + t1.add_dependency(t2.get_uuid()).unwrap(); + } + + // force-refresh depmap + rep.dependency_map(true).unwrap(); + + let t1 = rep.get_task(uuid1).unwrap().unwrap(); + let t2 = rep.get_task(uuid2).unwrap().unwrap(); + assert!(t1.has_tag(&stag(SyntheticTag::Blocked))); + assert!(!t1.has_tag(&stag(SyntheticTag::Unblocked))); + assert!(!t1.has_tag(&stag(SyntheticTag::Blocking))); + assert!(!t2.has_tag(&stag(SyntheticTag::Blocked))); + assert!(t2.has_tag(&stag(SyntheticTag::Unblocked))); + assert!(t2.has_tag(&stag(SyntheticTag::Blocking))); + } }