fix: unlink zip file in case of exceptions

composer.json

@@ -27,7 +27,7 @@
         "oat-sa/oatbox-extension-installer": "~1.1||dev-master",
         "oat-sa/lib-generis-search": "2.1.2",
         "oat-sa/generis": ">=15.24",
-        "oat-sa/tao-core": ">=52.1",
+        "oat-sa/tao-core": ">=53.12",
         "oat-sa/extension-tao-item": ">=v12.1.0",
         "oat-sa/extension-tao-itemqti": ">=29.14.5",
         "oat-sa/extension-tao-test": ">=15.16",

model/ZipExporter.php

@@ -31,6 +31,8 @@
 use core_kernel_classes_Literal;
 use core_kernel_classes_Property;
 use core_kernel_classes_Resource;
+use Exception;
+use oat\oatbox\log\LoggerAwareTrait;
 use oat\oatbox\service\ServiceManager;
 use oat\taoMediaManager\model\export\service\MediaResourcePreparerInterface;
 use oat\taoMediaManager\model\export\service\SharedStimulusCSSExporter;
@@ -48,6 +50,8 @@
  */
 class ZipExporter implements tao_models_classes_export_ExportHandler
 {
+    use LoggerAwareTrait;
+
     /**
      * @inheritDoc
      */
@@ -138,18 +142,29 @@ 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';
+        try {
+            $errors = [];
 
-        if ($zip->open($path, ZipArchive::CREATE) !== true) {
-            throw new common_Exception('Unable to create zipfile ' . $path);
-        }
+            $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) {
+                $zip->close();
+
+                return $path;
+            }
 
-        if ($zip->numFiles === 0) {
-            $errors = [];
             foreach ($exportFiles as $label => $files) {
                 $archivePath = '';
 
@@ -183,11 +198,21 @@ protected function createZipFile($filename, array $exportClasses = [], array $ex
             if (!empty($errors)) {
                 throw new ZipExporterFileErrorList($errors);
             }
-        }
 
-        $zip->close();
+            $zip->close();
+
+            return $path;
+        } catch (Exception $e) {
+            $this->getLogger()->error($e->getMessage() . $e->getTraceAsString());
 
-        return $path;
+            $zip->close();
+
+            if (is_file($path)) {
+                unlink($path);
+            }
+
+            throw $e;
+        }
     }
 
     public function getServiceManager()

test/unit/model/ZipExporterTest.php

@@ -13,14 +13,21 @@
 use PHPUnit\Framework\MockObject\Exception;
 use PHPUnit\Framework\TestCase;
 use Psr\Http\Message\StreamInterface;
+use Psr\Log\LoggerInterface;
 
 class ZipExporterTest extends TestCase
 {
     private const LINK = 'link';
 
     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;
 
@@ -32,25 +39,54 @@ class ZipExporterTest extends TestCase
 
     private ServiceManager $serviceManagerMock;
 
+    private LoggerInterface $loggerMock;
+
     private ZipExporter $sut;
 
     /**
      * @throws Exception
      */
     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);
         $this->mediaResourcePreparerMock = $this->createMock(MediaResourcePreparerInterface::class);
         $this->serviceManagerMock = $this->createMock(ServiceManager::class);
+        $this->loggerMock = $this->createMock(LoggerInterface::class);
 
         $this->sut = $this
             ->getMockBuilder(ZipExporterTester::class)
-            ->onlyMethods(['getServiceManager'])
+            ->onlyMethods(['getServiceManager', 'getLogger'])
             ->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
      */
@@ -76,6 +112,12 @@ public function testCreateZipFileThrowsMediaReferencesNotFoundException(): void
             ->method('getServiceManager')
             ->willReturn($this->serviceManagerMock);
 
+        $this
+            ->sut
+            ->expects(self::once())
+            ->method('getLogger')
+            ->willReturn($this->loggerMock);
+
         $this
             ->fileManagementMock
             ->expects(self::once())
@@ -94,19 +136,34 @@ public function testCreateZipFileThrowsMediaReferencesNotFoundException(): void
             ->method('get')
             ->willReturnOnConsecutiveCalls($this->fileManagementMock, $this->mediaResourcePreparerMock);
 
+        $this
+            ->loggerMock
+            ->expects(self::once())
+            ->method('error');
+
         $this->expectException(ZipExporterFileErrorList::class);
         $this->expectExceptionMessage(
             'Errors in zip file: <br>' .
             '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);
         }
     }
 }