From d948d423f88d1d378a1d8a540e0ff478f8d82728 Mon Sep 17 00:00:00 2001 From: Ivan Chepurnyi Date: Sun, 2 Apr 2017 21:56:38 -0700 Subject: [PATCH 01/17] Add file parser interface to support various import formats, created CSV parser implementation --- .../Model/Import/Source/CsvFileParser.php | 136 ++++++++++++++ .../Model/Import/Source/FileParserFactory.php | 114 ++++++++++++ .../Import/Source/FileParserInterface.php | 34 ++++ .../Model/Import/Source/CsvFileParserTest.php | 156 ++++++++++++++++ .../Import/Source/FileParserFactoryTest.php | 172 ++++++++++++++++++ .../Source/_files/custom_delimiter_data.csv | 5 + .../Import/Source/_files/simple_data.csv | 5 + .../Source/_files/uneven_row_values_data.csv | 5 + 8 files changed, 627 insertions(+) create mode 100644 app/code/Magento/ImportExport/Model/Import/Source/CsvFileParser.php create mode 100644 app/code/Magento/ImportExport/Model/Import/Source/FileParserFactory.php create mode 100644 app/code/Magento/ImportExport/Model/Import/Source/FileParserInterface.php create mode 100644 app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/CsvFileParserTest.php create mode 100644 app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParserFactoryTest.php create mode 100644 app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/_files/custom_delimiter_data.csv create mode 100644 app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/_files/simple_data.csv create mode 100644 app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/_files/uneven_row_values_data.csv diff --git a/app/code/Magento/ImportExport/Model/Import/Source/CsvFileParser.php b/app/code/Magento/ImportExport/Model/Import/Source/CsvFileParser.php new file mode 100644 index 0000000000000..a954c496060bc --- /dev/null +++ b/app/code/Magento/ImportExport/Model/Import/Source/CsvFileParser.php @@ -0,0 +1,136 @@ + + */ + + +namespace Magento\ImportExport\Model\Import\Source; + + +use Magento\Framework\App\Filesystem\DirectoryList; +use Magento\Framework\Filesystem; + +class CsvFileParser implements FileParserInterface +{ + /** + * Enclosure for values in CSV file + * + * @var string + */ + private $enclosure; + + /** + * Column delimiter + * + * @var string + */ + private $delimiter; + + /** + * Escape sequence character + * + * @var string + */ + private $escape; + + /** + * Null character for parsing value + * + * @var string + */ + private $nullPlaceholder; + + /** + * File reader + * + * @var Filesystem\File\ReadInterface + */ + private $file; + + /** + * Columns of CSV file + * + * @var string[] + */ + private $columns; + + public function __construct( + $filePath, + Filesystem $filesystem, + $directoryCode = DirectoryList::ROOT, + $nullPlaceholder = null, + $enclosure = '"', + $delimiter = ',', + $escape = '\\' + ) { + $this->nullPlaceholder = $nullPlaceholder; + $this->enclosure = $enclosure; + $this->delimiter = $delimiter; + $this->escape = $escape; + + $directory = $filesystem->getDirectoryRead($directoryCode); + if (!$directory->isFile($filePath)) { + throw new \InvalidArgumentException( + sprintf('File "%s" does not exists', $filePath) + ); + } + + $this->file = $directory->openFile($filePath); + $this->columns = $this->fetchCsvLine(); + } + + public function getColumnNames() + { + return $this->columns; + } + + public function fetchRow() + { + $row = $this->fetchCsvLine(); + + if ($row === false) { + return false; + } + + return $this->mapRowData($row); + } + + public function reset() + { + $this->file->seek(0); + $this->fetchCsvLine(); + } + + private function fetchCsvLine() + { + return $this->file->readCsv(0, $this->delimiter, $this->enclosure, $this->escape); + } + + private function mapRowData($row) + { + $result = []; + foreach ($this->columns as $index => $column) { + $value = isset($row[$index]) ? $row[$index] : ''; + + if ($this->nullPlaceholder && $value === $this->nullPlaceholder) { + $value = null; + } + + $result[] = $value; + } + return $result; + } + +} diff --git a/app/code/Magento/ImportExport/Model/Import/Source/FileParserFactory.php b/app/code/Magento/ImportExport/Model/Import/Source/FileParserFactory.php new file mode 100644 index 0000000000000..e3f73acd6103c --- /dev/null +++ b/app/code/Magento/ImportExport/Model/Import/Source/FileParserFactory.php @@ -0,0 +1,114 @@ + + */ + + +namespace Magento\ImportExport\Model\Import\Source; + + +use Magento\Framework\ObjectManagerInterface; + +/** + * Factory for creation of multiple products + * + */ +class FileParserFactory +{ + /** + * File parser creation map + * + * @var string[][] + */ + private $parserMap = []; + + /** + * @var ObjectManagerInterface + */ + private $objectManager; + + + public function __construct(ObjectManagerInterface $objectManager, array $parserMap) + { + $this->objectManager = $objectManager; + foreach ($parserMap as $item) { + $this->addParserMap($item); + } + } + + /** + * Creates an instance of file parser from file path + * + * @param string $filePath + * @param array $options + * + * @return FileParserInterface + */ + public function create($filePath, $options = []) + { + $extension = $this->extractFileExtension($filePath); + + $this->assertFileExtensionIsSupported($filePath, $extension); + + $parserInfo = $this->parserMap[$extension]; + + $arguments = [ + $parserInfo['argument'] => $filePath + ]; + + $arguments += $options; + + return $this->objectManager->create( + $parserInfo['class'], + $arguments + ); + } + + private function addParserMap($parserMap) + { + $this->assertParserMap($parserMap); + $this->parserMap[$parserMap['extension']] = [ + 'class' => $parserMap['class'], + 'argument' => $parserMap['argument'] + ]; + } + + private function extractFileExtension($filePath) + { + return substr($filePath, strrpos($filePath, '.') + 1, strlen($filePath)); + } + + private function assertParserMap($mapItem) + { + if (empty($mapItem['extension'])) { + throw new \InvalidArgumentException('Missing extension in parser definition'); + } + if (empty($mapItem['class'])) { + throw new \InvalidArgumentException('Missing class in parser definition'); + } + if (empty($mapItem['argument'])) { + throw new \InvalidArgumentException('Missing argument in parser definition'); + } + } + + private function assertFileExtensionIsSupported($filePath, $extension) + { + if (!isset($this->parserMap[$extension])) { + throw new \InvalidArgumentException( + sprintf('File "%s" is an invalid format', $filePath) + ); + } + } +} diff --git a/app/code/Magento/ImportExport/Model/Import/Source/FileParserInterface.php b/app/code/Magento/ImportExport/Model/Import/Source/FileParserInterface.php new file mode 100644 index 0000000000000..141cbc3cb635f --- /dev/null +++ b/app/code/Magento/ImportExport/Model/Import/Source/FileParserInterface.php @@ -0,0 +1,34 @@ + + */ + + +namespace Magento\ImportExport\Test\Unit\Model\Import\Source; + +use Magento\Framework\App\Filesystem\DirectoryList; +use Magento\Framework\Filesystem; +use Magento\ImportExport\Model\Import\Source\CsvFileParser; +use Magento\Setup\Module\Di\Code\Reader\Decorator\Directory; + +class CsvFileParserTest extends \PHPUnit_Framework_TestCase +{ + /** + * @test + * @expectedException \InvalidArgumentException + * @expectedExceptionMessage File "non_existing_file.csv" does not exists + */ + public function when_file_does_not_exist_it_throws_InvalidArgumentException() + { + new CsvFileParser('non_existing_file.csv', $this->createFileSystemStub()); + } + + /** + * @test + */ + public function when_valid_file_is_provided_it_reads_columns_from_file_header() + { + $parser = new CsvFileParser('simple_data.csv', $this->createFileSystemStub()); + + $this->assertSame(['column1', 'column2', 'column3'], $parser->getColumnNames()); + } + + /** + * @test + */ + public function when_valid_file_is_provided_it_reads_rows_and_skips_header() + { + $parser = new CsvFileParser('simple_data.csv', $this->createFileSystemStub()); + + $this->assertSame(['row1value1', 'row1value2', 'row1value3'], $parser->fetchRow()); + $this->assertSame(['row2value1', 'row2value2', 'row2value3'], $parser->fetchRow()); + } + + /** + * @test + */ + public function when_end_of_file_is_reached_it_returns_false() + { + $parser = new CsvFileParser('simple_data.csv', $this->createFileSystemStub()); + + $this->skipLines($parser, 4); + + $this->assertSame(false, $parser->fetchRow()); + } + + /** + * @test + */ + public function when_file_was_already_read_it_is_possible_to_read_it_again() + { + $parser = new CsvFileParser('simple_data.csv', $this->createFileSystemStub()); + + $this->skipLines($parser, 3); + + $parser->reset(); + + $this->assertSame(['row1value1', 'row1value2', 'row1value3'], $parser->fetchRow()); + $this->assertSame(['row2value1', 'row2value2', 'row2value3'], $parser->fetchRow()); + } + + /** + * @test + */ + public function when_custom_delimiter_is_specified_it_uses_it_for_parsing() + { + $parser = new CsvFileParser( + 'custom_delimiter_data.csv', + $this->createFileSystemStub(), + DirectoryList::ROOT, + null, + '"', + '|' + ); + + $this->assertSame(['row1value1', 'row1value2', 'row1value3'], $parser->fetchRow()); + } + + /** + * @test + */ + public function when_file_has_missing_row_values_it_returns_same_number_as_columns() + { + $parser = new CsvFileParser( + 'uneven_row_values_data.csv', + $this->createFileSystemStub() + ); + + $this->assertSame(['row1value1', '', ''], $parser->fetchRow()); + $this->assertSame(['row2value1', 'row2value2', 'row2value3'], $parser->fetchRow()); + $this->assertSame(['row3value1', 'row3value2', ''], $parser->fetchRow()); + $this->assertSame(['row4value1', 'row4value2', 'row4value3'], $parser->fetchRow()); + } + + /** + * @test + */ + public function when_null_placeholder_is_set_it_replaces_it_with_native_null_value() + { + $parser = new CsvFileParser( + 'simple_data.csv', + $this->createFileSystemStub(), + DirectoryList::ROOT, + 'row2value2' + ); + + $this->assertSame(['row1value1', 'row1value2', 'row1value3'], $parser->fetchRow()); + $this->assertSame(['row2value1', null, 'row2value3'], $parser->fetchRow()); + } + + private function createFileSystemStub() + { + $fileSystemStub = $this->getMockBuilder(Filesystem::class) + ->disableOriginalConstructor() + ->setMethods(['getDirectoryRead']) + ->getMock(); + + $readDirectoryFactory = new Filesystem\Directory\ReadFactory(new Filesystem\DriverPool()); + + $fileSystemStub->method('getDirectoryRead') + ->willReturn($readDirectoryFactory->create(__DIR__ . '/_files')); + + return $fileSystemStub; + } + + private function skipLines(CsvFileParser $parser, $numberOfLines) + { + for ($i = 0; $i < $numberOfLines; $i++) { + $parser->fetchRow(); + } + } +} diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParserFactoryTest.php b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParserFactoryTest.php new file mode 100644 index 0000000000000..55764534e5328 --- /dev/null +++ b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParserFactoryTest.php @@ -0,0 +1,172 @@ +createFileParserFactory([[]]); + } + + /** + * @expectedException \InvalidArgumentException + * @expectedExceptionMessage Missing class in parser definition + * @test + */ + public function when_class_is_empty_it_throws_InvalidArgumentException() + { + $this->createFileParserFactory([ + ['extension' => 'csv'] + ]); + } + + /** + * @expectedException \InvalidArgumentException + * @expectedExceptionMessage Missing argument in parser definition + * @test + */ + public function when_argument_is_empty_it_throws_InvalidArgumentException() + { + $this->createFileParserFactory([ + [ + 'extension' => 'csv', + 'class' => 'SomeClass' + ] + ]); + } + + + /** + * @expectedException \InvalidArgumentException + * @expectedExceptionMessage File "some/random/file-path.unknowntype" is an invalid format + * @test + */ + public function when_non_existing_file_type_is_provided_it_throws_InvalidArgumentException() + { + $factory = $this->createFileParserFactory( + [ + [ + 'extension' => 'myfile', + 'class' => FileParserInterface::class, + 'argument' => 'file' + ] + ] + ); + + $factory->create('some/random/file-path.unknowntype'); + } + + /** + * @test + */ + public function when_mapped_type_is_provided_it_creates_instance_via_object_manager() + { + $fileParser = $this->getMockForAbstractClass( + FileParserInterface::class, + [], + 'FileParserStub' + ); + + $factory = $this->createFileParserFactory( + [ + $this->createParserMapArray('myfile', 'FileParserStub', 'file') + ], + $this->createMockedObjectManagerForFileParser( + 'FileParserStub', + ['file' => 'some/random/file-path.myfile'], + $fileParser + ) + ); + + $this->assertSame($fileParser, $factory->create('some/random/file-path.myfile')); + } + + /** + * @test + */ + public function when_additional_options_are_provided_it_passes_them_to_constructor_of_file_parser() + { + $fileParser = $this->getMockForAbstractClass(FileParserInterface::class); + $fileParserClass = get_class($fileParser); + + + $factory = $this->createFileParserFactory( + [ + $this->createParserMapArray('myfile', $fileParserClass, 'file') + ], + $this->createMockedObjectManagerForFileParser( + $fileParserClass, + [ + 'file' => 'some/random/file-path.myfile', + 'option1' => 'option1', + 'option2' => 'option2' + ], + $fileParser + ) + ); + + $this->assertSame( + $fileParser, + $factory->create( + 'some/random/file-path.myfile', + [ + 'option1' => 'option1', + 'option2' => 'option2' + ] + ) + ); + } + + /** + * @return FileParserFactory + */ + private function createFileParserFactory($typeMap = [], ObjectManagerInterface $objectManager = null) + { + $factory = new FileParserFactory( + $objectManager ?: $this->getMockForAbstractClass(ObjectManagerInterface::class), + $typeMap + ); + return $factory; + } + + /** + * @return ObjectManagerInterface + */ + private function createMockedObjectManagerForFileParser( + $className, + $arguments, + FileParserInterface $fileParser + ) { + $objectManager = $this->getMockForAbstractClass(ObjectManagerInterface::class); + $objectManager->expects($this->once()) + ->method('create') + ->with($className, $arguments) + ->willReturn($fileParser); + + return $objectManager; + } + + private function createParserMapArray($extension, $className, $argumentName) + { + return [ + 'extension' => $extension, + 'class' => $className, + 'argument' => $argumentName + ]; + } +} diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/_files/custom_delimiter_data.csv b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/_files/custom_delimiter_data.csv new file mode 100644 index 0000000000000..d2621cbd78f16 --- /dev/null +++ b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/_files/custom_delimiter_data.csv @@ -0,0 +1,5 @@ +column1|column2|column3 +row1value1|row1value2|row1value3 +row2value1|row2value2|row2value3 +row3value1|row3value2|row3value3 +row4value1|row4value2|row4value3 diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/_files/simple_data.csv b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/_files/simple_data.csv new file mode 100644 index 0000000000000..0375816d3c5ff --- /dev/null +++ b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/_files/simple_data.csv @@ -0,0 +1,5 @@ +column1,column2,column3 +row1value1,row1value2,row1value3 +row2value1,row2value2,row2value3 +row3value1,row3value2,row3value3 +row4value1,row4value2,row4value3 diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/_files/uneven_row_values_data.csv b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/_files/uneven_row_values_data.csv new file mode 100644 index 0000000000000..5822f7ab0312e --- /dev/null +++ b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/_files/uneven_row_values_data.csv @@ -0,0 +1,5 @@ +column1,column2,column3 +row1value1 +row2value1,row2value2,row2value3 +row3value1,row3value2 +row4value1,row4value2,row4value3,row4value4 From 8b8bcaed15a077144ec497388e563bf4cd8bb811 Mon Sep 17 00:00:00 2001 From: Ivan Chepurnyi Date: Sun, 2 Apr 2017 22:00:05 -0700 Subject: [PATCH 02/17] Remove wrong license header and specify proper one --- .../Model/Import/Source/CsvFileParser.php | 16 ++-------------- .../Model/Import/Source/FileParserFactory.php | 16 ++-------------- .../Model/Import/Source/FileParserInterface.php | 4 ++++ .../Model/Import/Source/CsvFileParserTest.php | 15 ++------------- .../Import/Source/FileParserFactoryTest.php | 2 +- 5 files changed, 11 insertions(+), 42 deletions(-) diff --git a/app/code/Magento/ImportExport/Model/Import/Source/CsvFileParser.php b/app/code/Magento/ImportExport/Model/Import/Source/CsvFileParser.php index a954c496060bc..c27f8a571dc9a 100644 --- a/app/code/Magento/ImportExport/Model/Import/Source/CsvFileParser.php +++ b/app/code/Magento/ImportExport/Model/Import/Source/CsvFileParser.php @@ -1,21 +1,9 @@ + * Copyright © Magento, Inc. All rights reserved. + * See COPYING.txt for license details. */ - namespace Magento\ImportExport\Model\Import\Source; diff --git a/app/code/Magento/ImportExport/Model/Import/Source/FileParserFactory.php b/app/code/Magento/ImportExport/Model/Import/Source/FileParserFactory.php index e3f73acd6103c..9a095b17edfb8 100644 --- a/app/code/Magento/ImportExport/Model/Import/Source/FileParserFactory.php +++ b/app/code/Magento/ImportExport/Model/Import/Source/FileParserFactory.php @@ -1,21 +1,9 @@ + * Copyright © Magento, Inc. All rights reserved. + * See COPYING.txt for license details. */ - namespace Magento\ImportExport\Model\Import\Source; diff --git a/app/code/Magento/ImportExport/Model/Import/Source/FileParserInterface.php b/app/code/Magento/ImportExport/Model/Import/Source/FileParserInterface.php index 141cbc3cb635f..dcacc86a0ca95 100644 --- a/app/code/Magento/ImportExport/Model/Import/Source/FileParserInterface.php +++ b/app/code/Magento/ImportExport/Model/Import/Source/FileParserInterface.php @@ -1,4 +1,8 @@ + * Copyright © Magento, Inc. All rights reserved. + * See COPYING.txt for license details. */ diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParserFactoryTest.php b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParserFactoryTest.php index 55764534e5328..4260b0ac28f09 100644 --- a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParserFactoryTest.php +++ b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParserFactoryTest.php @@ -1,7 +1,7 @@ Date: Tue, 2 May 2017 18:26:21 +0200 Subject: [PATCH 03/17] New parser implementation with composite --- .../FileParser/CompositeParserFactory.php | 58 ++++++ .../FileParser/CorruptedFileException.php | 20 ++ .../CsvParser.php} | 78 ++++---- .../Source/FileParser/CsvParserFactory.php | 39 ++++ .../FileParser/ParserFactoryInterface.php | 36 ++++ .../ParserInterface.php} | 4 +- .../FileParser/UnsupportedPathException.php | 21 +++ .../Source/FileParser/ZipParserFactory.php | 40 ++++ .../Model/Import/Source/FileParserFactory.php | 102 ---------- .../Model/Import/Source/CsvFileParserTest.php | 145 --------------- .../FileParser/CompositeParserFactoryTest.php | 66 +++++++ .../FileParser/CorruptedPathExceptionTest.php | 36 ++++ .../FileParser/CsvParserFactoryTest.php | 91 +++++++++ .../Source/FileParser/CsvParserTest.php | 175 ++++++++++++++++++ .../Import/Source/FileParser/FakeFile.php | 89 +++++++++ .../Import/Source/FileParser/FakeParser.php | 50 +++++ .../Source/FileParser/FakeParserFactory.php | 43 +++++ .../UnsupportedPathExceptionTest.php | 36 ++++ .../FileParser/ZipParserFactoryTest.php | 55 ++++++ .../Import/Source/FileParser/_files/test.csv | 1 + .../Source/FileParser/_files/test_options.csv | 1 + .../Source/FileParser/_files/var/tmp/test.csv | 1 + .../Import/Source/FileParserFactoryTest.php | 172 ----------------- .../Source/_files/custom_delimiter_data.csv | 5 - .../Import/Source/_files/simple_data.csv | 5 - .../Source/_files/uneven_row_values_data.csv | 5 - 26 files changed, 906 insertions(+), 468 deletions(-) create mode 100644 app/code/Magento/ImportExport/Model/Import/Source/FileParser/CompositeParserFactory.php create mode 100644 app/code/Magento/ImportExport/Model/Import/Source/FileParser/CorruptedFileException.php rename app/code/Magento/ImportExport/Model/Import/Source/{CsvFileParser.php => FileParser/CsvParser.php} (53%) create mode 100644 app/code/Magento/ImportExport/Model/Import/Source/FileParser/CsvParserFactory.php create mode 100644 app/code/Magento/ImportExport/Model/Import/Source/FileParser/ParserFactoryInterface.php rename app/code/Magento/ImportExport/Model/Import/Source/{FileParserInterface.php => FileParser/ParserInterface.php} (87%) create mode 100644 app/code/Magento/ImportExport/Model/Import/Source/FileParser/UnsupportedPathException.php create mode 100644 app/code/Magento/ImportExport/Model/Import/Source/FileParser/ZipParserFactory.php delete mode 100644 app/code/Magento/ImportExport/Model/Import/Source/FileParserFactory.php delete mode 100644 app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/CsvFileParserTest.php create mode 100644 app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CompositeParserFactoryTest.php create mode 100644 app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CorruptedPathExceptionTest.php create mode 100644 app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CsvParserFactoryTest.php create mode 100644 app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CsvParserTest.php create mode 100644 app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/FakeFile.php create mode 100644 app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/FakeParser.php create mode 100644 app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/FakeParserFactory.php create mode 100644 app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/UnsupportedPathExceptionTest.php create mode 100644 app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/ZipParserFactoryTest.php create mode 100644 app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/_files/test.csv create mode 100644 app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/_files/test_options.csv create mode 100644 app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/_files/var/tmp/test.csv delete mode 100644 app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParserFactoryTest.php delete mode 100644 app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/_files/custom_delimiter_data.csv delete mode 100644 app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/_files/simple_data.csv delete mode 100644 app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/_files/uneven_row_values_data.csv diff --git a/app/code/Magento/ImportExport/Model/Import/Source/FileParser/CompositeParserFactory.php b/app/code/Magento/ImportExport/Model/Import/Source/FileParser/CompositeParserFactory.php new file mode 100644 index 0000000000000..d40ca41278b2f --- /dev/null +++ b/app/code/Magento/ImportExport/Model/Import/Source/FileParser/CompositeParserFactory.php @@ -0,0 +1,58 @@ +parserFactories = []; + + foreach ($parserFactories as $parserFactory) { + $this->addParserFactory($parserFactory); + } + } + + public function create($path, array $options = []) + { + foreach ($this->parserFactories as $parserFactory) { + $parser = $this->createParserIfSupported($parserFactory, $path, $options); + + if ($parser === false) { + continue; + } + + return $parser; + } + + $this->thereWasNoParserFound($path); + } + + public function addParserFactory(ParserFactoryInterface $parserFactory) + { + $this->parserFactories[] = $parserFactory; + } + + private function thereWasNoParserFound($path) + { + throw new UnsupportedPathException($path); + } + + private function createParserIfSupported(ParserFactoryInterface $parserFactory, $path, array $options) + { + try { + $parser = $parserFactory->create($path, $options); + } catch (UnsupportedPathException $e) { + return false; + } + + return $parser; + } +} diff --git a/app/code/Magento/ImportExport/Model/Import/Source/FileParser/CorruptedFileException.php b/app/code/Magento/ImportExport/Model/Import/Source/FileParser/CorruptedFileException.php new file mode 100644 index 0000000000000..f316dea0f1022 --- /dev/null +++ b/app/code/Magento/ImportExport/Model/Import/Source/FileParser/CorruptedFileException.php @@ -0,0 +1,20 @@ +fileName = $fileName; + parent::__construct($message ?: sprintf('File "%s" is corrupted', $fileName)); + } + + + public function getFileName() + { + return $this->fileName; + } +} diff --git a/app/code/Magento/ImportExport/Model/Import/Source/CsvFileParser.php b/app/code/Magento/ImportExport/Model/Import/Source/FileParser/CsvParser.php similarity index 53% rename from app/code/Magento/ImportExport/Model/Import/Source/CsvFileParser.php rename to app/code/Magento/ImportExport/Model/Import/Source/FileParser/CsvParser.php index c27f8a571dc9a..66bbcc0cf33c2 100644 --- a/app/code/Magento/ImportExport/Model/Import/Source/CsvFileParser.php +++ b/app/code/Magento/ImportExport/Model/Import/Source/FileParser/CsvParser.php @@ -4,13 +4,13 @@ * See COPYING.txt for license details. */ -namespace Magento\ImportExport\Model\Import\Source; +namespace Magento\ImportExport\Model\Import\Source\FileParser; use Magento\Framework\App\Filesystem\DirectoryList; use Magento\Framework\Filesystem; -class CsvFileParser implements FileParserInterface +class CsvParser implements ParserInterface { /** * Enclosure for values in CSV file @@ -34,11 +34,16 @@ class CsvFileParser implements FileParserInterface private $escape; /** - * Null character for parsing value + * List of CSV parsing options * - * @var string + * @var array */ - private $nullPlaceholder; + private $options = [ + 'delimiter' => ',', + 'enclosure' => '"', + 'escape' => '\\', + 'null' => null + ]; /** * File reader @@ -55,28 +60,16 @@ class CsvFileParser implements FileParserInterface private $columns; public function __construct( - $filePath, - Filesystem $filesystem, - $directoryCode = DirectoryList::ROOT, - $nullPlaceholder = null, - $enclosure = '"', - $delimiter = ',', - $escape = '\\' + Filesystem\File\ReadInterface $file, + $options = [] ) { - $this->nullPlaceholder = $nullPlaceholder; - $this->enclosure = $enclosure; - $this->delimiter = $delimiter; - $this->escape = $escape; - - $directory = $filesystem->getDirectoryRead($directoryCode); - if (!$directory->isFile($filePath)) { - throw new \InvalidArgumentException( - sprintf('File "%s" does not exists', $filePath) - ); - } - - $this->file = $directory->openFile($filePath); + $this->file = $file; + $this->options = $options + $this->options; $this->columns = $this->fetchCsvLine(); + + if ($this->columns === false) { + throw new \InvalidArgumentException('CSV file should contain at least 1 row'); + } } public function getColumnNames() @@ -101,24 +94,45 @@ public function reset() $this->fetchCsvLine(); } + private function fetchCsvLine() { - return $this->file->readCsv(0, $this->delimiter, $this->enclosure, $this->escape); + return $this->file->readCsv( + 0, + $this->options['delimiter'], + $this->options['enclosure'], + $this->options['escape'] + ); } private function mapRowData($row) { $result = []; foreach ($this->columns as $index => $column) { - $value = isset($row[$index]) ? $row[$index] : ''; + $result[] = $this->mapNullValue( + $this->extractRowValue($row, $index) + ); + } + return $result; + } - if ($this->nullPlaceholder && $value === $this->nullPlaceholder) { - $value = null; - } - $result[] = $value; + public function __destruct() + { + $this->file->close(); + } + + private function mapNullValue($value) + { + if ($value === $this->options['null']) { + $value = null; } - return $result; + return $value; + } + + private function extractRowValue($row, $index) + { + return (isset($row[$index]) ? $row[$index] : ''); } } diff --git a/app/code/Magento/ImportExport/Model/Import/Source/FileParser/CsvParserFactory.php b/app/code/Magento/ImportExport/Model/Import/Source/FileParser/CsvParserFactory.php new file mode 100644 index 0000000000000..9a980858dd480 --- /dev/null +++ b/app/code/Magento/ImportExport/Model/Import/Source/FileParser/CsvParserFactory.php @@ -0,0 +1,39 @@ +filesystem = $filesystem; + } + + public function create($filePath, array $options = []) + { + $directoryCode = $options['directory_code'] ?? DirectoryList::ROOT; + + if (substr($filePath,-4) !== '.csv') { + throw new UnsupportedPathException($filePath); + } + + $directory = $this->filesystem->getDirectoryRead($directoryCode); + + if (!$directory->isFile($filePath)) { + throw new \InvalidArgumentException(sprintf('File "%s" does not exists', $filePath)); + } + + return new CsvParser($directory->openFile($filePath)); + } +} diff --git a/app/code/Magento/ImportExport/Model/Import/Source/FileParser/ParserFactoryInterface.php b/app/code/Magento/ImportExport/Model/Import/Source/FileParser/ParserFactoryInterface.php new file mode 100644 index 0000000000000..91efd3c3e9853 --- /dev/null +++ b/app/code/Magento/ImportExport/Model/Import/Source/FileParser/ParserFactoryInterface.php @@ -0,0 +1,36 @@ + + */ + + +namespace Magento\ImportExport\Model\Import\Source\FileParser; + +/** + * File parser factory instance + * + */ +interface ParserFactoryInterface +{ + /** + * Creates a file parser instance for specified file + * + * @param string $path + * @param array $options + * + * @return ParserInterface + */ + public function create($path, array $options = []); +} diff --git a/app/code/Magento/ImportExport/Model/Import/Source/FileParserInterface.php b/app/code/Magento/ImportExport/Model/Import/Source/FileParser/ParserInterface.php similarity index 87% rename from app/code/Magento/ImportExport/Model/Import/Source/FileParserInterface.php rename to app/code/Magento/ImportExport/Model/Import/Source/FileParser/ParserInterface.php index dcacc86a0ca95..405159540b2c0 100644 --- a/app/code/Magento/ImportExport/Model/Import/Source/FileParserInterface.php +++ b/app/code/Magento/ImportExport/Model/Import/Source/FileParser/ParserInterface.php @@ -4,14 +4,14 @@ * See COPYING.txt for license details. */ -namespace Magento\ImportExport\Model\Import\Source; +namespace Magento\ImportExport\Model\Import\Source\FileParser; /** * File parser for import of source format * */ -interface FileParserInterface +interface ParserInterface { /** * Must return list of columns diff --git a/app/code/Magento/ImportExport/Model/Import/Source/FileParser/UnsupportedPathException.php b/app/code/Magento/ImportExport/Model/Import/Source/FileParser/UnsupportedPathException.php new file mode 100644 index 0000000000000..98ff955439572 --- /dev/null +++ b/app/code/Magento/ImportExport/Model/Import/Source/FileParser/UnsupportedPathException.php @@ -0,0 +1,21 @@ +path = $path; + } + + public function getPath() + { + return $this->path; + } + +} diff --git a/app/code/Magento/ImportExport/Model/Import/Source/FileParser/ZipParserFactory.php b/app/code/Magento/ImportExport/Model/Import/Source/FileParser/ZipParserFactory.php new file mode 100644 index 0000000000000..9522575864e53 --- /dev/null +++ b/app/code/Magento/ImportExport/Model/Import/Source/FileParser/ZipParserFactory.php @@ -0,0 +1,40 @@ + + */ + + +namespace Magento\ImportExport\Model\Import\Source\FileParser; + +use Magento\Framework\Filesystem; + +class ZipParserFactory implements ParserFactoryInterface +{ + private $filesystem; + + public function __construct(Filesystem $filesystem, ParserFactoryInterface $factory) + { + $this->filesystem = $filesystem; + } + + public function create($path, array $options = []) + { + if (substr($path, -4) !== '.zip') { + throw new UnsupportedPathException($path); + } + + throw new CorruptedFileException($path); + } +} diff --git a/app/code/Magento/ImportExport/Model/Import/Source/FileParserFactory.php b/app/code/Magento/ImportExport/Model/Import/Source/FileParserFactory.php deleted file mode 100644 index 9a095b17edfb8..0000000000000 --- a/app/code/Magento/ImportExport/Model/Import/Source/FileParserFactory.php +++ /dev/null @@ -1,102 +0,0 @@ -objectManager = $objectManager; - foreach ($parserMap as $item) { - $this->addParserMap($item); - } - } - - /** - * Creates an instance of file parser from file path - * - * @param string $filePath - * @param array $options - * - * @return FileParserInterface - */ - public function create($filePath, $options = []) - { - $extension = $this->extractFileExtension($filePath); - - $this->assertFileExtensionIsSupported($filePath, $extension); - - $parserInfo = $this->parserMap[$extension]; - - $arguments = [ - $parserInfo['argument'] => $filePath - ]; - - $arguments += $options; - - return $this->objectManager->create( - $parserInfo['class'], - $arguments - ); - } - - private function addParserMap($parserMap) - { - $this->assertParserMap($parserMap); - $this->parserMap[$parserMap['extension']] = [ - 'class' => $parserMap['class'], - 'argument' => $parserMap['argument'] - ]; - } - - private function extractFileExtension($filePath) - { - return substr($filePath, strrpos($filePath, '.') + 1, strlen($filePath)); - } - - private function assertParserMap($mapItem) - { - if (empty($mapItem['extension'])) { - throw new \InvalidArgumentException('Missing extension in parser definition'); - } - if (empty($mapItem['class'])) { - throw new \InvalidArgumentException('Missing class in parser definition'); - } - if (empty($mapItem['argument'])) { - throw new \InvalidArgumentException('Missing argument in parser definition'); - } - } - - private function assertFileExtensionIsSupported($filePath, $extension) - { - if (!isset($this->parserMap[$extension])) { - throw new \InvalidArgumentException( - sprintf('File "%s" is an invalid format', $filePath) - ); - } - } -} diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/CsvFileParserTest.php b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/CsvFileParserTest.php deleted file mode 100644 index f3903f188ad97..0000000000000 --- a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/CsvFileParserTest.php +++ /dev/null @@ -1,145 +0,0 @@ -createFileSystemStub()); - } - - /** - * @test - */ - public function when_valid_file_is_provided_it_reads_columns_from_file_header() - { - $parser = new CsvFileParser('simple_data.csv', $this->createFileSystemStub()); - - $this->assertSame(['column1', 'column2', 'column3'], $parser->getColumnNames()); - } - - /** - * @test - */ - public function when_valid_file_is_provided_it_reads_rows_and_skips_header() - { - $parser = new CsvFileParser('simple_data.csv', $this->createFileSystemStub()); - - $this->assertSame(['row1value1', 'row1value2', 'row1value3'], $parser->fetchRow()); - $this->assertSame(['row2value1', 'row2value2', 'row2value3'], $parser->fetchRow()); - } - - /** - * @test - */ - public function when_end_of_file_is_reached_it_returns_false() - { - $parser = new CsvFileParser('simple_data.csv', $this->createFileSystemStub()); - - $this->skipLines($parser, 4); - - $this->assertSame(false, $parser->fetchRow()); - } - - /** - * @test - */ - public function when_file_was_already_read_it_is_possible_to_read_it_again() - { - $parser = new CsvFileParser('simple_data.csv', $this->createFileSystemStub()); - - $this->skipLines($parser, 3); - - $parser->reset(); - - $this->assertSame(['row1value1', 'row1value2', 'row1value3'], $parser->fetchRow()); - $this->assertSame(['row2value1', 'row2value2', 'row2value3'], $parser->fetchRow()); - } - - /** - * @test - */ - public function when_custom_delimiter_is_specified_it_uses_it_for_parsing() - { - $parser = new CsvFileParser( - 'custom_delimiter_data.csv', - $this->createFileSystemStub(), - DirectoryList::ROOT, - null, - '"', - '|' - ); - - $this->assertSame(['row1value1', 'row1value2', 'row1value3'], $parser->fetchRow()); - } - - /** - * @test - */ - public function when_file_has_missing_row_values_it_returns_same_number_as_columns() - { - $parser = new CsvFileParser( - 'uneven_row_values_data.csv', - $this->createFileSystemStub() - ); - - $this->assertSame(['row1value1', '', ''], $parser->fetchRow()); - $this->assertSame(['row2value1', 'row2value2', 'row2value3'], $parser->fetchRow()); - $this->assertSame(['row3value1', 'row3value2', ''], $parser->fetchRow()); - $this->assertSame(['row4value1', 'row4value2', 'row4value3'], $parser->fetchRow()); - } - - /** - * @test - */ - public function when_null_placeholder_is_set_it_replaces_it_with_native_null_value() - { - $parser = new CsvFileParser( - 'simple_data.csv', - $this->createFileSystemStub(), - DirectoryList::ROOT, - 'row2value2' - ); - - $this->assertSame(['row1value1', 'row1value2', 'row1value3'], $parser->fetchRow()); - $this->assertSame(['row2value1', null, 'row2value3'], $parser->fetchRow()); - } - - private function createFileSystemStub() - { - $fileSystemStub = $this->getMockBuilder(Filesystem::class) - ->disableOriginalConstructor() - ->setMethods(['getDirectoryRead']) - ->getMock(); - - $readDirectoryFactory = new Filesystem\Directory\ReadFactory(new Filesystem\DriverPool()); - - $fileSystemStub->method('getDirectoryRead') - ->willReturn($readDirectoryFactory->create(__DIR__ . '/_files')); - - return $fileSystemStub; - } - - private function skipLines(CsvFileParser $parser, $numberOfLines) - { - for ($i = 0; $i < $numberOfLines; $i++) { - $parser->fetchRow(); - } - } -} diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CompositeParserFactoryTest.php b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CompositeParserFactoryTest.php new file mode 100644 index 0000000000000..d75269dbae76f --- /dev/null +++ b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CompositeParserFactoryTest.php @@ -0,0 +1,66 @@ +setExpectedException(UnsupportedPathException::class, 'Path "file.csv" is not supported'); + + $compositeFactory->create('file.csv'); + } + + public function testWhenPathIsNotSupportedByAnyFactory_NoParserIsCreated() + { + $compositeFactory = new CompositeParserFactory(); + $compositeFactory->addParserFactory(new FakeParserFactory(['file.csv' => new FakeParser()])); + + $this->setExpectedException(UnsupportedPathException::class, 'Path "file.zip" is not supported'); + + $compositeFactory->create('file.zip'); + } + + public function testWhenPathIsSupportedByFirstFactory_ParserIsReturned() + { + $expectedParser = new FakeParser(); + + $compositeFactory = new CompositeParserFactory(); + $compositeFactory->addParserFactory(new FakeParserFactory($expectedParser)); + $compositeFactory->addParserFactory(new FakeParserFactory(new FakeParser())); + + $this->assertSame($expectedParser, $compositeFactory->create('file.csv')); + } + + public function testWhenPathIsSupportedBySecondFactory_ParserIsReturned() + { + $expectedParser = new FakeParser(); + + $compositeFactory = new CompositeParserFactory(); + $compositeFactory->addParserFactory(new FakeParserFactory()); + $compositeFactory->addParserFactory(new FakeParserFactory(['file.csv' => $expectedParser])); + + $this->assertSame($expectedParser, $compositeFactory->create('file.csv')); + } + + + public function testWhenFactoryIsProvidedViaConstructor_ParserIsReturned() + { + $expectedParser = new FakeParser(); + + $compositeFactory = new CompositeParserFactory([new FakeParserFactory($expectedParser)]); + + $this->assertSame($expectedParser, $compositeFactory->create('file.csv')); + } + + +} diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CorruptedPathExceptionTest.php b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CorruptedPathExceptionTest.php new file mode 100644 index 0000000000000..81fc0154f1d11 --- /dev/null +++ b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CorruptedPathExceptionTest.php @@ -0,0 +1,36 @@ +assertEmpty($corruptedFileException->getFileName()); + } + + public function testWhenFileNameIsProvided_FileNameCanBeRetrievedLater() + { + $corruptedFileException = new CorruptedFileException('file.csv'); + + $this->assertSame('file.csv', $corruptedFileException->getFileName()); + } + + public function testWhenMessageIsProvided_MessageCanBeRetrievedLater() + { + $corruptedFileException = new CorruptedFileException('', 'My message'); + + $this->assertSame('My message', $corruptedFileException->getMessage()); + } + + public function testWhenNoMessageIsProvided_MessageIsGeneratedFromPath() + { + $corruptedFileException = new CorruptedFileException('file.csv'); + + $this->assertSame('File "file.csv" is corrupted', $corruptedFileException->getMessage()); + } +} diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CsvParserFactoryTest.php b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CsvParserFactoryTest.php new file mode 100644 index 0000000000000..24bc262cbf369 --- /dev/null +++ b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CsvParserFactoryTest.php @@ -0,0 +1,91 @@ +createCsvParserFactory(); + + $this->setExpectedException(FileParser\UnsupportedPathException::class); + + $factory->create('test.zip'); + } + + public function testWhenFileDoesNotExist_NoParserIsCreated() + { + $factory = $this->createCsvParserFactory(); + + $this->setExpectedException( + \InvalidArgumentException::class, + 'File "non_existing_file.csv" does not exists' + ); + + $factory->create('non_existing_file.csv'); + } + + public function testWhenFileIsNotAccesible_NoParserIsCreated() + { + $factory = $this->createCsvParserFactory(tempnam(sys_get_temp_dir(), 'non_created')); + + $this->setExpectedException( + \InvalidArgumentException::class, + 'File "test.csv" does not exists' + ); + + $factory->create('test.csv'); + } + + public function testWhenValidFileIsProvided_ParserIsCreated() + { + $factory = $this->createCsvParserFactory(); + + $this->assertCsvFile( + ['column1', 'column2', 'column3'], + $factory->create('test.csv') + ); + } + + public function testWhenCustomDirectoryIsProvided_ParserIsCreatedFromIt() + { + $factory = $this->createCsvParserFactory(); + + $this->assertCsvFile( + ['column1', 'column2'], + $factory->create('test.csv', ['directory_code' => DirectoryList::TMP]) + ); + } + + private function createTestFilesystem($baseDirectory = null) + { + $baseDirectory = $baseDirectory ?? __DIR__ . '/_files'; + + return new Filesystem( + new DirectoryList($baseDirectory), + new Filesystem\Directory\ReadFactory(new Filesystem\DriverPool()), + new Filesystem\Directory\WriteFactory(new Filesystem\DriverPool()) + ); + } + + private function createCsvParserFactory($baseDirectory = null) + { + return new FileParser\CsvParserFactory( + $this->createTestFilesystem($baseDirectory), + new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this) + ); + } + + private function assertCsvFile($expectedColumns, FileParser\CsvParser $csvParser) + { + $this->assertSame($expectedColumns, $csvParser->getColumnNames()); + } +} diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CsvParserTest.php b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CsvParserTest.php new file mode 100644 index 0000000000000..d958dacd8a6b4 --- /dev/null +++ b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CsvParserTest.php @@ -0,0 +1,175 @@ +setExpectedException(\InvalidArgumentException::class, 'CSV file should contain at least 1 row'); + $this->createParser(new FakeFile([])); + } + + public function testWhenValidFileIsProvided_readColumnsFromFileHeader() + { + $parser = $this->createParser(new FakeFile([ + 'column1,column2,column3', + 'row1value1,row1value2,row1value3', + ])); + + $this->assertSame(['column1', 'column2', 'column3'], $parser->getColumnNames()); + } + + public function testWhenValidFileIsProvided_rowsAreFetchedAndHeaderIsSkipped() + { + $parser = $this->createParser(new FakeFile([ + 'column1,column2,column3', + 'row1value1,row1value2,row1value3', + 'row2value1,row2value2,row2value3', + ])); + + $this->assertParsedFileContent( + [ + ['row1value1', 'row1value2', 'row1value3'], + ['row2value1', 'row2value2', 'row2value3'] + ], + $parser + ); + } + + public function testWhenEndOfFileIsReached_NoMoreRowsCanBeFetched() + { + $parser = $this->createParser(new FakeFile([ + 'column1,column2,column3', + 'row1value1,row1value2,row1value3', + 'row2value1,row2value2,row2value3', + ])); + + $this->skipRows(2, $parser); + + $this->assertFalse($parser->fetchRow()); + } + + public function testWhenFileWasAlreadyRead_ResetAllowsToReadItFromStart() + { + $parser = $this->createParser(new FakeFile([ + 'column1,column2,column3', + 'row1value1,row1value2,row1value3', + 'row2value1,row2value2,row2value3', + ])); + + $this->skipRows(2, $parser); + $parser->reset(); + + $this->assertParsedFileContent( + [ + ['row1value1', 'row1value2', 'row1value3'], + ['row2value1', 'row2value2', 'row2value3'] + ], + $parser + ); + } + + public function testWhenCustomDelimiterIsSpecified_dataIsParsedUsingThisDelimiter() + { + $parser = $this->createParser( + new FakeFile([ + 'column1|column2|column3', + 'row1value1|row1value2|row1value3', + 'row2value1|row2value2|row2value3' + ]), + ['delimiter' => '|'] + ); + + $this->assertParsedFileContent( + [ + ['row1value1', 'row1value2', 'row1value3'], + ['row2value1', 'row2value2', 'row2value3'] + ], + $parser + ); + } + + public function testWhenFileHasMissingRowValues_fetchedRowValuesAreSameAsNumberOfColumns() + { + $parser = $this->createParser(new FakeFile([ + 'column1,column2,column3', + 'row1value1', + 'row2value1,row2value2,row2value3', + 'row3value1,row3value2', + 'row4value1,row4value2,row4value3,row4value4', + ])); + + $this->assertParsedFileContent( + [ + ['row1value1', '', ''], + ['row2value1', 'row2value2', 'row2value3'], + ['row3value1', 'row3value2', ''], + ['row4value1', 'row4value2', 'row4value3'] + ], + $parser + ); + } + + + public function testWhenNullPlaceholderIsSet_fetchedRowValueIsReplacedWithNull() + { + $parser = $this->createParser( + new FakeFile([ + 'column1,column2,column3', + 'row1value1,row1value2,row1value3', + 'row2value1,row2value2,row2value3', + ]), + ['null' => 'row2value2'] + ); + + $this->assertParsedFileContent( + [ + ['row1value1', 'row1value2', 'row1value3'], + ['row2value1', null, 'row2value3'] + ], + $parser + ); + } + + public function testWhenParserIsDestroyed_InternalFileDescriptorIsClosed() + { + $csvFile = new FakeFile([ + 'column1,column2,column3' + ]); + + $parser = $this->createParser($csvFile); + unset($parser); + + $this->assertFalse($csvFile->isOpen()); + } + + private function assertParsedFileContent($expectedParsedFileTable, $parser) + { + foreach ($expectedParsedFileTable as $rowNumber => $row) { + $this->assertSame( + $row, + $parser->fetchRow(), + sprintf('Unexpected data on row #%d', $rowNumber + 1) + ); + } + } + + private function skipRows($numberOfRows, FileParser\CsvParser $parser) + { + for ($i = 0; $i < $numberOfRows; $i++) { + $parser->fetchRow(); + } + } + + private function createParser($file, $options = []) + { + return new FileParser\CsvParser($file, $options); + } +} diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/FakeFile.php b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/FakeFile.php new file mode 100644 index 0000000000000..f7abcc14d672f --- /dev/null +++ b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/FakeFile.php @@ -0,0 +1,89 @@ +lines = $lines; + $this->pointer = 0; + $this->isOpen = true; + } + + public function isOpen() + { + return $this->isOpen; + } + + public function readCsv($length = 0, $delimiter = ',', $enclosure = '"', $escape = '\\') + { + if ($this->isEndOfFile()) { + return false; + } + + return str_getcsv($this->readFileLine($length), $delimiter, $enclosure, $escape); + } + + private function readFileLine($length) + { + return $this->truncateLineToLength($this->lines[$this->pointer++], $length); + } + + private function isEndOfFile() + { + return !isset($this->lines[$this->pointer]); + } + + private function truncateLineToLength($line, $length) + { + if ($length > 0 && strlen($line) > $length) { + $line = substr($line, 0, $length); + } + return $line; + } + public function close() + { + $this->isOpen = false; + } + + public function read($length) + { + return ''; + } + + public function readLine($length, $ending = null) + { + return $this->readFileLine($length); + } + + public function tell() + { + return $this->pointer; + } + + public function seek($length, $whence = SEEK_SET) + { + $this->pointer = $length; + } + + public function eof() + { + return $this->isEndOfFile(); + } + + public function stat() + { + return []; + } +} diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/FakeParser.php b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/FakeParser.php new file mode 100644 index 0000000000000..5e66ca0351fea --- /dev/null +++ b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/FakeParser.php @@ -0,0 +1,50 @@ +columns = $columns; + $this->rows = $rows; + $this->position = 0; + } + + + public function getColumnNames() + { + return $this->columns; + } + + public function fetchRow() + { + if ($this->isEndOfRows()) { + return false; + } + + return $this->rows[$this->position++]; + } + + public function reset() + { + $this->position = 0; + } + + private function isEndOfRows() + { + return !isset($this->rows[$this->position]); + } + +} diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/FakeParserFactory.php b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/FakeParserFactory.php new file mode 100644 index 0000000000000..8f2b433c00429 --- /dev/null +++ b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/FakeParserFactory.php @@ -0,0 +1,43 @@ +parser = $parser; + } + + public function create($path, array $options = []) + { + if ($this->isParserMap()) { + return $this->findParserInMap($path); + } + + return $this->parser; + } + + private function findParserInMap($path) + { + if (!isset($this->parser[$path])) { + throw new UnsupportedPathException($path); + } + + return $this->parser[$path]; + } + + private function isParserMap(): bool + { + return is_array($this->parser); + } +} diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/UnsupportedPathExceptionTest.php b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/UnsupportedPathExceptionTest.php new file mode 100644 index 0000000000000..eb3d7bf44cc29 --- /dev/null +++ b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/UnsupportedPathExceptionTest.php @@ -0,0 +1,36 @@ +assertEmpty($notSupportedPathException->getPath()); + } + + public function testWhenFileNameIsProvided_FileNameCanBeRetrievedLater() + { + $notSupportedPathException = new UnsupportedPathException('file.csv'); + + $this->assertSame('file.csv', $notSupportedPathException->getPath()); + } + + public function testWhenMessageIsProvided_MessageCanBeRetrievedLater() + { + $notSupportedPathException = new UnsupportedPathException('', 'My message'); + + $this->assertSame('My message', $notSupportedPathException->getMessage()); + } + + public function testWhenNoMessageIsProvided_MessageIsGeneratedFromPath() + { + $notSupportedPathException = new UnsupportedPathException('file.csv'); + + $this->assertSame('Path "file.csv" is not supported', $notSupportedPathException->getMessage()); + } +} diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/ZipParserFactoryTest.php b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/ZipParserFactoryTest.php new file mode 100644 index 0000000000000..3e3b0868c9b39 --- /dev/null +++ b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/ZipParserFactoryTest.php @@ -0,0 +1,55 @@ + + */ + +namespace Magento\ImportExport\Test\Unit\Model\Import\Source\FileParser; + + +use Magento\Framework\App\Filesystem\DirectoryList; +use Magento\Framework\Filesystem; +use Magento\ImportExport\Model\Import\Source\FileParser; + +class ZipParserFactoryTest extends \PHPUnit_Framework_TestCase +{ + public function testWhenCsvFileIsProvided_ParserIsNotCreated() + { + $this->setExpectedException(FileParser\UnsupportedPathException::class, 'Path "file.csv" is not supported'); + + $parser = new FileParser\ZipParserFactory($this->createTestFilesystem(), new FakeParserFactory()); + + $parser->create('file.csv'); + } + + public function testWhenCorruptedZipFileIsProvided_ParserIsNotCreated() + { + $this->setExpectedException(FileParser\CorruptedFileException::class); + + $parser = new FileParser\ZipParserFactory($this->createTestFilesystem(), new FakeParserFactory()); + + $parser->create('corrupted.zip'); + } + + private function createTestFilesystem($baseDirectory = null) + { + $baseDirectory = $baseDirectory ?? __DIR__ . '/_files'; + + return new Filesystem( + new DirectoryList($baseDirectory), + new Filesystem\Directory\ReadFactory(new Filesystem\DriverPool()), + new Filesystem\Directory\WriteFactory(new Filesystem\DriverPool()) + ); + } +} diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/_files/test.csv b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/_files/test.csv new file mode 100644 index 0000000000000..3acca666172ea --- /dev/null +++ b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/_files/test.csv @@ -0,0 +1 @@ +column1,column2,column3 diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/_files/test_options.csv b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/_files/test_options.csv new file mode 100644 index 0000000000000..e1a3763d1e399 --- /dev/null +++ b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/_files/test_options.csv @@ -0,0 +1 @@ +column1|;column2;|column3 diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/_files/var/tmp/test.csv b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/_files/var/tmp/test.csv new file mode 100644 index 0000000000000..18288adde1528 --- /dev/null +++ b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/_files/var/tmp/test.csv @@ -0,0 +1 @@ +column1,column2 diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParserFactoryTest.php b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParserFactoryTest.php deleted file mode 100644 index 4260b0ac28f09..0000000000000 --- a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParserFactoryTest.php +++ /dev/null @@ -1,172 +0,0 @@ -createFileParserFactory([[]]); - } - - /** - * @expectedException \InvalidArgumentException - * @expectedExceptionMessage Missing class in parser definition - * @test - */ - public function when_class_is_empty_it_throws_InvalidArgumentException() - { - $this->createFileParserFactory([ - ['extension' => 'csv'] - ]); - } - - /** - * @expectedException \InvalidArgumentException - * @expectedExceptionMessage Missing argument in parser definition - * @test - */ - public function when_argument_is_empty_it_throws_InvalidArgumentException() - { - $this->createFileParserFactory([ - [ - 'extension' => 'csv', - 'class' => 'SomeClass' - ] - ]); - } - - - /** - * @expectedException \InvalidArgumentException - * @expectedExceptionMessage File "some/random/file-path.unknowntype" is an invalid format - * @test - */ - public function when_non_existing_file_type_is_provided_it_throws_InvalidArgumentException() - { - $factory = $this->createFileParserFactory( - [ - [ - 'extension' => 'myfile', - 'class' => FileParserInterface::class, - 'argument' => 'file' - ] - ] - ); - - $factory->create('some/random/file-path.unknowntype'); - } - - /** - * @test - */ - public function when_mapped_type_is_provided_it_creates_instance_via_object_manager() - { - $fileParser = $this->getMockForAbstractClass( - FileParserInterface::class, - [], - 'FileParserStub' - ); - - $factory = $this->createFileParserFactory( - [ - $this->createParserMapArray('myfile', 'FileParserStub', 'file') - ], - $this->createMockedObjectManagerForFileParser( - 'FileParserStub', - ['file' => 'some/random/file-path.myfile'], - $fileParser - ) - ); - - $this->assertSame($fileParser, $factory->create('some/random/file-path.myfile')); - } - - /** - * @test - */ - public function when_additional_options_are_provided_it_passes_them_to_constructor_of_file_parser() - { - $fileParser = $this->getMockForAbstractClass(FileParserInterface::class); - $fileParserClass = get_class($fileParser); - - - $factory = $this->createFileParserFactory( - [ - $this->createParserMapArray('myfile', $fileParserClass, 'file') - ], - $this->createMockedObjectManagerForFileParser( - $fileParserClass, - [ - 'file' => 'some/random/file-path.myfile', - 'option1' => 'option1', - 'option2' => 'option2' - ], - $fileParser - ) - ); - - $this->assertSame( - $fileParser, - $factory->create( - 'some/random/file-path.myfile', - [ - 'option1' => 'option1', - 'option2' => 'option2' - ] - ) - ); - } - - /** - * @return FileParserFactory - */ - private function createFileParserFactory($typeMap = [], ObjectManagerInterface $objectManager = null) - { - $factory = new FileParserFactory( - $objectManager ?: $this->getMockForAbstractClass(ObjectManagerInterface::class), - $typeMap - ); - return $factory; - } - - /** - * @return ObjectManagerInterface - */ - private function createMockedObjectManagerForFileParser( - $className, - $arguments, - FileParserInterface $fileParser - ) { - $objectManager = $this->getMockForAbstractClass(ObjectManagerInterface::class); - $objectManager->expects($this->once()) - ->method('create') - ->with($className, $arguments) - ->willReturn($fileParser); - - return $objectManager; - } - - private function createParserMapArray($extension, $className, $argumentName) - { - return [ - 'extension' => $extension, - 'class' => $className, - 'argument' => $argumentName - ]; - } -} diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/_files/custom_delimiter_data.csv b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/_files/custom_delimiter_data.csv deleted file mode 100644 index d2621cbd78f16..0000000000000 --- a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/_files/custom_delimiter_data.csv +++ /dev/null @@ -1,5 +0,0 @@ -column1|column2|column3 -row1value1|row1value2|row1value3 -row2value1|row2value2|row2value3 -row3value1|row3value2|row3value3 -row4value1|row4value2|row4value3 diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/_files/simple_data.csv b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/_files/simple_data.csv deleted file mode 100644 index 0375816d3c5ff..0000000000000 --- a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/_files/simple_data.csv +++ /dev/null @@ -1,5 +0,0 @@ -column1,column2,column3 -row1value1,row1value2,row1value3 -row2value1,row2value2,row2value3 -row3value1,row3value2,row3value3 -row4value1,row4value2,row4value3 diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/_files/uneven_row_values_data.csv b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/_files/uneven_row_values_data.csv deleted file mode 100644 index 5822f7ab0312e..0000000000000 --- a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/_files/uneven_row_values_data.csv +++ /dev/null @@ -1,5 +0,0 @@ -column1,column2,column3 -row1value1 -row2value1,row2value2,row2value3 -row3value1,row3value2 -row4value1,row4value2,row4value3,row4value4 From 0a898becfdc094e46c9637f48461fe08b7ed586e Mon Sep 17 00:00:00 2001 From: Ivan Chepurnyi Date: Tue, 2 May 2017 20:24:13 +0200 Subject: [PATCH 04/17] Move CSV parser instantiation via ObjectManager in factory, add FakeObjectManager for test harness --- .../Source/FileParser/CsvParserFactory.php | 19 +++++- .../FileParser/CsvParserFactoryTest.php | 18 +++++- .../Source/FileParser/FakeObjectManager.php | 58 +++++++++++++++++++ 3 files changed, 92 insertions(+), 3 deletions(-) create mode 100644 app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/FakeObjectManager.php diff --git a/app/code/Magento/ImportExport/Model/Import/Source/FileParser/CsvParserFactory.php b/app/code/Magento/ImportExport/Model/Import/Source/FileParser/CsvParserFactory.php index 9a980858dd480..d6937653bb71a 100644 --- a/app/code/Magento/ImportExport/Model/Import/Source/FileParser/CsvParserFactory.php +++ b/app/code/Magento/ImportExport/Model/Import/Source/FileParser/CsvParserFactory.php @@ -5,6 +5,7 @@ use Magento\Framework\App\Filesystem\DirectoryList; use Magento\Framework\Filesystem; +use Magento\Framework\ObjectManagerInterface; class CsvParserFactory implements ParserFactoryInterface { @@ -15,9 +16,17 @@ class CsvParserFactory implements ParserFactoryInterface */ private $filesystem; - public function __construct(Filesystem $filesystem) + /** + * Object Manager for CSV parser creation + * + * @var ObjectManagerInterface + */ + private $objectManager; + + public function __construct(Filesystem $filesystem, ObjectManagerInterface $objectManager) { $this->filesystem = $filesystem; + $this->objectManager = $objectManager; } public function create($filePath, array $options = []) @@ -34,6 +43,12 @@ public function create($filePath, array $options = []) throw new \InvalidArgumentException(sprintf('File "%s" does not exists', $filePath)); } - return new CsvParser($directory->openFile($filePath)); + return $this->objectManager->create( + CsvParser::class, + [ + 'file' => $directory->openFile($filePath), + 'options' => $options + ] + ); } } diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CsvParserFactoryTest.php b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CsvParserFactoryTest.php index 24bc262cbf369..c40777968207c 100644 --- a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CsvParserFactoryTest.php +++ b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CsvParserFactoryTest.php @@ -65,6 +65,22 @@ public function testWhenCustomDirectoryIsProvided_ParserIsCreatedFromIt() ); } + public function testWhenCustomCSVOptionsProvided_ParserIsCreatedFromIt() + { + $factory = $this->createCsvParserFactory(); + + $this->assertCsvFile( + ['column1', 'column2', 'column3'], + $factory->create( + 'test_options.csv', + [ + 'delimiter' => '|', + 'enclosure' => ';' + ] + ) + ); + } + private function createTestFilesystem($baseDirectory = null) { $baseDirectory = $baseDirectory ?? __DIR__ . '/_files'; @@ -80,7 +96,7 @@ private function createCsvParserFactory($baseDirectory = null) { return new FileParser\CsvParserFactory( $this->createTestFilesystem($baseDirectory), - new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this) + new FakeObjectManager() ); } diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/FakeObjectManager.php b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/FakeObjectManager.php new file mode 100644 index 0000000000000..62f5de6cdc523 --- /dev/null +++ b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/FakeObjectManager.php @@ -0,0 +1,58 @@ +resolveConstructorArguments($type, $arguments); + return new $type(...$arguments); + } + + public function get($type) + { + if (!isset($this->instances[$type])) { + $this->instances[$type] = $this->create($type); + } + + return $this->instances[$type]; + } + + public function configure(array $configuration) + { + // Fake object manager is not configurable + } + + private function resolveConstructorArguments($type, array $arguments) + { + $constructorSignature = new \ReflectionMethod($type, '__construct'); + $resolvedArguments = []; + foreach ($constructorSignature->getParameters() as $parameter) { + $arguments = $this->assertParameterValue($arguments, $parameter); + + $resolvedArguments[] = $arguments[$parameter->getName()] ?? $parameter->getDefaultValue(); + } + + return $resolvedArguments; + } + + private function assertParameterValue(array $arguments, $parameter): array + { + if (!isset($arguments[$parameter->getName()]) && !$parameter->isDefaultValueAvailable()) { + new \RuntimeException( + sprintf( + 'Cannot instantiate %s without default value for constructor argument $%s', + $parameter->getDeclaringClass()->getName(), + $parameter->getName() + ) + ); + } + return $arguments; + } + +} From 9b542f6c6bdb9e327fc45f653b53aff05f4e6d38 Mon Sep 17 00:00:00 2001 From: Ivan Chepurnyi Date: Tue, 2 May 2017 20:31:28 +0200 Subject: [PATCH 05/17] Give up copyright to Magento Inc. --- .../Source/FileParser/CompositeParserFactory.php | 4 ++++ .../Source/FileParser/CorruptedFileException.php | 4 ++++ .../Model/Import/Source/FileParser/CsvParser.php | 1 - .../Source/FileParser/CsvParserFactory.php | 4 ++++ .../Source/FileParser/ParserFactoryInterface.php | 16 ++-------------- .../Import/Source/FileParser/ParserInterface.php | 1 - .../FileParser/UnsupportedPathException.php | 4 ++++ .../Source/FileParser/ZipParserFactory.php | 16 ++-------------- .../FileParser/CorruptedPathExceptionTest.php | 4 ++++ .../Source/FileParser/FakeObjectManager.php | 4 ++++ .../FileParser/UnsupportedPathExceptionTest.php | 4 ++++ .../Source/FileParser/ZipParserFactoryTest.php | 16 ++-------------- 12 files changed, 34 insertions(+), 44 deletions(-) diff --git a/app/code/Magento/ImportExport/Model/Import/Source/FileParser/CompositeParserFactory.php b/app/code/Magento/ImportExport/Model/Import/Source/FileParser/CompositeParserFactory.php index d40ca41278b2f..ef53135166bb8 100644 --- a/app/code/Magento/ImportExport/Model/Import/Source/FileParser/CompositeParserFactory.php +++ b/app/code/Magento/ImportExport/Model/Import/Source/FileParser/CompositeParserFactory.php @@ -1,4 +1,8 @@ + * Copyright © Magento, Inc. All rights reserved. + * See COPYING.txt for license details. */ - namespace Magento\ImportExport\Model\Import\Source\FileParser; /** diff --git a/app/code/Magento/ImportExport/Model/Import/Source/FileParser/ParserInterface.php b/app/code/Magento/ImportExport/Model/Import/Source/FileParser/ParserInterface.php index 405159540b2c0..75cefe63857ef 100644 --- a/app/code/Magento/ImportExport/Model/Import/Source/FileParser/ParserInterface.php +++ b/app/code/Magento/ImportExport/Model/Import/Source/FileParser/ParserInterface.php @@ -6,7 +6,6 @@ namespace Magento\ImportExport\Model\Import\Source\FileParser; - /** * File parser for import of source format * diff --git a/app/code/Magento/ImportExport/Model/Import/Source/FileParser/UnsupportedPathException.php b/app/code/Magento/ImportExport/Model/Import/Source/FileParser/UnsupportedPathException.php index 98ff955439572..bacd3283666cd 100644 --- a/app/code/Magento/ImportExport/Model/Import/Source/FileParser/UnsupportedPathException.php +++ b/app/code/Magento/ImportExport/Model/Import/Source/FileParser/UnsupportedPathException.php @@ -1,4 +1,8 @@ + * Copyright © Magento, Inc. All rights reserved. + * See COPYING.txt for license details. */ - namespace Magento\ImportExport\Model\Import\Source\FileParser; use Magento\Framework\Filesystem; diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CorruptedPathExceptionTest.php b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CorruptedPathExceptionTest.php index 81fc0154f1d11..3f40c4396b840 100644 --- a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CorruptedPathExceptionTest.php +++ b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CorruptedPathExceptionTest.php @@ -1,4 +1,8 @@ + * Copyright © Magento, Inc. All rights reserved. + * See COPYING.txt for license details. */ namespace Magento\ImportExport\Test\Unit\Model\Import\Source\FileParser; - use Magento\Framework\App\Filesystem\DirectoryList; use Magento\Framework\Filesystem; use Magento\ImportExport\Model\Import\Source\FileParser; From 43df7efe747375c07a8a6d6f0a50a262e84a96e5 Mon Sep 17 00:00:00 2001 From: Ivan Chepurnyi Date: Tue, 2 May 2017 20:34:16 +0200 Subject: [PATCH 06/17] Rename variable in UnsupportedPathExceptionTest to add more consistency --- .../FileParser/UnsupportedPathExceptionTest.php | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/UnsupportedPathExceptionTest.php b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/UnsupportedPathExceptionTest.php index 140d87fea456e..53ade55f1b2aa 100644 --- a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/UnsupportedPathExceptionTest.php +++ b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/UnsupportedPathExceptionTest.php @@ -12,29 +12,29 @@ class UnsupportedPathExceptionTest extends \PHPUnit_Framework_TestCase { public function testWhenNoArgumentsAreProvided_FileNameIsEmpty() { - $notSupportedPathException = new UnsupportedPathException(); + $unsupportedPathException = new UnsupportedPathException(); - $this->assertEmpty($notSupportedPathException->getPath()); + $this->assertEmpty($unsupportedPathException->getPath()); } public function testWhenFileNameIsProvided_FileNameCanBeRetrievedLater() { - $notSupportedPathException = new UnsupportedPathException('file.csv'); + $unsupportedPathException = new UnsupportedPathException('file.csv'); - $this->assertSame('file.csv', $notSupportedPathException->getPath()); + $this->assertSame('file.csv', $unsupportedPathException->getPath()); } public function testWhenMessageIsProvided_MessageCanBeRetrievedLater() { - $notSupportedPathException = new UnsupportedPathException('', 'My message'); + $unsupportedPathException = new UnsupportedPathException('', 'My message'); - $this->assertSame('My message', $notSupportedPathException->getMessage()); + $this->assertSame('My message', $unsupportedPathException->getMessage()); } public function testWhenNoMessageIsProvided_MessageIsGeneratedFromPath() { - $notSupportedPathException = new UnsupportedPathException('file.csv'); + $unsupportedPathException = new UnsupportedPathException('file.csv'); - $this->assertSame('Path "file.csv" is not supported', $notSupportedPathException->getMessage()); + $this->assertSame('Path "file.csv" is not supported', $unsupportedPathException->getMessage()); } } From ff0447976f6c8f4070e0fd683b86464a0e2d0b88 Mon Sep 17 00:00:00 2001 From: Ivan Chepurnyi Date: Tue, 2 May 2017 22:41:02 +0200 Subject: [PATCH 07/17] Add ZipParserFactory composite implementation --- .../Source/FileParser/ZipParserFactory.php | 94 ++++++++++++++++- .../Source/FileParser/FakeParserFactory.php | 13 ++- .../FileParser/ZipParserFactoryTest.php | 98 ++++++++++++++++-- .../Source/FileParser/_files/complete.zip | Bin 0 -> 1224 bytes .../Source/FileParser/_files/corrupted.zip | Bin 0 -> 189 bytes .../FileParser/_files/custom_option.zip | Bin 0 -> 202 bytes .../Import/Source/FileParser/_files/empty.zip | Bin 0 -> 22 bytes .../Import/Source/FileParser/_files/test.tsv | 1 + 8 files changed, 197 insertions(+), 9 deletions(-) create mode 100644 app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/_files/complete.zip create mode 100644 app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/_files/corrupted.zip create mode 100644 app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/_files/custom_option.zip create mode 100644 app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/_files/empty.zip create mode 100644 app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/_files/test.tsv diff --git a/app/code/Magento/ImportExport/Model/Import/Source/FileParser/ZipParserFactory.php b/app/code/Magento/ImportExport/Model/Import/Source/FileParser/ZipParserFactory.php index d9fdf86293ddd..4da3c4c258cdf 100644 --- a/app/code/Magento/ImportExport/Model/Import/Source/FileParser/ZipParserFactory.php +++ b/app/code/Magento/ImportExport/Model/Import/Source/FileParser/ZipParserFactory.php @@ -6,23 +6,113 @@ namespace Magento\ImportExport\Model\Import\Source\FileParser; +use Magento\Framework\App\Filesystem\DirectoryList; use Magento\Framework\Filesystem; class ZipParserFactory implements ParserFactoryInterface { private $filesystem; + private $isZipAvailable; + private $parserFactory; - public function __construct(Filesystem $filesystem, ParserFactoryInterface $factory) + public function __construct( + Filesystem $filesystem, + ParserFactoryInterface $parserFactory, + $isZipAvailable = null + ) { $this->filesystem = $filesystem; + $this->parserFactory = $parserFactory; + $this->isZipAvailable = $isZipAvailable ?? extension_loaded('zip'); } public function create($path, array $options = []) + { + $this->assertZipFileIsParsable($path); + + $sourceDirectory = $this->filesystem->getDirectoryRead(DirectoryList::ROOT); + $writeDirectory = $this->filesystem->getDirectoryWrite(DirectoryList::TMP); + + $this->assertFileExistance($path, $sourceDirectory); + + $zip = new \ZipArchive(); + + if ($zip->open($sourceDirectory->getAbsolutePath($path)) !== true) { + throw new CorruptedFileException($path); + } + + $files = $this->fetchFileNamesFromZipFile($zip); + + foreach ($files as $file) { + $destinationFilename = $this->extractFileIntoDirectory($zip, $writeDirectory, $file); + + try { + $parser = $this->parserFactory->create( + $destinationFilename, + ['directory_code' => DirectoryList::TMP] + $options + ); + } catch (UnsupportedPathException $e) { + continue; + } finally { + $writeDirectory->delete($destinationFilename); + } + + return $parser; + } + + throw new UnsupportedPathException($path); + } + + private function assertZipFileIsParsable($path) { if (substr($path, -4) !== '.zip') { throw new UnsupportedPathException($path); } - throw new CorruptedFileException($path); + if (!$this->isZipAvailable) { + throw new UnsupportedPathException($path, 'Zip extension is not available'); + } + } + + private function assertFileExistance($path, $directory) + { + if (!$directory->isFile($path)) { + throw new UnsupportedPathException($path); + } + } + + private function fetchFileNamesFromZipFile(\ZipArchive $zip) + { + $files = []; + + for ($fileIndex = 0; $fileIndex < $zip->numFiles; $fileIndex++) { + $fileName = $zip->getNameIndex($fileIndex); + if ($this->isDirectoryZipEntry($fileName) || !$fileName) { + continue; + } + + $files[] = $fileName; + } + + return $files; + } + + private function isDirectoryZipEntry($fileName) + { + return substr($fileName, -1) === '/'; + } + + private function extractFileIntoDirectory( + \ZipArchive $zip, + Filesystem\Directory\WriteInterface $writeDirectory, + $fileName + ) { + $destinationFilename = 'tmp-' . uniqid() . '-' . basename($fileName); + + $destinationFileWriter = $writeDirectory->openFile($destinationFilename); + $destinationFileWriter->write($zip->getFromName($fileName)); + $destinationFileWriter->close(); + + return $destinationFilename; } } diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/FakeParserFactory.php b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/FakeParserFactory.php index 8f2b433c00429..4f86c88ab3663 100644 --- a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/FakeParserFactory.php +++ b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/FakeParserFactory.php @@ -29,6 +29,8 @@ public function create($path, array $options = []) private function findParserInMap($path) { + $path = $this->trimTemporaryFilePrefix($path); + if (!isset($this->parser[$path])) { throw new UnsupportedPathException($path); } @@ -36,8 +38,17 @@ private function findParserInMap($path) return $this->parser[$path]; } - private function isParserMap(): bool + private function isParserMap() { return is_array($this->parser); } + + private function trimTemporaryFilePrefix($path) + { + if (strpos($path, 'tmp-') === 0) { + $path = preg_replace('/tmp-[a-z0-9]+-/i', '', $path); + } + + return $path; + } } diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/ZipParserFactoryTest.php b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/ZipParserFactoryTest.php index d3c6b4394b904..a44a160af7c76 100644 --- a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/ZipParserFactoryTest.php +++ b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/ZipParserFactoryTest.php @@ -12,30 +12,116 @@ class ZipParserFactoryTest extends \PHPUnit_Framework_TestCase { - public function testWhenCsvFileIsProvided_ParserIsNotCreated() + public function testWhenZipIsDisabled_ParserIsNotCreated() { - $this->setExpectedException(FileParser\UnsupportedPathException::class, 'Path "file.csv" is not supported'); + $parser = new FileParser\ZipParserFactory( + $this->createTestFilesystem(), + new FakeParserFactory(), + false + ); + + $this->setExpectedException(FileParser\UnsupportedPathException::class, 'Zip extension is not available'); + + $parser->create('file.zip'); + } + public function testWhenCsvFileIsProvided_ParserIsNotCreated() + { $parser = new FileParser\ZipParserFactory($this->createTestFilesystem(), new FakeParserFactory()); + $this->setExpectedException(FileParser\UnsupportedPathException::class, 'Path "file.csv" is not supported'); + $parser->create('file.csv'); } public function testWhenCorruptedZipFileIsProvided_ParserIsNotCreated() { + $parser = new FileParser\ZipParserFactory($this->createTestFilesystem(), new FakeParserFactory()); + $this->setExpectedException(FileParser\CorruptedFileException::class); + $parser->create('corrupted.zip'); + } + + public function testWhenZipFileDoesNotExists_ParserIsNotCreated() + { $parser = new FileParser\ZipParserFactory($this->createTestFilesystem(), new FakeParserFactory()); - $parser->create('corrupted.zip'); + $this->setExpectedException(FileParser\UnsupportedPathException::class, 'Path "unknown.zip" is not supported'); + + $parser->create('unknown.zip'); + } + + + public function testWhenEmptyZipFileIsProvided_ParserIsNotCreated() + { + $this->setExpectedException(FileParser\UnsupportedPathException::class, 'Path "empty.zip" is not supported'); + + $parserFactory = new FileParser\ZipParserFactory($this->createTestFilesystem(), new FakeParserFactory()); + $parserFactory->create('empty.zip'); } - private function createTestFilesystem($baseDirectory = null) + public function testWhenProperZipFileIsProvided_FirstFileIsParsed() { - $baseDirectory = $baseDirectory ?? __DIR__ . '/_files'; + $expectedParser = new FakeParser(); + $parserFactory = new FileParser\ZipParserFactory( + $this->createTestFilesystem(), + new FakeParserFactory([ + 'test.csv' => $expectedParser + ]) + ); + + $this->assertSame( + $expectedParser, + $parserFactory->create('complete.zip') + ); + } + + public function testWhenProperZipFileIsProvided_SecondFileIsParsed() + { + $expectedParser = new FakeParser(); + + $parserFactory = new FileParser\ZipParserFactory( + $this->createTestFilesystem(), + new FakeParserFactory([ + 'test.tsv' => $expectedParser + ]) + ); + + $this->assertSame( + $expectedParser, + $parserFactory->create('complete.zip') + ); + } + + public function testWhenProperZipFileIsProvidedWithOptions_CustomCsvFileIsParsed() + { + $fileSystem = $this->createTestFilesystem(); + + $parserFactory = new FileParser\ZipParserFactory( + $fileSystem, + new FileParser\CsvParserFactory($fileSystem, new FakeObjectManager()) + ); + + $parser = $parserFactory->create( + 'custom_option.zip', + [ + 'delimiter' => '|', + 'enclosure' => ';' + ] + ); + + $this->assertSame( + ['column1', 'column2', 'column3'], + $parser->getColumnNames() + ); + } + + private function createTestFilesystem() + { return new Filesystem( - new DirectoryList($baseDirectory), + new DirectoryList(__DIR__ . '/_files'), new Filesystem\Directory\ReadFactory(new Filesystem\DriverPool()), new Filesystem\Directory\WriteFactory(new Filesystem\DriverPool()) ); diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/_files/complete.zip b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/_files/complete.zip new file mode 100644 index 0000000000000000000000000000000000000000..b4c17bb771e8a3df8fba1d1561c0f38784a4c232 GIT binary patch literal 1224 zcmaiy!Ab&A6o!wE+LRO(Eh40ai=c%Sim1yrff_VWkrQoXav4NU5(rU`(5|*pLCYSY zWrRi8B3gz}5G~yVLFe2%=9%VBB1dL~`~BzsenwFXO#pxnN7Y8*?B=U9hHsPNml)WU zsy$~_YwK%+psmz(lV<-Q3V~2303c2l?;mEH&D2vq(poZJPNQJ_BJVC51?~By;6d6B z^1G#*G-_mfnIngACjbk#6c5f<2x3qx@6C%1r*OM*?}oIHJpFj>KCL^a?VVjoxO&&k zVKqUvRLA^I{B>gl^GC(Vd?L8o$|HqFeKpb67OTp{&3ILDwi&V$>D)rLlCg5Bq?Jh~ zL{hiJ+c({$Q75@el$W3y`i5o8IV+V(Z@a^&Ae#PRf#9^}XBPPdfCj<&F+KH~3ro-b z;EiIB=9P-mR=YIO4$>5QNt&Nel1BgT=i(*Oq+LiNyOewRxy4Csha^oynxaI0ORs1A zmNQr#?d7%(sS4vb)oU529&+kG&S8$iH=g4ajOTo)aumAxwG@tEnJ|E#%^*Mt2jB<5 CN(!_9 literal 0 HcmV?d00001 diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/_files/corrupted.zip b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/_files/corrupted.zip new file mode 100644 index 0000000000000000000000000000000000000000..567e7db20f304a972b5051adb623a083d037e715 GIT binary patch literal 189 zcmWIWW@h1K0D-0@i@fwgyBhd`Y!K#^u_`YrEhtG%(JQGa2@T<7V4iZ3BNBv5E4UdL zS-vtdFtCUKReCBU<|-uT7eQ3>26!_v$uZ+FltBWhfq?<2m0?LEhy~XNv4s_43z`i9 T-mGjO#f(5045Y(A90pzhyZj~# literal 0 HcmV?d00001 diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/_files/custom_option.zip b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/_files/custom_option.zip new file mode 100644 index 0000000000000000000000000000000000000000..98f29d6425ce8c475db1949f374632c873502edb GIT binary patch literal 202 zcmWIWW@Zs#U|`^2xLv%+OJVDwUm%_o5DPHKFqEVgm&E56lw{`T73(DzmxYFKGBE$G z=7 Date: Wed, 3 May 2017 01:51:05 +0200 Subject: [PATCH 08/17] Remove redundant properties from CSVParser --- .../Import/Source/FileParser/CsvParser.php | 21 ------------------- 1 file changed, 21 deletions(-) diff --git a/app/code/Magento/ImportExport/Model/Import/Source/FileParser/CsvParser.php b/app/code/Magento/ImportExport/Model/Import/Source/FileParser/CsvParser.php index 90ab0cd3e6473..174f81717e72c 100644 --- a/app/code/Magento/ImportExport/Model/Import/Source/FileParser/CsvParser.php +++ b/app/code/Magento/ImportExport/Model/Import/Source/FileParser/CsvParser.php @@ -11,27 +11,6 @@ class CsvParser implements ParserInterface { - /** - * Enclosure for values in CSV file - * - * @var string - */ - private $enclosure; - - /** - * Column delimiter - * - * @var string - */ - private $delimiter; - - /** - * Escape sequence character - * - * @var string - */ - private $escape; - /** * List of CSV parsing options * From 2088f5547081555810f3721d256f11d472c92e99 Mon Sep 17 00:00:00 2001 From: Ivan Chepurnyi Date: Wed, 3 May 2017 01:52:33 +0200 Subject: [PATCH 09/17] Remove un-used fixture --- .../Test/Unit/Model/Import/Source/FileParser/_files/test.tsv | 1 - 1 file changed, 1 deletion(-) delete mode 100644 app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/_files/test.tsv diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/_files/test.tsv b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/_files/test.tsv deleted file mode 100644 index 236642a3bdc3a..0000000000000 --- a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/_files/test.tsv +++ /dev/null @@ -1 +0,0 @@ -column1 column2 column3 From 99d6a1717f19301f4e4400c8d385bf481d393bea Mon Sep 17 00:00:00 2001 From: Ivan Chepurnyi Date: Wed, 3 May 2017 01:54:25 +0200 Subject: [PATCH 10/17] Add relative file path resolution for CSV and ZIP parser factories --- .../Source/FileParser/CsvParserFactory.php | 1 + .../Source/FileParser/ZipParserFactory.php | 1 + .../Source/FileParser/CsvParserFactoryTest.php | 10 ++++++++++ .../Source/FileParser/ZipParserFactoryTest.php | 17 +++++++++++++++++ 4 files changed, 29 insertions(+) diff --git a/app/code/Magento/ImportExport/Model/Import/Source/FileParser/CsvParserFactory.php b/app/code/Magento/ImportExport/Model/Import/Source/FileParser/CsvParserFactory.php index 03af6cf3dad70..aee5fe1d5465d 100644 --- a/app/code/Magento/ImportExport/Model/Import/Source/FileParser/CsvParserFactory.php +++ b/app/code/Magento/ImportExport/Model/Import/Source/FileParser/CsvParserFactory.php @@ -42,6 +42,7 @@ public function create($filePath, array $options = []) } $directory = $this->filesystem->getDirectoryRead($directoryCode); + $filePath = $directory->getRelativePath($filePath); if (!$directory->isFile($filePath)) { throw new \InvalidArgumentException(sprintf('File "%s" does not exists', $filePath)); diff --git a/app/code/Magento/ImportExport/Model/Import/Source/FileParser/ZipParserFactory.php b/app/code/Magento/ImportExport/Model/Import/Source/FileParser/ZipParserFactory.php index 4da3c4c258cdf..13ab2ee0ec944 100644 --- a/app/code/Magento/ImportExport/Model/Import/Source/FileParser/ZipParserFactory.php +++ b/app/code/Magento/ImportExport/Model/Import/Source/FileParser/ZipParserFactory.php @@ -33,6 +33,7 @@ public function create($path, array $options = []) $sourceDirectory = $this->filesystem->getDirectoryRead(DirectoryList::ROOT); $writeDirectory = $this->filesystem->getDirectoryWrite(DirectoryList::TMP); + $path = $sourceDirectory->getRelativePath($path); $this->assertFileExistance($path, $sourceDirectory); $zip = new \ZipArchive(); diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CsvParserFactoryTest.php b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CsvParserFactoryTest.php index c40777968207c..59a90c9b200d5 100644 --- a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CsvParserFactoryTest.php +++ b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CsvParserFactoryTest.php @@ -55,6 +55,16 @@ public function testWhenValidFileIsProvided_ParserIsCreated() ); } + public function testWhenAbsolutePathIsProvided_ParserIsCreated() + { + $factory = $this->createCsvParserFactory(); + + $this->assertCsvFile( + ['column1', 'column2', 'column3'], + $factory->create(__DIR__ . '/_files/test.csv') + ); + } + public function testWhenCustomDirectoryIsProvided_ParserIsCreatedFromIt() { $factory = $this->createCsvParserFactory(); diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/ZipParserFactoryTest.php b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/ZipParserFactoryTest.php index a44a160af7c76..0945ac9f830a5 100644 --- a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/ZipParserFactoryTest.php +++ b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/ZipParserFactoryTest.php @@ -78,6 +78,23 @@ public function testWhenProperZipFileIsProvided_FirstFileIsParsed() ); } + public function testWhenProperZipFileWithAbsolutePathIsProvided_FirstFileIsParsed() + { + $expectedParser = new FakeParser(); + + $parserFactory = new FileParser\ZipParserFactory( + $this->createTestFilesystem(), + new FakeParserFactory([ + 'test.csv' => $expectedParser + ]) + ); + + $this->assertSame( + $expectedParser, + $parserFactory->create(__DIR__ . '/_files/complete.zip') + ); + } + public function testWhenProperZipFileIsProvided_SecondFileIsParsed() { $expectedParser = new FakeParser(); From 71c528817d998ce18d5ecd0455dbc2e3d9766d0c Mon Sep 17 00:00:00 2001 From: Ivan Chepurnyi Date: Wed, 3 May 2017 01:56:10 +0200 Subject: [PATCH 11/17] Add file import source implementation based on FileParser --- .../ImportExport/Model/Import/Source/File.php | 33 +++++ .../Model/Import/Source/FileFactory.php | 53 ++++++++ .../Model/Import/Source/FileFactoryTest.php | 77 ++++++++++++ .../Unit/Model/Import/Source/FileTest.php | 114 ++++++++++++++++++ .../Test/Unit/Model/Import/_files/test.csv | 4 + 5 files changed, 281 insertions(+) create mode 100644 app/code/Magento/ImportExport/Model/Import/Source/File.php create mode 100644 app/code/Magento/ImportExport/Model/Import/Source/FileFactory.php create mode 100644 app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileFactoryTest.php create mode 100644 app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileTest.php create mode 100644 app/code/Magento/ImportExport/Test/Unit/Model/Import/_files/test.csv diff --git a/app/code/Magento/ImportExport/Model/Import/Source/File.php b/app/code/Magento/ImportExport/Model/Import/Source/File.php new file mode 100644 index 0000000000000..c1e154e9c42f4 --- /dev/null +++ b/app/code/Magento/ImportExport/Model/Import/Source/File.php @@ -0,0 +1,33 @@ +parser = $parser; + parent::__construct($this->parser->getColumnNames()); + } + + protected function _getNextRow() + { + return $this->parser->fetchRow(); + } + + public function rewind() + { + $this->parser->reset(); + parent::rewind(); + } + + +} diff --git a/app/code/Magento/ImportExport/Model/Import/Source/FileFactory.php b/app/code/Magento/ImportExport/Model/Import/Source/FileFactory.php new file mode 100644 index 0000000000000..868271fe53625 --- /dev/null +++ b/app/code/Magento/ImportExport/Model/Import/Source/FileFactory.php @@ -0,0 +1,53 @@ +parserFactory = $parserFactory; + $this->objectManager = $objectManager; + } + + /** + * Creates file source from file path + * + * @param string $path + * @param array $options + * @return AbstractSource + */ + public function createFromFilePath($path, array $options = []) + { + return $this->createFromFileParser( + $this->parserFactory->create($path, $options) + ); + } + + /** + * Creates file source from file parser + * + * @param ParserInterface $parser + * @return AbstractSource + */ + public function createFromFileParser(ParserInterface $parser) + { + return $this->objectManager->create(File::class, ['parser' => $parser]); + } +} diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileFactoryTest.php b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileFactoryTest.php new file mode 100644 index 0000000000000..100ded933a791 --- /dev/null +++ b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileFactoryTest.php @@ -0,0 +1,77 @@ +setExpectedException(UnsupportedPathException::class, 'Path "test.csv" is not supported'); + + $fileFactory = new FileFactory( + new FakeParserFactory(), + new FakeObjectManager() + ); + + $fileFactory->createFromFilePath('test.csv'); + } + + public function testGivenParserFactoryConfigured_WhenSourceIsCreatedByPath_RightInstanceIsCreated() + { + $fileFactory = new FileFactory( + new FakeParserFactory(new FakeParser(['column1', 'column2'])), + new FakeObjectManager() + ); + + $this->assertSame( + ['column1', 'column2'], + $fileFactory->createFromFilePath('test.csv')->getColNames() + ); + } + + public function testGivenCsvParserOptions_WhenSource_IsCreatedByPath_OptionsArePassedToParser() + { + $objectManager = new FakeObjectManager(); + $filesystem = new Filesystem( + new DirectoryList(__DIR__ . '/FileParser/_files'), + new Filesystem\Directory\ReadFactory(new Filesystem\DriverPool()), + new Filesystem\Directory\WriteFactory(new Filesystem\DriverPool()) + ); + + $fileFactory = new FileFactory( + new CsvParserFactory( + $filesystem, + $objectManager + ), + $objectManager + ); + + $file = $fileFactory->createFromFilePath('test_options.csv', ['delimiter' => '|', 'enclosure' => ';']); + + $this->assertSame( + ['column1', 'column2', 'column3'], + $file->getColNames() + ); + } + + public function testWhenSourceIsCreatedWithParser_FileIsCreated() + { + $fileFactory = new FileFactory(new FakeParserFactory(), new FakeObjectManager()); + $file = $fileFactory->createFromFileParser(new FakeParser(['column1', 'column2'])); + $this->assertSame(['column1', 'column2'], $file->getColNames()); + } +} diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileTest.php b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileTest.php new file mode 100644 index 0000000000000..5fe87088c4213 --- /dev/null +++ b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileTest.php @@ -0,0 +1,114 @@ +assertSame(['column1', 'column2'], $file->getColNames()); + } + + public function testWhenEmptyFileParserIsProvided_IteratorIsNotValid() + { + $file = new File(new FakeParser( + ['column1', 'column2'] + )); + $file->rewind(); + $this->assertFalse($file->valid()); + } + + public function testWhenSomeDataInFileParserIsProvided_IteratorIsValid() + { + $file = new File(new FakeParser( + ['column1', 'column2'], + [ + ['value1', 'value2'] + ] + )); + + $file->rewind(); + $this->assertTrue($file->valid()); + } + + public function testWhenSomeDataInFileParserIsProvided_IteratorRowIsReturned() + { + $file = new File(new FakeParser( + ['column1', 'column2'], + [ + ['value1', 'value2'] + ] + )); + + $file->rewind(); + + $this->assertSame( + [ + 'column1' => 'value1', + 'column2' => 'value2' + ], + $file->current() + ); + } + + public function testWhenSpecificPositionIsSet_ProperIteratorRowIsReturned() + { + $file = new File(new FakeParser( + ['column1', 'column2'], + [ + ['wrong', 'wrong'], + ['correct1', 'correct2'], + ['wrong', 'wrong'], + ] + )); + + $file->rewind(); + $file->seek(1); + + $this->assertSame( + [ + 'column1' => 'correct1', + 'column2' => 'correct2' + ], + $file->current() + ); + } + + + public function testWhenIteratorIsRewinded_ParserRestarts() + { + $file = new File(new FakeParser( + ['column1', 'column2'], + [ + ['value1.1', 'value2.1'], + ['value1.2', 'value2.2'], + ['value1.3', 'value2.3'] + ] + )); + + $file->rewind(); + $file->next(); + $file->next(); + $file->rewind(); + + $this->assertSame( + [ + 'column1' => 'value1.1', + 'column2' => 'value2.1' + ], + $file->current() + ); + } + + +} diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/_files/test.csv b/app/code/Magento/ImportExport/Test/Unit/Model/Import/_files/test.csv new file mode 100644 index 0000000000000..ef071b0dab0bf --- /dev/null +++ b/app/code/Magento/ImportExport/Test/Unit/Model/Import/_files/test.csv @@ -0,0 +1,4 @@ +column1,"column2" +value1,value2 +3,4 +5 From 2ff1d0fc237dedc9149b557b3aa8e28427b8e04f Mon Sep 17 00:00:00 2001 From: Ivan Chepurnyi Date: Wed, 3 May 2017 01:57:46 +0200 Subject: [PATCH 12/17] Replace Adapter implementation with FileFactory, mark previous implementations as depracated --- .../ImportExport/Model/Import/Adapter.php | 101 +++++++++--- .../ImportExport/Model/Import/Source/Csv.php | 2 + .../ImportExport/Model/Import/Source/Zip.php | 2 + .../Test/Unit/Model/Import/AdapterTest.php | 156 ++++++++++++++++-- app/code/Magento/ImportExport/etc/di.xml | 13 ++ 5 files changed, 237 insertions(+), 37 deletions(-) diff --git a/app/code/Magento/ImportExport/Model/Import/Adapter.php b/app/code/Magento/ImportExport/Model/Import/Adapter.php index 0242ae6096316..5d3e483e08f81 100644 --- a/app/code/Magento/ImportExport/Model/Import/Adapter.php +++ b/app/code/Magento/ImportExport/Model/Import/Adapter.php @@ -5,7 +5,11 @@ */ namespace Magento\ImportExport\Model\Import; +use Magento\Framework\App\ObjectManager; use Magento\Framework\Filesystem\Directory\Write; +use Magento\ImportExport\Model\Import\Source\FileFactory; +use Magento\ImportExport\Model\Import\Source\FileParser\CorruptedFileException; +use Magento\ImportExport\Model\Import\Source\FileParser\UnsupportedPathException; /** * Import adapter model @@ -14,6 +18,15 @@ */ class Adapter { + /** @var FileFactory */ + private $fileSourceFactory; + + public function __construct(FileFactory $fileSourceFactory) + { + $this->fileSourceFactory = $fileSourceFactory; + } + + /** * Adapter factory. Checks for availability, loads and create instance of import adapter object. * @@ -25,29 +38,12 @@ class Adapter * @return AbstractSource * * @throws \Magento\Framework\Exception\LocalizedException + * @deprecated + * @see Adapter::createSourceByPath() */ public static function factory($type, $directory, $source, $options = null) { - if (!is_string($type) || !$type) { - throw new \Magento\Framework\Exception\LocalizedException( - __('The adapter type must be a non-empty string.') - ); - } - $adapterClass = 'Magento\ImportExport\Model\Import\Source\\' . ucfirst(strtolower($type)); - - if (!class_exists($adapterClass)) { - throw new \Magento\Framework\Exception\LocalizedException( - __('\'%1\' file extension is not supported', $type) - ); - } - $adapter = new $adapterClass($source, $directory, $options); - - if (!$adapter instanceof AbstractSource) { - throw new \Magento\Framework\Exception\LocalizedException( - __('Adapter must be an instance of \Magento\ImportExport\Model\Import\AbstractSource') - ); - } - return $adapter; + return self::createBackwardCompatibleInstance()->createSourceByPath($source, $options); } /** @@ -58,9 +54,72 @@ public static function factory($type, $directory, $source, $options = null) * @param mixed $options OPTIONAL Adapter constructor options * * @return AbstractSource + * @deprecated + * @see Adapter::createSourceByPath() */ public static function findAdapterFor($source, $directory, $options = null) { - return self::factory(pathinfo($source, PATHINFO_EXTENSION), $directory, $source, $options); + return self::createBackwardCompatibleInstance()->createSourceByPath($source, $options); + } + + + + /** + * Finds source for import by file path + * + * @param $path + * @param $options + * + * @return AbstractSource + */ + public function createSourceByPath($path, $options = []) + { + try { + $options = $this->mapBackwardCompatibleFileParserOption($options); + $parser = $this->fileSourceFactory->createFromFilePath($path, $options); + } catch (UnsupportedPathException $e) { + $this->throwUnsupportedFileException($path); + } catch (CorruptedFileException $e) { + $this->throwUnsupportedFileException($path); + } + + return $parser; + } + + + private function extractFileExtension($path) + { + $fileName = basename($path); + + if (strpos($path, '.') === false) { + return $fileName; + } + + $fileExtension = substr($fileName, strrpos($fileName, '.') + 1); + return $fileExtension; + } + + private function throwUnsupportedFileException($path) + { + $fileExtension = $this->extractFileExtension($path); + throw new \Magento\Framework\Exception\LocalizedException( + __('\'%1\' file extension is not supported', $fileExtension) + ); + } + + private function mapBackwardCompatibleFileParserOption($options) + { + if (!is_array($options)) { + $options = ['delimiter' => $options ?? ',']; + } + return $options; + } + + /** + * @return self + */ + private static function createBackwardCompatibleInstance() + { + return ObjectManager::getInstance()->create(self::class); } } diff --git a/app/code/Magento/ImportExport/Model/Import/Source/Csv.php b/app/code/Magento/ImportExport/Model/Import/Source/Csv.php index ad95ca4d45cc4..41b926ae8c7f3 100644 --- a/app/code/Magento/ImportExport/Model/Import/Source/Csv.php +++ b/app/code/Magento/ImportExport/Model/Import/Source/Csv.php @@ -7,6 +7,8 @@ /** * CSV import adapter + * + * @deprecated */ class Csv extends \Magento\ImportExport\Model\Import\AbstractSource { diff --git a/app/code/Magento/ImportExport/Model/Import/Source/Zip.php b/app/code/Magento/ImportExport/Model/Import/Source/Zip.php index b7fafc43ca4ed..ff92e035cb0da 100644 --- a/app/code/Magento/ImportExport/Model/Import/Source/Zip.php +++ b/app/code/Magento/ImportExport/Model/Import/Source/Zip.php @@ -7,6 +7,8 @@ /** * Zip import adapter. + * + * @deprecated */ class Zip extends Csv { diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/AdapterTest.php b/app/code/Magento/ImportExport/Test/Unit/Model/Import/AdapterTest.php index 63a7d3c2078b6..b01f9df54f631 100644 --- a/app/code/Magento/ImportExport/Test/Unit/Model/Import/AdapterTest.php +++ b/app/code/Magento/ImportExport/Test/Unit/Model/Import/AdapterTest.php @@ -5,33 +5,157 @@ */ namespace Magento\ImportExport\Test\Unit\Model\Import; -use Magento\ImportExport\Model\Import\Adapter as Adapter; +use Magento\Framework\App\Filesystem\DirectoryList; +use Magento\Framework\Exception\LocalizedException; +use Magento\Framework\Filesystem; +use Magento\ImportExport\Model\Import\Adapter; +use Magento\ImportExport\Model\Import\Source\FileFactory; +use Magento\ImportExport\Model\Import\Source\FileParser\CsvParserFactory; +use Magento\ImportExport\Model\Import\Source\FileParser\ParserFactoryInterface; +use Magento\ImportExport\Model\Import\Source\FileParser\ZipParserFactory; +use Magento\ImportExport\Test\Unit\Model\Import\Source\FileParser\FakeObjectManager; +use Magento\ImportExport\Test\Unit\Model\Import\Source\FileParser\FakeParser; +use Magento\ImportExport\Test\Unit\Model\Import\Source\FileParser\FakeParserFactory; class AdapterTest extends \PHPUnit_Framework_TestCase { - /** - * @var Adapter|\PHPUnit_Framework_MockObject_MockObject - */ - protected $adapter; + public function testFactory() + { + $this->markTestSkipped('Skipped because factory method has static modifier'); + } - protected function setUp() + public function testFindAdapterFor() + { + $this->markTestSkipped('Skipped because findAdapterFor method has static modifier'); + } + + public function testWhenFileIsNotSupported_FileExtensionRelatedExceptionIsThrown() { - $this->adapter = $this->getMock( - \Magento\ImportExport\Model\Import\Adapter::class, - [], - [], - '', - false + $adapter = new Adapter( + $this->createFileFactory() ); + + $this->setExpectedException(LocalizedException::class, '\'xyz\' file extension is not supported'); + + $adapter->createSourceByPath('file.xyz'); } - public function testFactory() + public function testWhenFileIsNotSupportedAndFileNameHasNoExtension_LocalizedExceptionWithBasenameIsThrown() { - $this->markTestSkipped('Skipped because factory method has static modifier'); + $adapter = new Adapter( + $this->createFileFactory() + ); + + $this->setExpectedException(LocalizedException::class, '\'file\' file extension is not supported'); + + $adapter->createSourceByPath('file'); } - public function testFindAdapterFor() + public function testWhenFileAdapterIsNotSupported_FileExtensionRelatedExceptionIsThrown() { - $this->markTestSkipped('Skipped because findAdapterFor method has static modifier'); + $adapter = new Adapter( + $this->createFileFactory( + new ZipParserFactory( + $this->createTestFilesystem(), + new FakeParserFactory() + ) + ) + ); + + $this->setExpectedException(LocalizedException::class, '\'zip\' file extension is not supported'); + + $adapter->createSourceByPath('corrupted.zip'); + } + + public function testWhenParserIsAvailable_FileSourceIsReturned() + { + $adapter = new Adapter( + $this->createFileFactory( + new FakeParserFactory( + new FakeParser(['column1', 'column2']) + ) + ) + ); + + $source = $adapter->createSourceByPath('test.csv'); + $this->assertSame(['column1', 'column2'], $source->getColNames()); + } + + public function testWhenParserOptionsAreProvided_FileSourceIsReadCorrectly() + { + $adapter = new Adapter( + $this->createFileFactory( + new CsvParserFactory( + $this->createTestFilesystem(), + new FakeObjectManager() + ) + ) + ); + + $source = $adapter->createSourceByPath( + 'test_options.csv', + [ + 'delimiter' => '|', + 'enclosure' => ';' + ] + ); + + $this->assertSame(['column1', 'column2', 'column3'], $source->getColNames()); + } + + public function testWhenParserOptionIsProvidedAsString_FileSourceIsReadCorrectly() + { + $adapter = new Adapter( + $this->createFileFactory( + new CsvParserFactory( + $this->createTestFilesystem(), + new FakeObjectManager() + ) + ) + ); + + $source = $adapter->createSourceByPath( + 'test_options.csv', + '|' + ); + + $this->assertSame(['column1', ';column2;', 'column3'], $source->getColNames()); + } + + public function testWhenParserOptionIsProvidedAsNull_FileSourceIsReadCorrectly() + { + $adapter = new Adapter( + $this->createFileFactory( + new CsvParserFactory( + $this->createTestFilesystem(), + new FakeObjectManager() + ) + ) + ); + + $source = $adapter->createSourceByPath( + 'test.csv', + null + ); + + $this->assertSame(['column1', 'column2', 'column3'], $source->getColNames()); + } + + + private function createTestFilesystem() + { + return new Filesystem( + new DirectoryList(__DIR__ . '/Source/FileParser/_files'), + new Filesystem\Directory\ReadFactory(new Filesystem\DriverPool()), + new Filesystem\Directory\WriteFactory(new Filesystem\DriverPool()) + ); + } + + private function createFileFactory(ParserFactoryInterface $parserFactory = null) + { + return new FileFactory( + $parserFactory ?? new FakeParserFactory(), + new FakeObjectManager() + ); } } diff --git a/app/code/Magento/ImportExport/etc/di.xml b/app/code/Magento/ImportExport/etc/di.xml index 47acf7a356d93..9b8d959514dde 100644 --- a/app/code/Magento/ImportExport/etc/di.xml +++ b/app/code/Magento/ImportExport/etc/di.xml @@ -10,6 +10,8 @@ + + @@ -17,4 +19,15 @@ + + + + + Magento\ImportExport\Model\Import\Source\FileParser\CsvParserFactory + + Magento\ImportExport\Model\Import\Source\FileParser\ZipParserFactory\Proxy + + + + From e8d21fc52aa5f01d8af0dd193ab006a0cae2fd5d Mon Sep 17 00:00:00 2001 From: Ivan Chepurnyi Date: Wed, 3 May 2017 02:30:20 +0200 Subject: [PATCH 13/17] Fix braces and spaces for PSR2 standard compliency --- app/code/Magento/ImportExport/Model/Import/Source/File.php | 2 -- .../ImportExport/Model/Import/Source/FileParser/CsvParser.php | 3 --- .../Model/Import/Source/FileParser/CsvParserFactory.php | 2 +- .../Import/Source/FileParser/UnsupportedPathException.php | 1 - .../Model/Import/Source/FileParser/ZipParserFactory.php | 3 +-- .../Import/Source/FileParser/CompositeParserFactoryTest.php | 2 -- .../Unit/Model/Import/Source/FileParser/FakeObjectManager.php | 1 - .../Test/Unit/Model/Import/Source/FileParser/FakeParser.php | 2 -- .../ImportExport/Test/Unit/Model/Import/Source/FileTest.php | 3 --- 9 files changed, 2 insertions(+), 17 deletions(-) diff --git a/app/code/Magento/ImportExport/Model/Import/Source/File.php b/app/code/Magento/ImportExport/Model/Import/Source/File.php index c1e154e9c42f4..af0d3c7a3c5b4 100644 --- a/app/code/Magento/ImportExport/Model/Import/Source/File.php +++ b/app/code/Magento/ImportExport/Model/Import/Source/File.php @@ -28,6 +28,4 @@ public function rewind() $this->parser->reset(); parent::rewind(); } - - } diff --git a/app/code/Magento/ImportExport/Model/Import/Source/FileParser/CsvParser.php b/app/code/Magento/ImportExport/Model/Import/Source/FileParser/CsvParser.php index 174f81717e72c..9d277f3d3ac28 100644 --- a/app/code/Magento/ImportExport/Model/Import/Source/FileParser/CsvParser.php +++ b/app/code/Magento/ImportExport/Model/Import/Source/FileParser/CsvParser.php @@ -72,7 +72,6 @@ public function reset() $this->fetchCsvLine(); } - private function fetchCsvLine() { return $this->file->readCsv( @@ -94,7 +93,6 @@ private function mapRowData($row) return $result; } - public function __destruct() { $this->file->close(); @@ -112,5 +110,4 @@ private function extractRowValue($row, $index) { return (isset($row[$index]) ? $row[$index] : ''); } - } diff --git a/app/code/Magento/ImportExport/Model/Import/Source/FileParser/CsvParserFactory.php b/app/code/Magento/ImportExport/Model/Import/Source/FileParser/CsvParserFactory.php index aee5fe1d5465d..5e2a6296904dd 100644 --- a/app/code/Magento/ImportExport/Model/Import/Source/FileParser/CsvParserFactory.php +++ b/app/code/Magento/ImportExport/Model/Import/Source/FileParser/CsvParserFactory.php @@ -37,7 +37,7 @@ public function create($filePath, array $options = []) { $directoryCode = $options['directory_code'] ?? DirectoryList::ROOT; - if (substr($filePath,-4) !== '.csv') { + if (substr($filePath, -4) !== '.csv') { throw new UnsupportedPathException($filePath); } diff --git a/app/code/Magento/ImportExport/Model/Import/Source/FileParser/UnsupportedPathException.php b/app/code/Magento/ImportExport/Model/Import/Source/FileParser/UnsupportedPathException.php index bacd3283666cd..4ff1bc3a7c635 100644 --- a/app/code/Magento/ImportExport/Model/Import/Source/FileParser/UnsupportedPathException.php +++ b/app/code/Magento/ImportExport/Model/Import/Source/FileParser/UnsupportedPathException.php @@ -21,5 +21,4 @@ public function getPath() { return $this->path; } - } diff --git a/app/code/Magento/ImportExport/Model/Import/Source/FileParser/ZipParserFactory.php b/app/code/Magento/ImportExport/Model/Import/Source/FileParser/ZipParserFactory.php index 13ab2ee0ec944..503ac87d257ce 100644 --- a/app/code/Magento/ImportExport/Model/Import/Source/FileParser/ZipParserFactory.php +++ b/app/code/Magento/ImportExport/Model/Import/Source/FileParser/ZipParserFactory.php @@ -19,8 +19,7 @@ public function __construct( Filesystem $filesystem, ParserFactoryInterface $parserFactory, $isZipAvailable = null - ) - { + ) { $this->filesystem = $filesystem; $this->parserFactory = $parserFactory; $this->isZipAvailable = $isZipAvailable ?? extension_loaded('zip'); diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CompositeParserFactoryTest.php b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CompositeParserFactoryTest.php index d75269dbae76f..c6763f1ac3096 100644 --- a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CompositeParserFactoryTest.php +++ b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CompositeParserFactoryTest.php @@ -61,6 +61,4 @@ public function testWhenFactoryIsProvidedViaConstructor_ParserIsReturned() $this->assertSame($expectedParser, $compositeFactory->create('file.csv')); } - - } diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/FakeObjectManager.php b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/FakeObjectManager.php index 6cdb0c889e0bf..537db6d710f9a 100644 --- a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/FakeObjectManager.php +++ b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/FakeObjectManager.php @@ -58,5 +58,4 @@ private function assertParameterValue(array $arguments, $parameter): array } return $arguments; } - } diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/FakeParser.php b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/FakeParser.php index 5e66ca0351fea..f27e4d988470c 100644 --- a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/FakeParser.php +++ b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/FakeParser.php @@ -14,7 +14,6 @@ class FakeParser implements ParserInterface private $columns; private $position; - public function __construct(array $columns = [], array $rows = []) { $this->columns = $columns; @@ -46,5 +45,4 @@ private function isEndOfRows() { return !isset($this->rows[$this->position]); } - } diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileTest.php b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileTest.php index 5fe87088c4213..aead73856d1f2 100644 --- a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileTest.php +++ b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileTest.php @@ -6,7 +6,6 @@ namespace Magento\ImportExport\Test\Unit\Model\Import\Source; - use Magento\ImportExport\Model\Import\Source\File; use Magento\ImportExport\Test\Unit\Model\Import\Source\FileParser\FakeParser; @@ -109,6 +108,4 @@ public function testWhenIteratorIsRewinded_ParserRestarts() $file->current() ); } - - } From 1de2b681597aa1ae662448d3afcf47a5505a4602 Mon Sep 17 00:00:00 2001 From: Ivan Chepurnyi Date: Wed, 3 May 2017 02:53:51 +0200 Subject: [PATCH 14/17] Add extension points for source options retrieval --- .../Controller/Adminhtml/Import/Validate.php | 15 ++++++++++++++- app/code/Magento/ImportExport/Model/Import.php | 14 +++++++++++++- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/app/code/Magento/ImportExport/Controller/Adminhtml/Import/Validate.php b/app/code/Magento/ImportExport/Controller/Adminhtml/Import/Validate.php index 0763602a8fddd..66b44cab00633 100644 --- a/app/code/Magento/ImportExport/Controller/Adminhtml/Import/Validate.php +++ b/app/code/Magento/ImportExport/Controller/Adminhtml/Import/Validate.php @@ -45,7 +45,7 @@ public function execute() $import->uploadSource(), $this->_objectManager->create(\Magento\Framework\Filesystem::class) ->getDirectoryWrite(DirectoryList::ROOT), - $data[$import::FIELD_FIELD_SEPARATOR] + $this->getSourceAdapterOptions($data) ); $this->processValidationResult($import->validateSource($source), $resultBlock); } catch (\Magento\Framework\Exception\LocalizedException $e) { @@ -121,4 +121,17 @@ private function getImport() } return $this->import; } + + /** + * Returns options for source adapter + * + * @param $data + * @return array + */ + protected function getSourceAdapterOptions($data) + { + return [ + 'delimiter' => $data[Import::FIELD_FIELD_SEPARATOR] + ]; + } } diff --git a/app/code/Magento/ImportExport/Model/Import.php b/app/code/Magento/ImportExport/Model/Import.php index 64219d48bff3f..69464d467172c 100644 --- a/app/code/Magento/ImportExport/Model/Import.php +++ b/app/code/Magento/ImportExport/Model/Import.php @@ -267,7 +267,7 @@ protected function _getSourceAdapter($sourceFile) return \Magento\ImportExport\Model\Import\Adapter::findAdapterFor( $sourceFile, $this->_filesystem->getDirectoryWrite(DirectoryList::ROOT), - $this->getData(self::FIELD_FIELD_SEPARATOR) + $this->getSourceAdapterOptions() ); } @@ -776,4 +776,16 @@ public function getDeletedItemsCount() { return $this->_getEntityAdapter()->getDeletedItemsCount(); } + + /** + * Returns options for source adapter + * + * @return array + */ + protected function getSourceAdapterOptions() + { + return [ + 'delimiter' => $this->getData(self::FIELD_FIELD_SEPARATOR) + ]; + } } From f654f06694664702fa72228e479227871afd047f Mon Sep 17 00:00:00 2001 From: Ivan Chepurnyi Date: Wed, 3 May 2017 03:02:45 +0200 Subject: [PATCH 15/17] Add default value to extension point --- .../ImportExport/Controller/Adminhtml/Import/Validate.php | 2 +- app/code/Magento/ImportExport/Model/Import.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/code/Magento/ImportExport/Controller/Adminhtml/Import/Validate.php b/app/code/Magento/ImportExport/Controller/Adminhtml/Import/Validate.php index 66b44cab00633..9d8ccf13b4933 100644 --- a/app/code/Magento/ImportExport/Controller/Adminhtml/Import/Validate.php +++ b/app/code/Magento/ImportExport/Controller/Adminhtml/Import/Validate.php @@ -131,7 +131,7 @@ private function getImport() protected function getSourceAdapterOptions($data) { return [ - 'delimiter' => $data[Import::FIELD_FIELD_SEPARATOR] + 'delimiter' => $data[Import::FIELD_FIELD_SEPARATOR] ?: ',' ]; } } diff --git a/app/code/Magento/ImportExport/Model/Import.php b/app/code/Magento/ImportExport/Model/Import.php index 69464d467172c..882235e1749bb 100644 --- a/app/code/Magento/ImportExport/Model/Import.php +++ b/app/code/Magento/ImportExport/Model/Import.php @@ -785,7 +785,7 @@ public function getDeletedItemsCount() protected function getSourceAdapterOptions() { return [ - 'delimiter' => $this->getData(self::FIELD_FIELD_SEPARATOR) + 'delimiter' => $this->getData(self::FIELD_FIELD_SEPARATOR) ?: ',' ]; } } From e5c632795d8cbd1efadfe76ead69b6ff6a1e1710 Mon Sep 17 00:00:00 2001 From: Ivan Chepurnyi Date: Thu, 18 May 2017 11:46:37 -0500 Subject: [PATCH 16/17] Fix static code analysis issues with test code --- .../Model/Import/Source/FileFactoryTest.php | 8 ++--- .../FileParser/CompositeParserFactoryTest.php | 18 +++++------ .../FileParser/CorruptedPathExceptionTest.php | 24 +++++++------- .../FileParser/CsvParserFactoryTest.php | 23 ++++++------- .../Source/FileParser/CsvParserTest.php | 32 +++++++++---------- .../UnsupportedPathExceptionTest.php | 24 +++++++------- .../FileParser/ZipParserFactoryTest.php | 31 ++++++++---------- .../Unit/Model/Import/Source/FileTest.php | 13 ++++---- 8 files changed, 82 insertions(+), 91 deletions(-) diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileFactoryTest.php b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileFactoryTest.php index 100ded933a791..03268bc027993 100644 --- a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileFactoryTest.php +++ b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileFactoryTest.php @@ -18,7 +18,7 @@ class FileFactoryTest extends \PHPUnit_Framework_TestCase { - public function testGivenParserFactoryEmpty_WhenSourceIsCreatedByPath_NoSourceIsCreated() + public function testGivenParserFactoryEmptyWhenSourceIsCreatedByPathThenNoSourceIsCreated() { $this->setExpectedException(UnsupportedPathException::class, 'Path "test.csv" is not supported'); @@ -30,7 +30,7 @@ public function testGivenParserFactoryEmpty_WhenSourceIsCreatedByPath_NoSourceIs $fileFactory->createFromFilePath('test.csv'); } - public function testGivenParserFactoryConfigured_WhenSourceIsCreatedByPath_RightInstanceIsCreated() + public function testGivenParserFactoryConfiguredWhenSourceIsCreatedByPathThenRightInstanceIsCreated() { $fileFactory = new FileFactory( new FakeParserFactory(new FakeParser(['column1', 'column2'])), @@ -43,7 +43,7 @@ public function testGivenParserFactoryConfigured_WhenSourceIsCreatedByPath_Right ); } - public function testGivenCsvParserOptions_WhenSource_IsCreatedByPath_OptionsArePassedToParser() + public function testWhenSourceIsCreatedByPathWithCsvOptionsThenOptionsArePassedToParser() { $objectManager = new FakeObjectManager(); $filesystem = new Filesystem( @@ -68,7 +68,7 @@ public function testGivenCsvParserOptions_WhenSource_IsCreatedByPath_OptionsAreP ); } - public function testWhenSourceIsCreatedWithParser_FileIsCreated() + public function testWhenSourceIsCreatedWithParserThenFileIsCreated() { $fileFactory = new FileFactory(new FakeParserFactory(), new FakeObjectManager()); $file = $fileFactory->createFromFileParser(new FakeParser(['column1', 'column2'])); diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CompositeParserFactoryTest.php b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CompositeParserFactoryTest.php index c6763f1ac3096..fff6800712061 100644 --- a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CompositeParserFactoryTest.php +++ b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CompositeParserFactoryTest.php @@ -11,26 +11,24 @@ class CompositeParserFactoryTest extends \PHPUnit_Framework_TestCase { - public function testWhenPathIsProvidedButNoFactoriesAreSetup_NoParserIsCreated() + public function testWhenPathIsProvidedButNoFactoriesAreSetupThenNoParserIsCreated() { - $compositeFactory = new CompositeParserFactory(); - $this->setExpectedException(UnsupportedPathException::class, 'Path "file.csv" is not supported'); + $compositeFactory = new CompositeParserFactory(); $compositeFactory->create('file.csv'); } - public function testWhenPathIsNotSupportedByAnyFactory_NoParserIsCreated() + public function testWhenPathIsNotSupportedByAnyFactoryThenNoParserIsCreated() { - $compositeFactory = new CompositeParserFactory(); - $compositeFactory->addParserFactory(new FakeParserFactory(['file.csv' => new FakeParser()])); - $this->setExpectedException(UnsupportedPathException::class, 'Path "file.zip" is not supported'); + $compositeFactory = new CompositeParserFactory(); + $compositeFactory->addParserFactory(new FakeParserFactory(['file.csv' => new FakeParser()])); $compositeFactory->create('file.zip'); } - public function testWhenPathIsSupportedByFirstFactory_ParserIsReturned() + public function testWhenPathIsSupportedByFirstFactoryThenParserIsReturned() { $expectedParser = new FakeParser(); @@ -41,7 +39,7 @@ public function testWhenPathIsSupportedByFirstFactory_ParserIsReturned() $this->assertSame($expectedParser, $compositeFactory->create('file.csv')); } - public function testWhenPathIsSupportedBySecondFactory_ParserIsReturned() + public function testWhenPathIsSupportedBySecondFactoryThenParserIsReturned() { $expectedParser = new FakeParser(); @@ -53,7 +51,7 @@ public function testWhenPathIsSupportedBySecondFactory_ParserIsReturned() } - public function testWhenFactoryIsProvidedViaConstructor_ParserIsReturned() + public function testWhenFactoryIsProvidedViaConstructorThenParserIsReturned() { $expectedParser = new FakeParser(); diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CorruptedPathExceptionTest.php b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CorruptedPathExceptionTest.php index 3f40c4396b840..abcb4a5a897b5 100644 --- a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CorruptedPathExceptionTest.php +++ b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CorruptedPathExceptionTest.php @@ -10,31 +10,31 @@ class CorruptedPathExceptionTest extends \PHPUnit_Framework_TestCase { - public function testWhenNoArgumentsAreProvided_FileNameIsEmpty() + public function testWhenNoArgumentsAreProvidedThenFileNameIsEmpty() { - $corruptedFileException = new CorruptedFileException(); + $exception = new CorruptedFileException(); - $this->assertEmpty($corruptedFileException->getFileName()); + $this->assertEmpty($exception->getFileName()); } - public function testWhenFileNameIsProvided_FileNameCanBeRetrievedLater() + public function testWhenFileNameIsProvidedThenFileNameCanBeRetrievedLater() { - $corruptedFileException = new CorruptedFileException('file.csv'); + $exception = new CorruptedFileException('file.csv'); - $this->assertSame('file.csv', $corruptedFileException->getFileName()); + $this->assertSame('file.csv', $exception->getFileName()); } - public function testWhenMessageIsProvided_MessageCanBeRetrievedLater() + public function testWhenMessageIsProvidedThenMessageCanBeRetrievedLater() { - $corruptedFileException = new CorruptedFileException('', 'My message'); + $exception = new CorruptedFileException('', 'My message'); - $this->assertSame('My message', $corruptedFileException->getMessage()); + $this->assertSame('My message', $exception->getMessage()); } - public function testWhenNoMessageIsProvided_MessageIsGeneratedFromPath() + public function testWhenNoMessageIsProvidedThenMessageIsGeneratedFromPath() { - $corruptedFileException = new CorruptedFileException('file.csv'); + $exception = new CorruptedFileException('file.csv'); - $this->assertSame('File "file.csv" is corrupted', $corruptedFileException->getMessage()); + $this->assertSame('File "file.csv" is corrupted', $exception->getMessage()); } } diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CsvParserFactoryTest.php b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CsvParserFactoryTest.php index 59a90c9b200d5..17023016800b3 100644 --- a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CsvParserFactoryTest.php +++ b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CsvParserFactoryTest.php @@ -12,40 +12,37 @@ class CsvParserFactoryTest extends \PHPUnit_Framework_TestCase { - public function testWhenFilePathIsNotCsv_NoParserIsCreated() + public function testWhenFilePathIsNotCsvThenNoParserIsCreated() { - $factory = $this->createCsvParserFactory(); - $this->setExpectedException(FileParser\UnsupportedPathException::class); + $factory = $this->createCsvParserFactory(); $factory->create('test.zip'); } - public function testWhenFileDoesNotExist_NoParserIsCreated() + public function testWhenFileDoesNotExistThenNoParserIsCreated() { - $factory = $this->createCsvParserFactory(); - $this->setExpectedException( \InvalidArgumentException::class, 'File "non_existing_file.csv" does not exists' ); + $factory = $this->createCsvParserFactory(); $factory->create('non_existing_file.csv'); } - public function testWhenFileIsNotAccesible_NoParserIsCreated() + public function testWhenFileIsNotAccesibleThenNoParserIsCreated() { - $factory = $this->createCsvParserFactory(tempnam(sys_get_temp_dir(), 'non_created')); - $this->setExpectedException( \InvalidArgumentException::class, 'File "test.csv" does not exists' ); + $factory = $this->createCsvParserFactory(tempnam(sys_get_temp_dir(), 'non_created')); $factory->create('test.csv'); } - public function testWhenValidFileIsProvided_ParserIsCreated() + public function testWhenValidFileIsProvidedThenParserIsCreated() { $factory = $this->createCsvParserFactory(); @@ -55,7 +52,7 @@ public function testWhenValidFileIsProvided_ParserIsCreated() ); } - public function testWhenAbsolutePathIsProvided_ParserIsCreated() + public function testWhenAbsolutePathIsProvidedThenParserIsCreated() { $factory = $this->createCsvParserFactory(); @@ -65,7 +62,7 @@ public function testWhenAbsolutePathIsProvided_ParserIsCreated() ); } - public function testWhenCustomDirectoryIsProvided_ParserIsCreatedFromIt() + public function testWhenCustomDirectoryIsProvidedThenParserIsCreatedFromIt() { $factory = $this->createCsvParserFactory(); @@ -75,7 +72,7 @@ public function testWhenCustomDirectoryIsProvided_ParserIsCreatedFromIt() ); } - public function testWhenCustomCSVOptionsProvided_ParserIsCreatedFromIt() + public function testWhenCustomCSVOptionsProvidedThenParserIsCreatedFromIt() { $factory = $this->createCsvParserFactory(); diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CsvParserTest.php b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CsvParserTest.php index d958dacd8a6b4..768485edca3af 100644 --- a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CsvParserTest.php +++ b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/CsvParserTest.php @@ -10,13 +10,14 @@ class CsvParserTest extends \PHPUnit_Framework_TestCase { - public function testWhenCsvFileIsEmpty_ParserIsNotCreated() + public function testWhenCsvFileIsEmptyThenParserIsNotCreated() { $this->setExpectedException(\InvalidArgumentException::class, 'CSV file should contain at least 1 row'); + $this->createParser(new FakeFile([])); } - public function testWhenValidFileIsProvided_readColumnsFromFileHeader() + public function testWhenValidFileIsProvidedThenReadColumnsFromFileHeader() { $parser = $this->createParser(new FakeFile([ 'column1,column2,column3', @@ -26,7 +27,7 @@ public function testWhenValidFileIsProvided_readColumnsFromFileHeader() $this->assertSame(['column1', 'column2', 'column3'], $parser->getColumnNames()); } - public function testWhenValidFileIsProvided_rowsAreFetchedAndHeaderIsSkipped() + public function testWhenValidFileIsProvidedThenRowsAreFetchedAndHeaderIsSkipped() { $parser = $this->createParser(new FakeFile([ 'column1,column2,column3', @@ -43,7 +44,7 @@ public function testWhenValidFileIsProvided_rowsAreFetchedAndHeaderIsSkipped() ); } - public function testWhenEndOfFileIsReached_NoMoreRowsCanBeFetched() + public function testWhenEndOfFileIsReachedThenNoMoreRowsCanBeFetched() { $parser = $this->createParser(new FakeFile([ 'column1,column2,column3', @@ -56,7 +57,7 @@ public function testWhenEndOfFileIsReached_NoMoreRowsCanBeFetched() $this->assertFalse($parser->fetchRow()); } - public function testWhenFileWasAlreadyRead_ResetAllowsToReadItFromStart() + public function testWhenFileWasAlreadyReadThenResetAllowsToReadItFromStart() { $parser = $this->createParser(new FakeFile([ 'column1,column2,column3', @@ -76,7 +77,7 @@ public function testWhenFileWasAlreadyRead_ResetAllowsToReadItFromStart() ); } - public function testWhenCustomDelimiterIsSpecified_dataIsParsedUsingThisDelimiter() + public function testWhenCustomDelimiterIsSpecifiedThenDataIsParsedUsingThisDelimiter() { $parser = $this->createParser( new FakeFile([ @@ -96,7 +97,7 @@ public function testWhenCustomDelimiterIsSpecified_dataIsParsedUsingThisDelimite ); } - public function testWhenFileHasMissingRowValues_fetchedRowValuesAreSameAsNumberOfColumns() + public function testWhenFileHasMissingRowValuesThenFetchedRowValuesAreSameAsNumberOfColumns() { $parser = $this->createParser(new FakeFile([ 'column1,column2,column3', @@ -118,7 +119,7 @@ public function testWhenFileHasMissingRowValues_fetchedRowValuesAreSameAsNumberO } - public function testWhenNullPlaceholderIsSet_fetchedRowValueIsReplacedWithNull() + public function testWhenNullPlaceholderIsSetThenFetchedRowValueIsReplacedWithNull() { $parser = $this->createParser( new FakeFile([ @@ -138,7 +139,7 @@ public function testWhenNullPlaceholderIsSet_fetchedRowValueIsReplacedWithNull() ); } - public function testWhenParserIsDestroyed_InternalFileDescriptorIsClosed() + public function testWhenParserIsDestroyedThenInternalFileDescriptorIsClosed() { $csvFile = new FakeFile([ 'column1,column2,column3' @@ -150,15 +151,14 @@ public function testWhenParserIsDestroyed_InternalFileDescriptorIsClosed() $this->assertFalse($csvFile->isOpen()); } - private function assertParsedFileContent($expectedParsedFileTable, $parser) + private function assertParsedFileContent($expectedCsvStructure, $parser) { - foreach ($expectedParsedFileTable as $rowNumber => $row) { - $this->assertSame( - $row, - $parser->fetchRow(), - sprintf('Unexpected data on row #%d', $rowNumber + 1) - ); + $actualCsvStructure = []; + while ($row = $parser->fetchRow()) { + $actualCsvStructure[] = $row; } + + $this->assertSame($expectedCsvStructure, $actualCsvStructure); } private function skipRows($numberOfRows, FileParser\CsvParser $parser) diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/UnsupportedPathExceptionTest.php b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/UnsupportedPathExceptionTest.php index 53ade55f1b2aa..c6a05d0f47888 100644 --- a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/UnsupportedPathExceptionTest.php +++ b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/UnsupportedPathExceptionTest.php @@ -10,31 +10,31 @@ class UnsupportedPathExceptionTest extends \PHPUnit_Framework_TestCase { - public function testWhenNoArgumentsAreProvided_FileNameIsEmpty() + public function testWhenNoArgumentsAreProvidedThenFileNameIsEmpty() { - $unsupportedPathException = new UnsupportedPathException(); + $exception = new UnsupportedPathException(); - $this->assertEmpty($unsupportedPathException->getPath()); + $this->assertEmpty($exception->getPath()); } - public function testWhenFileNameIsProvided_FileNameCanBeRetrievedLater() + public function testWhenFileNameIsProvidedThenFileNameCanBeRetrievedLater() { - $unsupportedPathException = new UnsupportedPathException('file.csv'); + $exception = new UnsupportedPathException('file.csv'); - $this->assertSame('file.csv', $unsupportedPathException->getPath()); + $this->assertSame('file.csv', $exception->getPath()); } - public function testWhenMessageIsProvided_MessageCanBeRetrievedLater() + public function testWhenMessageIsProvidedThenMessageCanBeRetrievedLater() { - $unsupportedPathException = new UnsupportedPathException('', 'My message'); + $exception = new UnsupportedPathException('', 'My message'); - $this->assertSame('My message', $unsupportedPathException->getMessage()); + $this->assertSame('My message', $exception->getMessage()); } - public function testWhenNoMessageIsProvided_MessageIsGeneratedFromPath() + public function testWhenNoMessageIsProvidedThenMessageIsGeneratedFromPath() { - $unsupportedPathException = new UnsupportedPathException('file.csv'); + $exception = new UnsupportedPathException('file.csv'); - $this->assertSame('Path "file.csv" is not supported', $unsupportedPathException->getMessage()); + $this->assertSame('Path "file.csv" is not supported', $exception->getMessage()); } } diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/ZipParserFactoryTest.php b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/ZipParserFactoryTest.php index 0945ac9f830a5..e2c4e0bde2a78 100644 --- a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/ZipParserFactoryTest.php +++ b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/ZipParserFactoryTest.php @@ -12,48 +12,45 @@ class ZipParserFactoryTest extends \PHPUnit_Framework_TestCase { - public function testWhenZipIsDisabled_ParserIsNotCreated() + public function testWhenZipIsDisabledThenParserIsNotCreated() { + $this->setExpectedException(FileParser\UnsupportedPathException::class, 'Zip extension is not available'); + $parser = new FileParser\ZipParserFactory( $this->createTestFilesystem(), new FakeParserFactory(), false ); - $this->setExpectedException(FileParser\UnsupportedPathException::class, 'Zip extension is not available'); - $parser->create('file.zip'); } - public function testWhenCsvFileIsProvided_ParserIsNotCreated() + public function testWhenCsvFileIsProvidedThenParserIsNotCreated() { - $parser = new FileParser\ZipParserFactory($this->createTestFilesystem(), new FakeParserFactory()); - $this->setExpectedException(FileParser\UnsupportedPathException::class, 'Path "file.csv" is not supported'); + $parser = new FileParser\ZipParserFactory($this->createTestFilesystem(), new FakeParserFactory()); $parser->create('file.csv'); } - public function testWhenCorruptedZipFileIsProvided_ParserIsNotCreated() + public function testWhenCorruptedZipFileIsProvidedThenParserIsNotCreated() { - $parser = new FileParser\ZipParserFactory($this->createTestFilesystem(), new FakeParserFactory()); - $this->setExpectedException(FileParser\CorruptedFileException::class); + $parser = new FileParser\ZipParserFactory($this->createTestFilesystem(), new FakeParserFactory()); $parser->create('corrupted.zip'); } - public function testWhenZipFileDoesNotExists_ParserIsNotCreated() + public function testWhenZipFileDoesNotExistsThenParserIsNotCreated() { - $parser = new FileParser\ZipParserFactory($this->createTestFilesystem(), new FakeParserFactory()); - $this->setExpectedException(FileParser\UnsupportedPathException::class, 'Path "unknown.zip" is not supported'); + $parser = new FileParser\ZipParserFactory($this->createTestFilesystem(), new FakeParserFactory()); $parser->create('unknown.zip'); } - public function testWhenEmptyZipFileIsProvided_ParserIsNotCreated() + public function testWhenEmptyZipFileIsProvidedThenParserIsNotCreated() { $this->setExpectedException(FileParser\UnsupportedPathException::class, 'Path "empty.zip" is not supported'); @@ -61,7 +58,7 @@ public function testWhenEmptyZipFileIsProvided_ParserIsNotCreated() $parserFactory->create('empty.zip'); } - public function testWhenProperZipFileIsProvided_FirstFileIsParsed() + public function testWhenProperZipFileIsProvidedThenFirstFileIsParsed() { $expectedParser = new FakeParser(); @@ -78,7 +75,7 @@ public function testWhenProperZipFileIsProvided_FirstFileIsParsed() ); } - public function testWhenProperZipFileWithAbsolutePathIsProvided_FirstFileIsParsed() + public function testWhenProperZipFileWithAbsolutePathIsProvidedThenFirstFileIsParsed() { $expectedParser = new FakeParser(); @@ -95,7 +92,7 @@ public function testWhenProperZipFileWithAbsolutePathIsProvided_FirstFileIsParse ); } - public function testWhenProperZipFileIsProvided_SecondFileIsParsed() + public function testWhenProperZipFileIsProvidedThenSecondFileIsParsed() { $expectedParser = new FakeParser(); @@ -112,7 +109,7 @@ public function testWhenProperZipFileIsProvided_SecondFileIsParsed() ); } - public function testWhenProperZipFileIsProvidedWithOptions_CustomCsvFileIsParsed() + public function testWhenProperZipFileIsProvidedWithOptionsThenCustomCsvFileIsParsed() { $fileSystem = $this->createTestFilesystem(); diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileTest.php b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileTest.php index aead73856d1f2..2c9ca0641cb14 100644 --- a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileTest.php +++ b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileTest.php @@ -11,14 +11,14 @@ class FileTest extends \PHPUnit_Framework_TestCase { - public function testWhenFileParserIsProvided_ColumnsAreReturnedProperly() + public function testWhenFileParserIsProvidedThenColumnsAreReturnedProperly() { $file = new File(new FakeParser(['column1', 'column2'])); $this->assertSame(['column1', 'column2'], $file->getColNames()); } - public function testWhenEmptyFileParserIsProvided_IteratorIsNotValid() + public function testWhenEmptyFileParserIsProvidedThenIteratorIsNotValid() { $file = new File(new FakeParser( ['column1', 'column2'] @@ -27,7 +27,7 @@ public function testWhenEmptyFileParserIsProvided_IteratorIsNotValid() $this->assertFalse($file->valid()); } - public function testWhenSomeDataInFileParserIsProvided_IteratorIsValid() + public function testWhenSomeDataInFileParserIsProvidedThenIteratorIsValid() { $file = new File(new FakeParser( ['column1', 'column2'], @@ -40,7 +40,7 @@ public function testWhenSomeDataInFileParserIsProvided_IteratorIsValid() $this->assertTrue($file->valid()); } - public function testWhenSomeDataInFileParserIsProvided_IteratorRowIsReturned() + public function testWhenSomeDataInFileParserIsProvidedThenIteratorRowIsReturned() { $file = new File(new FakeParser( ['column1', 'column2'], @@ -60,7 +60,7 @@ public function testWhenSomeDataInFileParserIsProvided_IteratorRowIsReturned() ); } - public function testWhenSpecificPositionIsSet_ProperIteratorRowIsReturned() + public function testWhenSpecificPositionIsSetThenProperIteratorRowIsReturned() { $file = new File(new FakeParser( ['column1', 'column2'], @@ -83,8 +83,7 @@ public function testWhenSpecificPositionIsSet_ProperIteratorRowIsReturned() ); } - - public function testWhenIteratorIsRewinded_ParserRestarts() + public function testWhenIteratorIsRewindedThenParserRestarts() { $file = new File(new FakeParser( ['column1', 'column2'], From 6b935168f70244e53dd6a043b01f4244a44fc8d4 Mon Sep 17 00:00:00 2001 From: Ivan Chepurnyi Date: Thu, 18 May 2017 11:56:50 -0500 Subject: [PATCH 17/17] Improve readability of ZipParserFactoryTest --- .../FileParser/ZipParserFactoryTest.php | 34 ++++++++++++------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/ZipParserFactoryTest.php b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/ZipParserFactoryTest.php index e2c4e0bde2a78..c968a9202c179 100644 --- a/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/ZipParserFactoryTest.php +++ b/app/code/Magento/ImportExport/Test/Unit/Model/Import/Source/FileParser/ZipParserFactoryTest.php @@ -29,7 +29,7 @@ public function testWhenCsvFileIsProvidedThenParserIsNotCreated() { $this->setExpectedException(FileParser\UnsupportedPathException::class, 'Path "file.csv" is not supported'); - $parser = new FileParser\ZipParserFactory($this->createTestFilesystem(), new FakeParserFactory()); + $parser = $this->createZipParserFactory(); $parser->create('file.csv'); } @@ -37,7 +37,7 @@ public function testWhenCorruptedZipFileIsProvidedThenParserIsNotCreated() { $this->setExpectedException(FileParser\CorruptedFileException::class); - $parser = new FileParser\ZipParserFactory($this->createTestFilesystem(), new FakeParserFactory()); + $parser = $this->createZipParserFactory(); $parser->create('corrupted.zip'); } @@ -45,7 +45,7 @@ public function testWhenZipFileDoesNotExistsThenParserIsNotCreated() { $this->setExpectedException(FileParser\UnsupportedPathException::class, 'Path "unknown.zip" is not supported'); - $parser = new FileParser\ZipParserFactory($this->createTestFilesystem(), new FakeParserFactory()); + $parser = $this->createZipParserFactory(); $parser->create('unknown.zip'); } @@ -54,7 +54,7 @@ public function testWhenEmptyZipFileIsProvidedThenParserIsNotCreated() { $this->setExpectedException(FileParser\UnsupportedPathException::class, 'Path "empty.zip" is not supported'); - $parserFactory = new FileParser\ZipParserFactory($this->createTestFilesystem(), new FakeParserFactory()); + $parserFactory = $this->createZipParserFactory(); $parserFactory->create('empty.zip'); } @@ -79,8 +79,7 @@ public function testWhenProperZipFileWithAbsolutePathIsProvidedThenFirstFileIsPa { $expectedParser = new FakeParser(); - $parserFactory = new FileParser\ZipParserFactory( - $this->createTestFilesystem(), + $parserFactory = $this->createZipParserFactory( new FakeParserFactory([ 'test.csv' => $expectedParser ]) @@ -96,8 +95,7 @@ public function testWhenProperZipFileIsProvidedThenSecondFileIsParsed() { $expectedParser = new FakeParser(); - $parserFactory = new FileParser\ZipParserFactory( - $this->createTestFilesystem(), + $parserFactory = $this->createZipParserFactory( new FakeParserFactory([ 'test.tsv' => $expectedParser ]) @@ -111,11 +109,11 @@ public function testWhenProperZipFileIsProvidedThenSecondFileIsParsed() public function testWhenProperZipFileIsProvidedWithOptionsThenCustomCsvFileIsParsed() { - $fileSystem = $this->createTestFilesystem(); - - $parserFactory = new FileParser\ZipParserFactory( - $fileSystem, - new FileParser\CsvParserFactory($fileSystem, new FakeObjectManager()) + $parserFactory = $this->createZipParserFactory( + new FileParser\CsvParserFactory( + $this->createTestFilesystem(), + new FakeObjectManager() + ) ); $parser = $parserFactory->create( @@ -140,4 +138,14 @@ private function createTestFilesystem() new Filesystem\Directory\WriteFactory(new Filesystem\DriverPool()) ); } + + private function createZipParserFactory($parserFactory = null): FileParser\ZipParserFactory + { + $parser = new FileParser\ZipParserFactory( + $this->createTestFilesystem(), + $parserFactory ?: new FakeParserFactory() + ); + + return $parser; + } }