Skip to content

[Serializer] Discriminator is removed when #[Ignore] attribute used on unrelated method #60214

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ruudk opened this issue Apr 14, 2025 · 13 comments · May be fixed by #60511
Open

[Serializer] Discriminator is removed when #[Ignore] attribute used on unrelated method #60214

ruudk opened this issue Apr 14, 2025 · 13 comments · May be fixed by #60511

Comments

@ruudk
Copy link
Contributor

ruudk commented Apr 14, 2025

Symfony version(s) affected

7.2.5

Description

Today I noticed something weird with the Serializer. I have a LoaderInterface that defines a discriminator mapping dynamically. This works great.

As soon as I start using the #[Ignore] attribute in an unrelated method, the discriminator is no longer added to the serialized result.

I created a small isolated reproducer that explains the problem.

How to reproduce

<?php

declare(strict_types=1);

use Symfony\Component\PropertyAccess\PropertyAccess;
use Symfony\Component\PropertyInfo\Extractor\PhpStanExtractor;
use Symfony\Component\PropertyInfo\Extractor\ReflectionExtractor;
use Symfony\Component\PropertyInfo\PropertyInfoExtractor;
use Symfony\Component\Serializer\Attribute\Ignore;
use Symfony\Component\Serializer\Encoder\JsonEncoder;
use Symfony\Component\Serializer\Mapping\ClassDiscriminatorFromClassMetadata;
use Symfony\Component\Serializer\Mapping\ClassDiscriminatorMapping;
use Symfony\Component\Serializer\Mapping\ClassMetadataInterface;
use Symfony\Component\Serializer\Mapping\Factory\ClassMetadataFactory;
use Symfony\Component\Serializer\Mapping\Loader\AttributeLoader;
use Symfony\Component\Serializer\Mapping\Loader\LoaderChain;
use Symfony\Component\Serializer\Mapping\Loader\LoaderInterface;
use Symfony\Component\Serializer\NameConverter\CamelCaseToSnakeCaseNameConverter;
use Symfony\Component\Serializer\NameConverter\MetadataAwareNameConverter;
use Symfony\Component\Serializer\Normalizer\ArrayDenormalizer;
use Symfony\Component\Serializer\Normalizer\BackedEnumNormalizer;
use Symfony\Component\Serializer\Normalizer\DateTimeNormalizer;
use Symfony\Component\Serializer\Normalizer\ObjectNormalizer;
use Symfony\Component\Serializer\Normalizer\PropertyNormalizer;
use Symfony\Component\Serializer\Normalizer\UnwrappingDenormalizer;
use Symfony\Component\Serializer\Serializer;

include 'vendor/autoload.php';

final class DiscriminatorLoader implements LoaderInterface
{
    #[Override]
    public function loadClassMetadata(ClassMetadataInterface $classMetadata) : bool
    {
        if (!in_array($classMetadata->getName(), [ParentInterface::class, ParentWithIgnoreAttributeInterface::class], true)) {
            return false;
        }

        $classMetadata->setClassDiscriminatorMapping(
            new ClassDiscriminatorMapping(
                'discriminator',
                ['Foo1' => Foo1::class, 'Foo2' => Foo2::class],
            ),
        );

        return true;
    }
}

$loader = new LoaderChain([
    new DiscriminatorLoader(),
    new AttributeLoader(),
]);

$classMetadataFactory = new ClassMetadataFactory($loader);
$propertyAccessor = PropertyAccess::createPropertyAccessor();
$reflectionExtractor = new ReflectionExtractor();
$nameConverter = new MetadataAwareNameConverter($classMetadataFactory, new CamelCaseToSnakeCaseNameConverter());
$propertyTypeExtractor = new PropertyInfoExtractor(
    listExtractors: [$reflectionExtractor],
    typeExtractors: [new PhpStanExtractor(), $reflectionExtractor],
    accessExtractors: [$reflectionExtractor],
    initializableExtractors: [$reflectionExtractor],
);
$classDiscriminatorResolver = new ClassDiscriminatorFromClassMetadata($classMetadataFactory);

