8000 Deprecate mixing qualified and unqualified names by morozov · Pull Request #6677 · doctrine/dbal · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Deprecate mixing qualified and unqualified names #6677

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.

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
merged 3 commits into from
Dec 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

< 8000 summary data-ga-click="Pull Requests, open view comments menu, type:semantic" data-view-component="true" class="Link--muted select-menu-button btn-link"> Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions UPGRADE.md
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
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
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__,
);

retu 10000 rn isset($identifier[0]) && ($identifier[0] === '`' || $identifier[0] === '"' || $identifier[0] === '[');
}

Expand Down
18 changes: 18 additions & 0 deletions src/Schema/SQLServerSchemaManager.php
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
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 @@

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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this flag write once? Should it be marked as such by using @readonly?

Copy link
Member Author
@morozov morozov Dec 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not exactly write once, it can be flipped once from false to true. @readonly might be confusing, if the reader assumes the same contract as the one provided by the readonly modifier:

[...] which prevents modification of the property after initialization.

This flag is initialized as false and can be modified at any point in time to true or never modified.


/**
* @param array<Table> $tables
* @param array<Sequence> $sequences
Expand Down Expand Up @@ -105,42 +117,54 @@

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 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 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 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);

Check warning on line 388 in src/Schema/Schema.php

View check run for this annotation

Codecov / codecov/patch

src/Schema/Schema.php#L388

Added line #L388 was not covered by tests
}

unset($this->_tables[$key]);

return $this;
}
Expand All @@ -343,8 +407,8 @@
/** @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