fix(serializer): read Symfony attributes through their public properties

[Maxcastel]

Q	A
Branch?	main
Tickets	n/a
License	MIT
Doc PR	n/a

Reading a Symfony serializer attribute went through a method_exists() guard that preferred the getter and fell back to the public property:

$groups = method_exists($attr, 'getGroups') ? $attr->getGroups() : $attr->groups;

The getter is deprecated:

#[\Deprecated('Use the "groups" property instead', 'symfony/serializer:7.4')]
public function getGroups(): array
{
    return $this->groups;
}

https://github.com/symfony/serializer/blob/7.4/Attribute/Groups.php

symfony/serializer CHANGELOG:

• 7.4 - Deprecate getters in attribute classes in favor of public properties
• 8.0 - Remove getters in attribute classes in favor of public properties

https://github.com/symfony/serializer/blob/8.0/CHANGELOG.md

tests

I also added tests to cover the attribute forwarding and make sure regressions in each individual read are caught. The existing tests did not detect incorrect values being forwarded, which is why the test coverage needed to be strengthened.

The forwarding of those attributes had no test at all. Mutating the loader went unnoticed by every suite in the repository: PropertyMetadataLoaderTest, the api-platform/serializer component suite, and tests/Functional all stayed green with this in place:

$attr instanceof MaxDepth => $attributeMetadata->setMaxDepth(0),
$attr instanceof SerializedName => $attributeMetadata->setSerializedName('aa'),

PropertyMetadataLoaderTest only asserted groups and the discriminator defaultType. maxDepth, serializedName, serializedPath, Ignore and Context were never asserted, in this component or anywhere else.

A single assertion per attribute is not enough either: replacing the read with the value the fixture happens to carry also survives:

// caught: "Failed asserting that 0 is identical to 2"
$attr instanceof MaxDepth => $attributeMetadata->setMaxDepth(0),

// survives: the only fixture declared MaxDepth(2)
$attr instanceof MaxDepth => $attributeMetadata->setMaxDepth(2),

So every attribute is now declared twice with distinct values, and the normalization and denormalization contexts differ from each other so that swapping the two reads fails. DiscriminatorMap is covered by a DataProvider
over two fixtures whose typeProperty, mapping and defaultType all differ.

Each of the 13 reads in the loader was then mutated one at a time, using the fixture's own value as the replacement. All 13 are caught.

method_exists($classMetadata, 'getAttributesMetadata')

if (method_exists($classMetadata, 'getAttributesMetadata')) { // @phpstan-ignore-line
    $attributesMetadata = $classMetadata->getAttributesMetadata();
} else {
    $attributesMetadata = $classMetadata->attributesMetadata; // @phpstan-ignore-line
}

The fallback is not just unreachable, it could never have worked: the property is private, and the getter is present on both bounds of the supported range:

private array $attributesMetadata = [];        // line 26

public function getAttributesMetadata(): array // line 47

https://github.com/symfony/serializer/blob/7.4/Mapping/ClassMetadata.php
https://github.com/symfony/serializer/blob/8.1/Mapping/ClassMetadata.php

method_exists(ClassDiscriminatorMapping::class, 'getDefaultType') (testForwardsDiscriminatorDefaultType)

if (!method_exists(ClassDiscriminatorMapping::class, 'getDefaultType')) { // @phpstan-ignore-line
    $this->markTestSkipped('ClassDiscriminatorMapping::getDefaultType() requires symfony/serializer 7.1+.');
}

Superseded by testForwardsDiscriminatorMapping, which asserts typeProperty, mapping and defaultType on two fixtures instead of defaultType on one. Its guard could no longer trigger either, with the Symfony floor at ^7.4:

$this->markTestSkipped('ClassDiscriminatorMapping::getDefaultType() requires symfony/serializer 7.1+.');

getDefaultType() is present and undeprecated on both bounds:

public function getDefaultType(): ?string // line 66

https://github.com/symfony/serializer/blob/7.4/Mapping/ClassDiscriminatorMapping.php
https://github.com/symfony/serializer/blob/8.1/Mapping/ClassDiscriminatorMapping.php