Skip to content

Commit fcbf4b7

Browse files
committed
MAGETWO-92279: An incorrect result of declaration:generate:whitelist execution
- make generator exclude tables from app/etc/db_schema.xml - fix unit tests - create setup integration test for declaration:generate:whitelist
1 parent 1a968e0 commit fcbf4b7

File tree

4 files changed

+346
-14
lines changed

4 files changed

+346
-14
lines changed

app/code/Magento/Developer/Console/Command/TablesWhitelistGenerateCommand.php

Lines changed: 48 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,16 @@ class TablesWhitelistGenerateCommand extends Command
4242
*/
4343
private $jsonPersistor;
4444

45+
/**
46+
* @var array
47+
*/
48+
private $primaryDbSchema;
49+
4550
/**
4651
* @param ComponentRegistrar $componentRegistrar
4752
* @param ReaderComposite $readerComposite
4853
* @param JsonPersistor $jsonPersistor
49-
* @param null $name
54+
* @param string|null $name
5055
*/
5156
public function __construct(
5257
ComponentRegistrar $componentRegistrar,
@@ -104,18 +109,21 @@ private function persistModule($moduleName)
104109
if (file_exists($whiteListFileName)) {
105110
$content = json_decode(file_get_contents($whiteListFileName), true);
106111
}
107-
$newContent = $this->readerComposite->read($moduleName);
108112

109-
//Do merge between what we have before, and what we have now.
113+
$newContent = $this->filterPrimaryTables($this->readerComposite->read($moduleName));
114+
115+
//Do merge between what we have before, and what we have now and filter to only certain attributes.
110116
$content = array_replace_recursive(
111117
$content,
112-
$this->selectNamesFromContent($newContent)
118+
$this->filterAttributeNames($newContent)
113119
);
114-
$this->jsonPersistor->persist($content, $whiteListFileName);
120+
if (!empty($content)) {
121+
$this->jsonPersistor->persist($content, $whiteListFileName);
122+
}
115123
}
116124

117125
/**
118-
* {@inheritdoc}
126+
* @inheritdoc
119127
*/
120128
protected function execute(InputInterface $input, OutputInterface $output)
121129
{
@@ -144,7 +152,7 @@ protected function execute(InputInterface $input, OutputInterface $output)
144152
* @param array $content
145153
* @return array
146154
*/
147-
private function selectNamesFromContent(array $content)
155+
private function filterAttributeNames(array $content)
148156
{
149157
$names = [];
150158
$types = ['column', 'index', 'constraint'];
@@ -163,4 +171,37 @@ private function selectNamesFromContent(array $content)
163171

164172
return $names;
165173
}
174+
175+
/**
176+
* Load db_schema content from the primary scope app/etc/db_schema.xml.
177+
*
178+
* @return array
179+
*/
180+
private function getPrimaryDbSchema()
181+
{
182+
if (!$this->primaryDbSchema) {
183+
$this->primaryDbSchema = $this->readerComposite->read('primary');
184+
}
185+
return $this->primaryDbSchema;
186+
}
187+
188+
/**
189+
* Filter tables from module db_schema.xml as they should not contain the primary system tables.
190+
*
191+
* @param array $moduleDbSchema
192+
* @return array
193+
* @SuppressWarnings(PHPMD.UnusedLocalVariable)
194+
*/
195+
private function filterPrimaryTables(array $moduleDbSchema)
196+
{
197+
$primaryDbSchema = $this->getPrimaryDbSchema();
198+
if (isset($moduleDbSchema['table']) && isset($primaryDbSchema['table'])) {
199+
foreach ($primaryDbSchema['table'] as $tableNameKey => $tableContents) {
200+
if (isset($moduleDbSchema['table'][$tableNameKey])) {
201+
unset($moduleDbSchema['table'][$tableNameKey]);
202+
}
203+
}
204+
}
205+
return $moduleDbSchema;
206+
}
166207
}

app/code/Magento/Developer/Test/Unit/Console/Command/TablesWhitelistGenerateCommandTest.php

Lines changed: 95 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,9 @@ class TablesWhitelistGenerateCommandTest extends \PHPUnit\Framework\TestCase
4545
*/
4646
private $jsonPersistorMock;
4747

