diff --git a/inc/sitemaps/class-post-type-sitemap-provider.php b/inc/sitemaps/class-post-type-sitemap-provider.php index a621464e18d..ff60ba2a717 100644 --- a/inc/sitemaps/class-post-type-sitemap-provider.php +++ b/inc/sitemaps/class-post-type-sitemap-provider.php @@ -566,8 +566,26 @@ protected function get_posts( $post_type, $count, $offset ) { $post_ids[] = $sanitized_post->ID; } + /** + * Filter to disable priming the post, term and featured-image caches for the sitemap. + * + * @since 28.0 + * + * @param bool $disable_cache_priming Whether to disable priming the caches. Defaults to false. + */ + $disable_priming = apply_filters( 'wpseo_disable_xml_sitemap_cache_priming', false ); + + if ( ! $disable_priming ) { + // Warm the post and term caches in bulk, so permalink and image building doesn't query per post. + _prime_post_caches( $post_ids, true, false ); + } + update_meta_cache( 'post', $post_ids ); + if ( ! $disable_priming && $this->include_images ) { + $this->get_image_parser()->prime_thumbnail_caches( $posts ); + } + return $posts; } diff --git a/inc/sitemaps/class-sitemap-image-parser.php b/inc/sitemaps/class-sitemap-image-parser.php index 1a883999708..119b4cb4398 100644 --- a/inc/sitemaps/class-sitemap-image-parser.php +++ b/inc/sitemaps/class-sitemap-image-parser.php @@ -79,7 +79,8 @@ public function get_images( $post ) { return $images; } - $thumbnail_id = get_post_thumbnail_id( $post->ID ); + // Pass the post object rather than its ID, so the post does not get re-fetched from the database. + $thumbnail_id = get_post_thumbnail_id( $post ); if ( $thumbnail_id ) { @@ -132,6 +133,34 @@ public function get_images( $post ) { return $images; } + /** + * Primes the meta caches of the featured images of the given posts. + * + * This parser reads each post's featured image file location from the attachment's + * meta individually; warming that meta cache in bulk avoids one query per post on + * setups without a persistent object cache. + * + * @param WP_Post[] $posts The posts to prime the featured-image caches for. + * + * @return void + */ + public function prime_thumbnail_caches( $posts ) { + + $thumbnail_ids = []; + + foreach ( $posts as $post ) { + $thumbnail_id = get_post_thumbnail_id( $post ); + + if ( $thumbnail_id ) { + $thumbnail_ids[] = $thumbnail_id; + } + } + + if ( ! empty( $thumbnail_ids ) ) { + update_meta_cache( 'post', array_unique( $thumbnail_ids ) ); + } + } + /** * Get the images in the term description. * diff --git a/src/repositories/indexable-repository.php b/src/repositories/indexable-repository.php index 2ddb473d297..b6333cf6d37 100644 --- a/src/repositories/indexable-repository.php +++ b/src/repositories/indexable-repository.php @@ -413,6 +413,16 @@ public function find_by_multiple_ids_and_type( $object_ids, $object_type, $auto_ $indexables_to_create = \array_diff( $object_ids, $indexables_available ); + if ( ! empty( $indexables_to_create ) ) { + // Warm the object caches for the whole batch, so each build below does not trigger its own uncached queries. + if ( $object_type === 'post' ) { + \_prime_post_caches( $indexables_to_create ); + } + elseif ( $object_type === 'term' ) { + \_prime_term_caches( $indexables_to_create ); + } + } + foreach ( $indexables_to_create as $indexable_to_create ) { $indexables[] = $this->builder->build_for_id_and_type( $indexable_to_create, $object_type ); } diff --git a/tests/Unit/Repositories/Indexable_Repository_Test.php b/tests/Unit/Repositories/Indexable_Repository_Test.php index f63c7859921..9da9175135e 100644 --- a/tests/Unit/Repositories/Indexable_Repository_Test.php +++ b/tests/Unit/Repositories/Indexable_Repository_Test.php @@ -2,6 +2,7 @@ namespace Yoast\WP\SEO\Tests\Unit\Repositories; +use Brain\Monkey\Functions; use Mockery; use wpdb; use Yoast\WP\Lib\ORM; @@ -698,6 +699,214 @@ public function test_find_posts_by_title_keywords_upgrades_stale_indexables() { ); } + /** + * Tests that no query is run when no object ids are passed. + * + * @covers ::find_by_multiple_ids_and_type + * + * @return void + */ + public function test_find_by_multiple_ids_and_type_with_empty_object_ids() { + $this->instance->expects( 'query' )->never(); + + $this->assertSame( [], $this->instance->find_by_multiple_ids_and_type( [], 'post' ) ); + } + + /** + * Tests that nothing is built or primed when all indexables already exist. + * + * @covers ::find_by_multiple_ids_and_type + * + * @return void + */ + public function test_find_by_multiple_ids_and_type_all_found() { + $indexable_one = Mockery::mock( Indexable_Mock::class ); + $indexable_one->object_id = 1; + $indexable_two = Mockery::mock( Indexable_Mock::class ); + $indexable_two->object_id = 2; + + $this->mock_query_for_object_ids( [ 1, 2 ], 'post', [ $indexable_one, $indexable_two ] ); + + Functions\expect( '_prime_post_caches' )->never(); + $this->builder->expects( 'build_for_id_and_type' )->never(); + + $this->mock_version_check( $indexable_one ); + $this->mock_version_check( $indexable_two ); + + $this->assertSame( + [ $indexable_one, $indexable_two ], + $this->instance->find_by_multiple_ids_and_type( [ 1, 2 ], 'post' ), + ); + } + + /** + * Tests that the post caches are primed once for the batch of missing posts before building. + * + * @covers ::find_by_multiple_ids_and_type + * + * @return void + */ + public function test_find_by_multiple_ids_and_type_creates_missing_posts() { + $indexable_one = Mockery::mock( Indexable_Mock::class ); + $indexable_one->object_id = 1; + $indexable_two = Mockery::mock( Indexable_Mock::class ); + $indexable_three = Mockery::mock( Indexable_Mock::class ); + + $this->mock_query_for_object_ids( [ 1, 2, 3 ], 'post', [ $indexable_one ] ); + + // The array_diff in the method preserves the keys of the original object id array. + Functions\expect( '_prime_post_caches' ) + ->once() + ->with( + [ + 1 => 2, + 2 => 3, + ], + ); + + $this->builder + ->expects( 'build_for_id_and_type' ) + ->once() + ->with( 2, 'post' ) + ->andReturn( $indexable_two ); + $this->builder + ->expects( 'build_for_id_and_type' ) + ->once() + ->with( 3, 'post' ) + ->andReturn( $indexable_three ); + + $this->mock_version_check( $indexable_one ); + $this->mock_version_check( $indexable_two ); + $this->mock_version_check( $indexable_three ); + + $this->assertSame( + [ $indexable_one, $indexable_two, $indexable_three ], + $this->instance->find_by_multiple_ids_and_type( [ 1, 2, 3 ], 'post' ), + ); + } + + /** + * Tests that the term caches are primed once for the batch of missing terms before building. + * + * @covers ::find_by_multiple_ids_and_type + * + * @return void + */ + public function test_find_by_multiple_ids_and_type_creates_missing_terms() { + $indexable_one = Mockery::mock( Indexable_Mock::class ); + $indexable_two = Mockery::mock( Indexable_Mock::class ); + + $this->mock_query_for_object_ids( [ 5, 6 ], 'term', [] ); + + Functions\expect( '_prime_term_caches' ) + ->once() + ->with( [ 5, 6 ] ); + Functions\expect( '_prime_post_caches' )->never(); + + $this->builder + ->expects( 'build_for_id_and_type' ) + ->once() + ->with( 5, 'term' ) + ->andReturn( $indexable_one ); + $this->builder + ->expects( 'build_for_id_and_type' ) + ->once() + ->with( 6, 'term' ) + ->andReturn( $indexable_two ); + + $this->mock_version_check( $indexable_one ); + $this->mock_version_check( $indexable_two ); + + $this->assertSame( + [ $indexable_one, $indexable_two ], + $this->instance->find_by_multiple_ids_and_type( [ 5, 6 ], 'term' ), + ); + } + + /** + * Tests that no caches are primed for object types other than post and term. + * + * @covers ::find_by_multiple_ids_and_type + * + * @return void + */ + public function test_find_by_multiple_ids_and_type_does_not_prime_other_object_types() { + $indexable = Mockery::mock( Indexable_Mock::class ); + + $this->mock_query_for_object_ids( [ 7 ], 'user', [] ); + + Functions\expect( '_prime_post_caches' )->never(); + Functions\expect( '_prime_term_caches' )->never(); + + $this->builder + ->expects( 'build_for_id_and_type' ) + ->once() + ->with( 7, 'user' ) + ->andReturn( $indexable ); + + $this->mock_version_check( $indexable ); + + $this->assertSame( [ $indexable ], $this->instance->find_by_multiple_ids_and_type( [ 7 ], 'user' ) ); + } + + /** + * Tests that nothing is built or primed when auto create is disabled. + * + * @covers ::find_by_multiple_ids_and_type + * + * @return void + */ + public function test_find_by_multiple_ids_and_type_without_auto_create() { + $indexable_one = Mockery::mock( Indexable_Mock::class ); + $indexable_one->object_id = 1; + + $this->mock_query_for_object_ids( [ 1, 2 ], 'post', [ $indexable_one ] ); + + Functions\expect( '_prime_post_caches' )->never(); + $this->builder->expects( 'build_for_id_and_type' )->never(); + + $this->mock_version_check( $indexable_one ); + + $this->assertSame( + [ $indexable_one ], + $this->instance->find_by_multiple_ids_and_type( [ 1, 2 ], 'post', false ), + ); + } + + /** + * Mocks the ORM query that retrieves indexables by their object ids and type. + * + * @param int[] $object_ids The object ids to expect in the query. + * @param string $object_type The object type to expect in the query. + * @param Indexable[] $indexables The indexables the query returns. + * + * @return void + */ + private function mock_query_for_object_ids( $object_ids, $object_type, $indexables ) { + $orm_object = Mockery::mock( ORM::class ); + + $orm_object + ->expects( 'where_in' ) + ->once() + ->with( 'object_id', $object_ids ) + ->andReturnSelf(); + + $orm_object + ->expects( 'where' ) + ->once() + ->with( 'object_type', $object_type ) + ->andReturnSelf(); + + $orm_object + ->expects( 'find_many' ) + ->once() + ->andReturn( $indexables ); + + $this->instance + ->expects( 'query' ) + ->andReturn( $orm_object ); + } + /** * Tests if the reset_permalink method fires when no type and subtype are passed. * diff --git a/tests/WP/Sitemaps/Post_Type_Sitemap_Provider_Test.php b/tests/WP/Sitemaps/Post_Type_Sitemap_Provider_Test.php index 8044e09ec4e..6956370584e 100644 --- a/tests/WP/Sitemaps/Post_Type_Sitemap_Provider_Test.php +++ b/tests/WP/Sitemaps/Post_Type_Sitemap_Provider_Test.php @@ -404,6 +404,141 @@ public function test_password_protected_post_parent_attachment() { $this->assertCount( 0, self::$class_instance->get_sitemap_links( 'attachment', 100, 0 ) ); } + /** + * Tests that generating sitemap links runs a constant number of queries, regardless of the post count. + * + * @covers ::get_sitemap_links + * + * @return void + */ + public function test_get_sitemap_links_query_count_does_not_grow_with_post_count() { + $original_structure = \get_option( 'permalink_structure' ); + + try { + // A %category% permalink structure makes get_permalink() look up each post's terms. + $this->set_permalink_structure( '/%category%/%postname%/' ); + + $this->create_posts_with_own_category( 3 ); + $queries_for_three_posts = $this->count_sitemap_links_queries(); + + $this->create_posts_with_own_category( 7 ); + $queries_for_ten_posts = $this->count_sitemap_links_queries(); + + $this->assertSame( + $queries_for_three_posts, + $queries_for_ten_posts, + 'Generating sitemap links should not run more queries when there are more posts', + ); + } + finally { + $this->set_permalink_structure( $original_structure ); + } + } + + /** + * Tests that the cache priming can be disabled through the wpseo_disable_xml_sitemap_cache_priming filter. + * + * @covers ::get_sitemap_links + * + * @return void + */ + public function test_cache_priming_can_be_disabled_via_filter() { + $original_structure = \get_option( 'permalink_structure' ); + + try { + // A %category% permalink structure makes get_permalink() look up each post's terms. + $this->set_permalink_structure( '/%category%/%postname%/' ); + + $this->create_posts_with_own_category( 3 ); + $queries_with_priming = $this->count_sitemap_links_queries(); + + \add_filter( 'wpseo_disable_xml_sitemap_cache_priming', '__return_true' ); + $queries_without_priming = $this->count_sitemap_links_queries(); + + $this->assertGreaterThan( + $queries_with_priming, + $queries_without_priming, + 'Disabling the cache priming through the filter should make per-post queries return', + ); + } + finally { + \remove_filter( 'wpseo_disable_xml_sitemap_cache_priming', '__return_true' ); + $this->set_permalink_structure( $original_structure ); + } + } + + /** + * Tests that generating sitemap links for posts with featured images runs a constant number + * of queries, regardless of the post count. + * + * @covers ::get_sitemap_links + * + * @return void + */ + public function test_get_sitemap_links_query_count_does_not_grow_with_featured_image_count() { + $this->create_posts_with_featured_images( 3 ); + $queries_for_three_posts = $this->count_sitemap_links_queries(); + + $this->create_posts_with_featured_images( 7 ); + $queries_for_ten_posts = $this->count_sitemap_links_queries(); + + $this->assertSame( + $queries_for_three_posts, + $queries_for_ten_posts, + 'Generating sitemap links should not run more queries when there are more posts with featured images', + ); + } + + /** + * Creates posts that each have their own featured image. + * + * @param int $count The number of posts to create. + * + * @return void + */ + private function create_posts_with_featured_images( $count ) { + $attachment_data = [ + 'post_mime_type' => 'image/jpeg', + 'post_title' => 'Test image', + 'post_content' => '', + 'post_status' => 'inherit', + ]; + + for ( $i = 0; $i < $count; $i++ ) { + $post_id = $this->factory->post->create(); + $thumbnail_id = \wp_insert_attachment( $attachment_data, 'featured.jpg', $post_id ); + \set_post_thumbnail( $post_id, $thumbnail_id ); + } + } + + /** + * Creates posts that each have their own category. + * + * @param int $count The number of posts to create. + * + * @return void + */ + private function create_posts_with_own_category( $count ) { + for ( $i = 0; $i < $count; $i++ ) { + $category_id = $this->factory->category->create(); + $this->factory->post->create( [ 'post_category' => [ $category_id ] ] ); + } + } + + /** + * Counts the number of database queries run to generate the post sitemap links, starting from a cold cache. + * + * @return int The number of queries. + */ + private function count_sitemap_links_queries() { + \wp_cache_flush(); + + $queries_before = \get_num_queries(); + self::$class_instance->get_sitemap_links( 'post', 100, 1 ); + + return ( \get_num_queries() - $queries_before ); + } + /** * Filter to exclude desired posts from the sitemap. *