Fixing tests, fixing issue with federated thumbnails.

api_tests/src/image.spec.ts

@@ -30,6 +30,7 @@ import {
   getPost,
   waitUntil,
   createPostWithThumbnail,
+  sampleImage,
 } from "./shared";
 const downloadFileSync = require("download-file-sync");
 
@@ -182,13 +183,13 @@ test("Images in remote post are proxied if setting enabled", async () => {
     gamma,
     community.community_view.community.id,
     upload.url,
-    "![](http://example.com/image2.png)",
+    `![](${sampleImage})`,
   );
   expect(post.post_view.post).toBeDefined();
 
   // remote image gets proxied after upload
   expect(
-    post.post_view.post.url?.startsWith(
+    post.post_view.post.thumbnail_url?.startsWith(
       "http://lemmy-gamma:8561/api/v3/image_proxy?url",
     ),
   ).toBeTruthy();
@@ -201,14 +202,20 @@ test("Images in remote post are proxied if setting enabled", async () => {
   let epsilonPost = await resolvePost(epsilon, post.post_view.post);
   expect(epsilonPost.post).toBeDefined();
 
-  // remote image gets proxied after federation
+  // Fetch the post again, the metadata should be backgrounded now
+  // Wait for the metadata to get fetched, since this is backgrounded now
+  let epsilonPost2 = await waitUntil(
+    () => getPost(epsilon, epsilonPost.post!.post.id),
+    p => p.post_view.post.thumbnail_url != undefined,
+  );
+
   expect(
-    epsilonPost.post!.post.url?.startsWith(
+    epsilonPost2.post_view.post.thumbnail_url?.startsWith(
       "http://lemmy-epsilon:8581/api/v3/image_proxy?url",
     ),
   ).toBeTruthy();
   expect(
-    epsilonPost.post!.post.body?.startsWith(
+    epsilonPost2.post_view.post.body?.startsWith(
       "![](http://lemmy-epsilon:8581/api/v3/image_proxy?url",
     ),
   ).toBeTruthy();
@@ -231,7 +238,7 @@ test("No image proxying if setting is disabled", async () => {
     alpha,
     community.community_view.community.id,
     upload.url,
-    "![](http://example.com/image2.png)",
+    `![](${sampleImage})`,
   );
   expect(post.post_view.post).toBeDefined();
 
@@ -239,7 +246,7 @@ test("No image proxying if setting is disabled", async () => {
   expect(
     post.post_view.post.url?.startsWith("http://127.0.0.1:8551/pictrs/image/"),
   ).toBeTruthy();
-  expect(post.post_view.post.body).toBe("![](http://example.com/image2.png)");
+  expect(post.post_view.post.body).toBe(`![](${sampleImage})`);
 
   let betaPost = await waitForPost(
     beta,
@@ -252,8 +259,7 @@ test("No image proxying if setting is disabled", async () => {
   expect(
     betaPost.post.url?.startsWith("http://127.0.0.1:8551/pictrs/image/"),
   ).toBeTruthy();
-  expect(betaPost.post.body).toBe("![](http://example.com/image2.png)");
-
+  expect(betaPost.post.body).toBe(`![](${sampleImage})`);
   // Make sure the alt text got federated
   expect(post.post_view.post.alt_text).toBe(betaPost.post.alt_text);
 });

api_tests/src/post.spec.ts

@@ -729,7 +729,7 @@ test("Block post that contains banned URL", async () => {
 
   await epsilon.editSite(editSiteForm);
 
-  await delay(500);
+  await delay();
 
   if (!betaCommunity) {
     throw "Missing beta community";

api_tests/src/shared.ts

@@ -81,6 +81,8 @@ import { ListingType } from "lemmy-js-client/dist/types/ListingType";
 
 export const fetchFunction = fetch;
 export const imageFetchLimit = 50;
+export const sampleImage =
+  "https://i.pinimg.com/originals/df/5f/5b/df5f5b1b174a2b4b6026cc6c8f9395c1.jpg";
 
 export let alphaUrl = "http://127.0.0.1:8541";
 export let betaUrl = "http://127.0.0.1:8551";
@@ -693,8 +695,8 @@ export async function saveUserSettingsBio(
 export async function saveUserSettingsFederated(
   api: LemmyHttp,
 ): Promise<SuccessResponse> {
-  let avatar = "https://image.flaticon.com/icons/png/512/35/35896.png";
-  let banner = "https://image.flaticon.com/icons/png/512/36/35896.png";
+  let avatar = sampleImage;
+  let banner = sampleImage;
   let bio = "a changed bio";
   let form: SaveUserSettings = {
     show_nsfw: false,

crates/api_common/src/request.rs

@@ -66,87 +66,91 @@ pub async fn fetch_link_metadata(url: &Url, context: &LemmyContext) -> LemmyResu
   })
 }
 
-/// Generate post thumbnail in background task, because some sites can be very slow to respond.
+/// Generates and saves a post thumbnail and metadata.
 ///
 /// Takes a callback to generate a send activity task, so that post can be federated with metadata.
 ///
 /// TODO: `federated_thumbnail` param can be removed once we federate full metadata and can
 ///       write it to db directly, without calling this function.
 ///       https://github.com/LemmyNet/lemmy/issues/4598
-pub fn generate_post_link_metadata(
+pub async fn generate_post_link_metadata(
+  post: Post,
+  custom_thumbnail: Option<Url>,
+  send_activity: impl FnOnce(Post) -> Option<SendActivityData> + Send + 'static,
+  local_site: Option<LocalSite>,
+  context: Data<LemmyContext>,
+) -> LemmyResult<()> {
+  let metadata = match &post.url {
+    Some(url) => fetch_link_metadata(url, &context).await.unwrap_or_default(),
+    _ => Default::default(),
+  };
+
+  let is_image_post = metadata
+    .content_type
+    .as_ref()
+    .is_some_and(|content_type| content_type.starts_with("image"));
+
+  // Decide if we are allowed to generate local thumbnail
+  let allow_sensitive = local_site_opt_to_sensitive(&local_site);
+  let allow_generate_thumbnail = allow_sensitive || !post.nsfw;
+
+  let thumbnail_url = if is_image_post {
+    if allow_generate_thumbnail {
+      match post.url {
+        Some(url) => generate_pictrs_thumbnail(&url, &context)
+          .await
+          .ok()
+          .map(Into::into),
+        None => None,
+      }
+    } else {
+      None
+    }
+  } else {
+    // Use custom thumbnail if available and its not an image post
+    if let Some(custom_thumbnail) = custom_thumbnail {
+      proxy_image_link(custom_thumbnail, &context).await.ok()
+    } else if allow_generate_thumbnail {
+      match metadata.opengraph_data.image {
+        Some(url) => generate_pictrs_thumbnail(&url, &context)
+          .await
+          .ok()
+          .map(Into::into),
+        None => None,
+      }
+    }
+    // Otherwise use opengraph preview image directly
+    else {
+      metadata.opengraph_data.image
+    }
+  };
+
+  let form = PostUpdateForm {
+    embed_title: Some(metadata.opengraph_data.title),
+    embed_description: Some(metadata.opengraph_data.description),
+    embed_video_url: Some(metadata.opengraph_data.embed_video_url),
+    thumbnail_url: Some(thumbnail_url),
+    url_content_type: Some(metadata.content_type),
+    ..Default::default()
+  };
+  let updated_post = Post::update(&mut context.pool(), post.id, &form).await?;
+  if let Some(send_activity) = send_activity(updated_post) {
+    ActivityChannel::submit_activity(send_activity, &context).await?;
+  }
+  Ok(())
+}
+
+/// Generates a post thumbnail in background task, because some sites can be very slow to respond.
+pub fn generate_post_link_metadata_background(
   post: Post,
   custom_thumbnail: Option<Url>,
-  federated_thumbnail: Option<Url>,
   send_activity: impl FnOnce(Post) -> Option<SendActivityData> + Send + 'static,
   local_site: Option<LocalSite>,
   context: Data<LemmyContext>,
 ) {
   spawn_try_task(async move {
-    let metadata = match &post.url {
-      Some(url) => fetch_link_metadata(url, &context).await.unwrap_or_default(),
-      _ => Default::default(),
-    };
-
-    let is_image_post = metadata
-      .content_type
-      .as_ref()
-      .is_some_and(|content_type| content_type.starts_with("image"));
-
-    // Decide if we are allowed to generate local thumbnail
-    let allow_sensitive = local_site_opt_to_sensitive(&local_site);
-    let allow_generate_thumbnail = allow_sensitive || !post.nsfw;
-
-    let thumbnail_url = if is_image_post {
-      if allow_generate_thumbnail {
-        match post.url {
-          Some(url) => generate_pictrs_thumbnail(&url, &context)
-            .await
-            .ok()
-            .map(Into::into),
-          None => None,
-        }
-      } else {
-        None
-      }
-    } else {
-      // Use custom thumbnail if available and its not an image post
-      if let Some(custom_thumbnail) = custom_thumbnail {
-        proxy_image_link(custom_thumbnail, &context).await.ok()
-      }
-      // Use federated thumbnail if available
-      else if let Some(federated_thumbnail) = federated_thumbnail {
-        proxy_image_link(federated_thumbnail, &context).await.ok()
-      }
-      // Generate local thumbnail if allowed
-      else if allow_generate_thumbnail {
-        match metadata.opengraph_data.image {
-          Some(url) => generate_pictrs_thumbnail(&url, &context)
-            .await
-            .ok()
-            .map(Into::into),
-          None => None,
-        }
-      }
-      // Otherwise use opengraph preview image directly
-      else {
-        metadata.opengraph_data.image
-      }
-    };
-
-    let form = PostUpdateForm {
-      embed_title: Some(metadata.opengraph_data.title),
-      embed_description: Some(metadata.opengraph_data.description),
-      embed_video_url: Some(metadata.opengraph_data.embed_video_url),
-      thumbnail_url: Some(thumbnail_url),
-      url_content_type: Some(metadata.content_type),
-      ..Default::default()
-    };
-    let updated_post = Post::update(&mut context.pool(), post.id, &form).await?;
-    if let Some(send_activity) = send_activity(updated_post) {
-      ActivityChannel::submit_activity(send_activity, &context).await?;
-    }
-    Ok(())
-  });
+    generate_post_link_metadata(post, custom_thumbnail, send_activity, local_site, context).await
+  })
 }
 
 /// Extract site metadata from HTML Opengraph attributes.

crates/api_crud/src/post/create.rs

@@ -157,11 +157,11 @@ pub async fn create_post(
   generate_post_link_metadata(
     updated_post.clone(),
     custom_thumbnail,
-    None,
     |post| Some(SendActivityData::CreatePost(post)),
     Some(local_site),
     context.reset_request_count(),
-  );
+  )
+  .await?;
 
   // They like their own post by default
   let person_id = local_user_view.person.id;

crates/api_crud/src/post/update.rs

@@ -112,11 +112,11 @@ pub async fn update_post(
   generate_post_link_metadata(
     updated_post.clone(),
     custom_thumbnail,
-    None,
     |post| Some(SendActivityData::UpdatePost(post)),
     Some(local_site),
     context.reset_request_count(),
-  );
+  )
+  .await?;
 
   build_post_response(
     context.deref(),

crates/apub/src/objects/post.rs

@@ -24,13 +24,8 @@ use chrono::{DateTime, Utc};
 use html2text::{from_read_with_decorator, render::text_renderer::TrivialDecorator};
 use lemmy_api_common::{
   context::LemmyContext,
-  request::generate_post_link_metadata,
-  utils::{
-    get_url_blocklist,
-    local_site_opt_to_slur_regex,
-    process_markdown_opt,
-    proxy_image_link_opt_apub,
-  },
+  request::generate_post_link_metadata_background,
+  utils::{get_url_blocklist, local_site_opt_to_slur_regex, process_markdown_opt},
 };
 use lemmy_db_schema::{
   source::{
@@ -237,8 +232,6 @@ impl Object for ApubPost {
 
       let alt_text = first_attachment.cloned().and_then(Attachment::alt_text);
 
-      let url = proxy_image_link_opt_apub(url, context).await?;
-
       let slur_regex = &local_site_opt_to_slur_regex(&local_site);
       let url_blocklist = get_url_blocklist(context).await?;
 
@@ -278,10 +271,9 @@ impl Object for ApubPost {
     let timestamp = page.updated.or(page.published).unwrap_or_else(naive_now);
     let post = Post::insert_apub(&mut context.pool(), timestamp, &form).await?;
 
-    generate_post_link_metadata(
+    generate_post_link_metadata_background(
       post.clone(),
       None,
-      page.image.map(|i| i.url),
       |_| None,
       local_site,
       context.reset_request_count(),

crates/db_schema/src/impls/images.rs

@@ -12,12 +12,11 @@ use crate::{
   utils::{get_conn, DbPool},
 };
 use diesel::{
-  dsl::exists,
   insert_into,
   result::Error,
-  select,
   ExpressionMethods,
   NotFound,
+  OptionalExtension,
   QueryDsl,
 };
 use diesel_async::RunQueryDsl;
@@ -60,13 +59,12 @@ impl RemoteImage {
 
   pub async fn validate(pool: &mut DbPool<'_>, link_: DbUrl) -> Result<(), Error> {
     let conn = &mut get_conn(pool).await?;
-
-    let exists = select(exists(
-      remote_image::table.filter(remote_image::link.eq(link_)),
-    ))
-    .get_result::<bool>(conn)
-    .await?;
-    if exists {
+    let res = remote_image::table
+      .find(link_)
+      .first::<RemoteImage>(conn)
+      .await
+      .optional()?;
+    if res.is_some() {
       Ok(())
     } else {
       Err(NotFound)
@@ -79,6 +77,7 @@ impl ImageDetails {
     let conn = &mut get_conn(pool).await?;
     insert_into(image_details::table)
       .values(form)
+      .on_conflict_do_nothing()
       .get_result::<Self>(conn)
       .await
   }

crates/db_schema/src/source/images.rs

@@ -43,8 +43,7 @@ pub struct LocalImageForm {
 /// is checked against this table to avoid Lemmy being used as a general purpose proxy.
 #[skip_serializing_none]
 #[derive(Clone, PartialEq, Eq, Debug, Serialize, Deserialize)]
-#[cfg_attr(feature = "full", derive(Queryable, Selectable, Identifiable, TS))]
-#[cfg_attr(feature = "full", ts(export))]
+#[cfg_attr(feature = "full", derive(Queryable, Selectable, Identifiable))]
 #[cfg_attr(feature = "full", diesel(table_name = remote_image))]
 #[cfg_attr(feature = "full", diesel(check_for_backend(diesel::pg::Pg)))]
 #[cfg_attr(feature = "full", diesel(primary_key(link)))]

docker/federation/docker-compose.yml

@@ -49,7 +49,7 @@ services:
 
   pictrs:
     restart: always
-    image: asonix/pictrs:0.5.0-rc.2
+    image: asonix/pictrs:0.5.13
     user: 991:991
     volumes:
       - ./volumes/pictrs_alpha:/mnt:Z