From 1500819064bc1f6be6bfe6d817ead79242015491 Mon Sep 17 00:00:00 2001 From: Guilherme Blanco Date: Wed, 29 Apr 2020 17:21:21 -0400 Subject: [PATCH 1/2] Segregated support of unique index and unique constraint. Added missing commits: 400d73c8a1e4f53459a877c8652811ee22c0913b, e91811cb3fe3101d7d8ac95c45f8bc1ba9bd913a, a4ecf053186b11bc31fbff79ef36248500383908, d7d1c397a35735383b34632d3a67bbc3a3371a45, c71b3ae0425e5c039b047825dbf8351a773e6785, 2f9da39ec645b54d08c95649c99d7b87a32bdc72 and 9aa4e79aab0477b4212b01d65de53193d9559e39 --- UPGRADE.md | 11 + src/Platforms/AbstractPlatform.php | 58 +++-- src/Platforms/MySqlPlatform.php | 12 +- src/Platforms/SQLServer2012Platform.php | 16 +- src/Platforms/SqlitePlatform.php | 2 +- src/Schema/AbstractSchemaManager.php | 3 +- src/Schema/Index.php | 11 +- src/Schema/SchemaException.php | 17 +- src/Schema/Table.php | 243 ++++++++++++++---- src/Schema/UniqueConstraint.php | 160 ++++++++++++ tests/Functional/ExceptionTest.php | 6 +- ...imaryKeyWithNewAutoIncrementColumnTest.php | 5 +- .../Schema/MySqlSchemaManagerTest.php | 4 +- .../SchemaManagerFunctionalTestCase.php | 4 +- tests/Platforms/AbstractPlatformTestCase.php | 24 +- .../AbstractSQLServerPlatformTestCase.php | 2 +- tests/Schema/ComparatorTest.php | 3 + tests/Schema/TableTest.php | 12 +- 18 files changed, 465 insertions(+), 128 deletions(-) create mode 100644 src/Schema/UniqueConstraint.php diff --git a/UPGRADE.md b/UPGRADE.md index 2b98e43bde5..c041608d795 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -1,5 +1,16 @@ # Upgrade to 3.0 +## BC BREAK: Doctrine\DBAL\Schema\Table constructor new parameter + +Deprecated parameter `$idGeneratorType` removed and added a new parameter `$uniqueConstraints`. +Constructor changed from: + +`__construct($name, array $columns = [], array $indexes = [], array $fkConstraints = [], $idGeneratorType = 0, array $options = [])` + +To the new constructor: + +`__construct($name, array $columns = [], array $indexes = [], array $uniqueConstraints = [], array $fkConstraints = [], array $options = [])` + ## BC BREAK: change in the behavior of `SchemaManager::dropDatabase()` When dropping a database, the DBAL no longer attempts to kill the client sessions that use the database. diff --git a/src/Platforms/AbstractPlatform.php b/src/Platforms/AbstractPlatform.php index ff000605e9a..931c77818a0 100644 --- a/src/Platforms/AbstractPlatform.php +++ b/src/Platforms/AbstractPlatform.php @@ -23,6 +23,7 @@ use Doctrine\DBAL\Schema\Sequence; use Doctrine\DBAL\Schema\Table; use Doctrine\DBAL\Schema\TableDiff; +use Doctrine\DBAL\Schema\UniqueConstraint; use Doctrine\DBAL\TransactionIsolationLevel; use Doctrine\DBAL\Types; use Doctrine\DBAL\Types\Type; @@ -1495,12 +1496,26 @@ public function getCreateTableSQL(Table $table, $createFlags = self::CREATE_INDE if (($createFlags & self::CREATE_INDEXES) > 0) { foreach ($table->getIndexes() as $index) { - if ($index->isPrimary()) { - $options['primary'] = $index->getQuotedColumns($this); - $options['primary_index'] = $index; - } else { + if (! $index->isPrimary()) { $options['indexes'][$index->getQuotedName($this)] = $index; + + continue; } + + $options['primary'] = $index->getQuotedColumns($this); + $options['primary_index'] = $index; + } + + foreach ($table->getUniqueConstraints() as $uniqueConstraint) { + $options['uniqueConstraints'][$uniqueConstraint->getQuotedName($this)] = $uniqueConstraint; + } + } + + if (($createFlags & self::CREATE_FOREIGNKEYS) > 0) { + $options['foreignKeys'] = []; + + foreach ($table->getForeignKeys() as $fkConstraint) { + $options['foreignKeys'][] = $fkConstraint; } } @@ -1513,6 +1528,7 @@ public function getCreateTableSQL(Table $table, $createFlags = self::CREATE_INDE && $this->_eventManager->hasListeners(Events::onSchemaCreateTableColumn) ) { $eventArgs = new SchemaCreateTableColumnEventArgs($column, $table, $this); + $this->_eventManager->dispatchEvent(Events::onSchemaCreateTableColumn, $eventArgs); $columnSql = array_merge($columnSql, $eventArgs->getSql()); @@ -1539,15 +1555,9 @@ public function getCreateTableSQL(Table $table, $createFlags = self::CREATE_INDE $columns[$columnData['name']] = $columnData; } - if (($createFlags & self::CREATE_FOREIGNKEYS) > 0) { - $options['foreignKeys'] = []; - foreach ($table->getForeignKeys() as $fkConstraint) { - $options['foreignKeys'][] = $fkConstraint; - } - } - if ($this->_eventManager !== null && $this->_eventManager->hasListeners(Events::onSchemaCreateTable)) { $eventArgs = new SchemaCreateTableEventArgs($table, $columns, $options, $this); + $this->_eventManager->dispatchEvent(Events::onSchemaCreateTable, $eventArgs); if ($eventArgs->isDefaultPrevented()) { @@ -1556,6 +1566,7 @@ public function getCreateTableSQL(Table $table, $createFlags = self::CREATE_INDE } $sql = $this->_getCreateTableSQL($tableName, $columns, $options); + if ($this->supportsCommentOnStatement()) { if ($table->hasOption('comment')) { $sql[] = $this->getCommentOnTableSQL($tableName, $table->getOption('comment')); @@ -1654,8 +1665,8 @@ protected function _getCreateTableSQL($name, array $columns, array $options = [] } $query = 'CREATE TABLE ' . $name . ' (' . $columnListSql; - $check = $this->getCheckDeclarationSQL($columns); + if (! empty($check)) { $query .= ', ' . $check; } @@ -2307,25 +2318,27 @@ public function getCheckDeclarationSQL(array $definition) * Obtains DBMS specific SQL code portion needed to set a unique * constraint declaration to be used in statements like CREATE TABLE. * - * @param string $name The name of the unique constraint. - * @param Index $index The index definition. + * @param string $name The name of the unique constraint. + * @param UniqueConstraint $constraint The unique constraint definition. * * @return string DBMS specific SQL code portion needed to set a constraint. * * @throws InvalidArgumentException */ - public function getUniqueConstraintDeclarationSQL($name, Index $index) + public function getUniqueConstraintDeclarationSQL($name, UniqueConstraint $constraint) { - $columns = $index->getColumns(); + $columns = $constraint->getQuotedColumns($this); $name = new Identifier($name); if (count($columns) === 0) { throw new InvalidArgumentException("Incomplete definition. 'columns' required."); } - return 'CONSTRAINT ' . $name->getQuotedName($this) . ' UNIQUE (' - . $this->getColumnsFieldDeclarationListSQL($columns) - . ')' . $this->getPartialIndexSQL($index); + $constraintFlags = array_merge(['UNIQUE'], array_map('strtoupper', $constraint->getFlags())); + $constraintName = $name->getQuotedName($this); + $columnListNames = $this->getColumnsFieldDeclarationListSQL($columns); + + return sprintf('CONSTRAINT %s %s (%s)', $constraintName, implode(' ', $constraintFlags), $columnListNames); } /** @@ -2348,9 +2361,8 @@ public function getIndexDeclarationSQL($name, Index $index) throw new InvalidArgumentException("Incomplete definition. 'columns' required."); } - return $this->getCreateIndexSQLFlags($index) . 'INDEX ' . $name->getQuotedName($this) . ' (' - . $this->getIndexFieldDeclarationListSQL($index) - . ')' . $this->getPartialIndexSQL($index); + return $this->getCreateIndexSQLFlags($index) . 'INDEX ' . $name->getQuotedName($this) + . ' (' . $this->getIndexFieldDeclarationListSQL($index) . ')' . $this->getPartialIndexSQL($index); } /** @@ -3022,7 +3034,7 @@ public function usesSequenceEmulatedIdentityColumns() /** * Returns the name of the sequence for a particular identity column in a particular table. * - * @see usesSequenceEmulatedIdentityColumns + * @see usesSequenceEmulatedIdentityColumns * * @param string $tableName The name of the table to return the sequence name for. * @param string $columnName The name of the identity column in the table to return the sequence name for. diff --git a/src/Platforms/MySqlPlatform.php b/src/Platforms/MySqlPlatform.php index 3c154945038..e5ef413149e 100644 --- a/src/Platforms/MySqlPlatform.php +++ b/src/Platforms/MySqlPlatform.php @@ -403,15 +403,15 @@ protected function _getCreateTableSQL($name, array $columns, array $options = [] $queryFields = $this->getColumnDeclarationListSQL($columns); if (isset($options['uniqueConstraints']) && ! empty($options['uniqueConstraints'])) { - foreach ($options['uniqueConstraints'] as $index => $definition) { - $queryFields .= ', ' . $this->getUniqueConstraintDeclarationSQL($index, $definition); + foreach ($options['uniqueConstraints'] as $constraintName => $definition) { + $queryFields .= ', ' . $this->getUniqueConstraintDeclarationSQL($constraintName, $definition); } } // add all indexes if (isset($options['indexes']) && ! empty($options['indexes'])) { - foreach ($options['indexes'] as $index => $definition) { - $queryFields .= ', ' . $this->getIndexDeclarationSQL($index, $definition); + foreach ($options['indexes'] as $indexName => $definition) { + $queryFields .= ', ' . $this->getIndexDeclarationSQL($indexName, $definition); } } @@ -752,9 +752,7 @@ private function getPreAlterTableAlterIndexForeignKeySQL(TableDiff $diff) continue; } - foreach ($diff->fromTable->getPrimaryKeyColumns() as $columnName) { - $column = $diff->fromTable->getColumn($columnName); - + foreach ($diff->fromTable->getPrimaryKeyColumns() as $columnName => $column) { // Check if an autoincrement column was dropped from the primary key. if (! $column->getAutoincrement() || in_array($columnName, $changedIndex->getColumns(), true)) { continue; diff --git a/src/Platforms/SQLServer2012Platform.php b/src/Platforms/SQLServer2012Platform.php index 07fefa3d14c..efc7fdb7851 100644 --- a/src/Platforms/SQLServer2012Platform.php +++ b/src/Platforms/SQLServer2012Platform.php @@ -331,8 +331,8 @@ protected function _getCreateTableSQL($name, array $columns, array $options = [] $columnListSql = $this->getColumnDeclarationListSQL($columns); if (isset($options['uniqueConstraints']) && ! empty($options['uniqueConstraints'])) { - foreach ($options['uniqueConstraints'] as $indexName => $definition) { - $columnListSql .= ', ' . $this->getUniqueConstraintDeclarationSQL($indexName, $definition); + foreach ($options['uniqueConstraints'] as $constraintName => $definition) { + $columnListSql .= ', ' . $this->getUniqueConstraintDeclarationSQL($constraintName, $definition); } } @@ -456,18 +456,6 @@ public function getDefaultConstraintDeclarationSQL($table, array $column) ' FOR ' . $columnName->getQuotedName($this); } - /** - * {@inheritDoc} - */ - public function getUniqueConstraintDeclarationSQL($name, Index $index) - { - $constraint = parent::getUniqueConstraintDeclarationSQL($name, $index); - - $constraint = $this->_appendUniqueConstraintDefinition($constraint, $index); - - return $constraint; - } - /** * {@inheritDoc} */ diff --git a/src/Platforms/SqlitePlatform.php b/src/Platforms/SqlitePlatform.php index d2ffe36e8f5..8b19ee92cde 100644 --- a/src/Platforms/SqlitePlatform.php +++ b/src/Platforms/SqlitePlatform.php @@ -963,8 +963,8 @@ public function getAlterTableSQL(TableDiff $diff) $table->getQuotedName($this), $columns, $this->getPrimaryIndexInAlteredTable($diff), + [], $this->getForeignKeysInAlteredTable($diff), - 0, $table->getOptions() ); $newTable->addOption('alter', true); diff --git a/src/Schema/AbstractSchemaManager.php b/src/Schema/AbstractSchemaManager.php index 084e4edc691..4e2bfe8cb1b 100644 --- a/src/Schema/AbstractSchemaManager.php +++ b/src/Schema/AbstractSchemaManager.php @@ -275,13 +275,14 @@ public function listTableDetails($name) { $columns = $this->listTableColumns($name); $foreignKeys = []; + if ($this->_platform->supportsForeignKeyConstraints()) { $foreignKeys = $this->listTableForeignKeys($name); } $indexes = $this->listTableIndexes($name); - return new Table($name, $columns, $indexes, $foreignKeys); + return new Table($name, $columns, $indexes, [], $foreignKeys); } /** diff --git a/src/Schema/Index.php b/src/Schema/Index.php index a85ccbb4526..947ec896af6 100644 --- a/src/Schema/Index.php +++ b/src/Schema/Index.php @@ -11,7 +11,6 @@ use function array_search; use function array_shift; use function count; -use function is_string; use function strtolower; class Index extends AbstractAsset implements Constraint @@ -79,18 +78,10 @@ public function __construct( } /** - * @param string $column - * - * @return void - * * @throws InvalidArgumentException */ - protected function _addColumn($column) + protected function _addColumn(string $column): void { - if (! is_string($column)) { - throw new InvalidArgumentException('Expecting a string as Index Column'); - } - $this->_columns[$column] = new Identifier($column); } diff --git a/src/Schema/SchemaException.php b/src/Schema/SchemaException.php index 6317679de0a..24f2a9f2dd4 100644 --- a/src/Schema/SchemaException.php +++ b/src/Schema/SchemaException.php @@ -22,7 +22,8 @@ class SchemaException extends DBALException public const SEQUENCE_ALREADY_EXISTS = 80; public const INDEX_INVALID_NAME = 90; public const FOREIGNKEY_DOESNT_EXIST = 100; - public const NAMESPACE_ALREADY_EXISTS = 110; + public const CONSTRAINT_DOESNT_EXIST = 110; + public const NAMESPACE_ALREADY_EXISTS = 120; /** * @param string $tableName @@ -146,6 +147,20 @@ public static function sequenceDoesNotExist($name) return new self("There exists no sequence with the name '" . $name . "'.", self::SEQUENCE_DOENST_EXIST); } + /** + * @param string $constraintName + * @param string $table + * + * @return SchemaException + */ + public static function uniqueConstraintDoesNotExist($constraintName, $table) + { + return new self( + sprintf('There exists no unique constraint with the name "%s" on table "%s".', $constraintName, $table), + self::CONSTRAINT_DOESNT_EXIST + ); + } + /** * @param string $fkName * @param string $table diff --git a/src/Schema/Table.php b/src/Schema/Table.php index ee695ff8b5c..79e322fcf34 100644 --- a/src/Schema/Table.php +++ b/src/Schema/Table.php @@ -8,8 +8,11 @@ use Doctrine\DBAL\Types\Type; use function array_filter; +use function array_keys; use function array_merge; use function in_array; +use function is_numeric; +use function is_string; use function preg_match; use function strlen; use function strtolower; @@ -24,14 +27,14 @@ class Table extends AbstractAsset /** @var Column[] */ protected $_columns = []; - /** @var Index[] */ - private $implicitIndexes = []; - /** @var Index[] */ protected $_indexes = []; - /** @var string|false */ - protected $_primaryKeyName = false; + /** @var string|null */ + protected $_primaryKeyName = null; + + /** @var UniqueConstraint[] */ + protected $uniqueConstraints = []; /** @var ForeignKeyConstraint[] */ protected $_fkConstraints = []; @@ -44,12 +47,15 @@ class Table extends AbstractAsset /** @var SchemaConfig|null */ protected $_schemaConfig = null; + /** @var Index[] */ + private $implicitIndexes = []; + /** * @param string $name * @param Column[] $columns * @param Index[] $indexes + * @param UniqueConstraint[] $uniqueConstraints * @param ForeignKeyConstraint[] $fkConstraints - * @param int $idGeneratorType * @param mixed[] $options * * @throws SchemaException @@ -58,8 +64,8 @@ public function __construct( $name, array $columns = [], array $indexes = [], + array $uniqueConstraints = [], array $fkConstraints = [], - $idGeneratorType = 0, array $options = [] ) { if (strlen($name) === 0) { @@ -76,6 +82,10 @@ public function __construct( $this->_addIndex($idx); } + foreach ($uniqueConstraints as $uniqueConstraint) { + $this->_addUniqueConstraint($uniqueConstraint); + } + foreach ($fkConstraints as $constraint) { $this->_addForeignKeyConstraint($constraint); } @@ -130,16 +140,15 @@ public function setPrimaryKey(array $columnNames, $indexName = false) } /** - * @param string[] $columnNames - * @param string|null $indexName - * @param string[] $flags - * @param mixed[] $options + * @param string[] $columnNames + * @param string[] $flags + * @param mixed[] $options * * @return self * * @throws SchemaException */ - public function addIndex(array $columnNames, $indexName = null, array $flags = [], array $options = []) + public function addIndex(array $columnNames, ?string $indexName = null, array $flags = [], array $options = []) { if ($indexName === null) { $indexName = $this->_generateIdentifierName( @@ -152,6 +161,30 @@ public function addIndex(array $columnNames, $indexName = null, array $flags = [ return $this->_addIndex($this->_createIndex($columnNames, $indexName, false, false, $flags, $options)); } + /** + * @param string[] $columnNames + * @param string[] $flags + * @param mixed[] $options + * + * @return self + */ + public function addUniqueConstraint( + array $columnNames, + ?string $indexName = null, + array $flags = [], + array $options = [] + ): Table { + if ($indexName === null) { + $indexName = $this->_generateIdentifierName( + array_merge([$this->getName()], $columnNames), + 'uniq', + $this->_getMaxIdentifierLength() + ); + } + + return $this->_addUniqueConstraint($this->_createUniqueConstraint($columnNames, $indexName, $flags, $options)); + } + /** * Drops the primary key from this table. * @@ -161,12 +194,12 @@ public function addIndex(array $columnNames, $indexName = null, array $flags = [ */ public function dropPrimaryKey() { - if ($this->_primaryKeyName === false) { + if ($this->_primaryKeyName === null) { return; } $this->dropIndex($this->_primaryKeyName); - $this->_primaryKeyName = false; + $this->_primaryKeyName = null; } /** @@ -181,6 +214,7 @@ public function dropPrimaryKey() public function dropIndex($name) { $name = $this->normalizeIdentifier($name); + if (! $this->hasIndex($name)) { throw SchemaException::indexDoesNotExist($name, $this->_name); } @@ -353,6 +387,7 @@ public function changeColumn($name, array $options) public function dropColumn($name) { $name = $this->normalizeIdentifier($name); + unset($this->_columns[$name]); return $this; @@ -409,9 +444,8 @@ public function addForeignKeyConstraint( $name, $options ); - $this->_addForeignKeyConstraint($constraint); - return $this; + return $this->_addForeignKeyConstraint($constraint); } /** @@ -467,7 +501,7 @@ protected function _addIndex(Index $indexCandidate) if ( (isset($this->_indexes[$indexName]) && ! in_array($indexName, $replacedImplicitIndexes, true)) || - ($this->_primaryKeyName !== false && $indexCandidate->isPrimary()) + ($this->_primaryKeyName !== null && $indexCandidate->isPrimary()) ) { throw SchemaException::indexAlreadyExists($indexName, $this->_name); } @@ -486,9 +520,39 @@ protected function _addIndex(Index $indexCandidate) } /** - * @return void - * - * @throws SchemaException + * @return self + */ + protected function _addUniqueConstraint(UniqueConstraint $constraint): Table + { + $mergedNames = array_merge([$this->getName()], $constraint->getColumns()); + $name = strlen($constraint->getName()) > 0 + ? $constraint->getName() + : $this->_generateIdentifierName($mergedNames, 'fk', $this->_getMaxIdentifierLength()); + + $name = $this->normalizeIdentifier($name); + + $this->uniqueConstraints[$name] = $constraint; + + // If there is already an index that fulfills this requirements drop the request. In the case of __construct + // calling this method during hydration from schema-details all the explicitly added indexes lead to duplicates. + // This creates computation overhead in this case, however no duplicate indexes are ever added (column based). + $indexName = $this->_generateIdentifierName($mergedNames, 'idx', $this->_getMaxIdentifierLength()); + + $indexCandidate = $this->_createIndex($constraint->getColumns(), $indexName, true, false); + + foreach ($this->_indexes as $existingIndex) { + if ($indexCandidate->isFullfilledBy($existingIndex)) { + return $this; + } + } + + $this->implicitIndexes[$this->normalizeIdentifier($indexName)] = $indexCandidate; + + return $this; + } + + /** + * @return self */ protected function _addForeignKeyConstraint(ForeignKeyConstraint $constraint) { @@ -498,7 +562,7 @@ protected function _addForeignKeyConstraint(ForeignKeyConstraint $constraint) $name = $constraint->getName(); } else { $name = $this->_generateIdentifierName( - array_merge((array) $this->getName(), $constraint->getLocalColumns()), + array_merge([$this->getName()], $constraint->getLocalColumns()), 'fk', $this->_getMaxIdentifierLength() ); @@ -511,7 +575,8 @@ protected function _addForeignKeyConstraint(ForeignKeyConstraint $constraint) // Add an explicit index on the foreign key columns. // If there is already an index that fulfils this requirements drop the request. // In the case of __construct calling this method during hydration from schema-details - // all the explicitly added indexes lead to duplicates. This creates computation overhead in this case, + // all the explicitly added indexes lead to duplicates. + // This creates computation overhead in this case, // however no duplicate indexes are ever added (based on columns). $indexName = $this->_generateIdentifierName( array_merge([$this->getName()], $constraint->getColumns()), @@ -522,12 +587,14 @@ protected function _addForeignKeyConstraint(ForeignKeyConstraint $constraint) foreach ($this->_indexes as $existingIndex) { if ($indexCandidate->isFullfilledBy($existingIndex)) { - return; + return $this; } } $this->_addIndex($indexCandidate); $this->implicitIndexes[$this->normalizeIdentifier($indexName)] = $indexCandidate; + + return $this; } /** @@ -556,6 +623,7 @@ public function hasForeignKey($name) public function getForeignKey($name) { $name = $this->normalizeIdentifier($name); + if (! $this->hasForeignKey($name)) { throw SchemaException::foreignKeyDoesNotExist($name, $this->_name); } @@ -575,6 +643,7 @@ public function getForeignKey($name) public function removeForeignKey($name) { $name = $this->normalizeIdentifier($name); + if (! $this->hasForeignKey($name)) { throw SchemaException::foreignKeyDoesNotExist($name, $this->_name); } @@ -582,6 +651,48 @@ public function removeForeignKey($name) unset($this->_fkConstraints[$name]); } + /** + * Returns whether this table has a unique constraint with the given name. + */ + public function hasUniqueConstraint(string $name): bool + { + $name = $this->normalizeIdentifier($name); + + return isset($this->uniqueConstraints[$name]); + } + + /** + * Returns the unique constraint with the given name. + * + * @throws SchemaException If the unique constraint does not exist. + */ + public function getUniqueConstraint(string $name): UniqueConstraint + { + $name = $this->normalizeIdentifier($name); + + if (! $this->hasUniqueConstraint($name)) { + throw SchemaException::uniqueConstraintDoesNotExist($name, $this->_name); + } + + return $this->uniqueConstraints[$name]; + } + + /** + * Removes the unique constraint with the given name. + * + * @throws SchemaException If the unique constraint does not exist. + */ + public function removeUniqueConstraint(string $name): void + { + $name = $this->normalizeIdentifier($name); + + if (! $this->hasForeignKey($name)) { + throw SchemaException::uniqueConstraintDoesNotExist($name, $this->_name); + } + + unset($this->uniqueConstraints[$name]); + } + /** * Returns ordered list of columns (primary keys are first, then foreign keys, then the rest) * @@ -589,26 +700,27 @@ public function removeForeignKey($name) */ public function getColumns() { - $primaryKey = $this->getPrimaryKey(); - $primaryKeyColumns = []; - - if ($primaryKey !== null) { - $primaryKeyColumns = $this->filterColumns($primaryKey->getColumns()); - } + $primaryKeyColumns = $this->hasPrimaryKey() ? $this->getPrimaryKeyColumns() : []; + $foreignKeyColumns = $this->getForeignKeyColumns(); + $remainderColumns = $this->filterColumns( + array_merge(array_keys($primaryKeyColumns), array_keys($foreignKeyColumns)), + true + ); - return array_merge($primaryKeyColumns, $this->getForeignKeyColumns(), $this->_columns); + return array_merge($primaryKeyColumns, $foreignKeyColumns, $remainderColumns); } /** - * Returns foreign key columns + * Returns the foreign key columns * * @return Column[] */ - private function getForeignKeyColumns() + public function getForeignKeyColumns() { $foreignKeyColumns = []; + foreach ($this->getForeignKeys() as $foreignKey) { - $foreignKeyColumns = array_merge($foreignKeyColumns, $foreignKey->getColumns()); + $foreignKeyColumns = array_merge($foreignKeyColumns, $foreignKey->getLocalColumns()); } return $this->filterColumns($foreignKeyColumns); @@ -621,10 +733,10 @@ private function getForeignKeyColumns() * * @return Column[] */ - private function filterColumns(array $columnNames) + private function filterColumns(array $columnNames, bool $reverse = false): array { - return array_filter($this->_columns, static function ($columnName) use ($columnNames): bool { - return in_array($columnName, $columnNames, true); + return array_filter($this->_columns, static function ($columnName) use ($columnNames, $reverse): bool { + return in_array($columnName, $columnNames, true) !== $reverse; }, ARRAY_FILTER_USE_KEY); } @@ -654,6 +766,7 @@ public function hasColumn($name) public function getColumn($name) { $name = $this->normalizeIdentifier($name); + if (! $this->hasColumn($name)) { throw SchemaException::columnDoesNotExist($name, $this->_name); } @@ -668,17 +781,15 @@ public function getColumn($name) */ public function getPrimaryKey() { - if ($this->_primaryKeyName !== false) { - return $this->getIndex($this->_primaryKeyName); - } - - return null; + return $this->_primaryKeyName !== null + ? $this->getIndex($this->_primaryKeyName) + : null; } /** * Returns the primary key columns. * - * @return string[] + * @return Column[] * * @throws DBALException */ @@ -690,7 +801,7 @@ public function getPrimaryKeyColumns() throw new DBALException('Table ' . $this->getName() . ' has no primary key.'); } - return $primaryKey->getColumns(); + return $this->filterColumns($primaryKey->getColumns()); } /** @@ -700,7 +811,7 @@ public function getPrimaryKeyColumns() */ public function hasPrimaryKey() { - return $this->_primaryKeyName !== false && $this->hasIndex($this->_primaryKeyName); + return $this->_primaryKeyName !== null && $this->hasIndex($this->_primaryKeyName); } /** @@ -744,6 +855,16 @@ public function getIndexes() return $this->_indexes; } + /** + * Returns the unique constraints. + * + * @return UniqueConstraint[] + */ + public function getUniqueConstraints(): array + { + return $this->uniqueConstraints; + } + /** * Returns the foreign key constraints. * @@ -825,16 +946,44 @@ public function __clone() } } + /** + * @param string[] $columnNames + * @param string[] $flags + * @param mixed[] $options + * + * @throws SchemaException + */ + private function _createUniqueConstraint( + array $columnNames, + string $indexName, + array $flags = [], + array $options = [] + ): UniqueConstraint { + if (preg_match('(([^a-zA-Z0-9_]+))', $this->normalizeIdentifier($indexName)) === 1) { + throw SchemaException::indexNameInvalid($indexName); + } + + foreach ($columnNames as $columnName => $indexColOptions) { + if (is_numeric($columnName) && is_string($indexColOptions)) { + $columnName = $indexColOptions; + } + + if (! $this->hasColumn($columnName)) { + throw SchemaException::columnDoesNotExist($columnName, $this->_name); + } + } + + return new UniqueConstraint($indexName, $columnNames, $flags, $options); + } + /** * Normalizes a given identifier. * * Trims quotes and lowercases the given identifier. * - * @param string|null $identifier The identifier to normalize. - * * @return string The normalized identifier. */ - private function normalizeIdentifier($identifier) + private function normalizeIdentifier(?string $identifier): string { if ($identifier === null) { return ''; diff --git a/src/Schema/UniqueConstraint.php b/src/Schema/UniqueConstraint.php new file mode 100644 index 00000000000..87a5f6e3b21 --- /dev/null +++ b/src/Schema/UniqueConstraint.php @@ -0,0 +1,160 @@ + Identifier) + * + * @var Identifier[] + */ + protected $columns = []; + + /** + * Platform specific flags. + * array($flagName => true) + * + * @var true[] + */ + protected $flags = []; + + /** + * Platform specific options. + * + * @var mixed[] + */ + private $options = []; + + /** + * @param string[] $columns + * @param string[] $flags + * @param mixed[] $options + */ + public function __construct(string $name, array $columns, array $flags = [], array $options = []) + { + $this->_setName($name); + + $this->options = $options; + + foreach ($columns as $column) { + $this->addColumn($column); + } + + foreach ($flags as $flag) { + $this->addFlag($flag); + } + } + + /** + * {@inheritdoc} + */ + public function getColumns() + { + return array_keys($this->columns); + } + + /** + * {@inheritdoc} + */ + public function getQuotedColumns(AbstractPlatform $platform) + { + $columns = []; + + foreach ($this->columns as $column) { + $columns[] = $column->getQuotedName($platform); + } + + return $columns; + } + + /** + * @return string[] + */ + public function getUnquotedColumns(): array + { + return array_map([$this, 'trimQuotes'], $this->getColumns()); + } + + /** + * Returns platform specific flags for unique constraint. + * + * @return string[] + */ + public function getFlags(): array + { + return array_keys($this->flags); + } + + /** + * Adds flag for a unique constraint that translates to platform specific handling. + * + * @return $this + * + * @example $uniqueConstraint->addFlag('CLUSTERED') + */ + public function addFlag(string $flag): UniqueConstraint + { + $this->flags[strtolower($flag)] = true; + + return $this; + } + + /** + * Does this unique constraint have a specific flag? + */ + public function hasFlag(string $flag): bool + { + return isset($this->flags[strtolower($flag)]); + } + + /** + * Removes a flag. + */ + public function removeFlag(string $flag): void + { + unset($this->flags[strtolower($flag)]); + } + + /** + * Does this unique constraint have a specific option? + */ + public function hasOption(string $name): bool + { + return isset($this->options[strtolower($name)]); + } + + /** + * @return mixed + */ + public function getOption(string $name) + { + return $this->options[strtolower($name)]; + } + + /** + * @return mixed[] + */ + public function getOptions(): array + { + return $this->options; + } + + /** + * Adds a new column to the unique constraint. + */ + protected function addColumn(string $column): void + { + $this->columns[$column] = new Identifier($column); + } +} diff --git a/tests/Functional/ExceptionTest.php b/tests/Functional/ExceptionTest.php index 4f817dc33cc..b09148cd7cf 100644 --- a/tests/Functional/ExceptionTest.php +++ b/tests/Functional/ExceptionTest.php @@ -28,7 +28,7 @@ use function unlink; use function version_compare; -use const PHP_OS; +use const PHP_OS_FAMILY; class ExceptionTest extends FunctionalTestCase { @@ -309,7 +309,7 @@ public function testConnectionExceptionSqLite(): void } // mode 0 is considered read-only on Windows - $mode = PHP_OS === 'Linux' ? 0444 : 0000; + $mode = PHP_OS_FAMILY !== 'Windows' ? 0444 : 0000; $filename = sprintf('%s/%s', sys_get_temp_dir(), 'doctrine_failed_connection_' . $mode . '.db'); @@ -434,7 +434,7 @@ private function tearDownForeignKeyConstraintViolationExceptionTest(): void private function isLinuxRoot(): bool { - return PHP_OS === 'Linux' && posix_getpwuid(posix_geteuid())['name'] === 'root'; + return PHP_OS_FAMILY !== 'Windows' && posix_getpwuid(posix_geteuid())['name'] === 'root'; } private function cleanupReadOnlyFile(string $filename): void diff --git a/tests/Functional/Platform/NewPrimaryKeyWithNewAutoIncrementColumnTest.php b/tests/Functional/Platform/NewPrimaryKeyWithNewAutoIncrementColumnTest.php index 74f95998c53..c22af8dc8cc 100644 --- a/tests/Functional/Platform/NewPrimaryKeyWithNewAutoIncrementColumnTest.php +++ b/tests/Functional/Platform/NewPrimaryKeyWithNewAutoIncrementColumnTest.php @@ -7,6 +7,8 @@ use Doctrine\DBAL\Schema\Comparator; use Doctrine\DBAL\Tests\FunctionalTestCase; +use function count; + final class NewPrimaryKeyWithNewAutoIncrementColumnTest extends FunctionalTestCase { protected function setUp(): void @@ -57,7 +59,8 @@ public function testAlterPrimaryKeyToAutoIncrementColumn(): void self::assertTrue($validationTable->hasColumn('new_id')); self::assertTrue($validationTable->getColumn('new_id')->getAutoincrement()); self::assertTrue($validationTable->hasPrimaryKey()); - self::assertSame(['new_id'], $validationTable->getPrimaryKeyColumns()); + self::assertEquals(1, count($validationTable->getPrimaryKeyColumns())); + self::assertArrayHasKey('new_id', $validationTable->getPrimaryKeyColumns()); } private function getPlatform(): AbstractPlatform diff --git a/tests/Functional/Schema/MySqlSchemaManagerTest.php b/tests/Functional/Schema/MySqlSchemaManagerTest.php index 9ee8be06490..f31fad44636 100644 --- a/tests/Functional/Schema/MySqlSchemaManagerTest.php +++ b/tests/Functional/Schema/MySqlSchemaManagerTest.php @@ -46,8 +46,8 @@ public function testSwitchPrimaryKeyColumns(): void $primaryKey = $table->getPrimaryKeyColumns(); self::assertCount(2, $primaryKey); - self::assertContains('bar_id', $primaryKey); - self::assertContains('foo_id', $primaryKey); + self::assertArrayHasKey('bar_id', $primaryKey); + self::assertArrayHasKey('foo_id', $primaryKey); } public function testDiffTableBug(): void diff --git a/tests/Functional/Schema/SchemaManagerFunctionalTestCase.php b/tests/Functional/Schema/SchemaManagerFunctionalTestCase.php index de9a7764e4c..f4d79310e6a 100644 --- a/tests/Functional/Schema/SchemaManagerFunctionalTestCase.php +++ b/tests/Functional/Schema/SchemaManagerFunctionalTestCase.php @@ -922,7 +922,7 @@ protected function createTestTable(string $name = 'test_table', array $data = [] */ protected function getTestTable(string $name, array $options = []): Table { - $table = new Table($name, [], [], [], false, $options); + $table = new Table($name, [], [], [], [], $options); $table->setSchemaConfig($this->schemaManager->createSchemaConfig()); $table->addColumn('id', 'integer', ['notnull' => true]); $table->setPrimaryKey(['id']); @@ -934,7 +934,7 @@ protected function getTestTable(string $name, array $options = []): Table protected function getTestCompositeTable(string $name): Table { - $table = new Table($name, [], [], [], false, []); + $table = new Table($name, [], [], [], [], []); $table->setSchemaConfig($this->schemaManager->createSchemaConfig()); $table->addColumn('id', 'integer', ['notnull' => true]); $table->addColumn('other_id', 'integer', ['notnull' => true]); diff --git a/tests/Platforms/AbstractPlatformTestCase.php b/tests/Platforms/AbstractPlatformTestCase.php index 45d74b79307..d2a63b83a92 100644 --- a/tests/Platforms/AbstractPlatformTestCase.php +++ b/tests/Platforms/AbstractPlatformTestCase.php @@ -14,6 +14,7 @@ use Doctrine\DBAL\Schema\Index; use Doctrine\DBAL\Schema\Table; use Doctrine\DBAL\Schema\TableDiff; +use Doctrine\DBAL\Schema\UniqueConstraint; use Doctrine\DBAL\Types\Type; use PHPUnit\Framework\TestCase; @@ -203,9 +204,9 @@ abstract public function getGenerateUniqueIndexSql(): string; public function testGeneratesPartialIndexesSqlOnlyWhenSupportingPartialIndexes(): void { - $where = 'test IS NULL AND test2 IS NOT NULL'; - $indexDef = new Index('name', ['test', 'test2'], false, false, [], ['where' => $where]); - $uniqueIndex = new Index('name', ['test', 'test2'], true, false, [], ['where' => $where]); + $where = 'test IS NULL AND test2 IS NOT NULL'; + $indexDef = new Index('name', ['test', 'test2'], false, false, [], ['where' => $where]); + $uniqueConstraint = new UniqueConstraint('name', ['test', 'test2'], [], []); $expected = ' WHERE ' . $where; @@ -215,14 +216,16 @@ public function testGeneratesPartialIndexesSqlOnlyWhenSupportingPartialIndexes() $actuals[] = $this->platform->getIndexDeclarationSQL('name', $indexDef); } - $actuals[] = $this->platform->getUniqueConstraintDeclarationSQL('name', $uniqueIndex); - $actuals[] = $this->platform->getCreateIndexSQL($indexDef, 'table'); + $uniqueConstraintSQL = $this->platform->getUniqueConstraintDeclarationSQL('name', $uniqueConstraint); + $indexSQL = $this->platform->getCreateIndexSQL($indexDef, 'table'); + + $this->assertStringEndsNotWith($expected, $uniqueConstraintSQL, 'WHERE clause should NOT be present'); foreach ($actuals as $actual) { if ($this->platform->supportsPartialIndexes()) { - self::assertStringEndsWith($expected, $actual, 'WHERE clause should be present'); + self::assertStringEndsWith($expected, $indexSQL, 'WHERE clause should be present'); } else { - self::assertStringEndsNotWith($expected, $actual, 'WHERE clause should NOT be present'); + self::assertStringEndsNotWith($expected, $indexSQL, 'WHERE clause should NOT be present'); } } } @@ -694,11 +697,11 @@ public function testQuotedColumnInForeignKeyPropagation(): void public function testQuotesReservedKeywordInUniqueConstraintDeclarationSQL(): void { - $index = new Index('select', ['foo'], true); + $constraint = new UniqueConstraint('select', ['foo'], [], []); self::assertSame( $this->getQuotesReservedKeywordInUniqueConstraintDeclarationSQL(), - $this->platform->getUniqueConstraintDeclarationSQL('select', $index) + $this->platform->getUniqueConstraintDeclarationSQL('select', $constraint) ); } @@ -776,6 +779,9 @@ public function testUsesSequenceEmulatedIdentityColumns(): void self::assertFalse($this->platform->usesSequenceEmulatedIdentityColumns()); } + /** + * @group DBAL-563 + */ public function testReturnsIdentitySequenceName(): void { $this->expectException(DBALException::class); diff --git a/tests/Platforms/AbstractSQLServerPlatformTestCase.php b/tests/Platforms/AbstractSQLServerPlatformTestCase.php index 48bf1ad082f..46c6b375263 100644 --- a/tests/Platforms/AbstractSQLServerPlatformTestCase.php +++ b/tests/Platforms/AbstractSQLServerPlatformTestCase.php @@ -1586,7 +1586,7 @@ public static function getReturnsForeignKeyReferentialActionSQL(): iterable protected function getQuotesReservedKeywordInUniqueConstraintDeclarationSQL(): string { - return 'CONSTRAINT [select] UNIQUE (foo) WHERE foo IS NOT NULL'; + return 'CONSTRAINT [select] UNIQUE (foo)'; } protected function getQuotesReservedKeywordInIndexDeclarationSQL(): string diff --git a/tests/Schema/ComparatorTest.php b/tests/Schema/ComparatorTest.php index c52e55b0c07..2ed3b477f5e 100644 --- a/tests/Schema/ComparatorTest.php +++ b/tests/Schema/ComparatorTest.php @@ -1274,6 +1274,7 @@ public function testForeignKeyRemovalWithRenamedLocalColumn(): void 'id_table1' => new Column('id_table1', Type::getType('integer')), ], [], + [], [ new ForeignKeyConstraint(['id_table1'], 'table1', ['id'], 'fk_table2_table1'), ] @@ -1287,6 +1288,7 @@ public function testForeignKeyRemovalWithRenamedLocalColumn(): void 'id_table3' => new Column('id_table3', Type::getType('integer')), ], [], + [], [ new ForeignKeyConstraint(['id_table3'], 'table3', ['id'], 'fk_table2_table3'), ] @@ -1299,6 +1301,7 @@ public function testForeignKeyRemovalWithRenamedLocalColumn(): void ), ]); $actual = Comparator::compareSchemas($fromSchema, $toSchema); + self::assertArrayHasKey('table2', $actual->changedTables); self::assertCount(1, $actual->orphanedForeignKeys); self::assertEquals('fk_table2_table1', $actual->orphanedForeignKeys[0]->getName()); diff --git a/tests/Schema/TableTest.php b/tests/Schema/TableTest.php index 039d0f84f05..0e5b3fd6b14 100644 --- a/tests/Schema/TableTest.php +++ b/tests/Schema/TableTest.php @@ -27,7 +27,7 @@ public function testCreateWithInvalidTableName(): void public function testGetName(): void { - $table = new Table('foo', [], [], []); + $table = new Table('foo', [], [], [], []); self::assertEquals('foo', $table->getName()); } @@ -167,7 +167,7 @@ public function testGetUnknownIndexThrowsException(): void { $this->expectException(SchemaException::class); - $table = new Table('foo', [], [], []); + $table = new Table('foo', [], [], [], []); $table->getIndex('unknownIndex'); } @@ -181,7 +181,7 @@ public function testAddTwoPrimaryThrowsException(): void new Index('the_primary', ['foo'], true, true), new Index('other_primary', ['bar'], true, true), ]; - $table = new Table('foo', $columns, $indexes, []); + $table = new Table('foo', $columns, $indexes, [], []); } public function testAddTwoIndexesWithSameNameThrowsException(): void @@ -194,14 +194,14 @@ public function testAddTwoIndexesWithSameNameThrowsException(): void new Index('an_idx', ['foo'], false, false), new Index('an_idx', ['bar'], false, false), ]; - $table = new Table('foo', $columns, $indexes, []); + $table = new Table('foo', $columns, $indexes, [], []); } public function testConstraints(): void { $constraint = new ForeignKeyConstraint([], 'foo', []); - $tableA = new Table('foo', [], [], [$constraint]); + $tableA = new Table('foo', [], [], [], [$constraint]); $constraints = $tableA->getForeignKeys(); self::assertCount(1, $constraints); @@ -210,7 +210,7 @@ public function testConstraints(): void public function testOptions(): void { - $table = new Table('foo', [], [], [], false, ['foo' => 'bar']); + $table = new Table('foo', [], [], [], [], ['foo' => 'bar']); self::assertTrue($table->hasOption('foo')); self::assertEquals('bar', $table->getOption('foo')); From 577f8a4d40f4b203cffa497806e912e6165cca51 Mon Sep 17 00:00:00 2001 From: Guilherme Blanco Date: Mon, 4 May 2020 10:00:04 -0400 Subject: [PATCH 2/2] Type hinted Table constructor --- src/Schema/Table.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Schema/Table.php b/src/Schema/Table.php index 79e322fcf34..09fa90b02c4 100644 --- a/src/Schema/Table.php +++ b/src/Schema/Table.php @@ -51,7 +51,6 @@ class Table extends AbstractAsset private $implicitIndexes = []; /** - * @param string $name * @param Column[] $columns * @param Index[] $indexes * @param UniqueConstraint[] $uniqueConstraints @@ -61,14 +60,14 @@ class Table extends AbstractAsset * @throws SchemaException */ public function __construct( - $name, + string $name, array $columns = [], array $indexes = [], array $uniqueConstraints = [], array $fkConstraints = [], array $options = [] ) { - if (strlen($name) === 0) { + if ($name === '') { throw InvalidTableName::new($name); }