Skip to content

Commit 265c54c

Browse files
committed
ACP-3507 Applied CR fixes.
1 parent 3021698 commit 265c54c

File tree

13 files changed

+176
-2
lines changed

13 files changed

+176
-2
lines changed

src/Spryker/Shared/AppWebhook/Transfer/app_webhook.transfer.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
<property name="mode" type="string"/>
77
<property name="identifier" type="string" description="This is the default identifier set for internal processing."/>
88
<property name="isRetry" type="bool" description="When this WebhookRequest was tried to be processed before we set this to not persist it again in the database."/>
9+
<property name="retries" type="int" description="The number of how often this webhook was retried."/>
910
</transfer>
1011

1112
<transfer name="WebhookResponse" strict="true">
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
<?php
2+
3+
/**
4+
* Copyright © 2016-present Spryker Systems GmbH. All rights reserved.
5+
* Use of this software requires acceptance of the Evaluation License Agreement. See LICENSE file.
6+
*/
7+
8+
namespace Spryker\Zed\AppWebhook;
9+
10+
use Spryker\Zed\Kernel\AbstractBundleConfig;
11+
12+
class AppWebhookConfig extends AbstractBundleConfig
13+
{
14+
/**
15+
* @api
16+
*
17+
* @var int
18+
*/
19+
protected const NUMBER_OF_ALLOWED_RETRIES = 9;
20+
21+
public function getAllowedNumberOfWebhookRetries(): int
22+
{
23+
return static::NUMBER_OF_ALLOWED_RETRIES;
24+
}
25+
}

src/Spryker/Zed/AppWebhook/AppWebhookDependencyProvider.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@
1010
use Spryker\Zed\Kernel\AbstractBundleDependencyProvider;
1111
use Spryker\Zed\Kernel\Container;
1212

