Resolve some comments

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<bool>,
+    /// 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<bool>,
 
     /// 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()

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?;
 

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}");
-            }
-        }
     });
 }
 

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<bool> {
-        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<bool> {
-        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<bool> {
-        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())
     }
 }
 

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<String, error::StorageError> {
-        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<bool, StorageError> {
+        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)