$serializer = new Serializer(
    [
        new UnwrappingDenormalizer($propertyAccessor),
        new DateTimeNormalizer(),
        new BackedEnumNormalizer(),
        new PropertyNormalizer(
            $classMetadataFactory,
            $nameConverter,
            $propertyTypeExtractor,
            $classDiscriminatorResolver,
            null,
            [],
        ),
        new ArrayDenormalizer(),
        new ObjectNormalizer(
            $classMetadataFactory,
            $nameConverter,
            $propertyAccessor,
            $propertyTypeExtractor,
            $classDiscriminatorResolver,
        ),
    ],
    [new JsonEncoder()],
);

interface ParentWithIgnoreAttributeInterface
{
    #[Ignore]
    public function getDisplayName() : string;
}

final class Foo1 implements ParentWithIgnoreAttributeInterface
{
    #[Override]
    public function getDisplayName() : string
    {
        return 'Some title';
    }

    public function __construct(
        public int $id,
    ) {
    }
}

// When `#[Ignore]` is used on the `getDisplayName` method in ParentWithIgnoreAttributeInterface, it does not show the discriminator:
// result: {"context":{"id":1}}
dump($serializer->serialize(new Foo1(1), 'json'));

interface ParentInterface
{
    // #[Ignore] commented out
    public function getDisplayName() : string;
}

final class Foo2 implements ParentInterface
{
    #[Override]
    public function getDisplayName() : string
    {
        return 'Some title';
    }

    public function __construct(
        public int $id,
    ) {
    }
}

// When `#[Ignore]` is removed from `getDisplayName` method in ParentInterface, it does show the discriminator:
// result: "{"context":{"discriminator":"Foo2","id":1}}
dump($serializer->serialize(new Foo2(1), 'json'));

Possible Solution

I have the feeling this is related to the PropertyNormalizer. When I remove that from the Serializer it works. But since this is a default normalizer, I'd like to keep this.

Additional Context

No response

@ruudk ruudk added the Bug label Apr 14, 2025
@ruudk ruudk changed the title [Serializer] Discriminator is removed when #[Ignore] attribute used on unrelated method [Serializer] Discriminator is removed when #[Ignore] attribute used on unrelated method Apr 14, 2025
@ruudk
Copy link
Contributor Author

ruudk commented Apr 14, 2025

Seems to be related to:

// Backward Compatibility with the code using this method written before the introduction of @Ignore
return false;

@alexdawn
Copy link

I'm getting a similar issue though I don't think i'm even using the #[Ignore] tag, unless there is a vendor bundle using it?

on symfony/serializer v6.4.19. my discriminator is being removed and my deserialized objects are now empty of data.

Same as @ruudk it is hitting the line symfony/src/Symfony/Component/Serializer/Normalizer/AbstractNormalizer.php

Lines 246 to 247 in d5b5581

which with the false value, it is now filtering out all data keys. Will try and find which bugfix introduced this issue as it was previously working for me on 6.4.*

@alexdawn
Copy link

Looking at my old commits and the composer.lock, I was running the deserializer with discriminator columns successfully on v6.4.9 however, my lasted commit if I downgrade from v6.4.19 to v6.4.9 that does not fix the issue, so the issue must come from another bundle?

@ruudk
Copy link
Contributor Author

ruudk commented May 22, 2025

I have a new situation where it breaks and this is a minimal reproducer:

#[\Symfony\Component\Serializer\Attribute\DiscriminatorMap::__construct(
    typeProperty: 'type',
    mapping: [
        'user' => UserActor::class,
        'system' => SystemActor::class,
    ],
)]
abstract readonly class Actor
{
    abstract public function equals(Actor $other) : bool;

    #[\Symfony\Component\Serializer\Attribute\Ignore]
    public function isSystem() : bool
    {
        return $this instanceof SystemActor;
    }

    #[\Symfony\Component\Serializer\Attribute\Ignore]
    public function isUser() : bool
    {
        return $this instanceof UserActor;
    }
}

