From 194f5062bb804502291148a63c9b583eaa3d3b55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Paris?= Date: Sat, 29 Jan 2022 12:28:23 +0100 Subject: [PATCH 1/4] Add method name in exception When we assert a given exception should be thrown, and get this instead, it is hard to figure out what went wrong. --- tests/Doctrine/Tests/Mocks/ConnectionMock.php | 10 ++++++++-- .../Tests/Mocks/DatabasePlatformMock.php | 17 ++++++++++++++--- tests/Doctrine/Tests/Mocks/DriverMock.php | 7 ++++++- .../Tests/ORM/Functional/Ticket/DDC3634Test.php | 6 +++++- 4 files changed, 33 insertions(+), 7 deletions(-) diff --git a/tests/Doctrine/Tests/Mocks/ConnectionMock.php b/tests/Doctrine/Tests/Mocks/ConnectionMock.php index 5176332649..e25c8e0d5d 100644 --- a/tests/Doctrine/Tests/Mocks/ConnectionMock.php +++ b/tests/Doctrine/Tests/Mocks/ConnectionMock.php @@ -87,12 +87,18 @@ public function delete($table, array $criteria, array $types = []) */ public function fetchColumn($statement, array $params = [], $colunm = 0, array $types = []) { - throw new BadMethodCallException('Call to deprecated method.'); + throw new BadMethodCallException(sprintf( + 'Call to deprecated method %s().', + __METHOD__ + )); } public function query(?string $sql = null): Result { - throw new BadMethodCallException('Call to deprecated method.'); + throw new BadMethodCallException(sprintf( + 'Call to deprecated method %s().', + __METHOD__ + )); } /** diff --git a/tests/Doctrine/Tests/Mocks/DatabasePlatformMock.php b/tests/Doctrine/Tests/Mocks/DatabasePlatformMock.php index 1e15c5e9df..aa87106d25 100644 --- a/tests/Doctrine/Tests/Mocks/DatabasePlatformMock.php +++ b/tests/Doctrine/Tests/Mocks/DatabasePlatformMock.php @@ -8,6 +8,8 @@ use Doctrine\DBAL\Exception as DBALException; use Doctrine\DBAL\Platforms\AbstractPlatform; +use function sprintf; + /** * Mock class for DatabasePlatform. */ @@ -15,7 +17,10 @@ class DatabasePlatformMock extends AbstractPlatform { public function prefersIdentityColumns(): bool { - throw new BadMethodCallException('Call to deprecated method.'); + throw new BadMethodCallException(sprintf( + 'Call to deprecated method %s().', + __METHOD__ + )); } public function supportsIdentityColumns(): bool @@ -25,7 +30,10 @@ public function supportsIdentityColumns(): bool public function prefersSequences(): bool { - throw new BadMethodCallException('Call to deprecated method.'); + throw new BadMethodCallException(sprintf( + 'Call to deprecated method %s().', + __METHOD__ + )); } public function supportsSequences(): bool @@ -94,7 +102,10 @@ public function getClobTypeDeclarationSQL(array $field) public function getName(): string { - throw new BadMethodCallException('Call to deprecated method.'); + throw new BadMethodCallException(sprintf( + 'Call to deprecated method %s().', + __METHOD__ + )); } /** diff --git a/tests/Doctrine/Tests/Mocks/DriverMock.php b/tests/Doctrine/Tests/Mocks/DriverMock.php index 867d13e7f3..04154e55f9 100644 --- a/tests/Doctrine/Tests/Mocks/DriverMock.php +++ b/tests/Doctrine/Tests/Mocks/DriverMock.php @@ -12,6 +12,8 @@ use Doctrine\DBAL\Schema\AbstractSchemaManager; use Exception; +use function sprintf; + /** * Mock class for Driver. */ @@ -70,7 +72,10 @@ public function setSchemaManager(AbstractSchemaManager $sm): void */ public function getName() { - throw new BadMethodCallException('Call to deprecated method.'); + throw new BadMethodCallException(sprintf( + 'Call to deprecated method %s().', + __METHOD__ + )); } /** diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3634Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3634Test.php index 3fa0adb5f1..e98e87f620 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3634Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3634Test.php @@ -20,6 +20,7 @@ use Doctrine\Tests\OrmFunctionalTestCase; use function debug_backtrace; +use function sprintf; use const PHP_INT_MAX; @@ -347,7 +348,10 @@ public function query($sql = null): Result /** {@inheritDoc} */ public function executeUpdate($query, array $params = [], array $types = []): int { - throw new BadMethodCallException('Call to deprecated method.'); + throw new BadMethodCallException(sprintf( + 'Call to deprecated method %s().', + __METHOD__ + )); } /** {@inheritDoc} */ From e89b680a285ea4c694cdaf9edb4490d904c23e54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Paris?= Date: Fri, 20 Aug 2021 20:39:26 +0200 Subject: [PATCH 2/4] Deprecate reliance on non-optimal defaults What was optimal 10 years ago no longer is, and things might change in the future. Using AUTO is still the best solution in most cases, and it should be easy to make it mean something else when it is not. --- UPGRADE.md | 36 +++++++++-- docs/en/reference/basic-mapping.rst | 37 ++++++++--- lib/Doctrine/ORM/Configuration.php | 17 +++++ .../ORM/Mapping/ClassMetadataFactory.php | 64 +++++++++++++++++-- phpstan-dbal2.neon | 5 ++ psalm-baseline.xml | 1 + .../ORM/Mapping/ClassMetadataFactoryTest.php | 56 ++++++++++++++++ 7 files changed, 196 insertions(+), 20 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index 107a44f0bf..6d2947f6e6 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -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. @@ -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 @@ -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 @@ -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" diff --git a/docs/en/reference/basic-mapping.rst b/docs/en/reference/basic-mapping.rst index f6f166b6fb..3bc4b5d33f 100644 --- a/docs/en/reference/basic-mapping.rst +++ b/docs/en/reference/basic-mapping.rst @@ -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 + + setIdentityGenerationPreferences([ + PostgreSQLPlatform::class => ClassMetadata::GENERATOR_TYPE_IDENTITY, + ]); .. _identifier-generation-strategies: @@ -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 diff --git a/lib/Doctrine/ORM/Configuration.php b/lib/Doctrine/ORM/Configuration.php index ca5063667c..c6eccc2540 100644 --- a/lib/Doctrine/ORM/Configuration.php +++ b/lib/Doctrine/ORM/Configuration.php @@ -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; @@ -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; @@ -68,6 +70,21 @@ class Configuration extends \Doctrine\DBAL\Configuration /** @var mixed[] */ protected $_attributes = []; + /** @psalm-var array, ClassMetadata::GENERATOR_TYPE_*> */ + private $identityGenerationPreferences = []; + + /** @psalm-param array, ClassMetadata::GENERATOR_TYPE_*> $value */ + public function setIdentityGenerationPreferences(array $value): void + { + $this->identityGenerationPreferences = $value; + } + + /** @psalm-return array, ClassMetadata::GENERATOR_TYPE_*> $value */ + public function getIdentityGenerationPreferences(): array + { + return $this->identityGenerationPreferences; + } + /** * Sets the directory where Doctrine generates any necessary proxy class files. * diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php b/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php index 333da89fee..34c98d37d5 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php @@ -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; @@ -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) { @@ -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()) { diff --git a/phpstan-dbal2.neon b/phpstan-dbal2.neon index 646aceadc5..c2a8bcb447 100644 --- a/phpstan-dbal2.neon +++ b/phpstan-dbal2.neon @@ -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\.$/' diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 1e43faeba9..38a2972861 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -537,6 +537,7 @@ $class $class + $platformFamily new UuidGenerator() diff --git a/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataFactoryTest.php b/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataFactoryTest.php index a268361a90..eae9e87119 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataFactoryTest.php +++ b/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataFactoryTest.php @@ -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; @@ -151,6 +153,60 @@ public function testGetMetadataForReturnsLoadedCustomIdGenerator(): void self::assertInstanceOf(CustomIdGenerator::class, $actual->idGenerator); } + /** @param array, 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(); From 2db1f76dee92961b76bf3ea747b0b958bd2c2329 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Wed, 11 Oct 2023 11:56:11 +0200 Subject: [PATCH 3/4] Rework AUTO keyword deafults to be IDENTITY always, except Oracle SEQUENCE. --- .../ORM/Mapping/ClassMetadataFactory.php | 56 +------------------ .../Mapping/Exception/CannotGenerateIds.php | 23 -------- .../ORM/Mapping/ClassMetadataFactoryTest.php | 46 --------------- 3 files changed, 3 insertions(+), 122 deletions(-) delete mode 100644 lib/Doctrine/ORM/Mapping/Exception/CannotGenerateIds.php diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php b/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php index 67d660b61f..733e84093e 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php @@ -7,7 +7,6 @@ use Doctrine\Common\EventManager; use Doctrine\DBAL\Platforms; use Doctrine\DBAL\Platforms\AbstractPlatform; -use Doctrine\Deprecations\Deprecation; use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\Event\LoadClassMetadataEventArgs; use Doctrine\ORM\Event\OnClassMetadataNotFoundEventArgs; @@ -29,7 +28,6 @@ use function assert; use function class_exists; -use function constant; use function count; use function end; use function explode; @@ -58,26 +56,8 @@ class ClassMetadataFactory extends AbstractClassMetadataFactory /** @var mixed[] */ private array $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, + private const NON_IDENTITY_DEFAULT_STRATEGY = [ 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', ]; public function setEntityManager(EntityManagerInterface $em): void @@ -634,43 +614,13 @@ private function determineIdGeneratorStrategy(AbstractPlatform $platform): int } } - foreach (self::LEGACY_DEFAULTS_FOR_ID_GENERATION as $platformFamily => $strategy) { + foreach (self::NON_IDENTITY_DEFAULT_STRATEGY 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()) { - return ClassMetadata::GENERATOR_TYPE_IDENTITY; - } - - if ($platform->supportsSequences()) { - return ClassMetadata::GENERATOR_TYPE_SEQUENCE; - } - - throw CannotGenerateIds::withPlatform($platform); + return ClassMetadata::GENERATOR_TYPE_IDENTITY; } private function truncateSequenceName(string $schemaElementName): string diff --git a/lib/Doctrine/ORM/Mapping/Exception/CannotGenerateIds.php b/lib/Doctrine/ORM/Mapping/Exception/CannotGenerateIds.php deleted file mode 100644 index a9895ceb21..0000000000 --- a/lib/Doctrine/ORM/Mapping/Exception/CannotGenerateIds.php +++ /dev/null @@ -1,23 +0,0 @@ -hasField('name')); } - public function testItThrowsWhenUsingAutoWithIncompatiblePlatform(): void - { - $cm1 = $this->createValidClassMetadata(); - $cm1->setIdGeneratorType(ClassMetadata::GENERATOR_TYPE_AUTO); - - $driver = $this->createMock(Driver::class); - $driver->method('getDatabasePlatform') - ->willReturn($this->createMock(AbstractPlatform::class)); - - $connection = new Connection([], $driver); - $entityManager = $this->createEntityManager(new MetadataDriverMock(), $connection); - $cmf = new ClassMetadataFactoryTestSubject(); - $cmf->setEntityManager($entityManager); - $cmf->setMetadataForClass($cm1->name, $cm1); - $this->expectException(CannotGenerateIds::class); - - $actual = $cmf->getMetadataFor($cm1->name); - } - public function testGetMetadataForReturnsLoadedCustomIdGenerator(): void { $cm1 = $this->createValidClassMetadata(); @@ -149,32 +129,6 @@ private function setUpCmfForPlatform(AbstractPlatform $platform, array $preferen 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(); From 2d31d7196111c82afb14c0111d3a78912874aedb Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Wed, 11 Oct 2023 12:06:00 +0200 Subject: [PATCH 4/4] Adapt tests, upgrade cs and fix static coding violations and baseline --- .../ORM/Mapping/ClassMetadataFactory.php | 1 - psalm-baseline.xml | 76 ++++++++++++++++++- .../SchemaTool/PostgreSqlSchemaToolTest.php | 8 -- .../ORM/Functional/Ticket/DDC832Test.php | 6 -- .../ORM/Mapping/ClassMetadataFactoryTest.php | 1 - 5 files changed, 75 insertions(+), 17 deletions(-) diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php b/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php index 733e84093e..ad782108a0 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php @@ -16,7 +16,6 @@ use Doctrine\ORM\Id\BigIntegerIdentityGenerator; use Doctrine\ORM\Id\IdentityGenerator; use Doctrine\ORM\Id\SequenceGenerator; -use Doctrine\ORM\Mapping\Exception\CannotGenerateIds; use Doctrine\ORM\Mapping\Exception\InvalidCustomGenerator; use Doctrine\ORM\Mapping\Exception\UnknownGeneratorType; use Doctrine\Persistence\Mapping\AbstractClassMetadataFactory; diff --git a/psalm-baseline.xml b/psalm-baseline.xml index cf0387199b..f6bbde11a6 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -296,10 +296,84 @@ - + + columnNames]]> + columnNames]]> + columnNames]]> + columnNames]]> + + + $mapping + $mapping + $mapping + $mapping + $overrideMapping + + + ReflectionClass|null + + + $definition + subClasses]]> + + + $className + $className + namespace . '\\' . $className]]> + + + class-string|null + + + reflClass]]> + + + $entity + + + $class + $className + + + declaredField]]]> + + + getProperty + getProperty + getProperty + getValue + getValue + getValue + instantiate + setValue + setValue + + + + + + + + + + $idGenerator + $table + + + $mapping !== false + $mapping !== false + + + array_values + + + table]]> + null + + $platformFamily diff --git a/tests/Doctrine/Tests/ORM/Functional/SchemaTool/PostgreSqlSchemaToolTest.php b/tests/Doctrine/Tests/ORM/Functional/SchemaTool/PostgreSqlSchemaToolTest.php index b13548560f..61aef5d404 100644 --- a/tests/Doctrine/Tests/ORM/Functional/SchemaTool/PostgreSqlSchemaToolTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/SchemaTool/PostgreSqlSchemaToolTest.php @@ -12,7 +12,6 @@ use Doctrine\ORM\Mapping\JoinColumn; use Doctrine\ORM\Mapping\ManyToOne; use Doctrine\ORM\Mapping\Table; -use Doctrine\Tests\Models\CMS\CmsAddress; use Doctrine\Tests\OrmFunctionalTestCase; use PHPUnit\Framework\Attributes\Group; @@ -32,13 +31,6 @@ protected function setUp(): void } } - public function testPostgresMetadataSequenceIncrementedBy10(): void - { - $address = $this->_em->getClassMetadata(CmsAddress::class); - - self::assertEquals(1, $address->sequenceGeneratorDefinition['allocationSize']); - } - #[Group('DDC-1657')] public function testUpdateSchemaWithPostgreSQLSchema(): void { diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC832Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC832Test.php index d3b908fd56..5cf9fb9d9c 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC832Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC832Test.php @@ -5,7 +5,6 @@ namespace Doctrine\Tests\ORM\Functional\Ticket; use Doctrine\DBAL\Platforms\OraclePlatform; -use Doctrine\DBAL\Platforms\PostgreSQLPlatform; use Doctrine\ORM\Mapping\Column; use Doctrine\ORM\Mapping\DiscriminatorColumn; use Doctrine\ORM\Mapping\DiscriminatorMap; @@ -43,11 +42,6 @@ public function tearDown(): void $sm->dropTable($platform->quoteIdentifier('TREE_INDEX')); $sm->dropTable($platform->quoteIdentifier('INDEX')); $sm->dropTable($platform->quoteIdentifier('LIKE')); - - if ($platform instanceof PostgreSQLPlatform) { - $sm->dropSequence($platform->quoteIdentifier('INDEX_id_seq')); - $sm->dropSequence($platform->quoteIdentifier('LIKE_id_seq')); - } } #[Group('DDC-832')] diff --git a/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataFactoryTest.php b/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataFactoryTest.php index 6b8fa4813d..752ad559b1 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataFactoryTest.php +++ b/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataFactoryTest.php @@ -9,7 +9,6 @@ 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;