Skip to:
Content

BuddyPress.org

Opened 5 years ago

Closed 5 years ago

#5815 closed defect (bug) (fixed)

bp_blogs_total_blogs_for_user() queries database on each page view in multisite

Reported by: wpdennis Owned by: boonebgorges
Milestone: 2.1 Priority: normal
Severity: normal Version: 1.2
Component: Blogs Keywords: has-patch commit
Cc:

Description

bp_blogs_total_blogs_for_user() gets triggered by BP_Blogs_Component->setup_nav() in multisite environments.

If the visitor isn't logged in, this is resulting in a database query on each page view. The caching fails, since the result for logged out users is always 0:

if ( !$count = wp_cache_get( 'bp_total_blogs_for_user_' . $user_id, 'bp' ) ) {

Maybe bp_blogs_total_blogs_for_user() could return 0 if the current user isn't logged in:

if (!is_user_logged_in())
    return 0;

And/or the caching condition should be:

if ( false === $count = wp_cache_get( 'bp_total_blogs_for_user_' . $user_id, 'bp' ) ) {

Attachments (2)

5815.01.patch (990 bytes) - added by r-a-y 5 years ago.
5815.02.patch (1.2 KB) - added by boonebgorges 5 years ago.

Download all attachments as: .zip

Change History (7)

#1 @wpdennis
5 years ago

Something like this seems to work:

function bp_blogs_total_blogs_for_user( $user_id = 0 ) {

	if ( empty( $user_id ) )
		$user_id = ( bp_displayed_user_id() ) ? bp_displayed_user_id() : bp_loggedin_user_id();

	if ( absint($user_id) === 0 )
		return 0;

	if (false === $count = wp_cache_get( 'bp_total_blogs_for_user_' . $user_id, 'bp' ) ) {
		$count = BP_Blogs_Blog::total_blog_count_for_user( $user_id );
		wp_cache_set( 'bp_total_blogs_for_user_' . $user_id, $count, 'bp' );
	}

	return $count;
}

@r-a-y
5 years ago

#2 @r-a-y
5 years ago

  • Keywords has-patch added; dev-feedback removed
  • Milestone changed from Awaiting Review to 2.1
  • Version set to 1.2

wpdennis: Always love your tickets relating performance!

Confirmed. Going to try and sneak this in to 2.1.

#3 @boonebgorges
5 years ago

We also need to add the strict type check, in case a legitimately logged-in user has 0 blogs - we don't want that to result in a cache miss.

#4 @r-a-y
5 years ago

  • Keywords commit added

Good catch! I think there are some older functions that suffer from not using the strict type check as well during cache checks.

#5 @boonebgorges
5 years ago

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

In 8935:

Improvements to bp_blogs_total_blogs_for_user()

  • Don't bother to check the cache or perform DB queries for users who are not logged in
  • Do a strict type check when checking the cache, to ensure that a cached value of 0 does not result in an unnecessary database hit

Fixes #5815

Props wpdennis, r-a-y

Note: See TracTickets for help on using tickets.