Skip to content

Use cakephp/database for foreignKey reflection #851

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.

Sign up for GitHub

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

Merged
markstory merged 4 commits into from
Apr 27, 2025
Merged

Use cakephp/database for foreignKey reflection #851

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2ca942d
Need 5.x-dev again
markstory Apr 26, 2025
25b9ea7
Make hasForeignKey tests easier to read
markstory Apr 26, 2025
480d417
Fix deprecation for duplicate short option aliases.
markstory Apr 26, 2025
632fc0a
Update getIndexes to use cakephp/database too
markstory Apr 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
},
"require-dev": {
"cakephp/bake": "^3.0",
"cakephp/cakephp": "^5.2.0",
"cakephp/cakephp": "5.x-dev",
"cakephp/cakephp-codesniffer": "^5.0",
"phpunit/phpunit": "^10.5.5 || ^11.1.3"
},
Expand Down
1 change: 0 additions & 1 deletion src/Command/BakeSimpleMigrationCommand.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,6 @@ public function buildOptionParser(ConsoleOptionParser $parser): ConsoleOptionPar
'boolean' => true,
'help' => 'Do not generate a test skeleton.',
])->addOption('force', [
'short' => 'f',
'boolean' => true,
'help' => 'Force overwriting existing file if a migration already exists with the same name.',
])->addOption('source', [
Expand Down
100 changes: 33 additions & 67 deletions src/Db/Adapter/SqliteAdapter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -558,13 +558,6 @@ protected function resolveIdentity(string $tableName): ?string
if ($result === null) {
return null;
}
// make sure the table does not have a PK-origin autoindex
// such an autoindex would indicate either that the primary key was specified as descending, or that this is a WITHOUT ROWID table
foreach ($this->getIndexes($tableName) as $idx) {
if ($idx['origin'] === 'pk') {
return null;
}
}

return $result;
}
Expand Down Expand Up @@ -759,12 +752,17 @@ protected function bufferIndicesAndTriggers(AlterInstructions $instructions, str
",
$params,
)->fetchAll('assoc');

$indexes = $this->getIndexes($tableName);
$indexMap = [];
foreach ($indexes as $index) {
$indexMap[$index['name']] = $index;
}

foreach ($rows as $row) {
switch ($row['type']) {
case 'index':
$info = $indexes[$row['name']];
$info = $indexMap[$row['name']];
$columns = array_map(
function ($column) {
if ($column === null) {
Expand Down Expand Up @@ -920,7 +918,7 @@ protected function validateForeignKeys(AlterInstructions $instructions, string $
foreach ($otherTables as $otherTable) {
$foreignKeyList = $this->getForeignKeys($otherTable['name']);
foreach ($foreignKeyList as $foreignKey) {
if (strcasecmp($foreignKey['table'], $tableName) === 0) {
if (strcasecmp($foreignKey['references'][0], $tableName) === 0) {
$tablesToCheck[] = $otherTable['name'];
break;
}
Expand Down Expand Up @@ -1244,24 +1242,7 @@ protected function getDropColumnInstructions(string $tableName, string $columnNa
protected function getIndexes(string $tableName): array
{
$dialect = $this->getSchemaDialect();

[$query, $params] = $dialect->describeIndexSql($tableName, []);
$result = $this->query($query, $params)->fetchAll('assoc');

$indexes = [];
foreach ($result as $index) {
// We can't use the dialect here as convertIndexDescription
// has complicated requirements
$indexData = $this->fetchAll(sprintf('pragma index_info(%s)', $this->quoteColumnName($index['name'])));
$cols = [];
foreach ($indexData as $indexItem) {
$cols[] = $indexItem['name'];
}
$indexes[$index['name']] = [
'origin' => $index['origin'],
'columns' => $cols,
];
}
$indexes = $dialect->describeIndexes($tableName);

return $indexes;
}
Expand All @@ -1279,10 +1260,10 @@ protected function resolveIndex(string $tableName, string|array $columns): array
$indexes = $this->getIndexes($tableName);
$matches = [];

foreach ($indexes as $name => $index) {
foreach ($indexes as $index) {
$indexCols = array_map('strtolower', $index['columns']);
if ($columns == $indexCols) {
$matches[] = $name;
$matches[] = $index['name'];
}
}

Expand All @@ -1305,8 +1286,8 @@ public function hasIndexByName(string $tableName, string $indexName): bool
$indexName = strtolower($indexName);
$indexes = $this->getIndexes($tableName);

foreach (array_keys($indexes) as $index) {
if ($indexName === strtolower($index)) {
foreach ($indexes as $index) {
if ($indexName === strtolower($index['name'])) {
return true;
}
}
Expand Down Expand Up @@ -1348,6 +1329,10 @@ protected function getDropIndexByColumnsInstructions(string $tableName, $columns
$indexNames = $this->resolveIndex($tableName, $columns);
$schema = $this->getSchemaName($tableName, true)['schema'];
foreach ($indexNames as $indexName) {
// Skip the implicit primary key that is reflected.
if ($indexName === 'primary') {
continue;
}
if (strpos($indexName, 'sqlite_autoindex_') !== 0) {
$instructions->addPostStep(sprintf(
'DROP INDEX %s%s',
Expand All @@ -1370,20 +1355,20 @@ protected function getDropIndexByNameInstructions(string $tableName, string $ind
$indexes = $this->getIndexes($tableName);

$found = false;
foreach (array_keys($indexes) as $index) {
if ($indexName === strtolower($index)) {
foreach ($indexes as $index) {
if ($indexName === strtolower($index['name'])) {
$found = true;
break;
}
}

if ($found) {
$schema = $this->getSchemaName($tableName, true)['schema'];
$instructions->addPostStep(sprintf(
'DROP INDEX %s%s',
$schema,
$this->quoteColumnName($indexName),
));
$instructions->addPostStep(sprintf(
'DROP INDEX %s%s',
$schema,
$this->quoteColumnName($indexName),
));
}

return $instructions;
Expand Down Expand Up @@ -1434,16 +1419,12 @@ protected function getPrimaryKey(string $tableName): array
*/
public function hasForeignKey(string $tableName, $columns, ?string $constraint = null): bool
{
if ($constraint !== null) {
return preg_match(
"/,?\s*CONSTRAINT\s*" . $this->possiblyQuotedIdentifierRegex($constraint) . '\s*FOREIGN\s+KEY/is',
$this->getDeclaringSql($tableName),
) === 1;
}

$columns = array_map('mb_strtolower', (array)$columns);

foreach ($this->getForeignKeys($tableName) as $key) {
if ($constraint !== null && $key['name'] == $constraint) {
return true;
}
if (array_map('mb_strtolower', $key['columns']) === $columns) {
return true;
}
Expand All @@ -1460,25 +1441,10 @@ public function hasForeignKey(string $tableName, $columns, ?string $constraint =
*/
protected function getForeignKeys(string $tableName): array
{
$foreignKeys = [];

// Can't use the dialect here because describeForeignKeySql()
// doesn't fetch all metadata. If we improve the metadata query
// we can simplify here too.
$query = sprintf('PRAGMA foreign_key_list(%s)', $this->quoteTableName($tableName));
$rows = $this->fetchAll($query);

foreach ($rows as $row) {
if (!isset($foreignKeys[$row['id']])) {
$foreignKeys[$row['id']] = [
'columns' => [],
'table' => $row['table'],
];
}
$foreignKeys[$row['id']]['columns'][$row['seq']] = $row['from'];
}
$dialect = $this->getSchemaDialect();
$keys = $dialect->describeForeignKeys($tableName);

return $foreignKeys;
return $keys;
}

/**
Expand Down Expand Up @@ -1572,14 +1538,14 @@ protected function getAddForeignKeyInstructions(Table $table, ForeignKey $foreig
$schema = $this->getSchemaName($tableName, true)['schema'];
$tmpTableName = $state['tmpTableName'];
$indexes = $this->getIndexes($tableName);
foreach (array_keys($indexes) as $indexName) {
if (strpos($indexName, 'sqlite_autoindex_') !== 0) {
foreach ($indexes as $index) {
if (strpos($index['name'], 'sqlite_autoindex_') !== 0) {
$sql .= sprintf(
'DROP INDEX %s%s; ',
$schema,
$this->quoteColumnName($indexName),
$this->quoteColumnName($index['name']),
);
$createIndexSQL = $this->getDeclaringIndexSQL($tableName, $indexName);
$createIndexSQL = $this->getDeclaringIndexSQL($tableName, $index['name']);
$sql .= preg_replace(
"/\b{$tableName}\b/",
$tmpTableName,
Expand Down
154 changes: 88 additions & 66 deletions tests/TestCase/Db/Adapter/SqliteAdapterTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1465,44 +1465,6 @@ public function testDropForeignKeyWithMultipleColumns()
$this->assertFalse($this->adapter->hasForeignKey($table->getName(), ['ref_table_field1', 'ref_table_id']));
}

public function testDropForeignKeyWithIdenticalMultipleColumns()
{
$refTable = new Table('ref_table', [], $this->adapter);
$refTable
->addColumn('field1', 'string')
->addIndex(['id', 'field1'], ['unique' => true])
->save();

$table = new Table('table', [], $this->adapter);
$keyOne = (new ForeignKey())
->setName('ref_table_fk_1')
->setColumns(['ref_table_id', 'ref_table_field1'])
->setReferencedTable('ref_table')
->setReferencedColumns(['id', 'field1']);
$keyTwo = (new ForeignKey())
->setName('ref_table_fk_2')
->setColumns(['ref_table_id', 'ref_table_field1'])
->setReferencedTable('ref_table')
->setReferencedColumns(['id', 'field1']);

$table
->addColumn('ref_table_id', 'integer', ['signed' => false])
->addColumn('ref_table_field1', 'string')
->addForeignKey($keyOne)
->addForeignKey($keyTwo)
->save();

$this->assertTrue($this->adapter->hasForeignKey($table->getName(), ['ref_table_id', 'ref_table_field1']));
$this->assertTrue($this->adapter->hasForeignKey($table->getName(), [], 'ref_table_fk_1'));
$this->assertTrue($this->adapter->hasForeignKey($table->getName(), [], 'ref_table_fk_2'));

$this->adapter->dropForeignKey($table->getName(), ['ref_table_id', 'ref_table_field1']);

$this->assertFalse($this->adapter->hasForeignKey($table->getName(), ['ref_table_id', 'ref_table_field1']));
$this->assertFalse($this->adapter->hasForeignKey($table->getName(), [], 'ref_table_fk_1'));
$this->assertFalse($this->adapter->hasForeignKey($table->getName(), [], 'ref_table_fk_2'));
}

public static function nonExistentForeignKeyColumnsProvider(): array
{
return [
Expand Down Expand Up @@ -2596,7 +2558,8 @@ public static function provideForeignKeysToCheck()
];
}

public function testHasNamedForeignKey()
#[DataProvider('hasNamedForeignKeyProvider')]
public function testHasNamedForeignKey(string $keySql, ?string $keyName, array $columns, bool $expected): void
{
$refTable = new Table('tbl_parent_1', [], $this->adapter);
$refTable->addColumn('column', 'string')->create();
Expand All @@ -2617,35 +2580,94 @@ public function testHasNamedForeignKey()
`column` VARCHAR NOT NULL, `parent_1_id` INTEGER NOT NULL,
`parent_2_id` INTEGER NOT NULL,
`parent_3_id` INTEGER NOT NULL,
CONSTRAINT `fk_parent_1_id` FOREIGN KEY (`parent_1_id`) REFERENCES `tbl_parent_1` (`id`),
CONSTRAINT [fk_[_brackets] FOREIGN KEY (`parent_1_id`) REFERENCES `tbl_parent_1` (`id`),
CONSTRAINT `fk_``_ticks` FOREIGN KEY (`parent_1_id`) REFERENCES `tbl_parent_1` (`id`),
CONSTRAINT \"fk_\"\"_double_quotes\" FOREIGN KEY (`parent_1_id`) REFERENCES `tbl_parent_1` (`id`),
CONSTRAINT 'fk_''_single_quotes' FOREIGN KEY (`parent_1_id`) REFERENCES `tbl_parent_1` (`id`),
CONSTRAINT fk_no_quotes FOREIGN KEY (`parent_1_id`) REFERENCES `tbl_parent_1` (`id`),
CONSTRAINT`fk_no_space`FOREIGN KEY(`parent_1_id`)REFERENCES`tbl_parent_1`(`id`),
constraint
`fk_lots_of_space` FOReign KEY (`parent_1_id`) REFERENCES `tbl_parent_1` (`id`),
FOREIGN KEY (`parent_2_id`) REFERENCES `tbl_parent_2` (`id`),
CONSTRAINT `check_constraint_1` CHECK (column<>'world'),
CONSTRAINT `fk_composite_key` FOREIGN KEY (`parent_3_id`,`column`) REFERENCES `tbl_parent_3` (`id`,`column`)
CONSTRAINT `check_constraint_2` CHECK (column<>'hello')
{$keySql}
)");

$this->assertTrue($this->adapter->hasForeignKey('tbl_child', [], 'fk_parent_1_id'));
$this->assertTrue($this->adapter->hasForeignKey('tbl_child', [], 'fk_[_brackets'));
$this->assertTrue($this->adapter->hasForeignKey('tbl_child', [], 'fk_`_ticks'));
$this->assertTrue($this->adapter->hasForeignKey('tbl_child', [], 'fk_"_double_quotes'));
$this->assertTrue($this->adapter->hasForeignKey('tbl_child', [], "fk_'_single_quotes"));
$this->assertTrue($this->adapter->hasForeignKey('tbl_child', [], 'fk_no_quotes'));
$this->assertTrue($this->adapter->hasForeignKey('tbl_child', [], 'fk_no_space'));
$this->assertTrue($this->adapter->hasForeignKey('tbl_child', [], 'fk_lots_of_space'));
$this->assertTrue($this->adapter->hasForeignKey('tbl_child', ['parent_1_id']));
$this->assertTrue($this->adapter->hasForeignKey('tbl_child', ['parent_2_id']));
$this->assertTrue($this->adapter->hasForeignKey('tbl_child', [], 'fk_composite_key'));
$this->assertTrue($this->adapter->hasForeignKey('tbl_child', ['parent_3_id', 'column']));
$this->assertFalse($this->adapter->hasForeignKey('tbl_child', [], 'check_constraint_1'));
$this->assertFalse($this->adapter->hasForeignKey('tbl_child', [], 'check_constraint_2'));
$this->assertSame($expected, $this->adapter->hasForeignKey('tbl_child', $columns, $keyName));
}

/**
* @return array
*/
public static function hasNamedForeignKeyProvider(): array
{
return [
// key sql, expected name, columns, expected presence
[
'CONSTRAINT `fk_parent_1_id` FOREIGN KEY (`parent_1_id`) REFERENCES `tbl_parent_1` (`id`)',
'fk_parent_1_id',
[],
true,
],
[
'CONSTRAINT [fk_[_brackets] FOREIGN KEY (`parent_1_id`) REFERENCES `tbl_parent_1` (`id`)',
'fk_[_brackets',
[],
true,
],
[
'CONSTRAINT `fk_``_ticks` FOREIGN KEY (`parent_1_id`) REFERENCES `tbl_parent_1` (`id`)',
'fk_`_ticks',
[],
true,
],
[
'CONSTRAINT "fk_""_double_quotes" FOREIGN KEY (`parent_1_id`) REFERENCES `tbl_parent_1` (`id`)',
'fk_"_double_quotes',
[],
true,
],
[
"CONSTRAINT 'fk_''_single_quotes' FOREIGN KEY (`parent_1_id`) REFERENCES `tbl_parent_1` (`id`)",
"fk_'_single_quotes",
[],
true,
],
[
'CONSTRAINT fk_no_quotes FOREIGN KEY (`parent_1_id`) REFERENCES `tbl_parent_1` (`id`)',
'fk_no_quotes',
[],
true,
],
[
'CONSTRAINT`fk_no_space`FOREIGN KEY(`parent_1_id`)REFERENCES`tbl_parent_1`(`id`)',
'fk_no_space',
[],
true,
],
[
'constraint
`fk_lots_of_space` FOReign KEY (`parent_1_id`) REFERENCES `tbl_parent_1` (`id`)',
'fk_lots_of_space',
[],
true,
],
[
'FOREIGN KEY (`parent_2_id`) REFERENCES `tbl_parent_2` (`id`)',
null,
['parent_2_id'],
true,
],
[
'CONSTRAINT `fk_parent_1_id` FOREIGN KEY (`parent_1_id`) REFERENCES `tbl_parent_1` (`id`)',
null,
['parent_1_id'],
true,
],
[
'CONSTRAINT `fk_composite_key` FOREIGN KEY (`parent_3_id`,`column`) REFERENCES `tbl_parent_3` (`id`,`column`)',
null,
['parent_3_id', 'column'],
true,
],
// Should not find check constraints
[
"CONSTRAINT `check_constraint_1` CHECK (column<>'world')",
'check_constraint_1',
[],
false,
],
];
}

#[DataProvider('providePhinxTypes')]
Expand Down
Loading