Skip to content

Commit

Permalink
Revert incomplete "expose operations" work (#466)
Browse files Browse the repository at this point in the history 
* Revert "Temporarily make get_task_operations non-public (#464)"

This reverts commit 8f3914b.

* Revert "Additional tests for Replica methods (#459)"

This reverts commit a5f116f.

* Revert "Apply the same tests to all storage backends (#453)"

This reverts commit c7c2cde.

* Revert "Replace `Storage::set_operations` with `sync_complete` and `remove_operation`.` (#451)"

This reverts commit d7a7d96.

* Revert "Add `get_task_operations` (#450)"

This reverts commit 8bfbb9d.

* Revert "Add an 'operations.uuid' column, generated and virtual (#449)"

This reverts commit 427ad78.

* Revert "Add a test for upgrading from 0.7.0 DBs (#448)"

This reverts commit fb84873.
  • Loading branch information
@djmitche
djmitche authored Oct 13, 2024
1 parent 8e61d17 commit ea3c68e
Show file tree
Hide file tree
Showing 9 changed files with 534 additions and 1,107 deletions.
10 changes: 0 additions & 10 deletions taskchampion/src/operation.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,16 +43,6 @@ impl Operation {
pub fn is_undo_point(&self) -> bool {
self == &Self::UndoPoint
}

/// Get the UUID for this function, if it has one.
pub fn get_uuid(&self) -> Option<Uuid> {
match self {
Operation::Create { uuid: u } => Some(*u),
Operation::Delete { uuid: u, .. } => Some(*u),
Operation::Update { uuid: u, .. } => Some(*u),
Operation::UndoPoint => None,
}
}
}

/// Operations are a sequence of [`Operation`] values, which can be committed in a single
Expand Down
180 changes: 46 additions & 134 deletions taskchampion/src/replica.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,29 +217,14 @@ impl Replica {
.map(move |tm| Task::new(TaskData::new(uuid, tm), depmap)))
}

/// Get an existing task by its UUID, as a [`TaskData`](crate::TaskData).
/// get an existing task by its UUID, as a [`TaskData`](crate::TaskData).
pub fn get_task_data(&mut self, uuid: Uuid) -> Result<Option<TaskData>> {
Ok(self
.taskdb
.get_task(uuid)?
.map(move |tm| TaskData::new(uuid, tm)))
}

/// Get the operations that led to the given task.
///
/// Note: the returned set of operations is not guaranteed to be sufficient to reconstruct the
/// task; that is, it may not begin with a `Create` operation. This can occur if the task was
/// created using a TaskChampion version before 0.8.0 or if older operations have been deleted.
///
/// This function was introduced in
/// [#373](https://github.com/GothenburgBitFactory/taskchampion/issues/373) but removed from
/// the public API until it actually returns _all_ task operations, and not just local
/// operations since the last sync.
#[allow(dead_code)]
pub(crate) fn get_task_operations(&mut self, uuid: Uuid) -> Result<Operations> {
self.taskdb.get_task_operations(uuid)
}

