Changeset 55169

Timestamp:

01/31/2023 04:54:22 PM (4 years ago)

Author:

spacedmonkey

Message:

Query: Use WP_Query in get_page_by_path.

Replace raw database queries in get_page_by_path with a call to WP_Query class. This has a number of benefits, including improved caching and priming of post caches. To maintain backwards compatibility, this function calls WP_Query to gets all matching posts of all post statuses.

Props spacedmonkey, peterwilsoncc, costdev,
Fixes #56689.

Location:

trunk

Files:

• src/wp-includes/post.php (modified) (4 diffs)
• tests/phpunit/tests/post/getPageByPath.php (modified) (1 diff)

trunk/src/wp-includes/post.php

@@ -5661 +5661 @@
  * @since 2.1.0
  *
- * @global wpdb $wpdb WordPress database abstraction object.
- *
  * @param string       $page_path Page path.
  * @param string       $output    Optional. The required return type. One of OBJECT, ARRAY_A, or ARRAY_N, which
@@ -5671 +5669 @@
  */
 function get_page_by_path( $page_path, $output = OBJECT, $post_type = 'page' ) {
-        global $wpdb;
-
-        $last_changed = wp_cache_get_last_changed( 'posts' );
-
-        $hash      = md5( $page_path . serialize( $post_type ) );
-        $cache_key = "get_page_by_path:$hash:$last_changed";
-        $cached    = wp_cache_get( $cache_key, 'posts' );
-        if ( false !== $cached ) {
-                // Special case: '0' is a bad `$page_path`.
-                if ( '0' === $cached || 0 === $cached ) {
-                        return;
-                } else {
-                        return get_post( $cached, $output );
-                }
-        }
-
-        $page_path     = rawurlencode( urldecode( $page_path ) );
-        $page_path     = str_replace( '%2F', '/', $page_path );
-        $page_path     = str_replace( '%20', ' ', $page_path );
-        $parts         = explode( '/', trim( $page_path, '/' ) );
-        $parts         = array_map( 'sanitize_title_for_query', $parts );
-        $escaped_parts = esc_sql( $parts );
-
-        $in_string = "'" . implode( "','", $escaped_parts ) . "'";
+        $page_path = rawurlencode( urldecode( $page_path ) );
+        $page_path = str_replace( '%2F', '/', $page_path );
+        $page_path = str_replace( '%20', ' ', $page_path );
+        $parts     = explode( '/', trim( $page_path, '/' ) );
+        $parts     = array_map( 'sanitize_title_for_query', $parts );
 
         if ( is_array( $post_type ) ) {
@@ -5702 +5681 @@
         }
 
-        $post_types          = esc_sql( $post_types );
-        $post_type_in_string = "'" . implode( "','", $post_types ) . "'";
-        $sql                 = "
-                SELECT ID, post_name, post_parent, post_type
-                FROM $wpdb->posts
-                WHERE post_name IN ($in_string)
-                AND post_type IN ($post_type_in_string)
-        ";
-
-        $pages = $wpdb->get_results( $sql, OBJECT_K );
+        $args = array(
+                'post_name__in'          => $parts,
+                'post_type'              => $post_types,
+                'post_status'            => 'all',
+                'posts_per_page'         => -1,
+                'update_post_term_cache' => false,
+                'update_post_meta_cache' => false,
+                'no_found_rows'          => true,
+                'orderby'                => 'none',
+        );
+
+        $query = new WP_Query( $args );
+        $posts = $query->get_posts();
+        $pages = array();
+
+        foreach ( $posts as $post ) {
+                $pages[ $post->ID ] = $post;
+        }
 
         $revparts = array_reverse( $parts );
 
         $foundid = 0;
-        foreach ( (array) $pages as $page ) {
+        foreach ( $pages as $page ) {
                 if ( $page->post_name == $revparts[0] ) {
                         $count = 0;
@@ -5742 +5729 @@
                 }
         }
-
-        // We cache misses as well as hits.
-        wp_cache_set( $cache_key, $foundid, 'posts' );
 
         if ( $foundid ) {

trunk/tests/phpunit/tests/post/getPageByPath.php

@@ -104 +104 @@
         }
 
+        /**
+         * @ticket 56689
+         *
+         * @covers ::get_page_by_path
+         */
+        public function test_should_match_nested_page_query_count() {
+                $p1 = self::factory()->post->create(
+                        array(
+                                'post_type' => 'page',
+                                'post_name' => 'foo',
+                        )
+                );
+
+                $p2 = self::factory()->post->create(
+                        array(
+                                'post_type'   => 'page',
+                                'post_name'   => 'bar',
+                                'post_parent' => $p1,
+                        )
+                );
+
+                $p3 = self::factory()->post->create(
+                        array(
+                                'post_type'   => 'page',
+                                'post_name'   => 'baz',
+                                'post_parent' => $p2,
+                        )
+                );
+
+                $queries_before = get_num_queries();
+                $found          = get_page_by_path( 'foo/bar/baz' );
+                $queries_after  = get_num_queries();
+                $cached_post    = wp_cache_get( $p1, 'posts' );
+
+                $this->assertSame( 1, $queries_after - $queries_before, 'Only one query should run' );
+                $this->assertSame( $p3, $found->ID, 'Check to see if the result is correct' );
+                $this->assertIsObject( $cached_post, 'The cached post is not an object' );
+        }
+
+        /**
+         * @ticket 56689
+         *
+         * @covers ::get_page_by_path
+         */
+        public function test_should_match_nested_page_query_count_status() {
+                $p1 = self::factory()->post->create(
+                        array(
+                                'post_type'   => 'page',
+                                'post_name'   => 'foo',
+                                'post_status' => 'draft',
+                        )
+                );
+
+                $p2 = self::factory()->post->create(
+                        array(
+                                'post_type'   => 'page',
+                                'post_name'   => 'bar',
+                                'post_parent' => $p1,
+                        )
+                );
+
+                $p3 = self::factory()->post->create(
+                        array(
+                                'post_type'   => 'page',
+                                'post_name'   => 'baz',
+                                'post_parent' => $p2,
+                        )
+                );
+
+                $queries_before = get_num_queries();
+                $found          = get_page_by_path( 'foo/bar/baz' );
+                $queries_after  = get_num_queries();
+                $cached_post    = wp_cache_get( $p1, 'posts' );
+
+                $this->assertSame( 1, $queries_after - $queries_before, 'Only one query should run' );
+                $this->assertSame( $p3, $found->ID, 'Check to see if the result is correct' );
+                $this->assertIsObject( $cached_post, 'The cached post is not an object' );
+        }
+
+        /**
+         * @ticket 56689
+         *
+         * @covers ::get_page_by_path
+         */
+        public function test_should_return_null_for_invalid_path() {
+                $queries_before = get_num_queries();
+                $get_1          = get_page_by_path( 'should/return/null/for/an/invalid/path' );
+                $get_2          = get_page_by_path( 'should/return/null/for/an/invalid/path' );
+                $queries_after  = get_num_queries();
+
+                $this->assertNull( $get_1, 'Invalid path should return null.' );
+                $this->assertSame( 1, $queries_after - $queries_before, 'Only one query should run.' );
+                $this->assertSame( $get_1, $get_2, 'The cached result should be the same as the uncached result.' );
+        }
+
         public function test_should_not_make_partial_match() {
                 $p1 = self::factory()->post->create(