Merge pull request #32 from djmitche/issue22

fix up some TODOs in replica.rs

Cargo.lock

@@ -131,6 +131,12 @@ dependencies = [
  "bitflags",
 ]
 
+[[package]]
+name = "either"
+version = "1.6.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "e78d4f1cc4ae33bbfc157ed5d5a5ef3bc29227303d595861deb238fcec4e9457"
+
 [[package]]
 name = "failure"
 version = "0.1.6"
@@ -176,6 +182,15 @@ dependencies = [
  "wasi",
 ]
 
+[[package]]
+name = "itertools"
+version = "0.9.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "284f18f85651fe11e8a991b2adb42cb078325c996ed026d994719efcfca1d54b"
+dependencies = [
+ "either",
+]
+
 [[package]]
 name = "itoa"
 version = "0.4.4"
@@ -607,6 +622,7 @@ dependencies = [
  "chrono",
  "clap",
  "failure",
+ "itertools",
  "kv",
  "lmdb-rkv",
  "proptest",

Cargo.toml

@@ -13,6 +13,7 @@ failure = {version = "0.1.5", features = ["derive"] }
 clap = "~2.33.0"
 kv = {version = "0.10.0", features = ["msgpack-value"]}
 lmdb-rkv = {version = "0.12.3"}
+itertools = "0.9.0"
 
 [dev-dependencies]
 proptest = "0.9.4"

docs/src/SUMMARY.md

@@ -2,5 +2,7 @@
 
 - [Installation](./installation.md)
 - [Usage](./usage.md)
+- [Internal Details](./internals.md)
+  - [Data Model](./data-model.md)
 ---
 - [Development Notes](./development-notes.md)

docs/src/data-model.md

@@ -0,0 +1,42 @@
+# Data Model
+
+A client manages a single offline instance of a single user's task list.
+The data model is only seen from the clients' perspective.
+
+## Task Database
+
+The task database is composed of an un-ordered collection of tasks, each keyed by a UUID.
+Each task in the database has an arbitrary-sized set of key/value properties, with string values.
+
+Tasks are only created and modified; "deleted" tasks continue to stick around and can be modified and even un-deleted.
+Tasks have an expiration time, after which they may be purged from the database.
+
+## Task Fields
+
+Each task can have any of the following fields.
+Timestamps are stored as UNIX epoch timestamps, in the form of an integer expressed in decimal notation.
+Note that it is possible for any field to be omitted.
+
+NOTE: This structure is based on https://taskwarrior.org/docs/design/task.html, but will diverge from that
+model over time.
+
+* `status` - one of `Pending`, `Completed`, `Deleted`, `Recurring`, or `Waiting`
+* `entry` (timestamp) - time that the task was created
+* `description` - the one-line summary of the task
+* `start` (timestamp) - if set, the task is active and this field gives the time the task was started
+* `end` (timestamp) - the time at which the task was deleted or completed
+* `due` (timestamp) - the time at which the task is due
+* `until` (timestamp) - the time after which recurrent child tasks should not be created
+* `wait` (timestamp) - the time before which this task is considered waiting and should not be shown
+* `modified` (timestamp) - time that the task was last modified
+* `scheduled` (timestamp) - time that the task is available to start
+* `recur` - recurrence frequency
+* `mask` - recurrence history
+* `imask` - for children of recurring tasks, the index into the `mask` property on the parent
+* `parent` - for children of recurring tasks, the uuid of the parent task
+* `project` - the task's project (usually a short identifier)
+* `priority` - the task's priority, one of `L`, `M`, or `H`.
+* `depends` - a comma (`,`) separated list of uuids of tasks on which this task depends
+* `tags` - a comma (`,`) separated list of tags for this task
+* `annotation_<timestamp>` - an annotation for this task, with the timestamp as part of the key
+* `udas` - user-defined attributes

docs/src/development-notes.md

@@ -4,20 +4,7 @@ Goals:
  * Reliable concurrency - clients do not diverge
  * Storage O(n) with n number of tasks
 
-# Data Model
-
-A client manages a single offline instance of a single user's task list.
-The data model is only seen from the clients' perspective.
-
-## Task Database
-
-The task database is composed of an un-ordered collection of tasks, each keyed by a UUID.
-Each task has an arbitrary-sized set of key/value properties, with string values.
-
-Tasks are only created, never deleted.
-See below for details on how tasks can "expire" from the task database.
-
-## Operations
+# Operations
 
 Every change to the task database is captured as an operation.
 Each operation has one of the forms 

docs/src/internals.md

@@ -0,0 +1,4 @@
+# Internal Details
+
+This section describes some of the internal details of TaskChampion.
+While this section is not required to use TaskChampion, understanding some of these details may help to understand how TaskChampion behaves.

src/errors.rs

@@ -4,4 +4,7 @@ use failure::Fail;
 pub enum Error {
     #[fail(display = "Task Database Error: {}", _0)]
     DBError(String),
+
+    #[fail(display = "Replica Error: {}", _0)]
+    ReplicaError(String),
 }

src/replica.rs

@@ -1,9 +1,11 @@
+use crate::errors::Error;
 use crate::operation::Operation;
 use crate::task::{Priority, Status, Task, TaskBuilder};
 use crate::taskdb::DB;
 use crate::taskstorage::TaskMap;
-use chrono::Utc;
+use chrono::{DateTime, Utc};
 use failure::Fallible;
+use itertools::join;
 use std::collections::HashMap;
 use uuid::Uuid;
 
@@ -77,7 +79,12 @@ impl Replica {
         status: Status,
         description: String,
     ) -> Fallible<TaskMut> {
-        // TODO: check that it doesn't exist
+        // check that it doesn't exist; this is a convenience check, as the task
+        // may already exist when this Create operation is finally sync'd with
+        // operations from other replicas
+        if self.taskdb.get_task(&uuid)?.is_some() {
+            return Err(Error::DBError(format!("Task {} already exists", uuid)).into());
+        }
         self.taskdb
             .apply(Operation::Create { uuid: uuid.clone() })?;
         self.update_task(uuid.clone(), "status", Some(String::from(status.as_ref())))?;
@@ -90,7 +97,11 @@ impl Replica {
 
     /// Delete a task.  The task must exist.
     pub fn delete_task(&mut self, uuid: Uuid) -> Fallible<()> {
-        // TODO: must verify task does exist
+        // check that it already exists; this is a convenience check, as the task may already exist
+        // when this Create operation is finally sync'd with operations from other replicas
+        if self.taskdb.get_task(&uuid)?.is_none() {
+            return Err(Error::DBError(format!("Task {} does not exist", uuid)).into());
+        }
         self.taskdb.apply(Operation::Delete { uuid })
     }
 
@@ -121,6 +132,8 @@ impl From<&TaskMap> for Task {
     }
 }
 
+// TODO: move this struct to crate::task, with a trait for update_task, since it is the reverse
+// of TaskBuilder::set
 /// TaskMut allows changes to a task.  It is intended for short-term use, such as changing a few
 /// properties, and should not be held for long periods of wall-clock time.
 pub struct TaskMut<'a> {
@@ -150,47 +163,108 @@ impl<'a> TaskMut<'a> {
         Ok(())
     }
 
-    /// Set the task's status
-    pub fn status(&mut self, status: Status) -> Fallible<()> {
+    fn set_string(&mut self, property: &str, value: Option<String>) -> Fallible<()> {
+        self.lastmod()?;
+        self.replica.update_task(self.uuid.clone(), property, value)
+    }
+
+    fn set_timestamp(&mut self, property: &str, value: Option<DateTime<Utc>>) -> Fallible<()> {
         self.lastmod()?;
         self.replica.update_task(
             self.uuid.clone(),
-            "status",
-            Some(String::from(status.as_ref())),
+            property,
+            value.map(|v| format!("{}", v.timestamp())),
         )
     }
 
-    // TODO: description
-    // TODO: start
-    // TODO: end
-    // TODO: due
-    // TODO: until
-    // TODO: wait
-    // TODO: scheduled
-    // TODO: recur
-    // TODO: mask
-    // TODO: imask
-    // TODO: parent
+    /// Set the task's status
+    pub fn status(&mut self, status: Status) -> Fallible<()> {
+        self.set_string("status", Some(String::from(status.as_ref())))
+    }
+
+    /// Set the task's description
+    pub fn description(&mut self, description: String) -> Fallible<()> {
+        self.set_string("description", Some(description))
+    }
+
+    /// Set the task's start time
+    pub fn start(&mut self, time: Option<DateTime<Utc>>) -> Fallible<()> {
+        self.set_timestamp("start", time)
+    }
+
+    /// Set the task's end time
+    pub fn end(&mut self, time: Option<DateTime<Utc>>) -> Fallible<()> {
+        self.set_timestamp("end", time)
+    }
+
+    /// Set the task's due time
+    pub fn due(&mut self, time: Option<DateTime<Utc>>) -> Fallible<()> {
+        self.set_timestamp("due", time)
+    }
+
+    /// Set the task's until time
+    pub fn until(&mut self, time: Option<DateTime<Utc>>) -> Fallible<()> {
+        self.set_timestamp("until", time)
+    }
+
+    /// Set the task's wait time
+    pub fn wait(&mut self, time: Option<DateTime<Utc>>) -> Fallible<()> {
+        self.set_timestamp("wait", time)
+    }
+
+    /// Set the task's scheduled time
+    pub fn scheduled(&mut self, time: Option<DateTime<Utc>>) -> Fallible<()> {
+        self.set_timestamp("scheduled", time)
+    }
+
+    /// Set the task's recur value
+    pub fn recur(&mut self, recur: Option<String>) -> Fallible<()> {
+        self.set_string("recur", recur)
+    }
+
+    /// Set the task's mask value
+    pub fn mask(&mut self, mask: Option<String>) -> Fallible<()> {
+        self.set_string("mask", mask)
+    }
+
+    /// Set the task's imask value
+    pub fn imask(&mut self, imask: Option<u64>) -> Fallible<()> {
+        self.set_string("imask", imask.map(|v| format!("{}", v)))
+    }
+
+    /// Set the task's parent task
+    pub fn parent(&mut self, parent: Option<Uuid>) -> Fallible<()> {
+        self.set_string("parent", parent.map(|v| format!("{}", v)))
+    }
 
     /// Set the task's project
-    pub fn project(&mut self, project: String) -> Fallible<()> {
-        self.lastmod()?;
-        self.replica
-            .update_task(self.uuid.clone(), "project", Some(project))
+    pub fn project(&mut self, project: Option<String>) -> Fallible<()> {
+        self.set_string("project", project)
     }
 
     /// Set the task's priority
-    pub fn priority(&mut self, priority: Priority) -> Fallible<()> {
-        self.lastmod()?;
-        self.replica.update_task(
-            self.uuid.clone(),
-            "priority",
-            Some(String::from(priority.as_ref())),
+    pub fn priority(&mut self, priority: Option<Priority>) -> Fallible<()> {
+        self.set_string("priority", priority.map(|v| String::from(v.as_ref())))
+    }
+
+    /// Set the task's depends; note that this completely replaces the list of tasks on which this
+    /// one depends.
+    pub fn depends(&mut self, depends: Vec<Uuid>) -> Fallible<()> {
+        self.set_string(
+            "depends",
+            if depends.len() > 0 {
+                Some(join(depends.iter(), ","))
+            } else {
+                None
+            },
         )
     }
 
-    // TODO: depends
-    // TODO: tags
+    /// Set the task's tags; note that this completely replaces the list of tags
+    pub fn tags(&mut self, tags: Vec<Uuid>) -> Fallible<()> {
+        self.set_string("tags", Some(join(tags.iter(), ",")))
+    }
+
     // TODO: annotations
     // TODO: udas
 }
@@ -209,7 +283,7 @@ mod tests {
         let mut tm = rep
             .new_task(uuid.clone(), Status::Pending, "a task".into())
             .unwrap();
-        tm.priority(Priority::L).unwrap();
+        tm.priority(Some(Priority::L)).unwrap();
 
         let t = rep.get_task(&uuid).unwrap().unwrap();
         assert_eq!(t.description, String::from("a task"));
@@ -237,12 +311,18 @@ mod tests {
         rep.new_task(uuid.clone(), Status::Pending, "another task".into())
             .unwrap();
 
-        let mut tm = rep.get_task_mut(&uuid).unwrap().unwrap();
-        tm.priority(Priority::L).unwrap();
-        tm.project("work".into()).unwrap();
-
         let t = rep.get_task(&uuid).unwrap().unwrap();
         assert_eq!(t.description, String::from("another task"));
+
+        let mut tm = rep.get_task_mut(&uuid).unwrap().unwrap();
+        tm.status(Status::Completed).unwrap();
+        tm.description("another task, updated".into()).unwrap();
+        tm.priority(Some(Priority::L)).unwrap();
+        tm.project(Some("work".into())).unwrap();
+
+        let t = rep.get_task(&uuid).unwrap().unwrap();
+        assert_eq!(t.status, Status::Completed);
+        assert_eq!(t.description, String::from("another task, updated"));
         assert_eq!(t.project, Some("work".into()));
     }
 

src/task/taskbuilder.rs

@@ -131,8 +131,6 @@ impl TaskBuilder {
             annotations: self.annotations,
             udas: self.udas,
         }
-
-        // TODO: check validity per https://taskwarrior.org/docs/design/task.html
     }
 }