Merge branch 'ACP2E-3230' of https://github.com/adobe-commerce-tier-4/magento2ce into PR-10-01-2024

chittima · chittima · commit 7dbcb265b637 · 2024-10-02T11:15:14.000-05:00
diff --git a/lib/internal/Magento/Framework/Setup/Declaration/Schema/Db/MySQL/DbSchemaWriter.php b/lib/internal/Magento/Framework/Setup/Declaration/Schema/Db/MySQL/DbSchemaWriter.php
@@ -7,6 +7,9 @@
 namespace Magento\Framework\Setup\Declaration\Schema\Db\MySQL;
 
 use Magento\Framework\App\ResourceConnection;
+use Magento\Framework\DB\Adapter\ConnectionException;
+use Magento\Framework\DB\Adapter\SqlVersionProvider;
+use Magento\Framework\DB\Adapter\AdapterInterface;
 use Magento\Framework\Setup\Declaration\Schema\Db\DbSchemaWriterInterface;
 use Magento\Framework\Setup\Declaration\Schema\Db\Statement;
 use Magento\Framework\Setup\Declaration\Schema\Db\StatementAggregator;
@@ -59,22 +62,30 @@ class DbSchemaWriter implements DbSchemaWriterInterface
      */
     private $dryRunLogger;
 
+    /**
+     * @var SqlVersionProvider
+     */
+    private $sqlVersionProvider;
+
     /**
      * @param ResourceConnection $resourceConnection
-     * @param StatementFactory   $statementFactory
-     * @param DryRunLogger       $dryRunLogger
-     * @param array              $tableOptions
+     * @param StatementFactory $statementFactory
+     * @param DryRunLogger $dryRunLogger
+     * @param SqlVersionProvider $sqlVersionProvider
+     * @param array $tableOptions
      */
     public function __construct(
         ResourceConnection $resourceConnection,
         StatementFactory $statementFactory,
         DryRunLogger $dryRunLogger,
+        SqlVersionProvider $sqlVersionProvider,
         array $tableOptions = []
     ) {
         $this->resourceConnection = $resourceConnection;
         $this->statementFactory = $statementFactory;
         $this->dryRunLogger = $dryRunLogger;
         $this->tableOptions = array_replace($this->tableOptions, $tableOptions);
+        $this->sqlVersionProvider = $sqlVersionProvider;
     }
 
     /**
@@ -271,15 +282,14 @@ public function compile(StatementAggregator $statementAggregator, $dryRun)
             $statementsSql = [];
             $statement = null;
 
-            /**
-             * @var Statement $statement
-             */
-            foreach ($statementBank as $statement) {
-                $statementsSql[] = $statement->getStatement();
-            }
-            $adapter = $this->resourceConnection->getConnection($statement->getResource());
-
             if ($dryRun) {
+                /**
+                 * @var Statement $statement
+                 */
+                foreach ($statementBank as $statement) {
+                    $statementsSql[] = $statement->getStatement();
+                }
+                $adapter = $this->resourceConnection->getConnection($statement->getResource());
                 $this->dryRunLogger->log(
                     sprintf(
                         $this->statementDirectives[$statement->getType()],
@@ -288,18 +298,81 @@ public function compile(StatementAggregator $statementAggregator, $dryRun)
                     )
                 );
             } else {
+                $this->doQuery($statementBank);
+                $statement = end($statementBank);
+                //Do post update, like SQL DML operations or etc...
+                foreach ($statement->getTriggers() as $trigger) {
+                    call_user_func($trigger);
+                }
+            }
+        }
+    }
+
+    /**
+     * Check if we can concatenate sql into one statement
+     *
+     * Due to issues with some versions of MariaBD such statements
+     * may produce errors, e.g. with foreign key definition with column modification
+     *
+     * @return bool
+     * @throws ConnectionException
+     */
+    private function isNeedToSplitSql() : bool
+    {
+        return str_contains($this->sqlVersionProvider->getSqlVersion(), SqlVersionProvider::MARIA_DB_10_4_VERSION) ||
+            str_contains($this->sqlVersionProvider->getSqlVersion(), SqlVersionProvider::MARIA_DB_10_6_VERSION);
+    }
+
+    /**
+     * Perform queries based on statements
+     *
+     * @param Statement[] $statementBank
+     * @return void
+     * @throws ConnectionException
+     */
+    private function doQuery(
+        array $statementBank
+    ) : void {
+        if (empty($statementBank)) {
+            return;
+        }
+
+        $statement = null;
+        $statementsSql = [];
+        foreach ($statementBank as $statement) {
+            $statementsSql[] = $statement->getStatement();
+        }
+        $adapter = $this->resourceConnection->getConnection($statement->getResource());
+
+        if ($this->isNeedToSplitSql()) {
+            $preparedStatements = $this->getPreparedStatements($statementBank);
+
+            if (!empty($preparedStatements['canBeCombinedStatements'])) {
                 $adapter->query(
                     sprintf(
                         $this->statementDirectives[$statement->getType()],
                         $adapter->quoteIdentifier($statement->getTableName()),
-                        implode(", ", $statementsSql)
+                        implode(", ", $preparedStatements['canBeCombinedStatements'])
+                    )
+                );
+            }
+            foreach ($preparedStatements['separatedStatements'] as $separatedStatement) {
+                $adapter->query(
+                    sprintf(
+                        $this->statementDirectives[$statement->getType()],
+                        $adapter->quoteIdentifier($statement->getTableName()),
+                        $separatedStatement
                     )
                 );
-                //Do post update, like SQL DML operations or etc...
-                foreach ($statement->getTriggers() as $trigger) {
-                    call_user_func($trigger);
-                }
             }
+        } else {
+            $adapter->query(
+                sprintf(
+                    $this->statementDirectives[$statement->getType()],
+                    $adapter->quoteIdentifier($statement->getTableName()),
+                    implode(", ", $statementsSql)
+                )
+            );
         }
     }
 
