Skip to content

Commit f42716b

Browse files
eddielauOlga Kopylova
eddielau
authored and
Olga Kopylova
committed
MAGETWO-33157: Conflict restriction checking is missing version checking in enable/disable module CLI
- added package version check into ConflictChecker
1 parent d2a38ec commit f42716b

File tree

4 files changed

+80
-24
lines changed

4 files changed

+80
-24
lines changed

‎dev/tests/unit/testsuite/Magento/Framework/Module/ConflictCheckerTest.php

Lines changed: 37 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@ public function testCheckConflictsWhenEnableModules(
2828
$packageInfoMock->expects($this->any())
2929
->method('getConflict')
3030
->will($this->returnValueMap($conflictReturnMap));
31+
$packageInfoMock->expects($this->any())
32+
->method('getVersion')
33+
->will($this->returnValue('0.1'));
3134
$packageInfoFactoryMock = $this->getMock('Magento\Framework\Module\PackageInfoFactory', [], [], '', false);
3235
$packageInfoFactoryMock->expects($this->once())
3336
->method('create')
@@ -43,25 +46,25 @@ public function checkConflictWhenEnableModuleDataProvider()
4346
{
4447
return [
4548
[
46-
[['Vendor_A', true, ['Vendor_B']], ['Vendor_B', true, []]],
49+
[['Vendor_A', true, ['Vendor_B' => '0.1']], ['Vendor_B', true, []]],
4750
['Vendor_A'],
4851
['Vendor_B'],
4952
['Vendor_B' => ['Vendor_A']]
5053
],
5154
[
52-
[['Vendor_A', true, ['Vendor_B']], ['Vendor_B', true, []]],
55+
[['Vendor_A', true, ['Vendor_B' => '0.1']], ['Vendor_B', true, []]],
5356
[],
5457
['Vendor_B'],
5558
['Vendor_B' => []]
5659
],
5760
[
58-
[['Vendor_B', true, ['Vendor_A']], ['Vendor_A', true, []]],
61+
[['Vendor_B', true, ['Vendor_A' => '0.1']], ['Vendor_A', true, []]],
5962
['Vendor_A'],
6063
['Vendor_B'],
6164
['Vendor_B' => ['Vendor_A']]
6265
],
6366
[
64-
[['Vendor_B', true, ['Vendor_A']], ['Vendor_A', true, []]],
67+
[['Vendor_B', true, ['Vendor_A' => '0.1']], ['Vendor_A', true, []]],
6568
[],
6669
['Vendor_B'],
6770
['Vendor_B' => []]
@@ -79,17 +82,45 @@ public function checkConflictWhenEnableModuleDataProvider()
7982
['Vendor_B' => [], 'Vendor_C' => []]
8083
],
8184
[
82-
[['Vendor_A', true, ['Vendor_C']], ['Vendor_B', true, []], ['Vendor_C', true, []]],
85+
[['Vendor_A', true, ['Vendor_C' => '0.1']], ['Vendor_B', true, []], ['Vendor_C', true, []]],
8386
['Vendor_A'],
8487
['Vendor_B', 'Vendor_C'],
8588
['Vendor_B' => [], 'Vendor_C' => ['Vendor_A']]
8689
],
8790
[
88-
[['Vendor_A', true, []], ['Vendor_B', true, ['Vendor_C']], ['Vendor_C', true, []]],
91+
[['Vendor_A', true, []], ['Vendor_B', true, ['Vendor_C' => '0.1']], ['Vendor_C', true, []]],
8992
['Vendor_A'],
9093
['Vendor_B', 'Vendor_C'],
9194
['Vendor_B' => ['Vendor_C'], 'Vendor_C' => ['Vendor_B']]
9295
],
9396
];
9497
}
98+
99+
public function testCheckConflictWhenEnableModuleDifferentVersion()
100+
{
101+
$moduleListMock = $this->getMock('Magento\Framework\Module\ModuleList', [], [], '', false);
102+
$packageInfoMock = $this->getMock('Magento\Framework\Module\PackageInfo', [], [], '', false);
103+
$moduleListMock->expects($this->any())
104+
->method('getNames')
105+
->will($this->returnValue(['Vendor_A', 'Vendor_B']));
106+
$packageInfoMock->expects($this->any())
107+
->method('getConflict')
108+
->will($this->returnValueMap([
109+
['Vendor_A', true, []],
110+
['Vendor_B', true, []],
111+
['Vendor_C', true, ['Vendor_A' => '0.2', 'Vendor_B' => '0.3']]
112+
]));
113+
$packageInfoMock->expects($this->any())
114+
->method('getVersion')
115+
->will($this->returnValueMap([['Vendor_A', '0.2'], ['Vendor_B', '0.4']]));
116+
$packageInfoFactoryMock = $this->getMock('Magento\Framework\Module\PackageInfoFactory', [], [], '', false);
117+
$packageInfoFactoryMock->expects($this->once())
118+
->method('create')
119+
->will($this->returnValue($packageInfoMock));
120+
$conflictChecker = new ConflictChecker($moduleListMock, $packageInfoFactoryMock);
121+
$this->assertEquals(
122+
['Vendor_C' => ['Vendor_A']],
123+
$conflictChecker->checkConflictsWhenEnableModules(['Vendor_C'])
124+
);
125+
}
95126
}

