bug #3847 [Turbo] Fix broadcasting an entity whose identifier is made of associations (Kocal)

This PR was merged into the 3.x branch.

Discussion
----------

[Turbo] Fix broadcasting an entity whose identifier is made of associations

| Q              | A
| -------------- | ---
| Bug fix?       | yes
| New feature?   | no
| Deprecations?  | no
| Documentation? | no
| Issues         | Fix #1856
| License        | MIT

Entities whose primary key is made up of associations (the usual join-entity shape) couldn't be broadcast at all, as reported in #1856. `ClassMetadata::getIdentifierValues()` returns the related entities themselves instead of their identifiers, and every consumer then runs `implode('-', $id)` on them, which throws `Object of class App\Entity\ClassA could not be converted to string`.

`IdAccessor::getIdentifierValues()` now reads the identifier and replaces any field mapped as an association with the identifier of the entity it points to. It checks the mapping of the field, not the class of the value, so it still works when the association is a lazy proxy, and an identifier held in a value object such as a UUID is left alone since it's a plain column.

Both producers now go through this accessor: `IdAccessor::getEntityId()`, and `BroadcastListener`, which used to read the identifier values itself and never reached the accessor at all: that's the exact path in the reported stack trace. The listener still gets its `EntityManager` from the flush event it's already handed, so nothing new is injected into it. This is covered by tests on the create path and the update path; the latter reloads the entity first, which under Doctrine ORM 2 turns the identifier associations into real proxies.

Commits
-------

3f2a40e [Turbo] Fix broadcasting an entity whose identifier is made of associations

Author: Kocal

src/Turbo/CHANGELOG.md

@@ -1,5 +1,9 @@
 # CHANGELOG
 
+## 3.5.0
+
+- Fix broadcasting an entity whose identifier is made of associations
+
 ## 3.2.0
 
 - Prevent installation alongside `symfony/mercure` 0.7.0 and 0.7.1, which are incompatible

src/Turbo/src/Broadcaster/IdAccessor.php

@@ -12,6 +12,7 @@
 namespace Symfony\UX\Turbo\Broadcaster;
 
 use Doctrine\Persistence\ManagerRegistry;
+use Doctrine\Persistence\ObjectManager;
 use Symfony\Component\PropertyAccess\PropertyAccess;
 use Symfony\Component\PropertyAccess\PropertyAccessorInterface;
 
@@ -27,14 +28,14 @@ public function __construct(
     }
 
     /**
-     * @return string[]|null
+     * @return array<array-key, mixed>|null
      */
     public function getEntityId(object $entity): ?array
     {
         $entityClass = $entity::class;
 
         if ($this->doctrine && $em = $this->doctrine->getManagerForClass($entityClass)) {
-            return $em->getClassMetadata($entityClass)->getIdentifierValues($entity);
+            return self::getIdentifierValues($em, $entity);
         }
 
         if ($this->propertyAccessor) {
@@ -43,4 +44,27 @@ public function getEntityId(object $entity): ?array
 
         return null;
     }
+
+    /**
+     * Same as ClassMetadata::getIdentifierValues(), except that an identifier which is itself
+     * an association is replaced by the identifier of the entity it points to: Doctrine hands
+     * back that related entity, which callers cannot turn into an identifier string.
+     *
+     * @internal
+     *
+     * @return array<string, mixed>
+     */
+    public static function getIdentifierValues(ObjectManager $em, object $entity): array
+    {
+        $metadata = $em->getClassMetadata($entity::class);
+        $id = $metadata->getIdentifierValues($entity);
+
+        foreach ($id as $field => $value) {
+            if ($metadata->hasAssociation($field)) {
+                $id[$field] = implode('-', self::getIdentifierValues($em, $value));
+            }
+        }
+
+        return $id;
+    }
 }

src/Turbo/src/Doctrine/BroadcastListener.php

@@ -19,6 +19,7 @@
 use Symfony\Contracts\Service\ResetInterface;
 use Symfony\UX\Turbo\Attribute\Broadcast;
 use Symfony\UX\Turbo\Broadcaster\BroadcasterInterface;
