From 73272fe041c6fb4c0434ffed24cc995c3f01048b Mon Sep 17 00:00:00 2001 From: Pavel Bystritsky Date: Thu, 26 Oct 2017 18:01:50 +0300 Subject: [PATCH] MAGETWO-4711: Improve error reporting for products images import. --- .../Model/Import/Product.php | 1 + .../Test/Unit/Model/Import/ProductTest.php | 64 +++++++++++++++++++ .../Model/Import/ProductTest.php | 57 +++++++++++++++-- .../_files/magento_additional_image_error.jpg | 0 4 files changed, 118 insertions(+), 4 deletions(-) create mode 100644 dev/tests/integration/testsuite/Magento/CatalogImportExport/Model/Import/_files/magento_additional_image_error.jpg diff --git a/app/code/Magento/CatalogImportExport/Model/Import/Product.php b/app/code/Magento/CatalogImportExport/Model/Import/Product.php index a0f1e25cf6512..84c3e7e7b5f5a 100644 --- a/app/code/Magento/CatalogImportExport/Model/Import/Product.php +++ b/app/code/Magento/CatalogImportExport/Model/Import/Product.php @@ -2042,6 +2042,7 @@ protected function uploadMediaFiles($fileName, $renameFileOff = false) $res = $this->_getUploader()->move($fileName, $renameFileOff); return $res['file']; } catch (\Exception $e) { + $this->_logger->critical($e); return ''; } } diff --git a/app/code/Magento/CatalogImportExport/Test/Unit/Model/Import/ProductTest.php b/app/code/Magento/CatalogImportExport/Test/Unit/Model/Import/ProductTest.php index 862276d35bac5..8461e3830cbec 100644 --- a/app/code/Magento/CatalogImportExport/Test/Unit/Model/Import/ProductTest.php +++ b/app/code/Magento/CatalogImportExport/Test/Unit/Model/Import/ProductTest.php @@ -1194,6 +1194,70 @@ public function testParseAttributesWithWrappedValuesWillReturnsLowercasedAttribu $this->assertArrayNotHasKey('PARAM2', $attributes); } + /** + * Test that errors occurred during importing images are logged. + * + * @param string $fileName + * @param bool $throwException + * @dataProvider uploadMediaFilesDataProvider + */ + public function testUploadMediaFiles(string $fileName, bool $throwException) + { + $exception = new \Exception(); + $expectedFileName = $fileName; + if ($throwException) { + $expectedFileName = ''; + $this->_logger->expects($this->once())->method('critical')->with($exception); + } + + $fileUploaderMock = $this + ->getMockBuilder(\Magento\CatalogImportExport\Model\Import\Uploader::class) + ->disableOriginalConstructor() + ->getMock(); + + $fileUploaderMock + ->expects($this->once()) + ->method('move') + ->willReturnCallback( + function ($name) use ($throwException, $exception) { + if ($throwException) { + throw $exception; + } + return ['file' => $name]; + } + ); + + $this->setPropertyValue( + $this->importProduct, + '_fileUploader', + $fileUploaderMock + ); + + $actualFileName = $this->invokeMethod( + $this->importProduct, + 'uploadMediaFiles', + [$fileName] + ); + + $this->assertEquals( + $expectedFileName, + $actualFileName + ); + } + + /** + * Data provider for testUploadMediaFiles. + * + * @return array + */ + public function uploadMediaFilesDataProvider() + { + return [ + ['test1.jpg', false], + ['test2.jpg', true], + ]; + } + public function getImagesFromRowDataProvider() { return [ diff --git a/dev/tests/integration/testsuite/Magento/CatalogImportExport/Model/Import/ProductTest.php b/dev/tests/integration/testsuite/Magento/CatalogImportExport/Model/Import/ProductTest.php index 0f9cb373e59ed..5debe398f085c 100644 --- a/dev/tests/integration/testsuite/Magento/CatalogImportExport/Model/Import/ProductTest.php +++ b/dev/tests/integration/testsuite/Magento/CatalogImportExport/Model/Import/ProductTest.php @@ -26,7 +26,7 @@ use Magento\Framework\Filesystem; use Magento\ImportExport\Model\Import; use Magento\Store\Model\Store; -use Magento\UrlRewrite\Model\UrlRewrite; +use Psr\Log\LoggerInterface; /** * Class ProductTest @@ -62,11 +62,20 @@ class ProductTest extends \Magento\TestFramework\Indexer\TestCase */ protected $objectManager; + /** + * @var LoggerInterface|\PHPUnit_Framework_MockObject_MockObject + */ + private $logger; + protected function setUp() { $this->objectManager = \Magento\TestFramework\Helper\Bootstrap::getObjectManager(); + $this->logger = $this->getMockBuilder(LoggerInterface::class) + ->disableOriginalConstructor() + ->getMock(); $this->_model = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->create( - \Magento\CatalogImportExport\Model\Import\Product::class + \Magento\CatalogImportExport\Model\Import\Product::class, + ['logger' => $this->logger] ); parent::setUp(); } @@ -659,6 +668,21 @@ public function testSaveMediaImage() $this->assertEquals('Additional Image Label Two', $additionalImageTwoItem->getLabel()); } + /** + * Test that errors occurred during importing images are logged. + * + * @magentoDataIsolation enabled + * @magentoAppIsolation enabled + * @magentoDataFixture mediaImportImageFixture + * @magentoDataFixture mediaImportImageFixtureError + * @SuppressWarnings(PHPMD.ExcessiveMethodLength) + */ + public function testSaveMediaImageError() + { + $this->logger->expects(self::once())->method('critical'); + $this->importDataForMediaTest('import_media.csv', 1); + } + /** * Copy fixture images into media import directory */ @@ -717,6 +741,30 @@ public static function mediaImportImageFixtureRollback() $mediaDirectory->delete('catalog'); } + /** + * Copy incorrect fixture image into media import directory. + */ + public static function mediaImportImageFixtureError() + { + /** @var \Magento\Framework\Filesystem\Directory\Write $mediaDirectory */ + $mediaDirectory = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->get( + \Magento\Framework\Filesystem::class + )->getDirectoryWrite( + DirectoryList::MEDIA + ); + $dirPath = $mediaDirectory->getAbsolutePath('import'); + $items = [ + [ + 'source' => __DIR__ . '/_files/magento_additional_image_error.jpg', + 'dest' => $dirPath . '/magento_additional_image_two.jpg', + ], + ]; + foreach ($items as $item) { + copy($item['source'], $item['dest']); + } + } + + /** * Export CSV string to array * @@ -1564,8 +1612,9 @@ public function testProductWithWrappedAdditionalAttributes() * Import and check data from file * * @param string $fileName + * @param int $expectedErrors */ - private function importDataForMediaTest($fileName) + private function importDataForMediaTest(string $fileName, int $expectedErrors = 0) { $filesystem = $this->objectManager->create(\Magento\Framework\Filesystem::class); $directory = $filesystem->getDirectoryWrite(DirectoryList::ROOT); @@ -1603,7 +1652,7 @@ private function importDataForMediaTest($fileName) $this->assertTrue($errors->getErrorsCount() == 0); $this->_model->importData(); - $this->assertTrue($this->_model->getErrorAggregator()->getErrorsCount() == 0); + $this->assertTrue($this->_model->getErrorAggregator()->getErrorsCount() == $expectedErrors); } /** diff --git a/dev/tests/integration/testsuite/Magento/CatalogImportExport/Model/Import/_files/magento_additional_image_error.jpg b/dev/tests/integration/testsuite/Magento/CatalogImportExport/Model/Import/_files/magento_additional_image_error.jpg new file mode 100644 index 0000000000000..e69de29bb2d1d