‎dev/tests/unit/testsuite/Magento/Framework/Module/PackageInfoTest.php

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,11 @@ public function setUp()
3131
->will($this->returnValue(['A' => [], 'B' => [], 'C' => [], 'D' => [], 'E' => []]));
3232

3333
$composerData = [
34-
'A' => '{"name":"a", "require":{"b":"0.1"}, "conflict":{"c":"0.1"}}',
35-
'B' => '{"name":"b", "require":{"d":"0.1"}}',
36-
'C' => '{"name":"c", "require":{"e":"0.1"}}',
37-
'D' => '{"name":"d", "conflict":{"c":"0.1"}}',
38-
'E' => '{"name":"e"}',
34+
'A' => '{"name":"a", "require":{"b":"0.1"}, "conflict":{"c":"0.1"}, "version":"0.1"}',
35+
'B' => '{"name":"b", "require":{"d":"0.1"}, "version":"0.1"}',
36+
'C' => '{"name":"c", "require":{"e":"0.1"}, "version":"0.1"}',
37+
'D' => '{"name":"d", "conflict":{"c":"0.1"}, "version":"0.1"}',
38+
'E' => '{"name":"e", "version":"0.1"}',
3939
];
4040
$fileIteratorMock = $this->getMock('Magento\Framework\Config\FileIterator', [], [], '', false);
4141
$fileIteratorMock->expects($this->once())
@@ -68,10 +68,10 @@ public function testGetRequireReturnModuleName()
6868

6969
public function testGetConflictReturnModuleName()
7070
{
71-
$this->assertEquals(['C'], $this->packageInfo->getConflict('A'));
71+
$this->assertEquals(['C' => '0.1'], $this->packageInfo->getConflict('A'));
7272
$this->assertEquals([], $this->packageInfo->getConflict('B'));
7373
$this->assertEquals([], $this->packageInfo->getConflict('C'));
74-
$this->assertEquals(['C'], $this->packageInfo->getConflict('D'));
74+
$this->assertEquals(['C' => '0.1'], $this->packageInfo->getConflict('D'));
7575
$this->assertEquals([], $this->packageInfo->getConflict('E'));
7676
}
7777

@@ -86,10 +86,10 @@ public function testGetRequireReturnPackageName()
8686

8787
public function testGetConflictReturnPackageName()
8888
{
89-
$this->assertEquals(['c'], $this->packageInfo->getConflict('A', false));
89+
$this->assertEquals(['c' => '0.1'], $this->packageInfo->getConflict('A', false));
9090
$this->assertEquals([], $this->packageInfo->getConflict('B', false));
9191
$this->assertEquals([], $this->packageInfo->getConflict('C', false));
92-
$this->assertEquals(['c'], $this->packageInfo->getConflict('D', false));
92+
$this->assertEquals(['c' => '0.1'], $this->packageInfo->getConflict('D', false));
9393
$this->assertEquals([], $this->packageInfo->getConflict('E', false));
9494
}
9595
}

