Test using WordPress Playground

The 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

• The Plugin and Theme Directories cannot be accessed within Playground.
• All changes will be lost when closing a tab with a Playground instance.
• All changes will be lost when refreshing the page.
• A fresh instance is created each time the link below is clicked.
• Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
  it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

LGTM! I'd approve this if I'd had the permissions.

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 props-bot label.

Unlinked Accounts

The following contributors have not linked their GitHub and WordPress.org accounts: @jonnynews.

Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases.

Core Committers: Use this line as a base for the props when committing in SVN:

Props spacedmonkey, peterwilsoncc, flixos90, sanchothefat, tillkruess, rmccue, mukesh27, adamsilverstein.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

I think we should consider implementing two helper functions to read and write cache keys with $last_changed awareness, so that that logic is abstracted and doesn't need to be duplicated in all these places.

Excellent idea!

I've pushed a couple of minor changes for the following:

• bd142f1 - CS alignment standards in various docs.
• 12a45c7 - a functional change to return the results of setting the cache/s to match the existing APIs for wp_cache_set and wp_cache_set_multiple

I've committed @felixarntz and @mukeshpanchal27's suggestions and merged in trunk.

I'm still a little concerned about passing in the salt values rather than the last changed names.

It requires a developer remember to maintain order when generating the salted value, for example the following will result in different underlying cache keys being hit.

$salts = array(
   wp_cache_get_last_changed( 'group1' ),
   wp_cache_get_last_changed( 'group2' ),
);
$cached = wp_cache_get_salted( 'key', 'group', $salt );

$salts = array(
   wp_cache_get_last_changed( 'group2' ),
   wp_cache_get_last_changed( 'group1' ),
);
$cached = wp_cache_get_salted( 'key', 'group', $salt );

If instead we pass the group names for the $salt parameter, we can sort them prior to getting the last changed values.

sort( $salt );
$real_salt = array();
foreach ( $salt as $s ) {
   $real_salt[] = wp_cache_get_last_changed( $salt );
}

A compromise possibility may be to accept the salts with keys and do a ksort, using Query as an example

$salt = array(
   'posts' => wp_cache_get_last_changed( 'posts' ),
   'terms' => wp_cache_get_last_changed( 'terms' ),
);

I don't know that I am invested enough to consider this a blocker but if it's something we can optimise while working on this ticket, it would be preferable than doing so in the future.

If instead we pass the group names for the $salt parameter, we can sort them prior to getting the last changed values.

+1

@peterwilsoncc I can see an argument for a helper function to make the salt specifically for wp-core queries, but sorting $salt seems risky and passing group names to wp_cache_get_salted() seems incorrect to me. Just my ¢2.

$salt = wp_cache_get_last_changed_salt([ 'users', 'comments' ])

$cached = wp_cache_get_salted( 'some-query', 'user-queries', $salt );

function wp_cache_get_last_changed_salt( $groups ) {
    if (! is_array( $groups ) ) {
        $groups = [$groups];
    }

    sort( $groups );

    return array_map(function ( $group ) {
        return wp_cache_get_last_changed( $salt )
    });
}

I can see an argument for a helper function to make the salt specifically for wp-core queries, but sorting $salt seems risky and passing group names to wp_cache_get_salted() seems incorrect to me. Just my ¢2.

I think we might be over thinking this a little. My perf to just commit this and we can see key sort later if we feel it is needed.

If we do add keys to the salts, then we could just do this.

$salt = array(
   'posts' => wp_cache_get_last_changed( 'posts' ),
   'terms' => wp_cache_get_last_changed( 'terms' ),
);
wp_cache_get_salted( $cache_key, $group, $salt );

if( is_array( $salt ) ) {
     ksort( $salt );
     $salt = array_values( $salt );
     $salt = implode( ':', $salt );
}

Above could be adde later without any sort of break.

Commited in r60697.