Skip to content

Commit

Permalink
Various other changes
Browse files Browse the repository at this point in the history
fixes #1772
fixes #4001
  • Loading branch information
Nutomic committed Dec 13, 2024
1 parent cfa866a commit d252be2
Show file tree
Hide file tree
Showing 16 changed files with 208 additions and 217 deletions.
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.

2 changes: 1 addition & 1 deletion api_tests/run-federation-test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ killall -s1 lemmy_server || true
popd

pnpm i
pnpm api-test-image || true
pnpm api-test || true

killall -s1 lemmy_server || true
killall -s1 pict-rs || true
Expand Down
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);
});
2 changes: 1 addition & 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 Down
42 changes: 42 additions & 0 deletions crates/api_common/src/image.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
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 {
pub file_type: Option<String>,
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,
pub file_type: Option<String>,
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
12 changes: 9 additions & 3 deletions crates/api_common/src/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,9 +263,15 @@ pub struct PictrsFile {
}

impl PictrsFile {
pub fn thumbnail_url(&self, protocol_and_hostname: &str) -> Result<Url, url::ParseError> {
pub fn image_url(&self, protocol_and_hostname: &str) -> Result<Url, url::ParseError> {
Url::parse(&format!(
"{protocol_and_hostname}/pictrs/image/{}",
"{protocol_and_hostname}/api/v4/image/{}",
self.file
))
}
pub fn delete_url(&self, protocol_and_hostname: &str) -> Result<Url, url::ParseError> {
Url::parse(&format!(
"{protocol_and_hostname}/api/v4/image/{}",
self.file
))
}
Expand Down Expand Up @@ -402,7 +408,7 @@ async fn generate_pictrs_thumbnail(image_url: &Url, context: &LemmyContext) -> L
pictrs_delete_token: image.delete_token.clone(),
};
let protocol_and_hostname = context.settings().get_protocol_and_hostname();
let thumbnail_url = image.thumbnail_url(&protocol_and_hostname)?;
let thumbnail_url = image.image_url(&protocol_and_hostname)?;

// Also store the details for the image
let details_form = image.details.build_image_details_form(&thumbnail_url);
Expand Down
4 changes: 2 additions & 2 deletions crates/api_common/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1177,7 +1177,7 @@ fn build_proxied_image_url(
protocol_and_hostname: &str,
) -> Result<Url, url::ParseError> {
Url::parse(&format!(
"{}/api/v4/image_proxy?url={}",
"{}/api/v4/image/proxy?url={}",
protocol_and_hostname,
encode(link.as_str())
))
Expand Down Expand Up @@ -1256,7 +1256,7 @@ mod tests {
)
.await?;
assert_eq!(
"https://lemmy-alpha/api/v4/image_proxy?url=http%3A%2F%2Flemmy-beta%2Fimage.png",
"https://lemmy-alpha/api/v4/image/proxy?url=http%3A%2F%2Flemmy-beta%2Fimage.png",
proxied.as_str()
);

Expand Down
Loading

0 comments on commit d252be2

Please sign in to comment.