Remove unnecessary public items (#512)

The `storage` crate exposed a lot of things it shouldn't, and changing
those things constitutes a breaking change. So, this removes them in one
go, allowing more flexibility in later development.

The following items are no longer available:

 - taskchampion::storage::SqliteStorage
 - taskchampion::storage::InMemoryStorage

src/replica.rs

@@ -51,7 +51,9 @@ impl Replica {
 
     #[cfg(test)]
     pub fn new_inmemory() -> Replica {
-        Replica::new(Box::new(crate::storage::InMemoryStorage::new()))
+        use crate::StorageConfig;
+
+        Replica::new(StorageConfig::InMemory.into_storage().unwrap())
     }
 
     /// Update an existing task.  If the value is Some, the property is added or updated.  If the

src/server/local/mod.rs

@@ -18,7 +18,7 @@ struct Version {
     history_segment: HistorySegment,
 }
 
-pub struct LocalServer {
+pub(crate) struct LocalServer {
     con: rusqlite::Connection,
 }
 
@@ -29,7 +29,7 @@ impl LocalServer {
     }
 
     /// A server which has no notion of clients, signatures, encryption, etc.
-    pub fn new<P: AsRef<Path>>(directory: P) -> Result<LocalServer> {
+    pub(crate) fn new<P: AsRef<Path>>(directory: P) -> Result<LocalServer> {
         let db_file = directory
             .as_ref()
             .join("taskchampion-local-sync-server.sqlite3");

src/server/op.rs

@@ -6,7 +6,7 @@ use uuid::Uuid;
 /// A SyncOp defines a single change to the task database, that can be synchronized
 /// via a server.
 #[derive(PartialEq, Eq, Clone, Debug, Serialize, Deserialize)]
-pub enum SyncOp {
+pub(crate) enum SyncOp {
     /// Create a new task.
     ///
     /// On application, if the task already exists, the operation does nothing.
@@ -54,7 +54,10 @@ impl SyncOp {
     // allows two different systems which have already applied op1 and op2, respectively, and thus
     // reached different states, to return to the same state by applying op2' and op1',
     // respectively.
-    pub fn transform(operation1: SyncOp, operation2: SyncOp) -> (Option<SyncOp>, Option<SyncOp>) {
+    pub(crate) fn transform(
+        operation1: SyncOp,
+        operation2: SyncOp,
+    ) -> (Option<SyncOp>, Option<SyncOp>) {
         match (&operation1, &operation2) {
             // Two creations or deletions of the same uuid reach the same state, so there's no need
             // for any further operations to bring the state together.
@@ -174,9 +177,9 @@ impl SyncOp {
 mod test {
     use super::*;
     use crate::errors::Result;
-    use crate::storage::{InMemoryStorage, TaskMap};
+    use crate::storage::TaskMap;
     use crate::taskdb::TaskDb;
-    use crate::Operations;
+    use crate::{Operations, StorageConfig};
     use chrono::{Duration, Utc};
     use pretty_assertions::assert_eq;
     use proptest::prelude::*;
@@ -433,8 +436,8 @@ mod test {
 
             let mut ops1 = Operations::new();
             let mut ops2 = Operations::new();
-            let mut db1 = TaskDb::new(Box::new(InMemoryStorage::new()));
-            let mut db2 = TaskDb::new(Box::new(InMemoryStorage::new()));
+            let mut db1 = TaskDb::new(StorageConfig::InMemory.into_storage().unwrap());
+            let mut db2 = TaskDb::new(StorageConfig::InMemory.into_storage().unwrap());
 
             // Ensure that any expected tasks already exist
             for o in [&o1, &o2] {

src/server/sync/mod.rs

@@ -9,7 +9,7 @@ use uuid::Uuid;
 
 use super::encryption::{Cryptor, Sealed, Secret, Unsealed};
 
-pub struct SyncServer {
+pub(crate) struct SyncServer {
     base_url: Url,
     client_id: Uuid,
     cryptor: Cryptor,
@@ -32,7 +32,11 @@ impl SyncServer {
     ///
     /// Pass a client_id to identify this client to the server.  Multiple replicas synchronizing the same task history
     /// should use the same client_id.
-    pub fn new(url: String, client_id: Uuid, encryption_secret: Vec<u8>) -> Result<SyncServer> {
+    pub(crate) fn new(
+        url: String,
+        client_id: Uuid,
+        encryption_secret: Vec<u8>,
+    ) -> Result<SyncServer> {
         let url = Url::parse(&url)
             .map_err(|_| Error::Server(format!("Could not parse {} as a URL", url)))?;
         Ok(SyncServer {

src/storage/config.rs

@@ -1,4 +1,4 @@
-use super::{InMemoryStorage, SqliteStorage, Storage};
+use super::{inmemory::InMemoryStorage, sqlite::SqliteStorage, Storage};
 use crate::errors::Result;
 use std::path::PathBuf;
 

src/storage/inmemory.rs

@@ -223,12 +223,12 @@ impl StorageTxn for Txn<'_> {
 /// InMemoryStorage is a simple in-memory task storage implementation.  It is not useful for
 /// production data, but is useful for testing purposes.
 #[derive(PartialEq, Debug, Clone)]
-pub struct InMemoryStorage {
+pub(super) struct InMemoryStorage {
     data: Data,
 }
 
 impl InMemoryStorage {
-    pub fn new() -> InMemoryStorage {
+    pub(super) fn new() -> InMemoryStorage {
         InMemoryStorage {
             data: Data {
                 tasks: HashMap::new(),

src/storage/mod.rs

@@ -21,9 +21,7 @@ mod config;
 mod inmemory;
 pub(crate) mod sqlite;
 
-pub use config::{AccessMode, StorageConfig};
-pub use inmemory::InMemoryStorage;
-pub use sqlite::SqliteStorage;
+pub use config::StorageConfig;
 
 #[doc(hidden)]
 /// For compatibility with 0.6 and earlier, [`Operation`] is re-exported here.
@@ -42,10 +40,10 @@ pub(crate) fn taskmap_with(mut properties: Vec<(String, String)>) -> TaskMap {
 }
 
 /// The type of VersionIds
-pub use crate::server::VersionId;
+use crate::server::VersionId;
 
-/// The default for base_version.
-pub(crate) const DEFAULT_BASE_VERSION: Uuid = crate::server::NIL_VERSION_ID;
+/// The default for base_version, if none exists in the DB.
+const DEFAULT_BASE_VERSION: Uuid = crate::server::NIL_VERSION_ID;
 
 /// A Storage transaction, in which storage operations are performed.
 ///

src/storage/sqlite.rs

@@ -1,6 +1,7 @@
 use crate::errors::{Error, Result};
 use crate::operation::Operation;
-use crate::storage::{AccessMode, Storage, StorageTxn, TaskMap, VersionId, DEFAULT_BASE_VERSION};
+use crate::storage::config::AccessMode;
+use crate::storage::{Storage, StorageTxn, TaskMap, VersionId, DEFAULT_BASE_VERSION};
 use anyhow::Context;
 use rusqlite::types::{FromSql, ToSql};
 use rusqlite::{params, Connection, OpenFlags, OptionalExtension, TransactionBehavior};
@@ -10,7 +11,7 @@ use uuid::Uuid;
 mod schema;
 
 #[derive(Debug, thiserror::Error, PartialEq, Eq)]
-pub enum SqliteError {
+pub(crate) enum SqliteError {
     #[error("SQLite transaction already committted")]
     TransactionAlreadyCommitted,
     #[error("Task storage was opened in read-only mode")]
@@ -77,13 +78,13 @@ impl ToSql for Operation {
 }
 
 /// SqliteStorage is an on-disk storage backed by SQLite3.
-pub struct SqliteStorage {
+pub(super) struct SqliteStorage {
     con: Connection,
     access_mode: AccessMode,
 }
 
 impl SqliteStorage {
-    pub fn new<P: AsRef<Path>>(
+    pub(super) fn new<P: AsRef<Path>>(
         directory: P,
         access_mode: AccessMode,
         create_if_missing: bool,

src/taskdb/mod.rs

@@ -28,10 +28,8 @@ impl TaskDb {
 
     #[cfg(test)]
     pub(crate) fn new_inmemory() -> TaskDb {
-        #[cfg(test)]
-        use crate::storage::InMemoryStorage;
-
-        TaskDb::new(Box::new(InMemoryStorage::new()))
+        use crate::StorageConfig;
+        TaskDb::new(StorageConfig::InMemory.into_storage().unwrap())
     }
 
     /// Apply `operations` to the database in a single transaction.

src/taskdb/snapshot.rs

@@ -1,5 +1,6 @@
 use crate::errors::{Error, Result};
-use crate::storage::{StorageTxn, TaskMap, VersionId};
+use crate::server::VersionId;
+use crate::storage::{StorageTxn, TaskMap};
 use flate2::{read::ZlibDecoder, write::ZlibEncoder, Compression};
 use serde::de::{Deserialize, Deserializer, MapAccess, Visitor};
 use serde::ser::{Serialize, SerializeMap, Serializer};
@@ -104,7 +105,7 @@ pub(super) fn apply_snapshot(
 #[cfg(test)]
 mod test {
     use super::*;
-    use crate::storage::{InMemoryStorage, Storage, TaskMap};
+    use crate::{storage::TaskMap, StorageConfig};
     use pretty_assertions::assert_eq;
 
     #[test]
@@ -130,7 +131,7 @@ mod test {
 
     #[test]
     fn test_round_trip() -> Result<()> {
-        let mut storage = InMemoryStorage::new();
+        let mut storage = StorageConfig::InMemory.into_storage().unwrap();
         let version = Uuid::new_v4();
 
         let task1 = (
@@ -159,7 +160,7 @@ mod test {
         };
 
         // apply that snapshot to a fresh bit of fake
-        let mut storage = InMemoryStorage::new();
+        let mut storage = StorageConfig::InMemory.into_storage().unwrap();
         {
             let mut txn = storage.txn()?;
             apply_snapshot(txn.as_mut(), version, &snap)?;

src/taskdb/sync.rs

@@ -220,17 +220,14 @@ fn apply_version(
 mod test {
     use super::*;
     use crate::server::test::TestServer;
-    use crate::storage::{InMemoryStorage, TaskMap};
-    use crate::taskdb::{snapshot::SnapshotTasks, TaskDb};
+    use crate::storage::TaskMap;
+    use crate::taskdb::snapshot::SnapshotTasks;
+    use crate::taskdb::TaskDb;
     use crate::{Operation, Operations};
     use chrono::Utc;
     use pretty_assertions::assert_eq;
     use uuid::Uuid;
 
-    fn newdb() -> TaskDb {
-        TaskDb::new(Box::new(InMemoryStorage::new()))
-    }
-
     fn expect_operations(mut got: Vec<Operation>, mut exp: Vec<Operation>) {
         got.sort();
         exp.sort();
@@ -241,10 +238,10 @@ mod test {
     fn test_sync() -> Result<()> {
         let mut server: Box<dyn Server> = TestServer::new().server();
 
-        let mut db1 = newdb();
+        let mut db1 = TaskDb::new_inmemory();
         sync(&mut server, db1.storage.txn()?.as_mut(), false).unwrap();
 
-        let mut db2 = newdb();
+        let mut db2 = TaskDb::new_inmemory();
         sync(&mut server, db2.storage.txn()?.as_mut(), false).unwrap();
 
         // make some changes in parallel to db1 and db2..
@@ -357,10 +354,10 @@ mod test {
     fn test_sync_create_delete() -> Result<()> {
         let mut server: Box<dyn Server> = TestServer::new().server();
 
-        let mut db1 = newdb();
+        let mut db1 = TaskDb::new_inmemory();
         sync(&mut server, db1.storage.txn()?.as_mut(), false).unwrap();
 
-        let mut db2 = newdb();
+        let mut db2 = TaskDb::new_inmemory();
         sync(&mut server, db2.storage.txn()?.as_mut(), false).unwrap();
 
         // create and update a task..
@@ -485,10 +482,10 @@ mod test {
     fn test_sync_conflicting_updates() -> Result<()> {
         let mut server: Box<dyn Server> = TestServer::new().server();
 
-        let mut db1 = newdb();
+        let mut db1 = TaskDb::new_inmemory();
         sync(&mut server, db1.storage.txn()?.as_mut(), false).unwrap();
 
-        let mut db2 = newdb();
+        let mut db2 = TaskDb::new_inmemory();
         sync(&mut server, db2.storage.txn()?.as_mut(), false).unwrap();
 
         // create and update a task..
@@ -597,7 +594,7 @@ mod test {
         let mut test_server = TestServer::new();
 
         let mut server: Box<dyn Server> = test_server.server();
-        let mut db1 = newdb();
+        let mut db1 = TaskDb::new_inmemory();
 
         let uuid = Uuid::new_v4();
         let mut ops = Operations::new();
@@ -641,7 +638,7 @@ mod test {
         test_server.delete_version(Uuid::nil());
 
         // sync to a new DB and check that we got the expected results
-        let mut db2 = newdb();
+        let mut db2 = TaskDb::new_inmemory();
         sync(&mut server, db2.storage.txn()?.as_mut(), false)?;
 
         let task = db2.get_task(uuid)?.unwrap();
@@ -655,7 +652,7 @@ mod test {
         let test_server = TestServer::new();
 
         let mut server: Box<dyn Server> = test_server.server();
-        let mut db1 = newdb();
+        let mut db1 = TaskDb::new_inmemory();
 
         let uuid = Uuid::new_v4();
         let mut ops = Operations::new();
@@ -678,7 +675,7 @@ mod test {
 
         let mut server: Box<dyn Server> = test_server.server();
 
-        let mut db = newdb();
+        let mut db = TaskDb::new_inmemory();
         sync(&mut server, db.storage.txn()?.as_mut(), false).unwrap();
 
         // add a task to db
@@ -726,7 +723,7 @@ mod test {
 
         let mut server: Box<dyn Server> = test_server.server();
 
-        let mut db = newdb();
+        let mut db = TaskDb::new_inmemory();
         sync(&mut server, db.storage.txn()?.as_mut(), false).unwrap();
 
         // add a task to db