Implement `WP_REST_Search_Controller` class #6489

felixarntz · 2018-04-29T16:46:34Z

Description

This PR aims at introducing a WP_REST_Search_Controller class to allow searching across multiple post types via the REST API. This will allow powering the link modal and other related ideas where results must not be restricted to a single post type.

While this PR should be implemented and evaluated here, the final implementation will then be merged back into core, with the gutenberg/v1 namespace changed to wp/v2. See https://core.trac.wordpress.org/ticket/39965 for the related ticket.

This addresses #2084.

How has this been tested?

For now I've only performed a few API calls to test things work as expected. I plan on writing unit tests later this week.

Types of changes

WP_REST_Search_Controller is introduced, and its routes are registered.
There is only a collection route here, as individual items can be accessed at their respective endpoints. Links to these singular resources are added in the response per item, however this currently only works with post types using the default WP_REST_Posts_Controller.
Only post types that have both public and show_in_rest set to true are searched. The post types to search can be further restricted with a type request parameter.
Only post statuses that have public set to true, plus inherit are searched.
The schema consists of only general fields that exist for every post. No metadata is included. Fields where the content may not actually be supported per post type are still included (with empty content), to have a consistent schema.
A major part of the implementation has been copied from core's WP_REST_Posts_Controller that already has a lot of the necessary functionality implemented. It's still too different though to extend it. We may wanna consider outsourcing some common functionality, but let's keep it simple for now.

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
My code has proper inline documentation.

…st types.

…port for individual search results.

Soean · 2018-04-30T07:19:26Z

lib/class-wp-rest-search-controller.php

