WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 4 years ago

#11254 closed defect (bug) (fixed)

update_usermeta and delete_usermeta do not update or destroy cache

Reported by: mark8barnes Owned by: ryan
Milestone: 3.3 Priority: normal
Severity: minor Version: 2.8.5
Component: Cache API Keywords: needs-testing has-patch
Focuses: Cc:

Description

User meta data is stored in the $current_user variable, which is created as GLOBAL by get_currentuserinfo. However, when update_usermeta and delete_usermeta are called, this variable remains unchanged.

Subsequent calls to get_currentuserinfo just return the existing global variable, and therefore return out of data meta information.

The solution is to check for the existence of the global variable $current_user within the update_usermeta and delete_usermeta functions and either update it or destroy it. Updating it might be more useful, and could be done without needing to recreate the whole variable.

The change could theoretically break plugins if they assumed the existence of the metadata in $current_userinfo, but I can't imagine why anyone would, and it would hardly be good practice if they did.

Attachments (3)

deprecated.diff (1.4 KB) - added by hawy.php 5 years ago.
deprecated2.diff (931 bytes) - added by hawy.php 5 years ago.
small_patch.diff (1.2 KB) - added by cronco 4 years ago.

Download all attachments as: .zip

Change History (21)

comment:1 @Denis-de-Bernardy6 years ago

  • Component changed from Database to Cache

comment:2 @nacin5 years ago

The *_usermeta functions are now deprecated. I imagine this applies to _user_meta as well though.

@hawy.php5 years ago

comment:3 @hawy.php5 years ago

  • Keywords needs-testing added

comment:4 @nacin5 years ago

Patch looks like a good approach. We only need the call to setup_userdata() as that will handle creating a new WP_User object if necessary.

I think I agree with not using set_current_user/wp_set_current_user here, as I don't think we should be triggering the set_current_user action there. Unless that's used as part of caching?

Calling setup_userdata() in update_user_meta() and delete_user_meta(), right after the calls to update_metadata() and delete_metadata(), should complete this for the non-deprecated functions.

comment:5 follow-up: @nacin5 years ago

It's been pointed out (thanks hawy) that setup_userdata() is incorrect. Proper approach would be $GLOBALS['current_user'] = new WP_User($id, $name);

We shouldn't need to call setup_userdata() at all, as that doesn't set up any meta in the back compat globals it maintains.

comment:6 in reply to: ↑ 5 @hawy.php5 years ago

Replying to nacin:

It's been pointed out (thanks hawy) that setup_userdata() is incorrect. Proper approach would be $GLOBALS['current_user'] = new WP_User($id, $name);

We shouldn't need to call setup_userdata() at all, as that doesn't set up any meta in the back compat globals it maintains.

we need setup_userdata() cause it fills the old global variable ( $userdata ) with user's metadata

so if we didn't call it
and assume we have meta_key = "somemeta" and it's value was "old_value"
and some one made

update_usermeta( 1, "somemeta","new_value");

then
var_dump($userdata);

we still get old_value

comment:7 @nacin5 years ago

Stand corrected again... $userdata is populated with meta keys by _fill_user().

comment:8 @hawy.php5 years ago

so the only edit here could be using super global $GLBALS as (nacin) wrote above instead of global $current_user;

@hawy.php5 years ago

comment:9 @nacin5 years ago

We need to do this to _user_meta too. Questioning whether we should side-step wp_set_current_user (the sole purpose of which would be to prevent the action from firing) or not...

comment:10 @nacin5 years ago

  • Status changed from new to assigned

Assigning to ryan.

comment:11 @ryan5 years ago

  • Milestone changed from 3.0 to 3.1

We'll probably want to fix this in clean_user_cache(). Since this is a long-standing bug, postponing to 3.1. Now that get_user_option() doesn't reference $current_user this is a lesser issue.

comment:12 @nacin5 years ago

  • Milestone changed from Awaiting Triage to 3.1

comment:14 @ryan5 years ago

  • Milestone changed from 3.1 to Future Release

comment:15 @cronco4 years ago

  • Cc cronco added

Wouldn't adding wp_cache_delete($id,'user_meta'); in clean_user_cache take care of this and also clean up the code for *_usermeta a bit?

Version 0, edited 4 years ago by cronco (next)

@cronco4 years ago

comment:16 @cronco4 years ago

  • Keywords has-patch added

comment:17 follow-up: @ryan4 years ago

  • Milestone changed from Future Release to 3.3

User and usermeta caching was fixed in 3.3.

comment:18 in reply to: ↑ 17 @duck_4 years ago

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

Replying to ryan:

User and usermeta caching was fixed in 3.3.

Marking as fixed.

Note: See TracTickets for help on using tickets.