Skip to content

Commit b9e23f6

Browse files
author
Oleksii Korshenko
authored
MAGETWO-88256: [Forwardport] Ensure DeploymentConfig Reader always returns an array #13792
2 parents 6a987dd + f8b86ac commit b9e23f6

File tree

3 files changed

+111
-9
lines changed

3 files changed

+111
-9
lines changed

lib/internal/Magento/Framework/App/DeploymentConfig/Reader.php

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@
99
use Magento\Framework\App\Filesystem\DirectoryList;
1010
use Magento\Framework\Config\File\ConfigFilePool;
1111
use Magento\Framework\Exception\FileSystemException;
12+
use Magento\Framework\Exception\RuntimeException;
1213
use Magento\Framework\Filesystem\DriverPool;
14+
use Magento\Framework\Phrase;
1315

1416
/**
1517
* Deployment configuration reader.
@@ -87,6 +89,7 @@ public function getFiles()
8789
* @param string $fileKey The file key (deprecated)
8890
* @return array
8991
* @throws FileSystemException If file can not be read
92+
* @throws RuntimeException If file is invalid
9093
* @throws \Exception If file key is not correct
9194
* @see FileReader
9295
*/
@@ -99,6 +102,9 @@ public function load($fileKey = null)
99102
$filePath = $path . '/' . $this->configFilePool->getPath($fileKey);
100103
if ($fileDriver->isExists($filePath)) {
101104
$result = include $filePath;
105+
if (!is_array($result)) {
106+
throw new RuntimeException(new Phrase("Invalid configuration file: '%1'", [$filePath]));
107+
}
102108
}
103109
} else {
104110
$configFiles = $this->configFilePool->getPaths();
@@ -108,11 +114,14 @@ public function load($fileKey = null)
108114
$configFile = $path . '/' . $this->configFilePool->getPath($fileKey);
109115
if ($fileDriver->isExists($configFile)) {
110116
$fileData = include $configFile;
117+
if (!is_array($fileData)) {
118+
throw new RuntimeException(new Phrase("Invalid configuration file: '%1'", [$configFile]));
119+
}
111120
} else {
112121
continue;
113122
}
114123
$allFilesData[$configFile] = $fileData;
115-
if (is_array($fileData) && count($fileData) > 0) {
124+
if ($fileData) {
116125
$result = array_replace_recursive($result, $fileData);
117126
}
118127
}

lib/internal/Magento/Framework/App/Test/Unit/DeploymentConfig/ReaderTest.php

Lines changed: 95 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,26 +8,29 @@
88

99
use Magento\Framework\App\DeploymentConfig\Reader;
1010
use Magento\Framework\App\Filesystem\DirectoryList;
11+
use Magento\Framework\Config\File\ConfigFilePool;
12+
use Magento\Framework\Filesystem\Driver\File;
13+
use Magento\Framework\Filesystem\DriverPool;
1114

1215
class ReaderTest extends \PHPUnit\Framework\TestCase
1316
{
1417
/**
15-
* @var \PHPUnit_Framework_MockObject_MockObject
18+
* @var \Magento\Framework\App\Filesystem\DirectoryList|\PHPUnit_Framework_MockObject_MockObject
1619
*/
1720
private $dirList;
1821

1922
/**
20-
* @var \Magento\Framework\Filesystem\DriverPool|\PHPUnit_Framework_MockObject_MockObject
23+
* @var DriverPool|\PHPUnit_Framework_MockObject_MockObject
2124
*/
2225
private $driverPool;
2326

2427
/**
25-
* @var \Magento\Framework\Filesystem\Driver\File|\PHPUnit_Framework_MockObject_MockObject
28+
* @var File|\PHPUnit_Framework_MockObject_MockObject
2629
*/
2730
private $fileDriver;
2831

2932
/**
30-
* @var \PHPUnit_Framework_MockObject_MockObject
33+
* @var ConfigFilePool|\PHPUnit_Framework_MockObject_MockObject
3134
*/
3235
private $configFilePool;
3336

@@ -38,7 +41,7 @@ protected function setUp()
3841
->method('getPath')
3942
->with(DirectoryList::CONFIG)
4043
->willReturn(__DIR__ . '/_files');
41-
$this->fileDriver = $this->createMock(\Magento\Framework\Filesystem\Driver\File::class);
44+
$this->fileDriver = $this->createMock(File::class);
4245
$this->fileDriver
4346
->expects($this->any())
4447
->method('isExists')
@@ -51,12 +54,12 @@ protected function setUp()
5154
[__DIR__ . '/_files/mergeTwo.php', true],
5255
[__DIR__ . '/_files/nonexistent.php', false]
5356
]));
54-
$this->driverPool = $this->createMock(\Magento\Framework\Filesystem\DriverPool::class);
57+
$this->driverPool = $this->createMock(DriverPool::class);
5558
$this->driverPool
5659
->expects($this->any())
5760
->method('getDriver')
5861
->willReturn($this->fileDriver);
59-
$this->configFilePool = $this->createMock(\Magento\Framework\Config\File\ConfigFilePool::class);
62+
$this->configFilePool = $this->createMock(ConfigFilePool::class);
6063
$this->configFilePool
6164
->expects($this->any())
6265
->method('getPaths')
@@ -100,13 +103,97 @@ public function testLoad()
100103
*/
101104
public function testCustomLoad($file, $expected)
102105
{
103-
$configFilePool = $this->createMock(\Magento\Framework\Config\File\ConfigFilePool::class);
106+
$configFilePool = $this->createMock(ConfigFilePool::class);
104107
$configFilePool->expects($this->any())->method('getPaths')->willReturn([$file]);
105108
$configFilePool->expects($this->any())->method('getPath')->willReturn($file);
106109
$object = new Reader($this->dirList, $this->driverPool, $configFilePool, $file);
107110
$this->assertSame($expected, $object->load($file));
108111
}
109112

