Fix CwaFixtureBuilder: service registration, explicit persists, idempotent flush

silverbackdan · silverbackdan · commit aa6f6ee0ef17 · 2026-06-17T17:59:37.000+01:00
- Register CwaFixtureBuilder as a bundle service (was requiring manual wiring)
- Add persistWithAssociations() to walk owning-side associations and persist
  every related entity explicitly — no Doctrine cascade relied upon
- Add public persist() method for app code to register app-specific entities
  (e.g. HtmlContent set on a BlogArticleData) before flush
- Call persistTimestampedFields() on ComponentGroup and ComponentPosition
  (were missing, causing stale timestamps)
- Phases 1–3 now run exactly once; phase 4 runs on every flush() call so
  positions added after the first flush (e.g. nav links after routes exist)
  are picked up correctly without re-running route generation
- GroupBuilder.getNewComponents()/getNewPageDataPositions() advance a cursor
  so only unprocessed items are created on each phase-4 run
- phaseThree() skips entities that already have a route (safe for re-run)
- Add 3 new tests: public persist(), two-flush positions, single route-gen call
diff --git a/src/Fixture/Builder/GroupBuilder.php b/src/Fixture/Builder/GroupBuilder.php
@@ -18,6 +18,8 @@ class GroupBuilder
     private array $components = [];
     private array $pageDataPositions = [];
     private int $nextSort = 10;
+    private int $processedComponentCount = 0;
+    private int $processedPositionCount = 0;
 
     public function __construct(
         private readonly string $name,
@@ -60,4 +62,22 @@ public function getPageDataPositions(): array
     {
         return $this->pageDataPositions;
     }
+
+    /** Returns only components added since the last call — safe to call on every flush(). */
+    public function getNewComponents(): array
+    {
+        $new = \array_slice($this->components, $this->processedComponentCount);
+        $this->processedComponentCount = \count($this->components);
+
+        return $new;
+    }
+
+    /** Returns only pageData positions added since the last call — safe to call on every flush(). */
+    public function getNewPageDataPositions(): array
+    {
+        $new = \array_slice($this->pageDataPositions, $this->processedPositionCount);
+        $this->processedPositionCount = \count($this->pageDataPositions);
+
+        return $new;
+    }
 }
diff --git a/src/Fixture/CwaFixtureBuilder.php b/src/Fixture/CwaFixtureBuilder.php
@@ -63,6 +63,12 @@ class CwaFixtureBuilder
     /** Maps spl_object_id(GroupBuilder) → ComponentGroup for use in phase 4 */
     private array $componentGroupMap = [];
 
+    /** Tracks object IDs already passed to persist() to avoid cycles in persistWithAssociations() */
+    private array $persistedEntities = [];
+
+    /** Phases 1–3 run exactly once; phase 4 runs on every flush() call to pick up new positions */
+    private bool $initialFlushDone = false;
+
     public function __construct(
         private readonly TimestampedDataPersister $timestampedPersister,
         private readonly RouteGeneratorInterface $routeGenerator,
@@ -157,9 +163,14 @@ public function pageData(
         return $builder;
     }
 
+    /**
+     * Explicitly persist an entity and walk its owning-side associations to persist related objects.
+     * Use this for app-specific entities that the builder doesn't manage (e.g. HtmlContent set on a PageData).
+     * Does not rely on Doctrine cascade — every related object is persisted explicitly.
+     */
     public function persist(object $entity): static
     {
-        $this->manager->persist($entity);
+        $this->persistWithAssociations($entity);
 
         return $this;
     }
@@ -173,12 +184,20 @@ public function getRoute(string $routeName): Route
         return $this->namedRoutes[$routeName];
     }
 
+    /**
+     * Phases 1–3 run exactly once (on first call).
+     * Phase 4 runs every call to pick up positions added after the first flush (e.g. nav links added after routes exist).
+     */
     public function flush(): void
     {
-        $this->phaseOne();
-        $this->evaluateNested();
-        $this->phaseTwo();
-        $this->phaseThree();
+        if (!$this->initialFlushDone) {
+            $this->phaseOne();
+            $this->evaluateNested();
+            $this->phaseTwo();
+            $this->phaseThree();
+            $this->initialFlushDone = true;
+        }
+
         $this->phaseFour();
     }
 
