From ea31ac69498105e516c32f120dd88c6531ed7709 Mon Sep 17 00:00:00 2001 From: Jonny Harris Date: Wed, 12 Oct 2022 11:23:48 +0100 Subject: [PATCH 1/7] Remove placeholders. --- src/wp-includes/class-wp-query.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/wp-includes/class-wp-query.php b/src/wp-includes/class-wp-query.php index 1f6f926b1a05e..7f92941df35ee 100644 --- a/src/wp-includes/class-wp-query.php +++ b/src/wp-includes/class-wp-query.php @@ -3092,6 +3092,7 @@ public function get_posts() { ); $new_request = str_replace( $fields, "{$wpdb->posts}.*", $this->request ); + $new_request = str_replace( $wpdb->placeholder_escape(), "", $new_request ); $key = md5( serialize( $cache_args ) . $new_request ); $last_changed = wp_cache_get_last_changed( 'posts' ); From f371e0ea3007e7fbccca12d38a76b5fa05e13da5 Mon Sep 17 00:00:00 2001 From: Jonny Harris Date: Wed, 12 Oct 2022 11:47:39 +0100 Subject: [PATCH 2/7] Add unit tests. --- src/wp-includes/class-wp-query.php | 2 +- tests/phpunit/tests/query/cacheResults.php | 84 ++++++++++++++++++++++ 2 files changed, 85 insertions(+), 1 deletion(-) diff --git a/src/wp-includes/class-wp-query.php b/src/wp-includes/class-wp-query.php index 7f92941df35ee..27e8d8ae03404 100644 --- a/src/wp-includes/class-wp-query.php +++ b/src/wp-includes/class-wp-query.php @@ -3092,7 +3092,7 @@ public function get_posts() { ); $new_request = str_replace( $fields, "{$wpdb->posts}.*", $this->request ); - $new_request = str_replace( $wpdb->placeholder_escape(), "", $new_request ); + $new_request = $wpdb->remove_placeholder_escape( $new_request ); $key = md5( serialize( $cache_args ) . $new_request ); $last_changed = wp_cache_get_last_changed( 'posts' ); diff --git a/tests/phpunit/tests/query/cacheResults.php b/tests/phpunit/tests/query/cacheResults.php index 723c78a2bf0d3..ea72f220419a2 100644 --- a/tests/phpunit/tests/query/cacheResults.php +++ b/tests/phpunit/tests/query/cacheResults.php @@ -191,6 +191,18 @@ public function data_query_cache() { ), ), ), + 'cache meta query search' => array( + 'args' => array( + 'cache_results' => true, + 'meta_query' => array( + array( + 'key' => 'color', + 'value' => '00', + 'compare' => 'LIKE', + ), + ), + ), + ), 'cache comment_count' => array( 'args' => array( 'cache_results' => true, @@ -209,6 +221,12 @@ public function data_query_cache() { ), ), ), + 'cache search query' => array( + 'args' => array( + 'cache_results' => true, + 's' => 'e', + ), + ), ); } @@ -827,6 +845,72 @@ public function test_query_cache_delete_meta() { $this->assertNotSame( $query1->found_posts, $query2->found_posts ); } + + /** + * @ticket 56802 + */ + public function test_query_cache_like_meta() { + static $placeholder; + $old_placeholder = $placeholder; + $p1 = self::$posts[0]; + + $args = array( + 'cache_results' => true, + 'fields' => 'ids', + 'meta_query' => array( + array( + 'key' => 'color', + 'value' => '00', + 'compare' => 'LIKE', + ), + ), + ); + $query1 = new WP_Query(); + $posts1 = $query1->query( $args ); + + // Force placeholder to be different. + $placeholder = rand(); + + $query2 = new WP_Query(); + $posts2 = $query2->query( $args ); + + $placeholder = $old_placeholder; + + $this->assertSame( $posts1, $posts2 ); + $this->assertContains( $p1, $posts2 ); + $this->assertSame( $query1->found_posts, $query2->found_posts ); + } + + /** + * @ticket 56802 + */ + public function test_query_cache_search() { + static $placeholder; + $old_placeholder = $placeholder; + // Post 0 already has the category foo. + $p1 = self::$posts[1]; + + $args = array( + 'cache_results' => true, + 'fields' => 'ids', + 's' => 'e', + ); + $query1 = new WP_Query(); + $posts1 = $query1->query( $args ); + + // Force placeholder to be different. + $placeholder = rand(); + + $query2 = new WP_Query(); + $posts2 = $query2->query( $args ); + + $placeholder = $old_placeholder; + + $this->assertSame( $posts1, $posts2 ); + $this->assertContains( $p1, $posts2 ); + $this->assertSame( $query1->found_posts, $query2->found_posts ); + } + /** * @ticket 22176 */ From 5e4d8ef479c08ea222ace4dbaa80ea0f6ff196bc Mon Sep 17 00:00:00 2001 From: Jonny Harris Date: Thu, 13 Oct 2022 17:47:57 +0100 Subject: [PATCH 3/7] Fix unit tests. --- src/wp-includes/class-wp-query.php | 3 +- src/wp-includes/class-wpdb.php | 7 +++-- tests/phpunit/tests/query/cacheResults.php | 32 ++++++++++------------ 3 files changed, 22 insertions(+), 20 deletions(-) diff --git a/src/wp-includes/class-wp-query.php b/src/wp-includes/class-wp-query.php index 27e8d8ae03404..dc596cb79a818 100644 --- a/src/wp-includes/class-wp-query.php +++ b/src/wp-includes/class-wp-query.php @@ -3088,7 +3088,8 @@ public function get_posts() { $cache_args['update_post_meta_cache'], $cache_args['update_post_term_cache'], $cache_args['lazy_load_term_meta'], - $cache_args['update_menu_item_cache'] + $cache_args['update_menu_item_cache'], + $cache_args['search_orderby_title'] ); $new_request = str_replace( $fields, "{$wpdb->posts}.*", $this->request ); diff --git a/src/wp-includes/class-wpdb.php b/src/wp-includes/class-wpdb.php index 88317f535e918..ee4a1f7bab2c4 100644 --- a/src/wp-includes/class-wpdb.php +++ b/src/wp-includes/class-wpdb.php @@ -2414,11 +2414,14 @@ public function log_query( $query, $query_time, $query_callstack, $query_start, * * @since 4.8.3 * + * @param bool $reset Optional. Whether to reset the placeholder. Default false. * @return string String to escape placeholders. */ - public function placeholder_escape() { + public function placeholder_escape( $reset = false ) { static $placeholder; - + if ( $reset ) { + $placeholder = null; + } if ( ! $placeholder ) { // If ext/hash is not present, compat.php's hash_hmac() does not support sha256. $algo = function_exists( 'hash' ) ? 'sha256' : 'sha1'; diff --git a/tests/phpunit/tests/query/cacheResults.php b/tests/phpunit/tests/query/cacheResults.php index ea72f220419a2..73fa58adad81d 100644 --- a/tests/phpunit/tests/query/cacheResults.php +++ b/tests/phpunit/tests/query/cacheResults.php @@ -850,11 +850,10 @@ public function test_query_cache_delete_meta() { * @ticket 56802 */ public function test_query_cache_like_meta() { - static $placeholder; - $old_placeholder = $placeholder; - $p1 = self::$posts[0]; + global $wpdb; + $p1 = self::$posts[0]; - $args = array( + $args = array( 'cache_results' => true, 'fields' => 'ids', 'meta_query' => array( @@ -865,18 +864,18 @@ public function test_query_cache_like_meta() { ), ), ); - $query1 = new WP_Query(); - $posts1 = $query1->query( $args ); + $query1 = new WP_Query(); + $posts1 = $query1->query( $args ); + $num_queries = get_num_queries(); // Force placeholder to be different. - $placeholder = rand(); + $wpdb->placeholder_escape( true ); $query2 = new WP_Query(); $posts2 = $query2->query( $args ); - $placeholder = $old_placeholder; - $this->assertSame( $posts1, $posts2 ); + $this->assertSame( $num_queries, get_num_queries() ); $this->assertContains( $p1, $posts2 ); $this->assertSame( $query1->found_posts, $query2->found_posts ); } @@ -885,28 +884,27 @@ public function test_query_cache_like_meta() { * @ticket 56802 */ public function test_query_cache_search() { - static $placeholder; - $old_placeholder = $placeholder; + global $wpdb; // Post 0 already has the category foo. $p1 = self::$posts[1]; - $args = array( + $args = array( 'cache_results' => true, 'fields' => 'ids', 's' => 'e', ); - $query1 = new WP_Query(); - $posts1 = $query1->query( $args ); + $query1 = new WP_Query(); + $posts1 = $query1->query( $args ); + $num_queries = get_num_queries(); // Force placeholder to be different. - $placeholder = rand(); + $wpdb->placeholder_escape( true ); $query2 = new WP_Query(); $posts2 = $query2->query( $args ); - $placeholder = $old_placeholder; - $this->assertSame( $posts1, $posts2 ); + $this->assertSame( $num_queries, get_num_queries() ); $this->assertContains( $p1, $posts2 ); $this->assertSame( $query1->found_posts, $query2->found_posts ); } From aec3d1158921ee956aae373693487b5ff592435e Mon Sep 17 00:00:00 2001 From: Jonny Harris Date: Thu, 13 Oct 2022 23:30:29 +0100 Subject: [PATCH 4/7] Improve comments. --- src/wp-includes/class-wpdb.php | 2 ++ tests/phpunit/tests/query/cacheResults.php | 16 ++++++++-------- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/wp-includes/class-wpdb.php b/src/wp-includes/class-wpdb.php index ee4a1f7bab2c4..c021d606be7e0 100644 --- a/src/wp-includes/class-wpdb.php +++ b/src/wp-includes/class-wpdb.php @@ -2413,8 +2413,10 @@ public function log_query( $query, $query_time, $query_callstack, $query_start, * Generates and returns a placeholder escape string for use in queries returned by ::prepare(). * * @since 4.8.3 + * @since 6.1.0 Added `$reset` parameter. * * @param bool $reset Optional. Whether to reset the placeholder. Default false. + * * @return string String to escape placeholders. */ public function placeholder_escape( $reset = false ) { diff --git a/tests/phpunit/tests/query/cacheResults.php b/tests/phpunit/tests/query/cacheResults.php index 73fa58adad81d..d1c317ed32b23 100644 --- a/tests/phpunit/tests/query/cacheResults.php +++ b/tests/phpunit/tests/query/cacheResults.php @@ -874,10 +874,10 @@ public function test_query_cache_like_meta() { $query2 = new WP_Query(); $posts2 = $query2->query( $args ); - $this->assertSame( $posts1, $posts2 ); - $this->assertSame( $num_queries, get_num_queries() ); - $this->assertContains( $p1, $posts2 ); - $this->assertSame( $query1->found_posts, $query2->found_posts ); + $this->assertSame( $posts1, $posts2, 'Results of first and second query should be the same' ); + $this->assertSame( $num_queries, get_num_queries(), 'Should not result in another database query.' ); + $this->assertContains( $p1, $posts2, 'Result should contain post 0.' ); + $this->assertSame( $query1->found_posts, $query2->found_posts, 'Found posts should match on both queries' ); } /** @@ -903,10 +903,10 @@ public function test_query_cache_search() { $query2 = new WP_Query(); $posts2 = $query2->query( $args ); - $this->assertSame( $posts1, $posts2 ); - $this->assertSame( $num_queries, get_num_queries() ); - $this->assertContains( $p1, $posts2 ); - $this->assertSame( $query1->found_posts, $query2->found_posts ); + $this->assertSame( $posts1, $posts2, 'Results of first and second query should be the same' ); + $this->assertSame( $num_queries, get_num_queries(), 'Should not result in another database query.' ); + $this->assertContains( $p1, $posts2, 'Result should contain post 0.' ); + $this->assertSame( $query1->found_posts, $query2->found_posts, 'Found posts should match on both queries' ); } /** From 917c0e9b4a374a6338c4e007cbaa9f4e6af1eb55 Mon Sep 17 00:00:00 2001 From: Jonny Harris Date: Fri, 14 Oct 2022 00:11:20 +0100 Subject: [PATCH 5/7] Why another solution. --- src/wp-includes/class-wp-query.php | 15 +++++- src/wp-includes/class-wpdb.php | 9 +--- tests/phpunit/tests/query/cacheResults.php | 55 +++++++++++----------- 3 files changed, 43 insertions(+), 36 deletions(-) diff --git a/src/wp-includes/class-wp-query.php b/src/wp-includes/class-wp-query.php index dc596cb79a818..c8c3cdaa0fc3c 100644 --- a/src/wp-includes/class-wp-query.php +++ b/src/wp-includes/class-wp-query.php @@ -3101,7 +3101,20 @@ public function get_posts() { $last_changed .= wp_cache_get_last_changed( 'terms' ); } - $cache_key = "wp_query:$key:$last_changed"; + $cache_key = "wp_query:$key:$last_changed"; + + /** + * Filters cache key. + * + * @since 6.1.0 + * + * @param string $cache_key Cache key. + * @param array $cache_args Query args used to generate the cache key. + * @param string $new_request SQL Query. + * @param WP_Query $query The WP_Query instance. + */ + $cache_key = apply_filters( 'wp_query_cache_key', $cache_key, $cache_args, $new_request, $this ); + $cache_found = false; if ( null === $this->posts ) { $cached_results = wp_cache_get( $cache_key, 'posts', false, $cache_found ); diff --git a/src/wp-includes/class-wpdb.php b/src/wp-includes/class-wpdb.php index c021d606be7e0..88317f535e918 100644 --- a/src/wp-includes/class-wpdb.php +++ b/src/wp-includes/class-wpdb.php @@ -2413,17 +2413,12 @@ public function log_query( $query, $query_time, $query_callstack, $query_start, * Generates and returns a placeholder escape string for use in queries returned by ::prepare(). * * @since 4.8.3 - * @since 6.1.0 Added `$reset` parameter. - * - * @param bool $reset Optional. Whether to reset the placeholder. Default false. * * @return string String to escape placeholders. */ - public function placeholder_escape( $reset = false ) { + public function placeholder_escape() { static $placeholder; - if ( $reset ) { - $placeholder = null; - } + if ( ! $placeholder ) { // If ext/hash is not present, compat.php's hash_hmac() does not support sha256. $algo = function_exists( 'hash' ) ? 'sha256' : 'sha1'; diff --git a/tests/phpunit/tests/query/cacheResults.php b/tests/phpunit/tests/query/cacheResults.php index d1c317ed32b23..999bfada307ac 100644 --- a/tests/phpunit/tests/query/cacheResults.php +++ b/tests/phpunit/tests/query/cacheResults.php @@ -33,6 +33,11 @@ class Test_Query_CacheResults extends WP_UnitTestCase { */ public static $author_id; + /** + * @var string + */ + protected $new_request; + public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ) { // Make some post objects. self::$posts = $factory->post->create_many( 5 ); @@ -57,6 +62,11 @@ public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ) { ); } + function set_up() { + parent::set_up(); + $this->new_request = null; + } + /** * @dataProvider data_query_cache * @ticket 22176 @@ -851,9 +861,8 @@ public function test_query_cache_delete_meta() { */ public function test_query_cache_like_meta() { global $wpdb; - $p1 = self::$posts[0]; - $args = array( + $args = array( 'cache_results' => true, 'fields' => 'ids', 'meta_query' => array( @@ -864,20 +873,13 @@ public function test_query_cache_like_meta() { ), ), ); - $query1 = new WP_Query(); - $posts1 = $query1->query( $args ); - $num_queries = get_num_queries(); - - // Force placeholder to be different. - $wpdb->placeholder_escape( true ); - $query2 = new WP_Query(); - $posts2 = $query2->query( $args ); + add_filter( 'wp_query_cache_key', array( $this, 'filter_wp_query_cache_key' ), 15, 3 ); + $query1 = new WP_Query(); + $query1->query( $args ); - $this->assertSame( $posts1, $posts2, 'Results of first and second query should be the same' ); - $this->assertSame( $num_queries, get_num_queries(), 'Should not result in another database query.' ); - $this->assertContains( $p1, $posts2, 'Result should contain post 0.' ); - $this->assertSame( $query1->found_posts, $query2->found_posts, 'Found posts should match on both queries' ); + $this->assertNotEmpty( $this->new_request, 'Check new request is not empty' ); + $this->assertStringNotContainsString( $wpdb->placeholder_escape(), $this->new_request, 'Check if request does not contain placeholder' ); } /** @@ -885,28 +887,25 @@ public function test_query_cache_like_meta() { */ public function test_query_cache_search() { global $wpdb; - // Post 0 already has the category foo. - $p1 = self::$posts[1]; - $args = array( + $args = array( 'cache_results' => true, 'fields' => 'ids', 's' => 'e', ); - $query1 = new WP_Query(); - $posts1 = $query1->query( $args ); - $num_queries = get_num_queries(); - // Force placeholder to be different. - $wpdb->placeholder_escape( true ); + add_filter( 'wp_query_cache_key', array( $this, 'filter_wp_query_cache_key' ), 15, 3 ); + $query1 = new WP_Query(); + $query1->query( $args ); - $query2 = new WP_Query(); - $posts2 = $query2->query( $args ); + $this->assertNotEmpty( $this->new_request, 'Check new request is not empty' ); + $this->assertStringNotContainsString( $wpdb->placeholder_escape(), $this->new_request, 'Check if request does not contain placeholder' ); + } + + public function filter_wp_query_cache_key( $cache_key, $cache_args, $new_request ) { + $this->new_request = $new_request; - $this->assertSame( $posts1, $posts2, 'Results of first and second query should be the same' ); - $this->assertSame( $num_queries, get_num_queries(), 'Should not result in another database query.' ); - $this->assertContains( $p1, $posts2, 'Result should contain post 0.' ); - $this->assertSame( $query1->found_posts, $query2->found_posts, 'Found posts should match on both queries' ); + return $cache_key; } /** From 8f6757f32313318cfa1dd034618749d02e1d7980 Mon Sep 17 00:00:00 2001 From: Jonny Harris Date: Fri, 14 Oct 2022 22:32:31 +0100 Subject: [PATCH 6/7] Fix alignment. --- src/wp-includes/class-wp-query.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/wp-includes/class-wp-query.php b/src/wp-includes/class-wp-query.php index c8c3cdaa0fc3c..fc9548435bb64 100644 --- a/src/wp-includes/class-wp-query.php +++ b/src/wp-includes/class-wp-query.php @@ -3104,14 +3104,14 @@ public function get_posts() { $cache_key = "wp_query:$key:$last_changed"; /** - * Filters cache key. + * Filters query cache key. * * @since 6.1.0 * - * @param string $cache_key Cache key. - * @param array $cache_args Query args used to generate the cache key. - * @param string $new_request SQL Query. - * @param WP_Query $query The WP_Query instance. + * @param string $cache_key Cache key. + * @param array $cache_args Query args used to generate the cache key. + * @param string $new_request SQL Query. + * @param WP_Query $query The WP_Query instance. */ $cache_key = apply_filters( 'wp_query_cache_key', $cache_key, $cache_args, $new_request, $this ); From 2278729e564fae805dc00bb953f1669541b52836 Mon Sep 17 00:00:00 2001 From: Jonny Harris Date: Fri, 14 Oct 2022 22:33:13 +0100 Subject: [PATCH 7/7] Improve unit tests. --- tests/phpunit/tests/query/cacheResults.php | 82 +++++++++------------- 1 file changed, 34 insertions(+), 48 deletions(-) diff --git a/tests/phpunit/tests/query/cacheResults.php b/tests/phpunit/tests/query/cacheResults.php index 999bfada307ac..43c8fc695ab04 100644 --- a/tests/phpunit/tests/query/cacheResults.php +++ b/tests/phpunit/tests/query/cacheResults.php @@ -33,6 +33,11 @@ class Test_Query_CacheResults extends WP_UnitTestCase { */ public static $author_id; + /** + * @var array + */ + protected $cache_args; + /** * @var string */ @@ -64,7 +69,9 @@ public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ) { function set_up() { parent::set_up(); + $this->cache_args = null; $this->new_request = null; + add_filter( 'wp_query_cache_key', array( $this, 'filter_wp_query_cache_key' ), 15, 3 ); } /** @@ -72,9 +79,16 @@ function set_up() { * @ticket 22176 */ public function test_query_cache( $args ) { + global $wpdb; + $query1 = new WP_Query(); $posts1 = $query1->query( $args ); + $placeholder = $wpdb->placeholder_escape(); + $this->assertNotEmpty( $this->new_request, 'Check new request is not empty' ); + $this->assertStringNotContainsString( $placeholder, $this->new_request, 'Check if request does not contain placeholder' ); + $this->assertStringNotContainsString( $placeholder, wp_json_encode( $this->cache_args ), 'Check if cache arrays does not contain placeholder' ); + $queries_before = get_num_queries(); $query2 = new WP_Query(); $posts2 = $query2->query( $args ); @@ -213,6 +227,18 @@ public function data_query_cache() { ), ), ), + 'cache meta query not search' => array( + 'args' => array( + 'cache_results' => true, + 'meta_query' => array( + array( + 'key' => 'color', + 'value' => 'ff', + 'compare' => 'NOT LIKE', + ), + ), + ), + ), 'cache comment_count' => array( 'args' => array( 'cache_results' => true, @@ -234,7 +260,13 @@ public function data_query_cache() { 'cache search query' => array( 'args' => array( 'cache_results' => true, - 's' => 'e', + 's' => 'title', + ), + ), + 'cache search query multiple terms' => array( + 'args' => array( + 'cache_results' => true, + 's' => 'Post title', ), ), ); @@ -855,54 +887,8 @@ public function test_query_cache_delete_meta() { $this->assertNotSame( $query1->found_posts, $query2->found_posts ); } - - /** - * @ticket 56802 - */ - public function test_query_cache_like_meta() { - global $wpdb; - - $args = array( - 'cache_results' => true, - 'fields' => 'ids', - 'meta_query' => array( - array( - 'key' => 'color', - 'value' => '00', - 'compare' => 'LIKE', - ), - ), - ); - - add_filter( 'wp_query_cache_key', array( $this, 'filter_wp_query_cache_key' ), 15, 3 ); - $query1 = new WP_Query(); - $query1->query( $args ); - - $this->assertNotEmpty( $this->new_request, 'Check new request is not empty' ); - $this->assertStringNotContainsString( $wpdb->placeholder_escape(), $this->new_request, 'Check if request does not contain placeholder' ); - } - - /** - * @ticket 56802 - */ - public function test_query_cache_search() { - global $wpdb; - - $args = array( - 'cache_results' => true, - 'fields' => 'ids', - 's' => 'e', - ); - - add_filter( 'wp_query_cache_key', array( $this, 'filter_wp_query_cache_key' ), 15, 3 ); - $query1 = new WP_Query(); - $query1->query( $args ); - - $this->assertNotEmpty( $this->new_request, 'Check new request is not empty' ); - $this->assertStringNotContainsString( $wpdb->placeholder_escape(), $this->new_request, 'Check if request does not contain placeholder' ); - } - public function filter_wp_query_cache_key( $cache_key, $cache_args, $new_request ) { + $this->cache_args = $cache_args; $this->new_request = $new_request; return $cache_key;