From 4c9b215783d15b19f4a6ba4367285bd679bfe1af Mon Sep 17 00:00:00 2001 From: RahatHameed Date: Thu, 11 Jul 2024 17:59:42 +0200 Subject: [PATCH] OXDEV-8489 Prepared & Consumed ModuleBlocklistService Prepared this service ModuleBlocklistService Consumed above service in ModuleSwitchService --- .../Exception/ModuleBlockListException.php | 2 +- .../Exception/ModuleDeactivationException.php | 2 +- .../ModuleSwitchInfrastructure.php | 9 +---- src/Module/Service/ModuleBlocklistService.php | 4 +- .../ModuleBlocklistServiceInterface.php | 3 +- src/Module/Service/ModuleSwitchService.php | 13 ++++++- .../Service/ModuleSwitchServiceInterface.php | 7 ++++ .../ModuleSwitchInfrastructureTest.php | 38 ++++--------------- .../Service/ModuleBlocklistServiceTest.php | 31 ++++++++++++--- .../Service/ModuleSwitchServiceTest.php | 19 ++++++++-- 10 files changed, 73 insertions(+), 55 deletions(-) diff --git a/src/Module/Exception/ModuleBlockListException.php b/src/Module/Exception/ModuleBlockListException.php index fac9aac..ff77191 100644 --- 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 index 68fc0b8..b4c40d0 100644 --- 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 index a3dd6d5..5d55530 100644 --- 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 index 76a5a9e..fae867c 100644 --- 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 index 16e34f3..49cfaf1 100644 --- 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 index 75ca5c0..414a962 100644 --- 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 index 0178c57..42ddb65 100644 --- 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 index 0860552..06bbc0f 100644 --- 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 index 7f6aff8..4b7c70c 100644 --- 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 index 1a87b27..bd8732c 100644 --- 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) ); }