From 2a9390d426b73908610a90b9f773b7d73859e1f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Paris?= Date: Thu, 26 Oct 2023 00:01:12 +0200 Subject: [PATCH] Make serialized SQL executors forward compatible The idea here is that instead of having a backward compatibility layer in the next major branch, we can have a forward compatibility layer in this branch. --- UPGRADE.md | 4 ++ .../ORM/Query/Exec/AbstractSqlExecutor.php | 45 +++++++++++++++--- .../Query/Exec/MultiTableDeleteExecutor.php | 8 ++-- .../Query/Exec/MultiTableUpdateExecutor.php | 6 ++- .../ORM/Query/Exec/SingleSelectExecutor.php | 6 ++- .../Exec/SingleTableDeleteUpdateExecutor.php | 8 ++-- psalm-baseline.xml | 33 ++++++++----- .../ParserResultSerializationTest.php | 26 ++++++++++ .../ParserResults/single_select_2_17_0.txt | Bin 0 -> 2369 bytes 9 files changed, 107 insertions(+), 29 deletions(-) create mode 100644 tests/Doctrine/Tests/ORM/Functional/ParserResults/single_select_2_17_0.txt diff --git a/UPGRADE.md b/UPGRADE.md index 1db52439499..4b8680a2bf6 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -1,5 +1,9 @@ # Upgrade to 2.17 +## Deprecate `Doctrine\ORM\Query\Exec\AbstractSqlExecutor::_sqlStatements` + +Use `Doctrine\ORM\Query\Exec\AbstractSqlExecutor::sqlStatements` instead. + ## Undeprecate `Doctrine\ORM\Proxy\Autoloader` It will be a full-fledged class, no longer extending diff --git a/lib/Doctrine/ORM/Query/Exec/AbstractSqlExecutor.php b/lib/Doctrine/ORM/Query/Exec/AbstractSqlExecutor.php index fc9660d1bb6..2374ad795e0 100644 --- a/lib/Doctrine/ORM/Query/Exec/AbstractSqlExecutor.php +++ b/lib/Doctrine/ORM/Query/Exec/AbstractSqlExecutor.php @@ -9,6 +9,10 @@ use Doctrine\DBAL\Result; use Doctrine\DBAL\Types\Type; +use function array_diff; +use function array_keys; +use function array_values; + /** * Base class for SQL statement executors. * @@ -18,12 +22,24 @@ */ abstract class AbstractSqlExecutor { - /** @var list|string */ + /** + * @deprecated use $sqlStatements instead + * + * @var list|string + */ protected $_sqlStatements; + /** @var list|string */ + protected $sqlStatements; + /** @var QueryCacheProfile */ protected $queryCacheProfile; + public function __construct() + { + $this->_sqlStatements = &$this->sqlStatements; + } + /** * Gets the SQL statements that are executed by the executor. * @@ -31,21 +47,18 @@ abstract class AbstractSqlExecutor */ public function getSqlStatements() { - return $this->_sqlStatements; + return $this->sqlStatements; } - /** @return void */ - public function setQueryCacheProfile(QueryCacheProfile $qcp) + public function setQueryCacheProfile(QueryCacheProfile $qcp): void { $this->queryCacheProfile = $qcp; } /** * Do not use query cache - * - * @return void */ - public function removeQueryCacheProfile() + public function removeQueryCacheProfile(): void { $this->queryCacheProfile = null; } @@ -60,4 +73,22 @@ public function removeQueryCacheProfile() * @return Result|int */ abstract public function execute(Connection $conn, array $params, array $types); + + /** @return list */ + public function __sleep(): array + { + /* Two reasons for this: + - we do not need to serialize the deprecated property, we can + rebuild the reference to the new property in __wakeup() + - not having the legacy property in the serialized data means the + serialized representation becomes compatible with 3.0.x, meaning + there will not be a deprecation warning about a missing property + when unserializing data */ + return array_values(array_diff(array_keys((array) $this), ["\0*\0_sqlStatements"])); + } + + public function __wakeup(): void + { + $this->_sqlStatements = &$this->sqlStatements; + } } diff --git a/lib/Doctrine/ORM/Query/Exec/MultiTableDeleteExecutor.php b/lib/Doctrine/ORM/Query/Exec/MultiTableDeleteExecutor.php index 79be7a09df5..bea246e2f5b 100644 --- a/lib/Doctrine/ORM/Query/Exec/MultiTableDeleteExecutor.php +++ b/lib/Doctrine/ORM/Query/Exec/MultiTableDeleteExecutor.php @@ -45,6 +45,8 @@ class MultiTableDeleteExecutor extends AbstractSqlExecutor */ public function __construct(AST\Node $AST, $sqlWalker) { + parent::__construct(); + $em = $sqlWalker->getEntityManager(); $conn = $em->getConnection(); $platform = $conn->getDatabasePlatform(); @@ -83,8 +85,8 @@ public function __construct(AST\Node $AST, $sqlWalker) // 3. Create and store DELETE statements $classNames = array_merge($primaryClass->parentClasses, [$primaryClass->name], $primaryClass->subClasses); foreach (array_reverse($classNames) as $className) { - $tableName = $quoteStrategy->getTableName($em->getClassMetadata($className), $platform); - $this->_sqlStatements[] = 'DELETE FROM ' . $tableName + $tableName = $quoteStrategy->getTableName($em->getClassMetadata($className), $platform); + $this->sqlStatements[] = 'DELETE FROM ' . $tableName . ' WHERE (' . $idColumnList . ') IN (' . $idSubselect . ')'; } @@ -117,7 +119,7 @@ public function execute(Connection $conn, array $params, array $types) $numDeleted = $conn->executeStatement($this->insertSql, $params, $types); // Execute DELETE statements - foreach ($this->_sqlStatements as $sql) { + foreach ($this->sqlStatements as $sql) { $conn->executeStatement($sql); } } catch (Throwable $exception) { diff --git a/lib/Doctrine/ORM/Query/Exec/MultiTableUpdateExecutor.php b/lib/Doctrine/ORM/Query/Exec/MultiTableUpdateExecutor.php index 12d690aab09..f1e491bd258 100644 --- a/lib/Doctrine/ORM/Query/Exec/MultiTableUpdateExecutor.php +++ b/lib/Doctrine/ORM/Query/Exec/MultiTableUpdateExecutor.php @@ -50,6 +50,8 @@ class MultiTableUpdateExecutor extends AbstractSqlExecutor */ public function __construct(AST\Node $AST, $sqlWalker) { + parent::__construct(); + $em = $sqlWalker->getEntityManager(); $conn = $em->getConnection(); $platform = $conn->getDatabasePlatform(); @@ -119,7 +121,7 @@ public function __construct(AST\Node $AST, $sqlWalker) } if ($affected) { - $this->_sqlStatements[$i] = $updateSql . ' WHERE (' . $idColumnList . ') IN (' . $idSubselect . ')'; + $this->sqlStatements[$i] = $updateSql . ' WHERE (' . $idColumnList . ') IN (' . $idSubselect . ')'; } } @@ -163,7 +165,7 @@ public function execute(Connection $conn, array $params, array $types) ); // Execute UPDATE statements - foreach ($this->_sqlStatements as $key => $statement) { + foreach ($this->sqlStatements as $key => $statement) { $paramValues = []; $paramTypes = []; diff --git a/lib/Doctrine/ORM/Query/Exec/SingleSelectExecutor.php b/lib/Doctrine/ORM/Query/Exec/SingleSelectExecutor.php index 11700a3f4f8..fbcab590e09 100644 --- a/lib/Doctrine/ORM/Query/Exec/SingleSelectExecutor.php +++ b/lib/Doctrine/ORM/Query/Exec/SingleSelectExecutor.php @@ -18,7 +18,9 @@ class SingleSelectExecutor extends AbstractSqlExecutor { public function __construct(SelectStatement $AST, SqlWalker $sqlWalker) { - $this->_sqlStatements = $sqlWalker->walkSelectStatement($AST); + parent::__construct(); + + $this->sqlStatements = $sqlWalker->walkSelectStatement($AST); } /** @@ -28,6 +30,6 @@ public function __construct(SelectStatement $AST, SqlWalker $sqlWalker) */ public function execute(Connection $conn, array $params, array $types) { - return $conn->executeQuery($this->_sqlStatements, $params, $types, $this->queryCacheProfile); + return $conn->executeQuery($this->sqlStatements, $params, $types, $this->queryCacheProfile); } } diff --git a/lib/Doctrine/ORM/Query/Exec/SingleTableDeleteUpdateExecutor.php b/lib/Doctrine/ORM/Query/Exec/SingleTableDeleteUpdateExecutor.php index bf67d86d30b..7f7496e397a 100644 --- a/lib/Doctrine/ORM/Query/Exec/SingleTableDeleteUpdateExecutor.php +++ b/lib/Doctrine/ORM/Query/Exec/SingleTableDeleteUpdateExecutor.php @@ -22,10 +22,12 @@ class SingleTableDeleteUpdateExecutor extends AbstractSqlExecutor /** @param SqlWalker $sqlWalker */ public function __construct(AST\Node $AST, $sqlWalker) { + parent::__construct(); + if ($AST instanceof AST\UpdateStatement) { - $this->_sqlStatements = $sqlWalker->walkUpdateStatement($AST); + $this->sqlStatements = $sqlWalker->walkUpdateStatement($AST); } elseif ($AST instanceof AST\DeleteStatement) { - $this->_sqlStatements = $sqlWalker->walkDeleteStatement($AST); + $this->sqlStatements = $sqlWalker->walkDeleteStatement($AST); } } @@ -40,6 +42,6 @@ public function execute(Connection $conn, array $params, array $types) $conn->ensureConnectedToPrimary(); } - return $conn->executeStatement($this->_sqlStatements, $params, $types); + return $conn->executeStatement($this->sqlStatements, $params, $types); } } diff --git a/psalm-baseline.xml b/psalm-baseline.xml index bcb47d1a2be..809e541b90f 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -1847,6 +1847,18 @@ null + + $_sqlStatements + $queryCacheProfile + $sqlStatements + + + sqlStatements]]> + + + _sqlStatements = &$this->sqlStatements]]> + _sqlStatements = &$this->sqlStatements]]> + @@ -1856,15 +1868,12 @@ int - _sqlStatements]]> + sqlStatements]]> MultiTableDeleteExecutor MultiTableDeleteExecutor - - _sqlStatements]]> - @@ -1874,40 +1883,40 @@ int - _sqlStatements]]> + sqlStatements]]> MultiTableUpdateExecutor MultiTableUpdateExecutor + MultiTableUpdateExecutor - _sqlStatements]]> + sqlStatements]]> - - _sqlStatements]]> - - _sqlStatements]]> + sqlStatements]]> SingleSelectExecutor + SingleSelectExecutor - executeStatement($this->_sqlStatements, $params, $types)]]> + executeStatement($this->sqlStatements, $params, $types)]]> int - _sqlStatements]]> + sqlStatements]]> SingleTableDeleteUpdateExecutor SingleTableDeleteUpdateExecutor + SingleTableDeleteUpdateExecutor diff --git a/tests/Doctrine/Tests/ORM/Functional/ParserResultSerializationTest.php b/tests/Doctrine/Tests/ORM/Functional/ParserResultSerializationTest.php index a9d73fc7c98..5d69c4c9209 100644 --- a/tests/Doctrine/Tests/ORM/Functional/ParserResultSerializationTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/ParserResultSerializationTest.php @@ -11,6 +11,7 @@ use Doctrine\Tests\OrmFunctionalTestCase; use Generator; use ReflectionMethod; +use ReflectionProperty; use function file_get_contents; use function rtrim; @@ -41,6 +42,30 @@ public function testSerializeParserResult(): void $this->assertInstanceOf(SingleSelectExecutor::class, $unserialized->getSqlExecutor()); } + public function testItSerializesParserResultWithAForwardCompatibleFormat(): void + { + $query = $this->_em + ->createQuery('SELECT u FROM Doctrine\Tests\Models\Company\CompanyEmployee u WHERE u.name = :name'); + + $parserResult = self::parseQuery($query); + $serialized = serialize($parserResult); + $this->assertStringNotContainsString( + '_sqlStatements', + $serialized, + 'ParserResult should not contain any reference to _sqlStatements, which is a legacy property.' + ); + $unserialized = unserialize($serialized); + + $r = new ReflectionProperty($unserialized->getSqlExecutor(), '_sqlStatements'); + $r->setAccessible(true); + + $this->assertSame( + $r->getValue($unserialized->getSqlExecutor()), + $unserialized->getSqlExecutor()->getSqlStatements(), + 'The legacy property should be populated with the same value as the new one.' + ); + } + /** * @dataProvider provideSerializedSingleSelectResults */ @@ -59,6 +84,7 @@ public static function provideSerializedSingleSelectResults(): Generator { yield '2.14.3' => [rtrim(file_get_contents(__DIR__ . '/ParserResults/single_select_2_14_3.txt'), "\n")]; yield '2.15.0' => [rtrim(file_get_contents(__DIR__ . '/ParserResults/single_select_2_15_0.txt'), "\n")]; + yield '2.17.0' => [rtrim(file_get_contents(__DIR__ . '/ParserResults/single_select_2_17_0.txt'), "\n")]; } private static function parseQuery(Query $query): ParserResult diff --git a/tests/Doctrine/Tests/ORM/Functional/ParserResults/single_select_2_17_0.txt b/tests/Doctrine/Tests/ORM/Functional/ParserResults/single_select_2_17_0.txt new file mode 100644 index 0000000000000000000000000000000000000000..aa854bfab92d1b06105aeefae72af587f45e042d GIT binary patch literal 2369 zcmbW3TW{Jh6vz9#PZ8yDnkoYHGNWngDAhU**r3v0kSsSj?L6f|W1|*T{qFl8I|-r7 zZhb)5=h!~C-?1|ngGhwuMQlr*(`vq$uD))l+^)W;(one|vq^0z2IA3(;ZTGJe_489 z?$XHv#T#3c;b`vmbZp@ztEJ9wQ(96=v2FHwzSGpM!7ZN$cVuSyj@N{ zPsUe4+>?ho3C@;*PGs+mo98McmhiufAdBRoQK>4oEcGMV_wt0+svl6asPPvL7yiVSNj_> zm8)B9;z0IyATVtG_HnY9?DF|8c+a`-_gqcx7>}`vztfkpxX~$vquJ;QbiDlsI!edW zlI&Ei*C;2M4waBlv_FV0`x)*YT9 zEbnNO!MI2_Sw6qdiPOj+IGmB&8Uz%{0lLRQEqn$+E$*y;bq*uj)L0H(AeCyr+Vx@k zm8qrfm^xH>(mv){Is6V3g0BjfJe}!W;VBrq;2r+0ayaT57Px@er}Km!K5zv&M-^pA zNkTCQ1SsQF8AF||^n%uY-gf_gIsDR~FY=o7||0vI?Igc+#!w9sh9DZ1i!*?y=S^ pb%^j$AnS-9uO@)*&zu8X<9kMSyuuT{n%Q;mtMTmc@9F93)lWDO6tVyS literal 0 HcmV?d00001