Skip to:
Content

BuddyPress.org

Opened 7 years ago

Closed 5 years ago

#5570 closed defect (bug) (worksforme)

`bp_get_member_profile_data` showing wrong values cached for other users

Reported by: frederick.ding Owned by:
Milestone: Priority: normal
Severity: major Version: 2.0
Component: Extended Profile Keywords: needs-patch reporter-feedback
Cc: espellcaste@…, will@…

Description

After upgrading to BP 2.0: on a members directory listing, if bp_get_member_profile_data('field=Name of Custom Field') is called inside the loop, and that current user does not have any value for that profile field, BuddyPress erroneously shows the value for previous members from the loop.

Although I traced the call to BP_XProfile_ProfileData::get_all_for_user($user_id), and from there to BP_XProfile_Group::get(), I am not familiar enough with BuddyPress to investigate specifically where the problem may lie. It also seems that this XProfile-extended listing is a custom case (BP does not do this by default, although the core files suggest making extended listings this way as a comment in the legacy template folder).

Attachments (2)

20140421-034716.png (104.2 KB) - added by frederick.ding 7 years ago.
Screenshot showing repeated profile data from other users
5570.test.patch (1.5 KB) - added by boonebgorges 7 years ago.

Download all attachments as: .zip

Change History (21)

@frederick.ding
7 years ago

Screenshot showing repeated profile data from other users

#1 @frederick.ding
7 years ago

  • Keywords dev-feedback added

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.
    • 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 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()?

#2 @r-a-y
7 years ago

  • Milestone changed from Awaiting Review to 2.0.1

Thanks for the detailed report, frederick.ding.

Moving to 2.0.1 for a closer look.

#3 @boonebgorges
7 years ago

  • Keywords reporter-feedback added; dev-feedback removed

frederick.ding -

Thanks very much for the analysis.

I've been working on this, and I can't seem to reproduce. I've tried in the interface as well as a unit test. Please see the attached 5570-test.patch. Does the logic in that test look correct to you? Three users; u1 and u3 have a value set for the field in question; then call bp_get_member_profile_data() inside of the bp_has_members() loop. If I understand your report correctly, the results should look like:

[u1] => 'foo'
[u2] => 'foo' // incorrect
[u3] => 'bar'

But what I'm seeing in my tests is the correct

[u1] => 'foo'
[u2] => false
[u3] => 'bar'

On top of this, I've ready through your analysis alongside the code and I'm afraid I don't completely follow the reasoning. I'm with you until this point:

When $fields is merged back into $groups 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.

You're correct that $groups[ $index ]->fields is assumed to be empty. But I don't see how $fields can contain another user's data. Yes, a SQL query has been loaded at this point in the codebase during this *page load*; but the information is stored in a variable whose scope is the function, and the function gets called different times for each user. Ie:

u1 -> bp_get_member_profile_data() -> BP_XProfile_ProfileData::get_data_for_user() -> BP_XProfile_Group::get()

u2 -> bp_get_member_profile_data() -> BP_XProfile_ProfileData::get_data_for_user() -> BP_XProfile_Group::get()

u3 -> bp_get_member_profile_data() -> BP_XProfile_ProfileData::get_data_for_user() -> BP_XProfile_Group::get()

