Skip to content

[LiveComponent] Add support for interfaces while (de)hydrating a Doctrine entity #2834

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
wants to merge 11 commits into
base: 2.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

use Doctrine\ORM\EntityManagerInterface;
use Doctrine\Persistence\ManagerRegistry;
use Doctrine\Persistence\Mapping\MappingException;
use Doctrine\Persistence\ObjectManager;

/**
Expand Down Expand Up @@ -61,8 +62,7 @@ public function dehydrate(object $object): mixed
$id = $this
->objectManagerFor($class = $object::class)
->getClassMetadata($class)
->getIdentifierValues($object)
;
->getIdentifierValues($object);

// Dehydrate ID values in case they are other entities
$id = array_map(fn ($id) => \is_object($id) && $this->supports($id::class) ? $this->dehydrate($id) : $id, $id);
Expand All @@ -81,14 +81,44 @@ public function dehydrate(object $object): mixed

private function objectManagerFor(string $class): ?ObjectManager
{
if (!class_exists($class)) {
if (class_exists($class)) {
// Keep the standard way for a class
// todo cache/warmup an array of classes that are "doctrine objects"
Comment on lines +85 to +86
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Keep the standard way for a class
// todo cache/warmup an array of classes that are "doctrine objects"

foreach ($this->managerRegistries as $registry) {
if ($om = $registry->getManagerForClass($class)) {
return self::ensureManagedObject($om, $class);
}
}

return null;
}

// todo cache/warmup an array of classes that are "doctrine objects"
foreach ($this->managerRegistries as $registry) {
if ($om = $registry->getManagerForClass($class)) {
return self::ensureManagedObject($om, $class);
if (interface_exists($class)) {
// special handler for interfaces
// For use cases : @see https://symfony.com/doc/current/doctrine/resolve_target_entity.html
// As today, getManagerForClass don't resolve nicely aliased interfaces
// The workaround is to enum over each object manager and trying to get metadata
// The metadata are indeed, resolved nicely
// Fore more details :
Comment on lines +97 to +102
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we turn this into two or three lines, insisting more on what we "do" than on what "was" the problem ?

// @see \Doctrine\ORM\Tools\ResolveTargetEntityListener

// todo cache/warmup an array of interfaces that are resolved "doctrine objects"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// todo cache/warmup an array of interfaces that are resolved "doctrine objects"

foreach ($this->managerRegistries as $registry) {
foreach ($registry->getManagers() as $om) {
// the getClassMetaData can indeed throw an exception
// so, we
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

try {
if (null !== $om->getClassMetadata($class)) {
return self::ensureManagedObject($om, $class);
}
} catch (MappingException $e) {
// I did not find a nice way to check if it is because the class is really unknown
// It is good to check for a specific exception ?
// eg: \Doctrine\Persistence\Mapping\MappingException
// @see \Doctrine\Persistence\Mapping\AbstractClassMetadataFactory::getMetadataFor
// throw $e;
Comment on lines +114 to +119
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We 100% can catch a specific exception if that allows another object manager to be selected during following iterations of the loop(s).

}
}
}
}

Expand Down
9 changes: 9 additions & 0 deletions src/LiveComponent/tests/Fixtures/Dto/Aliased.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<?php

namespace Symfony\UX\LiveComponent\Tests\Fixtures\Dto;

class Aliased
{
// public string $address;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// public string $address;

public string $name;
}
27 changes: 27 additions & 0 deletions src/LiveComponent/tests/Fixtures/Entity/AliasedEntity.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <[email protected]>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\UX\LiveComponent\Tests\Fixtures\Entity;

use Doctrine\ORM\Mapping as ORM;
use Doctrine\ORM\Mapping\Column;

#[ORM\Entity]
class AliasedEntity implements AliasedEntityInterface
{
#[ORM\Id]
#[ORM\GeneratedValue]
#[ORM\Column(type: 'integer')]
public $id;

#[Column(type: 'string')]
public ?string $name = null;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?php

namespace Symfony\UX\LiveComponent\Tests\Fixtures\Entity;

interface AliasedEntityInterface {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

}
4 changes: 4 additions & 0 deletions src/LiveComponent/tests/Fixtures/Kernel.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
use Symfony\Component\Security\Core\User\InMemoryUser;
use Symfony\UX\LiveComponent\LiveComponentBundle;
use Symfony\UX\LiveComponent\Tests\Fixtures\Component\Component1;
use Symfony\UX\LiveComponent\Tests\Fixtures\Entity\AliasedEntity;
use Symfony\UX\LiveComponent\Tests\Fixtures\Entity\AliasedEntityInterface;
use Symfony\UX\LiveComponent\Tests\Fixtures\Serializer\Entity2Normalizer;
use Symfony\UX\LiveComponent\Tests\Fixtures\Serializer\MoneyNormalizer;
use Symfony\UX\StimulusBundle\StimulusBundle;
Expand Down Expand Up @@ -170,6 +172,8 @@ protected function configureContainer(ContainerConfigurator $c): void
'alias' => 'XML',
],
],
'resolve_target_entities' => [
AliasedEntityInterface::class => AliasedEntity::class],
Comment on lines +175 to +176
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
'resolve_target_entities' => [
AliasedEntityInterface::class => AliasedEntity::class],
'resolve_target_entities' => [
AliasedEntityInterface::class => AliasedEntity::class,
],

],
];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,11 @@

namespace Symfony\UX\LiveComponent\Tests\Integration\Hydration;

use Doctrine\ORM\EntityManagerInterface;
use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
use Symfony\UX\LiveComponent\Hydration\DoctrineEntityHydrationExtension;
use Symfony\UX\LiveComponent\Tests\Fixtures\Entity\AliasedEntity;
use Symfony\UX\LiveComponent\Tests\Fixtures\Entity\AliasedEntityInterface;
use Symfony\UX\LiveComponent\Tests\Fixtures\Entity\CompositeIdEntity;
use Symfony\UX\LiveComponent\Tests\Fixtures\Entity\ForeignKeyIdEntity;
use Symfony\UX\LiveComponent\Tests\Fixtures\Factory\CompositeIdEntityFactory;
Expand Down Expand Up @@ -50,4 +53,34 @@ public function testForeignKeyId(): void
self::assertSame($foreignKeyIdEntity->id->id, $dehydrated);
self::assertSame($foreignKeyIdEntity, $extension->hydrate($dehydrated, ForeignKeyIdEntity::class));
}

public function testSupportInterface(): void
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public function testSupportInterface(): void
public function testSupportInterface()

{
/** @var DoctrineEntityHydrationExtension $extension */
$extension = self::getContainer()->get('ux.live_component.doctrine_entity_hydration_extension');

self::assertTrue($extension->supports(AliasedEntityInterface::class), 'AliasedEntityInterface should be supported');
self::assertTrue($extension->supports(AliasedEntity::class), 'AliasedEntity should be supported');
self::assertFalse($extension->supports('UnknownClass'), 'UnknownClass should not be supported');
}

public function testHydrationFromInterface(): void
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public function testHydrationFromInterface(): void
public function testHydrationFromInterface()

{
/** @var DoctrineEntityHydrationExtension $extension */
$extension = self::getContainer()->get('ux.live_component.doctrine_entity_hydration_extension');
$em = self::getContainer()->get(EntityManagerInterface::class);

$existingEntity = new AliasedEntity();
$existingEntity->name = 'foo';

$em->persist($existingEntity);
$em->flush();

$dehydratedData = $extension->dehydrate($existingEntity);

$entityFromDehydratation = $extension->hydrate($dehydratedData, AliasedEntityInterface::class);

self::assertSame($existingEntity, $entityFromDehydratation, 'instance should be the same');
self::assertNull($extension->hydrate(null, AliasedEntityInterface::class), 'should return null if null is passed');
}
}