Review

Author: soyuka

docs/adr/0000-subresources-definition.md

@@ -5,7 +5,7 @@
 
 ## Context and Problem Statement
 
-Subresources introduced in 2017 ([#904][pull/904]) the `ApiSubresource` annotation. This definition came along with its own set of issues ([#2706][issue/2706]) and needs a refreshment. On top of that, write support on subresources is a wanted feature and it is hard to implement currently ([#2598][pull/2598]) (See [0001-subresource-write-support](./0001-subresource-write-support.md)). How can we revamp the Subresource definition to improve the developer experience and reduce the complexity?
+Subresources introduced in 2017 ([#904][pull/904]) the `ApiSubresource` annotation. This definition came along with its own set of issues ([#2706][issue/2706]) and needs a refreshment. On top of that, write support on subresources is a wanted feature and it is hard to implement currently ([#2598][pull/2598]) (See [ADR-0001-subresource-write-support](./0001-subresource-write-support.md)). How can we revamp the Subresource definition to improve the developer experience and reduce the complexity?
 
 ## Considered Options
 
@@ -21,6 +21,7 @@ We choose to use multiple `ApiResource` annotations to declare subresources on a
 * The `ApiSubresource` annotation is declared on a Model's properties, which was identified as the root of several issues. For example, finding what class it is defined on ([#3458][issue/3458]). Having multiple `ApiResource` would improve a lot the declaration of our internal metadata and would cause less confusion for developers. 
 * The `path` of these multiple `ApiResource` needs to be implicitly described. 
 * An `ApiResource` is always defined on the Resource it represents: `/companies/1/users` outputs Users and should be defined on the `User` model.
+* PropertyInfo and Doctrine metadata can be used to define how is the Resource identified according to the given path.
 
 ### Examples
 
@@ -32,18 +33,26 @@ Get Users belonging to the company on (`/companies/1/users`);
  * @ApiResource(path="/companies/{companyId}/users")
  */
 class User {
+  /** @ApiProperty(identifier=true) */
+  public int $id;
+
+  /** @var Company[] */
   public array $companies = [];
 }
 ```
 
-With explicit identifiers:
+With explicit identifiers, the tuple is explained in [ADR-0002-identifiers](./0002-identifiers) `{parameterName: {Class, property}}`:
 
 ```php
 /**
- * @ApiResource(path="/users")
- * @ApiResource(path="/companies/{companyId}/users", identifiers={"companyId": {Company::class, "id"}})
+ * @ApiResource(path="/users", identifiers={"id": {User::class, "id"}})
+ * @ApiResource(path="/companies/{companyId}/users", identifiers={"companyId": {Company::class, "id"}, "id": {User::class, "id"}})
  */
 class User {
+  /** @ApiProperty(identifier=true) */
+  public int $id;
+
+  /** @var Company[] */
   public array $companies = [];
 }
 ```
@@ -55,31 +64,53 @@ Two-level subresource to get the Users belonging to the Company #1 located in Fr
  * @ApiResource(path="/users")
  * @ApiResource(path="/countries/{countryId}/companies/{companyId}/users")
  */
-class Users {
+class User {
+  /** @ApiProperty(identifier=true) */
+  public int $id;
+
+  /** @var Company[] */
   public array $companies = [];
 }
 
 class Company {
+  /** @ApiProperty(identifier=true) */
+  public int $id;
+
+  /** @var Country[] **/
   public array $countries = [];
 }
+
+class Country {
+  /** @ApiProperty(identifier=true) */
+  public string $shortName;
+}
 ```
 
 With explicit identifiers:
 
 ```php
 /**
- * @ApiResource(path="/users")
- * @ApiResource(path="/countries/{countryId}/companies/{companyId}/users", identifiers={"companyId": {Company::class, "id"}, "countryId": {Country::class, "shortName"}})
+ * @ApiResource(path="/users", identifiers={"id": {User::class, "id"}})
+ * @ApiResource(path="/countries/{countryId}/companies/{companyId}/users", identifiers={"companyId": {Company::class, "id"}, "countryId": {Country::class, "shortName"}, "id": {User::class, "id"}})
  */
-class Users {
+class User {
+  /** @ApiProperty(identifier=true) */
+  public int $id;
+
+  /** @var Company[] */
   public array $companies = [];
 }
 
 class Company {
+  /** @ApiProperty(identifier=true) */
+  public int $id;
+
+  /** @var Country[] **/
   public array $countries = [];
 }
 
 class Country {
+  /** @ApiProperty(identifier=true) */
   public string $shortName;
 }
 ```
@@ -92,41 +123,27 @@ Get the company employees or administrators `/companies/1/administrators`:
  * @ApiResource(path="/companies/{companyId}/administrators")
  * @ApiResource(path="/companies/{companyId}/employees")
  */
-class Users {
-  public array $companies;
-}
-
-class Company {
+class User {
+  /** @ApiProperty(identifier=true) */
   public int $id;
-  public Users $employees;
-  public Users $administrators;
-}
-```
-
-With explicit identifiers:
 
-```php
-/**
- * @ApiResource(path="/users")
- * @ApiResource(path="/companies/{companyId}/administrators", identifiers={"companyId": {Company::class, "id"}, "*": {Company::class, "administrators"}})
- * @ApiResource(path="/companies/{companyId}/employees", identifiers={"companyId": {Company::class, "id"}, "*": {Company::class, "employees"}})
- */
-class Users {
-  public array $companies;
+  /** @var Company[] */
+  public array $companies = [];
 }
 
 class Company {
+  /** @ApiProperty(identifier=true) */
   public int $id;
-  public Users $employees;
-  public Users $administrators;
+
+  /** @var User[] **/
+  public array $employees;
+
+  /** @var User[] **/
+  public array $administrators;
 }
 ```
 
-## TODO:
-
-* Without explicit identifiers, how do we map `companyId` to Company->id ?
-* Do we parse the path to find `administrators` and map it to the property ?
-* The Tuple `identifiers={pathParameter: {Class, property}}` should be redefined / validated (and what about `*` for collection?)
+This example will require a custom DataProvider as the discriminator needs to be explicit.
 
 ## Links