From 4a9f5fe3be47914b3f2b30aaf863761335cf1ad2 Mon Sep 17 00:00:00 2001 From: Jonny Harris Date: Thu, 10 Mar 2022 10:56:09 +0000 Subject: [PATCH] Taxonomy: Only store term_ids and object_ids in `WP_Term_Query` query caches. The query cache currently implemented in `WP_Term_Query` caches the final output of the query, depending on what fields are requested. This is wasteful, as if a user requests `fields` => `all`, then an unlimited array of `WP_Term` objects could be stored in the object cache. Instead of storing the whole WP_Term object, this change only the term_id is stored. To get an array the full WP_Term objects, the `_prime_term_caches` function is called with an array of ids. In instances where a persistent object cache is not in use, then this will result in another SQL query to be run. After `_prime_term_caches` is called if this term is requested again in the same page load, then it will already be loaded into memory. If a user runs `WP_Term_Query` with the fields param set to `all_with_object_id`, an array of objects containing both the term_id and object_ids are stored in cache. This change also improves the logic to load term meta caches. This change ensures that term meta is always primed for all terms loaded in the term query. Props Spacedmonkey, boonebgorges, jbpaul17, peterwilsoncc, flixos90, pbearne. Fixes #37189. git-svn-id: https://develop.svn.wordpress.org/trunk@52836 602fd350-edb4-49c9-b593-d223f7449a82 --- src/wp-includes/class-wp-term-query.php | 195 ++++++++++---------- tests/phpunit/tests/term/cache.php | 26 +-- tests/phpunit/tests/term/getTermBy.php | 17 +- tests/phpunit/tests/term/getTerms.php | 2 +- tests/phpunit/tests/term/isObjectInTerm.php | 6 +- 5 files changed, 131 insertions(+), 115 deletions(-) diff --git a/src/wp-includes/class-wp-term-query.php b/src/wp-includes/class-wp-term-query.php index 1583ed6ab6..81a5fcfa89 100644 --- a/src/wp-includes/class-wp-term-query.php +++ b/src/wp-includes/class-wp-term-query.php @@ -636,32 +636,16 @@ class WP_Term_Query { $selects = array(); switch ( $args['fields'] ) { - case 'all': - case 'all_with_object_id': - case 'tt_ids': - case 'slugs': - $selects = array( 't.*', 'tt.*' ); - if ( 'all_with_object_id' === $args['fields'] && ! empty( $args['object_ids'] ) ) { - $selects[] = 'tr.object_id'; - } - break; - case 'ids': - case 'id=>parent': - $selects = array( 't.term_id', 'tt.parent', 'tt.count', 'tt.taxonomy' ); - break; - case 'names': - $selects = array( 't.term_id', 'tt.parent', 'tt.count', 't.name', 'tt.taxonomy' ); - break; case 'count': $orderby = ''; $order = ''; $selects = array( 'COUNT(*)' ); break; - case 'id=>name': - $selects = array( 't.term_id', 't.name', 'tt.parent', 'tt.count', 'tt.taxonomy' ); - break; - case 'id=>slug': - $selects = array( 't.term_id', 't.slug', 'tt.parent', 'tt.count', 'tt.taxonomy' ); + default: + $selects = array( 't.term_id' ); + if ( 'all_with_object_id' === $args['fields'] && ! empty( $args['object_ids'] ) ) { + $selects[] = 'tr.object_id'; + } break; } @@ -688,7 +672,8 @@ class WP_Term_Query { $join .= " INNER JOIN $wpdb->term_taxonomy AS tt ON t.term_id = tt.term_id"; if ( ! empty( $this->query_vars['object_ids'] ) ) { - $join .= " INNER JOIN {$wpdb->term_relationships} AS tr ON tr.term_taxonomy_id = tt.term_taxonomy_id"; + $join .= " INNER JOIN {$wpdb->term_relationships} AS tr ON tr.term_taxonomy_id = tt.term_taxonomy_id"; + $distinct = 'DISTINCT'; } $where = implode( ' AND ', $this->sql_clauses['where'] ); @@ -748,8 +733,14 @@ class WP_Term_Query { $cache_key = "get_terms:$key:$last_changed"; $cache = wp_cache_get( $cache_key, 'terms' ); if ( false !== $cache ) { - if ( 'all' === $_fields || 'all_with_object_id' === $_fields ) { - $cache = $this->populate_terms( $cache ); + if ( 'ids' === $_fields ) { + $term_ids = wp_list_pluck( $cache, 'term_id' ); + $cache = array_map( 'intval', $term_ids ); + } elseif ( 'count' !== $_fields ) { + $term_ids = wp_list_pluck( $cache, 'term_id' ); + _prime_term_caches( $term_ids, $args['update_term_meta_cache'] ); + $term_objects = $this->populate_terms( $cache ); + $cache = $this->format_terms( $term_objects, $_fields ); } $this->terms = $cache; @@ -757,33 +748,27 @@ class WP_Term_Query { } if ( 'count' === $_fields ) { - $count = $wpdb->get_var( $this->request ); + $count = $wpdb->get_var( $this->request ); // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared wp_cache_set( $cache_key, $count, 'terms' ); return $count; } - $terms = $wpdb->get_results( $this->request ); - - if ( 'all' === $_fields || 'all_with_object_id' === $_fields ) { - update_term_cache( $terms ); - } - - // Prime termmeta cache. - if ( $args['update_term_meta_cache'] ) { - $term_ids = wp_list_pluck( $terms, 'term_id' ); - update_termmeta_cache( $term_ids ); - } + $terms = $wpdb->get_results( $this->request ); // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared if ( empty( $terms ) ) { wp_cache_add( $cache_key, array(), 'terms' ); return array(); } + $term_ids = wp_list_pluck( $terms, 'term_id' ); + _prime_term_caches( $term_ids, false ); + $term_objects = $this->populate_terms( $terms ); + if ( $child_of ) { foreach ( $taxonomies as $_tax ) { $children = _get_term_hierarchy( $_tax ); if ( ! empty( $children ) ) { - $terms = _get_term_children( $child_of, $terms, $_tax ); + $term_objects = _get_term_children( $child_of, $term_objects, $_tax ); } } } @@ -791,13 +776,13 @@ class WP_Term_Query { // Update term counts to include children. if ( $args['pad_counts'] && 'all' === $_fields ) { foreach ( $taxonomies as $_tax ) { - _pad_term_counts( $terms, $_tax ); + _pad_term_counts( $term_objects, $_tax ); } } // Make sure we show empty categories that have children. - if ( $hierarchical && $args['hide_empty'] && is_array( $terms ) ) { - foreach ( $terms as $k => $term ) { + if ( $hierarchical && $args['hide_empty'] && is_array( $term_objects ) ) { + foreach ( $term_objects as $k => $term ) { if ( ! $term->count ) { $children = get_term_children( $term->term_id, $term->taxonomy ); if ( is_array( $children ) ) { @@ -810,7 +795,7 @@ class WP_Term_Query { } // It really is empty. - unset( $terms[ $k ] ); + unset( $term_objects[ $k ] ); } } } @@ -836,56 +821,26 @@ class WP_Term_Query { $terms = $_terms; } - $_terms = array(); - if ( 'id=>parent' === $_fields ) { - foreach ( $terms as $term ) { - $_terms[ $term->term_id ] = $term->parent; - } - } elseif ( 'ids' === $_fields ) { - foreach ( $terms as $term ) { - $_terms[] = (int) $term->term_id; - } - } elseif ( 'tt_ids' === $_fields ) { - foreach ( $terms as $term ) { - $_terms[] = (int) $term->term_taxonomy_id; - } - } elseif ( 'names' === $_fields ) { - foreach ( $terms as $term ) { - $_terms[] = $term->name; - } - } elseif ( 'slugs' === $_fields ) { - foreach ( $terms as $term ) { - $_terms[] = $term->slug; - } - } elseif ( 'id=>name' === $_fields ) { - foreach ( $terms as $term ) { - $_terms[ $term->term_id ] = $term->name; - } - } elseif ( 'id=>slug' === $_fields ) { - foreach ( $terms as $term ) { - $_terms[ $term->term_id ] = $term->slug; - } - } - - if ( ! empty( $_terms ) ) { - $terms = $_terms; - } - // Hierarchical queries are not limited, so 'offset' and 'number' must be handled now. if ( $hierarchical && $number && is_array( $terms ) ) { if ( $offset >= count( $terms ) ) { - $terms = array(); + $terms = array(); + $term_objects = array(); } else { - $terms = array_slice( $terms, $offset, $number, true ); + $terms = array_slice( $terms, $offset, $number, true ); + $term_objects = array_slice( $term_objects, $offset, $number, true ); } } - wp_cache_add( $cache_key, $terms, 'terms' ); - - if ( 'all' === $_fields || 'all_with_object_id' === $_fields ) { - $terms = $this->populate_terms( $terms ); + // Prime termmeta cache. + if ( $args['update_term_meta_cache'] ) { + $term_ids = wp_list_pluck( $term_objects, 'term_id' ); + update_termmeta_cache( $term_ids ); } + wp_cache_add( $cache_key, $terms, 'terms' ); + $terms = $this->format_terms( $term_objects, $_fields ); + $this->terms = $terms; return $this->terms; } @@ -949,6 +904,53 @@ class WP_Term_Query { return $orderby; } + /** + * Format response depending on field requested. + * + * @since 6.0.0 + * + * @param WP_Term[] $term_objects Array of term objects. + * @param string $_fields Field to format. + * + * @return WP_Term[]|int[]|string[] Array of terms / strings / ints depending on field requested. + */ + protected function format_terms( $term_objects, $_fields ) { + $_terms = array(); + if ( 'id=>parent' === $_fields ) { + foreach ( $term_objects as $term ) { + $_terms[ $term->term_id ] = $term->parent; + } + } elseif ( 'ids' === $_fields ) { + foreach ( $term_objects as $term ) { + $_terms[] = (int) $term->term_id; + } + } elseif ( 'tt_ids' === $_fields ) { + foreach ( $term_objects as $term ) { + $_terms[] = (int) $term->term_taxonomy_id; + } + } elseif ( 'names' === $_fields ) { + foreach ( $term_objects as $term ) { + $_terms[] = $term->name; + } + } elseif ( 'slugs' === $_fields ) { + foreach ( $term_objects as $term ) { + $_terms[] = $term->slug; + } + } elseif ( 'id=>name' === $_fields ) { + foreach ( $term_objects as $term ) { + $_terms[ $term->term_id ] = $term->name; + } + } elseif ( 'id=>slug' === $_fields ) { + foreach ( $term_objects as $term ) { + $_terms[ $term->term_id ] = $term->slug; + } + } elseif ( 'all' === $_fields || 'all_with_object_id' === $_fields ) { + $_terms = $term_objects; + } + + return $_terms; + } + /** * Generate the ORDER BY clause for an 'orderby' param that is potentially related to a meta query. * @@ -1053,23 +1055,30 @@ class WP_Term_Query { * * @since 4.9.8 * - * @param array $term_ids Term IDs. - * @return array + * @param Object[]|int[] $terms List of objects or term ids. + * @return WP_Term[] Array of `WP_Term` objects. */ - protected function populate_terms( $term_ids ) { - $terms = array(); - - if ( ! is_array( $term_ids ) ) { - return $terms; + protected function populate_terms( $terms ) { + $term_objects = array(); + if ( ! is_array( $terms ) ) { + return $term_objects; } - foreach ( $term_ids as $key => $term_id ) { - $term = get_term( $term_id ); + foreach ( $terms as $key => $term_data ) { + if ( is_object( $term_data ) && property_exists( $term_data, 'term_id' ) ) { + $term = get_term( $term_data->term_id ); + if ( property_exists( $term_data, 'object_id' ) ) { + $term->object_id = (int) $term_data->object_id; + } + } else { + $term = get_term( $term_data ); + } + if ( $term instanceof WP_Term ) { - $terms[ $key ] = $term; + $term_objects[ $key ] = $term; } } - return $terms; + return $term_objects; } } diff --git a/tests/phpunit/tests/term/cache.php b/tests/phpunit/tests/term/cache.php index 13e2f720cd..d60e03074a 100644 --- a/tests/phpunit/tests/term/cache.php +++ b/tests/phpunit/tests/term/cache.php @@ -255,8 +255,8 @@ class Tests_Term_Cache extends WP_UnitTestCase { clean_term_cache( $term_id, 'post_tag' ); $num_queries = $wpdb->num_queries; - $term = get_term_by( 'slug', 'burrito', 'post_tag' ); - $num_queries++; + $term = get_term_by( 'slug', 'burrito', 'post_tag' ); + $num_queries = $num_queries + 2; $this->assertSame( 'Taco', $term->name ); $this->assertSame( $num_queries, $wpdb->num_queries ); @@ -286,8 +286,8 @@ class Tests_Term_Cache extends WP_UnitTestCase { clean_term_cache( $term_id, 'post_tag' ); $num_queries = $wpdb->num_queries; - $term = get_term_by( 'slug', 'burrito', 'post_tag' ); - $num_queries++; + $term = get_term_by( 'slug', 'burrito', 'post_tag' ); + $num_queries = $num_queries + 2; $this->assertSame( 'Taco', $term->name ); $this->assertSame( $num_queries, $wpdb->num_queries ); @@ -301,8 +301,8 @@ class Tests_Term_Cache extends WP_UnitTestCase { $num_queries = $wpdb->num_queries; // This should not hit cache. - $term = get_term_by( 'slug', 'burrito', 'post_tag' ); - $num_queries++; + $term = get_term_by( 'slug', 'burrito', 'post_tag' ); + $num_queries = $num_queries + 2; $this->assertSame( 'No Taco', $term->name ); $this->assertSame( $num_queries, $wpdb->num_queries ); } @@ -325,7 +325,7 @@ class Tests_Term_Cache extends WP_UnitTestCase { $num_queries = $wpdb->num_queries; get_term_by( 'name', 'Burrito', 'post_tag' ); - $num_queries++; + $num_queries = $num_queries + 2; $this->assertSame( $num_queries, $wpdb->num_queries ); // This should now hit cache. @@ -354,7 +354,7 @@ class Tests_Term_Cache extends WP_UnitTestCase { $num_queries = $wpdb->num_queries; get_term_by( 'name', 'Burrito', 'post_tag' ); - $num_queries++; + $num_queries = $num_queries + 2; $this->assertSame( $num_queries, $wpdb->num_queries ); // This should now hit cache. @@ -367,7 +367,7 @@ class Tests_Term_Cache extends WP_UnitTestCase { // This should not hit cache. get_term_by( 'name', 'burrito', 'post_tag' ); - $num_queries++; + $num_queries = $num_queries + 2; $this->assertSame( $num_queries, $wpdb->num_queries ); } @@ -388,8 +388,8 @@ class Tests_Term_Cache extends WP_UnitTestCase { $num_queries = $wpdb->num_queries; $last_changed = wp_cache_get( 'last_changed', 'terms' ); - $term1 = get_term_by( 'name', 'Burrito', 'post_tag' ); - $num_queries++; + $term1 = get_term_by( 'name', 'Burrito', 'post_tag' ); + $num_queries = $num_queries + 2; // Verify the term is cached. $term2 = get_term_by( 'name', 'Burrito', 'post_tag' ); @@ -431,8 +431,8 @@ class Tests_Term_Cache extends WP_UnitTestCase { clean_term_cache( $term_id, 'post_tag' ); $num_queries = $wpdb->num_queries; - $term = get_term_by( 'name', 'Burrito', 'post_tag' ); - $num_queries++; + $term = get_term_by( 'name', 'Burrito', 'post_tag' ); + $num_queries = $num_queries + 2; $this->assertInstanceOf( 'WP_Term', $term ); $this->assertSame( $term_id, $term->term_id ); $this->assertSame( $num_queries, $wpdb->num_queries ); diff --git a/tests/phpunit/tests/term/getTermBy.php b/tests/phpunit/tests/term/getTermBy.php index a7f19ead6c..cdc484e90a 100644 --- a/tests/phpunit/tests/term/getTermBy.php +++ b/tests/phpunit/tests/term/getTermBy.php @@ -5,6 +5,8 @@ */ class Tests_Term_GetTermBy extends WP_UnitTestCase { + protected $query = ''; + public function test_get_term_by_slug() { $term1 = wp_insert_term( 'Foo', 'category', array( 'slug' => 'foo' ) ); $term2 = get_term_by( 'slug', 'foo', 'category' ); @@ -123,7 +125,7 @@ class Tests_Term_GetTermBy extends WP_UnitTestCase { $num_queries = $wpdb->num_queries; $found = get_term_by( 'slug', 'foo', 'wptests_tax' ); - $num_queries++; + $num_queries = $num_queries + 2; $this->assertInstanceOf( 'WP_Term', $found ); $this->assertSame( $t, $found->term_id ); @@ -209,17 +211,16 @@ class Tests_Term_GetTermBy extends WP_UnitTestCase { * @ticket 21760 */ public function test_query_should_contain_limit_clause() { - global $wpdb; - $term_id = $this->factory->term->create( array( 'name' => 'burrito', 'taxonomy' => 'post_tag', ) ); - $found = get_term_by( 'name', 'burrito', 'post_tag' ); + add_filter( 'terms_pre_query', array( $this, 'get_query_from_filter' ), 10, 2 ); + $found = get_term_by( 'name', 'burrito', 'post_tag' ); $this->assertSame( $term_id, $found->term_id ); - $this->assertStringContainsString( 'LIMIT 1', $wpdb->last_query ); + $this->assertStringContainsString( 'LIMIT 1', $this->query ); } /** @@ -282,4 +283,10 @@ class Tests_Term_GetTermBy extends WP_UnitTestCase { $this->assertFalse( $found_by_slug ); $this->assertFalse( $found_by_name ); } + + public function get_query_from_filter( $terms, $wp_term_query ) { + $this->query = $wp_term_query->request; + + return $terms; + } } diff --git a/tests/phpunit/tests/term/getTerms.php b/tests/phpunit/tests/term/getTerms.php index 70ead58e51..d4d235798b 100644 --- a/tests/phpunit/tests/term/getTerms.php +++ b/tests/phpunit/tests/term/getTerms.php @@ -118,7 +118,7 @@ class Tests_Term_getTerms extends WP_UnitTestCase { $this->assertCount( 3, $terms ); $time1 = wp_cache_get( 'last_changed', 'terms' ); $this->assertNotEmpty( $time1 ); - $this->assertSame( $num_queries + 1, $wpdb->num_queries ); + $this->assertSame( $num_queries + 2, $wpdb->num_queries ); $num_queries = $wpdb->num_queries; diff --git a/tests/phpunit/tests/term/isObjectInTerm.php b/tests/phpunit/tests/term/isObjectInTerm.php index 9e4937fd6e..457b0e528c 100644 --- a/tests/phpunit/tests/term/isObjectInTerm.php +++ b/tests/phpunit/tests/term/isObjectInTerm.php @@ -145,7 +145,7 @@ class Tests_IsObjectInTerm extends WP_UnitTestCase { $num_queries = $wpdb->num_queries; $this->assertTrue( is_object_in_term( $o, 'wptests_tax', $terms[0] ) ); - $num_queries++; + $num_queries = $num_queries + 2; $this->assertSame( $num_queries, $wpdb->num_queries ); $this->assertFalse( is_object_in_term( $o, 'wptests_tax', $terms[1] ) ); @@ -166,14 +166,14 @@ class Tests_IsObjectInTerm extends WP_UnitTestCase { $num_queries = $wpdb->num_queries; $this->assertTrue( is_object_in_term( $o, 'wptests_tax', $terms[0] ) ); - $num_queries++; + $num_queries = $num_queries + 2; $this->assertSame( $num_queries, $wpdb->num_queries ); wp_set_object_terms( $o, $terms[1], 'wptests_tax' ); $num_queries = $wpdb->num_queries; $this->assertTrue( is_object_in_term( $o, 'wptests_tax', $terms[1] ) ); - $num_queries++; + $num_queries = $num_queries + 2; $this->assertSame( $num_queries, $wpdb->num_queries ); }