Merge pull request #68: deduplicate indexes if FK outer key is PK

cycle · Nov 24, 2023 · 7f2f426 · 7f2f426
2 parents 24284c5 + 490efa4
commit 7f2f426
Show file tree

Hide file tree

Showing 3 changed files with 77 additions and 5 deletions.
diff --git a/src/Generator/ForeignKeys.php b/src/Generator/ForeignKeys.php
@@ -14,9 +14,11 @@ public function run(Registry $registry): Registry
         foreach ($registry as $entity) {
             foreach ($entity->getForeignKeys() as $fk) {
                 $target = $registry->getEntity($fk->getTarget());
+                $targetSchema = $registry->getTableSchema($target);
 
-                if (!$registry->getTableSchema($target)->hasIndex($fk->getOuterColumns())) {
-                    $registry->getTableSchema($target)->index($fk->getOuterColumns())->unique();
+                $pkExists = \array_diff($fk->getOuterColumns(), $targetSchema->getPrimaryKeys()) === [];
+                if (!$pkExists && !$targetSchema->hasIndex($fk->getOuterColumns())) {
+                    $targetSchema->index($fk->getOuterColumns())->unique();
                 }
 
                 $registry->getTableSchema($entity)

diff --git a/tests/Schema/Fixtures/Author.php b/tests/Schema/Fixtures/Author.php
@@ -37,6 +37,18 @@ public static function defineWithoutPK(): Entity
         return $entity;
     }
 
+    public static function defineWithUser(): Entity
+    {
+        $entity = self::define();
+
+        $entity->getFields()->set(
+            'p_user_id',
+            (new Field())->setType('int(11)')->setColumn('user_id')
+        );
+
+        return $entity;
+    }
+
     public static function defineCompositePK(): Entity
     {
         $entity = self::define();

diff --git a/tests/Schema/Generator/ForeignKeysTest.php b/tests/Schema/Generator/ForeignKeysTest.php
@@ -5,6 +5,7 @@
 namespace Cycle\Schema\Tests\Generator;
 
 use Cycle\Schema\Compiler;
+use Cycle\Schema\Definition\Field;
 use Cycle\Schema\Definition\ForeignKey;
 use Cycle\Schema\Generator\ForeignKeys;
 use Cycle\Schema\Generator\RenderTables;
@@ -18,7 +19,7 @@ abstract class ForeignKeysTest extends BaseTest
 {
     public function testTableSchemaShouldBeModified(): void
     {
-        $author = Author::define();
+        $author = Author::defineWithUser();
         $user = User::define();
         $plain = Plain::define();
 
@@ -31,7 +32,7 @@ public function testTableSchemaShouldBeModified(): void
 
         $fk = new ForeignKey();
         $fk->setTarget('user');
-        $fk->setInnerColumns(['id']);
+        $fk->setInnerColumns(['user_id']);
         $fk->setOuterColumns(['id']);
         $fk->setAction('CASCADE');
         $fk->createIndex(true);
@@ -46,10 +47,67 @@ public function testTableSchemaShouldBeModified(): void
 
         $this->assertStringContainsString('authors', $expectedFk->getTable());
         $this->assertStringContainsString('users', $expectedFk->getForeignTable());
-        $this->assertSame(['id'], $expectedFk->getColumns());
+        $this->assertSame(['user_id'], $expectedFk->getColumns());
         $this->assertSame(['id'], $expectedFk->getForeignKeys());
         $this->assertSame('CASCADE', $expectedFk->getDeleteRule());
         $this->assertSame('CASCADE', $expectedFk->getUpdateRule());
         $this->assertTrue($expectedFk->hasIndex());
     }
+
+    public function testCreateIndex(): void
+    {
+        $author = Author::defineWithUser();
+        $user = User::define();
+        $user->getFields()->set('u_other_id', (new Field())->setType('integer')->setColumn('other_id'));
+        $plain = Plain::define();
+
+        $registry = new Registry($this->dbal);
+        $registry->register($author)->linkTable($author, 'default', 'authors');
+        $registry->register($user)->linkTable($user, 'default', 'users');
+        $registry->register($plain)->linkTable($plain, 'default', 'plain');
+
+        $this->assertSame([], $registry->getTableSchema($author)->getForeignKeys());
+
+        $fk = new ForeignKey();
+        $fk->setTarget('user');
+        $fk->setInnerColumns(['user_id']);
+        $fk->setOuterColumns(['other_id']);
+        $fk->setAction('CASCADE');
+        $fk->createIndex(true);
+
+        $author->getForeignKeys()->set($fk);
+
+        $compiler = new Compiler();
+        $compiler->compile($registry, [new RenderTables(), new ForeignKeys()]);
+
+        $this->assertCount(1, $registry->getTableSchema($user)->getIndexes());
+    }
+
+    public function testShouldNotCreateIndexOnPk(): void
+    {
+        $author = Author::defineWithUser();
+        $user = User::define();
+        $plain = Plain::define();
+
+        $registry = new Registry($this->dbal);
+        $registry->register($author)->linkTable($author, 'default', 'authors');
+        $registry->register($user)->linkTable($user, 'default', 'users');
+        $registry->register($plain)->linkTable($plain, 'default', 'plain');
+
+        $this->assertSame([], $registry->getTableSchema($author)->getForeignKeys());
+
+        $fk = new ForeignKey();
+        $fk->setTarget('user');
+        $fk->setInnerColumns(['user_id']);
+        $fk->setOuterColumns(['id']);
+        $fk->setAction('CASCADE');
+        $fk->createIndex(true);
+
+        $author->getForeignKeys()->set($fk);
+
+        $compiler = new Compiler();
+        $compiler->compile($registry, [new RenderTables(), new ForeignKeys()]);
+
+        $this->assertEmpty($registry->getTableSchema($user)->getIndexes());
+    }
 }