Skip to content

Commit 42dfc8b

Browse files
author
Joan He
committed
MAGETWO-29705: Fixed confusing Filesystem Read/Write Interfaces
1 parent 2f89843 commit 42dfc8b

File tree

15 files changed

+88
-156
lines changed

15 files changed

+88
-156
lines changed

‎app/code/Magento/Backup/Model/Backup.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
namespace Magento\Backup\Model;
77

88
use Magento\Framework\App\Filesystem\DirectoryList;
9+
use Magento\Framework\Filesystem\DriverPool;
910

1011
/**
1112
* Backup file item model
@@ -292,11 +293,10 @@ public function open($write = false)
292293

293294
try {
294295
/** @var \Magento\Framework\Filesystem\Directory\WriteInterface $varDirectory */
295-
$varDirectory = $this->_filesystem->getDirectoryWrite(DirectoryList::VAR_DIR);
296+
$varDirectory = $this->_filesystem->getDirectoryWrite(DirectoryList::VAR_DIR, DriverPool::ZLIB);
296297
$this->_stream = $varDirectory->openFile(
297298
$this->_getFilePath(),
298-
$mode,
299-
\Magento\Framework\Filesystem\DriverPool::ZLIB
299+
$mode
300300
);
301301
} catch (\Magento\Framework\Filesystem\FilesystemException $e) {
302302
throw new \Magento\Framework\Backup\Exception\NotEnoughPermissions(

‎app/code/Magento/Downloadable/Helper/Download.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ protected function _getHandle()
168168
// Strip down protocol from path
169169
$path = preg_replace('#.+://#', '', $path);
170170
}
171-
$this->_handle = $this->fileReadFactory->create($path, $protocol);
171+
$this->_handle = $this->fileReadFactory->createWithDriverCode($path, $protocol);
172172
} elseif ($this->_linkType == self::LINK_TYPE_FILE) {
173173
$this->_workingDirectory = $this->_filesystem->getDirectoryRead(DirectoryList::MEDIA);
174174
$fileExists = $this->_downloadableFile->ensureFileInFilesystem($this->_resourceFile);

‎dev/tests/unit/testsuite/Magento/Downloadable/Helper/DownloadTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ protected function _setupUrlMocks($size = self::FILE_SIZE, $url = self::URL, $ad
226226
$this->fileReadFactory->expects(
227227
$this->once()
228228
)->method(
229-
'create'
229+
'createWithDriverCode'
230230
)->will(
231231
$this->returnValue($this->_handleMock)
232232
);

‎dev/tests/unit/testsuite/Magento/Framework/App/View/Asset/PublisherTest.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
namespace Magento\Framework\App\View\Asset;
88

99
use Magento\Framework\App\Filesystem\DirectoryList;
10+
use Magento\Framework\Filesystem\DriverPool;
1011

1112
class PublisherTest extends \PHPUnit_Framework_TestCase
1213
{
@@ -56,8 +57,8 @@ protected function setUp()
5657
$this->filesystem->expects($this->any())
5758
->method('getDirectoryWrite')
5859
->will($this->returnValueMap([
59-
[DirectoryList::ROOT, $this->rootDirWrite],
60-
[DirectoryList::STATIC_VIEW, $this->staticDirWrite],
60+
[DirectoryList::ROOT, DriverPool::FILE, $this->rootDirWrite],
61+
[DirectoryList::STATIC_VIEW, DriverPool::FILE, $this->staticDirWrite],
6162
]));
6263
}
6364

‎dev/tests/unit/testsuite/Magento/Framework/Filesystem/Directory/ReadTest.php

Lines changed: 1 addition & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ public function testStat()
7777
$this->assertEquals(['some-stat-data'], $this->read->stat('correct-path'));
7878
}
7979

80-
public function testReadFileNoProtocol()
80+
public function testReadFile()
8181
{
8282
$path = 'filepath';
8383
$flag = 'flag';
@@ -95,32 +95,4 @@ public function testReadFileNoProtocol()
9595

9696
$this->assertEquals($contents, $this->read->readFile($path, $flag, $context));
9797
}
98-
99-
public function testReadFileCustomProtocol()
100-
{
101-
$path = 'filepath';
102-
$flag = 'flag';
103-
$context = 'context';
104-
$protocol = 'ftp';
105-
$contents = 'contents';
106-
107-
$fileMock = $this->getMock('Magento\Framework\Filesystem\File\Read', [], [], '', false);
108-
$fileMock->expects($this->once())
109-
->method('readAll')
110-
->with($flag, $context)
111-
->will($this->returnValue($contents));
112-
113-
$this->driver->expects($this->once())
114-
->method('getAbsolutePath')
115-
->with($this->path, $path, $protocol)
116-
->will($this->returnValue($path));
117-
$this->driver->expects($this->never())
118-
->method('fileGetContents');
119-
$this->fileFactory->expects($this->once())
120-
->method('create')
121-
->with($path, $protocol, $this->driver)
122-
->will($this->returnValue($fileMock));
123-
124-
$this->assertEquals($contents, $this->read->readFile($path, $flag, $context, $protocol));
125-
}
12698
}

