From 283c38c022eb06d87d457d238301ce8f8907d6bf Mon Sep 17 00:00:00 2001 From: Boone Gorges Date: Wed, 30 Sep 2015 01:34:54 +0000 Subject: [PATCH] Improve lazyloading of comment meta in `WP_Query` loops. Lazy-loading logic is moved to a method on `WP_Query`. This makes it possible for comment feeds to take advantage of metadata lazyloading, in addition to comments loaded via `comments_template()`. This new technique parallels the termmeta lazyloading technique introduced in [34704]. Fixes #34047. git-svn-id: https://develop.svn.wordpress.org/trunk@34711 602fd350-edb4-49c9-b593-d223f7449a82 --- src/wp-includes/comment-functions.php | 24 ------ src/wp-includes/comment-template.php | 4 + src/wp-includes/default-filters.php | 1 - src/wp-includes/query.php | 58 +++++++++++++++ tests/phpunit/tests/comment/metaCache.php | 89 +++++++++++++++++++++++ 5 files changed, 151 insertions(+), 25 deletions(-) diff --git a/src/wp-includes/comment-functions.php b/src/wp-includes/comment-functions.php index 1380422d4a..453b5a8107 100644 --- a/src/wp-includes/comment-functions.php +++ b/src/wp-includes/comment-functions.php @@ -2452,30 +2452,6 @@ function _prime_comment_caches( $comment_ids, $update_meta_cache = true ) { } } -/** - * Lazy load comment meta when inside of a `WP_Query` loop. - * - * @since 4.4.0 - * - * @param null $check The `$check` param passed from the 'pre_comment_metadata' hook. - * @param int $comment_id ID of the comment whose metadata is being cached. - * @return null In order not to short-circuit `get_metadata()`. - */ -function wp_lazyload_comment_meta( $check, $comment_id ) { - global $wp_query; - - if ( ! empty( $wp_query->comments ) ) { - // Don't use `wp_list_pluck()` to avoid by-reference manipulation. - $comment_ids = array(); - foreach ( $wp_query->comments as $comment ) { - $comment_ids[] = $comment->comment_ID; - } - update_meta_cache( 'comment', $comment_ids ); - } - - return $check; -} - // // Internal // diff --git a/src/wp-includes/comment-template.php b/src/wp-includes/comment-template.php index 7b6e44c094..abb87f656b 100644 --- a/src/wp-includes/comment-template.php +++ b/src/wp-includes/comment-template.php @@ -1320,6 +1320,10 @@ function comments_template( $file = '/comments.php', $separate_comments = false * @param int $post_ID Post ID. */ $wp_query->comments = apply_filters( 'comments_array', $comments_flat, $post->ID ); + + // Set up lazy-loading for comment metadata. + add_action( 'get_comment_metadata', array( $wp_query, 'lazyload_comment_meta' ), 10, 2 ); + $comments = &$wp_query->comments; $wp_query->comment_count = count($wp_query->comments); $wp_query->max_num_comment_pages = $comment_query->max_num_pages; diff --git a/src/wp-includes/default-filters.php b/src/wp-includes/default-filters.php index e3e2ad0c12..30974c2cae 100644 --- a/src/wp-includes/default-filters.php +++ b/src/wp-includes/default-filters.php @@ -200,7 +200,6 @@ add_filter( 'nav_menu_meta_box_object', '_wp_nav_menu_meta_box_object' ); add_filter( 'pingback_ping_source_uri', 'pingback_ping_source_uri' ); add_filter( 'xmlrpc_pingback_error', 'xmlrpc_pingback_error' ); add_filter( 'title_save_pre', 'trim' ); -add_filter( 'get_comment_metadata', 'wp_lazyload_comment_meta', 10, 2 ); add_filter( 'http_request_host_is_external', 'allowed_http_request_hosts', 10, 2 ); diff --git a/src/wp-includes/query.php b/src/wp-includes/query.php index 3ec605b513..f76c994ca1 100644 --- a/src/wp-includes/query.php +++ b/src/wp-includes/query.php @@ -1311,6 +1311,15 @@ class WP_Query { */ public $updated_term_meta_cache = false; + /** + * Whether the comment meta cache for matched posts has been primed. + * + * @since 4.4.0 + * @access protected + * @var bool + */ + public $updated_comment_meta_cache = false; + /** * Cached list of search stopwords. * @@ -3682,6 +3691,11 @@ class WP_Query { } } + // If comments have been fetched as part of the query, make sure comment meta lazy-loading is set up. + if ( ! empty( $this->comments ) ) { + add_action( 'get_comment_metadata', array( $this, 'lazyload_comment_meta' ), 10, 2 ); + } + if ( ! $q['suppress_filters'] ) { /** * Filter the array of retrieved posts after they've been fetched and @@ -4816,6 +4830,50 @@ class WP_Query { return $check; } + + /** + * Lazy-load comment meta when inside of a `WP_Query` loop. + * + * This method is public so that it can be used as a filter callback. As a rule, there is no need to invoke it + * directly, from either inside or outside the `WP_Query` object. + * + * @since 4.4.0 + * + * @param null $check The `$check` param passed from the 'pre_comment_metadata' hook. + * @param int $comment_id ID of the comment whose metadata is being cached. + * @return null In order not to short-circuit `get_metadata()`. + */ + public function lazyload_comment_meta( $check, $comment_id ) { + /* + * We only do this once per `WP_Query` instance. + * Can't use `remove_action()` because of non-unique object hashes. + */ + if ( $this->updated_comment_meta_cache ) { + return $check; + } + + // Don't use `wp_list_pluck()` to avoid by-reference manipulation. + $comment_ids = array(); + if ( is_array( $this->comments ) ) { + foreach ( $this->comments as $comment ) { + $comment_ids[] = $comment->comment_ID; + } + } + + /* + * Only update the metadata cache for comments belonging to these posts if the comment_id passed + * to `get_comment_meta()` matches one of those comments. This prevents a single call to + * `get_comment_meta()` from priming metadata for all `WP_Query` objects. + */ + if ( in_array( $comment_id, $comment_ids ) ) { + update_meta_cache( 'comment', $comment_ids ); + $this->updated_comment_meta_cache = true; + } elseif ( empty( $comment_ids ) ) { + $this->updated_comment_meta_cache = true; + } + + return $check; + } } /** diff --git a/tests/phpunit/tests/comment/metaCache.php b/tests/phpunit/tests/comment/metaCache.php index edec8b8598..7da45b1c81 100644 --- a/tests/phpunit/tests/comment/metaCache.php +++ b/tests/phpunit/tests/comment/metaCache.php @@ -121,4 +121,93 @@ class Tests_Comment_Meta_Cache extends WP_UnitTestCase { } } } + + /** + * @group 34047 + */ + public function test_comment_meta_should_be_lazy_loaded_in_comment_feed_queries() { + global $wpdb; + + $posts = $this->factory->post->create_many( 2, array( 'post_status' => 'publish' ) ); + + $now = time(); + $comments = array(); + for ( $i = 0; $i < 5; $i++ ) { + $comments[] = $this->factory->comment->create( array( + 'comment_post_ID' => $posts[0], + 'comment_date_gmt' => date( 'Y-m-d H:i:s', $now - ( 60 * $i ) ), + ) ); + } + + foreach ( $comments as $c ) { + add_comment_meta( $c, 'foo', 'bar' ); + } + + update_option( 'posts_per_rss', 3 ); + + $q = new WP_Query( array( + 'feed' => true, + 'withcomments' => true, + ) ); + + // First comment will cause the cache to be primed. + $num_queries = $wpdb->num_queries; + $this->assertSame( 'bar', get_comment_meta( $comments[0], 'foo', 'bar' ) ); + $num_queries++; + $this->assertSame( $num_queries, $wpdb->num_queries ); + + // Second comment from the results should not cause more queries. + $this->assertSame( 'bar', get_comment_meta( $comments[1], 'foo', 'bar' ) ); + $this->assertSame( $num_queries, $wpdb->num_queries ); + + // A comment from outside the results will not be primed. + $this->assertSame( 'bar', get_comment_meta( $comments[4], 'foo', 'bar' ) ); + $num_queries++; + $this->assertSame( $num_queries, $wpdb->num_queries ); + } + + /** + * @group 34047 + */ + public function test_comment_meta_should_be_lazy_loaded_in_single_post_comment_feed_queries() { + global $wpdb; + + $posts = $this->factory->post->create_many( 2, array( 'post_status' => 'publish' ) ); + + $now = time(); + $comments = array(); + for ( $i = 0; $i < 5; $i++ ) { + $comments[] = $this->factory->comment->create( array( + 'comment_post_ID' => $posts[0], + 'comment_date_gmt' => date( 'Y-m-d H:i:s', $now - ( 60 * $i ) ), + ) ); + } + + foreach ( $comments as $c ) { + add_comment_meta( $c, 'foo', 'bar' ); + } + + update_option( 'posts_per_rss', 3 ); + + $q = new WP_Query( array( + 'feed' => true, + 'withcomments' => true, + 'p' => $posts[0], + ) ); + + // First comment will cause the cache to be primed. + $num_queries = $wpdb->num_queries; + $this->assertSame( 'bar', get_comment_meta( $comments[0], 'foo', 'bar' ) ); + $num_queries++; + $this->assertSame( $num_queries, $wpdb->num_queries ); + + // Second comment from the results should not cause more queries. + $this->assertSame( 'bar', get_comment_meta( $comments[1], 'foo', 'bar' ) ); + $this->assertSame( $num_queries, $wpdb->num_queries ); + + // A comment from outside the results will not be primed. + $this->assertSame( 'bar', get_comment_meta( $comments[4], 'foo', 'bar' ) ); + $num_queries++; + $this->assertSame( $num_queries, $wpdb->num_queries ); + } }