OXDEV-8489 Prepared & Consumed ModuleBlocklistService

Prepared this service ModuleBlocklistService Consumed above service in ModuleSwitchService
OXID-eSales · Jul 15, 2024 · 4c9b215 · 4c9b215
1 parent 60a588d
commit 4c9b215
Show file tree

Hide file tree

Showing 10 changed files with 73 additions and 55 deletions.
diff --git a/src/Module/Exception/ModuleBlockListException.php b/src/Module/Exception/ModuleBlockListException.php
@@ -13,7 +13,7 @@
 
 final class ModuleBlockListException extends NotFound
 {
-    public const EXCEPTION_MESSAGE = "An error occurred while getting module blocklist.";
+    public const EXCEPTION_MESSAGE = "Failed to load module blocklist from YAML file.";
 
     public function __construct()
     {

diff --git a/src/Module/Exception/ModuleDeactivationException.php b/src/Module/Exception/ModuleDeactivationException.php
@@ -13,7 +13,7 @@
 
 final class ModuleDeactivationException extends NotFound
 {
-    public const EXCEPTION_MESSAGE = "An error occurred while deactivating the module.";
+    public const EXCEPTION_MESSAGE = "This Module is in the blocklist and cannot be deactivated.";
 
     public function __construct()
     {

diff --git a/src/Module/Infrastructure/ModuleSwitchInfrastructure.php b/src/Module/Infrastructure/ModuleSwitchInfrastructure.php
@@ -13,14 +13,12 @@
 use OxidEsales\EshopCommunity\Internal\Transition\Utility\ContextInterface;
 use OxidEsales\GraphQL\ConfigurationAccess\Module\Exception\ModuleActivationException;
 use OxidEsales\GraphQL\ConfigurationAccess\Module\Exception\ModuleDeactivationException;
-use OxidEsales\GraphQL\ConfigurationAccess\Module\Service\ModuleBlocklistServiceInterface;
 
 class ModuleSwitchInfrastructure implements ModuleSwitchInfrastructureInterface
 {
     public function __construct(
         private readonly ContextInterface $context,
-        private readonly ModuleActivationBridgeInterface $moduleActivationBridge,
-        private readonly ModuleBlocklistServiceInterface $moduleBlocklistService
+        private readonly ModuleActivationBridgeInterface $moduleActivationBridge
     ) {
     }
 
@@ -44,11 +42,6 @@ public function activateModule(string $moduleId): bool
      */
     public function deactivateModule(string $moduleId): bool
     {
-        $moduleBlocklist = $this->moduleBlocklistService->getModuleBlocklist();
-        if (in_array($moduleId, $moduleBlocklist, true)) {
-            throw new ModuleDeactivationException();
-        }
-
         try {
             $shopId = $this->context->getCurrentShopId();
             $this->moduleActivationBridge->deactivate(moduleId: $moduleId, shopId: $shopId);

diff --git a/src/Module/Service/ModuleBlocklistService.php b/src/Module/Service/ModuleBlocklistService.php
@@ -23,13 +23,13 @@ public function __construct(
     /**
      * @inheritDoc
      */
-    public function getModuleBlocklist(): array
+    public function isModuleBlocked(string $moduleId): bool
     {
         try {
             $resolvedPath = dirname(__FILE__) . '/' . $this->moduleBlocklist;
             $blocklistData = $this->yamlFileLoader->load($resolvedPath);
 
-            return $blocklistData['modules'];
+            return in_array($moduleId, $blocklistData['modules'], true);
         } catch (\Exception $e) {
             throw new ModuleBlockListException();
         }

diff --git a/src/Module/Service/ModuleBlocklistServiceInterface.php b/src/Module/Service/ModuleBlocklistServiceInterface.php
@@ -13,7 +13,6 @@ interface ModuleBlocklistServiceInterface
 {
     /**
      * @throws ModuleBlockListException
-     * @return  array{int : string}
      */
-    public function getModuleBlocklist(): array;
+    public function isModuleBlocked(string $moduleId): bool;
 }
diff --git a/src/Module/Service/ModuleSwitchService.php b/src/Module/Service/ModuleSwitchService.php
@@ -9,22 +9,33 @@
 
 namespace OxidEsales\GraphQL\ConfigurationAccess\Module\Service;
 
+use OxidEsales\GraphQL\ConfigurationAccess\Module\Exception\ModuleDeactivationException;
 use OxidEsales\GraphQL\ConfigurationAccess\Module\Infrastructure\ModuleSwitchInfrastructureInterface;
 
 class ModuleSwitchService implements ModuleSwitchServiceInterface
 {
     public function __construct(
-        private readonly ModuleSwitchInfrastructureInterface $moduleSwitchInfrastructure
+        private readonly ModuleSwitchInfrastructureInterface $moduleSwitchInfrastructure,
+        private readonly ModuleBlocklistServiceInterface $moduleBlocklistService
     ) {
     }
 
+    /**
+     * @inheritDoc
+     */
     public function activateModule(string $moduleId): bool
     {
         return $this->moduleSwitchInfrastructure->activateModule(moduleId: $moduleId);
     }
 
+    /**
+     * @inheritDoc
+     */
     public function deactivateModule(string $moduleId): bool
     {
+        if ($this->moduleBlocklistService->isModuleBlocked($moduleId)) {
+            throw new ModuleDeactivationException();
+        }
         return $this->moduleSwitchInfrastructure->deactivateModule(moduleId: $moduleId);
     }
 }
diff --git a/src/Module/Service/ModuleSwitchServiceInterface.php b/src/Module/Service/ModuleSwitchServiceInterface.php
@@ -7,8 +7,15 @@
 
 namespace OxidEsales\GraphQL\ConfigurationAccess\Module\Service;
 
+use OxidEsales\GraphQL\ConfigurationAccess\Module\Exception\ModuleActivationException;
+use OxidEsales\GraphQL\ConfigurationAccess\Module\Exception\ModuleDeactivationException;
+
 interface ModuleSwitchServiceInterface
 {
     public function activateModule(string $moduleId): bool;
+
+    /**
+     * @throws ModuleDeactivationException
+     */
     public function deactivateModule(string $moduleId): bool;
 }
diff --git a/tests/Unit/Module/Infrastructure/ModuleSwitchInfrastructureTest.php b/tests/Unit/Module/Infrastructure/ModuleSwitchInfrastructureTest.php
@@ -11,7 +11,6 @@
 
 use OxidEsales\EshopCommunity\Internal\Transition\Utility\ContextInterface;
 use OxidEsales\GraphQL\ConfigurationAccess\Module\Infrastructure\ModuleSwitchInfrastructure;
-use OxidEsales\GraphQL\ConfigurationAccess\Module\Service\ModuleBlocklistServiceInterface;
 use OxidEsales\EshopCommunity\Internal\Framework\Module\Setup\Bridge\ModuleActivationBridgeInterface;
 use OxidEsales\GraphQL\ConfigurationAccess\Module\Exception\ModuleActivationException;
 use OxidEsales\GraphQL\ConfigurationAccess\Module\Exception\ModuleDeactivationException;
@@ -35,15 +34,9 @@ public function testModuleActivationAndDeactivation(
             ->method($method)
             ->with($moduleId, $shopId);
 
-        $moduleBlocklistServiceMock = $this->createMock(ModuleBlocklistServiceInterface::class);
-        $moduleBlocklistServiceMock
-            ->method('getModuleBlocklist')
-            ->willReturn(['randomModuleId']);
-
         $sut = $this->getSut(
             context: $this->getContextMock(),
-            moduleActivationBridge: $moduleActivationBridgeMock,
-            moduleBlocklistService: $moduleBlocklistServiceMock
+            moduleActivationBridge: $moduleActivationBridgeMock
         );
 
         $result = ($method == 'activate') ? $sut->activateModule($moduleId) : $sut->deactivateModule($moduleId);
@@ -57,7 +50,6 @@ public function testModuleActivationAndDeactivation(
     public function testModuleActivationAndDeactivationExceptions(
         string $method,
         string $moduleId,
-        string $blockedModuleId,
         mixed $exceptionClass
     ): void {
 
@@ -66,17 +58,11 @@ public function testModuleActivationAndDeactivationExceptions(
             ->method($method)
             ->willThrowException(new \Exception());
 
-        $moduleBlocklistServiceMock = $this->createMock(ModuleBlocklistServiceInterface::class);
-        $moduleBlocklistServiceMock
-            ->method('getModuleBlocklist')
-            ->willReturn(["$blockedModuleId"]);
-
         $this->expectException($exceptionClass);
         $this->expectExceptionMessage($exceptionClass::EXCEPTION_MESSAGE);
 
         $sut = $this->getSut(
-            moduleActivationBridge: $moduleActivationBridgeMock,
-            moduleBlocklistService: $moduleBlocklistServiceMock
+            moduleActivationBridge: $moduleActivationBridgeMock
         );
 
         ($method === 'activate') ? $sut->activateModule($moduleId) : $sut->deactivateModule($moduleId);
@@ -103,37 +89,27 @@ public static function exceptionDataProvider(): \Generator
         yield 'test activate module throws exception' => [
             'method' => 'activate',
             'moduleId' => $moduleId,
-            'blockedModuleId' => uniqid(),
             'exceptionClass' => ModuleActivationException::class,
-        ];
 
-        yield 'test deactivate module throws exception when module is not in blockList' => [
-            'method' => 'deactivate',
-            'moduleId' => $moduleId,
-            'blockedModuleId' => uniqid(),
-            'exceptionClass' => ModuleDeactivationException::class,
         ];
 
-        yield 'test deactivate module throws exception when module is present in blockList' => [
+        yield 'test deactivate module throws exception' => [
             'method' => 'deactivate',
-            'moduleId' => $moduleId,
-            'blockedModuleId' => $moduleId,
+            'moduleId' => uniqid(),
             'exceptionClass' => ModuleDeactivationException::class,
+
         ];
     }
 
     public function getSut(
         ContextInterface $context = null,
-        ModuleActivationBridgeInterface $moduleActivationBridge = null,
-        ModuleBlocklistServiceInterface $moduleBlocklistService = null
+        ModuleActivationBridgeInterface $moduleActivationBridge = null
     ): ModuleSwitchInfrastructure {
         return new ModuleSwitchInfrastructure(
             context: $context
                 ??  $this->createStub(ContextInterface::class),
             moduleActivationBridge: $moduleActivationBridge
-                ??  $this->createStub(ModuleActivationBridgeInterface::class),
-            moduleBlocklistService: $moduleBlocklistService
-                ??  $this->createStub(ModuleBlocklistServiceInterface::class)
+                ??  $this->createStub(ModuleActivationBridgeInterface::class)
         );
     }
 }
diff --git a/tests/Unit/Module/Service/ModuleBlocklistServiceTest.php b/tests/Unit/Module/Service/ModuleBlocklistServiceTest.php
@@ -19,8 +19,13 @@
  */
 class ModuleBlocklistServiceTest extends TestCase
 {
-    public function testGetModuleBlocklistReturnsData(): void
-    {
+    /**
+     * @dataProvider blockListDataProvider
+     */
+    public function testIsModuleBlocked(
+        string $moduleId,
+        bool $expectedResult
+    ): void {
         $filePath = 'testFilePath.yaml';
         $expectedData = ['modules' => ['module1', 'module2']];
 
@@ -31,22 +36,36 @@ public function testGetModuleBlocklistReturnsData(): void
             ->willReturn($expectedData);
 
         $sut = $this->getSut(moduleBlockList: $filePath, yamlFileLoader: $yamlFileLoaderMock);
-        $actualResult = $sut->getModuleBlocklist();
+        $actualResult = $sut->isModuleBlocked(moduleId: $moduleId);
+
+        $this->assertSame($expectedResult, $actualResult);
+    }
+
+    public static function blockListDataProvider(): \Generator
+    {
+        yield 'isModuleBlocked returns true if Module is in the BlockList' => [
+            'moduleId' => 'module1',
+            'expectedResult' => true
+        ];
 
-        $this->assertSame($expectedData['modules'], $actualResult);
+        yield 'isModuleBlocked returns false if Module is not in the BlockList' => [
+            'moduleId' => 'unknownModuleId',
+            'expectedResult' => false
+        ];
     }
 
     public function testGetModuleBlocklistThrowsExceptionOnFailure(): void
     {
+        $moduleId = uniqid();
         $yamlFileLoaderMock = $this->createMock(YamlFileLoaderInfrastructureInterface::class);
         $yamlFileLoaderMock
             ->method('load')
-            ->will($this->throwException(new \Exception('File not found')));
+            ->will($this->throwException(new \Exception('Failed to load YAML file')));
 
         $this->expectException(ModuleBlockListException::class);
 
         $sut = $this->getSut(yamlFileLoader: $yamlFileLoaderMock);
-        $sut->getModuleBlocklist();
+        $sut->isModuleBlocked(moduleId: $moduleId);
     }
 
     private function getSut(

diff --git a/tests/Unit/Module/Service/ModuleSwitchServiceTest.php b/tests/Unit/Module/Service/ModuleSwitchServiceTest.php
@@ -10,6 +10,7 @@
 namespace OxidEsales\GraphQL\ConfigurationAccess\Tests\Unit\Module\Service;
 
 use OxidEsales\GraphQL\ConfigurationAccess\Module\Infrastructure\ModuleSwitchInfrastructureInterface;
+use OxidEsales\GraphQL\ConfigurationAccess\Module\Service\ModuleBlocklistServiceInterface;
 use OxidEsales\GraphQL\ConfigurationAccess\Module\Service\ModuleSwitchService;
 use PHPUnit\Framework\TestCase;
 
@@ -31,7 +32,16 @@ public function testModuleActivationAndDeactivation(
             ->with($moduleId)
             ->willReturn(true);
 
-        $sut = $this->getSut(moduleSwitchInfrastructure: $moduleSwitchInfrastructureMock);
+        $moduleBlocklistServiceMock = $this->createMock(ModuleBlocklistServiceInterface::class);
+        $moduleBlocklistServiceMock
+            ->method('isModuleBlocked')
+            ->with($moduleId)
+            ->willReturn(false);
+
+        $sut = $this->getSut(
+            moduleSwitchInfrastructure: $moduleSwitchInfrastructureMock,
+            moduleBlocklistService: $moduleBlocklistServiceMock
+        );
         $actualResult = ($method == 'activateModule')
             ? $sut->activateModule(moduleId: $moduleId)
             : $sut->deactivateModule(moduleId: $moduleId);
@@ -40,11 +50,14 @@ public function testModuleActivationAndDeactivation(
     }
 
     private function getSut(
-        ModuleSwitchInfrastructureInterface $moduleSwitchInfrastructure = null
+        ModuleSwitchInfrastructureInterface $moduleSwitchInfrastructure = null,
+        ModuleBlocklistServiceInterface $moduleBlocklistService = null
     ): ModuleSwitchService {
         return new ModuleSwitchService(
             moduleSwitchInfrastructure: $moduleSwitchInfrastructure
-                ?? $this->createStub(ModuleSwitchInfrastructureInterface::class)
+                ?? $this->createStub(ModuleSwitchInfrastructureInterface::class),
+            moduleBlocklistService: $moduleBlocklistService
+                ?? $this->createStub(ModuleBlocklistServiceInterface::class)
         );
     }