Check cache return value in metadata_exists(), get_metadata_raw(), an…

[josephscott]

Check cache return value in metadata_exists(), get_metadata_raw(), and update_meta_cache()

Trac ticket: https://core.trac.wordpress.org/ticket/66091

The meta functions read the {$meta_type}_meta cache group and only check the return value for falsiness before using it as an array:

$meta_cache = wp_cache_get( $object_id, $meta_type . '_meta' );

if ( ! $meta_cache ) {
        $meta_cache = update_meta_cache( $meta_type, array( $object_id ) );
        $meta_cache = $meta_cache[ $object_id ];
}

if ( isset( $meta_cache[ $meta_key ] ) ) {

If the cache returns anything other than an array, isset() on an object is a fatal error, and the other types silently produce wrong results.

The same thing happens in get_metadata_raw() ( GH ) and metadata_exists() ( GH ). Both fall back to update_meta_cache(), which reads the same group through wp_cache_get_multiple() and only checks for false ( GH ), so it hands the unusable value straight back.

A non-array cached value should be treated as a cache miss in all three places. This is another case for #66005, where WP core could use a validating cache function.

This also speaks to the broader issue of making sure return values are correctly checked before making use of them.

AI assistance: Yes
Tool(s): Claude Code
Model(s): Fable 5.1
Used for: talking over the general issue, exploring options - it wrote all of the tests.

---

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

[github-actions]

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.

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

Props josephscott, jonsurrell, westonruter, peterwilsoncc.

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

[github-actions]

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

• 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.

[sirreal]

This looks good to me.

[peterwilsoncc]

This is looking good but I've suggested a few items inline.

While testing, I added this to my test suite to ensure primed empty caches didn't result in a database query. It might be worth adding to the suite:

public function test_get_metadata_hits_cache_when_no_meta_data_set() {
	$post_id   = $this->factory()->post->create();
	$post_meta = get_post_meta( $post_id );

	// Delete all the meta data.
	foreach ( $post_meta as $meta_key => $meta_value ) {
		delete_post_meta( $post_id, $meta_key );
	}

	// Ensure the cache is flushed.
	wp_cache_delete( $post_id, 'post_meta' );

	$start_queries = get_num_queries();
	$meta_data     = get_post_meta( $post_id );
	$this->assertSame( 1, get_num_queries() - $start_queries, 'Expected database query getting meta with flushed cache' );
	$this->assertSame( array(), $meta_data, 'Post meta is expected to be an empty array' );

	$start_queries = get_num_queries();
	$meta_data     = get_post_meta( $post_id );
	$this->assertSame( 0, get_num_queries() - $start_queries, 'Expected no database query getting meta with primed cache' );
}

[peterwilsoncc]

It's not directly related but a valid cache of an empty array will follow the update_meta_cache() flow. update_meta_cache() will detect that the cache is primed so it's not a big deal.

Do you think it's worth including that as part of the fix for this ticket?

[josephscott]

I don't think that is worth including for this PR - where the focus was on avoiding fatal error conditions. I'd rather see that happen in a separate PR.

[josephscott]

I'd probably include test_get_metadata_hits_cache_when_no_meta_data_set() in a separate PR as well, associated with empty cache array idea.

[peterwilsoncc]

Thanks @josephscott, these update all look good to me.

I don't think that is worth including for this PR - where the focus was on avoiding fatal error conditions. I'd rather see that happen in a separate PR.

Works for me.

[github-actions]

A commit was made that fixes the Trac ticket referenced in the description of this pull request.

SVN changeset: 63958
GitHub commit: 985c197

This PR will be closed, but please confirm the accuracy of this and reopen if there is more work to be done.