Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use `updated_user_meta` Instead of `update_user_metadata` #623

Closed
patdumond opened this issue Nov 25, 2015 · 12 comments
Closed

Use `updated_user_meta` Instead of `update_user_metadata` #623

patdumond opened this issue Nov 25, 2015 · 12 comments
Assignees
Labels
Milestone

Comments

@patdumond
Copy link

@patdumond patdumond commented Nov 25, 2015

Reference: https://websharks.zencache.com/agent/tickets/9475

User experienced problems with logged-in user caching. The user did some troubleshooting and found that autoClearUserCacheFA2 is called on every page load when a User is logged in. Then narrowed it down that the filter update_user_metadata in Plugin.php is what is triggering autoClearUserCacheFA2 on every page load. So the user checked into the usermeta table in MySQL and verifited that it is NOT getting updated or changed on every page load.

User looked into the update_metadata(…) source and the filter update_user_metadata is called regardless if the database is hit or not. User then changed the filter in the Plugin.php from update_user_metadata to update_user_meta and the cache is still cleared on every page load for the logged in user. User then changed the filter in Plugin.php from update_user_meta to updated_user_meta and everything works perfectly. The cache for the logged in user is Not cleared on every page load and it is fast.

If there is no specific reason we used update_user_meta instead of updated_user_meta in ZenCache we should consider changing our code.

@jaswrks
Copy link

@jaswrks jaswrks commented Nov 28, 2015

@kristineds Would you like to take a shot at outlining this one all by yourself? I'm curious to see if you can search through the pro codebase and find our existing hook attached to update_user_metadata. Provide instructions for how to change this so that we are listening to updated_user_meta instead.

@raamdev raamdev added this to the Next Release (Pro) milestone Nov 28, 2015
@kristineds
Copy link

@kristineds kristineds commented Dec 1, 2015

@jaswsinc I've searched through the codebase and found these.


Next Actions

  • New feature branch in the websharks/zencache-pro repo.

  • Replace this line with the following:

    add_filter('updated_user_meta', array($this, 'autoClearUserCacheFA2'), 10, 2);
  • Replace this line and this line with the following:

    * @attaches-to `updated_user_meta` filter.
    
  • Submit PR.

@jaswrks
Copy link

@jaswrks jaswrks commented Dec 1, 2015

@kristineds Very close. We should use add_action instead of add_filter, because updated_user_meta is an action not a filter.

add_action('updated_user_meta', array($this, 'autoClearUserCacheFA2'), 10, 2);

Taking this a step further, we should also attach to a different handler. The current handler is designed to work as a filter (i.e., ...FA2 = filter argument 2), and it is named autoClearUserCacheFA2(). We should create a new method that is like this one, but change the number of arguments.

Why? Notice this line in the WP core where the do_action() call occurs. Note that it's a do_action() call and not an apply_filters() call. Also note that it's list of arguments has $object_id (i.e., the user ID) in the second position—$meta_id in the first position.

do_action( "updated_{$meta_type}_meta", $meta_id, $object_id, $meta_key, $_meta_value );

So what we need is this:

add_action('updated_user_meta', array($this, 'autoClearUserCacheA2'), 10, 2);

and then we need to create the autoClearUserCacheA2() method modeled after this one.

@jaswrks
Copy link

@jaswrks jaswrks commented Dec 2, 2015

↑ I updated the above to improve clarity.

@kristineds
Copy link

@kristineds kristineds commented Dec 4, 2015

@jaswsinc @raamdev Like this?


Next Actions

  • New feature branch in the websharks/zencache-pro repo.

  • Replace this line with the following:

    add_action('updated_user_meta', array($this, 'autoClearUserCacheA2'), 10, 2);
  • After this line:

    /*
    * Automatically clears cache files associated with a particular user.
    *
    * @since 150422 Rewrite.
    *
    * @attaches-to `updated_user_meta` hook.
    *
    * @param int    $meta_id    ID of updated metadata entry.
    * @param int    $object_id  Object ID.
    */
    $self->autoClearUserCacheA2 = function ($meta_id, $object_id) use ($self) {
        $self->autoClearUserCache($meta_id);
    };
  • Replace this line and this line with the following:

    * @attaches-to `updated_user_meta` hook.
    
  • Submit PR.

@raamdev
Copy link
Contributor

@raamdev raamdev commented Dec 4, 2015

@kristineds Yes, that looks correct, except $self->autoClearUserCache($meta_id); should be $self->autoClearUserCache($object_id);. The $object_id is the user ID whose cache we need to clear, whereas $meta_id is just the ID of updated metadata entry (autoClearUserCache() clears a user cache).


Could you go ahead and submit a PR for this? 😄

@jaswrks
Copy link

@jaswrks jaswrks commented Dec 4, 2015

@kristineds Really nice work on this. Fantastic!

Just that minor tweak Raam mentioned and that should do it.

@kristineds
Copy link

@kristineds kristineds commented Dec 5, 2015

@jaswsinc @raamdev Submitting PR for this: wpsharks/comet-cache-pro#195

Note: Is there a way I could edit that previous commit message title? I got it mixed up. .

@raamdev
Copy link
Contributor

@raamdev raamdev commented Dec 5, 2015

@kristineds writes...

Is there a way I could edit that previous commit message title? I got it mixed up.
---- Unfortunately, no. Once the commit is pushed to GitHub it can't be changed. You can undo your last commit before git push though; see http://stackoverflow.com/a/927386/130664

@raamdev
Copy link
Contributor

@raamdev raamdev commented Dec 5, 2015

Next Pro Release Changelog:

  • Bug Fix: With Logged-In User caching enabled, there were some scenarios where the user cache was being cleared on every page load when a user was logged in. ZenCache now hooks into updated_user_metadata instead of update_user_metadata to clear a specific user cache. Props @kristineds. See Issue #623.
@raamdev raamdev closed this Dec 5, 2015
@raamdev
Copy link
Contributor

@raamdev raamdev commented Dec 21, 2015

ZenCache Pro v151220 has been released and includes changes worked on as part of this GitHub Issue. See the release announcement for further details.


This issue will now be locked to further updates. If you have something to add related to this GitHub Issue, please open a new GitHub Issue and reference this one (#623).

@wpsharks wpsharks locked and limited conversation to collaborators Dec 21, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants