Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Manually merge 2.10.x into 2.11.x #4273

Merged
merged 11 commits into from
Sep 12, 2020
Merged

Manually merge 2.10.x into 2.11.x #4273

merged 11 commits into from
Sep 12, 2020

Conversation

greg0ire
Copy link
Member

@greg0ire greg0ire commented Sep 12, 2020

Review carefully, I'm not sure I didn't mess up

MyIgel and others added 10 commits September 2, 2020 23:03
If there is no constraint name defined the listTableForeignKeys should not invent a new one as a numerical id has no deeper meaning and makes schema changes adding information that was not there before.
Using `null` is an apropriate value as its already in use when handling not existing fk names.
SQLite: Fix wrongly detected reference constraint name on schema change
This is part of an effort to standardize workflows accross repositories.
We do not have a way to do it automatically yet, but it is still nice to
have one reference repository.
Handle both ways of PDO reporting failed connection
Revert full support for foreign key constraints for SQLite
@morozov
Copy link
Member

morozov commented Sep 12, 2020

This looks a bit different from my result (morozov@d26a383):

  1. The changes in AbstractSQLiteDriver.php are missing but they should have been introduced by Revert full support for foreign key constraints for SQLite #4255.
  2. testAcceptsForeignKeySupportsCreateDropForeignKeyConstraints() was introduced in Update PHPUnit to 9.3 #4201 and shouldn't be removed.

You can go over each mismatch individually or just use my version.

diff --git a/lib/Doctrine/DBAL/Driver/AbstractSQLiteDriver.php b/lib/Doctrine/DBAL/Driver/AbstractSQLiteDriver.php
index a8bbf60ce..26f8583f1 100644
--- a/lib/Doctrine/DBAL/Driver/AbstractSQLiteDriver.php
+++ b/lib/Doctrine/DBAL/Driver/AbstractSQLiteDriver.php
@@ -84,10 +84,6 @@ abstract class AbstractSQLiteDriver implements Driver, ExceptionConverterDriver
             return new ConnectionException($message, $exception);
         }
 
-        if (strpos($exception->getMessage(), 'FOREIGN KEY constraint failed') !== false) {
-            return new ForeignKeyConstraintViolationException($message, $exception);
-        }
-
         return new DriverException($message, $exception);
     }
 
diff --git a/tests/Doctrine/Tests/DBAL/Platforms/AbstractPlatformTestCase.php b/tests/Doctrine/Tests/DBAL/Platforms/AbstractPlatformTestCase.php
index 65d4c12de..ca17b35db 100644
--- a/tests/Doctrine/Tests/DBAL/Platforms/AbstractPlatformTestCase.php
+++ b/tests/Doctrine/Tests/DBAL/Platforms/AbstractPlatformTestCase.php
@@ -3,7 +3,6 @@
 namespace Doctrine\Tests\DBAL\Platforms;
 
 use Doctrine\Common\EventManager;
-use Doctrine\DBAL\DBALException;
 use Doctrine\DBAL\Events;
 use Doctrine\DBAL\Exception;
 use Doctrine\DBAL\Platforms\AbstractPlatform;
@@ -265,7 +264,7 @@ abstract class AbstractPlatformTestCase extends DbalTestCase
         if ($this->platform->supportsForeignKeyConstraints()) {
             self::assertIsString($this->platform->getCreateForeignKeySQL($fk, 'test'));
         } else {
-            $this->expectException(DBALException::class);
+            $this->expectException(Exception::class);
             $this->platform->getCreateForeignKeySQL($fk, 'test');
         }
     }
diff --git a/tests/Doctrine/Tests/DBAL/Schema/Visitor/CreateSchemaSqlCollectorTest.php b/tests/Doctrine/Tests/DBAL/Schema/Visitor/CreateSchemaSqlCollectorTest.php
index c40b4a53f..c4e1aca9e 100644
--- a/tests/Doctrine/Tests/DBAL/Schema/Visitor/CreateSchemaSqlCollectorTest.php
+++ b/tests/Doctrine/Tests/DBAL/Schema/Visitor/CreateSchemaSqlCollectorTest.php
@@ -74,7 +74,7 @@ class CreateSchemaSqlCollectorTest extends TestCase
         self::assertSame(['foo'], $this->visitor->getQueries());
     }
 
-    public function testAcceptsForeignKeyDoesNotSupportCreateDropForeignKeyConstraints(): void
+    public function testAcceptsForeignKeyDoesNotSupportForeignKeyConstraints(): void
     {
         $this->platformMock->method('supportsForeignKeyConstraints')
             ->willReturn(false);
@@ -87,6 +87,19 @@ class CreateSchemaSqlCollectorTest extends TestCase
         self::assertEmpty($this->visitor->getQueries());
     }
 
+    public function testAcceptsForeignKeySupportsForeignKeyConstraints(): void
+    {
+        $this->platformMock->method('supportsForeignKeyConstraints')
+            ->willReturn(true);
+
+        $table      = $this->createTableMock();
+        $foreignKey = $this->createForeignKeyConstraintMock();
+
+        $this->visitor->acceptForeignKey($table, $foreignKey);
+
+        self::assertSame(['foo'], $this->visitor->getQueries());
+    }
+
     public function testAcceptsSequences(): void
     {
         $sequence = $this->createSequenceMock();

@greg0ire
Copy link
Member Author

greg0ire commented Sep 12, 2020

I run vendor/bin/phpcbf on top of morozov/dbal@d26a383, it should be OK, thanks for the review!

@greg0ire greg0ire merged commit 76c4c33 into doctrine:2.11.x Sep 12, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants