is_user_member_of_blog() can be very slow when a user is a member of hundreds of sites

[sammybeats]

This problem would partly be fixed if the solution in #31746 were adopted, however it is still a separate issue.

is_user_member_of_blog() uses get_blogs_of_user() which is very slow when the user is a member of hundreds of sites. And because is_user_member_of_blog() is called on every admin page load by wp_user_settings() in admin-header.php, this adds a TON of database queries to every admin page load.

get_blog_details(), which is called on every blog of which the user is a member in get_blogs_of_user(), only needs to be done on the blog in question when is_user_member_of_blog() is called. I propose a solution like this, though it may duplicate too much code from get_blogs_of_user():

function is_user_member_of_blog( $user_id = 0, $blog_id = 0 ) {
    global $wpdb;

    $user_id = (int) $user_id;
    $blog_id = (int) $blog_id;

    if ( empty( $user_id ) )
	$user_id = get_current_user_id();

    if ( empty( $blog_id ) )
	$blog_id = get_current_blog_id();

    if ( empty( $user_id ) )
        return false;

    if( ! is_multisite() )
        return true;

    $keys = get_user_meta( $user_id );
    if ( empty( $keys ) )
        return false;

    $blog = get_blog_details( $blog_id );

    if( ! $blog || ! isset( $blog->domain ) || $blog->archived || $blog->spam || $blog->deleted )
        return false;

    if ( isset( $keys[ $wpdb->base_prefix . 'capabilities' ] ) && defined( 'MULTISITE' ) && $blog_id == 1 )
        return true;

    $is_member = false;

    $keys = array_keys( $keys );

    foreach ( $keys as $key ) {
        if ( 'capabilities' !== substr( $key, -12 ) )
            continue;
        if ( $wpdb->base_prefix && 0 !== strpos( $key, $wpdb->base_prefix ) )
            continue;
        $key_blog_id = str_replace( array( $wpdb->base_prefix, '_capabilities' ), '', $key );
        if ( ! is_numeric( $key_blog_id ) )
            continue;
        if ( $key_blog_id == $blog_id ) {
            $is_member = true;
            break;
        }
    }

    return $is_member;

}

I just tested it and it reduced my admin page load time by 5 seconds.

[BinaryKitten]

Replying to sammybeats:
Hi Sammybeats,

I like this conceptually, but possibly there is further scope for improvement

function is_user_member_of_blog( $user_id = 0, $blog_id = 0 ) {
    global $wpdb;

    if( ! is_multisite() )
        return true;

    $user_id = (int) $user_id;
    $blog_id = (int) $blog_id;

    if ( empty( $user_id ) )
        $user_id = get_current_user_id();

    if ( empty( $blog_id ) )
        $blog_id = get_current_blog_id();

    $blog = get_blog_details( $blog_id );

    if( ! $blog || ! isset( $blog->domain ) || $blog->archived || $blog->spam || $blog->deleted )
        return false;

    $keys = get_user_meta( $user_id );
    if ( empty( $keys ) )
        return false;

    $base_capabilities_key = $wpdb->base_prefix . '_capabilities';
    $site_capabilities_key = $wpdb->base_prefix . $blog_id . '_capabilities';

    if ( isset( $keys[ $base_capabilities_key ] ) && defined( 'MULTISITE' ) && $blog_id == 1 )
        return true;

    if ( isset( $keys[ $site_capabilities_key ] ) )
        return true;
 
}

Where there are many sites and many plugins that have added to the usermeta this prevents the need for the iteration.

[sammybeats]

Replying to BinaryKitten:

Yes! Definitely an improvement on what I posted, with one (questionable) change and one correction:

function is_user_member_of_blog( $user_id = 0, $blog_id = 0 ) {
    global $wpdb;

    if( ! is_multisite() )
        return true;

    $user_id = (int) $user_id;
    $blog_id = (int) $blog_id;

    if ( empty( $user_id ) )
        $user_id = get_current_user_id();

    //Technically not needed, but does save calls to get_blog_details and get_user_meta
    //in the event that the function is called when a user isn't logged in
    if ( empty( $user_id ) )
        return false;

    if ( empty( $blog_id ) )
        $blog_id = get_current_blog_id();

    $blog = get_blog_details( $blog_id );

    if( ! $blog || ! isset( $blog->domain ) || $blog->archived || $blog->spam || $blog->deleted )
        return false;

    $keys = get_user_meta( $user_id );
    if ( empty( $keys ) )
        return false;

    //no underscore before capabilities in $base_capabilities_key
    $base_capabilities_key = $wpdb->base_prefix . 'capabilities';
    $site_capabilities_key = $wpdb->base_prefix . $blog_id . '_capabilities';

    if ( isset( $keys[ $base_capabilities_key ] ) && defined( 'MULTISITE' ) && $blog_id == 1 )
        return true;

    if ( isset( $keys[ $site_capabilities_key ] ) )
        return true;

    return false;

}

[BinaryKitten]

Replying to sammybeats:

Yes! Definitely an improvement on what I posted, with one (questionable) change and one correction:

I can live with that. Those errors teach me not to try and code in a browser text box.

I've taken the code and created a patch/diff (see attached)

[BinaryKitten]

Patch for is_user_member_of_blog

[sammybeats]

Replying to BinaryKitten:

I've taken the code and created a patch/diff (see attached)

Thanks!

[BinaryKitten]

Patch i think is broken due to where i made it from. Attempting 2nd attempt from better location

[BinaryKitten]

Corrected Patch File

[BinaryKitten]

V3 diff - better base point

[rmccue]

@BinaryKitten: Thanks for the patch. :) Please ensure you have your editor set to use tabs rather than spaces, as per the WordPress coding standards.

[BinaryKitten]

v4 - with coding standards added to phpStorm

[BinaryKitten]

@rmccue - thanks for the reminder. I'm usually set to PSR-2 coding standards, forgot that core WP coding style is different.

updated the patch with the standards - hopefully this is working right.

[johnbillion]

This looks like a sane way to handle this. +1

It'll need tests, including ones to make sure that it continues to function as expected when a user is added to then removed from a blog, has their role removed, etc.

[BinaryKitten]

Replying to johnbillion:

This looks like a sane way to handle this. +1

It'll need tests, including ones to make sure that it continues to function as expected when a user is added to then removed from a blog, has their role removed, etc.

Hi @johnbillion

Would there be some guidance for this process?

cheers

[johnbillion]

Sorry I missed your comment @BinaryKitten.

The core handbook contains ​information on unit testing. The section on writing new tests is bare, but there are already two tests for is_user_member_of_blog(). The single site test is here and the multisite test is here.

With is_user_member_of_blog_(v4).diff​ applied, the single site test fails with the following error:

1) Tests_User::test_is_user_member_of_blog
Failed asserting that true is false.
~/tests/phpunit/tests/user.php:365

This means that there may be a caching issue when users are deleted, or your patch fails to account for deleted users. Take a look and see where you get to.

[johnbillion]

Chatted with binarykitten about this last week. Awaiting an updated patch.

[BinaryKitten]

Updated Patch - Based from Develop.git.wordpress.org repo, updated unit tests and code to match.

[BinaryKitten]

Added New patch file - /cc @johnbillion

[johnbillion]

In 33771:

Improve the efficiency of is_user_member_of_blog() by removing its use of get_blogs_of_user(). Adds additional tests.

Fixes #32472
Props BinaryKitten, sammybeats, johnbillion