‎dev/tests/unit/testsuite/Magento/Framework/Filesystem/File/ReadFactoryTest.php

Lines changed: 13 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -12,45 +12,25 @@
1212
*/
1313
class ReadFactoryTest extends \PHPUnit_Framework_TestCase
1414
{
15-
/**
16-
* @param string|null $protocol
17-
* @param \PHPUnit_Framework_MockObject_MockObject|null $driver
18-
* @dataProvider createProvider
19-
*/
20-
public function testCreate($protocol, $driver)
15+
public function testCreate()
2116
{
2217
$driverPool = $this->getMock('Magento\Framework\Filesystem\DriverPool', ['getDriver']);
23-
if ($protocol) {
24-
$driverMock = $this->getMockForAbstractClass('Magento\Framework\Filesystem\DriverInterface');
25-
$driverMock->expects($this->any())->method('isExists')->willReturn(true);
26-
$driverPool->expects($this->once())->method('getDriver')->willReturn($driverMock);
27-
} else {
28-
$driverPool->expects($this->never())->method('getDriver');
29-
}
30-
$factory = new ReadFactory($driverPool);
31-
$result = $factory->create('path', $protocol, $driver);
32-
$this->assertInstanceOf('Magento\Framework\Filesystem\File\Read', $result);
33-
}
34-
35-
/**
36-
* @return array
37-
*/
38-
public function createProvider()
39-
{
18+
$driverPool->expects($this->never())->method('getDriver');
4019
$driver = $this->getMockForAbstractClass('Magento\Framework\Filesystem\DriverInterface');
4120
$driver->expects($this->any())->method('isExists')->willReturn(true);
42-
return [
43-
[null, $driver],
44-
['custom_protocol', null]
45-
];
21+
$factory = new ReadFactory($driverPool);
22+
$result = $factory->create('path', $driver);
23+
$this->assertInstanceOf('Magento\Framework\Filesystem\File\Read', $result);
4624
}
4725

48-
/**
49-
* @expectedException \InvalidArgumentException
50-
*/
51-
public function testCreateException()
26+
public function testCreateWithDriverCode()
5227
{
53-
$factory = new ReadFactory(new DriverPool());
54-
$factory->create('path');
28+
$driverPool = $this->getMock('Magento\Framework\Filesystem\DriverPool', ['getDriver']);
29+
$driverMock = $this->getMockForAbstractClass('Magento\Framework\Filesystem\DriverInterface');
30+
$driverMock->expects($this->any())->method('isExists')->willReturn(true);
31+
$driverPool->expects($this->once())->method('getDriver')->willReturn($driverMock);
32+
$factory = new ReadFactory($driverPool);
33+
$result = $factory->createWithDriverCode('path', 'driverCode');
34+
$this->assertInstanceOf('Magento\Framework\Filesystem\File\Read', $result);
5535
}
5636
}

