From cf93e7996d1f9b48a404441238b8dd2a950d890a Mon Sep 17 00:00:00 2001 From: Felix Arntz Date: Wed, 22 Feb 2017 10:41:20 +0000 Subject: [PATCH] Multisite: Use `WP_Network_Query` in `WP_Network::get_by_path()`. An additional unit test has been introduced to verify the method works properly when using an external object cache. Props spacedmonkey, jeremyfelt. Fixes #37217. git-svn-id: https://develop.svn.wordpress.org/trunk@40102 602fd350-edb4-49c9-b593-d223f7449a82 --- src/wp-includes/class-wp-network.php | 50 +++++++++-------- tests/phpunit/tests/multisite/bootstrap.php | 60 +++++++++++++++++++++ 2 files changed, 84 insertions(+), 26 deletions(-) diff --git a/src/wp-includes/class-wp-network.php b/src/wp-includes/class-wp-network.php index 5ab29c2c58..0d2352940b 100644 --- a/src/wp-includes/class-wp-network.php +++ b/src/wp-includes/class-wp-network.php @@ -267,8 +267,6 @@ class WP_Network { * @return WP_Network|bool Network object if successful. False when no network is found. */ public static function get_by_path( $domain = '', $path = '', $segments = null ) { - global $wpdb; - $domains = array( $domain ); $pieces = explode( '.', $domain ); @@ -295,7 +293,11 @@ class WP_Network { if ( wp_using_ext_object_cache() ) { $using_paths = wp_cache_get( 'networks_have_paths', 'site-options' ); if ( false === $using_paths ) { - $using_paths = (int) $wpdb->get_var( "SELECT id FROM {$wpdb->site} WHERE path <> '/' LIMIT 1" ); + $using_paths = get_networks( array( + 'number' => 1, + 'count' => true, + 'path__not_in' => '/', + ) ); wp_cache_add( 'networks_have_paths', $using_paths, 'site-options' ); } } @@ -353,35 +355,31 @@ class WP_Network { return $pre; } - // @todo Consider additional optimization routes, perhaps as an opt-in for plugins. - // We already have paths covered. What about how far domains should be drilled down (including www)? - - $search_domains = "'" . implode( "', '", $wpdb->_escape( $domains ) ) . "'"; - if ( ! $using_paths ) { - $network = $wpdb->get_row( " - SELECT * FROM {$wpdb->site} - WHERE domain IN ({$search_domains}) - ORDER BY CHAR_LENGTH(domain) - DESC LIMIT 1 - " ); + $networks = get_networks( array( + 'number' => 1, + 'orderby' => array( + 'domain_length' => 'DESC', + ), + 'domain__in' => $domains, + ) ); - if ( ! empty( $network ) && ! is_wp_error( $network ) ) { - return new WP_Network( $network ); + if ( ! empty( $networks ) ) { + return array_shift( $networks ); } return false; - - } else { - $search_paths = "'" . implode( "', '", $wpdb->_escape( $paths ) ) . "'"; - $networks = $wpdb->get_results( " - SELECT * FROM {$wpdb->site} - WHERE domain IN ({$search_domains}) - AND path IN ({$search_paths}) - ORDER BY CHAR_LENGTH(domain) DESC, CHAR_LENGTH(path) DESC - " ); } + $networks = get_networks( array( + 'orderby' => array( + 'domain_length' => 'DESC', + 'path_length' => 'DESC', + ), + 'domain__in' => $domains, + 'path__in' => $paths, + ) ); + /* * Domains are sorted by length of domain, then by length of path. * The domain must match for the path to be considered. Otherwise, @@ -402,7 +400,7 @@ class WP_Network { } if ( true === $found ) { - return new WP_Network( $network ); + return $network; } return false; diff --git a/tests/phpunit/tests/multisite/bootstrap.php b/tests/phpunit/tests/multisite/bootstrap.php index 1945448231..6b9c9aee49 100644 --- a/tests/phpunit/tests/multisite/bootstrap.php +++ b/tests/phpunit/tests/multisite/bootstrap.php @@ -17,6 +17,7 @@ class Tests_Multisite_Bootstrap extends WP_UnitTestCase { 'wordpress.org/' => array( 'domain' => 'wordpress.org', 'path' => '/' ), 'make.wordpress.org/' => array( 'domain' => 'make.wordpress.org', 'path' => '/' ), 'wordpress.org/one/' => array( 'domain' => 'wordpress.org', 'path' => '/one/' ), + 'wordpress.org/one/b/' => array( 'domain' => 'wordpress.org', 'path' => '/one/b/' ), 'wordpress.net/' => array( 'domain' => 'wordpress.net', 'path' => '/' ), 'www.wordpress.net/' => array( 'domain' => 'www.wordpress.net', 'path' => '/' ), 'www.wordpress.net/two/' => array( 'domain' => 'www.wordpress.net', 'path' => '/two/' ), @@ -80,14 +81,73 @@ class Tests_Multisite_Bootstrap extends WP_UnitTestCase { array( 'wordpress.net/', 'wordpress.net', '/notapath/', 'A missing path on a top level domain should find the correct network.' ), array( 'www.wordpress.net/', 'www.wordpress.net', '/notapath/', 'A missing path should find the correct network.' ), array( 'wordpress.org/one/', 'www.wordpress.org', '/one/', 'Should find the path despite the www.' ), + array( 'wordpress.org/one/', 'wordpress.org', '/one/page/', 'A request with two path segments should find the correct network.' ), + array( 'wordpress.org/one/b/', 'wordpress.org', '/one/b/', 'A request with two valid path segments should find the correct network.' ), array( 'wordpress.org/', 'site1.wordpress.org', '/one/', 'Should not find path because domains do not match.' ), array( 'wordpress.net/three/', 'wordpress.net', '/three/', 'A network can have a path.' ), array( 'www.wordpress.net/two/', 'www.wordpress.net', '/two/', 'A www network with a path can coexist with a non-www network.' ), array( 'wordpress.net/', 'site1.wordpress.net', '/notapath/', 'An invalid subdomain should find the top level network domain.' ), array( 'wordpress.net/', 'site1.wordpress.net', '/three/', 'An invalid subdomain and path should find the top level network domain.' ), + array( 'wordpress.net/', 'x.y.wordpress.net', '/', 'An invalid two level subdomain should find the top level network domain.' ), ); } + /** + * @ticket 37217 + * @dataProvider data_get_network_by_path_not_using_paths + * + * @param string $expected_key The array key associated with expected data for the test. + * @param string $domain The requested domain. + * @param string $path The requested path. + * @param string $message The message to pass for failed tests. + */ + public function test_get_network_by_path_not_using_paths( $expected_key, $domain, $path, $message ) { + if ( ! wp_using_ext_object_cache() ) { + $this->markTestSkipped( 'Only testable with an external object cache.' ); + } + + // Temporarily store original object cache and using paths values. + $using_paths_orig = wp_cache_get( 'networks_have_paths', 'site-options' ); + + wp_cache_set( 'networks_have_paths', 0, 'site-options' ); + + $network = get_network_by_path( $domain, $path ); + + // Restore original object cache and using paths values. + wp_cache_set( 'networks_have_paths', $using_paths_orig, 'site-options' ); + + $this->assertEquals( self::$network_ids[ $expected_key ], $network->id, $message ); + } + + public function data_get_network_by_path_not_using_paths() { + return array( + array( 'wordpress.org/', 'wordpress.org', '/', 'A standard domain and path request should work.' ), + array( 'wordpress.net/', 'wordpress.net', '/notapath/', 'A network matching a top level domain should be found regardless of path.' ), + array( 'www.wordpress.net/', 'www.wordpress.net', '/notapath/', 'A network matching a domain should be found regardless of path.' ), + array( 'wordpress.org/', 'www.wordpress.org', '/one/', 'Should find the network despite the www and regardless of path.' ), + array( 'wordpress.org/', 'site1.wordpress.org', '/one/', 'Should find the network with the corresponding top level domain regardless of path.' ), + array( 'www.wordpress.net/', 'www.wordpress.net', '/two/', 'A www network can coexist with a non-www network.' ), + array( 'make.wordpress.org/', 'make.wordpress.org', '/notapath/', 'A subdomain network should be found regardless of path.' ), + array( 'wordpress.net/', 'x.y.wordpress.net', '/', 'An invalid two level subdomain should find the top level network domain.' ), + ); + } + + /** + * Even if a matching network is available, it should not match if the the filtered + * value for network path segments is fewer than the number of paths passed. + */ + public function test_get_network_by_path_with_forced_single_path_segment_returns_single_path_network() { + add_filter( 'network_by_path_segments_count', array( $this, 'filter_network_path_segments' ) ); + $network = get_network_by_path( 'wordpress.org', '/one/b/' ); + remove_filter( 'network_by_path_segments_count', array( $this, 'filter_network_path_segments' ) ); + + $this->assertEquals( self::$network_ids[ 'wordpress.org/one/' ], $network->id ); + } + + public function filter_network_path_segments() { + return 1; + } + /** * @ticket 27003 * @ticket 27927