#19500 closed defect (bug) (fixed)
User deletion doesn't clear user_meta cache
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 3.4 | Priority: | normal |
Severity: | normal | Version: | 3.2.1 |
Component: | Cache API | Keywords: | has-patch |
Focuses: | Cc: |
Description
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:
- Extend clean_user_cache() to delete from user_meta too
- 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".
- 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
Attachments (6)
Change History (20)
#3
@
13 years ago
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.
#5
follow-up:
↓ 6
@
13 years ago
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.
#6
in reply to:
↑ 5
@
13 years ago
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.
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.
#7
@
13 years ago
Perhaps we start passing objects instead of ids so we can clean the cache after delete.
#8
@
13 years ago
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.
#10
follow-up:
↓ 11
@
13 years ago
All TestWPUser pass for single site. test_is_user_member_of_blog() fails for multisite. It fails with and without the patch.
#11
in reply to:
↑ 10
@
13 years ago
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.
#12
@
13 years ago
Patches still need to deal with wpmu_delete_user(). Should be able to copy the delete_metadata_by_mid() loop from wp_delete_user().
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 getarray()
(in the case of no blogs). I think that array() might be the more semantic response, however.