Prime caches for post parents in post rest api controller

[spacedmonkey]

In the Post REST controller, for every post with post status of inherit and a parent post set, a lookup for the parent is done in the check_read_permission method. For a response with 100 posts, this could result in 100 queries to the posts table. Instead of looking up one by one, simple use _prime_post_caches and prime caches for all parents in one request.

Related #55592.

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

[furi3r]

Hello Jonny, I've added unit test to cover this change, also investigated other possible scenarios where we should need to prime parent ids, but it seems this was the right and only place I could find.

@TimothyBJacobs @peterwilsoncc There are two places that get the parent post object

​https://github.com/WordPress/wordpress-develop/blob/4e830858061dbe37cda462e9a861bc7401fc6f6b/src/wp-includes/rest-api/endpoints/class-wp-rest-posts-controller.php#L2040-L2045

and

​https://github.com/WordPress/wordpress-develop/blob/4e830858061dbe37cda462e9a861bc7401fc6f6b/src/wp-includes/rest-api/endpoints/class-wp-rest-posts-controller.php#L1615-L1621

So always loading the post parent, seems like the safest thing to do.

[spacedmonkey]

In 53506:

REST API: Prime caches for post parents in post REST API controller.

Prime caches for all post parents in the post REST API controller using the _prime_post_caches function. Post parent objects are required as part of the check_read_permission method’s permission check in post REST API controller.

Props spacedmonkey, furi3r, peterwilsoncc, mitogh, madpixels.
Fixes #55593.

Committed.

[SergeyBiryukov]

In 53507:

REST API: Some documentation and test improvements for update_post_parent_caches():

• Make the function description more specific, for consistency with update_post_author_caches().
• Add missing @covers tags for the unit test.

Follow-up to [53506].

See #55593.

[SergeyBiryukov]

It looks like there might be some kind of a race condition in the test added in [53506]:

1) WP_Test_REST_Posts_Controller::test_get_items_parent_ids_primed
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
-    0 => 1880
-    1 => 1881
+    0 => 2003
 )

/var/www/tests/phpunit/includes/abstract-testcase.php:820
/var/www/tests/phpunit/tests/rest-api/rest-posts-controller.php:1828
phpvfscomposer:///var/www/vendor/phpunit/phpunit/phpunit:97

See these PHP 7.4 jobs for example:
​https://github.com/WordPress/wordpress-develop/runs/6898050638?check_suite_focus=true#step:20:267
​https://github.com/WordPress/wordpress-develop/runs/6900501418?check_suite_focus=true#step:20:267

[SergeyBiryukov]

In 53509:

Tests: Some improvements for REST API cache priming tests:

• Give the test methods more specific names and move them closer together.
• Correct the @covers tags.

Follow-up to [53499], [53504], [53506], [53507], [53508].

See #55593, #55652.

[SergeyBiryukov]

Replying to SergeyBiryukov:

It looks like there might be some kind of a race condition in the test added in [53506]:

1) WP_Test_REST_Posts_Controller::test_get_items_parent_ids_primed
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
-    0 => 1880
-    1 => 1881
+    0 => 2003
 )

/var/www/tests/phpunit/includes/abstract-testcase.php:820
/var/www/tests/phpunit/tests/rest-api/rest-posts-controller.php:1828
phpvfscomposer:///var/www/vendor/phpunit/phpunit/phpunit:97

See these PHP 7.4 jobs for example:
​https://github.com/WordPress/wordpress-develop/runs/6898050638?check_suite_focus=true#step:20:267
​https://github.com/WordPress/wordpress-develop/runs/6900501418?check_suite_focus=true#step:20:267

Looks like it's PHP 7.4 with memcached specifically that fails, not the regular PHP 7.4 job.

[SergeyBiryukov]

In 53511:

Tests: Temporarily disable the failing REST API test for update_post_parent_caches().

This needs more investigation to address the test failure with persistent object cache.

Follow-up to [53506].

Props hellofromTonya.
See #55593.

[spacedmonkey]

@SergeyBiryukov I have put together a fix for the broken test. See ​#2815.

Fix broken test and uncomment it out.

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

This works for me, thanks!

• Could we use the same approach in the update_post_thumbnail_cache() test above? That one also fails for me locally, though not on GitHub for some reason.
• I think $primed_posts_found or $primed_ids_found would be more appropriate than $primed_query_found, as we're not searching for a query here.
• I was wondering if we could use something like this instead of the whole MockAction approach.

  // Verify that all $parent_ids are cached.
  $this->assertEmpty( array_diff( $parent_ids, _get_non_cached_ids( $parent_ids, 'posts' ) ) );
  

  Seems like a more straightforward way to check.

I was wondering if we could use something like this instead of the whole MockAction approach:

// Verify that all $parent_ids are cached.
$this->assertEmpty( array_diff( $parent_ids, _get_non_cached_ids( $parent_ids, 'posts' ) ) );

Seems like a more straightforward way to check.

Sorry, I think I meant something like this:

$cached_ids = array_keys( array_filter( wp_cache_get_multiple( $parent_ids, 'posts' ) ) );
$this->assertSameSets( $parent_ids, $cached_ids );

But that doesn't seem to work in my testing.

@SergeyBiryukov I worked out the root cause of the issue and pushed a fix.

Ah, that also works. Looks good 👍

[spacedmonkey]

In 53512:

Tests: Re-enable failing REST API test for update_post_parent_caches().

Ensure that that attachment objects are not primed in cache so that test passes when object caching is enabled.

Follow-up to [53506].

Props SergeyBiryukov.
See #55593.

Committed.

[desrosj]

There are a lot of caching related changes in 6.1 and a dev note should be written collecting them all. Marking this for inclusion.

[milana_cap]

Dev note: ​https://make.wordpress.org/core/2022/10/10/performance-improvements-to-the-rest-api/