From 25e83c466e87ff7f8b436f8ba9d4d789fe1bd2f0 Mon Sep 17 00:00:00 2001
From: Felix Ableitner <me@nutomic.com>
Date: Mon, 18 Mar 2024 11:43:52 +0100
Subject: [PATCH] When uploading new avatar, delete old one

---
 Cargo.lock                                 |  2 ++
 api_tests/run-federation-test.sh           |  2 +-
 api_tests/src/user.spec.ts                 | 36 +++++++++++++++++++++-
 crates/api/src/local_user/save_settings.rs | 13 ++++++++
 crates/db_schema/src/impls/images.rs       | 13 ++++++++
 5 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index b9ebfa540..d62325781 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -2577,6 +2577,7 @@ version = "0.19.3"
 dependencies = [
  "activitypub_federation",
  "actix-web",
+ "anyhow",
  "chrono",
  "encoding",
  "enum-map",
@@ -2589,6 +2590,7 @@ dependencies = [
  "lemmy_db_views_moderator",
  "lemmy_utils",
  "mime",
+ "moka",
  "once_cell",
  "pretty_assertions",
  "regex",
diff --git a/api_tests/run-federation-test.sh b/api_tests/run-federation-test.sh
index 969a95b3e..d571236be 100755
--- a/api_tests/run-federation-test.sh
+++ b/api_tests/run-federation-test.sh
@@ -11,7 +11,7 @@ killall -s1 lemmy_server || true
 popd
 
 pnpm i
-pnpm api-test || true
+pnpm api-test-user || true
 
 killall -s1 lemmy_server || true
 killall -s1 pict-rs || true
diff --git a/api_tests/src/user.spec.ts b/api_tests/src/user.spec.ts
index 4846d60f7..9f7e35700 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);
@@ -139,3 +140,36 @@ test("Create user with Arabic name", async () => {
   let alphaPerson = (await resolvePerson(alpha, apShortname)).person;
   expect(alphaPerson).toBeDefined();
 });
+
+test.only("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();
+  console.log(upload1);
+
+  let form1 = {
+    avatar: upload1.url
+  };
+  await saveUserSettings(alpha, form1);
+  const listMediaRes1 = await alphaImage.listMedia();
+  expect(listMediaRes1.images.length).toBe(1);
+  console.log(listMediaRes1);
+
+  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);
+});
\ No newline at end of file
diff --git a/crates/api/src/local_user/save_settings.rs b/crates/api/src/local_user/save_settings.rs
index 927496416..2aed3b17b 100644
--- a/crates/api/src/local_user/save_settings.rs
+++ b/crates/api/src/local_user/save_settings.rs
@@ -2,6 +2,7 @@ use actix_web::web::{Data, Json};
 use lemmy_api_common::{
   context::LemmyContext,
   person::SaveUserSettings,
+  request::delete_image_from_pictrs,
   utils::{
     get_url_blocklist,
     local_site_to_slur_regex,
@@ -14,6 +15,7 @@ use lemmy_api_common::{
 use lemmy_db_schema::{
   source::{
     actor_language::LocalUserLanguage,
+    images::LocalImage,
     local_user::{LocalUser, LocalUserUpdateForm},
     local_user_vote_display_mode::{LocalUserVoteDisplayMode, LocalUserVoteDisplayModeUpdateForm},
     person::{Person, PersonUpdateForm},
@@ -40,6 +42,17 @@ pub async fn save_user_settings(
   let bio = diesel_option_overwrite(
     process_markdown_opt(&data.bio, &slur_regex, &url_blocklist, &context).await?,
   );
+  if data.avatar.is_some() {
+    // Ignore errors because image may be stored externally.
+    if let Some(avatar) = &local_user_view.person.avatar {
+      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?;
+      }
+    }
+  }
 
   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/db_schema/src/impls/images.rs b/crates/db_schema/src/impls/images.rs
index 40d5c5853..498aa8450 100644
--- a/crates/db_schema/src/impls/images.rs
+++ b/crates/db_schema/src/impls/images.rs
@@ -14,6 +14,8 @@ use diesel::{
   QueryDsl,
 };
 use diesel_async::RunQueryDsl;
+use once_cell::sync::Lazy;
+use regex::Regex;
 use url::Url;
 
 impl LocalImage {
@@ -80,6 +82,17 @@ impl LocalImage {
       .execute(conn)
       .await
   }
+
+  pub async fn delete_by_url(pool: &mut DbPool<'_>, url: &DbUrl) -> Result<Self, Error> {
+    let conn = &mut get_conn(pool).await?;
+    static IMAGE_REGEX: Lazy<Regex> =
+      Lazy::new(|| Regex::new(r"^.*/pictrs/image/([a-z0-9-]+\.[a-z]+)$").expect("compile regex"));
+    let captures = IMAGE_REGEX.captures(url.as_str()).unwrap();
+    let alias = &captures[1];
+    diesel::delete(local_image::table.filter(local_image::pictrs_alias.eq(alias)))
+      .get_result(conn)
+      .await
+  }
 }
 
 impl RemoteImage {