[RFC] [WIP] Offer post, delete and put subresource

[torreytsui]

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	maybe - to be discussed below
Deprecations?	yes - to be discussed below
Tests pass?	yes
Fixed tickets	#2251, #571, #655
License	MIT
Doc PR	(to be added, once agreed on default behaviours)

TODO

• Resolve routes (POST, DELETE, PUT, PATCH) for subresources
• Implement BC (once the expected default behaviours of @ApiSubresource is confirmed)
• Implement behat test cases for
  • POST
    • Given Dummy->relatedDummies, add a relatedDummy and association should persist
  • DELETE
  • PUT
    • [WIP] Given Dummy->relatedDummy is null, create a relatedDummy and association should persist
    • Given Dummy->relatedDummy is non null, update the relatedDummy
  • PATCH
  • Nested levels
    • PUT
    • POST
    • PATCH
• [WIP] Further refactor to make it more maintainable
  • Support value object subresource (maybe separated PR)
  • Introduce intermediate metadata object(s) and factory(s) to separate concern
• Make sure specific subresource methods can be disabled
  @ApiResource(subresourceOperations={"get"})
• Review and make sure api doc is generated correctly (implement test cases if needed)
  \ApiPlatform\Core\Swagger\Serializer\DocumentationNormalizer::normalize
• Document predefined naming conventions to ease customisation
  (e.g., $subresource(s)_$method_subresource)
• Remove the try-catch workaround (which was for keeping git history only)
• Document and discuss replacement of Enable item route on collection subresources #1875, using different operation names for subresource item operations (e.g., subcollection_item_get_subresource instead of subcollection_get_subresource)

Subresource operations

There are two kinds of subresource operations, namely, collection operations and item operations. They are enabled slightly differently and according to if the the subresource is a collection or not, there is a different path setup as well as identifier setup.

Given an example where a property bar is a subresource of class Foo, the corresponding setup is populated as below.

/* @ApiResource */
class Foo
{
    /* @ApiSubresource */
    public $bar;
}

If bar is a collection (i.e., multiple occurrences), below operations are available:

Method	Collection	Operation type	Operation Name	Route Path	Identifier
GET	true	collection	bar_get_subresource	/foo/{id}/bars	n/a
POST	true	collection	bar_post_subresource	/foo/{id}/bars	n/a
GET	true	item	bar_item_get_subresource	/foo/{id}/bars/{bar}	bar
DELETE	true	item	bar_item_delete_subresource	/foo/{id}/bars/{bar}	bar
PUT	true	item	bar_item_put_subresource	/foo/{id}/bars/{bar}	bar

If bar is NOT a collection (i.e., single occurrence), below operations are available:

Method	Collection	Operation type	Operation Name	Route Path	Identifier
GET	false	item	bar_item_get_subresource	/foo/{id}/bars	n/a
DELETE	false	item	bar_item_delete_subresource	/foo/{id}/bars	n/a
PUT	false	item	bar_item_put_subresource	/foo/{id}/bars	n/a

Inheriting subresource overriding

Previously, only get_subresource and item_get_subresource were supported and therefore, it was clear when it comes to inheriting overriding. (Only path overriding is supported for now.)

Now, we in addition, support post_subresouce, item_delete_subresource, and item_put_subresource. This leads to two questions, when a subresource's path is overridden, (1) should the child subresource inherit from it? and (2) if we do, which subresource operation should the child inherit from?

(1) As we have been offering path overridden inheritance, to keep BC, I believe we should keep supporting it.

(2) To account for the inheritance unambiguity, a simple solution can be offering inheritance fallbacks (we can't simply stick with single method (e.g., GET) because we don't know what operations will be offered by the parent subresource), giving predefined weight / priority to the subresource operation and fallback from one to others, as below:

Continue with the above foo-bar example, given that the Bar class has a baz subresource, if the path of bar subresource is overridden in Foo class, what paths should baz subresource be and in what operations?

/*
 * @ApiResource(
 *   subresourceOperations={
 *     "bar_get_subresource"={"path"="/foo/get-bar"},
 *     "bar_post_subresource"={"path"="/foo/post-bar"},
 *     "bar_item_get_subresource"={"path"="/foo/get-item-bar/{bar}"},
 *     "bar_item_delete_subresource"={"path"="/foo/delete-item-bar/{bar}"},
 *     "bar_item_put_subresource"={"path"="/foo/put-item-bar/{bar}"},
 *   },
 * )
*/
class Foo
{
    /* @ApiSubresource */
    public $bar;
}

class Bar
{
    /* @ApiSubresource */
   public $baz;
}

For collection operations, it is inheriting subresource overriding in below order, from top to bottom (if defined):

