Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert incomplete "expose operations" work #466

Merged
merged 7 commits into from
Oct 13, 2024
+534 −1,107
Merged

Revert incomplete "expose operations" work #466

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a71cc0e
Revert "Temporarily make get_task_operations non-public (#464)"
djmitche Oct 12, 2024
8a0c02f
Revert "Additional tests for Replica methods (#459)"
djmitche Oct 12, 2024
0ce4a42
Revert "Apply the same tests to all storage backends (#453)"
djmitche Oct 12, 2024
350a708
Revert "Replace `Storage::set_operations` with `sync_complete` and `r…
djmitche Oct 12, 2024
f51c7ab
Revert "Add `get_task_operations` (#450)"
djmitche Oct 12, 2024
5556ac9
Revert "Add an 'operations.uuid' column, generated and virtual (#449)"
djmitche Oct 12, 2024
b36a476
Revert "Add a test for upgrading from 0.7.0 DBs (#448)"
djmitche Oct 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 0 additions & 10 deletions taskchampion/src/operation.rs
View file
Open in desktop
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
Open in desktop
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
Loading