`/wp/v2/pattern-directory/patterns` endpoint: slug parameter has no effect on the response #2625

anton-vlasenko · 2022-04-25T16:08:23Z

Trac ticket: https://core.trac.wordpress.org/ticket/55617

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

We must also implode slugs as implode doesn't work with multi-dimensional arrays.

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

Strings will be automatically converted to an array. Per suggestion here: WordPress#2625 (comment)

…nsient_key method as it would be helpful to make it available to child classes.

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

…supposed to be null.

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

spacedmonkey · 2022-04-27T13:34:11Z

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

+			}
+		}
+
+		return 'wp_remote_block_patterns_' . md5( implode( '-', $query_args ) );


Suggested change

return 'wp_remote_block_patterns_' . md5( implode( '-', $query_args ) );

return 'wp_remote_block_patterns_' . md5( serialize( $query_args ) );

Yes, I've been thinking about replacing implode with wp_json_encode (json_encode is faster than serialize) in the scope of another PR, but serialize will do too.
The result produced by implode doesn't depend on the keys of the $query_args array. Therefore, this increases the chance of collisions.
Fixed in 2c08eea
P.S. I didn't change it because I was afraid that it would cause a sharp increase in the number of requests to the wp/v2/pattern-directory/patterns endpoint once WordPress 6.0 is released. The old values stored in the cache will be invalidated because this PR replaces implode with serialize.
I hope it will not happen since not all WordPress sites are updated simultaneously.

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

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

spacedmonkey

There are a number of changes required here.

…an array.

Use WP_REST_Pattern_Directory_Controller_Test::setUp to set up the controller.

It seems there is some issue with the ::setUp method.

…n request keys.

anton-vlasenko · 2022-04-27T18:03:14Z

There are a number of changes required here.

Thank you for the review, @spacedmonkey!
I've replied to all code review comments.
I'd appreciate the second round of the review.

spacedmonkey

LGTM

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

…s_get_generated_correctly: add error messages.

costdev

LGTM 👍

ndiego · 2022-05-02T11:41:57Z

@anton-vlasenko I'm working on cleaning up the 6.0 Project Board. Are we waiting on anything else for this PR?

anton-vlasenko · 2022-05-02T12:07:54Z

@anton-vlasenko I'm working on cleaning up the 6.0 Project Board. Are we waiting on anything else for this PR?

@ndiego
From a technical point of view, this PR is ready to be merged.
2 core commiters have approved it.

I don't know if there are any problems from an organizational point of view.
I'd be grateful if you could please merge it (I don't know if you have the permission to merge to Beta 4).
Thank you.

cc @peterwilsoncc

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

…#2625 (comment)

hellofromtonya

Ready for commit.

hellofromtonya · 2022-05-02T14:01:04Z

Committed via changeset https://core.trac.wordpress.org/changeset/53333.

