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

Deprecate reliance on non-optimal defaults #8931

Merged
merged 1 commit into from
Oct 11, 2023
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
36 changes: 32 additions & 4 deletions UPGRADE.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,33 @@
# Upgrade to 2.17

## Deprecated: reliance on the non-optimal defaults that come with the `AUTO` identifier generation strategy

When the `AUTO` identifier generation strategy was introduced, the best
strategy at the time was selected for each database platform.
A lot of time has passed since then, and support for better strategies has been
added.

Because of that, it is now deprecated to rely on the historical defaults when
they differ from what we recommend now.

Instead, you should pick a strategy for each database platform you use, and it
will be used when using `AUTO`. As of now, only PostgreSQL is affected by this.
It is recommended that PostgreSQL user configure their new applications to use
`IDENTITY`:

```php
use Doctrine\DBAL\Platforms\PostgreSQLPlatform;
use Doctrine\ORM\Configuration;

assert($configuration instanceof Configuration);
$configuration->setIdentityGenerationPreferences([
PostgreSQLPlatform::CLASS => ClassMetadata::GENERATOR_TYPE_IDENTITY,
]);
```

If migrating an existing application is too costly, the deprecation can be
addressed by configuring `SEQUENCE` as the default strategy.

## Deprecate `EntityManagerInterface::getPartialReference()`

This method does not have a replacement and will be removed in 3.0.
Expand All @@ -14,7 +42,7 @@ Ensure `Doctrine\ORM\Configuration::setLazyGhostObjectEnabled(true)` is called t
## Deprecated accepting duplicate IDs in the identity map

For any given entity class and ID value, there should be only one object instance
representing the entity.
representing the entity.

In https://github.com/doctrine/orm/pull/10785, a check was added that will guard this
in the identity map. The most probable cause for violations of this rule are collisions
Expand Down Expand Up @@ -48,12 +76,12 @@ persister call `Doctrine\ORM\UnitOfWork::assignPostInsertId()` instead.
In ORM 3.0, a change will be made regarding how the `AttributeDriver` reports field mappings.
This change is necessary to be able to detect (and reject) some invalid mapping configurations.

To avoid surprises during 2.x upgrades, the new mode is opt-in. It can be activated on the
To avoid surprises during 2.x upgrades, the new mode is opt-in. It can be activated on the
`AttributeDriver` and `AnnotationDriver` by setting the `$reportFieldsWhereDeclared`
constructor parameter to `true`. It will cause `MappingException`s to be thrown when invalid
configurations are detected.

Not enabling the new mode will cause a deprecation notice to be raised. In ORM 3.0,
Not enabling the new mode will cause a deprecation notice to be raised. In ORM 3.0,
only the new mode will be available.

# Upgrade to 2.15
Expand All @@ -73,7 +101,7 @@ and will be an error in 3.0.

## Deprecated undeclared entity inheritance

As soon as an entity class inherits from another entity class, inheritance has to
As soon as an entity class inherits from another entity class, inheritance has to
be declared by adding the appropriate configuration for the root entity.

## Deprecated stubs for "concrete table inheritance"
Expand Down
37 changes: 27 additions & 10 deletions docs/en/reference/basic-mapping.rst
Original file line number Diff line number Diff line change
Expand Up @@ -422,9 +422,25 @@ the field that serves as the identifier with the ``#[Id]`` attribute.
# fields here

