Rebased: Suspend calling wp_cache_set_posts_last_changed() when touching post meta in WP_Sync_Post_Meta_Storage#11320
Conversation
…meta in WP_Sync_Post_Meta_Storage
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
src/wp-includes/collaboration/class-wp-sync-post-meta-storage.php
Outdated
Show resolved
Hide resolved
| * @return T Return value from the callback. | ||
| */ | ||
| private function with_suspended_posts_last_changed_update( Closure $callback ) { | ||
| $GLOBALS['__suspend_posts_last_changed_update'] = true; |
There was a problem hiding this comment.
Nice! I'd just use a counter here, such as
| $GLOBALS['__suspend_posts_last_changed_update'] = true; | |
| ++$GLOBALS['__suspend_posts_last_changed_update']; |
and later on
--$GLOBALS['__suspend_posts_last_changed_update'];with a check such as
0 === $GLOBALS['__suspend_posts_last_changed_update']just in case there's a call inside a call for whatever weird reason.
| try { | ||
| return $callback(); | ||
| } finally { | ||
| $GLOBALS['__suspend_posts_last_changed_update'] = false; |
There was a problem hiding this comment.
We should also update this one if we end up using it as a counter
| // update_post_meta returns false if the value is the same as the existing value. | ||
| update_post_meta( $post_id, wp_slash( self::AWARENESS_META_KEY ), wp_slash( $awareness ) ); | ||
| $this->with_suspended_posts_last_changed_update( | ||
| fn() => update_post_meta( $post_id, self::AWARENESS_META_KEY, wp_slash( $awareness ) ) |
There was a problem hiding this comment.
Do we need that private method actually? Or should we just inline it here?
There was a problem hiding this comment.
If we don't inline it, then we can avoid the need for try/catch and a counter approach, so probably worth it.
There was a problem hiding this comment.
the try/catch is extra safe, though, so i'll keep it
There was a problem hiding this comment.
I'm not sure if I follow how could we avoid try/catch and a counter – do you mean we can avoid duplicating that code in two places? Yeah that sounds good to me. Or do you mean we could avoid that entirely? In which case I'm super curious.
There was a problem hiding this comment.
Ignore this, I was misunderstanding what you meant by "call inside of a call"
|
Also, this change touches a pretty central data flow – can we add a few tests for this to be sure it doesn't regress over time? |
Unit tests added in dc85bf7 |
…sts-last-changed-update
4 participants
This is a rebased version of @westonruter's #11002 with conflicts fixed.
Trac ticket: https://core.trac.wordpress.org/ticket/64696
Use of AI Tools
Gemini CLI authored a commit to help add phpdoc for the
with_suspended_posts_last_changed_update()method.