@@ -309,6 +382,7 @@ public function compile(StatementAggregator $statementAggregator, $dryRun)
      * @param string $tableName
      * @param string $resource
      * @return int
+     * @throws \Zend_Db_Statement_Exception
      */
     private function getNextAutoIncrementValue(string $tableName, string $resource): int
     {
@@ -323,6 +397,59 @@ private function getNextAutoIncrementValue(string $tableName, string $resource):
         } else {
             return 1;
         }
+    }
 
+    /**
+     * Prepare list of modified columns from statement
+     *
+     * @param array $statementBank
+     * @return array
+     */
+    private function getModifiedColumns(array $statementBank) : array
+    {
+        $columns = [];
+        foreach ($statementBank as $statement) {
+            if ($statement->getType() === 'alter'
+                && str_contains($statement->getStatement(), 'MODIFY COLUMN')) {
+                $columns[] = $statement->getName();
+            }
+        }
+        return $columns;
+    }
+
+    /**
+     * Separate statements that can't be executed as one statement
+     *
+     * @param array $statementBank
+     * @return array
+     */
+    private function getPreparedStatements(array $statementBank) : array
+    {
+        $statementsSql = [];
+        foreach ($statementBank as $statement) {
+            $statementsSql[] = $statement->getStatement();
+        }
+        $result = ['separatedStatements' => [], 'canBeCombinedStatements' => []];
+        $modifiedColumns = $this->getModifiedColumns($statementBank);
+
+        foreach ($statementsSql as $statementSql) {
+            if (str_contains($statementSql, 'FOREIGN KEY')) {
+                $isThisColumnModified = false;
+                foreach ($modifiedColumns as $modifiedColumn) {
+                    if (str_contains($statementSql, '`' . $modifiedColumn . '`')) {
+                        $isThisColumnModified = true;
+                        break;
+                    }
+                }
+                if ($isThisColumnModified) {
+                    $result['separatedStatements'][] = $statementSql;
+                } else {
+                    $result['canBeCombinedStatements'][] = $statementSql;
+                }
+            } else {
+                $result['canBeCombinedStatements'][] = $statementSql;
+            }
+        }
+        return $result;
     }
 }
