Reorganize the core API (#60)

Cargo.lock

@@ -1424,6 +1424,7 @@ dependencies = [
  "env_logger",
  "log",
  "pretty_assertions",
+ "thiserror",
  "uuid",
 ]
 

core/Cargo.toml

@@ -9,6 +9,7 @@ license = "MIT"
 [dependencies]
 uuid.workspace = true
 anyhow.workspace = true
+thiserror.workspace = true
 log.workspace = true
 env_logger.workspace = true
 chrono.workspace = true

core/src/error.rs

@@ -0,0 +1,13 @@
+/// An error from the [`crate::Server`] type.
+///
+/// This type represents only circumstances outside the realm of the protocol, and not the specific
+/// results descriebd in the protocol documentation.
+#[derive(Debug, thiserror::Error)]
+pub enum ServerError {
+    /// There is no client with the given ClientId.
+    #[error("No such client")]
+    NoSuchClient,
+
+    #[error(transparent)]
+    Other(#[from] anyhow::Error),
+}

core/src/lib.rs

@@ -7,26 +7,17 @@
 //! This crate uses an abstract storage backend. Note that this does not implement the
 //! HTTP-specific portions of the protocol, nor provide any storage implementations.
 //!
-//! ## API Methods
+//! ## Usage
 //!
-//! The following API methods are implemented. These methods are documented in more detail in
+//! To use, create a new [`Server`] instance and call the relevant protocol API methods. The
-//! the protocol documentation.
+//! arguments and return values correspond closely to the protocol documentation.
-//!
-//! * [`add_version`]
-//! * [`get_child_version`]
-//! * [`add_snapshot`]
-//! * [`get_snapshot`]
-//!
-//! Each API method takes:
-//!
-//! * [`StorageTxn`] to access storage. Methods which modify storage will commit the transaction before returning.
-//! * [`ServerConfig`] providing basic configuration for the server's behavior.
-//! * `client_id` and a [`Client`] providing the client metadata.
 
+mod error;
 mod inmemory;
 mod server;
 mod storage;
 
+pub use error::*;
 pub use inmemory::*;
 pub use server::*;
 pub use storage::*;

server/src/api/add_snapshot.rs

@@ -1,8 +1,8 @@
-use crate::api::{client_id_header, failure_to_ise, ServerState, SNAPSHOT_CONTENT_TYPE};
+use crate::api::{client_id_header, server_error_to_actix, ServerState, SNAPSHOT_CONTENT_TYPE};
 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;
 
 /// Max snapshot size: 100MB
 const MAX_SIZE: usize = 100 * 1024 * 1024;
@@ -46,48 +46,27 @@ pub(crate) async fn service(
         return Err(error::ErrorBadRequest("No snapshot supplied"));
     }
 
-    // note that we do not open the transaction until the body has been read
+    server_state
-    // completely, to avoid blocking other storage access while that data is
+        .server
-    // in transit.
+        .add_snapshot(client_id, version_id, body.to_vec())
-    let mut txn = server_state.storage.txn().map_err(failure_to_ise)?;
+        .map_err(server_error_to_actix)?;
-
-    // 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()
-        }
-    };
-
-    add_snapshot(
-        txn,
-        &server_state.config,
-        client_id,
-        client,
-        version_id,
-        body.to_vec(),
-    )
-    .map_err(failure_to_ise)?;
     Ok(HttpResponse::Ok().body(""))
 }
 
 #[cfg(test)]
 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};
+    use taskchampion_sync_server_core::{InMemoryStorage, Storage, NIL_VERSION_ID};
     use uuid::Uuid;
 
     #[actix_rt::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 +75,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 +109,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 +117,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 +146,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 storage = InMemoryStorage::new();
-        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;
 
@@ -187,8 +166,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 storage = InMemoryStorage::new();
-        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/add_version.rs

@@ -1,12 +1,13 @@
 use crate::api::{
-    client_id_header, failure_to_ise, ServerState, HISTORY_SEGMENT_CONTENT_TYPE,
+    client_id_header, failure_to_ise, server_error_to_actix, ServerState,
-    PARENT_VERSION_ID_HEADER, SNAPSHOT_REQUEST_HEADER, VERSION_ID_HEADER,
+    HISTORY_SEGMENT_CONTENT_TYPE, PARENT_VERSION_ID_HEADER, SNAPSHOT_REQUEST_HEADER,
+    VERSION_ID_HEADER,
 };
 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,
+    AddVersionResult, ServerError, SnapshotUrgency, VersionId, NIL_VERSION_ID,
 };
 
 /// Max history segment size: 100MB
@@ -56,58 +57,47 @@ pub(crate) async fn service(
         return Err(error::ErrorBadRequest("Empty body"));
     }
 
