Skip to content

Commit aa3e8c1

Browse files
cyl3xcyl3x
authored
[6.6.x] fix: system config webhook check (#176)
1 parent c5f10e8 commit aa3e8c1

File tree

4 files changed

+294
-18
lines changed

4 files changed

+294
-18
lines changed

Diff for: ‎src/Webhook/Registration/WebhookSubscriber.php

+24-14
Original file line numberDiff line numberDiff line change
@@ -63,37 +63,47 @@ public function removeSalesChannelWebhookConfiguration(EntityDeletedEvent $event
6363

6464
/**
6565
* system-config should be written by {@see SettingsSaver} only, which checks the webhook on its own.
66-
* Just in case new/changed credentials will be saved via the normal system config save route.
66+
* Just in case new/changed credentials will be saved via the normal system config.
6767
*/
6868
public function checkWebhookBefore(BeforeSystemConfigMultipleChangedEvent $event): void
6969
{
70-
$routeName = (string) $this->requestStack->getMainRequest()?->attributes->getString('_route');
71-
72-
if (!\str_contains($routeName, 'api.action.core.save.system-config')) {
70+
if (!Feature::isActive('PAYPAL_SETTINGS_TWEAKS')) {
7371
return;
7472
}
7573

76-
if (Feature::isActive('PAYPAL_SETTINGS_TWEAKS')) {
77-
/** @var array<string, array<string, mixed>> $config */
78-
$config = $event->getConfig();
79-
$this->webhookSystemConfigHelper->checkWebhookBefore($config);
74+
if ($config = $this->getConfigToCheck($event)) {
75+
$this->webhookSystemConfigHelper->checkWebhookBefore([$event->getSalesChannelId() => $config]);
8076
}
8177
}
8278

8379
/**
8480
* system-config should be written by {@see SettingsSaver} only, which checks the webhook on its own.
85-
* Just in case new/changed credentials will be saved via the normal system config save route.
81+
* Just in case new/changed credentials will be saved via the normal system config.
8682
*/
8783
public function checkWebhookAfter(SystemConfigMultipleChangedEvent $event): void
8884
{
89-
$routeName = (string) $this->requestStack->getMainRequest()?->attributes->getString('_route');
90-
91-
if (!\str_contains($routeName, 'api.action.core.save.system-config')) {
85+
if (!Feature::isActive('PAYPAL_SETTINGS_TWEAKS')) {
9286
return;
9387
}
9488

95-
if (Feature::isActive('PAYPAL_SETTINGS_TWEAKS')) {
96-
$this->webhookSystemConfigHelper->checkWebhookAfter(\array_keys($event->getConfig()));
89+
if ($this->getConfigToCheck($event)) {
90+
$this->webhookSystemConfigHelper->checkWebhookAfter([$event->getSalesChannelId()]);
9791
}
9892
}
93+
94+
/**
95+
* @return array<string, mixed>|null
96+
*/
97+
private function getConfigToCheck(BeforeSystemConfigMultipleChangedEvent|SystemConfigMultipleChangedEvent $event): ?array
98+
{
99+
/** @var array<string, mixed> $config */
100+
$config = $event->getConfig();
101+
$routeName = (string) $this->requestStack->getMainRequest()?->attributes->getString('_route');
102+
103+
if (\str_contains($routeName, 'api.action.paypal.settings.save') || !$this->webhookSystemConfigHelper->needsCheck($config)) {
104+
return null;
105+
}
106+
107+
return $config;
108+
}
99109
}

Diff for: ‎src/Webhook/Registration/WebhookSystemConfigHelper.php

+10-2
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public function checkWebhookBefore(array $newData): array
5959
$errors = [];
6060

6161
foreach ($newData as $salesChannelId => $newSettings) {
62-
if ($salesChannelId === 'null') {
62+
if (!$salesChannelId || $salesChannelId === 'null') {
6363
$salesChannelId = null;
6464
}
6565

@@ -102,7 +102,7 @@ public function checkWebhookAfter(array $salesChannelIds): array
102102
$errors = [];
103103

104104
foreach ($salesChannelIds as $salesChannelId) {
105-
if ($salesChannelId === 'null') {
105+
if (!$salesChannelId || $salesChannelId === 'null') {
106106
$salesChannelId = null;
107107
}
108108

@@ -129,6 +129,14 @@ public function checkWebhookAfter(array $salesChannelIds): array
129129
return $errors;
130130
}
131131

132+
/**
133+
* @param array<string, mixed> $config
134+
*/
135+
public function needsCheck(array $config): bool
136+
{
137+
return !empty($this->filterSettings($config));
138+
}
139+
132140
private function fetchSettings(?string $salesChannelId, bool $inherit = false): array
133141
{
134142
$settings = [];

Diff for: ‎tests/Webhook/Registration/WebhookSubscriberTest.php

+177-2
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,14 @@
77

88
namespace Swag\PayPal\Test\Webhook\Registration;
99

10+
use PHPUnit\Framework\MockObject\MockObject;
1011
use PHPUnit\Framework\TestCase;
1112
use Psr\Log\NullLogger;
1213
use Shopware\Core\Framework\Context;
1314
use Shopware\Core\Framework\DataAbstractionLayer\EntityWriteResult;
1415
use Shopware\Core\Framework\DataAbstractionLayer\Event\EntityDeletedEvent;
1516
use Shopware\Core\Framework\DataAbstractionLayer\Write\EntityExistence;
17+
use Shopware\Core\Framework\Feature;
1618
use Shopware\Core\Framework\Log\Package;
1719
use Shopware\Core\System\SalesChannel\SalesChannelDefinition;
1820
use Shopware\Core\System\SalesChannel\SalesChannelEvents;
@@ -29,6 +31,7 @@
2931
use Swag\PayPal\Webhook\Registration\WebhookSystemConfigHelper;
3032
use Swag\PayPal\Webhook\WebhookRegistry;
3133
use Swag\PayPal\Webhook\WebhookService;
34+
use Symfony\Component\HttpFoundation\Request;
3235
use Symfony\Component\HttpFoundation\RequestStack;
3336
use Symfony\Component\Routing\RouterInterface;
3437

@@ -44,9 +47,15 @@ class WebhookSubscriberTest extends TestCase
4447

4548
private SystemConfigService $systemConfigService;
4649

50+
private WebhookSystemConfigHelper&MockObject $webhookSystemConfigHelper;
51+
52+
private RequestStack $requestStack;
53+
4754
protected function setUp(): void
4855
{
4956
$this->systemConfigService = SystemConfigServiceMock::createWithCredentials();
57+
$this->webhookSystemConfigHelper = $this->createMock(WebhookSystemConfigHelper::class);
58+
$this->requestStack = new RequestStack();
5059
}
5160

5261
public function testRemoveWebhookWithInheritedConfiguration(): void
@@ -73,6 +82,172 @@ public function testRemoveWebhookWithNoConfiguration(): void
7382
static::assertEmpty($this->systemConfigService->getString(Settings::WEBHOOK_ID, TestDefaults::SALES_CHANNEL));
7483
}
7584

85+
public function testCheckWebhookBefore(): void
86+
{
87+
Feature::skipTestIfInActive('PAYPAL_SETTINGS_TWEAKS', $this);
88+
89+
$event = new BeforeSystemConfigMultipleChangedEvent(['some-key' => 'some-value'], null);
90+
91+
$this->webhookSystemConfigHelper
92+
->expects(static::once())
93+
->method('checkWebhookBefore')
94+
->with(['' => ['some-key' => 'some-value']]);
95+
96+
$this->webhookSystemConfigHelper
97+
->expects(static::once())
98+
->method('needsCheck')
99+
->with(['some-key' => 'some-value'])
100+
->willReturn(true);
101+
102+
$this->createWebhookSubscriber(['' => null, TestDefaults::SALES_CHANNEL => null])
103+
->checkWebhookBefore($event);
104+
}
105+
106+
public function testCheckWebhookBeforeWithSalesChannelId(): void
107+
{
108+
Feature::skipTestIfInActive('PAYPAL_SETTINGS_TWEAKS', $this);
109+
110+
$event = new BeforeSystemConfigMultipleChangedEvent(['some-key' => 'some-value'], 'some-sales-channel-id');
111+
112+
$this->webhookSystemConfigHelper
113+
->expects(static::once())
114+
->method('checkWebhookBefore')
115+
->with(['some-sales-channel-id' => ['some-key' => 'some-value']]);
116+
117+
$this->webhookSystemConfigHelper
118+
->expects(static::once())
119+
->method('needsCheck')
120+
->with(['some-key' => 'some-value'])
121+
->willReturn(true);
122+
123+
$this->createWebhookSubscriber(['' => null, TestDefaults::SALES_CHANNEL => null])
124+
->checkWebhookBefore($event);
125+
}
126+
127+
public function testCheckWebhookBeforeWithSaveRoute(): void
128+
{
129+
Feature::skipTestIfInActive('PAYPAL_SETTINGS_TWEAKS', $this);
130+
131+
$request = new Request(attributes: ['_route' => 'api.action.paypal.settings.save']);
132+
$this->requestStack->push($request);
133+
134+
$event = new BeforeSystemConfigMultipleChangedEvent(['some-key' => 'some-value'], 'some-sales-channel-id');
135+
136+
$this->webhookSystemConfigHelper
137+
->expects(static::never())
138+
->method('checkWebhookBefore');
139+
140+
$this->webhookSystemConfigHelper
141+
->expects(static::never())
142+
->method('needsCheck');
143+
144+
$this->createWebhookSubscriber(['' => null, TestDefaults::SALES_CHANNEL => null])
145+
->checkWebhookBefore($event);
146+
}
147+
148+
public function testCheckWebhookBeforeWithNoCheckNeeded(): void
149+
{
150+
Feature::skipTestIfInActive('PAYPAL_SETTINGS_TWEAKS', $this);
151+
152+
$event = new BeforeSystemConfigMultipleChangedEvent(['some-key' => 'some-value'], 'some-sales-channel-id');
153+
154+
$this->webhookSystemConfigHelper
155+
->expects(static::never())
156+
->method('checkWebhookBefore');
157+
158+
$this->webhookSystemConfigHelper
159+
->expects(static::once())
160+
->method('needsCheck')
161+
->with(['some-key' => 'some-value'])
162+
->willReturn(false);
163+
164+
$this->createWebhookSubscriber(['' => null, TestDefaults::SALES_CHANNEL => null])
165+
->checkWebhookBefore($event);
166+
}
167+
168+
public function testCheckWebhookAfter(): void
169+
{
170+
Feature::skipTestIfInActive('PAYPAL_SETTINGS_TWEAKS', $this);
171+
172+
$event = new SystemConfigMultipleChangedEvent(['some-key' => 'some-value'], null);
173+
174+
$this->webhookSystemConfigHelper
175+
->expects(static::once())
176+
->method('checkWebhookAfter')
177+
->with(['']);
178+
179+
$this->webhookSystemConfigHelper
180+
->expects(static::once())
181+
->method('needsCheck')
182+
->with(['some-key' => 'some-value'])
183+
->willReturn(true);
184+
185+
$this->createWebhookSubscriber(['' => null, TestDefaults::SALES_CHANNEL => null])
186+
->checkWebhookAfter($event);
187+
}
188+
189+
public function testCheckWebhookAfterWithSalesChannelId(): void
190+
{
191+
Feature::skipTestIfInActive('PAYPAL_SETTINGS_TWEAKS', $this);
192+
193+
$event = new SystemConfigMultipleChangedEvent(['some-key' => 'some-value'], 'some-sales-channel-id');
194+
195+
$this->webhookSystemConfigHelper
196+
->expects(static::once())
197+
->method('checkWebhookAfter')
198+
->with(['some-sales-channel-id']);
199+
200+
$this->webhookSystemConfigHelper
201+
->expects(static::once())
202+
->method('needsCheck')
203+
->with(['some-key' => 'some-value'])
204+
->willReturn(true);
205+
206+
$this->createWebhookSubscriber(['' => null, TestDefaults::SALES_CHANNEL => null])
207+
->checkWebhookAfter($event);
208+
}
209+
210+
public function testCheckWebhookAfterWithSaveRoute(): void
211+
{
212+
Feature::skipTestIfInActive('PAYPAL_SETTINGS_TWEAKS', $this);
213+
214+
$request = new Request(attributes: ['_route' => 'api.action.paypal.settings.save']);
215+
$this->requestStack->push($request);
216+
217+
$event = new SystemConfigMultipleChangedEvent(['some-key' => 'some-value'], 'some-sales-channel-id');
218+
219+
$this->webhookSystemConfigHelper
220+
->expects(static::never())
221+
->method('checkWebhookAfter');
222+
223+
$this->webhookSystemConfigHelper
224+
->expects(static::never())
225+
->method('needsCheck');
226+
227+
$this->createWebhookSubscriber(['' => null, TestDefaults::SALES_CHANNEL => null])
228+
->checkWebhookAfter($event);
229+
}
230+
231+
public function testCheckWebhookAfterWithNoCheckNeeded(): void
232+
{
233+
Feature::skipTestIfInActive('PAYPAL_SETTINGS_TWEAKS', $this);
234+
235+
$event = new SystemConfigMultipleChangedEvent(['some-key' => 'some-value'], 'some-sales-channel-id');
236+
237+
$this->webhookSystemConfigHelper
238+
->expects(static::never())
239+
->method('checkWebhookAfter');
240+
241+
$this->webhookSystemConfigHelper
242+
->expects(static::once())
243+
->method('needsCheck')
244+
->with(['some-key' => 'some-value'])
245+
->willReturn(false);
246+
247+
$this->createWebhookSubscriber(['' => null, TestDefaults::SALES_CHANNEL => null])
248+
->checkWebhookAfter($event);
249+
}
250+
76251
public function testSubscribedEvents(): void
77252
{
78253
static::assertEqualsCanonicalizing([
@@ -105,8 +280,8 @@ private function createWebhookSubscriber(array $configuration): WebhookSubscribe
105280
new NullLogger(),
106281
$this->systemConfigService,
107282
$webhookService,
108-
$this->createMock(WebhookSystemConfigHelper::class),
109-
new RequestStack(),
283+
$this->webhookSystemConfigHelper,
284+
$this->requestStack,
110285
);
111286
}
112287

0 commit comments

Comments
 (0)