Skip to content

Commit

Permalink
Make serialized SQL executors forward compatible
Browse files Browse the repository at this point in the history
  • Loading branch information
greg0ire committed Oct 26, 2023
1 parent 16028e4 commit 70c7b9c
Show file tree
Hide file tree
Showing 8 changed files with 59 additions and 16 deletions.
4 changes: 4 additions & 0 deletions UPGRADE.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
37 changes: 30 additions & 7 deletions lib/Doctrine/ORM/Query/Exec/AbstractSqlExecutor.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@
use Doctrine\DBAL\Result;
use Doctrine\DBAL\Types\Type;

use function array_diff_key;
use function array_keys;

/**
* Base class for SQL statement executors.
*
Expand All @@ -18,34 +21,43 @@
*/
abstract class AbstractSqlExecutor
{
/** @var list<string>|string */
/**
* @deprecated use $sqlStatements instead
*
* @var list<string>|string
*/
protected $_sqlStatements;

Check failure on line 29 in lib/Doctrine/ORM/Query/Exec/AbstractSqlExecutor.php

View workflow job for this annotation

GitHub Actions / Static Analysis with Psalm

PropertyNotSetInConstructor

lib/Doctrine/ORM/Query/Exec/AbstractSqlExecutor.php:29:15: PropertyNotSetInConstructor: Property Doctrine\ORM\Query\Exec\AbstractSqlExecutor::$_sqlStatements is not defined in constructor of Doctrine\ORM\Query\Exec\AbstractSqlExecutor or in any methods called in the constructor (see https://psalm.dev/074)

/** @var list<string>|string */
protected $sqlStatements;

Check failure on line 32 in lib/Doctrine/ORM/Query/Exec/AbstractSqlExecutor.php

View workflow job for this annotation

GitHub Actions / Static Analysis with Psalm

PropertyNotSetInConstructor

lib/Doctrine/ORM/Query/Exec/AbstractSqlExecutor.php:32:15: PropertyNotSetInConstructor: Property Doctrine\ORM\Query\Exec\AbstractSqlExecutor::$sqlStatements is not defined in constructor of Doctrine\ORM\Query\Exec\AbstractSqlExecutor or in any methods called in the constructor (see https://psalm.dev/074)

/** @var QueryCacheProfile */
protected $queryCacheProfile;

Check failure on line 35 in lib/Doctrine/ORM/Query/Exec/AbstractSqlExecutor.php

View workflow job for this annotation

GitHub Actions / Static Analysis with Psalm

PropertyNotSetInConstructor

lib/Doctrine/ORM/Query/Exec/AbstractSqlExecutor.php:35:15: PropertyNotSetInConstructor: Property Doctrine\ORM\Query\Exec\AbstractSqlExecutor::$queryCacheProfile is not defined in constructor of Doctrine\ORM\Query\Exec\AbstractSqlExecutor or in any methods called in the constructor (see https://psalm.dev/074)

public function __construct()
{
$this->_sqlStatements = &$this->sqlStatements;

Check failure on line 39 in lib/Doctrine/ORM/Query/Exec/AbstractSqlExecutor.php

View workflow job for this annotation

GitHub Actions / Static Analysis with Psalm

UnsupportedPropertyReferenceUsage

lib/Doctrine/ORM/Query/Exec/AbstractSqlExecutor.php:39:9: UnsupportedPropertyReferenceUsage: This reference cannot be analyzed by Psalm. (see https://psalm.dev/321)

Check failure on line 39 in lib/Doctrine/ORM/Query/Exec/AbstractSqlExecutor.php

View workflow job for this annotation

GitHub Actions / Static Analysis with Psalm

UninitializedProperty

lib/Doctrine/ORM/Query/Exec/AbstractSqlExecutor.php:39:34: UninitializedProperty: Cannot use uninitialized property $this->sqlStatements (see https://psalm.dev/186)
}

/**
* Gets the SQL statements that are executed by the executor.
*
* @return mixed[]|string All the SQL update statements.
*/
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;
}
Expand All @@ -60,4 +72,15 @@ public function removeQueryCacheProfile()
* @return Result|int
*/
abstract public function execute(Connection $conn, array $params, array $types);

/** @return list<string> */

Check failure on line 76 in lib/Doctrine/ORM/Query/Exec/AbstractSqlExecutor.php

View workflow job for this annotation

GitHub Actions / Static Analysis with Psalm

MoreSpecificReturnType

lib/Doctrine/ORM/Query/Exec/AbstractSqlExecutor.php:76:17: MoreSpecificReturnType: The declared return type 'list<string>' for Doctrine\ORM\Query\Exec\AbstractSqlExecutor::__sleep is more specific than the inferred return type 'array<int<0, max>, array-key>' (see https://psalm.dev/070)
public function __sleep(): array
{
return array_diff_key(array_keys((array) $this), ['*_sqlStatements']);

Check failure on line 79 in lib/Doctrine/ORM/Query/Exec/AbstractSqlExecutor.php

View workflow job for this annotation

GitHub Actions / Static Analysis with Psalm

LessSpecificReturnStatement

lib/Doctrine/ORM/Query/Exec/AbstractSqlExecutor.php:79:16: LessSpecificReturnStatement: The type 'array<int<0, max>, array-key>' is more general than the declared return type 'list<string>' for Doctrine\ORM\Query\Exec\AbstractSqlExecutor::__sleep (see https://psalm.dev/129)
}

public function __wakeup(): void
{
$this->_sqlStatements = &$this->sqlStatements;

Check failure on line 84 in lib/Doctrine/ORM/Query/Exec/AbstractSqlExecutor.php

View workflow job for this annotation

GitHub Actions / Static Analysis with Psalm

UnsupportedPropertyReferenceUsage

lib/Doctrine/ORM/Query/Exec/AbstractSqlExecutor.php:84:9: UnsupportedPropertyReferenceUsage: This reference cannot be analyzed by Psalm. (see https://psalm.dev/321)
}
}
6 changes: 4 additions & 2 deletions lib/Doctrine/ORM/Query/Exec/MultiTableDeleteExecutor.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -84,7 +86,7 @@ public function __construct(AST\Node $AST, $sqlWalker)
$classNames = array_merge($primaryClass->parentClasses, [$primaryClass->name], $primaryClass->subClasses);
foreach (array_reverse($classNames) as $className) {
$tableName = $quoteStrategy->getTableName($em->getClassMetadata($className), $platform);

Check failure on line 88 in lib/Doctrine/ORM/Query/Exec/MultiTableDeleteExecutor.php

View workflow job for this annotation

GitHub Actions / coding-standards / Coding Standards (8.2)

Equals sign not aligned with surrounding assignments; expected 13 spaces but found 14 spaces
$this->_sqlStatements[] = 'DELETE FROM ' . $tableName
$this->sqlStatements[] = 'DELETE FROM ' . $tableName
. ' WHERE (' . $idColumnList . ') IN (' . $idSubselect . ')';
}

Expand Down Expand Up @@ -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) {

Check failure on line 122 in lib/Doctrine/ORM/Query/Exec/MultiTableDeleteExecutor.php

View workflow job for this annotation

GitHub Actions / Static Analysis with Psalm

PossiblyInvalidIterator

lib/Doctrine/ORM/Query/Exec/MultiTableDeleteExecutor.php:122:22: PossiblyInvalidIterator: Cannot iterate over string (see https://psalm.dev/165)
$conn->executeStatement($sql);
}
} catch (Throwable $exception) {
Expand Down
6 changes: 4 additions & 2 deletions lib/Doctrine/ORM/Query/Exec/MultiTableUpdateExecutor.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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 . ')';
}
}

Expand Down Expand Up @@ -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 = [];

Expand Down
6 changes: 4 additions & 2 deletions lib/Doctrine/ORM/Query/Exec/SingleSelectExecutor.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand All @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand All @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use Doctrine\Tests\OrmFunctionalTestCase;
use Generator;
use ReflectionMethod;
use ReflectionProperty;

use function file_get_contents;
use function rtrim;
Expand All @@ -33,12 +34,19 @@ public function testSerializeParserResult(): void

$parserResult = self::parseQuery($query);
$serialized = serialize($parserResult);
$this->assertSame(rtrim(file_get_contents(__DIR__ . '/ParserResults/single_select_3_0_0.txt'), "\n"), $serialized);
$unserialized = unserialize($serialized);

$this->assertInstanceOf(ParserResult::class, $unserialized);
$this->assertInstanceOf(ResultSetMapping::class, $unserialized->getResultSetMapping());
$this->assertEquals(['name' => [0]], $unserialized->getParameterMappings());
$this->assertInstanceOf(SingleSelectExecutor::class, $unserialized->getSqlExecutor());
$this->assertIsString($unserialized->getSqlExecutor()->getSqlStatements());

$r = new ReflectionProperty($unserialized->getSqlExecutor(), '_sqlStatements');
$r->setAccessible(true);

$this->assertIsString($r->getValue($unserialized->getSqlExecutor()));
}

/**
Expand Down
Binary file not shown.

0 comments on commit 70c7b9c

Please sign in to comment.