The only way that the data could bleed through from one user to another is if there were a bug in the cache or if it were being stored in some polluted global - *not* by the local variable being polluted from user to user within the loop. I've looked, but I can't find any evidence that either of these things were happening. (See my comment above about not being able to reproduce the behavior you've described.)

That said, it's possible that I'm misunderstanding here. Maybe I'm not grasping the crux of the issue? Maybe I've haven't tried the right things to reproduce the problem?

#4 @espellcaste
7 years ago

  • Cc espellcaste@… added

This ticket was mentioned in IRC in #buddypress-dev by paulgibbs. View the logs.


7 years ago

#6 @DJPaul
7 years ago

  • Milestone changed from 2.0.1 to Awaiting Review

Moving out of 2.0.1 pending further details from original reporter to let us/Boone investigate this further.

#7 @DJPaul
7 years ago

Hi frederick.ding; we need your help again to investigate this further. Can you take a read of Boone's last comment here and let us know if you have any further insights? Thank you.

#8 @DJPaul
7 years ago

  • Keywords reporter-feedback removed
  • Milestone Awaiting Review deleted

Closing as we've been unable to recreate and need further help from the original reporter, who we've not heard from in 3 months.

#9 @will_c
7 years ago

  • Cc will@… added
  • Keywords needs-patch added

Just wanted to chime in here. I realize that Frederick hasn't provided more information about this yet, but I am fairly certain that I'm facing the exact same issue. I am using BuddyPress 2.1.1, WordPress 4.0, and Memcached for caching.

As Fredrick suggested, I have also potentially traced this back to a caching issue. We have a custom members loop and display xprofile data for each user. We have the correct data the first time loading the page after clearing the cache, but after the first load (ie when using data from the cache), anyone that has an empty field inherits the data from the previous person to have filled in that field.

To replicate, you should be able to have 3 users and create a few basic profile fields. If you fill in profile fields for one user and not the others, when displaying xprofile field components in the members loop you should see the duplicated values for the users that have blank entries for those fields.

If I comment out line 326-336 (essentially removing the cache) everything works as expected.

I'm working on finding a better solution, but wanted to post here in case anyone else has faced this issue or has an existing workaround. I will update this post when I have more information.

#10 @will_c
7 years ago

Quick update. I am still unclear as to the root cause of this bug, but I was able to find a workaround. I'm wondering if the issue is related to this bug as well, but not sure.

It looks like the function bp_get_member_profile_data(field='Name or ID of field') is the cause of the problem. When using this function I am receiving duplicate information for users that don't have any field data filled in (only after the first load when retrieving data from the cache). I switched to using bp_get_profile_field_data() (example) and everything is working as expected. I have not taken a good look at the difference between these two functions, but hope to have some time this weekend to do so.

#11 @boonebgorges
7 years ago

will_c - Thanks for the updates. Unfortunately, I still can't seem to duplicate. See 5570.test.patch.

bp_get_member_profile_data() seems to be working as expected - both of the print_r() statements output 'foo' for the second user, but not the first or third.

If I comment out line 326-336 (essentially removing the cache) everything works as expected.

This is quite perplexing. The code you're referencing here doesn't actually cache any user data - it's only information from the bp_xprofile_groups table.

Can you please share the exact code that you're using for this purpose?

#12 @johnjamesjacoby
7 years ago

  • Keywords reporter-feedback added
  • Milestone set to 2.3

#13 follow-up: @jlooooo
6 years ago

Hi,

Just would like to mention I have this issue too. I use

$profile_bio = bp_get_member_profile_data('field=Bio'); 
if ($profile_bio) {
    echo profile_bio;
}

to output member profile data in the members loop and when a member does not have profile data it get's the value of the former member.

It seems to be related to Object Caching. I use xCache together with W3TC on production site. On localhost I don't use xCache and everything is fine. When I disable object cache in W3TC it's fixed too on production.

Is there a way to clear the Object Cache?

Using WP 4.1.1 and BP 2.2.1

#14 in reply to: ↑ 13 @boonebgorges
6 years ago

  • Milestone changed from 2.3 to Under Consideration

jlooooo - I'm afraid I'm still unable to reproduce. Even with persistent caching enabled, using code exactly like what you've provided within the context of the member loop, users without profile data for the 'Bio' field have $profile_bio == false.

There are ways to clear the cache, but that doesn't fix the underlying bug.

Can you please provide more of the code that you're using? Is this a custom template, or a plugin, or something else? The more details you can provide, the more likely I'll be able to reproduce.

Replying to jlooooo:

Hi,

Just would like to mention I have this issue too. I use

$profile_bio = bp_get_member_profile_data('field=Bio'); 
if ($profile_bio) {
    echo profile_bio;
}

to output member profile data in the members loop and when a member does not have profile data it get's the value of the former member.

It seems to be related to Object Caching. I use xCache together with W3TC on production site. On localhost I don't use xCache and everything is fine. When I disable object cache in W3TC it's fixed too on production.

Is there a way to clear the Object Cache?

Using WP 4.1.1 and BP 2.2.1

#15 @jlooooo
6 years ago

Hi,

It's a theme. I copied members-loop.php from wp-content/plugins/buddypress/bp-templates/bp-legacy/buddypress/members/members-loop.php to wp-content/themes/my-theme/buddypress/members/members-loop.php.

Under

/***
* If you want to show specific profile fields here you can,
* but it'll add an extra query for each member in the loop
* (only one regardless of the number of fields you show):
*
* bp_member_profile_data( 'field=the field name' );
*/

I added the code mentioned earlier.

I just added wp_cache_flush(); after

<?php while ( bp_members() ) : bp_the_member(); ?>

in members-loop.php in my-theme on production and now the correct values are shown.

Not sure how I can debug this.


#16 @boonebgorges
6 years ago

It's probably a bad idea to add wp_cache_flush() there - it means that the entire site cache will be flushed every time someone views the member directory, which could cause significant performance issues.

Not sure how I can debug this.

Well, the way I'd debug it is to start digging in to see where the cache pollution is taking place. Look first at bp_member_profile_data() and then bp_get_member_profile_data(). Presumably, it's calling BP_XProfile_ProfileData::get_all_for_user() to fill in the profile date. Is tha data that's being returned here correct? If not, go into that function and check to see if the values coming from bp_xprofile_get_groups() are correct. Etc. I'm afraid that, without being able to reproduce (and I've spent the last hour trying to reproduce!!), I can't be much more help than this.

#17 @jlooooo
6 years ago

Yes indeed not a good solution at all (wp_cache_flush()).

Tried to debug it further. BP_XProfile_ProfileData::get_all_for_user() is not returning the correct data. Didn't go into bp_xprofile_get_groups() yet.

Another function does return the correct data: $bio = xprofile_get_field_data('bio', $user_id);
Not sure why, but at least I don't have to flush the cache site wide anymore.

#18 @DJPaul
5 years ago

  • Milestone changed from Under Consideration to Awaiting Review

#19 @DJPaul
5 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to worksforme
  • Status changed from new to closed

We haven't been able to reproduce this problem in 2 years, and there's been no new comments in 15 months. It wouldn't surprise us to learn there could be cache issues, but we need more reports and hopefully more debug from someone who can recreate the problem before we can take things further.

Note: See TracTickets for help on using tickets.