+	/**
+	 * Gets the post statuses allowed for search.
+	 *
+	 * @since 1.0.0


since 2.9.0

Soean · 2018-04-30T07:19:50Z

lib/class-wp-rest-search-controller.php

+	/**
+	 * Gets the post types allowed for search.
+	 *
+	 * @since 1.0.0


since 2.9.0

felixarntz · 2018-04-30T09:39:52Z

Note that the test failure comes from not including the 'gutenberg' text domain in translation strings. Since we're gonna move this over to core once it's done and not merge it into the plugin, I think we can ignore this?

danielbachhuber · 2018-04-30T11:53:16Z

Since we're gonna move this over to core once it's done and not merge it into the plugin, I think we can ignore this?

Let's go ahead and include them for now. They're easy enough to remove after the fact.

danielbachhuber · 2018-04-30T11:57:04Z

lib/class-wp-rest-search-controller.php

+		// Retrieve the list of registered collection query parameters.
+		$registered = $this->get_collection_params();
+		$query_args = array(
+			'post_status'         => array_keys( $this->get_allowed_post_stati() ),


wp_link_query uses post_status=>publish. Is there a reason we're opening things up?

Any post status that identifies as its content as public should be searched. That is publish only in most cases, but plugins may add to that.

I also included inherit to be able to search for attachments. If that should not be possible, I'd be okay with removing that status.

We should focus on only supporting post_status=publish for now.

Support for post_status=inherit opens up a can of permissions-related worms. Custom post statuses should be handled in conjunction with #3144

danielbachhuber · 2018-04-30T12:01:26Z

lib/class-wp-rest-search-controller.php

+			'$schema'    => 'http://json-schema.org/draft-04/schema#',
+			'title'      => 'search',
+			'type'       => 'object',
+			'properties' => array(


I think we should drop: author, content, excerpt, date, date_gmt, guid, modified, modified_gmt, and status for a couple of reasons:

They aren't absolutely necessary for link search, which is the immediate problem we need to solve.

They'll significantly reduce complexity for the controller.

I disagree as this is supposed to be merged into core. A REST controller in core should be available to cover common use-cases beyond Gutenberg. If this was a first test for Gutenberg internally, I'd be okay with using only the few fields needed for that very case, but for core, the typical properties expected should be available.

Furthermore, they don't really add complexity, but just increase the response. I'll implement the new _fields filtering so that those fields are not even generated when they're not needed. Gutenberg can then make a request like wp/v2/search?search=mytext&_fields=id,title,type,link.

I'd be okay with using only the few fields needed for that very case, but for core, the typical properties expected should be available.

Can you explain how you define "typical properties", and where, say, modified_gmt fits in this category?

I'd define as typical properties those that exist on every post regardless of post type, i.e. things that are part of the wp_posts database table. In that regard I'd also like to align the format with that from the other post type-specific controllers.

felixarntz · 2018-05-03T09:46:53Z

With the changes included in the latest commit, only item data for the fields requested is generated. This means that a core version later than https://core.trac.wordpress.org/changeset/43087 is required to run this PR.

…detected by tests.

felixarntz · 2018-05-05T11:11:02Z

311413e provides tests for the whole controller functionality.

d7b1762 introduces a type_exclude parameter, allowing post types to be excluded from the search.

I think this is now in a solid state to be considered for core merge. For Gutenberg and the link modal specifically, a request wp/v2/search?search=mytext&type_exclude=attachment&_fields=id,title,type,link will do what's necessary to replicate the current logic.

felixarntz · 2018-05-05T11:30:37Z

Remaining test errors are only occurring because Gutenberg tests against the latest WordPress version, not against trunk. In core, all of this would work (again, since it requires https://core.trac.wordpress.org/changeset/43087).

felixarntz · 2018-05-09T14:22:43Z

After talking to @danielbachhuber, it became clear that there are essentially two possible approaches for implementing a search endpoint:

Either an endpoint that is tailored to searching all posts. That is what's currently implemented here.
Or an endpoint that is tailored to searching all content of your WordPress installation, which by default would still only search posts, but allow for extension by plugins. You could theoretically also search terms, comments, users, anything. Such an endpoint would need to contain a more limited amount of fields, namely id, title and link - arguably also type because it would no longer be clear that the current resource is a "post". If someone needed the full response objects in a request, that could easily be possible via the _embed parameter.

For Gutenberg itself, a simple endpoint that could be just like the one implemented here without the unneeded schema properties and collection params would be sufficient. The good thing is that that version will allow to later shape it into either of the two directions. However, for core, we should decide whether we move forward with the first or the second approach.
That would be a good discussion to have in one of the next REST API meetings.

…ndpoint

felixarntz · 2018-05-23T08:55:38Z

@danielbachhuber I made all changes you suggest, except changing the post statuses because I don't think it makes sense to get rid of them (see #6489 (comment)) - flagging this here since GH no longer shows it because the lines you commented have changed.

The gist is: Both custom post statuses and inherit are nothing new in the REST API - while Gutenberg doesn't necessarily need them for now, this controller is not exclusive to Gutenberg, and especially since post statuses in this regard are nothing we need to think about how to implement them, they should be included. #3144 is unrelated I think, since it deals with support custom post statuses in Gutenberg and its UI itself.

felixarntz · 2018-05-23T09:42:31Z

With the latest commit, I changed the UrlInput component to use the new search controller so that now posts of other post types are available as well.

danielbachhuber · 2018-05-23T11:49:26Z

@felixarntz Looking pretty good. It doesn't look like it searches against attachment in Gutenberg though? Can you explain why status=inherit support is necessary if we aren't using it?

If it is necessary, we'll need unit test coverage for it, including permissions checks.

felixarntz · 2018-05-23T13:19:32Z

@danielbachhuber

Can you explain why status=inherit support is necessary if we aren't using it?

I decided to exclude attachments from the search because they aren't currently searched in the link modal. We could change that if that is preferred, but regardless of that decision I think the controller should support attachments as this is going to be merged into WordPress core, and search is not only used in Gutenberg. I can add tests for it too.

danielbachhuber · 2018-05-23T13:20:56Z

We could change that if that is preferred, but regardless of that decision I think the controller should support attachments as this is going to be merged into WordPress core, and search is not only used in Gutenberg.

Where else is search used in WordPress core?

felixarntz · 2018-05-23T13:24:26Z

Where else is search used in WordPress core?

The main search functionality (search form in frontend) could use this for example. Plugins can use this. By making this only tailored to Gutenberg's very use-case, we force developers to unnecessarily rewrite a lot of code for only simple changes. Especially since this is nothing complicated and has been solved before, I don't think it's worth keeping it out.

danielbachhuber · 2018-05-23T13:43:45Z

The main search functionality (search form in frontend) could use this for example.

Does it?

Plugins can use this.

Can plugins write their own code to modify the behavior of this controller?

By making this only tailored to Gutenberg's very use-case, we force developers to unnecessarily rewrite a lot of code for only simple changes.

Can you share the code that's necessary to rewrite?

Especially since this is nothing complicated and has been solved before, I don't think it's worth keeping it out.

This is good read on the topic. Notably, features should be added based on genuine need, not hypothetical.

Based on the information presented, support for searching attachments doesn't appear necessary for our current use-case.

felixarntz · 2018-05-23T14:13:41Z

Does it?

Not yet obviously, but it would be a good use-case for this controller.

Can plugins write their own code to modify the behavior of this controller?

They cannot filter these things, but they can use the controller by making a request to it. If a plugin wants to search content including attachments, this controller would be worthless to them without it.

This is good read on the topic. Notably, features should be added based on genuine need, not hypothetical.

The WordPress search is a valid use-case for this controller, and without attachments it would be incomplete. What is the point of introducing a REST API controller that searches posts, but doesn't actually work for all posts? In addition, even a post of another post type can have a status of inherit in which case its parent post would determine the actual status. To quote the post, we should be "thinking of all users in the big picture" instead of just Gutenberg.

danielbachhuber · 2018-05-23T14:26:10Z

They cannot filter these things, but they can use the controller by making a request to it. If a plugin wants to search content including attachments, this controller would be worthless to them without it.

Plugins can't filter the behavior of this controller to adapt it to their needs?

The WordPress search is a valid use-case for this controller, and without attachments it would be incomplete.

"without attachments it would be incomplete" seems to communicate a hypothetical future, not the current reality.

To my knowledge, and correct me if I'm wrong, WordPress frontend search doesn't include attachments by default. Am I wrong?

What is the point of introducing a REST API controller that searches posts, but doesn't actually work for all posts?

It seems to solve our immediate needs for link search just fine.

In addition, even a post of another post type can have a status of inherit in which case its parent post would determine the actual status.

Can you point me to a real-world example?

To quote the post, we should be "thinking of all users in the big picture" instead of just Gutenberg.

This statement seems to cherry-pick a quote and ignore the main point of the post.

felixarntz · 2018-05-23T15:06:22Z

Plugins can't filter the behavior of this controller to adapt it to their needs?

It's currently not possible to filter things like the supported collection request parameters and how content is searched. Let me know if this should be added.

This statement seems to cherry-pick a quote and ignore the main point of the post.

I read the post and understand what it says, we apparently interpret it differently.

Anyway, the recent bits of the discussion seem to not move this forward. I think we need a third opinion on the topic of flexibility vs pragmatism here.

danielbachhuber · 2018-05-23T15:08:09Z

It's currently not possible to filter things like the supported collection request parameters and how content is searched. Let me know if this should be added.

Sure, filters would be a great addition.

I think we need a third opinion on the topic of flexibility vs pragmatism here.

Sounds good. cc @pento

danielbachhuber · 2018-05-25T13:44:39Z

It's currently not possible to filter things like the supported collection request parameters and how content is searched. Let me know if this should be added.

Sure, filters would be a great addition.

This was specifically requested in #4598

danielbachhuber · 2018-06-27T12:52:46Z

@felixarntz Are you alright with removing status for now so we can finish up this pull request and land it?

felixarntz · 2018-07-01T23:10:23Z

I talked with @rmccue for a while at WCEU, and we came up with a plan for this:

We agree that the search controller should not just replicate what the posts controller does, without the post type-specific stuff.
We also agree that a controller just called /search should be intended as a universal search controller, for all WordPress content, not only posts.
While it is currently impossible to search posts/terms/comments etc. altogether at once, we should implement the controller in a future-proof way that permits this. The first iteration would be okay to only support posts, closely followed by an iteration that should add support for terms and comments for individual searches as well.
We need to return data in a way that can be uniform for all content, as we of course need one uniform schema. We are thinking about:
- id
- title
- url
- type
- subtype
- possibly also content
As you can see, there are both a type and subtype in the above list. That is because these two values are necessary to fully identify what kind of content a certain item is. This is in line with the behavior for metadata after https://core.trac.wordpress.org/changeset/43378. For example, a type could be “post”, “term” or “comment”. A subtype could be “post”, “page”, “attachment”, “category” etc.
The request parameters available should be:
- search: Obvious, search string to look for.
- type: Allows to filter the type looked for. Initially, this will only support a single string, either “post”, “term” or “comment”. The default will be “post”. Alternatively, if we decide to only support posts for the first iteration, we can of course introduce “term” and “comment” later too. Having this parameter is future-proof as we could in theory at some point make it an array to allow cross-search between posts, terms etc., or even allow it to be empty and then search everything there is.
- subtype: Allows to filter the subtype looked for. This should support an array of subtypes (post types, taxonomies). The default value should be empty, in order to search everything identified by type. Since this parameter is a bit more complex, we might be okay leaving it out of the first iteration.
- plus the typical page and per_page parameters for pagination
An issue to figure out is the sort order. When searching posts, you often wanna sort them by date. However, not all types have a date. This is more complex, so we might be okay initially always sorting by the id.
Due to this being not a Gutenberg-exclusive enhancement, we suggest continuing work on this in the core ticket https://core.trac.wordpress.org/ticket/39965. Because it is pressing for Gutenberg though, the goal would be to have it be merged and backported as soon as possible, for one of the next minor releases.

danielbachhuber · 2018-07-01T23:55:42Z

we came up with a plan for this:

👍 Works for me.

Because it is pressing for Gutenberg though, the goal would be to have it be merged and backported as soon as possible, for one of the next minor releases.

One nice thing about landing in Gutenberg early is that we can iterate on the endpoint based on real-world usage, and then commit to core once we think it's finalized. I'd prefer to follow this approach; however, if you prefer to work on the endpoint design via Trac patches that sounds reasonable to me.

pento · 2018-07-03T05:04:39Z

I'm inclined to agree with @danielbachhuber, let's get this into Gutenberg first, iterating on Trac is slow and annoying. Once we have Gutenberg's usage locked down, we'll be in a good place to look at expanding it to more data types.

felixarntz · 2018-07-03T09:37:02Z

Sounds good to me. Will work on the new version either this weekend at WC Sevilla Contributor Day, or early next week. I'm inclined to create a new PR though and close this one, only referencing it from there. Is that good with you?

pento · 2018-07-10T23:28:36Z

Hey, @felixarntz: have you had a chance to look at this?

felixarntz · 2018-07-11T01:00:06Z

@pento I will tomorrow!

rmccue · 2018-07-11T02:11:05Z

Just chiming in to note that @felixarntz summarised the conversation well. The key point for the type/subtype bit is just making sure that we have forward compatibility for the changes we might want to make down the line.

Agreed with landing in Gutenberg first for iteration speed as well.

felixarntz · 2018-07-11T15:59:40Z

@danielbachhuber @pento @rmccue I opened #7894 for the new implementation of the global search controller. Let's continue there.

uamv · 2020-08-12T12:16:30Z

It's currently not possible to filter things like the supported collection request parameters and how content is searched. Let me know if this should be added.

Sure, filters would be a great addition.

This was specifically requested in #4598

Were any filters introduced here? I can't seem to locate any in core.

Felix Arntz added 4 commits April 29, 2018 18:19

Implement WP_REST_Search_Controller for API search across multiple po…

c16d7c5

…st types.

Include inherit post status in search.

d8afc44

Ensure read access to an individual search result is correctly checked.

d749b71

Only handle title, content, excerpt and author based on post type sup…

62b1059

…port for individual search results.

felixarntz self-assigned this Apr 29, 2018

felixarntz added [Status] In Progress Tracking issues with work in progress Needs Tests labels Apr 29, 2018

Felix Arntz added 3 commits April 29, 2018 18:51

Use default type validation against enum.

95aa81c

Fix spacing.

b3cb1cb

Fix code standard issues.

5341249

Soean reviewed Apr 30, 2018

View reviewed changes

lib/class-wp-rest-search-controller.php Outdated

/**

* Gets the post types allowed for search.

*

* @since 1.0.0

Copy link

Member

Soean Apr 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since 2.9.0

Fix incorrect since annotations.

d6a984f

danielbachhuber reviewed Apr 30, 2018

View reviewed changes

danielbachhuber requested changes Apr 30, 2018

View reviewed changes

Felix Arntz added 2 commits May 3, 2018 11:39

Add missing text domain to search controller.

703322c

Only generate item data for the fields specified in the request.

d80560f

Felix Arntz added 4 commits May 5, 2018 12:18

Add comprehensive unit tests for REST search controller and fix bugs …

311413e

…detected by tests.

Add missing test docs for REST search controller tests.

51d1f9b

Introduce a type_exclude parameter for the REST search endpoint.

d7b1762

Fix code style issues.

6ed2b37

felixarntz added Core REST API Task Task for Core REST API efforts and removed Needs Tests [Status] In Progress Tracking issues with work in progress labels May 5, 2018

Felix Arntz added 3 commits May 23, 2018 10:49

Remove author from _links in REST search controller.

972150d

Change REST search controller since annotations to 3.0.0.

3d447ea

Merge remote-tracking branch 'upstream/master' into add/rest-search-e…

14d2ffa

…ndpoint

Felix Arntz added 2 commits May 23, 2018 11:06

Order by relevance if a search term is given in REST search controller.

6e70454

Use the new REST search controller in UrlInput component.

8837556

felixarntz mentioned this pull request Jul 11, 2018

Implement forward-compatible REST API search controller #7894

Merged

4 tasks

felixarntz closed this Jul 11, 2018

aduth mentioned this pull request Jan 21, 2020

Add URL preview to Link UI #19387

Closed

6 tasks

Implement WP_REST_Search_Controller class #6489

Implement WP_REST_Search_Controller class #6489

Conversation

felixarntz commented Apr 29, 2018 • edited by rmccue Loading

Description

How has this been tested?

Types of changes

Checklist:

Soean Apr 30, 2018 • edited Loading

Choose a reason for hiding this comment

Soean Apr 30, 2018

Choose a reason for hiding this comment

felixarntz commented Apr 30, 2018

danielbachhuber commented Apr 30, 2018

danielbachhuber Apr 30, 2018

Choose a reason for hiding this comment

felixarntz May 3, 2018

Choose a reason for hiding this comment

danielbachhuber May 17, 2018

Choose a reason for hiding this comment

danielbachhuber Apr 30, 2018

Choose a reason for hiding this comment

felixarntz May 3, 2018 • edited Loading

Choose a reason for hiding this comment

danielbachhuber May 5, 2018

Choose a reason for hiding this comment

felixarntz May 9, 2018 • edited Loading

Choose a reason for hiding this comment

felixarntz commented May 3, 2018

felixarntz commented May 5, 2018 • edited Loading

felixarntz commented May 5, 2018 • edited Loading

felixarntz commented May 9, 2018 • edited Loading

felixarntz commented May 23, 2018 • edited Loading

felixarntz commented May 23, 2018

danielbachhuber commented May 23, 2018

felixarntz commented May 23, 2018

danielbachhuber commented May 23, 2018

felixarntz commented May 23, 2018

danielbachhuber commented May 23, 2018

felixarntz commented May 23, 2018 • edited Loading

danielbachhuber commented May 23, 2018

felixarntz commented May 23, 2018

danielbachhuber commented May 23, 2018

danielbachhuber commented May 25, 2018

danielbachhuber commented Jun 27, 2018

felixarntz commented Jul 1, 2018 • edited Loading

danielbachhuber commented Jul 1, 2018

pento commented Jul 3, 2018

felixarntz commented Jul 3, 2018 • edited Loading

pento commented Jul 10, 2018

felixarntz commented Jul 11, 2018

rmccue commented Jul 11, 2018

felixarntz commented Jul 11, 2018

uamv commented Aug 12, 2020

Implement `WP_REST_Search_Controller` class #6489

Implement `WP_REST_Search_Controller` class #6489

felixarntz commented Apr 29, 2018 •

edited by rmccue

Loading

Soean Apr 30, 2018 •

edited

Loading

felixarntz May 3, 2018 •

edited

Loading

felixarntz May 9, 2018 •

edited

Loading

felixarntz commented May 5, 2018 •

edited

Loading

felixarntz commented May 5, 2018 •

edited

Loading

felixarntz commented May 9, 2018 •

edited

Loading

felixarntz commented May 23, 2018 •

edited

Loading

felixarntz commented May 23, 2018 •

edited

Loading

felixarntz commented Jul 1, 2018 •

edited

Loading

felixarntz commented Jul 3, 2018 •

edited

Loading