Skip to content

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

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.

Sign up for GitHub

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

Closed
josephscott wants to merge 17 commits into from
+142 −7
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
bc1e528
Check cache return value in metadata_exists(), get_metadata_raw(), an…
josephscott Sep 10, 2026
a6d724f
Merge branch 'trunk' into 66091/meta-cache-return-checks
josephscott Sep 16, 2026
718ed75
Merge branch 'trunk' into 66091/meta-cache-return-checks
sirreal Sep 21, 2026
215ae90
Move the $invalid_ids check to happen earlier
josephscott Sep 21, 2026
e18afdb
Merge branch 'trunk' into 66091/meta-cache-return-checks
sirreal Sep 22, 2026
57b87e2
Update tests/phpunit/tests/meta.php
josephscott Sep 22, 2026
ec5f3ce
Merge branch 'trunk' into 66091/meta-cache-return-checks
josephscott Sep 22, 2026
4a16be7
Merge branch 'trunk' into 66091/meta-cache-return-checks
sirreal Sep 23, 2026
e86acd5
Rename $invalid_ids to $invalid_cache_ids
josephscott Sep 24, 2026
a3cf1c6
Add a unique message to each test assertion
josephscott Sep 24, 2026
11eeaab
Rename $cached_value to $invalid_cached_value in tests
josephscott Sep 24, 2026
4308d68
Merge branch 'trunk' into 66091/meta-cache-return-checks
josephscott Sep 24, 2026
0ebd216
Merge branch 'trunk' into 66091/meta-cache-return-checks
westonruter Sep 25, 2026
2920255
Add assertions which guard against PHPStan errors
westonruter Sep 25, 2026
54de664
Merge branch 'trunk' into 66091/meta-cache-return-checks
sirreal Sep 25, 2026
e2d0ca1
Merge branch 'trunk' into 66091/meta-cache-return-checks
peterwilsoncc Sep 25, 2026
0b9dffe
Merge branch 'trunk' into 66091/meta-cache-return-checks
peterwilsoncc Sep 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 27 additions & 7 deletions src/wp-includes/meta.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -632,6 +632,7 @@ function get_metadata( $meta_type, $object_id, $meta_key = '', $single = false )
* Retrieves raw metadata value for the specified object.
*
* @since 5.5.0
* @since 7.2.0 A cached value that is not an array is now treated as a cache miss.
*
* @param string $meta_type Type of object metadata is for. Accepts 'blog', 'post', 'comment', 'term',
* 'user', or any other object type with an associated meta table.
Expand Down Expand Up @@ -708,7 +709,8 @@ function get_metadata_raw( $meta_type, $object_id, $meta_key = '', $single = fal

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

if ( ! $meta_cache ) {
// A cached value that is not an array is unusable, treat it as a cache miss.
if ( ! $meta_cache || ! is_array( $meta_cache ) ) {

@peterwilsoncc peterwilsoncc Sep 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 josephscott Sep 24, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 josephscott Sep 24, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

$meta_cache = update_meta_cache( $meta_type, array( $object_id ) );
$meta_cache = $meta_cache[ $object_id ] ?? null;
}
Expand Down Expand Up @@ -792,6 +794,7 @@ function get_metadata_default( $meta_type, $object_id, $meta_key, $single = fals
* Determines if a meta field with the given key exists for the given object ID.
*
* @since 3.3.0
* @since 7.2.0 A cached value that is not an array is now treated as a cache miss.
*
* @param string $meta_type Type of object metadata is for. Accepts 'blog', 'post', 'comment', 'term',
* 'user', or any other object type with an associated meta table.
Expand All @@ -817,9 +820,10 @@ function metadata_exists( $meta_type, $object_id, $meta_key ) {

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

if ( ! $meta_cache ) {
// A cached value that is not an array is unusable, treat it as a cache miss.
if ( ! $meta_cache || ! is_array( $meta_cache ) ) {
$meta_cache = update_meta_cache( $meta_type, array( $object_id ) );
$meta_cache = $meta_cache[ $object_id ];
$meta_cache = $meta_cache[ $object_id ] ?? null;
}

if ( isset( $meta_cache[ $meta_key ] ) ) {
Expand Down Expand Up @@ -1165,6 +1169,7 @@ function delete_metadata_by_mid( $meta_type, $meta_id ) {
* Updates the metadata cache for the specified objects.
*
* @since 2.9.0
* @since 7.2.0 A cached value that is not an array is now treated as a cache miss.
*
* @global wpdb $wpdb WordPress database abstraction object.
*
Expand Down Expand Up @@ -1219,19 +1224,33 @@ function update_meta_cache( $meta_type, $object_ids ) {
return (bool) $check;
}

$cache_group = $meta_type . '_meta';
$non_cached_ids = array();
$cache = array();
$cache_values = wp_cache_get_multiple( $object_ids, $cache_group );
$cache_group = $meta_type . '_meta';
$non_cached_ids = array();
$invalid_cache_ids = array();
$cache = array();
$cache_values = wp_cache_get_multiple( $object_ids, $cache_group );

foreach ( $cache_values as $id => $cached_object ) {
if ( false === $cached_object ) {
$non_cached_ids[] = $id;
} elseif ( ! is_array( $cached_object ) ) {
// A cached value that is not an array is unusable, treat it as a cache miss.
$non_cached_ids[] = $id;
$invalid_cache_ids[] = $id;
} else {
$cache[ $id ] = $cached_object;
}
}

/*
* Remove unusable cached values so that the regenerated values can be added.
* A delete followed by an add is used instead of wp_cache_set_multiple()
* so that wp_suspend_cache_addition() is still respected.
*/
if ( ! empty( $invalid_cache_ids ) ) {
Comment thread
peterwilsoncc marked this conversation as resolved.
wp_cache_delete_multiple( $invalid_cache_ids, $cache_group );
}

if ( empty( $non_cached_ids ) ) {
return $cache;
}
Expand Down Expand Up @@ -1268,6 +1287,7 @@ function update_meta_cache( $meta_type, $object_ids ) {
}
$data[ $id ] = $cache[ $id ];
}

wp_cache_add_multiple( $data, $cache_group );

return $cache;
Expand Down
115 changes: 115 additions & 0 deletions tests/phpunit/tests/meta.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,121 @@ public function test_metadata_exists_with_filter() {
remove_filter( 'get_user_metadata', '__return_zero' );
}

/**
Comment thread
josephscott marked this conversation as resolved.
* Non-array values that can not be used as a meta cache entry.
*
* @return array<string, array{mixed}>
*/
public static function data_non_array_cache_values(): array {
return array(
'object' => array( new stdClass() ),
'string' => array( 'meta_value' ),
'integer' => array( 1 ),
'float' => array( 1.5 ),
'true' => array( true ),
);
}

/**
* @ticket 66091
*
* @dataProvider data_non_array_cache_values
*
* @param mixed $invalid_cached_value Unusable value to place in the meta cache.
*/
public function test_metadata_exists_treats_non_array_cache_value_as_miss( $invalid_cached_value ): void {
$this->assertTrue( wp_cache_set( self::$author->ID, $invalid_cached_value, 'user_meta' ), 'The unusable value should be placed in the cache, check test setup.' );

$this->assertTrue( metadata_exists( 'user', self::$author->ID, 'meta_key' ), 'An existing meta key should be reported as existing.' );
$this->assertFalse( metadata_exists( 'user', self::$author->ID, 'foobarbaz' ), 'A missing meta key should be reported as not existing.' );
$this->assertIsArray( wp_cache_get( self::$author->ID, 'user_meta' ), 'The unusable cache value should have been replaced.' );
}

/**
* @ticket 66091
*
* @dataProvider data_non_array_cache_values
*
* @param mixed $invalid_cached_value Unusable value to place in the meta cache.
*/
public function test_get_metadata_treats_non_array_cache_value_as_miss( $invalid_cached_value ): void {
$this->assertTrue( wp_cache_set( self::$author->ID, $invalid_cached_value, 'user_meta' ), 'The unusable value should be placed in the cache, check test setup.' );

$this->assertSame( 'meta_value', get_metadata( 'user', self::$author->ID, 'meta_key', true ), 'The single meta value should be returned.' );
$this->assertSame( array( 'meta_value' ), get_metadata( 'user', self::$author->ID, 'meta_key' ), 'The array of meta values should be returned.' );
$this->assertIsArray( wp_cache_get( self::$author->ID, 'user_meta' ), 'The unusable cache value should have been replaced.' );
}

/**
* @ticket 66091
*
* @dataProvider data_non_array_cache_values
*
* @param mixed $invalid_cached_value Unusable value to place in the meta cache.
*/
public function test_get_metadata_with_empty_key_treats_non_array_cache_value_as_miss( $invalid_cached_value ): void {
$this->assertTrue( wp_cache_set( self::$author->ID, $invalid_cached_value, 'user_meta' ), 'The unusable value should be placed in the cache, check test setup.' );

$meta = get_metadata( 'user', self::$author->ID );

$this->assertIsArray( $meta, 'All meta for the object should be returned as an array.' );
$this->assertSame( array( 'meta_value' ), $meta['meta_key'], 'The existing meta key should be included in the returned meta.' );
}

/**
* @ticket 66091
*
* @dataProvider data_non_array_cache_values
*
* @param mixed $invalid_cached_value Unusable value to place in the meta cache.
*/
public function test_update_meta_cache_replaces_non_array_cache_value( $invalid_cached_value ): void {
$this->assertTrue( wp_cache_set( self::$author->ID, $invalid_cached_value, 'user_meta' ), 'The unusable value should be placed in the cache, check test setup.' );

$meta_cache = update_meta_cache( 'user', array( self::$author->ID ) );

$this->assertIsArray( $meta_cache );
$this->assertArrayHasKey( self::$author->ID, $meta_cache );
$this->assertIsArray( $meta_cache[ self::$author->ID ], 'The returned meta cache for the object should be an array.' );
$this->assertSame( array( 'meta_value' ), $meta_cache[ self::$author->ID ]['meta_key'], 'The returned meta cache should include the existing meta key.' );

$cached = wp_cache_get( self::$author->ID, 'user_meta' );
$this->assertIsArray( $cached, 'The unusable cache value should have been replaced.' );
$this->assertSame( array( 'meta_value' ), $cached['meta_key'], 'The replaced cache value should include the existing meta key.' );
}

/**
* @ticket 66091
*/
public function test_update_meta_cache_replaces_non_array_cache_value_for_object_without_meta(): void {
$term_id = self::factory()->term->create();

$this->assertTrue( wp_cache_set( $term_id, new stdClass(), 'term_meta' ), 'The unusable value should be placed in the cache, check test setup.' );

$meta_cache = update_meta_cache( 'term', array( $term_id ) );
$this->assertIsArray( $meta_cache );
$this->assertArrayHasKey( $term_id, $meta_cache );

$this->assertSame( array(), $meta_cache[ $term_id ], 'The returned meta cache for an object without meta should be an empty array.' );
$this->assertSame( array(), wp_cache_get( $term_id, 'term_meta' ), 'The unusable cache value should have been replaced with an empty array.' );
}

/**
* @ticket 66091
*/
public function test_update_meta_cache_removes_non_array_cache_value_while_cache_addition_is_suspended(): void {
$this->assertTrue( wp_cache_set( self::$author->ID, new stdClass(), 'user_meta' ), 'The unusable value should be placed in the cache, check test setup.' );

wp_suspend_cache_addition( true );
$meta_cache = update_meta_cache( 'user', array( self::$author->ID ) );
wp_suspend_cache_addition( false );
$this->assertIsArray( $meta_cache );
$this->assertIsArray( $meta_cache[ self::$author->ID ] );

$this->assertSame( array( 'meta_value' ), $meta_cache[ self::$author->ID ]['meta_key'], 'The meta should still be returned while cache addition is suspended.' );
$this->assertFalse( wp_cache_get( self::$author->ID, 'user_meta' ), 'The unusable cache value should be removed but not replaced while cache addition is suspended.' );
}

/**
* @ticket 18158
*/
Expand Down
Loading