48+
/**
49+
* {@inheritdoc}
50+
*/
4851
protected function setUp()
4952
{
5053
$this->componentRegistrarMock = $this->getMockBuilder(ComponentRegistrar::class)
@@ -76,6 +79,47 @@ public function whitelistTableProvider()
7679
[
7780
'moduleName' => 'SomeModule',
7881
'whitelist' => [
82+
'primary' => [
83+
'table' =>
84+
[
85+
'patch_list' =>
86+
[
87+
'column' =>
88+
[
89+
'patch_id' =>
90+
[
91+
'type' => 'int',
92+
'name' => 'patch_id',
93+
'identity' => 'true',
94+
'comment' => 'Patch Auto Increment',
95+
],
96+
'patch_name' =>
97+
[
98+
'type' => 'varchar',
99+
'name' => 'patch_name',
100+
'length' => '1024',
101+
'nullable' => 'false',
102+
'comment' => 'Patch Class Name',
103+
],
104+
],
105+
'constraint' =>
106+
[
107+
'PRIMARY' =>
108+
[
109+
'column' =>
110+
[
111+
'patch_id' => 'patch_id',
112+
],
113+
'type' => 'primary',
114+
'name' => 'PRIMARY',
115+
],
116+
],
117+
'name' => 'patch_list',
118+
'resource' => 'default',
119+
'comment' => 'List of data/schema patches',
120+
],
121+
],
122+
],
79123
'SomeModule' => [
80124
'table' => [
81125
'first_table' => [
@@ -149,6 +193,47 @@ public function whitelistTableProvider()
149193
[
150194
'moduleName' => false,
151195
'whitelist' => [
196+
'primary' => [
197+
'table' =>
198+
[
199+
'patch_list' =>
200+
[
201+
'column' =>
202+
[
203+
'patch_id' =>
204+
[
205+
'type' => 'int',
206+
'name' => 'patch_id',
207+
'identity' => 'true',
208+
'comment' => 'Patch Auto Increment',
209+
],
210+
'patch_name' =>
211+
[
212+
'type' => 'varchar',
213+
'name' => 'patch_name',
214+
'length' => '1024',
215+
'nullable' => 'false',
216+
'comment' => 'Patch Class Name',
217+
],
218+
],
219+
'constraint' =>
220+
[
221+
'PRIMARY' =>
222+
[
223+
'column' =>
224+
[
225+
'patch_id' => 'patch_id',
226+
],
227+
'type' => 'primary',
228+
'name' => 'PRIMARY',
229+
],
230+
],
231+
'name' => 'patch_list',
232+
'resource' => 'default',
233+
'comment' => 'List of data/schema patches',
234+
],
235+
],
236+
],
152237
'SomeModule' => [
153238
'table' => [
154239
'first_table' => [
@@ -303,10 +388,14 @@ public function testCommand($moduleName, array $whiteListTables, array $expected
303388
$this->componentRegistrarMock->expects(self::once())
304389
->method('getPaths')
305390
->willReturn(['SomeModule' => 1, 'Module2' => 2]);
306-
$this->readerCompositeMock->expects(self::exactly(2))
391+
$this->readerCompositeMock->expects(self::exactly(3))
307392
->method('read')
308-
->withConsecutive(['SomeModule'], ['Module2'])
309-
->willReturnOnConsecutiveCalls($whiteListTables['SomeModule'], $whiteListTables['Module2']);
393+
->withConsecutive(['SomeModule'], ['primary'], ['Module2'])
394+
->willReturnOnConsecutiveCalls(
395+
$whiteListTables['SomeModule'],
396+
$whiteListTables['primary'],
397+
$whiteListTables['Module2']
398+
);
310399
$this->jsonPersistorMock->expects(self::exactly(2))
311400
->method('persist')
312401
->withConsecutive(
@@ -320,10 +409,10 @@ public function testCommand($moduleName, array $whiteListTables, array $expected
320409
]
321410
);
322411
} else {
323-
$this->readerCompositeMock->expects(self::once())
412+
$this->readerCompositeMock->expects(self::exactly(2))
324413
->method('read')
325-
->with($moduleName)
326-
->willReturn($whiteListTables['SomeModule']);
414+
->withConsecutive([$moduleName], ['primary'])
415+
->willReturnOnConsecutiveCalls($whiteListTables['SomeModule'], $whiteListTables['primary']);
327416
$this->jsonPersistorMock->expects(self::once())
328417
->method('persist')
329418
->with(

0 commit comments

Comments
 (0)