WordPress.org

Make WordPress Core

Opened 2 months ago

Last modified 4 weeks ago

#47684 new defect (bug)

Embeddable 'self' links don't work on the REST API /search endpoint

Reported by: chrisvanpatten Owned by:
Milestone: 5.3 Priority: normal
Severity: normal Version:
Component: REST API Keywords:
Focuses: rest-api Cc:

Description

WordPress 5.0 introduced a new "search" endpoint which allows you to search across multiple types and subtypes.

The responses are a "light" version of the full object responses, only including ID, type/subtype, title, and a few other small bits of data.

They also include a standard links object:

_links: {
	about: [
		{
			href: "https://redacted/wp-json/wp/v2/types/post"
		}
	],
	collection: [
		{
		href: "https://redacted/wp-json/wp/v2/search"
		}
	],
	self: [
		{
			embeddable: true,
			href: "https://redacted/wp-json/wp/v2/posts/43"
		}
	]
}

In particular, note that the self link, a reference to the full object endpoint/response, is marked as embeddable. Thus, I would expect that a request to /wp/v2/search?_embed=true would include the full post response with each search result object.

However, that's not the case — the embedding is completely ignored. This is because core intentionally skips past `self` links when "expanding" embeddable items:

// Ignore links to self, for obvious reasons.
if ( 'self' === $rel ) {
	continue;
}

These reasons are indeed obvious when directly viewing the object — however in the search endpoint, where the responses are abridged, it would make sense to be able to embed the full object within your search results.

Attachments (1)

47684.diff (704 bytes) - added by chrisvanpatten 2 months ago.
Initial patch

Download all attachments as: .zip

Change History (11)

#1 @chrisvanpatten
2 months ago

As an idea that could solve this while preserving the current behavior of not expanding the self links, I wonder if there could be an additional link, with a different relational keyword, marked as embeddable instead?

There are a few that could be relevant:

via: Identifies a resource that is the source of the information in the link's context.

related: Identifies a related resource.

original: The Target IRI points to an Original Resource.

describedby: Refers to a resource providing information about the link's context.

Not sure if any of those are viable (or if a second link is itself a viable option) but it could be an easy fix.

#2 @kadamwhite
2 months ago

My gut is that original would be the one we want here. It's not related because it's the same object, and technically the original post is described by the search endpoint result, and not the opposite; so original feels most accurate.

@chrisvanpatten
2 months ago

Initial patch

#3 @chrisvanpatten
2 months ago

I've added an initial patch above, which simply adds an original link, and moves the embeddable flag over. Since embeddable was not doing anything previously I'm not sure if this would be an acceptable change (it is technically a break in backcompat if someone was looking for that setting) but as this is only an initial patch I thought I'd post it and see how y'all felt :)

Last edited 2 months ago by chrisvanpatten (previous) (diff)

This ticket was mentioned in Slack in #core-restapi by chrisvanpatten. View the logs.


8 weeks ago

This ticket was mentioned in Slack in #core-restapi by timothybjacobs. View the logs.


7 weeks ago

#6 @kadamwhite
7 weeks ago

Sync'd with @rmccue on this and read through a bunch of RFCs to try to get some clarity here. self is still the most ideal here, I believe; referencing https://www.iana.org/assignments/link-relations/link-relations.xml "An Original Resource is a resource that exists or used to exist, and for which access to one of its prior states may be required." and is more about versioning than about identity.

describedby would be for info about the post type, via is more for an external source, and related is for another resource. If we don't want to use self, we could conceivably use item. We should look at how hard it would be to make self embeddable here and not elsewhere, though.

#7 @TimothyBlynJacobs
6 weeks ago

We should look at how hard it would be to make self embeddable here and not elsewhere, though.

The code preventing this is fun :)

// Ignore links to self, for obvious reasons.
if ( 'self' === $rel ) {
        continue;
}

From what I can tell, nothing immediately breaks by removing that restriction. However, if a developer accidentally marked a self link as embeddable in the "self" route, then you'd duplicate the item body. ( for instance, marking the self link in the post controller as embeddable ). However, we wouldn't get an infinite loop because we don't process embeds for the embedded responses themselves.

So if a developer made the mistake of improperly marking self as embeddable in an inappropriate place, it wouldn't be the end of the world.

We could try to account for this, but it wouldn't be perfect. For instance, we could conceivably check whether the self link is the same as the currently dispatching request. If the current route was wp/v2/posts/1, we'd skip the self link if it was wp/v2/posts/1. However, this wouldn't catch the collection route containing post #1.

I think lifting the restriction would be ok. Currently, it causes the Tests_REST_Server::test_link_embedding_self test to fail.

This ticket was mentioned in Slack in #core-restapi by timothybjacobs. View the logs.


6 weeks ago

This ticket was mentioned in Slack in #core-restapi by timothybjacobs. View the logs.


4 weeks ago

#10 @kadamwhite
4 weeks ago

  • Milestone changed from Awaiting Review to 5.3
Note: See TracTickets for help on using tickets.