From 62270382dc5725689b96059e03fbec28a6432499 Mon Sep 17 00:00:00 2001
From: Constantin Graf <constantin@solidtime.io>
Date: Wed, 18 Dec 2024 11:22:33 -0500
Subject: [PATCH] Fixed import lock

---
 app/Service/Import/ImportService.php          | 11 ++++++----
 .../Endpoint/Api/V1/ImportEndpointTest.php    |  2 ++
 .../Unit/Service/Import/ImportServiceTest.php | 21 +++++++++++++++++++
 3 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/app/Service/Import/ImportService.php b/app/Service/Import/ImportService.php
index 02a0502f..94103225 100644
--- a/app/Service/Import/ImportService.php
+++ b/app/Service/Import/ImportService.php
@@ -31,10 +31,13 @@ public function import(Organization $organization, string $importerType, string
         $lock = Cache::lock('import:'.$organization->getKey(), config('octane.max_execution_time', 60) + 1);
 
         if ($lock->get()) {
-            DB::transaction(function () use (&$importer, &$data, &$timezone): void {
-                $importer->importData($data, $timezone);
-            });
-            $lock->release();
+            try {
+                DB::transaction(function () use (&$importer, &$data, &$timezone): void {
+                    $importer->importData($data, $timezone);
+                });
+            } finally {
+                $lock->release();
+            }
         } else {
             throw new ImportException('Import is already in progress');
         }
diff --git a/tests/Unit/Endpoint/Api/V1/ImportEndpointTest.php b/tests/Unit/Endpoint/Api/V1/ImportEndpointTest.php
index a2beda61..175e3130 100644
--- a/tests/Unit/Endpoint/Api/V1/ImportEndpointTest.php
+++ b/tests/Unit/Endpoint/Api/V1/ImportEndpointTest.php
@@ -78,6 +78,7 @@ public function test_import_fails_if_user_does_not_have_permission(): void
 
     public function test_import_fails_if_data_can_not_be_base64_decoded(): void
     {
+        // Arrange
         $user = $this->createUserWithPermission([
             'import',
         ]);
@@ -98,6 +99,7 @@ public function test_import_fails_if_data_can_not_be_base64_decoded(): void
 
     public function test_import_return_error_message_if_import_fails(): void
     {
+        // Arrange
         $user = $this->createUserWithPermission([
             'import',
         ]);
diff --git a/tests/Unit/Service/Import/ImportServiceTest.php b/tests/Unit/Service/Import/ImportServiceTest.php
index c5631392..95e24e63 100644
--- a/tests/Unit/Service/Import/ImportServiceTest.php
+++ b/tests/Unit/Service/Import/ImportServiceTest.php
@@ -35,6 +35,8 @@ public function test_import_gets_importer_from_provider_runs_importer_and_return
         $report = $importService->import($organization, 'toggl_time_entries', $data, $timezone);
 
         // Assert
+        $lock = Cache::lock('import:'.$organization->getKey());
+        $this->assertTrue($lock->get());
         $this->assertSame(2, $report->timeEntriesCreated);
         $this->assertSame(2, $report->tagsCreated);
         $this->assertSame(1, $report->tasksCreated);
@@ -43,6 +45,25 @@ public function test_import_gets_importer_from_provider_runs_importer_and_return
         $this->assertSame(1, $report->clientsCreated);
     }
 
+    public function test_import_releases_lock_if_an_exception_happens_during_the_import(): void
+    {
+        // Arrange
+        Storage::fake(config('filesystems.default'));
+        $organization = Organization::factory()->create();
+        $timezone = 'Europe/Vienna';
+        $data = 'Invalid CSV data';
+
+        // Act
+        $importService = app(ImportService::class);
+        try {
+            $importService->import($organization, 'toggl_time_entries', $data, $timezone);
+        } catch (ImportException) {
+            // Assert
+            $lock = Cache::lock('import:'.$organization->getKey());
+            $this->assertTrue($lock->get());
+        }
+    }
+
     public function test_import_throws_exception_if_import_is_already_in_progress(): void
     {
         // Arrange