-
Notifications
You must be signed in to change notification settings - Fork 83
Add filter to disable local attachment storage #2392
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
Conversation
Introduces the `activitypub_store_attachments_locally` filter to allow users and plugins to disable local storage of attachments from incoming ActivityPub posts. This addresses feedback about providing an option for users with limited webspace who may prefer not to store remote images locally. The filter is applied in both the add() and update() methods of the Posts collection class, with full documentation on the first instance and a reference comment on the second instance following WordPress conventions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a new filter activitypub_store_attachments_locally that allows plugins or users to disable local storage of attachments from incoming ActivityPub posts, which can be useful for users with limited webspace.
- Introduces filter hook to control whether attachments should be stored locally
- Applies the filter in both the
add()andupdate()methods of the Posts collection class - Maintains backward compatibility by defaulting to
true(store locally)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
includes/collection/class-posts.php
Outdated
|
|
||
| if ( $store_locally ) { | ||
| Attachments::delete_ap_posts_directory( $post_id ); | ||
| Attachments::import_post_files( $activity['object']['attachment'], $post_id ); |
Copilot
AI
Oct 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When $store_locally is false, the existing attachments directory is not deleted, which could lead to orphaned files if a post previously had local attachments enabled and is now being updated with the filter returning false. Consider deleting the existing attachments directory before the conditional check, or add an else clause to handle cleanup when the filter is disabled.
| Attachments::import_post_files( $activity['object']['attachment'], $post_id ); | |
| Attachments::import_post_files( $activity['object']['attachment'], $post_id ); | |
| } else { | |
| Attachments::delete_ap_posts_directory( $post_id ); |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Refactors the attachment storage logic: - Extracts filter logic into a reusable maybe_import_attachments() method - Always deletes existing attachments on update before re-importing - This ensures the filter return value is respected even if it changed since the post was first added This addresses feedback that the filter value might change between add and update operations, so we should always clean up and re-evaluate whether to store attachments locally.
|
That is not exactly what I had in mind 🤔 I will try to provide an example later today. |
|
https://wordpress.org/plugins/regenerate-thumbnails/ Using a plugin like Regenerate Thumbnails can pose a significant risk when dealing with a large number of image files. In other words, in the case of Fediverse, performing bulk synchronization of images could be very dangerous. My suggestion is also based on the assumption of using the mediaLib endpoint(+FMO). By specifying the file size in advance, each instance can predictively generate the image itself. Additionally, retrieving and synchronizing the file URL of remote image objects also becomes easier. Inspired by FileBird, I think that using an S2S Media Library endpoint could make it possible not only to query collections or individual image objects, but even to transfer them in bulk as a compressed archive. |
|
The most painful idea I'm dealing with right now is that, However, once I'm trying to design a blog actor model that allows multiple intermediaries (proxies or agents) to exist between the author and the post, As you know, when enabling Multisite in WordPress, additional tables related to blog_id and site_id are created. Neither Mastodon nor Misskey supported ActivityPub from the beginning — their architectures had to be restructured to accommodate federation. So why do you think WordPress could achieve full federation without any core modification? In my opinion, a more proper and sustainable approach would be to first handle core extensions and housekeeping through a canonical plugin, and only then build the ActivityPub layer on top of that foundation. |
@pfefferle Have you had a chance to give this another thought? |
Follow-up to #2314
Addresses #2314 (comment)
Proposed changes:
activitypub_store_attachments_locallyfilter that allows users and plugins to disable local storage of attachments from incoming ActivityPub postsOther information:
Testing instructions:
Default behavior (no change):
wp-content/uploads/activitypub/ap_posts/{post_id}/Disabled storage:
functions.php:Changelog entry