Refactor HTTP implementation of API methods

Cargo.lock

@@ -822,6 +822,7 @@ checksum = "9b3b0c040a1fe6529d30b3c5944b280c7f0dcb2930d2c3062bca967b602583d0"
 dependencies = [
  "futures-channel",
  "futures-core",
+ "futures-executor",
  "futures-io",
  "futures-sink",
  "futures-task",
@@ -844,6 +845,17 @@ version = "0.3.8"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "847ce131b72ffb13b6109a221da9ad97a64cbe48feb1028356b836b47b8f1748"
 
+[[package]]
+name = "futures-executor"
+version = "0.3.8"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "4caa2b2b68b880003057c1dd49f1ed937e38f22fcf6c212188a121f08cf40a65"
+dependencies = [
+ "futures-core",
+ "futures-task",
+ "futures-util",
+]
+
 [[package]]
 name = "futures-io"
 version = "0.3.8"
@@ -2025,8 +2037,7 @@ version = "0.1.0"
 dependencies = [
  "actix-web",
  "failure",
- "serde",
- "serde_json",
+ "futures",
  "taskchampion",
 ]
 

sync-server/Cargo.toml

@@ -9,6 +9,5 @@ edition = "2018"
 [dependencies]
 actix-web = "3.3.0"
 failure = "0.1.8"
-serde = "1.0.117"
-serde_json = "1.0.59"
+futures = "0.3.8"
 taskchampion = { path = "../taskchampion" }

sync-server/src/api/add_version.rs

@@ -1,24 +1,57 @@
-use crate::api::ServerState;
-use crate::types::{ClientId, HistorySegment, VersionId};
-use actix_web::{error, http::StatusCode, post, web, HttpResponse, Responder, Result};
-use serde::{Deserialize, Serialize};
+use crate::api::{
+    ServerState, HISTORY_SEGMENT_CONTENT_TYPE, PARENT_VERSION_ID_HEADER, VERSION_ID_HEADER,
+};
+use crate::types::{AddVersionResult, ClientId, VersionId};
+use actix_web::{
+    error, http::StatusCode, post, web, HttpMessage, HttpRequest, HttpResponse, Result,
+};
+use futures::StreamExt;
 
-/// Request body to add_version
-#[derive(Serialize, Deserialize)]
-pub(crate) struct AddVersionRequest {
-    // TODO: temporary!
-    #[serde(default)]
-    history_segment: HistorySegment,
-}
+/// Max history segment size: 100MB
+const MAX_SIZE: usize = 100 * 1024 * 1024;
 
+/// Add a new version, after checking prerequisites.  The history segment should be transmitted in
+/// the request entity body and must have content-type
+/// `application/vnd.taskchampion.history-segment`.  The content can be encoded in any of the
+/// formats supported by actix-web.
+///
+/// On success, the response is a 200 OK with the new version ID in the `X-Version-Id` header.  If
+/// the version cannot be added due to a conflict, the response is a 409 CONFLICT with the expected
+/// parent version ID in the `X-Parent-Version-Id` header.
+///
+/// Returns other 4xx or 5xx responses on other errors.
 #[post("/client/{client_id}/add-version/{parent_version_id}")]
 pub(crate) async fn service(
+    req: HttpRequest,
     data: web::Data<ServerState>,
     web::Path((client_id, parent_version_id)): web::Path<(ClientId, VersionId)>,
-    body: web::Json<AddVersionRequest>,
-) -> Result<impl Responder> {
+    mut payload: web::Payload,
+) -> Result<HttpResponse> {
+    // check content-type
+    if req.content_type() != HISTORY_SEGMENT_CONTENT_TYPE {
+        return Err(error::ErrorBadRequest("Bad content-type"));
+    }
+
+    // read the body in its entirety
+    let mut body = web::BytesMut::new();
+    while let Some(chunk) = payload.next().await {
+        let chunk = chunk?;
+        // limit max size of in-memory payload
+        if (body.len() + chunk.len()) > MAX_SIZE {
+            return Err(error::ErrorBadRequest("overflow"));
+        }
+        body.extend_from_slice(&chunk);
+    }
+
     let result = data
-        .add_version(client_id, parent_version_id, &body.history_segment)
+        .add_version(client_id, parent_version_id, body.to_vec())
         .map_err(|e| error::InternalError::new(e, StatusCode::INTERNAL_SERVER_ERROR))?;
-    Ok(HttpResponse::Ok().json(result))
+    Ok(match result {
+        AddVersionResult::Ok(version_id) => HttpResponse::Ok()
+            .header(VERSION_ID_HEADER, version_id.to_string())
+            .body(""),
+        AddVersionResult::ExpectedParentVersion(parent_version_id) => HttpResponse::Conflict()
+            .header(PARENT_VERSION_ID_HEADER, parent_version_id.to_string())
+            .body(""),
+    })
 }

sync-server/src/api/get_child_version.rs

@@ -1,7 +1,17 @@
-use crate::api::ServerState;
+use crate::api::{
+    ServerState, HISTORY_SEGMENT_CONTENT_TYPE, PARENT_VERSION_ID_HEADER, VERSION_ID_HEADER,
+};
 use crate::types::{ClientId, VersionId};
 use actix_web::{error, get, http::StatusCode, web, HttpResponse, Result};
 
+/// Get a child version.
+///
+/// On succcess, the response is the same sequence of bytes originally sent to the server,
+/// with content-type `application/vnd.taskchampion.history-segment`.  The `X-Version-Id` and
+/// `X-Parent-Version-Id` headers contain the corresponding values.
+///
+/// If no such child exists, returns a 404 with no content.
+/// Returns other 4xx or 5xx responses on other errors.
 #[get("/client/{client_id}/get-child-version/{parent_version_id}")]
 pub(crate) async fn service(
     data: web::Data<ServerState>,
@@ -11,7 +21,14 @@ pub(crate) async fn service(
         .get_child_version(client_id, parent_version_id)
         .map_err(|e| error::InternalError::new(e, StatusCode::INTERNAL_SERVER_ERROR))?;
     if let Some(result) = result {
-        Ok(HttpResponse::Ok().json(result))
+        Ok(HttpResponse::Ok()
+            .content_type(HISTORY_SEGMENT_CONTENT_TYPE)
+            .header(VERSION_ID_HEADER, result.version_id.to_string())
+            .header(
+                PARENT_VERSION_ID_HEADER,
+                result.parent_version_id.to_string(),
+            )
+            .body(result.history_segment))
     } else {
         Err(error::ErrorNotFound("no such version"))
     }

sync-server/src/api/mod.rs

@@ -4,5 +4,15 @@ use std::sync::Arc;
 pub(crate) mod add_version;
 pub(crate) mod get_child_version;
 
+/// The content-type for history segments (opaque blobs of bytes)
+pub(crate) const HISTORY_SEGMENT_CONTENT_TYPE: &str =
+    "application/vnd.taskchampion.history-segment";
+
+/// The header names for version ID
+pub(crate) const VERSION_ID_HEADER: &str = "X-Version-Id";
+
+/// The header names for parent version ID
+pub(crate) const PARENT_VERSION_ID_HEADER: &str = "X-Parent-Version-Id";
+
 /// The type containing a reference to the SyncServer object in the Actix state.
 pub(crate) type ServerState = Arc<Box<dyn SyncServer>>;

sync-server/src/main.rs

@@ -1,5 +1,6 @@
 use actix_web::{App, HttpServer};
-use std::sync::Arc;
+use api::ServerState;
+use server::{NullSyncServer, SyncServer};
 
 mod api;
 mod server;
@@ -9,7 +10,8 @@ mod types;
 
 #[actix_web::main]
 async fn main() -> std::io::Result<()> {
-    let server_state = Arc::new(Box::new(server::NullSyncServer::new()));
+    let server_box: Box<dyn SyncServer> = Box::new(NullSyncServer::new());
+    let server_state = ServerState::new(server_box);
 
     HttpServer::new(move || {
         App::new()

sync-server/src/server/mod.rs

@@ -2,7 +2,7 @@ use crate::types::{AddVersionResult, ClientId, GetVersionResult, HistorySegment,
 use failure::Fallible;
 use taskchampion::Uuid;
 
-pub(crate) trait SyncServer {
+pub(crate) trait SyncServer: Sync + Send {
     fn get_child_version(
         &self,
         client_id: ClientId,
@@ -13,7 +13,7 @@ pub(crate) trait SyncServer {
         &self,
         client_id: ClientId,
         parent_version_id: VersionId,
-        history_segment: &HistorySegment,
+        history_segment: HistorySegment,
     ) -> Fallible<AddVersionResult>;
 }
 
@@ -45,8 +45,9 @@ impl SyncServer for NullSyncServer {
         &self,
         _client_id: ClientId,
         _parent_version_id: VersionId,
-        _history_segment: &HistorySegment,
+        _history_segment: HistorySegment,
     ) -> Fallible<AddVersionResult> {
-        Ok(AddVersionResult::Ok(Uuid::new_v4()))
+        //Ok(AddVersionResult::Ok(Uuid::new_v4()))
+        Ok(AddVersionResult::ExpectedParentVersion(Uuid::new_v4()))
     }
 }

sync-server/src/types.rs

@@ -1,4 +1,3 @@
-use serde::{Deserialize, Serialize};
 use taskchampion::Uuid;
 
 pub(crate) type HistorySegment = Vec<u8>;
@@ -6,7 +5,6 @@ pub(crate) type ClientId = Uuid;
 pub(crate) type VersionId = Uuid;
 
 /// Response to get_child_version
-#[derive(Serialize, Deserialize)]
 pub(crate) struct GetVersionResult {
     pub(crate) version_id: Uuid,
     pub(crate) parent_version_id: Uuid,
@@ -14,7 +12,6 @@ pub(crate) struct GetVersionResult {
 }
 
 /// Response to add_version
-#[derive(Serialize, Deserialize)]
 pub(crate) enum AddVersionResult {
     /// OK, version added with the given ID
     Ok(VersionId),