jenkins-bot has submitted this change and it was merged.
Change subject: Fold parseResponseXX calls into processResponse
......................................................................
Fold parseResponseXX calls into processResponse
TODO: reduce parallel switch structure
Bug: T98063
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, 137 insertions(+), 125 deletions(-)
Approvals:
Awight: Looks good to me, approved
jenkins-bot: Verified
diff --git a/adyen_gateway/adyen.adapter.php b/adyen_gateway/adyen.adapter.php
index a988068..a79abb8 100644
--- a/adyen_gateway/adyen.adapter.php
+++ b/adyen_gateway/adyen.adapter.php
@@ -461,20 +461,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_response = 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',
@@ -483,9 +487,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.
@@ -494,9 +498,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..38bd107 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_response->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_response->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 1144e57..742caeb 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_response->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_response->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_response->getRawResponse();
+ $logme .= 'Full response: ' .
$this->transaction_response->getRawResponse();
}
- $this->logger->warning( $logged );
+ $level = LogLevel::WARNING;
}
- if ( $logged ) {
+ if ( $logme ) {
+ $this->logger->log( $level, $logme );
+ //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,35 @@
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_response, as we can be
called by
// GatewayPage to process responses outside of do_transaction
if ( !$this->transaction_response ) {
$this->transaction_response = new
PaymentTransactionResponse();
}
+ $this->transaction_response->setCommunicationStatus(
+ $this->parseResponseCommunicationStatus( $response )
+ );
+ $this->transaction_response->setErrors(
+ $this->parseResponseErrors( $response )
+ );
+ $this->transaction_response->setData( $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 +497,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_response->setGatewayTransactionId(
$response['data']['x_document'] );
+ if ( isset( $response['x_document'] ) ) {
+
$this->transaction_response->setGatewayTransactionId( $response['x_document'] );
} else {
$this->logger->error( 'Astropay did not
post back their transaction ID in x_document' );
throw new ResponseProcessingException(
@@ -506,7 +513,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 +527,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 35d6300..17e80e2 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_response
- *
- * 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_response
+ * @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 );
@@ -1134,22 +1133,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_response->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 c05d902..8fab22b 100644
--- a/globalcollect_gateway/globalcollect.adapter.php
+++ b/globalcollect_gateway/globalcollect.adapter.php
@@ -1808,13 +1808,20 @@
/**
* Process the response and set transaction_response 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_response->setCommunicationStatus(
+ $this->parseResponseCommunicationStatus( $response )
+ );
+ $errors = $this->parseResponseErrors( $response );
+ $this->transaction_response->setErrors( $errors );
+ $data = $this->parseResponseData( $response );
+ $this->transaction_response->setData( $data );
//set the transaction result message
- $responseStatus = isset( $response['data']['STATUSID'] ) ?
$response['data']['STATUSID'] : '';
+ $responseStatus = isset( $data['STATUSID'] ) ?
$data['STATUSID'] : '';
$this->transaction_response->setTxnMessage( "Response Status: "
. $responseStatus ); //TODO: Translate for GC.
$this->transaction_response->setGatewayTransactionId(
$this->getData_Unstaged_Escaped( 'order_id' ) );
@@ -1823,8 +1830,8 @@
$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 ) {
+ $errMsg = $errObj['message'];
switch ( $errCode ) {
case 300620:
// Oh no! We've already used this order #
somewhere else! Restart!
@@ -1872,21 +1879,14 @@
//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_response->getErrors();
- // Matching previous behavior, which
set a random transaction_response
- // 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,
$errCode
diff --git a/paypal_gateway/paypal.adapter.php
b/paypal_gateway/paypal.adapter.php
index 3827d05..5c5981b 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_response->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 0b01149..8bd8076 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_response->setCommunicationStatus(
+ $this->parseResponseCommunicationStatus( $response )
+ );
+ $errors = $this->parseResponseErrors( $response );
+ $this->transaction_response->setErrors( $errors );
+ $data = $this->parseResponseData( $response );
+ $this->transaction_response->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: merged
Gerrit-Change-Id: I7dd03e4a37d9e7e3938c2c5747d19d448e319d5b
Gerrit-PatchSet: 7
Gerrit-Project: mediawiki/extensions/DonationInterface
Gerrit-Branch: master
Gerrit-Owner: Ejegg <eeggles...@wikimedia.org>
Gerrit-Reviewer: Awight <awi...@wikimedia.org>
Gerrit-Reviewer: Ejegg <eeggles...@wikimedia.org>
Gerrit-Reviewer: Ssmith <ssm...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
