Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Image api rework #5260

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion api_tests/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
"eslint": "^9.14.0",
"eslint-plugin-prettier": "^5.1.3",
"jest": "^29.5.0",
"lemmy-js-client": "0.20.0-api-v4.16",
"lemmy-js-client": "0.20.0-image-api-rework.3",
"prettier": "^3.2.5",
"ts-jest": "^29.1.0",
"typescript": "^5.5.4",
Expand Down
10 changes: 5 additions & 5 deletions api_tests/pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

84 changes: 38 additions & 46 deletions api_tests/src/image.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ jest.setTimeout(120000);

import {
UploadImage,
DeleteImage,
PurgePerson,
PurgePost,
DeleteImageParams,
} from "lemmy-js-client";
import {
alpha,
Expand Down Expand Up @@ -54,13 +54,12 @@ test("Upload image and delete it", async () => {
image: Buffer.from("test"),
};
const upload = await alphaImage.uploadImage(upload_form);
expect(upload.files![0].file).toBeDefined();
expect(upload.files![0].delete_token).toBeDefined();
expect(upload.url).toBeDefined();
expect(upload.delete_url).toBeDefined();
expect(upload.image_url).toBeDefined();
expect(upload.filename).toBeDefined();
expect(upload.delete_token).toBeDefined();

// ensure that image download is working. theres probably a better way to do this
const response = await fetch(upload.url ?? "");
const response = await fetch(upload.image_url ?? "");
const content = await response.text();
expect(content.length).toBeGreaterThan(0);

Expand All @@ -77,26 +76,21 @@ test("Upload image and delete it", async () => {
const previousThumbnails = 1;
expect(listAllMediaRes.images.length).toBe(previousThumbnails);

// The deleteUrl is a combination of the endpoint, delete token, and alias
let firstImage = listMediaRes.images[0];
let deleteUrl = `${alphaUrl}/pictrs/image/delete/${firstImage.local_image.pictrs_delete_token}/${firstImage.local_image.pictrs_alias}`;
expect(deleteUrl).toBe(upload.delete_url);

// Make sure the uploader is correct
expect(firstImage.person.actor_id).toBe(
expect(listMediaRes.images[0].person.actor_id).toBe(
`http://lemmy-alpha:8541/u/lemmy_alpha`,
);

// delete image
const delete_form: DeleteImage = {
token: upload.files![0].delete_token,
filename: upload.files![0].file,
const delete_form: DeleteImageParams = {
token: upload.delete_token,
filename: upload.filename,
};
const delete_ = await alphaImage.deleteImage(delete_form);
expect(delete_).toBe(true);
expect(delete_.success).toBe(true);

// ensure that image is deleted
const response2 = await fetch(upload.url ?? "");
const response2 = await fetch(upload.image_url ?? "");
const content2 = await response2.text();
expect(content2).toBe("");

Expand All @@ -119,13 +113,12 @@ test("Purge user, uploaded image removed", async () => {
image: Buffer.from("test"),
};
const upload = await user.uploadImage(upload_form);
expect(upload.files![0].file).toBeDefined();
expect(upload.files![0].delete_token).toBeDefined();
expect(upload.url).toBeDefined();
expect(upload.delete_url).toBeDefined();
expect(upload.filename).toBeDefined();
expect(upload.delete_token).toBeDefined();
expect(upload.image_url).toBeDefined();

// ensure that image download is working. theres probably a better way to do this
const response = await fetch(upload.url ?? "");
const response = await fetch(upload.image_url ?? "");
const content = await response.text();
expect(content.length).toBeGreaterThan(0);

Expand All @@ -138,7 +131,7 @@ test("Purge user, uploaded image removed", async () => {
expect(delete_.success).toBe(true);

// ensure that image is deleted
const response2 = await fetch(upload.url ?? "");
const response2 = await fetch(upload.image_url ?? "");
const content2 = await response2.text();
expect(content2).toBe("");
});
Expand All @@ -151,23 +144,22 @@ test("Purge post, linked image removed", async () => {
image: Buffer.from("test"),
};
const upload = await user.uploadImage(upload_form);
expect(upload.files![0].file).toBeDefined();
expect(upload.files![0].delete_token).toBeDefined();
expect(upload.url).toBeDefined();
expect(upload.delete_url).toBeDefined();
expect(upload.filename).toBeDefined();
expect(upload.delete_token).toBeDefined();
expect(upload.image_url).toBeDefined();

// ensure that image download is working. theres probably a better way to do this
const response = await fetch(upload.url ?? "");
const response = await fetch(upload.image_url ?? "");
const content = await response.text();
expect(content.length).toBeGreaterThan(0);

let community = await resolveBetaCommunity(user);
let post = await createPost(
user,
community.community!.community.id,
upload.url,
upload.image_url,
);
expect(post.post_view.post.url).toBe(upload.url);
expect(post.post_view.post.url).toBe(upload.image_url);
expect(post.post_view.image_details).toBeDefined();

// purge post
Expand All @@ -178,7 +170,7 @@ test("Purge post, linked image removed", async () => {
expect(delete_.success).toBe(true);

// ensure that image is deleted
const response2 = await fetch(upload.url ?? "");
const response2 = await fetch(upload.image_url ?? "");
const content2 = await response2.text();
expect(content2).toBe("");
});
Expand All @@ -200,11 +192,11 @@ test("Images in remote image post are proxied if setting enabled", async () => {
// remote image gets proxied after upload
expect(
post.thumbnail_url?.startsWith(
"http://lemmy-gamma:8561/api/v4/image_proxy?url",
"http://lemmy-gamma:8561/api/v4/image/proxy?url",
),
).toBeTruthy();
expect(
post.body?.startsWith("![](http://lemmy-gamma:8561/api/v4/image_proxy?url"),
post.body?.startsWith("![](http://lemmy-gamma:8561/api/v4/image/proxy?url"),
).toBeTruthy();

// Make sure that it ends with jpg, to be sure its an image
Expand All @@ -223,12 +215,12 @@ test("Images in remote image post are proxied if setting enabled", async () => {

expect(
epsilonPost.thumbnail_url?.startsWith(
"http://lemmy-epsilon:8581/api/v4/image_proxy?url",
"http://lemmy-epsilon:8581/api/v4/image/proxy?url",
),
).toBeTruthy();
expect(
epsilonPost.body?.startsWith(
"![](http://lemmy-epsilon:8581/api/v4/image_proxy?url",
"![](http://lemmy-epsilon:8581/api/v4/image/proxy?url",
),
).toBeTruthy();

Expand All @@ -250,7 +242,7 @@ test("Thumbnail of remote image link is proxied if setting enabled", async () =>
// remote image gets proxied after upload
expect(
post.thumbnail_url?.startsWith(
"http://lemmy-gamma:8561/api/v4/image_proxy?url",
"http://lemmy-gamma:8561/api/v4/image/proxy?url",
),
).toBeTruthy();

Expand All @@ -268,7 +260,7 @@ test("Thumbnail of remote image link is proxied if setting enabled", async () =>

expect(
epsilonPost.thumbnail_url?.startsWith(
"http://lemmy-epsilon:8581/api/v4/image_proxy?url",
"http://lemmy-epsilon:8581/api/v4/image/proxy?url",
),
).toBeTruthy();

Expand All @@ -292,14 +284,14 @@ test("No image proxying if setting is disabled", async () => {
let post = await createPost(
alpha,
community.community_view.community.id,
upload.url,
upload.image_url,
`![](${sampleImage})`,
);
expect(post.post_view.post).toBeDefined();

// remote image doesn't get proxied after upload
expect(
post.post_view.post.url?.startsWith("http://127.0.0.1:8551/pictrs/image/"),
post.post_view.post.url?.startsWith("http://lemmy-beta:8551/api/v4/image/"),
).toBeTruthy();
expect(post.post_view.post.body).toBe(`![](${sampleImage})`);

Expand All @@ -312,7 +304,7 @@ test("No image proxying if setting is disabled", async () => {

// remote image doesn't get proxied after federation
expect(
betaPost.post.url?.startsWith("http://127.0.0.1:8551/pictrs/image/"),
betaPost.post.url?.startsWith("http://lemmy-beta:8551/api/v4/image/"),
).toBeTruthy();
expect(betaPost.post.body).toBe(`![](${sampleImage})`);
// Make sure the alt text got federated
Expand All @@ -334,7 +326,7 @@ test("Make regular post, and give it a custom thumbnail", async () => {
alphaImage,
community.community_view.community.id,
wikipediaUrl,
upload1.url!,
upload1.image_url!,
);

// Wait for the metadata to get fetched, since this is backgrounded now
Expand All @@ -344,7 +336,7 @@ test("Make regular post, and give it a custom thumbnail", async () => {
);
expect(post.post_view.post.url).toBe(wikipediaUrl);
// Make sure it uses custom thumbnail
expect(post.post_view.post.thumbnail_url).toBe(upload1.url);
expect(post.post_view.post.thumbnail_url).toBe(upload1.image_url);
});

test("Create an image post, and make sure a custom thumbnail doesn't overwrite it", async () => {
Expand All @@ -363,14 +355,14 @@ test("Create an image post, and make sure a custom thumbnail doesn't overwrite i
let post = await createPostWithThumbnail(
alphaImage,
community.community_view.community.id,
upload1.url!,
upload2.url!,
upload1.image_url!,
upload2.image_url!,
);
post = await waitUntil(
() => getPost(alphaImage, post.post_view.post.id),
p => p.post_view.post.thumbnail_url != undefined,
);
expect(post.post_view.post.url).toBe(upload1.url);
expect(post.post_view.post.url).toBe(upload1.image_url);
// Make sure the custom thumbnail is ignored
expect(post.post_view.post.thumbnail_url == upload2.url).toBe(false);
expect(post.post_view.post.thumbnail_url == upload2.image_url).toBe(false);
});
10 changes: 9 additions & 1 deletion config/defaults.hjson
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
# or

# If enabled, all images from remote domains are rewritten to pass through
# `/api/v4/image_proxy`, including embedded images in markdown. Images are stored temporarily
# `/api/v4/image/proxy`, including embedded images in markdown. Images are stored temporarily
# in pict-rs for caching. This improves privacy as users don't expose their IP to untrusted
# servers, and decreases load on other servers. However it increases bandwidth use for the
# local server.
Expand All @@ -64,6 +64,14 @@
upload_timeout: 30
# Resize post thumbnails to this maximum width/height.
max_thumbnail_size: 512
# Maximum size for user avatar, community icon and site icon.
max_avatar_size: 512
# Maximum size for user, community and site banner.
#
# TODO: Unfortunately pictrs can only resize images to fit in a*a square, no rectangle.
# Otherwise we have to use crop, or use max_width/max_height which throws error
# if image is larger.
max_banner_size: 512
}
# Email sending configuration. All options except login/password are mandatory
email: {
Expand Down
5 changes: 0 additions & 5 deletions crates/api/src/local_user/save_settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,6 @@ pub async fn save_user_settings(
.as_deref(),
);

let avatar = diesel_url_update(data.avatar.as_deref())?;
replace_image(&avatar, &local_user_view.person.avatar, &context).await?;
let avatar = proxy_image_link_opt_api(avatar, &context).await?;

let banner = diesel_url_update(data.banner.as_deref())?;
replace_image(&banner, &local_user_view.person.banner, &context).await?;
let banner = proxy_image_link_opt_api(banner, &context).await?;
Expand Down Expand Up @@ -108,7 +104,6 @@ pub async fn save_user_settings(
bio,
matrix_user_id,
bot_account: data.bot_account,
avatar,
banner,
..Default::default()
};
Expand Down
46 changes: 46 additions & 0 deletions crates/api_common/src/image.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
use serde::{Deserialize, Serialize};
use serde_with::skip_serializing_none;
#[cfg(feature = "full")]
use ts_rs::TS;
use url::Url;

#[skip_serializing_none]
#[derive(Debug, Serialize, Deserialize, Clone, Default, PartialEq, Eq, Hash)]
#[cfg_attr(feature = "full", derive(TS))]
#[cfg_attr(feature = "full", ts(export))]
pub struct ImageGetParams {
#[cfg_attr(feature = "full", ts(optional))]
pub file_type: Option<String>,
#[cfg_attr(feature = "full", ts(optional))]
pub max_size: Option<i32>,
}

#[derive(Debug, Serialize, Deserialize, Clone, Default, PartialEq, Eq, Hash)]
#[cfg_attr(feature = "full", derive(TS))]
#[cfg_attr(feature = "full", ts(export))]
pub struct DeleteImageParams {
pub filename: String,
pub token: String,
}

#[skip_serializing_none]
#[derive(Debug, Serialize, Deserialize, Clone, Default, PartialEq, Eq, Hash)]
#[cfg_attr(feature = "full", derive(TS))]
#[cfg_attr(feature = "full", ts(export))]
pub struct ImageProxyParams {
pub url: String,
#[cfg_attr(feature = "full", ts(optional))]
pub file_type: Option<String>,
#[cfg_attr(feature = "full", ts(optional))]
pub max_size: Option<i32>,
}

#[skip_serializing_none]
#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq, Hash)]
#[cfg_attr(feature = "full", derive(TS))]
#[cfg_attr(feature = "full", ts(export))]
pub struct UploadImageResponse {
pub image_url: Url,
pub filename: String,
pub delete_token: String,
}
1 change: 1 addition & 0 deletions crates/api_common/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ pub mod community;
#[cfg(feature = "full")]
pub mod context;
pub mod custom_emoji;
pub mod image;
pub mod oauth_provider;
pub mod person;
pub mod post;
Expand Down
3 changes: 0 additions & 3 deletions crates/api_common/src/person.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,6 @@ pub struct SaveUserSettings {
/// The language of the lemmy interface
#[cfg_attr(feature = "full", ts(optional))]
pub interface_language: Option<String>,
/// A URL for your avatar.
#[cfg_attr(feature = "full", ts(optional))]
pub avatar: Option<String>,
/// A URL for your banner.
#[cfg_attr(feature = "full", ts(optional))]
pub banner: Option<String>,
Expand Down
Loading