diff --git a/taskchampion/src/replica.rs b/taskchampion/src/replica.rs index 2b833a323..576de2f7a 100644 --- a/taskchampion/src/replica.rs +++ b/taskchampion/src/replica.rs @@ -99,18 +99,25 @@ impl Replica { .map(move |tm| Task::new(uuid, tm))) } - /// Create a new task. The task must not already exist. + /// Create a new task. pub fn new_task(&mut self, status: Status, description: String) -> anyhow::Result { - self.add_undo_point(false)?; let uuid = Uuid::new_v4(); - let taskmap = self.taskdb.apply(SyncOp::Create { uuid })?; - trace!("task {} created", uuid); - let mut task = Task::new(uuid, taskmap).into_mut(self); + let mut task = self.create_task(uuid)?.into_mut(self); task.set_description(description)?; task.set_status(status)?; + trace!("task {} created", uuid); Ok(task.into_immut()) } + /// Create a new, empty task with the given UUID. This is useful for importing tasks, but + /// otherwise should be avoided in favor of `new_task`. If the task already exists, this + /// does nothing and returns the existing task. + pub fn create_task(&mut self, uuid: Uuid) -> anyhow::Result { + self.add_undo_point(false)?; + let taskmap = self.taskdb.apply(SyncOp::Create { uuid })?; + Ok(Task::new(uuid, taskmap)) + } + /// 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. diff --git a/taskchampion/src/taskdb/apply.rs b/taskchampion/src/taskdb/apply.rs index 1be3864c9..1e3a3fa83 100644 --- a/taskchampion/src/taskdb/apply.rs +++ b/taskchampion/src/taskdb/apply.rs @@ -4,7 +4,8 @@ use crate::storage::{ReplicaOp, StorageTxn, TaskMap}; /// Apply the given SyncOp to the replica, updating both the task data and adding a /// ReplicaOp to the list of operations. Returns the TaskMap of the task after the -/// operation has been applied (or an empty TaskMap for Delete). +/// operation has been applied (or an empty TaskMap for Delete). It is not an error +/// to create an existing task, nor to delete a nonexistent task. pub(super) fn apply_and_record(txn: &mut dyn StorageTxn, op: SyncOp) -> anyhow::Result { match op { SyncOp::Create { uuid } => { @@ -14,8 +15,9 @@ pub(super) fn apply_and_record(txn: &mut dyn StorageTxn, op: SyncOp) -> anyhow:: txn.commit()?; Ok(TaskMap::new()) } else { - // TODO: differentiate error types here? - Err(Error::Database(format!("Task {} already exists", uuid)).into()) + Ok(txn + .get_task(uuid)? + .expect("create_task failed but task does not exist")) } } SyncOp::Delete { uuid } => { @@ -29,7 +31,7 @@ pub(super) fn apply_and_record(txn: &mut dyn StorageTxn, op: SyncOp) -> anyhow:: txn.commit()?; Ok(TaskMap::new()) } else { - Err(Error::Database(format!("Task {} does not exist", uuid)).into()) + Ok(TaskMap::new()) } } SyncOp::Update { @@ -105,6 +107,7 @@ pub(super) fn apply_op(txn: &mut dyn StorageTxn, op: &SyncOp) -> anyhow::Result< #[cfg(test)] mod tests { use super::*; + use crate::storage::TaskMap; use crate::taskdb::TaskDb; use chrono::Utc; use pretty_assertions::assert_eq; @@ -133,24 +136,33 @@ mod tests { fn test_apply_create_exists() -> anyhow::Result<()> { let mut db = TaskDb::new_inmemory(); let uuid = Uuid::new_v4(); + { + let mut txn = db.storage.txn()?; + txn.create_task(uuid)?; + let mut taskmap = TaskMap::new(); + taskmap.insert("foo".into(), "bar".into()); + txn.set_task(uuid, taskmap)?; + txn.commit()?; + } + let op = SyncOp::Create { uuid }; { let mut txn = db.storage.txn()?; let taskmap = apply_and_record(txn.as_mut(), op.clone())?; - assert_eq!(taskmap.len(), 0); - assert_eq!( - apply_and_record(txn.as_mut(), op) - .err() - .unwrap() - .to_string(), - format!("Task Database Error: Task {} already exists", uuid) - ); + + assert_eq!(taskmap.len(), 1); + assert_eq!(taskmap.get("foo").unwrap(), "bar"); + txn.commit()?; } - // first op was applied - assert_eq!(db.sorted_tasks(), vec![(uuid, vec![])]); - assert_eq!(db.operations(), vec![ReplicaOp::Create { uuid }]); + // create did not delete the old task.. + assert_eq!( + db.sorted_tasks(), + vec![(uuid, vec![("foo".into(), "bar".into())])] + ); + // create was done "manually" above, and no new op was added + assert_eq!(db.operations(), vec![]); Ok(()) } @@ -384,13 +396,8 @@ mod tests { let op = SyncOp::Delete { uuid }; { let mut txn = db.storage.txn()?; - assert_eq!( - apply_and_record(txn.as_mut(), op) - .err() - .unwrap() - .to_string(), - format!("Task Database Error: Task {} does not exist", uuid) - ); + let taskmap = apply_and_record(txn.as_mut(), op)?; + assert_eq!(taskmap.len(), 0); txn.commit()?; }