Skip to:
Content

BuddyPress.org

Opened 6 years ago

Closed 6 years ago

#5445 closed enhancement (fixed)

Improve consistency of bp_core_get_user_* use

Reported by: boonebgorges Owned by:
Milestone: 2.0 Priority: normal
Severity: normal Version:
Component: Core Keywords:
Cc:

Description

There are a number of generic user-related functions that are used frequently throughout BuddyPress: bp_core_get_user_displayname(), bp_core_get_user_domain(), bp_core_get_core_userdata(), etc. These frequently used functions define what I think of as the "canonical" logic for determining the values in question. For example, _displayname() checks and sets the object cache as necessary; it prefers xprofile data if it can be found, followed by display_name and then user_nicename; it creates the relevant xprofile fielddata if none is found for that user. And so on.

However, there are many places in BP where we don't use these functions, but instead query directly for the data in question. This creates inconsistencies in the values displayed, and also makes it very difficult to ensure proper caching.

Let's use this ticket for a few things:

  • Generalizing the functions in question as necessary. In particular, it's critical to have the logic abstracted so that we can fetch more than one _displaname, _domain, etc with a single database call, while still respecting the canonical logic. The simplest way to do this is by creating new, plural functions (like bp_core_get_user_displaynames()), move most of the business logic there, and then turn the singular function into a wrapper. I've already done this locally for _displayname(), and it's working well.
  • Find places in BP where we are bypassing the canonical functions, and refactor to use them. For example, the xprofile component filters bp_user_query_populate_extras to override the display name. But this does not hit the proper caches, nor does it follow the precedence logic of the core displayname function. So it should be migrated over. And so on.

Change History (3)

#1 @boonebgorges
6 years ago

In 8024:

Cache userdata after running WP_User_Query in BP_User_Query

WP_User_Query does not cache data that is stored in wp_users in a bulk fashion.
BP, on the other hand, does cache this data, as 'bp_core_userdata_' . $user_id.
This cached userdata is then referenced in many places throughout BP, including
the building of user_domains and userlinks. Thus, it can save up to 20 queries
per page (assuming per_page=20) during query loops.

A corresponding change has been made to the 'fields' parameter passed to
WP_User_Query from BP_User_Query. Because BP caches entire wp_users rows (see
BP_Core_User::get_core_userdata()), we must ensure that BP_User_Query is also
fetching entire rows. We opt not to use WP_User_Query's 'fields=all' parameter,
because it triggers the casting of WP_User objects, which in turn requires
expensive capability lookups.

See #5445

#2 @boonebgorges
6 years ago

In 8027:

Introduce bp_core_user_displaynames(), for quick, bulk displayname lookup

The technique BP uses for determining a user's displayname takes into
consideration whether the xprofile component is active, whether the user has a
WP display_name, and a number of other factors. Because the values are used so
often throughout BP, they are then cached in WP's object cache. However, there
are a number of places throughout BuddyPress where displaynames are fetched
in a way that is inconsistent with the "canonical" workflow (ie, by querying
the xprofile tables correctly). This causes inconsistent results, and also can
result in performance degradation when the persistent cache is skipped.
Moreover, the singular nature of bp_core_get_user_displayname() meant that
doing it the "right" way meant introducing large numbers of queries to a given
page load.

This changeset introduces bp_core_get_user_displaynames(), which contains all
the critical logic of bp_core_get_user_displayname() - including fallbacks for
various setups and support for the object cache - but allows for fetching large
numbers of displaynames without multiple queries. bp_core_get_user_displayname()
is now a wrapper for the more general function.

The function has been swapped in throughout BuddyPress as appropriate.

See #5445

#3 @boonebgorges
6 years ago

  • Resolution set to fixed
  • Status changed from new to closed

I think I've caught all the instances for 2.0 that I'd planned to do, so I'm closing this one.

Note: See TracTickets for help on using tickets.