Allow collection paths with wildcard parameters (e.g. /users/{id}/tasks}

[ihorsamusenko]

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	#884
License	MIT
Doc PR	-

This is what I got so far. It is not covered by tests yet.
I just wanted to confirm if you consider this place as an appropriate one for these changes.
@dunglas what do you think?

[dunglas]

The IRI generator must work even in a non HTTP context (in CLI for instance), RequestStack cannot be a hard dependency.
I don't understand how blindly passing current parameters can help (for instance when generating IRI for resources not related with the currently related collection). Can you elaborate a bit more?

[dunglas]

What we can probably do is finding a convention to populate router parameters using data of the related resource.

Example: /foos/{foo_id}/bars/{id}

['foo_id' => $currentEntity->getFoo()->getId()].

However, I'm not sure it's worth it (it will introduce a lot of complexity for no real advantage compared to put every resources at the root).

[dunglas]

By the way, you can already do this kind of logic by decorating the api_platform.router service, maybe should we only add a documentation explaining how to do that.

[ihorsamusenko]

That's what I thought at the first place, that RequestStack can not be used in IriCoverter as a hard dependency.
I also do not see any benefits of having routes like "/foos/{foo_id}/bars/{id}" , you'd better just go to "/bars/{id}", right?
But I do see a good thing in an ability to have routes like "/foos/{foo_id}/bars".

Moreover, there is one more problem. When I define several "get" collection operations, e.g. "/bars" and "/foos/{foo_id}/bars" the @id of the collection is always taken from the first operation.

[ihorsamusenko]

@dunglas I changed the code, what do you think about it?

[ihorsamusenko]

@dunglas ping

[dunglas]

I don't get exactly what it does. Can you add a functional test to explicit it (and to be sure this is properly tested).

[ihorsamusenko]

@dunglas the point is that a recourse can participate in several collections e.g. /items, /item-owner/{item-owner-id}/items, /brand-new-items.

Some recourses even do not need to have a common collection where you can find each item of the recourse e.g. comments. It makes sense to have paths like /post/{post-id}/comments or /user/{user-id}/comments, but who does ever need just /comments?

The functional tests are passed well on my local machine, what could be the reason they don't here?

[dunglas]

The failure is related to CS errors, run PHP CS Fixer.

[ihorsamusenko]

@dunglas yes, it was CS errors, but along with swagger errors. I changed behavior of DocumentNormalizer to parse paths and to put placeholders {} as required path parameters.

What do you think? Can we merge it?

[ihorsamusenko]

@dunglas or do I get it wrong and Hydra is not supposed to do something like that?

[dunglas]

I need to find some time to review this PR, sorry for the delay @samusenkoiv.

[dunglas]

Can you explain how URL for such patterns are generated (from where come the value replacing the {id} placeholder)?

[ihorsamusenko]

@dunglas the URL is taken from the context. While the context is created in here https://github.com/api-platform/core/blob/master/src/EventListener/SerializeListener.php#L61, which means it is taken from the current request

[dunglas]

Please use !$parameters (the call to empty is useless).

[dunglas]

It should be @param RelatedToDummyFriend $relatedToDummyFriend

[ihorsamusenko]

@dunglas check it out

[teohhanhui]

Nested collections! This is what I've been thinking of for a long time.

But I don't think it's so simple to implement. This PR does not seem to change the IRI generation, which I think is crucial, e.g. /posts/{post_id}/comments/{id} instead of /comments/{id} to fully express the dependency. Or in other words, a comment can only exist within the context of a post.

[teohhanhui]

Also, I think this should be handled at the normalizer level, i.e. allowing to serialize / deserialize into a nested collection (as opposed to embedding directly, which stops making sense anyway when there are too many items in the collection).

Using custom routes like what's done in this PR is a band-aid solution that does not address the real issue. 😄

[ihorsamusenko]

@teohhanhui I think you got it wrong. The PR does not pretend to implement or help with nested collection.

Instead it does the following:

• Shows the real @id of a collection instead of a first found GET operation
• Allows to use placeholders in paths, which is not something weird or extraordinary or is it?

[ihorsamusenko]

@dunglas @teohhanhui so you guys think it is not worth merging the PR?

[dunglas]

I'm in favor of merging this one. What do you think @teohhanhui?

[ihorsamusenko]

Oops, that was by accident

[soyuka]

so this doesn't work: /posts/{post_id}/comments/{id}?

I think this isn't really optimized, would you be able to show me the SQL query that is executed? Does it serialize the parent too before rendering?

The regex should be tested thoroughly with unit tests. But, isn't there something better to use from the Symfony path tools or the router?

[esistgut]

This PR seems to be related to the question I asked here api-platform/api-platform#199
Can it cover my use case?

[soyuka]

@esistgut your case is not related.

@samusenkoiv I thought a bit more about this feature. As said in my previous comment I'd love to have the corresponding query and serialization informations.

Allows to use placeholders in paths, which is not something weird or extraordinary or is it?

IMO this can be misleading, because the id in a resource is already given by ItemOperation and we should not complexify the Operation declaration.

What we can and should do instead is to "allow operations to retrieve and serialize only the association of a parent resource". This will give the same result as your patch does but it'll go deeper and notify the data providers and the serialization about what should be fetched. I don't think it's easy to implement but I'd start with the IRI declaration and try to make the Router aware of "I want a CollectionOperation siblings to an ItemOperation". This also means that instead of trying to parse the routing parameters manually, it'll define a proper Operation that our ApiLoader will directly load and that parameters should be interpreted by the Symfony router:

/api/entity/{id}/association.{_format}
/api/entity/{id}/association/{association_id}.{_format}

Where {id} can be one of:

• 10
• foo=10;bar=12

There is a strategy in place for composite identifiers in the Doctrine data provider. The rest should be handled in the IriConverter and related.

[teohhanhui]

Definitely agree with @soyuka re the routing.

[teohhanhui]

@samusenkoiv

Allows to use placeholders in paths, which is not something weird or extraordinary or is it?

I do not understand under what circumstances it would be appropriate to use? (Other than the nested collection use case you've described, because I think this is not the right way to get there.)

[soyuka]

Closing in favor of #904