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.

git-svn-id: https://develop.svn.wordpress.org/trunk@53333 602fd350-edb4-49c9-b593-d223f7449a82

Author: hellofromtonya

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

@@ -119,16 +119,7 @@ public function get_items( $request ) {
 			$query_args['slug'] = $slug;
 		}
 
-		/*
-		 * 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 );
 
 		/*
 		 * Use network-wide transient to improve performance. The locale is the only site
@@ -337,6 +328,11 @@ public function get_collection_params() {
 			'minimum'     => 1,
 		);
 
+		$query_params['slug'] = array(
+			'description' => __( 'Limit results to those matching a pattern (slug).' ),
+			'type'        => 'array',
+		);
+
 		/**
 		 * Filter collection parameters for the block pattern directory controller.
 		 *
@@ -346,4 +342,36 @@ public function get_collection_params() {
 		 */
 		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 ) );
+	}
 }

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

@@ -21,6 +21,15 @@ class WP_REST_Pattern_Directory_Controller_Test extends WP_Test_REST_Controller_
 	 */
 	protected static $contributor_id;
 
+	/**
+	 * 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.
 	 *
@@ -34,6 +43,8 @@ public static function wpSetUpBeforeClass( $factory ) {
 				'role' => 'contributor',
 			)
 		);
+
+		static::$controller = new WP_REST_Pattern_Directory_Controller();
 	}
 
 	/**
@@ -42,7 +53,7 @@ public static function wpSetUpBeforeClass( $factory ) {
 	 * @param WP_REST_Response[] $pattern An individual pattern from the REST API response.
 	 */
 	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}';
 
 		$this->assertTrue(
@@ -322,12 +333,11 @@ public function test_delete_item() {
 	 * @since 5.8.0
 	 */
 	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() )
 		);
 
 		$this->assertPatternMatchesSchema( $prepared_pattern );
@@ -340,12 +350,11 @@ public function test_prepare_item() {
 	 * @since 5.8.0
 	 */
 	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() )
 		);
 
 		$this->assertPatternMatchesSchema( $prepared_pattern );
@@ -399,6 +408,96 @@ 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,
+			),
+		);
+	}
+
 	/**
 	 * Simulate a successful outbound HTTP requests, to keep tests pure and performant.
 	 *

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

@@ -10385,6 +10385,11 @@ mockedApiResponse.Schema = {
                             "type": "integer",
                             "minimum": 1,
                             "required": false
+                        },
+                        "slug": {
+                            "description": "Limit results to those matching a pattern (slug).",
+                            "type": "array",
+                            "required": false
                         }
                     }
                 }