Skip to content
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

Disallow mixing qualified and unqualified names #6679

Merged
merged 2 commits into from
Dec 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions UPGRADE.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,21 @@ awareness about deprecated code.

# Upgrade to 5.0

## BC BREAK: Removed `AbstractAsset::isIdentifierQuoted()`

The `AbstractAsset::isIdentifierQuoted()` method has been removed.

## BC BREAK: Disallowed 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 forbidden.

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 forbidden.

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.

## BC BREAK: Removed `AbstractAsset::getQuotedName()`

The `AbstractAsset::getQuotedName()` method has been removed.
Expand Down
12 changes: 0 additions & 12 deletions psalm.xml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -62,18 +62,6 @@
-->
<referencedMethod name="Doctrine\DBAL\Schema\AbstractAsset::getNameParser" />
<referencedMethod name="Doctrine\DBAL\Schema\AbstractAsset::setName" />

<!--
https://github.com/doctrine/dbal/pull/6674
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>
<DocblockTypeContradiction>
Expand Down
19 changes: 0 additions & 19 deletions src/Schema/AbstractAsset.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
use Doctrine\DBAL\Schema\Name\OptionallyQualifiedName;
use Doctrine\DBAL\Schema\Name\Parser;
use Doctrine\DBAL\Schema\Name\UnqualifiedName;
use Doctrine\Deprecations\Deprecation;

use function array_map;
use function assert;
Expand Down Expand Up @@ -118,24 +117,6 @@ public function isQuoted(): bool
return $this->_quoted;
}

/**
* 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] === '[');
}

/**
* Trim quotes from the identifier.
*/
Expand Down
25 changes: 25 additions & 0 deletions src/Schema/Exception/ImproperlyQualifiedName.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
<?php

declare(strict_types=1);

namespace Doctrine\DBAL\Schema\Exception;

use Doctrine\DBAL\Schema\Name\OptionallyQualifiedName;
use Doctrine\DBAL\Schema\SchemaException;
use InvalidArgumentException;

use function sprintf;

/** @psalm-immutable */
class ImproperlyQualifiedName extends InvalidArgumentException implements SchemaException
{
public static function fromUnqualifiedName(OptionallyQualifiedName $name): self
{
return new self(sprintf('Schema uses qualified names, but %s is unqualified.', $name->toString()));
}

public static function fromQualifiedName(OptionallyQualifiedName $name): self
{
return new self(sprintf('Schema uses unqualified names, but %s is qualified.', $name->toString()));
}
}
56 changes: 25 additions & 31 deletions src/Schema/Schema.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use Doctrine\DBAL\Exception;
use Doctrine\DBAL\Platforms\AbstractPlatform;
use Doctrine\DBAL\Schema\Exception\ImproperlyQualifiedName;
use Doctrine\DBAL\Schema\Exception\InvalidName;
use Doctrine\DBAL\Schema\Exception\NamespaceAlreadyExists;
use Doctrine\DBAL\Schema\Exception\SequenceAlreadyExists;
Expand All @@ -20,7 +21,6 @@
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 count;
Expand Down Expand Up @@ -217,22 +217,12 @@

if ($qualifier !== 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.',
);
throw ImproperlyQualifiedName::fromQualifiedName($name);
}

$key = $qualifier->getValue() . '.' . $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.',
);
throw ImproperlyQualifiedName::fromUnqualifiedName($name);
}

return strtolower($key);
Expand Down Expand Up @@ -271,26 +261,14 @@
return $name;
}

/**
* Returns the unquoted representation of a given asset name.
*/
private function getUnquotedAssetName(string $assetName): string
{
if ($this->isIdentifierQuoted($assetName)) {
return $this->trimQuotes($assetName);
}

return $assetName;
}

/**
* Does this schema have a namespace with the given name?
*/
public function hasNamespace(string $name): bool
{
$name = strtolower($this->getUnquotedAssetName($name));
$key = $this->getNamespaceKey($name);

return isset($this->namespaces[$name]);
return isset($this->namespaces[$key]);
}