‎lib/internal/Magento/Framework/Module/ConflictChecker.php

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -60,18 +60,20 @@ public function checkConflictsWhenEnableModules($moduleNames)
6060
}
6161

6262
/**
63-
* Check if module is conflicted
63+
* Check if two modules are conflicted
6464
*
65-
* @param string $enabledModule
66-
* @param string $moduleName
65+
* @param string $moduleA
66+
* @param string $moduleB
6767
* @return bool
6868
*/
69-
private function checkIfConflict($enabledModule, $moduleName)
69+
private function checkIfConflict($moduleA, $moduleB)
7070
{
71-
if (array_search($enabledModule, $this->packageInfo->getConflict($moduleName)) !== false) {
71+
if (isset($this->packageInfo->getConflict($moduleB)[$moduleA]) &&
72+
$this->packageInfo->getConflict($moduleB)[$moduleA] === $this->packageInfo->getVersion($moduleA)) {
7273
return true;
7374
}
74-
if (array_search($moduleName, $this->packageInfo->getConflict($enabledModule)) !== false) {
75+
if (isset($this->packageInfo->getConflict($moduleA)[$moduleB]) &&
76+
$this->packageInfo->getConflict($moduleA)[$moduleB] === $this->packageInfo->getVersion($moduleB)) {
7577
return true;
7678
}
7779

‎lib/internal/Magento/Framework/Module/PackageInfo.php

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,13 @@ class PackageInfo
1717
*/
1818
private $packageModuleMap;
1919

20+
/**
21+
* Map of modules to package current version, contains depending module's name
22+
*
23+
* @var string[]
24+
*/
25+
private $modulePackageVersionMap;
26+
2027
/**
2128
* "require" field of each module
2229
*
@@ -25,7 +32,7 @@ class PackageInfo
2532
private $requireMap;
2633

2734
/**
28-
* "conflict" field of each module
35+
* "conflict" field of each module, contains conflicting module's name and version
2936
*
3037
* @var array[]
3138
*/
@@ -76,11 +83,12 @@ private function load()
7683
foreach ($rawData as $moduleName => $jsonData) {
7784
$jsonData = \Zend_Json::decode($jsonData);
7885
$this->packageModuleMap[$jsonData['name']] = $moduleName;
86+
$this->modulePackageVersionMap[$moduleName] = $jsonData['version'];
7987
if (!empty($jsonData['require'])) {
8088
$this->requireMap[$moduleName] = array_keys($jsonData['require']);
8189
}
8290
if (!empty($jsonData['conflict'])) {
83-
$this->conflictMap[$moduleName] = array_keys($jsonData['conflict']);
91+
$this->conflictMap[$moduleName] = $jsonData['conflict'];
8492
}
8593
}
8694
}
@@ -147,11 +155,26 @@ public function getConflict($moduleName, $returnModuleName = true)
147155
$conflict = [];
148156
if (isset($this->conflictMap[$moduleName])) {
149157
if ($returnModuleName) {
150-
$conflict = $this->convertToModuleNames($this->conflictMap[$moduleName]);
158+
$conflict = array_combine(
159+
$this->convertToModuleNames(array_keys($this->conflictMap[$moduleName])),
160+
$this->conflictMap[$moduleName]
161+
);
151162
} else {
152163
$conflict = $this->conflictMap[$moduleName];
153164
}
154165
}
155166
return $conflict;
156167
}
168+
169+
/**
170+
* Get package version of a module
171+
*
172+
* @param string $moduleName
173+
* @return string
174+
*/
175+
public function getVersion($moduleName)
176+
{
177+
$this->load();
178+
return $this->modulePackageVersionMap[$moduleName];
179+
}
157180
}

0 commit comments

Comments
 (0)