Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/Laravel/Routing/IriConverter.php
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,17 @@ public function getIriFromResource(object|string $resource, int $referenceType =
}

$identifiersExtractorOperation = $operation;

// The IRI of an item operation with a custom URI template points to the canonical operation declared with "itemUriTemplate"
if (!isset($context['item_uri_template']) && $operation instanceof HttpOperation && !$operation instanceof CollectionOperationInterface && null !== ($itemUriTemplate = $operation->getItemUriTemplate())) {

Copy link
Copy Markdown
Member

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.

Copy link
Copy Markdown
Contributor Author

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.

// An unresolvable template leaves the operation alone rather than breaking every IRI of the resource
if ($canonicalOperation = $this->operationMetadataFactory->create($itemUriTemplate)) {
$operation = $canonicalOperation;
$identifiersExtractorOperation = $operation;
$context['item_uri_template'] = $itemUriTemplate;
}
}

// In symfony the operation name is the route name, try to find one if none provided
if (
!$operation->getName()
Expand Down
68 changes: 68 additions & 0 deletions src/Metadata/ApiResource.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The 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 (op ?? resource ?? null), and the fact that the default also lands on GetCollection/Post, where itemUriTemplate already had a meaning. That one is semantically coherent (both already mean "items of this collection point at that template"), just worth stating.

Also needs a docs PR on api-platform/docs before this ships.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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 GetCollection and Post the option keeps the meaning it already had.

Docs PR on api-platform/docs once the shape settles, which I would rather not write twice.

) {
parent::__construct(
shortName: $shortName,
Expand Down Expand Up @@ -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;
Expand Down
6 changes: 5 additions & 1 deletion src/Metadata/Extractor/XmlResourceExtractor.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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),
]);
Expand Down Expand Up @@ -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)) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that itemUriTemplate lives on HttpOperation, this hardcoded list is out of step with the class model: Delete, NotExposed, Error, McpTool and McpResource all extend HttpOperation, so they inherit the getter/wither and receive the resource-level cascade, yet XML/YAML rejects the option on them with "not allowed". The list also rejects user subclasses of Get. is_a($operation['class'], HttpOperation::class, true) expresses the actual rule. Same at YamlResourceExtractor.php:356.

Related: none of Delete/NotExposed/Error/McpTool/McpResource got a constructor parameter, so the option is unreachable from PHP attributes on them while still arriving via cascade. Worth making deliberate either way.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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 HttpOperation thread: is_a($class, HttpOperation::class, true) if the option stays there, or the new interface if it moves onto the five operations that have an item IRI. Same for your related point — whichever set ends up carrying the getter should also carry the constructor parameter, so the option is reachable from attributes and not only through the cascade. I will do both in one go once you pick.

$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']));
Expand Down
6 changes: 5 additions & 1 deletion src/Metadata/Extractor/YamlResourceExtractor.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -89,6 +92,7 @@ private function buildResources(array $resourcesYaml, string $path): void
try {
$base = $this->buildExtendedBase($resourceYamlDatum);
$this->resources[$resourceName][$resourcesCount + $key] = array_merge($base, [
'itemUriTemplate' => $this->phpize($resourceYamlDatum, 'itemUriTemplate', 'string'),
'operations' => $this->buildOperations($resourceYamlDatum, $base),
'graphQlOperations' => $this->buildGraphQlOperations($resourceYamlDatum, $base),
]);
Expand Down Expand Up @@ -349,7 +353,7 @@ private function buildOperations(array $resource, array $root): ?array
}
}

