From 907b5fc52724e5ec28baeb25323187c683ecd0a0 Mon Sep 17 00:00:00 2001 From: "Dominik Schilling (ocean90)" Date: Thu, 29 Sep 2016 22:35:32 +0000 Subject: [PATCH] Taxonomy: Use `WP_Term_Query` in `get_term_by()`. `WP_Term_Query` already supports querying terms by 'slug', 'name', and 'term_taxonomy_id'. Its additional arguments allow us to generate nearly the same SQL queries as before. This change has one yuge benefit: the term queries are now cached. Add tests to increase coverage of `get_term_by()`. Props spacedmonkey, boonebgorges, johnjamesjacoby, pento, ocean90. Fixes #21760. git-svn-id: https://develop.svn.wordpress.org/trunk@38677 602fd350-edb4-49c9-b593-d223f7449a82 --- src/wp-includes/taxonomy.php | 54 ++++---- tests/phpunit/tests/term/cache.php | 171 +++++++++++++++++++++++++ tests/phpunit/tests/term/getTermBy.php | 105 +++++++++++++++ 3 files changed, 306 insertions(+), 24 deletions(-) diff --git a/src/wp-includes/taxonomy.php b/src/wp-includes/taxonomy.php index 5767ebf7a3..a39fa47518 100644 --- a/src/wp-includes/taxonomy.php +++ b/src/wp-includes/taxonomy.php @@ -927,49 +927,55 @@ function get_term( $term, $taxonomy = '', $output = OBJECT, $filter = 'raw' ) { * or `$term` was not found. */ function get_term_by( $field, $value, $taxonomy = '', $output = OBJECT, $filter = 'raw' ) { - global $wpdb; // 'term_taxonomy_id' lookups don't require taxonomy checks. if ( 'term_taxonomy_id' !== $field && ! taxonomy_exists( $taxonomy ) ) { return false; } - $tax_clause = $wpdb->prepare( "AND tt.taxonomy = %s", $taxonomy ); - - if ( 'slug' == $field ) { - $_field = 't.slug'; - $value = sanitize_title($value); - if ( empty($value) ) - return false; - } elseif ( 'name' == $field ) { - // Assume already escaped - $value = wp_unslash($value); - $_field = 't.name'; - } elseif ( 'term_taxonomy_id' == $field ) { - $value = (int) $value; - $_field = 'tt.term_taxonomy_id'; - - // No `taxonomy` clause when searching by 'term_taxonomy_id'. - $tax_clause = ''; - } else { + if ( 'id' === $field || 'term_id' === $field ) { $term = get_term( (int) $value, $taxonomy, $output, $filter ); - if ( is_wp_error( $term ) || is_null( $term ) ) { + if ( is_wp_error( $term ) || null === $term ) { $term = false; } return $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 $_field = %s", $value ) . " $tax_clause LIMIT 1" ); - if ( ! $term ) + $args = array( + 'get' => 'all', + 'number' => 1, + 'taxonomy' => $taxonomy, + 'update_term_meta_cache' => false, + 'orderby' => 'none', + ); + + switch ( $field ) { + case 'slug' : + $args['slug'] = $value; + break; + case 'name' : + $args['name'] = $value; + break; + case 'term_taxonomy_id' : + $args['term_taxonomy_id'] = $value; + unset( $args[ 'taxonomy' ] ); + break; + default : + return false; + } + + $terms = get_terms( $args ); + if ( is_wp_error( $terms ) || empty( $terms ) ) { return false; + } + + $term = array_shift( $terms ); // In the case of 'term_taxonomy_id', override the provided `$taxonomy` with whatever we find in the db. if ( 'term_taxonomy_id' === $field ) { $taxonomy = $term->taxonomy; } - wp_cache_add( $term->term_id, $term, 'terms' ); - return get_term( $term, $taxonomy, $output, $filter ); } diff --git a/tests/phpunit/tests/term/cache.php b/tests/phpunit/tests/term/cache.php index 8d9f51ada2..1ce755bf3a 100644 --- a/tests/phpunit/tests/term/cache.php +++ b/tests/phpunit/tests/term/cache.php @@ -219,4 +219,175 @@ class Tests_Term_Cache extends WP_UnitTestCase { $this->assertSame( $p, $term->object_id ); } } + + /** + * @ticket 21760 + */ + function test_get_term_by_slug_cache() { + global $wpdb; + + $term_id = $this->factory->term->create( array( 'slug' => 'burrito', 'name' => 'Taco', 'taxonomy' => 'post_tag' ) ); + + clean_term_cache( $term_id, 'post_tag' ); + $num_queries = $wpdb->num_queries; + + $term = get_term_by( 'slug', 'burrito', 'post_tag' ); + $num_queries++; + $this->assertEquals( 'Taco', $term->name ); + $this->assertEquals( $num_queries, $wpdb->num_queries ); + + // This should now hit cache. + $term = get_term_by( 'slug', 'burrito', 'post_tag' ); + $this->assertEquals( 'Taco', $term->name ); + $this->assertEquals( $num_queries, $wpdb->num_queries ); + + $this->assertEquals( get_term( $term_id, 'post_tag' ), $term ); + $this->assertEquals( $num_queries, $wpdb->num_queries ); + } + + /** + * @ticket 21760 + */ + function test_get_term_by_slug_cache_update() { + global $wpdb; + + $term_id = $this->factory->term->create( array( 'slug' => 'burrito', 'name' => 'Taco', 'taxonomy' => 'post_tag' ) ); + + clean_term_cache( $term_id, 'post_tag' ); + $num_queries = $wpdb->num_queries; + + $term = get_term_by( 'slug', 'burrito', 'post_tag' ); + $num_queries++; + $this->assertEquals( 'Taco', $term->name ); + $this->assertEquals( $num_queries, $wpdb->num_queries ); + + // This should now hit cache. + $term = get_term_by( 'slug', 'burrito', 'post_tag' ); + $this->assertEquals( 'Taco', $term->name ); + $this->assertEquals( $num_queries, $wpdb->num_queries ); + + // Update the tag which invalidates the cache. + wp_update_term( $term_id, 'post_tag', array( 'name' => 'No Taco' ) ); + $num_queries = $wpdb->num_queries; + + // This should not hit cache. + $term = get_term_by( 'slug', 'burrito', 'post_tag' ); + $num_queries++; + $this->assertEquals( 'No Taco', $term->name ); + $this->assertEquals( $num_queries, $wpdb->num_queries ); + } + + /** + * @ticket 21760 + */ + function test_get_term_by_name_cache() { + global $wpdb; + + $term_id = $this->factory->term->create( array( 'name' => 'Burrito', 'slug' => 'noburrito', 'taxonomy' => 'post_tag' ) ); + + clean_term_cache( $term_id, 'post_tag' ); + $num_queries = $wpdb->num_queries; + + get_term_by( 'name', 'Burrito', 'post_tag' ); + $num_queries++; + $this->assertEquals( $num_queries, $wpdb->num_queries ); + + // This should now hit cache. + $term = get_term_by( 'name', 'Burrito', 'post_tag' ); + $this->assertEquals( $num_queries, $wpdb->num_queries ); + + $this->assertEquals( get_term( $term_id, 'post_tag' ), $term ); + $this->assertEquals( $num_queries, $wpdb->num_queries ); + } + + /** + * @ticket 21760 + */ + function test_get_term_by_name_cache_update() { + global $wpdb; + + $term_id = $this->factory->term->create( array( 'name' => 'Burrito', 'slug' => 'noburrito', 'taxonomy' => 'post_tag' ) ); + + clean_term_cache( $term_id, 'post_tag' ); + $num_queries = $wpdb->num_queries; + + get_term_by( 'name', 'Burrito', 'post_tag' ); + $num_queries++; + $this->assertEquals( $num_queries, $wpdb->num_queries ); + + // This should now hit cache. + get_term_by( 'name', 'Burrito', 'post_tag' ); + $this->assertEquals( $num_queries, $wpdb->num_queries ); + + // Update the tag which invalidates the cache. + wp_update_term( $term_id, 'post_tag', array( 'slug' => 'taco' ) ); + $num_queries = $wpdb->num_queries; + + // This should not hit cache. + get_term_by( 'name', 'burrito', 'post_tag' ); + $num_queries++; + $this->assertEquals( $num_queries, $wpdb->num_queries ); + } + + /** + * @ticket 21760 + */ + function test_invalidating_term_caches_should_fail_when_invalidation_is_suspended() { + global $wpdb; + + $term_id = $this->factory->term->create( array( 'name' => 'Burrito', 'taxonomy' => 'post_tag' ) ); + + clean_term_cache( $term_id, 'post_tag' ); + $num_queries = $wpdb->num_queries; + $last_changed = wp_cache_get( 'last_changed', 'terms' ); + + $term1 = get_term_by( 'name', 'Burrito', 'post_tag' ); + $num_queries++; + + // Verify the term is cached. + $term2 = get_term_by( 'name', 'Burrito', 'post_tag' ); + $this->assertEquals( $num_queries, $wpdb->num_queries ); + $this->assertEquals( $term1, $term2 ); + + $suspend = wp_suspend_cache_invalidation(); + + // Update the tag. + wp_update_term( $term_id, 'post_tag', array( 'name' => 'Taco' ) ); + $num_queries = $wpdb->num_queries; + + // Verify that the cached term still matches the initial cached term. + $term3 = get_term_by( 'name', 'Burrito', 'post_tag' ); + $this->assertEquals( $num_queries, $wpdb->num_queries ); + $this->assertEquals( $term1, $term3 ); + + // Verify that last changed has not been updated as part of an invalidation routine. + $this->assertSame( $last_changed, wp_cache_get( 'last_changed', 'terms' ) ); + + // Clean up. + wp_suspend_cache_invalidation( $suspend ); + } + + /** + * @ticket 21760 + */ + public function test_get_term_by_does_not_prime_term_meta_cache() { + global $wpdb; + + $term_id = $this->factory->term->create( array( 'name' => 'Burrito', 'taxonomy' => 'post_tag' ) ); + add_term_meta( $term_id, 'foo', 'bar' ); + + clean_term_cache( $term_id, 'post_tag' ); + $num_queries = $wpdb->num_queries; + + $term = get_term_by( 'name', 'Burrito', 'post_tag' ); + $num_queries++; + $this->assertTrue( $term instanceof WP_Term ); + $this->assertSame( $term_id, $term->term_id ); + $this->assertEquals( $num_queries, $wpdb->num_queries ); + + $term_meta = get_term_meta( $term_id, 'foo', true ); + $num_queries++; + $this->assertSame( $term_meta, 'bar' ); + $this->assertEquals( $num_queries, $wpdb->num_queries ); + } } diff --git a/tests/phpunit/tests/term/getTermBy.php b/tests/phpunit/tests/term/getTermBy.php index 9d80ab643e..bd71d3fc49 100644 --- a/tests/phpunit/tests/term/getTermBy.php +++ b/tests/phpunit/tests/term/getTermBy.php @@ -4,6 +4,34 @@ * @group taxonomy */ class Tests_Term_GetTermBy extends WP_UnitTestCase { + + function test_get_term_by_slug() { + $term1 = wp_insert_term( 'Foo', 'category', array( 'slug' => 'foo' ) ); + $term2 = get_term_by( 'slug', 'foo', 'category' ); + $this->assertEquals( get_term( $term1['term_id'], 'category' ), $term2 ); + } + + function test_get_term_by_name() { + $term1 = wp_insert_term( 'Foo', 'category', array( 'slug' => 'foo' ) ); + $term2 = get_term_by( 'name', 'Foo', 'category' ); + $this->assertEquals( get_term( $term1['term_id'], 'category' ), $term2 ); + } + + function test_get_term_by_id() { + $term1 = wp_insert_term( 'Foo', 'category', array( 'slug' => 'foo' ) ); + $term2 = get_term_by( 'id', $term1['term_id'], 'category' ); + $this->assertEquals( get_term( $term1['term_id'], 'category' ), $term2 ); + } + + /** + * 'term_id' is an alias of 'id'. + */ + function test_get_term_by_term_id() { + $term1 = wp_insert_term( 'Foo', 'category', array( 'slug' => 'foo' ) ); + $term2 = get_term_by( 'term_id', $term1['term_id'], 'category' ); + $this->assertEquals( get_term( $term1['term_id'], 'category' ), $term2 ); + } + /** * @ticket 21651 */ @@ -13,6 +41,12 @@ class Tests_Term_GetTermBy extends WP_UnitTestCase { $this->assertEquals( get_term( $term1['term_id'], 'category' ), $term2 ); } + function test_get_term_by_unknown() { + wp_insert_term( 'Foo', 'category', array( 'slug' => 'foo' ) ); + $term2 = get_term_by( 'unknown', 'foo', 'category' ); + $this->assertFalse( $term2 ); + } + /** * @ticket 33281 */ @@ -87,4 +121,75 @@ class Tests_Term_GetTermBy extends WP_UnitTestCase { $this->assertSame( $t, $found->term_id ); $this->assertSame( $num_queries, $wpdb->num_queries ); } + + /** + * @ticket 21760 + */ + public function test_should_unslash_name() { + register_taxonomy( 'wptests_tax', 'post' ); + $term_name = 'Foo " \o/'; + $term_name_slashed = wp_slash( $term_name ); + $t = self::factory()->term->create( array( + 'taxonomy' => 'wptests_tax', + 'name' => $term_name_slashed, + ) ); + + $found = get_term_by( 'name', $term_name_slashed, 'wptests_tax' ); + + $this->assertTrue( $found instanceof WP_Term ); + $this->assertSame( $t, $found->term_id ); + $this->assertSame( $term_name, $found->name ); + } + + /** + * @ticket 21760 + */ + public function test_should_sanitize_slug() { + register_taxonomy( 'wptests_tax', 'post' ); + $t1 = self::factory()->term->create( array( + 'taxonomy' => 'wptests_tax', + 'slug' => 'foo-foo', + ) ); + + // Whitespace should get replaced by a '-'. + $found1 = get_term_by( 'slug', 'foo foo', 'wptests_tax' ); + + $this->assertTrue( $found1 instanceof WP_Term ); + $this->assertSame( $t1, $found1->term_id ); + + $t2 = self::factory()->term->create( array( + 'taxonomy' => 'wptests_tax', + 'slug' => '%e4%bb%aa%e8%a1%a8%e7%9b%98', + ) ); + + // Slug should get urlencoded. + $found2 = get_term_by( 'slug', '仪表盘', 'wptests_tax' ); + + $this->assertTrue( $found2 instanceof WP_Term ); + $this->assertSame( $t2, $found2->term_id ); + } + + /** + * @ticket 21760 + */ + public function test_query_should_not_contain_order_by_clause() { + global $wpdb; + + $term_id = $this->factory->term->create( array( 'name' => 'burrito', 'taxonomy' => 'post_tag' ) ); + $found = get_term_by( 'name', 'burrito', 'post_tag' ); + $this->assertSame( $term_id, $found->term_id ); + $this->assertNotContains( 'ORDER BY', $wpdb->last_query ); + } + + /** + * @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' ); + $this->assertSame( $term_id, $found->term_id ); + $this->assertContains( 'LIMIT 1', $wpdb->last_query ); + } }