Skip to content

Commit 03c8718

Browse files
jigneshbhavanijigneshbhavani
committed
Cache API: Do not cache a miss while cache addition is suspended.
[63354] replaced wp_cache_add() with wp_cache_set() in get_instance() on WP_Post, WP_Term, WP_Comment, WP_Site and WP_Network. Only add() checks wp_suspend_cache_addition(), so all five began caching a miss while additions are suspended. Replacing an unusable cached value is not an addition, so only a genuine miss now defers to the flag. See #66006.
1 parent 4b2da6e commit 03c8718

10 files changed

Lines changed: 182 additions & 10 deletions

File tree

‎src/wp-includes/class-wp-comment.php‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -266,15 +266,22 @@ public static function get_instance( $id ) {
266266

267267
// A cached value that is not usable as a comment is treated as a cache miss.
268268
if ( ! is_object( $_comment ) || ! isset( $_comment->comment_ID ) ) {
269+
$unusable_value_cached = ( false !== $_comment );
270+
269271
/** @var object{ comment_ID: string, comment_post_ID: string, comment_author: string, comment_author_email: string, comment_author_url: string, comment_author_IP: string, comment_date: string, comment_date_gmt: string, comment_content: string, comment_karma: string, comment_approved: string, comment_agent: string, comment_type: string, comment_parent: string, user_id: string }|null $_comment */
270272
$_comment = $wpdb->get_row( $wpdb->prepare( "SELECT * FROM $wpdb->comments WHERE comment_ID = %d LIMIT 1", $comment_id ) );
271273

272274
if ( ! $_comment ) {
273275
return false;
274276
}
275277

276-
// Not wp_cache_add(), since an unusable cached value may still be present and must be replaced.
277-
wp_cache_set( $_comment->comment_ID, $_comment, 'comment' );
278+
/*
279+
* Not wp_cache_add(), since an unusable cached value must be replaced. Replacing an
280+
* existing entry is not an addition, so only a genuine miss defers to the flag.
281+
*/
282+
if ( $unusable_value_cached || ! wp_suspend_cache_addition() ) {
283+
wp_cache_set( $_comment->comment_ID, $_comment, 'comment' );
284+
}
278285
}
279286

280287
return new WP_Comment( $_comment );

‎src/wp-includes/class-wp-network.php‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,14 +113,21 @@ public static function get_instance( $network_id ) {
113113
&&
114114
! is_numeric( $_network )
115115
) {
116+
$unusable_value_cached = ( false !== $_network );
117+
116118
$_network = $wpdb->get_row( $wpdb->prepare( "SELECT * FROM {$wpdb->site} WHERE id = %d LIMIT 1", $network_id ) );
117119

118120
if ( empty( $_network ) || is_wp_error( $_network ) ) {
119121
$_network = -1;
120122
}
121123

122-
// Not wp_cache_add(), since an unusable cached value may still be present and must be replaced.
123-
wp_cache_set( $network_id, $_network, 'networks' );
124+
/*
125+
* Not wp_cache_add(), since an unusable cached value must be replaced. Replacing an
126+
* existing entry is not an addition, so only a genuine miss defers to the flag.
127+
*/
128+
if ( $unusable_value_cached || ! wp_suspend_cache_addition() ) {
129+
wp_cache_set( $network_id, $_network, 'networks' );
130+
}
124131
}
125132

126133
if ( is_numeric( $_network ) ) {

‎src/wp-includes/class-wp-post.php‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,8 @@ public static function get_instance( $post_id ) {
288288

289289
// A cached value that is not usable as a post is treated as a cache miss.
290290
if ( ! ( $_post instanceof stdClass || $_post instanceof WP_Post ) || ! isset( $_post->ID ) ) {
291+
$unusable_value_cached = ( false !== $_post );
292+
291293
$_post = $wpdb->get_row( $wpdb->prepare( "SELECT * FROM $wpdb->posts WHERE ID = %d LIMIT 1", $post_id ) );
292294

293295
if ( ! $_post ) {
@@ -296,8 +298,13 @@ public static function get_instance( $post_id ) {
296298

297299
$_post = sanitize_post( $_post, 'raw' );
298300

299-
// Not wp_cache_add(), since an unusable cached value may still be present and must be replaced.
300-
wp_cache_set( (int) $_post->ID, $_post, 'posts' );
301+
/*
302+
* Not wp_cache_add(), since an unusable cached value must be replaced. Replacing an
303+
* existing entry is not an addition, so only a genuine miss defers to the flag.
304+
*/
305+
if ( $unusable_value_cached || ! wp_suspend_cache_addition() ) {
306+
wp_cache_set( (int) $_post->ID, $_post, 'posts' );
307+
}
301308
} elseif ( empty( $_post->filter ) || 'raw' !== $_post->filter ) {
302309
$_post = sanitize_post( $_post, 'raw' );
303310
}

‎src/wp-includes/class-wp-site.php‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -182,14 +182,21 @@ public static function get_instance( $site_id ) {
182182
&&
183183
! is_numeric( $_site )
184184
) {
185+
$unusable_value_cached = ( false !== $_site );
186+
185187
$_site = $wpdb->get_row( $wpdb->prepare( "SELECT * FROM {$wpdb->blogs} WHERE blog_id = %d LIMIT 1", $site_id ) );
186188

187189
if ( empty( $_site ) || is_wp_error( $_site ) ) {
188190
$_site = -1;
189191
}
190192

191-
// Not wp_cache_add(), since an unusable cached value may still be present and must be replaced.
192-
wp_cache_set( $site_id, $_site, 'sites' );
193+
/*
194+
* Not wp_cache_add(), since an unusable cached value must be replaced. Replacing an
195+
* existing entry is not an addition, so only a genuine miss defers to the flag.
196+
*/
197+
if ( $unusable_value_cached || ! wp_suspend_cache_addition() ) {
198+
wp_cache_set( $site_id, $_site, 'sites' );
199+
}
193200
}
194201

195202
if ( is_numeric( $_site ) ) {

‎src/wp-includes/class-wp-term.php‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,8 @@ public static function get_instance( $term_id, $taxonomy = null ) {
133133
|| ! isset( $_term->term_id, $_term->taxonomy )
134134
|| ( $taxonomy && $taxonomy !== $_term->taxonomy )
135135
) {
136+
$unusable_value_cached = ( false !== $_term );
137+
136138
// Any term found in the cache is not a match, so don't use it.
137139
$_term = false;
138140

@@ -185,8 +187,13 @@ public static function get_instance( $term_id, $taxonomy = null ) {
185187

186188
// Don't cache terms that are shared between taxonomies.
187189
if ( 1 === count( $terms ) ) {
188-
// Not wp_cache_add(), since an unusable cached value may still be present and must be replaced.
189-
wp_cache_set( $term_id, $_term, 'terms' );
190+
/*
191+
* Not wp_cache_add(), since an unusable cached value must be replaced. Replacing an
192+
* existing entry is not an addition, so only a genuine miss defers to the flag.
193+
*/
194+
if ( $unusable_value_cached || ! wp_suspend_cache_addition() ) {
195+
wp_cache_set( $term_id, $_term, 'terms' );
196+
}
190197
}
191198
}
192199

‎tests/phpunit/tests/comment/wpComment.php‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,4 +247,25 @@ public function test_post_fields_should_be_null_for_an_unattached_comment(): voi
247247
$this->assertFalse( $is_set, 'Expected __isset() to return false for an unattached comment.' );
248248
$this->assertNull( $title, 'Expected __get() not to read the field from the global post.' );
249249
}
250+
251+
/**
252+
* Tests that a cache miss is not stored while cache addition is suspended.
253+
*
254+
* @ticket 66006
255+
*/
256+
public function test_get_instance_does_not_cache_a_miss_while_cache_addition_is_suspended(): void {
257+
clean_comment_cache( self::$comment_id );
258+
$this->assertFalse( wp_cache_get( self::$comment_id, 'comment' ), 'The comment was cached before the test began.' );
259+
260+
$suspend = wp_suspend_cache_addition();
261+
wp_suspend_cache_addition( true );
262+
263+
$comment = WP_Comment::get_instance( self::$comment_id );
264+
265+
wp_suspend_cache_addition( $suspend );
266+
267+
$this->assertInstanceOf( WP_Comment::class, $comment, 'A comment object was not returned.' );
268+
$this->assertSame( self::$comment_id, (int) $comment->comment_ID, 'The wrong comment was returned.' );
269+
$this->assertFalse( wp_cache_get( self::$comment_id, 'comment' ), 'The comment was added to the cache.' );
270+
}
250271
}

‎tests/phpunit/tests/multisite/network.php‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -760,4 +760,27 @@ protected function _get_next_network_id() {
760760
static::factory()->network->create();
761761
return (int) $wpdb->get_var( 'SELECT id FROM ' . $wpdb->site . ' ORDER BY id DESC LIMIT 1' ) + 1;
762762
}
763+
764+
/**
765+
* Tests that a cache miss is not stored while cache addition is suspended.
766+
*
767+
* @ticket 66006
768+
*
769+
* @covers WP_Network::get_instance
770+
*/
771+
public function test_get_instance_does_not_cache_a_miss_while_cache_addition_is_suspended(): void {
772+
clean_network_cache( self::$different_network_id );
773+
$this->assertFalse( wp_cache_get( self::$different_network_id, 'networks' ), 'The network was cached before the test began.' );
774+
775+
$suspend = wp_suspend_cache_addition();
776+
wp_suspend_cache_addition( true );
777+
778+
$network = WP_Network::get_instance( self::$different_network_id );
779+
780+
wp_suspend_cache_addition( $suspend );
781+
782+
$this->assertInstanceOf( WP_Network::class, $network, 'A network object was not returned.' );
783+
$this->assertSame( self::$different_network_id, (int) $network->id, 'The wrong network was returned.' );
784+
$this->assertFalse( wp_cache_get( self::$different_network_id, 'networks' ), 'The network was added to the cache.' );
785+
}
763786
}

‎tests/phpunit/tests/multisite/wpSite.php‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,4 +307,25 @@ public function data_get_instance_treats_a_numeric_cache_value_as_a_cached_miss(
307307
'a float as a string' => array( '3.5' ),
308308
);
309309
}
310+
311+
/**
312+
* Tests that a cache miss is not stored while cache addition is suspended.
313+
*
314+
* @ticket 66006
315+
*/
316+
public function test_get_instance_does_not_cache_a_miss_while_cache_addition_is_suspended(): void {
317+
clean_blog_cache( self::$site_id );
318+
$this->assertFalse( wp_cache_get( self::$site_id, 'sites' ), 'The site was cached before the test began.' );
319+
320+
$suspend = wp_suspend_cache_addition();
321+
wp_suspend_cache_addition( true );
322+
323+
$site = WP_Site::get_instance( self::$site_id );
324+
325+
wp_suspend_cache_addition( $suspend );
326+
327+
$this->assertInstanceOf( WP_Site::class, $site, 'A site object was not returned.' );
328+
$this->assertSame( self::$site_id, (int) $site->blog_id, 'The wrong site was returned.' );
329+
$this->assertFalse( wp_cache_get( self::$site_id, 'sites' ), 'The site was added to the cache.' );
330+
}
310331
}

‎tests/phpunit/tests/post/wpPost.php‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,4 +140,53 @@ public function data_get_instance_treats_a_poisoned_cache_value_as_a_cache_miss(
140140
'a WP_Post without ID' => array( new WP_Post( new stdClass() ) ),
141141
);
142142
}
143+
144+
/**
145+
* Tests that a cache miss is not stored while cache addition is suspended.
146+
*
147+
* @ticket 66006
148+
*
149+
* @covers WP_Post::get_instance
150+
*/
151+
public function test_get_instance_does_not_cache_a_miss_while_cache_addition_is_suspended(): void {
152+
clean_post_cache( self::$post_id );
153+
$this->assertFalse( wp_cache_get( self::$post_id, 'posts' ), 'The post was cached before the test began.' );
154+
155+
$suspend = wp_suspend_cache_addition();
156+
wp_suspend_cache_addition( true );
157+
158+
$post = WP_Post::get_instance( self::$post_id );
159+
160+
wp_suspend_cache_addition( $suspend );
161+
162+
$this->assertInstanceOf( WP_Post::class, $post, 'A post object was not returned.' );
163+
$this->assertSame( self::$post_id, $post->ID, 'The wrong post was returned.' );
164+
$this->assertFalse( wp_cache_get( self::$post_id, 'posts' ), 'The post was added to the cache.' );
165+
}
166+
167+
/**
168+
* Tests that a poisoned cache value is still replaced while cache addition is suspended.
169+
*
170+
* Replacing an entry that already exists does not grow the cache, so it is not an addition.
171+
*
172+
* @ticket 66006
173+
*
174+
* @covers WP_Post::get_instance
175+
*/
176+
public function test_get_instance_replaces_a_poisoned_cache_value_while_cache_addition_is_suspended(): void {
177+
wp_cache_set( self::$post_id, 'not-a-post', 'posts' );
178+
179+
$suspend = wp_suspend_cache_addition();
180+
wp_suspend_cache_addition( true );
181+
182+
$post = WP_Post::get_instance( self::$post_id );
183+
184+
wp_suspend_cache_addition( $suspend );
185+
186+
$this->assertInstanceOf( WP_Post::class, $post, 'A post object was not returned.' );
187+
188+
$cached = wp_cache_get( self::$post_id, 'posts' );
189+
$this->assertIsObject( $cached, 'The poisoned value was left in the cache.' );
190+
$this->assertSame( self::$post_id, (int) $cached->ID, 'The wrong post was cached.' );
191+
}
143192
}