final readonly class SystemActor extends Actor
{
    public function equals(Actor $other) : bool
    {
        return $other instanceof self;
    }
}

final readonly class UserActor extends Actor
{
    public function __construct(
        public int $userId,
    ) {}

    #[Override]
    public function equals(Actor $other) : bool
    {
        if ( ! $other instanceof self) {
            return false;
        }

        return $this->userId === $other->userId;
    }
}

I have the following test:

#[Test]
    public function it_should_serialize_system_actor() : void
    {
        // Arrange
        $actor = new SystemActor();

        // Act
        $serialized = $this->serializer->serialize($actor, 'json');

        // Assert
        self::assertSame('{"type":"system"}', $serialized);

        // Act
        $deserialized = $this->serializer->deserialize($serialized, Actor::class, 'json');

        // Assert
        self::assertEquals($actor, $deserialized);
    }

    #[Test]
    public function it_should_serialize_user_actor() : void
    {
        // Arrange
        $actor = new UserActor(1);

        // Act
        $serialized = $this->serializer->serialize($actor, 'json');

        // Assert
        self::assertSame('{"type":"user","user_id":1}', $serialized);

        // Act
        $deserialized = $this->serializer->deserialize($serialized, Actor::class, 'json');

        // Assert
        self::assertEquals($actor, $deserialized);
    }

The first one succeeds.
The second one fails:

Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'{"type":"user","user_id":1}'
+'{"user_id":1}'

When I remove the constructor from UserActor, it does add "type":"user".

@ruudk
Copy link
Contributor Author

ruudk commented May 22, 2025

I think I know more why this happens. It seems that the ObjectNormalizer does correctly support the DiscriminatorMap together with Ignore. See:

if (null !== $this->classDiscriminatorResolver) {
$class = \is_object($classOrObject) ? $classOrObject::class : $classOrObject;
if (null !== $discriminatorMapping = $this->classDiscriminatorResolver->getMappingForMappedObject($classOrObject)) {
$allowedAttributes[] = $attributesAsString ? $discriminatorMapping->getTypeProperty() : new AttributeMetadata($discriminatorMapping->getTypeProperty());
}
if (null !== $discriminatorMapping = $this->classDiscriminatorResolver->getMappingForClass($class)) {
$attributes = [];
foreach ($discriminatorMapping->getTypesMapping() as $mappedClass) {
$attributes[] = parent::getAllowedAttributes($mappedClass, $context, $attributesAsString);
}
$allowedAttributes = array_merge($allowedAttributes, ...$attributes);
}
}

But when you use constructor promoted properties (or just public properties) you're using the PropertyNormalizer, it does not override the getAllowedAttributes method, and relies on AbstractNormalizer:

protected function getAllowedAttributes(string|object $classOrObject, array $context, bool $attributesAsString = false): array|bool
{
$allowExtraAttributes = $context[self::ALLOW_EXTRA_ATTRIBUTES] ?? $this->defaultContext[self::ALLOW_EXTRA_ATTRIBUTES];
if (!$this->classMetadataFactory) {
if (!$allowExtraAttributes) {
throw new LogicException(\sprintf('A class metadata factory must be provided in the constructor when setting "%s" to false.', self::ALLOW_EXTRA_ATTRIBUTES));
}
return false;
}
$groups = $this->getGroups($context);
$allowedAttributes = [];
$ignoreUsed = false;
foreach ($this->classMetadataFactory->getMetadataFor($classOrObject)->getAttributesMetadata() as $attributeMetadata) {
if ($ignore = $attributeMetadata->isIgnored()) {
$ignoreUsed = true;
}
// If you update this check, update accordingly the one in Symfony\Component\PropertyInfo\Extractor\SerializerExtractor::getProperties()
if (
!$ignore
&& ([] === $groups || \in_array('*', $groups, true) || array_intersect($attributeMetadata->getGroups(), $groups))
&& $this->isAllowedAttribute($classOrObject, $name = $attributeMetadata->getName(), null, $context)
) {
$allowedAttributes[] = $attributesAsString ? $name : $attributeMetadata;
}
}
if (!$ignoreUsed && [] === $groups && $allowExtraAttributes) {
// Backward Compatibility with the code using this method written before the introduction of @Ignore
return false;
}
return $allowedAttributes;
}

