Replace FK overwrite deprecation with exception

Removed the deprecation warning triggered when overwriting an existing
foreign key constraint, and replaced it with a ForeignKeyAlreadyExists
exception to enforce correct behavior.

Author: santysisi

UPGRADE.md

@@ -8,6 +8,11 @@ awareness about deprecated code.
 
 # Upgrade to 5.0
 
+## BC BREAK: changes in `Table` class
+
+The `Table` class no longer accepts `foreignKeyConstraints`
+with duplicate names.
+
 ## BC BREAK: changes in `TableDiff` constructor
 
 The `TableDiff` class constructor no longer accepts `droppedForeignKeys`

src/Platforms/SQLitePlatform.php

@@ -27,6 +27,7 @@
 use Doctrine\DBAL\Types;
 
 use function array_merge;
+use function assert;
 use function count;
 use function explode;
 use function implode;
@@ -861,23 +862,8 @@ private function getForeignKeysInAlteredTable(TableDiff $diff): array
     {
         $oldTable    = $diff->getOldTable();
         $foreignKeys = $oldTable->getForeignKeys();
+        $nameMap     = $this->getDiffColumnNameMap($diff);
         $keysByName  = [];
-        foreach ($foreignKeys as $key => $foreignKey) {
-            $constraintName = $foreignKey->getObjectName();
-
-            if ($constraintName === null) {
-                continue;
-            }
-
-            $constraintKey = strtolower(
-                $constraintName->getIdentifier()
-                    ->getValue(),
-            );
-
-            $keysByName[$constraintKey] = $key;
-        }
-
-        $nameMap = $this->getDiffColumnNameMap($diff);
 
         foreach ($foreignKeys as $key => $constraint) {
             $changed = false;
@@ -900,6 +886,17 @@ private function getForeignKeysInAlteredTable(TableDiff $diff): array
                 $changed = true;
             }
 
+            $constraintName = $constraint->getObjectName();
+
+            if ($constraintName !== null) {
+                $constraintKey = strtolower(
+                    $constraintName->getIdentifier()
+                        ->getValue(),
+                );
+
+                $keysByName[$constraintKey] = $key;
+            }
+
             if (! $changed) {
                 continue;
             }
@@ -921,7 +918,8 @@ private function getForeignKeysInAlteredTable(TableDiff $diff): array
                     ->getValue(),
             );
 
-            unset($foreignKeys[$keysByName[$constraintKey]]);
+            assert(isset($keysByName[$constraintKey]));
+            unset($foreignKeys[$keysByName[$constraintKey]], $keysByName[$constraintKey]);
         }
 
         foreach ($diff->getAddedForeignKeys() as $constraint) {
@@ -933,11 +931,10 @@ private function getForeignKeysInAlteredTable(TableDiff $diff): array
                         ->getValue(),
                 );
 
-                if (isset($keysByName[$constraintKey])) {
-                    $foreignKeys[$keysByName[$constraintKey]] = $constraint;
-                } else {
-                    $foreignKeys[] = $constraint;
-                }
+                assert(! isset($keysByName[$constraintKey]));
+                $foreignKeys[] = $constraint;
+
+                $keysByName[$constraintKey] = count($foreignKeys) - 1;
             } else {
                 $foreignKeys[] = $constraint;
             }

src/Schema/Exception/ForeignKeyAlreadyExists.php

@@ -0,0 +1,26 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\DBAL\Schema\Exception;
+
+use Doctrine\DBAL\Schema\Name\OptionallyQualifiedName;
+use Doctrine\DBAL\Schema\Name\UnqualifiedName;
+use Doctrine\DBAL\Schema\SchemaException;
+use LogicException;
+
+use function sprintf;
+
+final class ForeignKeyAlreadyExists extends LogicException implements SchemaException
+{
+    public static function new(OptionallyQualifiedName $tableName, UnqualifiedName $foreignKeyName): self
+    {
+        return new self(
+            sprintf(
+                'A foreign key constraint named %s already exists on table %s.',
+                $foreignKeyName->toString(),
+                $tableName->toString(),
+            ),
+        );
+    }
+}

src/Schema/Table.php

@@ -6,6 +6,7 @@
 
 use Doctrine\DBAL\Schema\Exception\ColumnAlreadyExists;
 use Doctrine\DBAL\Schema\Exception\ColumnDoesNotExist;
+use Doctrine\DBAL\Schema\Exception\ForeignKeyAlreadyExists;
 use Doctrine\DBAL\Schema\Exception\ForeignKeyDoesNotExist;
 use Doctrine\DBAL\Schema\Exception\IndexAlreadyExists;
 use Doctrine\DBAL\Schema\Exception\IndexDoesNotExist;
@@ -839,12 +840,7 @@ private function _addForeignKeyConstraint(ForeignKeyConstraint $constraint): sel
         }
 
         if (isset($this->foreignKeyConstraints[$key])) {
-            Deprecation::trigger(
-                'doctrine/dbal',
-                'https://github.com/doctrine/dbal/pull/7125',
-                'Overwriting an existing foreign key constraint ("%s") is deprecated.',
-                $key,
-            );
+            throw ForeignKeyAlreadyExists::new($this->name, UnqualifiedName::unquoted($key));
         }
 
         $this->foreignKeyConstraints[$key] = $constraint;

tests/Schema/TableTest.php

@@ -8,6 +8,7 @@
 use Doctrine\DBAL\Platforms\MySQLPlatform;
 use Doctrine\DBAL\Platforms\SQLitePlatform;
 use Doctrine\DBAL\Schema\Column;
+use Doctrine\DBAL\Schema\Exception\ForeignKeyAlreadyExists;
 use Doctrine\DBAL\Schema\Exception\IndexDoesNotExist;
 use Doctrine\DBAL\Schema\Exception\InvalidForeignKeyConstraintDefinition;
 use Doctrine\DBAL\Schema\Exception\InvalidIndexDefinition;
@@ -1797,7 +1798,7 @@ public function testOverwritingForeignKeyConstraint(): void
 
         $table->addForeignKeyConstraint('bar', ['id'], ['id']);
 
-        $this->expectDeprecationWithIdentifier('https://github.com/doctrine/dbal/pull/7125');
+        $this->expectException(ForeignKeyAlreadyExists::class);
         $table->addForeignKeyConstraint('baz', ['id'], ['id']);
     }
 }