/**
Expand Down Expand Up @@ -333,17 +311,33 @@
*/
public function createNamespace(string $name): self
{
$unquotedName = strtolower($this->getUnquotedAssetName($name));
$key = $this->getNamespaceKey($name);

if (isset($this->namespaces[$unquotedName])) {
throw NamespaceAlreadyExists::new($unquotedName);
if (isset($this->namespaces[$key])) {
throw NamespaceAlreadyExists::new($name);
}

$this->namespaces[$unquotedName] = $name;
$this->namespaces[$key] = $name;

return $this;
}

/**
* Returns the key that will be used to store the given namespace name in the collection of namespaces.
*/
private function getNamespaceKey(string $name): string
{
$parser = Parsers::getUnqualifiedNameParser();

try {
$parsedName = $parser->parse($name);
} catch (Parser\Exception $e) {
throw InvalidName::fromParserException($name, $e);

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

View check run for this annotation

Codecov / codecov/patch

src/Schema/Schema.php#L334-L335

Added lines #L334 - L335 were not covered by tests
}

return strtolower($parsedName->getIdentifier()->getValue());
}

/**
* Creates a new table.
*/
Expand Down
34 changes: 11 additions & 23 deletions tests/Schema/SchemaTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
namespace Doctrine\DBAL\Tests\Schema;

use Doctrine\DBAL\Exception;
use Doctrine\DBAL\Schema\Exception\ImproperlyQualifiedName;
use Doctrine\DBAL\Schema\Exception\InvalidName;
use Doctrine\DBAL\Schema\Name\Identifier;
use Doctrine\DBAL\Schema\Schema;
Expand All @@ -13,16 +14,13 @@
use Doctrine\DBAL\Schema\Sequence;
use Doctrine\DBAL\Schema\Table;
use Doctrine\DBAL\Types\Types;
use Doctrine\Deprecations\PHPUnit\VerifyDeprecations;
use PHPUnit\Framework\TestCase;

use function array_shift;
use function strlen;

class SchemaTest extends TestCase
{
use VerifyDeprecations;

public function testAddTable(): void
{
$tableName = 'public.foo';
Expand Down Expand Up @@ -345,18 +343,14 @@ public function testCreatesNamespaceThroughAddingSequenceImplicitly(): void

public function testAddObjectWithQualifiedNameAfterUnqualifiedName(): void
{
$this->expectDeprecationWithIdentifier(
'https://github.com/doctrine/dbal/pull/6677#user-content-qualified-names',
);
$this->expectException(ImproperlyQualifiedName::class);

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',
);
$this->expectException(ImproperlyQualifiedName::class);

new Schema([new Table('public.t'), new Table('t')]);
}
Expand All @@ -365,9 +359,7 @@ public function testReferenceByQualifiedNameAmongUnqualifiedNames(): void
{
$schema = new Schema([new Table('t')]);

$this->expectDeprecationWithIdentifier(
'https://github.com/doctrine/dbal/pull/6677#user-content-qualified-names',
);
$this->expectException(ImproperlyQualifiedName::class);

$schema->hasTable('public.t');
}
Expand All @@ -376,9 +368,7 @@ public function testReferenceByUnqualifiedNameAmongQualifiedNames(): void
{
$schema = new Schema([new Table('public.t')]);

$this->expectDeprecationWithIdentifier(
'https://github.com/doctrine/dbal/pull/6677#user-content-unqualified-names',
);
$this->expectException(ImproperlyQualifiedName::class);

$schema->hasTable('t');
}
Expand All @@ -388,23 +378,21 @@ public function testAddObjectWithQualifiedNameAfterUnqualifiedNameWithDefaultNam
$schemaConfig = new SchemaConfig();
$schemaConfig->setName('public');

$this->expectNoDeprecationWithIdentifier(
'https://github.com/doctrine/dbal/pull/6677#user-content-qualified-names',
);
$schema = new Schema([new Table('t'), new Table('public.s')], [], $schemaConfig);

new Schema([new Table('t'), new Table('public.s')], [], $schemaConfig);
self::assertTrue($schema->hasTable('t'));
self::assertTrue($schema->hasTable('public.s'));
}

public function testAddObjectWithUnqualifiedNameAfterQualifiedNameWithDefaultNamespace(): void
{
$schemaConfig = new SchemaConfig();
$schemaConfig->setName('public');

$this->expectNoDeprecationWithIdentifier(
'https://github.com/doctrine/dbal/pull/6677#user-content-unqualified-names',
);
$schema = new Schema([new Table('public.t'), new Table('s')], [], $schemaConfig);

new Schema([new Table('public.t'), new Table('s')], [], $schemaConfig);
self::assertTrue($schema->hasTable('public.t'));
self::assertTrue($schema->hasTable('s'));
}

public function testReferencingByUnqualifiedNameAmongQualifiedNamesWithDefaultNamespace(): void
Expand Down