if (\in_array((string) $class, [GetCollection::class, Post::class], true)) {
if (\in_array((string) $class, [GetCollection::class, Post::class, Get::class, Patch::class, Put::class], true)) {
$datum['itemUriTemplate'] = $this->phpize($operation, 'itemUriTemplate', 'string');
} elseif (isset($operation['itemUriTemplate'])) {
throw new InvalidArgumentException(\sprintf('"itemUriTemplate" option is not allowed on a %s operation.', $class));
Expand Down
1 change: 1 addition & 0 deletions src/Metadata/Extractor/schema/resources.xsd
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
</xsd:sequence>
<xsd:attributeGroup ref="extendedBase"/>
<xsd:attribute type="xsd:string" name="uriTemplate"/>
<xsd:attribute type="xsd:string" name="itemUriTemplate"/>
<xsd:attribute type="xsd:string" name="class" use="required"/>
</xsd:complexType>

Expand Down
4 changes: 3 additions & 1 deletion src/Metadata/Get.php
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ public function __construct(
?bool $throwOnNotFound = null,
array $extraProperties = [],
?bool $map = null,
?string $itemUriTemplate = null,
) {
parent::__construct(
uriTemplate: $uriTemplate,
Expand Down Expand Up @@ -189,7 +190,8 @@ class: $class,
jsonStream: $jsonStream,
throwOnNotFound: $throwOnNotFound,
extraProperties: $extraProperties,
map: $map
map: $map,
itemUriTemplate: $itemUriTemplate
);
}
}
18 changes: 3 additions & 15 deletions src/Metadata/GetCollection.php
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ public function __construct(
?bool $jsonStream = null,
array $extraProperties = [],
?bool $throwOnNotFound = null,
private ?string $itemUriTemplate = null,
?string $itemUriTemplate = null,
?bool $map = null,
) {
parent::__construct(
Expand Down Expand Up @@ -190,20 +190,8 @@ class: $class,
strictQueryParameterValidation: $strictQueryParameterValidation,
hideHydraOperation: $hideHydraOperation,
stateOptions: $stateOptions,
map: $map
map: $map,
itemUriTemplate: $itemUriTemplate
);
}

public function getItemUriTemplate(): ?string
{
return $this->itemUriTemplate;
}

public function withItemUriTemplate(string $itemUriTemplate): self
{
$self = clone $this;
$self->itemUriTemplate = $itemUriTemplate;

return $self;
}
}
14 changes: 14 additions & 0 deletions src/Metadata/HttpOperation.php
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@ public function __construct(
?bool $throwOnNotFound = null,
array $extraProperties = [],
?bool $map = null,
protected ?string $itemUriTemplate = null,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Putting itemUriTemplate on HttpOperation has a side effect that reaches well beyond IRI generation, because SerializerContextBuilder.php:71 gates on method_exists($operation, 'getItemUriTemplate'). Before this PR only Post could satisfy it on a non-collection operation; now every Get/Put/Patch on a resource that uses the resource-level default does, so $context['item_uri_template'] is set at the root of ordinary item requests — including on the canonical Get, which cascade makes point at itself.

Branches that consequently switch on for plain item requests:

  • JsonLd/Serializer/ItemNormalizer.php:84 — resources with output: no longer short-circuit to parent::normalize()
  • JsonLd/Serializer/ItemNormalizer.php:147-166@type resolution takes the item_uri_template branch
  • JsonLd/ContextBuilder.php:136getResourceContext() stops emitting @id
  • Symfony/Routing/IriConverter.php:137 and :145 — skips the skolem fallback and the getResourceClass() inheritance handling
  • JsonApi/Serializer/ItemNormalizer.php:120

MCP is the one I'd worry about most. McpTool and McpResource extend HttpOperation (src/Metadata/McpTool.php:26, McpResource.php:26) and are built through getOperationWithDefaults() (MetadataCollectionFactoryTrait.php:104), so they receive the cascade too. Mcp/Routing/IriConverter.php:37 suppresses IRI generation for MCP operations only while !isset($context['item_uri_template']) — and Mcp/State/StructuredContentProcessor.php:61 goes through SerializerContextBuilder. Net effect: any resource with a resource-level itemUriTemplate starts emitting @id/IRIs in MCP structured content. That looks unintended.

None of this is covered by tests. Worth either scoping the cascade (skip operations whose own uriTemplate already equals the itemUriTemplate, and skip MCP operations) or explicitly deciding these are wanted.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The 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...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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 McpTool/McpResource says they want an @id, and they get one purely because they extend HttpOperation.

The root cause of most of that list is narrower than it looks. SerializerContextBuilder gates on method_exists($operation, 'getItemUriTemplate') and a non-null value, so the branches only flip because the cascade hands the canonical Get a template pointing at itself. Skipping the cascade when an operation's own uriTemplate already equals the resource itemUriTemplate removes the whole plain-item-request fallout, whatever we decide about the class model.

On your follow-up, "where it makes sense": my reading is Get, Put, Patch plus the two that already had it, Post and GetCollection, and nothing else. Delete, NotExposed, Error, McpTool and McpResource have no item IRI to canonicalize. So the shape would be an interface plus a small trait carrying the getter and the wither, implemented by those five, rather than a constructor parameter on HttpOperation. SerializerContextBuilder's method_exists() gate then stays false for MCP by construction, and the extractor rule in the other thread becomes that interface instead of is_a(..., HttpOperation::class).

That is a rewrite of the metadata half of the PR, so I would rather have your yes before doing it. Three questions:

  1. Interface + trait on the five operations, or keep HttpOperation and scope the cascade?
  2. Skip the cascade when the operation's own uriTemplate equals the template — regardless of 1?
  3. If the interface route, should the five get the constructor parameter as well, so it is reachable from PHP attributes and not only through cascade?

) {
$this->formats = (null === $formats || \is_array($formats)) ? $formats : [$formats];
$this->inputFormats = (null === $inputFormats || \is_array($inputFormats)) ? $inputFormats : [$inputFormats];
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 3 additions & 1 deletion src/Metadata/Patch.php
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ public function __construct(
?bool $throwOnNotFound = null,
array $extraProperties = [],
?bool $map = null,
?string $itemUriTemplate = null,
) {
parent::__construct(
method: 'PATCH',
Expand Down Expand Up @@ -190,7 +191,8 @@ class: $class,
jsonStream: $jsonStream,
throwOnNotFound: $throwOnNotFound,
extraProperties: $extraProperties,
map: $map
map: $map,
itemUriTemplate: $itemUriTemplate
);
}
}
18 changes: 3 additions & 15 deletions src/Metadata/Post.php
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ public function __construct(
?bool $jsonStream = null,
?bool $throwOnNotFound = null,
array $extraProperties = [],
private ?string $itemUriTemplate = null,
?string $itemUriTemplate = null,
?bool $strictQueryParameterValidation = null,
?bool $hideHydraOperation = null,
?bool $map = null,
Expand Down Expand Up @@ -191,20 +191,8 @@ class: $class,
jsonStream: $jsonStream,
throwOnNotFound: $throwOnNotFound,
extraProperties: $extraProperties,
map: $map
map: $map,
itemUriTemplate: $itemUriTemplate
);
}

public function getItemUriTemplate(): ?string
{
return $this->itemUriTemplate;
}

public function withItemUriTemplate(string $itemUriTemplate): self
{
$self = clone $this;
$self->itemUriTemplate = $itemUriTemplate;

return $self;
}
}
4 changes: 3 additions & 1 deletion src/Metadata/Put.php
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ public function __construct(
?bool $hideHydraOperation = null,
private ?bool $allowCreate = null,
?bool $map = null,
?string $itemUriTemplate = null,
) {
parent::__construct(
method: 'PUT',
Expand Down Expand Up @@ -191,7 +192,8 @@ class: $class,
jsonStream: $jsonStream,
throwOnNotFound: $throwOnNotFound,
extraProperties: $extraProperties,
map: $map
map: $map,
itemUriTemplate: $itemUriTemplate
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ final class XmlResourceAdapter implements ResourceAdapterInterface
{
private const ATTRIBUTES = [
'uriTemplate',
'itemUriTemplate',
'shortName',
'description',
'routePrefix',
Expand Down
Loading
Loading