Adding admin purging of DB items and pictures. #904 #1331 #1809

dessalines · 2021-10-01T16:22:32Z

No description provided.

crates/api/src/site.rs

crates/db_schema/src/source/moderator.rs

- Added pictrs_config block, for API_KEY - Clear out image columns after purging

dessalines · 2022-06-01T18:36:01Z

This is getting close, I just need to build out the front end for it.

Nutomic · 2022-06-02T16:33:26Z

crates/utils/src/settings/structs.rs

+  /// Set a custom pictrs API key. ( Required for deleting images )
+  #[default("API_KEY")]
+  #[doku(example = "API_KEY")]
+  pub api_key: String,


I think you should put an actual (testing) api key here, to show how it looks. And not specify any default i guess.

I'll remove the default, but I'm not sure what would be better than just API_KEY as a test.

Once this is merged, in lemmy-ansible, I plan to generate this key in the same way the postgres password is.

Its not meant to be a test, but an example, so it should look similar to a real value. With API_KEY, people might think that they need an env var for this.

Nutomic · 2022-06-02T16:33:45Z

crates/utils/src/settings/structs.rs

+  /// Address where pictrs is available (for image hosting)
+  #[default("http://pictrs:8080")]
+  #[doku(example = "http://localhost:8080")]
+  pub url: String,


I think its enough to specify only default in this case.

Nutomic · 2022-06-02T16:35:08Z

crates/routes/src/images.rs

+struct PictrsPurgeParams {
+  file: Option<String>,
+  alias: Option<String>,
+}


You should be able to make this work a bit more cleanly using enum.

https://serde.rs/enum-representations.html

Nutomic · 2022-06-02T16:35:59Z

crates/routes/src/images.rs

+  Ok(HttpResponse::build(res.status()).body(BodyStream::new(res.bytes_stream())))
+}
+
+fn pictrs_config(settings: Settings) -> Result<PictrsConfig, LemmyError> {


Put this method into impl settings, and then probably make the member private.

Nutomic · 2022-06-02T16:40:10Z

crates/db_schema/src/impls/moderator.rs

+      .set(form)
+      .get_result::<Self>(conn)
+  }
+}


Would be nice to use some macro to generate these crud methods, and avoid all the copy-paste. For example https://github.com/shssoichiro/diesel-derives-extra (but looks unmaintained). Anyway, not necessarily in this pr.

Nutomic · 2022-06-02T16:40:30Z

crates/apub/src/activities/deletion/delete_user.rs

+      actor.id,
+      context.pool(),
+      &context.settings(),
+      context.client(),


Same, just pass in context. I would start passing in context directly as soon as there are 2 or more params from it.

Unfortunately that'd require lemmy_utils pulling in lemmy_websocket, since that's where LemmyContext lives.

Nutomic · 2022-06-02T16:40:48Z

crates/apub/src/activities/block/block_user.rs

+            blocked_person.id,
+            context.pool(),
+            &context.settings(),
+            context.client(),


Just pass in context.

Nutomic · 2022-06-02T16:42:19Z

crates/api_common/src/utils.rs

@@ -505,13 +506,99 @@ pub async fn check_private_instance_and_federation_enabled(
  Ok(())
 }

-pub async fn remove_user_data(banned_person_id: PersonId, pool: &DbPool) -> Result<(), LemmyError> {
+pub async fn purge_image_posts_for_person(


This file is already quite big, so please put all the purge/remove related stuff into a new file. Or turn utils into a folder with multiple files.

Can do that in a separate PR, I'll make an issue for it. Utils touches almost every file so that'd make this PR huge.

Nutomic · 2022-06-02T16:45:29Z

crates/api_common/src/request.rs

+        "Could not purge Pictrs image with incorrect domain: {}",
+        image_url.as_str()
+      )));
+    }


Is this check necessary? If something is wrong, pictrs will return an error on its own. But if someone changes the instance url, this would prevent image purges for no reason.

You're probably right, it's not necessary.

Nutomic · 2022-06-02T16:47:51Z

crates/api_common/src/request.rs

