From efe7bf02f68bbf42090be1029141965fbd4f91b9 Mon Sep 17 00:00:00 2001 From: Florent Blaison Date: Mon, 3 Aug 2020 15:47:09 +0200 Subject: [PATCH] refactor from code review --- src/GooglePlay/Acknowledger.php | 73 +++++++++++-------- .../Exception/AlreadyAcknowledgeException.php | 29 -------- .../GooglePlay/GooglePlayAcknowledgerTest.php | 42 ++--------- 3 files changed, 50 insertions(+), 94 deletions(-) delete mode 100644 src/GooglePlay/Exception/AlreadyAcknowledgeException.php diff --git a/src/GooglePlay/Acknowledger.php b/src/GooglePlay/Acknowledger.php index 0d01c8f..c464b92 100644 --- a/src/GooglePlay/Acknowledger.php +++ b/src/GooglePlay/Acknowledger.php @@ -4,9 +4,6 @@ use Exception; use Google_Service_AndroidPublisher; -use Google_Service_AndroidPublisher_ProductPurchasesAcknowledgeRequest; -use Google_Service_AndroidPublisher_SubscriptionPurchasesAcknowledgeRequest; -use ReceiptValidator\GooglePlay\Exception\AlreadyAcknowledgeException; use ReceiptValidator\RunTimeException; /** @@ -61,7 +58,7 @@ public function __construct( $purchaseToken, $strategy = self::ACKNOWLEDGE_STRATEGY_EXPLICIT ) { - if (! in_array($strategy, [self::ACKNOWLEDGE_STRATEGY_EXPLICIT, self::ACKNOWLEDGE_STRATEGY_IMPLICIT])) { + if (!in_array($strategy, [self::ACKNOWLEDGE_STRATEGY_EXPLICIT, self::ACKNOWLEDGE_STRATEGY_IMPLICIT])) { throw new RuntimeException(sprintf('Invalid strategy provided %s', $strategy)); } @@ -76,58 +73,72 @@ public function __construct( * @param string $type * @param string $developerPayload * - * @return bool * @throws RunTimeException * + * @return bool */ public function acknowledge(string $type = self::SUBSCRIPTION, string $developerPayload = '') { try { switch ($type) { case self::SUBSCRIPTION: - $subscriptionPurchase = $this->androidPublisherService->purchases_subscriptions->get( - $this->packageName, - $this->productId, - $this->purchaseToken - ); - - if ($this->strategy === self::ACKNOWLEDGE_STRATEGY_EXPLICIT - && $subscriptionPurchase->getAcknowledgementState() === 1) { - throw AlreadyAcknowledgeException::fromSubscriptionPurchase($subscriptionPurchase); - } - - if ($subscriptionPurchase->getAcknowledgementState() != 1) { + if ($this->strategy === self::ACKNOWLEDGE_STRATEGY_EXPLICIT) { + // Here exception might be thrown as previously, so no BC break here $this->androidPublisherService->purchases_subscriptions->acknowledge( $this->packageName, $this->productId, $this->purchaseToken, - new Google_Service_AndroidPublisher_SubscriptionPurchasesAcknowledgeRequest( + new \Google_Service_AndroidPublisher_SubscriptionPurchasesAcknowledgeRequest( ['developerPayload' => $developerPayload] ) ); + } elseif ($this->strategy === self::ACKNOWLEDGE_STRATEGY_IMPLICIT) { + $subscriptionPurchase = $this->androidPublisherService->purchases_subscriptions->get( + $this->packageName, + $this->productId, + $this->purchaseToken + ); + + if ($subscriptionPurchase->getAcknowledgementState() !== AbstractResponse::ACKNOWLEDGEMENT_STATE_DONE) { + $this->androidPublisherService->purchases_subscriptions->acknowledge( + $this->packageName, + $this->productId, + $this->purchaseToken, + new \Google_Service_AndroidPublisher_SubscriptionPurchasesAcknowledgeRequest( + ['developerPayload' => $developerPayload] + ) + ); + } } break; case self::PRODUCT: - $productPurchase = $this->androidPublisherService->purchases_products->get( - $this->packageName, - $this->productId, - $this->purchaseToken - ); - - if ($this->strategy === self::ACKNOWLEDGE_STRATEGY_EXPLICIT - && $productPurchase->getAcknowledgementState() === 1) { - throw AlreadyAcknowledgeException::fromProductPurchase($productPurchase); - } - - if ($productPurchase->getAcknowledgementState() != 1) { + if ($this->strategy === self::ACKNOWLEDGE_STRATEGY_EXPLICIT) { + // Here exception might be thrown as previously, so no BC break here $this->androidPublisherService->purchases_products->acknowledge( $this->packageName, $this->productId, $this->purchaseToken, - new Google_Service_AndroidPublisher_ProductPurchasesAcknowledgeRequest( + new \Google_Service_AndroidPublisher_ProductPurchasesAcknowledgeRequest( ['developerPayload' => $developerPayload] ) ); + } elseif ($this->strategy === self::ACKNOWLEDGE_STRATEGY_IMPLICIT) { + $productPurchase = $this->androidPublisherService->purchases_products->get( + $this->packageName, + $this->productId, + $this->purchaseToken + ); + + if ($productPurchase->getAcknowledgementState() !== AbstractResponse::ACKNOWLEDGEMENT_STATE_DONE) { + $this->androidPublisherService->purchases_products->acknowledge( + $this->packageName, + $this->productId, + $this->purchaseToken, + new \Google_Service_AndroidPublisher_ProductPurchasesAcknowledgeRequest( + ['developerPayload' => $developerPayload] + ) + ); + } } break; default: diff --git a/src/GooglePlay/Exception/AlreadyAcknowledgeException.php b/src/GooglePlay/Exception/AlreadyAcknowledgeException.php deleted file mode 100644 index 066a41e..0000000 --- a/src/GooglePlay/Exception/AlreadyAcknowledgeException.php +++ /dev/null @@ -1,29 +0,0 @@ -getOrderId()) - ); - } - - public static function fromProductPurchase( - \Google_Service_AndroidPublisher_ProductPurchase $productPurchase - ) { - return new static( - \sprintf('Product purchase %s already acknowledged', $productPurchase->getOrderId()) - ); - } -} diff --git a/tests/GooglePlay/GooglePlayAcknowledgerTest.php b/tests/GooglePlay/GooglePlayAcknowledgerTest.php index 6da35c9..f5d10b0 100755 --- a/tests/GooglePlay/GooglePlayAcknowledgerTest.php +++ b/tests/GooglePlay/GooglePlayAcknowledgerTest.php @@ -9,7 +9,6 @@ use Google_Service_AndroidPublisher_SubscriptionPurchase; use PHPUnit\Framework\TestCase; use ReceiptValidator\GooglePlay\Acknowledger; -use ReceiptValidator\GooglePlay\Exception\AlreadyAcknowledgeException; /** * @group library @@ -48,12 +47,6 @@ public function testValidateWithNonAcknowledgedPurchase(): void $googleServiceAndroidPublisherMock->purchases_products = $purchasesProductsMock; $googleServiceAndroidPublisherMock->purchases_subscriptions = $purchasesSubscriptionsMock; - $purchasesProductsMock->expects($this->once())->method('get') - ->with( - $packageName, - $productId, - $purchaseToken - )->willReturn($productPurchaseMock); $purchasesProductsMock->expects($this->once())->method('acknowledge') ->with( $packageName, @@ -64,12 +57,6 @@ public function testValidateWithNonAcknowledgedPurchase(): void ) ); - $purchasesSubscriptionsMock->expects($this->once())->method('get') - ->with( - $packageName, - $productId, - $purchaseToken - )->willReturn($subscriptionPurchaseMock); $purchasesSubscriptionsMock->expects($this->once())->method('acknowledge') ->with( $packageName, @@ -169,35 +156,30 @@ public function testValidateWithAcknowledgedPurchaseAndImplicitStrategy(): void public function testValidateWithAcknowledgedPurchaseAndExplicitStrategyForSubscription(): void { - $this->expectException(AlreadyAcknowledgeException::class); - $packageName = 'testPackage'; $productId = '15'; $purchaseToken = 'testPurchaseToken'; // mock objects $googleServiceAndroidPublisherMock = $this->getMockBuilder(Google_Service_AndroidPublisher::class) - ->disableOriginalConstructor()->getMock(); + ->disableOriginalConstructor() + ->getMock(); // subscriptions $purchasesSubscriptionsMock = $this->getMockBuilder( Google_Service_AndroidPublisher_Resource_PurchasesSubscriptions::class ) - ->disableOriginalConstructor()->getMock(); + ->disableOriginalConstructor() + ->getMock(); $subscriptionPurchaseMock = $this->getMockBuilder(Google_Service_AndroidPublisher_SubscriptionPurchase::class) - ->disableOriginalConstructor()->getMock(); + ->disableOriginalConstructor() + ->getMock(); $subscriptionPurchaseMock->expects($this->any())->method('getAcknowledgementState')->willReturn(1); // mock expectations $googleServiceAndroidPublisherMock->purchases_subscriptions = $purchasesSubscriptionsMock; - $purchasesSubscriptionsMock->expects($this->once())->method('get') - ->with( - $packageName, - $productId, - $purchaseToken - )->willReturn($subscriptionPurchaseMock); - $purchasesSubscriptionsMock->expects($this->never())->method('acknowledge') + $purchasesSubscriptionsMock->expects($this->once())->method('acknowledge') ->with( $packageName, $productId, @@ -219,8 +201,6 @@ public function testValidateWithAcknowledgedPurchaseAndExplicitStrategyForSubscr public function testValidateWithAcknowledgedPurchaseAndExplicitStrategyForProduct(): void { - $this->expectException(AlreadyAcknowledgeException::class); - $packageName = 'testPackage'; $productId = '15'; $purchaseToken = 'testPurchaseToken'; @@ -241,13 +221,7 @@ public function testValidateWithAcknowledgedPurchaseAndExplicitStrategyForProduc // mock expectations $googleServiceAndroidPublisherMock->purchases_products = $purchasesProductsMock; - $purchasesProductsMock->expects($this->once())->method('get') - ->with( - $packageName, - $productId, - $purchaseToken - )->willReturn($productPurchaseMock); - $purchasesProductsMock->expects($this->never())->method('acknowledge') + $purchasesProductsMock->expects($this->once())->method('acknowledge') ->with( $packageName, $productId,