[NSDTRINITY-90] fix for not uploaded physical files in asset import and clean up temporary unzipped files (#15)

mike4git · web-flow · commit d628f57520ba · 2025-05-20T12:29:05.000+02:00
in co-working with @lukadschaak
diff --git a/src/Controller/Admin/Base/AbstractImportBaseController.php b/src/Controller/Admin/Base/AbstractImportBaseController.php
@@ -6,6 +6,7 @@
 use Neusta\Pimcore\ImportExportBundle\Toolbox\Repository\ImportRepositoryInterface;
 use Pimcore\Model\Element\AbstractElement;
 use Pimcore\Model\Element\DuplicateFullPathException;
+use Psr\Log\LoggerInterface;
 use Symfony\Component\HttpFoundation\File\UploadedFile;
 use Symfony\Component\HttpFoundation\JsonResponse;
 use Symfony\Component\HttpFoundation\Request;
@@ -39,6 +40,7 @@ abstract class AbstractImportBaseController
      * @param ImportRepositoryInterface<TElement> $repository
      */
     public function __construct(
+        protected LoggerInterface $logger,
         protected ImportRepositoryInterface $repository,
         protected string $elementType = 'Element',
     ) {
@@ -66,6 +68,12 @@ public function import(Request $request): JsonResponse
             }
         } catch (\Exception $e) {
             return $this->createJsonResponse(false, $e->getMessage(), 500);
+        } finally {
+            try {
+                $this->cleanUp();
+            } catch (\Throwable $cleanupError) {
+                $this->logger->warning($cleanupError->getMessage());
+            }
         }
 
         return $this->createJsonResponse(true, $this->createResultMessage());
@@ -123,4 +131,9 @@ protected function createResultMessage(): string
      * @throws DuplicateFullPathException
      */
     abstract protected function importByFile(UploadedFile $file, string $format): array;
+
+    protected function cleanUp(): void
+    {
+        // implement clean ups in subclasses if necessary
+    }
 }
diff --git a/src/Controller/Admin/ImportAssetsController.php b/src/Controller/Admin/ImportAssetsController.php
@@ -7,6 +7,7 @@
 use Neusta\Pimcore\ImportExportBundle\Import\ZipImporter;
 use Neusta\Pimcore\ImportExportBundle\Toolbox\Repository\AssetRepository;
 use Pimcore\Model\Asset;
+use Psr\Log\LoggerInterface;
 use Symfony\Component\HttpFoundation\File\UploadedFile;
 use Symfony\Component\HttpFoundation\JsonResponse;
 use Symfony\Component\HttpFoundation\Request;
@@ -21,11 +22,12 @@ final class ImportAssetsController extends AbstractImportBaseController
      * @param Importer<\ArrayObject<int|string, mixed>, Asset> $importer
      */
     public function __construct(
+        LoggerInterface $logger,
         private Importer $importer,
         private ZipImporter $zipImporter,
         AssetRepository $assetRepository,
     ) {
-        parent::__construct($assetRepository, 'Asset');
+        parent::__construct($logger, $assetRepository, 'Asset');
     }
 
     #[Route(
@@ -52,10 +54,7 @@ protected function importByFile(UploadedFile $file, string $format): array
                     && $asset->getKey()
                     && \array_key_exists($asset->getKey(), $zipContent[$asset->getType()])
                 ) {
-                    $asset->setData($zipContent[$asset->getType()][$asset->getKey()]);
-                    if ($this->overwrite) {
-                        $asset->save(['versionNote' => 'added by pimcore-import-export-bundle']);
-                    }
+                    $asset->setData(file_get_contents($zipContent[$asset->getType()][$asset->getKey()]->getRealPath()));
                 }
             }
 
@@ -71,4 +70,9 @@ protected function importByFile(UploadedFile $file, string $format): array
             throw new \Exception('Error reading uploaded file: ' . $e->getMessage(), 0, $e);
         }
     }
+
+    protected function cleanUp(): void
+    {
+        $this->zipImporter->cleanUp();
+    }
 }
diff --git a/src/Controller/Admin/ImportDataObjectsController.php b/src/Controller/Admin/ImportDataObjectsController.php
@@ -6,6 +6,7 @@
 use Neusta\Pimcore\ImportExportBundle\Import\Importer;
 use Neusta\Pimcore\ImportExportBundle\Toolbox\Repository\DataObjectRepository;
 use Pimcore\Model\DataObject;
+use Psr\Log\LoggerInterface;
 use Symfony\Component\HttpFoundation\File\UploadedFile;
 use Symfony\Component\HttpFoundation\JsonResponse;
 use Symfony\Component\HttpFoundation\Request;
