From 998b4b50fe0c8eb728571d551869070119622b31 Mon Sep 17 00:00:00 2001 From: soyuka Date: Tue, 1 Sep 2026 16:45:24 +0200 Subject: [PATCH 1/2] feat(state): apply uri variable provider values A parameter provider declared on a uri variable ran, but its value was discarded: handlePathParameters() received $uriVariables by value and only returned the Operation, so the array forwarded to the Doctrine links handler kept the raw route value. Transforming a uri variable was therefore only possible through a global UriVariableTransformerInterface service, which cannot tell which variable it is transforming. Write the resolved value back so uriVariables: ['x' => new Link(provider: ...)] transforms the value used to query the resource, as QueryParameter already does for filters. ReadLinkParameterProvider is excluded: it sets the value to a hydrated resource for security expressions, which must not reach the query as an identifier. It declares this via PreservesUriVariableInterface, so user providers doing the same can opt out too. Providers are otherwise assumed to transform their value, which keeps callable providers working. In listeners mode ParameterProvider was wired standalone with no decorated inner and called separately by ReadListener, so a write-back could not cross the two calls. It now decorates the read chain in both modes, which also stops ReadListener from handing ReadProvider a stale Operation. --- .../PreservesUriVariableInterface.php | 25 ++++++++ .../ReadLinkParameterProvider.php | 2 +- src/State/Provider/ParameterProvider.php | 32 +++++++--- .../Resources/config/symfony/events.php | 6 +- src/Symfony/EventListener/ReadListener.php | 2 - .../Entity/Base64UriVariableDummy.php | 52 ++++++++++++++++ .../UriVariableParameterProviderTest.php | 61 +++++++++++++++++++ 7 files changed, 167 insertions(+), 13 deletions(-) create mode 100644 src/State/ParameterProvider/PreservesUriVariableInterface.php create mode 100644 tests/Fixtures/TestBundle/Entity/Base64UriVariableDummy.php create mode 100644 tests/Functional/Parameters/UriVariableParameterProviderTest.php diff --git a/src/State/ParameterProvider/PreservesUriVariableInterface.php b/src/State/ParameterProvider/PreservesUriVariableInterface.php new file mode 100644 index 00000000000..47ecaf40b11 --- /dev/null +++ b/src/State/ParameterProvider/PreservesUriVariableInterface.php @@ -0,0 +1,25 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\State\ParameterProvider; + +/** + * Marks a ParameterProviderInterface whose value must not replace the uri variable it was computed from. + * + * A provider is otherwise assumed to transform its parameter value, and the result becomes the uri + * variable the resource is queried with. Implement this interface when the value is something else, + * typically a resource resolved for a security expression rather than an identifier. + */ +interface PreservesUriVariableInterface +{ +} diff --git a/src/State/ParameterProvider/ReadLinkParameterProvider.php b/src/State/ParameterProvider/ReadLinkParameterProvider.php index 4a43f6c3c20..cbeaf003b09 100644 --- a/src/State/ParameterProvider/ReadLinkParameterProvider.php +++ b/src/State/ParameterProvider/ReadLinkParameterProvider.php @@ -27,7 +27,7 @@ /** * Checks if the linked resources have security attributes and prepares them for access checking. */ -final class ReadLinkParameterProvider implements ParameterProviderInterface +final class ReadLinkParameterProvider implements ParameterProviderInterface, PreservesUriVariableInterface { /** * @param ProviderInterface $locator diff --git a/src/State/Provider/ParameterProvider.php b/src/State/Provider/ParameterProvider.php index 05bd072cb55..a163e692b3b 100644 --- a/src/State/Provider/ParameterProvider.php +++ b/src/State/Provider/ParameterProvider.php @@ -19,6 +19,7 @@ use ApiPlatform\State\Exception\ParameterNotSupportedException; use ApiPlatform\State\Exception\ProviderNotFoundException; use ApiPlatform\State\ParameterNotFound; +use ApiPlatform\State\ParameterProvider\PreservesUriVariableInterface; use ApiPlatform\State\ParameterProvider\ReadLinkParameterProvider; use ApiPlatform\State\ProviderInterface; use ApiPlatform\State\StopwatchAwareInterface; @@ -108,7 +109,7 @@ public function provide(Operation $operation, array $uriVariables = [], array $c * @param array $uriVariables * @param array $context */ - private function handlePathParameters(HttpOperation $operation, array $uriVariables, array $context): HttpOperation + private function handlePathParameters(HttpOperation $operation, array &$uriVariables, array $context): HttpOperation { foreach ($operation->getUriVariables() ?? [] as $key => $uriVariable) { $uriVariable = $uriVariable->withKey($key); @@ -136,11 +137,32 @@ private function handlePathParameters(HttpOperation $operation, array $uriVariab if (($op = $this->callParameterProvider($operation, $uriVariable, $values, $context)) instanceof HttpOperation) { $context['operation'] = $operation = $op; } + + // A provider that resolves the parameter to a resource leaves an object in the value, + // which is not the identifier the resource is queried with. + if (!$uriVariable->getValue() instanceof ParameterNotFound + && !$this->resolveProvider($operation, $uriVariable->getProvider()) instanceof PreservesUriVariableInterface + ) { + $uriVariables[$key] = $uriVariable->getValue(); + } } return $operation; } + private function resolveProvider(Operation $operation, mixed $provider): mixed + { + if (!\is_string($provider)) { + return $provider; + } + + if (!$this->locator->has($provider)) { + throw new ProviderNotFoundException(\sprintf('Provider "%s" not found on operation "%s"', $provider, $operation->getName())); + } + + return $this->locator->get($provider); + } + /** * @param array $context */ @@ -162,13 +184,7 @@ private function callParameterProvider(Operation $operation, Parameter $paramete return $operation; } - if (\is_string($provider)) { - if (!$this->locator->has($provider)) { - throw new ProviderNotFoundException(\sprintf('Provider "%s" not found on operation "%s"', $provider, $operation->getName())); - } - - $provider = $this->locator->get($provider); - } + $provider = $this->resolveProvider($operation, $provider); if (($op = $provider->provide($parameter, $values, $context)) instanceof Operation) { $operation = $op; diff --git a/src/Symfony/Bundle/Resources/config/symfony/events.php b/src/Symfony/Bundle/Resources/config/symfony/events.php index ae428bb459d..1d621b2d617 100644 --- a/src/Symfony/Bundle/Resources/config/symfony/events.php +++ b/src/Symfony/Bundle/Resources/config/symfony/events.php @@ -57,9 +57,12 @@ ->arg(1, service('api_platform.serializer.context_builder')) ->arg('$logger', service('logger')->nullOnInvalid()); + // Outermost decorator of the read chain (access checkers sit at 0) so parameters are + // resolved, and their values propagated to the uriVariables, before anything reads them. $services->set('api_platform.state_provider.parameter', ParameterProvider::class) + ->decorate('api_platform.state_provider.read', null, -10) ->args([ - null, + service('api_platform.state_provider.parameter.inner'), tagged_locator('api_platform.parameter_provider', 'key'), ]); @@ -68,7 +71,6 @@ service('api_platform.state_provider.read'), service('api_platform.metadata.resource.metadata_collection_factory'), service('api_platform.uri_variables.converter'), - service('api_platform.state_provider.parameter')->nullOnInvalid(), ]) ->tag('kernel.event_listener', ['event' => 'kernel.request', 'method' => 'onKernelRequest', 'priority' => 4]); diff --git a/src/Symfony/EventListener/ReadListener.php b/src/Symfony/EventListener/ReadListener.php index 203d2d27644..bb41310e1f4 100644 --- a/src/Symfony/EventListener/ReadListener.php +++ b/src/Symfony/EventListener/ReadListener.php @@ -43,7 +43,6 @@ public function __construct( private readonly ProviderInterface $provider, ?ResourceMetadataCollectionFactoryInterface $resourceMetadataCollectionFactory = null, ?UriVariablesConverterInterface $uriVariablesConverter = null, - private readonly ?ProviderInterface $parameterProvider = null, ) { $this->resourceMetadataCollectionFactory = $resourceMetadataCollectionFactory; $this->uriVariablesConverter = $uriVariablesConverter; @@ -88,7 +87,6 @@ public function onKernelRequest(RequestEvent $event): void 'uri_variables' => $uriVariables, 'resource_class' => $operation->getClass(), ]; - $this->parameterProvider?->provide($operation, $uriVariables, $context); $this->provider->provide($operation, $uriVariables, $context); } } diff --git a/tests/Fixtures/TestBundle/Entity/Base64UriVariableDummy.php b/tests/Fixtures/TestBundle/Entity/Base64UriVariableDummy.php new file mode 100644 index 00000000000..6518e9d391b --- /dev/null +++ b/tests/Fixtures/TestBundle/Entity/Base64UriVariableDummy.php @@ -0,0 +1,52 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Tests\Fixtures\TestBundle\Entity; + +use ApiPlatform\Metadata\Get; +use ApiPlatform\Metadata\Link; +use ApiPlatform\Metadata\Parameter; +use Doctrine\ORM\Mapping as ORM; + +#[ORM\Entity] +// The plain Get must come first so IRI generation resolves to {id}, not the encodedName template. +#[Get] +#[Get( + uriTemplate: '/base64_uri_variable_dummies/encoded/{encodedName}', + uriVariables: [ + 'encodedName' => new Link( + fromClass: self::class, + identifiers: ['name'], + provider: [self::class, 'decodeName'], + ), + ], +)] +class Base64UriVariableDummy +{ + #[ORM\Id] + #[ORM\GeneratedValue] + #[ORM\Column(type: 'integer')] + public ?int $id = null; + + #[ORM\Column] + public string $name; + + /** + * @param array $parameters + * @param array $context + */ + public static function decodeName(Parameter $parameter, array $parameters = [], array $context = []): void + { + $parameter->setValue(base64_decode((string) $parameter->getValue(), true)); + } +} diff --git a/tests/Functional/Parameters/UriVariableParameterProviderTest.php b/tests/Functional/Parameters/UriVariableParameterProviderTest.php new file mode 100644 index 00000000000..ff70921f567 --- /dev/null +++ b/tests/Functional/Parameters/UriVariableParameterProviderTest.php @@ -0,0 +1,61 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Tests\Functional\Parameters; + +use ApiPlatform\Symfony\Bundle\Test\ApiTestCase; +use ApiPlatform\Tests\Fixtures\TestBundle\Entity\Base64UriVariableDummy; +use ApiPlatform\Tests\RecreateSchemaTrait; +use ApiPlatform\Tests\SetupClassResourcesTrait; + +final class UriVariableParameterProviderTest extends ApiTestCase +{ + use RecreateSchemaTrait; + use SetupClassResourcesTrait; + + protected static ?bool $alwaysBootKernel = false; + + /** + * @return class-string[] + */ + public static function getResources(): array + { + return [Base64UriVariableDummy::class]; + } + + protected function setUp(): void + { + $this->recreateSchema([Base64UriVariableDummy::class]); + } + + /** + * @see https://github.com/api-platform/core/pull/8431 + */ + public function testLinkParameterProviderDecodesUriVariableBeforeQuery(): void + { + $container = static::getContainer(); + if ('mongodb' === $container->getParameter('kernel.environment')) { + $this->markTestSkipped(); + } + + $manager = $this->getManager(); + $dummy = new Base64UriVariableDummy(); + $dummy->name = 'Blip'; + $manager->persist($dummy); + $manager->flush(); + + $response = self::createClient()->request('GET', '/base64_uri_variable_dummies/encoded/'.base64_encode('Blip')); + self::assertResponseStatusCodeSame(200); + self::assertEquals('Blip', $response->toArray()['name']); + } +} From 1a36c4633419811aa602584ecf75f50ad8feebde Mon Sep 17 00:00:00 2001 From: soyuka Date: Wed, 2 Sep 2026 11:06:55 +0200 Subject: [PATCH 2/2] feat(state): opt-in uri variable resource write The resolved resource is kept out of the uri variables by default because Doctrine queries the resource with an identifier. A custom provider is often easier to write against the resource itself, so allow opting in: per service through the ReadLinkParameterProvider constructor, or per link through the `write_uri_variable` extra property. PreservesUriVariableInterface therefore carries a method rather than being a pure marker, so the decision can depend on the parameter. Opting in on a Doctrine-backed link still fails, as the links handler binds the identifier with an explicit type; making getIdentifierValue() resource aware is tracked separately. Also restore ReadListener's $parameterProvider argument. The class is public, so removing it breaks the BC promise even though the bundle no longer passes it. Passing it now triggers a deprecation, it will be removed in 6.0. --- .../PreservesUriVariableInterface.php | 14 ++++-- .../ReadLinkParameterProvider.php | 11 +++++ src/State/Provider/ParameterProvider.php | 17 +++++-- src/Symfony/EventListener/ReadListener.php | 9 +++- .../Tests/EventListener/ReadListenerTest.php | 9 ++++ .../WriteUriVariableLinkResource.php | 47 +++++++++++++++++++ .../UriVariableParameterProviderTest.php | 24 +++++++++- 7 files changed, 119 insertions(+), 12 deletions(-) create mode 100644 tests/Fixtures/TestBundle/ApiResource/WriteUriVariableLinkResource.php diff --git a/src/State/ParameterProvider/PreservesUriVariableInterface.php b/src/State/ParameterProvider/PreservesUriVariableInterface.php index 47ecaf40b11..555c8f0c8a7 100644 --- a/src/State/ParameterProvider/PreservesUriVariableInterface.php +++ b/src/State/ParameterProvider/PreservesUriVariableInterface.php @@ -13,13 +13,19 @@ namespace ApiPlatform\State\ParameterProvider; +use ApiPlatform\Metadata\Parameter; + /** - * Marks a ParameterProviderInterface whose value must not replace the uri variable it was computed from. + * Implemented by a ParameterProviderInterface whose value is not always the uri variable it was computed + * from, typically because it resolves a resource for a security expression instead of an identifier. * - * A provider is otherwise assumed to transform its parameter value, and the result becomes the uri - * variable the resource is queried with. Implement this interface when the value is something else, - * typically a resource resolved for a security expression rather than an identifier. + * A provider that does not implement this interface is assumed to transform its parameter value, and the + * result becomes the uri variable the resource is queried with. */ interface PreservesUriVariableInterface { + /** + * Whether the uri variable this parameter was computed from must be left untouched. + */ + public function preservesUriVariable(Parameter $parameter): bool; } diff --git a/src/State/ParameterProvider/ReadLinkParameterProvider.php b/src/State/ParameterProvider/ReadLinkParameterProvider.php index cbeaf003b09..c1aba50e9d6 100644 --- a/src/State/ParameterProvider/ReadLinkParameterProvider.php +++ b/src/State/ParameterProvider/ReadLinkParameterProvider.php @@ -31,13 +31,24 @@ final class ReadLinkParameterProvider implements ParameterProviderInterface, Pre { /** * @param ProviderInterface $locator + * @param bool $writeUriVariable whether the resolved resource replaces the uri variable it was + * resolved from, which lets a custom provider work on the resource + * instead of the identifier. Doctrine-backed resources need the + * identifier, so this is opt-in; it can also be set per link + * through the `write_uri_variable` extra property. */ public function __construct( private readonly ProviderInterface $locator, private readonly ResourceMetadataCollectionFactoryInterface $resourceMetadataCollectionFactory, + private readonly bool $writeUriVariable = false, ) { } + public function preservesUriVariable(Parameter $parameter): bool + { + return !($parameter->getExtraProperties()['write_uri_variable'] ?? $this->writeUriVariable); + } + public function provide(Parameter $parameter, array $parameters = [], array $context = []): ?Operation { $operation = $context['operation']; diff --git a/src/State/Provider/ParameterProvider.php b/src/State/Provider/ParameterProvider.php index a163e692b3b..1f8eb93be77 100644 --- a/src/State/Provider/ParameterProvider.php +++ b/src/State/Provider/ParameterProvider.php @@ -138,11 +138,7 @@ private function handlePathParameters(HttpOperation $operation, array &$uriVaria $context['operation'] = $operation = $op; } - // A provider that resolves the parameter to a resource leaves an object in the value, - // which is not the identifier the resource is queried with. - if (!$uriVariable->getValue() instanceof ParameterNotFound - && !$this->resolveProvider($operation, $uriVariable->getProvider()) instanceof PreservesUriVariableInterface - ) { + if (!$uriVariable->getValue() instanceof ParameterNotFound && !$this->preservesUriVariable($operation, $uriVariable)) { $uriVariables[$key] = $uriVariable->getValue(); } } @@ -150,6 +146,17 @@ private function handlePathParameters(HttpOperation $operation, array &$uriVaria return $operation; } + /** + * A provider may resolve the parameter to a resource, leaving an object in the value where the + * resource is queried with an identifier. + */ + private function preservesUriVariable(Operation $operation, Parameter $parameter): bool + { + $provider = $this->resolveProvider($operation, $parameter->getProvider()); + + return $provider instanceof PreservesUriVariableInterface && $provider->preservesUriVariable($parameter); + } + private function resolveProvider(Operation $operation, mixed $provider): mixed { if (!\is_string($provider)) { diff --git a/src/Symfony/EventListener/ReadListener.php b/src/Symfony/EventListener/ReadListener.php index bb41310e1f4..c9173b352b7 100644 --- a/src/Symfony/EventListener/ReadListener.php +++ b/src/Symfony/EventListener/ReadListener.php @@ -18,6 +18,7 @@ use ApiPlatform\Metadata\Resource\Factory\ResourceMetadataCollectionFactoryInterface; use ApiPlatform\Metadata\UriVariablesConverterInterface; use ApiPlatform\Metadata\Util\CloneTrait; +use ApiPlatform\State\Provider\ParameterProvider; use ApiPlatform\State\ProviderInterface; use ApiPlatform\State\UriVariablesResolverTrait; use ApiPlatform\State\Util\OperationRequestInitiatorTrait; @@ -37,13 +38,19 @@ final class ReadListener use UriVariablesResolverTrait; /** - * @param ProviderInterface $provider + * @param ProviderInterface $provider + * @param ProviderInterface|null $parameterProvider no longer used, the parameter provider decorates the read chain */ public function __construct( private readonly ProviderInterface $provider, ?ResourceMetadataCollectionFactoryInterface $resourceMetadataCollectionFactory = null, ?UriVariablesConverterInterface $uriVariablesConverter = null, + ?ProviderInterface $parameterProvider = null, ) { + if (null !== $parameterProvider) { + trigger_deprecation('api-platform/core', '5.0', 'Passing a "%s" as 4th argument to "%s" is deprecated and will be removed in 6.0, parameters are now provided by "%s" decorating the read provider chain.', ProviderInterface::class, self::class, ParameterProvider::class); + } + $this->resourceMetadataCollectionFactory = $resourceMetadataCollectionFactory; $this->uriVariablesConverter = $uriVariablesConverter; } diff --git a/src/Symfony/Tests/EventListener/ReadListenerTest.php b/src/Symfony/Tests/EventListener/ReadListenerTest.php index 3a008f35578..42e1ab13132 100644 --- a/src/Symfony/Tests/EventListener/ReadListenerTest.php +++ b/src/Symfony/Tests/EventListener/ReadListenerTest.php @@ -23,6 +23,7 @@ use ApiPlatform\State\ProviderInterface; use ApiPlatform\Symfony\EventListener\ReadListener; use PHPUnit\Framework\Attributes\DataProvider; +use PHPUnit\Framework\Attributes\IgnoreDeprecations; use PHPUnit\Framework\TestCase; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Event\RequestEvent; @@ -146,4 +147,12 @@ public function testReadNullWithPostMethod(): void ) ); } + + #[IgnoreDeprecations] + public function testDeprecatedParameterProvider(): void + { + $this->expectUserDeprecationMessage('Since api-platform/core 5.0: Passing a "ApiPlatform\State\ProviderInterface" as 4th argument to "ApiPlatform\Symfony\EventListener\ReadListener" is deprecated and will be removed in 6.0, parameters are now provided by "ApiPlatform\State\Provider\ParameterProvider" decorating the read provider chain.'); + + new ReadListener($this->createStub(ProviderInterface::class), null, null, $this->createStub(ProviderInterface::class)); + } } diff --git a/tests/Fixtures/TestBundle/ApiResource/WriteUriVariableLinkResource.php b/tests/Fixtures/TestBundle/ApiResource/WriteUriVariableLinkResource.php new file mode 100644 index 00000000000..e0b8d3044ea --- /dev/null +++ b/tests/Fixtures/TestBundle/ApiResource/WriteUriVariableLinkResource.php @@ -0,0 +1,47 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Tests\Fixtures\TestBundle\ApiResource; + +use ApiPlatform\Metadata\Get; +use ApiPlatform\Metadata\Link; +use ApiPlatform\Metadata\Operation; +use ApiPlatform\State\ParameterProvider\ReadLinkParameterProvider; +use ApiPlatform\Tests\Fixtures\TestBundle\Entity\Dummy; + +#[Get( + uriTemplate: '/write_uri_variable_link_resources/{id}', + uriVariables: [ + 'id' => new Link( + provider: ReadLinkParameterProvider::class, + fromClass: Dummy::class, + extraProperties: ['write_uri_variable' => true], + ), + ], + provider: [self::class, 'provide'] +)] +class WriteUriVariableLinkResource +{ + public string $id; + public string $dummyName; + + public static function provide(Operation $operation, array $uriVariables = []) + { + $r = new self(); + $r->id = '1'; + // Fails with a TypeError unless the resolved Dummy replaced the identifier. + $r->dummyName = $uriVariables['id']->getName(); + + return $r; + } +} diff --git a/tests/Functional/Parameters/UriVariableParameterProviderTest.php b/tests/Functional/Parameters/UriVariableParameterProviderTest.php index ff70921f567..a9437169b34 100644 --- a/tests/Functional/Parameters/UriVariableParameterProviderTest.php +++ b/tests/Functional/Parameters/UriVariableParameterProviderTest.php @@ -14,7 +14,9 @@ namespace ApiPlatform\Tests\Functional\Parameters; use ApiPlatform\Symfony\Bundle\Test\ApiTestCase; +use ApiPlatform\Tests\Fixtures\TestBundle\ApiResource\WriteUriVariableLinkResource; use ApiPlatform\Tests\Fixtures\TestBundle\Entity\Base64UriVariableDummy; +use ApiPlatform\Tests\Fixtures\TestBundle\Entity\Dummy; use ApiPlatform\Tests\RecreateSchemaTrait; use ApiPlatform\Tests\SetupClassResourcesTrait; @@ -30,12 +32,12 @@ final class UriVariableParameterProviderTest extends ApiTestCase */ public static function getResources(): array { - return [Base64UriVariableDummy::class]; + return [Base64UriVariableDummy::class, WriteUriVariableLinkResource::class, Dummy::class]; } protected function setUp(): void { - $this->recreateSchema([Base64UriVariableDummy::class]); + $this->recreateSchema([Base64UriVariableDummy::class, Dummy::class]); } /** @@ -58,4 +60,22 @@ public function testLinkParameterProviderDecodesUriVariableBeforeQuery(): void self::assertResponseStatusCodeSame(200); self::assertEquals('Blip', $response->toArray()['name']); } + + public function testReadLinkParameterProviderWritesResolvedResourceWhenOptedIn(): void + { + $container = static::getContainer(); + if ('mongodb' === $container->getParameter('kernel.environment')) { + $this->markTestSkipped(); + } + + $manager = $this->getManager(); + $dummy = new Dummy(); + $dummy->setName('hi'); + $manager->persist($dummy); + $manager->flush(); + + $response = self::createClient()->request('GET', '/write_uri_variable_link_resources/'.$dummy->getId()); + self::assertResponseStatusCodeSame(200); + self::assertEquals('hi', $response->toArray()['dummyName']); + } }