From 7ca605e2169dbc874d4e795fb41d5cd44d93aef2 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandrocelaya@gmail.com>
Date: Mon, 11 Nov 2024 09:31:23 +0100
Subject: [PATCH] Remove unnecessary flush calls when used in wrapInTransaction

---
 module/Rest/src/ApiKey/Repository/ApiKeyRepository.php | 1 -
 module/Rest/src/Service/ApiKeyService.php              | 8 +++-----
 module/Rest/test/Service/ApiKeyServiceTest.php         | 7 -------
 3 files changed, 3 insertions(+), 13 deletions(-)

diff --git a/module/Rest/src/ApiKey/Repository/ApiKeyRepository.php b/module/Rest/src/ApiKey/Repository/ApiKeyRepository.php
index e1fbf3a6..6b282a07 100644
--- a/module/Rest/src/ApiKey/Repository/ApiKeyRepository.php
+++ b/module/Rest/src/ApiKey/Repository/ApiKeyRepository.php
@@ -36,7 +36,6 @@ class ApiKeyRepository extends EntitySpecificationRepository implements ApiKeyRe
 
             $initialApiKey = ApiKey::fromMeta(ApiKeyMeta::fromParams(key: $apiKey));
             $em->persist($initialApiKey);
-            $em->flush();
 
             return $initialApiKey;
         });
diff --git a/module/Rest/src/Service/ApiKeyService.php b/module/Rest/src/Service/ApiKeyService.php
index 09d1bb76..f60c2179 100644
--- a/module/Rest/src/Service/ApiKeyService.php
+++ b/module/Rest/src/Service/ApiKeyService.php
@@ -27,7 +27,6 @@ readonly class ApiKeyService implements ApiKeyServiceInterface
         return $this->em->wrapInTransaction(function () use ($apiKeyMeta) {
             $apiKey = ApiKey::fromMeta($this->ensureUniqueName($apiKeyMeta));
             $this->em->persist($apiKey);
-            $this->em->flush();
 
             return $apiKey;
         });
@@ -120,7 +119,7 @@ readonly class ApiKeyService implements ApiKeyServiceInterface
             return $apiKey;
         }
 
-        return $this->em->wrapInTransaction(function () use ($apiKeyRenaming, $apiKey) {
+        $this->em->wrapInTransaction(function () use ($apiKeyRenaming, $apiKey): void {
             if ($this->repo->nameExists($apiKeyRenaming->newName)) {
                 throw new InvalidArgumentException(
                     sprintf('Another API key with name "%s" already exists', $apiKeyRenaming->newName),
@@ -128,10 +127,9 @@ readonly class ApiKeyService implements ApiKeyServiceInterface
             }
 
             $apiKey->name = $apiKeyRenaming->newName;
-            $this->em->flush();
-
-            return $apiKey;
         });
+
+        return $apiKey;
     }
 
     private function findByKey(string $key): ApiKey|null
diff --git a/module/Rest/test/Service/ApiKeyServiceTest.php b/module/Rest/test/Service/ApiKeyServiceTest.php
index ee33e109..bf80ae60 100644
--- a/module/Rest/test/Service/ApiKeyServiceTest.php
+++ b/module/Rest/test/Service/ApiKeyServiceTest.php
@@ -45,7 +45,6 @@ class ApiKeyServiceTest extends TestCase
         $this->repo->expects($this->once())->method('nameExists')->with(
             ! empty($name) ? $name : $this->isType('string'),
         )->willReturn(false);
-        $this->em->expects($this->once())->method('flush');
         $this->em->expects($this->once())->method('persist')->with($this->isInstanceOf(ApiKey::class));
 
         $meta = ApiKeyMeta::fromParams(name: $name, expirationDate: $date, roleDefinitions: $roles);
@@ -87,7 +86,6 @@ class ApiKeyServiceTest extends TestCase
             $callCount++;
             return $callCount < 3;
         });
-        $this->em->expects($this->once())->method('flush');
         $this->em->expects($this->once())->method('persist')->with($this->isInstanceOf(ApiKey::class));
 
         $this->service->create(ApiKeyMeta::create());
@@ -97,7 +95,6 @@ class ApiKeyServiceTest extends TestCase
     public function exceptionIsThrownWhileCreatingIfExplicitlyProvidedNameIsInUse(): void
     {
         $this->repo->expects($this->once())->method('nameExists')->with('the_name')->willReturn(true);
-        $this->em->expects($this->never())->method('flush');
         $this->em->expects($this->never())->method('persist');
 
         $this->expectException(InvalidArgumentException::class);
@@ -219,7 +216,6 @@ class ApiKeyServiceTest extends TestCase
 
         $this->repo->expects($this->once())->method('findOneBy')->with(['name' => 'old'])->willReturn(null);
         $this->repo->expects($this->never())->method('nameExists');
-        $this->em->expects($this->never())->method('flush');
 
         $this->expectException(InvalidArgumentException::class);
         $this->expectExceptionMessage('API key with name "old" could not be found');
@@ -235,7 +231,6 @@ class ApiKeyServiceTest extends TestCase
 
         $this->repo->expects($this->once())->method('findOneBy')->with(['name' => 'same_value'])->willReturn($apiKey);
         $this->repo->expects($this->never())->method('nameExists');
-        $this->em->expects($this->never())->method('flush');
 
         $result = $this->service->renameApiKey($renaming);
 
@@ -250,7 +245,6 @@ class ApiKeyServiceTest extends TestCase
 
         $this->repo->expects($this->once())->method('findOneBy')->with(['name' => 'old'])->willReturn($apiKey);
         $this->repo->expects($this->once())->method('nameExists')->with('new')->willReturn(true);
-        $this->em->expects($this->never())->method('flush');
 
         $this->expectException(InvalidArgumentException::class);
         $this->expectExceptionMessage('Another API key with name "new" already exists');
@@ -266,7 +260,6 @@ class ApiKeyServiceTest extends TestCase
 
         $this->repo->expects($this->once())->method('findOneBy')->with(['name' => 'old'])->willReturn($apiKey);
         $this->repo->expects($this->once())->method('nameExists')->with('new')->willReturn(false);
-        $this->em->expects($this->once())->method('flush');
 
         $result = $this->service->renameApiKey($renaming);