diff --git a/UPGRADE.md b/UPGRADE.md index 58391d5361..810f890a6c 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -8,6 +8,22 @@ awareness about deprecated code. # Upgrade to 4.3 +## Deprecated `AbstractAsset::isIdentifierQuoted()` + +The `AbstractAsset::isIdentifierQuoted()` method has been deprecated. Parse the name and introspect its identifiers +individually using `Identifier::isQuoted()` instead. + +## Deprecated mixing unqualified and qualified names in a schema without a default namespace + +If a schema lacks a default namespace configuration and has at least one object with an unqualified name, adding or +referencing objects with qualified names is deprecated. + +If a schema lacks a default namespace configuration and has at least one object with a qualified name, adding or +referencing objects with unqualified names is deprecated. + +Mixing unqualified and qualified names is permitted as long as the schema is configured to use a default namespace. In +this case, the default namespace will be used to resolve unqualified names. + ## Deprecated `AbstractAsset::getQuotedName()` The `AbstractAsset::getQuotedName()` method has been deprecated. Use `NamedObject::getObjectName()` or diff --git a/psalm.xml.dist b/psalm.xml.dist index a735ef8e32..1411f09005 100644 --- a/psalm.xml.dist +++ b/psalm.xml.dist @@ -147,6 +147,12 @@ TODO: remove in 5.0.0 --> + + + diff --git a/src/Schema/AbstractAsset.php b/src/Schema/AbstractAsset.php index eeb589bf1e..1b33c96339 100644 --- a/src/Schema/AbstractAsset.php +++ b/src/Schema/AbstractAsset.php @@ -301,9 +301,19 @@ public function isQuoted(): bool /** * Checks if this identifier is quoted. + * + * @deprecated Parse the name and introspect its identifiers individually using {@see Identifier::isQuoted()} + * instead. */ protected function isIdentifierQuoted(string $identifier): bool { + Deprecation::triggerIfCalledFromOutside( + 'doctrine/dbal', + 'https://github.com/doctrine/dbal/pull/6677', + '%s is deprecated and will be removed in 5.0.', + __METHOD__, + ); + return isset($identifier[0]) && ($identifier[0] === '`' || $identifier[0] === '"' || $identifier[0] === '['); } diff --git a/src/Schema/SQLServerSchemaManager.php b/src/Schema/SQLServerSchemaManager.php index f93d99bba6..c820c5aa42 100644 --- a/src/Schema/SQLServerSchemaManager.php +++ b/src/Schema/SQLServerSchemaManager.php @@ -273,6 +273,15 @@ public function createComparator(/* ComparatorConfig $config = new ComparatorCon ); } + public function createSchemaConfig(): SchemaConfig + { + $config = parent::createSchemaConfig(); + + $config->setName($this->getCurrentSchemaName()); + + return $config; + } + /** @throws Exception */ private function getDatabaseCollation(): string { @@ -291,6 +300,15 @@ private function getDatabaseCollation(): string return $this->databaseCollation; } + /** @throws Exception */ + private function getCurrentSchemaName(): ?string + { + $schemaName = $this->connection->fetchOne('SELECT SCHEMA_NAME()'); + assert($schemaName !== false); + + return $schemaName; + } + protected function selectTableNames(string $databaseName): Result { // The "sysdiagrams" table must be ignored as it's internal SQL Server table for Database Diagrams diff --git a/src/Schema/Schema.php b/src/Schema/Schema.php index c7f40e1616..cc4ca0f0f7 100644 --- a/src/Schema/Schema.php +++ b/src/Schema/Schema.php @@ -11,15 +11,15 @@ use Doctrine\DBAL\Schema\Exception\SequenceDoesNotExist; use Doctrine\DBAL\Schema\Exception\TableAlreadyExists; use Doctrine\DBAL\Schema\Exception\TableDoesNotExist; -use Doctrine\DBAL\Schema\Name\OptionallyQualifiedName; use Doctrine\DBAL\Schema\Name\Parser\UnqualifiedNameParser; use Doctrine\DBAL\Schema\Name\Parsers; use Doctrine\DBAL\Schema\Name\UnqualifiedName; use Doctrine\DBAL\SQL\Builder\CreateSchemaObjectsSQLBuilder; use Doctrine\DBAL\SQL\Builder\DropSchemaObjectsSQLBuilder; +use Doctrine\Deprecations\Deprecation; use function array_values; -use function str_contains; +use function count; use function strtolower; /** @@ -35,10 +35,16 @@ * Every asset in the doctrine schema has a name. A name consists of either a * namespace.local name pair or just a local unqualified name. * - * The abstraction layer that covers a PostgreSQL schema is the namespace of an + * Objects in a schema can be referenced by unqualified names or qualified + * names but not both. Whether a given schema uses qualified or unqualified + * names is determined at runtime by the presence of objects with unqualified + * names and namespaces. + * + * The abstraction layer that covers a PostgreSQL schema is the namespace of a * database object (asset). A schema can have a name, which will be used as * default namespace for the unqualified database objects that are created in - * the schema. + * the schema. If a schema uses qualified names and has a name, unqualified + * names will be resolved against the corresponding namespace. * * In the case of MySQL where cross-database queries are allowed this leads to * databases being "misinterpreted" as namespaces. This is intentional, however @@ -65,6 +71,12 @@ class Schema extends AbstractOptionallyNamedObject protected SchemaConfig $_schemaConfig; + /** + * Indicates whether the schema uses unqualified names for its objects. Once this flag is set to true, it won't be + * unset even after the objects with unqualified names have been dropped from the schema. + */ + private bool $usesUnqualifiedNames = false; + /** * @param array $tables * @param array $sequences @@ -105,42 +117,54 @@ protected function getNameParser(): UnqualifiedNameParser protected function _addTable(Table $table): void { - $namespaceName = $table->getNamespaceName(); - $tableName = $this->normalizeName($table); + $resolvedName = $this->resolveName($table); - if (isset($this->_tables[$tableName])) { - throw TableAlreadyExists::new($tableName); + $key = $this->getKeyFromResolvedName($resolvedName); + + if (isset($this->_tables[$key])) { + throw TableAlreadyExists::new($resolvedName->getName()); } - if ( - $namespaceName !== null - && ! $table->isInDefaultNamespace($this->getName()) - && ! $this->hasNamespace($namespaceName) - ) { - $this->createNamespace($namespaceName); + $namespaceName = $resolvedName->getNamespaceName(); + + if ($namespaceName !== null) { + if ( + ! $table->isInDefaultNamespace($this->getName()) + && ! $this->hasNamespace($namespaceName) + ) { + $this->createNamespace($namespaceName); + } + } else { + $this->usesUnqualifiedNames = true; } - $this->_tables[$tableName] = $table; + $this->_tables[$key] = $table; } protected function _addSequence(Sequence $sequence): void { - $namespaceName = $sequence->getNamespaceName(); - $seqName = $this->normalizeName($sequence); + $resolvedName = $this->resolveName($sequence); + + $key = $this->getKeyFromResolvedName($resolvedName); - if (isset($this->_sequences[$seqName])) { - throw SequenceAlreadyExists::new($seqName); + if (isset($this->_sequences[$key])) { + throw SequenceAlreadyExists::new($resolvedName->getName()); } - if ( - $namespaceName !== null - && ! $sequence->isInDefaultNamespace($this->getName()) - && ! $this->hasNamespace($namespaceName) - ) { - $this->createNamespace($namespaceName); + $namespaceName = $resolvedName->getNamespaceName(); + + if ($namespaceName !== null) { + if ( + ! $sequence->isInDefaultNamespace($this->getName()) + && ! $this->hasNamespace($namespaceName) + ) { + $this->createNamespace($namespaceName); + } + } else { + $this->usesUnqualifiedNames = true; } - $this->_sequences[$seqName] = $sequence; + $this->_sequences[$key] = $sequence; } /** @@ -165,44 +189,81 @@ public function getTables(): array public function getTable(string $name): Table { - $name = $this->getFullQualifiedAssetName($name); - if (! isset($this->_tables[$name])) { + $key = $this->getKeyFromName($name); + if (! isset($this->_tables[$key])) { throw TableDoesNotExist::new($name); } - return $this->_tables[$name]; + return $this->_tables[$key]; } - private function getFullQualifiedAssetName(string $name): string + /** + * Returns the key that will be used to store the given object in a collection of such objects based on its name. + * + * If the schema uses unqualified names, the object name must be unqualified. If the schema uses qualified names, + * the object name must be qualified. + * + * The resulting key is the lower-cased full object name. Lower-casing is + * actually wrong, but we have to do it to keep our sanity. If you are + * using database objects that only differentiate in the casing (FOO vs + * Foo) then you will NOT be able to use Doctrine Schema abstraction. + * + * @param AbstractAsset $asset + * + * @template N of Name + */ + private function getKeyFromResolvedName(AbstractAsset $asset): string { - $name = $this->getUnquotedAssetName($name); - - if (! str_contains($name, '.')) { - $name = $this->getName() . '.' . $name; + $key = $asset->getName(); + + if ($asset->getNamespaceName() !== null) { + if ($this->usesUnqualifiedNames) { + Deprecation::trigger( + 'doctrine/dbal', + 'https://github.com/doctrine/dbal/pull/6677#user-content-qualified-names', + 'Using qualified names to create or reference objects in a schema that uses unqualified ' + . 'names is deprecated.', + ); + } + + $key = $this->getName() . '.' . $key; + } elseif (count($this->namespaces) > 0) { + Deprecation::trigger( + 'doctrine/dbal', + 'https://github.com/doctrine/dbal/pull/6677#user-content-unqualified-names', + 'Using unqualified names to create or reference objects in a schema that uses qualified ' + . 'names and lacks a default namespace configuration is deprecated.', + ); } - return strtolower($name); + return strtolower($key); } /** - * The normalized name is qualified and lower-cased. Lower-casing is - * actually wrong, but we have to do it to keep our sanity. If you are - * using database objects that only differentiate in the casing (FOO vs - * Foo) then you will NOT be able to use Doctrine Schema abstraction. + * Returns the key that will be used to store the given object with the given name in a collection of such objects. * - * Every non-namespaced element is prefixed with this schema name. - * - * @param AbstractAsset $asset + * If the schema configuration has the default namespace, an unqualified name will be resolved to qualified against + * that namespace. */ - private function normalizeName(AbstractAsset $asset): string + private function getKeyFromName(string $name): string { - $name = $asset->getName(); + return $this->getKeyFromResolvedName($this->resolveName(new Identifier($name))); + } - if ($asset->getNamespaceName() === null) { - $name = $this->getName() . '.' . $name; + /** + * Resolves the qualified or unqualified name against the current schema name and returns a qualified name. + * + * @param AbstractAsset $asset A database object with optionally qualified name. + * + * @template N of Name + */ + private function resolveName(AbstractAsset $asset): AbstractAsset + { + if ($asset->getNamespaceName() === null && $this->name !== null) { + return new Identifier($this->getName() . '.' . $asset->getName()); } - return strtolower($name); + return $asset; } /** @@ -232,26 +293,26 @@ public function hasNamespace(string $name): bool */ public function hasTable(string $name): bool { - $name = $this->getFullQualifiedAssetName($name); + $key = $this->getKeyFromName($name); - return isset($this->_tables[$name]); + return isset($this->_tables[$key]); } public function hasSequence(string $name): bool { - $name = $this->getFullQualifiedAssetName($name); + $key = $this->getKeyFromName($name); - return isset($this->_sequences[$name]); + return isset($this->_sequences[$key]); } public function getSequence(string $name): Sequence { - $name = $this->getFullQualifiedAssetName($name); - if (! $this->hasSequence($name)) { + $key = $this->getKeyFromName($name); + if (! isset($this->_sequences[$key])) { throw SequenceDoesNotExist::new($name); } - return $this->_sequences[$name]; + return $this->_sequences[$key]; } /** @return list */ @@ -322,9 +383,12 @@ public function renameTable(string $oldName, string $newName): self */ public function dropTable(string $name): self { - $name = $this->getFullQualifiedAssetName($name); - $this->getTable($name); - unset($this->_tables[$name]); + $key = $this->getKeyFromName($name); + if (! isset($this->_tables[$key])) { + throw TableDoesNotExist::new($name); + } + + unset($this->_tables[$key]); return $this; } @@ -343,8 +407,8 @@ public function createSequence(string $name, int $allocationSize = 1, int $initi /** @return $this */ public function dropSequence(string $name): self { - $name = $this->getFullQualifiedAssetName($name); - unset($this->_sequences[$name]); + $key = $this->getKeyFromName($name); + unset($this->_sequences[$key]); return $this; } diff --git a/tests/Functional/Schema/SchemaManagerFunctionalTestCase.php b/tests/Functional/Schema/SchemaManagerFunctionalTestCase.php index be7d15f02c..f71a5ad447 100644 --- a/tests/Functional/Schema/SchemaManagerFunctionalTestCase.php +++ b/tests/Functional/Schema/SchemaManagerFunctionalTestCase.php @@ -8,9 +8,7 @@ use Doctrine\DBAL\Exception\DatabaseObjectNotFoundException; use Doctrine\DBAL\Platforms\AbstractPlatform; use Doctrine\DBAL\Platforms\OraclePlatform; -use Doctrine\DBAL\Platforms\PostgreSQLPlatform; use Doctrine\DBAL\Platforms\SQLitePlatform; -use Doctrine\DBAL\Platforms\SQLServerPlatform; use Doctrine\DBAL\Schema\AbstractAsset; use Doctrine\DBAL\Schema\AbstractSchemaManager; use Doctrine\DBAL\Schema\ForeignKeyConstraint; @@ -1091,16 +1089,16 @@ public function testGetNonExistingTable(): void public function testListTableDetailsWithFullQualifiedTableName(): void { - $platform = $this->connection->getDatabasePlatform(); - - if ($platform instanceof PostgreSQLPlatform) { - $defaultSchemaName = 'public'; - } elseif ($platform instanceof SQLServerPlatform) { - $defaultSchemaName = 'dbo'; - } else { + if (! $this->connection->getDatabasePlatform()->supportsSchemas()) { self::markTestSkipped('Test only works on platforms that support schemas.'); } + $schemaConfig = $this->schemaManager->createSchemaConfig(); + + $defaultSchemaName = $schemaConfig->getName(); + + self::assertNotNull($defaultSchemaName); + $primaryTableName = 'primary_table'; $foreignTableName = 'foreign_table'; diff --git a/tests/Schema/SchemaTest.php b/tests/Schema/SchemaTest.php index 92c6677fa6..62582fcd18 100644 --- a/tests/Schema/SchemaTest.php +++ b/tests/Schema/SchemaTest.php @@ -228,7 +228,10 @@ public function testHasTableForQuotedAsset(): void public function testHasNamespace(): void { - $schema = new Schema(); + $schemaConfig = new SchemaConfig(); + $schemaConfig->setName('public'); + + $schema = new Schema([], [], $schemaConfig); self::assertFalse($schema->hasNamespace('foo')); @@ -283,7 +286,10 @@ public function testThrowsExceptionOnCreatingNamespaceTwice(): void public function testCreatesNamespaceThroughAddingTableImplicitly(): void { - $schema = new Schema(); + $schemaConfig = new SchemaConfig(); + $schemaConfig->setName('public'); + + $schema = new Schema([], [], $schemaConfig); self::assertFalse($schema->hasNamespace('foo')); @@ -310,7 +316,10 @@ public function testCreatesNamespaceThroughAddingTableImplicitly(): void public function testCreatesNamespaceThroughAddingSequenceImplicitly(): void { - $schema = new Schema(); + $schemaConfig = new SchemaConfig(); + $schemaConfig->setName('public'); + + $schema = new Schema([], [], $schemaConfig); self::assertFalse($schema->hasNamespace('foo')); @@ -335,6 +344,84 @@ public function testCreatesNamespaceThroughAddingSequenceImplicitly(): void self::assertFalse($schema->hasNamespace('moo')); } + public function testAddObjectWithQualifiedNameAfterUnqualifiedName(): void + { + $this->expectDeprecationWithIdentifier( + 'https://github.com/doctrine/dbal/pull/6677#user-content-qualified-names', + ); + + new Schema([new Table('t'), new Table('public.t')]); + } + + public function testAddObjectWithUnqualifiedNameAfterQualifiedName(): void + { + $this->expectDeprecationWithIdentifier( + 'https://github.com/doctrine/dbal/pull/6677#user-content-unqualified-names', + ); + + new Schema([new Table('public.t'), new Table('t')]); + } + + public function testReferenceByQualifiedNameAmongUnqualifiedNames(): void + { + $schema = new Schema([new Table('t')]); + + $this->expectDeprecationWithIdentifier( + 'https://github.com/doctrine/dbal/pull/6677#user-content-qualified-names', + ); + + $schema->hasTable('public.t'); + } + + public function testReferenceByUnqualifiedNameAmongQualifiedNames(): void + { + $schema = new Schema([new Table('public.t')]); + + $this->expectDeprecationWithIdentifier( + 'https://github.com/doctrine/dbal/pull/6677#user-content-unqualified-names', + ); + + $schema->hasTable('t'); + } + + public function testAddObjectWithQualifiedNameAfterUnqualifiedNameWithDefaultNamespace(): void + { + $schemaConfig = new SchemaConfig(); + $schemaConfig->setName('public'); + + $this->expectNoDeprecationWithIdentifier( + 'https://github.com/doctrine/dbal/pull/6677#user-content-qualified-names', + ); + + new Schema([new Table('t'), new Table('public.s')], [], $schemaConfig); + } + + public function testAddObjectWithUnqualifiedNameAfterQualifiedNameWithDefaultNamespace(): void + { + $schemaConfig = new SchemaConfig(); + $schemaConfig->setName('public'); + + $this->expectNoDeprecationWithIdentifier( + 'https://github.com/doctrine/dbal/pull/6677#user-content-unqualified-names', + ); + + new Schema([new Table('public.t'), new Table('s')], [], $schemaConfig); + } + + public function testReferencingByUnqualifiedNameAmongQualifiedNamesWithDefaultNamespace(): void + { + $schemaConfig = new SchemaConfig(); + $schemaConfig->setName('public'); + + $schema = new Schema([new Table('public.t')], [], $schemaConfig); + + self::assertTrue($schema->hasTable('t')); + self::assertTrue($schema->hasTable('public.t')); + + self::assertFalse($schema->hasTable('s')); + self::assertFalse($schema->hasTable('public.s')); + } + public function testQualifiedName(): void { $schemaConfig = new SchemaConfig();