Skip to content

Revamp ApiSubresource #3689

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

vincentchalamon
Copy link
Contributor

@vincentchalamon vincentchalamon commented Aug 27, 2020

Q A
Bug fix? no
New feature? yes
BC breaks? yes
Deprecations? yes
Tickets fixes #2706, #1628, #3177
License MIT
Doc PR TODO
  • ADR document
  • Add support for the new path attribute on ApiResource
  • Allow setting ApiResource several times (annotation, xml and yaml formats)
  • Patch the ApiLoader to use it
  • Patch the IRI Converter to use it
  • Patch the DataProviders to use it
  • Patch the GraphQL subsystem to use it
  • Patch the DataPersisters to use it
  • Patch the DocumentationNormalizers to use it
  • Add doc PR
  • Deprecate the old ApiSubresource subsystem

Examples:

/**
 * @ApiResource(path="/users")
 * @ApiResource(path="/companies/{companyId}/users", identifiers={"companyId": {Company::class, "id"})
 * @ApiResource(path="/companies/{companyId}/users")
 * @ApiResource(path="/countries/{countryId}/companies/{companyId}/users", identifiers={"companyId": {Company::class, "id"}, "countryId": {Country::class, "id"}})
 *
 * @ApiResource(path="/companies/{companyId}/administrators", identifiers={"companyId": {Company::class, "id"}, "??": {CompanyClass, "administrators"}})
 * @ApiResource(path="/companies/{companyId}/employees", identifiers={"companyId": {Company::class, "id"}, "??": {CompanyClass, "employees"}})
 */

@soyuka
Copy link
Member

soyuka commented Sep 22, 2020

I think we should base this branch over the #3664 now.

The next thing to do is to "merge" the behavior of both identifiedBy and identifiers on the subresource's metadata.
At the end, the following parts should be removed:

$subresourceIdentifiers = [];
foreach ($attributes['subresource_context']['identifiers'] as list($identifier, , $isItem, $identifiedBy)) {
if ($isItem) {
$subresourceIdentifiers[$identifier] = [$identifiedBy[0] => current($identifiers)];
next($identifiers);
continue;
}
$subresourceIdentifiers[$identifier] = [];
}

if (isset($attributes['subresource_context'])) {
$subresourceIdentifiersKeys = [];
$i = 0;
foreach ($attributes['subresource_context']['identifiers'] as list($identifier, , $isItem)) {
if ($isItem) {
$subresourceIdentifiersKeys[] = $identifiersKeys[$i++] ?? $identifier;
}
}
$identifiersKeys = $subresourceIdentifiersKeys;
}
.

@soyuka soyuka force-pushed the feature/subresources branch from 051c5d5 to d73a2ad Compare October 2, 2020 12:56
@soyuka soyuka changed the base branch from master to next October 2, 2020 12:57
@soyuka soyuka force-pushed the feature/subresources branch from d73a2ad to 8389b4e Compare October 2, 2020 16:55
@jamesisaac
Copy link
Contributor

Would this address #3177 ? Haven't seen it mentioned specifically in the linked issues, but that's quite a big downside currently to Subresources vs normal ApiResources.

@vincentchalamon
Copy link
Contributor Author

@jamesisaac I think it'll fix this issue too, cause the subresources will be deprecated in favor of multiple ApiResources. So in final, the security option of ApiResource will be used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RFC] Subresource metadata How to add new entry in a ManyToMany?
4 participants