Merge branch 'MAGETWO-87191' of github.com:magento-trigger/magento2ce into MAGETWO-88054

rganin · rganin · commit 9d16fe455a07 · 2018-02-27T10:47:48.000+02:00
diff --git a/app/etc/di.xml b/app/etc/di.xml
@@ -1419,14 +1419,6 @@
         <arguments>
             <argument name="triggers" xsi:type="array">
                 <item name="migrateDataFromSameTable" xsi:type="object">Magento\Framework\Setup\Declaration\Schema\Db\MySQL\DDL\Triggers\MigrateDataFrom</item>
-                <item name="migrateDataFromAnotherTable" xsi:type="object">Magento\Framework\Setup\Declaration\Schema\Db\MySQL\DDL\Triggers\MigrateDataFromAnotherTable</item>
-            </argument>
-        </arguments>
-    </type>
-    <type name="Magento\Framework\Setup\Declaration\Schema\Operations\CreateTable">
-        <arguments>
-            <argument name="triggers" xsi:type="array">
-                <item name="migrateDataFromAnotherTable" xsi:type="object">Magento\Framework\Setup\Declaration\Schema\Db\MySQL\DDL\Triggers\MigrateDataFromAnotherTable</item>
             </argument>
         </arguments>
     </type>
@@ -1516,4 +1508,9 @@
             <argument name="schemaPatchReader" xsi:type="object">\Magento\Framework\Setup\Patch\SchemaPatchReader</argument>
         </arguments>
     </type>
+    <type name="Magento\Framework\Setup\Declaration\Schema\Db\SchemaBuilder">
+        <arguments>
+            <argument name="logger" xsi:type="object">\Magento\Framework\Setup\Patch\DataPatchReader</argument>
+        </arguments>
+    </type>
 </config>
diff --git a/lib/internal/Magento/Framework/Setup/Declaration/Schema/Db/MySQL/DDL/Triggers/MigrateDataFromAnotherTable.php b/lib/internal/Magento/Framework/Setup/Declaration/Schema/Db/MySQL/DDL/Triggers/MigrateDataFromAnotherTable.php
diff --git a/lib/internal/Magento/Framework/Setup/Declaration/Schema/Db/SchemaBuilder.php b/lib/internal/Magento/Framework/Setup/Declaration/Schema/Db/SchemaBuilder.php
@@ -11,6 +11,7 @@
 use Magento\Framework\Setup\Declaration\Schema\Dto\Schema;
 use Magento\Framework\Setup\Declaration\Schema\Dto\Table;
 use Magento\Framework\Setup\Declaration\Schema\Sharding;
