Add a binary that uses a Postgres backend

Building of this binary is controlled with features, allowing downstream
users to build just the SQLite version and not be concerned with the
tokio-postgres dependency tree (which includes links to OpenSSL and
other details). The Postgres version is disabled by default.

This does not change the binary name for the SQLite build, just to avoid
confusion for people upgrading to the new version.

.github/workflows/checks.yml

@@ -66,7 +66,13 @@ jobs:
         uses: actions-rs/cargo@v1.0.3
         with:
           command: rustdoc
-          args: -p taskchampion-sync-server --all-features -- -Z unstable-options  --check -Dwarnings
+          args: -p taskchampion-sync-server --bin taskchampion-sync-server --all-features -- -Z unstable-options  --check -Dwarnings
+
+      - name: taskchampion-sync-server-postgres
+        uses: actions-rs/cargo@v1.0.3
+        with:
+          command: rustdoc
+          args: -p taskchampion-sync-server --bin taskchampion-sync-server-postgres --all-features -- -Z unstable-options  --check -Dwarnings
 
       - name: taskchampion-sync-server-core
         uses: actions-rs/cargo@v1.0.3
@@ -80,6 +86,12 @@ jobs:
           command: rustdoc
           args: -p taskchampion-sync-server-storage-sqlite --all-features -- -Z unstable-options  --check -Dwarnings
 
+      - name: taskchampion-sync-server-storage-postgres
+        uses: actions-rs/cargo@v1.0.3
+        with:
+          command: rustdoc
+          args: -p taskchampion-sync-server-storage-postgres --all-features -- -Z unstable-options  --check -Dwarnings
+
   fmt:
     runs-on: ubuntu-latest
     name: "Formatting"

Cargo.lock