‎tests/phpunit/tests/term/wpTerm.php‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,4 +187,27 @@ public function data_get_instance_treats_a_poisoned_cache_value_as_a_cache_miss(
187187
'a WP_Term without term_id' => array( new WP_Term( new stdClass() ) ),
188188
);
189189
}
190+
191+
/**
192+
* Tests that a cache miss is not stored while cache addition is suspended.
193+
*
194+
* @ticket 66006
195+
*
196+
* @covers WP_Term::get_instance
197+
*/
198+
public function test_get_instance_does_not_cache_a_miss_while_cache_addition_is_suspended(): void {
199+
clean_term_cache( self::$term_id, 'wptests_tax' );
200+
$this->assertFalse( wp_cache_get( self::$term_id, 'terms' ), 'The term was cached before the test began.' );
201+
202+
$suspend = wp_suspend_cache_addition();
203+
wp_suspend_cache_addition( true );
204+
205+
$term = WP_Term::get_instance( self::$term_id, 'wptests_tax' );
206+
207+
wp_suspend_cache_addition( $suspend );
208+
209+
$this->assertInstanceOf( WP_Term::class, $term, 'A term object was not returned.' );
210+
$this->assertSame( self::$term_id, $term->term_id, 'The wrong term was returned.' );
211+
$this->assertFalse( wp_cache_get( self::$term_id, 'terms' ), 'The term was added to the cache.' );
212+
}
190213
}

0 commit comments

Comments
 (0)