From 74b04e027de1ef57e9140134ab6645f20d5b1fd5 Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Tue, 24 Dec 2024 20:29:53 -0500 Subject: [PATCH] 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 | 4 +++- src/server/local/mod.rs | 4 ++-- src/server/op.rs | 15 +++++++++------ src/server/sync/mod.rs | 8 ++++++-- src/storage/config.rs | 2 +- src/storage/inmemory.rs | 4 ++-- src/storage/mod.rs | 10 ++++------ src/storage/sqlite.rs | 9 +++++---- src/taskdb/mod.rs | 6 ++---- src/taskdb/snapshot.rs | 9 +++++---- src/taskdb/sync.rs | 31 ++++++++++++++----------------- 11 files changed, 53 insertions(+), 49 deletions(-) diff --git a/src/replica.rs b/src/replica.rs index 225856711..2742361d7 100644 --- a/src/replica.rs +++ b/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 diff --git a/src/server/local/mod.rs b/src/server/local/mod.rs index a5a4d2f98..3c3e6bd18 100644 --- a/src/server/local/mod.rs +++ b/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>(directory: P) -> Result { + pub(crate) fn new>(directory: P) -> Result { let db_file = directory .as_ref() .join("taskchampion-local-sync-server.sqlite3"); diff --git a/src/server/op.rs b/src/server/op.rs index edd9bd467..b2fa4d32e 100644 --- a/src/server/op.rs +++ b/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, Option) { + pub(crate) fn transform( + operation1: SyncOp, + operation2: SyncOp, + ) -> (Option, Option) { 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] { diff --git a/src/server/sync/mod.rs b/src/server/sync/mod.rs index e54ce10bd..dfde64d70 100644 --- a/src/server/sync/mod.rs +++ b/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) -> Result { + pub(crate) fn new( + url: String, + client_id: Uuid, + encryption_secret: Vec, + ) -> Result { let url = Url::parse(&url) .map_err(|_| Error::Server(format!("Could not parse {} as a URL", url)))?; Ok(SyncServer { diff --git a/src/storage/config.rs b/src/storage/config.rs index 4e3be0d4d..44671f7d6 100644 --- a/src/storage/config.rs +++ b/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; diff --git a/src/storage/inmemory.rs b/src/storage/inmemory.rs index d26a480de..6681118f4 100644 --- a/src/storage/inmemory.rs +++ b/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(), diff --git a/src/storage/mod.rs b/src/storage/mod.rs index 09274e127..2bf57844a 100644 --- a/src/storage/mod.rs +++ b/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. /// diff --git a/src/storage/sqlite.rs b/src/storage/sqlite.rs index f7ee14cd9..617a8aa19 100644 --- a/src/storage/sqlite.rs +++ b/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>( + pub(super) fn new>( directory: P, access_mode: AccessMode, create_if_missing: bool, diff --git a/src/taskdb/mod.rs b/src/taskdb/mod.rs index 23d6c3d32..c43561fab 100644 --- a/src/taskdb/mod.rs +++ b/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. diff --git a/src/taskdb/snapshot.rs b/src/taskdb/snapshot.rs index 176da28e5..074022237 100644 --- a/src/taskdb/snapshot.rs +++ b/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)?; diff --git a/src/taskdb/sync.rs b/src/taskdb/sync.rs index f14feadef..9a6b9079e 100644 --- a/src/taskdb/sync.rs +++ b/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, mut exp: Vec) { got.sort(); exp.sort(); @@ -241,10 +238,10 @@ mod test { fn test_sync() -> Result<()> { let mut server: Box = 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 = 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 = 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 = 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 = 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 = 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 = 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