diff --git a/src/State/ParameterProvider/PreservesUriVariableInterface.php b/src/State/ParameterProvider/PreservesUriVariableInterface.php new file mode 100644 index 00000000000..555c8f0c8a7 --- /dev/null +++ b/src/State/ParameterProvider/PreservesUriVariableInterface.php @@ -0,0 +1,31 @@ + + * + * 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; + +use ApiPlatform\Metadata\Parameter; + +/** + * 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 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 4a43f6c3c20..c1aba50e9d6 100644 --- a/src/State/ParameterProvider/ReadLinkParameterProvider.php +++ b/src/State/ParameterProvider/ReadLinkParameterProvider.php @@ -27,17 +27,28 @@ /** * 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 + * @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 05bd072cb55..1f8eb93be77 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,39 @@ private function handlePathParameters(HttpOperation $operation, array $uriVariab if (($op = $this->callParameterProvider($operation, $uriVariable, $values, $context)) instanceof HttpOperation) { $context['operation'] = $operation = $op; } + + if (!$uriVariable->getValue() instanceof ParameterNotFound && !$this->preservesUriVariable($operation, $uriVariable)) { + $uriVariables[$key] = $uriVariable->getValue(); + } } 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)) { + 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 +191,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..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,14 +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, - private readonly ?ProviderInterface $parameterProvider = 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; } @@ -88,7 +94,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/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/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..a9437169b34 --- /dev/null +++ b/tests/Functional/Parameters/UriVariableParameterProviderTest.php @@ -0,0 +1,81 @@ + + * + * 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\ApiResource\WriteUriVariableLinkResource; +use ApiPlatform\Tests\Fixtures\TestBundle\Entity\Base64UriVariableDummy; +use ApiPlatform\Tests\Fixtures\TestBundle\Entity\Dummy; +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, WriteUriVariableLinkResource::class, Dummy::class]; + } + + protected function setUp(): void + { + $this->recreateSchema([Base64UriVariableDummy::class, Dummy::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']); + } + + 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']); + } +}