Ejegg has uploaded a new change for review.
https://gerrit.wikimedia.org/r/209154
Change subject: WIP fold parseResponseXX calls into processResponse
......................................................................
WIP fold parseResponseXX calls into processResponse
FIXME: this is not the win I was hoping for. Turns our there's
not a ton of parallel switch structure in many gateways.
Change-Id: I7dd03e4a37d9e7e3938c2c5747d19d448e319d5b
---
M adyen_gateway/adyen.adapter.php
M amazon_gateway/amazon.adapter.php
M astropay_gateway/astropay.adapter.php
M gateway_common/GatewayPage.php
M gateway_common/gateway.adapter.php
M globalcollect_gateway/globalcollect.adapter.php
M paypal_gateway/paypal.adapter.php
M tests/Adapter/Astropay/AstropayTest.php
M worldpay_gateway/worldpay.adapter.php
9 files changed, 140 insertions(+), 128 deletions(-)
git pull
ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/DonationInterface
refs/changes/54/209154/1
diff --git a/adyen_gateway/adyen.adapter.php b/adyen_gateway/adyen.adapter.php
index dbffef8..d811270 100644
--- a/adyen_gateway/adyen.adapter.php
+++ b/adyen_gateway/adyen.adapter.php
@@ -462,20 +462,24 @@
return $queryvals;
}
- function processResponse( $response ) {
+ /**
+ * For Adyen, we only call this on the donor's return to the
ResultSwitcher
+ * @param array $response GET/POST params from request
+ * @throws ResponseProcessingException
+ */
+ public function processResponse( $response ) {
// Always called outside do_transaction, so just make a new
response object
$this->transaction_result = new PaymentTransactionResponse();
- if ( empty( $response ) || empty( $response['data'] ) ) {
+ if ( empty( $response ) ) {
$this->logger->info( "No response from gateway" );
throw new ResponseProcessingException(
'No response from gateway',
ResponseCodes::NO_RESPONSE
);
}
- $request_vars = $response['data'];
- $this->logger->info( "Processing user return data: " . print_r(
$request_vars, TRUE ) );
+ $this->logger->info( "Processing user return data: " . print_r(
$response , TRUE ) );
- if ( !$this->checkResponseSignature( $request_vars ) ) {
+ if ( !$this->checkResponseSignature( $response ) ) {
$this->logger->info( "Bad signature in response" );
throw new ResponseProcessingException(
'Bad signature in response',
@@ -484,9 +488,9 @@
}
$this->logger->debug( 'Good signature' );
- $gateway_txn_id = isset( $request_vars[ 'pspReference' ] ) ?
$request_vars[ 'pspReference' ] : '';
+ $gateway_txn_id = isset( $response [ 'pspReference' ] ) ?
$response [ 'pspReference' ] : '';
- $result_code = isset( $request_vars[ 'authResult' ] ) ?
$request_vars[ 'authResult' ] : '';
+ $result_code = isset( $response [ 'authResult' ] ) ? $response
[ 'authResult' ] : '';
if ( $result_code == 'PENDING' || $result_code == 'AUTHORISED'
) {
// Both of these are listed as pending because we have
to submit a capture
// request on 'AUTHORIZATION' ipn message receipt.
@@ -495,9 +499,9 @@
}
else {
$this->finalizeInternalStatus( FinalStatus::FAILED );
- $this->logger->info( "Negative response from gateway.
Full response: " . print_r( $request_vars, TRUE ) );
+ $this->logger->info( "Negative response from gateway.
Full response: " . print_r( $response , TRUE ) );
throw new ResponseProcessingException(
- "Negative response from gateway. Full response:
" . print_r( $request_vars, TRUE ),
+ "Negative response from gateway. Full response:
" . print_r( $response , TRUE ),
ResponseCodes::UNKNOWN
);
}
diff --git a/amazon_gateway/amazon.adapter.php
b/amazon_gateway/amazon.adapter.php
index 68ca6f8..f723b93 100644
--- a/amazon_gateway/amazon.adapter.php
+++ b/amazon_gateway/amazon.adapter.php
@@ -50,7 +50,7 @@
"status" => "gateway_status",
"buyerEmail" => "email",
"transactionDate" => "date_collect",
- "buyerName" => "fname", // This is dealt with in
processResponse()
+ "buyerName" => "fname", // This is dealt with in
addDataFromURI()
"errorMessage" => "error_message",
"paymentMethod" => "payment_submethod",
"referenceId" => "contribution_tracking_id",
@@ -424,8 +424,28 @@
$this->logger->info( "Added data to session for txnid $txnid.
Now serving email $email." );
}
- function processResponse( $response ) {
- if ( ( $this->getCurrentTransaction() == 'VerifySignature' ) &&
( $response['data'] == true ) ) {
+ /**
+ * We would call this function for the VerifySignature transaction, if
we
+ * ever used that.
+ * @param DomDocument $response
+ * @throws ResponseProcessingException
+ */
+ public function processResponse( $response ) {
+ $this->transaction_result->setErrors(
$this->parseResponseErrors( $response ) );
+ if ( $this->getCurrentTransaction() !== 'VerifySignature' ) {
+ return;
+ }
+ $statuses = $response->getElementsByTagName(
'VerificationStatus' );
+ $verified = false;
+ $commStatus = false;
+ foreach ( $statuses as $node ) {
+ $commStatus = true;
+ if ( strtolower( $node->nodeValue ) == 'success' ) {
+ $verified = true;
+ }
+ }
+ $this->transaction_result->setCommunicationStatus( $commStatus
);
+ if ( !$verified ) {
$this->logger->info( "Transaction failed in response
data verification." );
$this->finalizeInternalStatus( FinalStatus::FAILED );
}
@@ -473,22 +493,6 @@
$opts[CURLOPT_CAPATH] = __DIR__ . "/ca-bundle.crt";
return $opts;
- }
-
- // FIXME: this should return an array, dagnabbit!
- public function parseResponseData( $response ) {
- // The XML string isn't really all that useful, so just return
TRUE if the signature
- // was verified
- if ( $this->getCurrentTransaction() == 'VerifySignature' ) {
- $statuses = $response->getElementsByTagName(
'VerificationStatus' );
- foreach ( $statuses as $node ) {
- if ( strtolower( $node->nodeValue ) ==
'success' ) {
- return TRUE;
- }
- }
- }
-
- return FALSE;
}
function parseResponseCommunicationStatus( $response ) {
diff --git a/astropay_gateway/astropay.adapter.php
b/astropay_gateway/astropay.adapter.php
index b03678f..659cb98 100644
--- a/astropay_gateway/astropay.adapter.php
+++ b/astropay_gateway/astropay.adapter.php
@@ -15,6 +15,7 @@
* GNU General Public License for more details.
*
*/
+use Psr\Log\LogLevel;
/**
* AstropayAdapter
@@ -403,59 +404,45 @@
}
function parseResponseErrors( $response ) {
- $logged = false;
+ $logme = false;
$errors = array();
$code = 'internal-0000';
+ $level = LogLevel::ERROR;
if ( $response === NULL ) {
- $logged = 'Astropay response was not valid JSON. Full
response: ' .
+ $logme = 'Astropay response was not valid JSON. Full
response: ' .
$this->transaction_result->getRawResponse();
- $this->logger->error( $logged );
} else if ( !isset( $response['status'] ) ) {
- $logged = 'Astropay response does not have a status
code. Full response: ' .
+ $logme = 'Astropay response does not have a status
code. Full response: ' .
$this->transaction_result->getRawResponse();
- $this->logger->error( $logged );
} else if ( $response['status'] !== '0' ) {
- $logged = "Astropay response has non-zero status
{$response['status']}. ";
+ $logme = "Astropay response has non-zero status
{$response['status']}. ";
if ( isset( $response['desc'] ) ) {
// They don't give us codes to distinguish
failure modes, so we
// have to parse the description.
if ( preg_match( '/invoice already used/i',
$response['desc'] ) ) {
$code =
ResponseCodes::DUPLICATE_ORDER_ID;
}
- $logged .= 'Error description: ' .
$response['desc'];
+ $logme .= 'Error description: ' .
$response['desc'];
} else {
- $logged .= 'Full response: ' .
$this->transaction_result->getRawResponse();
+ $logme .= 'Full response: ' .
$this->transaction_result->getRawResponse();
}
- $this->logger->warning( $logged );
+ $level = LogLevel::WARNING;
}
- if ( $logged ) {
+ if ( $logme ) {
+ $this->logger->log( $logme, $level );
+ //TODO: move translation and debug display out of this
fn
$generic = $this->getErrorMapByCodeAndTranslate(
'internal-0000' );
$debug = $this->getGlobal( 'DisplayDebug' );
- $errors[$code] = $debug ? $logged : $generic;
+ $errors[$code] = array(
+ 'logLevel' => $level,
+ 'debugInfo' => $debug,
+ 'message' => $debug ? $logme : $generic
+ );
}
return $errors;
- }
-
- public function parseResponseData( $response ) {
- $data = array();
-
- switch( $this->getCurrentTransaction() ) {
- case 'NewInvoice':
- if ( $response !== NULL && isset(
$response['link'] ) ) {
- // aargh, side effects!
- $this->setTransactionResult(
$response['link'], 'redirect' );
- $data['redirect'] = $response['link'];
- }
- break;
- case 'PaymentStatus':
- // getFormattedResponse has already parsed the
response into an array
- $data = $response;
- break;
- }
- return $data;
}
static function getCurrencies() {
@@ -473,15 +460,34 @@
return $currencies;
}
- function processResponse( $response ) {
+ /**
+ * Processes JSON data from Astropay API, and also processes GET/POST
params
+ * on donor's return to ResultSwitcher
+ * @param array $response JSON response decoded to array, or GET/POST
+ * params from request
+ * @throws ResponseProcessingException
+ */
+ public function processResponse( $response ) {
// May need to initialize transaction_result, as we can be
called by
// GatewayPage to process responses outside of do_transaction
if ( !$this->transaction_result ) {
$this->transaction_result = new
PaymentTransactionResponse();
}
+ $this->transaction_result->setCommunicationStatus(
+ $this->parseResponseCommunicationStatus( $response )
+ );
+ $this->transaction_result->setErrors(
+ $this->parseResponseErrors( $response )
+ );
+ if ( !$response ) {
+ throw new ResponseProcessingException(
+ 'Missing or badly formatted response',
+ ResponseCodes::NO_RESPONSE
+ );
+ }
switch( $this->getCurrentTransaction() ) {
case 'PaymentStatus':
- if ( !$this->verifyStatusSignature(
$response['data'] ) ) {
+ if ( !$this->verifyStatusSignature( $response )
) {
$this->logger->error( 'Bad signature in
response to PaymentStatus call.' );
throw new ResponseProcessingException(
'Bad signature in response to
PaymentStatus call.',
@@ -490,15 +496,15 @@
}
break;
case 'ProcessReturn':
- if ( !$this->verifyStatusSignature(
$response['data'] ) ) {
+ if ( !$this->verifyStatusSignature( $response )
) {
$this->logger->error( 'Bad signature in
data POSTed to resultswitcher.' );
throw new ResponseProcessingException(
'Bad signature in data POSTed
to resultswitcher.',
ResponseCodes::BAD_SIGNATURE
);
}
- if ( isset( $response['data']['x_document'] ) )
{
-
$this->transaction_result->setGatewayTransactionId(
$response['data']['x_document'] );
+ if ( isset( $response['x_document'] ) ) {
+
$this->transaction_result->setGatewayTransactionId( $response['x_document'] );
} else {
$this->logger->error( 'Astropay did not
post back their transaction ID in x_document' );
throw new ResponseProcessingException(
@@ -506,7 +512,7 @@
ResponseCodes::MISSING_TRANSACTION_ID
);
}
- $status = $this->findCodeAction(
'PaymentStatus', 'result', $response['data']['result'] );
+ $status = $this->findCodeAction(
'PaymentStatus', 'result', $response['result'] );
$this->logger->info( "Payment status $status
coming back to ResultSwitcher" );
$this->finalizeInternalStatus( $status );
break;
@@ -520,6 +526,9 @@
array( 'order_id' )
);
}
+ if ( isset( $response['link'] ) ) {
+ $this->setTransactionResult(
$response['link'], 'redirect' );
+ }
break;
}
}
diff --git a/gateway_common/GatewayPage.php b/gateway_common/GatewayPage.php
index 0964e08..ec1ccf0 100644
--- a/gateway_common/GatewayPage.php
+++ b/gateway_common/GatewayPage.php
@@ -468,11 +468,9 @@
$this->getOutput()->allowClickjacking();
// FIXME: do we really need this again?
$this->getOutput()->addModules( 'iframe.liberator' );
- // processResponse expects something with data, so
let's feed it
- // all the GET and POST vars
- $response = array(
- 'data' => $this->getRequest()->getValues(),
- );
+ // processResponse expects some data, so let's feed it
all the
+ // GET and POST vars
+ $response = $this->getRequest()->getValues();
// TODO: run the whole set of getResponseStatus,
getResponseErrors
// and getResponseData first. Maybe do_transaction
with a
// communication_type of 'incoming' and a way to
provide the
diff --git a/gateway_common/gateway.adapter.php
b/gateway_common/gateway.adapter.php
index 2b547d7..217621f 100644
--- a/gateway_common/gateway.adapter.php
+++ b/gateway_common/gateway.adapter.php
@@ -27,13 +27,12 @@
//all the particulars of the child classes. Aaaaall.
/**
- * Perform any additional processing on the response obtained from the
server
- * and set properties of transaction_result
- *
- * TODO: feed this the raw (or formatted) response and have it
determine status, errors, and data
- * @param array $response The internal response object as an array
- *
- * @throws ResponseProcessingException with an actionable error code
and any variables to retry
+ * Process the API response obtained from the payment processor and set
+ * properties of transaction_result
+ * @param array|DomDocument $response Cleaned-up response returned from
+ * @see getFormattedResponse. Type depends on
$this->getResponseType
+ * @throws ResponseProcessingException with an actionable error code
and any
+ * variables to retry
*/
public function processResponse( $response );
@@ -1139,22 +1138,12 @@
if ( $txn_ok === true ) { //We have something to slice and dice.
$this->logger->info( "RETURNED FROM CURL:" . print_r(
$this->getTransactionAllResults(), true ) );
- //set the status of the response. This is the
COMMUNICATION status, and has nothing
- //to do with the result of the transaction.
+ // Decode the response according to
$this->getResponseType
$formatted = $this->getFormattedResponse(
$this->transaction_result->getRawResponse() );
- $this->setTransactionResult(
$this->parseResponseCommunicationStatus( $formatted ), 'status' );
- //set errors
- //TODO: This "errors" business is becoming a bit of a
misnomer, as the result code and message
- //are frequently packaged togther in the same place,
whether the transaction passed or failed.
- $this->setTransactionResult(
$this->parseResponseErrors( $formatted ), 'errors' );
-
- //if we're still okay (hey, even if we're not), get
relevant data.
- $this->setTransactionResult( $this->parseResponseData(
$formatted ), 'data' );
-
- // Process the formatted response data. This will then
drive the result action
+ // Process the formatted response. This will then drive
the result action
try{
- $this->processResponse(
$this->getTransactionAllResults() );
+ $this->processResponse( $formatted );
} catch ( ResponseProcessingException $ex ) {
$errCode = $ex->getErrorCode();
$retryVars = $ex->getRetryVars();
diff --git a/globalcollect_gateway/globalcollect.adapter.php
b/globalcollect_gateway/globalcollect.adapter.php
index a3e2057..52e2f14 100644
--- a/globalcollect_gateway/globalcollect.adapter.php
+++ b/globalcollect_gateway/globalcollect.adapter.php
@@ -1808,13 +1808,20 @@
/**
* Process the response and set transaction_result properties
*
- * @param array $response The response array
+ * @param DomDocument $response Cleaned-up XML from the GlobalCollect
API
*
* @throws ResponseProcessingException with code and potentially retry
vars.
*/
public function processResponse( $response ) {
+ $this->transaction_result->setCommunicationStatus(
+ $this->parseResponseCommunicationStatus( $response )
+ );
+ $errors = $this->parseResponseErrors( $response );
+ $this->transaction_result->setErrors( $errors );
+ $data = $this->parseResponseData( $response );
+ $this->transaction_result->setData( $data );
//set the transaction result message
- $responseStatus = isset( $response['data']['STATUSID'] ) ?
$response['data']['STATUSID'] : '';
+ $responseStatus = isset( $data['STATUSID'] ) ?
$data['STATUSID'] : '';
$this->transaction_result->setTxnMessage( "Response Status: " .
$responseStatus ); //TODO: Translate for GC.
$this->transaction_result->setGatewayTransactionId(
$this->getData_Unstaged_Escaped( 'order_id' ) );
@@ -1823,7 +1830,7 @@
$retryVars = array();
// We are also curious to know if there were any recoverable
errors
- foreach ( $response['errors'] as $errCode => $errMsg ) {
+ foreach ( $errors as $errCode => $errObj ) {
switch ( $errCode ) {
case 300620:
@@ -1831,7 +1838,7 @@
$this->logger->error( 'Order ID
collission! Starting again.' );
$retryVars[] = 'order_id';
$retErrCode = $errCode;
- $retErrMsg = $errMsg;
+ $retErrMsg = $errObj['message'];
break;
case 430260: //wow: If we were a point of sale,
we'd be calling security.
case 430357: //lost or stolen card
@@ -1872,23 +1879,16 @@
//Yes: That's an 8-digit error code
that buckets a silly number of validation issues, some of which are
legitimately ours.
//The only way to tell is to search the
English message.
//@TODO: Refactor all 3rd party error
handling for GC. This whole switch should definitely be in parseResponseErrors;
It is very silly that this is here at all.
- $enhanced =
$this->transaction_result->getErrors();
- // Matching previous behavior, which
set a random transaction_result
- // key to the raw message of the last
error inspected
- $raw = '';
- foreach ( $enhanced as $code =>
$errArray ) {
- $raw = $errArray['debugInfo'];
- }
$not_errors = array( //add more of
these stupid things here, if log noise makes you want to
'/NULL VALUE NOT ALLOWED FOR
EXPIRYDATE/',
'/DID NOT PASS THE LUHNCHECK/',
);
- foreach ($not_errors as $regex){
- if ( preg_match( $regex, $raw )
){
+ foreach ( $not_errors as $regex ){
+ if ( preg_match( $regex,
$errObj['debugInfo'] ) ){
//not a system error,
but definitely the end of the payment attempt. Log it to info and leave.
- $this->logger->info(
__FUNCTION__ . ": $raw" );
+ $this->logger->info(
__FUNCTION__ . ": {$errObj['debugInfo']}" );
throw new
ResponseProcessingException(
- $errMsg,
+
$errObj['message'],
$errCode
);
}
@@ -1897,7 +1897,8 @@
case 21000050 : //REQUEST {0} VALUE {2} OF
FIELD {1} IS NOT A NUMBER WITH MINLENGTH {3}, MAXLENGTH {4} AND PRECISION {5}
: More validation pain.
//say something painful here.
- $errMsg = 'Blocking validation problems
with this payment. Investigation required! ' . $this->getLogDebugJSON();
+ $retErrCode = $errCode;
+ $retErrMsg = 'Blocking validation
problems with this payment. Investigation required! ' .
$this->getLogDebugJSON();
case 400120:
/* INSERTATTEMPT PAYMENT FOR ORDER
ALREADY FINAL FOR COMBINATION.
* They already gave us money or
failed...
@@ -1907,7 +1908,7 @@
* @TODO: This absolutely happens IRL.
Attempt to handle gracefully once we figure out what that means.
*/
default:
- $this->logger->error( __FUNCTION__ . "
Error $errCode : $errMsg" );
+ $this->logger->error( __FUNCTION__ . "
Error $errCode : {$errObj['message']}" );
break;
}
}
diff --git a/paypal_gateway/paypal.adapter.php
b/paypal_gateway/paypal.adapter.php
index 3827d05..acdea2d 100644
--- a/paypal_gateway/paypal.adapter.php
+++ b/paypal_gateway/paypal.adapter.php
@@ -60,7 +60,9 @@
$this->accountInfo = array();
}
function defineReturnValueMap() {}
- function processResponse( $response ) {}
+ function processResponse( $response ) {
+ $this->transaction_result->setCommunicationStatus( true );
+ }
function defineDataConstraints() {}
function defineOrderIDMeta() {
$this->order_id_meta = array (
diff --git a/tests/Adapter/Astropay/AstropayTest.php
b/tests/Adapter/Astropay/AstropayTest.php
index bde5032..3e78349 100644
--- a/tests/Adapter/Astropay/AstropayTest.php
+++ b/tests/Adapter/Astropay/AstropayTest.php
@@ -210,16 +210,14 @@
// Next lines mimic Astropay resultswitcher
$gateway->setCurrentTransaction( 'ProcessReturn' );
$response = array(
- 'data' => array(
- 'result' => '9',
- 'x_amount' => '100.00',
- 'x_amount_usd' => '42.05',
- 'x_control' =>
'DDF89085AC70C0B0628150C51D64419D8592769F2439E3936570E26D24881730',
- 'x_description' => 'Donation to the Wikimedia
Foundation',
- 'x_document' => '32869',
- 'x_iduser' => '08feb2d12771bbcfeb86',
- 'x_invoice' => '123456789',
- )
+ 'result' => '9',
+ 'x_amount' => '100.00',
+ 'x_amount_usd' => '42.05',
+ 'x_control' =>
'DDF89085AC70C0B0628150C51D64419D8592769F2439E3936570E26D24881730',
+ 'x_description' => 'Donation to the Wikimedia
Foundation',
+ 'x_document' => '32869',
+ 'x_iduser' => '08feb2d12771bbcfeb86',
+ 'x_invoice' => '123456789',
);
$gateway->processResponse( $response );
@@ -237,16 +235,14 @@
$gateway->setCurrentTransaction( 'ProcessReturn' );
$response = array(
- 'data' => array(
- 'result' => '8', // rejected by bank
- 'x_amount' => '100.00',
- 'x_amount_usd' => '42.05',
- 'x_control' =>
'706F57BC3E74906B14B1DEB946F027104513797CC62AC0F5107BC98F42D5DC95',
- 'x_description' => 'Donation to the Wikimedia
Foundation',
- 'x_document' => '32869',
- 'x_iduser' => '08feb2d12771bbcfeb86',
- 'x_invoice' => '123456789',
- )
+ 'result' => '8', // rejected by bank
+ 'x_amount' => '100.00',
+ 'x_amount_usd' => '42.05',
+ 'x_control' =>
'706F57BC3E74906B14B1DEB946F027104513797CC62AC0F5107BC98F42D5DC95',
+ 'x_description' => 'Donation to the Wikimedia
Foundation',
+ 'x_document' => '32869',
+ 'x_iduser' => '08feb2d12771bbcfeb86',
+ 'x_invoice' => '123456789',
);
$gateway->processResponse( $response );
diff --git a/worldpay_gateway/worldpay.adapter.php
b/worldpay_gateway/worldpay.adapter.php
index 52dfff2..295316a 100644
--- a/worldpay_gateway/worldpay.adapter.php
+++ b/worldpay_gateway/worldpay.adapter.php
@@ -834,8 +834,17 @@
return $errors;
}
+ /**
+ * @param DomDocument $response
+ */
public function processResponse( $response ) {
- $data = $response['data'];
+ $this->transaction_result->setCommunicationStatus(
+ $this->parseResponseCommunicationStatus( $response )
+ );
+ $errors = $this->parseResponseErrors( $response );
+ $this->transaction_result->setErrors( $errors );
+ $data = $this->parseResponseData( $response );
+ $this->transaction_result->setData( $data );
switch ( $this->getCurrentTransaction() ) {
case 'GenerateToken':
$this->addRequiredData( $data, array(
--
To view, visit https://gerrit.wikimedia.org/r/209154
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7dd03e4a37d9e7e3938c2c5747d19d448e319d5b
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/DonationInterface
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
