Awight has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/344042 )
Change subject: [WIP] Encapsulate errors
......................................................................
[WIP] Encapsulate errors
TODO:
* Figure out how to represent validation errors and internal errors under a
polymorphic base.
* Write has* and clear* accessors.
* Write i18n glue. Figure out how to store messages that take params.
Change-Id: I39bcc0c84609e681fb9c15db51058ee3ccd7fad8
---
A gateway_common/PaymentError.php
M gateway_common/PaymentResult.php
M gateway_common/PaymentTransactionResponse.php
M gateway_common/gateway.adapter.php
4 files changed, 67 insertions(+), 85 deletions(-)
git pull
ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/DonationInterface
refs/changes/42/344042/1
diff --git a/gateway_common/PaymentError.php b/gateway_common/PaymentError.php
new file mode 100644
index 0000000..e1f90bc
--- /dev/null
+++ b/gateway_common/PaymentError.php
@@ -0,0 +1,13 @@
+<?php
+
+class PaymentsError {
+ protected $error_code;
+ protected $debug_message;
+ protected $log_level;
+
+ function __construct__( $error_code, $debug_message, $log_level ) {
+ $this->error_code = $error_code;
+ $this->debug_message = $debug_message;
+ $this->log_level = $log_level;
+ }
+}
diff --git a/gateway_common/PaymentResult.php b/gateway_common/PaymentResult.php
index 77acb8f..ac73e9e 100644
--- a/gateway_common/PaymentResult.php
+++ b/gateway_common/PaymentResult.php
@@ -73,9 +73,9 @@
public static function newEmpty() {
$response = new PaymentResult();
- $response->errors = array(
- 'internal-0000' => 'Internal error: no results yet.',
- );
+ $response->errors = array( new PaymentError(
+ 'internal-0000', 'Internal error: no results yet.'
+ ) );
$response->failed = true;
return $response;
}
diff --git a/gateway_common/PaymentTransactionResponse.php
b/gateway_common/PaymentTransactionResponse.php
index a116db3..f903af8 100644
--- a/gateway_common/PaymentTransactionResponse.php
+++ b/gateway_common/PaymentTransactionResponse.php
@@ -7,12 +7,7 @@
*/
class PaymentTransactionResponse {
/**
- * @var array keys are error codes, values are themselves arrays of the
form
- * 'message' => 'for public consumption',
- * 'debugInfo' => 'diagnostic information',
- * 'logLevel' => LogLevel::WARNING
- * If there weren't any, this should be present and empty after a
- * transaction.
+ * @var array List of PaymentErrors
*/
protected $errors = array();
@@ -107,19 +102,18 @@
}
/**
- * @param array $errors
+ * @param array $errors List of PaymentError
*/
- public function setErrors( $errors ) {
- $this->errors = $errors;
+ public function addErrors( $errors ) {
+ $this->errors += $errors;
}
/**
* Add an error to the internal $errors array
- * @param string $code identifies the error type
- * @param array $error as specified in @see errors
+ * @param PaymentError $error
*/
- public function addError( $code, $error ) {
- $this->errors[$code] = $error;
+ public function addError( $error ) {
+ $this->errors[] = $error;
}
/**
diff --git a/gateway_common/gateway.adapter.php
b/gateway_common/gateway.adapter.php
index 8eb8861..de8b23d 100644
--- a/gateway_common/gateway.adapter.php
+++ b/gateway_common/gateway.adapter.php
@@ -144,6 +144,8 @@
/**
* $transaction_response is the member var that keeps track of the
results of
* the latest discrete transaction with the gateway.
+ * TODO: We might want to deprecate this and pass out of do_transaction
+ * using functional style.
* @var PaymentTransactionResponse
*/
protected $transaction_response;
@@ -153,16 +155,7 @@
*/
protected $final_status;
/**
- * @var array Map of errors preventing this transaction from continuing.
- * Structure is like:
- * [
- * # An i18n key name to an error message that will display
atop of
- * # the screen, indicating that something general needs fixing.
- * 'general' => 'warning-currency-fallback',
- * # Example of a very specific error string that only the
gateway
- * # could calculate.
- * 'address' => 'key saying: "Address is required for
E-Commerce transactions."',
- * ]
+ * @var array PaymentError[] List of errors preventing this transaction
from continuing.
*/
protected $errors = array();
@@ -908,16 +901,13 @@
$this->session_addDonorData();
if ( !$this->validatedOK() ){
//If the data didn't validate okay, prevent all data
transmissions.
- // TODO: Rename variable to "response".
- $return = new PaymentTransactionResponse();
- $return->setCommunicationStatus( false );
- $return->setMessage( 'Failed data validation' );
- foreach ( $this->errors as $code => $error ) {
- $return->addError( $code, array( 'message' =>
$error, 'logLevel' => LogLevel::INFO, 'debugInfo' => '' ) );
- }
// TODO: should we set $this->transaction_response ?
+ $response = new PaymentTransactionResponse();
+ $response->setCommunicationStatus( false );
+ $response->setMessage( 'Failed data validation' );
+ $response->addErrors( $this->errors );
$this->logger->info( "Failed Validation. Aborting
$transaction " . print_r( $this->errors, true ) );
- return $return;
+ return $response;
}
$retryCount = 0;
@@ -964,7 +954,7 @@
* * post_process_<strtolower($transaction)>
*
* @param string $transaction Name of the transaction being performed
- * @param &string() $retryVars Reference to an array of variables that
caused the
+ * @param &string[] $retryVars Reference to an array of variables that
caused the
* transaction to fail.
*
* @return PaymentTransactionResponse
@@ -988,12 +978,10 @@
$this->logger->info( "Failed pre-process checks
for transaction type $transaction." );
$this->transaction_response->setCommunicationStatus( false );
$this->transaction_response->setMessage(
$this->getErrorMapByCodeAndTranslate( 'internal-0000' ) );
- $this->transaction_response->setErrors( array(
- 'internal-0000' => array(
- 'debugInfo' => "Failed
pre-process checks for transaction type $transaction.",
- 'message' =>
$this->getErrorMapByCodeAndTranslate( 'internal-0000' ),
- 'logLevel' => LogLevel::INFO
- )
+ $this->transaction_response->addError( new
PaymentsError(
+ 'internal-0000',
+ "Failed pre-process checks for
transaction type $transaction.",
+ LogLevel::INFO
) );
return $this->transaction_response;
}
@@ -1042,12 +1030,10 @@
$this->transaction_response->setCommunicationStatus(
false );
$this->transaction_response->setMessage(
$this->getErrorMapByCodeAndTranslate( 'internal-0001' ) );
- $this->transaction_response->setErrors( array(
- 'internal-0001' => array(
- 'debugInfo' => 'Malformed gateway
definition. Cannot continue: Aborting.\n' . $e->getMessage(),
- 'message' =>
$this->getErrorMapByCodeAndTranslate( 'internal-0001' ),
- 'logLevel' => LogLevel::CRITICAL
- )
+ $this->transaction_response->addError( new
PaymentsError(
+ 'internal-0001',
+ 'Malformed gateway definition. Cannot continue:
Aborting.\n' . $e->getMessage(),
+ LogLevel::CRITICAL
) );
return $this->transaction_response;
@@ -1066,12 +1052,13 @@
try {
$this->processResponse( $formatted );
} catch ( ResponseProcessingException $ex ) {
+ // TODO: Should we integrate
ResponseProcessingException with PaymentsError?
$errCode = $ex->getErrorCode();
$retryVars = $ex->getRetryVars();
- $this->transaction_response->addError(
$errCode, array(
- 'message' =>
$this->getErrorMapByCodeAndTranslate( $errCode ),
- 'debugInfo' => $ex->getMessage(),
- 'logLevel' => LogLevel::ERROR
+ $this->transaction_response->addError( new
PaymentsError(
+ $errCode,
+ $ex->getMessage(),
+ LogLevel::ERROR
) );
}
@@ -1081,12 +1068,10 @@
$this->transaction_response->setCommunicationStatus(
false );
$this->transaction_response->setMessage(
$this->getErrorMapByCodeAndTranslate( 'internal-0002' ) );
- $this->transaction_response->setErrors( array(
- 'internal-0002' => array(
- 'debugInfo' => $logMessage,
- 'message' =>
$this->getErrorMapByCodeAndTranslate( 'internal-0002' ),
- 'logLevel' => LogLevel::ERROR
- )
+ $this->transaction_response->addError( new
PaymentsError(
+ 'internal-0002'
+ $logMessage,
+ LogLevel::ERROR
) );
}
@@ -1099,12 +1084,10 @@
// Set this by key so that the result object still has
all the cURL data
$this->transaction_response->setCommunicationStatus(
false );
$this->transaction_response->setMessage(
$this->getErrorMapByCodeAndTranslate( $errCode ) );
- $this->transaction_response->setErrors( array(
- $errCode => array(
- 'debugInfo' => "$transaction
Communication failed (errcode $errCode), will reattempt!",
- 'message' =>
$this->getErrorMapByCodeAndTranslate( $errCode ),
- 'logLevel' => LogLevel::CRITICAL
- )
+ $this->transaction_response->addError( new
PaymentsError(
+ $errCode
+ "$transaction Communication failed (errcode
$errCode), will reattempt!",
+ LogLevel::CRITICAL
) );
}
@@ -1126,12 +1109,10 @@
$this->logger->info( "Failed post-process
checks for transaction type $transaction." );
$this->transaction_response->setCommunicationStatus( false );
$this->transaction_response->setMessage(
$this->getErrorMapByCodeAndTranslate( 'internal-0000' ) );
- $this->transaction_response->setErrors( array(
- 'internal-0000' => array(
- 'debugInfo' => "Failed
post-process checks for transaction type $transaction.",
- 'message' =>
$this->getErrorMapByCodeAndTranslate( 'internal-0000' ),
- 'logLevel' => LogLevel::INFO
- )
+ $this->transaction_response->addError( new
PaymentsError(
+ 'internal-0000',
+ "Failed post-process checks for
transaction type $transaction.",
+ LogLevel::INFO
) );
return $this->transaction_response;
}
@@ -2129,11 +2110,8 @@
*/
public function getTransactionErrors() {
- if ( $this->transaction_response &&
$this->transaction_response->getErrors() ) {
- $simplify = function( $error ) {
- return $error['message'];
- };
- return array_map( $simplify,
$this->transaction_response->getErrors() );
+ if ( $this->transaction_response ) {
+ return $this->transaction_response->getErrors();
} else {
return array();
}
@@ -2162,15 +2140,10 @@
/**
* Add errors those already stored.
*
- * @param array $errors Map from field or field group key to error
string.
- * May contain one or multiple erroring fields.
- * Merged by key rather than as a list, hence the awkward, surprise
plural.
- *
- * TODO: Encapsulate errors as objects. Provide both merge list and
append item to list methods?
- * FIXME: Nasty that array_merge overwrites rather than appending. Fix
while encapsulating the list.
+ * @param array $errors List of PaymentErrors.
*/
- public function mergeError( $errors ) {
- $this->errors = array_merge( $this->errors, $errors );
+ public function addErrors( $errors ) {
+ $this->errors += $errors;
}
/**
@@ -2463,7 +2436,7 @@
}
// TODO: Rewrite as something modular? It's in-between
validation and normalization...
- if ( isset( $this->errors['currency_code'] ) ) {
+ if ( $this->hasValidationError( 'currency_code' ) ) {
// Try to fall back to a default currency, clearing the
error if
// successful.
$this->fallbackToDefaultCurrency();
@@ -2545,7 +2518,7 @@
$this->logger->info( "Unsupported currency $oldCurrency forced
to $defaultCurrency" );
// Clear the currency error.
- unset( $this->errors['currency_code'] );
+ $this->clearValidationError( 'currency_code' );
$notify = $this->getGlobal( 'NotifyOnConvert' );
@@ -2558,7 +2531,9 @@
$this->dataObj->getVal( 'language' ),
array( $defaultCurrency )
);
- $this->mergeError( $error );
+ $this->addError( new ValidationError(
+ //XXX
+ ) );
}
}
--
To view, visit https://gerrit.wikimedia.org/r/344042
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I39bcc0c84609e681fb9c15db51058ee3ccd7fad8
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/DonationInterface
Gerrit-Branch: master
Gerrit-Owner: Awight <awi...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
