From e6924c0066fa4600118b0450a930cb9eead90443 Mon Sep 17 00:00:00 2001 From: Recca Tsai Date: Wed, 18 Jan 2017 08:47:33 +0800 Subject: [PATCH] fix bug --- src/Action/Api/CreateTransactionAction.php | 22 ++- src/Action/Api/GetTransactionDataAction.php | 4 +- src/Action/CancelAction.php | 2 - src/Action/CaptureAction.php | 8 +- src/Action/CaptureLogisticsAction.php | 8 +- src/Action/ConvertPaymentLogisticsAction.php | 2 +- src/Action/NotifyAction.php | 2 +- src/Action/RefundAction.php | 2 - src/Action/SyncAction.php | 7 - src/EcpayApi.php | 17 +-- src/EcpayLogisticsApi.php | 26 ++-- .../Api/CreateTransactionActionTest.php | 11 +- tests/Action/CancelActionTest.php | 1 - tests/Action/CaptureActionTest.php | 3 +- tests/Action/CaptureLogisticsActionTest.php | 1 - tests/Action/NotifyActionTest.php | 1 - tests/Action/RefundActionTest.php | 1 - tests/EcpayApiTest.php | 136 ------------------ tests/EcpayLogisticsApiTest.php | 47 +----- 19 files changed, 54 insertions(+), 247 deletions(-) diff --git a/src/Action/Api/CreateTransactionAction.php b/src/Action/Api/CreateTransactionAction.php index caee963..1927062 100644 --- a/src/Action/Api/CreateTransactionAction.php +++ b/src/Action/Api/CreateTransactionAction.php @@ -20,20 +20,16 @@ public function execute($request) $details = ArrayObject::ensureArrayObject($request->getModel()); - $result = $this->api->createTransaction((array) $details); - - if (isset($result['RtnCode']) === true) { - $details->replace($result); - - return; + if (empty($details['GoodsAmount']) === true) { + $result = $this->api->createCvsMapTransaction((array) $details); + $endpoint = $this->api->getApiEndpoint(); + + throw new HttpPostRedirect( + $endpoint, + $result + ); } - - $endpoint = $this->api->getApiEndpoint(); - - throw new HttpPostRedirect( - $endpoint, - $result - ); + $details->replace($this->api->createTransaction((array) $details)); } /** diff --git a/src/Action/Api/GetTransactionDataAction.php b/src/Action/Api/GetTransactionDataAction.php index d2d2594..bd60de4 100644 --- a/src/Action/Api/GetTransactionDataAction.php +++ b/src/Action/Api/GetTransactionDataAction.php @@ -19,9 +19,7 @@ public function execute($request) $details = ArrayObject::ensureArrayObject($request->getModel()); - $result = $this->api->getTransactionData((array) $details); - - $details->replace($result); + $details->replace($this->api->getTransactionData((array) $details)); } /** diff --git a/src/Action/CancelAction.php b/src/Action/CancelAction.php index 66f0157..3e29e7e 100644 --- a/src/Action/CancelAction.php +++ b/src/Action/CancelAction.php @@ -27,8 +27,6 @@ public function execute($request) $details = ArrayObject::ensureArrayObject($request->getModel()); $this->gateway->execute(new CancelTransaction($details)); - - $this->gateway->execute(new Sync($details)); } /** diff --git a/src/Action/CaptureAction.php b/src/Action/CaptureAction.php index 28ee38a..621c93c 100644 --- a/src/Action/CaptureAction.php +++ b/src/Action/CaptureAction.php @@ -13,8 +13,9 @@ use Payum\Core\Exception\RequestNotSupportedException; use Payum\Core\Security\GenericTokenFactoryAwareTrait; use Payum\Core\Security\GenericTokenFactoryAwareInterface; +use PayumTW\Ecpay\Action\Api\BaseApiAwareAction; -class CaptureAction implements ActionInterface, GatewayAwareInterface, GenericTokenFactoryAwareInterface +class CaptureAction extends BaseApiAwareAction implements ActionInterface, GatewayAwareInterface, GenericTokenFactoryAwareInterface { use GatewayAwareTrait; use GenericTokenFactoryAwareTrait; @@ -33,7 +34,10 @@ public function execute($request) $this->gateway->execute($httpRequest); if (isset($httpRequest->request['RtnCode']) === true) { - $this->gateway->execute(new Sync($details)); + if ($this->api->verifyHash($httpRequest->request) === false) { + $httpRequest->request['RtnCode'] = '10400002'; + } + $details->replace($httpRequest->request); return; } diff --git a/src/Action/CaptureLogisticsAction.php b/src/Action/CaptureLogisticsAction.php index c8292b0..39e816c 100644 --- a/src/Action/CaptureLogisticsAction.php +++ b/src/Action/CaptureLogisticsAction.php @@ -13,8 +13,9 @@ use Payum\Core\Exception\RequestNotSupportedException; use Payum\Core\Security\GenericTokenFactoryAwareTrait; use Payum\Core\Security\GenericTokenFactoryAwareInterface; +use PayumTW\Ecpay\Action\Api\BaseApiAwareAction; -class CaptureLogisticsAction implements ActionInterface, GatewayAwareInterface, GenericTokenFactoryAwareInterface +class CaptureLogisticsAction extends BaseApiAwareAction implements ActionInterface, GatewayAwareInterface, GenericTokenFactoryAwareInterface { use GatewayAwareTrait; use GenericTokenFactoryAwareTrait; @@ -32,8 +33,9 @@ public function execute($request) $httpRequest = new GetHttpRequest(); $this->gateway->execute($httpRequest); + // CVS if (isset($httpRequest->request['CVSStoreID']) === true) { - $this->gateway->execute(new Sync($details)); + $details->replace($httpRequest->request); return; } @@ -41,7 +43,7 @@ public function execute($request) $token = $request->getToken(); $targetUrl = $token->getTargetUrl(); - if ($details['GoodsAmount'] === 0) { + if (empty($details['GoodsAmount']) === true) { if (empty($details['ServerReplyURL']) === true) { $details['ServerReplyURL'] = $targetUrl; } diff --git a/src/Action/ConvertPaymentLogisticsAction.php b/src/Action/ConvertPaymentLogisticsAction.php index c00552f..f6a6033 100644 --- a/src/Action/ConvertPaymentLogisticsAction.php +++ b/src/Action/ConvertPaymentLogisticsAction.php @@ -26,7 +26,7 @@ public function execute($request) $details['MerchantTradeNo'] = $payment->getNumber(); $details['ReceiverEmail'] = $payment->getClientEmail(); - $details['GoodsAmount'] = (int) $payment->getTotalAmount(); + $details['GoodsAmount'] = $payment->getTotalAmount(); $details['TradeDesc'] = $payment->getDescription(); $details['AllPayLogisticsID'] = $details['MerchantTradeNo']; diff --git a/src/Action/NotifyAction.php b/src/Action/NotifyAction.php index c346b77..abfb23c 100644 --- a/src/Action/NotifyAction.php +++ b/src/Action/NotifyAction.php @@ -34,7 +34,7 @@ public function execute($request) throw new HttpResponse('0|CheckMacValue verify fail.', 400, ['Content-Type' => 'text/plain']); } - $this->gateway->execute(new Sync($details)); + $details->replace($httpRequest->request); throw new HttpResponse('1|OK', 200, ['Content-Type' => 'text/plain']); } diff --git a/src/Action/RefundAction.php b/src/Action/RefundAction.php index 09d25d6..fc3df15 100644 --- a/src/Action/RefundAction.php +++ b/src/Action/RefundAction.php @@ -27,8 +27,6 @@ public function execute($request) $details = ArrayObject::ensureArrayObject($request->getModel()); $this->gateway->execute(new RefundTransaction($details)); - - $this->gateway->execute(new Sync($details)); } /** diff --git a/src/Action/SyncAction.php b/src/Action/SyncAction.php index 6dc1d3b..d5e7cf0 100644 --- a/src/Action/SyncAction.php +++ b/src/Action/SyncAction.php @@ -26,13 +26,6 @@ public function execute($request) $details = ArrayObject::ensureArrayObject($request->getModel()); - $httpRequest = new GetHttpRequest(); - $this->gateway->execute($httpRequest); - - $details->replace([ - 'response' => $httpRequest->request, - ]); - $this->gateway->execute(new GetTransactionData($details)); } diff --git a/src/EcpayApi.php b/src/EcpayApi.php index 871c38f..0a74836 100644 --- a/src/EcpayApi.php +++ b/src/EcpayApi.php @@ -170,19 +170,10 @@ public function refundTransaction($params) */ public function getTransactionData($params) { - $details = []; - if (empty($params['response']) === false) { - if ($this->verifyHash($params['response']) === false) { - $details['RtnCode'] = '10400002'; - } else { - $details = $params['response']; - } - } else { - $this->api->ServiceURL = $this->getApiEndpoint('QueryTradeInfo'); - $this->api->Query['MerchantTradeNo'] = $params['MerchantTradeNo']; - $details = $this->api->QueryTradeInfo(); - $details['RtnCode'] = $details['TradeStatus'] === '1' ? '1' : '2'; - } + $this->api->ServiceURL = $this->getApiEndpoint('QueryTradeInfo'); + $this->api->Query['MerchantTradeNo'] = $params['MerchantTradeNo']; + $details = $this->api->QueryTradeInfo(); + $details['RtnCode'] = $details['TradeStatus'] === '1' ? '1' : '2'; return $details; } diff --git a/src/EcpayLogisticsApi.php b/src/EcpayLogisticsApi.php index 4c66577..6c96fdf 100644 --- a/src/EcpayLogisticsApi.php +++ b/src/EcpayLogisticsApi.php @@ -183,10 +183,6 @@ public function createPrintFamilyC2CBillTransaction(array $params) */ public function createTransaction(array $params) { - if ($params['GoodsAmount'] === 0) { - return $this->createCvsMapTransaction($params); - } - $this->api->Send = array_merge($this->api->Send, [ 'MerchantTradeNo' => '', 'MerchantTradeDate' => date('Y/m/d H:i:s'), @@ -217,6 +213,9 @@ public function createTransaction(array $params) array_intersect_key($params, $this->api->Send) ); + $this->api->Send['GoodsAmount'] = (int) $this->api->Send['GoodsAmount']; + $this->api->Send['CollectionAmount'] = (int) $this->api->Send['CollectionAmount']; + if (empty($this->api->Send['LogisticsType']) === true) { $this->api->Send['LogisticsType'] = LogisticsType::CVS; switch ($this->api->Send['LogisticsSubType']) { @@ -380,13 +379,18 @@ public function cancelTransaction($params) */ public function getTransactionData($params) { - $details = []; - if (empty($params['response']) === false) { - $details = $params['response']; - } else { - $details = $params; - } + $supportedParams = [ + 'AllPayLogisticsID' => '', + 'PlatformID' => '', + ]; + + $this->api->Send = array_merge($this->api->Send, $supportedParams); + + $this->api->Send = array_replace( + $this->api->Send, + array_intersect_key($params, $this->api->Send) + ); - return $details; + return $this->api->QueryLogisticsInfo(); } } diff --git a/tests/Action/Api/CreateTransactionActionTest.php b/tests/Action/Api/CreateTransactionActionTest.php index 3220af7..a1ab566 100644 --- a/tests/Action/Api/CreateTransactionActionTest.php +++ b/tests/Action/Api/CreateTransactionActionTest.php @@ -24,6 +24,7 @@ public function test_api_http_post_redirect() $request = m::spy('PayumTW\Ecpay\Request\Api\CreateTransaction'); $details = new ArrayObject([ 'foo' => 'bar', + 'GoodsAmount' => 1 ]); $endpoint = 'foo.endpoint'; @@ -57,7 +58,6 @@ public function test_api_http_post_redirect() $request->shouldHaveReceived('getModel')->twice(); $api->shouldHaveReceived('createTransaction')->with((array) $details)->once(); - $api->shouldHaveReceived('getApiEndpoint')->once(); } public function test_logistics_api_http_post_redirect() @@ -85,7 +85,7 @@ public function test_logistics_api_http_post_redirect() ->shouldReceive('getModel')->andReturn($details); $api - ->shouldReceive('createTransaction')->andReturn((array) $details) + ->shouldReceive('createCvsMapTransaction')->andReturn((array) $details) ->shouldReceive('getApiEndpoint')->andReturn($endpoint); $action = new CreateTransactionAction(); @@ -104,7 +104,7 @@ public function test_logistics_api_http_post_redirect() } $request->shouldHaveReceived('getModel')->twice(); - $api->shouldHaveReceived('createTransaction')->with((array) $details)->once(); + $api->shouldHaveReceived('createCvsMapTransaction')->with((array) $details)->once(); $api->shouldHaveReceived('getApiEndpoint')->once(); } @@ -118,7 +118,10 @@ public function test_logistics_api_when_isset_rtn_code() $api = m::spy('PayumTW\Ecpay\Api'); $request = m::spy('PayumTW\Ecpay\Request\Api\CreateTransaction'); - $details = new ArrayObject(['RtnCode' => '300']); + $details = new ArrayObject([ + 'RtnCode' => '300', + 'GoodsAmount' => 1 + ]); /* |------------------------------------------------------------ diff --git a/tests/Action/CancelActionTest.php b/tests/Action/CancelActionTest.php index aed6061..ac3a5bf 100644 --- a/tests/Action/CancelActionTest.php +++ b/tests/Action/CancelActionTest.php @@ -44,6 +44,5 @@ public function test_cancel() $request->shouldHaveReceived('getModel')->twice(); $gateway->shouldHaveReceived('execute')->with(m::type('PayumTW\Ecpay\Request\Api\CancelTransaction'))->once(); - $gateway->shouldHaveReceived('execute')->with(m::type('Payum\Core\Request\Sync'))->once(); } } diff --git a/tests/Action/CaptureActionTest.php b/tests/Action/CaptureActionTest.php index b83da12..6e60b80 100644 --- a/tests/Action/CaptureActionTest.php +++ b/tests/Action/CaptureActionTest.php @@ -79,6 +79,7 @@ public function test_execute_when_rtn_code_exists() |------------------------------------------------------------ */ + $api = m::spy('PayumTW\Ecpay\Api'); $gateway = m::spy('Payum\Core\GatewayInterface'); $tokenFactory = m::spy('Payum\Core\Security\GenericTokenFactoryInterface'); $request = m::spy('Payum\Core\Request\Capture'); @@ -106,6 +107,7 @@ public function test_execute_when_rtn_code_exists() }); $action = new CaptureAction(); + $action->setApi($api); $action->setGateway($gateway); $action->setGenericTokenFactory($tokenFactory); $action->execute($request); @@ -118,6 +120,5 @@ public function test_execute_when_rtn_code_exists() $request->shouldHaveReceived('getModel')->twice(); $gateway->shouldHaveReceived('execute')->with(m::type('Payum\Core\Request\GetHttpRequest'))->once(); - $gateway->shouldHaveReceived('execute')->with(m::type('Payum\Core\Request\Sync'))->once(); } } diff --git a/tests/Action/CaptureLogisticsActionTest.php b/tests/Action/CaptureLogisticsActionTest.php index 38ab0de..df8ea95 100644 --- a/tests/Action/CaptureLogisticsActionTest.php +++ b/tests/Action/CaptureLogisticsActionTest.php @@ -168,6 +168,5 @@ public function test_execute_when_csv_store_id_exists() $request->shouldHaveReceived('getModel')->twice(); $gateway->shouldHaveReceived('execute')->with(m::type('Payum\Core\Request\GetHttpRequest'))->once(); - $gateway->shouldHaveReceived('execute')->with(m::type('Payum\Core\Request\Sync'))->once(); } } diff --git a/tests/Action/NotifyActionTest.php b/tests/Action/NotifyActionTest.php index 8c68fca..148706a 100644 --- a/tests/Action/NotifyActionTest.php +++ b/tests/Action/NotifyActionTest.php @@ -67,7 +67,6 @@ public function test_notify_success() $request->shouldHaveReceived('getModel')->twice(); $gateway->shouldHaveReceived('execute')->with(m::type('Payum\Core\Request\GetHttpRequest'))->once(); $api->shouldHaveReceived('verifyHash')->with($returnValue)->once(); - $gateway->shouldHaveReceived('execute')->with(m::type('Payum\Core\Request\Sync'))->once(); } public function test_notify_when_checksum_fail() diff --git a/tests/Action/RefundActionTest.php b/tests/Action/RefundActionTest.php index 7b690ac..b110ccc 100644 --- a/tests/Action/RefundActionTest.php +++ b/tests/Action/RefundActionTest.php @@ -44,6 +44,5 @@ public function test_refund() $request->shouldHaveReceived('getModel')->twice(); $gateway->shouldHaveReceived('execute')->with(m::type('PayumTW\Ecpay\Request\Api\RefundTransaction'))->once(); - $gateway->shouldHaveReceived('execute')->with(m::type('Payum\Core\Request\Sync'))->once(); } } diff --git a/tests/EcpayApiTest.php b/tests/EcpayApiTest.php index f896d96..90512dc 100644 --- a/tests/EcpayApiTest.php +++ b/tests/EcpayApiTest.php @@ -213,142 +213,6 @@ public function test_refund_transaction() $sdk->shouldHaveReceived('AioChargeback')->once(); } - public function test_get_transaction_data_when_response_from_request() - { - /* - |------------------------------------------------------------ - | Arrange - |------------------------------------------------------------ - */ - - $httpClient = m::spy('Payum\Core\HttpClientInterface'); - $message = m::spy('Http\Message\MessageFactory'); - $sdk = m::spy('PayumTW\Ecpay\Bridge\Ecpay\AllInOne'); - - $sdk->ChargeBack = [ - 'MerchantTradeNo' => '', - 'TradeNo' => '', - 'ChargeBackTotalAmount' => 0, - 'Remark' => '', - ]; - - $options = [ - 'MerchantID' => '2000132', - 'HashKey' => '5294y06JbISpM5x9', - 'HashIV' => 'v77hoKGq4kWxNNIS', - 'sandbox' => false, - ]; - - $params = [ - 'response' => [ - 'MerchantID' => '2000132', - 'MerchantTradeNo' => '57CBC66A39F82', - 'PayAmt' => '340', - 'PaymentDate' => '2016/09/04 15:03:08', - 'PaymentType' => 'Credit_CreditCard', - 'PaymentTypeChargeFee' => '3', - 'RedeemAmt' => '0', - 'RtnCode' => '1', - 'RtnMsg' => 'Succeeded', - 'SimulatePaid' => '0', - 'TradeAmt' => '340', - 'TradeDate' => '2016/09/04 14:59:13', - 'TradeNo' => '1609041459136128', - 'CheckMacValue' => '6812D213BF2C5B9377EBF101607BF2DF', - ], - ]; - - /* - |------------------------------------------------------------ - | Act - |------------------------------------------------------------ - */ - - $api = new EcpayApi($options, $httpClient, $message, $sdk); - - /* - |------------------------------------------------------------ - | Assert - |------------------------------------------------------------ - */ - - $this->assertSame($params['response'], $api->getTransactionData($params)); - $this->assertSame($options['HashKey'], $sdk->HashKey); - $this->assertSame($options['HashIV'], $sdk->HashIV); - $this->assertSame($options['MerchantID'], $sdk->MerchantID); - } - - public function test_get_transaction_data_when_response_from_request_and_verify_hash_is_fail() - { - /* - |------------------------------------------------------------ - | Arrange - |------------------------------------------------------------ - */ - - $httpClient = m::spy('Payum\Core\HttpClientInterface'); - $message = m::spy('Http\Message\MessageFactory'); - $sdk = m::spy('PayumTW\Ecpay\Bridge\Ecpay\AllInOne'); - - $sdk->ChargeBack = [ - 'MerchantTradeNo' => '', - 'TradeNo' => '', - 'ChargeBackTotalAmount' => 0, - 'Remark' => '', - ]; - - $options = [ - 'MerchantID' => '2000132', - 'HashKey' => '5294y06JbISpM5x9', - 'HashIV' => 'v77hoKGq4kWxNNIS', - 'sandbox' => false, - ]; - - $params = [ - 'response' => [ - 'MerchantID' => '2000132', - 'MerchantTradeNo' => '57CBC66A39F82', - 'PayAmt' => '340', - 'PaymentDate' => '2016/09/04 15:03:08', - 'PaymentType' => 'Credit_CreditCard', - 'PaymentTypeChargeFee' => '3', - 'RedeemAmt' => '0', - 'RtnCode' => '1', - 'RtnMsg' => 'Succeeded', - 'SimulatePaid' => '0', - 'TradeAmt' => '340', - 'TradeDate' => '2016/09/04 14:59:13', - 'TradeNo' => '1609041459136128', - 'CheckMacValue' => '', - ], - ]; - - /* - |------------------------------------------------------------ - | Act - |------------------------------------------------------------ - */ - - $sdk->shouldReceive('CheckOutFeedback')->andThrow('Exception'); - - $api = new EcpayApi($options, $httpClient, $message, $sdk); - - /* - |------------------------------------------------------------ - | Assert - |------------------------------------------------------------ - */ - - $this->assertSame([ - 'RtnCode' => '10400002', - ], $api->getTransactionData($params)); - $this->assertSame($options['HashKey'], $sdk->HashKey); - $this->assertSame($options['HashIV'], $sdk->HashIV); - $this->assertSame($options['MerchantID'], $sdk->MerchantID); - - $sdk->shouldHaveReceived('CheckOutFeedback')->once(); - } - public function test_get_transaction_data_when_response_from_query_info() { /* diff --git a/tests/EcpayLogisticsApiTest.php b/tests/EcpayLogisticsApiTest.php index 1837cc4..a75136b 100644 --- a/tests/EcpayLogisticsApiTest.php +++ b/tests/EcpayLogisticsApiTest.php @@ -54,7 +54,7 @@ public function test_create_cvs_map_transaction() |------------------------------------------------------------ */ - $api->createTransaction($params); + $api->createCvsMapTransaction($params); $this->assertSame($params['ServerReplyURL'], $sdk->Send['ServerReplyURL']); $this->assertSame($params['MerchantTradeNo'], $sdk->Send['MerchantTradeNo']); $this->assertSame($params['LogisticsSubType'], $sdk->Send['LogisticsSubType']); @@ -82,13 +82,12 @@ public function test_create_transaction() ]; $params = [ - 'GoodsAmount' => 1, 'MerchantTradeNo' => 'MerchantTradeNo', 'MerchantTradeDate' => 'MerchantTradeDate', 'LogisticsType' => 'LogisticsType', 'LogisticsSubType' => 'LogisticsSubType', - 'GoodsAmount' => 'GoodsAmount', - 'CollectionAmount' => 'CollectionAmount', + 'GoodsAmount' => 1, + 'CollectionAmount' => 1, 'IsCollection' => 'IsCollection', 'GoodsName' => 'GoodsName', 'SenderName' => 'SenderName', @@ -143,44 +142,4 @@ public function test_create_transaction() $sdk->shouldHaveReceived('BGCreateShippingOrder')->once(); } - - public function test_get_transaction_data() - { - /* - |------------------------------------------------------------ - | Arrange - |------------------------------------------------------------ - */ - - $httpClient = m::spy('Payum\Core\HttpClientInterface'); - $message = m::spy('Http\Message\MessageFactory'); - $sdk = m::spy('PayumTW\Ecpay\Bridge\Ecpay\EcpayLogistics'); - $options = [ - 'MerchantID' => '2000132', - 'HashKey' => '5294y06JbISpM5x9', - 'HashIV' => 'v77hoKGq4kWxNNIS', - 'sandbox' => false, - ]; - - /* - |------------------------------------------------------------ - | Act - |------------------------------------------------------------ - */ - - $api = new EcpayLogisticsApi($options, $httpClient, $message, $sdk); - - /* - |------------------------------------------------------------ - | Assert - |------------------------------------------------------------ - */ - - $params = ['foo' => 'bar']; - $this->assertSame($params, $api->getTransactionData($params)); - $params = [ - 'response' => ['foo' => 'bar'], - ]; - $this->assertSame($params['response'], $api->getTransactionData($params)); - } }