GothenburgBitFactory/taskwarrior
1
0
Fork 0
mirror of https://github.com/GothenburgBitFactory/taskwarrior.git synced 2025-06-26 10:54:26 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

Always pass Uuids by value

Browse source 
Rust handles this well.  Fixes #125.
This commit is contained in:
Dustin J. Mitchell 2021-01-09 22:09:06 +00:00
parent ca1b7da9bf
commit 45d3e38c63
8 changed files with 78 additions and 78 deletions
Download patch file Download diff file Expand all files Collapse all files

16
cli/src/invocation/filter.rs
View file

@ -125,18 +125,18 @@ pub(super) fn filtered_tasks(
let task = match id {
TaskId::WorkingSetId(id) => replica.get_working_set_task(*id)?,
TaskId::PartialUuid(_) => unreachable!(), // not present in absolute id list
TaskId::Uuid(id) => replica.get_task(id)?,
TaskId::Uuid(id) => replica.get_task(*id)?,
};
if let Some(task) = task {
// if we have already seen this task, skip ahead..
let uuid = *task.get_uuid();
let uuid = task.get_uuid();
if seen.contains(&uuid) {
continue;
}
seen.insert(uuid);
let working_set_id = replica.get_working_set_index(&uuid)?;
let working_set_id = replica.get_working_set_index(uuid)?;
if match_task(filter, &task, uuid, working_set_id) {
res.push(task);
@ -150,7 +150,7 @@ pub(super) fn filtered_tasks(
log::debug!("Scanning all tasks in the task database");
for (uuid, task) in replica.all_tasks()?.drain() {
// Yikes, slow! https://github.com/djmitche/taskchampion/issues/108
let working_set_id = replica.get_working_set_index(&uuid)?;
let working_set_id = replica.get_working_set_index(uuid)?;
if match_task(filter, &task, uuid, working_set_id) {
res.push(task);
}
@ -160,7 +160,7 @@ pub(super) fn filtered_tasks(
log::debug!("Scanning only the working set (pending tasks)");
for (i, task) in replica.working_set()?.drain(..).enumerate() {
if let Some(task) = task {
let uuid = *task.get_uuid();
let uuid = task.get_uuid();
if match_task(filter, &task, uuid, Some(i)) {
res.push(task);
}
@ -186,13 +186,13 @@ mod test {
let _t = replica.new_task(Status::Pending, s!("C")).unwrap();
replica.rebuild_working_set(true).unwrap();
let t1uuid = *t1.get_uuid();
let t1uuid = t1.get_uuid();
let filter = Filter {
conditions: vec![Condition::IdList(vec![
TaskId::Uuid(t1uuid), // A
TaskId::WorkingSetId(1), // A (again, dups filtered)
TaskId::Uuid(*t2.get_uuid()), // B
TaskId::Uuid(t2.get_uuid()), // B
])],
};
let mut filtered: Vec<_> = filtered_tasks(&mut replica, &filter)
@ -212,7 +212,7 @@ mod test {
let _t = replica.new_task(Status::Pending, s!("C")).unwrap();
replica.rebuild_working_set(true).unwrap();
let t1uuid = *t1.get_uuid();
let t1uuid = t1.get_uuid();
let t2uuid = t2.get_uuid().to_string();
let t2partial = t2uuid[..13].to_owned();

20
cli/src/invocation/report.rs
View file

@ -18,15 +18,15 @@ impl WorkingSet {
Ok(Self(
working_set
.iter()
.map(|opt| opt.as_ref().map(|t| *t.get_uuid()))
.map(|opt| opt.as_ref().map(|t| t.get_uuid()))
.collect(),
))
}
fn index(&self, target: &Uuid) -> Option<usize> {
fn index(&self, target: Uuid) -> Option<usize> {
for (i, uuid) in self.0.iter().enumerate() {
if let Some(uuid) = uuid {
if uuid == target {
if *uuid == target {
return Some(i);
}
}
@ -51,10 +51,10 @@ fn sort_tasks(tasks: &mut Vec<Task>, report: &Report, working_set: &WorkingSet)
(Some(a_id), Some(b_id)) => a_id.cmp(&b_id),
(Some(_), None) => Ordering::Less,
(None, Some(_)) => Ordering::Greater,
(None, None) => a_uuid.cmp(b_uuid),
(None, None) => a_uuid.cmp(&b_uuid),
}
}
SortBy::Uuid => a.get_uuid().cmp(b.get_uuid()),
SortBy::Uuid => a.get_uuid().cmp(&b.get_uuid()),
SortBy::Description => a.get_description().cmp(b.get_description()),
};
// If this sort property is equal, go on to the next..
@ -165,7 +165,7 @@ mod test {
replica.rebuild_working_set(true).unwrap();
[*t1.get_uuid(), *t2.get_uuid(), *t3.get_uuid()]
[t1.get_uuid(), t2.get_uuid(), t3.get_uuid()]
}
#[test]
@ -237,7 +237,7 @@ mod test {
let mut tasks: Vec<_> = replica.all_tasks().unwrap().values().cloned().collect();
sort_tasks(&mut tasks, &report, &working_set);
let got_uuids: Vec<_> = tasks.iter().map(|t| *t.get_uuid()).collect();
let got_uuids: Vec<_> = tasks.iter().map(|t| t.get_uuid()).collect();
let mut exp_uuids = uuids.to_vec();
exp_uuids.sort();
assert_eq!(got_uuids, exp_uuids);
@ -291,7 +291,7 @@ mod test {
// get the task that's not in the working set, which should show
// a uuid for its id column
let task = replica.get_task(&uuids[1]).unwrap().unwrap();
let task = replica.get_task(uuids[1]).unwrap().unwrap();
assert_eq!(
task_column(&task, &column, &working_set),
uuids[1].to_string()
@ -323,7 +323,7 @@ mod test {
// make task A active
replica
.get_task(&uuids[0])
.get_task(uuids[0])
.unwrap()
.unwrap()
.into_mut(&mut replica)
@ -363,7 +363,7 @@ mod test {
// add some tags to task A
let mut t1 = replica
.get_task(&uuids[0])
.get_task(uuids[0])
.unwrap()
.unwrap()
.into_mut(&mut replica);

30
taskchampion/src/replica.rs
View file

@ -69,7 +69,7 @@ impl Replica {
}
/// Add the given uuid to the working set, returning its index.
pub(crate) fn add_to_working_set(&mut self, uuid: &Uuid) -> Fallible<usize> {
pub(crate) fn add_to_working_set(&mut self, uuid: Uuid) -> Fallible<usize> {
self.taskdb.add_to_working_set(uuid)
}
@ -94,7 +94,7 @@ impl Replica {
let mut res = Vec::with_capacity(working_set.len());
for item in working_set.iter() {
res.push(match item {
Some(u) => match self.taskdb.get_task(u)? {
Some(u) => match self.taskdb.get_task(*u)? {
Some(tm) => Some(Task::new(*u, tm)),
None => None,
},
@ -105,11 +105,11 @@ impl Replica {
}
/// Get an existing task by its UUID
pub fn get_task(&mut self, uuid: &Uuid) -> Fallible<Option<Task>> {
pub fn get_task(&mut self, uuid: Uuid) -> Fallible<Option<Task>> {
Ok(self
.taskdb
.get_task(uuid)?
.map(move |tm| Task::new(*uuid, tm)))
.map(move |tm| Task::new(uuid, tm)))
}
/// Get an existing task by its working set index
@ -119,7 +119,7 @@ impl Replica {
if let Some(uuid) = working_set[i as usize] {
return Ok(self
.taskdb
.get_task(&uuid)?
.get_task(uuid)?
.map(move |tm| Task::new(uuid, tm)));
}
}
@ -127,11 +127,11 @@ impl Replica {
}
/// Get the working set index for the given task uuid
pub fn get_working_set_index(&mut self, uuid: &Uuid) -> Fallible<Option<usize>> {
pub fn get_working_set_index(&mut self, uuid: Uuid) -> Fallible<Option<usize>> {
let working_set = self.taskdb.working_set()?;
for (i, u) in working_set.iter().enumerate() {
if let Some(ref u) = u {
if u == uuid {
if let Some(u) = u {
if *u == uuid {
return Ok(Some(i));
}
}
@ -154,13 +154,13 @@ impl Replica {
/// 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) -> Fallible<()> {
fn delete_task(&mut self, uuid: Uuid) -> Fallible<()> {
// 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: *uuid })?;
self.taskdb.apply(Operation::Delete { uuid })?;
trace!("task {} deleted", uuid);
Ok(())
}
@ -220,7 +220,7 @@ mod tests {
assert_eq!(t.get_status(), Status::Completed);
// check tha values have changed in storage, too
let t = rep.get_task(&t.get_uuid()).unwrap().unwrap();
let t = rep.get_task(t.get_uuid()).unwrap().unwrap();
assert_eq!(t.get_description(), "past tense");
assert_eq!(t.get_status(), Status::Completed);
}
@ -233,7 +233,7 @@ mod tests {
let uuid = t.get_uuid();
rep.delete_task(uuid).unwrap();
assert_eq!(rep.get_task(&uuid).unwrap(), None);
assert_eq!(rep.get_task(uuid).unwrap(), None);
}
#[test]
@ -245,14 +245,14 @@ mod tests {
.unwrap();
let uuid = t.get_uuid();
let t = rep.get_task(&uuid).unwrap().unwrap();
let t = rep.get_task(uuid).unwrap().unwrap();
assert_eq!(t.get_description(), String::from("another task"));
let mut t = t.into_mut(&mut rep);
t.set_status(Status::Deleted).unwrap();
t.set_description("gone".into()).unwrap();
let t = rep.get_task(&uuid).unwrap().unwrap();
let t = rep.get_task(uuid).unwrap().unwrap();
assert_eq!(t.get_status(), Status::Deleted);
assert_eq!(t.get_description(), "gone");
@ -286,6 +286,6 @@ mod tests {
fn get_does_not_exist() {
let mut rep = Replica::new_inmemory();
let uuid = Uuid::new_v4();
assert_eq!(rep.get_task(&uuid).unwrap(), None);
assert_eq!(rep.get_task(uuid).unwrap(), None);
}
}

8
taskchampion/src/task.rs
View file

@ -167,8 +167,8 @@ impl Task {
Task { uuid, taskmap }
}
pub fn get_uuid(&self) -> &Uuid {
&self.uuid
pub fn get_uuid(&self) -> Uuid {
self.uuid
}
pub fn get_taskmap(&self) -> &TaskMap {
@ -254,7 +254,7 @@ impl<'r> TaskMut<'r> {
pub fn set_status(&mut self, status: Status) -> Fallible<()> {
if status == Status::Pending {
let uuid = self.uuid;
self.replica.add_to_working_set(&uuid)?;
self.replica.add_to_working_set(uuid)?;
}
self.set_string("status", Some(String::from(status.to_taskmap())))
}
@ -353,7 +353,7 @@ impl<'r> TaskMut<'r> {
#[cfg(test)]
fn reload(&mut self) -> Fallible<()> {
let uuid = self.uuid;
let task = self.replica.get_task(&uuid)?.unwrap();
let task = self.replica.get_task(uuid)?.unwrap();
self.task.taskmap = task.taskmap;
Ok(())
}

18
taskchampion/src/taskdb.rs
View file

@ -54,7 +54,7 @@ impl TaskDB {
}
}
Operation::Delete { ref uuid } => {
if !txn.delete_task(uuid)? {
if !txn.delete_task(*uuid)? {
return Err(Error::DBError(format!("Task {} does not exist", uuid)).into());
}
}
@ -65,7 +65,7 @@ impl TaskDB {
timestamp: _,
} => {
// update if this task exists, otherwise ignore
if let Some(mut task) = txn.get_task(uuid)? {
if let Some(mut task) = txn.get_task(*uuid)? {
match value {
Some(ref val) => task.insert(property.to_string(), val.clone()),
None => task.remove(property),
@ -99,7 +99,7 @@ impl TaskDB {
}
/// Get a single task, by uuid.
pub fn get_task(&mut self, uuid: &Uuid) -> Fallible<Option<TaskMap>> {
pub fn get_task(&mut self, uuid: Uuid) -> Fallible<Option<TaskMap>> {
let mut txn = self.storage.txn()?;
txn.get_task(uuid)
}
@ -123,7 +123,7 @@ impl TaskDB {
// working set.
for elt in txn.get_working_set()? {
if let Some(uuid) = elt {
if let Some(task) = txn.get_task(&uuid)? {
if let Some(task) = txn.get_task(uuid)? {
if in_working_set(&task) {
new_ws.push(Some(uuid));
seen.insert(uuid);
@ -143,7 +143,7 @@ impl TaskDB {
txn.clear_working_set()?;
for elt in new_ws.drain(0..new_ws.len()) {
if let Some(uuid) = elt {
txn.add_to_working_set(&uuid)?;
txn.add_to_working_set(uuid)?;
}
}
} else {
@ -159,7 +159,7 @@ impl TaskDB {
// end of the list, whether renumbering or not
for (uuid, task) in txn.all_tasks()? {
if !seen.contains(&uuid) && in_working_set(&task) {
txn.add_to_working_set(&uuid)?;
txn.add_to_working_set(uuid)?;
}
}
@ -169,11 +169,11 @@ impl TaskDB {
/// Add the given uuid to the working set and return its index; if it is already in the working
/// set, its index is returned. This does *not* renumber any existing tasks.
pub fn add_to_working_set(&mut self, uuid: &Uuid) -> Fallible<usize> {
pub fn add_to_working_set(&mut self, uuid: Uuid) -> Fallible<usize> {
let mut txn = self.storage.txn()?;
// search for an existing entry for this task..
for (i, elt) in txn.get_working_set()?.iter().enumerate() {
if *elt == Some(*uuid) {
if *elt == Some(uuid) {
// (note that this drops the transaction with no changes made)
return Ok(i);
}
@ -544,7 +544,7 @@ mod tests {
txn.clear_working_set()?;
for i in &[1usize, 3, 4] {
txn.add_to_working_set(&uuids[*i])?;
txn.add_to_working_set(uuids[*i])?;
}
txn.commit()?;

24
taskchampion/src/taskstorage/inmemory.rs
View file

@ -43,8 +43,8 @@ impl<'t> Txn<'t> {
}
impl<'t> TaskStorageTxn for Txn<'t> {
fn get_task(&mut self, uuid: &Uuid) -> Fallible<Option<TaskMap>> {
match self.data_ref().tasks.get(uuid) {
fn get_task(&mut self, uuid: Uuid) -> Fallible<Option<TaskMap>> {
match self.data_ref().tasks.get(&uuid) {
None => Ok(None),
Some(t) => Ok(Some(t.clone())),
}
@ -64,8 +64,8 @@ impl<'t> TaskStorageTxn for Txn<'t> {
Ok(())
}
fn delete_task(&mut self, uuid: &Uuid) -> Fallible<bool> {
Ok(self.mut_data_ref().tasks.remove(uuid).is_some())
fn delete_task(&mut self, uuid: Uuid) -> Fallible<bool> {
Ok(self.mut_data_ref().tasks.remove(&uuid).is_some())
}
fn all_tasks<'a>(&mut self) -> Fallible<Vec<(Uuid, TaskMap)>> {
@ -108,9 +108,9 @@ impl<'t> TaskStorageTxn for Txn<'t> {
Ok(self.data_ref().working_set.clone())
}
fn add_to_working_set(&mut self, uuid: &Uuid) -> Fallible<usize> {
fn add_to_working_set(&mut self, uuid: Uuid) -> Fallible<usize> {
let working_set = &mut self.mut_data_ref().working_set;
working_set.push(Some(*uuid));
working_set.push(Some(uuid));
Ok(working_set.len())
}
@ -194,8 +194,8 @@ mod test {
{
let mut txn = storage.txn()?;
txn.add_to_working_set(&uuid1)?;
txn.add_to_working_set(&uuid2)?;
txn.add_to_working_set(uuid1)?;
txn.add_to_working_set(uuid2)?;
txn.commit()?;
}
@ -216,16 +216,16 @@ mod test {
{
let mut txn = storage.txn()?;
txn.add_to_working_set(&uuid1)?;
txn.add_to_working_set(&uuid2)?;
txn.add_to_working_set(uuid1)?;
txn.add_to_working_set(uuid2)?;
txn.commit()?;
}
{
let mut txn = storage.txn()?;
txn.clear_working_set()?;
txn.add_to_working_set(&uuid2)?;
txn.add_to_working_set(&uuid1)?;
txn.add_to_working_set(uuid2)?;
txn.add_to_working_set(uuid1)?;
txn.commit()?;
}

30
taskchampion/src/taskstorage/kv.rs
View file

@ -105,7 +105,7 @@ impl<'t> Txn<'t> {
}
impl<'t> TaskStorageTxn for Txn<'t> {
fn get_task(&mut self, uuid: &Uuid) -> Fallible<Option<TaskMap>> {
fn get_task(&mut self, uuid: Uuid) -> Fallible<Option<TaskMap>> {
let bucket = self.tasks_bucket();
let buf = match self.kvtxn().get(bucket, uuid.into()) {
Ok(buf) => buf,
@ -136,7 +136,7 @@ impl<'t> TaskStorageTxn for Txn<'t> {
Ok(())
}
fn delete_task(&mut self, uuid: &Uuid) -> Fallible<bool> {
fn delete_task(&mut self, uuid: Uuid) -> Fallible<bool> {
let bucket = self.tasks_bucket();
let kvtxn = self.kvtxn();
match kvtxn.del(bucket, uuid.into()) {
@ -275,7 +275,7 @@ impl<'t> TaskStorageTxn for Txn<'t> {
Ok(res)
}
fn add_to_working_set(&mut self, uuid: &Uuid) -> Fallible<usize> {
fn add_to_working_set(&mut self, uuid: Uuid) -> Fallible<usize> {
let working_set_bucket = self.working_set_bucket();
let numbers_bucket = self.numbers_bucket();
let kvtxn = self.kvtxn();
@ -289,7 +289,7 @@ impl<'t> TaskStorageTxn for Txn<'t> {
kvtxn.set(
working_set_bucket,
next_index.into(),
Msgpack::to_value_buf(*uuid)?,
Msgpack::to_value_buf(uuid)?,
)?;
kvtxn.set(
numbers_bucket,
@ -372,7 +372,7 @@ mod test {
}
{
let mut txn = storage.txn()?;
let task = txn.get_task(&uuid)?;
let task = txn.get_task(uuid)?;
assert_eq!(task, Some(taskmap_with(vec![])));
}
Ok(())
@ -403,7 +403,7 @@ mod test {
let uuid = Uuid::new_v4();
{
let mut txn = storage.txn()?;
let task = txn.get_task(&uuid)?;
let task = txn.get_task(uuid)?;
assert_eq!(task, None);
}
Ok(())
@ -421,7 +421,7 @@ mod test {
}
{
let mut txn = storage.txn()?;
let task = txn.get_task(&uuid)?;
let task = txn.get_task(uuid)?;
assert_eq!(
task,
Some(taskmap_with(vec![("k".to_string(), "v".to_string())]))
@ -437,7 +437,7 @@ mod test {
let uuid = Uuid::new_v4();
{
let mut txn = storage.txn()?;
assert!(!txn.delete_task(&uuid)?);
assert!(!txn.delete_task(uuid)?);
}
Ok(())
}
@ -454,7 +454,7 @@ mod test {
}
{
let mut txn = storage.txn()?;
assert!(txn.delete_task(&uuid)?);
assert!(txn.delete_task(uuid)?);
}
Ok(())
}
@ -640,8 +640,8 @@ mod test {
{
let mut txn = storage.txn()?;
txn.add_to_working_set(&uuid1)?;
txn.add_to_working_set(&uuid2)?;
txn.add_to_working_set(uuid1)?;
txn.add_to_working_set(uuid2)?;
txn.commit()?;
}
@ -663,16 +663,16 @@ mod test {
{
let mut txn = storage.txn()?;
txn.add_to_working_set(&uuid1)?;
txn.add_to_working_set(&uuid2)?;
txn.add_to_working_set(uuid1)?;
txn.add_to_working_set(uuid2)?;
txn.commit()?;
}
{
let mut txn = storage.txn()?;
txn.clear_working_set()?;
txn.add_to_working_set(&uuid2)?;
txn.add_to_working_set(&uuid1)?;
txn.add_to_working_set(uuid2)?;
txn.add_to_working_set(uuid1)?;
txn.commit()?;
}

6
taskchampion/src/taskstorage/mod.rs
View file

@ -44,7 +44,7 @@ pub(crate) const DEFAULT_BASE_VERSION: Uuid = crate::server::NO_VERSION_ID;
/// It is safe and performant to drop transactions that did not modify any data without committing.
pub trait TaskStorageTxn {
/// Get an (immutable) task, if it is in the storage
fn get_task(&mut self, uuid: &Uuid) -> Fallible<Option<TaskMap>>;
fn get_task(&mut self, uuid: Uuid) -> Fallible<Option<TaskMap>>;
/// Create an (empty) task, only if it does not already exist. Returns true if
/// the task was created (did not already exist).
@ -55,7 +55,7 @@ pub trait TaskStorageTxn {
fn set_task(&mut self, uuid: Uuid, task: TaskMap) -> Fallible<()>;
/// Delete a task, if it exists. Returns true if the task was deleted (already existed)
fn delete_task(&mut self, uuid: &Uuid) -> Fallible<bool>;
fn delete_task(&mut self, uuid: Uuid) -> Fallible<bool>;
/// Get the uuids and bodies of all tasks in the storage, in undefined order.
fn all_tasks(&mut self) -> Fallible<Vec<(Uuid, TaskMap)>>;
@ -86,7 +86,7 @@ pub trait TaskStorageTxn {
/// Add a task to the working set and return its (one-based) index. This index will be one greater
/// than the highest used index.
fn add_to_working_set(&mut self, uuid: &Uuid) -> Fallible<usize>;
fn add_to_working_set(&mut self, uuid: Uuid) -> Fallible<usize>;
/// Update the working set task at the given index. This cannot add a new item to the
/// working set.