Changeset 53333

Timestamp:

05/02/2022 01:58:48 PM (4 years ago)

Author:

hellofromTonya

Message:

REST API: Fixes /wp/v2/pattern-directory/patterns endpoint response for slug parameter.

[53218] introduced a bug of a wrong response from the wp/v2/pattern-directory/patterns endpoint with a slug parameter. As the response is cached, it can result in an incorrect list of available patterns supported by the current theme.

This commit resolves by:

• Limiting the slug to an array in the query parameters.
• When set, parsing and sorting the slug(s) and then serializing the sorted query args as part of the hashed transient keys.

Props antonvlasenko, timothyblynjacobs, spacedmonkey, costdev, hellofromTonya.

Follow-up to [53218], [53152], [51208].
Fixes #55617.

Location:

trunk

Files:

• src/wp-includes/rest-api/endpoints/class-wp-rest-pattern-directory-controller.php (modified) (3 diffs)
• tests/phpunit/tests/rest-api/rest-pattern-directory-controller.php (modified) (6 diffs)
• tests/qunit/fixtures/wp-api-generated.js (modified) (1 diff)

trunk/src/wp-includes/rest-api/endpoints/class-wp-rest-pattern-directory-controller.php

@@ -120 +120 @@
                 }
 
-                /*
-                 * Include a hash of the query args, so that different requests are stored in
-                 * separate caches.
-                 *
-                 * MD5 is chosen for its speed, low-collision rate, universal availability, and to stay
-                 * under the character limit for `_site_transient_timeout_{...}` keys.
-                 *
-                 * @link https://stackoverflow.com/questions/3665247/fastest-hash-for-non-cryptographic-uses
-                 */
-                $transient_key = 'wp_remote_block_patterns_' . md5( implode( '-', $query_args ) );
+                $transient_key = $this->get_transient_key( $query_args );
 
                 /*
@@ -338 +329 @@
                 );
 
+                $query_params['slug'] = array(
+                        'description' => __( 'Limit results to those matching a pattern (slug).' ),
+                        'type'        => 'array',
+                );
+
                 /**
                  * Filter collection parameters for the block pattern directory controller.
@@ -347 +343 @@
                 return apply_filters( 'rest_pattern_directory_collection_params', $query_params );
         }
+
+        /*
+         * Include a hash of the query args, so that different requests are stored in
+         * separate caches.
+         *
+         * MD5 is chosen for its speed, low-collision rate, universal availability, and to stay
+         * under the character limit for `_site_transient_timeout_{...}` keys.
+         *
+         * @link https://stackoverflow.com/questions/3665247/fastest-hash-for-non-cryptographic-uses
+         *
+         * @since 6.0.0
+         *
+         * @param array $query_args Query arguments to generate a transient key from.
+         * @return string Transient key.
+         */
+        protected function get_transient_key( $query_args ) {
+
+                if ( isset( $query_args['slug'] ) ) {
+                        // This is an additional precaution because the "sort" function expects an array.
+                        $query_args['slug'] = wp_parse_list( $query_args['slug'] );
+
+                        // Empty arrays should not affect the transient key.
+                        if ( empty( $query_args['slug'] ) ) {
+                                unset( $query_args['slug'] );
+                        } else {
+                                // Sort the array so that the transient key doesn't depend on the order of slugs.
+                                sort( $query_args['slug'] );
+                        }
+                }
+
+                return 'wp_remote_block_patterns_' . md5( serialize( $query_args ) );
+        }
 }

trunk/tests/phpunit/tests/rest-api/rest-pattern-directory-controller.php

