From 85ee89f4e802cd5034558208e8e932d70cf30620 Mon Sep 17 00:00:00 2001 From: Nutomic Date: Wed, 27 Mar 2024 14:00:52 +0100 Subject: [PATCH] When uploading new icon/avatar/banner, delete old one (#4573) --- api_tests/src/user.spec.ts | 34 +++++++++++++++++++++- crates/api/src/local_user/save_settings.rs | 6 +++- crates/api_common/src/request.rs | 21 +++++++++++++ crates/api_crud/src/community/update.rs | 4 +++ crates/api_crud/src/site/update.rs | 7 ++++- crates/db_schema/src/impls/images.rs | 9 ++++-- 6 files changed, 76 insertions(+), 5 deletions(-) diff --git a/api_tests/src/user.spec.ts b/api_tests/src/user.spec.ts index 73f3f3942..5af054918 100644 --- a/api_tests/src/user.spec.ts +++ b/api_tests/src/user.spec.ts @@ -19,8 +19,9 @@ import { getPost, getComments, fetchFunction, + alphaImage, } from "./shared"; -import { LemmyHttp, SaveUserSettings } from "lemmy-js-client"; +import { LemmyHttp, SaveUserSettings, UploadImage } from "lemmy-js-client"; import { GetPosts } from "lemmy-js-client/dist/types/GetPosts"; beforeAll(setupLogins); @@ -159,3 +160,34 @@ test("Create user with accept-language", async () => { // which is automatically enabled by backend expect(langs).toStrictEqual(["und", "de", "en", "fr"]); }); + +test("Set a new avatar, old avatar is deleted", async () => { + const listMediaRes = await alphaImage.listMedia(); + expect(listMediaRes.images.length).toBe(0); + const upload_form1: UploadImage = { + image: Buffer.from("test1"), + }; + const upload1 = await alphaImage.uploadImage(upload_form1); + expect(upload1.url).toBeDefined(); + + let form1 = { + avatar: upload1.url, + }; + await saveUserSettings(alpha, form1); + const listMediaRes1 = await alphaImage.listMedia(); + expect(listMediaRes1.images.length).toBe(1); + + const upload_form2: UploadImage = { + image: Buffer.from("test2"), + }; + const upload2 = await alphaImage.uploadImage(upload_form2); + expect(upload2.url).toBeDefined(); + + let form2 = { + avatar: upload1.url, + }; + await saveUserSettings(alpha, form2); + // make sure only the new avatar is kept + const listMediaRes2 = await alphaImage.listMedia(); + expect(listMediaRes2.images.length).toBe(1); +}); diff --git a/crates/api/src/local_user/save_settings.rs b/crates/api/src/local_user/save_settings.rs index 927496416..972760a00 100644 --- a/crates/api/src/local_user/save_settings.rs +++ b/crates/api/src/local_user/save_settings.rs @@ -1,7 +1,9 @@ -use actix_web::web::{Data, Json}; +use activitypub_federation::config::Data; +use actix_web::web::Json; use lemmy_api_common::{ context::LemmyContext, person::SaveUserSettings, + request::replace_image, utils::{ get_url_blocklist, local_site_to_slur_regex, @@ -40,6 +42,8 @@ pub async fn save_user_settings( let bio = diesel_option_overwrite( process_markdown_opt(&data.bio, &slur_regex, &url_blocklist, &context).await?, ); + replace_image(&data.avatar, &local_user_view.person.avatar, &context).await?; + replace_image(&data.banner, &local_user_view.person.banner, &context).await?; let avatar = proxy_image_link_opt_api(&data.avatar, &context).await?; let banner = proxy_image_link_opt_api(&data.banner, &context).await?; diff --git a/crates/api_common/src/request.rs b/crates/api_common/src/request.rs index 74ee9ee47..7209c5871 100644 --- a/crates/api_common/src/request.rs +++ b/crates/api_common/src/request.rs @@ -3,6 +3,7 @@ use crate::{ post::{LinkMetadata, OpenGraphData}, utils::proxy_image_link, }; +use activitypub_federation::config::Data; use encoding::{all::encodings, DecoderTrap}; use lemmy_db_schema::{ newtypes::DbUrl, @@ -312,6 +313,26 @@ async fn is_image_content_type(client: &ClientWithMiddleware, url: &Url) -> Resu } } +/// When adding a new avatar or similar image, delete the old one. +pub async fn replace_image( + new_image: &Option, + old_image: &Option, + context: &Data, +) -> Result<(), LemmyError> { + if new_image.is_some() { + // Ignore errors because image may be stored externally. + if let Some(avatar) = &old_image { + let image = LocalImage::delete_by_url(&mut context.pool(), avatar) + .await + .ok(); + if let Some(image) = image { + delete_image_from_pictrs(&image.pictrs_alias, &image.pictrs_delete_token, context).await?; + } + } + } + Ok(()) +} + #[cfg(test)] #[allow(clippy::unwrap_used)] #[allow(clippy::indexing_slicing)] diff --git a/crates/api_crud/src/community/update.rs b/crates/api_crud/src/community/update.rs index 83ffded13..51c57e1c8 100644 --- a/crates/api_crud/src/community/update.rs +++ b/crates/api_crud/src/community/update.rs @@ -4,6 +4,7 @@ use lemmy_api_common::{ build_response::build_community_response, community::{CommunityResponse, EditCommunity}, context::LemmyContext, + request::replace_image, send_activity::{ActivityChannel, SendActivityData}, utils::{ check_community_mod_action, @@ -42,6 +43,9 @@ pub async fn update_community( let description = process_markdown_opt(&data.description, &slur_regex, &url_blocklist, &context).await?; is_valid_body_field(&data.description, false)?; + let old_community = Community::read(&mut context.pool(), data.community_id).await?; + replace_image(&data.icon, &old_community.icon, &context).await?; + replace_image(&data.banner, &old_community.banner, &context).await?; let description = diesel_option_overwrite(description); let icon = proxy_image_link_opt_api(&data.icon, &context).await?; diff --git a/crates/api_crud/src/site/update.rs b/crates/api_crud/src/site/update.rs index 6d419a6d8..530dbb47f 100644 --- a/crates/api_crud/src/site/update.rs +++ b/crates/api_crud/src/site/update.rs @@ -1,7 +1,9 @@ use crate::site::{application_question_check, site_default_post_listing_type_check}; -use actix_web::web::{Data, Json}; +use activitypub_federation::config::Data; +use actix_web::web::Json; use lemmy_api_common::{ context::LemmyContext, + request::replace_image, site::{EditSite, SiteResponse}, utils::{ get_url_blocklist, @@ -63,6 +65,9 @@ pub async fn update_site( SiteLanguage::update(&mut context.pool(), discussion_languages.clone(), &site).await?; } + replace_image(&data.icon, &site.icon, &context).await?; + replace_image(&data.banner, &site.banner, &context).await?; + let slur_regex = local_site_to_slur_regex(&local_site); let url_blocklist = get_url_blocklist(&context).await?; let sidebar = process_markdown_opt(&data.sidebar, &slur_regex, &url_blocklist, &context).await?; diff --git a/crates/db_schema/src/impls/images.rs b/crates/db_schema/src/impls/images.rs index 40d5c5853..a9aeb1f16 100644 --- a/crates/db_schema/src/impls/images.rs +++ b/crates/db_schema/src/impls/images.rs @@ -74,12 +74,17 @@ impl LocalImage { query.load::(conn).await } - pub async fn delete_by_alias(pool: &mut DbPool<'_>, alias: &str) -> Result { + pub async fn delete_by_alias(pool: &mut DbPool<'_>, alias: &str) -> Result { let conn = &mut get_conn(pool).await?; diesel::delete(local_image::table.filter(local_image::pictrs_alias.eq(alias))) - .execute(conn) + .get_result(conn) .await } + + pub async fn delete_by_url(pool: &mut DbPool<'_>, url: &DbUrl) -> Result { + let alias = url.as_str().split('/').last().ok_or(NotFound)?; + Self::delete_by_alias(pool, alias).await + } } impl RemoteImage {