[blob] Move HTTP request/response types to comm-lib

Summary:
These types can (and generally should) be shared with the `BlobServiceClient` living in the comm-lib.
Unified them with types existing in comm-lib so request and response structs are now shared between client and server

Depends on D13616

Test Plan: cargo check

Reviewers: kamil, varun, will

Reviewed By: kamil

Subscribers: ashoat, tomek

Differential Revision: https://phab.comm.dev/D13617

services/blob/src/http/handlers/blob.rs

@@ -12,8 +12,10 @@ use actix_web::{
 };
 use async_stream::try_stream;
 use base64::Engine;
+use comm_lib::blob::types::http::{
+  AssignHolderRequest, AssignHolderResponse, RemoveHolderRequest,
+};
 use comm_lib::http::multipart;
-use serde::{Deserialize, Serialize};
 use tokio_stream::StreamExt;
 use tracing::{debug, info, instrument, trace, warn};
 use tracing_futures::Instrument;
@@ -114,24 +116,13 @@ pub async fn get_blob_handler(
   )
 }
 
-#[derive(Deserialize, Debug)]
-pub struct AssignHolderPayload {
-  holder: String,
-  blob_hash: String,
-}
-
-#[derive(Serialize)]
-struct AssignHolderResponnse {
-  data_exists: bool,
-}
-
 #[instrument(name = "assign_holder", skip(service))]
 pub async fn assign_holder_handler(
   service: web::Data<BlobService>,
-  payload: web::Json<AssignHolderPayload>,
+  payload: web::Json<AssignHolderRequest>,
 ) -> actix_web::Result<HttpResponse> {
   info!("Assign holder request");
-  let AssignHolderPayload { holder, blob_hash } = payload.into_inner();
+  let AssignHolderRequest { holder, blob_hash } = payload.into_inner();
   validate_identifier!(holder);
   validate_identifier!(blob_hash);
 
@@ -142,7 +133,7 @@ pub async fn assign_holder_handler(
 
   service.assign_holder(blob_hash, holder).await?;
 
-  let response = AssignHolderResponnse { data_exists };
+  let response = AssignHolderResponse { data_exists };
   Ok(HttpResponse::Ok().json(web::Json(response)))
 }
 
@@ -212,23 +203,13 @@ pub async fn upload_blob_handler(
   Ok(HttpResponse::NoContent().finish())
 }
 
-#[derive(Deserialize, Debug)]
-pub struct RemoveHolderPayload {
-  holder: String,
-  blob_hash: String,
-  /// If true, the blob will be deleted intantly
-  /// after the last holder is revoked.
-  #[serde(default)]
-  instant_delete: bool,
-}
-
 #[instrument(name = "remove_holder", skip(service))]
 pub async fn remove_holder_handler(
   service: web::Data<BlobService>,
-  payload: web::Json<RemoveHolderPayload>,
+  payload: web::Json<RemoveHolderRequest>,
 ) -> actix_web::Result<HttpResponse> {
   info!("Revoke holder request");
-  let RemoveHolderPayload {
+  let RemoveHolderRequest {
     holder,
     blob_hash,
     instant_delete,

services/blob/src/http/handlers/holders.rs

@@ -1,56 +1,30 @@
 use actix_web::error::ErrorBadRequest;
 use actix_web::{web, HttpResponse};
-use serde::{Deserialize, Serialize};
+use comm_lib::blob::types::http::{
+  AssignHoldersRequest, AssignHoldersResponse, BlobInfo,
+  HolderAssignmentResult, RemoveHoldersRequest, RemoveHoldersResponse,
+};
 use tracing::{info, instrument, trace, warn};
 
 use crate::service::BlobService;
 
-#[derive(Serialize, Deserialize, Debug)]
-#[serde(rename_all = "camelCase")]
-pub struct BlobHashAndHolder {
-  blob_hash: String,
-  holder: String,
-}
-
-#[derive(Deserialize, Debug)]
-#[serde(rename_all = "camelCase")]
-pub struct AssignHoldersPayload {
-  requests: Vec<BlobHashAndHolder>,
-}
-
-#[derive(Serialize, Debug)]
-#[serde(rename_all = "camelCase")]
-struct HolderAssignmentResult {
-  #[serde(flatten)]
-  request: BlobHashAndHolder,
-  success: bool,
-  data_exists: bool,
-  holder_already_exists: bool,
-}
-#[derive(Serialize, Debug)]
-#[serde(rename_all = "camelCase")]
-struct AssignHoldersResponse {
-  results: Vec<HolderAssignmentResult>,
-}
-
 #[instrument(name = "assign_multiple_holders", skip_all)]
 pub async fn assign_holders_handler(
   service: web::Data<BlobService>,
-  payload: web::Json<AssignHoldersPayload>,
+  payload: web::Json<AssignHoldersRequest>,
 ) -> actix_web::Result<HttpResponse> {
   use crate::database::DBError;
   use crate::service::BlobServiceError;
 
-  let AssignHoldersPayload { requests } = payload.into_inner();
+  let AssignHoldersRequest { requests } = payload.into_inner();
   info!("Assign holder request for {} holders", requests.len());
   validate_request(&requests)?;
 
   let blob_hashes = requests.iter().map(|it| &it.blob_hash).collect();
   let existing_blobs = service.find_existing_blobs(blob_hashes).await?;
-
   let mut results = Vec::with_capacity(requests.len());
   for item in requests {
-    let BlobHashAndHolder { blob_hash, holder } = &item;
+    let BlobInfo { blob_hash, holder } = &item;
     let data_exists = existing_blobs.contains(blob_hash);
     let result = match service.assign_holder(blob_hash, holder).await {
       Ok(()) => HolderAssignmentResult {
@@ -84,26 +58,12 @@ pub async fn assign_holders_handler(
   Ok(HttpResponse::Ok().json(web::Json(response)))
 }
 
-#[derive(Deserialize, Debug)]
-#[serde(rename_all = "camelCase")]
-pub struct RemoveHoldersPayload {
-  requests: Vec<BlobHashAndHolder>,
-  #[serde(default)]
-  instant_delete: bool,
-}
-
-#[derive(Serialize, Debug)]
-#[serde(rename_all = "camelCase")]
-pub struct RemoveHoldersResponse {
-  failed_requests: Vec<BlobHashAndHolder>,
-}
-
 #[instrument(name = "remove_multiple_holders", skip_all)]
 pub async fn remove_holders_handler(
   service: web::Data<BlobService>,
-  payload: web::Json<RemoveHoldersPayload>,
+  payload: web::Json<RemoveHoldersRequest>,
 ) -> actix_web::Result<HttpResponse> {
-  let RemoveHoldersPayload {
+  let RemoveHoldersRequest {
     requests,
     instant_delete,
   } = payload.into_inner();
@@ -121,7 +81,7 @@ pub async fn remove_holders_handler(
   for item in requests {
     trace!("Removing item: {:?}", &item);
 
-    let BlobHashAndHolder { holder, blob_hash } = &item;
+    let BlobInfo { holder, blob_hash } = &item;
     if let Err(err) = service
       .revoke_holder(blob_hash, holder, instant_delete)
       .await
@@ -139,12 +99,11 @@ pub async fn remove_holders_handler(
  * have invalid format. See [`comm_lib::tools::is_valid_identifier`] for
  * valid format conditions
  */
-fn validate_request(items: &[BlobHashAndHolder]) -> actix_web::Result<()> {
+fn validate_request(items: &[BlobInfo]) -> actix_web::Result<()> {
   use comm_lib::tools::is_valid_identifier;
-  let all_valid =
-    items.iter().all(|BlobHashAndHolder { holder, blob_hash }| {
-      is_valid_identifier(holder) && is_valid_identifier(blob_hash)
-    });
+  let all_valid = items.iter().all(|BlobInfo { holder, blob_hash }| {
+    is_valid_identifier(holder) && is_valid_identifier(blob_hash)
+  });
 
   if !all_valid {
     return Err(ErrorBadRequest("One or more requests have invalid format"));

shared/comm-lib/src/blob/client.rs

@@ -13,7 +13,12 @@ pub use reqwest::Error as ReqwestError;
 pub use reqwest::StatusCode;
 pub use reqwest::Url;
 
-use crate::auth::{AuthorizationCredential, UserIdentity};
+use crate::{
+  auth::{AuthorizationCredential, UserIdentity},
+  blob::types::http::{
+    AssignHolderRequest, AssignHolderResponse, RemoveHolderRequest,
+  },
+};
 
 #[derive(From, Error, Debug, Display)]
 pub enum BlobServiceError {
@@ -216,9 +221,10 @@ impl BlobServiceClient {
     debug!("Revoke holder request");
     let url = self.get_blob_url(None)?;
 
-    let payload = RevokeHolderRequest {
+    let payload = RemoveHolderRequest {
       holder: holder.to_string(),
       blob_hash: blob_hash.to_string(),
+      instant_delete: false,
     };
     debug!("Request payload: {:?}", payload);
 
@@ -395,18 +401,6 @@ fn handle_http_error(status_code: StatusCode) -> BlobServiceError {
 
 type BlobResult<T> = Result<T, BlobServiceError>;
 
-#[derive(serde::Deserialize)]
-struct AssignHolderResponse {
-  data_exists: bool,
-}
-#[derive(Debug, serde::Serialize)]
-struct AssignHolderRequest {
-  blob_hash: String,
-  holder: String,
-}
-// they have the same layout so we can simply alias
-type RevokeHolderRequest = AssignHolderRequest;
-
 #[cfg(feature = "http")]
 impl crate::http::auth_service::HttpAuthenticatedService for BlobServiceClient {
   fn make_authenticated(

shared/comm-lib/src/blob/types.rs

@@ -2,8 +2,73 @@ use derive_more::Constructor;
 use hex::ToHex;
 use sha2::{Digest, Sha256};
 
+pub mod http {
+  use serde::{Deserialize, Serialize};
+
+  pub use super::BlobInfo;
+
+  // Assign multiple holders
+  #[derive(Serialize, Deserialize, Debug)]
+  #[serde(rename_all = "camelCase")]
+  pub struct AssignHoldersRequest {
+    pub requests: Vec<BlobInfo>,
+  }
+
+  #[derive(Serialize, Deserialize, Debug)]
+  #[serde(rename_all = "camelCase")]
+  pub struct HolderAssignmentResult {
+    #[serde(flatten)]
+    pub request: BlobInfo,
+    pub success: bool,
+    pub data_exists: bool,
+    pub holder_already_exists: bool,
+  }
+  #[derive(Serialize, Deserialize, Debug)]
+  #[serde(rename_all = "camelCase")]
+  pub struct AssignHoldersResponse {
+    pub results: Vec<HolderAssignmentResult>,
+  }
+
+  // Remove multiple holders
+  #[derive(Deserialize, Debug)]
+  #[serde(rename_all = "camelCase")]
+  pub struct RemoveHoldersRequest {
+    pub requests: Vec<BlobInfo>,
+    #[serde(default)]
+    pub instant_delete: bool,
+  }
+  #[derive(Serialize, Debug)]
+  #[serde(rename_all = "camelCase")]
+  pub struct RemoveHoldersResponse {
+    pub failed_requests: Vec<BlobInfo>,
+  }
+
+  // Single holder endpoint types
+
+  #[derive(Serialize, Deserialize, Debug)]
+  pub struct AssignHolderRequest {
+    pub blob_hash: String,
+    pub holder: String,
+  }
+  #[derive(Serialize, Deserialize, Debug)]
+  pub struct AssignHolderResponse {
+    pub data_exists: bool,
+  }
+
+  #[derive(Serialize, Deserialize, Debug)]
+  pub struct RemoveHolderRequest {
+    pub blob_hash: String,
+    pub holder: String,
+    /// If true, the blob will be deleted intantly
+    /// after the last holder is revoked.
+    #[serde(default)]
+    pub instant_delete: bool,
+  }
+}
+
 /// Blob owning information - stores both blob_hash and holder
-#[derive(Clone, Debug, Constructor)]
+#[derive(Clone, Debug, Constructor, serde::Serialize, serde::Deserialize)]
+#[serde(rename_all = "camelCase")]
 pub struct BlobInfo {
   pub blob_hash: String,
   pub holder: String,