Change local_image to reference person_id, to track thumbnail creators. (#5664)

* Migrations before column fixes

* Change local_image to reference person_id, to track thumbnail creators.

- Fixes #5564

* Fixing API tests.

* Increase the post query duration time. #5661

* Adding thumbnail_and_post_id column

* Forgot ts-optional

* Fixing post spec tests.

* Try using promise.allSettled to ignore errors.

* Using consistent pictrs api key.

* Dont fetch thumbnails for user API.

* Dont delete thumbnails for user deletion.

* Dont filter out thumbnail images when fetching or deleting.

* Update crates/api_common/src/utils.rs

Co-authored-by: Richard Schwab <gitrichardschwab-7a2qxq42kj@central-intelligence.agency>

* Addressing PR comments

* Add comment.

* Rename to thumbnail_for_post_id

* Addressing PR comments

---------

Co-authored-by: Richard Schwab <gitrichardschwab-7a2qxq42kj@central-intelligence.agency>

api_tests/package.json

@@ -22,19 +22,20 @@
     "api-test-tags": "jest -i tags.spec.ts"
   },
   "devDependencies": {
-    "@eslint/js": "^9.25.1",
+    "@eslint/js": "^9.26.0",
     "@types/jest": "^29.5.12",
-    "@types/node": "^22.15.3",
-    "@typescript-eslint/eslint-plugin": "^8.31.0",
-    "@typescript-eslint/parser": "^8.31.0",
-    "eslint": "^9.25.1",
-    "eslint-plugin-prettier": "^5.2.6",
+    "@types/node": "^22.15.14",
+    "@typescript-eslint/eslint-plugin": "^8.32.0",
+    "@typescript-eslint/parser": "^8.32.0",
+    "eslint": "^9.26.0",
+    "eslint-plugin-prettier": "^5.4.0",
     "jest": "^29.5.0",
-    "lemmy-js-client": "1.0.0-remove-page-limit.1",
+    "joi": "^17.13.3",
+    "lemmy-js-client": "1.0.0-local-image-user.3",
     "prettier": "^3.5.3",
     "ts-jest": "^29.3.2",
     "tsoa": "^6.6.0",
     "typescript": "^5.8.3",
-    "typescript-eslint": "^8.31.0"
+    "typescript-eslint": "^8.32.0"
   }
 }

api_tests/prepare-drone-federation-test.sh

@@ -3,8 +3,7 @@
 #   it is expected that this script is called by run-federation-test.sh script.
 set -e
 
-if [ -z "$LEMMY_LOG_LEVEL" ];
-then
+if [ -z "$LEMMY_LOG_LEVEL" ]; then
   LEMMY_LOG_LEVEL=info
 fi
 
@@ -16,11 +15,10 @@ export LEMMY_TEST_FAST_FEDERATION=1 # by default, the persistent federation queu
 PICTRS_PATH="api_tests/pict-rs"
 PICTRS_EXPECTED_HASH="7f7ac2a45ef9b13403ee139b7512135be6b060ff2f6460e0c800e18e1b49d2fd  api_tests/pict-rs"
 
-# Pictrs setup. Download file with hash check and up to 3 retries. 
+# Pictrs setup. Download file with hash check and up to 3 retries.
 if [ ! -f "$PICTRS_PATH" ]; then
   count=0
-  while [ ! -f "$PICTRS_PATH" ] && [ "$count" -lt 3 ]
-  do
+  while [ ! -f "$PICTRS_PATH" ] && [ "$count" -lt 3 ]; do
     # This one sometimes goes down
     curl "https://git.asonix.dog/asonix/pict-rs/releases/download/v0.5.17-pre.9/pict-rs-linux-amd64" -o "$PICTRS_PATH"
     # curl "https://codeberg.org/asonix/pict-rs/releases/download/v0.5.5/pict-rs-linux-amd64" -o "$PICTRS_PATH"

api_tests/src/comment.spec.ts