In most cases using the automatic generator strategy (``#[GeneratedValue]``) is
what you want. It defaults to the identifier generation mechanism your current
database vendor prefers: AUTO_INCREMENT with MySQL, sequences with PostgreSQL
and Oracle and so on.
what you want, but for backwards-compatibility reasons it might not. It
defaults to the identifier generation mechanism your current database
vendor preferred at the time that strategy was introduced:
``AUTO_INCREMENT`` with MySQL, sequences with PostgreSQL and Oracle and
so on.
We now recommend using ``IDENTITY`` for PostgreSQL, and you can achieve
that while still using the ``AUTO`` strategy, by configuring what it
defaults to.

.. code-block:: php

<?php
use Doctrine\DBAL\Platforms\PostgreSQLPlatform;
use Doctrine\ORM\Configuration;

$config = new Configuration();
$config->setIdentityGenerationPreferences([
PostgreSQLPlatform::class => ClassMetadata::GENERATOR_TYPE_IDENTITY,
]);

.. _identifier-generation-strategies:

Expand All @@ -441,17 +457,18 @@ Here is the list of possible generation strategies:

- ``AUTO`` (default): Tells Doctrine to pick the strategy that is
preferred by the used database platform. The preferred strategies
are IDENTITY for MySQL, SQLite, MsSQL and SQL Anywhere and SEQUENCE
for Oracle and PostgreSQL. This strategy provides full portability.
- ``SEQUENCE``: Tells Doctrine to use a database sequence for ID
generation. This strategy does currently not provide full
portability. Sequences are supported by Oracle, PostgreSql and
SQL Anywhere.
are ``IDENTITY`` for MySQL, SQLite, MsSQL and SQL Anywhere and, for
historical reasons, ``SEQUENCE`` for Oracle and PostgreSQL. This
strategy provides full portability.
- ``IDENTITY``: Tells Doctrine to use special identity columns in
the database that generate a value on insertion of a row. This
strategy does currently not provide full portability and is
supported by the following platforms: MySQL/SQLite/SQL Anywhere
(AUTO\_INCREMENT), MSSQL (IDENTITY) and PostgreSQL (SERIAL).
(``AUTO_INCREMENT``), MSSQL (``IDENTITY``) and PostgreSQL (``SERIAL``).
- ``SEQUENCE``: Tells Doctrine to use a database sequence for ID
generation. This strategy does currently not provide full
portability. Sequences are supported by Oracle, PostgreSql and
SQL Anywhere.
- ``UUID`` (deprecated): Tells Doctrine to use the built-in Universally
Unique Identifier generator. This strategy provides full portability.
- ``NONE``: Tells Doctrine that the identifiers are assigned (and
Expand Down
17 changes: 17 additions & 0 deletions lib/Doctrine/ORM/Configuration.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
use Doctrine\Common\Cache\Psr6\CacheAdapter;
use Doctrine\Common\Cache\Psr6\DoctrineProvider;
use Doctrine\Common\Persistence\PersistentObject;
use Doctrine\DBAL\Platforms\AbstractPlatform;
use Doctrine\Deprecations\Deprecation;
use Doctrine\ORM\Cache\CacheConfiguration;
use Doctrine\ORM\Cache\Exception\CacheException;
Expand All @@ -27,6 +28,7 @@
use Doctrine\ORM\Exception\ProxyClassesAlwaysRegenerating;
use Doctrine\ORM\Exception\UnknownEntityNamespace;
use Doctrine\ORM\Internal\Hydration\AbstractHydrator;
use Doctrine\ORM\Mapping\ClassMetadata;
use Doctrine\ORM\Mapping\ClassMetadataFactory;
use Doctrine\ORM\Mapping\DefaultEntityListenerResolver;
use Doctrine\ORM\Mapping\DefaultNamingStrategy;
Expand Down Expand Up @@ -68,6 +70,21 @@ class Configuration extends \Doctrine\DBAL\Configuration
/** @var mixed[] */
protected $_attributes = [];

/** @psalm-var array<class-string<AbstractPlatform>, ClassMetadata::GENERATOR_TYPE_*> */
private $identityGenerationPreferences = [];

/** @psalm-param array<class-string<AbstractPlatform>, ClassMetadata::GENERATOR_TYPE_*> $value */
public function setIdentityGenerationPreferences(array $value): void
{
$this->identityGenerationPreferences = $value;
}

/** @psalm-return array<class-string<AbstractPlatform>, ClassMetadata::GENERATOR_TYPE_*> $value */
public function getIdentityGenerationPreferences(): array
{
return $this->identityGenerationPreferences;
}

/**
* Sets the directory where Doctrine generates any necessary proxy class files.
*
Expand Down
64 changes: 58 additions & 6 deletions lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,13 @@

use function assert;
use function class_exists;
use function constant;
use function count;
use function end;
use function explode;
use function get_class;
use function in_array;
use function is_a;
use function is_subclass_of;
use function str_contains;
use function strlen;
Expand Down Expand Up @@ -71,6 +73,28 @@ class ClassMetadataFactory extends AbstractClassMetadataFactory
/** @var mixed[] */
private $embeddablesActiveNesting = [];

private const LEGACY_DEFAULTS_FOR_ID_GENERATION = [
'Doctrine\DBAL\Platforms\MySqlPlatform' => ClassMetadata::GENERATOR_TYPE_IDENTITY,
'Doctrine\DBAL\Platforms\PostgreSqlPlatform' => ClassMetadata::GENERATOR_TYPE_SEQUENCE,
Platforms\DB2Platform::class => ClassMetadata::GENERATOR_TYPE_IDENTITY,
Platforms\MySQLPlatform::class => ClassMetadata::GENERATOR_TYPE_IDENTITY,
Platforms\OraclePlatform::class => ClassMetadata::GENERATOR_TYPE_SEQUENCE,
Platforms\PostgreSQLPlatform::class => ClassMetadata::GENERATOR_TYPE_SEQUENCE,
Platforms\SQLServerPlatform::class => ClassMetadata::GENERATOR_TYPE_IDENTITY,
Platforms\SqlitePlatform::class => ClassMetadata::GENERATOR_TYPE_IDENTITY,
];

private const RECOMMENDED_STRATEGY = [
'Doctrine\DBAL\Platforms\MySqlPlatform' => 'IDENTITY',
'Doctrine\DBAL\Platforms\PostgreSqlPlatform' => 'IDENTITY',
Platforms\DB2Platform::class => 'IDENTITY',
Platforms\MySQLPlatform::class => 'IDENTITY',
Platforms\OraclePlatform::class => 'SEQUENCE',
Platforms\PostgreSQLPlatform::class => 'IDENTITY',
Platforms\SQLServerPlatform::class => 'IDENTITY',
Platforms\SqlitePlatform::class => 'IDENTITY',
];

/** @return void */
public function setEntityManager(EntityManagerInterface $em)
{
Expand Down Expand Up @@ -725,14 +749,42 @@ private function completeIdGeneratorMapping(ClassMetadataInfo $class): void
}
}

/** @psalm-return ClassMetadata::GENERATOR_TYPE_SEQUENCE|ClassMetadata::GENERATOR_TYPE_IDENTITY */
/** @psalm-return ClassMetadata::GENERATOR_TYPE_* */
private function determineIdGeneratorStrategy(AbstractPlatform $platform): int
{
if (
$platform instanceof Platforms\OraclePlatform
|| $platform instanceof Platforms\PostgreSQLPlatform
) {
return ClassMetadata::GENERATOR_TYPE_SEQUENCE;
assert($this->em !== null);
foreach ($this->em->getConfiguration()->getIdentityGenerationPreferences() as $platformFamily => $strategy) {
if (is_a($platform, $platformFamily)) {
return $strategy;
}
}

foreach (self::LEGACY_DEFAULTS_FOR_ID_GENERATION as $platformFamily => $strategy) {
if (is_a($platform, $platformFamily)) {
$recommendedStrategyName = self::RECOMMENDED_STRATEGY[$platformFamily];
if ($strategy !== constant('Doctrine\ORM\Mapping\ClassMetadata::GENERATOR_TYPE_' . $recommendedStrategyName)) {
Deprecation::trigger(
'doctrine/orm',
'https://github.com/doctrine/orm/issues/8893',
<<<'DEPRECATION'
Relying on non-optimal defaults for ID generation is deprecated.
Instead, configure identifier generation strategies explicitly through
configuration.
We currently recommend "%s" for "%s", so you should use
$configuration->setIdentityGenerationPreferences([
"%s" => ClassMetadata::GENERATOR_TYPE_%s,
]);
DEPRECATION
,
$recommendedStrategyName,
$platformFamily,
$platformFamily,
$recommendedStrategyName
);
}

return $strategy;
}
}

if ($platform->supportsIdentityColumns()) {
Expand Down
5 changes: 5 additions & 0 deletions phpstan-dbal2.neon
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ parameters:
count: 1
path: lib/Doctrine/ORM/Query/AST/Functions/SubstringFunction.php

-
message: '#^Class Doctrine\\DBAL\\Platforms\\MySQLPlatform not found\.$#'
count: 2
path: lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php

# Symfony cache supports passing a key prefix to the clear method.
- '/^Method Psr\\Cache\\CacheItemPoolInterface\:\:clear\(\) invoked with 1 parameter, 0 required\.$/'

Expand Down
1 change: 1 addition & 0 deletions psalm-baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,7 @@
<code>$class</code>
<code>$class</code>
<code><![CDATA[new $definition['class']()]]></code>
<code>$platformFamily</code>
</ArgumentTypeCoercion>
<DeprecatedClass>
<code>new UuidGenerator()</code>
Expand Down
56 changes: 56 additions & 0 deletions tests/Doctrine/Tests/ORM/Mapping/ClassMetadataFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
use Doctrine\DBAL\Connection;
use Doctrine\DBAL\Driver;
use Doctrine\DBAL\Platforms\AbstractPlatform;
use Doctrine\DBAL\Platforms\OraclePlatform;
use Doctrine\DBAL\Platforms\PostgreSQLPlatform;
use Doctrine\Deprecations\PHPUnit\VerifyDeprecations;
use Doctrine\ORM\Configuration;
use Doctrine\ORM\EntityManagerInterface;
Expand Down Expand Up @@ -151,6 +153,60 @@ public function testGetMetadataForReturnsLoadedCustomIdGenerator(): void
self::assertInstanceOf(CustomIdGenerator::class, $actual->idGenerator);
}

/** @param array<class-string<AbstractPlatform>, ClassMetadata::GENERATOR_TYPE_*> $preferences */
private function setUpCmfForPlatform(AbstractPlatform $platform, array $preferences = []): ClassMetadataFactoryTestSubject
{
$cmf = new ClassMetadataFactoryTestSubject();
$driver = $this->createMock(Driver::class);
$driver->method('getDatabasePlatform')
->willReturn($platform);
$entityManager = $this->createEntityManager(
new MetadataDriverMock(),
new Connection([], $driver)
);
$cmf->setEntityManager($entityManager);
$entityManager->getConfiguration()->setIdentityGenerationPreferences($preferences);

return $cmf;
}

public function testRelyingOnLegacyIdGenerationDefaultsIsDeprecatedIfItResultsInASuboptimalDefault(): void
{
$cm = $this->createValidClassMetadata();
$cm->setIdGeneratorType(ClassMetadata::GENERATOR_TYPE_AUTO);

$cmf = $this->setUpCmfForPlatform(new PostgreSQLPlatform());
$cmf->setMetadataForClass($cm->name, $cm);

$this->expectDeprecationWithIdentifier('https://github.com/doctrine/orm/issues/8893');
$cmf->getMetadataFor($cm->name);
}

public function testSpecifyingIdGenerationStrategyThroughConfigurationFixesTheDeprecation(): void
{
$cm = $this->createValidClassMetadata();
$cm->setIdGeneratorType(ClassMetadata::GENERATOR_TYPE_AUTO);

$cmf = $this->setUpCmfForPlatform(new PostgreSQLPlatform(), [
PostgreSQLPlatform::class => ClassMetadata::GENERATOR_TYPE_IDENTITY,
]);
$cmf->setMetadataForClass($cm->name, $cm);

$this->expectNoDeprecationWithIdentifier('https://github.com/doctrine/orm/issues/8893');
$cmf->getMetadataFor($cm->name);
}

public function testRelyingOnLegacyIdGenerationDefaultsIsOKIfItResultsInTheCurrentlyRecommendedStrategyBeingUsed(): void
{
$cm = $this->createValidClassMetadata();
$cm->setIdGeneratorType(ClassMetadata::GENERATOR_TYPE_AUTO);
$cmf = $this->setUpCmfForPlatform(new OraclePlatform());
$cmf->setMetadataForClass($cm->name, $cm);

$this->expectNoDeprecationWithIdentifier('https://github.com/doctrine/orm/issues/8893');
$cmf->getMetadataFor($cm->name);
}

public function testGetMetadataForThrowsExceptionOnUnknownCustomGeneratorClass(): void
{
$cm1 = $this->createValidClassMetadata();
Expand Down