-    // note that we do not open the transaction until the body has been read
+    loop {
-    // completely, to avoid blocking other storage access while that data is
+        return match server_state
-    // in transit.
+            .server
-    let mut txn = server_state.storage.txn().map_err(failure_to_ise)?;
+            .add_version(client_id, parent_version_id, body.to_vec())
-
+        {
-    // get, or create, the client
+            Ok((AddVersionResult::Ok(version_id), snap_urgency)) => {
-    let client = match txn.get_client(client_id).map_err(failure_to_ise)? {
+                let mut rb = HttpResponse::Ok();
-        Some(client) => client,
+                rb.append_header((VERSION_ID_HEADER, version_id.to_string()));
-        None => {
+                match snap_urgency {
-            txn.new_client(client_id, NIL_VERSION_ID)
+                    SnapshotUrgency::None => {}
-                .map_err(failure_to_ise)?;
+                    SnapshotUrgency::Low => {
-            txn.get_client(client_id).map_err(failure_to_ise)?.unwrap()
+                        rb.append_header((SNAPSHOT_REQUEST_HEADER, "urgency=low"));
-        }
+                    }
-    };
+                    SnapshotUrgency::High => {
-
+                        rb.append_header((SNAPSHOT_REQUEST_HEADER, "urgency=high"));
-    let (result, snap_urgency) = add_version(
+                    }
-        txn,
+                };
-        &server_state.config,
+                Ok(rb.finish())
-        client_id,
+            }
-        client,
+            Ok((AddVersionResult::ExpectedParentVersion(parent_version_id), _)) => {
-        parent_version_id,
+                let mut rb = HttpResponse::Conflict();
-        body.to_vec(),
+                rb.append_header((PARENT_VERSION_ID_HEADER, parent_version_id.to_string()));
-    )
+                Ok(rb.finish())
-    .map_err(failure_to_ise)?;
+            }
-
+            Err(ServerError::NoSuchClient) => {
-    Ok(match result {
+                // Create a new client and repeat the `add_version` call.
-        AddVersionResult::Ok(version_id) => {
+                let mut txn = server_state.server.txn().map_err(server_error_to_actix)?;
-            let mut rb = HttpResponse::Ok();
+                txn.new_client(client_id, NIL_VERSION_ID)
-            rb.append_header((VERSION_ID_HEADER, version_id.to_string()));
+                    .map_err(failure_to_ise)?;
-            match snap_urgency {
+                txn.commit().map_err(failure_to_ise)?;
-                SnapshotUrgency::None => {}
+                continue;
-                SnapshotUrgency::Low => {
+            }
-                    rb.append_header((SNAPSHOT_REQUEST_HEADER, "urgency=low"));
+            Err(e) => Err(server_error_to_actix(e)),
-                }
+        };
-                SnapshotUrgency::High => {
+    }
-                    rb.append_header((SNAPSHOT_REQUEST_HEADER, "urgency=high"));
-                }
-            };
-            rb.finish()
-        }
-        AddVersionResult::ExpectedParentVersion(parent_version_id) => {
-            let mut rb = HttpResponse::Conflict();
-            rb.append_header((PARENT_VERSION_ID_HEADER, parent_version_id.to_string()));
-            rb.finish()
-        }
-    })
 }
 
 #[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 +108,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 +116,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;
 
@@ -155,12 +145,55 @@ mod test {
         assert_eq!(resp.headers().get("X-Parent-Version-Id"), None);
     }
 
+    #[actix_rt::test]
+    async fn test_auto_add_client() {
+        let client_id = Uuid::new_v4();
+        let version_id = Uuid::new_v4();
+        let parent_version_id = Uuid::new_v4();
+        let server = WebServer::new(Default::default(), InMemoryStorage::new());
+        let app = App::new().configure(|sc| server.config(sc));
+        let app = test::init_service(app).await;
+
+        let uri = format!("/v1/client/add-version/{}", parent_version_id);
+        let req = test::TestRequest::post()
+            .uri(&uri)
+            .append_header((
+                "Content-Type",
+                "application/vnd.taskchampion.history-segment",
+            ))
+            .append_header((CLIENT_ID_HEADER, client_id.to_string()))
+            .set_payload(b"abcd".to_vec())
+            .to_request();
+        let resp = test::call_service(&app, req).await;
+        assert_eq!(resp.status(), StatusCode::OK);
+
+        // the returned version ID is random, but let's check that it's not
+        // the passed parent version ID, at least
+        let new_version_id = resp.headers().get("X-Version-Id").unwrap();
+        let new_version_id = Uuid::parse_str(new_version_id.to_str().unwrap()).unwrap();
+        assert!(new_version_id != version_id);
+
+        // Shapshot should be requested, since there is no existing snapshot
+        let snapshot_request = resp.headers().get("X-Snapshot-Request").unwrap();
+        assert_eq!(snapshot_request, "urgency=high");
+
+        assert_eq!(resp.headers().get("X-Parent-Version-Id"), None);
+
+        // Check that the client really was created
+        {
+            let mut txn = server.server_state.server.txn().unwrap();
+            let client = txn.get_client(client_id).unwrap().unwrap();
+            assert_eq!(client.latest_version_id, new_version_id);
+            assert_eq!(client.snapshot, None);
+        }
+    }
+
     #[actix_rt::test]
     async fn test_conflict() {
         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 +201,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 +228,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 storage = InMemoryStorage::new();
-        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;
 
@@ -215,8 +248,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 storage = InMemoryStorage::new();
-        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_child_version.rs

@@ -1,10 +1,10 @@
 use crate::api::{
-    client_id_header, failure_to_ise, ServerState, HISTORY_SEGMENT_CONTENT_TYPE,
+    client_id_header, server_error_to_actix, ServerState, HISTORY_SEGMENT_CONTENT_TYPE,
     PARENT_VERSION_ID_HEADER, VERSION_ID_HEADER,
 };
 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, ServerError, VersionId};
 
 /// Get a child version.
 ///