@comma8600
Copy link

I think you are onto something though my example isn't using any constructor promotion or public properties. Possibly the readonly or the protected shared property? Is causing something similar to occur?

@ruudk
Copy link
Contributor Author

ruudk commented May 22, 2025

So far I cannot reproduce it in the Symfony project with a test so maybe it does has something to do with readonly indeed. Will try, thanks!

@ruudk
Copy link
Contributor Author

ruudk commented May 22, 2025

I just tested and removing readonly from the class and or properties does not make a difference.

@ruudk
Copy link
Contributor Author

ruudk commented May 22, 2025

I have the solution, we need to copy

protected function getAllowedAttributes(string|object $classOrObject, array $context, bool $attributesAsString = false): array|bool
{
if (false === $allowedAttributes = parent::getAllowedAttributes($classOrObject, $context, $attributesAsString)) {
return false;
}
if (null !== $this->classDiscriminatorResolver) {
$class = \is_object($classOrObject) ? $classOrObject::class : $classOrObject;
if (null !== $discriminatorMapping = $this->classDiscriminatorResolver->getMappingForMappedObject($classOrObject)) {
$allowedAttributes[] = $attributesAsString ? $discriminatorMapping->getTypeProperty() : new AttributeMetadata($discriminatorMapping->getTypeProperty());
}
if (null !== $discriminatorMapping = $this->classDiscriminatorResolver->getMappingForClass($class)) {
$attributes = [];
foreach ($discriminatorMapping->getTypesMapping() as $mappedClass) {
$attributes[] = parent::getAllowedAttributes($mappedClass, $context, $attributesAsString);
}
$allowedAttributes = array_merge($allowedAttributes, ...$attributes);
}
}
return $allowedAttributes;
}
to PropertyNormalizer and then it works as expected.

I will create a PR.

ruudk added a commit to ruudk/symfony that referenced this issue May 22, 2025
Fixes symfony#60214

Currently it's not possible to serialize an object using the PropertyNormalizer when a
DiscriminatorMap attribute is used.

It produces the following error:

> Symfony\Component\Serializer\Exception\NotNormalizableValueException: Type property "type" not found
> for the abstract object "Symfony\Component\Serializer\Tests\Fixtures\DummyMessageInterface".

The ObjectNormalizer overrides the `getAllowedAttributes` from AbstractNormalizer and adds support for
discriminators. But the PropertyNormalizer does not do this. Therefore it doesn't work.

For now, we copy the logic from ObjectNormalizer to PropertyNormalizer and the problem goes away.
@ruudk
Copy link
Contributor Author

ruudk commented May 22, 2025

@comma8600
Copy link

Nice work though I'm guessing I must have a different issue as that did not help resolve my empty objects:

use App\DTO\TDFDetailedRecord;
use App\DTO\TDFFooterRecord;
use App\DTO\TDFHeaderRecord;
use App\DTO\TDFRecordInterface;
use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
use Symfony\Component\Serializer\Encoder\CsvEncoder;
use Symfony\Component\Serializer\SerializerInterface;

class SerializationTDFRecordsTest extends KernelTestCase
{
    private SerializerInterface $serializer;

    public function setUp(): void
    {
        self::bootKernel();
        $container = self::getContainer();
        $this->serializer = $container->get(SerializerInterface::class);
    }

    public function testHeaderCsv(): void
    {
        $data = 'c0,c1,c2,c3,c4' . PHP_EOL . 'H,foo,20210428,135055,000000';

        $deserialized = $this->serializer->deserialize($data, TDFRecordInterface::class, 'csv', [
            CsvEncoder::AS_COLLECTION_KEY => false // this option means a single row doesn't get wrapped in an additional array
        ]);

        $this->assertInstanceOf(TDFHeaderRecord::class, $deserialized);
        $this->assertEquals('foo', $deserialized->getCode());
    }


