diff --git a/.github/workflows/api-functional-tests.yml b/.github/workflows/api-functional-tests.yml index 0746d50..b7a906b 100644 --- a/.github/workflows/api-functional-tests.yml +++ b/.github/workflows/api-functional-tests.yml @@ -11,6 +11,7 @@ jobs: # Only run api tests on feature branches. Simple documentation updates, formatting can be ignored if: contains(github.head_ref, 'feature') + name: ${{ matrix.job_title }} strategy: fail-fast: true matrix: @@ -25,6 +26,8 @@ jobs: redis: "redis:6.2" varnish: "varnish:7.0" nginx: "nginx:1.18" + job_title: "2.4.5" + - magento: magento/project-community-edition:>=2.4.5 <2.4.6 php: 8.1 composer: 2 @@ -34,6 +37,7 @@ jobs: redis: "redis:6.2" varnish: "varnish:7.0" nginx: "nginx:1.18" + job_title: "2.4.6" services: elasticsearch: @@ -68,7 +72,7 @@ jobs: - 5672:5672 - 15672:15672 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Set PHP Version uses: shivammathur/setup-php@v2 with: diff --git a/.github/workflows/integration-tests.yml b/.github/workflows/integration-tests.yml index 3536898..a5f5a41 100644 --- a/.github/workflows/integration-tests.yml +++ b/.github/workflows/integration-tests.yml @@ -11,6 +11,7 @@ jobs: # Only run integration tests on feature branches. Simple documentation updates, formatting can be ignored if: contains(github.head_ref, 'feature') + name: ${{ matrix.job_title }} strategy: fail-fast: true matrix: @@ -25,6 +26,7 @@ jobs: redis: "redis:6.2" varnish: "varnish:7.0" nginx: "nginx:1.18" + job_title: "2.4.5" - magento: magento/project-community-edition:>=2.4.5 <2.4.6 php: 8.1 composer: 2 @@ -34,6 +36,7 @@ jobs: redis: "redis:6.2" varnish: "varnish:7.0" nginx: "nginx:1.18" + job_title: "2.4.6" services: elasticsearch: @@ -68,7 +71,7 @@ jobs: - 5672:5672 - 15672:15672 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Set PHP Version uses: shivammathur/setup-php@v2 with: @@ -128,6 +131,9 @@ jobs: sed -i "s/'db-password' => '123123q'/'db-password' => 'password'/" etc/install-config-mysql.php.dist sed -i "s/'elasticsearch-host' => 'localhost'/'elasticsearch-host' => '127.0.0.1'/" etc/install-config-mysql.php.dist sed -i "s/'amqp-host' => 'localhost'/'amqp-host' => '127.0.0.1'/" etc/install-config-mysql.php.dist + sed -i "s/'consumers-wait-for-messages' => '0'/'consumers-wait-for-messages' => '1'/" etc/install-config-mysql.php.dist + mkdir etc/di/preferences/cli + cp ../../../vendor/mage-os/mageos-async-events/Test/_files/ce.php ./etc/di/preferences/cli - run: ../../../vendor/bin/phpunit ../../../vendor/mage-os/mageos-async-events/Test/Integration working-directory: ../magento2/dev/tests/integration diff --git a/Api/Data/ResultInterface.php b/Api/Data/ResultInterface.php new file mode 100644 index 0000000..f991823 --- /dev/null +++ b/Api/Data/ResultInterface.php @@ -0,0 +1,53 @@ +setData(self::DATA, $eventData); } + + public function getIsSuccessful(): bool + { + return $this->getSuccess(); + } + + public function setIsSuccessful(bool $isSuccessful): void + { + $this->setSuccess($isSuccessful); + } + + public function getIsRetryable(): bool + { + return (bool) $this->getData(self::IS_RETRYABLE); + } + + public function setIsRetryable(bool $isRetryable): void + { + $this->setData(self::IS_RETRYABLE, $isRetryable); + } + + public function getRetryAfter(): ?int + { + return $this->getData(self::RETRY_AFTER); + } + + public function setRetryAfter(int $retryAfter): void + { + $this->setData(self::RETRY_AFTER, $retryAfter); + } } diff --git a/Model/RetryHandler.php b/Model/RetryHandler.php index a0ac24a..86e0eb0 100644 --- a/Model/RetryHandler.php +++ b/Model/RetryHandler.php @@ -66,15 +66,21 @@ public function process(array $message): void foreach ($asyncEvents as $asyncEvent) { $handler = $asyncEvent->getMetadata(); $notifier = $this->notifierFactory->create($handler); - $response = $notifier->notify($asyncEvent, [ + $result = $notifier->notify($asyncEvent, [ 'data' => $data ]); - $response->setUuid($uuid); - $this->log($response); + $result->setUuid($uuid); + $this->log($result); - if (!$response->getSuccess()) { + if (!$result->getIsSuccessful() && $result->getIsRetryable()) { if ($deathCount < $maxDeaths) { - $this->retryManager->place($deathCount + 1, $subscriptionId, $data, $uuid); + $this->retryManager->place( + ++$deathCount, + $subscriptionId, + $data, + $uuid, + $result->getRetryAfter() + ); } else { $this->retryManager->kill($subscriptionId, $data); } diff --git a/Service/AsyncEvent/EventDispatcher.php b/Service/AsyncEvent/EventDispatcher.php index c5e26e6..c28ad02 100644 --- a/Service/AsyncEvent/EventDispatcher.php +++ b/Service/AsyncEvent/EventDispatcher.php @@ -63,7 +63,7 @@ public function dispatch(string $eventName, mixed $output, int $storeId = 0): vo $notifier = $this->notifierFactory->create($handler); - $response = $notifier->notify( + $result = $notifier->notify( $asyncEvent, [ 'data' => $output @@ -71,11 +71,11 @@ public function dispatch(string $eventName, mixed $output, int $storeId = 0): vo ); $uuid = $this->identityService->generateId(); - $response->setUuid($uuid); + $result->setUuid($uuid); - $this->log($response); + $this->log($result); - if (!$response->getSuccess()) { + if (!$result->getIsSuccessful() && $result->getIsRetryable()) { $this->retryManager->init($asyncEvent->getSubscriptionId(), $output, $uuid); } } diff --git a/Service/AsyncEvent/HttpNotifier.php b/Service/AsyncEvent/HttpNotifier.php index 5385935..30521de 100644 --- a/Service/AsyncEvent/HttpNotifier.php +++ b/Service/AsyncEvent/HttpNotifier.php @@ -64,11 +64,7 @@ public function notify(AsyncEventInterface $asyncEvent, array $data): NotifierRe ] ); - $notifierResult->setSuccess( - $response->getStatusCode() >= 200 - && $response->getStatusCode() < 300 - ); - + $notifierResult->setIsSuccessful(true); $notifierResult->setResponseData($response->getBody()->getContents()); } catch (RequestException $exception) { @@ -76,7 +72,7 @@ public function notify(AsyncEventInterface $asyncEvent, array $data): NotifierRe * Catch a RequestException, so we cover even the network layer exceptions which might sometimes * not have a response. */ - $notifierResult->setSuccess(false); + $notifierResult->setIsSuccessful(false); if ($exception->hasResponse()) { $response = $exception->getResponse(); @@ -84,6 +80,14 @@ public function notify(AsyncEventInterface $asyncEvent, array $data): NotifierRe $exceptionMessage = !empty($responseContent) ? $responseContent : $response->getReasonPhrase(); $notifierResult->setResponseData($exceptionMessage); + $notifierResult->setIsRetryable(true); + + if ($response->hasHeader('Retry-After')) { + $retryAfter = $response->getHeader('Retry-After')[0]; + if (is_numeric($retryAfter)) { + $notifierResult->setRetryAfter((int) $retryAfter); + } + } } else { $notifierResult->setResponseData( $exception->getMessage() diff --git a/Service/AsyncEvent/NotifierInterface.php b/Service/AsyncEvent/NotifierInterface.php index 61eba99..e8c55d5 100644 --- a/Service/AsyncEvent/NotifierInterface.php +++ b/Service/AsyncEvent/NotifierInterface.php @@ -5,7 +5,7 @@ namespace MageOS\AsyncEvents\Service\AsyncEvent; use MageOS\AsyncEvents\Api\Data\AsyncEventInterface; -use MageOS\AsyncEvents\Helper\NotifierResult; +use MageOS\AsyncEvents\Api\Data\ResultInterface; interface NotifierInterface { @@ -14,7 +14,7 @@ interface NotifierInterface * * @param AsyncEventInterface $asyncEvent * @param array $data - * @return NotifierResult + * @return ResultInterface */ - public function notify(AsyncEventInterface $asyncEvent, array $data): NotifierResult; + public function notify(AsyncEventInterface $asyncEvent, array $data): ResultInterface; } diff --git a/Service/AsyncEvent/RetryManager.php b/Service/AsyncEvent/RetryManager.php index 002b646..8b1ba34 100644 --- a/Service/AsyncEvent/RetryManager.php +++ b/Service/AsyncEvent/RetryManager.php @@ -4,6 +4,7 @@ namespace MageOS\AsyncEvents\Service\AsyncEvent; +use MageOS\AsyncEvents\Api\RetryManagementInterface; use MageOS\AsyncEvents\Helper\QueueMetadataInterface; use Magento\Framework\Amqp\ConfigPool; use Magento\Framework\Amqp\Topology\BindingInstallerInterface; @@ -12,7 +13,7 @@ use Magento\Framework\MessageQueue\Topology\Config\QueueConfigItemFactory; use Magento\Framework\Serialize\SerializerInterface; -class RetryManager +class RetryManager implements RetryManagementInterface { public const DEATH_COUNT = 'death_count'; public const SUBSCRIPTION_ID = 'subscription_id'; @@ -70,11 +71,15 @@ public function init(int $subscriptionId, mixed $data, string $uuid): void * @param int $subscriptionId * @param mixed $data * @param string $uuid + * @param int|null $backoff * @return void */ - public function place(int $deathCount, int $subscriptionId, mixed $data, string $uuid): void + public function place(int $deathCount, int $subscriptionId, mixed $data, string $uuid, ?int $backoff): void { - $backoff = $this->calculateBackoff($deathCount); + if (!$backoff) { + $backoff = $this->calculateBackoff($deathCount); + } + $queueName = 'event.delay.' . $backoff; $retryRoutingKey = 'event.retry.' . $backoff; @@ -120,6 +125,7 @@ public function kill(int $subscriptionId, mixed $data): void private function assertDelayQueue(int $backoff, string $queueName, string $retryRoutingKey): void { $config = $this->configPool->get('amqp'); + $backoff = abs($backoff); $queueConfigItem = $this->queueConfigItemFactory->create(); $queueConfigItem->setData([ diff --git a/Test/Integration/EventRetryTest.php b/Test/Integration/EventRetryTest.php index 72a488a..92f01ed 100644 --- a/Test/Integration/EventRetryTest.php +++ b/Test/Integration/EventRetryTest.php @@ -22,12 +22,12 @@ class EventRetryTest extends TestCase /** @var PublisherInterface|null */ private ?PublisherInterface $publisher; - /** @var PublisherConsumerController|null */ - private ?PublisherConsumerController $publisherConsumerController; - /** @var Json|null */ private ?Json $json; + /** @var ResourceConnection|null */ + private ?ResourceConnection $connection; + protected function setUp(): void { Bootstrap::getObjectManager()->configure([ @@ -54,7 +54,6 @@ protected function setUp(): void * * @magentoDataFixture MageOS_AsyncEvents::Test/_files/http_async_events.php * @magentoDbIsolation disabled - * @magentoConfigFixture default/system/async_events/max_deaths 3 */ public function testRetry() { @@ -68,18 +67,23 @@ public function testRetry() ] ); - $this->publisherConsumerController = Bootstrap::getObjectManager()->create( + $consumerInitParams = Bootstrap::getInstance()->getAppInitParams(); + + $consumerInitParams['TESTS_BASE_DIR'] = INTEGRATION_TESTS_DIR; + $consumerInitParams['INTEGRATION_TESTS_CLI_AUTOLOADER'] = INTEGRATION_TESTS_DIR . '/framework/autoload.php'; + + $publisherConsumerController = Bootstrap::getObjectManager()->create( PublisherConsumerController::class, [ 'consumers' => ['event.trigger.consumer', 'event.retry.consumer'], 'logFilePath' => TESTS_TEMP_DIR . "/MessageQueueTestLog.txt", 'maxMessages' => 10, - 'appInitParams' => Bootstrap::getInstance()->getAppInitParams() + 'appInitParams' => $consumerInitParams ] ); try { - $this->publisherConsumerController->startConsumers(); + $publisherConsumerController->startConsumers(); sleep(16); } catch (EnvironmentPreconditionException $e) { $this->markTestSkipped($e->getMessage()); @@ -88,7 +92,7 @@ public function testRetry() $e->getMessage() ); } finally { - $this->publisherConsumerController->stopConsumers(); + $publisherConsumerController->stopConsumers(); } $table = $this->connection->getTableName('async_event_subscriber_log'); @@ -100,8 +104,10 @@ public function testRetry() $events = $connection->fetchAll($select); + $this->assertNotEmpty($events); + foreach ($events as $event) { - // An uuid batch should be retired for 3 times after the first attempt. 1 + 3 + // A batch should be retired for 3 times after the first attempt. 1 + 3 $this->assertEquals(4, $event['events']); } } diff --git a/Test/Integration/FailoverTopologyTest.php b/Test/Integration/FailoverTopologyTest.php index efaebc3..d976c0a 100644 --- a/Test/Integration/FailoverTopologyTest.php +++ b/Test/Integration/FailoverTopologyTest.php @@ -1,6 +1,5 @@ retryManager->place(2, 1, 'test', 'uuid'); - $this->retryManager->place(3, 1, 'test', 'uuid'); - $this->retryManager->place(4, 1, 'test', 'uuid'); - $this->retryManager->place(5, 1, 'test', 'uuid'); - $this->retryManager->place(6, 1, 'test', 'uuid'); - $this->retryManager->place(7, 1, 'test', 'uuid'); - $this->retryManager->place(8, 1, 'test', 'uuid'); - $this->retryManager->place(9, 1, 'test', 'uuid'); - $this->retryManager->place(10, 1, 'test', 'uuid'); + $this->retryManager->place(2, 1, 'test', 'uuid', null); + $this->retryManager->place(3, 1, 'test', 'uuid', null); + $this->retryManager->place(4, 1, 'test', 'uuid', null); + $this->retryManager->place(5, 1, 'test', 'uuid', null); + $this->retryManager->place(6, 1, 'test', 'uuid', null); + $this->retryManager->place(7, 1, 'test', 'uuid', null); + $this->retryManager->place(8, 1, 'test', 'uuid', null); + $this->retryManager->place(9, 1, 'test', 'uuid', null); + $this->retryManager->place(10, 1, 'test', 'uuid', null); $bindings = $this->helper->getExchangeBindings('event.failover'); diff --git a/Test/_files/ce.php b/Test/_files/ce.php new file mode 100644 index 0000000..665a33b --- /dev/null +++ b/Test/_files/ce.php @@ -0,0 +1,9 @@ + \MageOS\AsyncEvents\Test\Integration\TestConfig::class +]; diff --git a/Test/_files/http_async_events.php b/Test/_files/http_async_events.php index fc5896a..1d05bb8 100644 --- a/Test/_files/http_async_events.php +++ b/Test/_files/http_async_events.php @@ -11,7 +11,7 @@ $connection->insertOnDuplicate('async_event_subscriber', [ 'subscription_id' => 1, 'event_name' => 'example.event', - 'recipient_url' => 'https://mock.codes/500', + 'recipient_url' => 'https://mock.codes/503', 'status' => 1, 'metadata' => 'http', 'verification_token' => 'secret',