fix(database): handle loading circular eager relations (#1556)

Author: WendellAdriel

packages/database/src/Builder/ModelInspector.php

@@ -365,7 +365,7 @@ public function getSelectFields(): ImmutableArray
         return $selectFields;
     }
 
-    public function resolveRelations(string $relationString, string $parent = ''): array
+    public function resolveRelations(string $relationString, string $parent = '', array $visitedPaths = []): array
     {
         if ($relationString === '') {
             return [];
@@ -384,6 +384,11 @@ public function resolveRelations(string $relationString, string $parent = ''): a
         unset($relationNames[0]);
 
         $relationModel = inspect($currentRelation);
+        $modelType = $relationModel->getName();
+
+        if (in_array($modelType, $visitedPaths, true)) {
+            return [$currentRelationName => $currentRelation->setParent($parent)];
+        }
 
         $newRelationString = implode('.', $relationNames);
         $currentRelation->setParent($parent);
@@ -395,10 +400,13 @@ public function resolveRelations(string $relationString, string $parent = ''): a
 
         $relations = [$currentRelationName => $currentRelation];
 
-        return [...$relations, ...$relationModel->resolveRelations($newRelationString, $newParent)];
+        return [
+            ...$relations,
+            ...$relationModel->resolveRelations($newRelationString, $newParent, [...$visitedPaths, $this->getName()]),
+        ];
     }
 
-    public function resolveEagerRelations(string $parent = ''): array
+    public function resolveEagerRelations(string $parent = '', array $visitedPaths = []): array
     {
         if (! $this->isObjectModel()) {
             return [];
@@ -418,14 +426,23 @@ public function resolveEagerRelations(string $parent = ''): array
                 continue;
             }
 
-            $relations[$property->getName()] = $currentRelation->setParent($parent);
             $newParent = ltrim(sprintf(
                 '%s.%s',
                 $parent,
                 $currentRelationName,
             ), '.');
 
-            foreach (inspect($currentRelation)->resolveEagerRelations($newParent) as $name => $nestedEagerRelation) {
+            $relationModel = inspect($currentRelation);
+            $modelType = $relationModel->getName();
+
+            if (in_array($modelType, $visitedPaths, true)) {
+                continue;
+            }
+
+            $relations[$property->getName()] = $currentRelation->setParent($parent);
+            $newVisitedPaths = [...$visitedPaths, $this->getName()];
+
+            foreach ($relationModel->resolveEagerRelations($newParent, $newVisitedPaths) as $name => $nestedEagerRelation) {
                 $relations[$name] = $nestedEagerRelation;
             }
         }

tests/Fixtures/Migrations/CreateProfileWithEagerTable.php

@@ -0,0 +1,30 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Tests\Tempest\Fixtures\Migrations;
+
+use Tempest\Database\MigratesDown;
+use Tempest\Database\MigratesUp;
+use Tempest\Database\QueryStatement;
+use Tempest\Database\QueryStatements\CreateTableStatement;
+use Tempest\Database\QueryStatements\DropTableStatement;
+use Tests\Tempest\Fixtures\Models\ProfileWithEager;
+
+final class CreateProfileWithEagerTable implements MigratesUp, MigratesDown
+{
+    private(set) string $name = '0000-00-06_create_profiles_table';
+
+    public function up(): QueryStatement
+    {
+        return CreateTableStatement::forModel(ProfileWithEager::class)
+            ->primary()
+            ->text('bio')
+            ->belongsTo('profiles.user_id', 'users.id', nullable: true);
+    }
+
+    public function down(): QueryStatement
+    {
+        return DropTableStatement::forModel(ProfileWithEager::class);
+    }
+}

tests/Fixtures/Migrations/CreateUserWithEagerTable.php

@@ -0,0 +1,29 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Tests\Tempest\Fixtures\Migrations;
+
+use Tempest\Database\MigratesDown;
+use Tempest\Database\MigratesUp;
+use Tempest\Database\QueryStatement;
+use Tempest\Database\QueryStatements\CreateTableStatement;
+use Tempest\Database\QueryStatements\DropTableStatement;
+use Tests\Tempest\Fixtures\Models\UserWithEager;
+
+final class CreateUserWithEagerTable implements MigratesUp, MigratesDown
+{
+    private(set) string $name = '0000-00-05_create_users_table';
+
+    public function up(): QueryStatement
+    {
+        return CreateTableStatement::forModel(UserWithEager::class)
+            ->primary()
+            ->text('name');
+    }
+
+    public function down(): QueryStatement
+    {
+        return DropTableStatement::forModel(UserWithEager::class);
+    }
+}

tests/Fixtures/Models/ProfileWithEager.php

@@ -0,0 +1,26 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Tests\Tempest\Fixtures\Models;
+
+use Tempest\Database\BelongsTo;
+use Tempest\Database\Eager;
+use Tempest\Database\IsDatabaseModel;
+use Tempest\Database\PrimaryKey;
+use Tempest\Database\Table;
+
+#[Table('profiles')]
+final class ProfileWithEager
+{
+    use IsDatabaseModel;
+
+    public function __construct(
+        public PrimaryKey $id,
+        public string $bio,
+        public ?int $user_id,
+        #[Eager]
+        #[BelongsTo]
+        public ?UserWithEager $user = null,
+    ) {}
+}

tests/Fixtures/Models/UserWithEager.php

@@ -0,0 +1,25 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Tests\Tempest\Fixtures\Models;
+
+use Tempest\Database\Eager;
+use Tempest\Database\HasOne;
+use Tempest\Database\IsDatabaseModel;
+use Tempest\Database\PrimaryKey;
+use Tempest\Database\Table;
+
+#[Table('users')]
+final class UserWithEager
+{
+    use IsDatabaseModel;
+
+    public function __construct(
+        public PrimaryKey $id,
+        public string $name,
+        #[Eager]
+        #[HasOne]
+        public ?ProfileWithEager $profile = null,
+    ) {}
+}

tests/Integration/Database/CircularEagerLoadingTest.php

@@ -0,0 +1,66 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Tests\Tempest\Integration\Database;
+
+use Tempest\Database\Migrations\CreateMigrationsTable;
+use Tests\Tempest\Fixtures\Migrations\CreateProfileWithEagerTable;
+use Tests\Tempest\Fixtures\Migrations\CreateUserWithEagerTable;
+use Tests\Tempest\Fixtures\Models\ProfileWithEager;
+use Tests\Tempest\Fixtures\Models\UserWithEager;
+use Tests\Tempest\Integration\FrameworkIntegrationTestCase;
+
+use function Tempest\Database\inspect;
+use function Tempest\Database\query;
+
+/**
+ * @internal
+ */
+final class CircularEagerLoadingTest extends FrameworkIntegrationTestCase
+{
+    public function test_circular_eager_loading_does_not_cause_infinite_loop(): void
+    {
+        $userInspector = inspect(UserWithEager::class);
+        $eagerRelations = $userInspector->resolveEagerRelations();
+
+        $this->assertArrayHasKey('profile', $eagerRelations);
+        $this->assertArrayNotHasKey('profile.user', $eagerRelations);
+        $this->assertCount(1, $eagerRelations);
+    }
+
+    public function test_circular_with_relations_does_not_cause_infinite_loop(): void
+    {
+        $userInspector = inspect(UserWithEager::class);
+        $relations = $userInspector->resolveRelations('profile.user.profile');
+
+        $this->assertArrayHasKey('profile', $relations);
+        $this->assertArrayHasKey('user', $relations);
+        $this->assertCount(2, $relations);
+    }
+
+    public function test_it_saves_and_loads_relations_without_causing_infinite_loop(): void
+    {
+        $this->migrate(
+            CreateMigrationsTable::class,
+            CreateUserWithEagerTable::class,
+            CreateProfileWithEagerTable::class,
+        );
+
+        $user = query(UserWithEager::class)->create(
+            name: 'John Doe',
+        );
+
+        $profile = query(ProfileWithEager::class)->create(
+            bio: 'Test',
+            user_id: $user->id->value,
+        );
+
+        $profile = query(ProfileWithEager::class)->findById($profile->id);
+
+        $user = query(UserWithEager::class)->findById($user->id);
+
+        $this->assertTrue($profile->id->equals($user->profile->id));
+        $this->assertTrue($user->id->equals($profile->user->id));
+    }
+}