‎dev/tests/unit/testsuite/Magento/Framework/Filesystem/File/WriteFactoryTest.php

Lines changed: 16 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -5,63 +5,41 @@
55
*/
66
namespace Magento\Framework\Filesystem\File;
77

8-
use Magento\Framework\Filesystem\DriverPool;
9-
108
/**
119
* Class WriteFactoryTest
1210
*/
1311
class WriteFactoryTest extends \PHPUnit_Framework_TestCase
1412
{
15-
/**
16-
* @param string|null $protocol
17-
* @param \PHPUnit_Framework_MockObject_MockObject|null $driver
18-
* @dataProvider createProvider
19-
*/
20-
public function testCreate($protocol, $driver)
13+
public function testCreate()
2114
{
2215
$driverPool = $this->getMock('Magento\Framework\Filesystem\DriverPool', ['getDriver']);
23-
if ($protocol) {
24-
$driverMock = $this->getMockForAbstractClass('Magento\Framework\Filesystem\DriverInterface');
25-
$driverMock->expects($this->any())->method('isExists')->willReturn(true);
26-
$driverPool->expects($this->once())->method('getDriver')->willReturn($driverMock);
27-
} else {
28-
$driverPool->expects($this->never())->method('getDriver');
29-
}
16+
$driverPool->expects($this->never())->method('getDriver');
3017
$factory = new WriteFactory($driverPool);
31-
$result = $factory->create('path', $protocol, $driver);
32-
$this->assertInstanceOf('Magento\Framework\Filesystem\File\Write', $result);
33-
}
34-
35-
/**
36-
* @return array
37-
*/
38-
public function createProvider()
39-
{
4018
$driver = $this->getMockForAbstractClass('Magento\Framework\Filesystem\DriverInterface');
4119
$driver->expects($this->any())->method('isExists')->willReturn(true);
42-
return [
43-
[null, $driver],
44-
['custom_protocol', null]
45-
];
20+
$result = $factory->create('path', $driver);
21+
$this->assertInstanceOf('Magento\Framework\Filesystem\File\Write', $result);
4622
}
4723

48-
/**
49-
* @expectedException \InvalidArgumentException
50-
*/
51-
public function testCreateException()
24+
public function testCreateWithDriverCode()
5225
{
53-
$factory = new WriteFactory(new DriverPool());
54-
$factory->create('path');
26+
$driverPool = $this->getMock('Magento\Framework\Filesystem\DriverPool', ['getDriver']);
27+
$driverMock = $this->getMockForAbstractClass('Magento\Framework\Filesystem\DriverInterface');
28+
$driverMock->expects($this->any())->method('isExists')->willReturn(true);
29+
$driverPool->expects($this->once())->method('getDriver')->willReturn($driverMock);
30+
$factory = new WriteFactory($driverPool);
31+
$result = $factory->createWithDriverCode('path', 'driverCode');
32+
$this->assertInstanceOf('Magento\Framework\Filesystem\File\Write', $result);
5533
}
5634

5735
public function testCreateWithMode()
5836
{
59-
$driver = $this->getMockForAbstractClass('Magento\Framework\Filesystem\DriverInterface');
60-
$driver->expects($this->any())->method('isExists')->willReturn(false);
6137
$driverPool = $this->getMock('Magento\Framework\Filesystem\DriverPool', ['getDriver']);
62-
$driverPool->expects($this->once())->method('getDriver')->with('protocol')->willReturn($driver);
38+
$driverPool->expects($this->never())->method('getDriver');
39+
$driver = $this->getMockForAbstractClass('Magento\Framework\Filesystem\DriverInterface');
40+
$driver->expects($this->any())->method('isExists')->willReturn(true);
6341
$factory = new WriteFactory($driverPool);
64-
$result = $factory->create('path', 'protocol', null, 'a+');
42+
$result = $factory->create('path', $driver, 'a+');
6543
$this->assertInstanceOf('Magento\Framework\Filesystem\File\Write', $result);
6644
}
6745
}

