Skip to content

Commit

Permalink
Fixes ECP-8518
Browse files Browse the repository at this point in the history
- remove amqp in queue xml config to use default (db) if amqp service is not installed.
- include additional admin message when using 'queue' notification processing
- add unit tests
  • Loading branch information
factorin-j committed Aug 17, 2024
1 parent 495ec49 commit 6560aea
Show file tree
Hide file tree
Showing 8 changed files with 123 additions and 10 deletions.
6 changes: 5 additions & 1 deletion Cron/WebhookProcessor.php
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@ public function doProcessWebhook()

// Loop through and process notifications.
$count = 0;
$queued = 0;

/** @var Notification[] $notifications */
foreach ($notifications as $notification) {
// ignore duplicate notification
Expand Down Expand Up @@ -128,6 +130,7 @@ public function doProcessWebhook()

if ($this->configHelper->useQueueProcessor()) {
$this->notificationPublisher->execute($notification);
$queued++;
$count++;
} elseif ($this->webhookHelper->processNotification($notification)) {
$count++;
Expand All @@ -139,7 +142,8 @@ public function doProcessWebhook()
"Cronjob updated %s notification(s)", $count
), [
'pspReference' => $notification->getPspreference(),
'merchantReference' => $notification->getMerchantReference()
'merchantReference' => $notification->getMerchantReference(),
'queued' => $queued
]);
}
}
Expand Down
20 changes: 17 additions & 3 deletions Model/Queue/Notification/Publisher.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,29 +3,43 @@
namespace Adyen\Payment\Model\Queue\Notification;

use Adyen\Payment\Api\Data\NotificationInterface;
use Adyen\Payment\Model\ResourceModel\Notification;
use Exception;
use Magento\Framework\MessageQueue\PublisherInterface;

class Publisher
{
private const TOPIC_NAME = "adyen.notification";
public const TOPIC_NAME = "adyen.notification";

/** @var PublisherInterface $publisher */
private $publisher;

/** @var Notification $notificationResource */
private $notificationResource;

/**
* @param PublisherInterface $publisher
* @param Notification $notificationResource
*/
public function __construct(PublisherInterface $publisher)
{
public function __construct(
PublisherInterface $publisher,
Notification $notificationResource
) {
$this->publisher = $publisher;
$this->notificationResource = $notificationResource;
}

/**
* @param NotificationInterface $notification
* @return void
* @throws Exception
*/
public function execute(NotificationInterface $notification): void
{
// Set processing=true to skip adding duplicate queue entries
$notification->setProcessing(true);
$this->notificationResource->save($notification);

$this->publisher->publish(self::TOPIC_NAME, $notification);
}
}
67 changes: 67 additions & 0 deletions Test/Unit/Model/Queue/Notification/ConsumerTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
<?php

namespace Adyen\Payment\Test\Unit\Model\Queue\Notification;

use Adyen\Payment\Helper\Webhook;
use Adyen\Payment\Logger\AdyenLogger;
use Adyen\Payment\Model\Notification;
use Adyen\Payment\Model\Queue\Notification\Consumer;
use Adyen\Payment\Test\Unit\AbstractAdyenTestCase;
use Exception;
use PHPUnit\Framework\MockObject\MockObject;

class ConsumerTest extends AbstractAdyenTestCase
{
/** @var Webhook|MockObject $webhookMock */
private $webhookMock;

/** @var AdyenLogger|MockObject $adyenLoggerMock */
private $adyenLoggerMock;

/** @var Notification|MockObject $notificationMock */
private $notificationMock;

/** @var Consumer $consumer */
private $consumer;

/**
* @return void
*/
protected function setUp(): void
{
$this->webhookMock = $this->createMock(Webhook::class);
$this->adyenLoggerMock = $this->createMock(AdyenLogger::class);
$this->notificationMock = $this->createMock(Notification::class);
$this->consumer = new Consumer($this->webhookMock, $this->adyenLoggerMock);
}

/**
* @return void
* @throws Exception
*/
public function testExecute(): void
{
$this->webhookMock->expects($this->once())
->method('processNotification')
->with($this->notificationMock)
->willReturn(true);

$this->assertTrue($this->consumer->execute($this->notificationMock));
}

/**
* @return void
* @throws Exception
*/
public function testExecuteThrowsException(): void
{
$this->webhookMock->expects($this->once())
->method('processNotification')
->with($this->notificationMock)
->willThrowException(new Exception());
$this->adyenLoggerMock->expects($this->once())->method('addAdyenWarning');

$this->expectException(Exception::class);
$this->consumer->execute($this->notificationMock);
}
}
31 changes: 31 additions & 0 deletions Test/Unit/Model/Queue/Notification/PublisherTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?php

namespace Adyen\Payment\Test\Unit\Model\Queue\Notification;

use Adyen\Payment\Model\Notification;
use Adyen\Payment\Model\Queue\Notification\Publisher;
use Adyen\Payment\Model\ResourceModel\Notification as NotificationResourceModel;
use Adyen\Payment\Test\Unit\AbstractAdyenTestCase;
use Exception;
use Magento\Framework\MessageQueue\PublisherInterface;

class PublisherTest extends AbstractAdyenTestCase
{
/**
* @return void
* @throws Exception
*/
public function testExecute(): void
{
$publisherMock = $this->createMock(PublisherInterface::class);
$notificationResourceMock = $this->createMock(NotificationResourceModel::class);
$notificationMock = $this->createMock(Notification::class);

$notificationMock->expects($this->once())->method('setProcessing')->with(true);
$notificationResourceMock->expects($this->once())->method('save')->with($notificationMock);
$publisherMock->expects($this->once())->method('publish')->with(Publisher::TOPIC_NAME, $notificationMock);

$publisher = new Publisher($publisherMock, $notificationResourceMock);
$publisher->execute($notificationMock);
}
}
2 changes: 1 addition & 1 deletion etc/adminhtml/system/adyen_testing_performance.xml
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
<label>Webhook processor</label>
<source_model>Adyen\Payment\Model\Config\Source\NotificationProcessor</source_model>
<config_path>payment/adyen_abstract/webhook_notification_processor</config_path>
<comment>Use cron or queue (async) to process webhook notifications</comment>
<comment>Use cron or queue (async) to process webhook notifications. Queue performs better if you have AMQP service installed, like RabbitMQ.</comment>
</field>
<field id="notifications_ip_check" translate="label" type="select" sortOrder="40" showInDefault="1" showInWebsite="1" showInStore="1">
<label>Check webhook's IP address</label>
Expand Down
1 change: 0 additions & 1 deletion etc/queue_consumer.xml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,5 @@
xsi:noNamespaceSchemaLocation="urn:magento:framework-message-queue:etc/consumer.xsd">
<consumer name="adyen.notification"
queue="adyen.notification"
connection="amqp"
handler="Adyen\Payment\Model\Queue\Notification\Consumer::execute"/>
</config>
4 changes: 1 addition & 3 deletions etc/queue_publisher.xml
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
<?xml version="1.0"?>
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="urn:magento:framework-message-queue:etc/publisher.xsd">
<publisher topic="adyen.notification">
<connection name="amqp" exchange="magento"/>
</publisher>
<publisher topic="adyen.notification"/>
</config>
2 changes: 1 addition & 1 deletion etc/queue_topology.xml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<?xml version="1.0"?>
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="urn:magento:framework-message-queue:etc/topology.xsd">
<exchange name="magento" type="topic" connection="amqp">
<exchange name="magento" type="topic">
<binding id="NotificationBinding"
topic="adyen.notification"
destinationType="queue"
Expand Down

0 comments on commit 6560aea

Please sign in to comment.