diff --git a/src/Webhook/Registration/WebhookSubscriber.php b/src/Webhook/Registration/WebhookSubscriber.php index 11e8c4797..44d2b25a7 100644 --- a/src/Webhook/Registration/WebhookSubscriber.php +++ b/src/Webhook/Registration/WebhookSubscriber.php @@ -62,33 +62,39 @@ public function removeSalesChannelWebhookConfiguration(EntityDeletedEvent $event /** * system-config should be written by {@see SettingsSaver} only, which checks the webhook on its own. - * Just in case new/changed credentials will be saved via the normal system config save route. + * Just in case new/changed credentials will be saved via the normal system config. */ public function checkWebhookBefore(BeforeSystemConfigMultipleChangedEvent $event): void { - $routeName = (string) $this->requestStack->getMainRequest()?->attributes->getString('_route'); - - if (!\str_contains($routeName, 'api.action.core.save.system-config')) { - return; + if ($config = $this->getConfigToCheck($event)) { + $this->webhookSystemConfigHelper->checkWebhookBefore([$event->getSalesChannelId() => $config]); } - - /** @var array> $config */ - $config = $event->getConfig(); - $this->webhookSystemConfigHelper->checkWebhookBefore($config); } /** * system-config should be written by {@see SettingsSaver} only, which checks the webhook on its own. - * Just in case new/changed credentials will be saved via the normal system config save route. + * Just in case new/changed credentials will be saved via the normal system config. */ public function checkWebhookAfter(SystemConfigMultipleChangedEvent $event): void { + if ($this->getConfigToCheck($event)) { + $this->webhookSystemConfigHelper->checkWebhookAfter([$event->getSalesChannelId()]); + } + } + + /** + * @return array|null + */ + private function getConfigToCheck(BeforeSystemConfigMultipleChangedEvent|SystemConfigMultipleChangedEvent $event): ?array + { + /** @var array $config */ + $config = $event->getConfig(); $routeName = (string) $this->requestStack->getMainRequest()?->attributes->getString('_route'); - if (!\str_contains($routeName, 'api.action.core.save.system-config')) { - return; + if (\str_contains($routeName, 'api.action.paypal.settings.save') || !$this->webhookSystemConfigHelper->needsCheck($config)) { + return null; } - $this->webhookSystemConfigHelper->checkWebhookAfter(\array_keys($event->getConfig())); + return $config; } } diff --git a/src/Webhook/Registration/WebhookSystemConfigHelper.php b/src/Webhook/Registration/WebhookSystemConfigHelper.php index 7f9ccaac8..71e6f52f6 100644 --- a/src/Webhook/Registration/WebhookSystemConfigHelper.php +++ b/src/Webhook/Registration/WebhookSystemConfigHelper.php @@ -59,7 +59,7 @@ public function checkWebhookBefore(array $newData): array $errors = []; foreach ($newData as $salesChannelId => $newSettings) { - if ($salesChannelId === 'null') { + if (!$salesChannelId || $salesChannelId === 'null') { $salesChannelId = null; } @@ -102,7 +102,7 @@ public function checkWebhookAfter(array $salesChannelIds): array $errors = []; foreach ($salesChannelIds as $salesChannelId) { - if ($salesChannelId === 'null') { + if (!$salesChannelId || $salesChannelId === 'null') { $salesChannelId = null; } @@ -129,6 +129,14 @@ public function checkWebhookAfter(array $salesChannelIds): array return $errors; } + /** + * @param array $config + */ + public function needsCheck(array $config): bool + { + return !empty($this->filterSettings($config)); + } + private function fetchSettings(?string $salesChannelId, bool $inherit = false): array { $settings = []; diff --git a/tests/Webhook/Registration/WebhookSubscriberTest.php b/tests/Webhook/Registration/WebhookSubscriberTest.php index 185504d23..eee9f6c58 100644 --- a/tests/Webhook/Registration/WebhookSubscriberTest.php +++ b/tests/Webhook/Registration/WebhookSubscriberTest.php @@ -7,6 +7,7 @@ namespace Swag\PayPal\Test\Webhook\Registration; +use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Psr\Log\NullLogger; use Shopware\Core\Framework\Context; @@ -29,6 +30,7 @@ use Swag\PayPal\Webhook\Registration\WebhookSystemConfigHelper; use Swag\PayPal\Webhook\WebhookRegistry; use Swag\PayPal\Webhook\WebhookService; +use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\RequestStack; use Symfony\Component\Routing\RouterInterface; @@ -44,9 +46,15 @@ class WebhookSubscriberTest extends TestCase private SystemConfigService $systemConfigService; + private WebhookSystemConfigHelper&MockObject $webhookSystemConfigHelper; + + private RequestStack $requestStack; + protected function setUp(): void { $this->systemConfigService = SystemConfigServiceMock::createWithCredentials(); + $this->webhookSystemConfigHelper = $this->createMock(WebhookSystemConfigHelper::class); + $this->requestStack = new RequestStack(); } public function testRemoveWebhookWithInheritedConfiguration(): void @@ -73,6 +81,156 @@ public function testRemoveWebhookWithNoConfiguration(): void static::assertEmpty($this->systemConfigService->getString(Settings::WEBHOOK_ID, TestDefaults::SALES_CHANNEL)); } + public function testCheckWebhookBefore(): void + { + $event = new BeforeSystemConfigMultipleChangedEvent(['some-key' => 'some-value'], null); + + $this->webhookSystemConfigHelper + ->expects(static::once()) + ->method('checkWebhookBefore') + ->with(['' => ['some-key' => 'some-value']]); + + $this->webhookSystemConfigHelper + ->expects(static::once()) + ->method('needsCheck') + ->with(['some-key' => 'some-value']) + ->willReturn(true); + + $this->createWebhookSubscriber(['' => null, TestDefaults::SALES_CHANNEL => null]) + ->checkWebhookBefore($event); + } + + public function testCheckWebhookBeforeWithSalesChannelId(): void + { + $event = new BeforeSystemConfigMultipleChangedEvent(['some-key' => 'some-value'], 'some-sales-channel-id'); + + $this->webhookSystemConfigHelper + ->expects(static::once()) + ->method('checkWebhookBefore') + ->with(['some-sales-channel-id' => ['some-key' => 'some-value']]); + + $this->webhookSystemConfigHelper + ->expects(static::once()) + ->method('needsCheck') + ->with(['some-key' => 'some-value']) + ->willReturn(true); + + $this->createWebhookSubscriber(['' => null, TestDefaults::SALES_CHANNEL => null]) + ->checkWebhookBefore($event); + } + + public function testCheckWebhookBeforeWithSaveRoute(): void + { + $request = new Request(attributes: ['_route' => 'api.action.paypal.settings.save']); + $this->requestStack->push($request); + + $event = new BeforeSystemConfigMultipleChangedEvent(['some-key' => 'some-value'], 'some-sales-channel-id'); + + $this->webhookSystemConfigHelper + ->expects(static::never()) + ->method('checkWebhookBefore'); + + $this->webhookSystemConfigHelper + ->expects(static::never()) + ->method('needsCheck'); + + $this->createWebhookSubscriber(['' => null, TestDefaults::SALES_CHANNEL => null]) + ->checkWebhookBefore($event); + } + + public function testCheckWebhookBeforeWithNoCheckNeeded(): void + { + $event = new BeforeSystemConfigMultipleChangedEvent(['some-key' => 'some-value'], 'some-sales-channel-id'); + + $this->webhookSystemConfigHelper + ->expects(static::never()) + ->method('checkWebhookBefore'); + + $this->webhookSystemConfigHelper + ->expects(static::once()) + ->method('needsCheck') + ->with(['some-key' => 'some-value']) + ->willReturn(false); + + $this->createWebhookSubscriber(['' => null, TestDefaults::SALES_CHANNEL => null]) + ->checkWebhookBefore($event); + } + + public function testCheckWebhookAfter(): void + { + $event = new SystemConfigMultipleChangedEvent(['some-key' => 'some-value'], null); + + $this->webhookSystemConfigHelper + ->expects(static::once()) + ->method('checkWebhookAfter') + ->with(['']); + + $this->webhookSystemConfigHelper + ->expects(static::once()) + ->method('needsCheck') + ->with(['some-key' => 'some-value']) + ->willReturn(true); + + $this->createWebhookSubscriber(['' => null, TestDefaults::SALES_CHANNEL => null]) + ->checkWebhookAfter($event); + } + + public function testCheckWebhookAfterWithSalesChannelId(): void + { + $event = new SystemConfigMultipleChangedEvent(['some-key' => 'some-value'], 'some-sales-channel-id'); + + $this->webhookSystemConfigHelper + ->expects(static::once()) + ->method('checkWebhookAfter') + ->with(['some-sales-channel-id']); + + $this->webhookSystemConfigHelper + ->expects(static::once()) + ->method('needsCheck') + ->with(['some-key' => 'some-value']) + ->willReturn(true); + + $this->createWebhookSubscriber(['' => null, TestDefaults::SALES_CHANNEL => null]) + ->checkWebhookAfter($event); + } + + public function testCheckWebhookAfterWithSaveRoute(): void + { + $request = new Request(attributes: ['_route' => 'api.action.paypal.settings.save']); + $this->requestStack->push($request); + + $event = new SystemConfigMultipleChangedEvent(['some-key' => 'some-value'], 'some-sales-channel-id'); + + $this->webhookSystemConfigHelper + ->expects(static::never()) + ->method('checkWebhookAfter'); + + $this->webhookSystemConfigHelper + ->expects(static::never()) + ->method('needsCheck'); + + $this->createWebhookSubscriber(['' => null, TestDefaults::SALES_CHANNEL => null]) + ->checkWebhookAfter($event); + } + + public function testCheckWebhookAfterWithNoCheckNeeded(): void + { + $event = new SystemConfigMultipleChangedEvent(['some-key' => 'some-value'], 'some-sales-channel-id'); + + $this->webhookSystemConfigHelper + ->expects(static::never()) + ->method('checkWebhookAfter'); + + $this->webhookSystemConfigHelper + ->expects(static::once()) + ->method('needsCheck') + ->with(['some-key' => 'some-value']) + ->willReturn(false); + + $this->createWebhookSubscriber(['' => null, TestDefaults::SALES_CHANNEL => null]) + ->checkWebhookAfter($event); + } + public function testSubscribedEvents(): void { static::assertEqualsCanonicalizing([ @@ -105,8 +263,8 @@ private function createWebhookSubscriber(array $configuration): WebhookSubscribe new NullLogger(), $this->systemConfigService, $webhookService, - $this->createMock(WebhookSystemConfigHelper::class), - new RequestStack(), + $this->webhookSystemConfigHelper, + $this->requestStack, ); } diff --git a/tests/Webhook/Registration/WebhookSystemConfigHelperTest.php b/tests/Webhook/Registration/WebhookSystemConfigHelperTest.php new file mode 100644 index 000000000..434e18ef0 --- /dev/null +++ b/tests/Webhook/Registration/WebhookSystemConfigHelperTest.php @@ -0,0 +1,83 @@ + + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Swag\PayPal\Test\Webhook\Registration; + +use PHPUnit\Framework\Attributes\CoversClass; +use PHPUnit\Framework\Attributes\DataProvider; +use PHPUnit\Framework\TestCase; +use Psr\Log\NullLogger; +use Shopware\Core\Framework\Log\Package; +use Shopware\Core\System\SystemConfig\SystemConfigService; +use Swag\PayPal\Setting\Service\SettingsValidationServiceInterface; +use Swag\PayPal\Setting\Settings; +use Swag\PayPal\Webhook\Registration\WebhookSystemConfigHelper; +use Swag\PayPal\Webhook\WebhookServiceInterface; + +/** + * @internal + */ +#[Package('checkout')] +#[CoversClass(WebhookSystemConfigHelper::class)] +class WebhookSystemConfigHelperTest extends TestCase +{ + private const WEBHOOK_KEYS = [ + Settings::CLIENT_ID, + Settings::CLIENT_SECRET, + Settings::CLIENT_ID_SANDBOX, + Settings::CLIENT_SECRET_SANDBOX, + Settings::SANDBOX, + Settings::WEBHOOK_ID, + ]; + + private WebhookSystemConfigHelper $helper; + + protected function setUp(): void + { + $this->helper = new WebhookSystemConfigHelper( + new NullLogger(), + $this->createMock(WebhookServiceInterface::class), + $this->createMock(SystemConfigService::class), + $this->createMock(SettingsValidationServiceInterface::class), + ); + } + + /** + * @param array $config + */ + #[DataProvider('providerNeedsCheck')] + public function testNeedsCheck(bool $expected, array $config): void + { + static::assertSame($expected, $this->helper->needsCheck($config)); + } + + public static function providerNeedsCheck(): \Generator + { + foreach (self::WEBHOOK_KEYS as $key) { + yield \sprintf('needs check for "%s"', $key) => [ + true, + [$key => 'some-value'], + ]; + + yield \sprintf('needs check for "%s" with null value', $key) => [ + true, + [$key => null], + ]; + } + + foreach (Settings::DEFAULT_VALUES as $key => $value) { + if (\in_array($key, self::WEBHOOK_KEYS, true)) { + continue; + } + + yield \sprintf('no check for "%s"', $key) => [ + false, + [$key => $value], + ]; + } + } +}