Ejegg has uploaded a new change for review. https://gerrit.wikimedia.org/r/325713
Change subject: Update SmashPig library ...................................................................... Update SmashPig library Change-Id: Ifa66a5822a66939cb0885bfbd9c4b501c1a12e6d --- M composer/installed.json M wikimedia/smash-pig/Core/DataStores/PaymentsInitialDatabase.php M wikimedia/smash-pig/Core/DataStores/PendingDatabase.php M wikimedia/smash-pig/Core/QueueConsumers/BaseQueueConsumer.php M wikimedia/smash-pig/Maintenance/ConsumePendingQueue.php A wikimedia/smash-pig/Maintenance/DeleteExpiredPendingMessages.php M wikimedia/smash-pig/PaymentProviders/Adyen/Jobs/RecordCaptureJob.php M wikimedia/smash-pig/PaymentProviders/Adyen/Tests/config_test.yaml M wikimedia/smash-pig/PaymentProviders/Adyen/Tests/phpunit/RecordCaptureJobTest.php M wikimedia/smash-pig/PaymentProviders/Amazon/Actions/AssociateRefundParent.php M wikimedia/smash-pig/PaymentProviders/Amazon/Actions/CloseOrderReference.php A wikimedia/smash-pig/PaymentProviders/Amazon/Actions/ReconstructMerchantReference.php M wikimedia/smash-pig/PaymentProviders/Amazon/AmazonApi.php M wikimedia/smash-pig/PaymentProviders/Amazon/ExpatriatedMessages/AmazonMessage.php M wikimedia/smash-pig/PaymentProviders/Amazon/ExpatriatedMessages/PaymentCapture.php A wikimedia/smash-pig/PaymentProviders/Amazon/Tests/AmazonTestCase.php M wikimedia/smash-pig/PaymentProviders/Amazon/Tests/Data/responses/getOrderReferenceDetails.json A wikimedia/smash-pig/PaymentProviders/Amazon/Tests/phpunit/ActionsTest.php M wikimedia/smash-pig/PaymentProviders/Amazon/Tests/phpunit/ApiTest.php M wikimedia/smash-pig/PaymentProviders/PayPal/Job.php M wikimedia/smash-pig/PaymentProviders/PayPal/Listener.php M wikimedia/smash-pig/PaymentProviders/PayPal/PayPalPaymentsAPI.php M wikimedia/smash-pig/PaymentProviders/PayPal/Tests/MockPayPalPaymentsAPI.php M wikimedia/smash-pig/PaymentProviders/PayPal/Tests/phpunit/CaptureIncomingMessageTest.php M wikimedia/smash-pig/SmashPig.yaml 25 files changed, 466 insertions(+), 116 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/wikimedia/fundraising/crm/vendor refs/changes/13/325713/1 diff --git a/composer/installed.json b/composer/installed.json index fedd4ca..9597ac1 100644 --- a/composer/installed.json +++ b/composer/installed.json @@ -1224,7 +1224,7 @@ "source": { "type": "git", "url": "https://gerrit.wikimedia.org/r/wikimedia/fundraising/SmashPig.git", - "reference": "da8e788fa090599d5df2c54abbf6d8a70460f740" + "reference": "bbba45f67287ea94afd4eec5a982685ff33e23be" }, "require": { "amzn/login-and-pay-with-amazon-sdk-php": "dev-master", @@ -1241,7 +1241,7 @@ "jakub-onderka/php-parallel-lint": "^0.9", "phpunit/phpunit": "^4.8" }, - "time": "2016-11-15 19:22:43", + "time": "2016-12-06 19:11:55", "type": "library", "installation-source": "source", "autoload": { diff --git a/wikimedia/smash-pig/Core/DataStores/PaymentsInitialDatabase.php b/wikimedia/smash-pig/Core/DataStores/PaymentsInitialDatabase.php index d40238c..857377e 100644 --- a/wikimedia/smash-pig/Core/DataStores/PaymentsInitialDatabase.php +++ b/wikimedia/smash-pig/Core/DataStores/PaymentsInitialDatabase.php @@ -75,6 +75,25 @@ $this->prepareAndExecute( $sql, $message ); } + public function updatePaymentStatus( + $gateway, $contributionTrackingId, $orderId, $status + ) { + $sql = 'UPDATE payments_initial + SET payments_final_status = :status + WHERE gateway = :gateway + AND contribution_tracking_id = :ct_id + AND order_id = :order_id'; + + $params = array( + 'gateway' => $gateway, + 'ct_id' => $contributionTrackingId, + 'order_id' => $orderId, + 'status' => $status + ); + + $this->prepareAndExecute( $sql, $params ); + } + protected function getConfigKey() { return 'data-store/fredge-db'; } diff --git a/wikimedia/smash-pig/Core/DataStores/PendingDatabase.php b/wikimedia/smash-pig/Core/DataStores/PendingDatabase.php index 4d9bcfc..c54e4b3 100644 --- a/wikimedia/smash-pig/Core/DataStores/PendingDatabase.php +++ b/wikimedia/smash-pig/Core/DataStores/PendingDatabase.php @@ -159,6 +159,26 @@ } /** + * Delete expired messages, optionally by gateway + * + * @param int $originalDate Oldest date to keep + * @param string|null $gateway + * @return int Number of rows deleted + */ + public function deleteOldMessages( $originalDate, $gateway = null ) { + $sql = 'DELETE FROM pending WHERE date < :date'; + $params = array( + 'date' => UtcDate::getUtcDatabaseString( $originalDate ), + ); + if ( $gateway ) { + $sql .= ' AND gateway = :gateway'; + $params['gateway'] = $gateway; + } + $executed = $this->prepareAndExecute( $sql, $params ); + return $executed->rowCount(); + } + + /** * Parse a database row and return the normalized message. */ protected function messageFromDbRow( $row ) { diff --git a/wikimedia/smash-pig/Core/QueueConsumers/BaseQueueConsumer.php b/wikimedia/smash-pig/Core/QueueConsumers/BaseQueueConsumer.php index 5367c51..05b72b9 100644 --- a/wikimedia/smash-pig/Core/QueueConsumers/BaseQueueConsumer.php +++ b/wikimedia/smash-pig/Core/QueueConsumers/BaseQueueConsumer.php @@ -101,10 +101,18 @@ } $timeOk = $this->timeLimit === 0 || time() <= $startTime + $this->timeLimit; $countOk = $this->messageLimit === 0 || $processed < $this->messageLimit; - $debugMessage = 'Data is ' . ( $data === null ? '' : 'not ' ) . 'null, ' . - "time limit ($this->timeLimit) is " . ( $timeOk ? 'not ' : '' ) . 'elapsed, ' . - "message limit ($this->messageLimit) is " . ( $countOk ? 'not ' : '' ) . 'reached.'; - Logger::debug( $debugMessage ); + + $debugMessages = array(); + if ( $data === null ) { + $debugMessages[] = 'Queue is empty.'; + } else if ( !$timeOk ) { + $debugMessages[] = "Time limit ($this->timeLimit) is elapsed."; + } else if ( !$countOk ) { + $debugMessages[] = "Message limit ($this->messageLimit) is reached."; + } + if ( !empty( $debugMessages ) ) { + Logger::debug( implode( ' ', $debugMessages ) ); + } } while( $timeOk && $countOk && $data !== null ); return $processed; diff --git a/wikimedia/smash-pig/Maintenance/ConsumePendingQueue.php b/wikimedia/smash-pig/Maintenance/ConsumePendingQueue.php index 197e319..cabdd40 100644 --- a/wikimedia/smash-pig/Maintenance/ConsumePendingQueue.php +++ b/wikimedia/smash-pig/Maintenance/ConsumePendingQueue.php @@ -4,7 +4,6 @@ require ( 'MaintenanceBase.php' ); use SmashPig\Core\Logging\Logger; -use SmashPig\Core\DataStores\PendingDatabase; use SmashPig\Core\QueueConsumers\PendingQueueConsumer; $maintClass = '\SmashPig\Maintenance\ConsumePendingQueue'; @@ -13,11 +12,6 @@ * Reads messages out of the pending queue and inserts them into a db table */ class ConsumePendingQueue extends MaintenanceBase { - - /** - * @var PendingDatabase - */ - protected $pendingDatabase; public function __construct() { parent::__construct(); diff --git a/wikimedia/smash-pig/Maintenance/DeleteExpiredPendingMessages.php b/wikimedia/smash-pig/Maintenance/DeleteExpiredPendingMessages.php new file mode 100644 index 0000000..8d2977f --- /dev/null +++ b/wikimedia/smash-pig/Maintenance/DeleteExpiredPendingMessages.php @@ -0,0 +1,42 @@ +<?php +namespace SmashPig\Maintenance; + +require ( 'MaintenanceBase.php' ); + +use SmashPig\Core\Logging\Logger; +use SmashPig\Core\DataStores\PendingDatabase; +use SmashPig\Core\UtcDate; + +$maintClass = '\SmashPig\Maintenance\DeleteExpiredPendingMessages'; + +/** + * Deletes old messages from the pending table + */ +class DeleteExpiredPendingMessages extends MaintenanceBase { + + public function __construct() { + parent::__construct(); + $this->addOption( 'gateway', 'gateway to delete messages for' ); + $this->addOption( 'days', 'age in days of oldest messages to keep', 30 ); + } + + /** + * Do the actual work of the script. + */ + public function execute() { + $pendingDatabase = PendingDatabase::get(); + $gateway = $this->getOption( 'gateway' ); + $days = $this->getOption( 'days' ); + $deleteBefore = UtcDate::getUtcTimestamp( "-$days days" ); + + $startTime = time(); + $deleted = $pendingDatabase->deleteOldMessages( $deleteBefore, $gateway ); + + $elapsedTime = time() - $startTime; + Logger::info( + "Deleted $deleted pending messages in $elapsedTime seconds." + ); + } +} + +require ( RUN_MAINTENANCE_IF_MAIN ); diff --git a/wikimedia/smash-pig/PaymentProviders/Adyen/Jobs/RecordCaptureJob.php b/wikimedia/smash-pig/PaymentProviders/Adyen/Jobs/RecordCaptureJob.php index bdcb117..8531ed8 100644 --- a/wikimedia/smash-pig/PaymentProviders/Adyen/Jobs/RecordCaptureJob.php +++ b/wikimedia/smash-pig/PaymentProviders/Adyen/Jobs/RecordCaptureJob.php @@ -1,6 +1,7 @@ <?php namespace SmashPig\PaymentProviders\Adyen\Jobs; use SmashPig\Core\Configuration; +use SmashPig\Core\DataStores\PaymentsInitialDatabase; use SmashPig\Core\DataStores\PendingDatabase; use SmashPig\Core\Jobs\RunnableJob; use SmashPig\Core\Logging\Logger; @@ -60,6 +61,14 @@ SourceFields::addToMessage( $queueMessage ); $config->object( 'data-store/verified' )->push( $queueMessage ); + PaymentsInitialDatabase::get() + ->updatePaymentStatus( + 'adyen', + $dbMessage['contribution_tracking_id'], + $dbMessage['order_id'], + 'complete' + ); + // Remove it from the pending database $logger->debug( 'Removing donor details message from pending database' ); $db->deleteMessage( $dbMessage ); diff --git a/wikimedia/smash-pig/PaymentProviders/Adyen/Tests/config_test.yaml b/wikimedia/smash-pig/PaymentProviders/Adyen/Tests/config_test.yaml index 33e8128..6f249d0 100644 --- a/wikimedia/smash-pig/PaymentProviders/Adyen/Tests/config_test.yaml +++ b/wikimedia/smash-pig/PaymentProviders/Adyen/Tests/config_test.yaml @@ -24,6 +24,11 @@ constructor-parameters: - 'sqlite::memory:' + fredge-db: + class: PDO + constructor-parameters: + - 'sqlite::memory:' + payment-provider: adyen: api: diff --git a/wikimedia/smash-pig/PaymentProviders/Adyen/Tests/phpunit/RecordCaptureJobTest.php b/wikimedia/smash-pig/PaymentProviders/Adyen/Tests/phpunit/RecordCaptureJobTest.php index ce3edf9..f59e1b2 100644 --- a/wikimedia/smash-pig/PaymentProviders/Adyen/Tests/phpunit/RecordCaptureJobTest.php +++ b/wikimedia/smash-pig/PaymentProviders/Adyen/Tests/phpunit/RecordCaptureJobTest.php @@ -3,11 +3,14 @@ use SmashPig\Core\Configuration; use SmashPig\Core\Context; use SmashPig\Core\DataStores\KeyedOpaqueStorableObject; +use SmashPig\Core\DataStores\PaymentsInitialDatabase; use SmashPig\Core\DataStores\PendingDatabase; use SmashPig\Core\QueueConsumers\BaseQueueConsumer; use SmashPig\PaymentProviders\Adyen\Jobs\RecordCaptureJob; use SmashPig\PaymentProviders\Adyen\Tests\AdyenTestConfiguration; use SmashPig\Tests\BaseSmashPigUnitTestCase; +use SmashPig\Tests\PaymentsInitialDatabaseTest; +use SmashPig\Tests\TestingDatabase; /** * Verify Adyen RecordCapture job functions @@ -23,21 +26,43 @@ */ protected $pendingDatabase; protected $pendingMessage; + /** + * @var PaymentsInitialDatabase + */ + protected $paymentsInitDatabase; + protected $paymentsInitMessage; public function setUp() { parent::setUp(); $this->config = AdyenTestConfiguration::createWithSuccessfulApi(); Context::initWithLogger( $this->config ); + + $this->paymentsInitDatabase = PaymentsInitialDatabase::get(); + $this->paymentsInitDatabase->createTable(); + $this->paymentsInitMessage = PaymentsInitialDatabaseTest::generateTestMessage(); + $this->paymentsInitMessage['gateway'] = 'adyen'; + $this->paymentsInitMessage['payments_final_status'] = 'pending'; + $this->paymentsInitDatabase->storeMessage( + $this->paymentsInitMessage + ); + $this->pendingDatabase = PendingDatabase::get(); + $this->pendingDatabase->createTable(); $this->pendingMessage = json_decode( file_get_contents( __DIR__ . '/../Data/pending.json' ) , true ); + foreach( array( 'order_id', 'contribution_tracking_id' ) as $key ) { + $this->pendingMessage[$key] = $this->paymentsInitMessage[$key]; + } + $this->pendingMessage['correlationId'] = 'adyen-' . + $this->pendingMessage['order_id']; $this->pendingMessage['captured'] = true; $this->pendingDatabase->storeMessage( $this->pendingMessage ); } public function tearDown() { - $this->pendingDatabase->deleteMessage( $this->pendingMessage ); + TestingDatabase::clearStatics( $this->paymentsInitDatabase ); + TestingDatabase::clearStatics( $this->pendingDatabase ); parent::tearDown(); } @@ -49,6 +74,7 @@ 'SmashPig\PaymentProviders\Adyen\ExpatriatedMessages\Capture', file_get_contents( __DIR__ . '/../Data/capture.json' ) ); + $capture->merchantReference = $this->pendingMessage['order_id']; $job = RecordCaptureJob::factory( $capture ); $this->assertTrue( $job->execute() ); @@ -86,5 +112,15 @@ ); } } + $initMessage = $this->paymentsInitDatabase + ->fetchMessageByGatewayOrderId( + 'adyen', $this->pendingMessage['order_id'] + ); + + $this->assertEquals( + 'complete', + $initMessage['payments_final_status'], + 'Did not mark payments_initial row as complete' + ); } } diff --git a/wikimedia/smash-pig/PaymentProviders/Amazon/Actions/AssociateRefundParent.php b/wikimedia/smash-pig/PaymentProviders/Amazon/Actions/AssociateRefundParent.php index 094ca66..9f790d0 100644 --- a/wikimedia/smash-pig/PaymentProviders/Amazon/Actions/AssociateRefundParent.php +++ b/wikimedia/smash-pig/PaymentProviders/Amazon/Actions/AssociateRefundParent.php @@ -5,22 +5,23 @@ use SmashPig\Core\Messages\ListenerMessage; use SmashPig\Core\SmashPigException; use SmashPig\PaymentProviders\Amazon\AmazonApi; +use SmashPig\PaymentProviders\Amazon\ExpatriatedMessages\RefundCompleted; /** * Associate refunds with their parent contribution */ class AssociateRefundParent implements IListenerMessageAction { - const MESSAGE_CLASS = 'SmashPig\PaymentProviders\Amazon\ExpatriatedMessages\RefundCompleted'; public function execute( ListenerMessage $msg ) { // Bail out if not a refund - if ( get_class( $msg ) !== self::MESSAGE_CLASS ) { + if ( !( $msg instanceof RefundCompleted ) ) { return true; } - $refundId = $msg->getGatewayTransactionId(); - Logger::info( "Looking up ID of original transaction for refund $refundId" ); + + $orderReferenceId = $msg->getOrderReferenceId(); + Logger::info( "Looking up capture ID for order reference $orderReferenceId" ); try { - $parentId = AmazonApi::findRefundParentId( $refundId ); + $parentId = AmazonApi::get()->findCaptureId( $orderReferenceId ); $msg->setParentId( $parentId ); return true; } catch ( SmashPigException $ex ) { diff --git a/wikimedia/smash-pig/PaymentProviders/Amazon/Actions/CloseOrderReference.php b/wikimedia/smash-pig/PaymentProviders/Amazon/Actions/CloseOrderReference.php index 34acd0e..13bf1ad 100644 --- a/wikimedia/smash-pig/PaymentProviders/Amazon/Actions/CloseOrderReference.php +++ b/wikimedia/smash-pig/PaymentProviders/Amazon/Actions/CloseOrderReference.php @@ -1,35 +1,48 @@ <?php namespace SmashPig\PaymentProviders\Amazon\Actions; +use Exception; use SmashPig\Core\Actions\IListenerMessageAction; use SmashPig\Core\Context; use SmashPig\Core\Logging\Logger; use SmashPig\Core\Messages\ListenerMessage; +use SmashPig\PaymentProviders\Amazon\ExpatriatedMessages\CaptureCompleted; class CloseOrderReference implements IListenerMessageAction { - const MESSAGE_CLASS = 'SmashPig\PaymentProviders\Amazon\ExpatriatedMessages\CaptureCompleted'; public function execute( ListenerMessage $msg ) { // only close after successful capture - if ( get_class( $msg ) !== self::MESSAGE_CLASS ) { + if ( !( $msg instanceof CaptureCompleted ) ) { return true; } $config = Context::get()->getConfiguration(); $client = $config->object( 'payments-client', true ); - $captureId = $msg->getGatewayTransactionId(); - $orderReferenceId = substr( $captureId, 0, 19 ); + $orderReferenceId = $msg->getOrderReferenceId(); Logger::info( "Closing order reference $orderReferenceId" ); - $response = $client->closeOrderReference( array( - 'amazon_order_reference_id' => $orderReferenceId, - ) )->toArray(); - if ( !empty( $response['Error'] ) ) { - Logger::info( - "Error losing order reference $orderReferenceId: " . - $response['Error']['Code'] . ': ' . - $response['Error']['Message'] + // Failure is unexpected, but shouldn't stop us recording + // the successful capture + try { + $response = $client->closeOrderReference( + array( + 'amazon_order_reference_id' => $orderReferenceId, + ) + )->toArray(); + + if ( !empty( $response['Error'] ) ) { + Logger::warning( + "Error closing order reference $orderReferenceId: " . + $response['Error']['Code'] . ': ' . + $response['Error']['Message'] + ); + return false; + } + } catch( Exception $ex ) { + Logger::warning( + "Error closing order reference $orderReferenceId: " . + $ex->getMessage() ); return false; } diff --git a/wikimedia/smash-pig/PaymentProviders/Amazon/Actions/ReconstructMerchantReference.php b/wikimedia/smash-pig/PaymentProviders/Amazon/Actions/ReconstructMerchantReference.php new file mode 100644 index 0000000..3bda459 --- /dev/null +++ b/wikimedia/smash-pig/PaymentProviders/Amazon/Actions/ReconstructMerchantReference.php @@ -0,0 +1,42 @@ +<?php namespace SmashPig\PaymentProviders\Amazon\Actions; + +use SmashPig\Core\Actions\IListenerMessageAction; +use SmashPig\Core\Logging\Logger; +use SmashPig\Core\Messages\ListenerMessage; +use SmashPig\Core\SmashPigException; +use SmashPig\PaymentProviders\Amazon\AmazonApi; +use SmashPig\PaymentProviders\Amazon\ExpatriatedMessages\PaymentCapture; +use SmashPig\PaymentProviders\Amazon\Tests\AmazonTestConfiguration; + +/** + * Looks up our reference ID for transactions pushed through manually + */ +class ReconstructMerchantReference implements IListenerMessageAction { + + public function execute( ListenerMessage $msg ) { + // Bail out if not a PaymentCapture + if ( !( $msg instanceof PaymentCapture ) ) { + return true; + } + $captureReference = $msg->getOrderId(); + if ( substr( $captureReference, 0, 10 ) !== 'AUTHORIZE_' ) { + // We only have to fix Amazon-generated IDs with that prefix + return true; + } + + $orderReferenceId = $msg->getOrderReferenceId(); + Logger::info( + "Looking up merchant reference for OrderReference $orderReferenceId" + ); + try { + $orderId = AmazonApi::get()->findMerchantReference( $orderReferenceId ); + if ( $orderId ) { + $msg->setOrderId( $orderId ); + } + return true; + } catch ( SmashPigException $ex ) { + Logger::error( $ex->getMessage() ); + return false; + } + } +} diff --git a/wikimedia/smash-pig/PaymentProviders/Amazon/AmazonApi.php b/wikimedia/smash-pig/PaymentProviders/Amazon/AmazonApi.php index d02fc51..6e27411 100644 --- a/wikimedia/smash-pig/PaymentProviders/Amazon/AmazonApi.php +++ b/wikimedia/smash-pig/PaymentProviders/Amazon/AmazonApi.php @@ -1,6 +1,7 @@ <?php namespace SmashPig\PaymentProviders\Amazon; use PayWithAmazon\IpnHandlerInterface; +use PayWithAmazon\PaymentsClientInterface; use SmashPig\Core\Context; use SmashPig\Core\SmashPigException; @@ -10,8 +11,30 @@ class AmazonApi { /** - * @param $headers - * @param $body + * @var PaymentsClientInterface + */ + protected $client; + + /** + * @var AmazonApi + */ + protected static $instance; + + private function __construct() { + $config = Context::get()->getConfiguration(); + $this->client = $config->object( 'payments-client', true ); + } + + public static function get() { + if ( !self::$instance ) { + self::$instance = new self(); + } + return self::$instance; + } + + /** + * @param array $headers Associative array of HTTP headers + * @param string $body HTTP request body (should be JSON-encoded) * @return IpnHandlerInterface */ public static function createIpnHandler( $headers, $body ) { @@ -20,24 +43,18 @@ return new $klass( $headers, $body ); } - public static function findRefundParentId( $refundId ) { - $config = Context::get()->getConfiguration(); - $client = $config->object( 'payments-client', true ); - - // The order reference ID is the first 19 characters of the refund ID - $orderReferenceId = substr( $refundId, 0, 19 ); - - $getDetailsResult = $client->getOrderReferenceDetails( array( - 'amazon_order_reference_id' => $orderReferenceId, - ) )->toArray(); - if ( !empty( $getDetailsResult['Error'] ) ) { - throw new SmashPigException( $getDetailsResult['Error']['Message'] ); - } - + /** + * @param string $orderReferenceId + * @return string Amazon's ID for the first successful capture associated + * with this order reference + * @throws SmashPigException + */ + public function findCaptureId( $orderReferenceId ) { // The order reference details should contain an IdList with all of the // authorizations that have been made against the order reference. We // should only ever have one authorization per order reference. - $details = $getDetailsResult['GetOrderReferenceDetailsResult']['OrderReferenceDetails']; + $details = $this->getOrderReferenceDetails( $orderReferenceId ); + if ( !isset( $details['IdList'] ) || !isset( $details['IdList']['member'] ) ) { throw new SmashPigException( "No authorizations found for order reference $orderReferenceId!" @@ -46,7 +63,7 @@ $authorizationIds = (array) $details['IdList']['member']; // Check the status of each authorization against the order reference foreach ( $authorizationIds as $id ) { - $authResult = $client->getAuthorizationDetails( array( + $authResult = $this->client->getAuthorizationDetails( array( 'amazon_authorization_id' => $id, ) )->toArray(); if ( !empty( $authResult['Error'] ) ) { @@ -68,4 +85,36 @@ "No successful authorizations found for order reference $orderReferenceId!" ); } + + /** + * @param string $orderReferenceId + * @return string|null Merchant reference for the order ID, or null if + * not set + */ + public function findMerchantReference( $orderReferenceId ) { + $details = $this->getOrderReferenceDetails( $orderReferenceId ); + + if ( isset( $details['SellerOrderAttributes']['SellerOrderId'] ) ) { + return $details['SellerOrderAttributes']['SellerOrderId']; + } + return null; + } + + /** + * @param string $orderReferenceId 19 character Amazon order ID + * @return array OrderReferenceDetails as an associative array + * @see https://payments.amazon.com/documentation/apireference/201752660 + * @throws SmashPigException + */ + protected function getOrderReferenceDetails( $orderReferenceId ) { + $getDetailsResult = $this->client->getOrderReferenceDetails( + array( + 'amazon_order_reference_id' => $orderReferenceId, + ) + )->toArray(); + if ( !empty( $getDetailsResult['Error'] ) ) { + throw new SmashPigException( $getDetailsResult['Error']['Message'] ); + } + return $getDetailsResult['GetOrderReferenceDetailsResult']['OrderReferenceDetails']; + } } diff --git a/wikimedia/smash-pig/PaymentProviders/Amazon/ExpatriatedMessages/AmazonMessage.php b/wikimedia/smash-pig/PaymentProviders/Amazon/ExpatriatedMessages/AmazonMessage.php index 7a4abff..d07d987 100644 --- a/wikimedia/smash-pig/PaymentProviders/Amazon/ExpatriatedMessages/AmazonMessage.php +++ b/wikimedia/smash-pig/PaymentProviders/Amazon/ExpatriatedMessages/AmazonMessage.php @@ -41,7 +41,7 @@ $this->correlationId = 'amazon-' . $this->gateway_txn_id; } - public function getGatewayTransactionId() { - return $this->gateway_txn_id; + public function getOrderReferenceId() { + return substr( $this->gateway_txn_id, 0, 19); } } diff --git a/wikimedia/smash-pig/PaymentProviders/Amazon/ExpatriatedMessages/PaymentCapture.php b/wikimedia/smash-pig/PaymentProviders/Amazon/ExpatriatedMessages/PaymentCapture.php index a0f6c41..bda3953 100644 --- a/wikimedia/smash-pig/PaymentProviders/Amazon/ExpatriatedMessages/PaymentCapture.php +++ b/wikimedia/smash-pig/PaymentProviders/Amazon/ExpatriatedMessages/PaymentCapture.php @@ -21,11 +21,8 @@ $this->setGatewayIds( $details['AmazonCaptureId'] ); $captureReferenceId = $details['CaptureReferenceId']; - $this->completion_message_id = "amazon-$captureReferenceId"; - $this->order_id = $captureReferenceId; - $parts = explode( '-', $captureReferenceId ); - $this->contribution_tracking_id = $parts[0]; + $this->setOrderId( $captureReferenceId ); $this->date = UtcDate::getUtcTimestamp( $details['CreationTimestamp'] ); @@ -51,4 +48,24 @@ return $queueMsg; } + + /** + * Set fields derived from the order ID + * + * @param string $orderId + */ + public function setOrderId( $orderId ) { + $this->order_id = $orderId; + $this->completion_message_id = "amazon-$orderId"; + + $parts = explode( '-', $orderId ); + $this->contribution_tracking_id = $parts[0]; + } + + /** + * @return string + */ + public function getOrderId() { + return $this->order_id; + } } diff --git a/wikimedia/smash-pig/PaymentProviders/Amazon/Tests/AmazonTestCase.php b/wikimedia/smash-pig/PaymentProviders/Amazon/Tests/AmazonTestCase.php new file mode 100644 index 0000000..3e95efd --- /dev/null +++ b/wikimedia/smash-pig/PaymentProviders/Amazon/Tests/AmazonTestCase.php @@ -0,0 +1,30 @@ +<?php +namespace SmashPig\PaymentProviders\Amazon\Tests; + +use ReflectionClass; +use SmashPig\Core\Context; +use SmashPig\Tests\BaseSmashPigUnitTestCase; + +class AmazonTestCase extends BaseSmashPigUnitTestCase { + + protected $mockClient; + + public function setUp() { + parent::setUp(); + chdir( __DIR__ ); // So the mock client can find its response files + $config = AmazonTestConfiguration::instance(); + Context::initWithLogger( $config ); + $this->mockClient = $config->object( 'payments-client', true ); + $this->mockClient->calls = array(); + $this->mockClient->returns = array(); + $this->mockClient->exceptions = array(); + } + + public function tearDown() { + parent::tearDown(); + $api = new ReflectionClass( 'SmashPig\PaymentProviders\Amazon\AmazonApi' ); + $instance = $api->getProperty( 'instance' ); + $instance->setAccessible( true ); + $instance->setValue( null ); + } +} diff --git a/wikimedia/smash-pig/PaymentProviders/Amazon/Tests/Data/responses/getOrderReferenceDetails.json b/wikimedia/smash-pig/PaymentProviders/Amazon/Tests/Data/responses/getOrderReferenceDetails.json index 04c8f42..3d6ffc6 100644 --- a/wikimedia/smash-pig/PaymentProviders/Amazon/Tests/Data/responses/getOrderReferenceDetails.json +++ b/wikimedia/smash-pig/PaymentProviders/Amazon/Tests/Data/responses/getOrderReferenceDetails.json @@ -13,7 +13,9 @@ "P01-0133129-0199515-A019658" ] }, - "SellerOrderAttributes": [], + "SellerOrderAttributes": { + "SellerOrderId": "123456789-0" + }, "OrderTotal": { "CurrencyCode": "USD", "Amount": "10.00" @@ -32,4 +34,4 @@ "RequestId": "896332ae-e316-4eb6-865e-79278e1aa214" }, "ResponseStatus": "200" -} \ No newline at end of file +} diff --git a/wikimedia/smash-pig/PaymentProviders/Amazon/Tests/phpunit/ActionsTest.php b/wikimedia/smash-pig/PaymentProviders/Amazon/Tests/phpunit/ActionsTest.php new file mode 100644 index 0000000..9b34964 --- /dev/null +++ b/wikimedia/smash-pig/PaymentProviders/Amazon/Tests/phpunit/ActionsTest.php @@ -0,0 +1,31 @@ +<?php +namespace SmashPig\PaymentProviders\Amazon\Tests; + +use SmashPig\PaymentProviders\Amazon\Actions\ReconstructMerchantReference; +use SmashPig\PaymentProviders\Amazon\ExpatriatedMessages\CaptureCompleted; + +class ActionsTest extends AmazonTestCase { + + public function testReconstructMerchantId() { + $captureCompleted = $this->loadJson( __DIR__ . "/../Data/IPN/CaptureCompleted.json" ); + $captureCompleted["CaptureDetails"]["CaptureReferenceId"] = 'AUTHORIZE_123456767'; + $message = new CaptureCompleted( $captureCompleted ); + $this->assertEquals( 'AUTHORIZE_123456767', $message->getOrderId() ); + $action = new ReconstructMerchantReference(); + $action->execute( $message ); + // This ID comes from getOrderReferenceDetails.json + $this->assertEquals( '123456789-0', $message->getOrderId() ); + } + + /** + * Don't waste API calls when it's not an AUTHORIZE_ id + */ + public function testReconstructMerchantIdNotNeeded() { + $captureCompleted = $this->loadJson( __DIR__ . "/../Data/IPN/CaptureCompleted.json" ); + $message = new CaptureCompleted( $captureCompleted ); + $action = new ReconstructMerchantReference(); + $action->execute( $message ); + $this->assertEquals( '98765432-1', $message->getOrderId() ); + $this->assertEmpty( $this->mockClient->calls ); + } +} diff --git a/wikimedia/smash-pig/PaymentProviders/Amazon/Tests/phpunit/ApiTest.php b/wikimedia/smash-pig/PaymentProviders/Amazon/Tests/phpunit/ApiTest.php index ecbefb9..abbaabf 100644 --- a/wikimedia/smash-pig/PaymentProviders/Amazon/Tests/phpunit/ApiTest.php +++ b/wikimedia/smash-pig/PaymentProviders/Amazon/Tests/phpunit/ApiTest.php @@ -1,29 +1,14 @@ <?php namespace SmashPig\PaymentProviders\Amazon\Tests; -use SmashPig\Core\Context; use SmashPig\PaymentProviders\Amazon\AmazonApi; -use SmashPig\Tests\BaseSmashPigUnitTestCase; -class ApiTest extends BaseSmashPigUnitTestCase { - - protected $mockClient; - - public function setUp() { - parent::setUp(); - chdir( __DIR__ . '/..' ); // So the mock client can find its response files - $config = AmazonTestConfiguration::instance(); - Context::initWithLogger( $config ); - $this->mockClient = $config->object( 'payments-client', true ); - $this->mockClient->calls = array(); - $this->mockClient->returns = array(); - $this->mockClient->exceptions = array(); - } +class ApiTest extends AmazonTestCase { public function testFindParent() { $this->mockClient->returns['getAuthorizationDetails'][] = 'Declined'; $this->mockClient->returns['getAuthorizationDetails'][] = 'Closed'; - $parentId = AmazonApi::findRefundParentId( 'P01-0133129-0199515-R019658' ); + $parentId = AmazonApi::get()->findCaptureId( 'P01-0133129-0199515' ); $this->assertEquals( 'P01-0133129-0199515-C019658', $parentId, 'Did not get the right refund parent ID' ); } } diff --git a/wikimedia/smash-pig/PaymentProviders/PayPal/Job.php b/wikimedia/smash-pig/PaymentProviders/PayPal/Job.php index f253e0d..9f949c7 100644 --- a/wikimedia/smash-pig/PaymentProviders/PayPal/Job.php +++ b/wikimedia/smash-pig/PaymentProviders/PayPal/Job.php @@ -98,6 +98,15 @@ } } + // If someone's PayPal account is set to their name we don't want + // it to go in the address box. They should put in a business name + // or something. + if ( isset( $new_msg->supplemental_address_1 ) + && $new_msg->supplemental_address_1 === + "{$new_msg->first_name} {$new_msg->last_name}" ) { + unset( $new_msg->supplemental_address_1 ); + } + // FIXME once recurring uses normalized msg it needs this too $new_msg->date = strtotime( $new_msg->date ); } diff --git a/wikimedia/smash-pig/PaymentProviders/PayPal/Listener.php b/wikimedia/smash-pig/PaymentProviders/PayPal/Listener.php index 2cc1150..4148554 100644 --- a/wikimedia/smash-pig/PaymentProviders/PayPal/Listener.php +++ b/wikimedia/smash-pig/PaymentProviders/PayPal/Listener.php @@ -1,10 +1,10 @@ <?php namespace SmashPig\PaymentProviders\PayPal; +use RuntimeException; use SmashPig\Core\Configuration; use SmashPig\Core\Http\IHttpActionHandler; use SmashPig\Core\Http\Request; use SmashPig\Core\Http\Response; -use SmashPig\Core\Listeners\ListenerSecurityException; use SmashPig\Core\Logging\Logger; class Listener implements IHttpActionHandler { @@ -18,23 +18,32 @@ // Don't store blank messages. if ( empty( $requestValues ) ) { - return; + return false; } - // Don't store invalid messages. - $valid = $this->config->object( 'api' )->validate( $requestValues ); - if ( !$valid ) { - // This will tell them to resend later. + $valid = false; + try { + $valid = $this->config->object( 'api' )->validate( $requestValues ); + } catch ( RuntimeException $e ) { + // Tried to validate a bunch of times and got nonsense responses. + Logger::error( $e->getMessage() ); + // 403 should tell them to send it again later. $response->setStatusCode( 403, 'Failed verification' ); return false; } - // Dump the request right into the queue with no validation. - $job = new Job; - $job->payload = $requestValues; - $this->config->object( 'data-store/jobs-paypal' )->push( $job ); - Logger::info( 'Pushed new message to jobs-paypal: ' . - print_r( $requestValues, true ) ); + if ( $valid ) { + $job = new Job; + $job->payload = $requestValues; + $this->config->object( 'data-store/jobs-paypal' )->push( $job ); + Logger::info( 'Pushed new message to jobs-paypal: ' . + print_r( $requestValues, true ) ); + return true; + } + + Logger::info( 'INVALID IPN message: ' . print_r( $requestValues, true ) ); + return false; + } } diff --git a/wikimedia/smash-pig/PaymentProviders/PayPal/PayPalPaymentsAPI.php b/wikimedia/smash-pig/PaymentProviders/PayPal/PayPalPaymentsAPI.php index 84120d3..f46939c 100644 --- a/wikimedia/smash-pig/PaymentProviders/PayPal/PayPalPaymentsAPI.php +++ b/wikimedia/smash-pig/PaymentProviders/PayPal/PayPalPaymentsAPI.php @@ -18,41 +18,44 @@ * @return bool */ function validate( $post_fields = array() ) { - $url = Configuration::getDefaultConfig()->val( 'postback-url' ); - $ch = curl_init(); - curl_setopt( $ch, CURLOPT_URL, $url ); - curl_setopt( $ch, CURLOPT_HEADER, 0 ); - curl_setopt( $ch, CURLOPT_FOLLOWLOCATION, 0 ); - curl_setopt( $ch, CURLOPT_RETURNTRANSFER, 1 ); - curl_setopt( $ch, CURLOPT_POST, 1 ); - // TODO we can put VERIFIED in config and generalize this - // Always capture the cURL output - $curlDebugLog = fopen( 'php://temp', 'r+' ); - curl_setopt( $ch, CURLOPT_VERBOSE, true ); - curl_setopt( $ch, CURLOPT_STDERR, $curlDebugLog ); + // https://www.paypal-knowledge.com/infocenter/index?page=content&id=FAQ1336&actp=LIST + // PayPal randomly fails to validate messages, so try a few times. + $max_attempts = 7; - $response = $this->curl( $ch, $post_fields ); + for ( $i = 0; $i < $max_attempts; $i++ ) { + $url = Configuration::getDefaultConfig()->val( 'postback-url' ); + $ch = curl_init(); + curl_setopt( $ch, CURLOPT_URL, $url ); + curl_setopt( $ch, CURLOPT_HEADER, 0 ); + curl_setopt( $ch, CURLOPT_FOLLOWLOCATION, 0 ); + curl_setopt( $ch, CURLOPT_RETURNTRANSFER, 1 ); + curl_setopt( $ch, CURLOPT_POST, 1 ); + // TODO we can put VERIFIED in config and generalize this - // Read the logging output - rewind( $curlDebugLog ); - $logged = fread( $curlDebugLog, 8192 ); - fclose( $curlDebugLog ); - Logger::debug( "cURL verbose logging: $logged" ); + // Always capture the cURL output + $curlDebugLog = fopen( 'php://temp', 'r+' ); + curl_setopt( $ch, CURLOPT_VERBOSE, true ); + curl_setopt( $ch, CURLOPT_STDERR, $curlDebugLog ); - if ( $response === 'VERIFIED' ) { - return true; - } elseif ( $response === 'INVALID' ) { - return false; - } else { - // TODO: Log txn_id. This is annoying because of the random document formats. - Logger::debug( - "Unknown response from PayPal IPN PB: [{$response}].\n" . - "Verbose logging: $logged" - ); - // FIXME: The same thing happens for "INVALID" and totally broken - // responses. Differentiate. - return false; + $response = $this->curl( $ch, $post_fields ); + + // Read the logging output + rewind( $curlDebugLog ); + $logged = fread( $curlDebugLog, 8192 ); + fclose( $curlDebugLog ); + Logger::debug( "cURL verbose logging: $logged" ); + + if ( $response === 'VERIFIED' ) { + return true; + } elseif ( $response === 'INVALID' ) { + return false; + } + // Must be an HTML page, keep trying. } + + throw new RuntimeException( 'Failed to validate message after ' . + $max_attempts . ' attempts: ' . print_r( $post_fields, true ) ); } + } diff --git a/wikimedia/smash-pig/PaymentProviders/PayPal/Tests/MockPayPalPaymentsAPI.php b/wikimedia/smash-pig/PaymentProviders/PayPal/Tests/MockPayPalPaymentsAPI.php index ac0ef67..e35a52c 100644 --- a/wikimedia/smash-pig/PaymentProviders/PayPal/Tests/MockPayPalPaymentsAPI.php +++ b/wikimedia/smash-pig/PaymentProviders/PayPal/Tests/MockPayPalPaymentsAPI.php @@ -7,6 +7,9 @@ if ( CaptureIncomingMessageTest::$fail_verification ) { return 'INVALID'; } + if ( CaptureIncomingMessageTest::$paypal_is_broken ) { + return 'lkjasjdhfiuasdgjgbasdd'; + } return 'VERIFIED'; } } diff --git a/wikimedia/smash-pig/PaymentProviders/PayPal/Tests/phpunit/CaptureIncomingMessageTest.php b/wikimedia/smash-pig/PaymentProviders/PayPal/Tests/phpunit/CaptureIncomingMessageTest.php index fa9a8c6..6020013 100644 --- a/wikimedia/smash-pig/PaymentProviders/PayPal/Tests/phpunit/CaptureIncomingMessageTest.php +++ b/wikimedia/smash-pig/PaymentProviders/PayPal/Tests/phpunit/CaptureIncomingMessageTest.php @@ -21,6 +21,7 @@ public $config; static $fail_verification = false; + static $paypal_is_broken = false; // filename and the queue it should get dropped in static $message_data = array( @@ -53,6 +54,12 @@ ) ); } + } + + public function tearDown() { + self::$fail_verification = false; + self::$paypal_is_broken = false; + parent::tearDown(); } private function capture( $msg ) { @@ -108,6 +115,13 @@ if ( isset( $message['contribution_tracking_id'] ) ) { $this->assertEquals( $message['contribution_tracking_id'], $message['order_id'] ); } + + if ( isset( $message['supplemental_address_1'] ) ) { + $this->assertNotEquals( + $message['supplemental_address_1'], + "{$message['first_name']} {$message['last_name']}" + ); + } } } @@ -119,4 +133,12 @@ $this->assertFalse( $this->capture( $jobMessage ) ); } + // FIXME: not really testing anything. Would like to verify that it tried + // N times. Bubble that information up somehow. + public function testPayPalIsBroken() { + self::$paypal_is_broken = true; + $jobMessage = array( 'txn_type' => 'fail' ); + $this->assertFalse( $this->capture( $jobMessage ) ); + } + } diff --git a/wikimedia/smash-pig/SmashPig.yaml b/wikimedia/smash-pig/SmashPig.yaml index 9c8eab8..eab2a66 100644 --- a/wikimedia/smash-pig/SmashPig.yaml +++ b/wikimedia/smash-pig/SmashPig.yaml @@ -313,6 +313,7 @@ amazon: actions: + - SmashPig\PaymentProviders\Amazon\Actions\ReconstructMerchantReference - SmashPig\PaymentProviders\Amazon\Actions\CloseOrderReference - SmashPig\PaymentProviders\Amazon\Actions\AssociateRefundParent - SmashPig\PaymentProviders\Amazon\Actions\AddMessageToQueue -- To view, visit https://gerrit.wikimedia.org/r/325713 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ifa66a5822a66939cb0885bfbd9c4b501c1a12e6d Gerrit-PatchSet: 1 Gerrit-Project: wikimedia/fundraising/crm/vendor Gerrit-Branch: master Gerrit-Owner: Ejegg <eeggles...@wikimedia.org> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits