Skip to content

Commit 2dc82e6

Browse files
eddielauOlga Kopylova
eddielau
authored and
Olga Kopylova
committed
MAGETWO-33157: Conflict restriction checking is missing version checking in enable/disable module CLI
- updated according to CR feedback
1 parent 80100c1 commit 2dc82e6

File tree

4 files changed

+30
-55
lines changed

4 files changed

+30
-55
lines changed

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

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -46,49 +46,49 @@ public function checkConflictWhenEnableModuleDataProvider()
4646
{
4747
return [
4848
[
49-
[['Vendor_A', true, ['Vendor_B' => '0.1']], ['Vendor_B', true, []]],
49+
[['Vendor_A', ['Vendor_B' => '0.1']], ['Vendor_B', []]],
5050
['Vendor_A'],
5151
['Vendor_B'],
5252
['Vendor_B' => ['Vendor_A']]
5353
],
5454
[
55-
[['Vendor_A', true, ['Vendor_B' => '0.1']], ['Vendor_B', true, []]],
55+
[['Vendor_A', ['Vendor_B' => '0.1']], ['Vendor_B', []]],
5656
[],
5757
['Vendor_B'],
5858
['Vendor_B' => []]
5959
],
6060
[
61-
[['Vendor_B', true, ['Vendor_A' => '0.1']], ['Vendor_A', true, []]],
61+
[['Vendor_B', ['Vendor_A' => '0.1']], ['Vendor_A', []]],
6262
['Vendor_A'],
6363
['Vendor_B'],
6464
['Vendor_B' => ['Vendor_A']]
6565
],
6666
[
67-
[['Vendor_B', true, ['Vendor_A' => '0.1']], ['Vendor_A', true, []]],
67+
[['Vendor_B', ['Vendor_A' => '0.1']], ['Vendor_A', []]],
6868
[],
6969
['Vendor_B'],
7070
['Vendor_B' => []]
7171
],
7272
[
73-
[['Vendor_A', true, []], ['Vendor_B', true, []]],
73+
[['Vendor_A', []], ['Vendor_B', []]],
7474
['Vendor_A'],
7575
['Vendor_B'],
7676
['Vendor_B' => []]
7777
],
7878
[
79-
[['Vendor_A', true, []], ['Vendor_B', true, []], ['Vendor_C', true, []]],
79+
[['Vendor_A', []], ['Vendor_B', []], ['Vendor_C', []]],
8080
['Vendor_A'],
8181
['Vendor_B', 'Vendor_C'],
8282
['Vendor_B' => [], 'Vendor_C' => []]
8383
],
8484
[
85-
[['Vendor_A', true, ['Vendor_C' => '0.1']], ['Vendor_B', true, []], ['Vendor_C', true, []]],
85+
[['Vendor_A', ['Vendor_C' => '0.1']], ['Vendor_B', []], ['Vendor_C', []]],
8686
['Vendor_A'],
8787
['Vendor_B', 'Vendor_C'],
8888
['Vendor_B' => [], 'Vendor_C' => ['Vendor_A']]
8989
],
9090
[
91-
[['Vendor_A', true, []], ['Vendor_B', true, ['Vendor_C' => '0.1']], ['Vendor_C', true, []]],
91+
[['Vendor_A', []], ['Vendor_B', ['Vendor_C' => '0.1']], ['Vendor_C', []]],
9292
['Vendor_A'],
9393
['Vendor_B', 'Vendor_C'],
9494
['Vendor_B' => ['Vendor_C'], 'Vendor_C' => ['Vendor_B']]
@@ -106,9 +106,9 @@ public function testCheckConflictWhenEnableModuleDifferentVersion()
106106
$packageInfoMock->expects($this->any())
107107
->method('getConflict')
108108
->will($this->returnValueMap([
109-
['Vendor_A', true, []],
110-
['Vendor_B', true, []],
111-
['Vendor_C', true, ['Vendor_A' => '0.2', 'Vendor_B' => '0.3']]
109+
['Vendor_A', []],
110+
['Vendor_B', []],
111+
['Vendor_C', ['Vendor_A' => '0.2', 'Vendor_B' => '0.3']]
112112
]));
113113
$packageInfoMock->expects($this->any())
114114
->method('getVersion')

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,11 @@ public function setUp()
3636
{
3737
$this->packageInfoMock = $this->getMock('Magento\Framework\Module\PackageInfo', [], [], '', false);
3838
$requireMap = [
39-
['A', true, ['B']],
40-
['B', true, ['D', 'E']],
41-
['C', true, ['E']],
42-
['D', true, ['A']],
43-
['E', true, []],
39+
['A', ['B']],
40+
['B', ['D', 'E']],
41+
['C', ['E']],
42+
['D', ['A']],
43+
['E', []],
4444
];
4545
$this->packageInfoMock
4646
->expects($this->any())

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

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -75,30 +75,13 @@ public function testGetConflictReturnModuleName()
7575
$this->assertEquals([], $this->packageInfo->getConflict('E'));
7676
}
7777

78-
public function testGetRequireReturnPackageName()
79-
{
80-
$this->assertEquals(['b'], $this->packageInfo->getRequire('A', false));
81-
$this->assertEquals(['d'], $this->packageInfo->getRequire('B', false));
82-
$this->assertEquals(['e'], $this->packageInfo->getRequire('C', false));
83-
$this->assertEquals([], $this->packageInfo->getRequire('D', false));
84-
$this->assertEquals([], $this->packageInfo->getRequire('E', false));
85-
}
86-
87-
public function testGetConflictReturnPackageName()
88-
{
89-
$this->assertEquals(['c' => '0.1'], $this->packageInfo->getConflict('A', false));
90-
$this->assertEquals([], $this->packageInfo->getConflict('B', false));
91-
$this->assertEquals([], $this->packageInfo->getConflict('C', false));
92-
$this->assertEquals(['c' => '0.1'], $this->packageInfo->getConflict('D', false));
93-
$this->assertEquals([], $this->packageInfo->getConflict('E', false));
94-
}
95-
9678
public function testGetVersion()
9779
{
9880
$this->assertEquals('0.1', $this->packageInfo->getVersion('A'));
9981
$this->assertEquals('0.2', $this->packageInfo->getVersion('B'));
10082
$this->assertEquals('0.1', $this->packageInfo->getVersion('C'));
10183
$this->assertEquals('0.3', $this->packageInfo->getVersion('D'));
10284
$this->assertEquals('0.4', $this->packageInfo->getVersion('E'));
85+
$this->assertEquals('', $this->packageInfo->getVersion('F'));
10386
}
10487
}

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

Lines changed: 13 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,9 @@ private function load()
8484
foreach ($rawData as $moduleName => $jsonData) {
8585
$jsonData = \Zend_Json::decode($jsonData);
8686
$this->packageModuleMap[$jsonData['name']] = $moduleName;
87-
$this->modulePackageVersionMap[$moduleName] = $jsonData['version'];
87+
if (isset($jsonData['version'])) {
88+
$this->modulePackageVersionMap[$moduleName] = $jsonData['version'];
89+
}
8890
if (!empty($jsonData['require'])) {
8991
$this->requireMap[$moduleName] = array_keys($jsonData['require']);
9092
}
@@ -123,46 +125,36 @@ private function convertToModuleNames($packageNames)
123125
}
124126

125127
/**
126-
* Get all package names a module requires
128+
* Get all module names a module requires
127129
*
128130
* @param string $moduleName
129-
* @param bool $returnModuleName
130131
* @return array
131132
*/
132-
public function getRequire($moduleName, $returnModuleName = true)
133+
public function getRequire($moduleName)
133134
{
134135
$this->load();
135136
$require = [];
136137
if (isset($this->requireMap[$moduleName])) {
137-
if ($returnModuleName) {
138-
$require = $this->convertToModuleNames($this->requireMap[$moduleName]);
139-
} else {
140-
$require = $this->requireMap[$moduleName];
141-
}
138+
$require = $this->convertToModuleNames($this->requireMap[$moduleName]);
142139
}
143140
return $require;
144141
}
145142

146143
/**
147-
* Get all package names a module conflicts
144+
* Get all module names a module conflicts
148145
*
149146
* @param string $moduleName
150-
* @param bool $returnModuleName
151147
* @return array
152148
*/
153-
public function getConflict($moduleName, $returnModuleName = true)
149+
public function getConflict($moduleName)
154150
{
155151
$this->load();
156152
$conflict = [];
157153
if (isset($this->conflictMap[$moduleName])) {
158-
if ($returnModuleName) {
159-
$conflict = array_combine(
160-
$this->convertToModuleNames(array_keys($this->conflictMap[$moduleName])),
161-
$this->conflictMap[$moduleName]
162-
);
163-
} else {
164-
$conflict = $this->conflictMap[$moduleName];
165-
}
154+
$conflict = array_combine(
155+
$this->convertToModuleNames(array_keys($this->conflictMap[$moduleName])),
156+
$this->conflictMap[$moduleName]
157+
);
166158
}
167159
return $conflict;
168160
}
@@ -176,6 +168,6 @@ public function getConflict($moduleName, $returnModuleName = true)
176168
public function getVersion($moduleName)
177169
{
178170
$this->load();
179-
return $this->modulePackageVersionMap[$moduleName];
171+
return isset($this->modulePackageVersionMap[$moduleName]) ? $this->modulePackageVersionMap[$moduleName] : '';
180172
}
181173
}

0 commit comments

Comments
 (0)