Changeset 53333

Timestamp:

05/02/2022 01:58:48 PM (2 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
                         }
                     }