From 013d700586f9aef16ff12883019e7142b0195413 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Klocek?= Date: Sun, 9 Jul 2023 18:46:13 +0200 Subject: [PATCH] [blob-service] Delete old database code from http module Summary: Part of [[ https://linear.app/comm/issue/ENG-4269/implement-the-database-schema | ENG-4269 ]]. Removed all code related to the old database from the HTTP server part of the service. (gRPC still uses it). Renamed `http/context.rs` to `http/errors.rs` because the only code left there is error handling. Depends on D8458. Test Plan: Blob service builds and runs, integration tests pass Reviewers: michal, varun, jon, patryk Reviewed By: varun Subscribers: ashoat, tomek Differential Revision: https://phab.comm.dev/D8459 --- services/blob/src/database/client.rs | 8 +-- .../blob/src/http/{context.rs => errors.rs} | 66 +------------------ services/blob/src/http/handlers/blob.rs | 10 +-- services/blob/src/http/mod.rs | 20 +----- services/blob/src/main.rs | 2 +- 5 files changed, 9 insertions(+), 97 deletions(-) rename services/blob/src/http/{context.rs => errors.rs} (58%) diff --git a/services/blob/src/database/client.rs b/services/blob/src/database/client.rs index e0fcf2c824..160687708f 100644 --- a/services/blob/src/database/client.rs +++ b/services/blob/src/database/client.rs @@ -1,12 +1,6 @@ -// TODO: Remove this when possible -#![allow(unused)] - use aws_sdk_dynamodb::{ operation::put_item::PutItemOutput, - types::{ - AttributeValue, Delete, DeleteRequest, KeysAndAttributes, PutRequest, - TransactWriteItem, Update, WriteRequest, - }, + types::{AttributeValue, Delete, TransactWriteItem, Update}, Error as DynamoDBError, }; use chrono::Utc; diff --git a/services/blob/src/http/context.rs b/services/blob/src/http/errors.rs similarity index 58% rename from services/blob/src/http/context.rs rename to services/blob/src/http/errors.rs index 741dc49d5a..3c0cb5724a 100644 --- a/services/blob/src/http/context.rs +++ b/services/blob/src/http/errors.rs @@ -1,75 +1,15 @@ -use crate::database::errors::{BlobDBError, Error as DBError}; -use crate::database::old::{BlobItem, DatabaseClient, ReverseIndexItem}; -use crate::s3::{Error as S3Error, S3Client, S3Path}; -use crate::service::BlobServiceError; use actix_web::error::{ ErrorBadRequest, ErrorConflict, ErrorInternalServerError, ErrorNotFound, ErrorServiceUnavailable, }; use actix_web::{Error as HttpError, HttpResponse, ResponseError}; -use anyhow::Result; use aws_sdk_dynamodb::Error as DynamoDBError; use http::StatusCode; use tracing::{debug, error, trace, warn}; -/// This structure is passed to every HTTP request handler -/// It should be cloneable because each HTTP worker thread receives a copy -#[derive(Clone)] -pub struct AppContext { - pub db: DatabaseClient, - pub s3: S3Client, -} - -impl AppContext { - pub async fn find_s3_path_by_reverse_index( - &self, - reverse_index_item: &ReverseIndexItem, - ) -> Result { - let blob_hash = &reverse_index_item.blob_hash; - match self.db.find_blob_item(&blob_hash).await { - Ok(Some(BlobItem { s3_path, .. })) => Ok(s3_path), - Ok(None) => { - debug!("No blob found for {:?}", reverse_index_item); - Err(ErrorNotFound("blob not found")) - } - Err(err) => Err(handle_db_error(err)), - } - } -} - -pub fn handle_db_error(db_error: DBError) -> HttpError { - match db_error { - DBError::AwsSdk(DynamoDBError::InternalServerError(_)) - | DBError::AwsSdk(DynamoDBError::ProvisionedThroughputExceededException( - _, - )) - | DBError::AwsSdk(DynamoDBError::RequestLimitExceeded(_)) => { - warn!("AWS transient error occurred"); - ErrorServiceUnavailable("please retry") - } - DBError::Blob(blob_err) => { - error!("Encountered Blob database error: {}", blob_err); - ErrorInternalServerError("Internal error") - } - err => { - error!("Encountered an unexpected error: {}", err); - ErrorInternalServerError("unexpected error") - } - } -} - -pub fn handle_s3_error(s3_error: S3Error) -> HttpError { - match s3_error { - S3Error::EmptyUpload => { - warn!("Empty upload. Aborting"); - ErrorBadRequest("Empty upload") - } - err => { - error!("Encountered S3 error: {:?}", err); - ErrorInternalServerError("Internal error") - } - } -} +use crate::database::errors::{BlobDBError, Error as DBError}; +use crate::s3::Error as S3Error; +use crate::service::BlobServiceError; pub(super) fn handle_blob_service_error(err: &BlobServiceError) -> HttpError { trace!("Handling blob service error: {:?}", err); diff --git a/services/blob/src/http/handlers/blob.rs b/services/blob/src/http/handlers/blob.rs index c4a848191d..25ca401a74 100644 --- a/services/blob/src/http/handlers/blob.rs +++ b/services/blob/src/http/handlers/blob.rs @@ -1,16 +1,10 @@ -#![allow(unused)] - -use crate::constants::S3_MULTIPART_UPLOAD_MINIMUM_CHUNK_SIZE; -use crate::database::old::{BlobItem, ReverseIndexItem}; -use crate::http::context::{handle_blob_service_error, handle_s3_error}; +use crate::http::errors::handle_blob_service_error; use crate::service::BlobService; use crate::tools::BoxedError; use crate::validate_identifier; -use super::{handle_db_error, AppContext}; use actix_web::error::{ - ErrorBadRequest, ErrorConflict, ErrorInternalServerError, ErrorNotFound, - ErrorRangeNotSatisfiable, + ErrorBadRequest, ErrorInternalServerError, ErrorRangeNotSatisfiable, }; use actix_web::{ http::header::{ByteRangeSpec, Range}, diff --git a/services/blob/src/http/mod.rs b/services/blob/src/http/mod.rs index 5688dc66fd..a58c6b66a8 100644 --- a/services/blob/src/http/mod.rs +++ b/services/blob/src/http/mod.rs @@ -1,5 +1,3 @@ -use crate::database::old::DatabaseClient; -use crate::s3::S3Client; use crate::{config::CONFIG, service::BlobService}; use actix_cors::Cors; @@ -7,15 +5,11 @@ use actix_web::{web, App, HttpServer}; use anyhow::Result; use tracing::info; -mod context; -use context::AppContext; +mod errors; mod utils; mod handlers { pub(super) mod blob; - - // convenience exports to be used in handlers - use super::context::{handle_db_error, AppContext}; } fn cors_config() -> Cors { @@ -34,25 +28,15 @@ fn cors_config() -> Cors { .expose_any_header() } -pub async fn run_http_server( - db_client: DatabaseClient, - s3_client: S3Client, - blob_service: BlobService, -) -> Result<()> { +pub async fn run_http_server(blob_service: BlobService) -> Result<()> { info!( "Starting HTTP server listening at port {}", CONFIG.http_port ); HttpServer::new(move || { - // context that is passed to every handler - let ctx = AppContext { - db: db_client.to_owned(), - s3: s3_client.to_owned(), - }; App::new() .wrap(tracing_actix_web::TracingLogger::default()) .wrap(cors_config()) - .app_data(web::Data::new(ctx)) .app_data(web::Data::new(blob_service.to_owned())) .service( web::resource("/blob/{holder}") diff --git a/services/blob/src/main.rs b/services/blob/src/main.rs index fde55bece0..15e0482204 100644 --- a/services/blob/src/main.rs +++ b/services/blob/src/main.rs @@ -43,7 +43,7 @@ async fn main() -> Result<()> { ); tokio::select! { - http_result = crate::http::run_http_server(db.clone(), s3.clone(), service) => http_result, + http_result = crate::http::run_http_server(service) => http_result, grpc_result = crate::grpc::run_grpc_server(db, s3) => grpc_result, } }