Skip to content

Commit

Permalink
Merge pull request doctrine#6677 from morozov/deprecate-mixing-schema…
Browse files Browse the repository at this point in the history 
…-name-types

Deprecate mixing qualified and unqualified names
  • Loading branch information
@morozov
morozov authored Dec 29, 2024
2 parents d00d5c5 + dca4e60 commit 69d5e34
Show file tree
Hide file tree
Showing 7 changed files with 270 additions and 71 deletions.
16 changes: 16 additions & 0 deletions UPGRADE.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions psalm.xml.dist
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,12 @@
TODO: remove in 5.0.0
-->
<referencedMethod name="Doctrine\DBAL\Schema\AbstractAsset::getQuotedName" />

<!--
https://github.com/doctrine/dbal/pull/6677
TODO: remove in 5.0.0
-->
<referencedMethod name="Doctrine\DBAL\Schema\AbstractAsset::isIdentifierQuoted" />
</errorLevel>
</DeprecatedMethod>
<DeprecatedProperty>
Expand Down
10 changes: 10 additions & 0 deletions src/Schema/AbstractAsset.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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] === '[');
}

Expand Down
18 changes: 18 additions & 0 deletions src/Schema/SQLServerSchemaManager.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -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
Expand Down
182 changes: 123 additions & 59 deletions src/Schema/Schema.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -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
Expand All @@ -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<Table> $tables
* @param array<Sequence> $sequences
Expand Down Expand Up @@ -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;
}

/**
Expand All @@ -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<N> $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<OptionallyQualifiedName> $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<N> $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;
}

/**
Expand Down Expand Up @@ -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<Sequence> */
Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand Down
Loading

0 comments on commit 69d5e34

Please sign in to comment.