User deletion doesn't clear user_meta cache

[duck_]

wp_delete_user() and wpmu_delete_user() leave user_meta in the cache. Example:

get_user_meta( $user_id );
wp_delete_user( $user_id );
var_dump( is_user_member_of_blog( $user_id ) ); // bool(true) instead of bool(false)

Possible solutions:

1. Extend clean_user_cache() to delete from user_meta too
2. delete_user_meta() in wp_delete_user() instead of direct query

Ryan wasn't keen on 1 as "Most of the time clean_user_cache() doesn't need to touch meta cache".

2. will introduce the delete(d)_user_meta actions when using wp_delete_user().

wp_cache_delete( $id, 'user_meta' ) in wp_delete_user() was also floated as a stop-gap measure, but since this will be fixed in a future release we can ignore the suggestion.

See: ticket:19160:6, #11761

With the help of duck_, 19500.diff​ passes the skipped unit tests. It required a change to get_blogs_of_user(), as in single-site, you end up with false, while in multisite, you'll get array() (in the case of no blogs). I think that array() might be the more semantic response, however.

Looking through core and a few plugins, most places use empty() or is_array() or (array) when dealing with the return value so they are safe with either array or false. One place does this though:

array_key_exists($blog_id, get_blogs_of_user($user_id)) ) 

Perhaps returning an empty array for both single and multi would be safest, as well as more semantic.

19500.2.diff moves clean_user_cache( $id ) ahead of the queries that delete the user in wpmu_delete_user(). Without this, calls to wpmu_delete_user() do not successfully clear the user cache because the user no longer exists. This causes...

$user = WP_User::get_data_by( 'id', $id );

...to fail in clear_user_cache(), and the $user variable caches are never cleared.

Replying to johnjamesjacoby:

19500.2.diff moves clean_user_cache( $id ) ahead of the queries that delete the user in wpmu_delete_user(). Without this, calls to wpmu_delete_user() do not successfully clear the user cache because the user no longer exists. This causes...

$user = WP_User::get_data_by( 'id', $id );

...to fail in clear_user_cache(), and the $user variable caches are never cleared.

#20460.

Also Ryan on #19690: "We usually clean [cache] after the delete" because of possibility of actions firing in between. Doesn't look like an issue in this case, but something to think about.

Perhaps we start passing objects instead of ids so we can clean the cache after delete.

In IRC we decided to start passing objects. Back-compat support for IDs should be retained. The situation where an ID is passed and the object cannot be fetched should be handled without warnings.

In [20522]:

Pass full user objects to clean_user_cache(). See #19500, fixes #20460.

Prevents notices when clean_user_cache() is called for a user that has been removed from the database.

All TestWPUser pass for single site. test_is_user_member_of_blog() fails for multisite. It fails with and without the patch.

Return array() for all the cases.

Replying to ryan:

All TestWPUser pass for single site. test_is_user_member_of_blog() fails for multisite. It fails with and without the patch.

There was a typo in the patch. remove_user_from_blog() was using the undefined variable $user_id and not $id. Updated patch also removes check for is_array() in is_user_member_of_blog() and the falsey check in get_blogs_of_user() as $blogs is initialised as an empty array.

Patches still need to deal with wpmu_delete_user(). Should be able to copy the delete_metadata_by_mid() loop from wp_delete_user().

meta delete loop for wpmu_delete_user()

In [20581]:

• Return empty arrays instead of false for all conditions in get_blogs_of_user().
• When deleting a user, use a delete_metadata_by_mid() loop over the meta so that the meta cache is cleared.
• Use remove_user_from_blog() for DRYness.

Props nacin, duck_
Fixes #19500

​http://unit-tests.trac.wordpress.org/changeset/696