Changeset 53496 for trunk

Timestamp:

06/14/2022 11:41:33 AM (2 years ago)

Author:

spacedmonkey

Message:

Taxonomy: Fix caching issues in WP_Term_Query class.

Introduced [52836] when passing child_of or pad_counts parameters to get_terms or WP_Term_Query class, the array of terms received by the query, was not correctly cached. This
change simplifies the logic in WP_Term_Query and ensures terms are correctly cached. This change also, improves performance, by only caching an array of term ids where possible.

Props denishua, spacedmonkey, oztaser, peterwilsoncc, SergeyBiryukov, georgestephanis, jnz31, knutsp, mukesh27, costdev.
Fixes #55837.

Location:

trunk

Files:

• src/wp-includes/class-wp-term-query.php (modified) (5 diffs)
• src/wp-includes/version.php (modified) (1 diff)
• tests/phpunit/tests/term/getTerms.php (modified) (3 diffs)

trunk/src/wp-includes/class-wp-term-query.php

@@ -771 +771 @@
         $cache_args = wp_array_slice_assoc( $args, array_keys( $this->query_var_defaults ) );
 
-        unset( $cache_args['pad_counts'], $cache_args['update_term_meta_cache'] );
+        unset( $cache_args['update_term_meta_cache'] );
 
         if ( 'count' !== $_fields && 'all_with_object_id' !== $_fields ) {
@@ -784 +784 @@
         if ( false !== $cache ) {
             if ( 'ids' === $_fields ) {
-                $term_ids = wp_list_pluck( $cache, 'term_id' );
-                $cache    = array_map( 'intval', $term_ids );
+                $cache = array_map( 'intval', $cache );
             } elseif ( 'count' !== $_fields ) {
-                $term_ids = wp_list_pluck( $cache, 'term_id' );
+                if ( ( 'all_with_object_id' === $_fields && ! empty( $args['object_ids'] ) ) || ( 'all' === $_fields && $args['pad_counts'] ) ) {
+                    $term_ids = wp_list_pluck( $cache, 'term_id' );
+                } else {
+                    $term_ids = array_map( 'intval', $cache );
+                }
                 _prime_term_caches( $term_ids, $args['update_term_meta_cache'] );
                 $term_objects = $this->populate_terms( $cache );
@@ -850 +853 @@
         }
 
-        /*
-         * When querying for terms connected to objects, we may get
-         * duplicate results. The duplicates should be preserved if
-         * `$fields` is 'all_with_object_id', but should otherwise be
-         * removed.
-         */
-        if ( ! empty( $args['object_ids'] ) && 'all_with_object_id' !== $_fields ) {
-            $_tt_ids = array();
-            $_terms  = array();
-            foreach ( $terms as $term ) {
-                if ( isset( $_tt_ids[ $term->term_id ] ) ) {
-                    continue;
-                }
-
-                $_tt_ids[ $term->term_id ] = 1;
-                $_terms[]                  = $term;
-            }
-
-            $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();
+        if ( $hierarchical && $number && is_array( $term_objects ) ) {
+            if ( $offset >= count( $term_objects ) ) {
                 $term_objects = array();
             } else {
-                $terms        = array_slice( $terms, $offset, $number, true );
                 $term_objects = array_slice( $term_objects, $offset, $number, true );
             }
@@ -888 +868 @@
         }
 
-        wp_cache_add( $cache_key, $terms, 'terms' );
-        $terms = $this->format_terms( $term_objects, $_fields );
-
-        $this->terms = $terms;
+        if ( 'all_with_object_id' === $_fields && ! empty( $args['object_ids'] ) ) {
+            $term_cache = array();
+            foreach ( $term_objects as $term ) {
+                $object            = new stdClass();
+                $object->term_id   = $term->term_id;
+                $object->object_id = $term->object_id;
+                $term_cache[]      = $object;
+            }
+        } elseif ( 'all' === $_fields && $args['pad_counts'] ) {
+            $term_cache = array();
+            foreach ( $term_objects as $term ) {
+                $object          = new stdClass();
+                $object->term_id = $term->term_id;
+                $object->count   = $term->count;
+                $term_cache[]    = $object;
+            }
+        } else {
+            $term_cache = wp_list_pluck( $term_objects, 'term_id' );
+        }
+        wp_cache_add( $cache_key, $term_cache, 'terms' );
+        $this->terms = $this->format_terms( $term_objects, $_fields );
+
         return $this->terms;
     }
@@ -1120 +1118 @@
                     $term->object_id = (int) $term_data->object_id;
                 }
+                if ( property_exists( $term_data, 'count' ) ) {
+                    $term->count = (int) $term_data->count;
+                }
             } else {
                 $term = get_term( $term_data );

trunk/src/wp-includes/version.php

@@ -24 +24 @@
  * @global int $wp_db_version
  */
-$wp_db_version = 51917;
+$wp_db_version = 53496;
 
 /**

trunk/tests/phpunit/tests/term/getTerms.php

@@ -745 +745 @@
         );
         $this->assertCount( 1, $terms );
+    }
+
+    /**
+     * @ticket 55837
+     * @covers ::get_terms
+     */
+    public function test_get_terms_child_of_cache() {
+        $parent = self::factory()->category->create();
+        self::factory()->category->create( array( 'parent' => $parent ) );
+
+        $args  = array(
+            'fields'     => 'ids',
+            'child_of'   => $parent,
+            'hide_empty' => false,
+        );
+        $terms = get_terms( 'category', $args );
+        $this->assertCount( 1, $terms, 'The first call to get_terms() did not return 1 term' );
+
+        $terms2 = get_terms( 'category', $args );
+        $this->assertCount( 1, $terms2, 'The second call to get_terms() did not return 1 term' );
+        $this->assertSameSets( $terms, $terms2, 'Results are not the same after caching' );
     }
 
@@ -2591 +2612 @@
 
     /**
+     * @ticket 55837
+     * @covers ::get_terms
+     */
+    public function test_pad_counts_cached() {
+        register_taxonomy( 'wptests_tax_1', 'post', array( 'hierarchical' => true ) );
+
+        $posts = self::factory()->post->create_many( 3 );
+
+        $t1 = self::factory()->term->create(
+            array(
+                'taxonomy' => 'wptests_tax_1',
+            )
+        );
+        $t2 = self::factory()->term->create(
+            array(
+                'taxonomy' => 'wptests_tax_1',
+                'parent'   => $t1,
+            )
+        );
+        $t3 = self::factory()->term->create(
+            array(
+                'taxonomy' => 'wptests_tax_1',
+                'parent'   => $t2,
+            )
+        );
+
+        wp_set_object_terms( $posts[0], array( $t1 ), 'wptests_tax_1' );
+        wp_set_object_terms( $posts[1], array( $t2 ), 'wptests_tax_1' );
+        wp_set_object_terms( $posts[2], array( $t3 ), 'wptests_tax_1' );
+
+        $found = get_terms(
+            'wptests_tax_1',
+            array(
+                'pad_counts' => true,
+            )
+        );
+
+        $this->assertSameSets( array( $t1, $t2, $t3 ), wp_list_pluck( $found, 'term_id' ), 'Check to see if results are as expected' );
+
+        foreach ( $found as $f ) {
+            if ( $t1 === $f->term_id ) {
+                $this->assertSame( 3, $f->count, 'Check to see if term 1, has the correct count' );
+            } elseif ( $t2 === $f->term_id ) {
+                $this->assertSame( 2, $f->count, 'Check to see if term 2, has the correct count' );
+            } else {
+                $this->assertSame( 1, $f->count, 'Check to see if term 3, has the correct count' );
+            }
+        }
+
+        $found = get_terms(
+            'wptests_tax_1',
+            array(
+                'pad_counts' => true,
+            )
+        );
+
+        $this->assertSameSets( array( $t1, $t2, $t3 ), wp_list_pluck( $found, 'term_id' ), 'Check to see if results are as expected on second run' );
+
+        foreach ( $found as $f ) {
+            if ( $t1 === $f->term_id ) {
+                $this->assertSame( 3, $f->count, 'Check to see if term 1, has the correct count on second run' );
+            } elseif ( $t2 === $f->term_id ) {
+                $this->assertSame( 2, $f->count, 'Check to see if term 2, has the correct count on second run' );
+            } else {
+                $this->assertSame( 1, $f->count, 'Check to see if term 3, has the correct count on second run' );
+            }
+        }
+    }
+
+    /**
      * @ticket 20635
      */
@@ -3098 +3189 @@
                     'taxonomy' => self::$taxonomy,
                     'fields'   => 'slugs',
-                ),
-            ),
-            'meta cache off, pad count on  vs meta cache on, pad count off' => array(
-                array(
-                    'taxonomy'               => self::$taxonomy,
-                    'pad_counts'             => true,
-                    'update_term_meta_cache' => false,
-                ),
-                array(
-                    'taxonomy' => self::$taxonomy,
-                    'fields'   => 'ids',
                 ),
             ),