Changeset 35537

Timestamp:

11/05/2015 04:44:59 PM (11 years ago)

Author:

boonebgorges

Message:

Make get_term() behave more consistently in the context of shared terms.

When WP_Term was introduced in [34997], the $taxonomy parameter for
get_term() was made optional. This meant that, when the optional param was
omitted, get_term() had no way of determining which term was intended when
the term_id was shared between multiple taxonomies. As a (somewhat sneaky) way
of fixing things, get_term() split any shared terms it found. But this could
cause problems with developer expectations: it's not clear why requesting a
term should result in a database update, much less a potential change in the
ID of a term.

In place of this technique, this changeset introduces a number of changes that
make the handling of shared terms a bit less insane:

• When a taxonomy is provided to get_term(), and a cached term is found matching the term_id, make sure the taxonomy also matches before returning it.
• When a taxonomy is not provided, ensure that the term is not shared before adding it to the cache.
• When a term is shared between taxonomies and no taxonomy is provided, return a WP_Error rather than splitting the term.
• When a term is shared between taxonomies, only one of which is valid, return the term from that taxonomy.

Props boonebgorges, dlh.
Fixes #34533.

Location:

trunk

Files:

• src/wp-includes/class-wp-term.php (modified) (2 diffs)
• src/wp-includes/taxonomy-functions.php (modified) (1 diff)
• tests/phpunit/tests/term/getTerm.php (modified) (2 diffs)

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

@@ -116 +116 @@
          * @global wpdb $wpdb WordPress database abstraction object.
          *
-         * @param int $term_id Term ID.
-         * @return WP_Term|false Term object, false otherwise.
-         */
-        public static function get_instance( $term_id ) {
+         * @param int    $term_id  Term ID.
+         * @param string $taxonomy Optional. Limit matched terms to those matching `$taxonomy`. Only used for
+         *                         disambiguating potentially shared terms.
+         * @return WP_Term|WP_Error|false Term object, if found. WP_Error if `$term_id` is shared between taxonomies and
+         *                                there's insufficient data to distinguish which term is intended.
+         *                                False for other failures.
+         */
+        public static function get_instance( $term_id, $taxonomy = null ) {
                 global $wpdb;
 
@@ -130 +134 @@
 
                 // If there isn't a cached version, hit the database.
-                if ( ! $_term ) {
-                        $_term = $wpdb->get_row( $wpdb->prepare( "SELECT t.*, tt.* FROM $wpdb->terms AS t INNER JOIN $wpdb->term_taxonomy AS tt ON t.term_id = tt.term_id WHERE t.term_id = %d LIMIT 1", $term_id ) );
+                if ( ! $_term || ( $taxonomy && $taxonomy !== $_term->taxonomy ) ) {
+                        // Grab all matching terms, in case any are shared between taxonomies.
+                        $terms = $wpdb->get_results( $wpdb->prepare( "SELECT t.*, tt.* FROM $wpdb->terms AS t INNER JOIN $wpdb->term_taxonomy AS tt ON t.term_id = tt.term_id WHERE t.term_id = %d", $term_id ) );
+                        if ( ! $terms ) {
+                                return false;
+                        }
+
+                        // If a taxonomy was specified, find a match.
+                        if ( $taxonomy ) {
+                                foreach ( $terms as $match ) {
+                                        if ( $taxonomy === $match->taxonomy ) {
+                                                $_term = $match;
+                                                break;
+                                        }
+                                }
+
+                        // If only one match was found, it's the one we want.
+                        } elseif ( 1 === count( $terms ) ) {
+                                $_term = reset( $terms );
+
+                        // Otherwise, the term must be shared between taxonomies.
+                        } else {
+                                // If the term is shared only with invalid taxonomies, return the one valid term.
+                                foreach ( $terms as $t ) {
+                                        if ( ! taxonomy_exists( $t->taxonomy ) ) {
+                                                continue;
+                                        }
+
+                                        // Only hit if we've already identified a term in a valid taxonomy.
+                                        if ( $_term ) {
+                                                return new WP_Error( 'ambiguous_term_id', __( 'Term ID is shared between multiple taxonomies' ), $term_id );
+                                        }
+
+                                        $_term = $t;
+                                }
+                        }
+
                         if ( ! $_term ) {
                                 return false;
                         }
 
+                        // Don't return terms from invalid taxonomies.
+                        if ( ! taxonomy_exists( $_term->taxonomy ) ) {
+                                return new WP_Error( 'invalid_taxonomy', __( 'Invalid taxonomy' ) );
+                        }
+
                         $_term = sanitize_term( $_term, $_term->taxonomy, 'raw' );
-                        wp_cache_add( $term_id, $_term, 'terms' );
+
+                        // Don't cache terms that are shared between taxonomies.
+                        if ( 1 === count( $terms ) ) {
+                                wp_cache_add( $term_id, $_term, 'terms' );
+                        }
                 }
 

trunk/src/wp-includes/taxonomy-functions.php

@@ -765 +765 @@
                 }
         } else {
-                $_term = WP_Term::get_instance( $term );
-        }
-
-        // If `$taxonomy` was provided, make sure it matches the taxonomy of the located term.
-        if ( $_term && $taxonomy && $taxonomy !== $_term->taxonomy ) {
-                // If there are two terms with the same ID, split the other one to a new term.
-                $new_term_id = _split_shared_term( $_term->term_id, $_term->term_taxonomy_id );
-
-                // If no split occurred, this is an invalid request. Return null (not WP_Error) for back compat.
-                if ( $new_term_id === $_term->term_id ) {
-                        return null;
-
-                // The term has been split. Refetch the term from the proper taxonomy.
-                } else {
-                        return get_term( $_term->term_id, $taxonomy, $output, $filter );
-                }
-        }
-
-        if ( ! $_term ) {
+                $_term = WP_Term::get_instance( $term, $taxonomy );
+        }
+
+        if ( is_wp_error( $_term ) ) {
+                return $_term;
+        } elseif ( ! $_term ) {
                 return null;
         }

trunk/tests/phpunit/tests/term/getTerm.php

@@ -10 +10 @@
         }
 