+use Symfony\UX\Turbo\Broadcaster\IdAccessor;
 
 /**
  * Detects changes made from Doctrine entities and broadcasts updates to the broadcasters.
@@ -92,7 +93,7 @@ public function postFlush(EventArgs $eventArgs): void
         try {
             foreach ($this->createdEntities as $entity) {
                 $options = $this->createdEntities[$entity];
-                $id = $em->getClassMetadata($entity::class)->getIdentifierValues($entity);
+                $id = IdAccessor::getIdentifierValues($em, $entity);
                 foreach ($options as $option) {
                     $option['id'] = $id;
                     $this->broadcaster->broadcast($entity, Broadcast::ACTION_CREATE, $option);
@@ -148,7 +149,7 @@ private function storeEntitiesToPublish(EntityManagerInterface $em, object $enti
 
         if ($options = $this->broadcastedClasses[$class]) {
             if ($this->createdEntities !== $objectStorage) {
-                $id = $em->getClassMetadata($class)->getIdentifierValues($entity);
+                $id = IdAccessor::getIdentifierValues($em, $entity);
                 foreach ($options as $k => $option) {
                     $options[$k]['id'] = $id;
                 }

src/Turbo/tests/Broadcaster/IdAccessorTest.php

@@ -0,0 +1,43 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\UX\Turbo\Tests\Broadcaster;
+
+use Doctrine\Persistence\ManagerRegistry;
+use PHPUnit\Framework\TestCase;
+use Symfony\UX\Turbo\Broadcaster\IdAccessor;
+use Symfony\UX\Turbo\Tests\Fixtures\Entity\Membership;
+use Symfony\UX\Turbo\Tests\Fixtures\Entity\Player;
+use Symfony\UX\Turbo\Tests\Fixtures\Entity\Team;
+use Symfony\UX\Turbo\Tests\Fixtures\EntityManagerFactory;
+
+class IdAccessorTest extends TestCase
+{
+    public function testGetEntityIdReturnsTheIdentifierValue(): void
+    {
+        $this->assertSame(['id' => 42], $this->createIdAccessor()->getEntityId(new Player(42)));
+    }
+
+    public function testGetEntityIdResolvesAnAssociationToItsOwnIdentifier(): void
+    {
+        $membership = new Membership(new Player(1), new Team(2));
+
+        $this->assertSame(['player' => '1', 'team' => '2'], $this->createIdAccessor()->getEntityId($membership));
+    }
+
+    private function createIdAccessor(): IdAccessor
+    {
+        $doctrine = $this->createStub(ManagerRegistry::class);
+        $doctrine->method('getManagerForClass')->willReturn(EntityManagerFactory::create());
+
+        return new IdAccessor(doctrine: $doctrine);
+    }
+}

src/Turbo/tests/Doctrine/BroadcastListenerTest.php

@@ -0,0 +1,72 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\UX\Turbo\Tests\Doctrine;
+
+use Doctrine\ORM\EntityManager;
+use Doctrine\ORM\Events;
+use PHPUnit\Framework\TestCase;
+use Symfony\UX\Turbo\Doctrine\BroadcastListener;
+use Symfony\UX\Turbo\Tests\Fixtures\CollectingBroadcaster;
+use Symfony\UX\Turbo\Tests\Fixtures\Entity\Membership;
+use Symfony\UX\Turbo\Tests\Fixtures\Entity\Player;
+use Symfony\UX\Turbo\Tests\Fixtures\Entity\Team;
+use Symfony\UX\Turbo\Tests\Fixtures\EntityManagerFactory;
+
+class BroadcastListenerTest extends TestCase
+{
+    public function testBroadcastACreatedEntityWhoseIdentifierIsMadeOfAssociations(): void
+    {
+        $entityManager = EntityManagerFactory::create();
+        $broadcaster = $this->listenTo($entityManager);
+
+        $entityManager->persist($player = new Player(1));
+        $entityManager->persist($team = new Team(2));
+        $entityManager->persist(new Membership($player, $team));
+        $entityManager->flush();
+
+        $this->assertSame([['create', ['id' => ['player' => '1', 'team' => '2']]]], $broadcaster->broadcasts);
+    }
+
+    public function testBroadcastAnUpdatedEntityWhoseIdentifierIsMadeOfAssociations(): void
+    {
+        $entityManager = EntityManagerFactory::create();
+
+        $entityManager->persist($player = new Player(1));
+        $entityManager->persist($team = new Team(2));
+        $entityManager->persist(new Membership($player, $team));
+        $entityManager->flush();
+        $entityManager->clear();
+
+        // Reloading makes the identifier associations lazy, a state the create path never reaches.
+        $membership = $entityManager->find(Membership::class, ['player' => 1, 'team' => 2]);
+        $this->assertNotNull($membership);
+
+        $broadcaster = $this->listenTo($entityManager);
+
+        $membership->role = 'captain';
+        $entityManager->flush();
+
+        $this->assertSame([['update', ['id' => ['player' => '1', 'team' => '2']]]], $broadcaster->broadcasts);
+    }
+
+    private function listenTo(EntityManager $entityManager): CollectingBroadcaster
+    {
+        $broadcaster = new CollectingBroadcaster();
+
+        $entityManager->getEventManager()->addEventListener(
+            [Events::onFlush, Events::postFlush],
+            new BroadcastListener($broadcaster),
+        );
+
+        return $broadcaster;
+    }
+}

src/Turbo/tests/Fixtures/CollectingBroadcaster.php

@@ -0,0 +1,28 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\UX\Turbo\Tests\Fixtures;
+
+use Symfony\UX\Turbo\Broadcaster\BroadcasterInterface;
+
+/**
+ * @internal
+ */
+final class CollectingBroadcaster implements BroadcasterInterface
+{
+    /** @var list<array{string, array<string, mixed>}> */
+    public array $broadcasts = [];
+
+    public function broadcast(object $entity, string $action, array $options): void
+    {
+        $this->broadcasts[] = [$action, $options];
+    }
+}

