diff --git a/TODO.txt b/TODO.txt index 8cf4eba45..0d8c7da95 100644 --- a/TODO.txt +++ b/TODO.txt @@ -6,7 +6,7 @@ - tags: `tags. = ""` * add HTTP API * add pending-task indexing to Replica -* abstract server, storage, etc. into traits +* abstract server into trait * implement snapshot requests * implement backups * implement client-side encryption diff --git a/src/bin/task.rs b/src/bin/task.rs index 841a04e6c..e777fd317 100644 --- a/src/bin/task.rs +++ b/src/bin/task.rs @@ -1,6 +1,6 @@ extern crate clap; use clap::{App, Arg, SubCommand}; -use taskwarrior_rust::{Replica, DB}; +use taskwarrior_rust::{taskstorage, Replica, DB}; use uuid::Uuid; fn main() { @@ -16,7 +16,7 @@ fn main() { .subcommand(SubCommand::with_name("list").about("lists tasks")) .get_matches(); - let mut replica = Replica::new(DB::new().into()); + let mut replica = Replica::new(DB::new(Box::new(taskstorage::InMemoryStorage::new())).into()); match matches.subcommand() { ("add", Some(matches)) => { diff --git a/src/lib.rs b/src/lib.rs index e5374f4f3..5c5f3aba6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -10,7 +10,7 @@ mod replica; mod server; mod task; mod taskdb; -mod taskstorage; +pub mod taskstorage; mod tdb2; pub use operation::Operation; diff --git a/src/operation.rs b/src/operation.rs index f21c5b465..cba88c808 100644 --- a/src/operation.rs +++ b/src/operation.rs @@ -146,7 +146,7 @@ mod test { // check that the two operation sequences have the same effect, enforcing the invariant of // the transform function. - let mut db1 = DB::new(); + let mut db1 = DB::new_inmemory(); if let Some(ref o) = setup { db1.apply(o.clone()).unwrap(); } @@ -155,7 +155,7 @@ mod test { db1.apply(o).unwrap(); } - let mut db2 = DB::new(); + let mut db2 = DB::new_inmemory(); if let Some(ref o) = setup { db2.apply(o.clone()).unwrap(); } diff --git a/src/replica.rs b/src/replica.rs index 3c98c2db1..0d7a49ff0 100644 --- a/src/replica.rs +++ b/src/replica.rs @@ -70,7 +70,7 @@ mod tests { #[test] fn create() { - let mut rep = Replica::new(DB::new().into()); + let mut rep = Replica::new(DB::new_inmemory().into()); let uuid = Uuid::new_v4(); rep.create_task(uuid.clone()).unwrap(); @@ -79,7 +79,7 @@ mod tests { #[test] fn delete() { - let mut rep = Replica::new(DB::new().into()); + let mut rep = Replica::new(DB::new_inmemory().into()); let uuid = Uuid::new_v4(); rep.create_task(uuid.clone()).unwrap(); @@ -89,7 +89,7 @@ mod tests { #[test] fn update() { - let mut rep = Replica::new(DB::new().into()); + let mut rep = Replica::new(DB::new_inmemory().into()); let uuid = Uuid::new_v4(); rep.create_task(uuid.clone()).unwrap(); @@ -102,7 +102,7 @@ mod tests { #[test] fn get_does_not_exist() { - let rep = Replica::new(DB::new().into()); + let rep = Replica::new(DB::new_inmemory().into()); let uuid = Uuid::new_v4(); assert_eq!(rep.get_task(&uuid), None); } diff --git a/src/taskdb.rs b/src/taskdb.rs index 8f77e5fed..be5dc8388 100644 --- a/src/taskdb.rs +++ b/src/taskdb.rs @@ -1,16 +1,16 @@ use crate::errors::Error; use crate::operation::Operation; use crate::server::{Server, VersionAdd}; -use crate::taskstorage::{InMemoryStorage, TaskMap}; +use crate::taskstorage::{TaskMap, TaskStorage}; use failure::Fallible; use serde::{Deserialize, Serialize}; use std::collections::HashMap; use std::str; use uuid::Uuid; -#[derive(Debug, Clone)] +#[derive(Debug)] pub struct DB { - storage: InMemoryStorage, + storage: Box, } #[derive(Serialize, Deserialize, Debug)] @@ -20,11 +20,14 @@ struct Version { } impl DB { - /// Create a new, empty database - pub fn new() -> DB { - DB { - storage: InMemoryStorage::new(), - } + /// Create a new DB with the given backend storage + pub fn new(storage: Box) -> DB { + DB { storage } + } + + #[cfg(test)] + pub fn new_inmemory() -> DB { + DB::new(Box::new(crate::taskstorage::InMemoryStorage::new())) } /// Apply an operation to the DB. Aside from synchronization operations, this is the only way @@ -212,7 +215,7 @@ mod tests { #[test] fn test_apply_create() { - let mut db = DB::new(); + let mut db = DB::new_inmemory(); let uuid = Uuid::new_v4(); let op = Operation::Create { uuid }; db.apply(op.clone()).unwrap(); @@ -223,7 +226,7 @@ mod tests { #[test] fn test_apply_create_exists() { - let mut db = DB::new(); + let mut db = DB::new_inmemory(); let uuid = Uuid::new_v4(); let op = Operation::Create { uuid }; db.apply(op.clone()).unwrap(); @@ -238,7 +241,7 @@ mod tests { #[test] fn test_apply_create_update() { - let mut db = DB::new(); + let mut db = DB::new_inmemory(); let uuid = Uuid::new_v4(); let op1 = Operation::Create { uuid }; db.apply(op1.clone()).unwrap(); @@ -259,7 +262,7 @@ mod tests { #[test] fn test_apply_create_update_delete_prop() { - let mut db = DB::new(); + let mut db = DB::new_inmemory(); let uuid = Uuid::new_v4(); let op1 = Operation::Create { uuid }; db.apply(op1.clone()).unwrap(); @@ -301,7 +304,7 @@ mod tests { #[test] fn test_apply_update_does_not_exist() { - let mut db = DB::new(); + let mut db = DB::new_inmemory(); let uuid = Uuid::new_v4(); let op = Operation::Update { uuid, @@ -320,7 +323,7 @@ mod tests { #[test] fn test_apply_create_delete() { - let mut db = DB::new(); + let mut db = DB::new_inmemory(); let uuid = Uuid::new_v4(); let op1 = Operation::Create { uuid }; db.apply(op1.clone()).unwrap(); @@ -334,7 +337,7 @@ mod tests { #[test] fn test_apply_delete_not_present() { - let mut db = DB::new(); + let mut db = DB::new_inmemory(); let uuid = Uuid::new_v4(); let op1 = Operation::Delete { uuid }; diff --git a/src/taskstorage/inmemory.rs b/src/taskstorage/inmemory.rs new file mode 100644 index 000000000..798be6053 --- /dev/null +++ b/src/taskstorage/inmemory.rs @@ -0,0 +1,101 @@ +use crate::operation::Operation; +use crate::taskstorage::{TaskMap, TaskStorage}; +use std::collections::hash_map::Entry; +use std::collections::HashMap; +use uuid::Uuid; + +#[derive(PartialEq, Debug, Clone)] +pub struct InMemoryStorage { + // The current state, with all pending operations applied + tasks: HashMap, + + // The version at which `operations` begins + base_version: u64, + + // Operations applied since `base_version`, in order. + // + // INVARIANT: Given a snapshot at `base_version`, applying these operations produces `tasks`. + operations: Vec, +} + +impl InMemoryStorage { + pub fn new() -> InMemoryStorage { + InMemoryStorage { + tasks: HashMap::new(), + base_version: 0, + operations: vec![], + } + } +} + +impl TaskStorage for InMemoryStorage { + /// Get an (immutable) task, if it is in the storage + fn get_task(&self, uuid: &Uuid) -> Option { + match self.tasks.get(uuid) { + None => None, + Some(t) => Some(t.clone()), + } + } + + /// Create a task, only if it does not already exist. Returns true if + /// the task was created (did not already exist). + fn create_task(&mut self, uuid: Uuid, task: TaskMap) -> bool { + if let ent @ Entry::Vacant(_) = self.tasks.entry(uuid) { + ent.or_insert(task); + true + } else { + false + } + } + + /// Set a task, overwriting any existing task. + fn set_task(&mut self, uuid: Uuid, task: TaskMap) { + self.tasks.insert(uuid, task); + } + + /// Delete a task, if it exists. Returns true if the task was deleted (already existed) + fn delete_task(&mut self, uuid: &Uuid) -> bool { + if let Some(_) = self.tasks.remove(uuid) { + true + } else { + false + } + } + + fn get_task_uuids<'a>(&'a self) -> Box + 'a> { + Box::new(self.tasks.keys().map(|u| u.clone())) + } + + /// Add an operation to the list of operations in the storage. Note that this merely *stores* + /// the operation; it is up to the TaskDB to apply it. + fn add_operation(&mut self, op: Operation) { + self.operations.push(op); + } + + /// Get the current base_version for this storage -- the last version synced from the server. + fn base_version(&self) -> u64 { + return self.base_version; + } + + /// Get the current set of outstanding operations (operations that have not been sync'd to the + /// server yet) + fn operations<'a>(&'a self) -> Box + 'a> { + Box::new(self.operations.iter()) + } + + /// Apply the next version from the server. This replaces the existing base_version and + /// operations. It's up to the caller (TaskDB) to ensure this is done consistently. + fn update_version(&mut self, version: u64, new_operations: Vec) { + // ensure that we are applying the versions in order.. + assert_eq!(version, self.base_version + 1); + self.base_version = version; + self.operations = new_operations; + } + + /// Record the outstanding operations as synced to the server in the given version. + fn local_operations_synced(&mut self, version: u64) { + assert_eq!(version, self.base_version + 1); + self.base_version = version; + self.operations = vec![]; + } +} diff --git a/src/taskstorage/mod.rs b/src/taskstorage/mod.rs index bbc7707a2..dd8e233de 100644 --- a/src/taskstorage/mod.rs +++ b/src/taskstorage/mod.rs @@ -1,100 +1,50 @@ -use crate::operation::Operation; -use std::collections::hash_map::Entry; +use crate::Operation; use std::collections::HashMap; +use std::fmt; use uuid::Uuid; +mod inmemory; + +pub use inmemory::InMemoryStorage; + +/// An in-memory representation of a task as a simple hashmap pub type TaskMap = HashMap; -#[derive(PartialEq, Debug, Clone)] -pub struct InMemoryStorage { - // The current state, with all pending operations applied - tasks: HashMap, - - // The version at which `operations` begins - base_version: u64, - - // Operations applied since `base_version`, in order. - // - // INVARIANT: Given a snapshot at `base_version`, applying these operations produces `tasks`. - operations: Vec, -} - -impl InMemoryStorage { - pub fn new() -> InMemoryStorage { - InMemoryStorage { - tasks: HashMap::new(), - base_version: 0, - operations: vec![], - } - } - +/// A trait for objects able to act as backing storage for a TaskDB. This API is optimized to be +/// easy to implement, with all of the semantic meaning of the data located in the TaskDB +/// implementation, which is the sole consumer of this trait. +pub trait TaskStorage: fmt::Debug { /// Get an (immutable) task, if it is in the storage - pub fn get_task(&self, uuid: &Uuid) -> Option { - match self.tasks.get(uuid) { - None => None, - Some(t) => Some(t.clone()), - } - } + fn get_task(&self, uuid: &Uuid) -> Option; /// Create a task, only if it does not already exist. Returns true if /// the task was created (did not already exist). - pub fn create_task(&mut self, uuid: Uuid, task: TaskMap) -> bool { - if let ent @ Entry::Vacant(_) = self.tasks.entry(uuid) { - ent.or_insert(task); - true - } else { - false - } - } + fn create_task(&mut self, uuid: Uuid, task: TaskMap) -> bool; /// Set a task, overwriting any existing task. - pub fn set_task(&mut self, uuid: Uuid, task: TaskMap) { - self.tasks.insert(uuid, task); - } + fn set_task(&mut self, uuid: Uuid, task: TaskMap); /// Delete a task, if it exists. Returns true if the task was deleted (already existed) - pub fn delete_task(&mut self, uuid: &Uuid) -> bool { - if let Some(_) = self.tasks.remove(uuid) { - true - } else { - false - } - } + fn delete_task(&mut self, uuid: &Uuid) -> bool; - pub fn get_task_uuids<'a>(&'a self) -> impl Iterator + 'a { - self.tasks.keys().map(|u| u.clone()) - } + /// Get the uuids of all tasks in the storage, in undefined order. + fn get_task_uuids<'a>(&'a self) -> Box + 'a>; /// Add an operation to the list of operations in the storage. Note that this merely *stores* /// the operation; it is up to the TaskDB to apply it. - pub fn add_operation(&mut self, op: Operation) { - self.operations.push(op); - } + fn add_operation(&mut self, op: Operation); /// Get the current base_version for this storage -- the last version synced from the server. - pub fn base_version(&self) -> u64 { - return self.base_version; - } + fn base_version(&self) -> u64; /// Get the current set of outstanding operations (operations that have not been sync'd to the /// server yet) - pub fn operations(&self) -> impl Iterator { - self.operations.iter() - } + fn operations<'a>(&'a self) -> Box + 'a>; /// Apply the next version from the server. This replaces the existing base_version and /// operations. It's up to the caller (TaskDB) to ensure this is done consistently. - pub(crate) fn update_version(&mut self, version: u64, new_operations: Vec) { - // ensure that we are applying the versions in order.. - assert_eq!(version, self.base_version + 1); - self.base_version = version; - self.operations = new_operations; - } + fn update_version(&mut self, version: u64, new_operations: Vec); /// Record the outstanding operations as synced to the server in the given version. - pub(crate) fn local_operations_synced(&mut self, version: u64) { - assert_eq!(version, self.base_version + 1); - self.base_version = version; - self.operations = vec![]; - } + fn local_operations_synced(&mut self, version: u64); } diff --git a/tests/operation_transform_invariant.rs b/tests/operation_transform_invariant.rs index f76850891..4a588d707 100644 --- a/tests/operation_transform_invariant.rs +++ b/tests/operation_transform_invariant.rs @@ -1,8 +1,12 @@ use chrono::Utc; use proptest::prelude::*; -use taskwarrior_rust::{Operation, DB}; +use taskwarrior_rust::{taskstorage, Operation, DB}; use uuid::Uuid; +fn newdb() -> DB { + DB::new(Box::new(taskstorage::InMemoryStorage::new())) +} + fn uuid_strategy() -> impl Strategy { prop_oneof![ Just(Uuid::parse_str("83a2f9ef-f455-4195-b92e-a54c161eebfc").unwrap()), @@ -37,27 +41,30 @@ proptest! { fn transform_invariant_holds(o1 in operation_strategy(), o2 in operation_strategy()) { let (o1p, o2p) = Operation::transform(o1.clone(), o2.clone()); - let mut db1 = DB::new(); + let mut db1 = newdb(); + let mut db2 = newdb(); // Ensure that any expected tasks already exist if let Operation::Update{ ref uuid, .. } = o1 { let _ = db1.apply(Operation::Create{uuid: uuid.clone()}); + let _ = db2.apply(Operation::Create{uuid: uuid.clone()}); } if let Operation::Update{ ref uuid, .. } = o2 { let _ = db1.apply(Operation::Create{uuid: uuid.clone()}); + let _ = db2.apply(Operation::Create{uuid: uuid.clone()}); } if let Operation::Delete{ ref uuid } = o1 { let _ = db1.apply(Operation::Create{uuid: uuid.clone()}); + let _ = db2.apply(Operation::Create{uuid: uuid.clone()}); } if let Operation::Delete{ ref uuid } = o2 { let _ = db1.apply(Operation::Create{uuid: uuid.clone()}); + let _ = db2.apply(Operation::Create{uuid: uuid.clone()}); } - let mut db2 = db1.clone(); - // if applying the initial operations fail, that indicates the operation was invalid // in the base state, so consider the case successful. if let Err(_) = db1.apply(o1) { diff --git a/tests/sync.rs b/tests/sync.rs index aed8bbc47..8c684b4f9 100644 --- a/tests/sync.rs +++ b/tests/sync.rs @@ -1,15 +1,19 @@ use chrono::Utc; -use taskwarrior_rust::{Operation, Server, DB}; +use taskwarrior_rust::{taskstorage, Operation, Server, DB}; use uuid::Uuid; +fn newdb() -> DB { + DB::new(Box::new(taskstorage::InMemoryStorage::new())) +} + #[test] fn test_sync() { let mut server = Server::new(); - let mut db1 = DB::new(); + let mut db1 = newdb(); db1.sync("me", &mut server); - let mut db2 = DB::new(); + let mut db2 = newdb(); db2.sync("me", &mut server); // make some changes in parallel to db1 and db2.. @@ -66,10 +70,10 @@ fn test_sync() { fn test_sync_create_delete() { let mut server = Server::new(); - let mut db1 = DB::new(); + let mut db1 = newdb(); db1.sync("me", &mut server); - let mut db2 = DB::new(); + let mut db2 = newdb(); db2.sync("me", &mut server); // create and update a task.. diff --git a/tests/sync_action_sequences.rs b/tests/sync_action_sequences.rs index 7a92d6e89..4f18976e2 100644 --- a/tests/sync_action_sequences.rs +++ b/tests/sync_action_sequences.rs @@ -1,8 +1,12 @@ use chrono::Utc; use proptest::prelude::*; -use taskwarrior_rust::{Operation, Server, DB}; +use taskwarrior_rust::{taskstorage, Operation, Server, DB}; use uuid::Uuid; +fn newdb() -> DB { + DB::new(Box::new(taskstorage::InMemoryStorage::new())) +} + #[derive(Debug)] enum Action { Op(Operation), @@ -43,7 +47,7 @@ proptest! { // another. So, the generated sequences focus on a single task UUID. fn transform_sequences_of_operations(action_sequence in action_sequence_strategy()) { let mut server = Server::new(); - let mut dbs = [DB::new(), DB::new(), DB::new()]; + let mut dbs = [newdb(), newdb(), newdb()]; for (action, db) in action_sequence { println!("{:?} on db {}", action, db);