Update handler #2381
Merged
Update handler #2381
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Refactors inbox handling to schedule and cache incoming activities before processing, reducing duplicate processing and improving efficiency. Introduces a new process_inbox method to collect user IDs and process activities in batches, and replaces maybe_handle_inbox_request with schedule_inbox_request to support the new workflow.
Introduces a deduplicate method in Inbox_Collection to merge recipients and remove duplicate inbox posts with the same GUID, addressing race conditions. Updates handler logic to use the new deduplication and recipient management, replacing transient-based storage with post-based deduplication and recipient aggregation.
Refactors the scheduled action and handler method from process_inbox to process_inbox_request for clarity and consistency. Updates the action hook and method name accordingly.
Co-authored-by: Copilot <[email protected]>
Changed the scheduled delay for processing inbox activities from 5 to 30 seconds in class-inbox.php. Added comprehensive tests for recipient addition and deduplication logic in class-test-inbox.php, including handling of duplicate and non-existent GUIDs. Updated handler test to reflect changes in scheduling and context filtering.
Replaces the 'activitypub_process_inbox' hook with 'activitypub_process_inbox_request' and renames the handler to 'handle_inbox_request'. Updates logic to use Inbox_Collection for recipient management and reduces the scheduled delay. Test coverage is improved to verify scheduling and hook firing behavior for different inbox contexts.
Co-authored-by: Copilot <[email protected]>
Removed conditional check for existing inbox items and always add activity to the inbox collection. This streamlines the process and ensures all activities are handled consistently.
Co-authored-by: Konstantin Obenland <[email protected]>
Co-authored-by: Konstantin Obenland <[email protected]>
Replaces direct access of the 'id' field with the object_to_uri() function for determining the activity ID in the inbox handler. Also adds the necessary import for object_to_uri. Minor whitespace cleanup in the collection class.
Deleted tests related to immediate and scheduled processing of inbox requests without and with activity IDs. These tests were likely redundant or no longer relevant to the current implementation.
Introduces the 'activitypub_skip_inbox_storage' filter to optionally skip inbox storage for specific activities, such as for debugging or reducing load for certain activity types like 'Delete'.
Removed the legacy Inbox handler and its tests, migrating inbox processing logic into the REST controllers. The REST controllers now handle inbox activity scheduling, deduplication, and action hooks directly. This change streamlines the codebase and consolidates inbox logic within the REST layer.
Refactor Posts collection and related handlers to accept and manage multiple recipients (user IDs) for each post. Add methods to add, remove, and query recipients, and update all relevant handlers and tests to use arrays of user IDs instead of single values. Includes comprehensive unit tests for multi-recipient scenarios and ensures backward compatibility with single recipient usage.
Refactored activity type handling to use snake_case in Actors_Inbox_Controller. Enhanced test coverage for deduplication by simulating race conditions with manual inbox post creation and improved inbox cleanup in tests. Updated assertions to use get_by_guid for inbox item retrieval and adjusted test logic for error handling.
pfefferle
changed the base branch from
trunk
to
deduplicate-single-inbox-requests
Adjusted the @Covers annotations in the test class to use fully qualified class names and updated method references to reflect recent changes in the Actors_Inbox_Controller class. This improves test coverage accuracy and aligns with namespace conventions.
Modified PHPUnit tests to assert that user ID parameters passed to handler hooks are arrays and contain the expected user ID, instead of being single values. Also updated the update handler test to use the new 'activitypub_handled_inbox_update' hook and ensured proper initialization and cleanup.
There was a problem hiding this comment.
Pull Request Overview
This PR refactors ActivityPub handlers to support multiple recipients for activities. The core change is updating handler methods to accept arrays of user IDs instead of single user IDs, enabling posts and interactions to be associated with multiple local users.
Key changes:
- Modified all activity handlers to accept
$user_idsarrays instead of single$user_idparameters - Updated the
Postscollection class to manage multiple recipients per post using separate post meta entries - Changed hook signatures from
activitypub_inbox_*toactivitypub_handled_inbox_*for Update and Create handlers
Reviewed Changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| includes/collection/class-posts.php | Added recipient management methods and updated add/update to handle multiple recipients |
| includes/handler/class-update.php | Changed to accept user ID arrays and updated hook name from activitypub_inbox_update to activitypub_handled_inbox_update |
| includes/handler/class-create.php | Updated to accept user ID arrays and changed hook name to activitypub_handled_inbox_create |
| includes/handler/class-follow.php | Modified to accept user ID arrays with backward compatibility handling |
| includes/handler/class-like.php | Updated hook to pass user IDs as array |
| includes/handler/class-undo.php | Updated hook to cast user ID to array |
| includes/handler/class-delete.php | Updated hooks to cast user ID to array |
| includes/handler/class-accept.php | Updated hook to cast user ID to array |
| includes/handler/class-announce.php | Updated hook to cast user ID to array |
| includes/handler/class-reject.php | Updated hook to cast user ID to array |
| includes/handler/class-move.php | Updated hook to cast user ID to array |
| includes/handler/class-quote-request.php | Updated hook to cast user ID to array |
| includes/handler/class-collection-sync.php | Updated hook to cast user ID to array |
| tests/phpunit/tests/includes/collection/class-test-posts.php | Added comprehensive tests for multiple recipient functionality |
| tests/phpunit/tests/includes/handler/class-test-update.php | Updated test to use new hook name and verify array handling |
| tests/phpunit/tests/includes/handler/class-test-follow.php | Updated assertions to verify user IDs as arrays |
| tests/phpunit/tests/includes/handler/class-test-like.php | Updated assertions to verify user IDs as arrays |
| tests/phpunit/tests/includes/handler/class-test-undo.php | Updated assertions to verify user IDs as arrays |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Removed recipient sanitization and empty check from the Posts::add method, streamlining the logic to directly check for existing posts by GUID and update if found. This simplifies the method and delegates recipient validation elsewhere.
Updated the condition to check for WP_Error instead of WP_Post when determining if a post exists before updating. This ensures that errors are handled correctly and updates are only performed on valid posts.
Changed the docblock for the $user_ids parameter from 'array' to 'int[]' to clarify that it is an array of integers.
Refactored the Posts::add and Posts::update methods to cast recipients to an array without additional sanitization or validation. This streamlines recipient processing and removes redundant code.
Deleted tests that verify Posts::add and Posts::update return errors when called with empty recipients. This may reflect a change in requirements or error handling logic.
Deleted the obsolete test_update_with_multiple_recipients() method and corrected a variable name in the attachment test to use the correct post ID. This streamlines the test suite and ensures attachment checks are accurate.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated 12 comments.
Comments suppressed due to low confidence (1)
tests/phpunit/tests/includes/collection/class-test-posts.php:1
- This test verification code appears to be in the wrong test method. It's located inside test_add_existing_post_adds_recipients() but seems to be validating attachment behavior unrelated to the recipient functionality being tested. This code likely belongs in the test_update_with_new_attachments() method or should be removed from this test.
<?php
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
obenland
requested changes
Oct 29, 2025
Co-authored-by: Copilot <[email protected]>
Updated handler classes to consistently treat $user_ids as an array when invoking actions and documentation. In class-reject.php, ensured only a single user ID is passed to Following::reject. This improves type consistency and prevents potential issues when $user_ids is provided as a single value or an array.
Changed the 'id' field in the update_activity array from 'update-test' to 'existing-post' to better reflect the test scenario in class-test-posts.php.
obenland
approved these changes
Oct 29, 2025
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * @param array $activity The activity object data. | ||
| * @param int $user_id The local user ID. | ||
| * @param array $activity The activity object data. | ||
| * @param int|int[] $recipients The id(s) of the local blog-user(s). |
There was a problem hiding this comment.
The PHPDoc type annotation is inconsistent with the actual parameter name. The parameter is named $recipients in the function signature (line 125) but the documentation refers to it generically. While the type is correct, the parameter name in the docblock should match the actual parameter name for clarity.
| * | ||
| * @param array $activity The activity-object. | ||
| * @param int $user_id The id of the local blog-user. | ||
| * @param int[] $user_ids The id of the local blog-user. |
There was a problem hiding this comment.
PHPDoc description is grammatically incorrect. The parameter is now an array of user IDs (int[]) but the description says 'The id' (singular) instead of 'The ids' (plural). It should read 'The ids of the local blog-users' to match the array type and be consistent with other similar documentation in the file.
Suggested change
| * @param int[] $user_ids The id of the local blog-user. | |
| * @param int[] $user_ids The ids of the local blog-users. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR refactors ActivityPub handlers to support multiple recipients for activities. The core change is updating handler methods to accept arrays of user IDs instead of single user IDs, enabling posts and interactions to be associated with multiple local users.
Proposed changes:
$user_idsarrays instead of single$user_idparametersPostscollection class to manage multiple recipients per post using separate post meta entriesactivitypub_inbox_*toactivitypub_handled_inbox_*for Update and Create handlersOther information:
Testing instructions:
Changelog entry
Changelog Entry Details
Significance
Type
Message
Refactored activity handling to support multiple recipients per activity, allowing posts and interactions to be linked to several local users at once.