src/Turbo/tests/Fixtures/Entity/Membership.php

@@ -0,0 +1,38 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\UX\Turbo\Tests\Fixtures\Entity;
+
+use Doctrine\ORM\Mapping as ORM;
+use Symfony\UX\Turbo\Attribute\Broadcast;
+
+/**
+ * An entity whose primary key is composed of two associations, so Doctrine
+ * stores the related entities themselves in the identifier properties.
+ *
+ * @internal
+ */
+#[ORM\Entity]
+#[Broadcast]
+class Membership
+{
+    public function __construct(
+        #[ORM\Id]
+        #[ORM\ManyToOne(targetEntity: Player::class)]
+        public Player $player,
+        #[ORM\Id]
+        #[ORM\ManyToOne(targetEntity: Team::class)]
+        public Team $team,
+        #[ORM\Column]
+        public string $role = 'member',
+    ) {
+    }
+}

src/Turbo/tests/Fixtures/Entity/Player.php

@@ -0,0 +1,28 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\UX\Turbo\Tests\Fixtures\Entity;
+
+use Doctrine\ORM\Mapping as ORM;
+
+/**
+ * @internal
+ */
+#[ORM\Entity]
+class Player
+{
+    public function __construct(
+        #[ORM\Id]
+        #[ORM\Column]
+        public int $id,
+    ) {
+    }
+}

src/Turbo/tests/Fixtures/Entity/Team.php

@@ -0,0 +1,28 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\UX\Turbo\Tests\Fixtures\Entity;
+
+use Doctrine\ORM\Mapping as ORM;
+
+/**
+ * @internal
+ */
+#[ORM\Entity]
+class Team
+{
+    public function __construct(
+        #[ORM\Id]
+        #[ORM\Column]
+        public int $id,
+    ) {
+    }
+}

src/Turbo/tests/Fixtures/EntityManagerFactory.php

@@ -0,0 +1,47 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\UX\Turbo\Tests\Fixtures;
+
+use Doctrine\DBAL\DriverManager;
+use Doctrine\ORM\EntityManager;
+use Doctrine\ORM\ORMSetup;
+use Doctrine\ORM\Tools\SchemaTool;
+
+/**
+ * Creates an in-memory sqlite EntityManager holding every entity under Fixtures/Entity.
+ *
+ * @internal
+ */
+final class EntityManagerFactory
+{
+    public static function create(): EntityManager
+    {
+        $config = ORMSetup::createAttributeMetadataConfiguration(
+            paths: [__DIR__.'/Entity'],
+            isDevMode: true,
+        );
+
+        // @phpstan-ignore function.alreadyNarrowedType (ORM 2 has no native lazy objects, and needs none)
+        if (method_exists($config, 'enableNativeLazyObjects')) {
+            $config->enableNativeLazyObjects(true);
+        }
+
+        $entityManager = new EntityManager(
+            DriverManager::getConnection(['driver' => 'pdo_sqlite', 'memory' => true], $config),
+            $config,
+        );
+
+        new SchemaTool($entityManager)->createSchema($entityManager->getMetadataFactory()->getAllMetadata());
+
+        return $entityManager;
+    }
+}