fix: unlink zip file in case of exceptions

model/ZipExporter.php

@@ -31,6 +31,7 @@
 use core_kernel_classes_Literal;
 use core_kernel_classes_Property;
 use core_kernel_classes_Resource;
+use Exception;
 use oat\oatbox\service\ServiceManager;
 use oat\taoMediaManager\model\export\service\MediaResourcePreparerInterface;
 use oat\taoMediaManager\model\export\service\SharedStimulusCSSExporter;
@@ -138,17 +139,27 @@ private function getSavePath(string $unsafePath): string
         return $safePath;
     }
 
+    /**
+     * @throws common_Exception
+     * @throws Exception
+     */
     protected function createZipFile($filename, array $exportClasses = [], array $exportFiles = []): string
     {
-        $zip = new ZipArchive();
         $baseDir = tao_helpers_Export::getExportPath();
         $path = $baseDir . '/' . $filename . '.zip';
 
+        $zip = new ZipArchive();
         if ($zip->open($path, ZipArchive::CREATE) !== true) {
             throw new common_Exception('Unable to create zipfile ' . $path);
         }
 
-        if ($zip->numFiles === 0) {
+        if ($zip->numFiles !== 0) {
+            $zip->close();
+
+            return $path;
+        }
+
+        try {
             $errors = [];
             foreach ($exportFiles as $label => $files) {
                 $archivePath = '';
@@ -183,11 +194,19 @@ protected function createZipFile($filename, array $exportClasses = [], array $ex
             if (!empty($errors)) {
                 throw new ZipExporterFileErrorList($errors);
             }
-        }
 
-        $zip->close();
+            $zip->close();
+
+            return $path;
+        } catch (Exception $e) {
+            $zip->close();
 
-        return $path;
+            if (is_file($path)) {
+                unlink($path);
+            }
+
+            throw $e;
+        }
     }
 
     public function getServiceManager()

test/unit/model/ZipExporterTest.php

@@ -20,7 +20,13 @@ class ZipExporterTest extends TestCase
 
     private const FILENAME = 'test';
 
-    private const TMP_TAO_EXPORT_TEST_ZIP = '/tmp/tao_export/test.zip';
+    private const EXPORT_DIR = '/tmp/tao_export/';
+
+    private const RESOURCES_DIR = __DIR__ . '/../../resources/';
+
+    private const TEST_ZIP = 'test.zip';
+
+    private const EMPTY_ZIP = 'empty.zip';
 
     private StreamInterface $streamMock;
 
@@ -39,6 +45,10 @@ class ZipExporterTest extends TestCase
      */
     public function setUp(): void
     {
+        if (!is_dir(self::EXPORT_DIR)) {
+            mkdir(self::EXPORT_DIR);
+        }
+
         $this->streamMock = $this->createMock(StreamInterface::class);
         $this->resourceMock = $this->createMock(core_kernel_classes_Resource::class);
         $this->fileManagementMock = $this->createMock(FileManagement::class);
@@ -51,6 +61,28 @@ public function setUp(): void
             ->getMock();
     }
 
+    /**
+     * @throws common_Exception
+     */
+    public function testCreateZipFile()
+    {
+        $exportClasses = [
+            'foo'
+        ];
+
+        $exportFiles = [
+            'foo' => [
+                $this->resourceMock
+            ]
+        ];
+
+        copy(self::RESOURCES_DIR . self::TEST_ZIP, self::EXPORT_DIR . self::TEST_ZIP);
+
+        $this->sut->createZipFile(self::FILENAME, $exportClasses, $exportFiles);
+
+        $this->addToAssertionCount(1);
+    }
+
     /**
      * @throws common_Exception
      */
@@ -100,13 +132,23 @@ public function testCreateZipFileThrowsMediaReferencesNotFoundException(): void
             'Error in Asset class "foo": Media references to Image: foo.jpg FilePath: foo.jpg could not be found.'
         );
 
+        copy(self::RESOURCES_DIR . self::EMPTY_ZIP, self::EXPORT_DIR . self::TEST_ZIP);
+
         $this->sut->createZipFile(self::FILENAME, $exportClasses, $exportFiles);
     }
 
     public function tearDown(): void
     {
-        if (is_file(self::TMP_TAO_EXPORT_TEST_ZIP)) {
-            unlink(self::TMP_TAO_EXPORT_TEST_ZIP);
+        if (is_file(self::EXPORT_DIR . self::TEST_ZIP)) {
+            unlink(self::EXPORT_DIR . self::TEST_ZIP);
+        }
+
+        if (is_file(self::EXPORT_DIR . self::EMPTY_ZIP)) {
+            unlink(self::EXPORT_DIR . self::EMPTY_ZIP);
+        }
+
+        if (is_dir(self::EXPORT_DIR)) {
+            rmdir(self::EXPORT_DIR);
         }
     }
 }