From 88b36e07e173ebf766a0c7ef9fea1aedbeb20eec Mon Sep 17 00:00:00 2001 From: Simon Podlipsky Date: Thu, 10 Nov 2022 14:22:22 +0100 Subject: [PATCH 1/4] refactor: use list type in SchemaTool (#10199) --- lib/Doctrine/ORM/Tools/SchemaTool.php | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/Doctrine/ORM/Tools/SchemaTool.php b/lib/Doctrine/ORM/Tools/SchemaTool.php index 2597ad656ab..d71b7f1d9d3 100644 --- a/lib/Doctrine/ORM/Tools/SchemaTool.php +++ b/lib/Doctrine/ORM/Tools/SchemaTool.php @@ -110,7 +110,7 @@ public function createSchema(array $classes) * * @psalm-param list $classes * - * @return string[] The SQL statements needed to create the schema for the classes. + * @return list The SQL statements needed to create the schema for the classes. */ public function getCreateSchemaSql(array $classes) { @@ -140,7 +140,7 @@ private function processingNotRequired( * * @param mixed[] $indexData index or unique constraint data * - * @return string[] Column names from combined fields and columns mappings + * @return list Column names from combined fields and columns mappings */ private function getIndexColumns(ClassMetadata $class, array $indexData): array { @@ -856,7 +856,7 @@ public function dropDatabase() /** * Gets the SQL needed to drop the database schema for the connections database. * - * @return string[] + * @return list */ public function getDropDatabaseSQL() { @@ -874,7 +874,7 @@ public function getDropDatabaseSQL() * * @psalm-param list $classes * - * @return string[] + * @return list */ public function getDropSchemaSQL(array $classes) { @@ -947,11 +947,11 @@ public function updateSchema(array $classes, $saveMode = false) * Gets the sequence of SQL statements that need to be performed in order * to bring the given class mappings in-synch with the relational schema. * - * @param mixed[] $classes The classes to consider. - * @param bool $saveMode If TRUE, only generates SQL for a partial update - * that does not include SQL for dropping assets which are scheduled for deletion. + * @param bool $saveMode If TRUE, only generates SQL for a partial update + * that does not include SQL for dropping assets which are scheduled for deletion. + * @param list $classes The classes to consider. * - * @return string[] The sequence of SQL statements. + * @return list The sequence of SQL statements. */ public function getUpdateSchemaSql(array $classes, $saveMode = false) { From 90ececcc85ef5b37dccfa8ed96f64fe3bb37e73c Mon Sep 17 00:00:00 2001 From: Willem Verspyck Date: Thu, 10 Nov 2022 14:43:40 +0100 Subject: [PATCH 2/4] Allow "Expr\Func" as condition in join (#10202) --- lib/Doctrine/ORM/Query/Expr/Join.php | 16 +++++----- lib/Doctrine/ORM/QueryBuilder.php | 30 +++++++++---------- tests/Doctrine/Tests/ORM/QueryBuilderTest.php | 17 +++++++++++ 3 files changed, 40 insertions(+), 23 deletions(-) diff --git a/lib/Doctrine/ORM/Query/Expr/Join.php b/lib/Doctrine/ORM/Query/Expr/Join.php index a22e2c8386d..fa7363d1f1c 100644 --- a/lib/Doctrine/ORM/Query/Expr/Join.php +++ b/lib/Doctrine/ORM/Query/Expr/Join.php @@ -37,19 +37,19 @@ class Join */ protected $conditionType; - /** @var string|Comparison|Composite|null */ + /** @var string|Comparison|Composite|Func|null */ protected $condition; /** @var string|null */ protected $indexBy; /** - * @param string $joinType The condition type constant. Either INNER_JOIN or LEFT_JOIN. - * @param string $join The relationship to join. - * @param string|null $alias The alias of the join. - * @param string|null $conditionType The condition type constant. Either ON or WITH. - * @param string|Comparison|Composite|null $condition The condition for the join. - * @param string|null $indexBy The index for the join. + * @param string $joinType The condition type constant. Either INNER_JOIN or LEFT_JOIN. + * @param string $join The relationship to join. + * @param string|null $alias The alias of the join. + * @param string|null $conditionType The condition type constant. Either ON or WITH. + * @param string|Comparison|Composite|Func|null $condition The condition for the join. + * @param string|null $indexBy The index for the join. * @psalm-param self::INNER_JOIN|self::LEFT_JOIN $joinType * @psalm-param self::ON|self::WITH|null $conditionType */ @@ -93,7 +93,7 @@ public function getConditionType() return $this->conditionType; } - /** @return string|Comparison|Composite|null */ + /** @return string|Comparison|Composite|Func|null */ public function getCondition() { return $this->condition; diff --git a/lib/Doctrine/ORM/QueryBuilder.php b/lib/Doctrine/ORM/QueryBuilder.php index 7008be40728..15842db0c97 100644 --- a/lib/Doctrine/ORM/QueryBuilder.php +++ b/lib/Doctrine/ORM/QueryBuilder.php @@ -992,11 +992,11 @@ public function indexBy($alias, $indexBy) * ->join('u.Phonenumbers', 'p', Expr\Join::WITH, 'p.is_primary = 1'); * * - * @param string $join The relationship to join. - * @param string $alias The alias of the join. - * @param string|null $conditionType The condition type constant. Either ON or WITH. - * @param string|Expr\Comparison|Expr\Composite|null $condition The condition for the join. - * @param string|null $indexBy The index for the join. + * @param string $join The relationship to join. + * @param string $alias The alias of the join. + * @param string|null $conditionType The condition type constant. Either ON or WITH. + * @param string|Expr\Comparison|Expr\Composite|Expr\Func|null $condition The condition for the join. + * @param string|null $indexBy The index for the join. * @psalm-param Expr\Join::ON|Expr\Join::WITH|null $conditionType * * @return $this @@ -1019,11 +1019,11 @@ public function join($join, $alias, $conditionType = null, $condition = null, $i * ->from('User', 'u') * ->innerJoin('u.Phonenumbers', 'p', Expr\Join::WITH, 'p.is_primary = 1'); * - * @param string $join The relationship to join. - * @param string $alias The alias of the join. - * @param string|null $conditionType The condition type constant. Either ON or WITH. - * @param string|Expr\Comparison|Expr\Composite|null $condition The condition for the join. - * @param string|null $indexBy The index for the join. + * @param string $join The relationship to join. + * @param string $alias The alias of the join. + * @param string|null $conditionType The condition type constant. Either ON or WITH. + * @param string|Expr\Comparison|Expr\Composite|Expr\Func|null $condition The condition for the join. + * @param string|null $indexBy The index for the join. * @psalm-param Expr\Join::ON|Expr\Join::WITH|null $conditionType * * @return $this @@ -1060,11 +1060,11 @@ public function innerJoin($join, $alias, $conditionType = null, $condition = nul * ->leftJoin('u.Phonenumbers', 'p', Expr\Join::WITH, 'p.is_primary = 1'); * * - * @param string $join The relationship to join. - * @param string $alias The alias of the join. - * @param string|null $conditionType The condition type constant. Either ON or WITH. - * @param string|Expr\Comparison|Expr\Composite|null $condition The condition for the join. - * @param string|null $indexBy The index for the join. + * @param string $join The relationship to join. + * @param string $alias The alias of the join. + * @param string|null $conditionType The condition type constant. Either ON or WITH. + * @param string|Expr\Comparison|Expr\Composite|Expr\Func|null $condition The condition for the join. + * @param string|null $indexBy The index for the join. * @psalm-param Expr\Join::ON|Expr\Join::WITH|null $conditionType * * @return $this diff --git a/tests/Doctrine/Tests/ORM/QueryBuilderTest.php b/tests/Doctrine/Tests/ORM/QueryBuilderTest.php index 21e08503698..b6fe839bbb8 100644 --- a/tests/Doctrine/Tests/ORM/QueryBuilderTest.php +++ b/tests/Doctrine/Tests/ORM/QueryBuilderTest.php @@ -183,6 +183,23 @@ public function testComplexInnerJoinWithCompositeCondition(): void ); } + public function testComplexInnerJoinWithFuncCondition(): void + { + $qb = $this->entityManager->createQueryBuilder(); + $qb + ->select('u', 'a') + ->from(CmsUser::class, 'u') + ->innerJoin('u.articles', 'a', Join::WITH, $qb->expr()->in( + 'u.id', + [1, 2, 3] + )); + + $this->assertValidQueryBuilder( + $qb, + 'SELECT u, a FROM Doctrine\Tests\Models\CMS\CmsUser u INNER JOIN u.articles a WITH u.id IN(1, 2, 3)' + ); + } + public function testComplexInnerJoinWithIndexBy(): void { $qb = $this->entityManager->createQueryBuilder() From 7e45ad935c7eb095496446222559b01767bbf02e Mon Sep 17 00:00:00 2001 From: "Alexander M. Turek" Date: Thu, 10 Nov 2022 22:29:50 +0100 Subject: [PATCH 3/4] Psalm 4.30.0, PHPStan 1.9.2 (#10213) --- composer.json | 4 ++-- lib/Doctrine/ORM/Query/Expr/Base.php | 5 +++-- psalm-baseline.xml | 13 ++----------- 3 files changed, 7 insertions(+), 15 deletions(-) diff --git a/composer.json b/composer.json index e4c80322e1a..e6525f34d5a 100644 --- a/composer.json +++ b/composer.json @@ -42,13 +42,13 @@ "doctrine/annotations": "^1.13", "doctrine/coding-standard": "^9.0.2 || ^10.0", "phpbench/phpbench": "^0.16.10 || ^1.0", - "phpstan/phpstan": "~1.4.10 || 1.9.0", + "phpstan/phpstan": "~1.4.10 || 1.9.2", "phpunit/phpunit": "^7.5 || ^8.5 || ^9.5", "psr/log": "^1 || ^2 || ^3", "squizlabs/php_codesniffer": "3.7.1", "symfony/cache": "^4.4 || ^5.4 || ^6.0", "symfony/yaml": "^3.4 || ^4.0 || ^5.0 || ^6.0", - "vimeo/psalm": "4.29.0" + "vimeo/psalm": "4.30.0" }, "conflict": { "doctrine/annotations": "<1.13 || >= 2.0" diff --git a/lib/Doctrine/ORM/Query/Expr/Base.php b/lib/Doctrine/ORM/Query/Expr/Base.php index 06b92629e04..bab52dbc432 100644 --- a/lib/Doctrine/ORM/Query/Expr/Base.php +++ b/lib/Doctrine/ORM/Query/Expr/Base.php @@ -5,6 +5,7 @@ namespace Doctrine\ORM\Query\Expr; use InvalidArgumentException; +use Stringable; use function count; use function get_class; @@ -30,10 +31,10 @@ abstract class Base /** @var string */ protected $postSeparator = ')'; - /** @psalm-var list */ + /** @var list */ protected $allowedClasses = []; - /** @psalm-var list */ + /** @var list */ protected $parts = []; /** @param mixed $args */ diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 32c84df56a9..1c43c7c1f63 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -1,5 +1,5 @@ - + IterableResult @@ -2039,18 +2039,9 @@ $parts - - - $this->parts - - - $this->parts[0] - - - + $part - $this->parts[0] From 953e42d0591bf4982881d970b68ac70d1be40660 Mon Sep 17 00:00:00 2001 From: "Alexander M. Turek" Date: Fri, 11 Nov 2022 10:50:07 +0100 Subject: [PATCH 4/4] Add a constructor to CacheKey (#10212) --- UPGRADE.md | 5 ++++ lib/Doctrine/ORM/Cache/CacheKey.php | 16 +++++++++++ lib/Doctrine/ORM/Cache/CollectionCacheKey.php | 3 +- lib/Doctrine/ORM/Cache/EntityCacheKey.php | 3 +- lib/Doctrine/ORM/Cache/QueryCacheKey.php | 3 +- lib/Doctrine/ORM/Cache/TimestampCacheKey.php | 2 +- psalm.xml | 6 ++++ tests/Doctrine/Tests/Mocks/CacheKeyMock.php | 4 +-- .../Doctrine/Tests/ORM/Cache/CacheKeyTest.php | 28 +++++++++++++++++++ 9 files changed, 64 insertions(+), 6 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index 5600d38fb3d..b0ce5d8711c 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -1,5 +1,10 @@ # Upgrade to 2.14 +## Deprecated constructing a `CacheKey` without `$hash` + +The `Doctrine\ORM\Cache\CacheKey` class has an explicit constructor now with +an optional parameter `$hash`. That parameter will become mandatory in 3.0. + ## Deprecated `AttributeDriver::$entityAnnotationClasses` If you need to change the behavior of `AttributeDriver::isTransient()`, diff --git a/lib/Doctrine/ORM/Cache/CacheKey.php b/lib/Doctrine/ORM/Cache/CacheKey.php index 5f4305d6865..59b4b297c11 100644 --- a/lib/Doctrine/ORM/Cache/CacheKey.php +++ b/lib/Doctrine/ORM/Cache/CacheKey.php @@ -4,6 +4,8 @@ namespace Doctrine\ORM\Cache; +use Doctrine\Deprecations\Deprecation; + /** * Defines entity / collection / query key to be stored in the cache region. * Allows multiple roles to be stored in the same cache region. @@ -17,4 +19,18 @@ abstract class CacheKey * @var string */ public $hash; + + public function __construct(?string $hash = null) + { + if ($hash === null) { + Deprecation::trigger( + 'doctrine/orm', + 'https://github.com/doctrine/orm/pull/10212', + 'Calling %s() without providing a value for the $hash parameter is deprecated.', + __METHOD__ + ); + } else { + $this->hash = $hash; + } + } } diff --git a/lib/Doctrine/ORM/Cache/CollectionCacheKey.php b/lib/Doctrine/ORM/Cache/CollectionCacheKey.php index ec2c35db994..4475bfd3939 100644 --- a/lib/Doctrine/ORM/Cache/CollectionCacheKey.php +++ b/lib/Doctrine/ORM/Cache/CollectionCacheKey.php @@ -52,6 +52,7 @@ public function __construct($entityClass, $association, array $ownerIdentifier) $this->ownerIdentifier = $ownerIdentifier; $this->entityClass = (string) $entityClass; $this->association = (string) $association; - $this->hash = str_replace('\\', '.', strtolower($entityClass)) . '_' . implode(' ', $ownerIdentifier) . '__' . $association; + + parent::__construct(str_replace('\\', '.', strtolower($entityClass)) . '_' . implode(' ', $ownerIdentifier) . '__' . $association); } } diff --git a/lib/Doctrine/ORM/Cache/EntityCacheKey.php b/lib/Doctrine/ORM/Cache/EntityCacheKey.php index 730914d6979..3f22f4b3912 100644 --- a/lib/Doctrine/ORM/Cache/EntityCacheKey.php +++ b/lib/Doctrine/ORM/Cache/EntityCacheKey.php @@ -42,6 +42,7 @@ public function __construct($entityClass, array $identifier) $this->identifier = $identifier; $this->entityClass = $entityClass; - $this->hash = str_replace('\\', '.', strtolower($entityClass) . '_' . implode(' ', $identifier)); + + parent::__construct(str_replace('\\', '.', strtolower($entityClass) . '_' . implode(' ', $identifier))); } } diff --git a/lib/Doctrine/ORM/Cache/QueryCacheKey.php b/lib/Doctrine/ORM/Cache/QueryCacheKey.php index ba0bd1c037f..e7004f1fc13 100644 --- a/lib/Doctrine/ORM/Cache/QueryCacheKey.php +++ b/lib/Doctrine/ORM/Cache/QueryCacheKey.php @@ -41,9 +41,10 @@ public function __construct( int $cacheMode = Cache::MODE_NORMAL, ?TimestampCacheKey $timestampKey = null ) { - $this->hash = $cacheId; $this->lifetime = $lifetime; $this->cacheMode = $cacheMode; $this->timestampKey = $timestampKey; + + parent::__construct($cacheId); } } diff --git a/lib/Doctrine/ORM/Cache/TimestampCacheKey.php b/lib/Doctrine/ORM/Cache/TimestampCacheKey.php index 0cbfb674872..9dff0020cf9 100644 --- a/lib/Doctrine/ORM/Cache/TimestampCacheKey.php +++ b/lib/Doctrine/ORM/Cache/TimestampCacheKey.php @@ -12,6 +12,6 @@ class TimestampCacheKey extends CacheKey /** @param string $space Result cache id */ public function __construct($space) { - $this->hash = (string) $space; + parent::__construct((string) $space); } } diff --git a/psalm.xml b/psalm.xml index 5a78c1bd48e..420a9c97279 100644 --- a/psalm.xml +++ b/psalm.xml @@ -141,6 +141,12 @@ + + + + + + diff --git a/tests/Doctrine/Tests/Mocks/CacheKeyMock.php b/tests/Doctrine/Tests/Mocks/CacheKeyMock.php index aa4bf505dbc..017ae23602d 100644 --- a/tests/Doctrine/Tests/Mocks/CacheKeyMock.php +++ b/tests/Doctrine/Tests/Mocks/CacheKeyMock.php @@ -13,9 +13,9 @@ */ class CacheKeyMock extends CacheKey { - /** @param string $hash The string hash that represend this cache key */ + /** @param string $hash The string hash that represents this cache key */ public function __construct(string $hash) { - $this->hash = $hash; + parent::__construct($hash); } } diff --git a/tests/Doctrine/Tests/ORM/Cache/CacheKeyTest.php b/tests/Doctrine/Tests/ORM/Cache/CacheKeyTest.php index 4ed0cf68071..4dc6c23d0c7 100644 --- a/tests/Doctrine/Tests/ORM/Cache/CacheKeyTest.php +++ b/tests/Doctrine/Tests/ORM/Cache/CacheKeyTest.php @@ -4,6 +4,8 @@ namespace Doctrine\Tests\ORM\Cache; +use Doctrine\Deprecations\PHPUnit\VerifyDeprecations; +use Doctrine\ORM\Cache\CacheKey; use Doctrine\ORM\Cache\CollectionCacheKey; use Doctrine\ORM\Cache\EntityCacheKey; use Doctrine\Tests\DoctrineTestCase; @@ -11,6 +13,8 @@ /** @group DDC-2183 */ class CacheKeyTest extends DoctrineTestCase { + use VerifyDeprecations; + public function testEntityCacheKeyIdentifierCollision(): void { $key1 = new EntityCacheKey('Foo', ['id' => 1]); @@ -66,4 +70,28 @@ public function testCollectionCacheKeyAssociationCollision(): void self::assertNotEquals($key1->hash, $key2->hash); } + + public function testConstructor(): void + { + $key = new class ('my-hash') extends CacheKey { + }; + + self::assertSame('my-hash', $key->hash); + } + + public function testDeprecatedConstructor(): void + { + $this->expectDeprecationWithIdentifier('https://github.com/doctrine/orm/pull/10212'); + + $key = new class extends CacheKey { + public function __construct() + { + $this->hash = 'my-hash'; + + parent::__construct(); + } + }; + + self::assertSame('my-hash', $key->hash); + } }