diff --git a/lib/internal/Magento/Framework/Setup/Test/Unit/Declaration/Schema/Db/DbSchemaWriterTest.php b/lib/internal/Magento/Framework/Setup/Test/Unit/Declaration/Schema/Db/DbSchemaWriterTest.php
@@ -0,0 +1,153 @@
+<?php
+/**
+ * Copyright 2024 Adobe
+ * All Rights Reserved.
+ */
+declare(strict_types=1);
+
+namespace Magento\Framework\Setup\Test\Unit\Declaration\Schema\Db;
+
+use Magento\Framework\App\ResourceConnection;
+use Magento\Framework\DB\Adapter\SqlVersionProvider;
+use Magento\Framework\DB\Adapter\AdapterInterface;
+use Magento\Framework\Setup\Declaration\Schema\Db\Statement;
+use Magento\Framework\Setup\Declaration\Schema\Db\StatementAggregator;
+use Magento\Framework\Setup\Declaration\Schema\Db\StatementFactory;
+use Magento\Framework\Setup\Declaration\Schema\DryRunLogger;
+use Magento\Framework\Setup\Declaration\Schema\Db\MySQL\DbSchemaWriter;
+use PHPUnit\Framework\MockObject\MockObject;
+use PHPUnit\Framework\TestCase;
+
+class DbSchemaWriterTest extends TestCase
+{
+    /**
+     * @var ResourceConnection|MockObject
+     */
+    private $resourceConnection;
+
+    /**
+     * @var StatementFactory|MockObject
+     */
+    private $statementFactory;
+
+    /**
+     * @var DryRunLogger|MockObject
+     */
+    private $dryRunLogger;
+
+    /**
+     * @var SqlVersionProvider|MockObject
+     */
+    private $sqlVersionProvider;
+
+    /**
+     * @var AdapterInterface|MockObject
+     */
+    private $adapter;
+
+    /**
+     * @var DbSchemaWriter
+     */
+    private $model;
+
+    protected function setUp(): void
+    {
+        $this->resourceConnection = $this->getMockBuilder(ResourceConnection::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+        $this->statementFactory = $this->getMockBuilder(StatementFactory::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+        $this->dryRunLogger = $this->getMockBuilder(DryRunLogger::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+        $this->sqlVersionProvider = $this->getMockBuilder(SqlVersionProvider::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        $this->adapter = $this->getMockBuilder(AdapterInterface::class)
+            ->getMockForAbstractClass();
+        $this->resourceConnection->expects($this->any())
+            ->method('getConnection')
+            ->willReturn($this->adapter);
+
+        $this->model = new DbSchemaWriter(
+            $this->resourceConnection,
+            $this->statementFactory,
+            $this->dryRunLogger,
+            $this->sqlVersionProvider
+        );
+    }
+
+    /**
+     * Test to check that column modification and adding fk are run as separate queries with MariaDb
+     *
+     * @param string $dbVersion
+     * @param int $numberOfQueries
+     * @return void
+     *
+     * @dataProvider compileDataProvider
+     */
+    public function testCompileWithColumnModificationAndFK(string $dbVersion, int $numberOfQueries) : void
+    {
+        $dryRun = false;
+        $statementAggregator = $this->getMockBuilder(StatementAggregator::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+        $statement1 = $this->getMockBuilder(Statement::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+        $statement1->expects($this->any())
+            ->method('getResource')
+            ->willReturn('resource');
+        $statement1->expects($this->any())
+            ->method('getType')
+            ->willReturn('alter');
+        $statement1->expects($this->any())
+            ->method('getName')
+            ->willReturn('column');
+        $statement1->expects($this->any())
+            ->method('getStatement')
+            ->willReturn('MODIFY COLUMN `column1` varchar(64) NOT NULL');
+
+        $statement2 = $this->getMockBuilder(Statement::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+        $statement2->expects($this->any())
+            ->method('getResource')
+            ->willReturn('resource');
+        $statement2->expects($this->any())
+            ->method('getType')
+            ->willReturn('alter');
+        $statement2->expects($this->any())
+            ->method('getName')
+            ->willReturn('FK_COLUMN');
+        $statement2->expects($this->any())
+            ->method('getStatement')
+            ->willReturn('ADD CONSTRAINT `FK_COLUMN` FOREIGN KEY (`column`)');
+
+        $statementBank = [$statement1, $statement2];
+        $statementAggregator->expects($this->any())
+            ->method('getStatementsBank')
+            ->willReturn([$statementBank]);
+        $this->sqlVersionProvider->expects($this->any())
+            ->method('getSqlVersion')
+            ->willReturn($dbVersion);
+        $this->adapter->expects($this->exactly($numberOfQueries))
+            ->method('query');
+
+        $this->model->compile($statementAggregator, $dryRun);
+    }
+
+    /**
+     * @return array
+     */
+    public static function compileDataProvider() : array
+    {
+        return [
+            [SqlVersionProvider::MARIA_DB_10_4_VERSION, 2],
+            [SqlVersionProvider::MARIA_DB_10_6_VERSION, 2],
+            [SqlVersionProvider::MYSQL_8_0_VERSION, 1],
+        ];
+    }
+}
