Refactor core into a type with methods

This introduces a `Server` type that tracks the config and storage.

server/src/api/add_snapshot.rs

@@ -2,7 +2,7 @@ use crate::api::{client_id_header, failure_to_ise, ServerState, SNAPSHOT_CONTENT
 use actix_web::{error, post, web, HttpMessage, HttpRequest, HttpResponse, Result};
 use futures::StreamExt;
 use std::sync::Arc;
-use taskchampion_sync_server_core::{add_snapshot, VersionId, NIL_VERSION_ID};
+use taskchampion_sync_server_core::{VersionId, NIL_VERSION_ID};
 
 /// Max snapshot size: 100MB
 const MAX_SIZE: usize = 100 * 1024 * 1024;
@@ -49,27 +49,24 @@ pub(crate) async fn service(
     // note that we do not open the transaction until the body has been read
     // completely, to avoid blocking other storage access while that data is
     // in transit.
-    let mut txn = server_state.storage.txn().map_err(failure_to_ise)?;
+    let client = {
+        let mut txn = server_state.server.txn().map_err(failure_to_ise)?;
 
-    // get, or create, the client
-    let client = match txn.get_client(client_id).map_err(failure_to_ise)? {
-        Some(client) => client,
-        None => {
-            txn.new_client(client_id, NIL_VERSION_ID)
-                .map_err(failure_to_ise)?;
-            txn.get_client(client_id).map_err(failure_to_ise)?.unwrap()
+        // get, or create, the client
+        match txn.get_client(client_id).map_err(failure_to_ise)? {
+            Some(client) => client,
+            None => {
+                txn.new_client(client_id, NIL_VERSION_ID)
+                    .map_err(failure_to_ise)?;
+                txn.get_client(client_id).map_err(failure_to_ise)?.unwrap()
+            }
         }
     };
 
-    add_snapshot(
-        txn,
-        &server_state.config,
-        client_id,
-        client,
-        version_id,
-        body.to_vec(),
-    )
-    .map_err(failure_to_ise)?;
+    server_state
+        .server
+        .add_snapshot(client_id, client, version_id, body.to_vec())
+        .map_err(failure_to_ise)?;
     Ok(HttpResponse::Ok().body(""))
 }
 
@@ -77,7 +74,7 @@ pub(crate) async fn service(
 mod test {
     use super::*;
     use crate::api::CLIENT_ID_HEADER;
-    use crate::Server;
+    use crate::WebServer;
     use actix_web::{http::StatusCode, test, App};
     use pretty_assertions::assert_eq;
     use taskchampion_sync_server_core::{InMemoryStorage, Storage};
@@ -87,7 +84,7 @@ mod test {
     async fn test_success() -> anyhow::Result<()> {
         let client_id = Uuid::new_v4();
         let version_id = Uuid::new_v4();
-        let storage: Box<dyn Storage> = Box::new(InMemoryStorage::new());
+        let storage = InMemoryStorage::new();
 
         // set up the storage contents..
         {
@@ -96,7 +93,7 @@ mod test {
             txn.add_version(client_id, version_id, NIL_VERSION_ID, vec![])?;
         }
 
-        let server = Server::new(Default::default(), storage);
+        let server = WebServer::new(Default::default(), storage);
         let app = App::new().configure(|sc| server.config(sc));
         let app = test::init_service(app).await;
 
@@ -130,7 +127,7 @@ mod test {
     async fn test_not_added_200() {
         let client_id = Uuid::new_v4();
         let version_id = Uuid::new_v4();
-        let storage: Box<dyn Storage> = Box::new(InMemoryStorage::new());
+        let storage = InMemoryStorage::new();
 
         // set up the storage contents..
         {
@@ -138,7 +135,7 @@ mod test {
             txn.new_client(client_id, NIL_VERSION_ID).unwrap();
         }
 
-        let server = Server::new(Default::default(), storage);
+        let server = WebServer::new(Default::default(), storage);
         let app = App::new().configure(|sc| server.config(sc));
         let app = test::init_service(app).await;
 
@@ -167,8 +164,8 @@ mod test {
     async fn test_bad_content_type() {
         let client_id = Uuid::new_v4();
         let version_id = Uuid::new_v4();
-        let storage: Box<dyn Storage> = Box::new(InMemoryStorage::new());
-        let server = Server::new(Default::default(), storage);
+        let storage = InMemoryStorage::new();
+        let server = WebServer::new(Default::default(), storage);
         let app = App::new().configure(|sc| server.config(sc));
         let app = test::init_service(app).await;
 
@@ -187,8 +184,8 @@ mod test {
     async fn test_empty_body() {
         let client_id = Uuid::new_v4();
         let version_id = Uuid::new_v4();
-        let storage: Box<dyn Storage> = Box::new(InMemoryStorage::new());
-        let server = Server::new(Default::default(), storage);
+        let storage = InMemoryStorage::new();
+        let server = WebServer::new(Default::default(), storage);
         let app = App::new().configure(|sc| server.config(sc));
         let app = test::init_service(app).await;
 

server/src/api/add_version.rs

@@ -5,9 +5,7 @@ use crate::api::{
 use actix_web::{error, post, web, HttpMessage, HttpRequest, HttpResponse, Result};
 use futures::StreamExt;
 use std::sync::Arc;
-use taskchampion_sync_server_core::{
-    add_version, AddVersionResult, SnapshotUrgency, VersionId, NIL_VERSION_ID,
-};
+use taskchampion_sync_server_core::{AddVersionResult, SnapshotUrgency, VersionId, NIL_VERSION_ID};
 
 /// Max history segment size: 100MB
 const MAX_SIZE: usize = 100 * 1024 * 1024;
@@ -59,27 +57,23 @@ pub(crate) async fn service(
     // note that we do not open the transaction until the body has been read
     // completely, to avoid blocking other storage access while that data is
     // in transit.
-    let mut txn = server_state.storage.txn().map_err(failure_to_ise)?;
+    let client = {
+        let mut txn = server_state.server.txn().map_err(failure_to_ise)?;
 
-    // get, or create, the client
-    let client = match txn.get_client(client_id).map_err(failure_to_ise)? {
-        Some(client) => client,
-        None => {
-            txn.new_client(client_id, NIL_VERSION_ID)
-                .map_err(failure_to_ise)?;
-            txn.get_client(client_id).map_err(failure_to_ise)?.unwrap()
+        match txn.get_client(client_id).map_err(failure_to_ise)? {
+            Some(client) => client,
+            None => {
+                txn.new_client(client_id, NIL_VERSION_ID)
+                    .map_err(failure_to_ise)?;
+                txn.get_client(client_id).map_err(failure_to_ise)?.unwrap()
+            }
         }
     };
 
-    let (result, snap_urgency) = add_version(
-        txn,
-        &server_state.config,
-        client_id,
-        client,
-        parent_version_id,
-        body.to_vec(),
-    )
-    .map_err(failure_to_ise)?;
+    let (result, snap_urgency) = server_state
+        .server
+        .add_version(client_id, client, parent_version_id, body.to_vec())
+        .map_err(failure_to_ise)?;
 
     Ok(match result {
         AddVersionResult::Ok(version_id) => {
@@ -107,7 +101,7 @@ pub(crate) async fn service(
 #[cfg(test)]
 mod test {
     use crate::api::CLIENT_ID_HEADER;
-    use crate::Server;
+    use crate::WebServer;
     use actix_web::{http::StatusCode, test, App};
     use pretty_assertions::assert_eq;
     use taskchampion_sync_server_core::{InMemoryStorage, Storage};
@@ -118,7 +112,7 @@ mod test {
         let client_id = Uuid::new_v4();
         let version_id = Uuid::new_v4();
         let parent_version_id = Uuid::new_v4();
-        let storage: Box<dyn Storage> = Box::new(InMemoryStorage::new());
+        let storage = InMemoryStorage::new();
 
         // set up the storage contents..
         {
@@ -126,7 +120,7 @@ mod test {
             txn.new_client(client_id, Uuid::nil()).unwrap();
         }
 
-        let server = Server::new(Default::default(), storage);
+        let server = WebServer::new(Default::default(), storage);
         let app = App::new().configure(|sc| server.config(sc));
         let app = test::init_service(app).await;
 
@@ -160,7 +154,7 @@ mod test {
         let client_id = Uuid::new_v4();
         let version_id = Uuid::new_v4();
         let parent_version_id = Uuid::new_v4();
-        let storage: Box<dyn Storage> = Box::new(InMemoryStorage::new());
+        let storage = InMemoryStorage::new();
 
         // set up the storage contents..
         {
@@ -168,7 +162,7 @@ mod test {
             txn.new_client(client_id, version_id).unwrap();
         }
 
-        let server = Server::new(Default::default(), storage);
+        let server = WebServer::new(Default::default(), storage);
         let app = App::new().configure(|sc| server.config(sc));
         let app = test::init_service(app).await;
 
@@ -195,8 +189,8 @@ mod test {
     async fn test_bad_content_type() {
         let client_id = Uuid::new_v4();
         let parent_version_id = Uuid::new_v4();
-        let storage: Box<dyn Storage> = Box::new(InMemoryStorage::new());
-        let server = Server::new(Default::default(), storage);
+        let storage = InMemoryStorage::new();
+        let server = WebServer::new(Default::default(), storage);
         let app = App::new().configure(|sc| server.config(sc));
         let app = test::init_service(app).await;
 
@@ -215,8 +209,8 @@ mod test {
     async fn test_empty_body() {
         let client_id = Uuid::new_v4();
         let parent_version_id = Uuid::new_v4();
-        let storage: Box<dyn Storage> = Box::new(InMemoryStorage::new());
-        let server = Server::new(Default::default(), storage);
+        let storage = InMemoryStorage::new();
+        let server = WebServer::new(Default::default(), storage);
         let app = App::new().configure(|sc| server.config(sc));
         let app = test::init_service(app).await;
 

server/src/api/get_child_version.rs

@@ -4,7 +4,7 @@ use crate::api::{
 };
 use actix_web::{error, get, web, HttpRequest, HttpResponse, Result};
 use std::sync::Arc;
-use taskchampion_sync_server_core::{get_child_version, GetVersionResult, VersionId};
+use taskchampion_sync_server_core::{GetVersionResult, VersionId};
 
 /// Get a child version.
 ///
@@ -22,23 +22,22 @@ pub(crate) async fn service(
 ) -> Result<HttpResponse> {
     let parent_version_id = path.into_inner();
 
-    let mut txn = server_state.storage.txn().map_err(failure_to_ise)?;
+    let (client, client_id) = {
+        let mut txn = server_state.server.txn().map_err(failure_to_ise)?;
 
-    let client_id = client_id_header(&req)?;
+        let client_id = client_id_header(&req)?;
 
-    let client = txn
-        .get_client(client_id)
+        let client = txn
+            .get_client(client_id)
+            .map_err(failure_to_ise)?
+            .ok_or_else(|| error::ErrorNotFound("no such client"))?;
+        (client, client_id)
+    };
+
+    return match server_state
+        .server
+        .get_child_version(client_id, client, parent_version_id)
         .map_err(failure_to_ise)?
-        .ok_or_else(|| error::ErrorNotFound("no such client"))?;
-
-    return match get_child_version(
-        txn,
-        &server_state.config,
-        client_id,
-        client,
-        parent_version_id,
-    )
-    .map_err(failure_to_ise)?
     {
         GetVersionResult::Success {
             version_id,
@@ -57,7 +56,7 @@ pub(crate) async fn service(
 #[cfg(test)]
 mod test {
     use crate::api::CLIENT_ID_HEADER;
-    use crate::Server;
+    use crate::WebServer;
     use actix_web::{http::StatusCode, test, App};
     use pretty_assertions::assert_eq;
     use taskchampion_sync_server_core::{InMemoryStorage, Storage, NIL_VERSION_ID};
@@ -68,7 +67,7 @@ mod test {
         let client_id = Uuid::new_v4();
         let version_id = Uuid::new_v4();
         let parent_version_id = Uuid::new_v4();
-        let storage: Box<dyn Storage> = Box::new(InMemoryStorage::new());
+        let storage = InMemoryStorage::new();
 
         // set up the storage contents..
         {
@@ -78,7 +77,7 @@ mod test {
                 .unwrap();
         }
 
-        let server = Server::new(Default::default(), storage);
+        let server = WebServer::new(Default::default(), storage);
         let app = App::new().configure(|sc| server.config(sc));
         let app = test::init_service(app).await;
 
@@ -111,8 +110,8 @@ mod test {
     async fn test_client_not_found() {
         let client_id = Uuid::new_v4();
         let parent_version_id = Uuid::new_v4();
-        let storage: Box<dyn Storage> = Box::new(InMemoryStorage::new());
-        let server = Server::new(Default::default(), storage);
+        let storage = InMemoryStorage::new();
+        let server = WebServer::new(Default::default(), storage);
         let app = App::new().configure(|sc| server.config(sc));
         let app = test::init_service(app).await;
 
@@ -131,7 +130,7 @@ mod test {
     async fn test_version_not_found_and_gone() {
         let client_id = Uuid::new_v4();
         let test_version_id = Uuid::new_v4();
-        let storage: Box<dyn Storage> = Box::new(InMemoryStorage::new());
+        let storage = InMemoryStorage::new();
 
         // create the client and a single version.
         {
@@ -140,7 +139,7 @@ mod test {
             txn.add_version(client_id, test_version_id, NIL_VERSION_ID, b"vers".to_vec())
                 .unwrap();
         }
-        let server = Server::new(Default::default(), storage);
+        let server = WebServer::new(Default::default(), storage);
         let app = App::new().configure(|sc| server.config(sc));
         let app = test::init_service(app).await;
 

server/src/api/get_snapshot.rs

@@ -3,7 +3,6 @@ use crate::api::{
 };
 use actix_web::{error, get, web, HttpRequest, HttpResponse, Result};
 use std::sync::Arc;
-use taskchampion_sync_server_core::get_snapshot;
 
 /// Get a snapshot.
 ///
@@ -18,17 +17,19 @@ pub(crate) async fn service(
     req: HttpRequest,
     server_state: web::Data<Arc<ServerState>>,
 ) -> Result<HttpResponse> {
-    let mut txn = server_state.storage.txn().map_err(failure_to_ise)?;
-
     let client_id = client_id_header(&req)?;
 
-    let client = txn
-        .get_client(client_id)
-        .map_err(failure_to_ise)?
-        .ok_or_else(|| error::ErrorNotFound("no such client"))?;
+    let client = {
+        let mut txn = server_state.server.txn().map_err(failure_to_ise)?;
+        txn.get_client(client_id)
+            .map_err(failure_to_ise)?
+            .ok_or_else(|| error::ErrorNotFound("no such client"))?
+    };
 
-    if let Some((version_id, data)) =
-        get_snapshot(txn, &server_state.config, client_id, client).map_err(failure_to_ise)?
+    if let Some((version_id, data)) = server_state
+        .server
+        .get_snapshot(client_id, client)
+        .map_err(failure_to_ise)?
     {
         Ok(HttpResponse::Ok()
             .content_type(SNAPSHOT_CONTENT_TYPE)
@@ -42,7 +43,7 @@ pub(crate) async fn service(
 #[cfg(test)]
 mod test {
     use crate::api::CLIENT_ID_HEADER;
-    use crate::Server;
+    use crate::WebServer;
     use actix_web::{http::StatusCode, test, App};
     use chrono::{TimeZone, Utc};
     use pretty_assertions::assert_eq;
@@ -52,7 +53,7 @@ mod test {
     #[actix_rt::test]
     async fn test_not_found() {
         let client_id = Uuid::new_v4();
-        let storage: Box<dyn Storage> = Box::new(InMemoryStorage::new());
+        let storage = InMemoryStorage::new();
 
         // set up the storage contents..
         {
@@ -60,7 +61,7 @@ mod test {
             txn.new_client(client_id, Uuid::new_v4()).unwrap();
         }
 
-        let server = Server::new(Default::default(), storage);
+        let server = WebServer::new(Default::default(), storage);
         let app = App::new().configure(|sc| server.config(sc));
         let app = test::init_service(app).await;
 
@@ -78,7 +79,7 @@ mod test {
         let client_id = Uuid::new_v4();
         let version_id = Uuid::new_v4();
         let snapshot_data = vec![1, 2, 3, 4];
-        let storage: Box<dyn Storage> = Box::new(InMemoryStorage::new());
+        let storage = InMemoryStorage::new();
 
         // set up the storage contents..
         {
@@ -96,7 +97,7 @@ mod test {
             .unwrap();
         }
 
-        let server = Server::new(Default::default(), storage);
+        let server = WebServer::new(Default::default(), storage);
         let app = App::new().configure(|sc| server.config(sc));
         let app = test::init_service(app).await;
 

server/src/api/mod.rs

@@ -1,5 +1,5 @@
 use actix_web::{error, http::StatusCode, web, HttpRequest, Result, Scope};
-use taskchampion_sync_server_core::{ClientId, ServerConfig, Storage};
+use taskchampion_sync_server_core::{ClientId, Server};
 
 mod add_snapshot;
 mod add_version;
@@ -27,8 +27,7 @@ pub(crate) const SNAPSHOT_REQUEST_HEADER: &str = "X-Snapshot-Request";
 
 /// The type containing a reference to the persistent state for the server
 pub(crate) struct ServerState {
-    pub(crate) storage: Box<dyn Storage>,
-    pub(crate) config: ServerConfig,
+    pub(crate) server: Server,
 }
 
 pub(crate) fn api_scope() -> Scope {

server/src/bin/taskchampion-sync-server.rs

@@ -3,7 +3,7 @@
 use actix_web::{middleware::Logger, App, HttpServer};
 use clap::{arg, builder::ValueParser, value_parser, Command};
 use std::ffi::OsString;
-use taskchampion_sync_server::Server;
+use taskchampion_sync_server::WebServer;
 use taskchampion_sync_server_core::ServerConfig;
 use taskchampion_sync_server_storage_sqlite::SqliteStorage;
 
@@ -44,8 +44,11 @@ async fn main() -> anyhow::Result<()> {
     let snapshot_versions: u32 = *matches.get_one("snapshot-versions").unwrap();
     let snapshot_days: i64 = *matches.get_one("snapshot-days").unwrap();
 
-    let config = ServerConfig::from_args(snapshot_days, snapshot_versions)?;
-    let server = Server::new(config, Box::new(SqliteStorage::new(data_dir)?));
+    let config = ServerConfig {
+        snapshot_days,
+        snapshot_versions,
+    };
+    let server = WebServer::new(config, SqliteStorage::new(data_dir)?);
 
     log::info!("Serving on port {}", port);
     HttpServer::new(move || {
@@ -67,7 +70,7 @@ mod test {
 
     #[actix_rt::test]
     async fn test_index_get() {
-        let server = Server::new(Default::default(), Box::new(InMemoryStorage::new()));
+        let server = WebServer::new(Default::default(), InMemoryStorage::new());
         let app = App::new().configure(|sc| server.config(sc));
         let app = test::init_service(app).await;
 

server/src/lib.rs

@@ -5,7 +5,7 @@ mod api;
 use actix_web::{get, middleware, web, Responder};
 use api::{api_scope, ServerState};
 use std::sync::Arc;
-use taskchampion_sync_server_core::{ServerConfig, Storage};
+use taskchampion_sync_server_core::{Server, ServerConfig, Storage};
 
 #[get("/")]
 async fn index() -> impl Responder {
@@ -14,15 +14,17 @@ async fn index() -> impl Responder {
 
 /// A Server represents a sync server.
 #[derive(Clone)]
-pub struct Server {
+pub struct WebServer {
     server_state: Arc<ServerState>,
 }
 
-impl Server {
+impl WebServer {
     /// Create a new sync server with the given storage implementation.
-    pub fn new(config: ServerConfig, storage: Box<dyn Storage>) -> Self {
+    pub fn new<ST: Storage + 'static>(config: ServerConfig, storage: ST) -> Self {
         Self {
-            server_state: Arc::new(ServerState { config, storage }),
+            server_state: Arc::new(ServerState {
+                server: Server::new(config, storage),
+            }),
         }
     }
 
@@ -49,7 +51,7 @@ mod test {
 
     #[actix_rt::test]
     async fn test_cache_control() {
-        let server = Server::new(Default::default(), Box::new(InMemoryStorage::new()));
+        let server = WebServer::new(Default::default(), InMemoryStorage::new());
         let app = App::new().configure(|sc| server.config(sc));
         let app = test::init_service(app).await;