Merge pull request #132 from djmitche/postgres-bin

Add a binary that uses a Postgres backend

.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()
+        )
+    }
+}