Skip to content

Support for WP REST Cache #1630

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

Merged
merged 11 commits into from
Jun 23, 2025
Merged

Support for WP REST Cache #1630

merged 11 commits into from
Jun 23, 2025

Conversation

obenland
Copy link
Member

@obenland obenland commented Apr 30, 2025

See https://wordpress.org/support/topic/caching-activity-pub-endpoints/
See https://epiph.yt/en/blog/2025/accidental-ddos-through-activitypub-plugin/

Proposed changes:

  • Adds integration with Rest Cache plugin.
  • Adds Activitypub endpoints to allowlist and manages cache invalidation.

Other information:

  • Have you written new tests for your changes, if applicable?

Testing instructions:

  • Install the WP Rest Cache plugin and set it up.
  • Send requests to various Activitypub REST API endpoints and make sure they get cached.
  • Create a user update, something that doesn't create a new post, and make sure it flushes the cache.
  • Send requests to various Activitypub REST API endpoints and make sure they get cached.
  • Comment on a post and make sure it flushes the cache.

Changelog entry

  • Automatically create a changelog entry from the details below.
Changelog Entry Details

Significance

  • Patch
  • Minor
  • Major

Type

  • Added - for new features
  • Changed - for changes in existing functionality
  • Deprecated - for soon-to-be removed features
  • Removed - for now removed features
  • Fixed - for any bug fixes
  • Security - in case of vulnerabilities

Message

Added support for the WP Rest Cache plugin to help with caching REST API responses.

/cc @MatzeKitt

@obenland obenland self-assigned this Apr 30, 2025
@github-actions github-actions bot added [Focus] Compatibility Ensuring the plugin plays well with other plugins [Status] In Progress labels Apr 30, 2025
@obenland
Copy link
Member Author

obenland commented Apr 30, 2025

Proper cache invalidation will be the biggest challenge. New posts, new comments, user meta updates, blog user option updates, etc.

It would also be nice if we could avoid blanket-invalidating all cached ActivityPub endpoints for all of these, and be more targeted.

@MatzeKitt
Copy link
Contributor

So we need to keep track of any change regarding any of the endpoints? Also, do we need to check whether the change does actually change the API endpoint response or should we always invalidate the cache when it might change the response?

I guess we currently have no decorated list of changes we need to look for?

@obenland
Copy link
Member Author

obenland commented May 6, 2025

Also, do we need to check whether the change does actually change the API endpoint response or should we always invalidate the cache when it might change the response?

Looking a bit more at the plugin, I think we'll have to be pretty broad about invalidating caches. Most of the endpoints are user/actor-based, and WP REST Cache doesn't have a concept for that. It's post type and post_id, and that seems to be pretty much it.

I think your approach of defining a custom type and invalidating all associated endpoints is probably the safest.

@obenland obenland marked this pull request as ready for review June 17, 2025 16:54
@Copilot Copilot AI review requested due to automatic review settings June 17, 2025 16:54
@obenland
Copy link
Member Author

Need to do a bit more testing before this is ready for review.

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds integration with the WP Rest Cache plugin to allowlist ActivityPub endpoints and invalidate caches when posts or comments change.

  • Checks for the WP Rest Cache plugin in integration/load.php and initializes the integration.
  • Introduces Activitypub\Integration\WP_Rest_Cache to register allowed endpoints, set cache metadata, and flush caches on status transitions.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
integration/load.php Plugin existence check and call to initialize the WP Rest Cache hook
integration/class-wp-rest-cache.php New class registering filters/actions for caching ActivityPub routes
Comments suppressed due to low confidence (2)

integration/class-wp-rest-cache.php:1

  • No unit or integration tests were added for this caching integration. Consider adding tests to verify allowed endpoints, object type settings, single-item logic, and cache invalidation on status transitions.
<?php

integration/load.php:126

  • The call to WP_Rest_Cache::init() will fail without importing or fully qualifying the Activitypub\Integration\WP_Rest_Cache class. Add use Activitypub\Integration\WP_Rest_Cache; at the top or call \Activitypub\Integration\WP_Rest_Cache::init().
	if ( class_exists( 'WP_Rest_Cache_Plugin\Includes\Plugin' ) ) {

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@pfefferle
Copy link
Member

What about user updates? Shouldn't we have hooks to invalideate the User endpoints on changes? Example: https://notiz.blog/wp-api/activitypub/1.0/actors/1

* @return array
*/
public static function add_activitypub_endpoints( $endpoints ) {
$endpoints[ ACTIVITYPUB_REST_NAMESPACE ] = array( 'actors', 'collections', 'comments', 'interactions', 'nodeinfo', 'posts', 'users' );
Copy link
Member

Choose a reason for hiding this comment

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

isn't the namespace simply activitypub or activitypub/1.0?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, activitypub/1.0

Copy link
Member

Choose a reason for hiding this comment

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

I meant, can we wildcard all the subpathes?

Copy link
Member

Choose a reason for hiding this comment

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

with this specific list, we would have to take care that every new endpoint will also be added here... that is a bit fragile!?!

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah! Yeah, unfortunately the plugin is pretty literal in its allowlist processing:

https://github.com/RedCastor/wp-rest-cache/blob/c6f94c7b61f30770424fa5dd5f4ccd4e0d7f52b9/includes/api/class-endpoint-api.php#L241-L251

We could grab all of our registered routes and format them to fit this list, but it would be like using a sledgehammer to crack a nut. I'm not too concerned, our endpoint routes have been pretty stable and chances are that any new endpoints are likely to be sub-paths of actors, users, etc

@obenland
Copy link
Member Author

What about user updates? Shouldn't we have hooks to invalideate the User endpoints on changes?

They should work, it's a step in the testing instructions "Create a user update, something that doesn't create a new post, and make sure it flushes the cache."

User updates should trigger a flush through the post status change in the Outbox post they produce.

Co-authored-by: Matthias Pfefferle <pfefferle@users.noreply.github.com>
@obenland obenland requested a review from pfefferle June 23, 2025 14:56
obenland and others added 2 commits June 23, 2025 10:56
Co-authored-by: Matthias Pfefferle <pfefferle@users.noreply.github.com>
@obenland obenland merged commit e4ee4d9 into trunk Jun 23, 2025
11 checks passed
@obenland obenland deleted the add/wp-rest-cache-support branch June 23, 2025 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Compatibility Ensuring the plugin plays well with other plugins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants