Make serialized SQL executors forward compatible

doctrine · Oct 26, 2023 · f72e503 · f72e503
1 parent 16028e4
commit f72e503
Show file tree

Hide file tree

Showing 9 changed files with 82 additions and 29 deletions.
diff --git 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
@@ -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,34 +22,43 @@
  */
 abstract class AbstractSqlExecutor
 {
-    /** @var list<string>|string */
+    /**
+     * @deprecated use $sqlStatements instead
+     *
+     * @var list<string>|string
+     */
     protected $_sqlStatements;
 
+    /** @var list<string>|string */
+    protected $sqlStatements;
+
     /** @var QueryCacheProfile */
     protected $queryCacheProfile;
 
+    public function __construct()
+    {
+        $this->_sqlStatements = &$this->sqlStatements;
+    }
+
     /**
      * 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;
     }
@@ -60,4 +73,15 @@ public function removeQueryCacheProfile()
      * @return Result|int
      */
     abstract public function execute(Connection $conn, array $params, array $types);
+
+    /** @return list<string> */
+    public function __sleep(): array
+    {
+        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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -1847,6 +1847,18 @@
     <PossiblyNullPropertyAssignmentValue>
       <code>null</code>
     </PossiblyNullPropertyAssignmentValue>
+    <PropertyNotSetInConstructor>
+      <code>$_sqlStatements</code>
+      <code>$queryCacheProfile</code>
+      <code>$sqlStatements</code>
+    </PropertyNotSetInConstructor>
+    <UninitializedProperty>
+      <code><![CDATA[$this->sqlStatements]]></code>
+    </UninitializedProperty>
+    <UnsupportedPropertyReferenceUsage>
+      <code><![CDATA[$this->_sqlStatements = &$this->sqlStatements]]></code>
+      <code><![CDATA[$this->_sqlStatements = &$this->sqlStatements]]></code>
+    </UnsupportedPropertyReferenceUsage>
   </file>
   <file src="lib/Doctrine/ORM/Query/Exec/MultiTableDeleteExecutor.php">
     <InvalidReturnStatement>
@@ -1856,15 +1868,12 @@
       <code>int</code>
     </InvalidReturnType>
     <PossiblyInvalidIterator>
-      <code><![CDATA[$this->_sqlStatements]]></code>
+      <code><![CDATA[$this->sqlStatements]]></code>
     </PossiblyInvalidIterator>
     <PropertyNotSetInConstructor>
       <code>MultiTableDeleteExecutor</code>
       <code>MultiTableDeleteExecutor</code>
     </PropertyNotSetInConstructor>
-    <UninitializedProperty>
-      <code><![CDATA[$this->_sqlStatements]]></code>
-    </UninitializedProperty>
   </file>
   <file src="lib/Doctrine/ORM/Query/Exec/MultiTableUpdateExecutor.php">
     <InvalidReturnStatement>
@@ -1874,40 +1883,40 @@
       <code>int</code>
     </InvalidReturnType>
     <PossiblyInvalidIterator>
-      <code><![CDATA[$this->_sqlStatements]]></code>
+      <code><![CDATA[$this->sqlStatements]]></code>
     </PossiblyInvalidIterator>
     <PropertyNotSetInConstructor>
       <code>MultiTableUpdateExecutor</code>
       <code>MultiTableUpdateExecutor</code>
+      <code>MultiTableUpdateExecutor</code>
     </PropertyNotSetInConstructor>
     <PropertyTypeCoercion>
-      <code><![CDATA[$this->_sqlStatements]]></code>
+      <code><![CDATA[$this->sqlStatements]]></code>
     </PropertyTypeCoercion>
-    <UninitializedProperty>
-      <code><![CDATA[$this->_sqlStatements]]></code>
-    </UninitializedProperty>
   </file>
   <file src="lib/Doctrine/ORM/Query/Exec/SingleSelectExecutor.php">
     <PossiblyInvalidArgument>
-      <code><![CDATA[$this->_sqlStatements]]></code>
+      <code><![CDATA[$this->sqlStatements]]></code>
     </PossiblyInvalidArgument>
     <PropertyNotSetInConstructor>
       <code>SingleSelectExecutor</code>
+      <code>SingleSelectExecutor</code>
     </PropertyNotSetInConstructor>
   </file>
   <file src="lib/Doctrine/ORM/Query/Exec/SingleTableDeleteUpdateExecutor.php">
     <InvalidReturnStatement>
-      <code><![CDATA[$conn->executeStatement($this->_sqlStatements, $params, $types)]]></code>
+      <code><![CDATA[$conn->executeStatement($this->sqlStatements, $params, $types)]]></code>
     </InvalidReturnStatement>
     <InvalidReturnType>
       <code>int</code>
     </InvalidReturnType>
     <PossiblyInvalidArgument>
-      <code><![CDATA[$this->_sqlStatements]]></code>
+      <code><![CDATA[$this->sqlStatements]]></code>
     </PossiblyInvalidArgument>
     <PropertyNotSetInConstructor>
       <code>SingleTableDeleteUpdateExecutor</code>
       <code>SingleTableDeleteUpdateExecutor</code>
+      <code>SingleTableDeleteUpdateExecutor</code>
     </PropertyNotSetInConstructor>
   </file>
   <file src="lib/Doctrine/ORM/Query/Expr.php">

diff --git 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;
@@ -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()));
     }
 
     /**

diff --git a/tests/Doctrine/Tests/ORM/Functional/ParserResults/single_select_3_0_0.txt b/tests/Doctrine/Tests/ORM/Functional/ParserResults/single_select_3_0_0.txt