-
-
Notifications
You must be signed in to change notification settings - Fork 978
feat(metadata): resource and operation level itemUriTemplate #8479
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -982,6 +982,61 @@ public function __construct( | |
| protected array $extraProperties = [], | ||
| ?bool $map = null, | ||
| protected ?array $mcp = null, | ||
| /** | ||
| * The `itemUriTemplate` option is the URI template of the operation that item IRIs of this | ||
| * resource point to. It is the default for every operation of the resource, and an operation | ||
| * declaring its own `itemUriTemplate` wins over it. | ||
| * | ||
| * Use it when items are reachable through several URIs and one of them is the canonical one: | ||
| * the `@id` of an item read or written through a custom URI then points to the canonical | ||
| * operation rather than to the URI the request came from. On `GetCollection` and `Post`, | ||
| * where the option already existed, it keeps its meaning: the items of that collection, and | ||
| * the item just created, are given that URI. | ||
| * | ||
| * <div data-code-selector> | ||
| * | ||
| * ```php | ||
| * <?php | ||
| * // api/src/Entity/Book.php | ||
| * use ApiPlatform\Metadata\ApiResource; | ||
| * use ApiPlatform\Metadata\Get; | ||
| * use ApiPlatform\Metadata\Patch; | ||
| * | ||
| * #[ApiResource( | ||
| * itemUriTemplate: '/books/{id}', | ||
| * operations: [ | ||
| * new Get('/books/{id}'), | ||
| * new Patch('/books/{id}/cover'), | ||
| * ], | ||
| * )] | ||
| * class Book | ||
| * { | ||
| * // ... | ||
| * } | ||
| * ``` | ||
| * | ||
| * ```yaml | ||
| * # api/config/api_platform/resources.yaml | ||
| * resources: | ||
| * App\Entity\Book: | ||
| * - itemUriTemplate: /books/{id} | ||
| * ``` | ||
| * | ||
| * ```xml | ||
| * <?xml version="1.0" encoding="UTF-8" ?> | ||
| * <!-- api/config/api_platform/resources.xml --> | ||
| * <resources | ||
| * xmlns="https://api-platform.com/schema/metadata/resources-3.0" | ||
| * xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | ||
| * xsi:schemaLocation="https://api-platform.com/schema/metadata/resources-3.0 | ||
| * https://api-platform.com/schema/metadata/resources-3.0.xsd"> | ||
| * <resource class="App\Entity\Book" itemUriTemplate="/books/{id}" /> | ||
| * </resources> | ||
| * ``` | ||
| * | ||
| * </div> | ||
| */ | ||
| protected ?string $itemUriTemplate = null, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A user-facing option deserves a docblock in the style of the neighbouring parameters — in particular the precedence rule ( Also needs a docs PR on api-platform/docs before this ships.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added, in the style of the neighbouring parameters, with the precedence rule and the note that on Docs PR on api-platform/docs once the shape settles, which I would rather not write twice. |
||
| ) { | ||
| parent::__construct( | ||
| shortName: $shortName, | ||
|
|
@@ -1093,6 +1148,19 @@ public function withUriTemplate(string $uriTemplate): static | |
| return $self; | ||
| } | ||
|
|
||
| public function getItemUriTemplate(): ?string | ||
| { | ||
| return $this->itemUriTemplate; | ||
| } | ||
|
|
||
| public function withItemUriTemplate(?string $itemUriTemplate = null): static | ||
| { | ||
| $self = clone $this; | ||
| $self->itemUriTemplate = $itemUriTemplate; | ||
|
|
||
| return $self; | ||
| } | ||
|
|
||
| public function getTypes(): ?array | ||
| { | ||
| return $this->types; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,9 +17,12 @@ | |
| use ApiPlatform\Doctrine\Orm\State\Options as OrmOptions; | ||
| use ApiPlatform\Elasticsearch\State\Options as ElasticsearchOptions; | ||
| use ApiPlatform\Metadata\Exception\InvalidArgumentException; | ||
| use ApiPlatform\Metadata\Get; | ||
| use ApiPlatform\Metadata\GetCollection; | ||
| use ApiPlatform\Metadata\HeaderParameter; | ||
| use ApiPlatform\Metadata\Patch; | ||
| use ApiPlatform\Metadata\Post; | ||
| use ApiPlatform\Metadata\Put; | ||
| use ApiPlatform\Metadata\QueryParameter; | ||
| use ApiPlatform\OpenApi\Model\ExternalDocumentation; | ||
| use ApiPlatform\OpenApi\Model\Operation as OpenApiOperation; | ||
|
|
@@ -64,6 +67,7 @@ protected function extractPath(string $path): void | |
| foreach ($xml->resource as $resource) { | ||
| $base = $this->buildExtendedBase($resource); | ||
| $this->resources[$this->resolve((string) $resource['class'])][] = array_merge($base, [ | ||
| 'itemUriTemplate' => $this->phpize($resource, 'itemUriTemplate', 'string'), | ||
| 'operations' => $this->buildOperations($resource, $base), | ||
| 'graphQlOperations' => $this->buildGraphQlOperations($resource, $base), | ||
| ]); | ||
|
|
@@ -411,7 +415,7 @@ private function buildOperations(\SimpleXMLElement $resource, array $root): ?arr | |
| } | ||
| } | ||
|
|
||
| if (\in_array((string) $operation['class'], [GetCollection::class, Post::class], true)) { | ||
| if (\in_array((string) $operation['class'], [GetCollection::class, Post::class, Get::class, Patch::class, Put::class], true)) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now that Related: none of
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed, the hardcoded list contradicts the class model. I have not changed it yet because the right rule falls out of the |
||
| $datum['itemUriTemplate'] = $this->phpize($operation, 'itemUriTemplate', 'string'); | ||
| } elseif (isset($operation['itemUriTemplate'])) { | ||
| throw new InvalidArgumentException(\sprintf('"itemUriTemplate" option is not allowed on a %s operation.', $operation['class'])); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -225,6 +225,7 @@ public function __construct( | |
| ?bool $throwOnNotFound = null, | ||
| array $extraProperties = [], | ||
| ?bool $map = null, | ||
| protected ?string $itemUriTemplate = null, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Putting Branches that consequently switch on for plain item requests:
MCP is the one I'd worry about most. None of this is covered by tests. Worth either scoping the cascade (skip operations whose own
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm actually wondering if we shouldn't just put the item_uri_template where it makes sense...
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed on all of it, and the MCP one is the worst: nothing about The root cause of most of that list is narrower than it looks. On your follow-up, "where it makes sense": my reading is That is a rewrite of the metadata half of the PR, so I would rather have your yes before doing it. Three questions:
|
||
| ) { | ||
| $this->formats = (null === $formats || \is_array($formats)) ? $formats : [$formats]; | ||
| $this->inputFormats = (null === $inputFormats || \is_array($inputFormats)) ? $inputFormats : [$inputFormats]; | ||
|
|
@@ -316,6 +317,19 @@ public function withUriTemplate(?string $uriTemplate = null): static | |
| return $self; | ||
| } | ||
|
|
||
| public function getItemUriTemplate(): ?string | ||
| { | ||
| return $this->itemUriTemplate; | ||
| } | ||
|
|
||
| public function withItemUriTemplate(?string $itemUriTemplate = null): static | ||
| { | ||
| $self = clone $this; | ||
| $self->itemUriTemplate = $itemUriTemplate; | ||
|
|
||
| return $self; | ||
| } | ||
|
|
||
| public function getTypes(): ?array | ||
| { | ||
| return $this->types; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same three points as the Symfony counterpart: nullable
create()result dereferenced at line 147 with no guard, missing$context['item_uri_template'], and placement before the name-resolution fallback.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same two fixes applied here: the
create()result is only used when it resolves, and the branch sets$context['item_uri_template']. Placement waits on the same decision as the Symfony one.