@@ -21,43 +21,35 @@ pub(crate) async fn service(
     path: web::Path<VersionId>,
 ) -> Result<HttpResponse> {
     let parent_version_id = path.into_inner();
-
-    let mut txn = server_state.storage.txn().map_err(failure_to_ise)?;
-
     let client_id = client_id_header(&req)?;
 
-    let client = txn
+    return match server_state
-        .get_client(client_id)
+        .server
-        .map_err(failure_to_ise)?
+        .get_child_version(client_id, parent_version_id)
-        .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 {
+        Ok(GetVersionResult::Success {
             version_id,
             parent_version_id,
             history_segment,
-        } => Ok(HttpResponse::Ok()
+        }) => Ok(HttpResponse::Ok()
             .content_type(HISTORY_SEGMENT_CONTENT_TYPE)
             .append_header((VERSION_ID_HEADER, version_id.to_string()))
             .append_header((PARENT_VERSION_ID_HEADER, parent_version_id.to_string()))
             .body(history_segment)),
-        GetVersionResult::NotFound => Err(error::ErrorNotFound("no such version")),
+        Ok(GetVersionResult::NotFound) => Err(error::ErrorNotFound("no such version")),
-        GetVersionResult::Gone => Err(error::ErrorGone("version has been deleted")),
+        Ok(GetVersionResult::Gone) => Err(error::ErrorGone("version has been deleted")),
+        // Note that the HTTP client cannot differentiate `NotFound` and `NoSuchClient`, as both
+        // are a 404 NOT FOUND response. In either case, the HTTP client will typically attempt
+        // to add a new version, which may create the new client at the same time.
+        Err(ServerError::NoSuchClient) => Err(error::ErrorNotFound("no such client")),
+        Err(e) => Err(server_error_to_actix(e)),
     };
 }
 
 #[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 +60,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 +70,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 +103,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 storage = InMemoryStorage::new();
-        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;
 
@@ -131,7 +123,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 +132,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

@@ -1,9 +1,8 @@
 use crate::api::{
-    client_id_header, failure_to_ise, ServerState, SNAPSHOT_CONTENT_TYPE, VERSION_ID_HEADER,
+    client_id_header, server_error_to_actix, ServerState, SNAPSHOT_CONTENT_TYPE, VERSION_ID_HEADER,
 };
 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,12 @@ 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
+    if let Some((version_id, data)) = server_state
-        .get_client(client_id)
+        .server
-        .map_err(failure_to_ise)?
+        .get_snapshot(client_id)
-        .ok_or_else(|| error::ErrorNotFound("no such client"))?;
+        .map_err(server_error_to_actix)?
-
-    if let Some((version_id, data)) =
-        get_snapshot(txn, &server_state.config, client_id, client).map_err(failure_to_ise)?
     {
         Ok(HttpResponse::Ok()
             .content_type(SNAPSHOT_CONTENT_TYPE)
@@ -42,7 +36,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 +46,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 +54,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 +72,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 +90,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 actix_web::{error, web, HttpRequest, Result, Scope};
-use taskchampion_sync_server_core::{ClientId, ServerConfig, Storage};
+use taskchampion_sync_server_core::{ClientId, Server, ServerError};
 
 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) server: Server,
-    pub(crate) config: ServerConfig,
 }
 
 pub(crate) fn api_scope() -> Scope {
@@ -39,9 +38,17 @@ pub(crate) fn api_scope() -> Scope {
         .service(add_snapshot::service)
 }
 
-/// Convert a failure::Error to an Actix ISE
+/// Convert a `anyhow::Error` to an Actix ISE
-fn failure_to_ise(err: anyhow::Error) -> impl actix_web::ResponseError {
+fn failure_to_ise(err: anyhow::Error) -> actix_web::Error {
-    error::InternalError::new(err, StatusCode::INTERNAL_SERVER_ERROR)
+    error::ErrorInternalServerError(err)
+}
+
+/// Convert a ServerError to an Actix error
+fn server_error_to_actix(err: ServerError) -> actix_web::Error {
+    match err {
+        ServerError::NoSuchClient => error::ErrorNotFound(err),
+        ServerError::Other(err) => error::ErrorInternalServerError(err),
+    }
 }
 
 /// Get the client id

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 config = ServerConfig {
-    let server = Server::new(config, Box::new(SqliteStorage::new(data_dir)?));
+        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;