From e84871931f068a33b1d3e16eb43fd1e0163c7ba7 Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Thu, 26 Nov 2020 11:32:20 -0500 Subject: [PATCH] Refactor HTTP implementation of API methods --- Cargo.lock | 15 +++++- sync-server/Cargo.toml | 3 +- sync-server/src/api/add_version.rs | 63 ++++++++++++++++++------ sync-server/src/api/get_child_version.rs | 21 +++++++- sync-server/src/api/mod.rs | 10 ++++ sync-server/src/main.rs | 6 ++- sync-server/src/server/mod.rs | 9 ++-- sync-server/src/types.rs | 3 -- 8 files changed, 100 insertions(+), 30 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6f24b0e1f..4c744d952 100644 --- a/Cargo.lock +++ b/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", ] diff --git a/sync-server/Cargo.toml b/sync-server/Cargo.toml index d243e2647..3c638aa7c 100644 --- a/sync-server/Cargo.toml +++ b/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" } diff --git a/sync-server/src/api/add_version.rs b/sync-server/src/api/add_version.rs index cdc96db2a..40cb5fdd0 100644 --- a/sync-server/src/api/add_version.rs +++ b/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, web::Path((client_id, parent_version_id)): web::Path<(ClientId, VersionId)>, - body: web::Json, -) -> Result { + mut payload: web::Payload, +) -> Result { + // 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(""), + }) } diff --git a/sync-server/src/api/get_child_version.rs b/sync-server/src/api/get_child_version.rs index 4383a6fbc..0667ec034 100644 --- a/sync-server/src/api/get_child_version.rs +++ b/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, @@ -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")) } diff --git a/sync-server/src/api/mod.rs b/sync-server/src/api/mod.rs index cdab25fca..9dcca18bc 100644 --- a/sync-server/src/api/mod.rs +++ b/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>; diff --git a/sync-server/src/main.rs b/sync-server/src/main.rs index 191908fb2..7caa360f8 100644 --- a/sync-server/src/main.rs +++ b/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 = Box::new(NullSyncServer::new()); + let server_state = ServerState::new(server_box); HttpServer::new(move || { App::new() diff --git a/sync-server/src/server/mod.rs b/sync-server/src/server/mod.rs index df4377366..71961468b 100644 --- a/sync-server/src/server/mod.rs +++ b/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; } @@ -45,8 +45,9 @@ impl SyncServer for NullSyncServer { &self, _client_id: ClientId, _parent_version_id: VersionId, - _history_segment: &HistorySegment, + _history_segment: HistorySegment, ) -> Fallible { - Ok(AddVersionResult::Ok(Uuid::new_v4())) + //Ok(AddVersionResult::Ok(Uuid::new_v4())) + Ok(AddVersionResult::ExpectedParentVersion(Uuid::new_v4())) } } diff --git a/sync-server/src/types.rs b/sync-server/src/types.rs index f4b28901d..69dbe2fbc 100644 --- a/sync-server/src/types.rs +++ b/sync-server/src/types.rs @@ -1,4 +1,3 @@ -use serde::{Deserialize, Serialize}; use taskchampion::Uuid; pub(crate) type HistorySegment = Vec; @@ -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),