Method	Operation type	Operation Name	Route Path
GET	collection	bar_get_subresource	/foo/{id}/get-bars
POST	collection	bar_post_subresource	/foo/{id}/post-bars

As GET operation is defined, the overridden path /foo/{id}/get-bars is inherited by the child subresource and the baz subresouce have these subresource collection operation paths.

Method	Operation type	Operation Name	Route Path
GET	collection	bar_baz_get_subresource	/foo/{id}/get-bars/{bar}/baz
POST	collection	bar_baz_post_subresource	/foo/{id}/get-bars/{bar}/baz

For item operations, it is inheriting subresource overriding in below order, from top to bottom (if defined):

Method	Operation type	Operation Name	Route Path
GET	item	bar_item_get_subresource	/foo/{id}/get-bars/{bar}
PUT	item	bar_item_put_subresource	/foo/{id}/put-bars/{bar}
DELETE	item	bar_item_delete_subresource	/foo/{id}/delete-bars/{bar}

As GET operation is defined, the overridden path /foo/{id}/get-bars/{bar} is inherited by the child subresource and the baz subresouce have these subresource item operation paths.

Method	Operation type	Operation Name	Route Path
GET	item	bar_baz_item_get_subresource	/foo/{id}/get-bars/{bar}/baz/{baz}
PUT	item	bar_baz_item_put_subresource	/foo/{id}/get-bars/{bar}/baz/{baz}
DELETE	item	bar_baz_item_delete_subresource	/foo/{id}/get-bars/{bar}/baz/{baz}

Above example assumed that both bar and baz are collection. In a situation where a subresource is not a collection, the same fallback priority apply (from GET, PUT, to DELETE).

Expected default behaviours

Currently, classes defined with @ApiSubresource will be automatically enabled with GET method.

In this PR, POST, PUT, DELETE, and PATCH are offered to subresource. This leads to a question, for classes defined with @ApiSubresource, what methods should be enabled by default?

https://api-platform.com/docs/core/operations/#operations

To be in line with @ApiResource, which enables GET (and if is collection also POST), PUT and DELETE (and if is using JSON API also PATCH), @ApiSubresource makes sense to follow this behaviour. However if we go for this, it can potentially break projects which are using subresource at the moment, being unaware of the new operations offered and exposed unwanted operations to public.

What do you think is the best way to handle this?

At this moment, as a proof of concept,I have followed the default behaviours of @ApiResource.

[torreytsui]

It looks like that there is a bc breaking change in webonyx/graphql-php 0.13.1 and caused the CI failure

[soyuka]

#2594 I'll bring this onto master today

[alanpoulain]

@torreytsui did you see that there is already a PR for this feature? #2428
It would be really great if you could work on it together.

[vincentchalamon]

@torreytsui I'm currently working on a similar PR: #2428.

After discussing with the core-team, we rejected the PUT to only keep the POST & DELETE on a subresource (cf. comments in #1628, don't hesitate to ask me if you have any questions about it). Check the Behat scenarios for basic examples, like addind an existing object to a subresource collection.

Also, with @soyuka we wanted to refactor the ApiSubresource annotation configuration to make it more maintainable, and centralize its configuration in the same annotation (currently, it's split in 3 parts). But to prevent a huge, endless & unreadable PR, we should split those 2 main features in several PRs.

Would be awesome if you could help us on it 😃

[torreytsui]

@alanpoulain I didn't see #2428. That will be awesome to work on this feature together. I have got many questions / issues around it to discuss and I'm sure a team work will lead us to a better, more maintainable solution.

@vincentchalamon It will be a awesome to work with you. Thanks for linking #1628.

I agree that we should split them in different PRs. I also want to refactor the SubresourceOperationFactory to a more maintainable straightforward shape (basically, I want to make it not needing to reference the parent when it processes the subresource - so it is a generalisation). It will reduce the effort needed to implement more subresource features, also reduce bugs. I'm happy to raise a separate PR and discuss there. What do you guy think?

Edit: After generalisation, adding support of PUT / POST / DELETE, changes in the SubresourceOperationFactory can be as simple as that (b39d3ab#diff-fdc8e25f68e3044cf8be883d3f3a7e23R146)

Regarding to the REST discussion, I will start by writing up my thought on it in the #1628. Think it is a more appropriate place for incubating that ideas.

[malaimo2900]

@torreytsui Is this Pull Request usable using the current API Platform code?

[torreytsui]

@malaimo2900 The PR is abandoned unfortunately and it is out of sync with the core.

It can serve as a proof of concept or reference only.

The features this PR tries to implement are complex and introduce many impacts. In my opinion, it will need a wider discussion with the core team members.

[vincentchalamon]

Subresources have been deprecated in 2.7 and removed in 3.0 in favor of new ApiResource metadata