From 655189c35d1980930fa234a2ef8f9c3f23f7e017 Mon Sep 17 00:00:00 2001 From: Claudio Zizza Date: Sun, 22 Dec 2024 22:50:04 +0100 Subject: [PATCH 1/6] Create website schema validation workflow --- .github/workflows/website-schema.yml | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 .github/workflows/website-schema.yml diff --git a/.github/workflows/website-schema.yml b/.github/workflows/website-schema.yml new file mode 100644 index 0000000000..e251588e5d --- /dev/null +++ b/.github/workflows/website-schema.yml @@ -0,0 +1,21 @@ + +name: "Website config validation" + +on: + pull_request: + branches: + - "*.x" + paths: + - ".doctrine-project.json" + - ".github/workflows/website-schema.yml" + push: + branches: + - "*.x" + paths: + - ".doctrine-project.json" + - ".github/workflows/website-schema.yml" + +jobs: + json-validate: + name: "Validate JSON schema" + uses: "doctrine/.github/.github/workflows/website-schema.yml@7.1.0" From cb6b4ebcdc3d935f5cb218199fce519055a9797f Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Tue, 24 Dec 2024 15:46:46 -0800 Subject: [PATCH 2/6] Do not index objects scoped to a table by name First, these keys are not used in the code. Second, they cause clashing between multiple unnamed unique constraints. --- src/Platforms/AbstractPlatform.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Platforms/AbstractPlatform.php b/src/Platforms/AbstractPlatform.php index 2c49c7d185..53679b8751 100644 --- a/src/Platforms/AbstractPlatform.php +++ b/src/Platforms/AbstractPlatform.php @@ -829,7 +829,7 @@ private function buildCreateTableSQL(Table $table, bool $createForeignKeys): arr foreach ($table->getIndexes() as $index) { if (! $index->isPrimary()) { - $options['indexes'][$index->getQuotedName($this)] = $index; + $options['indexes'][] = $index; continue; } @@ -839,7 +839,7 @@ private function buildCreateTableSQL(Table $table, bool $createForeignKeys): arr } foreach ($table->getUniqueConstraints() as $uniqueConstraint) { - $options['uniqueConstraints'][$uniqueConstraint->getQuotedName($this)] = $uniqueConstraint; + $options['uniqueConstraints'][] = $uniqueConstraint; } if ($createForeignKeys) { From baad09a1a37b86d7a0c1339f4809ab800e59d38e Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Wed, 25 Dec 2024 09:25:11 -0800 Subject: [PATCH 3/6] Use proper unnamed unique constraint DDL syntax --- src/Platforms/AbstractPlatform.php | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/Platforms/AbstractPlatform.php b/src/Platforms/AbstractPlatform.php index 53679b8751..ea32cd4918 100644 --- a/src/Platforms/AbstractPlatform.php +++ b/src/Platforms/AbstractPlatform.php @@ -1170,8 +1170,13 @@ public function getCreateSchemaSQL(string $schemaName): string */ public function getCreateUniqueConstraintSQL(UniqueConstraint $constraint, string $tableName): string { - return 'ALTER TABLE ' . $tableName . ' ADD CONSTRAINT ' . $constraint->getQuotedName($this) . ' UNIQUE' - . ' (' . implode(', ', $constraint->getQuotedColumns($this)) . ')'; + $sql = 'ALTER TABLE ' . $tableName . ' ADD'; + + if ($constraint->getName() !== '') { + $sql .= ' CONSTRAINT ' . $constraint->getQuotedName($this); + } + + return $sql . ' UNIQUE (' . implode(', ', $constraint->getQuotedColumns($this)) . ')'; } /** @@ -1546,9 +1551,10 @@ public function getUniqueConstraintDeclarationSQL(UniqueConstraint $constraint): throw new InvalidArgumentException('Incomplete definition. "columns" required.'); } - $chunks = ['CONSTRAINT']; + $chunks = []; if ($constraint->getName() !== '') { + $chunks[] = 'CONSTRAINT'; $chunks[] = $constraint->getQuotedName($this); } From 343f8a7422cbb01cfaf3c23b46fe0327bfa94660 Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Wed, 25 Dec 2024 09:21:06 -0800 Subject: [PATCH 4/6] Add unnamed constraint tests --- .../Schema/ForeignKeyConstraintTest.php | 49 +++++++++++++++++++ .../Schema/UniqueConstraintTest.php | 36 ++++++++++++++ 2 files changed, 85 insertions(+) create mode 100644 tests/Functional/Schema/ForeignKeyConstraintTest.php create mode 100644 tests/Functional/Schema/UniqueConstraintTest.php diff --git a/tests/Functional/Schema/ForeignKeyConstraintTest.php b/tests/Functional/Schema/ForeignKeyConstraintTest.php new file mode 100644 index 0000000000..49e57c299f --- /dev/null +++ b/tests/Functional/Schema/ForeignKeyConstraintTest.php @@ -0,0 +1,49 @@ +dropTableIfExists('users'); + $this->dropTableIfExists('roles'); + $this->dropTableIfExists('teams'); + + $roles = new Table('roles'); + $roles->addColumn('id', Types::INTEGER); + $roles->setPrimaryKey(['id']); + + $teams = new Table('teams'); + $teams->addColumn('id', Types::INTEGER); + $teams->setPrimaryKey(['id']); + + $users = new Table('users', [ + new Column('id', Type::getType(Types::INTEGER)), + new Column('role_id', Type::getType(Types::INTEGER)), + new Column('team_id', Type::getType(Types::INTEGER)), + ], [], [], [ + new ForeignKeyConstraint(['role_id'], 'roles', ['id']), + new ForeignKeyConstraint(['team_id'], 'teams', ['id']), + ]); + $users->setPrimaryKey(['id']); + + $sm = $this->connection->createSchemaManager(); + $sm->createTable($roles); + $sm->createTable($teams); + $sm->createTable($users); + + $table = $sm->introspectTable('users'); + + self::assertCount(2, $table->getForeignKeys()); + } +} diff --git a/tests/Functional/Schema/UniqueConstraintTest.php b/tests/Functional/Schema/UniqueConstraintTest.php new file mode 100644 index 0000000000..3c06427eb6 --- /dev/null +++ b/tests/Functional/Schema/UniqueConstraintTest.php @@ -0,0 +1,36 @@ +dropTableIfExists('users'); + + $users = new Table('users', [ + new Column('id', Type::getType(Types::INTEGER)), + new Column('username', Type::getType(Types::STRING), ['length' => 32]), + new Column('email', Type::getType(Types::STRING), ['length' => 255]), + ], [], [ + new UniqueConstraint('', ['username']), + new UniqueConstraint('', ['email']), + ], []); + + $sm = $this->connection->createSchemaManager(); + $sm->createTable($users); + + // we want to assert that the two empty names don't clash, but introspection of unique constraints is currently + // not supported. for now, we just assert that the table can be created without exceptions. + $this->expectNotToPerformAssertions(); + } +} From 64dcfd9bb1692d4fc13def32bbd51c1cbae06cf0 Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Sun, 22 Dec 2024 19:55:57 -0800 Subject: [PATCH 5/6] Remove redundant isset checks --- src/Platforms/AbstractMySQLPlatform.php | 6 +++--- src/Platforms/AbstractPlatform.php | 6 +++--- src/Platforms/PostgreSQLPlatform.php | 4 ++-- src/Platforms/SQLServerPlatform.php | 6 +++--- src/Platforms/SQLitePlatform.php | 6 +++--- 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/Platforms/AbstractMySQLPlatform.php b/src/Platforms/AbstractMySQLPlatform.php index eb2aa6299b..7c9faecb48 100644 --- a/src/Platforms/AbstractMySQLPlatform.php +++ b/src/Platforms/AbstractMySQLPlatform.php @@ -237,21 +237,21 @@ protected function _getCreateTableSQL(string $name, array $columns, array $optio { $queryFields = $this->getColumnDeclarationListSQL($columns); - if (isset($options['uniqueConstraints']) && ! empty($options['uniqueConstraints'])) { + if (! empty($options['uniqueConstraints'])) { foreach ($options['uniqueConstraints'] as $definition) { $queryFields .= ', ' . $this->getUniqueConstraintDeclarationSQL($definition); } } // add all indexes - if (isset($options['indexes']) && ! empty($options['indexes'])) { + if (! empty($options['indexes'])) { foreach ($options['indexes'] as $definition) { $queryFields .= ', ' . $this->getIndexDeclarationSQL($definition); } } // attach all primary keys - if (isset($options['primary']) && ! empty($options['primary'])) { + if (! empty($options['primary'])) { $keyColumns = array_unique(array_values($options['primary'])); $queryFields .= ', PRIMARY KEY(' . implode(', ', $keyColumns) . ')'; } diff --git a/src/Platforms/AbstractPlatform.php b/src/Platforms/AbstractPlatform.php index abd8947496..7144b56b85 100644 --- a/src/Platforms/AbstractPlatform.php +++ b/src/Platforms/AbstractPlatform.php @@ -987,17 +987,17 @@ protected function _getCreateTableSQL(string $name, array $columns, array $optio { $columnListSql = $this->getColumnDeclarationListSQL($columns); - if (isset($options['uniqueConstraints']) && ! empty($options['uniqueConstraints'])) { + if (! empty($options['uniqueConstraints'])) { foreach ($options['uniqueConstraints'] as $definition) { $columnListSql .= ', ' . $this->getUniqueConstraintDeclarationSQL($definition); } } - if (isset($options['primary']) && ! empty($options['primary'])) { + if (! empty($options['primary'])) { $columnListSql .= ', PRIMARY KEY(' . implode(', ', array_unique(array_values($options['primary']))) . ')'; } - if (isset($options['indexes']) && ! empty($options['indexes'])) { + if (! empty($options['indexes'])) { foreach ($options['indexes'] as $definition) { $columnListSql .= ', ' . $this->getIndexDeclarationSQL($definition); } diff --git a/src/Platforms/PostgreSQLPlatform.php b/src/Platforms/PostgreSQLPlatform.php index ad182f8f43..b5858acf2d 100644 --- a/src/Platforms/PostgreSQLPlatform.php +++ b/src/Platforms/PostgreSQLPlatform.php @@ -385,7 +385,7 @@ protected function _getCreateTableSQL(string $name, array $columns, array $optio { $queryFields = $this->getColumnDeclarationListSQL($columns); - if (isset($options['primary']) && ! empty($options['primary'])) { + if (! empty($options['primary'])) { $keyColumns = array_unique(array_values($options['primary'])); $queryFields .= ', PRIMARY KEY(' . implode(', ', $keyColumns) . ')'; } @@ -396,7 +396,7 @@ protected function _getCreateTableSQL(string $name, array $columns, array $optio $sql = [$query]; - if (isset($options['indexes']) && ! empty($options['indexes'])) { + if (! empty($options['indexes'])) { foreach ($options['indexes'] as $index) { $sql[] = $this->getCreateIndexSQL($index, $name); } diff --git a/src/Platforms/SQLServerPlatform.php b/src/Platforms/SQLServerPlatform.php index 5c0024a0a9..3cd06f973b 100644 --- a/src/Platforms/SQLServerPlatform.php +++ b/src/Platforms/SQLServerPlatform.php @@ -208,13 +208,13 @@ protected function _getCreateTableSQL(string $name, array $columns, array $optio $columnListSql = $this->getColumnDeclarationListSQL($columns); - if (isset($options['uniqueConstraints']) && ! empty($options['uniqueConstraints'])) { + if (! empty($options['uniqueConstraints'])) { foreach ($options['uniqueConstraints'] as $definition) { $columnListSql .= ', ' . $this->getUniqueConstraintDeclarationSQL($definition); } } - if (isset($options['primary']) && ! empty($options['primary'])) { + if (! empty($options['primary'])) { $flags = ''; if (isset($options['primary_index']) && $options['primary_index']->hasFlag('nonclustered')) { $flags = ' NONCLUSTERED'; @@ -235,7 +235,7 @@ protected function _getCreateTableSQL(string $name, array $columns, array $optio $sql = [$query]; - if (isset($options['indexes']) && ! empty($options['indexes'])) { + if (! empty($options['indexes'])) { foreach ($options['indexes'] as $index) { $sql[] = $this->getCreateIndexSQL($index, $name); } diff --git a/src/Platforms/SQLitePlatform.php b/src/Platforms/SQLitePlatform.php index ccb7c46229..c9d4e9b66b 100644 --- a/src/Platforms/SQLitePlatform.php +++ b/src/Platforms/SQLitePlatform.php @@ -273,7 +273,7 @@ protected function _getCreateTableSQL(string $name, array $columns, array $optio { $queryFields = $this->getColumnDeclarationListSQL($columns); - if (isset($options['uniqueConstraints']) && ! empty($options['uniqueConstraints'])) { + if (! empty($options['uniqueConstraints'])) { foreach ($options['uniqueConstraints'] as $definition) { $queryFields .= ', ' . $this->getUniqueConstraintDeclarationSQL($definition); } @@ -300,13 +300,13 @@ protected function _getCreateTableSQL(string $name, array $columns, array $optio return $query; } - if (isset($options['indexes']) && ! empty($options['indexes'])) { + if (! empty($options['indexes'])) { foreach ($options['indexes'] as $indexDef) { $query[] = $this->getCreateIndexSQL($indexDef, $name); } } - if (isset($options['unique']) && ! empty($options['unique'])) { + if (! empty($options['unique'])) { foreach ($options['unique'] as $indexDef) { $query[] = $this->getCreateIndexSQL($indexDef, $name); } From d2b09cff2b24e6aa7e476bbb0a6008133721aa78 Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Tue, 24 Dec 2024 13:17:02 -0800 Subject: [PATCH 6/6] Deprecate empty index name --- UPGRADE.md | 3 ++- src/Schema/Index.php | 6 +++--- tests/Schema/IndexTest.php | 27 ++++++++++++++++----------- 3 files changed, 21 insertions(+), 15 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index a144a26949..b44a3a0c77 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -40,7 +40,8 @@ The `Sequence::isAutoIncrementsFor()` method has been deprecated. ## Deprecated using invalid database object names -Using the following objects with an empty name is deprecated: `Column`, `View`, `Sequence`, `Identifier`. +Using the following objects with an empty name is deprecated: `Table`, `Column`, `Index`, `View`, `Sequence`, +`Identifier`. Using the following objects with a qualified name is deprecated: `Column`, `ForeignKeyConstraint`, `Index`, `Schema`, `UniqueConstraint`. If the object name contains a dot, the name should be quoted. diff --git a/src/Schema/Index.php b/src/Schema/Index.php index 28a7469bec..591b1ac0f8 100644 --- a/src/Schema/Index.php +++ b/src/Schema/Index.php @@ -17,8 +17,8 @@ use function count; use function strtolower; -/** @extends AbstractOptionallyNamedObject */ -class Index extends AbstractOptionallyNamedObject +/** @extends AbstractNamedObject */ +class Index extends AbstractNamedObject { /** * Asset identifier instances of the column names the index is associated with. @@ -51,7 +51,7 @@ public function __construct( array $flags = [], private readonly array $options = [], ) { - parent::__construct($name); + parent::__construct($name ?? ''); $this->_isUnique = $isUnique || $isPrimary; $this->_isPrimary = $isPrimary; diff --git a/tests/Schema/IndexTest.php b/tests/Schema/IndexTest.php index 0eb13502be..1f53bd2c5f 100644 --- a/tests/Schema/IndexTest.php +++ b/tests/Schema/IndexTest.php @@ -4,14 +4,16 @@ namespace Doctrine\DBAL\Tests\Schema; -use Doctrine\DBAL\Exception; use Doctrine\DBAL\Schema\Index; use Doctrine\DBAL\Schema\Name\Identifier; +use Doctrine\Deprecations\PHPUnit\VerifyDeprecations; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; class IndexTest extends TestCase { + use VerifyDeprecations; + /** @param mixed[] $options */ private function createIndex(bool $unique = false, bool $primary = false, array $options = []): Index { @@ -174,21 +176,24 @@ public function testOptions(): void self::assertSame(['where' => 'name IS NULL'], $idx2->getOptions()); } - /** @throws Exception */ - public function testGetNonNullObjectName(): void + public function testEmptyName(): void { - $index = new Index('idx_user_id', ['user_id']); - $name = $index->getObjectName(); + $this->expectDeprecationWithIdentifier('https://github.com/doctrine/dbal/pull/6646'); - self::assertNotNull($name); - self::assertEquals(Identifier::unquoted('idx_user_id'), $name->getIdentifier()); + new Index(null, ['user_id']); } - /** @throws Exception */ - public function testGetNullObjectName(): void + public function testQualifiedName(): void { - $index = new Index(null, ['user_id']); + $this->expectDeprecationWithIdentifier('https://github.com/doctrine/dbal/pull/6592'); + + new Index('auth.idx_user_id', ['user_id']); + } + + public function testGetObjectName(): void + { + $index = new Index('idx_user_id', ['user_id']); - self::assertNull($index->getObjectName()); + self::assertEquals(Identifier::unquoted('idx_user_id'), $index->getObjectName()->getIdentifier()); } }