Improvement: Do not log validation error mollie#839

michielgerritsen · Jan 6, 2025 · 0fc0d49 · 0fc0d49
1 parent edfeaba
commit 0fc0d49
Show file tree

Hide file tree

Showing 6 changed files with 155 additions and 1 deletion.
diff --git a/Model/Methods/Reorder.php b/Model/Methods/Reorder.php
@@ -28,6 +28,7 @@
 use Mollie\Payment\Model\Client\Payments as PaymentsApi;
 use Mollie\Payment\Model\Mollie;
 use Mollie\Payment\Service\Mollie\GetApiMethod;
+use Mollie\Payment\Service\Mollie\LogException;
 use Mollie\Payment\Service\OrderLockService;
 use Mollie\Payment\Service\Mollie\MollieApiClient;
 use Mollie\Payment\Service\Mollie\Timeout;
@@ -72,6 +73,7 @@ public function __construct(
         MollieApiClient $mollieApiClient,
         TransactionToOrderRepositoryInterface $transactionToOrderRepository,
         GetApiMethod $getApiMethod,
+        LogException $logException,
         RequestInterface $request,
         $formBlockType,
         $infoBlockType,
@@ -100,6 +102,7 @@ public function __construct(
             $mollieApiClient,
             $transactionToOrderRepository,
             $getApiMethod,
+            $logException,
             $formBlockType,
             $infoBlockType,
             $commandPool,

diff --git a/Model/Methods/Voucher.php b/Model/Methods/Voucher.php
@@ -28,6 +28,7 @@
 use Mollie\Payment\Model\Client\Payments as PaymentsApi;
 use Mollie\Payment\Model\Mollie;
 use Mollie\Payment\Service\Mollie\GetApiMethod;
+use Mollie\Payment\Service\Mollie\LogException;
 use Mollie\Payment\Service\OrderLockService;
 use Mollie\Payment\Service\Mollie\MollieApiClient;
 use Mollie\Payment\Service\Mollie\Timeout;
@@ -68,6 +69,7 @@ public function __construct(
         MollieApiClient $mollieApiClient,
         TransactionToOrderRepositoryInterface $transactionToOrderRepository,
         GetApiMethod $getApiMethod,
+        LogException $logException,
         $formBlockType,
         $infoBlockType,
         QuoteHasMealVoucherProducts $quoteHasMealVoucherProducts,
@@ -96,6 +98,7 @@ public function __construct(
             $mollieApiClient,
             $transactionToOrderRepository,
             $getApiMethod,
+            $logException,
             $formBlockType,
             $infoBlockType,
             $commandPool,

diff --git a/Model/Mollie.php b/Model/Mollie.php
@@ -36,6 +36,7 @@
 use Mollie\Payment\Model\Client\Payments as PaymentsApi;
 use Mollie\Payment\Model\Client\ProcessTransactionResponse;
 use Mollie\Payment\Service\Mollie\GetApiMethod;
+use Mollie\Payment\Service\Mollie\LogException;
 use Mollie\Payment\Service\OrderLockService;
 use Mollie\Payment\Service\Mollie\Timeout;
 use Mollie\Payment\Service\Mollie\Wrapper\MollieApiClientFallbackWrapper;
@@ -121,6 +122,10 @@ class Mollie extends Adapter
      * @var GetApiMethod
      */
     private $getApiMethod;
+    /**
+     * @var LogException
+     */
+    private $logException;
 
     public function __construct(
         ManagerInterface $eventManager,
@@ -142,6 +147,7 @@ public function __construct(
         \Mollie\Payment\Service\Mollie\MollieApiClient $mollieApiClient,
         TransactionToOrderRepositoryInterface $transactionToOrderRepository,
         GetApiMethod $getApiMethod,
+        LogException $logException,
         $formBlockType,
         $infoBlockType,
         CommandPoolInterface $commandPool = null,
@@ -179,6 +185,7 @@ public function __construct(
         $this->mollieApiClient = $mollieApiClient;
         $this->transactionToOrderRepository = $transactionToOrderRepository;
         $this->getApiMethod = $getApiMethod;
+        $this->logException = $logException;
     }
 
     public function getCode()
@@ -280,7 +287,7 @@ private function startTransactionUsingTheOrdersApi(OrderInterface $order, Mollie
                 return $this->ordersApi->startTransaction($order, $mollieApi);
             });
         } catch (\Exception $exception) {
-            $this->mollieHelper->addTolog('error', $exception->getMessage());
+            $this->logException->execute($exception);
         }
 
         $methodCode = $this->mollieHelper->getMethodCode($order);

diff --git a/Service/Mollie/LogException.php b/Service/Mollie/LogException.php
@@ -0,0 +1,44 @@
+<?php
+/*
+ * Copyright Magmodules.eu. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+
+declare(strict_types=1);
+
+namespace Mollie\Payment\Service\Mollie;
+
+use Mollie\Payment\Config;
+
+class LogException
+{
+    /**
+     * @var Config
+     */
+    private $config;
+
+    private $messagesToSkip = [
+        'The \'billingAddress.familyName\' field contains characters that are not allowed'
+    ];
+
+    public function __construct(
+        Config $config,
+        array $messagesToSkip = []
+    ) {
+        $this->config = $config;
+        $this->messagesToSkip = array_merge($this->messagesToSkip, $messagesToSkip);
+    }
+
+    public function execute(\Exception $exception): void
+    {
+        $message = method_exists($exception, 'getPlainMessage') ?
+            $exception->getPlainMessage() :
+            $exception->getMessage();
+
+        if (in_array($message, $this->messagesToSkip)) {
+            return;
+        }
+
+        $this->config->addTolog('error', $message);
+    }
+}
diff --git a/Test/Fakes/ConfigFake.php b/Test/Fakes/ConfigFake.php
@@ -0,0 +1,31 @@
+<?php
+/*
+ * Copyright Magmodules.eu. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+
+declare(strict_types=1);
+
+namespace Mollie\Payment\Test\Fakes;
+
+use Mollie\Payment\Config;
+
+class ConfigFake extends Config
+{
+    private $loggedMessages = [];
+
+    public function addTolog($type, $message)
+    {
+        parent::addToLog($type, $message);
+
+        $this->loggedMessages[] = [
+            'type' => $type,
+            'message' => $message,
+        ];
+    }
+
+    public function getLoggedMessages(): array
+    {
+        return $this->loggedMessages;
+    }
+}
diff --git a/Test/Integration/Service/Mollie/LogExceptionTest.php b/Test/Integration/Service/Mollie/LogExceptionTest.php
@@ -0,0 +1,66 @@
+<?php
+/*
+ * Copyright Magmodules.eu. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+
+declare(strict_types=1);
+
+namespace Mollie\Payment\Test\Integration\Service\Mollie;
+
+use Mollie\Api\Exceptions\ApiException;
+use Mollie\Payment\Service\Mollie\LogException;
+use Mollie\Payment\Test\Fakes\ConfigFake;
+use Mollie\Payment\Test\Integration\IntegrationTestCase;
+
+class LogExceptionTest extends IntegrationTestCase
+{
+    public function testLogsMessages(): void
+    {
+        $exception = new ApiException('This is a test exception triggered in ' . __FILE__);
+
+        $configFake = $this->objectManager->create(ConfigFake::class);
+
+        /** @var LogException $instance */
+        $instance = $this->objectManager->create(LogException::class, [
+            'config' => $configFake,
+        ]);
+        $instance->execute($exception);
+
+        $this->assertCount(1, $configFake->getLoggedMessages());
+    }
+
+    public function testSkipsSomeMessages(): void
+    {
+        $exception = new ApiException('The \'billingAddress.familyName\' field contains characters that are not allowed');
+
+        $configFake = $this->objectManager->create(ConfigFake::class);
+
+        /** @var LogException $instance */
+        $instance = $this->objectManager->create(LogException::class, [
+            'config' => $configFake,
+        ]);
+        $instance->execute($exception);
+
+        $this->assertCount(0, $configFake->getLoggedMessages());
+    }
+
+    public function testCanAddOwnExceptions(): void
+    {
+        $exception = new ApiException('Some random message');
+
+        $configFake = $this->objectManager->create(ConfigFake::class);
+
+        /** @var LogException $instance */
+        $instance = $this->objectManager->create(LogException::class, [
+            // Normally you would do this through di.xml, but when testing we can do it like this.
+            'messagesToSkip' => [
+                'Some random message',
+            ],
+            'config' => $configFake,
+        ]);
+        $instance->execute($exception);
+
+        $this->assertCount(0, $configFake->getLoggedMessages());
+    }
+}