@@ -211,8 +230,9 @@ private function evaluateNested(): void
 
         $newPageDataSpecs = \array_slice($this->pageDataSpecs, $existingPageDataCount);
         foreach ($newPageDataSpecs as $spec) {
-            $this->timestampedPersister->persistTimestampedFields($spec['builder']->getPageData(), true);
-            $this->manager->persist($spec['builder']->getPageData());
+            $pageData = $spec['builder']->getPageData();
+            $this->timestampedPersister->persistTimestampedFields($pageData, true);
+            $this->persistWithAssociations($pageData);
             $hasNew = true;
         }
 
@@ -224,7 +244,7 @@ private function evaluateNested(): void
                 $page->layout = $layoutBuilder->getLayout();
             }
             $this->timestampedPersister->persistTimestampedFields($page, true);
-            $this->manager->persist($page);
+            $this->persistWithAssociations($page);
             $hasNew = true;
         }
 
@@ -238,7 +258,7 @@ private function phaseOne(): void
         foreach ($this->layoutBuilders as $layoutBuilder) {
             $layout = $layoutBuilder->getLayout();
             $this->timestampedPersister->persistTimestampedFields($layout, true);
-            $this->manager->persist($layout);
+            $this->persistWithAssociations($layout);
         }
 
         foreach ($this->pageSpecs as $spec) {
@@ -248,7 +268,7 @@ private function phaseOne(): void
                 $page->layout = $layoutBuilder->getLayout();
             }
             $this->timestampedPersister->persistTimestampedFields($page, true);
-            $this->manager->persist($page);
+            $this->persistWithAssociations($page);
         }
 
         foreach ($this->pageDataSpecs as $spec) {
@@ -257,7 +277,7 @@ private function phaseOne(): void
                 $pageData->page = $this->pageSpecs[$spec['templateRef']]['builder']->getPage();
             }
             $this->timestampedPersister->persistTimestampedFields($pageData, true);
-            $this->manager->persist($pageData);
+            $this->persistWithAssociations($pageData);
         }
 
         $this->manager->flush();
