Extra check if image is not used by other product

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;
                 }
             }

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');
     }
 
     /**

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 */

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"

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).