When uploading new icon/avatar/banner, delete old one (#4573)

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

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

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<String>,
+  old_image: &Option<DbUrl>,
+  context: &Data<LemmyContext>,
+) -> 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)]

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

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

crates/db_schema/src/impls/images.rs

@@ -74,12 +74,17 @@ impl LocalImage {
     query.load::<LocalImage>(conn).await
   }
 
-  pub async fn delete_by_alias(pool: &mut DbPool<'_>, alias: &str) -> Result<usize, Error> {
+  pub async fn delete_by_alias(pool: &mut DbPool<'_>, alias: &str) -> Result<Self, Error> {
     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<Self, Error> {
+    let alias = url.as_str().split('/').last().ok_or(NotFound)?;
+    Self::delete_by_alias(pool, alias).await
+  }
 }
 
 impl RemoteImage {