/// Create a new task, setting `modified`, `description`, `status`, and `entry`.
///
/// This uses the high-level task interface. To create a task with the low-level
Expand Down Expand Up @@ -480,43 +465,11 @@ impl Replica {
mod tests {
use super::*;
use crate::task::Status;
use chrono::{DateTime, TimeZone};
use chrono::TimeZone;
use pretty_assertions::assert_eq;
use std::collections::HashSet;
use uuid::Uuid;

const JUST_NOW: Option<DateTime<Utc>> = DateTime::from_timestamp(1800000000, 0);

/// Rewrite automatically-created dates to "just-now" or `JUST_NOW` for ease of testing.
fn clean_op(op: Operation) -> Operation {
if let Operation::Update {
uuid,
property,
mut old_value,
mut value,
..
} = op
{
if property == "modified" || property == "end" || property == "entry" {
if value.is_some() {
value = Some("just-now".into());
}
if old_value.is_some() {
old_value = Some("just-now".into());
}
}
Operation::Update {
uuid,
property,
old_value,
value,
timestamp: JUST_NOW.unwrap(),
}
} else {
op
}
}

#[test]
fn new_task() {
let mut rep = Replica::new_inmemory();
Expand Down Expand Up @@ -557,6 +510,37 @@ mod tests {

// and check for the corresponding operations, cleaning out the timestamps
// and modified properties as these are based on the current time
let now = Utc::now();
let clean_op = |op: Operation| {
if let Operation::Update {
uuid,
property,
mut old_value,
mut value,
..
} = op
{
// rewrite automatically-created dates to "just-now" for ease
// of testing
if property == "modified" || property == "end" || property == "entry" {
if value.is_some() {
value = Some("just-now".into());
}
if old_value.is_some() {
old_value = Some("just-now".into());
}
}
Operation::Update {
uuid,
property,
old_value,
value,
timestamp: now,
}
} else {
op
}
};
assert_eq!(
rep.taskdb
.operations()
Expand All @@ -571,56 +555,56 @@ mod tests {
property: "modified".into(),
old_value: None,
value: Some("just-now".into()),
timestamp: JUST_NOW.unwrap(),
timestamp: now,
},
Operation::Update {
uuid: t.get_uuid(),
property: "description".into(),
old_value: None,
value: Some("a task".into()),
timestamp: JUST_NOW.unwrap(),
timestamp: now,
},
Operation::Update {
uuid: t.get_uuid(),
property: "status".into(),
old_value: None,
value: Some("pending".into()),
timestamp: JUST_NOW.unwrap(),
timestamp: now,
},
Operation::Update {
uuid: t.get_uuid(),
property: "entry".into(),
old_value: None,
value: Some("just-now".into()),
timestamp: JUST_NOW.unwrap(),
timestamp: now,
},
Operation::Update {
uuid: t.get_uuid(),
property: "modified".into(),
old_value: Some("just-now".into()),
value: Some("just-now".into()),
timestamp: JUST_NOW.unwrap(),
timestamp: now,
},
Operation::Update {
uuid: t.get_uuid(),
property: "description".into(),
old_value: Some("a task".into()),
value: Some("past tense".into()),
timestamp: JUST_NOW.unwrap(),
timestamp: now,
},
Operation::Update {
uuid: t.get_uuid(),
property: "end".into(),
old_value: None,
value: Some("just-now".into()),
timestamp: JUST_NOW.unwrap(),
timestamp: now,
},
Operation::Update {
uuid: t.get_uuid(),
property: "status".into(),
old_value: Some("pending".into()),
value: Some("completed".into()),
timestamp: JUST_NOW.unwrap(),
timestamp: now,
},
]
);
Expand All @@ -630,11 +614,6 @@ mod tests {

// num_undo_points includes only the undo point
assert_eq!(rep.num_undo_points().unwrap(), 1);

// A second undo point is counted.
let ops = vec![Operation::UndoPoint];
rep.commit_operations(ops).unwrap();
assert_eq!(rep.num_undo_points().unwrap(), 2);
}

#[test]
Expand Down Expand Up @@ -670,13 +649,6 @@ mod tests {
assert_eq!(all_tasks.len(), 2);
assert_eq!(all_tasks.get(&uuid1).unwrap().get_uuid(), uuid1);
assert_eq!(all_tasks.get(&uuid2).unwrap().get_uuid(), uuid2);

let mut all_uuids = rep.all_task_uuids().unwrap();
all_uuids.sort();
let mut exp_uuids = vec![uuid1, uuid2];
exp_uuids.sort();
assert_eq!(all_uuids.len(), 2);
assert_eq!(all_uuids, exp_uuids);
}

#[test]
Expand Down Expand Up @@ -824,37 +796,6 @@ mod tests {
Ok(())
}

#[test]
fn commit_reversed_operations() -> Result<()> {
let uuid1 = Uuid::new_v4();
let uuid2 = Uuid::new_v4();
let uuid3 = Uuid::new_v4();

let mut rep = Replica::new_inmemory();

let mut ops = Operations::new();
ops.push(Operation::UndoPoint);
rep.create_task(uuid1, &mut ops).unwrap();
ops.push(Operation::UndoPoint);
rep.create_task(uuid2, &mut ops).unwrap();
rep.commit_operations(dbg!(ops))?;
assert_eq!(rep.num_undo_points().unwrap(), 2);

// Trying to reverse-commit the wrong operations fails.
let ops = vec![Operation::Delete {
uuid: uuid3,
old_task: TaskMap::new(),
}];
assert!(!rep.commit_reversed_operations(ops)?);

// Commiting the correct operations succeeds
let ops = rep.get_undo_operations()?;
assert!(rep.commit_reversed_operations(ops)?);
assert_eq!(rep.num_undo_points().unwrap(), 1);

Ok(())
}

#[test]
fn get_and_modify() {
let mut rep = Replica::new_inmemory();
Expand Down Expand Up @@ -885,49 +826,20 @@ mod tests {
}

#[test]
fn get_task_data_and_operations() {
fn get_task_data() {
let mut rep = Replica::new_inmemory();

let uuid1 = Uuid::new_v4();
let uuid2 = Uuid::new_v4();
let uuid = Uuid::new_v4();
let mut ops = Operations::new();
let mut t = rep.create_task(uuid1, &mut ops).unwrap();
let mut t = rep.create_task(uuid, &mut ops).unwrap();
t.set_description("another task".into(), &mut ops).unwrap();
let mut t2 = rep.create_task(uuid2, &mut ops).unwrap();
t2.set_description("a distraction!".into(), &mut ops)
.unwrap();
rep.commit_operations(ops).unwrap();

let t = rep.get_task_data(uuid1).unwrap().unwrap();
assert_eq!(t.get_uuid(), uuid1);
let t = rep.get_task_data(uuid).unwrap().unwrap();
assert_eq!(t.get_uuid(), uuid);
assert_eq!(t.get("description"), Some("another task"));
assert_eq!(
rep.get_task_operations(uuid1)
.unwrap()
.into_iter()
.map(clean_op)
.collect::<Vec<_>>(),
vec![
Operation::Create { uuid: uuid1 },
Operation::Update {
uuid: uuid1,
property: "modified".into(),
old_value: None,
value: Some("just-now".into()),
timestamp: JUST_NOW.unwrap(),
},
Operation::Update {
uuid: uuid1,
property: "description".into(),
old_value: None,
value: Some("another task".into()),
timestamp: JUST_NOW.unwrap(),
},
]
);

assert!(rep.get_task_data(Uuid::new_v4()).unwrap().is_none());
assert_eq!(rep.get_task_operations(Uuid::new_v4()).unwrap(), vec![]);
}

#[test]
Expand Down
Loading

0 comments on commit ea3c68e

Please sign in to comment.