Improve selective fields performance - #5024
Conversation
There was a problem hiding this comment.
Thanks for your pull request. It looks like this may be your first contribution to the BuddyBoss Platform open source project. Please note that this project and all contributions to it are public and bounded by the GPL v2.0 license, and that a record of the contribution (including all personal information you submit with it, including your full name and email address) is maintained indefinitely and may be redistributed with this project. If you are not okay with these terms, please close this pull request. Alternatively, you can let us know about your concerns by adding a comment to this pull request.
There was a problem hiding this comment.
Pull request overview
This PR updates the Activity REST controllers to become _fields-aware earlier in response preparation, aiming to avoid doing expensive per-item work when clients request only a small subset of fields. It also adds a dedicated PHPUnit test suite to lock in both response-shape correctness and measurable query-count savings.
Changes:
- Make
BP_REST_Activity_Endpoint::prepare_item_for_response()conditionally build a number of expensive fields based on_fields, and ensure nested activity objects in “envelope” responses are prepared without the outer request’s_fields. - Strip
_fieldswhen preparing nested activity comments (both from the activity controller and the comment controller) so comment payloads aren’t inadvertently narrowed by an envelope-level selection. - Cache the activity item schema on the controller instance (built once per request) and add new tests covering field trimming, embed behavior, nested fields, and query-count reductions.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| tests/phpunit/testcases/activity/rest-fields.php | Adds comprehensive tests to validate _fields payload trimming and verify expensive work (queries/hooks) is skipped when fields aren’t selected. |
| src/bp-activity/classes/class-bp-rest-activity-endpoint.php | Implements field-aware response building, strips _fields for nested activities, and caches schema per request. |
| src/bp-activity/classes/class-bp-rest-activity-comment-endpoint.php | Ensures envelope-level _fields does not narrow nested comment objects built via the activity endpoint. |
Suppressed comments (4)
tests/phpunit/testcases/activity/rest-fields.php:378
- These tests add a filter and remove it afterwards, but if the request/errors/assertions abort early the filter may remain attached and affect later tests. Using
try/finallyensures cleanup runs regardless of test outcome.
add_filter( 'bp_get_activity_content_body', $counter );
$this->get_first_item( array( '_fields' => 'id,user_id' ) );
remove_filter( 'bp_get_activity_content_body', $counter );
tests/phpunit/testcases/activity/rest-fields.php:396
- Same cleanup issue as above: if the request triggers an error before
remove_filter()is reached, the filter can leak into other tests. Atry/finallyblock makes the test suite more robust.
add_filter( 'bp_get_activity_content_body', $counter );
$this->get_first_item( array( '_fields' => 'id,content' ) );
remove_filter( 'bp_get_activity_content_body', $counter );
tests/phpunit/testcases/activity/rest-fields.php:414
- Like the other filter-based tests, this can leak the
bp_activity_get_edit_datafilter into later tests if an error occurs before cleanup. Wrapping the request intry/finallyguarantees the filter is always removed.
add_filter( 'bp_activity_get_edit_data', $counter );
$this->get_first_item( array( '_fields' => 'id,user_id' ) );
remove_filter( 'bp_activity_get_edit_data', $counter );
src/bp-activity/classes/class-bp-rest-activity-endpoint.php:2261
bb_pro_activity_post_feature_image_instance()->bb_get_feature_image_data()is called for every prepared activity even when a request is using a narrow_fieldsselection. Since this key is not part of the schema-driven$fieldslist, the controller can’t currently skip the work, so narrow selections may still pay for this potentially expensive lookup only to have WordPress trim it away later.
$data['bb_activity_post_feature_image'] = array();
if ( ! empty( $activity->id ) ) {
if ( function_exists( 'bb_pro_activity_post_feature_image_instance' ) ) {
$feature_image_data = bb_pro_activity_post_feature_image_instance()->bb_get_feature_image_data( $activity->id );
if ( ! empty( $feature_image_data ) ) {
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
|
||
| $data['is_comment_closed'] = function_exists( 'bb_is_close_activity_comments_enabled' ) && bb_is_close_activity_comments_enabled() ? bb_is_activity_comments_closed( $activity->id ) : false; | ||
| $data['activity_status'] = $activity->status; |
| $include_content = rest_is_field_included( 'content', $fields ); | ||
| $include_embed_data = rest_is_field_included( 'preview_data', $fields ) || rest_is_field_included( 'link_embed_url', $fields ); | ||
|
|
| /** | ||
| * Filters the activity schema. | ||
| * | ||
| * @param string $schema The endpoint schema. | ||
| */ | ||
| return apply_filters( 'bp_rest_activity_schema', $this->add_additional_fields_schema( $this->schema ) ); |
General Note
Current build-in selective fields engine for set of our endpoints is doing full sql queries (performance) and then filtering response to return just requested fields. We want to have faster sql query + smaller payload, thats why this branch is needed.
Notes to Developer
Notes to Reviewer