‎lib/internal/Magento/Framework/Filesystem.php

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*/
88
namespace Magento\Framework;
99

10-
use Magento\Framework\Filesystem\File\ReadInterface;
10+
use Magento\Framework\Filesystem\DriverPool;
1111

1212
class Filesystem
1313
{
@@ -68,14 +68,16 @@ public function getDirectoryRead($code)
6868
/**
6969
* Create an instance of directory with read permissions
7070
*
71-
* @param string $code
71+
* @param string $directoryCode
72+
* @param string $driverCode
7273
* @return \Magento\Framework\Filesystem\Directory\WriteInterface
7374
* @throws \Magento\Framework\Filesystem\FilesystemException
7475
*/
75-
public function getDirectoryWrite($code)
76+
public function getDirectoryWrite($directoryCode, $driverCode = DriverPool::FILE)
7677
{
78+
$code = $directoryCode . '_' . $driverCode;
7779
if (!array_key_exists($code, $this->writeInstances)) {
78-
$this->writeInstances[$code] = $this->writeFactory->create($this->getDirPath($code));
80+
$this->writeInstances[$code] = $this->writeFactory->create($this->getDirPath($directoryCode), $driverCode);
7981
}
8082
return $this->writeInstances[$code];
8183
}

‎lib/internal/Magento/Framework/Filesystem/Directory/Read.php

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -181,15 +181,13 @@ public function isReadable($path = null)
181181
* Open file in read mode
182182
*
183183
* @param string $path
184-
* @param string|null $protocol
185184
*
186185
* @return \Magento\Framework\Filesystem\File\ReadInterface
187186
*/
188-
public function openFile($path, $protocol = null)
187+
public function openFile($path)
189188
{
190189
return $this->fileFactory->create(
191190
$this->driver->getAbsolutePath($this->path, $path),
192-
$protocol,
193191
$this->driver
194192
);
195193
}
@@ -200,21 +198,14 @@ public function openFile($path, $protocol = null)
200198
* @param string $path
201199
* @param string|null $flag
202200
* @param resource|null $context
203-
* @param string|null $protocol
204201
* @return string
205202
* @throws FilesystemException
206203
*/
207-
public function readFile($path, $flag = null, $context = null, $protocol = null)
204+
public function readFile($path, $flag = null, $context = null)
208205
{
209-
$absolutePath = $this->driver->getAbsolutePath($this->path, $path, $protocol);
206+
$absolutePath = $this->driver->getAbsolutePath($this->path, $path);
207+
return $this->driver->fileGetContents($absolutePath, $flag, $context);
210208

211-
if (is_null($protocol)) {
212-
return $this->driver->fileGetContents($absolutePath, $flag, $context);
213-
}
214-
215-
/** @var \Magento\Framework\Filesystem\File\Read $fileReader */
216-
$fileReader = $this->fileFactory->create($absolutePath, $protocol, $this->driver);
217-
return $fileReader->readAll($flag, $context);
218209
}
219210

220211
/**

‎lib/internal/Magento/Framework/Filesystem/Directory/ReadFactory.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,12 @@ public function __construct(DriverPool $driverPool)
3030
* Create a readable directory
3131
*
3232
* @param string $path
33-
* @param string $protocol
33+
* @param string $driverCode
3434
* @return ReadInterface
3535
*/
36-
public function create($path, $protocol = DriverPool::FILE)
36+
public function create($path, $driverCode = DriverPool::FILE)
3737
{
38-
$driver = $this->driverPool->getDriver($protocol);
38+
$driver = $this->driverPool->getDriver($driverCode);
3939
$factory = new \Magento\Framework\Filesystem\File\ReadFactory($this->driverPool);
4040
return new Read($factory, $driver, $path);
4141
}

0 commit comments

Comments
 (0)