+        /**
+         * Utility function for generating two shared terms, in the 'wptests_tax' and 'wptests_tax_2' taxonomies.
+         *
+         * @return array Array of term_id/old_term_id/term_taxonomy_id triplets.
+         */
+        protected function generate_shared_terms() {
+                global $wpdb;
+
+                register_taxonomy( 'wptests_tax_2', 'post' );
+
+                $t1 = wp_insert_term( 'Foo', 'wptests_tax' );
+                $t2 = wp_insert_term( 'Foo', 'wptests_tax_2' );
+
+                // Manually modify because shared terms shouldn't naturally occur.
+                $wpdb->update( $wpdb->term_taxonomy,
+                        array( 'term_id' => $t1['term_id'] ),
+                        array( 'term_taxonomy_id' => $t2['term_taxonomy_id'] ),
+                        array( '%d' ),
+                        array( '%d' )
+                );
+
+                return array(
+                        array(
+                                'term_id' => $t1['term_id'],
+                                'old_term_id' => $t1['term_id'],
+                                'term_taxonomy_id' => $t1['term_taxonomy_id'],
+                        ),
+                        array(
+                                'term_id' => $t1['term_id'],
+                                'old_term_id' => $t2['term_id'],
+                                'term_taxonomy_id' => $t2['term_taxonomy_id'],
+                        ),
+                );
+        }
+
         public function test_should_return_error_for_empty_term() {
                 $found = get_term( '', 'wptests_tax' );
@@ -115 +150 @@
                 $this->assertNull( get_term( $term_id, 'category' ) );
         }
+
+        /**
+         * @ticket 34533
+         */
+        public function test_should_return_wp_error_when_term_is_shared_and_no_taxonomy_is_specified() {
+                $terms = $this->generate_shared_terms();
+
+                $found = get_term( $terms[0]['term_id'] );
+
+                $this->assertWPError( $found );
+        }
+
+        /**
+         * @ticket 34533
+         */
+        public function test_should_return_term_when_term_is_shared_and_correct_taxonomy_is_specified() {
+                $terms = $this->generate_shared_terms();
+
+                $found = get_term( $terms[0]['term_id'], 'wptests_tax' );
+
+                $this->assertInstanceOf( 'WP_Term', $found );
+                $this->assertSame( $terms[0]['term_id'], $found->term_id );
+        }
+
+        /**
+         * @ticket 34533
+         */
+        public function test_should_return_null_when_term_is_shared_and_incorrect_taxonomy_is_specified() {
+                $terms = $this->generate_shared_terms();
+
+                $found = get_term( $terms[0]['term_id'], 'post_tag' );
+
+                $this->assertNull( $found );
+        }
+
+        /**
+         * @ticket 34533
+         */
+        public function test_shared_term_in_cache_should_be_ignored_when_specifying_a_different_taxonomy() {
+                global $wpdb;
+
+                $terms = $this->generate_shared_terms();
+
+                // Prime cache for 'wptests_tax'.
+                get_term( $terms[0]['term_id'], 'wptests_tax' );
+                $num_queries = $wpdb->num_queries;
+
+                // Database should be hit again.
+                $found = get_term( $terms[1]['term_id'], 'wptests_tax_2' );
+                $num_queries++;
+
+                $this->assertSame( $num_queries, $wpdb->num_queries );
+                $this->assertInstanceOf( 'WP_Term', $found );
+                $this->assertSame( 'wptests_tax_2', $found->taxonomy );
+        }
+
+        /**
+         * @ticket 34533
+         */
+        public function test_should_return_error_when_only_matching_term_is_in_an_invalid_taxonomy() {
+                $t = self::factory()->term->create( array( 'taxonomy' => 'wptests_tax' ) );
+
+                _unregister_taxonomy( 'wptests_tax' );
+
+                $found = get_term( $t );
+                $this->assertWPError( $found );
+                $this->assertSame( 'invalid_taxonomy', $found->get_error_code() );
+        }
+
+        /**
+         * @ticket 34533
+         */
+        public function test_term_should_be_returned_when_id_is_shared_only_with_invalid_taxonomies() {
+                $terms = $this->generate_shared_terms();
+
+                _unregister_taxonomy( 'wptests_tax' );
+
+                $found = get_term( $terms[1]['term_id'] );
+                $this->assertInstanceOf( 'WP_Term', $found );
+                $this->assertSame( 'wptests_tax_2', $found->taxonomy );
+                $this->assertSame( $terms[1]['term_id'], $found->term_id );
+        }
 }