…ay arguments (#40900) * Backport WordPress/wordpress-develop#2591 to Gutenberg. * Backport WordPress/wordpress-develop#2625 to Gutenberg. * Update PHPDoc block. * Fix comments * Replace WP with WordPress.

anton-vlasenko added 9 commits April 25, 2022 12:49

Fix a bug in the WP_REST_Pattern_Directory_Controller:get_items.

7571960

We must also implode slugs as implode doesn't work with multi-dimensional arrays.

Update ticket reference.

f06d3d0

Fix code style.

b504030

Add slug to the collection parameters. Previously, it was missing.

b1598ab

Improve PHPDOC blocks.

e284e1e

Update wp-api-generated.js schema.

7fb829d

Update wp-api-generated.js schema.

07aa848

Update wp-api-generated.js schema.

fd1642d

Update comment.

5c9a336

TimothyBJacobs reviewed Apr 25, 2022

View reviewed changes

src/wp-includes/rest-api/endpoints/class-wp-rest-pattern-directory-controller.php Outdated Show resolved Hide resolved

"Slug" must be of type array.

5e923bc

Strings will be automatically converted to an array. Per suggestion here: WordPress#2625 (comment)

anton-vlasenko requested a review from TimothyBJacobs April 26, 2022 09:18

Change the scope of the WP_REST_Pattern_Directory_Controller::get_tra…

c2122fb

…nsient_key method as it would be helpful to make it available to child classes.

TimothyBJacobs reviewed Apr 26, 2022

View reviewed changes

src/wp-includes/rest-api/endpoints/class-wp-rest-pattern-directory-controller.php Outdated Show resolved Hide resolved

TimothyBJacobs reviewed Apr 26, 2022

View reviewed changes

src/wp-includes/rest-api/endpoints/class-wp-rest-pattern-directory-controller.php Outdated Show resolved Hide resolved

Replace array_key_exist with isset because $query_args['slug'] isn't …

005de4a

…supposed to be null.

anton-vlasenko commented Apr 26, 2022

View reviewed changes

src/wp-includes/rest-api/endpoints/class-wp-rest-pattern-directory-controller.php Outdated Show resolved Hide resolved

anton-vlasenko requested a review from TimothyBJacobs April 26, 2022 23:43

spacedmonkey self-requested a review April 27, 2022 13:29

spacedmonkey reviewed Apr 27, 2022

View reviewed changes

src/wp-includes/rest-api/endpoints/class-wp-rest-pattern-directory-controller.php Outdated Show resolved Hide resolved

spacedmonkey reviewed Apr 27, 2022

View reviewed changes

tests/phpunit/tests/rest-api/rest-pattern-directory-controller.php Outdated Show resolved Hide resolved

spacedmonkey reviewed Apr 27, 2022

View reviewed changes

tests/phpunit/tests/rest-api/rest-pattern-directory-controller.php Show resolved Hide resolved

spacedmonkey reviewed Apr 27, 2022

View reviewed changes

tests/phpunit/tests/rest-api/rest-pattern-directory-controller.php Outdated Show resolved Hide resolved

spacedmonkey reviewed Apr 27, 2022

View reviewed changes

tests/phpunit/tests/rest-api/rest-pattern-directory-controller.php Outdated Show resolved Hide resolved

spacedmonkey reviewed Apr 27, 2022

View reviewed changes

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

spacedmonkey requested changes Apr 27, 2022

View reviewed changes

anton-vlasenko added 3 commits April 27, 2022 16:15

Pass $query_args['slugs'] through wp_parse_list to ensure that it is …

0a2c2eb

…an array.

Refactor WP_REST_Pattern_Directory_Controller_Test.

7d1c512

Use WP_REST_Pattern_Directory_Controller_Test::setUp to set up the controller.

Refactor the test to use the static $controller property.

1257482

It seems there is some issue with the ::setUp method.

Use serialize instead of implode because transient keys must depend o…

2c08eea

…n request keys.

anton-vlasenko requested a review from spacedmonkey April 27, 2022 18:12

spacedmonkey approved these changes Apr 28, 2022

View reviewed changes

costdev reviewed Apr 29, 2022

View reviewed changes

tests/phpunit/tests/rest-api/rest-pattern-directory-controller.php Outdated Show resolved Hide resolved

anton-vlasenko added 2 commits April 29, 2022 15:49

Improve WP_REST_Pattern_Directory_Controller_Test::test_transient_key…

0411a81

…s_get_generated_correctly: add error messages.

Improve WP_REST_Pattern_Directory_Controller_Test::test_transient_key…

5653f25

…s_get_generated_correctly: add error messages.

gziolo assigned anton-vlasenko Apr 29, 2022

anton-vlasenko added 2 commits April 29, 2022 16:20

Update the test per @costdev 's suggestion.

0879fe0

Update the test per @costdev 's suggestion.

8ec23a0

costdev approved these changes Apr 29, 2022

View reviewed changes

hellofromtonya reviewed May 2, 2022

View reviewed changes

src/wp-includes/rest-api/endpoints/class-wp-rest-pattern-directory-controller.php Outdated Show resolved Hide resolved

Micro-optimisations per @hellofromtonya 's suggestion here: WordPress…

1d8f893

…#2625 (comment)

hellofromtonya approved these changes May 2, 2022

View reviewed changes

hellofromtonya closed this May 2, 2022

anton-vlasenko mentioned this pull request May 4, 2022

Backport fixes for wp/v2/block-patterns/patterns and /wp/v2/pattern-directory/patterns endpoints to Gutenberg WordPress/gutenberg#40824

Closed

3 tasks

anton-vlasenko added a commit to WordPress/gutenberg that referenced this pull request May 5, 2022

Backport WordPress/wordpress-develop#2625 to Gutenberg.

c1b3456

anton-vlasenko mentioned this pull request May 6, 2022

WP_REST_Pattern_Directory_Controller::get_items doesn't support array arguments WordPress/gutenberg#40900

Merged

WordPress deleted a comment Apr 24, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`/wp/v2/pattern-directory/patterns` endpoint: slug parameter has no effect on the response #2625

`/wp/v2/pattern-directory/patterns` endpoint: slug parameter has no effect on the response #2625

anton-vlasenko commented Apr 25, 2022

spacedmonkey Apr 27, 2022

anton-vlasenko Apr 27, 2022 •

edited

Loading

spacedmonkey left a comment

anton-vlasenko commented Apr 27, 2022

spacedmonkey left a comment

costdev left a comment

ndiego commented May 2, 2022 •

edited

Loading

anton-vlasenko commented May 2, 2022

hellofromtonya left a comment

hellofromtonya commented May 2, 2022

	return 'wp_remote_block_patterns_' . md5( implode( '-', $query_args ) );
	return 'wp_remote_block_patterns_' . md5( serialize( $query_args ) );

/wp/v2/pattern-directory/patterns endpoint: slug parameter has no effect on the response #2625

/wp/v2/pattern-directory/patterns endpoint: slug parameter has no effect on the response #2625

Conversation

anton-vlasenko commented Apr 25, 2022

spacedmonkey Apr 27, 2022

Choose a reason for hiding this comment

anton-vlasenko Apr 27, 2022 • edited Loading

Choose a reason for hiding this comment

spacedmonkey left a comment

Choose a reason for hiding this comment

anton-vlasenko commented Apr 27, 2022

spacedmonkey left a comment

Choose a reason for hiding this comment

costdev left a comment

Choose a reason for hiding this comment

ndiego commented May 2, 2022 • edited Loading

anton-vlasenko commented May 2, 2022

hellofromtonya left a comment

Choose a reason for hiding this comment

hellofromtonya commented May 2, 2022

`/wp/v2/pattern-directory/patterns` endpoint: slug parameter has no effect on the response #2625

`/wp/v2/pattern-directory/patterns` endpoint: slug parameter has no effect on the response #2625

anton-vlasenko Apr 27, 2022 •

edited

Loading

ndiego commented May 2, 2022 •

edited

Loading