Skip to content

Commit 2922b17

Browse files
ndm2ndm2
authored
Fix FK constraint violations on implicit table drops for sqlite (cakephp#2131)
1 parent 852c4b1 commit 2922b17

File tree

2 files changed

+290
-8
lines changed

2 files changed

+290
-8
lines changed

‎src/Phinx/Db/Adapter/SQLiteAdapter.php

+83-7
Original file line numberDiff line numberDiff line change
@@ -449,7 +449,7 @@ protected function getChangePrimaryKeyInstructions(Table $table, $newColumns): A
449449
if (!empty($primaryKey)) {
450450
$instructions->merge(
451451
// FIXME: array access is a hack to make this incomplete implementation work with a correct getPrimaryKey implementation
452-
$this->getDropPrimaryKeyInstructions($table, $primaryKey[0])
452+
$this->getDropPrimaryKeyInstructions($table, $primaryKey[0], false)
453453
);
454454
}
455455

@@ -820,11 +820,16 @@ protected function copyDataToNewTable(string $tableName, string $tmpTableName, a
820820
*
821821
* @param \Phinx\Db\Util\AlterInstructions $instructions The instructions to modify
822822
* @param string $tableName The table name to copy the data to
823+
* @param bool $validateForeignKeys Whether to validate foreign keys after the copy and drop operations. Note that
824+
* enabling this option only has an effect when the `foreign_keys` PRAGMA is set to `ON`!
823825
* @return \Phinx\Db\Util\AlterInstructions
824826
*/
825-
protected function copyAndDropTmpTable(AlterInstructions $instructions, string $tableName): AlterInstructions
826-
{
827-
$instructions->addPostStep(function ($state) use ($tableName) {
827+
protected function copyAndDropTmpTable(
828+
AlterInstructions $instructions,
829+
string $tableName,
830+
bool $validateForeignKeys = true
831+
): AlterInstructions {
832+
$instructions->addPostStep(function ($state) use ($tableName, $validateForeignKeys) {
828833
$this->copyDataToNewTable(
829834
$state['tmpTableName'],
830835
$tableName,
@@ -846,7 +851,15 @@ protected function copyAndDropTmpTable(AlterInstructions $instructions, string $
846851
)
847852
);
848853

854+
$foreignKeysEnabled = (bool)$this->fetchRow('PRAGMA foreign_keys')['foreign_keys'];
855+
if ($foreignKeysEnabled) {
856+
$this->execute('PRAGMA foreign_keys = OFF');
857+
}
849858
$this->execute(sprintf('DROP TABLE %s', $this->quoteTableName($tableName)));
859+
if ($foreignKeysEnabled) {
860+
$this->execute('PRAGMA foreign_keys = ON');
861+
}
862+
850863
$this->execute(sprintf(
851864
'ALTER TABLE %s RENAME TO %s',
852865
$this->quoteTableName($state['tmpTableName']),
@@ -857,12 +870,70 @@ protected function copyAndDropTmpTable(AlterInstructions $instructions, string $
857870
$this->execute($row['sql']);
858871
}
859872

873+
if (
874+
$foreignKeysEnabled &&
875+
$validateForeignKeys
876+
) {
877+
$this->validateForeignKeys($tableName);
878+
}
879+
860880
return $state;
861881
});
862882

863883
return $instructions;
864884
}
865885

886+
/**
887+
* Validates the foreign key constraints of the given table, and of those
888+
* tables whose constraints are targeting it.
889+
*
890+
* @param string $tableName The name of the table for which to check constraints.
891+
* @return void
892+
* @throws \RuntimeException In case of a foreign key constraint violation.
893+
*/
894+
protected function validateForeignKeys(string $tableName): void
895+
{
896+
$tablesToCheck = [
897+
$tableName,
898+
];
899+
900+
$otherTables = $this
901+
->query(
902+
"SELECT name FROM sqlite_master WHERE type = 'table' AND name != ?",
903+
[$tableName]
904+
)
905+
->fetchAll();
906+
907+
foreach ($otherTables as $otherTable) {
908+
$foreignKeyList = $this->getTableInfo($otherTable['name'], 'foreign_key_list');
909+
foreach ($foreignKeyList as $foreignKey) {
910+
if (strcasecmp($foreignKey['table'], $tableName) === 0) {
911+
$tablesToCheck[] = $otherTable['name'];
912+
break;
913+
}
914+
}
915+
}
916+
917+
$tablesToCheck = array_unique(array_map('strtolower', $tablesToCheck));
918+
919+
foreach ($tablesToCheck as $tableToCheck) {
920+
$schema = $this->getSchemaName($tableToCheck, true)['schema'];
921+
922+
$stmt = $this->query(
923+
sprintf('PRAGMA %sforeign_key_check(%s)', $schema, $this->quoteTableName($tableToCheck))
924+
);
925+
$row = $stmt->fetch();
926+
$stmt->closeCursor();
927+
928+
if (is_array($row)) {
929+
throw new RuntimeException(sprintf(
930+
'Integrity constraint violation: FOREIGN KEY constraint on `%s` failed.',
931+
$tableToCheck
932+
));
933+
}
934+
}
935+
}
936+
866937
/**
867938
* Returns the columns and type to use when copying a table to another in the process
868939
* of altering a table
@@ -1312,10 +1383,15 @@ protected function getAddPrimaryKeyInstructions(Table $table, string $column): A
13121383
/**
13131384
* @param \Phinx\Db\Table\Table $table Table
13141385
* @param string $column Column Name
1386+
* @param bool $validateForeignKeys Whether to validate foreign keys after the copy and drop operations. Note that
1387+
* enabling this option only has an effect when the `foreign_keys` PRAGMA is set to `ON`!
13151388
* @return \Phinx\Db\Util\AlterInstructions
13161389
*/
1317-
protected function getDropPrimaryKeyInstructions(Table $table, string $column): AlterInstructions
1318-
{
1390+
protected function getDropPrimaryKeyInstructions(
1391+
Table $table,
1392+
string $column,
1393+
bool $validateForeignKeys = true
1394+
): AlterInstructions {
13191395
$instructions = $this->beginAlterByCopyTable($table->getName());
13201396

13211397
$instructions->addPostStep(function ($state) {
@@ -1335,7 +1411,7 @@ protected function getDropPrimaryKeyInstructions(Table $table, string $column):
13351411
return $newState + $state;
13361412
});
13371413

1338-
return $this->copyAndDropTmpTable($instructions, $table->getName());
1414+
return $this->copyAndDropTmpTable($instructions, $table->getName(), $validateForeignKeys);
13391415
}
13401416

13411417
/**

‎tests/Phinx/Db/Adapter/SQLiteAdapterTest.php

+207-1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
use InvalidArgumentException;
77
use Phinx\Db\Adapter\SQLiteAdapter;
88
use Phinx\Db\Table\Column;
9+
use Phinx\Db\Table\ForeignKey;
910
use Phinx\Util\Expression;
1011
use Phinx\Util\Literal;
1112
use Symfony\Component\Console\Input\ArrayInput;
@@ -1408,6 +1409,208 @@ public function testAlterTableWithConstraints()
14081409
}
14091410
}
14101411

1412+
/**
1413+
* Tests that operations that trigger implicit table drops will not cause
1414+
* a foreign key constraint violation error.
1415+
*/
1416+
public function testAlterTableDoesNotViolateRestrictedForeignKeyConstraint()
1417+
{
1418+
$this->adapter->execute('PRAGMA foreign_keys = ON');
1419+
1420+
$articlesTable = new \Phinx\Db\Table('articles', [], $this->adapter);
1421+
$articlesTable
1422+
->insert(['id' => 1])
1423+
->save();
1424+
1425+
$commentsTable = new \Phinx\Db\Table('comments', [], $this->adapter);
1426+
$commentsTable
1427+
->addColumn('article_id', 'integer')
1428+
->addForeignKey('article_id', 'articles', 'id', [
1429+
'update' => ForeignKey::RESTRICT,
1430+
'delete' => ForeignKey::RESTRICT,
1431+
])
1432+
->insert(['id' => 1, 'article_id' => 1])
1433+
->save();
1434+
1435+
$this->assertTrue($this->adapter->hasForeignKey('comments', ['article_id']));
1436+
1437+
$articlesTable
1438+
->addColumn('new_column', 'integer')
1439+
->update();
1440+
1441+
$articlesTable
1442+
->renameColumn('new_column', 'new_column_renamed')
1443+
->update();
1444+
1445+
$articlesTable
1446+
->changeColumn('new_column_renamed', 'integer', [
1447+
'default' => 1,
1448+
])
1449+
->update();
1450+
1451+
$articlesTable
1452+
->removeColumn('new_column_renamed')
1453+
->update();
1454+
1455+
$articlesTable
1456+
->addIndex('id', ['name' => 'ID_IDX'])
1457+
->update();
1458+
1459+
$articlesTable
1460+
->removeIndex('id')
1461+
->update();
1462+
1463+
$articlesTable
1464+
->addForeignKey('id', 'comments', 'id')
1465+
->update();
1466+
1467+
$articlesTable
1468+
->dropForeignKey('id')
1469+
->update();
1470+
1471+
$articlesTable
1472+
->addColumn('id2', 'integer')
1473+
->addIndex('id', ['unique' => true])
1474+
->changePrimaryKey('id2')
1475+
->update();
1476+
}
1477+
1478+
/**
1479+
* Tests that foreign key constraint violations introduced around the table
1480+
* alteration process (being it implicitly by the process itself or by the user)
1481+
* will trigger an error accordingly.
1482+
*/
1483+
public function testAlterTableDoesViolateForeignKeyConstraintOnTargetTableChange()
1484+
{
1485+
$articlesTable = new \Phinx\Db\Table('articles', [], $this->adapter);
1486+
$articlesTable
1487+
->insert(['id' => 1])
1488+
->save();
1489+
1490+
$commentsTable = new \Phinx\Db\Table('comments', [], $this->adapter);
1491+
$commentsTable
1492+
->addColumn('article_id', 'integer')
1493+
->addForeignKey('article_id', 'articles', 'id', [
1494+
'update' => ForeignKey::RESTRICT,
1495+
'delete' => ForeignKey::RESTRICT,
1496+
])
1497+
->insert(['id' => 1, 'article_id' => 1])
1498+
->save();
1499+
1500+
$this->assertTrue($this->adapter->hasForeignKey('comments', ['article_id']));
1501+
1502+
$this->adapter->execute('PRAGMA foreign_keys = OFF');
1503+
$this->adapter->execute('DELETE FROM articles');
1504+
$this->adapter->execute('PRAGMA foreign_keys = ON');
1505+
1506+
$this->expectException(\RuntimeException::class);
1507+
$this->expectExceptionMessage('Integrity constraint violation: FOREIGN KEY constraint on `comments` failed.');
1508+
1509+
$articlesTable
1510+
->addColumn('new_column', 'integer')
1511+
->update();
1512+
}
1513+
1514+
/**
1515+
* Tests that foreign key constraint violations introduced around the table
1516+
* alteration process (being it implicitly by the process itself or by the user)
1517+
* will trigger an error accordingly.
1518+
*/
1519+
public function testAlterTableDoesViolateForeignKeyConstraintOnSourceTableChange()
1520+
{
1521+
$adapter = $this
1522+
->getMockBuilder(SQLiteAdapter::class)
1523+
->setConstructorArgs([SQLITE_DB_CONFIG, new ArrayInput([]), new NullOutput()])
1524+
->onlyMethods(['validateForeignKeys'])
1525+
->getMock();
1526+
1527+
$articlesTable = new \Phinx\Db\Table('articles', [], $adapter);
1528+
$articlesTable
1529+
->insert(['id' => 1])
1530+
->save();
1531+
1532+
$commentsTable = new \Phinx\Db\Table('comments', [], $adapter);
1533+
$commentsTable
1534+
->addColumn('article_id', 'integer')
1535+
->addForeignKey('article_id', 'articles', 'id', [
1536+
'update' => ForeignKey::RESTRICT,
1537+
'delete' => ForeignKey::RESTRICT,
1538+
])
1539+
->insert(['id' => 1, 'article_id' => 1])
1540+
->save();
1541+
1542+
$this->assertTrue($adapter->hasForeignKey('comments', ['article_id']));
1543+
1544+
$adapterReflection = new \ReflectionObject($adapter);
1545+
$validateForeignKeysReflection = $adapterReflection->getParentClass()->getMethod('validateForeignKeys');
1546+
$validateForeignKeysReflection->setAccessible(true);
1547+
1548+
$adapter
1549+
->expects($this->once())
1550+
->method('validateForeignKeys')
1551+
->willReturnCallback(function (string $tableName) use ($adapter, $validateForeignKeysReflection) {
1552+
$adapter->execute('PRAGMA foreign_keys = OFF');
1553+
$adapter->execute('DELETE FROM articles');
1554+
$adapter->execute('PRAGMA foreign_keys = ON');
1555+
1556+
$validateForeignKeysReflection->invoke($adapter, $tableName);
1557+
});
1558+
1559+
$this->expectException(\RuntimeException::class);
1560+
$this->expectExceptionMessage('Integrity constraint violation: FOREIGN KEY constraint on `comments` failed.');
1561+
1562+
$commentsTable
1563+
->addColumn('new_column', 'integer')
1564+
->update();
1565+
}
1566+
1567+
/**
1568+
* Tests that the adapter's foreign key validation does not apply when
1569+
* the `foreign_keys` pragma is set to `OFF`.
1570+
*/
1571+
public function testAlterTableForeignKeyConstraintValidationNotRunningWithDisabledForeignKeys()
1572+
{
1573+
$articlesTable = new \Phinx\Db\Table('articles', [], $this->adapter);
1574+
$articlesTable
1575+
->insert(['id' => 1])
1576+
->save();
1577+
1578+
$commentsTable = new \Phinx\Db\Table('comments', [], $this->adapter);
1579+
$commentsTable
1580+
->addColumn('article_id', 'integer')
1581+
->addForeignKey('article_id', 'articles', 'id', [
1582+
'update' => ForeignKey::RESTRICT,
1583+
'delete' => ForeignKey::RESTRICT,
1584+
])
1585+
->insert(['id' => 1, 'article_id' => 1])
1586+
->save();
1587+
1588+
$this->assertTrue($this->adapter->hasForeignKey('comments', ['article_id']));
1589+
1590+
$this->adapter->execute('PRAGMA foreign_keys = OFF');
1591+
$this->adapter->execute('DELETE FROM articles');
1592+
1593+
$noException = false;
1594+
try {
1595+
$articlesTable
1596+
->addColumn('new_column1', 'integer')
1597+
->update();
1598+
1599+
$noException = true;
1600+
} finally {
1601+
$this->assertTrue($noException);
1602+
}
1603+
1604+
$this->adapter->execute('PRAGMA foreign_keys = ON');
1605+
1606+
$this->expectException(\RuntimeException::class);
1607+
$this->expectExceptionMessage('Integrity constraint violation: FOREIGN KEY constraint on `comments` failed.');
1608+
1609+
$articlesTable
1610+
->addColumn('new_column2', 'integer')
1611+
->update();
1612+
}
1613+
14111614
public function testLiteralSupport()
14121615
{
14131616
$createQuery = <<<'INPUT'
@@ -2366,7 +2569,10 @@ public function testForeignKeyReferenceCorrectAfterChangePrimaryKey()
23662569
$table->addForeignKey($refTableColumnId, $refTable->getName(), 'id');
23672570
$table->save();
23682571

2369-
$refTable->changePrimaryKey($refTableColumnAdditionalId)->save();
2572+
$refTable
2573+
->addIndex('id', ['unique' => true])
2574+
->changePrimaryKey($refTableColumnAdditionalId)
2575+
->save();
23702576

23712577
$this->assertTrue($this->adapter->hasForeignKey($table->getName(), [$refTableColumnId]));
23722578
$this->assertFalse($this->adapter->hasTable("tmp_{$refTable->getName()}"));

0 commit comments

Comments
 (0)