    public function testFooterCsv(): void
    {
        $data = 'c0,c1,c2,c3,c4' . PHP_EOL
            . 'T,foo,20210428,135055,000000';

        $deserialized = $this->serializer->deserialize($data, TDFRecordInterface::class, 'csv', [
            CsvEncoder::AS_COLLECTION_KEY => false // this option means a single row doesn't get wrapped in an additional array
        ]);

        $this->assertInstanceOf(TDFFooterRecord::class, $deserialized);
        $this->assertEquals('foo', $deserialized->getCode());
    }


    public function testBodyCsv(): void
    {
        $data = 'c0,c1,c2,c3,c4,c5,c6,c7,c8,c9,c10,c11,c12' . PHP_EOL
            . 'D,foo,"bar","baz","x","y","z","77","20220913","101811","code","postcode",,,';

        /** @var TDFDetailedRecord */
        $deserialized = $this->serializer->deserialize($data, TDFRecordInterface::class, 'csv', [
            CsvEncoder::AS_COLLECTION_KEY => false // this option means a single row doesn't get wrapped in an additional array
        ]);

        $this->assertInstanceOf(TDFDetailedRecord::class, $deserialized);
        $this->assertEquals('foo', $deserialized->getCourierEventCode());
    }
}

@comma8600
Copy link

comma8600 commented May 22, 2025

Ah ignore the above example I've got a much more minimal test where it is failing for:

I've taken the existing symfony/symfony project 6.4 tag tests and added an identical test to the discriminator for json but swapping in the csv formatter:

    private function serializerWithClassDiscriminatorCsv()
    {
        $classMetadataFactory = new ClassMetadataFactory(new AttributeLoader());

        return new Serializer([new ObjectNormalizer($classMetadataFactory, null, null, new ReflectionExtractor(), new ClassDiscriminatorFromClassMetadata($classMetadataFactory))], ['csv' => new CsvEncoder()]);
    }


    public function testDeserializeAndSerializeInterfacedObjectsWithTheClassMetadataDiscriminatorResolverCsv()
    {
        $example = new DummyMessageNumberOne();
        $example->one = 1;

        $csvData = 'type,one,two' . PHP_EOL .'one,1,' . PHP_EOL;

        $serializer = $this->serializerWithClassDiscriminatorCsv();
        $deserialized = $serializer->deserialize($csvData, DummyMessageInterface::class, 'csv');
        $this->assertEquals($example, $deserialized);

        $serialized = $serializer->serialize($deserialized, 'csv');
        $this->assertEquals($csvData, $serialized);
    }

give the result:


There was 1 error:

1) Symfony\Component\Serializer\Tests\SerializerTest::testDeserializeAndSerializeInterfacedObjectsWithTheClassMetadataDiscriminatorResolverCsv
Symfony\Component\Serializer\Exception\NotNormalizableValueException: Type property "type" not found for the abstract object "Symfony\Component\Serializer\Tests\Fixtures\DummyMessageInterface".

/Users/alexdawn/projects/symfony/src/Symfony/Component/Serializer/Exception/NotNormalizableValueException.php:32
/Users/alexdawn/projects/symfony/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php:855
/Users/alexdawn/projects/symfony/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php:332
/Users/alexdawn/projects/symfony/src/Symfony/Component/Serializer/Serializer.php:249
/Users/alexdawn/projects/symfony/src/Symfony/Component/Serializer/Serializer.php:154
/Users/alexdawn/projects/symfony/src/Symfony/Component/Serializer/Tests/SerializerTest.php:468

serialization works on the $example object and gives an identical string to the $csvData. It's just going the opposite directions which is failing

@ruudk
Copy link
Contributor Author

ruudk commented May 22, 2025

I believe this is a different, but similar issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants