[buddypress-trac] [BuddyPress Trac] #5570: `bp_get_member_profile_data` showing wrong values cached for other users

[buddypress-trac]

#5570: `bp_get_member_profile_data` showing wrong values cached for other users
----------------------------+------------------------------
 Reporter:  frederick.ding  |       Owner:
     Type:  defect (bug)    |      Status:  new
 Priority:  normal          |   Milestone:  Awaiting Review
Component:  XProfile        |     Version:  2.0
 Severity:  major           |  Resolution:
 Keywords:  dev-feedback    |
----------------------------+------------------------------
Changes (by frederick.ding):

 * keywords:   => dev-feedback


Comment:

 I did some more tracing to track down the cause, and I think I've found
 it. My observations:

  - [7808] changed the relevant function,
 `BP_XProfile_ProfileData::get_all_for_user()`, to use
 `BP_XProfile_Group::get()` **instead of the direct SQL query that had
 worked before**, for caching purposes.

  - Within `BP_XProfile_Group::get()`, the problem occurs when `$fields` is
 merged back into `$groups`.
   - [[https://buddypress.trac.wordpress.org/browser/trunk/bp-xprofile/bp-
 xprofile-classes.php?rev=8298#L217|If I'm reading the source correctly]],
 it loads user-specific data into `$fields` from
 `BP_XProfile_ProfileData::get_data_for_user()`, which behaves correctly.
 `var_dump` at that line shows only the expected data.
   - Throughout the function, `$fields` **has correct and expected
 values**.
   - Most of the code for `BP_XProfile_Group::get()` isn't user-specific.
 The SQL query that loads data into `$groups` is **__not__** user-specific,
 so that array contains data for ALL users. `var_dump` on `$groups` at any
 point in the function's execution shows full data, including data values
 for other users.
   - When `$fields` is merged back into `$groups`
 [[https://buddypress.trac.wordpress.org/browser/trunk/bp-xprofile/bp-
 xprofile-classes.php?rev=8298#L278|on lines 279-299]], the value of
 `$groups[$index]->fields` is assumed to be an empty array; in reality, the
 SQL query has already loaded data from other users. Thus, when a user-
 specific field is appended to `$groups[$index]->fields`, it ends up being
 __appended__ to a collection of other users' information.

  - Consequently, when `BP_XProfile_ProfileData::get_all_for_user()` is
 used, fields that are not set for that particular user adopt the most
 recently retrieved value for a previous user. Notice that nowhere in the
 code for that function does it check that a given field returned belongs
 to that user, since it assumes that `BP_XProfile_Group::get()` with a
 provided user ID will do the needed filtering.

 As I've mentioned before, I'm not super familiar with the architecture of
 BuddyPress, but if what I've written here is correct, that is the cause
 for the problems.

 One possible solution, which fixed it for me:

 {{{
                 // Merge the field array back in with the group array
                 foreach( (array) $groups as $group ) {

                         // Indexes may have been shifted after previous
 deletions, so we get a
                         // fresh one each time through the loop
                         $index = array_search( $group, $groups );

                         // 2 new lines added below
                         if ( !empty( $user_id ) )
                                 $groups[$index]->fields = array();
 }}}

 This may not be ideal, since it might be resetting a group's fields array
 after data was queried -- is it even necessary to execute a query to load
 values into `$groups` if user-specific data is being retrieved using
 `BP_XProfile_ProfileData::get_data_for_user()`?

--
Ticket URL: <https://buddypress.trac.wordpress.org/ticket/5570#comment:1>
BuddyPress Trac <http://buddypress.org/>
BuddyPress Trac