@@ -20,10 +21,11 @@ final class ImportDataObjectsController extends AbstractImportBaseController
      * @param Importer<\ArrayObject<int|string, mixed>, DataObject> $importer
      */
     public function __construct(
+        LoggerInterface $logger,
         DataObjectRepository $repository,
         private Importer $importer,
     ) {
-        parent::__construct($repository, 'DataObject');
+        parent::__construct($logger, $repository, 'DataObject');
     }
 
     #[Route(
diff --git a/src/Controller/Admin/ImportDocumentsController.php b/src/Controller/Admin/ImportDocumentsController.php
@@ -6,6 +6,7 @@
 use Neusta\Pimcore\ImportExportBundle\Import\Importer;
 use Neusta\Pimcore\ImportExportBundle\Toolbox\Repository\DocumentRepository;
 use Pimcore\Model\Document;
+use Psr\Log\LoggerInterface;
 use Symfony\Component\HttpFoundation\File\UploadedFile;
 use Symfony\Component\HttpFoundation\JsonResponse;
 use Symfony\Component\HttpFoundation\Request;
@@ -20,10 +21,11 @@ final class ImportDocumentsController extends AbstractImportBaseController
      * @param Importer<\ArrayObject<int|string, mixed>, Document> $importer
      */
     public function __construct(
+        LoggerInterface $logger,
         DocumentRepository $repository,
         private Importer $importer,
     ) {
-        parent::__construct($repository, 'Document');
+        parent::__construct($logger, $repository, 'Document');
     }
 
     #[Route(
diff --git a/src/Import/ZipImporter.php b/src/Import/ZipImporter.php
@@ -3,14 +3,18 @@
 namespace Neusta\Pimcore\ImportExportBundle\Import;
 
 use Symfony\Component\Filesystem\Filesystem;
+use Symfony\Component\Finder\Finder;
 
 class ZipImporter
 {
+    /** @var string */
+    private const ZIPIMPORT_DIR_PREFIX = 'zipimport_';
     private Filesystem $filesystem;
     private string $tempDir;
 
-    public function __construct(?string $tempDir = null)
-    {
+    public function __construct(
+        ?string $tempDir = null,
+    ) {
         $this->filesystem = new Filesystem();
         $this->tempDir = $tempDir ?? sys_get_temp_dir();
     }
@@ -47,7 +51,7 @@ public function import(string $zipPath): array
 
     private function createTempExtractPath(): string
     {
-        $extractPath = $this->tempDir . '/zipimport_' . uniqid();
+        $extractPath = $this->tempDir . '/' . self::ZIPIMPORT_DIR_PREFIX . uniqid();
         $this->filesystem->mkdir($extractPath);
 
         return $extractPath;
@@ -66,7 +70,7 @@ private function findYamlFile(string $dir): ?string
     }
 
     /**
-     * @return array<string, array<string, string>>
+     * @return array<string, array<string, \SplFileInfo>>
      */
     private function collectAssetDirectories(string $basePath): array
     {
@@ -88,7 +92,7 @@ private function collectAssetDirectories(string $basePath): array
                 continue;
             }
 
-            $files = $this->collectFilesFromDir($fullPath);
+            $files = $this->collectFileInfosFromDir($fullPath);
             if (!empty($files)) {
                 $assets[$entry] = $files;
             }
@@ -98,22 +102,31 @@ private function collectAssetDirectories(string $basePath): array
     }
 
     /**
-     * @return array<string, string>
+     * @return array<string, \SplFileInfo>
      */
-    private function collectFilesFromDir(string $dir): array
+    private function collectFileInfosFromDir(string $dir): array
     {
         $files = [];
         $rii = new \RecursiveIteratorIterator(new \RecursiveDirectoryIterator($dir));
         foreach ($rii as $file) {
             if ($file->isFile()) {
                 $filename = $file->getFilename();
-                $content = file_get_contents($file->getPathname());
-                if (false !== $content) {
-                    $files[$filename] = $content;
-                }
+                $files[$filename] = new \SplFileInfo($file->getPathname());
             }
         }
 
         return $files;
     }
+
+    public function cleanUp(): void
+    {
+        $finder = new Finder();
+        $finder->directories()
+            ->in($this->tempDir)
+            ->name(self::ZIPIMPORT_DIR_PREFIX . '*');
+
+        foreach ($finder as $dir) {
+            $this->filesystem->remove($dir->getRealPath());
+        }
+    }
 }
