From 01d90e2778f594a9eb93b2f30b850c7bf13cb63c Mon Sep 17 00:00:00 2001 From: RahatHameed Date: Mon, 8 Jul 2024 17:53:41 +0200 Subject: [PATCH 01/19] OXDEV-8216 Prepared ModuleSwitchInfrastructure --- .../ModuleSwitchInfrastructure.php | 54 +++++++++ .../ModuleSwitchInfrastructureInterface.php | 24 ++++ .../ModuleActivationExceptionTest.php | 3 +- .../ModuleDeactivationExceptionTest.php | 2 +- .../ModuleSwitchInfrastructureTest.php | 114 ++++++++++++++++++ 5 files changed, 194 insertions(+), 3 deletions(-) create mode 100644 src/Module/Infrastructure/ModuleSwitchInfrastructure.php create mode 100644 src/Module/Infrastructure/ModuleSwitchInfrastructureInterface.php create mode 100644 tests/Unit/Module/Infrastructure/ModuleSwitchInfrastructureTest.php diff --git a/src/Module/Infrastructure/ModuleSwitchInfrastructure.php b/src/Module/Infrastructure/ModuleSwitchInfrastructure.php new file mode 100644 index 0000000..5d55530 --- /dev/null +++ b/src/Module/Infrastructure/ModuleSwitchInfrastructure.php @@ -0,0 +1,54 @@ +context->getCurrentShopId(); + $this->moduleActivationBridge->activate(moduleId: $moduleId, shopId: $shopId); + } catch (\Exception $exception) { + throw new ModuleActivationException(); + } + + return true; + } + + /** + * @inheritDoc + */ + public function deactivateModule(string $moduleId): bool + { + try { + $shopId = $this->context->getCurrentShopId(); + $this->moduleActivationBridge->deactivate(moduleId: $moduleId, shopId: $shopId); + } catch (\Exception $exception) { + throw new ModuleDeactivationException(); + } + + return true; + } +} diff --git a/src/Module/Infrastructure/ModuleSwitchInfrastructureInterface.php b/src/Module/Infrastructure/ModuleSwitchInfrastructureInterface.php new file mode 100644 index 0000000..0558cbc --- /dev/null +++ b/src/Module/Infrastructure/ModuleSwitchInfrastructureInterface.php @@ -0,0 +1,24 @@ +createMock(ModuleActivationBridgeInterface::class); + $moduleActivationBridgeMock + ->method($method) + ->with($moduleId, $shopId); + + $sut = $this->getSut( + context: $this->getContextMock(), + moduleActivationBridge: $moduleActivationBridgeMock + ); + + $result = ($method == 'activate') ? $sut->activateModule($moduleId) : $sut->deactivateModule($moduleId); + + $this->assertTrue($result); + } + + /** + * @dataProvider exceptionDataProvider + */ + public function testModuleActivationAndDeactivationExceptions( + string $method, + string $moduleId, + mixed $exceptionClass + ): void { + + $moduleActivationBridgeMock = $this->createMock(ModuleActivationBridgeInterface::class); + $moduleActivationBridgeMock + ->method($method) + ->willThrowException(new \Exception()); + + $this->expectException($exceptionClass); + $this->expectExceptionMessage($exceptionClass::EXCEPTION_MESSAGE); + + $sut = $this->getSut( + moduleActivationBridge: $moduleActivationBridgeMock + ); + + ($method === 'activate') ? $sut->activateModule($moduleId) : $sut->deactivateModule($moduleId); + } + + public static function activationDataProvider(): \Generator + { + yield 'test activate module' => [ + 'method' => 'activate', + 'moduleId' => uniqid(), + 'shopId' => 1 + ]; + + yield 'test deactivate module' => [ + 'method' => 'deactivate', + 'moduleId' => uniqid(), + 'shopId' => 1 + ]; + } + + public static function exceptionDataProvider(): \Generator + { + yield 'test activate module throws exception' => [ + 'method' => 'activate', + 'moduleId' => uniqid(), + 'exceptionClass' => ModuleActivationException::class, + + ]; + + yield 'test deactivate module throws exception' => [ + 'method' => 'deactivate', + 'moduleId' => uniqid(), + 'exceptionClass' => ModuleDeactivationException::class, + + ]; + } + + public function getSut( + ContextInterface $context = null, + ModuleActivationBridgeInterface $moduleActivationBridge = null + ): ModuleSwitchInfrastructure { + return new ModuleSwitchInfrastructure( + context: $context + ?? $this->createStub(ContextInterface::class), + moduleActivationBridge: $moduleActivationBridge + ?? $this->createStub(ModuleActivationBridgeInterface::class) + ); + } +} From 9c1493ed58798c836f77a5ee3cb62094a49e4e0a Mon Sep 17 00:00:00 2001 From: RahatHameed Date: Wed, 10 Jul 2024 18:35:20 +0200 Subject: [PATCH 02/19] OXDEV-8216 Codeception test coverage --- .../Acceptance/Module/ModuleSwitchCest.php | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 tests/Codeception/Acceptance/Module/ModuleSwitchCest.php diff --git a/tests/Codeception/Acceptance/Module/ModuleSwitchCest.php b/tests/Codeception/Acceptance/Module/ModuleSwitchCest.php new file mode 100644 index 0000000..f4b1d61 --- /dev/null +++ b/tests/Codeception/Acceptance/Module/ModuleSwitchCest.php @@ -0,0 +1,60 @@ +login($this->getAdminUsername(), $this->getAdminPassword()); + + $result = $this->runModuleMutation( + I: $I, + queryName: $example['queryName'], + field: $example['field'] + ); + + $I->assertArrayNotHasKey('errors', $result); + $response = $result['data'][$example['queryName']]; + $I->assertTrue($response); + } + + private function runModuleMutation( + AcceptanceTester $I, + string $queryName, + string $field + ): array { + $I->login($this->getAdminUsername(), $this->getAdminPassword()); + $I->sendGQLQuery( + 'mutation { + ' . $queryName . '(' . $field . ': "' . self::TEST_MODULE_ID . '") + }' + ); + + $I->seeResponseIsJson(); + return $I->grabJsonResponseAsArray(); + } + + protected function moduleSwitchDataProvider(): \Generator + { + yield ['queryName' => 'activateModule', 'field' => 'moduleId']; + yield ['queryName' => 'deactivateModule', 'field' => 'moduleId']; + } +} From 4dc6330cc46795d7d91322b3e3c6fc27dad02e7d Mon Sep 17 00:00:00 2001 From: RahatHameed Date: Wed, 10 Jul 2024 18:35:58 +0200 Subject: [PATCH 03/19] OXDEV-8216 Mention mutations in changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a3de70..05a279f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,10 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [1.2.0] - Unreleased + +### Added +- Mutations to activate and deactive a module. ## [1.2.0] - Unreleased From b6bfb4029df2e2368b3413d5ed2ec243fc6e64be Mon Sep 17 00:00:00 2001 From: RahatHameed Date: Fri, 12 Jul 2024 19:28:58 +0200 Subject: [PATCH 04/19] OXDEV-8889 Prepared YamlFileLoaderInfrastructure --- .../YamlFileLoaderInfrastructure.php | 33 +++++++++ .../YamlFileLoaderInfrastructureInterface.php | 19 ++++++ .../YamlFileLoaderInfrastructureTest.php | 68 +++++++++++++++++++ 3 files changed, 120 insertions(+) create mode 100644 src/Module/Infrastructure/YamlFileLoaderInfrastructure.php create mode 100644 src/Module/Infrastructure/YamlFileLoaderInfrastructureInterface.php create mode 100644 tests/Unit/Module/Infrastructure/YamlFileLoaderInfrastructureTest.php diff --git a/src/Module/Infrastructure/YamlFileLoaderInfrastructure.php b/src/Module/Infrastructure/YamlFileLoaderInfrastructure.php new file mode 100644 index 0000000..054ced0 --- /dev/null +++ b/src/Module/Infrastructure/YamlFileLoaderInfrastructure.php @@ -0,0 +1,33 @@ +projectYamlDao->loadDIConfigFile($filePath); + $yamlData = $configWrapper->getConfigAsArray(); + } catch (\Exception $e) { + throw new ModuleBlockListException(); + } + + return $yamlData; + } +} diff --git a/src/Module/Infrastructure/YamlFileLoaderInfrastructureInterface.php b/src/Module/Infrastructure/YamlFileLoaderInfrastructureInterface.php new file mode 100644 index 0000000..8f3e77f --- /dev/null +++ b/src/Module/Infrastructure/YamlFileLoaderInfrastructureInterface.php @@ -0,0 +1,19 @@ + ['module1', 'module2']]; + $configWrapperMock = $this->createMock(DIConfigWrapper::class); + $configWrapperMock + ->method('getConfigAsArray') + ->willReturn($expectedData); + + $projectYamlDaoMock = $this->createMock(ProjectYamlDaoInterface::class); + $projectYamlDaoMock + ->method('loadDIConfigFile') + ->with($filePath) + ->willReturn($configWrapperMock); + + $sut = $this->getSut(projectYamlDao: $projectYamlDaoMock); + $actualResult = $sut->load($filePath); + + $this->assertSame($expectedData, $actualResult); + } + + public function testLoaderThrowsException(): void + { + $filePath = 'testFile.yaml'; + $projectYamlDaoMock = $this->createMock(ProjectYamlDaoInterface::class); + $projectYamlDaoMock + ->method('loadDIConfigFile') + ->with($filePath) + ->willThrowException(new \Exception()); + + $this->expectException(ModuleBlockListException::class); + + $sut = $this->getSut(projectYamlDao: $projectYamlDaoMock); + $sut->load($filePath); + } + + private function getSut( + ProjectYamlDaoInterface $projectYamlDao = null + ): YamlFileLoaderInfrastructure { + return new YamlFileLoaderInfrastructure( + projectYamlDao: $projectYamlDao + ?? $this->createStub(YamlFileLoaderInfrastructureInterface::class), + ); + } +} From cabb2a6e6c86ac9437cdf09a304337ea294419ff Mon Sep 17 00:00:00 2001 From: RahatHameed Date: Thu, 11 Jul 2024 17:59:42 +0200 Subject: [PATCH 05/19] OXDEV-8489 Prepared & Consumed ModuleBlocklistService Prepared this service ModuleBlocklistService Consumed above service in ModuleSwitchService --- module_blocklist.yaml | 3 + .../Exception/ModuleBlockListException.php | 22 +++++ src/Module/Service/ModuleBlocklistService.php | 37 +++++++++ .../ModuleBlocklistServiceInterface.php | 18 ++++ src/Module/Service/ModuleSwitchService.php | 0 .../Service/ModuleSwitchServiceInterface.php | 0 src/Module/services.yaml | 11 +++ .../ModuleBlockListExceptionTest.php | 27 ++++++ .../ModuleSwitchInfrastructureTest.php | 3 +- .../Service/ModuleBlocklistServiceTest.php | 82 +++++++++++++++++++ .../Service/ModuleSwitchServiceTest.php | 0 11 files changed, 202 insertions(+), 1 deletion(-) create mode 100644 module_blocklist.yaml create mode 100644 src/Module/Exception/ModuleBlockListException.php create mode 100644 src/Module/Service/ModuleBlocklistService.php create mode 100644 src/Module/Service/ModuleBlocklistServiceInterface.php create mode 100644 src/Module/Service/ModuleSwitchService.php create mode 100644 src/Module/Service/ModuleSwitchServiceInterface.php create mode 100644 tests/Unit/Module/Exception/ModuleBlockListExceptionTest.php create mode 100644 tests/Unit/Module/Service/ModuleBlocklistServiceTest.php create mode 100644 tests/Unit/Module/Service/ModuleSwitchServiceTest.php diff --git a/module_blocklist.yaml b/module_blocklist.yaml new file mode 100644 index 0000000..80ae9ad --- /dev/null +++ b/module_blocklist.yaml @@ -0,0 +1,3 @@ +modules: + - oe_graphql_base + - oe_graphql_configuration_access diff --git a/src/Module/Exception/ModuleBlockListException.php b/src/Module/Exception/ModuleBlockListException.php new file mode 100644 index 0000000..ff77191 --- /dev/null +++ b/src/Module/Exception/ModuleBlockListException.php @@ -0,0 +1,22 @@ +moduleBlocklist; + $blocklistData = $this->yamlFileLoader->load($resolvedPath); + + 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 new file mode 100644 index 0000000..49cfaf1 --- /dev/null +++ b/src/Module/Service/ModuleBlocklistServiceInterface.php @@ -0,0 +1,18 @@ +assertInstanceOf(ModuleBlockListException::class, $exception); + $this->assertSame(ModuleBlockListException::EXCEPTION_MESSAGE, $exception->getMessage()); + } +} diff --git a/tests/Unit/Module/Infrastructure/ModuleSwitchInfrastructureTest.php b/tests/Unit/Module/Infrastructure/ModuleSwitchInfrastructureTest.php index d9b034b..06bbc0f 100644 --- a/tests/Unit/Module/Infrastructure/ModuleSwitchInfrastructureTest.php +++ b/tests/Unit/Module/Infrastructure/ModuleSwitchInfrastructureTest.php @@ -85,9 +85,10 @@ public static function activationDataProvider(): \Generator public static function exceptionDataProvider(): \Generator { + $moduleId = uniqid(); yield 'test activate module throws exception' => [ 'method' => 'activate', - 'moduleId' => uniqid(), + 'moduleId' => $moduleId, 'exceptionClass' => ModuleActivationException::class, ]; diff --git a/tests/Unit/Module/Service/ModuleBlocklistServiceTest.php b/tests/Unit/Module/Service/ModuleBlocklistServiceTest.php new file mode 100644 index 0000000..4b7c70c --- /dev/null +++ b/tests/Unit/Module/Service/ModuleBlocklistServiceTest.php @@ -0,0 +1,82 @@ + ['module1', 'module2']]; + + $yamlFileLoaderMock = $this->createMock(YamlFileLoaderInfrastructureInterface::class); + $yamlFileLoaderMock + ->method('load') + ->with($this->stringContains($filePath)) + ->willReturn($expectedData); + + $sut = $this->getSut(moduleBlockList: $filePath, yamlFileLoader: $yamlFileLoaderMock); + $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 + ]; + + 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('Failed to load YAML file'))); + + $this->expectException(ModuleBlockListException::class); + + $sut = $this->getSut(yamlFileLoader: $yamlFileLoaderMock); + $sut->isModuleBlocked(moduleId: $moduleId); + } + + private function getSut( + string $moduleBlockList = null, + YamlFileLoaderInfrastructureInterface $yamlFileLoader = null + ): ModuleBlocklistService { + return new ModuleBlocklistService( + moduleBlocklist: $moduleBlockList + ?? 'testFilePath.yaml', + yamlFileLoader: $yamlFileLoader + ?? $this->createStub(YamlFileLoaderInfrastructureInterface::class) + ); + } +} diff --git a/tests/Unit/Module/Service/ModuleSwitchServiceTest.php b/tests/Unit/Module/Service/ModuleSwitchServiceTest.php new file mode 100644 index 0000000..e69de29 From abd82d59cd8aabc4b4d9398343194dd07ff8289f Mon Sep 17 00:00:00 2001 From: RahatHameed Date: Mon, 15 Jul 2024 16:18:16 +0200 Subject: [PATCH 06/19] OXDEV-8489 Extended ModuleDeactivationException --- src/Module/Exception/ModuleDeactivationException.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/Module/Exception/ModuleDeactivationException.php b/src/Module/Exception/ModuleDeactivationException.php index 553084c..5828d17 100644 --- a/src/Module/Exception/ModuleDeactivationException.php +++ b/src/Module/Exception/ModuleDeactivationException.php @@ -14,9 +14,13 @@ final class ModuleDeactivationException extends NotFound { private const EXCEPTION_MESSAGE = "An error occurred while deactivating the module."; + public const BLOCKED_MODULE_MESSAGE = 'Module "%s" is in the blocklist and cannot be deactivated.'; - public function __construct() + public function __construct(string $message = null) { - parent::__construct(self::EXCEPTION_MESSAGE); + if (empty($message)) { + $message = self::EXCEPTION_MESSAGE; + } + parent::__construct($message); } } From 92d877c7af1eab8a2b6bc7ae95d6c5f2ae97eefa Mon Sep 17 00:00:00 2001 From: RahatHameed Date: Mon, 15 Jul 2024 16:52:50 +0200 Subject: [PATCH 07/19] OXDEV-8489 Mentioned modules block list in changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 05a279f..0192790 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [1.2.0] - Unreleased ### Added +- Prevention of deactivation of certain modules mentioned in modules_blocklist.yaml. - Mutations to activate and deactive a module. ## [1.2.0] - Unreleased @@ -37,6 +38,7 @@ This is stable release for v1.1.0. No changes have been made since v1.1.0-rc.1. - Initial release +[1.2.0]: https://github.com/OXID-eSales/graphql-configuration-access/compare/v1.1.0...v1.2.0 [1.1.0]: https://github.com/OXID-eSales/graphql-configuration-access/compare/v1.1.0-rc.1...v1.1.0 [1.1.0-rc.1]: https://github.com/OXID-eSales/graphql-configuration-access/compare/v1.0.0...v1.1.0-rc.1 [1.0.0]: https://github.com/OXID-eSales/graphql-configuration-access/releases/tag/v1.0.0 From 9eb58dc9fa77f8d01b99288a01dcd66af623081b Mon Sep 17 00:00:00 2001 From: marcelmanzel Date: Tue, 13 Aug 2024 14:36:39 +0200 Subject: [PATCH 08/19] OXDEV-8489: Cleanup rebase conflicts --- CHANGELOG.md | 8 +- .../ModuleSwitchInfrastructure.php | 54 -------- .../ModuleSwitchInfrastructureInterface.php | 24 ---- .../Service/ModuleActivationService.php | 9 +- src/Module/Service/ModuleSwitchService.php | 0 .../Service/ModuleSwitchServiceInterface.php | 0 .../ModuleSwitchInfrastructureTest.php | 115 ------------------ .../Service/ModuleActivationsServiceTest.php | 14 ++- 8 files changed, 21 insertions(+), 203 deletions(-) delete mode 100644 src/Module/Infrastructure/ModuleSwitchInfrastructure.php delete mode 100644 src/Module/Infrastructure/ModuleSwitchInfrastructureInterface.php delete mode 100644 src/Module/Service/ModuleSwitchService.php delete mode 100644 src/Module/Service/ModuleSwitchServiceInterface.php delete mode 100644 tests/Unit/Module/Infrastructure/ModuleSwitchInfrastructureTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 0192790..36ce835 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,11 +3,6 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [1.2.0] - Unreleased - -### Added -- Prevention of deactivation of certain modules mentioned in modules_blocklist.yaml. -- Mutations to activate and deactive a module. ## [1.2.0] - Unreleased @@ -16,16 +11,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Module list and filtering option on basis of module name and status - Activation of given theme by themeId - Mutations to activate and deactive a module. +- Prevention of deactivation of certain modules mentioned in modules_blocklist.yaml. ## [1.1.0] - 2024-07-05 This is stable release for v1.1.0. No changes have been made since v1.1.0-rc.1. ## [1.1.0-rc.1] - 2024-05-30 -[.gitignore](.gitignore) ### Added - PHP 8.2 support - Module activation dependency on GraphQL Base module -- Activate a given theme by themeId. ### Changed - PHPUnit upgraded to version 10.x diff --git a/src/Module/Infrastructure/ModuleSwitchInfrastructure.php b/src/Module/Infrastructure/ModuleSwitchInfrastructure.php deleted file mode 100644 index 5d55530..0000000 --- a/src/Module/Infrastructure/ModuleSwitchInfrastructure.php +++ /dev/null @@ -1,54 +0,0 @@ -context->getCurrentShopId(); - $this->moduleActivationBridge->activate(moduleId: $moduleId, shopId: $shopId); - } catch (\Exception $exception) { - throw new ModuleActivationException(); - } - - return true; - } - - /** - * @inheritDoc - */ - public function deactivateModule(string $moduleId): bool - { - try { - $shopId = $this->context->getCurrentShopId(); - $this->moduleActivationBridge->deactivate(moduleId: $moduleId, shopId: $shopId); - } catch (\Exception $exception) { - throw new ModuleDeactivationException(); - } - - return true; - } -} diff --git a/src/Module/Infrastructure/ModuleSwitchInfrastructureInterface.php b/src/Module/Infrastructure/ModuleSwitchInfrastructureInterface.php deleted file mode 100644 index 0558cbc..0000000 --- a/src/Module/Infrastructure/ModuleSwitchInfrastructureInterface.php +++ /dev/null @@ -1,24 +0,0 @@ -moduleBlocklistService->isModuleBlocked($moduleId)) { + throw new ModuleDeactivationException( + sprintf(ModuleDeactivationException::BLOCKED_MODULE_MESSAGE, $moduleId) + ); + } + $shopId = $this->context->getCurrentShopId(); try { diff --git a/src/Module/Service/ModuleSwitchService.php b/src/Module/Service/ModuleSwitchService.php deleted file mode 100644 index e69de29..0000000 diff --git a/src/Module/Service/ModuleSwitchServiceInterface.php b/src/Module/Service/ModuleSwitchServiceInterface.php deleted file mode 100644 index e69de29..0000000 diff --git a/tests/Unit/Module/Infrastructure/ModuleSwitchInfrastructureTest.php b/tests/Unit/Module/Infrastructure/ModuleSwitchInfrastructureTest.php deleted file mode 100644 index 06bbc0f..0000000 --- a/tests/Unit/Module/Infrastructure/ModuleSwitchInfrastructureTest.php +++ /dev/null @@ -1,115 +0,0 @@ -createMock(ModuleActivationBridgeInterface::class); - $moduleActivationBridgeMock - ->method($method) - ->with($moduleId, $shopId); - - $sut = $this->getSut( - context: $this->getContextMock(), - moduleActivationBridge: $moduleActivationBridgeMock - ); - - $result = ($method == 'activate') ? $sut->activateModule($moduleId) : $sut->deactivateModule($moduleId); - - $this->assertTrue($result); - } - - /** - * @dataProvider exceptionDataProvider - */ - public function testModuleActivationAndDeactivationExceptions( - string $method, - string $moduleId, - mixed $exceptionClass - ): void { - - $moduleActivationBridgeMock = $this->createMock(ModuleActivationBridgeInterface::class); - $moduleActivationBridgeMock - ->method($method) - ->willThrowException(new \Exception()); - - $this->expectException($exceptionClass); - $this->expectExceptionMessage($exceptionClass::EXCEPTION_MESSAGE); - - $sut = $this->getSut( - moduleActivationBridge: $moduleActivationBridgeMock - ); - - ($method === 'activate') ? $sut->activateModule($moduleId) : $sut->deactivateModule($moduleId); - } - - public static function activationDataProvider(): \Generator - { - yield 'test activate module' => [ - 'method' => 'activate', - 'moduleId' => uniqid(), - 'shopId' => 1 - ]; - - yield 'test deactivate module' => [ - 'method' => 'deactivate', - 'moduleId' => uniqid(), - 'shopId' => 1 - ]; - } - - public static function exceptionDataProvider(): \Generator - { - $moduleId = uniqid(); - yield 'test activate module throws exception' => [ - 'method' => 'activate', - 'moduleId' => $moduleId, - 'exceptionClass' => ModuleActivationException::class, - - ]; - - yield 'test deactivate module throws exception' => [ - 'method' => 'deactivate', - 'moduleId' => uniqid(), - 'exceptionClass' => ModuleDeactivationException::class, - - ]; - } - - public function getSut( - ContextInterface $context = null, - ModuleActivationBridgeInterface $moduleActivationBridge = null - ): ModuleSwitchInfrastructure { - return new ModuleSwitchInfrastructure( - context: $context - ?? $this->createStub(ContextInterface::class), - moduleActivationBridge: $moduleActivationBridge - ?? $this->createStub(ModuleActivationBridgeInterface::class) - ); - } -} diff --git a/tests/Unit/Module/Service/ModuleActivationsServiceTest.php b/tests/Unit/Module/Service/ModuleActivationsServiceTest.php index 68a2e2e..e4769ff 100644 --- a/tests/Unit/Module/Service/ModuleActivationsServiceTest.php +++ b/tests/Unit/Module/Service/ModuleActivationsServiceTest.php @@ -14,6 +14,7 @@ use OxidEsales\GraphQL\ConfigurationAccess\Module\Exception\ModuleActivationException; use OxidEsales\GraphQL\ConfigurationAccess\Module\Exception\ModuleDeactivationException; use OxidEsales\GraphQL\ConfigurationAccess\Module\Service\ModuleActivationService; +use OxidEsales\GraphQL\ConfigurationAccess\Module\Service\ModuleBlocklistServiceInterface; use OxidEsales\GraphQL\ConfigurationAccess\Tests\Unit\UnitTestCase; /** @@ -34,6 +35,12 @@ public function testModuleActivationAndDeactivation( ->method($method) ->with($moduleId, $shopId); + $moduleBlocklistServiceMock = $this->createMock(ModuleBlocklistServiceInterface::class); + $moduleBlocklistServiceMock + ->method('isModuleBlocked') + ->with($moduleId) + ->willReturn(false); + $sut = $this->getSut( context: $this->getContextMock($shopId), moduleActivationBridge: $moduleActivationBridgeMock @@ -94,13 +101,16 @@ public static function exceptionDataProvider(): \Generator public function getSut( ContextInterface $context = null, - ModuleActivationBridgeInterface $moduleActivationBridge = null + ModuleActivationBridgeInterface $moduleActivationBridge = null, + ModuleBlocklistServiceInterface $moduleBlocklistService = null ): ModuleActivationService { return new ModuleActivationService( context: $context ?? $this->createStub(ContextInterface::class), moduleActivationBridge: $moduleActivationBridge - ?? $this->createStub(ModuleActivationBridgeInterface::class) + ?? $this->createStub(ModuleActivationBridgeInterface::class), + moduleBlocklistService: $moduleBlocklistService + ?? $this->createStub(ModuleBlocklistServiceInterface::class) ); } } From f5ceb068da45ae9e61f3c977b5db91a8647e95f6 Mon Sep 17 00:00:00 2001 From: marcelmanzel Date: Tue, 13 Aug 2024 16:20:21 +0200 Subject: [PATCH 09/19] OXDEV-8489: Cleanup rebase conflicts --- tests/Unit/Module/Service/ModuleSwitchServiceTest.php | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 tests/Unit/Module/Service/ModuleSwitchServiceTest.php diff --git a/tests/Unit/Module/Service/ModuleSwitchServiceTest.php b/tests/Unit/Module/Service/ModuleSwitchServiceTest.php deleted file mode 100644 index e69de29..0000000 From fbe4b63ca3b4aac7b5446e9b537d8fc0bc715875 Mon Sep 17 00:00:00 2001 From: marcelmanzel Date: Wed, 14 Aug 2024 08:36:19 +0200 Subject: [PATCH 10/19] OXDEV-8489: Use normal error class instead of NotFound for several exceptions --- src/Module/Exception/ModuleActivationException.php | 4 ++-- src/Module/Exception/ModuleBlockListException.php | 4 ++-- src/Module/Exception/ModuleDeactivationException.php | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Module/Exception/ModuleActivationException.php b/src/Module/Exception/ModuleActivationException.php index ccc51f9..2e190b3 100644 --- a/src/Module/Exception/ModuleActivationException.php +++ b/src/Module/Exception/ModuleActivationException.php @@ -9,9 +9,9 @@ namespace OxidEsales\GraphQL\ConfigurationAccess\Module\Exception; -use OxidEsales\GraphQL\Base\Exception\NotFound; +use OxidEsales\GraphQL\Base\Exception\Error; -final class ModuleActivationException extends NotFound +final class ModuleActivationException extends Error { private const EXCEPTION_MESSAGE = "An error occurred while activating the module."; diff --git a/src/Module/Exception/ModuleBlockListException.php b/src/Module/Exception/ModuleBlockListException.php index ff77191..ce78c8a 100644 --- a/src/Module/Exception/ModuleBlockListException.php +++ b/src/Module/Exception/ModuleBlockListException.php @@ -9,9 +9,9 @@ namespace OxidEsales\GraphQL\ConfigurationAccess\Module\Exception; -use OxidEsales\GraphQL\Base\Exception\NotFound; +use OxidEsales\GraphQL\Base\Exception\Error; -final class ModuleBlockListException extends NotFound +final class ModuleBlockListException extends Error { public const EXCEPTION_MESSAGE = "Failed to load module blocklist from YAML file."; diff --git a/src/Module/Exception/ModuleDeactivationException.php b/src/Module/Exception/ModuleDeactivationException.php index 5828d17..1b9f4a0 100644 --- a/src/Module/Exception/ModuleDeactivationException.php +++ b/src/Module/Exception/ModuleDeactivationException.php @@ -9,9 +9,9 @@ namespace OxidEsales\GraphQL\ConfigurationAccess\Module\Exception; -use OxidEsales\GraphQL\Base\Exception\NotFound; +use OxidEsales\GraphQL\Base\Exception\Error; -final class ModuleDeactivationException extends NotFound +final class ModuleDeactivationException extends Error { private const EXCEPTION_MESSAGE = "An error occurred while deactivating the module."; public const BLOCKED_MODULE_MESSAGE = 'Module "%s" is in the blocklist and cannot be deactivated.'; From 588954fdd479eb3eaa5315384939edab177111bc Mon Sep 17 00:00:00 2001 From: marcelmanzel Date: Wed, 14 Aug 2024 08:48:18 +0200 Subject: [PATCH 11/19] OXDEV-8489: Create new ModuleDeactivationBlockedException --- .../ModuleDeactivationBlockedException.php | 22 +++++++++++++ .../Exception/ModuleDeactivationException.php | 8 ++--- .../Service/ModuleActivationService.php | 5 ++- .../ModuleActivationServiceInterface.php | 2 ++ .../ModuleBlockListExceptionTest.php | 2 +- ...ModuleDeactivationBlockedExceptionTest.php | 31 +++++++++++++++++++ .../Service/ModuleActivationsServiceTest.php | 16 ++++++++++ 7 files changed, 76 insertions(+), 10 deletions(-) create mode 100644 src/Module/Exception/ModuleDeactivationBlockedException.php create mode 100644 tests/Unit/Module/Exception/ModuleDeactivationBlockedExceptionTest.php diff --git a/src/Module/Exception/ModuleDeactivationBlockedException.php b/src/Module/Exception/ModuleDeactivationBlockedException.php new file mode 100644 index 0000000..994758f --- /dev/null +++ b/src/Module/Exception/ModuleDeactivationBlockedException.php @@ -0,0 +1,22 @@ +moduleBlocklistService->isModuleBlocked($moduleId)) { - throw new ModuleDeactivationException( - sprintf(ModuleDeactivationException::BLOCKED_MODULE_MESSAGE, $moduleId) - ); + throw new ModuleDeactivationBlockedException($moduleId); } $shopId = $this->context->getCurrentShopId(); diff --git a/src/Module/Service/ModuleActivationServiceInterface.php b/src/Module/Service/ModuleActivationServiceInterface.php index 1181e9c..0ae22b9 100644 --- a/src/Module/Service/ModuleActivationServiceInterface.php +++ b/src/Module/Service/ModuleActivationServiceInterface.php @@ -9,6 +9,7 @@ use OxidEsales\GraphQL\ConfigurationAccess\Module\Exception\ModuleActivationException; use OxidEsales\GraphQL\ConfigurationAccess\Module\Exception\ModuleDeactivationException; +use OxidEsales\GraphQL\ConfigurationAccess\Module\Exception\ModuleDeactivationBlockedException; interface ModuleActivationServiceInterface { @@ -19,6 +20,7 @@ public function activateModule(string $moduleId): bool; /** * @throws ModuleDeactivationException + * @throws ModuleDeactivationBlockedException */ public function deactivateModule(string $moduleId): bool; } diff --git a/tests/Unit/Module/Exception/ModuleBlockListExceptionTest.php b/tests/Unit/Module/Exception/ModuleBlockListExceptionTest.php index a7177cf..5435824 100644 --- a/tests/Unit/Module/Exception/ModuleBlockListExceptionTest.php +++ b/tests/Unit/Module/Exception/ModuleBlockListExceptionTest.php @@ -22,6 +22,6 @@ public function testException(): void $exception = new ModuleBlockListException(); $this->assertInstanceOf(ModuleBlockListException::class, $exception); - $this->assertSame(ModuleBlockListException::EXCEPTION_MESSAGE, $exception->getMessage()); + $this->assertSame('Failed to load module blocklist from YAML file.', $exception->getMessage()); } } diff --git a/tests/Unit/Module/Exception/ModuleDeactivationBlockedExceptionTest.php b/tests/Unit/Module/Exception/ModuleDeactivationBlockedExceptionTest.php new file mode 100644 index 0000000..fc08ba7 --- /dev/null +++ b/tests/Unit/Module/Exception/ModuleDeactivationBlockedExceptionTest.php @@ -0,0 +1,31 @@ +assertInstanceOf(ModuleDeactivationBlockedException::class, $exception); + $this->assertSame( + sprintf('Module "%s" is in the blocklist and cannot be deactivated.', $moduleId), + $exception->getMessage() + ); + } +} diff --git a/tests/Unit/Module/Service/ModuleActivationsServiceTest.php b/tests/Unit/Module/Service/ModuleActivationsServiceTest.php index e4769ff..7626722 100644 --- a/tests/Unit/Module/Service/ModuleActivationsServiceTest.php +++ b/tests/Unit/Module/Service/ModuleActivationsServiceTest.php @@ -12,6 +12,7 @@ use OxidEsales\EshopCommunity\Internal\Transition\Utility\ContextInterface; use OxidEsales\EshopCommunity\Internal\Framework\Module\Setup\Bridge\ModuleActivationBridgeInterface; use OxidEsales\GraphQL\ConfigurationAccess\Module\Exception\ModuleActivationException; +use OxidEsales\GraphQL\ConfigurationAccess\Module\Exception\ModuleDeactivationBlockedException; use OxidEsales\GraphQL\ConfigurationAccess\Module\Exception\ModuleDeactivationException; use OxidEsales\GraphQL\ConfigurationAccess\Module\Service\ModuleActivationService; use OxidEsales\GraphQL\ConfigurationAccess\Module\Service\ModuleBlocklistServiceInterface; @@ -73,6 +74,21 @@ public function testModuleActivationAndDeactivationExceptions( ($method === 'activate') ? $sut->activateModule($moduleId) : $sut->deactivateModule($moduleId); } + public function testModuleDeactivationBlockedException() + { + $moduleBlockListServiceMock = $this->createMock(ModuleBlocklistServiceInterface::class); + $moduleBlockListServiceMock + ->method('isModuleBlocked') + ->willReturn(true); + + $sut = $this->getSut( + moduleBlocklistService: $moduleBlockListServiceMock + ); + + $this->expectException(ModuleDeactivationBlockedException::class); + $sut->deactivateModule(uniqid()); + } + public static function activationDataProvider(): \Generator { yield 'test activate module' => [ From e47dfd75b861c1cb88afdaa0bbf3feead30a0b84 Mon Sep 17 00:00:00 2001 From: marcelmanzel Date: Wed, 14 Aug 2024 13:42:44 +0200 Subject: [PATCH 12/19] OXDEV-8489: Move Infrastructure code to service and remove exception --- .../Exception/ModuleBlockListException.php | 22 ------ .../YamlFileLoaderInfrastructure.php | 33 --------- .../YamlFileLoaderInfrastructureInterface.php | 19 ------ src/Module/Service/ModuleBlocklistService.php | 21 ++---- .../ModuleBlocklistServiceInterface.php | 5 -- src/Module/services.yaml | 7 +- .../ModuleBlockListExceptionTest.php | 27 -------- .../ModuleListInfrastructureTest.php | 2 - .../YamlFileLoaderInfrastructureTest.php | 68 ------------------- .../Service/ModuleBlocklistServiceTest.php | 45 +++++------- 10 files changed, 27 insertions(+), 222 deletions(-) delete mode 100644 src/Module/Exception/ModuleBlockListException.php delete mode 100644 src/Module/Infrastructure/YamlFileLoaderInfrastructure.php delete mode 100644 src/Module/Infrastructure/YamlFileLoaderInfrastructureInterface.php delete mode 100644 tests/Unit/Module/Exception/ModuleBlockListExceptionTest.php rename tests/Unit/Module/{Service => Infrastructure}/ModuleListInfrastructureTest.php (93%) delete mode 100644 tests/Unit/Module/Infrastructure/YamlFileLoaderInfrastructureTest.php diff --git a/src/Module/Exception/ModuleBlockListException.php b/src/Module/Exception/ModuleBlockListException.php deleted file mode 100644 index ce78c8a..0000000 --- a/src/Module/Exception/ModuleBlockListException.php +++ /dev/null @@ -1,22 +0,0 @@ -projectYamlDao->loadDIConfigFile($filePath); - $yamlData = $configWrapper->getConfigAsArray(); - } catch (\Exception $e) { - throw new ModuleBlockListException(); - } - - return $yamlData; - } -} diff --git a/src/Module/Infrastructure/YamlFileLoaderInfrastructureInterface.php b/src/Module/Infrastructure/YamlFileLoaderInfrastructureInterface.php deleted file mode 100644 index 8f3e77f..0000000 --- a/src/Module/Infrastructure/YamlFileLoaderInfrastructureInterface.php +++ /dev/null @@ -1,19 +0,0 @@ -moduleBlocklist; - $blocklistData = $this->yamlFileLoader->load($resolvedPath); + $resolvedPath = dirname(__FILE__) . '/../' . $this->moduleBlocklistPath; + $configWrapper = $this->projectYamlDao->loadDIConfigFile($resolvedPath); + $blocklistData = $configWrapper->getConfigAsArray(); - return in_array($moduleId, $blocklistData['modules'], true); - } catch (\Exception $e) { - throw new ModuleBlockListException(); - } + return in_array($moduleId, $blocklistData['modules'], true); } } diff --git a/src/Module/Service/ModuleBlocklistServiceInterface.php b/src/Module/Service/ModuleBlocklistServiceInterface.php index 49cfaf1..c86b34b 100644 --- a/src/Module/Service/ModuleBlocklistServiceInterface.php +++ b/src/Module/Service/ModuleBlocklistServiceInterface.php @@ -7,12 +7,7 @@ namespace OxidEsales\GraphQL\ConfigurationAccess\Module\Service; -use OxidEsales\GraphQL\ConfigurationAccess\Module\Exception\ModuleBlockListException; - interface ModuleBlocklistServiceInterface { - /** - * @throws ModuleBlockListException - */ public function isModuleBlocked(string $moduleId): bool; } diff --git a/src/Module/services.yaml b/src/Module/services.yaml index 948f3dd..504dd14 100644 --- a/src/Module/services.yaml +++ b/src/Module/services.yaml @@ -1,5 +1,5 @@ parameters: - oxidesales.graphqlconfigurationaccess.modules_block_list: '../../../module_blocklist.yaml' + oxidesales.graphqlconfigurationaccess.modules_block_list_path: '../../module_blocklist.yaml' services: _defaults: @@ -25,10 +25,7 @@ services: OxidEsales\GraphQL\ConfigurationAccess\Module\Service\ModuleActivationServiceInterface: class: OxidEsales\GraphQL\ConfigurationAccess\Module\Service\ModuleActivationService - OxidEsales\GraphQL\ConfigurationAccess\Module\Infrastructure\YamlFileLoaderInfrastructureInterface: - class: OxidEsales\GraphQL\ConfigurationAccess\Module\Infrastructure\YamlFileLoaderInfrastructure - OxidEsales\GraphQL\ConfigurationAccess\Module\Service\ModuleBlocklistServiceInterface: class: OxidEsales\GraphQL\ConfigurationAccess\Module\Service\ModuleBlocklistService arguments: - $moduleBlocklist: '%oxidesales.graphqlconfigurationaccess.modules_block_list%' + $moduleBlocklistPath: '%oxidesales.graphqlconfigurationaccess.modules_block_list_path%' diff --git a/tests/Unit/Module/Exception/ModuleBlockListExceptionTest.php b/tests/Unit/Module/Exception/ModuleBlockListExceptionTest.php deleted file mode 100644 index 5435824..0000000 --- a/tests/Unit/Module/Exception/ModuleBlockListExceptionTest.php +++ /dev/null @@ -1,27 +0,0 @@ -assertInstanceOf(ModuleBlockListException::class, $exception); - $this->assertSame('Failed to load module blocklist from YAML file.', $exception->getMessage()); - } -} diff --git a/tests/Unit/Module/Service/ModuleListInfrastructureTest.php b/tests/Unit/Module/Infrastructure/ModuleListInfrastructureTest.php similarity index 93% rename from tests/Unit/Module/Service/ModuleListInfrastructureTest.php rename to tests/Unit/Module/Infrastructure/ModuleListInfrastructureTest.php index f7d3a83..c13d780 100644 --- a/tests/Unit/Module/Service/ModuleListInfrastructureTest.php +++ b/tests/Unit/Module/Infrastructure/ModuleListInfrastructureTest.php @@ -11,10 +11,8 @@ use OxidEsales\EshopCommunity\Internal\Framework\Module\Configuration\Bridge\ShopConfigurationDaoBridgeInterface; use OxidEsales\EshopCommunity\Internal\Framework\Module\Configuration\DataObject\ShopConfiguration; -use OxidEsales\GraphQL\ConfigurationAccess\Module\DataType\ModuleDataTypeFactoryInterface; use OxidEsales\GraphQL\ConfigurationAccess\Module\Infrastructure\ModuleListInfrastructure; use OxidEsales\GraphQL\ConfigurationAccess\Tests\Unit\UnitTestCase; -use OxidEsales\GraphQL\ConfigurationAccess\Module\DataType\ModuleDataTypeInterface; use OxidEsales\EshopCommunity\Internal\Framework\Module\Configuration\DataObject\ModuleConfiguration; use OxidEsales\GraphQL\ConfigurationAccess\Module\Exception\ModulesNotFoundException; diff --git a/tests/Unit/Module/Infrastructure/YamlFileLoaderInfrastructureTest.php b/tests/Unit/Module/Infrastructure/YamlFileLoaderInfrastructureTest.php deleted file mode 100644 index 3d08656..0000000 --- a/tests/Unit/Module/Infrastructure/YamlFileLoaderInfrastructureTest.php +++ /dev/null @@ -1,68 +0,0 @@ - ['module1', 'module2']]; - $configWrapperMock = $this->createMock(DIConfigWrapper::class); - $configWrapperMock - ->method('getConfigAsArray') - ->willReturn($expectedData); - - $projectYamlDaoMock = $this->createMock(ProjectYamlDaoInterface::class); - $projectYamlDaoMock - ->method('loadDIConfigFile') - ->with($filePath) - ->willReturn($configWrapperMock); - - $sut = $this->getSut(projectYamlDao: $projectYamlDaoMock); - $actualResult = $sut->load($filePath); - - $this->assertSame($expectedData, $actualResult); - } - - public function testLoaderThrowsException(): void - { - $filePath = 'testFile.yaml'; - $projectYamlDaoMock = $this->createMock(ProjectYamlDaoInterface::class); - $projectYamlDaoMock - ->method('loadDIConfigFile') - ->with($filePath) - ->willThrowException(new \Exception()); - - $this->expectException(ModuleBlockListException::class); - - $sut = $this->getSut(projectYamlDao: $projectYamlDaoMock); - $sut->load($filePath); - } - - private function getSut( - ProjectYamlDaoInterface $projectYamlDao = null - ): YamlFileLoaderInfrastructure { - return new YamlFileLoaderInfrastructure( - projectYamlDao: $projectYamlDao - ?? $this->createStub(YamlFileLoaderInfrastructureInterface::class), - ); - } -} diff --git a/tests/Unit/Module/Service/ModuleBlocklistServiceTest.php b/tests/Unit/Module/Service/ModuleBlocklistServiceTest.php index 4b7c70c..8026f29 100644 --- a/tests/Unit/Module/Service/ModuleBlocklistServiceTest.php +++ b/tests/Unit/Module/Service/ModuleBlocklistServiceTest.php @@ -9,8 +9,8 @@ namespace OxidEsales\GraphQL\ConfigurationAccess\Tests\Unit\Module\Service; -use OxidEsales\GraphQL\ConfigurationAccess\Module\Exception\ModuleBlockListException; -use OxidEsales\GraphQL\ConfigurationAccess\Module\Infrastructure\YamlFileLoaderInfrastructureInterface; +use OxidEsales\EshopCommunity\Internal\Framework\DIContainer\Dao\ProjectYamlDaoInterface; +use OxidEsales\EshopCommunity\Internal\Framework\DIContainer\DataObject\DIConfigWrapper; use OxidEsales\GraphQL\ConfigurationAccess\Module\Service\ModuleBlocklistService; use PHPUnit\Framework\TestCase; @@ -26,16 +26,23 @@ public function testIsModuleBlocked( string $moduleId, bool $expectedResult ): void { + $reflector = new \ReflectionClass(ModuleBlocklistService::class); + $classFilePath = pathinfo($reflector->getFileName())['dirname']; $filePath = 'testFilePath.yaml'; $expectedData = ['modules' => ['module1', 'module2']]; - $yamlFileLoaderMock = $this->createMock(YamlFileLoaderInfrastructureInterface::class); - $yamlFileLoaderMock - ->method('load') - ->with($this->stringContains($filePath)) + $configWrapperMock = $this->createMock(DIConfigWrapper::class); + $configWrapperMock + ->method('getConfigAsArray') ->willReturn($expectedData); - $sut = $this->getSut(moduleBlockList: $filePath, yamlFileLoader: $yamlFileLoaderMock); + $projectYamlDaoMock = $this->createMock(ProjectYamlDaoInterface::class); + $projectYamlDaoMock + ->method('loadDIConfigFile') + ->with($classFilePath . '/../' . $filePath) + ->willReturn($configWrapperMock); + + $sut = $this->getSut(moduleBlockListPath: $filePath, projectYamlDao: $projectYamlDaoMock); $actualResult = $sut->isModuleBlocked(moduleId: $moduleId); $this->assertSame($expectedResult, $actualResult); @@ -54,29 +61,13 @@ public static function blockListDataProvider(): \Generator ]; } - public function testGetModuleBlocklistThrowsExceptionOnFailure(): void - { - $moduleId = uniqid(); - $yamlFileLoaderMock = $this->createMock(YamlFileLoaderInfrastructureInterface::class); - $yamlFileLoaderMock - ->method('load') - ->will($this->throwException(new \Exception('Failed to load YAML file'))); - - $this->expectException(ModuleBlockListException::class); - - $sut = $this->getSut(yamlFileLoader: $yamlFileLoaderMock); - $sut->isModuleBlocked(moduleId: $moduleId); - } - private function getSut( - string $moduleBlockList = null, - YamlFileLoaderInfrastructureInterface $yamlFileLoader = null + string $moduleBlockListPath, + ProjectYamlDaoInterface $projectYamlDao ): ModuleBlocklistService { return new ModuleBlocklistService( - moduleBlocklist: $moduleBlockList - ?? 'testFilePath.yaml', - yamlFileLoader: $yamlFileLoader - ?? $this->createStub(YamlFileLoaderInfrastructureInterface::class) + moduleBlocklistPath: $moduleBlockListPath, + projectYamlDao: $projectYamlDao ); } } From f561105142d3c6f1a69a938e1ce05f2b7461d109 Mon Sep 17 00:00:00 2001 From: marcelmanzel Date: Wed, 14 Aug 2024 13:49:17 +0200 Subject: [PATCH 13/19] OXDEV-8489: Fix module configuration deletion in ModuleSettingBaseCest --- tests/Codeception/Acceptance/Module/ModuleSettingBaseCest.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/Codeception/Acceptance/Module/ModuleSettingBaseCest.php b/tests/Codeception/Acceptance/Module/ModuleSettingBaseCest.php index 2991ac6..cbb63cf 100644 --- a/tests/Codeception/Acceptance/Module/ModuleSettingBaseCest.php +++ b/tests/Codeception/Acceptance/Module/ModuleSettingBaseCest.php @@ -96,6 +96,8 @@ protected function removeConfiguration(string $moduleId): void { $shopConfiguration = $this->getShopConfiguration(); $shopConfiguration->deleteModuleConfiguration($moduleId); + $shopConfigurationDao = $this->getShopConfigurationDao(); + $shopConfigurationDao->save($shopConfiguration, 1); } protected function getShopConfigurationDao(): ShopConfigurationDao From 79bbeebc3089672a118a1be4d8bc6580ccbe8b6d Mon Sep 17 00:00:00 2001 From: marcelmanzel Date: Wed, 14 Aug 2024 14:13:27 +0200 Subject: [PATCH 14/19] OXDEV-8489: Add a few argument checks to tests and integration test --- .../Service/ModuleBlockListServiceTest.php | 33 +++++++++++++++++++ .../Service/ModuleActivationsServiceTest.php | 6 +++- 2 files changed, 38 insertions(+), 1 deletion(-) create mode 100644 tests/Integration/Service/ModuleBlockListServiceTest.php diff --git a/tests/Integration/Service/ModuleBlockListServiceTest.php b/tests/Integration/Service/ModuleBlockListServiceTest.php new file mode 100644 index 0000000..9027215 --- /dev/null +++ b/tests/Integration/Service/ModuleBlockListServiceTest.php @@ -0,0 +1,33 @@ +get(ProjectYamlDaoInterface::class) + ); + + $this->assertTrue($sut->isModuleBlocked('oe_graphql_base')); + $this->assertTrue($sut->isModuleBlocked('oe_graphql_configuration_access')); + } +} diff --git a/tests/Unit/Module/Service/ModuleActivationsServiceTest.php b/tests/Unit/Module/Service/ModuleActivationsServiceTest.php index 7626722..3dd7dde 100644 --- a/tests/Unit/Module/Service/ModuleActivationsServiceTest.php +++ b/tests/Unit/Module/Service/ModuleActivationsServiceTest.php @@ -63,11 +63,13 @@ public function testModuleActivationAndDeactivationExceptions( $moduleActivationBridgeMock = $this->createMock(ModuleActivationBridgeInterface::class); $moduleActivationBridgeMock ->method($method) + ->with($moduleId, 1) ->willThrowException(new \Exception()); $this->expectException($exceptionClass); $sut = $this->getSut( + $this->getContextMock(), moduleActivationBridge: $moduleActivationBridgeMock ); @@ -76,9 +78,11 @@ public function testModuleActivationAndDeactivationExceptions( public function testModuleDeactivationBlockedException() { + $moduleId = uniqid(); $moduleBlockListServiceMock = $this->createMock(ModuleBlocklistServiceInterface::class); $moduleBlockListServiceMock ->method('isModuleBlocked') + ->with($moduleId) ->willReturn(true); $sut = $this->getSut( @@ -86,7 +90,7 @@ public function testModuleDeactivationBlockedException() ); $this->expectException(ModuleDeactivationBlockedException::class); - $sut->deactivateModule(uniqid()); + $sut->deactivateModule($moduleId); } public static function activationDataProvider(): \Generator From 5bf46841dba770a6cb00ee02dab06277c550515e Mon Sep 17 00:00:00 2001 From: marcelmanzel Date: Wed, 14 Aug 2024 17:07:00 +0200 Subject: [PATCH 15/19] OXDEV-8489: Rename identifier to themeId for switchTheme-mutation --- src/Theme/Controller/ThemeSwitchController.php | 6 +++--- .../Infrastructure/ThemeSwitchInfrastructure.php | 4 ++-- .../ThemeSwitchInfrastructureInterface.php | 2 +- src/Theme/Service/ThemeSwitchService.php | 4 ++-- src/Theme/Service/ThemeSwitchServiceInterface.php | 2 +- .../Codeception/Acceptance/Theme/ThemeSwitchCest.php | 2 +- .../Theme/Controller/ThemeSwitchControllerTest.php | 10 +++++----- .../Infrastructure/ThemeSwitchInfrastructureTest.php | 12 ++++++------ tests/Unit/Theme/Service/ThemeSwitchServiceTest.php | 6 +++--- 9 files changed, 24 insertions(+), 24 deletions(-) diff --git a/src/Theme/Controller/ThemeSwitchController.php b/src/Theme/Controller/ThemeSwitchController.php index 8b6d9d4..d00a5f7 100644 --- a/src/Theme/Controller/ThemeSwitchController.php +++ b/src/Theme/Controller/ThemeSwitchController.php @@ -23,14 +23,14 @@ public function __construct( /** * Mutation of Configuration Access Module - * @param string $identifier + * @param string $themeId * @return bool */ #[Mutation] #[Logged] #[Right('CHANGE_CONFIGURATION')] - public function switchTheme(string $identifier): bool + public function switchTheme(string $themeId): bool { - return $this->themeSwitchService->switchTheme($identifier); + return $this->themeSwitchService->switchTheme($themeId); } } diff --git a/src/Theme/Infrastructure/ThemeSwitchInfrastructure.php b/src/Theme/Infrastructure/ThemeSwitchInfrastructure.php index 1386c00..87efcc4 100644 --- a/src/Theme/Infrastructure/ThemeSwitchInfrastructure.php +++ b/src/Theme/Infrastructure/ThemeSwitchInfrastructure.php @@ -21,11 +21,11 @@ public function __construct( ) { } - public function switchTheme(string $identifier): bool + public function switchTheme(string $themeId): bool { try { $coreThemeService = $this->coreThemeFactory->create(); - if (!$coreThemeService->load($identifier)) { + if (!$coreThemeService->load($themeId)) { throw new ThemeActivationException(self::THEME_NOT_EXIST); } $coreThemeService->activate(); diff --git a/src/Theme/Infrastructure/ThemeSwitchInfrastructureInterface.php b/src/Theme/Infrastructure/ThemeSwitchInfrastructureInterface.php index f27daa0..6f782ed 100644 --- a/src/Theme/Infrastructure/ThemeSwitchInfrastructureInterface.php +++ b/src/Theme/Infrastructure/ThemeSwitchInfrastructureInterface.php @@ -14,5 +14,5 @@ interface ThemeSwitchInfrastructureInterface /** *@throws ThemeActivationException */ - public function switchTheme(string $identifier): bool; + public function switchTheme(string $themeId): bool; } diff --git a/src/Theme/Service/ThemeSwitchService.php b/src/Theme/Service/ThemeSwitchService.php index da6a065..ba742b1 100644 --- a/src/Theme/Service/ThemeSwitchService.php +++ b/src/Theme/Service/ThemeSwitchService.php @@ -17,8 +17,8 @@ public function __construct( private readonly ThemeSwitchInfrastructureInterface $themeSwitchInfrastructure ) { } - public function switchTheme(string $identifier): bool + public function switchTheme(string $themeId): bool { - return $this->themeSwitchInfrastructure->switchTheme(identifier: $identifier); + return $this->themeSwitchInfrastructure->switchTheme(themeId: $themeId); } } diff --git a/src/Theme/Service/ThemeSwitchServiceInterface.php b/src/Theme/Service/ThemeSwitchServiceInterface.php index 041a7ca..3cfb462 100644 --- a/src/Theme/Service/ThemeSwitchServiceInterface.php +++ b/src/Theme/Service/ThemeSwitchServiceInterface.php @@ -9,5 +9,5 @@ interface ThemeSwitchServiceInterface { - public function switchTheme(string $identifier): bool; + public function switchTheme(string $themeId): bool; } diff --git a/tests/Codeception/Acceptance/Theme/ThemeSwitchCest.php b/tests/Codeception/Acceptance/Theme/ThemeSwitchCest.php index 89b3c10..b15f3d3 100644 --- a/tests/Codeception/Acceptance/Theme/ThemeSwitchCest.php +++ b/tests/Codeception/Acceptance/Theme/ThemeSwitchCest.php @@ -34,7 +34,7 @@ private function runThemeSwitchMutation(AcceptanceTester $I): array $I->sendGQLQuery( 'mutation switchThemeCest{ - switchTheme(identifier: "' . $themeId . '") + switchTheme(themeId: "' . $themeId . '") }' ); diff --git a/tests/Unit/Theme/Controller/ThemeSwitchControllerTest.php b/tests/Unit/Theme/Controller/ThemeSwitchControllerTest.php index eb6eb9f..9bc7aee 100644 --- a/tests/Unit/Theme/Controller/ThemeSwitchControllerTest.php +++ b/tests/Unit/Theme/Controller/ThemeSwitchControllerTest.php @@ -20,17 +20,17 @@ class ThemeSwitchControllerTest extends TestCase { /** @dataProvider switchThemeProvider */ public function testSwitchTheme( - string $identifier, + string $themeId, bool $expectedResult ): void { $themeSwitchServiceMock = $this->createMock(ThemeSwitchServiceInterface::class); $themeSwitchServiceMock ->method('switchTheme') - ->with($identifier) + ->with($themeId) ->willReturn($expectedResult); $themeSwitchController = new ThemeSwitchController($themeSwitchServiceMock); - $response = $themeSwitchController->switchTheme($identifier); + $response = $themeSwitchController->switchTheme($themeId); $this->assertSame($expectedResult, $response); } @@ -38,12 +38,12 @@ public function testSwitchTheme( public static function switchThemeProvider(): \Generator { yield 'test switch theme successful case' => [ - 'identifier' => 'validThemeId', + 'themeId' => 'validThemeId', 'expectedResult' => true ]; yield 'test switch theme failure case' => [ - 'identifier' => 'invalidThemeId', + 'themeId' => 'invalidThemeId', 'expectedResult' => false ]; } diff --git a/tests/Unit/Theme/Infrastructure/ThemeSwitchInfrastructureTest.php b/tests/Unit/Theme/Infrastructure/ThemeSwitchInfrastructureTest.php index 2714167..467fbf7 100644 --- a/tests/Unit/Theme/Infrastructure/ThemeSwitchInfrastructureTest.php +++ b/tests/Unit/Theme/Infrastructure/ThemeSwitchInfrastructureTest.php @@ -25,23 +25,23 @@ class ThemeSwitchInfrastructureTest extends TestCase private const THEME_NOT_EXIST = "The specified theme doesn't exist."; public function testSwitchTheme(): void { - $identifier = 'apex'; + $themeId = 'apex'; $coreThemeMock = $this->createMock(Theme::class); - $coreThemeMock->expects($this->once())->method('load')->with($identifier)->willReturn(true); + $coreThemeMock->expects($this->once())->method('load')->with($themeId)->willReturn(true); $coreThemeMock->expects($this->once())->method('activate'); $coreThemeFactoryMock = $this->getCoreThemeFactoryMock(coreThemeMock: $coreThemeMock); $sut = $this->getSut(coreThemeFactory: $coreThemeFactoryMock); - $serviceResponse = $sut->switchTheme($identifier); + $serviceResponse = $sut->switchTheme($themeId); $this->assertTrue($serviceResponse); } public function testThemeNotActivatedException(): void { - $identifier = 'apex'; + $themeId = 'apex'; $coreThemeMock = $this->createMock(Theme::class); - $coreThemeMock->expects($this->once())->method('load')->with($identifier)->willReturn(true); + $coreThemeMock->expects($this->once())->method('load')->with($themeId)->willReturn(true); $coreThemeMock->expects($this->once())->method('activate') ->will($this->throwException( new StandardException(self::THEME_NOT_ACTIVATED) @@ -52,7 +52,7 @@ public function testThemeNotActivatedException(): void $this->expectException(ThemeActivationException::class); $this->expectExceptionMessage(self::THEME_NOT_ACTIVATED); - $sut->switchTheme($identifier); + $sut->switchTheme($themeId); } public function testThemeNotFoundException(): void diff --git a/tests/Unit/Theme/Service/ThemeSwitchServiceTest.php b/tests/Unit/Theme/Service/ThemeSwitchServiceTest.php index 02fcab2..70cb8d0 100644 --- a/tests/Unit/Theme/Service/ThemeSwitchServiceTest.php +++ b/tests/Unit/Theme/Service/ThemeSwitchServiceTest.php @@ -22,15 +22,15 @@ class ThemeSwitchServiceTest extends TestCase public function testSwitchTheme( bool $expectedResult ): void { - $identifier = uniqid(); + $themeId = uniqid(); $themeSwitchInfrastructureMock = $this->createMock(ThemeSwitchInfrastructureInterface::class); $themeSwitchInfrastructureMock ->method('switchTheme') - ->with($identifier) + ->with($themeId) ->willReturn($expectedResult); $themeSwitchInfrastructure = new ThemeSwitchService($themeSwitchInfrastructureMock); - $response = $themeSwitchInfrastructure->switchTheme($identifier); + $response = $themeSwitchInfrastructure->switchTheme($themeId); $this->assertSame($expectedResult, $response); } From 26a8ab46d62e4755d2c40c11c974a982398aec3c Mon Sep 17 00:00:00 2001 From: marcelmanzel Date: Wed, 14 Aug 2024 17:09:13 +0200 Subject: [PATCH 16/19] OXDEV-8489: Add information about blocking modules from deactivation --- README.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/README.md b/README.md index fa08a17..857803e 100644 --- a/README.md +++ b/README.md @@ -64,6 +64,12 @@ $ vendor/bin/oe-console oe:module:activate oe_graphql_configuration_access A good starting point is to check the [How to use section in the GraphQL Base Module](https://github.com/OXID-eSales/graphql-base-module/#how-to-use) +## Blocking modules from deactivation via GraphQL + +The file module_blockilst.yaml contains a list of modules which are necessary to handle configurations or de/activate +modules via GraphQL or should be blocked for deactivation via GraphQL in general. Modules like ``oe_graphql_base`` and +``oe_graphql_configuration_access`` are listed there. + ## Testing ### Linting, syntax check, static analysis From 1056fcb75e82e04d3cd5abd19483a0aba015be36 Mon Sep 17 00:00:00 2001 From: RahatHameed Date: Thu, 15 Aug 2024 12:00:54 +0200 Subject: [PATCH 17/19] OXDEV-8489 Added module status check to block activation - Added logic to check if a module is in the blocklist before activation. - Throws ModuleActivationBlockedException if the module is blocked. - Mentioned this change in readme and chanelog files. --- CHANGELOG.md | 4 +-- README.md | 4 +-- .../ModuleActivationBlockedException.php | 22 +++++++++++++ .../Service/ModuleActivationService.php | 5 +++ .../Service/ModuleActivationsServiceTest.php | 33 +++++++++++++++---- 5 files changed, 58 insertions(+), 10 deletions(-) create mode 100644 src/Module/Exception/ModuleActivationBlockedException.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 36ce835..7b4e628 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,8 +10,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Theme list and filtering option on basis of theme name and status - Module list and filtering option on basis of module name and status - Activation of given theme by themeId -- Mutations to activate and deactive a module. -- Prevention of deactivation of certain modules mentioned in modules_blocklist.yaml. +- Mutations to de/activate a module. +- Prevention of de/activation of certain modules mentioned in modules_blocklist.yaml. ## [1.1.0] - 2024-07-05 This is stable release for v1.1.0. No changes have been made since v1.1.0-rc.1. diff --git a/README.md b/README.md index 857803e..0d4aa83 100644 --- a/README.md +++ b/README.md @@ -64,10 +64,10 @@ $ vendor/bin/oe-console oe:module:activate oe_graphql_configuration_access A good starting point is to check the [How to use section in the GraphQL Base Module](https://github.com/OXID-eSales/graphql-base-module/#how-to-use) -## Blocking modules from deactivation via GraphQL +## Blocking modules from de/activation via GraphQL The file module_blockilst.yaml contains a list of modules which are necessary to handle configurations or de/activate -modules via GraphQL or should be blocked for deactivation via GraphQL in general. Modules like ``oe_graphql_base`` and +modules via GraphQL or should be blocked for de/activation via GraphQL in general. Modules like ``oe_graphql_base`` and ``oe_graphql_configuration_access`` are listed there. ## Testing diff --git a/src/Module/Exception/ModuleActivationBlockedException.php b/src/Module/Exception/ModuleActivationBlockedException.php new file mode 100644 index 0000000..a2dc549 --- /dev/null +++ b/src/Module/Exception/ModuleActivationBlockedException.php @@ -0,0 +1,22 @@ +moduleBlocklistService->isModuleBlocked($moduleId)) { + throw new ModuleActivationBlockedException($moduleId); + } + $shopId = $this->context->getCurrentShopId(); try { diff --git a/tests/Unit/Module/Service/ModuleActivationsServiceTest.php b/tests/Unit/Module/Service/ModuleActivationsServiceTest.php index 3dd7dde..f5a8b14 100644 --- a/tests/Unit/Module/Service/ModuleActivationsServiceTest.php +++ b/tests/Unit/Module/Service/ModuleActivationsServiceTest.php @@ -12,6 +12,7 @@ use OxidEsales\EshopCommunity\Internal\Transition\Utility\ContextInterface; use OxidEsales\EshopCommunity\Internal\Framework\Module\Setup\Bridge\ModuleActivationBridgeInterface; use OxidEsales\GraphQL\ConfigurationAccess\Module\Exception\ModuleActivationException; +use OxidEsales\GraphQL\ConfigurationAccess\Module\Exception\ModuleActivationBlockedException; use OxidEsales\GraphQL\ConfigurationAccess\Module\Exception\ModuleDeactivationBlockedException; use OxidEsales\GraphQL\ConfigurationAccess\Module\Exception\ModuleDeactivationException; use OxidEsales\GraphQL\ConfigurationAccess\Module\Service\ModuleActivationService; @@ -26,7 +27,7 @@ class ModuleActivationsServiceTest extends UnitTestCase /** * @dataProvider activationDataProvider */ - public function testModuleActivationAndDeactivation( + public function testModuleActivationAndDeactivationSuccess( string $method, ): void { $shopId = 1; @@ -55,7 +56,7 @@ public function testModuleActivationAndDeactivation( /** * @dataProvider exceptionDataProvider */ - public function testModuleActivationAndDeactivationExceptions( + public function testModuleActivationAndDeactivationThrowsExceptions( string $method, mixed $exceptionClass ): void { @@ -76,8 +77,13 @@ public function testModuleActivationAndDeactivationExceptions( ($method === 'activate') ? $sut->activateModule($moduleId) : $sut->deactivateModule($moduleId); } - public function testModuleDeactivationBlockedException() - { + /** + * @dataProvider moduleBlockedExceptionDataProvider + */ + public function testModuleActivationAndDeactivationBlockedException( + string $method, + mixed $exceptionClass + ) { $moduleId = uniqid(); $moduleBlockListServiceMock = $this->createMock(ModuleBlocklistServiceInterface::class); $moduleBlockListServiceMock @@ -89,8 +95,23 @@ public function testModuleDeactivationBlockedException() moduleBlocklistService: $moduleBlockListServiceMock ); - $this->expectException(ModuleDeactivationBlockedException::class); - $sut->deactivateModule($moduleId); + $this->expectException($exceptionClass); + ($method === 'activate') ? $sut->activateModule($moduleId) : $sut->deactivateModule($moduleId); + } + + public static function moduleBlockedExceptionDataProvider(): \Generator + { + yield 'test activate module blocked exception' => [ + 'method' => 'activate', + 'exceptionClass' => ModuleActivationBlockedException::class + + ]; + + yield 'test deactivate module blocked exception' => [ + 'method' => 'deactivate', + 'exceptionClass' => ModuleDeactivationBlockedException::class + + ]; } public static function activationDataProvider(): \Generator From 450882f3880141808fb06816c3f267bf5ff769fd Mon Sep 17 00:00:00 2001 From: marcelmanzel Date: Thu, 15 Aug 2024 13:37:15 +0200 Subject: [PATCH 18/19] OXDEV-8489: Add ModuleActivationBlockedExceptionTest.php --- .../ModuleActivationBlockedExceptionTest.php | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 tests/Unit/Module/Exception/ModuleActivationBlockedExceptionTest.php diff --git a/tests/Unit/Module/Exception/ModuleActivationBlockedExceptionTest.php b/tests/Unit/Module/Exception/ModuleActivationBlockedExceptionTest.php new file mode 100644 index 0000000..2275215 --- /dev/null +++ b/tests/Unit/Module/Exception/ModuleActivationBlockedExceptionTest.php @@ -0,0 +1,31 @@ +assertInstanceOf(ModuleActivationBlockedException::class, $exception); + $this->assertSame( + sprintf('Module "%s" is in the blocklist and cannot be activated.', $moduleId), + $exception->getMessage() + ); + } +} From 8603b629bfc6a7366d01a5f5c4a1800050c821aa Mon Sep 17 00:00:00 2001 From: marcelmanzel Date: Thu, 15 Aug 2024 13:47:00 +0200 Subject: [PATCH 19/19] OXDEV-8489: Remove constructor of ThemeDataType --- src/Theme/DataType/ThemeDataType.php | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/Theme/DataType/ThemeDataType.php b/src/Theme/DataType/ThemeDataType.php index 77f37c6..07cda0f 100644 --- a/src/Theme/DataType/ThemeDataType.php +++ b/src/Theme/DataType/ThemeDataType.php @@ -15,13 +15,4 @@ #[Type] class ThemeDataType extends AbstractComponentDataType implements ThemeDataTypeInterface { - public function __construct( - string $id, - string $title, - string $version, - string $description, - bool $active - ) { - parent::__construct($id, $title, $version, $description, $active); - } }