@@ -23 +23 @@
 
         /**
+         * An instance of WP_REST_Pattern_Directory_Controller class.
+         *
+         * @since 6.0.0
+         *
+         * @var WP_REST_Pattern_Directory_Controller
+         */
+        private static $controller;
+
+        /**
          * Set up class test fixtures.
          *
@@ -35 +44 @@
                         )
                 );
+
+                static::$controller = new WP_REST_Pattern_Directory_Controller();
         }
 
@@ -43 +54 @@
          */
         public function assertPatternMatchesSchema( $pattern ) {
-                $schema     = ( new WP_REST_Pattern_Directory_Controller() )->get_item_schema();
+                $schema     = static::$controller->get_item_schema();
                 $pattern_id = isset( $pattern->id ) ? $pattern->id : '{pattern ID is missing}';
 
@@ -323 +334 @@
          */
         public function test_prepare_item() {
-                $controller                   = new WP_REST_Pattern_Directory_Controller();
                 $raw_patterns                 = json_decode( self::get_raw_response( 'browse-all' ) );
                 $raw_patterns[0]->extra_field = 'this should be removed';
 
-                $prepared_pattern = $controller->prepare_response_for_collection(
-                        $controller->prepare_item_for_response( $raw_patterns[0], new WP_REST_Request() )
+                $prepared_pattern = static::$controller->prepare_response_for_collection(
+                        static::$controller->prepare_item_for_response( $raw_patterns[0], new WP_REST_Request() )
                 );
 
@@ -341 +351 @@
          */
         public function test_prepare_item_search() {
-                $controller                   = new WP_REST_Pattern_Directory_Controller();
                 $raw_patterns                 = json_decode( self::get_raw_response( 'search' ) );
                 $raw_patterns[0]->extra_field = 'this should be removed';
 
-                $prepared_pattern = $controller->prepare_response_for_collection(
-                        $controller->prepare_item_for_response( $raw_patterns[0], new WP_REST_Request() )
+                $prepared_pattern = static::$controller->prepare_response_for_collection(
+                        static::$controller->prepare_item_for_response( $raw_patterns[0], new WP_REST_Request() )
                 );
 
@@ -398 +407 @@
         public function test_get_item_schema() {
                 $this->markTestSkipped( "The controller's schema is hardcoded, so tests would not be meaningful." );
+        }
+
+        /**
+         * Tests if the transient key gets generated correctly.
+         *
+         * @dataProvider data_get_query_parameters
+         *
+         * @covers WP_REST_Pattern_Directory_Controller::get_transient_key
+         *
+         * @since 6.0.0
+         *
+         * @ticket 55617
+         *
+         * @param array     $parameters_1   Expected query arguments.
+         * @param array     $parameters_2   Actual query arguments.
+         * @param string    $message        An error message to display.
+         * @param bool      $assert_same    Assertion type (assertSame vs assertNotSame).
+         */
+        public function test_transient_keys_get_generated_correctly( $parameters_1, $parameters_2, $message, $assert_same = true ) {
+                $reflection_method = new ReflectionMethod( static::$controller, 'get_transient_key' );
+                $reflection_method->setAccessible( true );
+
+                $result_1 = $reflection_method->invoke( self::$controller, $parameters_1 );
+                $result_2 = $reflection_method->invoke( self::$controller, $parameters_2 );
+
+                $this->assertIsString( $result_1, 'Transient key #1 must be a string.' );
+                $this->assertNotEmpty( $result_1, 'Transient key #1 must not be empty.' );
+
+                $this->assertIsString( $result_2, 'Transient key #2 must be a string.' );
+                $this->assertNotEmpty( $result_2, 'Transient key #2 must not be empty.' );
+
+                if ( $assert_same ) {
+                        $this->assertSame( $result_1, $result_2, $message );
+                } else {
+                        $this->assertNotSame( $result_1, $result_2, $message );
+                }
+
+        }
+
+        /**
+         * @since 6.0.0
+         *
+         * @ticket 55617
+         */
+        public function data_get_query_parameters() {
+                return array(
+                        'same key and empty slugs'              => array(
+                                'parameters_1' => array(
+                                        'parameter_1' => 1,
+                                        'slug'        => array(),
+                                ),
+                                'parameters_2' => array(
+                                        'parameter_1' => 1,
+                                ),
+                                'message'      => 'Empty slugs should not affect the transient key.',
+                        ),
+                        'same key and slugs in different order' => array(
+                                'parameters_1' => array(
+                                        'parameter_1' => 1,
+                                        'slug'        => array( 0, 2 ),
+                                ),
+                                'parameters_2' => array(
+                                        'parameter_1' => 1,
+                                        'slug'        => array( 2, 0 ),
+                                ),
+                                'message'      => 'The order of slugs should not affect the transient key.',
+                        ),
+                        'same key and different slugs'          => array(
+                                'parameters_1' => array(
+                                        'parameter_1' => 1,
+                                        'slug'        => array( 'some_slug' ),
+                                ),
+                                'parameters_2' => array(
+                                        'parameter_1' => 1,
+                                        'slug'        => array( 'some_other_slug' ),
+                                ),
+                                'message'      => 'Transient keys must not match.',
+                                false,
+                        ),
+                        'different keys'                        => array(
+                                'parameters_1' => array(
+                                        'parameter_1' => 1,
+                                ),
+                                'parameters_2' => array(
+                                        'parameter_2' => 1,
+                                ),
+                                'message'      => 'Transient keys must depend on array keys.',
+                                false,
+                        ),
+                );
         }
 

trunk/tests/qunit/fixtures/wp-api-generated.js

@@ -10386 +10386 @@
                             "minimum": 1,
                             "required": false
+                        },
+                        "slug": {
+                            "description": "Limit results to those matching a pattern (slug).",
+                            "type": "array",
+                            "required": false
                         }
                     }