@@ -313,6 +333,9 @@ private function phaseThree(): void
         foreach ($this->orderedRouteSpecs as $spec) {
             if ('page' === $spec['type']) {
                 $page = $spec['builder']->getPage();
+                if (null !== $page->getRoute()) {
+                    continue;
+                }
                 if (null !== $spec['route']) {
                     $route = $this->createExplicitRoute($spec['route'], $spec['routeName']);
                     $route->setPage($page);
@@ -330,6 +353,9 @@ private function phaseThree(): void
                 }
             } else {
                 $pageData = $spec['builder']->getPageData();
+                if (null !== $pageData->getRoute()) {
+                    continue;
+                }
                 if (null !== $spec['route']) {
                     $route = $this->createExplicitRoute($spec['route'], $spec['routeName']);
                     $route->setPageData($pageData);
@@ -385,19 +411,19 @@ private function createPositions(GroupBuilder $groupBuilder): bool
 
         $hasAny = false;
 
-        foreach ($groupBuilder->getComponents() as $item) {
+        foreach ($groupBuilder->getNewComponents() as $item) {
             $component = $item['component'];
             $position = new ComponentPosition();
             $position->sortValue = $item['sort'];
             $position->component = $component;
             $componentGroup->addComponentPosition($position);
             $this->timestampedPersister->persistTimestampedFields($position, true);
-            $this->manager->persist($component);
+            $this->persistWithAssociations($component);
             $this->manager->persist($position);
             $hasAny = true;
         }
 
-        foreach ($groupBuilder->getPageDataPositions() as $item) {
+        foreach ($groupBuilder->getNewPageDataPositions() as $item) {
             $position = new ComponentPosition();
             $position->sortValue = $item['sort'];
             $position->pageDataProperty = $item['property'];
@@ -425,4 +451,57 @@ private function deriveRouteName(string $path): string
 
         return $slug ?: 'root';
     }
+
+    /**
+     * Persists an entity and recursively persists all owning-side associated objects.
+     * Does not rely on Doctrine cascade — every object is persisted explicitly.
+     * Uses spl_object_id tracking to prevent cycles.
+     */
+    private function persistWithAssociations(object $entity): void
+    {
+        $oid = spl_object_id($entity);
+        if (isset($this->persistedEntities[$oid])) {
+            return;
+        }
+        $this->persistedEntities[$oid] = true;
+        $this->manager->persist($entity);
+
+        try {
+            $metadata = $this->manager->getClassMetadata($entity::class);
+            foreach ($metadata->getAssociationNames() as $assocName) {
+                if ($metadata->isAssociationInverseSide($assocName)) {
+                    continue;
+                }
+                $related = $this->readProperty($entity, $assocName);
+                if (null === $related) {
+                    continue;
+                }
+                if (is_iterable($related)) {
+                    foreach ($related as $item) {
+                        if (is_object($item)) {
+                            $this->persistWithAssociations($item);
+                        }
+                    }
+                } else {
+                    $this->persistWithAssociations($related);
+                }
+            }
+        } catch (\Exception) {
+            // Entity class not in Doctrine metadata (e.g. during unit tests with stubs)
+        }
+    }
+
+    private function readProperty(object $entity, string $property): mixed
+    {
+        $class = new \ReflectionClass($entity);
+        do {
+            if ($class->hasProperty($property)) {
+                $prop = $class->getProperty($property);
+
+                return $prop->isInitialized($entity) ? $prop->getValue($entity) : null;
+            }
+        } while ($class = $class->getParentClass());
+
+        return null;
+    }
 }
diff --git a/tests/Fixture/CwaFixtureBuilderTest.php b/tests/Fixture/CwaFixtureBuilderTest.php
@@ -15,6 +15,7 @@
 use Doctrine\Persistence\ObjectManager;
 use PHPUnit\Framework\TestCase;
 use Silverback\ApiComponentsBundle\Entity\Core\AbstractComponent;
+use Silverback\ApiComponentsBundle\Entity\Core\AbstractPage;
 use Silverback\ApiComponentsBundle\Entity\Core\AbstractPageData;
 use Silverback\ApiComponentsBundle\Entity\Core\ComponentGroup;
 use Silverback\ApiComponentsBundle\Entity\Core\ComponentPosition;
@@ -400,6 +401,71 @@ public function test_parent_route_is_created_before_child_route(): void
         $this->assertSame(spl_object_id($parentPageData), $createOrder[0]);
     }
 
+    // --- Association graph auto-persist ---
+
+    public function test_public_persist_persists_entity_via_manager(): void
+    {
+        $persisted = [];
+        $em = $this->collectingEm($persisted);
+
+        $component = new class extends AbstractComponent {};
+
+        $builder = $this->makeBuilder($em, $this->autoRouteGenerator());
+        $builder->persist($component);
+        $builder->flush();
+
+        $this->assertContains($component, $persisted);
+    }
+
+    public function test_flush_second_call_only_processes_new_positions(): void
+    {
+        $persisted = [];
+        $em = $this->collectingEm($persisted);
+
+        $component1 = new class extends AbstractComponent {};
+        $component2 = new class extends AbstractComponent {};
+
+        $routeGenerator = $this->autoRouteGenerator();
+        $builder = $this->makeBuilder($em, $routeGenerator);
+        $builder->layout('main', 'CwaLayoutPrimary');
+        $navGroup = $builder->layout('main', 'CwaLayoutPrimary')->group('nav');
+        $builder->page('home', 'Template', layout: 'main', route: '/');
+
+        $navGroup->add($component1);
+        $builder->flush(); // first flush — component1 processed
+
+        $navGroup->add($component2);
+        $builder->flush(); // second flush — only component2 should be processed
+
+        $positions = array_values(array_filter($persisted, static fn ($e) => $e instanceof ComponentPosition));
+        // component1 → position1 in flush 1; component2 → position2 in flush 2
+        $this->assertCount(2, $positions);
+        $components = array_map(static fn ($p) => $p->component, $positions);
+        $this->assertContains($component1, $components);
+        $this->assertContains($component2, $components);
+    }
+
+    public function test_phases_one_to_three_run_only_once_across_multiple_flushes(): void
+    {
+        $routeGenerator = $this->createMock(RouteGeneratorInterface::class);
+        $routeGenerator->expects($this->once()) // must be called exactly once despite two flush() calls
+            ->method('create')
+            ->willReturnCallback(static function (object $entity): Route {
+                $route = new Route();
+                $route->setPath('/' . spl_object_id($entity));
+                $route->setName((string) spl_object_id($entity));
+                $entity->setRoute($route);
+                return $route;
+            });
+
+        $builder = $this->makeBuilder(routeGenerator: $routeGenerator);
+        $builder->layout('main', 'CwaLayoutPrimary');
+        $builder->page('home', 'Template', layout: 'main'); // will call routeGenerator->create() once
+
+        $builder->flush();
+        $builder->flush(); // second flush must NOT call routeGenerator->create() again
+    }
+
     public function test_parent_pagedata_route_created_before_child_pagedata_route(): void
     {
         $createOrder = [];
