feat: improves iri_only implementation

[PommeThibaudeau]

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tickets	no
License	MIT

adds an "iri_only" parameter to the "collectionOperations" annotation. allows the display of resources in IRI list form

[teohhanhui]

Sorry, I don't understand what this PR is trying to achieve. It's already possible to set iri_only in the normalization context: #3275

[PommeThibaudeau]

The purpose is to get this: http://tinyurl.com/rzzdjm9 instead of that: http://tinyurl.com/rrg34e3. Kevin Dunglas asked me to do that to enhance support of vulcain.

[soyuka]

This just improves the iri_only feature by:

• returning an array of iris (/book/1 instead of {"@id": "/book/1"})
• fixing the context accordingly

This work is almost done and we worked on it with @dunglas. It'll improve Vulcain support in which the selector would become /hydra:member/* instead of /hydra:member/*/@id/* currently.

[teohhanhui]

Hmm... I see. I think we should reuse the same iri_only option in the normalization context.

But actually, if we're going by JSON-LD spec, both forms are equivalent. So if the client relies on a specific form, it's the client's mistake. It'd follow that this should be done as a bug fix targeting 2.5.

[soyuka]

iri_only is only available on master

[teohhanhui]

Okay, that makes it easier then. We can fix its behaviour without introducing this iriOnly attribute (which is badly named and doesn't belong at this level).

But if the idea is to replace it with something better, then I'd say it belongs at the collection operation level only, like the OP in this PR says, not per the current implementation of this PR.

i.e. it should be:

/*
 * @ApiResource(
 *     collectionOperations={
 *         "get"={
 *             "iri_only"=true,
 *         },
 *     },
 * )
 */

and not:

/*
 * @ApiResource(
 *     iriOnly=true,
 * )
 */

[PommeThibaudeau]

It is both now, like a generic attribute.

[teohhanhui]

Taking a step back, I think there's more value to be had with an attribute such as iriOnly if it means "always normalize all relations as IRI, never embed" (since embedding is counter-productive for Vulcain), and not just for collection items. And the naming would make more sense too (still weird, but at least it starts to make sense)...

[teohhanhui]

I'm taking another step back. It seems to me that iri_only would indeed make a lot of sense as a serialization "mode" to instruct the normalizer to never embed (resource) relations.

Ideally, an API resource or its operations should never need to be marked as such (because it should work equally well with or without a proxy such as Vulcain, forcing the user to decide this at this level doesn't make sense), but it should be a mode that can be triggered by some mechanism (i.e. it needs to be a dynamic choice) which would cause this iri_only option to be passed in while calling normalize.

So I feel we're going more in the wrong direction with this PR and should rethink this.

[teohhanhui]

/cc @dunglas for my observations above.

[PommeThibaudeau]

I'm not sure I understand what you mean (given my level on API P. Atm).
But you can use this feature without the embedded context shown in the jsonld playground.
I added a ContextAction condition to allow different context link in case the resource's context is not the same as the operation's context. Is it what you meant ?

[dunglas]

I don't understand what you mean @teohhanhui. This is not directly related to Vulcain. Having an option to not embed resources (which is what a pure REST API should prefer) doesn't have to be dynamic. Actually, in a perfect world, supporting resources embedding shouldn't even be supported as it's just a performance hack on top of REST.

I'm not against having a dynamic way to trigger this option (like for pagination), but it can be done in the future. This feature looks good enough as is for me.

[dunglas]

This can be simplified a lot. Generating different context URLs per operation is useless: https://json-ld.org/playground/#startTab=tab-expanded&json-ld=%7B%22%40context%22%3A%7B%22%40vocab%22%3A%22http%3A%2F%2Fschema.org%2F%22%2C%22hydra%3Amember%22%3A%7B%22%40type%22%3A%22%40id%22%7D%7D%2C%22%40type%22%3A%22Person%22%2C%22name%22%3A%22Jane%20Doe%22%2C%22jobTitle%22%3A%22Professor%22%2C%22telephone%22%3A%22(425)%20123-4567%22%2C%22url%22%3A%22http%3A%2F%2Fwww.janedoe.com%22%7D&frame=%7B%7D&context=%7B%7D

Just add the following in all generated context if the iri_attribute is set, and done!

    "hydra:member": {
      "@type": "@id"
    }

[teohhanhui]

Actually, in a perfect world, supporting resources embedding shouldn't even be supported as it's just a performance hack on top of REST.

I completely disagree on this. Embedding is an intergral part of hypermedia and is not "just a performance hack". In fact, the fact that JSON-LD (and by extension, Hydra) supports embedding is a strong argument against your point. Each resource is uniquely identified by an IRI, so it's flexibility, not a hack.

[dunglas]

@teohhanhui for JSON-LD and hypermedia in general, there are a lot of use cases for compound documents. For REST APIs however, it's now considered a Best Practice (especially with HTTP/2) to have granular endpoints: https://medium.com/apis-you-wont-hate/lets-stop-building-apis-around-a-network-hack-9a68f7e83dd2

Anyway we're going far away from the topic.

Taking a step back, I think there's more value to be had with an attribute such as iriOnly if it means "always normalize all relations as IRI, never embed" (since embedding is counter-productive for Vulcain), and not just for collection items. And the naming would make more sense too (still weird, but at least it starts to make sense)...

That was exactly my goal when I introduced this attribute. I'm not very happy with the name either. I would be glad to find a better name.

Regarding the "dynamic" mechanism. We could introduce that later, has we've done for client-side pagination for instance. We already have all the infrastructure to do that if needed (but I'm not sure that we need it, most users - for simplicity, cache etc - will choose between a Vulcain-like API, or one relying on embedded documents, usually not both at the same time).

[Richard87]

For an outside perspective, this is almost exactly what i need, but on a property level.

I have a Tree view, with Parent and Child beeing the same component, so modifying normalization_groups for the operation doesn't work.

I would like to add a @ApiProperty(iri_only=true) to my "@Orm\OneToMany` property, and only have a list of ID's returned.

Or use @MaxDepth(0), or @MaxDepth(1, iri_only=true) or similar...

For some more context, I have a Component with these properties:

/**
 * @var Component|null
 * @ORM\ManyToOne(targetEntity="App\Entity\Component", cascade={"persist"}, inversedBy="children")
 * @ORM\JoinColumn(onDelete="CASCADE")
 * @ORM\Cache("NONSTRICT_READ_WRITE")
 */
protected $parent;

/**
 * @var Component[]|ArrayCollection
 * @ORM\OneToMany(targetEntity="App\Entity\Component", cascade={"persist"}, mappedBy="parent")
 * @ORM\OrderBy({"order" = "ASC"})
 * @Groups({"query", "public"})
 * @ORM\Cache("NONSTRICT_READ_WRITE")
 * @MaxDepth(1)
 */
protected $children;

When I get a list of these, I only want to have the id's (or @id) from children, since the content is already included elsewhere in the list.

The end result would be that I recreate the tree on the frontend. If I created the tree in API-Platform (what I'm doing today, because of this limitation, Doctrine executes alot of queries because of the recursive nature. With the end result of some hacky optimizations to trick Doctrine into believing it already fetched the whole collection...

[alanpoulain]

Thank you @PierreThibaudeau!