@@ -1863,6 +1863,7 @@ dependencies = [
  "serde",
  "serde_json",
  "taskchampion-sync-server-core",
+ "taskchampion-sync-server-storage-postgres",
  "taskchampion-sync-server-storage-sqlite",
  "temp-env",
  "tempfile",

README.md

@@ -17,7 +17,7 @@ for more on how to use this project.
 
 ## Repository Guide
 
-The repository is comprised of three crates:
+The repository is comprised of four crates:
 
  - `taskchampion-sync-server-core` implements the core of the protocol
  - `taskchampion-sync-server-storage-sqlite` implements an SQLite backend for the core
@@ -60,16 +60,34 @@ cargo build --release
 After build the binary is located in
 `target/release/taskchampion-sync-server`.
 
-### Building the Container
+#### Building the Postgres backend
 
-To build the container, execute the following commands.
+The storage backend is controlled by Cargo features `postres` and `sqlite`.
+By default, only the `sqlite` feature is enabled.
+To enable building the Postgres backend, add `--features postgres`.
+The Postgres binary is located in
+`target/release/taskchampion-sync-server-postgres`.
 
+### Building the Docker Images
+
+To build the images, execute the following commands.
+
+SQLite:
 ```sh
 source .env
 docker build \
   --build-arg RUST_VERSION=${RUST_VERSION} \
   --build-arg ALPINE_VERSION=${ALPINE_VERSION} \
-  -t taskchampion-sync-server .
+  -t taskchampion-sync-server docker/sqlite
+```
+
+Postgres:
+```sh
+source .env
+docker build \
+  --build-arg RUST_VERSION=${RUST_VERSION} \
+  --build-arg ALPINE_VERSION=${ALPINE_VERSION} \
+  -t taskchampion-sync-server-postgres docker/postgres
 ```
 
 Now to run it, simply exec.

postgres/src/lib.rs

@@ -21,7 +21,7 @@
 //! An external application may:
 //!  - Add additional tables to the database
 //!  - Add additional columns to the `clients` table. If those columns do not have default
-//!    values, calls to [`Txn::new_client`] will fail. It is possible to configure
+//!    values, calls to `Txn::new_client` will fail. It is possible to configure
 //!    `taskchampion-sync-server` to never call this method.
 //!  - Insert rows into the `clients` table, using default values for all columns except
 //!    `client_id` and application-specific columns.

server/Cargo.toml

@@ -5,9 +5,25 @@ authors = ["Dustin J. Mitchell <dustin@mozilla.com>"]
 edition = "2021"
 publish = false
 
+[features]
+# By default, only build the SQLite backend.
+default = ["sqlite"]
+sqlite = ["dep:taskchampion-sync-server-storage-sqlite"]
+postgres = ["dep:taskchampion-sync-server-storage-postgres"]
+
+[[bin]]
+# The simple binary name is the SQLite build.
+name = "taskchampion-sync-server"
+required-features = ["sqlite"]
+
+[[bin]]
+name = "taskchampion-sync-server-postgres"
+required-features = ["postgres"]
+
 [dependencies]
 taskchampion-sync-server-core = { path = "../core" }
-taskchampion-sync-server-storage-sqlite = { path = "../sqlite" }
+taskchampion-sync-server-storage-sqlite = { path = "../sqlite", optional = true }
+taskchampion-sync-server-storage-postgres = { path = "../postgres", optional = true }
 uuid.workspace = true
 actix-web.workspace = true
 anyhow.workspace = true

server/src/api/add_snapshot.rs

@@ -56,8 +56,10 @@ pub(crate) async fn service(
 
 #[cfg(test)]
 mod test {
-    use crate::WebServer;
-    use crate::{api::CLIENT_ID_HEADER, WebConfig};
+    use crate::{
+        api::CLIENT_ID_HEADER,
+        web::{WebConfig, WebServer},
+    };
     use actix_web::{http::StatusCode, test, App};
     use pretty_assertions::assert_eq;
     use taskchampion_sync_server_core::{InMemoryStorage, ServerConfig, Storage, NIL_VERSION_ID};

server/src/api/add_version.rs

@@ -101,8 +101,10 @@ pub(crate) async fn service(
 
 #[cfg(test)]
 mod test {
-    use crate::WebServer;
-    use crate::{api::CLIENT_ID_HEADER, WebConfig};
+    use crate::{
+        api::CLIENT_ID_HEADER,
+        web::{WebConfig, WebServer},
+    };
     use actix_web::{http::StatusCode, test, App};
     use pretty_assertions::assert_eq;
     use taskchampion_sync_server_core::{InMemoryStorage, ServerConfig, Storage};

server/src/api/get_child_version.rs

@@ -49,8 +49,10 @@ pub(crate) async fn service(
 
 #[cfg(test)]
 mod test {
-    use crate::WebServer;
-    use crate::{api::CLIENT_ID_HEADER, WebConfig};
+    use crate::{
+        api::CLIENT_ID_HEADER,
+        web::{WebConfig, WebServer},
+    };
     use actix_web::{http::StatusCode, test, App};
     use pretty_assertions::assert_eq;
     use taskchampion_sync_server_core::{InMemoryStorage, ServerConfig, Storage, NIL_VERSION_ID};

server/src/api/get_snapshot.rs

@@ -34,8 +34,10 @@ pub(crate) async fn service(
 
 #[cfg(test)]
 mod test {
-    use crate::WebServer;
-    use crate::{api::CLIENT_ID_HEADER, WebConfig};
+    use crate::{
+        api::CLIENT_ID_HEADER,
+        web::{WebConfig, WebServer},
+    };
     use actix_web::{http::StatusCode, test, App};
     use chrono::{TimeZone, Utc};
     use pretty_assertions::assert_eq;

server/src/api/mod.rs

@@ -1,7 +1,7 @@
 use actix_web::{error, web, HttpRequest, Result, Scope};
 use taskchampion_sync_server_core::{ClientId, Server, ServerError};
 
-use crate::WebConfig;
+use crate::web::WebConfig;
 
 mod add_snapshot;
 mod add_version;
@@ -89,6 +89,7 @@ mod test {
             web_config: WebConfig {
                 client_id_allowlist: None,
                 create_clients: true,
+                ..WebConfig::default()
             },
         };
         let req = actix_web::test::TestRequest::default()
@@ -106,6 +107,7 @@ mod test {
             web_config: WebConfig {
                 client_id_allowlist: Some([client_id_ok].into()),
                 create_clients: true,
+                ..WebConfig::default()
             },
         };
         let req = actix_web::test::TestRequest::default()

server/src/args.rs

@@ -0,0 +1,299 @@
+use crate::web::WebConfig;
+use clap::{arg, builder::ValueParser, value_parser, ArgAction, ArgMatches, Command};
+use taskchampion_sync_server_core::ServerConfig;
+use uuid::Uuid;
+
+pub fn command() -> Command {
+    let defaults = ServerConfig::default();
+    let default_snapshot_versions = defaults.snapshot_versions.to_string();
+    let default_snapshot_days = defaults.snapshot_days.to_string();
+    Command::new("taskchampion-sync-server")
+        .version(env!("CARGO_PKG_VERSION"))
+        .about("Server for TaskChampion")
+        .arg(
+            arg!(-l --listen <ADDRESS>)
+                .help("Address and Port on which to listen on. Can be an IP Address or a DNS name followed by a colon and a port e.g. localhost:8080")
+                .value_delimiter(',')
+                .value_parser(ValueParser::string())
+                .env("LISTEN")
+                .action(ArgAction::Append)
+                .required(true),
+        )
+        .arg(
+            arg!(-C --"allow-client-id" <CLIENT_ID> "Client IDs to allow (can be repeated; if not specified, all clients are allowed)")
+                .value_delimiter(',')
+                .value_parser(value_parser!(Uuid))
+                .env("CLIENT_ID")
+                .action(ArgAction::Append)
+                .required(false),
+        )
+        .arg(
+            arg!("create-clients": --"no-create-clients" "If a client does not exist in the database, do not create it")
+                .env("CREATE_CLIENTS")
+                .default_value("true")
+                .action(ArgAction::SetFalse)
+                .required(false),
+        )
+        .arg(
+            arg!(--"snapshot-versions" <NUM> "Target number of versions between snapshots")
+                .value_parser(value_parser!(u32))
+                .env("SNAPSHOT_VERSIONS")
+                .default_value(default_snapshot_versions),
+        )
+        .arg(
+            arg!(--"snapshot-days" <NUM> "Target number of days between snapshots")
+                .value_parser(value_parser!(i64))
+                .env("SNAPSHOT_DAYS")
+                .default_value(default_snapshot_days),
+        )
+}
+
+/// Create a ServerConfig from these args.
+pub fn server_config_from_matches(matches: &ArgMatches) -> ServerConfig {
+    ServerConfig {
+        snapshot_versions: *matches.get_one("snapshot-versions").unwrap(),
+        snapshot_days: *matches.get_one("snapshot-days").unwrap(),
+    }
+}
+
+/// Create a WebConfig from these args.
+pub fn web_config_from_matches(matches: &ArgMatches) -> WebConfig {
+    WebConfig {
+        client_id_allowlist: matches
+            .get_many("allow-client-id")
+            .map(|ids| ids.copied().collect()),
+        create_clients: matches.get_one("create-clients").copied().unwrap_or(true),
+        listen_addresses: matches
+            .get_many::<String>("listen")
+            .unwrap()
+            .cloned()
+            .collect(),
+    }
+}
+
+#[cfg(test)]
+mod test {
+    #![allow(clippy::bool_assert_comparison)]
+
+    use super::*;
+    use crate::web::WebServer;
+    use actix_web::{self, App};
+    use clap::ArgMatches;
+    use taskchampion_sync_server_core::InMemoryStorage;
+    use temp_env::{with_var, with_var_unset, with_vars, with_vars_unset};
+
+    /// Get the list of allowed client IDs, sorted.
+    fn allowed(matches: ArgMatches) -> Option<Vec<Uuid>> {
+        web_config_from_matches(&matches)
+            .client_id_allowlist
+            .map(|ids| ids.into_iter().collect::<Vec<_>>())
+            .map(|mut ids| {
+                ids.sort();
+                ids
+            })
+    }
+
+    #[test]
+    fn command_listen_two() {
+        with_var_unset("LISTEN", || {
+            let matches = command().get_matches_from([
+                "tss",
+                "--listen",
+                "localhost:8080",
+                "--listen",
+                "otherhost:9090",
+            ]);
+            assert_eq!(
+                web_config_from_matches(&matches).listen_addresses,
+                vec!["localhost:8080".to_string(), "otherhost:9090".to_string()]
+            );
+        });
+    }
+
+    #[test]
+    fn command_listen_two_env() {
+        with_var("LISTEN", Some("localhost:8080,otherhost:9090"), || {
+            let matches = command().get_matches_from(["tss"]);
+            assert_eq!(
+                web_config_from_matches(&matches).listen_addresses,
+                vec!["localhost:8080".to_string(), "otherhost:9090".to_string()]
+            );
+        });
+    }
+
+    #[test]
+    fn command_allowed_client_ids_none() {
+        with_var_unset("CLIENT_ID", || {
+            let matches = command().get_matches_from(["tss", "--listen", "localhost:8080"]);
+            assert_eq!(allowed(matches), None);
+        });
+    }
+
+    #[test]
+    fn command_allowed_client_ids_one() {
+        with_var_unset("CLIENT_ID", || {
+            let matches = command().get_matches_from([
+                "tss",
+                "--listen",
+                "localhost:8080",
+                "-C",
+                "711d5cf3-0cf0-4eb8-9eca-6f7f220638c0",
+            ]);
+            assert_eq!(
+                allowed(matches),
+                Some(vec![Uuid::parse_str(
+                    "711d5cf3-0cf0-4eb8-9eca-6f7f220638c0"
+                )
+                .unwrap()])
+            );
+        });
+    }
+
+    #[test]
+    fn command_allowed_client_ids_one_env() {
+        with_var(
+            "CLIENT_ID",
+            Some("711d5cf3-0cf0-4eb8-9eca-6f7f220638c0"),
+            || {
+                let matches = command().get_matches_from(["tss", "--listen", "localhost:8080"]);
+                assert_eq!(
+                    allowed(matches),
+                    Some(vec![Uuid::parse_str(
+                        "711d5cf3-0cf0-4eb8-9eca-6f7f220638c0"
+                    )
+                    .unwrap()])
+                );
+            },
+        );
+    }
+
+    #[test]
+    fn command_allowed_client_ids_two() {
+        with_var_unset("CLIENT_ID", || {
+            let matches = command().get_matches_from([
+                "tss",
+                "--listen",
+                "localhost:8080",
+                "-C",
+                "711d5cf3-0cf0-4eb8-9eca-6f7f220638c0",
+                "-C",
+                "bbaf4b61-344a-4a39-a19e-8caa0669b353",
+            ]);
+            assert_eq!(
+                allowed(matches),
+                Some(vec![
+                    Uuid::parse_str("711d5cf3-0cf0-4eb8-9eca-6f7f220638c0").unwrap(),
+                    Uuid::parse_str("bbaf4b61-344a-4a39-a19e-8caa0669b353").unwrap()
+                ])
+            );
+        });
+    }
+
+    #[test]
+    fn command_allowed_client_ids_two_env() {
+        with_var(
+            "CLIENT_ID",
+            Some("711d5cf3-0cf0-4eb8-9eca-6f7f220638c0,bbaf4b61-344a-4a39-a19e-8caa0669b353"),
+            || {
+                let matches = command().get_matches_from(["tss", "--listen", "localhost:8080"]);
+                assert_eq!(
+                    allowed(matches),
+                    Some(vec![
+                        Uuid::parse_str("711d5cf3-0cf0-4eb8-9eca-6f7f220638c0").unwrap(),
+                        Uuid::parse_str("bbaf4b61-344a-4a39-a19e-8caa0669b353").unwrap()
+                    ])
+                );
+            },
+        );
+    }
+
+    #[test]
+    fn command_snapshot() {
+        with_vars_unset(["SNAPSHOT_DAYS", "SNAPSHOT_VERSIONS"], || {
+            let matches = command().get_matches_from([
+                "tss",
+                "--listen",
+                "localhost:8080",
+                "--snapshot-days",
+                "13",
+                "--snapshot-versions",
+                "20",
+            ]);
+            let server_config = server_config_from_matches(&matches);
+            assert_eq!(server_config.snapshot_days, 13i64);
+            assert_eq!(server_config.snapshot_versions, 20u32);
+        });
+    }
+
+    #[test]
+    fn command_snapshot_env() {
+        with_vars(
+            [
+                ("SNAPSHOT_DAYS", Some("13")),
+                ("SNAPSHOT_VERSIONS", Some("20")),
+            ],
+            || {
+                let matches = command().get_matches_from(["tss", "--listen", "localhost:8080"]);
+                let server_config = server_config_from_matches(&matches);
+                assert_eq!(server_config.snapshot_days, 13i64);
+                assert_eq!(server_config.snapshot_versions, 20u32);
+            },
+        );
+    }
+
+    #[test]
+    fn command_create_clients_default() {
+        with_var_unset("CREATE_CLIENTS", || {
+            let matches = command().get_matches_from(["tss", "--listen", "localhost:8080"]);
+            let server_config = web_config_from_matches(&matches);
+            assert_eq!(server_config.create_clients, true);
+        });
+    }
+
+    #[test]
+    fn command_create_clients_cmdline() {
+        with_var_unset("CREATE_CLIENTS", || {
+            let matches = command().get_matches_from([
+                "tss",
+                "--listen",
+                "localhost:8080",
+                "--no-create-clients",
+            ]);
+            let server_config = web_config_from_matches(&matches);
+            assert_eq!(server_config.create_clients, false);
+        });
+    }
+
+    #[test]
+    fn command_create_clients_env_true() {
+        with_vars([("CREATE_CLIENTS", Some("true"))], || {
+            let matches = command().get_matches_from(["tss", "--listen", "localhost:8080"]);
+            let server_config = web_config_from_matches(&matches);
+            assert_eq!(server_config.create_clients, true);
+        });
+    }
+
+    #[test]
+    fn command_create_clients_env_false() {
+        with_vars([("CREATE_CLIENTS", Some("false"))], || {
+            let matches = command().get_matches_from(["tss", "--listen", "localhost:8080"]);
+            let server_config = web_config_from_matches(&matches);
+            assert_eq!(server_config.create_clients, false);
+        });
+    }
+
+    #[actix_rt::test]
+    async fn test_index_get() {
+        let server = WebServer::new(
+            ServerConfig::default(),
+            WebConfig::default(),
+            InMemoryStorage::new(),
+        );
+        let app = App::new().configure(|sc| server.config(sc));
+        let app = actix_web::test::init_service(app).await;
+
+        let req = actix_web::test::TestRequest::get().uri("/").to_request();
+        let resp = actix_web::test::call_service(&app, req).await;
+        assert!(resp.status().is_success());
+    }
+}

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

@@ -0,0 +1,66 @@
+#![deny(clippy::all)]
+
+use clap::{arg, builder::ValueParser, ArgMatches, Command};
+use std::ffi::OsString;
+use taskchampion_sync_server::{args, web};
+use taskchampion_sync_server_storage_postgres::PostgresStorage;
+
+fn command() -> Command {
+    args::command().arg(
+        arg!(-c --"connection" <DIR> "LibPQ-style connection URI")
+            .value_parser(ValueParser::os_string())
+            .help("See https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNSTRING-URIS")
+            .required(true)
+            .env("CONNECTION")
+    )
+}
+
+fn connection_from_matches(matches: &ArgMatches) -> String {
+    matches
+        .get_one::<OsString>("connection")
+        .unwrap()
+        .to_str()
+        .expect("--connection must be valid UTF-8")
+        .to_string()
+}
+
+#[actix_web::main]
+async fn main() -> anyhow::Result<()> {
+    env_logger::init();
+    let matches = command().get_matches();
+    let server_config = args::server_config_from_matches(&matches);
+    let web_config = args::web_config_from_matches(&matches);
+    let connection = connection_from_matches(&matches);
+    let storage = PostgresStorage::new(connection).await?;
+
+    let server = web::WebServer::new(server_config, web_config, storage);
+    server.run().await
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+    use temp_env::{with_var, with_var_unset};
+
+    #[test]
+    fn command_connection() {
+        with_var_unset("CONNECTION", || {
+            let matches = command().get_matches_from([
+                "tss",
+                "--connection",
+                "postgresql:/foo/bar",
+                "--listen",
+                "localhost:8080",
+            ]);
+            assert_eq!(connection_from_matches(&matches), "postgresql:/foo/bar");
+        });
+    }
+
+    #[test]
+    fn command_connection_env() {
+        with_var("CONNECTION", Some("postgresql:/foo/bar"), || {
+            let matches = command().get_matches_from(["tss", "--listen", "localhost:8080"]);
+            assert_eq!(connection_from_matches(&matches), "postgresql:/foo/bar");
+        });
+    }
+}

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

@@ -1,271 +1,40 @@
 #![deny(clippy::all)]
 
-use actix_web::{
-    dev::ServiceResponse,
-    http::StatusCode,
-    middleware::{ErrorHandlerResponse, ErrorHandlers, Logger},
-    App, HttpServer,
-};
-use clap::{arg, builder::ValueParser, value_parser, ArgAction, Command};
-use std::{collections::HashSet, ffi::OsString};
-use taskchampion_sync_server::{WebConfig, WebServer};
-use taskchampion_sync_server_core::ServerConfig;
+use clap::{arg, builder::ValueParser, ArgMatches, Command};
+use std::ffi::OsString;
+use taskchampion_sync_server::{args, web};
 use taskchampion_sync_server_storage_sqlite::SqliteStorage;
-use uuid::Uuid;
 
 fn command() -> Command {
-    let defaults = ServerConfig::default();
-    let default_snapshot_versions = defaults.snapshot_versions.to_string();
-    let default_snapshot_days = defaults.snapshot_days.to_string();
-    Command::new("taskchampion-sync-server")
-        .version(env!("CARGO_PKG_VERSION"))
-        .about("Server for TaskChampion")
-        .arg(
-            arg!(-l --listen <ADDRESS>)
-                .help("Address and Port on which to listen on. Can be an IP Address or a DNS name followed by a colon and a port e.g. localhost:8080")
-                .value_delimiter(',')
-                .value_parser(ValueParser::string())
-                .env("LISTEN")
-                .action(ArgAction::Append)
-                .required(true),
-        )
-        .arg(
-            arg!(-d --"data-dir" <DIR> "Directory in which to store data")
-                .value_parser(ValueParser::os_string())
-                .env("DATA_DIR")
-                .default_value("/var/lib/taskchampion-sync-server"),
-        )
-        .arg(
-            arg!(-C --"allow-client-id" <CLIENT_ID> "Client IDs to allow (can be repeated; if not specified, all clients are allowed)")
-                .value_delimiter(',')
-                .value_parser(value_parser!(Uuid))
-                .env("CLIENT_ID")
-                .action(ArgAction::Append)
-                .required(false),
-        )
-        .arg(
-            arg!("create-clients": --"no-create-clients" "If a client does not exist in the database, do not create it")
-                .env("CREATE_CLIENTS")
-                .default_value("true")
-                .action(ArgAction::SetFalse)
-                .required(false),
-        )
-        .arg(
-            arg!(--"snapshot-versions" <NUM> "Target number of versions between snapshots")
-                .value_parser(value_parser!(u32))
-                .env("SNAPSHOT_VERSIONS")
-                .default_value(default_snapshot_versions),
-        )
-        .arg(
-            arg!(--"snapshot-days" <NUM> "Target number of days between snapshots")
-                .value_parser(value_parser!(i64))
-                .env("SNAPSHOT_DAYS")
-                .default_value(default_snapshot_days),
-        )
+    args::command().arg(
+        arg!(-d --"data-dir" <DIR> "Directory in which to store data")
+            .value_parser(ValueParser::os_string())
+            .env("DATA_DIR")
+            .default_value("/var/lib/taskchampion-sync-server"),
+    )
 }
 
-fn print_error<B>(res: ServiceResponse<B>) -> actix_web::Result<ErrorHandlerResponse<B>> {
-    if let Some(err) = res.response().error() {
-        log::error!("Internal Server Error caused by:\n{err:?}");
-    }
-    Ok(ErrorHandlerResponse::Response(res.map_into_left_body()))
-}
-
-struct ServerArgs {
-    data_dir: OsString,
-    snapshot_versions: u32,
-    snapshot_days: i64,
-    client_id_allowlist: Option<HashSet<Uuid>>,
-    create_clients: bool,
-    listen_addresses: Vec<String>,
-}
-
-impl ServerArgs {
-    fn new(matches: clap::ArgMatches) -> Self {
-        Self {
-            data_dir: matches.get_one::<OsString>("data-dir").unwrap().clone(),
-            snapshot_versions: *matches.get_one("snapshot-versions").unwrap(),
-            snapshot_days: *matches.get_one("snapshot-days").unwrap(),
-            client_id_allowlist: matches
-                .get_many("allow-client-id")
-                .map(|ids| ids.copied().collect()),
-            create_clients: matches.get_one("create-clients").copied().unwrap_or(true),
-            listen_addresses: matches
-                .get_many::<String>("listen")
-                .unwrap()
-                .cloned()
-                .collect(),
-        }
-    }
+fn data_dir_from_matches(matches: &ArgMatches) -> OsString {
+    matches.get_one::<OsString>("data-dir").unwrap().clone()
 }
 
 #[actix_web::main]
 async fn main() -> anyhow::Result<()> {
     env_logger::init();
     let matches = command().get_matches();
+    let server_config = args::server_config_from_matches(&matches);
+    let web_config = args::web_config_from_matches(&matches);
+    let data_dir = data_dir_from_matches(&matches);
+    let storage = SqliteStorage::new(data_dir)?;
 
-    let server_args = ServerArgs::new(matches);
-    let config = ServerConfig {
-        snapshot_days: server_args.snapshot_days,
-        snapshot_versions: server_args.snapshot_versions,
-    };
-    let server = WebServer::new(
-        config,
-        WebConfig {
-            client_id_allowlist: server_args.client_id_allowlist,
-            create_clients: server_args.create_clients,
-        },
-        SqliteStorage::new(server_args.data_dir)?,
-    );
-
-    let mut http_server = HttpServer::new(move || {
-        App::new()
-            .wrap(ErrorHandlers::new().handler(StatusCode::INTERNAL_SERVER_ERROR, print_error))
-            .wrap(Logger::default())
-            .configure(|cfg| server.config(cfg))
-    });
-    for listen_address in server_args.listen_addresses {
-        log::info!("Serving on {listen_address}");
-        http_server = http_server.bind(listen_address)?
-    }
-    http_server.run().await?;
-    Ok(())
+    let server = web::WebServer::new(server_config, web_config, storage);
+    server.run().await
 }
 
 #[cfg(test)]
 mod test {
-    #![allow(clippy::bool_assert_comparison)]
-
     use super::*;
-    use actix_web::{self, App};
-    use clap::ArgMatches;
-    use taskchampion_sync_server_core::InMemoryStorage;
-    use temp_env::{with_var, with_var_unset, with_vars, with_vars_unset};
-
-    /// Get the list of allowed client IDs, sorted.
-    fn allowed(matches: ArgMatches) -> Option<Vec<Uuid>> {
-        ServerArgs::new(matches)
-            .client_id_allowlist
-            .map(|ids| ids.into_iter().collect::<Vec<_>>())
-            .map(|mut ids| {
-                ids.sort();
-                ids
-            })
-    }
-
-    #[test]
-    fn command_listen_two() {
-        with_var_unset("LISTEN", || {
-            let matches = command().get_matches_from([
-                "tss",
-                "--listen",
-                "localhost:8080",
-                "--listen",
-                "otherhost:9090",
-            ]);
-            assert_eq!(
-                ServerArgs::new(matches).listen_addresses,
-                vec!["localhost:8080".to_string(), "otherhost:9090".to_string()]
-            );
-        });
-    }
-
-    #[test]
-    fn command_listen_two_env() {
-        with_var("LISTEN", Some("localhost:8080,otherhost:9090"), || {
-            let matches = command().get_matches_from(["tss"]);
-            assert_eq!(
-                ServerArgs::new(matches).listen_addresses,
-                vec!["localhost:8080".to_string(), "otherhost:9090".to_string()]
-            );
-        });
-    }
-
-    #[test]
-    fn command_allowed_client_ids_none() {
-        with_var_unset("CLIENT_ID", || {
-            let matches = command().get_matches_from(["tss", "--listen", "localhost:8080"]);
-            assert_eq!(allowed(matches), None);
-        });
-    }
-
-    #[test]
-    fn command_allowed_client_ids_one() {
-        with_var_unset("CLIENT_ID", || {
-            let matches = command().get_matches_from([
-                "tss",
-                "--listen",
-                "localhost:8080",
-                "-C",
-                "711d5cf3-0cf0-4eb8-9eca-6f7f220638c0",
-            ]);
-            assert_eq!(
-                allowed(matches),
-                Some(vec![Uuid::parse_str(
-                    "711d5cf3-0cf0-4eb8-9eca-6f7f220638c0"
-                )
-                .unwrap()])
-            );
-        });
-    }
-
-    #[test]
-    fn command_allowed_client_ids_one_env() {
-        with_var(
-            "CLIENT_ID",
-            Some("711d5cf3-0cf0-4eb8-9eca-6f7f220638c0"),
-            || {
-                let matches = command().get_matches_from(["tss", "--listen", "localhost:8080"]);
-                assert_eq!(
-                    allowed(matches),
-                    Some(vec![Uuid::parse_str(
-                        "711d5cf3-0cf0-4eb8-9eca-6f7f220638c0"
-                    )
-                    .unwrap()])
-                );
-            },
-        );
-    }
-
-    #[test]
-    fn command_allowed_client_ids_two() {
-        with_var_unset("CLIENT_ID", || {
-            let matches = command().get_matches_from([
-                "tss",
-                "--listen",
-                "localhost:8080",
-                "-C",
-                "711d5cf3-0cf0-4eb8-9eca-6f7f220638c0",
-                "-C",
-                "bbaf4b61-344a-4a39-a19e-8caa0669b353",
-            ]);
-            assert_eq!(
-                allowed(matches),
-                Some(vec![
-                    Uuid::parse_str("711d5cf3-0cf0-4eb8-9eca-6f7f220638c0").unwrap(),
-                    Uuid::parse_str("bbaf4b61-344a-4a39-a19e-8caa0669b353").unwrap()
-                ])
-            );
-        });
-    }
-
-    #[test]
-    fn command_allowed_client_ids_two_env() {
-        with_var(
-            "CLIENT_ID",
-            Some("711d5cf3-0cf0-4eb8-9eca-6f7f220638c0,bbaf4b61-344a-4a39-a19e-8caa0669b353"),
-            || {
-                let matches = command().get_matches_from(["tss", "--listen", "localhost:8080"]);
-                assert_eq!(
-                    allowed(matches),
-                    Some(vec![
-                        Uuid::parse_str("711d5cf3-0cf0-4eb8-9eca-6f7f220638c0").unwrap(),
-                        Uuid::parse_str("bbaf4b61-344a-4a39-a19e-8caa0669b353").unwrap()
-                    ])
-                );
-            },
-        );
-    }
+    use temp_env::{with_var, with_var_unset};
 
     #[test]
     fn command_data_dir() {
@@ -277,7 +46,7 @@ mod test {
                 "--listen",
                 "localhost:8080",
             ]);
-            assert_eq!(ServerArgs::new(matches).data_dir, "/foo/bar");
+            assert_eq!(data_dir_from_matches(&matches), "/foo/bar");
         });
     }
 
@@ -285,97 +54,7 @@ mod test {
     fn command_data_dir_env() {
         with_var("DATA_DIR", Some("/foo/bar"), || {
             let matches = command().get_matches_from(["tss", "--listen", "localhost:8080"]);
-            assert_eq!(ServerArgs::new(matches).data_dir, "/foo/bar");
+            assert_eq!(data_dir_from_matches(&matches), "/foo/bar");
         });
     }
-
-    #[test]
-    fn command_snapshot() {
-        with_vars_unset(["SNAPSHOT_DAYS", "SNAPSHOT_VERSIONS"], || {
-            let matches = command().get_matches_from([
-                "tss",
-                "--listen",
-                "localhost:8080",
-                "--snapshot-days",
-                "13",
-                "--snapshot-versions",
-                "20",
-            ]);
-            let server_args = ServerArgs::new(matches);
-            assert_eq!(server_args.snapshot_days, 13i64);
-            assert_eq!(server_args.snapshot_versions, 20u32);
-        });
-    }
-
-    #[test]
-    fn command_snapshot_env() {
-        with_vars(
-            [
-                ("SNAPSHOT_DAYS", Some("13")),
-                ("SNAPSHOT_VERSIONS", Some("20")),
-            ],
-            || {
-                let matches = command().get_matches_from(["tss", "--listen", "localhost:8080"]);
-                let server_args = ServerArgs::new(matches);
-                assert_eq!(server_args.snapshot_days, 13i64);
-                assert_eq!(server_args.snapshot_versions, 20u32);
-            },
-        );
-    }
-
-    #[test]
-    fn command_create_clients_default() {
-        with_var_unset("CREATE_CLIENTS", || {
-            let matches = command().get_matches_from(["tss", "--listen", "localhost:8080"]);
-            let server_args = ServerArgs::new(matches);
-            assert_eq!(server_args.create_clients, true);
-        });
-    }
-
-    #[test]
-    fn command_create_clients_cmdline() {
-        with_var_unset("CREATE_CLIENTS", || {
-            let matches = command().get_matches_from([
-                "tss",
-                "--listen",
-                "localhost:8080",
-                "--no-create-clients",
-            ]);
-            let server_args = ServerArgs::new(matches);
-            assert_eq!(server_args.create_clients, false);
-        });
-    }
-
-    #[test]
-    fn command_create_clients_env_true() {
-        with_vars([("CREATE_CLIENTS", Some("true"))], || {
-            let matches = command().get_matches_from(["tss", "--listen", "localhost:8080"]);
-            let server_args = ServerArgs::new(matches);
-            assert_eq!(server_args.create_clients, true);
-        });
-    }
-
-    #[test]
-    fn command_create_clients_env_false() {
-        with_vars([("CREATE_CLIENTS", Some("false"))], || {
-            let matches = command().get_matches_from(["tss", "--listen", "localhost:8080"]);
-            let server_args = ServerArgs::new(matches);
-            assert_eq!(server_args.create_clients, false);
-        });
-    }
-
-    #[actix_rt::test]
-    async fn test_index_get() {
-        let server = WebServer::new(
-            ServerConfig::default(),
-            WebConfig::default(),
-            InMemoryStorage::new(),
-        );
-        let app = App::new().configure(|sc| server.config(sc));
-        let app = actix_web::test::init_service(app).await;
-
-        let req = actix_web::test::TestRequest::get().uri("/").to_request();
-        let resp = actix_web::test::call_service(&app, req).await;
-        assert!(resp.status().is_success());
-    }
 }

server/src/lib.rs

@@ -1,91 +1,5 @@
 #![deny(clippy::all)]
 
-mod api;
-
-use actix_web::{get, middleware, web, Responder};
-use api::{api_scope, ServerState};
-use std::{collections::HashSet, sync::Arc};
-use taskchampion_sync_server_core::{Server, ServerConfig, Storage};
-use uuid::Uuid;
-
-#[get("/")]
-async fn index() -> impl Responder {
-    format!("TaskChampion sync server v{}", env!("CARGO_PKG_VERSION"))
-}
-
-/// A Server represents a sync server.
-#[derive(Clone)]
-pub struct WebServer {
-    server_state: Arc<ServerState>,
-}
-
-/// Configuration for WebServer (as distinct from [`ServerConfig`]).
-pub struct WebConfig {
-    pub client_id_allowlist: Option<HashSet<Uuid>>,
-    pub create_clients: bool,
-}
-
-impl Default for WebConfig {
-    fn default() -> Self {
-        Self {
-            client_id_allowlist: Default::default(),
-            create_clients: true,
-        }
-    }
-}
-
-impl WebServer {
-    /// Create a new sync server with the given storage implementation.
-    pub fn new<ST: Storage + 'static>(
-        config: ServerConfig,
-        web_config: WebConfig,
-        storage: ST,
-    ) -> Self {
-        Self {
-            server_state: Arc::new(ServerState {
-                server: Server::new(config, storage),
-                web_config,
-            }),
-        }
-    }
-
-    /// Get an Actix-web service for this server.
-    pub fn config(&self, cfg: &mut web::ServiceConfig) {
-        cfg.service(
-            web::scope("")
-                .app_data(web::Data::new(self.server_state.clone()))
-                .wrap(
-                    middleware::DefaultHeaders::new().add(("Cache-Control", "no-store, max-age=0")),
-                )
-                .service(index)
-                .service(api_scope()),
-        );
-    }
-}
-
-#[cfg(test)]
-mod test {
-    use super::*;
-    use actix_web::{test, App};
-    use pretty_assertions::assert_eq;
-    use taskchampion_sync_server_core::InMemoryStorage;
-
-    #[actix_rt::test]
-    async fn test_cache_control() {
-        let server = WebServer::new(
-            ServerConfig::default(),
-            WebConfig::default(),
-            InMemoryStorage::new(),
-        );
-        let app = App::new().configure(|sc| server.config(sc));
-        let app = test::init_service(app).await;
-
-        let req = test::TestRequest::get().uri("/").to_request();
-        let resp = test::call_service(&app, req).await;
-        assert!(resp.status().is_success());
-        assert_eq!(
-            resp.headers().get("Cache-Control").unwrap(),
-            &"no-store, max-age=0".to_string()
-        )
-    }
-}
+pub mod api;
+pub mod args;
+pub mod web;

server/src/web.rs

@@ -0,0 +1,118 @@
+use crate::api::{api_scope, ServerState};
+use actix_web::{
+    dev::ServiceResponse,
+    get,
+    http::StatusCode,
+    middleware,
+    middleware::{ErrorHandlerResponse, ErrorHandlers, Logger},
+    web, App, HttpServer, Responder,
+};
+use std::{collections::HashSet, sync::Arc};
+use taskchampion_sync_server_core::{Server, ServerConfig, Storage};
+use uuid::Uuid;
+
+fn print_error<B>(res: ServiceResponse<B>) -> actix_web::Result<ErrorHandlerResponse<B>> {
+    if let Some(err) = res.response().error() {
+        log::error!("Internal Server Error caused by:\n{err:?}");
+    }
+    Ok(ErrorHandlerResponse::Response(res.map_into_left_body()))
+}
+
+/// Configuration for WebServer (as distinct from [`ServerConfig`]).
+pub struct WebConfig {
+    pub client_id_allowlist: Option<HashSet<Uuid>>,
+    pub create_clients: bool,
+    pub listen_addresses: Vec<String>,
+}
+
+impl Default for WebConfig {
+    fn default() -> Self {
+        Self {
+            client_id_allowlist: Default::default(),
+            create_clients: true,
+            listen_addresses: vec![],
+        }
+    }
+}
+
+#[get("/")]
+async fn index() -> impl Responder {
+    format!("TaskChampion sync server v{}", env!("CARGO_PKG_VERSION"))
+}
+
+/// A Server represents a sync server.
+#[derive(Clone)]
+pub struct WebServer {
+    pub(crate) server_state: Arc<ServerState>,
+}
+
+impl WebServer {
+    /// Create a new sync server with the given storage implementation.
+    pub fn new<ST: Storage + 'static>(
+        config: ServerConfig,
+        web_config: WebConfig,
+        storage: ST,
+    ) -> Self {
+        Self {
+            server_state: Arc::new(ServerState {
+                server: Server::new(config, storage),
+                web_config,
+            }),
+        }
+    }
+
+    pub fn config(&self, cfg: &mut web::ServiceConfig) {
+        cfg.service(
+            web::scope("")
+                .app_data(web::Data::new(self.server_state.clone()))
+                .wrap(
+                    middleware::DefaultHeaders::new().add(("Cache-Control", "no-store, max-age=0")),
+                )
+                .service(index)
+                .service(api_scope()),
+        );
+    }
+
+    pub async fn run(self) -> anyhow::Result<()> {
+        let listen_addresses = self.server_state.web_config.listen_addresses.clone();
+        let mut http_server = HttpServer::new(move || {
+            App::new()
+                .wrap(ErrorHandlers::new().handler(StatusCode::INTERNAL_SERVER_ERROR, print_error))
+                .wrap(Logger::default())
+                .configure(|cfg| self.config(cfg))
+        });
+        for listen_address in listen_addresses {
+            log::info!("Serving on {listen_address}");
+            http_server = http_server.bind(listen_address)?
+        }
+        http_server.run().await?;
+        Ok(())
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+    use actix_web::{test, App};
+    use pretty_assertions::assert_eq;
+    use taskchampion_sync_server_core::InMemoryStorage;
+
+    #[actix_rt::test]
+    async fn test_cache_control() {
+        let server = WebServer::new(
+            ServerConfig::default(),
+            WebConfig::default(),
+            InMemoryStorage::new(),
+        );
+        let app = App::new().configure(|sc| server.config(sc));
+        let app = test::init_service(app).await;
+
+        let req = test::TestRequest::get().uri("/").to_request();
+        let resp = test::call_service(&app, req).await;
+        assert!(resp.status().is_success());
+        assert_eq!(
+            resp.headers().get("Cache-Control").unwrap(),
+            &"no-store, max-age=0".to_string()
+        )
+    }
+}