13+
/**
14+
* @method \Spryker\Zed\AppWebhook\AppWebhookConfig getConfig()
15+
*/
1316
class AppWebhookDependencyProvider extends AbstractBundleDependencyProvider
1417
{
1518
/**

src/Spryker/Zed/AppWebhook/Business/AppWebhookBusinessFactory.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,15 @@
1717
/**
1818
* @method \Spryker\Zed\AppWebhook\Persistence\AppWebhookEntityManagerInterface getEntityManager()
1919
* @method \Spryker\Zed\AppWebhook\Persistence\AppWebhookRepositoryInterface getRepository()
20+
* @method \Spryker\Zed\AppWebhook\AppWebhookConfig getConfig()
2021
*/
2122
class AppWebhookBusinessFactory extends AbstractBusinessFactory
2223
{
2324
public function createWebhookHandler(): WebhookHandler
2425
{
2526
return new WebhookHandler(
2627
$this->getAppWebhookHandlerPlugins(),
28+
$this->getConfig(),
2729
$this->getEntityManager(),
2830
$this->createIdentifierBuilder(),
2931
);
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<?php
2+
3+
/**
4+
* Copyright © 2016-present Spryker Systems GmbH. All rights reserved.
5+
* Use of this software requires acceptance of the Evaluation License Agreement. See LICENSE file.
6+
*/
7+
8+
namespace Spryker\Zed\AppWebhook\Business\Exception;
9+
10+
class AllowedNumberOfRetriesExceededException extends AppWebhookException
11+
{
12+
}
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<?php
2+
3+
/**
4+
* Copyright © 2016-present Spryker Systems GmbH. All rights reserved.
5+
* Use of this software requires acceptance of the Evaluation License Agreement. See LICENSE file.
6+
*/
7+
8+
namespace Spryker\Zed\AppWebhook\Business\Exception;
9+
10+
use Exception;
11+
12+
class AppWebhookException extends Exception
13+
{
14+
}
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
<?php
2+
3+
/**
4+
* Copyright © 2016-present Spryker Systems GmbH. All rights reserved.
5+
* Use of this software requires acceptance of the Evaluation License Agreement. See LICENSE file.
6+
*/
7+
8+
namespace Spryker\Zed\AppWebhook\Business\MessageBuilder;
9+
10+
class MessageBuilder
11+
{
12+
public static function allowedNumberOfRetriesExceeded(): string
13+
{
14+
return 'Allowed number of retries exceeded.';
15+
}
16+
}

src/Spryker/Zed/AppWebhook/Business/WebhookHandler/WebhookHandler.php

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,10 @@
99

1010
use Generated\Shared\Transfer\WebhookRequestTransfer;
1111
use Generated\Shared\Transfer\WebhookResponseTransfer;
12+
use Spryker\Zed\AppWebhook\AppWebhookConfig;
13+
use Spryker\Zed\AppWebhook\Business\Exception\AllowedNumberOfRetriesExceededException;
1214
use Spryker\Zed\AppWebhook\Business\Identifier\IdentifierBuilderInterface;
15+
use Spryker\Zed\AppWebhook\Business\MessageBuilder\MessageBuilder;
1316
use Spryker\Zed\AppWebhook\Persistence\AppWebhookEntityManagerInterface;
1417
use Throwable;
1518

@@ -20,6 +23,7 @@ class WebhookHandler
2023
*/
2124
public function __construct(
2225
protected array $webhookHandlerPlugins,
26+
protected AppWebhookConfig $appWebhookConfig,
2327
protected AppWebhookEntityManagerInterface $appWebhookEntityManager,
2428
protected IdentifierBuilderInterface $identifierBuilder
2529
) {
@@ -36,6 +40,13 @@ public function handleWebhook(WebhookRequestTransfer $webhookRequestTransfer, We
3640
$this->appWebhookEntityManager->saveWebhookRequest($webhookRequestTransfer);
3741
}
3842

43+
// Delete the webhook when the number of retries is exceeded and throw an exception.
44+
if ($webhookRequestTransfer->getIsRetry() === true && $webhookRequestTransfer->getRetries() >= $this->appWebhookConfig->getAllowedNumberOfWebhookRetries()) {
45+
$this->appWebhookEntityManager->deleteWebhookRequest($webhookRequestTransfer);
46+
47+
throw new AllowedNumberOfRetriesExceededException(MessageBuilder::allowedNumberOfRetriesExceeded());
48+
}
49+
3950
try {
4051
foreach ($this->webhookHandlerPlugins as $webhookHandlerPlugin) {
4152
if (!$webhookHandlerPlugin->canHandle($webhookRequestTransfer)) {

src/Spryker/Zed/AppWebhook/Persistence/AppWebhookPersistenceFactory.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
/**
1616
* @method \Spryker\Zed\AppWebhook\Persistence\AppWebhookEntityManagerInterface getEntityManager()
1717
* @method \Spryker\Zed\AppWebhook\Persistence\AppWebhookRepositoryInterface getRepository()
18+
* @method \Spryker\Zed\AppWebhook\AppWebhookConfig getConfig()
1819
*/
1920
class AppWebhookPersistenceFactory extends AbstractPersistenceFactory
2021
{

src/Spryker/Zed/AppWebhook/Persistence/Propel/Mapper/WebhookInboxMapper.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@ public function mapWebhookInboxEntityToWebhookRequestTransfer(
1616
SpyWebhookInbox $spyWebhookInbox,
1717
WebhookRequestTransfer $webhookRequestTransfer
1818
): WebhookRequestTransfer {
19-
return $webhookRequestTransfer->fromArray(json_decode($spyWebhookInbox->getWebhook(), true));
19+
$webhookRequestTransfer->fromArray(json_decode($spyWebhookInbox->getWebhook(), true));
20+
$webhookRequestTransfer->setRetries($spyWebhookInbox->getRetries());
21+
22+
return $webhookRequestTransfer;
2023
}
2124
}

tests/SprykerTest/Shared/AppWebhook/_support/Helper/AppWebhookHelper.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ public function handleWebhook(
105105
};
106106
}
107107

108-
public function haveWebhookRequestPersisted(WebhookRequestTransfer $webhookRequestTransfer, string $message = ''): void
108+
public function haveWebhookRequestPersisted(WebhookRequestTransfer $webhookRequestTransfer, string $message = '', ?int $numberOfRetries = null): void
109109
{
110110
$spyWebhookInboxEntity = new SpyWebhookInbox();
111111
$spyWebhookInboxEntity
@@ -114,6 +114,10 @@ public function haveWebhookRequestPersisted(WebhookRequestTransfer $webhookReque
114114
->setMessage($message)
115115
->setSequenceNumber($this->getSequenceNumber($webhookRequestTransfer->getIdentifierOrFail()));
116116

117+
if ($numberOfRetries) {
118+
$spyWebhookInboxEntity->setRetries($numberOfRetries + 1);
119+
}
120+
117121
$spyWebhookInboxEntity->save();
118122
}
119123

tests/SprykerTest/Zed/AppWebhook/Business/AppWebhookFacadeTest.php

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use Generated\Shared\Transfer\WebhookResponseTransfer;
1515
use Ramsey\Uuid\Uuid;
1616
use Spryker\Zed\AppWebhook\AppWebhookDependencyProvider;
17+
use Spryker\Zed\AppWebhook\Business\Exception\AllowedNumberOfRetriesExceededException;
1718
use Spryker\Zed\AppWebhook\Business\Identifier\IdentifierBuilderInterface;
1819
use SprykerTest\Zed\AppWebhook\AppWebhookBusinessTester;
1920

@@ -370,6 +371,86 @@ public function testGivenAnUnprocessedWebhookRequestWhenItWillBeRetriedAndIsSucc
370371
$this->tester->assertWebhookIsNotPersisted($identifier);
371372
}
372373

374+
/**
375+
* @group single
376+
*/
377+
public function testGivenAnUnprocessedWebhookRequestWhenItWillBeRetriedAndTheNumberOfRetriesIsHigherThanTheConfiguredAllowedNumberOfRetriesThenItWillBeDeleted(): void
378+
{
379+
// Arrange
380+
$identifier = Uuid::uuid4()->toString(); // The one that is already persisted
381+
382+
$webhookRequestTransfer = new WebhookRequestTransfer();
383+
$webhookRequestTransfer
384+
->setContent('{foo: bar}')
385+
->setMode('async')
386+
->setIdentifier($identifier)
387+
->setIsRetry(true)
388+
->setRetries($this->tester->getModuleConfig()->getAllowedNumberOfWebhookRetries() + 1);
389+
390+
$this->tester->haveWebhookRequestPersisted($webhookRequestTransfer, 'First message', $this->tester->getModuleConfig()->getAllowedNumberOfWebhookRetries());
391+
392+
$webhookResponseTransfer = new WebhookResponseTransfer();
393+
394+
$callable = function (WebhookRequestTransfer $webhookRequestTransfer, WebhookResponseTransfer $webhookResponseTransfer) use ($identifier) {
395+
$webhookResponseTransfer
396+
->setIsHandled(false); // This will make it fail
397+
398+
return $webhookResponseTransfer;
399+
};
400+
401+
$webhookHandlerPlugin = $this->tester->createSuccessfulWebhookHandlerPlugin($callable);
402+
403+
$this->tester->setDependency(AppWebhookDependencyProvider::PLUGINS_WEBHOOK_HANDLER, [$webhookHandlerPlugin]);
404+
405+
// Expect
406+
$this->expectException(AllowedNumberOfRetriesExceededException::class);
407+
408+
// Act
409+
$webhookResponseTransfer = $this->tester->getFacade()->handleWebhook($webhookRequestTransfer, $webhookResponseTransfer);
410+
411+
// Assert
412+
// Ensure that the WebhookRequest is deleted from the persistence as it seems to never possible to handle it.
413+
$this->tester->assertWebhookIsNotPersisted($identifier);
414+
}
415+
416+
/**
417+
* @group single
418+
*/
419+
public function testGivenAnUnprocessedWebhookRequestWhenItWillBeRetriedAndTheNumberOfRetriesIsHigherThanTheConfiguredAllowedNumberOfRetriesThenAnExceptionIsThrown(): void
420+
{
421+
// Arrange
422+
$identifier = Uuid::uuid4()->toString(); // The one that is already persisted
423+
424+
$webhookRequestTransfer = new WebhookRequestTransfer();
425+
$webhookRequestTransfer
426+
->setContent('{foo: bar}')
427+
->setMode('async')
428+
->setIdentifier($identifier)
429+
->setIsRetry(true)
430+
->setRetries($this->tester->getModuleConfig()->getAllowedNumberOfWebhookRetries() + 1);
431+
432+
$this->tester->haveWebhookRequestPersisted($webhookRequestTransfer, 'First message', $this->tester->getModuleConfig()->getAllowedNumberOfWebhookRetries());
433+
434+
$webhookResponseTransfer = new WebhookResponseTransfer();
435+
436+
$callable = function (WebhookRequestTransfer $webhookRequestTransfer, WebhookResponseTransfer $webhookResponseTransfer) use ($identifier) {
437+
$webhookResponseTransfer
438+
->setIsHandled(false); // This will make it fail
439+
440+
return $webhookResponseTransfer;
441+
};
442+
443+
$webhookHandlerPlugin = $this->tester->createSuccessfulWebhookHandlerPlugin($callable);
444+
445+
$this->tester->setDependency(AppWebhookDependencyProvider::PLUGINS_WEBHOOK_HANDLER, [$webhookHandlerPlugin]);
446+
447+
// Expect
448+
$this->expectException(AllowedNumberOfRetriesExceededException::class);
449+
450+
// Act
451+
$webhookResponseTransfer = $this->tester->getFacade()->handleWebhook($webhookRequestTransfer, $webhookResponseTransfer);
452+
}
453+
373454
/**
374455
* @group single
375456
*/

tests/SprykerTest/Zed/AppWebhook/_support/AppWebhookBusinessTester.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
* @SuppressWarnings(PHPMD)
2929
*
3030
* @method \Spryker\Zed\AppWebhook\Business\AppWebhookFacadeInterface getFacade(?string $moduleName = NULL)
31+
* @method \Spryker\Zed\AppWebhook\AppWebhookConfig getModuleConfig(?string $moduleName = NULL)
3132
*/
3233
class AppWebhookBusinessTester extends Actor
3334
{

0 commit comments

Comments
 (0)