From ca56e501a229d1c4b9dd34952d81b5255167986a Mon Sep 17 00:00:00 2001
From: neri <neri@ctdo.de>
Date: Tue, 22 Nov 2022 21:32:04 +0100
Subject: [PATCH] refactor upload endpoint

---
 src/upload.rs | 101 ++++++++++++++++++++++++++------------------------
 1 file changed, 53 insertions(+), 48 deletions(-)

diff --git a/src/upload.rs b/src/upload.rs
index de2910c..bd2584e 100644
--- a/src/upload.rs
+++ b/src/upload.rs
@@ -36,86 +36,82 @@ pub async fn upload(
     expiry_watch_sender: web::Data<Sender<()>>,
     config: web::Data<Config>,
 ) -> Result<HttpResponse, Error> {
-    let (file_id, file_name) = create_unique_file(&config).await.map_err(|file_err| {
+    let (file_id, file_path) = create_unique_file(&config).await.map_err(|file_err| {
         log::error!("could not create file {:?}", file_err);
         error::ErrorInternalServerError("could not create file")
     })?;
 
-    let UploadConfig {
-        original_name,
-        content_type,
-        valid_till,
-        delete_on_download,
-    } = multipart::parse_multipart(payload, &file_name, &config).await?;
-
-    let file_name = original_name.clone().unwrap_or_else(|| {
+    let upload_config = multipart::parse_multipart(payload, &file_path, &config).await?;
+    let file_name = upload_config.original_name.clone().unwrap_or_else(|| {
         format!(
             "{file_id}.{}",
-            mime_relations::get_extension(&content_type).unwrap_or("txt")
+            mime_relations::get_extension(&upload_config.content_type).unwrap_or("txt")
         )
     });
-    let db_insert = sqlx::query(
-        "INSERT INTO Files (file_id, file_name, content_type, valid_till, delete_on_download) \
-        VALUES ($1, $2, $3, $4, $5)",
-    )
-    .bind(&file_id)
-    .bind(&file_name)
-    .bind(&content_type.to_string())
-    .bind(valid_till)
-    .bind(delete_on_download)
-    .execute(db.as_ref())
-    .await;
-    if let Err(db_err) = db_insert {
-        log::error!("could not insert into datebase {:?}", db_err);
-        fs::remove_file(file_name).await.map_err(|file_err| {
-            log::error!("could not remove file {:?}", file_err);
-            error::ErrorInternalServerError(
-                "could not insert file into database; could not remove file",
-            )
-        })?;
-        return Err(error::ErrorInternalServerError(
-            "could not insert file into database",
-        ));
-    }
+
+    insert_file_metadata(&file_id, file_name, &upload_config, db).await?;
 
     log::info!(
         "{} create new file {} (valid_till: {}, content_type: {}, delete_on_download: {})",
         req.connection_info().realip_remote_addr().unwrap_or("-"),
         file_id,
-        valid_till,
-        content_type,
-        delete_on_download
+        upload_config.content_type,
+        upload_config.delete_on_download,
+        upload_config.valid_till
     );
 
     expiry_watch_sender.send(()).await.unwrap();
 
-    let redirect = if let Some(original_name) = original_name.as_ref() {
-        let encoded_name = urlencoding::encode(original_name);
-        format!("/upload/{file_id}/{encoded_name}")
-    } else {
-        format!("/upload/{file_id}")
-    };
-
-    let url = get_file_url(&req, &file_id, original_name.as_deref());
+    let redirect = get_redirect_url(&file_id, upload_config.original_name.as_deref());
+    let url = get_file_url(&req, &file_id, upload_config.original_name.as_deref());
     Ok(HttpResponse::SeeOther()
         .insert_header((LOCATION, redirect))
         .body(format!("{url}\n")))
 }
 
+async fn insert_file_metadata(
+    file_id: &String,
+    file_name: String,
+    upload_config: &UploadConfig,
+    db: web::Data<sqlx::Pool<sqlx::Postgres>>,
+) -> Result<(), Error> {
+    let db_insert = sqlx::query(
+        "INSERT INTO Files (file_id, file_name, content_type, valid_till, delete_on_download) \
+        VALUES ($1, $2, $3, $4, $5)",
+    )
+    .bind(file_id)
+    .bind(&file_name)
+    .bind(&upload_config.content_type.to_string())
+    .bind(upload_config.valid_till)
+    .bind(upload_config.delete_on_download)
+    .execute(db.as_ref())
+    .await;
+    if let Err(db_err) = db_insert {
+        log::error!("could not insert into datebase {:?}", db_err);
+        if let Err(file_err) = fs::remove_file(file_name).await {
+            log::error!("could not remove file {:?}", file_err);
+        }
+        return Err(error::ErrorInternalServerError(
+            "could not insert file into database",
+        ));
+    }
+    Ok(())
+}
+
 async fn create_unique_file(
     config: &web::Data<Config>,
 ) -> Result<(String, PathBuf), std::io::Error> {
     loop {
         let file_id = gen_file_id();
-        let mut file_name = config.files_dir.clone();
-        file_name.push(&file_id);
+        let mut file_path = config.files_dir.clone();
+        file_path.push(&file_id);
         match OpenOptions::new()
             .write(true)
             .create_new(true)
-            .open(&file_name)
+            .open(&file_path)
             .await
         {
-            Ok(_) => return Ok((file_id, file_name)),
+            Ok(_) => return Ok((file_id, file_path)),
             Err(error) if error.kind() == ErrorKind::AlreadyExists => continue,
             Err(error) => return Err(error),
         }
@@ -140,6 +136,15 @@ fn get_file_url(req: &HttpRequest, id: &str, name: Option<&str>) -> String {
     }
 }
 
+fn get_redirect_url(id: &str, name: Option<&str>) -> String {
+    if let Some(name) = name {
+        let encoded_name = urlencoding::encode(name);
+        format!("/upload/{id}/{encoded_name}")
+    } else {
+        format!("/upload/{id}")
+    }
+}
+
 pub async fn uploaded(
     req: HttpRequest,
     path: web::Path<(String, Option<String>)>,