From 8a0b7c05d300e2ce157eeaf863c29403ee86a748 Mon Sep 17 00:00:00 2001 From: Dessalines Date: Wed, 8 May 2024 16:12:34 -0400 Subject: [PATCH] Refactoring image inserts to use transactions. --- crates/api_common/src/request.rs | 6 +- crates/api_common/src/utils.rs | 10 +-- crates/db_schema/src/impls/images.rs | 61 +++++++++++++------ crates/db_schema/src/schema.rs | 1 - crates/db_schema/src/source/images.rs | 1 - crates/routes/src/images.rs | 35 +++++------ .../up.sql | 3 +- 7 files changed, 66 insertions(+), 51 deletions(-) diff --git a/crates/api_common/src/request.rs b/crates/api_common/src/request.rs index bca48b8ca..1f46d2c58 100644 --- a/crates/api_common/src/request.rs +++ b/crates/api_common/src/request.rs @@ -11,7 +11,7 @@ use encoding_rs::{Encoding, UTF_8}; use lemmy_db_schema::{ newtypes::DbUrl, source::{ - images::{ImageDetails, ImageDetailsForm, LocalImage, LocalImageForm}, + images::{ImageDetailsForm, LocalImage, LocalImageForm}, local_site::LocalSite, post::{Post, PostUpdateForm}, }, @@ -346,14 +346,12 @@ async fn generate_pictrs_thumbnail(image_url: &Url, context: &LemmyContext) -> L pictrs_alias: image.file.clone(), pictrs_delete_token: image.delete_token.clone(), }; - LocalImage::create(&mut context.pool(), &form).await?; - let protocol_and_hostname = context.settings().get_protocol_and_hostname(); let thumbnail_url = image.thumbnail_url(&protocol_and_hostname)?; // Also store the details for the image let details_form = image.details.build_image_details_form(&thumbnail_url); - ImageDetails::create(&mut context.pool(), &details_form).await?; + LocalImage::create(&mut context.pool(), &form, &details_form).await?; Ok(thumbnail_url) } else { diff --git a/crates/api_common/src/utils.rs b/crates/api_common/src/utils.rs index d0adb73ad..3f3162e20 100644 --- a/crates/api_common/src/utils.rs +++ b/crates/api_common/src/utils.rs @@ -16,7 +16,7 @@ use lemmy_db_schema::{ community::{Community, CommunityModerator, CommunityUpdateForm}, community_block::CommunityBlock, email_verification::{EmailVerification, EmailVerificationForm}, - images::{ImageDetails, RemoteImage}, + images::RemoteImage, instance::Instance, instance_block::InstanceBlock, local_site::LocalSite, @@ -938,15 +938,13 @@ pub async fn process_markdown( // Create images and image detail rows for link in links { - RemoteImage::create(&mut context.pool(), &link).await?; - // Insert image details for the remote image let details_res = fetch_pictrs_proxied_image_details(&link, context).await; if let Ok(details) = details_res { let proxied = build_proxied_image_url(&link, &context.settings().get_protocol_and_hostname())?; let details_form = details.build_image_details_form(&proxied); - ImageDetails::create(&mut context.pool(), &details_form).await?; + RemoteImage::create(&mut context.pool(), &details_form).await?; } } Ok(text) @@ -984,14 +982,12 @@ async fn proxy_image_link_internal( } else if image_mode == PictrsImageMode::ProxyAllImages { let proxied = build_proxied_image_url(&link, &context.settings().get_protocol_and_hostname())?; - RemoteImage::create(&mut context.pool(), &link).await?; - // This should fail softly, since pictrs might not even be running let details_res = fetch_pictrs_proxied_image_details(&link, context).await; if let Ok(details) = details_res { let details_form = details.build_image_details_form(&proxied); - ImageDetails::create(&mut context.pool(), &details_form).await?; + RemoteImage::create(&mut context.pool(), &details_form).await?; } Ok(proxied.into()) diff --git a/crates/db_schema/src/impls/images.rs b/crates/db_schema/src/impls/images.rs index c699329db..d11b4ec05 100644 --- a/crates/db_schema/src/impls/images.rs +++ b/crates/db_schema/src/impls/images.rs @@ -19,15 +19,29 @@ use diesel::{ OptionalExtension, QueryDsl, }; -use diesel_async::RunQueryDsl; -use url::Url; +use diesel_async::{AsyncPgConnection, RunQueryDsl}; impl LocalImage { - pub async fn create(pool: &mut DbPool<'_>, form: &LocalImageForm) -> Result { + pub async fn create( + pool: &mut DbPool<'_>, + form: &LocalImageForm, + image_details_form: &ImageDetailsForm, + ) -> Result { let conn = &mut get_conn(pool).await?; - insert_into(local_image::table) - .values(form) - .get_result::(conn) + conn + .build_transaction() + .run(|conn| { + Box::pin(async move { + let local_insert = insert_into(local_image::table) + .values(form) + .get_result::(conn) + .await; + + ImageDetails::create(conn, image_details_form).await?; + + local_insert + }) as _ + }) .await } @@ -45,15 +59,26 @@ impl LocalImage { } impl RemoteImage { - pub async fn create(pool: &mut DbPool<'_>, link_: &Url) -> Result { + pub async fn create(pool: &mut DbPool<'_>, form: &ImageDetailsForm) -> Result { let conn = &mut get_conn(pool).await?; - let form = RemoteImageForm { - link: link_.clone().into(), - }; - insert_into(remote_image::table) - .values(form) - .on_conflict_do_nothing() - .execute(conn) + conn + .build_transaction() + .run(|conn| { + Box::pin(async move { + let remote_image_form = RemoteImageForm { + link: form.link.clone(), + }; + let remote_insert = insert_into(remote_image::table) + .values(remote_image_form) + .on_conflict_do_nothing() + .execute(conn) + .await; + + ImageDetails::create(conn, form).await?; + + remote_insert + }) as _ + }) .await } @@ -73,12 +98,14 @@ impl RemoteImage { } impl ImageDetails { - pub async fn create(pool: &mut DbPool<'_>, form: &ImageDetailsForm) -> Result { - let conn = &mut get_conn(pool).await?; + pub(crate) async fn create( + conn: &mut AsyncPgConnection, + form: &ImageDetailsForm, + ) -> Result { insert_into(image_details::table) .values(form) .on_conflict_do_nothing() - .get_result::(conn) + .get_result::(conn) .await } } diff --git a/crates/db_schema/src/schema.rs b/crates/db_schema/src/schema.rs index 73a1b109a..c3102b578 100644 --- a/crates/db_schema/src/schema.rs +++ b/crates/db_schema/src/schema.rs @@ -315,7 +315,6 @@ diesel::table! { width -> Int4, height -> Int4, content_type -> Text, - published -> Timestamptz, } } diff --git a/crates/db_schema/src/source/images.rs b/crates/db_schema/src/source/images.rs index 8ef1c180a..681ef560a 100644 --- a/crates/db_schema/src/source/images.rs +++ b/crates/db_schema/src/source/images.rs @@ -71,7 +71,6 @@ pub struct ImageDetails { pub width: i32, pub height: i32, pub content_type: String, - pub published: DateTime, } #[derive(Debug, Clone, TypedBuilder)] diff --git a/crates/routes/src/images.rs b/crates/routes/src/images.rs index ff0e1c051..7d3b51bfe 100644 --- a/crates/routes/src/images.rs +++ b/crates/routes/src/images.rs @@ -10,7 +10,10 @@ use actix_web::{ HttpResponse, }; use futures::stream::{Stream, StreamExt}; -use lemmy_api_common::{context::LemmyContext, request::PictrsFileDetails}; +use lemmy_api_common::{ + context::LemmyContext, + request::{PictrsFileDetails, PictrsResponse}, +}; use lemmy_db_schema::source::{ images::{LocalImage, LocalImageForm, RemoteImage}, local_site::LocalSite, @@ -19,7 +22,7 @@ use lemmy_db_views::structs::LocalUserView; use lemmy_utils::{error::LemmyResult, rate_limit::RateLimitCell, REQWEST_TIMEOUT}; use reqwest::Body; use reqwest_middleware::{ClientWithMiddleware, RequestBuilder}; -use serde::{Deserialize, Serialize}; +use serde::Deserialize; use std::time::Duration; use url::Url; use urlencoding::decode; @@ -42,18 +45,6 @@ pub fn config( .service(web::resource("/pictrs/image/delete/{token}/{filename}").route(web::get().to(delete))); } -#[derive(Debug, Serialize, Deserialize)] -struct Image { - file: String, - delete_token: String, -} - -#[derive(Debug, Serialize, Deserialize)] -struct Images { - msg: String, - files: Option>, -} - #[derive(Deserialize)] struct PictrsGetParams { format: Option, @@ -114,15 +105,21 @@ async fn upload( .await?; let status = res.status(); - let images = res.json::().await?; + let images = res.json::().await?; if let Some(images) = &images.files { - for uploaded_image in images { + for image in images { let form = LocalImageForm { local_user_id: Some(local_user_view.local_user.id), - pictrs_alias: uploaded_image.file.to_string(), - pictrs_delete_token: uploaded_image.delete_token.to_string(), + pictrs_alias: image.file.to_string(), + pictrs_delete_token: image.delete_token.to_string(), }; - LocalImage::create(&mut context.pool(), &form).await?; + + let protocol_and_hostname = context.settings().get_protocol_and_hostname(); + let thumbnail_url = image.thumbnail_url(&protocol_and_hostname)?; + + // Also store the details for the image + let details_form = image.details.build_image_details_form(&thumbnail_url); + LocalImage::create(&mut context.pool(), &form, &details_form).await?; } } diff --git a/migrations/2024-05-05-162540_add_image_detail_table/up.sql b/migrations/2024-05-05-162540_add_image_detail_table/up.sql index 92dbec569..9b2ed9658 100644 --- a/migrations/2024-05-05-162540_add_image_detail_table/up.sql +++ b/migrations/2024-05-05-162540_add_image_detail_table/up.sql @@ -10,7 +10,6 @@ CREATE TABLE image_details ( link text PRIMARY KEY, width integer NOT NULL, height integer NOT NULL, - content_type text NOT NULL, - published timestamptz DEFAULT now() NOT NULL + content_type text NOT NULL );