+use Magento\Framework\Setup\Exception;
 
 /**
  * This type of builder is responsible for converting ENTIRE data, that comes from db
@@ -98,15 +99,15 @@ public function build(Schema $schema)
                 $table->addColumns($columns);
                 //Process indexes
                 foreach ($indexesData as $indexData) {
-                    $indexData['columns'] = $this->resolveInternalRelations($columns, $indexData);
                     $indexData['table'] = $table;
+                    $indexData['columns'] = $this->resolveInternalRelations($columns, $indexData);
                     $index = $this->elementFactory->create('index', $indexData);
                     $indexes[$index->getName()] = $index;
                 }
                 //Process internal constraints
                 foreach ($constrainsData as $constraintData) {
-                    $constraintData['columns'] = $this->resolveInternalRelations($columns, $constraintData);
                     $constraintData['table'] = $table;
+                    $constraintData['columns'] = $this->resolveInternalRelations($columns, $constraintData);
                     $constraint = $this->elementFactory->create($constraintData['type'], $constraintData);
                     $constraints[$constraint->getName()] = $constraint;
                 }
@@ -159,19 +160,29 @@ private function processReferenceKeys(array $tables)
      * @param  Column[] $columns
      * @param  array    $data
      * @return Column[]
+     * @throws Exception
      */
     private function resolveInternalRelations(array $columns, array $data)
     {
         if (!is_array($data['column'])) {
-            throw new \InvalidArgumentException("Cannot find columns for internal index");
+            throw new Exception(
+                __("Cannot find columns for internal index")
+            );
         }
 
         $referenceColumns = [];
         foreach ($data['column'] as $columnName) {
             if (!isset($columns[$columnName])) {
-                //Depends on business logic, can either ignore non-existing column
-                //or throw exception if db is not consistent and there is no column
-                //that was specified for key
+                $tableName = isset($data['table']) ? $data['table']->getName() : '';
+                trigger_error(
+                    __(
+                        'Column %1 does not exist for index/constraint %2 in table %3.',
+                        $columnName,
+                        $data['name'],
+                        $tableName
+                    ),
+                    E_USER_WARNING
+                );
             } else {
                 $referenceColumns[] = $columns[$columnName];
             }
diff --git a/lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php b/lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php
@@ -228,7 +228,14 @@ public function applySchemaPatch($moduleName = null)
                 $schemaPatch->apply();
                 $this->patchHistory->fixPatch(get_class($schemaPatch));
             } catch (\Exception $e) {
-                throw new Exception($e->getMessage());
+                throw new Exception(
+                    __(
+                        'Unable to apply patch %1 for module %2. Original exception message: %3',
+                        get_class($schemaPatch),
+                        $moduleName,
+                        $e->getMessage()
+                    )
+                );
             } finally {
                 unset($schemaPatch);
             }
diff --git a/lib/internal/Magento/Framework/Setup/Test/Unit/Declaration/Schema/Db/SchemaBuilderTest.php b/lib/internal/Magento/Framework/Setup/Test/Unit/Declaration/Schema/Db/SchemaBuilderTest.php
@@ -266,9 +266,56 @@ private function createTimestampColumn($name, Table $table)
      * @param array $references
      * @param array $constraints
      * @param array $indexes
-     * @SuppressWarnings(PHPMD.ExcessiveMethodLength)
      */
     public function testBuild(array $columns, array $references, array $constraints, array $indexes)
+    {
+        $this->prepareSchemaMocks($columns, $references, $constraints, $indexes);
+        $resourceConnectionMock = $this->getMockBuilder(ResourceConnection::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+        /** @var Schema $schema */
+        $schema = $this->objectManagerHelper->getObject(
+            Schema::class,
+            ['resourceConnection' => $resourceConnectionMock]
+        );
+        $this->model->build($schema);
+    }
+
+    /**
+     * @dataProvider dataProvider
+     * @param array $columns
+     * @param array $references
+     * @param array $constraints
+     * @param array $indexes
+     * @expectedException \PHPUnit\Framework\Exception
+     * @expectedExceptionMessage
+     * User Warning: Column unknown_column does not exist for index/constraint FIRST_INDEX in table second_table
+     */
+    public function testBuildUnknownIndexColumn(array $columns, array $references, array $constraints, array $indexes)
+    {
+        $indexes['second_table']['FIRST_INDEX']['column'][] = 'unknown_column';
+        $this->prepareSchemaMocks($columns, $references, $constraints, $indexes);
+        $resourceConnectionMock = $this->getMockBuilder(ResourceConnection::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+        /** @var Schema $schema */
+        $schema = $this->objectManagerHelper->getObject(
+            Schema::class,
+            ['resourceConnection' => $resourceConnectionMock]
+        );
+        $this->model->build($schema);
+    }
+
+    /**
+     * Prepare mocks for test.
+     *
+     * @param array $columns
+     * @param array $references
+     * @param array $constraints
+     * @param array $indexes
+     * @SuppressWarnings(PHPMD.ExcessiveMethodLength)
+     */
+    private function prepareSchemaMocks(array $columns, array $references, array $constraints, array $indexes)
     {
         $withContext = [['first_table', 'default'], ['second_table', 'default']];
         $this->shardingMock->expects(self::once())
@@ -278,26 +325,26 @@ public function testBuild(array $columns, array $references, array $constraints,
             ->method('readTables')
             ->with('default')
             ->willReturn(['first_table', 'second_table']);
-        $this->dbSchemaReaderMock->expects(self::exactly(2))
+        $this->dbSchemaReaderMock->expects($this->any())
             ->method('getTableOptions')
             ->withConsecutive(...array_values($withContext))
             ->willReturnOnConsecutiveCalls(
                 ['Engine' => 'innodb', 'Comment' => ''],
                 ['Engine' => 'innodb', 'Comment' => 'Not null comment']
             );
-        $this->dbSchemaReaderMock->expects(self::exactly(2))
+        $this->dbSchemaReaderMock->expects($this->any())
             ->method('readColumns')
             ->withConsecutive(...array_values($withContext))
             ->willReturnOnConsecutiveCalls($columns['first_table'], $columns['second_table']);
-        $this->dbSchemaReaderMock->expects(self::exactly(2))
+        $this->dbSchemaReaderMock->expects($this->any())
             ->method('readIndexes')
             ->withConsecutive(...array_values($withContext))
             ->willReturnOnConsecutiveCalls([], $indexes['second_table']);
-        $this->dbSchemaReaderMock->expects(self::exactly(2))
+        $this->dbSchemaReaderMock->expects($this->any())
             ->method('readConstraints')
             ->withConsecutive(...array_values($withContext))
             ->willReturnOnConsecutiveCalls($constraints['first_table'], []);
-        $this->dbSchemaReaderMock->expects(self::exactly(2))
+        $this->dbSchemaReaderMock->expects($this->any())
             ->method('readReferences')
             ->withConsecutive(...array_values($withContext))
             ->willReturnOnConsecutiveCalls($references['first_table'], []);
@@ -322,7 +369,7 @@ public function testBuild(array $columns, array $references, array $constraints,
         );
         $table->addColumns([$firstColumn, $foreignColumn, $timestampColumn]);
         $table->addConstraints([$foreignKey, $primaryKey]);
-        $this->elementFactoryMock->expects(self::exactly(9))
+        $this->elementFactoryMock->expects($this->any())
             ->method('create')
             ->withConsecutive(
                 [
@@ -426,14 +473,6 @@ public function testBuild(array $columns, array $references, array $constraints,
                 $index,
                 $foreignKey
             );
-        $resourceConnectionMock = $this->getMockBuilder(ResourceConnection::class)
-            ->disableOriginalConstructor()
-            ->getMock();
-        /** @var Schema $schema */
-        $schema = $this->objectManagerHelper->getObject(
-            Schema::class,
-            ['resourceConnection' => $resourceConnectionMock]
-        );
-        $this->model->build($schema);
+
     }
 }
