From a72054f1c6a108472ccaa5c4af2d59cedee06d25 Mon Sep 17 00:00:00 2001 From: soyuka Date: Sat, 6 Jun 2026 08:31:16 +0200 Subject: [PATCH] fix(serializer): warn when readableLink false cannot be honored on untyped collections MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a property is annotated readableLink: false but its element type cannot be resolved (e.g. `getFoos(): array` without `@return list` phpdoc and without `#[ApiProperty(nativeType: ...)]`), the to-many resource arm in AbstractItemNormalizer::getAttributeValue cannot match and the inner serializer embeds the related objects — silently ignoring the user's intent. Rather than peeking at runtime values (which would silently rescue misconfiguration and diverge runtime from declared type), trigger a deprecation in dev pointing the user to declare the element type. The canonical to-many arm already honors readableLink: false correctly when the element type is declared. Refs #8179 --- src/Serializer/AbstractItemNormalizer.php | 2 + .../Tests/AbstractItemNormalizerTest.php | 60 +++++++++++++ .../ReadableLinkArrayCollection/Api.php | 44 ++++++++++ .../ReadableLinkArrayCollection/Client.php | 87 +++++++++++++++++++ .../ReadableLinkArrayCollectionTest.php | 80 +++++++++++++++++ 5 files changed, 273 insertions(+) create mode 100644 tests/Fixtures/TestBundle/ApiResource/ReadableLinkArrayCollection/Api.php create mode 100644 tests/Fixtures/TestBundle/ApiResource/ReadableLinkArrayCollection/Client.php create mode 100644 tests/Functional/Serializer/ReadableLinkArrayCollectionTest.php diff --git a/src/Serializer/AbstractItemNormalizer.php b/src/Serializer/AbstractItemNormalizer.php index 7c7b5dae38..bc3b7d7a58 100644 --- a/src/Serializer/AbstractItemNormalizer.php +++ b/src/Serializer/AbstractItemNormalizer.php @@ -1060,6 +1060,8 @@ protected function getAttributeValue(object $object, string $attribute, ?string if ($type instanceof CollectionType) { if (($subType = $type->getCollectionValueType()) instanceof ObjectType) { $context = $this->createOperationContext($context, $subType->getClassName(), $propertyMetadata); + } elseif (false === $propertyMetadata->isReadableLink()) { + trigger_deprecation('api-platform/core', '4.3', 'Property "%s::$%s" sets "readableLink: false" but its collection element type cannot be resolved. Declare it via PHPDoc "@return list" or "#[ApiProperty(nativeType: ...)]"; otherwise "readableLink: false" cannot be honored and related items will be embedded.', $object::class, $attribute); } $childContext = $this->createChildContext($context, $attribute, $format); diff --git a/src/Serializer/Tests/AbstractItemNormalizerTest.php b/src/Serializer/Tests/AbstractItemNormalizerTest.php index 55eb5dcdad..b848d76969 100644 --- a/src/Serializer/Tests/AbstractItemNormalizerTest.php +++ b/src/Serializer/Tests/AbstractItemNormalizerTest.php @@ -223,6 +223,66 @@ public function testNormalizeNullableToManyRelationReturnsNull(): void ])); } + public function testNormalizeArrayBackedRelationWithReadableLinkFalseTriggersDeprecation(): void + { + if (!method_exists(PropertyInfoExtractor::class, 'getType')) { + $this->markTestSkipped('Requires symfony/type-info native types.'); + } + + $relatedDummy = new RelatedDummy(); + $relatedDummy->setId(2); + + $dummy = new Dummy(); + $dummy->setName('foo'); + + $propertyNameCollectionFactoryProphecy = $this->prophesize(PropertyNameCollectionFactoryInterface::class); + $propertyNameCollectionFactoryProphecy->create(Dummy::class, Argument::type('array'))->willReturn(new PropertyNameCollection(['name', 'relatedDummies'])); + + $propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class); + $propertyMetadataFactoryProphecy->create(Dummy::class, 'name', Argument::type('array'))->willReturn((new ApiProperty())->withNativeType(Type::string())->withDescription('')->withReadable(true)); + $propertyMetadataFactoryProphecy->create(Dummy::class, 'relatedDummies', Argument::type('array'))->willReturn((new ApiProperty())->withNativeType(Type::array())->withReadable(true)->withWritable(false)->withReadableLink(false)); + + $iriConverterProphecy = $this->prophesize(IriConverterInterface::class); + $iriConverterProphecy->getIriFromResource($dummy, Argument::cetera())->willReturn('/dummies/1'); + + $propertyAccessorProphecy = $this->prophesize(PropertyAccessorInterface::class); + $propertyAccessorProphecy->getValue($dummy, 'name')->willReturn('foo'); + $propertyAccessorProphecy->getValue($dummy, 'relatedDummies')->willReturn([$relatedDummy]); + + $resourceClassResolverProphecy = $this->prophesize(ResourceClassResolverInterface::class); + $resourceClassResolverProphecy->getResourceClass(null, Dummy::class)->willReturn(Dummy::class); + $resourceClassResolverProphecy->getResourceClass($dummy, null)->willReturn(Dummy::class); + $resourceClassResolverProphecy->isResourceClass(Dummy::class)->willReturn(true); + + $serializerProphecy = $this->prophesize(SerializerInterface::class); + $serializerProphecy->willImplement(NormalizerInterface::class); + $serializerProphecy->normalize(Argument::any(), null, Argument::type('array'))->willReturn('foo'); + + $normalizer = new class($propertyNameCollectionFactoryProphecy->reveal(), $propertyMetadataFactoryProphecy->reveal(), $iriConverterProphecy->reveal(), $resourceClassResolverProphecy->reveal(), $propertyAccessorProphecy->reveal(), null, null, [], null, null) extends AbstractItemNormalizer {}; + $normalizer->setSerializer($serializerProphecy->reveal()); + + $deprecations = []; + set_error_handler(static function (int $errno, string $errstr) use (&$deprecations): bool { + $deprecations[] = $errstr; + + return true; + }, \E_USER_DEPRECATED); + + try { + $normalizer->normalize($dummy, null, [ + 'resources' => [], + ]); + } finally { + restore_error_handler(); + } + + $matched = array_filter($deprecations, static fn (string $m): bool => str_contains($m, 'relatedDummies') && str_contains($m, Dummy::class)); + $this->assertNotEmpty( + $matched, + \sprintf("Expected deprecation about relatedDummies. Got:\n - %s", implode("\n - ", $deprecations)) + ); + } + public function testNormalizeWithSecuredProperty(): void { $dummy = new SecuredDummy(); diff --git a/tests/Fixtures/TestBundle/ApiResource/ReadableLinkArrayCollection/Api.php b/tests/Fixtures/TestBundle/ApiResource/ReadableLinkArrayCollection/Api.php new file mode 100644 index 0000000000..fbf47ea2f1 --- /dev/null +++ b/tests/Fixtures/TestBundle/ApiResource/ReadableLinkArrayCollection/Api.php @@ -0,0 +1,44 @@ + + * + * 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\ReadableLinkArrayCollection; + +use ApiPlatform\Metadata\ApiProperty; +use ApiPlatform\Metadata\ApiResource; +use ApiPlatform\Metadata\Get; +use ApiPlatform\Metadata\Operation; + +#[ApiResource( + shortName: 'ReadableLinkArrayCollectionApi', + operations: [ + new Get( + uriTemplate: '/readable_link_array_collection_apis/{id}', + uriVariables: ['id'], + provider: [self::class, 'provide'], + ), + ], +)] +class Api +{ + public function __construct( + #[ApiProperty(identifier: true)] + public int $id = 1, + public string $label = 'default', + ) { + } + + public static function provide(Operation $operation, array $uriVariables = []): self + { + return new self((int) ($uriVariables['id'] ?? 1)); + } +} diff --git a/tests/Fixtures/TestBundle/ApiResource/ReadableLinkArrayCollection/Client.php b/tests/Fixtures/TestBundle/ApiResource/ReadableLinkArrayCollection/Client.php new file mode 100644 index 0000000000..6c3529ff14 --- /dev/null +++ b/tests/Fixtures/TestBundle/ApiResource/ReadableLinkArrayCollection/Client.php @@ -0,0 +1,87 @@ + + * + * 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\ReadableLinkArrayCollection; + +use ApiPlatform\Metadata\ApiProperty; +use ApiPlatform\Metadata\ApiResource; +use ApiPlatform\Metadata\Get; +use ApiPlatform\Metadata\Operation; + +#[ApiResource( + shortName: 'ReadableLinkArrayCollectionClient', + operations: [ + new Get( + uriTemplate: '/readable_link_array_collection_clients/{id}', + uriVariables: ['id'], + provider: [self::class, 'provide'], + ), + ], +)] +class Client +{ + #[ApiProperty(identifier: true)] + public int $id = 1; + + #[ApiProperty(readableLink: false)] + public ?Api $singleApi = null; + + private array $typedExchangeApis = []; + + private array $untypedExchangeApis = []; + + /** + * @return list + */ + #[ApiProperty(readableLink: false)] + public function getTypedExchangeApis(): array + { + return $this->typedExchangeApis; + } + + /** + * @param list $typedExchangeApis + */ + public function setTypedExchangeApis(array $typedExchangeApis): void + { + $this->typedExchangeApis = $typedExchangeApis; + } + + #[ApiProperty(readableLink: false)] + public function getUntypedExchangeApis(): array + { + return $this->untypedExchangeApis; + } + + public function setUntypedExchangeApis(array $untypedExchangeApis): void + { + $this->untypedExchangeApis = $untypedExchangeApis; + } + + public static function provide(Operation $operation, array $uriVariables = []): self + { + $client = new self(); + $client->id = (int) ($uriVariables['id'] ?? 1); + $client->singleApi = new Api(2, 'single'); + $client->setTypedExchangeApis([ + new Api(3, 'exchange-a'), + new Api(4, 'exchange-b'), + ]); + $client->setUntypedExchangeApis([ + new Api(5, 'exchange-c'), + new Api(6, 'exchange-d'), + ]); + + return $client; + } +} diff --git a/tests/Functional/Serializer/ReadableLinkArrayCollectionTest.php b/tests/Functional/Serializer/ReadableLinkArrayCollectionTest.php new file mode 100644 index 0000000000..7e89060538 --- /dev/null +++ b/tests/Functional/Serializer/ReadableLinkArrayCollectionTest.php @@ -0,0 +1,80 @@ + + * + * 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\Serializer; + +use ApiPlatform\Symfony\Bundle\Test\ApiTestCase; +use ApiPlatform\Tests\Fixtures\TestBundle\ApiResource\ReadableLinkArrayCollection\Api; +use ApiPlatform\Tests\Fixtures\TestBundle\ApiResource\ReadableLinkArrayCollection\Client; +use ApiPlatform\Tests\SetupClassResourcesTrait; + +final class ReadableLinkArrayCollectionTest extends ApiTestCase +{ + use SetupClassResourcesTrait; + + protected static ?bool $alwaysBootKernel = false; + + public static function getResources(): array + { + return [Client::class, Api::class]; + } + + public function testToOneRelationWithReadableLinkFalseRendersIri(): void + { + $response = self::createClient()->request('GET', '/readable_link_array_collection_clients/1', [ + 'headers' => ['Accept' => 'application/ld+json'], + ]); + + $this->assertResponseIsSuccessful(); + $body = $response->toArray(); + $this->assertSame('/readable_link_array_collection_apis/2', $body['singleApi']); + } + + public function testTypedArrayCollectionWithReadableLinkFalseRendersIris(): void + { + $response = self::createClient()->request('GET', '/readable_link_array_collection_clients/1', [ + 'headers' => ['Accept' => 'application/ld+json'], + ]); + + $this->assertResponseIsSuccessful(); + $body = $response->toArray(); + $this->assertSame( + ['/readable_link_array_collection_apis/3', '/readable_link_array_collection_apis/4'], + $body['typedExchangeApis'], + ); + } + + public function testUntypedArrayCollectionWithReadableLinkFalseTriggersDeprecation(): void + { + $deprecations = []; + set_error_handler(static function (int $errno, string $errstr) use (&$deprecations): bool { + $deprecations[] = $errstr; + + return true; + }, \E_USER_DEPRECATED); + + try { + self::createClient()->request('GET', '/readable_link_array_collection_clients/1', [ + 'headers' => ['Accept' => 'application/ld+json'], + ]); + } finally { + restore_error_handler(); + } + + $matched = array_filter($deprecations, static fn (string $m): bool => str_contains($m, 'untypedExchangeApis')); + $this->assertNotEmpty( + $matched, + \sprintf("Expected deprecation about untypedExchangeApis. Got:\n - %s", implode("\n - ", $deprecations)) + ); + } +}