113+
/**
114+
* Test Reader::load() will throw exception in case of invalid configuration file(single file).
115+
*
116+
* @expectedException \Magento\Framework\Exception\RuntimeException
117+
* @expectedExceptionMessageRegExp /Invalid configuration file: \'.*\/\_files\/emptyConfig\.php\'/
118+
* @return void
119+
*/
120+
public function testLoadInvalidConfigurationFileWithFileKey()
121+
{
122+
$fileDriver = $this->getMockBuilder(File::class)
123+
->disableOriginalConstructor()
124+
->getMock();
125+
$fileDriver->expects($this->once())
126+
->method('isExists')
127+
->willReturn(true);
128+
/** @var DriverPool|\PHPUnit_Framework_MockObject_MockObject $driverPool */
129+
$driverPool = $this->getMockBuilder(DriverPool::class)
130+
->disableOriginalConstructor()
131+
->getMock();
132+
$driverPool
133+
->expects($this->once())
134+
->method('getDriver')
135+
->willReturn($fileDriver);
136+
/** @var ConfigFilePool|\PHPUnit_Framework_MockObject_MockObject $configFilePool */
137+
$configFilePool = $this->getMockBuilder(ConfigFilePool::class)
138+
->disableOriginalConstructor()
139+
->getMock();
140+
$configFilePool
141+
->expects($this->once())
142+
->method('getPath')
143+
->with($this->identicalTo('testConfig'))
144+
->willReturn('emptyConfig.php');
145+
$object = new Reader($this->dirList, $driverPool, $configFilePool);
146+
$object->load('testConfig');
147+
}
148+
149+
/**
150+
* Test Reader::load() will throw exception in case of invalid configuration file(multiple files).
151+
*
152+
* @expectedException \Magento\Framework\Exception\RuntimeException
153+
* @expectedExceptionMessageRegExp /Invalid configuration file: \'.*\/\_files\/emptyConfig\.php\'/
154+
* @return void
155+
*/
156+
public function testLoadInvalidConfigurationFile()
157+
{
158+
$fileDriver = $this->getMockBuilder(File::class)
159+
->disableOriginalConstructor()
160+
->getMock();
161+
$fileDriver->expects($this->exactly(2))
162+
->method('isExists')
163+
->willReturn(true);
164+
/** @var DriverPool|\PHPUnit_Framework_MockObject_MockObject $driverPool */
165+
$driverPool = $this->getMockBuilder(DriverPool::class)
166+
->disableOriginalConstructor()
167+
->getMock();
168+
$driverPool
169+
->expects($this->once())
170+
->method('getDriver')
171+
->willReturn($fileDriver);
172+
/** @var ConfigFilePool|\PHPUnit_Framework_MockObject_MockObject $configFilePool */
173+
$configFilePool = $this->getMockBuilder(ConfigFilePool::class)
174+
->disableOriginalConstructor()
175+
->getMock();
176+
$configFilePool->expects($this->exactly(2))
177+
->method('getPaths')
178+
->willReturn(
179+
[
180+
'configKeyOne' => 'config.php',
181+
'testConfig' => 'emptyConfig.php'
182+
]
183+
);
184+
$configFilePool->expects($this->exactly(2))
185+
->method('getPath')
186+
->withConsecutive(
187+
[$this->identicalTo('configKeyOne')],
188+
[$this->identicalTo('testConfig')]
189+
)->willReturnOnConsecutiveCalls(
190+
'config.php',
191+
'emptyConfig.php'
192+
);
193+
$object = new Reader($this->dirList, $driverPool, $configFilePool);
194+
$object->load();
195+
}
196+
110197
/**
111198
* @return array
112199
*/
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
//Example of wrong(empty) configuration file.

0 commit comments

Comments
 (0)