Skip to content

Commit

Permalink
Image api rework (#5260)
Browse files Browse the repository at this point in the history 
* Split image endpoints into API v3 and v4

* Move into subfolders

* Upload avatar endpoint and other changes

* Various other changes

fixes #1772
fixes #4001

* clippy

* config options

* fix ts bindings

* fix api tests

* Add option to disable image upload (fixes #1118)

* split files into upload, download

* move sitemap to top level, not in api

* simplify code

* add upload user banner

* community icon/banner

* site icon/banner

* update js client

* wip

* add delete endpoints

* change comment

* optimization

Co-authored-by: dullbananas <[email protected]>

* move fn

* 1024px banner

* dont use static client

* fix api tests

* shear

* proxy pictrs in request.rs (fixes #5270)

* clippy

* try to fix api tests

* skip api tests

* create user

* debug

* dbg

* test

* image

* run another

* fixed?

* clippy

* fix

* fix health check

---------

Co-authored-by: dullbananas <[email protected]>
  • Loading branch information
@Nutomic @dullbananas
Nutomic and dullbananas authored Jan 13, 2025
1 parent c08e216 commit a91a03a
Show file tree
Hide file tree
Showing 36 changed files with 1,187 additions and 889 deletions.
2 changes: 1 addition & 1 deletion api_tests/package.json
View file
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-reports-combined.3",
"lemmy-js-client": "0.20.0-image-api-rework.8",
"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
View file

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

88 changes: 40 additions & 48 deletions api_tests/src/image.spec.ts
View file
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 @@ -41,8 +41,8 @@ afterAll(async () => {
});

test("Upload image and delete it", async () => {
const healthz = await fetch(alphaUrl + "/pictrs/healthz");
expect(healthz.status).toBe(200);
const health = await alpha.imageHealth();
expect(health.success).toBeTruthy();

// Before running this test, you need to delete all previous images in the DB
await deleteAllImages(alpha);
Expand All @@ -53,13 +53,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 @@ -76,26 +75,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 @@ -118,13 +112,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 @@ -137,7 +130,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 @@ -150,23 +143,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 @@ -177,7 +169,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 @@ -199,11 +191,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 @@ -222,12 +214,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 @@ -249,7 +241,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 @@ -267,7 +259,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 @@ -291,14 +283,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 @@ -311,7 +303,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 @@ -333,7 +325,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 @@ -343,7 +335,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 @@ -362,14 +354,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);
});
8 changes: 2 additions & 6 deletions api_tests/src/shared.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import {
CommunityId,
CommunityVisibility,
CreatePrivateMessageReport,
DeleteImage,
EditCommunity,
GetCommunityPendingFollowsCountResponse,
GetReplies,
Expand All @@ -18,6 +17,7 @@ import {
ListReports,
ListReportsResponse,
MyUserInfo,
DeleteImageParams,
PersonId,
PostView,
PrivateMessageReportResponse,
Expand Down Expand Up @@ -714,17 +714,13 @@ export async function saveUserSettingsBio(
export async function saveUserSettingsFederated(
api: LemmyHttp,
): Promise<SuccessResponse> {
let avatar = sampleImage;
let banner = sampleImage;
let bio = "a changed bio";
let form: SaveUserSettings = {
show_nsfw: false,
blur_nsfw: true,
default_post_sort_type: "Hot",
default_listing_type: "All",
interface_language: "",
avatar,
banner,
display_name: "user321",
show_avatars: false,
send_notifications_to_email: false,
Expand Down Expand Up @@ -939,7 +935,7 @@ export async function deleteAllImages(api: LemmyHttp) {
Promise.all(
imagesRes.images
.map(image => {
const form: DeleteImage = {
const form: DeleteImageParams = {
token: image.local_image.pictrs_delete_token,
filename: image.local_image.pictrs_alias,
};
Expand Down
36 changes: 14 additions & 22 deletions api_tests/src/user.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import {
fetchFunction,
alphaImage,
unfollows,
saveUserSettingsBio,
getMyUser,
getPersonDetails,
} from "./shared";
Expand Down Expand Up @@ -192,43 +191,36 @@ test("Set a new avatar, old avatar is deleted", async () => {
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);
await alpha.uploadUserAvatar(upload_form1);
const listMediaRes1 = await alphaImage.listMedia();
expect(listMediaRes1.images.length).toBe(1);

let my_user1 = await alpha.getMyUser();
expect(my_user1.local_user_view.person.avatar).toBeDefined();

const upload_form2: UploadImage = {
image: Buffer.from("test2"),
};
const upload2 = await alphaImage.uploadImage(upload_form2);
expect(upload2.url).toBeDefined();

let form2 = {
avatar: upload2.url,
};
await saveUserSettings(alpha, form2);
await alpha.uploadUserAvatar(upload_form2);
// make sure only the new avatar is kept
const listMediaRes2 = await alphaImage.listMedia();
expect(listMediaRes2.images.length).toBe(1);

// Upload that same form2 avatar, make sure it isn't replaced / deleted
await saveUserSettings(alpha, form2);
await alpha.uploadUserAvatar(upload_form2);
// make sure only the new avatar is kept
const listMediaRes3 = await alphaImage.listMedia();
expect(listMediaRes3.images.length).toBe(1);

// Now try to save a user settings, with the icon missing,
// and make sure it doesn't clear the data, or delete the image
await saveUserSettingsBio(alpha);
let my_user = await getMyUser(alpha);
expect(my_user.local_user_view.person.avatar).toBe(upload2.url);

// make sure only the new avatar is kept
const listMediaRes4 = await alphaImage.listMedia();
expect(listMediaRes4.images.length).toBe(1);

// delete the avatar
await alpha.deleteUserAvatar();
// make sure only the new avatar is kept
const listMediaRes5 = await alphaImage.listMedia();
expect(listMediaRes5.images.length).toBe(0);
let my_user2 = await alpha.getMyUser();
expect(my_user2.local_user_view.person.avatar).toBeUndefined();
});
Loading

0 comments on commit a91a03a

Please sign in to comment.