From 06e437e0c3607510fae728f0be4c95217213242f Mon Sep 17 00:00:00 2001 From: Alex P Date: Mon, 4 Oct 2021 19:16:50 +0300 Subject: [PATCH] Refactor logging, add missing logging --- .../src/Endpoint/class-paymentsendpoint.php | 28 ++------------ modules/ppcp-wc-gateway/services.php | 3 +- .../class-authorizedpaymentsprocessor.php | 14 ++++++- .../Endpoint/PaymentsEndpointTest.php | 13 ++----- .../AuthorizedPaymentsProcessorTest.php | 38 ++++++++----------- 5 files changed, 37 insertions(+), 59 deletions(-) diff --git a/modules/ppcp-api-client/src/Endpoint/class-paymentsendpoint.php b/modules/ppcp-api-client/src/Endpoint/class-paymentsendpoint.php index 6e3ec6b6d..66e0a6bcd 100644 --- a/modules/ppcp-api-client/src/Endpoint/class-paymentsendpoint.php +++ b/modules/ppcp-api-client/src/Endpoint/class-paymentsendpoint.php @@ -138,6 +138,7 @@ class PaymentsEndpoint { * * @return Authorization * @throws RuntimeException If the request fails. + * @throws PayPalApiException If the request fails. */ public function capture( string $authorization_id ): Authorization { $bearer = $this->bearer->bearer(); @@ -155,39 +156,18 @@ class PaymentsEndpoint { $json = json_decode( $response['body'] ); if ( is_wp_error( $response ) ) { - $error = new RuntimeException( - __( 'Could not capture authorized payment.', 'woocommerce-paypal-payments' ) - ); - $this->logger->log( - 'warning', - $error->getMessage(), - array( - 'args' => $args, - 'response' => $response, - ) - ); - throw $error; + throw new RuntimeException( 'Could not capture authorized payment.' ); } $status_code = (int) wp_remote_retrieve_response_code( $response ); if ( 201 !== $status_code ) { - $error = new PayPalApiException( + throw new PayPalApiException( $json, $status_code ); - $this->logger->log( - 'warning', - $error->getMessage(), - array( - 'args' => $args, - 'response' => $response, - ) - ); - throw $error; } - $authorization = $this->authorizations_factory->from_paypal_response( $json ); - return $authorization; + return $this->authorizations_factory->from_paypal_response( $json ); } /** diff --git a/modules/ppcp-wc-gateway/services.php b/modules/ppcp-wc-gateway/services.php index 93c04b827..a8b001c9a 100644 --- a/modules/ppcp-wc-gateway/services.php +++ b/modules/ppcp-wc-gateway/services.php @@ -220,7 +220,8 @@ return array( 'wcgateway.processor.authorized-payments' => static function ( $container ): AuthorizedPaymentsProcessor { $order_endpoint = $container->get( 'api.endpoint.order' ); $payments_endpoint = $container->get( 'api.endpoint.payments' ); - return new AuthorizedPaymentsProcessor( $order_endpoint, $payments_endpoint ); + $logger = $container->get( 'woocommerce.logger.woocommerce' ); + return new AuthorizedPaymentsProcessor( $order_endpoint, $payments_endpoint, $logger ); }, 'wcgateway.admin.render-authorize-action' => static function ( $container ): RenderAuthorizeAction { diff --git a/modules/ppcp-wc-gateway/src/Processor/class-authorizedpaymentsprocessor.php b/modules/ppcp-wc-gateway/src/Processor/class-authorizedpaymentsprocessor.php index 86f542cc0..453f0652d 100644 --- a/modules/ppcp-wc-gateway/src/Processor/class-authorizedpaymentsprocessor.php +++ b/modules/ppcp-wc-gateway/src/Processor/class-authorizedpaymentsprocessor.php @@ -10,6 +10,7 @@ declare(strict_types=1); namespace WooCommerce\PayPalCommerce\WcGateway\Processor; use Exception; +use Psr\Log\LoggerInterface; use WooCommerce\PayPalCommerce\ApiClient\Endpoint\OrderEndpoint; use WooCommerce\PayPalCommerce\ApiClient\Endpoint\PaymentsEndpoint; use WooCommerce\PayPalCommerce\ApiClient\Entity\Authorization; @@ -42,19 +43,29 @@ class AuthorizedPaymentsProcessor { */ private $payments_endpoint; + /** + * The logger. + * + * @var LoggerInterface + */ + private $logger; + /** * AuthorizedPaymentsProcessor constructor. * * @param OrderEndpoint $order_endpoint The Order endpoint. * @param PaymentsEndpoint $payments_endpoint The Payments endpoint. + * @param LoggerInterface $logger The logger. */ public function __construct( OrderEndpoint $order_endpoint, - PaymentsEndpoint $payments_endpoint + PaymentsEndpoint $payments_endpoint, + LoggerInterface $logger ) { $this->order_endpoint = $order_endpoint; $this->payments_endpoint = $payments_endpoint; + $this->logger = $logger; } /** @@ -83,6 +94,7 @@ class AuthorizedPaymentsProcessor { try { $this->capture_authorizations( ...$authorizations ); } catch ( Exception $exception ) { + $this->logger->error( 'Failed to capture authorization: ' . $exception->getMessage() ); return self::FAILED; } diff --git a/tests/PHPUnit/ApiClient/Endpoint/PaymentsEndpointTest.php b/tests/PHPUnit/ApiClient/Endpoint/PaymentsEndpointTest.php index 87b2ca9e9..65112a61d 100644 --- a/tests/PHPUnit/ApiClient/Endpoint/PaymentsEndpointTest.php +++ b/tests/PHPUnit/ApiClient/Endpoint/PaymentsEndpointTest.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace WooCommerce\PayPalCommerce\ApiClient\Endpoint; +use Psr\Log\NullLogger; use Requests_Utility_CaseInsensitiveDictionary; use WooCommerce\PayPalCommerce\ApiClient\Authentication\Bearer; use WooCommerce\PayPalCommerce\ApiClient\Entity\Authorization; @@ -235,10 +236,6 @@ class PaymentsEndpointTest extends TestCase $authorizationFactory = Mockery::mock(AuthorizationFactory::class); - $logger = Mockery::mock(LoggerInterface::class); - $logger->expects('log'); - $logger->expects('debug'); - $headers = Mockery::mock(Requests_Utility_CaseInsensitiveDictionary::class); $headers->shouldReceive('getAll'); $rawResponse = [ @@ -250,7 +247,7 @@ class PaymentsEndpointTest extends TestCase $host, $bearer, $authorizationFactory, - $logger + new NullLogger() ); expect('wp_remote_get')->andReturn($rawResponse); @@ -281,15 +278,11 @@ class PaymentsEndpointTest extends TestCase 'headers' => $headers, ]; - $logger = Mockery::mock(LoggerInterface::class); - $logger->expects('log'); - $logger->expects('debug'); - $testee = new PaymentsEndpoint( $host, $bearer, $authorizationFactory, - $logger + new NullLogger() ); expect('wp_remote_get')->andReturn($rawResponse); diff --git a/tests/PHPUnit/WcGateway/Processor/AuthorizedPaymentsProcessorTest.php b/tests/PHPUnit/WcGateway/Processor/AuthorizedPaymentsProcessorTest.php index 2c8ad9e75..d653f466a 100644 --- a/tests/PHPUnit/WcGateway/Processor/AuthorizedPaymentsProcessorTest.php +++ b/tests/PHPUnit/WcGateway/Processor/AuthorizedPaymentsProcessorTest.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace WooCommerce\PayPalCommerce\WcGateway\Processor; +use Psr\Log\NullLogger; use WC_Order; use WooCommerce\PayPalCommerce\ApiClient\Endpoint\OrderEndpoint; use WooCommerce\PayPalCommerce\ApiClient\Endpoint\PaymentsEndpoint; @@ -29,6 +30,8 @@ class AuthorizedPaymentsProcessorTest extends TestCase private $paymentsEndpoint; + private $testee; + public function setUp(): void { parent::setUp(); @@ -42,25 +45,24 @@ class AuthorizedPaymentsProcessorTest extends TestCase ->with($this->paypalOrderId) ->andReturnUsing(function () { return $this->paypalOrder; - }); + }) + ->byDefault(); $this->paymentsEndpoint = Mockery::mock(PaymentsEndpoint::class); + + $this->testee = new AuthorizedPaymentsProcessor($this->orderEndpoint, $this->paymentsEndpoint, new NullLogger()); } public function testSuccess() { - $testee = new AuthorizedPaymentsProcessor($this->orderEndpoint, $this->paymentsEndpoint); - $this->paymentsEndpoint ->expects('capture') ->with($this->authorizationId) ->andReturn($this->createAuthorization($this->authorizationId, AuthorizationStatus::CAPTURED)); - $this->assertEquals(AuthorizedPaymentsProcessor::SUCCESSFUL, $testee->process($this->wcOrder)); + $this->assertEquals(AuthorizedPaymentsProcessor::SUCCESSFUL, $this->testee->process($this->wcOrder)); } public function testCapturesAllCaptureable() { - $testee = new AuthorizedPaymentsProcessor($this->orderEndpoint, $this->paymentsEndpoint); - $authorizations = [ $this->createAuthorization('id1', AuthorizationStatus::CREATED), $this->createAuthorization('id2', AuthorizationStatus::VOIDED), @@ -79,50 +81,40 @@ class AuthorizedPaymentsProcessorTest extends TestCase ->andReturn($this->createAuthorization($authorization->id(), AuthorizationStatus::CAPTURED)); } - $this->assertEquals(AuthorizedPaymentsProcessor::SUCCESSFUL, $testee->process($this->wcOrder)); + $this->assertEquals(AuthorizedPaymentsProcessor::SUCCESSFUL, $this->testee->process($this->wcOrder)); } public function testInaccessible() { - $orderEndpoint = Mockery::mock(OrderEndpoint::class); - $orderEndpoint + $this->orderEndpoint ->expects('order') ->with($this->paypalOrderId) ->andThrow(RuntimeException::class); - $testee = new AuthorizedPaymentsProcessor($orderEndpoint, $this->paymentsEndpoint); - - $this->assertEquals(AuthorizedPaymentsProcessor::INACCESSIBLE, $testee->process($this->wcOrder)); + $this->assertEquals(AuthorizedPaymentsProcessor::INACCESSIBLE, $this->testee->process($this->wcOrder)); } public function testNotFound() { - $orderEndpoint = Mockery::mock(OrderEndpoint::class); - $orderEndpoint + $this->orderEndpoint ->expects('order') ->with($this->paypalOrderId) ->andThrow(new RuntimeException('text', 404)); - $testee = new AuthorizedPaymentsProcessor($orderEndpoint, $this->paymentsEndpoint); - - $this->assertEquals(AuthorizedPaymentsProcessor::NOT_FOUND, $testee->process($this->wcOrder)); + $this->assertEquals(AuthorizedPaymentsProcessor::NOT_FOUND, $this->testee->process($this->wcOrder)); } public function testCaptureFails() { - $testee = new AuthorizedPaymentsProcessor($this->orderEndpoint, $this->paymentsEndpoint); - $this->paymentsEndpoint ->expects('capture') ->with($this->authorizationId) ->andThrow(RuntimeException::class); - $this->assertEquals(AuthorizedPaymentsProcessor::FAILED, $testee->process($this->wcOrder)); + $this->assertEquals(AuthorizedPaymentsProcessor::FAILED, $this->testee->process($this->wcOrder)); } public function testAlreadyCaptured() { - $testee = new AuthorizedPaymentsProcessor($this->orderEndpoint, $this->paymentsEndpoint); - $this->paypalOrder = $this->createPaypalOrder([$this->createAuthorization($this->authorizationId, AuthorizationStatus::CAPTURED)]); - $this->assertEquals(AuthorizedPaymentsProcessor::ALREADY_CAPTURED, $testee->process($this->wcOrder)); + $this->assertEquals(AuthorizedPaymentsProcessor::ALREADY_CAPTURED, $this->testee->process($this->wcOrder)); } private function createWcOrder(string $paypalOrderId): WC_Order {