Skip to content
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

[Live Component] DoctrineObjectNormalizer is being used to normalize my non-Doctrine object #452

Closed
benr77 opened this issue Sep 8, 2022 · 10 comments · Fixed by #453
Closed
Labels
Bug Bug Fix Status: Reviewed Has been reviewed by a maintainer

Comments

@benr77
Copy link

benr77 commented Sep 8, 2022

Using v2.3 of the Live Component.

I'm using a DTO as a data class with a form in a live component. It's working OK with scalar values in the DTO.

However, when I try and use Money/Money object as one of the form data properties, it borks with:

ErrorException:
Warning: Undefined array key 0

  at vendor/doctrine/orm/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php:883
  at Doctrine\ORM\Mapping\ClassMetadataInfo->getIdentifierValues(object(Money))
     (vendor/symfony/ux-live-component/src/Normalizer/DoctrineObjectNormalizer.php:44)
  at Symfony\UX\LiveComponent\Normalizer\DoctrineObjectNormalizer->normalize(object(Money), 'json', array('live-component' => true, 'cache_key' => '87beb01efbb89bc8a83bf4e8e91ba0dc', 'circular_reference_limit_counters' => array('0000000000000d700000000000000000' => 1)))
     (vendor/symfony/serializer/Serializer.php:161)
  at Symfony\Component\Serializer\Serializer->normalize(object(Money), 'json', array('live-component' => true, 'cache_key' => '87beb01efbb89bc8a83bf4e8e91ba0dc', 'circular_reference_limit_counters' => array('0000000000000d700000000000000000' => 1)))
     (vendor/symfony/serializer/Normalizer/AbstractObjectNormalizer.php:216)
  at Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer->normalize(object(DefaultPriceData), 'json', array('live-component' => true, 'cache_key' => '87beb01efbb89bc8a83bf4e8e91ba0dc', 'circular_reference_limit_counters' => array('0000000000000d700000000000000000' => 1)))
     (vendor/symfony/serializer/Serializer.php:161)
  at Symfony\Component\Serializer\Serializer->normalize(object(DefaultPriceData), 'json', array('live-component' => true))
     (vendor/symfony/ux-live-component/src/LiveComponentHydrator.php:87)file=vendor/symfony/serializer/Serializer.php&line=161#line161)-> normalize (line 161)

Looking at the DoctrineObjectNormalizer I see it is given a priority of 100, making it kick in way before my custom Money normalizer which is already registered with the serializer.

Why is the DoctrineObjectNormalizer given such a high priority, and why is it trying to normalize my non-Doctrine object?

What would be the best way to get around this?

Thank you.

@kbond
Copy link
Member

kbond commented Sep 8, 2022

Hey @benr77, strange, the doctrine one should be skipped as the object is not managed. I'll look into this.

I'm also questioning the priority. Currently, I don't think a custom doctrine entity normalizer can be used.

@kbond
Copy link
Member

kbond commented Sep 8, 2022

#453 changes the priority from 100 to -100. I added a test that I think covers your scenario above (dto object with a normalizer) but couldn't recreate the error you received.

@benr77
Copy link
Author

benr77 commented Sep 9, 2022

My serializer is currently registered with a priority of -700, which IIRC is to ensure it is checked before ObjectNormalizer (priority -1000) but can still be overridden easily by client implementations (it's from a reusable bundle).

But DoctrineObjectNormalizer should not agree to normalize a non-Doctrine object surely?

My form data class just looks like this:

use Money\Money;

final class DefaultPriceData
{
    public function __construct(
        public string $itemType,
        public Money $price
    ) {
    }
}

Thanks

@benr77
Copy link
Author

benr77 commented Sep 9, 2022

On further investigation, it turns out that DoctrineObjectNormalizer does consider Money\Money to be managed by Doctrine.

https://github.com/symfony/ux-live-component/blob/17a2586f0fe86c4efbcabd39b3a90bcd453a39b2/src/Normalizer/DoctrineObjectNormalizer.php#L85

This line returns EntityManager for Money\Money.

It would seem that this is because I have a Doctrine embeddable defined to allow persisting Money objects to the database.

<embeddable name="Money\Money">
    <field name="amount" type="decimal" precision="0" scale="0" />
    <field name="currency" type="currency"/>
</embeddable>

So I'm guessing that the actual issue is simply that the DoctrineObjectNormalizer needs to check it's a Doctrine entity rather than just any old object that Doctrine is aware of.

@kbond
Copy link
Member

kbond commented Sep 9, 2022

Ahhh, now it makes sense! I'll work on a fix.

@kbond
Copy link
Member

kbond commented Sep 9, 2022

Hmm, still can't reproduce the issue. In #453, I've added a test that demonstrates using an embeddable as an entity property but also a dto property. I've also confirmed that:

private function objectManagerFor(string $class): ?ObjectManager

Returns null for the embeddable (as we would expect). What version of doctrine/orm are you using? I do think #453 does fix your issue (but only because of the priority change). Can you confirm?

@benr77
Copy link
Author

benr77 commented Sep 9, 2022

I'm using doctrine/orm 2.13.1

Changing the priority for DoctrineObjectNormalizer to -100 does not fix the issue (this is all I've changed - not used the rest of #453)

If I remove the code that registers the embeddable from my bundle, the Live Component starts working, but I run into an "Invalid Checksum" error instead. Not sure if this is related or not.

Finally, the code that adds the embeddable does so using DoctrineOrmMappingsPass in the bundle file:

public function build(ContainerBuilder $container): void
    {
        parent::build($container);

        $modelDir = realpath(__DIR__ . '/Doctrine/Mapping');
        $mappings = [
            $modelDir => 'Money',
        ];

        if (class_exists(DoctrineOrmMappingsPass::class)) {
            $container->addCompilerPass(
                DoctrineOrmMappingsPass::createXmlMappingDriver(
                    $mappings,
                    ['doctrine.orm.entity_manager'],
                    false
                )
            );
        }
    }

Not sure if any of this is helpful sorry.

@kbond
Copy link
Member

kbond commented Sep 9, 2022

Finally solved! Was a tricky one: when embeddables are mapped using annotations, ManagerRegistry::getManagerForClass() returns null but when mapped using xml, it returns the EntityManager! I've added this scenario to the test in #453.

@benr77
Copy link
Author

benr77 commented Sep 9, 2022

You sir, are a legend.

Isn't this a Doctrine bug then?

@kbond
Copy link
Member

kbond commented Sep 9, 2022

Found a bit of context. I actually solved this exact thing in zenstruck/foundry earlier this year!

Refs:

In short, it looks like it is a bug but fixing it causes other problems so they're leaving alone for now. Sounds like it will all be refactored/fixed in ORM 3.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Bug Fix Status: Reviewed Has been reviewed by a maintainer
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants