From 5416f8afc685d08a2688350fe7914489dc83f1f6 Mon Sep 17 00:00:00 2001 From: Tobias de Bruijn Date: Fri, 5 Jul 2024 13:47:45 +0200 Subject: [PATCH] Resolve some comments --- server/chroma/src/config.rs | 15 +++++- server/chroma/src/main.rs | 1 + server/chroma/src/routes/v1/photo/create.rs | 7 --- server/dal/src/database/photo.rs | 59 ++------------------- server/dal/src/storage_engine.rs | 58 ++++++++++++-------- 5 files changed, 52 insertions(+), 88 deletions(-) diff --git a/server/chroma/src/config.rs b/server/chroma/src/config.rs index 78279be..be6b312 100644 --- a/server/chroma/src/config.rs +++ b/server/chroma/src/config.rs @@ -38,6 +38,10 @@ pub struct Config { /// but should be `false` or left unspecified when targeting /// Amazon S3. pub s3_force_path_style: Option, + /// Create a bucket on startup. This should only + /// be used when working with MinIO. + /// The provided access key should have bucket creation privileges. + pub s3_create_bucket_on_startup: Option, /// OAuth2 client ID created in Koala pub koala_client_id: String, @@ -152,13 +156,20 @@ impl Config { .unwrap_or(&self.koala_base_uri) } - /// Force S3 path styles instead of virtual hosts + /// Force S3 path styles instead of virtual hosts. /// - /// See also: `s3_force_path_style` field + /// See also: `s3_force_path_style` field. pub fn s3_force_path_style(&self) -> bool { self.s3_force_path_style.unwrap_or(false) } + /// Create an S3 bucket on application startup. + /// + /// See also: `s3_create_bucket_on_startup` field. + pub fn s3_create_bucket_on_startup(&self) -> bool { + self.s3_create_bucket_on_startup.unwrap_or(false) + } + /// Get configured service tokens pub fn service_tokens(&self) -> Vec<&str> { self.service_tokens.split(',').collect() diff --git a/server/chroma/src/main.rs b/server/chroma/src/main.rs index e9f988a..5137ab8 100644 --- a/server/chroma/src/main.rs +++ b/server/chroma/src/main.rs @@ -53,6 +53,7 @@ async fn main() -> Result<()> { access_key_id: config.s3_access_key_id.clone().unwrap(), secret_access_key: config.s3_secret_access_key.clone().unwrap(), use_path_style: config.s3_force_path_style(), + create_bucket: config.s3_create_bucket_on_startup(), }) .await?; diff --git a/server/chroma/src/routes/v1/photo/create.rs b/server/chroma/src/routes/v1/photo/create.rs index 243e360..14f243d 100644 --- a/server/chroma/src/routes/v1/photo/create.rs +++ b/server/chroma/src/routes/v1/photo/create.rs @@ -233,13 +233,6 @@ fn resize_and_save( ); return; } - - match photo.set_quality_created(quality, true).await { - Ok(_) => {} - Err(e) => { - warn!("Failed to set quality created flag for photo: {e}"); - } - } }); } diff --git a/server/dal/src/database/photo.rs b/server/dal/src/database/photo.rs index a8ebe03..f69ab0e 100644 --- a/server/dal/src/database/photo.rs +++ b/server/dal/src/database/photo.rs @@ -211,67 +211,14 @@ impl<'a> Photo<'a> { } /// Check whether an image quality has been created yet. - /// This will always return true for [PhotoQuality::Original]. /// /// # Errors /// /// If a database error occurs pub async fn is_quality_created(&self, quality: &PhotoQuality) -> DbResult { - match quality { - PhotoQuality::Original => Ok(true), - PhotoQuality::W400 => self.is_quality_w400_created().await, - PhotoQuality::W1600 => self.is_quality_w1600_created().await, - } - } - - async fn is_quality_w400_created(&self) -> DbResult { - let value: bool = - sqlx::query_scalar("SELECT w400_created FROM photo_metadata WHERE id = $1") - .bind(&self.id) - .fetch_one(&**self.db) - .await?; - Ok(value) - } - - async fn is_quality_w1600_created(&self) -> DbResult { - let value: bool = - sqlx::query_scalar("SELECT w1600_created FROM photo_metadata WHERE id = $1") - .bind(&self.id) - .fetch_one(&**self.db) - .await?; - Ok(value) - } - - /// Set whether an image quality has been created or not. - /// This is a no-op for [PhotoQuality::Original]. - /// - /// # Errors - /// - /// If a database error occurs - pub async fn set_quality_created(&self, quality: PhotoQuality, created: bool) -> DbResult<()> { - match quality { - PhotoQuality::Original => Ok(()), - PhotoQuality::W400 => self.set_quality_w400_created(created).await, - PhotoQuality::W1600 => self.set_quality_w1600_created(created).await, - } - } - - async fn set_quality_w400_created(&self, created: bool) -> DbResult<()> { - sqlx::query("UPDATE photo_metadata SET w400_created = $1 WHERE id = $2") - .bind(created) - .bind(&self.id) - .execute(&**self.db) - .await?; - Ok(()) - } - - async fn set_quality_w1600_created(&self, created: bool) -> DbResult<()> { - sqlx::query("UPDATE photo_metadata SET w1600_created = $1 WHERE id = $2") - .bind(created) - .bind(&self.id) - .execute(&**self.db) - .await?; - Ok(()) + PhotoS3Url::get_for_photo(&self.db, &self.id, quality) + .await + .map(|maybe_url| maybe_url.is_some()) } } diff --git a/server/dal/src/storage_engine.rs b/server/dal/src/storage_engine.rs index 75deb3a..e25b5a7 100644 --- a/server/dal/src/storage_engine.rs +++ b/server/dal/src/storage_engine.rs @@ -3,8 +3,9 @@ use std::ops::Deref; use aws_credential_types::Credentials; use aws_sdk_s3::types::ByteStream; use aws_sdk_s3::{Client, Config}; +use aws_smithy_http::result::SdkError; use aws_types::region::Region; -use tracing::instrument; +use tracing::{info, instrument}; use crate::database::PhotoQuality; use crate::storage_engine::error::StorageError; @@ -57,12 +58,15 @@ pub struct S3Config { pub access_key_id: String, pub secret_access_key: String, pub use_path_style: bool, + pub create_bucket: bool, } #[derive(Debug, Clone)] pub struct Storage { client: Client, bucket_name: String, + use_path_style: bool, + endpoint_url: String, } impl Deref for Storage { @@ -78,7 +82,7 @@ impl Storage { let client = Client::from_conf( Config::builder() .force_path_style(config.use_path_style) - .endpoint_url(config.endpoint_url) + .endpoint_url(config.endpoint_url.clone()) .region(Some(Region::new(config.region))) .credentials_provider(Credentials::from_keys( config.access_key_id, @@ -88,11 +92,17 @@ impl Storage { .build(), ); - Self::setup_bucket(&client, &config.bucket_name).await?; + if config.create_bucket && !Self::bucket_exists(&client, &config.bucket_name).await? { + Self::create_bucket(&client, &config.bucket_name).await?; + } + + Self::set_bucket_policy(&client, &config.bucket_name).await?; Ok(Storage { client, bucket_name: config.bucket_name, + endpoint_url: config.endpoint_url, + use_path_style: config.use_path_style, }) } @@ -101,25 +111,12 @@ impl Storage { photo_id: S, photo_quality: &PhotoQuality, ) -> Result { - let url = format!( - "https://{}.s3.amazonaws.com/{}_{}", - self.bucket_name, - photo_id.as_ref(), - photo_quality - ); - // - // let response = self - // .client - // .get_object() - // .bucket(&self.bucket_name) - // .key(Self::format_id_with_quality( - // photo_id.as_ref(), - // photo_quality, - // )) - // .send(PresigningConfig::builder().build()?) - // .await?; - // - // let url = String::from(response.uri().to_string().split('?').next().unwrap()); + let qstring = Self::format_id_with_quality(photo_id.as_ref(), photo_quality); + let url = if self.use_path_style { + format!("{}/{}/{}", self.endpoint_url, self.bucket_name, qstring) + } else { + format!("{}/{}", self.endpoint_url, qstring) + }; Ok(url) } @@ -184,8 +181,23 @@ impl Storage { Ok(()) } + async fn create_bucket(client: &Client, bucket_name: &String) -> Result<(), StorageError> { + client.create_bucket().bucket(bucket_name).send().await?; + Ok(()) + } + + async fn bucket_exists(client: &Client, bucket_name: &String) -> Result { + match client.head_bucket().bucket(bucket_name).send().await { + Ok(_) => Ok(true), + Err(SdkError::ServiceError(e)) if e.err().is_not_found() => Ok(false), + Err(e) => Err(e.into()), + } + } + #[instrument(skip_all)] - async fn setup_bucket(client: &Client, bucket_name: &String) -> Result<(), StorageError> { + async fn set_bucket_policy(client: &Client, bucket_name: &String) -> Result<(), StorageError> { + info!("Setting bucket policy"); + client .put_bucket_policy() .bucket(bucket_name)