@@ -54,7 +54,7 @@ let postOnAlphaRes: PostResponse;
 
 beforeAll(async () => {
   await setupLogins();
-  await Promise.all([followBeta(alpha), followBeta(gamma)]);
+  await Promise.allSettled([followBeta(alpha), followBeta(gamma)]);
   betaCommunity = (await resolveBetaCommunity(alpha)).community;
   if (betaCommunity) {
     postOnAlphaRes = await createPost(alpha, betaCommunity.community.id);
@@ -705,7 +705,7 @@ test("Check that activity from another instance is sent to third instance", asyn
     commentRes.comment_view,
   );
 
-  await Promise.all([unfollowRemotes(alpha), unfollowRemotes(gamma)]);
+  await Promise.allSettled([unfollowRemotes(alpha), unfollowRemotes(gamma)]);
 });
 
 test("Fetch in_reply_tos: A is unsubbed from B, B makes a post, and some embedded comments, A subs to B, B updates the lowest level comment, A fetches both the post and all the inreplyto comments for that post.", async () => {

api_tests/src/image.spec.ts

@@ -37,16 +37,13 @@ import {
 beforeAll(setupLogins);
 
 afterAll(async () => {
-  await Promise.all([unfollows(), deleteAllMedia(alpha)]);
+  await Promise.allSettled([unfollows(), deleteAllMedia(alpha)]);
 });
 
 test("Upload image and delete it", async () => {
   const health = await alpha.imageHealth();
   expect(health.success).toBeTruthy();
 
-  // Before running this test, you need to delete all previous images in the DB
-  await deleteAllMedia(alpha);
-
   // Upload test image. We use a simple string buffer as pictrs doesn't require an actual image
   // in testing mode.
   const upload_form: UploadImage = {

api_tests/src/post.spec.ts

@@ -462,7 +462,7 @@ test("Search for a post", async () => {
   expect(betaPost?.post.name).toBeDefined();
 });
 
-test.only("Enforce site ban federation for local user", async () => {
+test("Enforce site ban federation for local user", async () => {
   if (!betaCommunity) {
     throw "Missing beta community";
   }

api_tests/src/shared.ts

@@ -768,7 +768,7 @@ export async function unfollowRemotes(api: LemmyHttp): Promise<MyUserInfo> {
   let my_user = await getMyUser(api);
   let remoteFollowed =
     my_user.follows.filter(c => c.community.local == false) ?? [];
-  await Promise.all(
+  await Promise.allSettled(
     remoteFollowed.map(cu => followCommunity(api, false, cu.community.id)),
   );
 
@@ -930,7 +930,7 @@ export async function deleteAllMedia(api: LemmyHttp) {
   const imagesRes = await api.listMediaAdmin({
     limit: imageFetchLimit,
   });
-  Promise.all(
+  Promise.allSettled(
     imagesRes.images
       .map(image => {
         const form: DeleteImageParams = {
@@ -943,14 +943,14 @@ export async function deleteAllMedia(api: LemmyHttp) {
 }
 
 export async function unfollows() {
-  await Promise.all([
+  await Promise.allSettled([
     unfollowRemotes(alpha),
     unfollowRemotes(beta),
     unfollowRemotes(gamma),
     unfollowRemotes(delta),
     unfollowRemotes(epsilon),
   ]);
-  await Promise.all([
+  await Promise.allSettled([
     purgeAllPosts(alpha),
     purgeAllPosts(beta),
     purgeAllPosts(gamma),
@@ -962,7 +962,7 @@ export async function unfollows() {
 export async function purgeAllPosts(api: LemmyHttp) {
   // The best way to get all federated items, is to find the posts
   let res = await api.getPosts({ type_: "All", limit: 50 });
-  await Promise.all(
+  await Promise.allSettled(
     Array.from(new Set(res.posts.map(p => p.post.id)))
       .map(post_id => api.purgePost({ post_id }))
       // Ignore errors

crates/api/src/local_user/list_media.rs

@@ -19,9 +19,9 @@ pub async fn list_media(
     None
   };
 
-  let images = LocalImageView::get_all_paged_by_local_user_id(
+  let images = LocalImageView::get_all_paged_by_person_id(
     &mut context.pool(),
-    local_user_view.local_user.id,
+    local_user_view.person.id,
     cursor_data,
     data.page_back,
     data.limit,

crates/api_common/src/request.rs

@@ -219,7 +219,7 @@ pub async fn generate_post_link_metadata(
   };
 
   let image_url = if is_image_post {
-    post.url
+    post.url.clone()
   } else {
     metadata.opengraph_data.image.clone()
   };
@@ -233,7 +233,7 @@ pub async fn generate_post_link_metadata(
       .ok()
       .or(Some(url.into()))
   } else if let (true, Some(url)) = (allow_generate_thumbnail, image_url.clone()) {
-    generate_pictrs_thumbnail(&url, &context)
+    generate_pictrs_thumbnail(&post, &url, &context)
       .await
       .map_err(|e| warn!("Failed to generate thumbnail: {e}"))
       .ok()
@@ -443,7 +443,11 @@ pub async fn delete_image_from_pictrs(alias: &str, context: &LemmyContext) -> Le
 }
 
 /// Retrieves the image with local pict-rs and generates a thumbnail. Returns the thumbnail url.
-async fn generate_pictrs_thumbnail(image_url: &Url, context: &LemmyContext) -> LemmyResult<Url> {
+async fn generate_pictrs_thumbnail(
+  post: &Post,
+  image_url: &Url,
+  context: &LemmyContext,
+) -> LemmyResult<Url> {
   let pictrs_config = context.settings().pictrs()?;
 
   match pictrs_config.image_mode {
@@ -482,10 +486,10 @@ async fn generate_pictrs_thumbnail(image_url: &Url, context: &LemmyContext) -> L
     .ok_or(LemmyErrorType::PictrsResponseError(res.msg))?;
 
   let form = LocalImageForm {
-    // This is none because its an internal request.
-    // IE, a local user shouldn't get to delete the thumbnails for their link posts
-    local_user_id: None,
     pictrs_alias: image.file.clone(),
+    // For thumbnails, the person_id is the post creator
+    person_id: post.creator_id,
+    thumbnail_for_post_id: Some(Some(post.id)),
   };
   let protocol_and_hostname = context.settings().get_protocol_and_hostname();
   let thumbnail_url = image.image_url(&protocol_and_hostname)?;

crates/api_common/src/utils.rs

@@ -549,19 +549,15 @@ pub async fn purge_post_images(
   }
 }
 
-/// Delete a local_user's images
+/// Delete local images attributed to a person
 async fn delete_local_user_images(person_id: PersonId, context: &LemmyContext) -> LemmyResult<()> {
-  if let Ok(local_user) = LocalUserView::read_person(&mut context.pool(), person_id).await {
-    let pictrs_uploads =
-      LocalImageView::get_all_by_local_user_id(&mut context.pool(), local_user.local_user.id)
-        .await?;
+  let pictrs_uploads = LocalImageView::get_all_by_person_id(&mut context.pool(), person_id).await?;
 
-    // Delete their images
-    for upload in pictrs_uploads {
-      delete_image_from_pictrs(&upload.local_image.pictrs_alias, context)
-        .await
-        .ok();
-    }
+  // Delete their images
+  for upload in pictrs_uploads {
+    delete_image_from_pictrs(&upload.local_image.pictrs_alias, context)
+      .await
+      .ok();
   }
   Ok(())
 }

crates/db_schema/src/impls/images.rs

@@ -1,5 +1,5 @@
 use crate::{
-  newtypes::{DbUrl, LocalUserId},
+  newtypes::{DbUrl, PersonId},
   source::images::{ImageDetails, ImageDetailsInsertForm, LocalImage, LocalImageForm, RemoteImage},
   utils::{get_conn, DbPool},
 };
@@ -65,14 +65,14 @@ impl LocalImage {
   pub async fn delete_by_alias_and_user(
     pool: &mut DbPool<'_>,
     alias: &str,
-    local_user_id: LocalUserId,
+    person_id: PersonId,
   ) -> LemmyResult<Self> {
     let conn = &mut get_conn(pool).await?;
     diesel::delete(
       local_image::table.filter(
         local_image::pictrs_alias
           .eq(alias)
-          .and(local_image::local_user_id.eq(local_user_id)),
+          .and(local_image::person_id.eq(person_id)),
       ),
     )
     .get_result(conn)

crates/db_schema/src/source/images.rs

@@ -1,4 +1,4 @@
-use crate::newtypes::{DbUrl, LocalUserId};
+use crate::newtypes::{DbUrl, PersonId, PostId};
 use chrono::{DateTime, Utc};
 use serde::{Deserialize, Serialize};
 use serde_with::skip_serializing_none;
@@ -25,26 +25,26 @@ use {
 )]
 #[cfg_attr(feature = "full", ts(export))]
 #[cfg_attr(feature = "full", diesel(table_name = local_image))]
-#[cfg_attr(
-  feature = "full",
-  diesel(belongs_to(crate::source::local_user::LocalUser))
-)]
+#[cfg_attr(feature = "full", diesel(belongs_to(crate::source::person::Person)))]
 #[cfg_attr(feature = "full", diesel(check_for_backend(diesel::pg::Pg)))]
 #[cfg_attr(feature = "full", diesel(primary_key(pictrs_alias)))]
 #[cfg_attr(feature = "full", cursor_keys_module(name = local_image_keys))]
 pub struct LocalImage {
-  #[cfg_attr(feature = "full", ts(optional))]
-  pub local_user_id: Option<LocalUserId>,
   pub pictrs_alias: String,
   pub published: DateTime<Utc>,
+  pub person_id: PersonId,
+  #[cfg_attr(feature = "full", ts(optional))]
+  /// This means the image is an auto-generated thumbnail, for a post.
+  pub thumbnail_for_post_id: Option<PostId>,
 }
 
 #[derive(Debug, Clone)]
 #[cfg_attr(feature = "full", derive(Insertable, AsChangeset))]
 #[cfg_attr(feature = "full", diesel(table_name = local_image))]
 pub struct LocalImageForm {
-  pub local_user_id: Option<LocalUserId>,
   pub pictrs_alias: String,
+  pub person_id: PersonId,
+  pub thumbnail_for_post_id: Option<Option<PostId>>,
 }
 
 /// Stores all images which are hosted on remote domains. When attempting to proxy an image, it

crates/db_schema_file/src/schema.rs

@@ -393,9 +393,10 @@ diesel::table! {
 
 diesel::table! {
     local_image (pictrs_alias) {
-        local_user_id -> Nullable<Int4>,
         pictrs_alias -> Text,
         published -> Timestamptz,
+        person_id -> Int4,
+        thumbnail_for_post_id -> Nullable<Int4>,
     }
 }
 
@@ -1117,7 +1118,8 @@ diesel::joinable!(inbox_combined -> person_post_mention (person_post_mention_id)
 diesel::joinable!(inbox_combined -> private_message (private_message_id));
 diesel::joinable!(instance_actions -> instance (instance_id));
 diesel::joinable!(instance_actions -> person (person_id));
-diesel::joinable!(local_image -> local_user (local_user_id));
+diesel::joinable!(local_image -> person (person_id));
+diesel::joinable!(local_image -> post (thumbnail_for_post_id));
 diesel::joinable!(local_site -> site (site_id));
 diesel::joinable!(local_site_rate_limit -> local_site (local_site_id));
 diesel::joinable!(local_user -> person (person_id));

crates/db_views/local_image/src/impls.rs

@@ -1,27 +1,27 @@
 use crate::LocalImageView;
-use diesel::{ExpressionMethods, JoinOnDsl, QueryDsl, SelectableHelper};
+use diesel::{ExpressionMethods, QueryDsl, SelectableHelper};
 use diesel_async::RunQueryDsl;
 use i_love_jesus::SortDirection;
 use lemmy_db_schema::{
-  newtypes::{LocalUserId, PaginationCursor},
+  newtypes::{PaginationCursor, PersonId},
   source::images::{local_image_keys as key, LocalImage},
   traits::PaginationCursorBuilder,
   utils::{get_conn, limit_fetch, paginate, DbPool},
 };
-use lemmy_db_schema_file::schema::{local_image, local_user, person};
+use lemmy_db_schema_file::schema::{local_image, person, post};
 use lemmy_utils::error::{LemmyErrorExt, LemmyErrorType, LemmyResult};
 
 impl LocalImageView {
   #[diesel::dsl::auto_type(no_type_alias)]
   fn joins() -> _ {
     local_image::table
-      .inner_join(local_user::table)
-      .inner_join(person::table.on(local_user::person_id.eq(person::id)))
+      .inner_join(person::table)
+      .left_join(post::table)
   }
 
-  pub async fn get_all_paged_by_local_user_id(
+  pub async fn get_all_paged_by_person_id(
     pool: &mut DbPool<'_>,
-    user_id: LocalUserId,
+    person_id: PersonId,
     cursor_data: Option<LocalImage>,
     page_back: Option<bool>,
     limit: Option<i64>,
@@ -30,7 +30,7 @@ impl LocalImageView {
     let limit = limit_fetch(limit)?;
 
     let query = Self::joins()
-      .filter(local_image::local_user_id.eq(user_id))
+      .filter(local_image::person_id.eq(person_id))
       .select(Self::as_select())
       .limit(limit)
       .into_boxed();
@@ -44,13 +44,13 @@ impl LocalImageView {
       .with_lemmy_type(LemmyErrorType::NotFound)
   }
 
-  pub async fn get_all_by_local_user_id(
+  pub async fn get_all_by_person_id(
     pool: &mut DbPool<'_>,
-    user_id: LocalUserId,
+    person_id: PersonId,
   ) -> LemmyResult<Vec<Self>> {
     let conn = &mut get_conn(pool).await?;
     Self::joins()
-      .filter(local_image::local_user_id.eq(user_id))
+      .filter(local_image::person_id.eq(person_id))
       .select(Self::as_select())
       .load::<Self>(conn)
       .await

crates/db_views/local_image/src/lib.rs

@@ -1,4 +1,4 @@
-use lemmy_db_schema::source::{images::LocalImage, person::Person};
+use lemmy_db_schema::source::{images::LocalImage, person::Person, post::Post};
 use serde::{Deserialize, Serialize};
 use serde_with::skip_serializing_none;
 #[cfg(feature = "full")]
@@ -21,4 +21,7 @@ pub struct LocalImageView {
   pub local_image: LocalImage,
   #[cfg_attr(feature = "full", diesel(embed))]
   pub person: Person,
+  #[cfg_attr(feature = "full", diesel(embed))]
+  #[cfg_attr(feature = "full", ts(optional))]
+  pub post: Option<Post>,
 }

crates/routes/src/images/delete.rs

@@ -131,7 +131,7 @@ pub async fn delete_image(
   LocalImage::delete_by_alias_and_user(
     &mut context.pool(),
     &data.filename,
-    local_user_view.local_user.id,
+    local_user_view.person.id,
   )
   .await?;
 

crates/routes/src/images/upload.rs

@@ -221,8 +221,9 @@ pub async fn do_upload_image(
     // but still a user may upload multiple and so we need to store all links in db for
     // to allow deletion via web ui.
     let form = LocalImageForm {
-      local_user_id: Some(local_user_view.local_user.id),
       pictrs_alias: image.file.to_string(),
+      person_id: local_user_view.person.id,
+      thumbnail_for_post_id: None,
     };
 
     let protocol_and_hostname = context.settings().get_protocol_and_hostname();

docker/docker-compose.yml

@@ -66,7 +66,7 @@ services:
     # entrypoint: /sbin/tini -- /usr/local/bin/pict-rs -p /mnt -m 4 --image-format webp
     environment:
       - PICTRS_OPENTELEMETRY_URL=http://otel:4137
-      - PICTRS__API_KEY=API_KEY
+      - PICTRS__SERVER__API_KEY=my-pictrs-key
       - PICTRS__MEDIA__VIDEO_CODEC=vp9
       - PICTRS__MEDIA__GIF__MAX_WIDTH=256
       - PICTRS__MEDIA__GIF__MAX_HEIGHT=256

docker/federation/docker-compose.yml

@@ -53,6 +53,8 @@ services:
     user: 991:991
     volumes:
       - ./volumes/pictrs_alpha:/mnt:Z
+    environment:
+      - PICTRS__SERVER__API_KEY=my-pictrs-key
 
   lemmy-alpha-ui:
     <<: *ui-default

docker/federation/lemmy_delta.hjson

@@ -10,4 +10,8 @@
   database: {
     connection: "postgres://lemmy:password@postgres_delta:5432/lemmy"
   }
+  pictrs: {
+    api_key: "my-pictrs-key"
+    image_mode: StoreLinkPreviews
+  }
 }

docker/lemmy.hjson

@@ -20,7 +20,7 @@
 
   pictrs: {
     url: "http://pictrs:8080/"
-    # api_key: "API_KEY"
+    api_key: "my-pictrs-key"
     image_mode: None
   }
 

migrations/2025-05-06-145536_local_image_person/down.sql

@@ -0,0 +1,46 @@
+ALTER TABLE local_image
+    ADD COLUMN local_user_id int REFERENCES local_user (id) ON UPDATE CASCADE ON DELETE CASCADE;
+
+UPDATE
+    local_image AS li
+SET
+    local_user_id = lu.id
+FROM
+    local_user AS lu
+WHERE
+    li.person_id = lu.person_id;
+
+-- You need to have the exact correct column order, so this needs to be re-created
+--
+-- Rename the table
+ALTER TABLE local_image RENAME TO local_image_old;
+
+-- Rename a few constraints
+ALTER TABLE local_image_old RENAME CONSTRAINT image_upload_pkey TO image_upload_pkey_old;
+
+-- Create the old one again
+CREATE TABLE local_image (
+    local_user_id integer,
+    pictrs_alias text NOT NULL,
+    published timestamp with time zone DEFAULT now() NOT NULL
+);
+
+ALTER TABLE ONLY local_image
+    ADD CONSTRAINT image_upload_pkey PRIMARY KEY (pictrs_alias);
+
+CREATE INDEX idx_image_upload_local_user_id ON local_image USING btree (local_user_id);
+
+ALTER TABLE ONLY local_image
+    ADD CONSTRAINT image_upload_local_user_id_fkey FOREIGN KEY (local_user_id) REFERENCES local_user (id) ON UPDATE CASCADE ON DELETE CASCADE;
+
+-- Insert the data again
+INSERT INTO local_image (local_user_id, pictrs_alias, published)
+SELECT
+    local_user_id,
+    pictrs_alias,
+    published
+FROM
+    local_image_old;
+
+DROP TABLE local_image_old;
+

migrations/2025-05-06-145536_local_image_person/up.sql

@@ -0,0 +1,31 @@
+-- Since local thumbnails could be generated from posts of external users,
+-- use the person_id instead of local_user_id for the LocalImage table.
+--
+-- Also connect the thumbnail to a post id.
+--
+-- See https://github.com/LemmyNet/lemmy/issues/5564
+ALTER TABLE local_image
+    ADD COLUMN person_id int NOT NULL DEFAULT 0 REFERENCES person (id) ON UPDATE CASCADE ON DELETE CASCADE,
+    ADD COLUMN thumbnail_for_post_id int REFERENCES post (id) ON UPDATE CASCADE ON DELETE CASCADE;
+
+-- Update historical person_id columns
+-- Note: The local_user_id rows are null for thumbnails, so there's nothing you can do there.
+UPDATE
+    local_image AS li
+SET
+    person_id = lu.person_id
+FROM
+    local_user AS lu
+WHERE
+    li.local_user_id = lu.id;
+
+-- Remove the default
+ALTER TABLE local_image
+    ALTER COLUMN person_id DROP DEFAULT;
+
+-- Remove the local_user_id column
+ALTER TABLE local_image
+    DROP COLUMN local_user_id;
+
+CREATE INDEX idx_image_upload_person_id ON local_image (person_id);
+