From ef12e1a2f80403ea82eb17ef54cc88a48c127b4d Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Sat, 18 Dec 2021 23:39:56 +0000 Subject: [PATCH 1/4] Define UDAs --- docs/src/tasks.md | 11 ++ taskchampion/src/task/task.rs | 188 +++++++++++++++++++++++++++++++--- 2 files changed, 186 insertions(+), 13 deletions(-) diff --git a/docs/src/tasks.md b/docs/src/tasks.md index f21dd475d..f8debd20c 100644 --- a/docs/src/tasks.md +++ b/docs/src/tasks.md @@ -38,3 +38,14 @@ The following keys, and key formats, are defined: The following are not yet implemented: * `dep_` - indicates this task depends on `` (value is an empty string) + +### UDAs + +Any unrecognized keys are treated as "user-defined attributes" (UDAs). +These attributes can be used to store additional data associated with a task. +For example, applications that synchronize tasks with other systems such as calendars or team planning services might store unique identifiers for those systems as UDAs. +The application defining a UDA defines the format of the value. + +UDAs _should_ have a namespaced structure of the form `.`, where `` identifies the application defining the UDA. +For example, a service named "DevSync" synchronizing tasks from GitHub might use UDAs like `devsync.github.issue-id`. +Note that many existing UDAs for Taskwarrior integrations do not follow this pattern. diff --git a/taskchampion/src/task/task.rs b/taskchampion/src/task/task.rs index d38555e19..5d01293d1 100644 --- a/taskchampion/src/task/task.rs +++ b/taskchampion/src/task/task.rs @@ -6,6 +6,7 @@ use chrono::prelude::*; use log::trace; use std::convert::AsRef; use std::convert::TryInto; +use std::str::FromStr; use uuid::Uuid; /* The Task and TaskMut classes wrap the underlying [`TaskMap`], which is a simple key/value map. @@ -46,6 +47,18 @@ pub struct TaskMut<'r> { updated_modified: bool, } +/// An enum containing all of the key names defined in the data model, with the exception +/// of the properties containing data (`tag_..`, etc.) +#[derive(strum_macros::AsRefStr, strum_macros::EnumString)] +#[strum(serialize_all = "kebab-case")] +enum Prop { + Description, + Modified, + Start, + Status, + Wait, +} + impl Task { pub(crate) fn new(uuid: Uuid, taskmap: TaskMap) -> Task { Task { uuid, taskmap } @@ -71,14 +84,14 @@ impl Task { pub fn get_status(&self) -> Status { self.taskmap - .get("status") + .get(Prop::Status.as_ref()) .map(|s| Status::from_taskmap(s)) .unwrap_or(Status::Pending) } pub fn get_description(&self) -> &str { self.taskmap - .get("description") + .get(Prop::Description.as_ref()) .map(|s| s.as_ref()) .unwrap_or("") } @@ -86,7 +99,7 @@ impl Task { /// Get the wait time. If this value is set, it will be returned, even /// if it is in the past. pub fn get_wait(&self) -> Option> { - self.get_timestamp("wait") + self.get_timestamp(Prop::Wait.as_ref()) } /// Determine whether this task is waiting now. @@ -100,7 +113,7 @@ impl Task { /// Determine whether this task is active -- that is, that it has been started /// and not stopped. pub fn is_active(&self) -> bool { - self.taskmap.contains_key("start") + self.taskmap.contains_key(Prop::Start.as_ref()) } /// Determine whether a given synthetic tag is present on this task. All other @@ -161,12 +174,37 @@ impl Task { }) } + /// Get the named user defined attributes (UDA). This will return None + /// for any key defined in the Task data model, regardless of whether + /// it is set or not. + pub fn get_uda(&self, key: &str) -> Option<&str> { + if Task::is_known_key(key) { + return None; + } + self.taskmap.get(key).map(|s| s.as_ref()) + } + + /// Get the user defined attributes (UDAs) of this task, in arbitrary order. + pub fn get_udas(&self) -> impl Iterator + '_ { + self.taskmap + .iter() + .filter(|(p, _)| !Task::is_known_key(p)) + .map(|(p, v)| (p.as_ref(), v.as_ref())) + } + pub fn get_modified(&self) -> Option> { - self.get_timestamp("modified") + self.get_timestamp(Prop::Modified.as_ref()) } // -- utility functions + fn is_known_key(key: &str) -> bool { + Prop::from_str(key).is_ok() + || key.starts_with("tag_") + || key.starts_with("annotation_") + || key.starts_with("dep_") + } + fn get_timestamp(&self, property: &str) -> Option> { if let Some(ts) = self.taskmap.get(property) { if let Ok(ts) = ts.parse() { @@ -191,19 +229,22 @@ impl<'r> TaskMut<'r> { let uuid = self.uuid; self.replica.add_to_working_set(uuid)?; } - self.set_string("status", Some(String::from(status.to_taskmap()))) + self.set_string( + Prop::Status.as_ref(), + Some(String::from(status.to_taskmap())), + ) } pub fn set_description(&mut self, description: String) -> anyhow::Result<()> { - self.set_string("description", Some(description)) + self.set_string(Prop::Description.as_ref(), Some(description)) } pub fn set_wait(&mut self, wait: Option>) -> anyhow::Result<()> { - self.set_timestamp("wait", wait) + self.set_timestamp(Prop::Wait.as_ref(), wait) } pub fn set_modified(&mut self, modified: DateTime) -> anyhow::Result<()> { - self.set_timestamp("modified", Some(modified)) + self.set_timestamp(Prop::Modified.as_ref(), Some(modified)) } /// Start the task by creating "start": "", if the task is not already @@ -212,12 +253,12 @@ impl<'r> TaskMut<'r> { if self.is_active() { return Ok(()); } - self.set_timestamp("start", Some(Utc::now())) + self.set_timestamp(Prop::Start.as_ref(), Some(Utc::now())) } /// Stop the task by removing the `start` key pub fn stop(&mut self) -> anyhow::Result<()> { - self.set_timestamp("start", None) + self.set_timestamp(Prop::Start.as_ref(), None) } /// Mark this task as complete @@ -255,15 +296,50 @@ impl<'r> TaskMut<'r> { self.set_string(format!("annotation_{}", entry.timestamp()), None) } + /// Set a user-defined attribute (UDA). This will fail if the key is defined by the data + /// model. + pub fn set_uda(&mut self, key: S1, value: S2) -> anyhow::Result<()> + where + S1: Into, + S2: Into, + { + let key = key.into(); + if Task::is_known_key(&key) { + anyhow::bail!( + "Property name {} as special meaning in a task and cannot be used as a UDA", + key + ); + } + self.set_string(key, Some(value.into())) + } + + /// Remove a user-defined attribute (UDA). This will fail if the key is defined by the data + /// model. + pub fn remove_uda(&mut self, key: S) -> anyhow::Result<()> + where + S: Into, + { + let key = key.into(); + if Task::is_known_key(&key) { + anyhow::bail!( + "Property name {} as special meaning in a task and cannot be used as a UDA", + key + ); + } + self.set_string(key, None) + } + // -- utility functions fn lastmod(&mut self) -> anyhow::Result<()> { if !self.updated_modified { let now = format!("{}", Utc::now().timestamp()); self.replica - .update_task(self.task.uuid, "modified", Some(now.clone()))?; + .update_task(self.task.uuid, Prop::Modified.as_ref(), Some(now.clone()))?; trace!("task {}: set property modified={:?}", self.task.uuid, now); - self.task.taskmap.insert(String::from("modified"), now); + self.task + .taskmap + .insert(String::from(Prop::Modified.as_ref()), now); self.updated_modified = true; } Ok(()) @@ -634,4 +710,90 @@ mod test { assert!(!task.taskmap.contains_key("tag_abc")); }); } + + #[test] + fn test_get_udas() { + let task = Task::new( + Uuid::new_v4(), + vec![ + ("description".into(), "not a uda".into()), + ("modified".into(), "not a uda".into()), + ("start".into(), "not a uda".into()), + ("status".into(), "not a uda".into()), + ("wait".into(), "not a uda".into()), + ("start".into(), "not a uda".into()), + ("tag_abc".into(), "not a uda".into()), + ("dep_1234".into(), "not a uda".into()), + ("annotation_1234".into(), "not a uda".into()), + ("githubid".into(), "123".into()), + ] + .drain(..) + .collect(), + ); + + let udas: Vec<_> = task.get_udas().collect(); + assert_eq!(udas, vec![("githubid", "123")]); + } + + #[test] + fn test_get_uda() { + let task = Task::new( + Uuid::new_v4(), + vec![ + ("description".into(), "not a uda".into()), + ("dep_1234".into(), "not a uda".into()), + ("githubid".into(), "123".into()), + ] + .drain(..) + .collect(), + ); + + assert_eq!(task.get_uda("description"), None); // invalid UDA + assert_eq!(task.get_uda("dep_1234"), None); // invalid UDA + assert_eq!(task.get_uda("githubid"), Some("123")); + assert_eq!(task.get_uda("jiraid"), None); + } + + #[test] + fn test_set_uda() { + with_mut_task(|mut task| { + task.set_uda("githubid", "123").unwrap(); + + let udas: Vec<_> = task.get_udas().collect(); + assert_eq!(udas, vec![("githubid", "123")]); + + task.set_uda("jiraid", "TW-1234").unwrap(); + + let mut udas: Vec<_> = task.get_udas().collect(); + udas.sort_unstable(); + assert_eq!(udas, vec![("githubid", "123"), ("jiraid", "TW-1234")]); + }) + } + + #[test] + fn test_set_uda_invalid() { + with_mut_task(|mut task| { + assert!(task.set_uda("modified", "123").is_err()); + assert!(task.set_uda("tag_abc", "123").is_err()); + }) + } + + #[test] + fn test_rmmove_uda() { + with_mut_task(|mut task| { + task.set_string("githubid", Some("123".into())).unwrap(); + task.remove_uda("githubid").unwrap(); + + let udas: Vec<_> = task.get_udas().collect(); + assert_eq!(udas, vec![]); + }) + } + + #[test] + fn test_remove_uda_invalid() { + with_mut_task(|mut task| { + assert!(task.remove_uda("modified").is_err()); + assert!(task.remove_uda("tag_abc").is_err()); + }) + } } From b255ad2a7de446aeb84455042b624edab7c8838c Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Mon, 27 Dec 2021 00:01:14 +0000 Subject: [PATCH 2/4] use namespace.key for UDAs in the API, with legacy support --- docs/src/tasks.md | 2 +- taskchampion/src/task/task.rs | 178 ++++++++++++++++++++++++++++------ 2 files changed, 147 insertions(+), 33 deletions(-) diff --git a/docs/src/tasks.md b/docs/src/tasks.md index f8debd20c..eb6756e22 100644 --- a/docs/src/tasks.md +++ b/docs/src/tasks.md @@ -48,4 +48,4 @@ The application defining a UDA defines the format of the value. UDAs _should_ have a namespaced structure of the form `.`, where `` identifies the application defining the UDA. For example, a service named "DevSync" synchronizing tasks from GitHub might use UDAs like `devsync.github.issue-id`. -Note that many existing UDAs for Taskwarrior integrations do not follow this pattern. +Note that many existing UDAs for Taskwarrior integrations do not follow this pattern; these are referred to as legacy UDAs. diff --git a/taskchampion/src/task/task.rs b/taskchampion/src/task/task.rs index 5d01293d1..ca4363ad6 100644 --- a/taskchampion/src/task/task.rs +++ b/taskchampion/src/task/task.rs @@ -59,6 +59,25 @@ enum Prop { Wait, } +#[allow(clippy::ptr_arg)] +fn uda_string_to_tuple(key: &String) -> (&str, &str) { + if let Some((ns, key)) = key.split_once('.') { + (ns, key) + } else { + ("", key.as_ref()) + } +} + +fn uda_tuple_to_string(namespace: impl Into, key: impl Into) -> String { + // TODO: maybe not Into + let namespace = namespace.into(); + if namespace.is_empty() { + key.into() + } else { + format!("{}.{}", namespace, key.into()) + } +} + impl Task { pub(crate) fn new(uuid: Uuid, taskmap: TaskMap) -> Task { Task { uuid, taskmap } @@ -177,15 +196,31 @@ impl Task { /// Get the named user defined attributes (UDA). This will return None /// for any key defined in the Task data model, regardless of whether /// it is set or not. - pub fn get_uda(&self, key: &str) -> Option<&str> { + pub fn get_uda(&self, namespace: &str, key: &str) -> Option<&str> { + self.get_legacy_uda(uda_tuple_to_string(namespace, key).as_ref()) + } + + /// Get the user defined attributes (UDAs) of this task, in arbitrary order. Each key is split + /// on the first `.` character. Legacy keys that do not contain `.` are represented as `("", + /// key)`. + pub fn get_udas(&self) -> impl Iterator + '_ { + self.taskmap + .iter() + .filter(|(k, _)| !Task::is_known_key(k)) + .map(|(k, v)| (uda_string_to_tuple(k), v.as_ref())) + } + + /// Get the named user defined attribute (UDA) in a legacy format. This will return None for + /// any key defined in the Task data model, regardless of whether it is set or not. + pub fn get_legacy_uda(&self, key: &str) -> Option<&str> { if Task::is_known_key(key) { return None; } self.taskmap.get(key).map(|s| s.as_ref()) } - /// Get the user defined attributes (UDAs) of this task, in arbitrary order. - pub fn get_udas(&self) -> impl Iterator + '_ { + /// Like `get_udas`, but returning each UDA key as a single string. + pub fn get_legacy_udas(&self) -> impl Iterator + '_ { self.taskmap .iter() .filter(|(p, _)| !Task::is_known_key(p)) @@ -298,11 +333,33 @@ impl<'r> TaskMut<'r> { /// Set a user-defined attribute (UDA). This will fail if the key is defined by the data /// model. - pub fn set_uda(&mut self, key: S1, value: S2) -> anyhow::Result<()> - where - S1: Into, - S2: Into, - { + pub fn set_uda( + &mut self, + namespace: impl Into, + key: impl Into, + value: impl Into, + ) -> anyhow::Result<()> { + let key = uda_tuple_to_string(namespace, key); + self.set_legacy_uda(key, value) + } + + /// Remove a user-defined attribute (UDA). This will fail if the key is defined by the data + /// model. + pub fn remove_uda( + &mut self, + namespace: impl Into, + key: impl Into, + ) -> anyhow::Result<()> { + let key = uda_tuple_to_string(namespace, key); + self.remove_legacy_uda(key) + } + + /// Set a user-defined attribute (UDA), where the key is a legacy key. + pub fn set_legacy_uda( + &mut self, + key: impl Into, + value: impl Into, + ) -> anyhow::Result<()> { let key = key.into(); if Task::is_known_key(&key) { anyhow::bail!( @@ -313,12 +370,8 @@ impl<'r> TaskMut<'r> { self.set_string(key, Some(value.into())) } - /// Remove a user-defined attribute (UDA). This will fail if the key is defined by the data - /// model. - pub fn remove_uda(&mut self, key: S) -> anyhow::Result<()> - where - S: Into, - { + /// Remove a user-defined attribute (UDA), where the key is a legacy key. + pub fn remove_legacy_uda(&mut self, key: impl Into) -> anyhow::Result<()> { let key = key.into(); if Task::is_known_key(&key) { anyhow::bail!( @@ -726,13 +779,18 @@ mod test { ("dep_1234".into(), "not a uda".into()), ("annotation_1234".into(), "not a uda".into()), ("githubid".into(), "123".into()), + ("jira.url".into(), "h://x".into()), ] .drain(..) .collect(), ); - let udas: Vec<_> = task.get_udas().collect(); - assert_eq!(udas, vec![("githubid", "123")]); + let mut udas: Vec<_> = task.get_udas().collect(); + udas.sort_unstable(); + assert_eq!( + udas, + vec![(("", "githubid"), "123"), (("jira", "url"), "h://x")] + ); } #[test] @@ -741,48 +799,102 @@ mod test { Uuid::new_v4(), vec![ ("description".into(), "not a uda".into()), - ("dep_1234".into(), "not a uda".into()), ("githubid".into(), "123".into()), + ("jira.url".into(), "h://x".into()), ] .drain(..) .collect(), ); - assert_eq!(task.get_uda("description"), None); // invalid UDA - assert_eq!(task.get_uda("dep_1234"), None); // invalid UDA - assert_eq!(task.get_uda("githubid"), Some("123")); - assert_eq!(task.get_uda("jiraid"), None); + assert_eq!(task.get_uda("", "description"), None); // invalid UDA + assert_eq!(task.get_uda("", "githubid"), Some("123")); + assert_eq!(task.get_uda("jira", "url"), Some("h://x")); + assert_eq!(task.get_uda("bugzilla", "url"), None); + } + + #[test] + fn test_get_legacy_uda() { + let task = Task::new( + Uuid::new_v4(), + vec![ + ("description".into(), "not a uda".into()), + ("dep_1234".into(), "not a uda".into()), + ("githubid".into(), "123".into()), + ("jira.url".into(), "h://x".into()), + ] + .drain(..) + .collect(), + ); + + assert_eq!(task.get_legacy_uda("description"), None); // invalid UDA + assert_eq!(task.get_legacy_uda("dep_1234"), None); // invalid UDA + assert_eq!(task.get_legacy_uda("githubid"), Some("123")); + assert_eq!(task.get_legacy_uda("jira.url"), Some("h://x")); + assert_eq!(task.get_legacy_uda("bugzilla.url"), None); } #[test] fn test_set_uda() { with_mut_task(|mut task| { - task.set_uda("githubid", "123").unwrap(); - + task.set_uda("jira", "url", "h://y").unwrap(); let udas: Vec<_> = task.get_udas().collect(); - assert_eq!(udas, vec![("githubid", "123")]); + assert_eq!(udas, vec![(("jira", "url"), "h://y")]); - task.set_uda("jiraid", "TW-1234").unwrap(); + task.set_uda("", "jiraid", "TW-1234").unwrap(); let mut udas: Vec<_> = task.get_udas().collect(); udas.sort_unstable(); - assert_eq!(udas, vec![("githubid", "123"), ("jiraid", "TW-1234")]); + assert_eq!( + udas, + vec![(("", "jiraid"), "TW-1234"), (("jira", "url"), "h://y")] + ); + }) + } + + #[test] + fn test_set_legacy_uda() { + with_mut_task(|mut task| { + task.set_legacy_uda("jira.url", "h://y").unwrap(); + let udas: Vec<_> = task.get_udas().collect(); + assert_eq!(udas, vec![(("jira", "url"), "h://y")]); + + task.set_legacy_uda("jiraid", "TW-1234").unwrap(); + + let mut udas: Vec<_> = task.get_udas().collect(); + udas.sort_unstable(); + assert_eq!( + udas, + vec![(("", "jiraid"), "TW-1234"), (("jira", "url"), "h://y")] + ); }) } #[test] fn test_set_uda_invalid() { with_mut_task(|mut task| { - assert!(task.set_uda("modified", "123").is_err()); - assert!(task.set_uda("tag_abc", "123").is_err()); + assert!(task.set_uda("", "modified", "123").is_err()); + assert!(task.set_uda("", "tag_abc", "123").is_err()); + assert!(task.set_legacy_uda("modified", "123").is_err()); + assert!(task.set_legacy_uda("tag_abc", "123").is_err()); }) } #[test] - fn test_rmmove_uda() { + fn test_remove_uda() { + with_mut_task(|mut task| { + task.set_string("github.id", Some("123".into())).unwrap(); + task.remove_uda("github", "id").unwrap(); + + let udas: Vec<_> = task.get_udas().collect(); + assert_eq!(udas, vec![]); + }) + } + + #[test] + fn test_remove_legacy_uda() { with_mut_task(|mut task| { task.set_string("githubid", Some("123".into())).unwrap(); - task.remove_uda("githubid").unwrap(); + task.remove_legacy_uda("githubid").unwrap(); let udas: Vec<_> = task.get_udas().collect(); assert_eq!(udas, vec![]); @@ -792,8 +904,10 @@ mod test { #[test] fn test_remove_uda_invalid() { with_mut_task(|mut task| { - assert!(task.remove_uda("modified").is_err()); - assert!(task.remove_uda("tag_abc").is_err()); + assert!(task.remove_uda("", "modified").is_err()); + assert!(task.remove_uda("", "tag_abc").is_err()); + assert!(task.remove_legacy_uda("modified").is_err()); + assert!(task.remove_legacy_uda("tag_abc").is_err()); }) } } From e94c29ae2ff278b88a951c86a078f5ed09ce7d11 Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Mon, 27 Dec 2021 00:09:02 +0000 Subject: [PATCH 3/4] use better trait bounds --- taskchampion/src/task/task.rs | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/taskchampion/src/task/task.rs b/taskchampion/src/task/task.rs index d52c8d3a4..fa22b026a 100644 --- a/taskchampion/src/task/task.rs +++ b/taskchampion/src/task/task.rs @@ -60,21 +60,21 @@ enum Prop { } #[allow(clippy::ptr_arg)] -fn uda_string_to_tuple(key: &String) -> (&str, &str) { +fn uda_string_to_tuple(key: &str) -> (&str, &str) { if let Some((ns, key)) = key.split_once('.') { (ns, key) } else { - ("", key.as_ref()) + ("", key) } } -fn uda_tuple_to_string(namespace: impl Into, key: impl Into) -> String { - // TODO: maybe not Into - let namespace = namespace.into(); +fn uda_tuple_to_string(namespace: impl AsRef, key: impl AsRef) -> String { + let namespace = namespace.as_ref(); + let key = key.as_ref(); if namespace.is_empty() { key.into() } else { - format!("{}.{}", namespace, key.into()) + format!("{}.{}", namespace, key) } } @@ -335,8 +335,8 @@ impl<'r> TaskMut<'r> { /// model. pub fn set_uda( &mut self, - namespace: impl Into, - key: impl Into, + namespace: impl AsRef, + key: impl AsRef, value: impl Into, ) -> anyhow::Result<()> { let key = uda_tuple_to_string(namespace, key); @@ -347,8 +347,8 @@ impl<'r> TaskMut<'r> { /// model. pub fn remove_uda( &mut self, - namespace: impl Into, - key: impl Into, + namespace: impl AsRef, + key: impl AsRef, ) -> anyhow::Result<()> { let key = uda_tuple_to_string(namespace, key); self.remove_legacy_uda(key) @@ -388,9 +388,9 @@ impl<'r> TaskMut<'r> { if !self.updated_modified { let now = format!("{}", Utc::now().timestamp()); trace!("task {}: set property modified={:?}", self.task.uuid, now); - self.task.taskmap = self - .replica - .update_task(self.task.uuid, Prop::Modified.as_ref(), Some(now))?; + self.task.taskmap = + self.replica + .update_task(self.task.uuid, Prop::Modified.as_ref(), Some(now))?; self.updated_modified = true; } Ok(()) From bc8bb5255110856c2dd50a64b9887dab7b3728a3 Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Mon, 27 Dec 2021 00:14:40 +0000 Subject: [PATCH 4/4] do not use str.split_once, as it is not in MSRV --- taskchampion/src/task/task.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/taskchampion/src/task/task.rs b/taskchampion/src/task/task.rs index fa22b026a..40a85ba87 100644 --- a/taskchampion/src/task/task.rs +++ b/taskchampion/src/task/task.rs @@ -61,10 +61,13 @@ enum Prop { #[allow(clippy::ptr_arg)] fn uda_string_to_tuple(key: &str) -> (&str, &str) { - if let Some((ns, key)) = key.split_once('.') { - (ns, key) + let mut iter = key.splitn(2, '.'); + let first = iter.next().unwrap(); + let second = iter.next(); + if let Some(second) = second { + (first, second) } else { - ("", key) + ("", first) } }