From e548d941f4603a9fe8f11354b05b561040213e9d Mon Sep 17 00:00:00 2001 From: Johan Kromhout Date: Thu, 9 Jan 2025 11:17:51 +0100 Subject: [PATCH] Fix: Contractual base gets overwritten in manage Prior to this change, when saving a test entity, the contractual base field in manage would be set to null. This is because the contractual base field is used for the production entities. Other random coin fields will not get overwritten. This change makes sure the current value from Manage is reapplied, so sp-dasboard will not mutate the value. Fixes https://github.com/SURFnet/sp-dashboard/issues/1343 --- .../PublishEntityTestCommandHandler.php | 7 +- .../Domain/Service/ContractualBaseService.php | 16 +++++ .../PublishEntityTestCommandHandlerTest.php | 58 +++++++++++---- .../Service/ContractualBaseServiceTest.php | 71 +++++++++++++++++++ 4 files changed, 133 insertions(+), 19 deletions(-) diff --git a/src/Surfnet/ServiceProviderDashboard/Application/CommandHandler/Entity/PublishEntityTestCommandHandler.php b/src/Surfnet/ServiceProviderDashboard/Application/CommandHandler/Entity/PublishEntityTestCommandHandler.php index ebbe5c5c1..24cd77613 100644 --- a/src/Surfnet/ServiceProviderDashboard/Application/CommandHandler/Entity/PublishEntityTestCommandHandler.php +++ b/src/Surfnet/ServiceProviderDashboard/Application/CommandHandler/Entity/PublishEntityTestCommandHandler.php @@ -24,19 +24,18 @@ use Surfnet\ServiceProviderDashboard\Application\Exception\InvalidArgumentException; use Surfnet\ServiceProviderDashboard\Application\Service\EntityServiceInterface; use Surfnet\ServiceProviderDashboard\Domain\Entity\Constants; -use Surfnet\ServiceProviderDashboard\Domain\Entity\Contact; -use Surfnet\ServiceProviderDashboard\Domain\Entity\Entity\JiraTicketNumber; use Surfnet\ServiceProviderDashboard\Domain\Entity\ManageEntity; use Surfnet\ServiceProviderDashboard\Domain\Repository\PublishEntityRepository; +use Surfnet\ServiceProviderDashboard\Domain\Service\ContractualBaseService; use Surfnet\ServiceProviderDashboard\Infrastructure\HttpClient\Exceptions\RuntimeException\PublishMetadataException; use Symfony\Component\HttpFoundation\RequestStack; -use Symfony\Component\HttpFoundation\Session\Flash\FlashBagInterface; use function array_key_exists; class PublishEntityTestCommandHandler implements CommandHandler { public function __construct( private readonly PublishEntityRepository $publishClient, + private readonly ContractualBaseService $contractualBaseHelper, private readonly EntityServiceInterface $entityService, private readonly LoggerInterface $logger, private readonly RequestStack $requestStack, @@ -53,6 +52,7 @@ public function handle(PublishEntityTestCommand $command): void if ($entity->isManageEntity()) { // The entity as it is now known in Manage $pristineEntity = $this->entityService->getPristineManageEntityById($entity->getId(), $entity->getEnvironment()); + $this->contractualBaseHelper->writeContractualBaseForTestEntity($entity, $pristineEntity); } try { $this->logger->info( @@ -61,7 +61,6 @@ public function handle(PublishEntityTestCommand $command): void $entity->getMetaData()->getNameEn() ) ); - $publishResponse = $this->publishClient->publish( $entity, $pristineEntity, diff --git a/src/Surfnet/ServiceProviderDashboard/Domain/Service/ContractualBaseService.php b/src/Surfnet/ServiceProviderDashboard/Domain/Service/ContractualBaseService.php index 310f830aa..49413d95b 100644 --- a/src/Surfnet/ServiceProviderDashboard/Domain/Service/ContractualBaseService.php +++ b/src/Surfnet/ServiceProviderDashboard/Domain/Service/ContractualBaseService.php @@ -53,4 +53,20 @@ public function writeContractualBase(ManageEntity $entity): void // 4. Set the coin value on the entity $entity->getMetaData()?->getCoin()->setContractualBase($contractualBase); } + + public function writeContractualBaseForTestEntity(ManageEntity $entity, ?ManageEntity $originalEntity): void + { + if ($entity->getEnvironment() !== Constants::ENVIRONMENT_TEST) { + return; + } + + if ($originalEntity === null) { + return; + } + + $originalContractualBase = $originalEntity->getMetaData()?->getCoin()->getContractualBase(); + if ($originalContractualBase !== null) { + $entity->getMetaData()?->getCoin()->setContractualBase($originalContractualBase); + } + } } diff --git a/tests/integration/Application/CommandHandler/Entity/PublishEntityTestCommandHandlerTest.php b/tests/integration/Application/CommandHandler/Entity/PublishEntityTestCommandHandlerTest.php index 184e41acc..d07d8bdd7 100644 --- a/tests/integration/Application/CommandHandler/Entity/PublishEntityTestCommandHandlerTest.php +++ b/tests/integration/Application/CommandHandler/Entity/PublishEntityTestCommandHandlerTest.php @@ -20,8 +20,10 @@ use Surfnet\ServiceProviderDashboard\Application\Service\EntityServiceInterface; use Surfnet\ServiceProviderDashboard\Domain\Entity\Contact; +use Surfnet\ServiceProviderDashboard\Domain\Entity\Entity\Coin; +use Surfnet\ServiceProviderDashboard\Domain\Entity\Entity\MetaData; +use Surfnet\ServiceProviderDashboard\Domain\Service\ContractualBaseService; use Surfnet\ServiceProviderDashboard\Infrastructure\Manage\Client\PublishEntityClient; -use Surfnet\ServiceProviderDashboard\Infrastructure\Manage\Client\QueryClient; use Mockery as m; use Mockery\Adapter\Phpunit\MockeryTestCase; use Mockery\Mock; @@ -56,6 +58,7 @@ public function setUp(): void $this->commandHandler = new PublishEntityTestCommandHandler( $this->client, + new ContractualBaseService(), $this->entityService, $this->logger, $this->requestStack @@ -67,14 +70,25 @@ public function setUp(): void public function test_it_can_publish_to_manage() { $manageEntity = m::mock(ManageEntity::class); + $metaData = m::mock(MetaData::class); + $coin = m::mock(Coin::class); + $manageEntity - ->shouldReceive('getMetaData->getNameEn') + ->shouldReceive('getMetaData') + ->andReturn($metaData); + + $metaData + ->shouldReceive('getNameEn') ->andReturn('Test Entity Name') - ->shouldReceive('geMetaData->getManageId') - ->shouldReceive('getProtocol->geProtocol') - ->shouldReceive('setIdpAllowAll') - ->shouldReceive('setIdpWhitelistRaw') - ->andReturn(Constants::TYPE_OPENID_CONNECT_TNG); + ->shouldReceive('getManageId') + ->shouldReceive('getProtocol') + ->andReturn(Constants::TYPE_OPENID_CONNECT_TNG) + ->shouldReceive('getCoin') + ->andReturn($coin); + + $coin + ->shouldReceive('getContractualBase') + ->andReturn('some_contractual_base_value'); $this->logger ->shouldReceive('info') @@ -97,8 +111,7 @@ public function test_it_can_publish_to_manage() ->andReturn('uuid'); $manageEntity->shouldReceive('setId'); $manageEntity - ->shouldReceive('getEnvironment') - ->andReturn('test'); + ->shouldReceive('getEnvironment'); $this->entityService ->shouldReceive('getPristineManageEntityById') @@ -117,14 +130,29 @@ public function test_it_can_publish_to_manage() public function test_it_handles_failing_publish() { $manageEntity = m::mock(ManageEntity::class); + $metaData = m::mock(MetaData::class); + $coin = m::mock(Coin::class); + $manageEntity - ->shouldReceive('getMetaData->getNameEn') + ->shouldReceive('getMetaData') + ->andReturn($metaData); + + $metaData + ->shouldReceive('getNameEn') ->andReturn('Test Entity Name') - ->shouldReceive('geMetaData->getManageId') - ->shouldReceive('getProtocol->geProtocol') - ->shouldReceive('setIdpAllowAll') - ->shouldReceive('setIdpWhitelistRaw') - ->andReturn(Constants::TYPE_OPENID_CONNECT_TNG); + ->shouldReceive('getManageId') + ->shouldReceive('getProtocol') + ->andReturn(Constants::TYPE_OPENID_CONNECT_TNG) + ->shouldReceive('getCoin') + ->andReturn($coin); + + $coin + ->shouldReceive('getContractualBase') + ->andReturn('some_contractual_base_value'); + + $coin + ->shouldReceive('setContractualBase') + ->with('some_contractual_base_value'); $manageEntity ->shouldReceive('getAllowedIdentityProviders->getAllowedIdentityProviders') diff --git a/tests/unit/Domain/Service/ContractualBaseServiceTest.php b/tests/unit/Domain/Service/ContractualBaseServiceTest.php index edcc7aff2..6dca1ffff 100644 --- a/tests/unit/Domain/Service/ContractualBaseServiceTest.php +++ b/tests/unit/Domain/Service/ContractualBaseServiceTest.php @@ -55,6 +55,77 @@ public function testWriteContractualBase( $this->assertEquals($expectedContractualBase, $entity->getMetaData()->getCoin()->getContractualBase()); } + public function testContractualBaseForTest(): void + { + $entity = $this->createMockEntity( + Constants::ENVIRONMENT_TEST, + Constants::TYPE_SAML, + Constants::SERVICE_TYPE_INSTITUTE + ); + $this->assertNull($entity->getMetaData()->getCoin()->getContractualBase()); + + $pristine = $this->createMockEntity( + Constants::ENVIRONMENT_TEST, + Constants::TYPE_SAML, + Constants::SERVICE_TYPE_INSTITUTE, + ); + $pristine->getMetaData()->getCoin()->setContractualBase(Constants::CONTRACTUAL_BASE_IX); + + $this->service->writeContractualBaseForTestEntity($entity, $pristine); + + $this->assertSame(Constants::CONTRACTUAL_BASE_IX, $entity->getMetaData()->getCoin()->getContractualBase()); + } + + public function testWriteContractualBaseForTestEntityWithNonTestEnvironment(): void + { + $entity = $this->createMockEntity( + Constants::ENVIRONMENT_PRODUCTION, + Constants::TYPE_SAML, + Constants::SERVICE_TYPE_INSTITUTE + ); + $pristine = $this->createMockEntity( + Constants::ENVIRONMENT_TEST, + Constants::TYPE_SAML, + Constants::SERVICE_TYPE_INSTITUTE + ); + $pristine->getMetaData()->getCoin()->setContractualBase(Constants::CONTRACTUAL_BASE_IX); + + $this->service->writeContractualBaseForTestEntity($entity, $pristine); + + $this->assertNull($entity->getMetaData()->getCoin()->getContractualBase()); + } + + public function testWriteContractualBaseForTestEntityWithNullPristineEntity(): void + { + $entity = $this->createMockEntity( + Constants::ENVIRONMENT_TEST, + Constants::TYPE_SAML, + Constants::SERVICE_TYPE_INSTITUTE + ); + + $this->service->writeContractualBaseForTestEntity($entity, null); + + $this->assertNull($entity->getMetaData()->getCoin()->getContractualBase()); + } + + public function testWriteContractualBaseForTestEntityWithNullOriginalContractualBase(): void + { + $entity = $this->createMockEntity( + Constants::ENVIRONMENT_TEST, + Constants::TYPE_SAML, + Constants::SERVICE_TYPE_INSTITUTE + ); + $pristine = $this->createMockEntity( + Constants::ENVIRONMENT_TEST, + Constants::TYPE_SAML, + Constants::SERVICE_TYPE_INSTITUTE + ); + + $this->service->writeContractualBaseForTestEntity($entity, $pristine); + + $this->assertNull($entity->getMetaData()->getCoin()->getContractualBase()); + } + public function contractualBaseDataProvider(): array { return [