+  settings: &Settings,
+  image_url: &Url,
+) -> Result<(), LemmyError> {
+  if let Some(pictrs_config) = settings.pictrs_config.to_owned() {


Use the pictrs_config function you defined in lemmy_routes (after moving it to lemmy_utils).

Nutomic · 2022-06-02T16:50:15Z

crates/api/src/site/purge/comment.rs

+
+    let post_id = comment.post_id;
+
+    // TODO read comments for pictrs images and purge them


This is about inline images in comments, right? And similarly for inline images in post body, community/user profile. Those are generally tricky to manage. Easiest option would be not to permit inline images, otherwise we will have to add some functionality to retrieve them sooner or later.

Yep. We could have something that scans through every comment, post body, bio, community sidebar... ( there's probably a few others I'm not thinking of), does a regex search for pictrs URLs, and tries to remove them. IMO that's a little excessive, but it might be necessary later on. I'll just leave that TODO in there to remind us.

Nutomic · 2022-06-02T16:50:38Z

crates/api/src/site/mod_log.rs

+        Vec::new(),
+        Vec::new(),
+        Vec::new(),
+      )


Should work with simply Default::Default()

dessalines · 2022-06-03T17:16:05Z

I have the front end for this done also, everything works.

dessalines · 2022-06-03T18:02:45Z

Actually something in the merge from main broke image uploads, I'm tracing it down.

dessalines · 2022-06-03T18:12:35Z

@Nutomic This line breaks pictrs functionality. All uploads and such return the error:

Middleware error: Request object is not clonable. Are you passing a streaming body?

I think the only option is for me to create a separate pictrs reqwest client without that middleware.

Nutomic · 2022-06-06T19:30:20Z

Yes it should be pretty straightforward to define a Lazy<Client> in images.rs. Also we should open an issue in the upstream repo to get this fixed.

https://github.com/TrueLayer/reqwest-middleware

dessalines · 2022-06-08T15:09:28Z

Yes it should be pretty straightforward to define a Lazy<Client> in images.rs. Also we should open an issue in the upstream repo to get this fixed.

I did this in main.rs, a special pictrs client. That way it can be reused.

Nutomic reviewed Oct 2, 2021

View reviewed changes

crates/api/src/site.rs Outdated Show resolved Hide resolved

Nutomic reviewed Oct 2, 2021

View reviewed changes

crates/db_schema/src/source/moderator.rs Show resolved Hide resolved

dessalines added 3 commits October 15, 2021 17:01

First pass at adding admin purge. #904 #1331

377c5b2

Breaking out purge into 4 tables for the 4 purgeable types.

1671eb6

Using CommunitySafe instead in view

4d3a210

dessalines force-pushed the admin_purge branch from e9f9d80 to 4d3a210 Compare October 15, 2021 21:51

dessalines added 6 commits March 17, 2022 15:20

Merge branch 'main' into admin_purge

47a2683

Merge branch 'main' into admin_purge

2175426

Fix db_schema features flags.

7bd0ff2

Attempting to pass API key.

020b658

Adding pictrs image purging

d011b7a

- Added pictrs_config block, for API_KEY - Clear out image columns after purging

Merge branch 'main' into admin_purge

0886369

dessalines changed the title ~~First pass at adding admin purge. #904 #1331~~ Adding admin purging of DB items and pictures. #904 #1331 Jun 1, 2022

dessalines added the breaking change label Jun 1, 2022

Remove the remove_images field from a few of the purge API calls.

f1b0c23

Nutomic reviewed Jun 2, 2022

View reviewed changes

crates/api/src/site/mod_log.rs Outdated

Vec::new(),

Vec::new(),

Vec::new(),

)

Copy link

Member

Nutomic Jun 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should work with simply Default::Default()

dessalines added 2 commits June 3, 2022 13:03

Fix some suggestions by @Nutomic.

06d51a2

Merge branch 'main' into admin_purge

3456da1

dessalines requested a review from Nutomic June 3, 2022 17:15

dessalines marked this pull request as ready for review June 3, 2022 17:15

dessalines marked this pull request as draft June 3, 2022 18:02

Add separate pictrs reqwest client.

c57c5cb

dessalines marked this pull request as ready for review June 3, 2022 23:12

Update defaults.hjson

01fb200

dessalines mentioned this pull request Jun 8, 2022

Middleware error: Request object is not clonable. Are you passing a streaming body? TrueLayer/reqwest-middleware#44

Closed

dessalines added 2 commits June 8, 2022 11:09

Merge branch 'main' into admin_purge

dbfbd2c

Merge branch 'main' into admin_purge

7de237e

Nutomic approved these changes Jun 13, 2022

View reviewed changes

Merge branch 'main' into admin_purge

a90fa0b

Nutomic enabled auto-merge (squash) June 13, 2022 18:52

Nutomic merged commit 4e12e25 into main Jun 13, 2022

Nutomic mentioned this pull request Dec 13, 2022

Have a way of purging "orphaned" user-uploaded images #1331

Closed

asimons04 mentioned this pull request Jun 21, 2023

Log User Media Uploads to the Database #3236

Closed

4 tasks

Djones4822 mentioned this pull request Jul 6, 2023

[Bug]: Images uploaded and linked in post bodies are not purged but the references to them are lost #3504

Open

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding admin purging of DB items and pictures. #904 #1331 #1809

Adding admin purging of DB items and pictures. #904 #1331 #1809

dessalines commented Oct 1, 2021

dessalines commented Jun 1, 2022

Nutomic Jun 2, 2022

dessalines Jun 3, 2022

Nutomic Jun 6, 2022

Nutomic Jun 2, 2022

Nutomic Jun 2, 2022

Nutomic Jun 2, 2022 •

edited

Loading

Nutomic Jun 2, 2022

Nutomic Jun 2, 2022 •

edited

Loading

dessalines Jun 3, 2022

Nutomic Jun 2, 2022 •

edited

Loading

Nutomic Jun 2, 2022 •

edited

Loading

dessalines Jun 3, 2022

Nutomic Jun 2, 2022

dessalines Jun 3, 2022

Nutomic Jun 2, 2022

Nutomic Jun 2, 2022

dessalines Jun 3, 2022

Nutomic Jun 2, 2022

dessalines commented Jun 3, 2022

dessalines commented Jun 3, 2022

dessalines commented Jun 3, 2022

Nutomic commented Jun 6, 2022 •

edited

Loading

dessalines commented Jun 8, 2022


		let post_id = comment.post_id;

		// TODO read comments for pictrs images and purge them

Adding admin purging of DB items and pictures. #904 #1331 #1809

Adding admin purging of DB items and pictures. #904 #1331 #1809

Conversation

dessalines commented Oct 1, 2021

dessalines commented Jun 1, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nutomic Jun 2, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nutomic Jun 2, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nutomic Jun 2, 2022 • edited Loading

Choose a reason for hiding this comment

Nutomic Jun 2, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dessalines commented Jun 3, 2022

dessalines commented Jun 3, 2022

dessalines commented Jun 3, 2022

Nutomic commented Jun 6, 2022 • edited Loading

dessalines commented Jun 8, 2022

Nutomic Jun 2, 2022 •

edited

Loading

Nutomic Jun 2, 2022 •

edited

Loading

Nutomic Jun 2, 2022 •

edited

Loading

Nutomic Jun 2, 2022 •

edited

Loading

Nutomic commented Jun 6, 2022 •

edited

Loading