diff --git a/Model/Resource/Storage/ImageStorage.php b/Model/Resource/Storage/ImageStorage.php index 1461567..f6015d6 100644 --- a/Model/Resource/Storage/ImageStorage.php +++ b/Model/Resource/Storage/ImageStorage.php @@ -274,8 +274,27 @@ protected function removeObsoleteImages(Product $product, array $existingImages, [$storagePath] )); - // remove from file system - @unlink(self::PRODUCT_IMAGE_PATH . $storagePath); + // check if the image is used by other products + // (this cannot be the case in standard Magento) + $usageCount = $this->db->fetchSingleCell(" + SELECT COUNT(*) + FROM `{$this->metaData->productEntityTable}_varchar` + WHERE + attribute_id IN (" . $this->db->getMarks($this->metaData->imageAttributeIds) . ") AND + value = ? + ", array_merge( + $this->metaData->imageAttributeIds, + [$storagePath] + )); + + // only remove image from filesystem if it is not used by other products + // note! this only checks if the image has a role in a product, not if it is used in a gallery + // the real check would be too slow + if ($usageCount == 0) { + // remove from file system + @unlink(self::PRODUCT_IMAGE_PATH . $storagePath); + } + } } @@ -284,6 +303,7 @@ protected function removeObsoleteImages(Product $product, array $existingImages, /** * Load data from existing product images + * Returns image.jpg before image_1.jpg, image_2.jpg, ... * * @param Product $product */ @@ -293,13 +313,22 @@ protected function loadExistingImageData(Product $product) SELECT M.`value_id`, M.`value`, M.`disabled` FROM {$this->metaData->mediaGalleryTable} M INNER JOIN {$this->metaData->mediaGalleryValueToEntityTable} E ON E.`value_id` = M.`value_id` - WHERE E.`entity_id` = ? AND M.`attribute_id` = ? + WHERE E.`entity_id` = ? AND M.`attribute_id` = ? + ORDER BY M.`value_id` ", [ $product->id, $this->metaData->mediaGalleryAttributeId ]); } + /** + * An image "exists" if its path is stored in the database gallery for this product (perhaps with a suffix) + * otherwise it is "new". + * + * @param Product $product + * @param array $imageData + * @return array + */ public function splitNewAndExistingImages(Product $product, array $imageData) { $existingImages = []; @@ -317,7 +346,7 @@ public function splitNewAndExistingImages(Product $product, array $imageData) if ($simpleStoragePath === $image->getDefaultStoragePath()) { $found = true; $image->valueId = $imageDatum['value_id']; - $image->setActualStoragePath($imageDatum['value']); + $image->setActualStoragePath($storagePath); break; } } diff --git a/Test/Integration/ImportTest.php b/Test/Integration/ImportTest.php index 14fc45b..84223b4 100644 --- a/Test/Integration/ImportTest.php +++ b/Test/Integration/ImportTest.php @@ -429,6 +429,7 @@ public function testMissingCategories() public function testImages() { @unlink(BP . '/pub/media/catalog/product/d/u/duck1.jpg'); + @unlink(BP . '/pub/media/catalog/product/d/u/duck1_1.jpg'); @unlink(BP . '/pub/media/catalog/product/d/u/duck2.png'); @unlink(BP . '/pub/media/catalog/product/d/u/duck3.png'); @unlink(BP . '/pub/media/catalog/product/d/u/duck3_1.png'); @@ -468,6 +469,7 @@ public function testImages() $this->assertEquals([], $errors); $this->assertTrue(file_exists(BP . '/pub/media/catalog/product/d/u/duck1.jpg')); + $this->assertFalse(file_exists(BP . '/pub/media/catalog/product/d/u/duck1_1.jpg')); $this->assertTrue(file_exists(BP . '/pub/media/catalog/product/d/u/duck2.png')); $media = [ @@ -536,6 +538,7 @@ public function testImages() $productS = self::$repository->get("ducky1-product-import", false, 0, true); $this->assertEquals('/d/u/duck1.jpg', $productS->getThumbnail()); + $this->assertFalse(file_exists(BP . '/pub/media/catalog/product/d/u/duck1_1.jpg')); $this->assertEquals('/d/u/duck2.png', $productS->getImage()); $this->assertEquals('/d/u/duck3_1.png', $productS->getSmallImage()); @@ -575,6 +578,7 @@ public function testImages() $this->assertEquals([], $errors); $this->assertTrue(file_exists(BP . '/pub/media/catalog/product/d/u/duck1.jpg')); + $this->assertFalse(file_exists(BP . '/pub/media/catalog/product/d/u/duck1_1.jpg')); $this->assertFalse(file_exists(BP . '/pub/media/catalog/product/d/u/duck2.png')); $media = [ @@ -607,6 +611,7 @@ public function testImages() $this->assertEquals([], $errors); $this->assertTrue(file_exists(BP . '/pub/media/catalog/product/d/u/duck1.jpg')); + $this->assertFalse(file_exists(BP . '/pub/media/catalog/product/d/u/duck1_1.jpg')); $this->assertFalse(file_exists(BP . '/pub/media/catalog/product/d/u/duck2.png')); $media = [ @@ -623,7 +628,6 @@ public function testImages() $this->assertEquals('/d/u/duck1.jpg', $productS->getThumbnail()); $this->assertEquals(null, $productS->getImage()); $this->assertEquals(null, $productS->getSmallImage()); - } private function checkImageData($product, $mediaData, $valueData) @@ -1676,6 +1680,10 @@ private function checkDownloadable($downloadable) { $this->assertTrue(file_exists(BP . "/pub/media/downloadable/files/samples/d/u/duck3.png")); $this->assertTrue(!file_exists(BP . "/pub/media/downloadable/files/links/d/u/duck1_1.jpg")); + + @unlink(BP . '/pub/media/downloadable/files/links/d/u/duck1.jpg'); + @unlink(BP . '/pub/media/downloadable/files/link_samples/d/u/duck2.png'); + @unlink(BP . '/pub/media/downloadable/files/samples/d/u/duck3.png'); } /** diff --git a/Test/Integration/ValidatorTest.php b/Test/Integration/ValidatorTest.php index b4ec67e..4c372f7 100644 --- a/Test/Integration/ValidatorTest.php +++ b/Test/Integration/ValidatorTest.php @@ -243,6 +243,8 @@ public function testImageValidation() public function testImageRoleValidation() { + @unlink(BP . '/pub/media/catalog/product/d/u/duck1.jpg'); + $objectManager = \Magento\TestFramework\Helper\Bootstrap::getObjectManager(); /** @var Validator $validator */ diff --git a/composer.json b/composer.json index 0f51888..0ddaf1e 100644 --- a/composer.json +++ b/composer.json @@ -1,6 +1,6 @@ { "name": "bigbridge/product-import", - "description": "Imports products with raw database queries", + "description": "Fast product import library for Magento 2", "type": "magento2-module", "require": { "php": "~7.0" diff --git a/doc/design-considerations.md b/doc/design-considerations.md index 90281ac..8a6e62a 100644 --- a/doc/design-considerations.md +++ b/doc/design-considerations.md @@ -219,9 +219,11 @@ https://sourceforge.net/p/magmi/patches/23/ ## Images -All images are places in a temporary location in the validation phase, before being processed further. This ensures that all images are valid when being processed. +All images are placed in a temporary location in the validation phase, before being processed further. This ensures that all images are valid when being processed. Make sure to remove all images from their temporary location later. +When an image is updated, the library must check if a database entry exists for this image and if the file still exists that belongs to this database entry. + ## Default values I came back from the idea of setting default values (attribute_set_id, visibility, etc) for new products. They slow the importer down a bit, they make the system a little less flexible , and they provide a false sense of security. I want to make the user think at least a few minutes about these values. She will have to do this eventually anyway. And they were some attributes that I could not settle on to make them defaults (url_key, website_ids).