Skip to content

Commit 1192b37

Browse files
rganinrganin
committed
MAGETWO-87191: [Improvement] Add reporting instead of ignore action on schema processing
1 parent 574045c commit 1192b37

File tree

4 files changed

+228
-22
lines changed

4 files changed

+228
-22
lines changed

‎lib/internal/Magento/Framework/Setup/Declaration/Schema/Db/MySQL/DDL/Triggers/MigrateDataFromAnotherTable.php

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use Magento\Framework\Setup\Declaration\Schema\Db\DDLTriggerInterface;
1212
use Magento\Framework\Setup\Declaration\Schema\Dto\Column;
1313
use Magento\Framework\Setup\Declaration\Schema\Dto\ElementInterface;
14+
use Magento\Framework\Setup\Exception;
1415

1516
/**
1617
* Used to migrate data from one column to another in scope of one table.
@@ -82,6 +83,14 @@ public function getCallback(ElementInterface $column)
8283
$this->resourceConnection->getTableName($tableName)
8384
)
8485
);
86+
} else {
87+
throw new Exception(
88+
__(
89+
'Table `%1` does not exist for connection `%2`.',
90+
$tableMigrateFrom,
91+
$column->getTable()->getResource()
92+
)
93+
);
8594
}
8695
};
8796
}

‎lib/internal/Magento/Framework/Setup/Declaration/Schema/Db/SchemaBuilder.php

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use Magento\Framework\Setup\Declaration\Schema\Dto\Schema;
1212
use Magento\Framework\Setup\Declaration\Schema\Dto\Table;
1313
use Magento\Framework\Setup\Declaration\Schema\Sharding;
14+
use Magento\Framework\Setup\Exception;
1415

1516
/**
1617
* This type of builder is responsible for converting ENTIRE data, that comes from db
@@ -98,15 +99,15 @@ public function build(Schema $schema)
9899
$table->addColumns($columns);
99100
//Process indexes
100101
foreach ($indexesData as $indexData) {
101-
$indexData['columns'] = $this->resolveInternalRelations($columns, $indexData);
102102
$indexData['table'] = $table;
103+
$indexData['columns'] = $this->resolveInternalRelations($columns, $indexData);
103104
$index = $this->elementFactory->create('index', $indexData);
104105
$indexes[$index->getName()] = $index;
105106
}
106107
//Process internal constraints
107108
foreach ($constrainsData as $constraintData) {
108-
$constraintData['columns'] = $this->resolveInternalRelations($columns, $constraintData);
109109
$constraintData['table'] = $table;
110+
$constraintData['columns'] = $this->resolveInternalRelations($columns, $constraintData);
110111
$constraint = $this->elementFactory->create($constraintData['type'], $constraintData);
111112
$constraints[$constraint->getName()] = $constraint;
112113
}
@@ -159,19 +160,28 @@ private function processReferenceKeys(array $tables)
159160
* @param Column[] $columns
160161
* @param array $data
161162
* @return Column[]
163+
* @throws Exception
162164
*/
163165
private function resolveInternalRelations(array $columns, array $data)
164166
{
165167
if (!is_array($data['column'])) {
166-
throw new \InvalidArgumentException("Cannot find columns for internal index");
168+
throw new Exception(
169+
__("Cannot find columns for internal index")
170+
);
167171
}
168172

169173
$referenceColumns = [];
170174
foreach ($data['column'] as $columnName) {
171175
if (!isset($columns[$columnName])) {
172-
//Depends on business logic, can either ignore non-existing column
173-
//or throw exception if db is not consistent and there is no column
174-
//that was specified for key
176+
$tableName = isset($data['table']) ? $data['table']->getName() : '';
177+
throw new Exception(
178+
__(
179+
'Column %1 does not exist for index/constraint %2 in table %3.',
180+
$columnName,
181+
$data['name'],
182+
$tableName
183+
)
184+
);
175185
} else {
176186
$referenceColumns[] = $columns[$columnName];
177187
}

‎lib/internal/Magento/Framework/Setup/Test/Unit/Declaration/Schema/Db/MySQL/DDL/Triggers/MigrateDataFromAnotherTableTest.php

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
namespace Magento\Framework\Setup\Test\Unit\Declaration\Schema\Db\MySQL\DDL\Triggers;
7+
8+
use Magento\Framework\App\ResourceConnection;
9+
use Magento\Framework\DB\Adapter\AdapterInterface;
10+
use Magento\Framework\DB\Select;
11+
use Magento\Framework\DB\SelectFactory;
12+
use Magento\Framework\Setup\Declaration\Schema\Db\MySQL\DDL\Triggers\MigrateDataFromAnotherTable;
13+
use Magento\Framework\Setup\Declaration\Schema\Dto\Column;
14+
use Magento\Framework\Setup\Declaration\Schema\Dto\Table;
15+
16+
class MigrateDataFromAnotherTableTest extends \PHPUnit\Framework\TestCase
17+
{
18+
/**
19+
* @var ResourceConnection|\PHPUnit_Framework_MockObject_MockObject
20+
*/
21+
private $resourceConnectionMock;
22+
23+
/**
24+
* @var SelectFactory|\PHPUnit_Framework_MockObject_MockObject
25+
*/
26+
private $selectFactoryMock;
27+
28+
/**
29+
* @var MigrateDataFromAnotherTable
30+
*/
31+
private $model;
32+
33+
protected function setUp()
34+
{
35+
$this->resourceConnectionMock = $this->createMock(ResourceConnection::class);
36+
$this->selectFactoryMock = $this->createMock(SelectFactory::class);
37+
$this->model = new MigrateDataFromAnotherTable(
38+
$this->resourceConnectionMock,
39+
$this->selectFactoryMock
40+
);
41+
}
42+
43+
public function testTriggerTestTable()
44+
{
45+
$columnMock = $this->getMockBuilder(Column::class)
46+
->setMethods(['getOnCreate', 'getName', 'getTable'])
47+
->disableOriginalConstructor()
48+
->getMock();
49+
$columnMock->expects($this->any())
50+
->method('getOnCreate')
51+
->willReturn('migrateDataFromAnotherTable(source_table,source_column)');
52+
$columnMock->expects($this->any())->method('getName')->willReturn('target_column');
53+
54+
$tableMock = $this->getMockBuilder(Table::class)
55+
->disableOriginalConstructor()
56+
->setMethods(['getName', 'getResource'])
57+
->getMock();
58+
$tableMock->expects($this->any())->method('getName')->willReturn('target_table');
59+
$tableMock->expects($this->any())->method('getResource')->willReturn('default');
60+
$columnMock->expects($this->any())->method('getTable')->willReturn($tableMock);
61+
$selectMock = $this->createMock(Select::class);
62+
63+
$this->resourceConnectionMock->expects($this->any())->method('getTableName')->willReturnArgument(0);
64+
65+
$adapterMock = $this->createMock(AdapterInterface::class);
66+
$adapterMock->expects($this->any())
67+
->method('insertFromSelect')
68+
->with(
69+
$selectMock,
70+
'target_table'
71+
)->willReturn('INSERT FROM SELECT QUERY STRING');
72+
$adapterMock->expects($this->once())
73+
->method('query')
74+
->with('INSERT FROM SELECT QUERY STRING');
75+
$adapterMock->expects($this->once())
76+
->method('isTableExists')
77+
->with('source_table')
78+
->willReturn(true);
79+
$this->resourceConnectionMock->expects($this->any())
80+
->method('getConnection')
81+
->with('default')
82+
->willReturn($adapterMock);
83+
$selectMock->expects($this->once())
84+
->method('from')
85+
->with(
86+
'source_table',
87+
['target_column' => 'source_column'],
88+
null
89+
);
90+
91+
$this->selectFactoryMock->expects($this->once())
92+
->method('create')
93+
->with($adapterMock)
94+
->willReturn($selectMock);
95+
$this->model->getCallback($columnMock)();
96+
}
97+
98+
/**
99+
* @expectedException \Magento\Framework\Setup\Exception
100+
* @expectedExceptionMessage Table `source_table` does not exist for connection `default`
101+
*/
102+
public function testTriggerUnexistentTestTable()
103+
{
104+
$columnMock = $this->getMockBuilder(Column::class)
105+
->setMethods(['getOnCreate', 'getName', 'getTable'])
106+
->disableOriginalConstructor()
107+
->getMock();
108+
$columnMock->expects($this->any())
109+
->method('getOnCreate')
110+
->willReturn('migrateDataFromAnotherTable(source_table,source_column)');
111+
$columnMock->expects($this->any())->method('getName')->willReturn('target_column');
112+
113+
$tableMock = $this->getMockBuilder(Table::class)
114+
->disableOriginalConstructor()
115+
->setMethods(['getName', 'getResource'])
116+
->getMock();
117+
$tableMock->expects($this->any())->method('getName')->willReturn('target_table');
118+
$tableMock->expects($this->any())->method('getResource')->willReturn('default');
119+
$columnMock->expects($this->any())->method('getTable')->willReturn($tableMock);
120+
$selectMock = $this->createMock(Select::class);
121+
122+
$this->resourceConnectionMock->expects($this->any())
123+
->method('getTableName')
124+
->willReturnArgument(0);
125+
126+
$adapterMock = $this->createMock(AdapterInterface::class);
127+
$adapterMock->expects($this->never())->method('query');
128+
$adapterMock->expects($this->once())
129+
->method('isTableExists')
130+
->with('source_table')
131+
->willReturn(false);
132+
$this->resourceConnectionMock->expects($this->any())
133+
->method('getConnection')
134+
->with('default')
135+
->willReturn($adapterMock);
136+
$selectMock->expects($this->once())
137+
->method('from')
138+
->with(
139+
'source_table',
140+
['target_column' => 'source_column'],
141+
null
142+
);
143+
$this->selectFactoryMock->expects($this->once())
144+
->method('create')
145+
->with($adapterMock)
146+
->willReturn($selectMock);
147+
$this->model->getCallback($columnMock)();
148+
}
149+
}

‎lib/internal/Magento/Framework/Setup/Test/Unit/Declaration/Schema/Db/SchemaBuilderTest.php

Lines changed: 54 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -266,9 +266,55 @@ private function createTimestampColumn($name, Table $table)
266266
* @param array $references
267267
* @param array $constraints
268268
* @param array $indexes
269-
* @SuppressWarnings(PHPMD.ExcessiveMethodLength)
270269
*/
271270
public function testBuild(array $columns, array $references, array $constraints, array $indexes)
271+
{
272+
$this->prepareSchemaMocks($columns, $references, $constraints, $indexes);
273+
$resourceConnectionMock = $this->getMockBuilder(ResourceConnection::class)
274+
->disableOriginalConstructor()
275+
->getMock();
276+
/** @var Schema $schema */
277+
$schema = $this->objectManagerHelper->getObject(
278+
Schema::class,
279+
['resourceConnection' => $resourceConnectionMock]
280+
);
281+
$this->model->build($schema);
282+
}
283+
284+
/**
285+
* @dataProvider dataProvider
286+
* @param array $columns
287+
* @param array $references
288+
* @param array $constraints
289+
* @param array $indexes
290+
* @expectedException \Magento\Framework\Setup\Exception
291+
* @expectedExceptionMessage Column unknown_column does not exist for index/constraint FIRST_INDEX in table second_table
292+
*/
293+
public function testBuildUnknownIndexColumn(array $columns, array $references, array $constraints, array $indexes)
294+
{
295+
$indexes['second_table']['FIRST_INDEX']['column'][] = 'unknown_column';
296+
$this->prepareSchemaMocks($columns, $references, $constraints, $indexes);
297+
$resourceConnectionMock = $this->getMockBuilder(ResourceConnection::class)
298+
->disableOriginalConstructor()
299+
->getMock();
300+
/** @var Schema $schema */
301+
$schema = $this->objectManagerHelper->getObject(
302+
Schema::class,
303+
['resourceConnection' => $resourceConnectionMock]
304+
);
305+
$this->model->build($schema);
306+
}
307+
308+
/**
309+
* Prepare mocks for test.
310+
*
311+
* @param array $columns
312+
* @param array $references
313+
* @param array $constraints
314+
* @param array $indexes
315+
* @SuppressWarnings(PHPMD.ExcessiveMethodLength)
316+
*/
317+
private function prepareSchemaMocks(array $columns, array $references, array $constraints, array $indexes)
272318
{
273319
$withContext = [['first_table', 'default'], ['second_table', 'default']];
274320
$this->shardingMock->expects(self::once())
@@ -278,26 +324,26 @@ public function testBuild(array $columns, array $references, array $constraints,
278324
->method('readTables')
279325
->with('default')
280326
->willReturn(['first_table', 'second_table']);
281-
$this->dbSchemaReaderMock->expects(self::exactly(2))
327+
$this->dbSchemaReaderMock->expects($this->any())
282328
->method('getTableOptions')
283329
->withConsecutive(...array_values($withContext))
284330
->willReturnOnConsecutiveCalls(
285331
['Engine' => 'innodb', 'Comment' => ''],
286332
['Engine' => 'innodb', 'Comment' => 'Not null comment']
287333
);
288-
$this->dbSchemaReaderMock->expects(self::exactly(2))
334+
$this->dbSchemaReaderMock->expects($this->any())
289335
->method('readColumns')
290336
->withConsecutive(...array_values($withContext))
291337
->willReturnOnConsecutiveCalls($columns['first_table'], $columns['second_table']);
292-
$this->dbSchemaReaderMock->expects(self::exactly(2))
338+
$this->dbSchemaReaderMock->expects($this->any())
293339
->method('readIndexes')
294340
->withConsecutive(...array_values($withContext))
295341
->willReturnOnConsecutiveCalls([], $indexes['second_table']);
296-
$this->dbSchemaReaderMock->expects(self::exactly(2))
342+
$this->dbSchemaReaderMock->expects($this->any())
297343
->method('readConstraints')
298344
->withConsecutive(...array_values($withContext))
299345
->willReturnOnConsecutiveCalls($constraints['first_table'], []);
300-
$this->dbSchemaReaderMock->expects(self::exactly(2))
346+
$this->dbSchemaReaderMock->expects($this->any())
301347
->method('readReferences')
302348
->withConsecutive(...array_values($withContext))
303349
->willReturnOnConsecutiveCalls($references['first_table'], []);
@@ -322,7 +368,7 @@ public function testBuild(array $columns, array $references, array $constraints,
322368
);
323369
$table->addColumns([$firstColumn, $foreignColumn, $timestampColumn]);
324370
$table->addConstraints([$foreignKey, $primaryKey]);
325-
$this->elementFactoryMock->expects(self::exactly(9))
371+
$this->elementFactoryMock->expects($this->any())
326372
->method('create')
327373
->withConsecutive(
328374
[
@@ -426,14 +472,6 @@ public function testBuild(array $columns, array $references, array $constraints,
426472
$index,
427473
$foreignKey
428474
);
429-
$resourceConnectionMock = $this->getMockBuilder(ResourceConnection::class)
430-
->disableOriginalConstructor()
431-
->getMock();
432-
/** @var Schema $schema */
433-
$schema = $this->objectManagerHelper->getObject(
434-
Schema::class,
435-
['resourceConnection' => $resourceConnectionMock]
436-
);
437-
$this->model->build($schema);
475+
438476
}
439477
}

0 commit comments

Comments
 (0)