jenkins-bot has submitted this change and it was merged.

Change subject: Split staging from unstaging
......................................................................


Split staging from unstaging

The combined stage_ helpers were muddying things, and were often only
working correctly when used in the unstaged -> staged direction.

Many of these functions were working on staged source data, when they
should have been working on the unstaged data.  This was mostly working
by accident, because we copy the normalized data on top of both the staged
and unstaged data before the staging code runs.

Change-Id: If2d241fed6cc055123abfc97d58e25bdd34ae0f8
---
M adyen_gateway/adyen.adapter.php
M amazon_gateway/amazon.adapter.php
M gateway_common/GatewayPage.php
M gateway_common/gateway.adapter.php
M gateway_forms/RapidHtml.php
M globalcollect_gateway/globalcollect.adapter.php
M globalcollect_gateway/globalcollect_gateway.body.php
M globalcollect_gateway/scripts/orphan_adapter.php
M paypal_gateway/paypal.adapter.php
M tests/Adapter/WorldPay/WorldPayTest.php
M tests/GatewayPageTest.php
M tests/GatewayValidationTest.php
M worldpay_gateway/worldpay.adapter.php
13 files changed, 243 insertions(+), 282 deletions(-)

Approvals:
  Ejegg: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/adyen_gateway/adyen.adapter.php b/adyen_gateway/adyen.adapter.php
index 007eaa7..b2641aa 100644
--- a/adyen_gateway/adyen.adapter.php
+++ b/adyen_gateway/adyen.adapter.php
@@ -199,7 +199,7 @@
                                case 'donate':
                                        $formaction = $this->getGlobal( 
'BaseURL' ) . '/hpp/pay.shtml';
                                        $this->runAntifraudHooks();
-                                       $this->addData( array ( 'risk_score' => 
$this->risk_score ) ); //this will also fire off staging again.
+                                       $this->addRequestData( array ( 
'risk_score' => $this->risk_score ) ); //this will also fire off staging again.
                                        if ( $this->getValidationAction() != 
'process' ) {
                                                // copied from base class.
                                                $this->log( "Failed pre-process 
checks for transaction type $transaction.", LOG_INFO );
@@ -516,44 +516,15 @@
        protected function stage_language( $type = 'request' ) {
        */
 
-       /**
-        * Stage: amount
-        *
-        * For example: JPY 1000.05 get changed to 100005. This need to be 
100000.
-        * For example: JPY 1000.95 get changed to 100095. This need to be 
100000.
-        *
-        * @param string        $type   request|response
-        */
-       protected function stage_amount( $type = 'request' ) {
-               if ( !isset( $this->staged_data['amount'] ) || !isset( 
$this->staged_data['currency_code'] ) ) {
-                       //can't do anything with amounts at all. Just go home.
-                       return;
-               }
-
-               switch ( $type ) {
-                       case 'request':
-                               if ( !DataValidator::is_fractional_currency( 
$this->staged_data['currency_code'] ) ) {
-                                       $this->staged_data['amount'] = floor( 
$this->staged_data['amount'] );
-                               }
-
-                               $this->staged_data['amount'] = 
$this->staged_data['amount'] * 100;
-                               break;
-
-                       case 'response':
-                               $this->staged_data['amount'] = 
$this->staged_data['amount'] / 100;
-                               break;
-               }
-       }
-
-       protected function stage_risk_score( $type = 'request' ) {
+       protected function stage_risk_score() {
                //This isn't smart enough to grab a new value here;
                //Late-arriving values have to trigger a restage via addData or
                //this will always equil the risk_score at the time of object
                //construction. Still need the formatting, though.
-               $this->staged_data['risk_score'] = ( string ) round( 
$this->staged_data['risk_score'] );
+               $this->staged_data['risk_score'] = ( string ) round( 
$this->unstaged_data['risk_score'] );
        }
 
-       protected function stage_hpp_signature( $type = 'request' ) {
+       protected function stage_hpp_signature() {
                $keys = array(
                        'amount',
                        'currency_code',
@@ -577,7 +548,7 @@
                $this->staged_data['hpp_signature'] = 
$this->calculateSignature( $sig_values );
        }
 
-       protected function stage_billing_signature( $type = 'request' ) {
+       protected function stage_billing_signature() {
                $keys = array(
                        'street',
                        'city',
diff --git a/amazon_gateway/amazon.adapter.php 
b/amazon_gateway/amazon.adapter.php
index ebab94b..767b817 100644
--- a/amazon_gateway/amazon.adapter.php
+++ b/amazon_gateway/amazon.adapter.php
@@ -26,7 +26,7 @@
                parent::__construct( $options );
 
                if ($this->getData_Unstaged_Escaped( 'payment_method' ) == null 
) {
-                       $this->addData(
+                       $this->addRequestData(
                                array( 'payment_method' => 'amazon' )
                        );
                }
@@ -403,7 +403,7 @@
                        }
                }
                //TODO: consider prioritizing the session vars
-               $this->addData( $add_data ); //using the gateway's addData 
function restages everything
+               $this->addResponseData( $add_data ); //using the gateway's 
addData function restages everything
 
                $txnid = $this->dataObj->getVal_Escaped( 'gateway_txn_id' );
                $email = $this->dataObj->getVal_Escaped( 'email' );
diff --git a/gateway_common/GatewayPage.php b/gateway_common/GatewayPage.php
index 984baaa..f92dc29 100644
--- a/gateway_common/GatewayPage.php
+++ b/gateway_common/GatewayPage.php
@@ -327,7 +327,7 @@
                        $newAmount = floor( $usdAmount * 
$conversionRates[$defaultCurrency] );
                }
 
-               $this->adapter->addData( array(
+               $this->adapter->addRequestData( array(
                        'amount' => $newAmount,
                        'currency_code' => $defaultCurrency
                ) );
diff --git a/gateway_common/gateway.adapter.php 
b/gateway_common/gateway.adapter.php
index 51faed6..5afc88c 100644
--- a/gateway_common/gateway.adapter.php
+++ b/gateway_common/gateway.adapter.php
@@ -150,7 +150,7 @@
         * Called in the constructor, this function should be used to define
         * pieces of default data particular to the gateway. It will be up to
         * the child class to poke the data through to the data object
-        * (probably with $this->addData()).
+        * (probably with $this->addRequestData()).
         * DO NOT set default payment information here (or anywhere, really).
         * That would be naughty.
         */
@@ -378,7 +378,7 @@
 
                $this->account_config = $acctConfig[ $this->account_name ];
 
-               $this->addData( array(
+               $this->addRequestData( array(
                        'gateway_account' => $this->account_name,
                ) );
        }
@@ -507,20 +507,18 @@
        }
 
        /**
-        *  A helper function to let us stash extra data after the form has 
been submitted.
+        * A helper function to let us stash extra data after the form has been 
submitted.
         *
         * @param array  $dataArray An associative array of data.
-        * @param string $pipelineStage 'request' to the gateway or 'response' 
from the
-        *                              gateway depending on what you're 
actually doing.
         */
-       public function addData( $dataArray, $pipelineStage = 'request' ) {
+       public function addRequestData( $dataArray ) {
                $this->dataObj->addData( $dataArray );
 
                $calculated_fields = $this->dataObj->getCalculatedFields();
                $data_fields = array_keys( $dataArray );
                $data_fields = array_merge( $data_fields, $calculated_fields );
 
-               foreach ( $data_fields as $value){
+               foreach ( $data_fields as $value ) {
                        $this->refreshGatewayValueFromSource( $value );
                }
 
@@ -530,8 +528,23 @@
                //function, to calculate any other staged var.
                $changed_staged_vars = array_intersect( $this->staged_vars, 
$data_fields );
                if ( count( $changed_staged_vars ) ) {
-                       $this->stageData( $pipelineStage );
+                       $this->stageData();
                }
+       }
+
+       /**
+        * Stash and process response data.
+        *
+        * @param array  $dataArray An associative array of data.
+        */
+       public function addResponseData( $dataArray ) {
+               foreach ( $dataArray as $key => $value ) {
+                       $this->staged_data[$key] = $value;
+               }
+
+               $this->unstageData();
+
+               $this->dataObj->addData( $this->unstaged_data );
        }
 
        /**
@@ -1989,25 +2002,38 @@
        }
 
        /**
-        * @param string $type Whatever types of staging you feel like having 
in your child class.
-        * ...but usually request and response. I think.
+        * Run any staging functions provided by the adapter
         */
-       protected function stageData( $type = 'request' ) {
-               if ( $type === 'request' ){
-                       //reset from our normalized unstaged data so we never 
double-stage
-                       $this->staged_data = $this->unstaged_data;
-               }
+       protected function stageData() {
+               // Copy data, the default is to not change the values.
+               //reset from our normalized unstaged data so we never 
double-stage
+               $this->staged_data = $this->unstaged_data;
+
+               // This allows transactions to each stage different data.
                $this->defineStagedVars();
-               //If we tried to piggyback off the same loop, all the vars 
wouldn't be ready, and some staging functions will require
-               //multiple variables.
+
                foreach ( $this->staged_vars as $field ) {
                        $function_name = 'stage_' . $field;
-                       $this->executeIfFunctionExists( $function_name, $type );
+                       $this->executeIfFunctionExists( $function_name );
                }
 
                // Format the staged data
-               if ($type === 'request'){
-                       $this->formatStagedData();
+               $this->formatStagedData();
+       }
+
+       /**
+        * Run any unstaging functions to decode processor responses
+        */
+       protected function unstageData() {
+               // Copy from staged data, 
+               $this->unstaged_data = $this->staged_data;
+
+               // FIXME: we should have a list of unstaged_vars for each 
transaction.
+               $this->defineStagedVars();
+
+               foreach ( $this->staged_vars as $field ) {
+                       $function_name = 'unstage_' . $field;
+                       $this->executeIfFunctionExists( $function_name );
                }
        }
 
@@ -2047,57 +2073,86 @@
                }
        }
 
+       /**
+        * Stage: amount
+        *
+        * For example: JPY 1000.05 get changed to 100005. This need to be 
100000.
+        * For example: JPY 1000.95 get changed to 100095. This need to be 
100000.
+        */
+       protected function stage_amount() {
+               if ( !$this->getData_Unstaged_Escaped( 'amount' )
+                       || !$this->getData_Unstaged_Escaped( 'currency_code' )
+               ) {
+                       //can't do anything with amounts at all. Just go home.
+                       unset( $this->staged_data['amount'] );
+                       return;
+               }
+
+               $amount = $this->getData_Unstaged_Escaped( 'amount' );
+               if ( !DataValidator::is_fractional_currency( 
$this->getData_Unstaged_Escaped( 'currency_code' ) ) ) {
+                       $amount = floor( $amount );
+               }
+
+               $this->staged_data['amount'] = $amount * 100;
+       }
+
+       protected function unstage_amount() {
+               $this->unstaged_data['amount'] = $this->getData_Staged( 
'amount' ) / 100;
+       }
+
        /**
-        * Stage the street address. In the event that there isn't anything in
-        * there, we need to send something along so that AVS checks get 
triggered
-        * at all.
+        * Stage the street address
+        *
+        * In the event that there isn't anything in there, we need to send
+        * something along so that AVS checks get triggered at all.
+        *
         * The zero is intentional: Allegedly, Some banks won't perform the 
check
         * if the address line contains no numerical data.
-        * @param string $type request|response
         */
-       protected function stage_street( $type = 'request' ) {
-               if ( !isset( $this->staged_data['street'] ) ) {
+       protected function stage_street() {
+               if ( !isset( $this->unstaged_data['street'] ) ) {
                        //nothing to do.
                        return;
                }
-               if ( $type === 'request' ){
-                       $is_garbage = false;
-                       $street = trim( $this->staged_data['street'] );
-                       ( strlen( $street ) === 0 ) ? $is_garbage = true : null;
-                       ( !DataValidator::validate_not_just_punctuation( 
$street ) ) ? $is_garbage = true : null;
 
-                       if ( $is_garbage ){
-                               $this->staged_data['street'] = 'N0NE PROVIDED'; 
//The zero is intentional. See function comment.
-                       }
+               $is_garbage = false;
+               $street = trim( $this->unstaged_data['street'] );
+               ( strlen( $street ) === 0 ) ? $is_garbage = true : null;
+               ( !DataValidator::validate_not_just_punctuation( $street ) ) ? 
$is_garbage = true : null;
+
+               if ( $is_garbage ){
+                       $this->staged_data['street'] = 'N0NE PROVIDED'; //The 
zero is intentional. See function comment.
                }
        }
 
        /**
-        * Stage the zip. In the event that there isn't anything in
-        * there, we need to send something along so that AVS checks get 
triggered
-        * at all.
-        * @param string $type request|response
+        * Stage the zip / postal code
+        *
+        * In the event that there isn't anything in there, we need to send
+        * something along so that AVS checks get triggered at all.
         */
-       protected function stage_zip( $type = 'request' ) {
-               if ( !isset( $this->staged_data['zip'] ) ) {
+       protected function stage_zip() {
+               if ( !isset( $this->unstaged_data['zip'] ) ) {
                        //nothing to do.
                        return;
                }
-               if ( $type === 'request' && strlen( trim( 
$this->staged_data['zip'] ) ) === 0  ){
+               if ( strlen( trim( $this->unstaged_data['zip'] ) ) === 0  ){
                        //it would be nice to check for more here, but the 
world has some
                        //straaaange postal codes...
-                       $this->staged_data['zip'] = '0';
+                       $this->unstaged_data['zip'] = '0';
                }
 
                //country-based zip grooming to make AVS (marginally) happy
-               switch ($this->getData_Staged( 'country' ) ){
+               switch ($this->getData_Unstaged_Escaped( 'country' ) ){
                        case 'CA':
                                //Canada goes "A0A 0A0"
-                               $this->staged_data['zip'] = strtoupper( 
$this->staged_data['zip'] );
+                               $this->staged_data['zip'] = strtoupper( 
$this->unstaged_data['zip'] );
                                //In the event that they only forgot the space, 
help 'em out.
                                $regex = 
'/[A-Z]{1}\d{1}[A-Z]{1}\d{1}[A-Z]{1}\d{1}/';
-                               if ( strlen( $this->staged_data['zip'] ) === 6 
&& preg_match( $regex, $this->staged_data['zip'] ) ) {
-                                       $zip = $this->staged_data['zip'];
+                               if ( strlen( $this->staged_data['zip'] ) === 6
+                                       && preg_match( $regex, 
$this->unstaged_data['zip'] )
+                               ) {
+                                       $zip = $this->unstaged_data['zip'];
                                        $this->staged_data['zip'] = substr( 
$zip, 0, 3 ) . ' ' . substr( $zip, 3, 3 );
                                }
                                break;
@@ -3272,7 +3327,7 @@
                $_SESSION[$gateway_ident . 'EditToken'] = $unsalted;
                $salted = $this->token_getSaltedSessionToken();
 
-               $this->addData( array ( 'token' => $salted ) );
+               $this->addRequestData( array ( 'token' => $salted ) );
        }
 
        /**
@@ -3322,7 +3377,7 @@
 
                        // match token
                        if ( !$this->dataObj->isSomething( 'token' ) ) {
-                               $this->addData( array ( 'token' => $token ) );
+                               $this->addRequestData( array ( 'token' => 
$token ) );
                        }
                        $token_check = $this->getData_Unstaged_Escaped( 'token' 
);
 
@@ -3414,13 +3469,13 @@
                        return;
                } else if ( $this->session_getLastRapidHTMLForm() ) { //This 
will take care of it if this is an ajax request, or a 3rd party return hit
                        $new_ff = $this->session_getLastRapidHTMLForm();
-                       $this->addData( array ( 'ffname' => $new_ff ) );
+                       $this->addRequestData( array ( 'ffname' => $new_ff ) );
 
                        //and debug log a little
                        $this->log( "Setting form to last successful 
('$new_ff')", LOG_DEBUG );
                } else if ( GatewayFormChooser::isValidForm( $ffname . 
"-$country", $country, $currency, $payment_method, $payment_submethod, 
$recurring, $gateway ) ) {
                        //if the country-specific version exists, use that.
-                       $this->addData( array ( 'ffname' => $ffname . 
"-$country" ) );
+                       $this->addRequestData( array ( 'ffname' => $ffname . 
"-$country" ) );
 
                        //I'm only doing this for serious legacy purposes. This 
mess needs to stop itself. To help with the mess-stopping...
                        $message = "ffname '$ffname' was invalid, but the 
country-specific '$ffname-$country' works. utm_source = '$utm', referrer = 
'$ref'";
@@ -3428,7 +3483,7 @@
                } else {
                        //Invalid form. Go get one that is valid, and squak in 
the error logs.
                        $new_ff = GatewayFormChooser::getOneValidForm( 
$country, $currency, $payment_method, $payment_submethod, $recurring, $gateway 
);
-                       $this->addData( array ( 'ffname' => $new_ff ) );
+                       $this->addRequestData( array ( 'ffname' => $new_ff ) );
 
                        //now construct a useful error message
                        $this->log(
@@ -3701,7 +3756,7 @@
                }
 
                //tell DonationData about it
-               $this->addData( array ( 'order_id' => $id ) );
+               $this->addRequestData( array ( 'order_id' => $id ) );
                return $id;
        }
 
diff --git a/gateway_forms/RapidHtml.php b/gateway_forms/RapidHtml.php
index d6cff83..b79cd93 100644
--- a/gateway_forms/RapidHtml.php
+++ b/gateway_forms/RapidHtml.php
@@ -515,7 +515,7 @@
                                        $params = array_merge( $preserve, 
$params );
                                }
                                //If this is just the one thing, we might move 
this inside DonationData for clarity's sake...
-                               $this->gateway->addData( array ( 'ffname_retry' 
=> GatewayFormChooser::buildPaymentsFormURL( $back_form, $params ) ) );
+                               $this->gateway->addRequestData( array ( 
'ffname_retry' => GatewayFormChooser::buildPaymentsFormURL( $back_form, $params 
) ) );
                        }
                } else {
                        //No special type... let's add this to the form stack 
and call it good.
diff --git a/globalcollect_gateway/globalcollect.adapter.php 
b/globalcollect_gateway/globalcollect.adapter.php
index 8fea4fd..de70aa6 100644
--- a/globalcollect_gateway/globalcollect.adapter.php
+++ b/globalcollect_gateway/globalcollect.adapter.php
@@ -326,7 +326,7 @@
                        'effort_id' => '1',
                );
 
-               $this->addData( $defaults );
+               $this->addRequestData( $defaults );
        }
 
        /**
@@ -1108,8 +1108,11 @@
                }
 
                $is_orphan = false;
-               if ( count( $qsResults ) ){ //nothing unusual here.
-                       $this->addData( $qsResults );
+               if ( count( $qsResults ) ){
+                       // Nothing unusual here.  Oh, except we are reading 
query parameters from
+                       // what we hope is a redirect back from the processor, 
caused by an earlier
+                       // transaction.
+                       $this->addResponseData( $qsResults );
                        $logmsg = 'CVV Result from querystring: ' . 
$this->getData_Unstaged_Escaped( 'cvv_result' );
                        $logmsg .= ', AVS Result from querystring: ' . 
$this->getData_Unstaged_Escaped( 'avs_result' );
                        $this->log( $logmsg );
@@ -1155,7 +1158,7 @@
                                        }
                                }
                        }
-                       $this->addData( $xmlResults );
+                       $this->addResponseData( $xmlResults );
                        $logmsg = 'CVV Result from XML: ' . 
$this->getData_Unstaged_Escaped( 'cvv_result' );
                        $logmsg .= ', AVS Result from XML: ' . 
$this->getData_Unstaged_Escaped( 'avs_result' );
                        $this->log( $logmsg );
@@ -1266,7 +1269,7 @@
                                }
 
                                if ( count( $addme ) ){
-                                       $this->addData( $addme );
+                                       $this->addResponseData( $addme );
                                }
                        }
 
@@ -1454,7 +1457,7 @@
                                        $this->log( "inside " . 
$data['ORDERID'] );
                                        $this->normalizeOrderID( 
$data['ORDERID'] );
                                        $this->log( print_r( 
$this->getOrderIDMeta(), true ) );
-                                       $this->addData( array ( 'order_id' => 
$data['ORDERID'] ) );
+                                       $this->addRequestData( array ( 
'order_id' => $data['ORDERID'] ) );
                                        $this->log( print_r( 
$this->getOrderIDMeta(), true ) );
                                        $this->session_addDonorData();
                                }
@@ -1859,39 +1862,38 @@
                );
        }
 
-       protected function stage_language( $type = 'request' ) {
-               $language = strtolower( $this->staged_data['language'] );
+       protected function stage_language() {
+               $language = strtolower( $this->getData_Unstaged_Escaped( 
'language' ) );
 
-               switch ( $type ) {
-                       case 'request':
-                               if ( !in_array( $language, 
$this->getAvailableLanguages() ) ) {
-                                       $fallbacks = Language::getFallbacksFor( 
$language );
-                                       foreach ( $fallbacks as $fallback ) {
-                                               if ( in_array( $fallback, 
$this->getAvailableLanguages() ) ) {
-                                                       $language = $fallback;
-                                                       break;
-                                               }
-                                       }
+               if ( !in_array( $language, $this->getAvailableLanguages() ) ) {
+                       $fallbacks = Language::getFallbacksFor( $language );
+                       foreach ( $fallbacks as $fallback ) {
+                               if ( in_array( $fallback, 
$this->getAvailableLanguages() ) ) {
+                                       $language = $fallback;
+                                       break;
                                }
+                       }
+               }
 
-                               if ( !in_array( $language, 
$this->getAvailableLanguages() ) ){
-                                       $language = 'en';
-                               }
+               if ( !in_array( $language, $this->getAvailableLanguages() ) ){
+                       $language = 'en';
+               }
 
-                               if ( $language === 'zh' ) { //Handles GC's 
mutant Chinese code.
-                                       $language = 'sc';
-                               }
-
-                               break;
-                       case 'response':
-                               if ( $language === 'sc' ){
-                                       $language = 'zh';
-                               }
-                               break;
+               if ( $language === 'zh' ) { //Handles GC's mutant Chinese code.
+                       $language = 'sc';
                }
 
                $this->staged_data['language'] = $language;
+       }
 
+       protected function unstage_language() {
+               $language = strtolower( $this->staged_data['language'] );
+
+               if ( $language === 'sc' ){
+                       $language = 'zh';
+               }
+
+               $this->unstaged_data['language'] = $language;
        }
 
        /**
@@ -1934,43 +1936,11 @@
        }
 
        /**
-        * Stage: amount
-        *
-        * For example: JPY 1000.05 get changed to 100005. This need to be 
100000.
-        * For example: JPY 1000.95 get changed to 100095. This need to be 
100000.
-        *
-        * @param string        $type   request|response
-        */
-       protected function stage_amount( $type = 'request' ) {
-               if ( !isset( $this->staged_data['amount'] ) || !isset( 
$this->staged_data['currency_code'] ) ) {
-                       //can't do anything with amounts at all. Just go home.
-                       return;
-               }
-               switch ( $type ) {
-                       case 'request':
-
-                               if ( !DataValidator::is_fractional_currency( 
$this->staged_data['currency_code'] ) ) {
-                                       $this->staged_data['amount'] = floor( 
$this->staged_data['amount'] );
-                               }
-
-                               $this->staged_data['amount'] = 
$this->staged_data['amount'] * 100;
-
-                               break;
-                       case 'response':
-                               $this->staged_data['amount'] = 
$this->staged_data['amount'] / 100;
-                               break;
-               }
-       }
-
-       /**
         * Stage: card_num
-        *
-        * @param string        $type   request|response
         */
-       protected function stage_card_num( $type = 'request' ) {
-               //I realize that the $type isn't used. Voodoo.
-               if ( array_key_exists( 'card_num', $this->staged_data ) ) {
-                       $this->staged_data['card_num'] = str_replace( ' ', '', 
$this->staged_data['card_num'] );
+       protected function stage_card_num() {
+               if ( array_key_exists( 'card_num', $this->unstaged_data ) ) {
+                       $this->staged_data['card_num'] = str_replace( ' ', '', 
$this->unstaged_data['card_num'] );
                }
        }
 
@@ -1979,7 +1949,7 @@
         * Stages the payment product ID for GC.
         * Not what I had in mind to begin with, but this *completely* blew up.
         */
-       public function stage_payment_product( $type = 'request' ) {
+       public function stage_payment_product() {
                //cc used to look in card_type, but that's been an alias for 
payment_submethod for a while. DonationData takes care of it.
                $payment_method = array_key_exists( 'payment_method', 
$this->staged_data ) ? $this->staged_data['payment_method'] : false;
                $payment_submethod = array_key_exists( 'payment_submethod', 
$this->staged_data ) ? $this->staged_data['payment_submethod'] : false;
@@ -1997,10 +1967,6 @@
                                'discover' => '128',
                                'cb' => '130',
                        );
-
-                       if ( $type === 'response' ) {
-                               $types = array_flip( $types );
-                       }
 
                        if ( (!is_null( $payment_submethod ) ) && 
array_key_exists( $payment_submethod, $types ) ) {
                                $this->staged_data['payment_product'] = 
$types[$payment_submethod];
@@ -2034,6 +2000,7 @@
                                }
                        }
 
+                       // FIXME: that's one hell of a staging function.  Move 
this to a do_transaction helper.
                        if ( $enable3ds ) {
                                $this->log( "3dSecure enabled for $currency in 
$country" );
                                
$this->transactions['INSERT_ORDERWITHPAYMENT']['values']['AUTHENTICATIONINDICATOR']
 = '1';
@@ -2056,43 +2023,37 @@
         * Stage branch_code for Direct Debit.
         * Check the data constraints, and zero-pad out to that number where 
possible.
         * Exceptions for the defaults are set in stage_country so we can see 
them all in the same place
-        * @param string $type request|response
         */
-       protected function stage_branch_code( $type = 'request' ) {
-               if ( $type === 'request' && isset( 
$this->staged_data['branch_code'] ) ) {
-                       $newval = DataValidator::getZeroPaddedValue( 
$this->staged_data['branch_code'], 
$this->dataConstraints['branch_code']['length'] );
-                       if ( $newval ) {
-                               $this->staged_data['branch_code'] = $newval;
-                       }
-               }
+       protected function stage_branch_code() {
+               $this->stageAndZeroPad( 'branch_code' );
        }
 
        /**
         * Stage bank_code for Direct Debit.
         * Check the data constraints, and zero-pad out to that number where 
possible.
         * Exceptions for the defaults are set in stage_country so we can see 
them all in the same place
-        * @param string $type request|response
         */
-       protected function stage_bank_code( $type = 'request' ) {
-               if ( $type === 'request' && isset( 
$this->staged_data['bank_code'] ) ) {
-                       $newval = DataValidator::getZeroPaddedValue( 
$this->staged_data['bank_code'], $this->dataConstraints['bank_code']['length'] 
);
-                       if ( $newval ) {
-                               $this->staged_data['bank_code'] = $newval;
-                       }
-               }
+       protected function stage_bank_code() {
+               $this->stageAndZeroPad( 'bank_code' );
        }
 
        /**
         * Stage account_number for Direct Debit.
         * Check the data constraints, and zero-pad out to that number where 
possible.
         * Exceptions for the defaults are set in stage_country so we can see 
them all in the same place
-        * @param string $type request|response
         */
-       protected function stage_account_number( $type = 'request' ) {
-               if ( $type === 'request' && isset( 
$this->staged_data['account_number'] ) ) {
-                       $newval = DataValidator::getZeroPaddedValue( 
$this->staged_data['account_number'], 
$this->dataConstraints['account_number']['length'] );
+       protected function stage_account_number() {
+               $this->stageAndZeroPad( 'account_number' );
+       }
+
+       /**
+        * Helper to stage a zero-padded number
+        */
+       protected function stageAndZeroPad( $key ) {
+               if ( isset( $this->unstaged_data[$key] ) ) {
+                       $newval = DataValidator::getZeroPaddedValue( 
$this->unstaged_data[$key], $this->dataConstraints[$key]['length'] );
                        if ( $newval ) {
-                               $this->staged_data['account_number'] = $newval;
+                               $this->staged_data[$key] = $newval;
                        }
                }
        }
@@ -2101,9 +2062,8 @@
         * Stage: setupStagePaymentMethodForDirectDebit
         *
         * @param string        $payment_submethod
-        * @param string        $type   request|response
         */
-       protected function setupStagePaymentMethodForDirectDebit( 
$payment_submethod, $type = 'request' ) {
+       protected function setupStagePaymentMethodForDirectDebit( 
$payment_submethod ) {
 
                // DATECOLLECT is required on all Direct Debit
                $this->addKeyToTransaction('DATECOLLECT');
@@ -2123,9 +2083,8 @@
         * Stage: setupStagePaymentMethodForEWallets
         *
         * @param string        $payment_submethod
-        * @param string        $type   request|response
         */
-       protected function setupStagePaymentMethodForEWallets( 
$payment_submethod, $type = 'request' ) {
+       protected function setupStagePaymentMethodForEWallets( 
$payment_submethod ) {
 
                // DESCRIPTOR is required on WebMoney, assuming it is required 
for all.
                $this->addKeyToTransaction('DESCRIPTOR');
@@ -2141,8 +2100,6 @@
        /**
         * Stage: payment_method
         *
-        * @param string        $type   request|response
-        *
         * @todo
         * - Need to implement this for credit card if necessary
         * - ISSUERID will need to provide a dropdown for rtbt_eps and 
rtbt_ideal.
@@ -2150,10 +2107,9 @@
         * - DATECOLLECT is using gmdate('Ymd')
         * - DIRECTDEBITTEXT will need to be translated. This is what appears 
on the bank statement for donations for a client. This is hardcoded to: 
Wikimedia Foundation
         */
-       protected function stage_payment_method( $type = 'request' ) {
-
-               $payment_method = array_key_exists( 'payment_method', 
$this->staged_data ) ? $this->staged_data['payment_method']: false;
-               $payment_submethod = array_key_exists( 'payment_submethod', 
$this->staged_data ) ? $this->staged_data['payment_submethod']: false;
+       protected function stage_payment_method() {
+               $payment_method = array_key_exists( 'payment_method', 
$this->unstaged_data ) ? $this->unstaged_data['payment_method']: false;
+               $payment_submethod = array_key_exists( 'payment_submethod', 
$this->unstaged_data ) ? $this->unstaged_data['payment_submethod']: false;
 
                //Having to front-load the country in the payment submethod is 
pretty lame.
                //If we don't have one deliberately set...
@@ -2165,19 +2121,19 @@
                        }
                }
 
-               // These will be grouped and ordred by payment product id
+               // These will be grouped and ordered by payment product id
                switch ( $payment_submethod )  {
 
                        /* Bank transfer */
                        case 'bt':
 
                                // Brazil
-                               if ( $this->staged_data['country'] == 'BR' ) {
+                               if ( $this->unstaged_data['country'] == 'BR' ) {
                                        
$this->dataConstraints['direct_debit_text']['city'] = 50;
                                }
 
                                // Korea - Manual does not specify North or 
South
-                               if ( $this->staged_data['country'] == 'KR' ) {
+                               if ( $this->unstaged_data['country'] == 'KR' ) {
                                        
$this->dataConstraints['direct_debit_text']['city'] = 50;
                                }
                                break;
@@ -2241,10 +2197,10 @@
 
                switch ($payment_method) {
                case 'dd':
-                       $this->setupStagePaymentMethodForDirectDebit( 
$payment_submethod, $type);
+                       $this->setupStagePaymentMethodForDirectDebit( 
$payment_submethod );
                        break;
                case 'ew':
-                       $this->setupStagePaymentMethodForEWallets( 
$payment_submethod, $type);
+                       $this->setupStagePaymentMethodForEWallets( 
$payment_submethod );
                        break;
                }
        }
@@ -2253,11 +2209,9 @@
         * Stage: recurring
         * Adds the recurring payment pieces to the structure of
         * INSERT_ORDERWITHPAYMENT if the recurring field is populated.
-        *
-        * @param string        $type   request|response
         */
-       protected function stage_recurring( $type = 'request' ){
-               if ( ! $this->getData_Staged( 'recurring' ) ){
+       protected function stage_recurring(){
+               if ( !$this->getData_Unstaged_Escaped( 'recurring' ) ) {
                        return;
                } else {
                        
$this->transactions['INSERT_ORDERWITHPAYMENT']['request']['REQUEST']['PARAMS']['ORDER'][]
 = 'ORDERTYPE';
@@ -2269,15 +2223,9 @@
         * Stage: country
         * This should be a catch-all for establishing weird country-based 
rules.
         * Right now, we only have the one, but there could be more here later.
-        *
-        * @param string        $type   request|response
         */
-       protected function stage_country( $type = 'request' ){
-               if ( $type != 'request' ){
-                       return; //nothing to do here.
-               }
-
-               switch ( $this->getData_Staged( 'country' ) ){
+       protected function stage_country() {
+               switch ( $this->getData_Unstaged_Escaped( 'country' ) ){
                        case 'AR' :
                                
$this->transactions['INSERT_ORDERWITHPAYMENT']['request']['REQUEST']['PARAMS']['ORDER'][]
 = 'USAGETYPE';
                                
$this->transactions['INSERT_ORDERWITHPAYMENT']['request']['REQUEST']['PARAMS']['ORDER'][]
 = 'PURCHASETYPE';
@@ -2287,24 +2235,22 @@
                }
        }
 
-       protected function stage_contribution_tracking_id( $type = 'request' ) {
-               $ctid = $this->staged_data['contribution_tracking_id'];
-               if ( $type === 'request' ) {
-                       //append timestamp to ctid
-                       $ctid .= '.' . (( microtime( true ) * 1000 ) % 100000); 
//least significant five
-               } elseif ( $type === 'response' ) {
-                       $ctid = explode( '.', $ctid );
-                       $ctid = $ctid[0];
-               }
+       protected function stage_contribution_tracking_id() {
+               $ctid = $this->unstaged_data['contribution_tracking_id'];
+               //append timestamp to ctid
+               $ctid .= '.' . (( microtime( true ) * 1000 ) % 100000); //least 
significant five
                $this->staged_data['contribution_tracking_id'] = $ctid;
        }
 
-       protected function stage_fiscal_number( $type = 'request' ) {
-               if ( $type != 'request' ){
-                       return; //nothing to do here.
-               }
+       protected function unstage_contribution_tracking_id() {
+               $ctid = $this->staged_data['contribution_tracking_id'];
+               $ctid = explode( '.', $ctid );
+               $ctid = $ctid[0];
+               $this->unstaged_data['contribution_tracking_id'] = $ctid;
+       }
 
-               $this->staged_data['fiscal_number'] = preg_replace( 
"/[\.\/\-]/", "", $this->getData_Staged( 'fiscal_number' ) );
+       protected function stage_fiscal_number() {
+               $this->staged_data['fiscal_number'] = preg_replace( 
"/[\.\/\-]/", "", $this->getData_Unstaged_Escaped( 'fiscal_number' ) );
        }
 
        /**
@@ -2327,32 +2273,23 @@
 
        /**
         * Stage: returnto
-        *
-        * @param string        $type   request|response
         */
-       protected function stage_returnto( $type = 'request' ) {
+       protected function stage_returnto() {
+               // Get the default returnto
+               $returnto = $this->getData_Unstaged_Escaped( 'returnto' );
 
-               if ( $type === 'request' ) {
-
-                       // Get the default returnto
-                       $returnto = $this->getData_Staged( 'returnto' );
-
-                       if ( $this->getData_Unstaged_Escaped( 'payment_method' 
) === 'cc' ){
-
-                               // Add order ID to the returnto URL, only if 
it's not already there.
-                               //TODO: This needs to be more robust (like 
actually pulling the
-                               //qstring keys, resetting the values, and 
putting it all back)
-                               //but for now it'll keep us alive.
-                               if ( $this->getOrderIDMeta( 'generate' ) && 
!is_null( $returnto ) && !strpos( $returnto, 'order_id' ) ) {
-                                       $queryArray = array( 'order_id' => 
$this->staged_data['order_id'] );
-                                       $this->staged_data['returnto'] = 
wfAppendQuery( $returnto, $queryArray );
-                               }
-
-                       } else {
-
-                               // Do we want to set this here?
-                               $this->staged_data['returnto'] = 
$this->getThankYouPage();
+               if ( $this->getData_Unstaged_Escaped( 'payment_method' ) === 
'cc' ) {
+                       // Add order ID to the returnto URL, only if it's not 
already there.
+                       //TODO: This needs to be more robust (like actually 
pulling the
+                       //qstring keys, resetting the values, and putting it 
all back)
+                       //but for now it'll keep us alive.
+                       if ( $this->getOrderIDMeta( 'generate' ) && !is_null( 
$returnto ) && !strpos( $returnto, 'order_id' ) ) {
+                               $queryArray = array( 'order_id' => 
$this->unstaged_data['order_id'] );
+                               $this->staged_data['returnto'] = wfAppendQuery( 
$returnto, $queryArray );
                        }
+               } else {
+                       // FIXME: Do we want to set this here?
+                       $this->staged_data['returnto'] = 
$this->getThankYouPage();
                }
        }
 
diff --git a/globalcollect_gateway/globalcollect_gateway.body.php 
b/globalcollect_gateway/globalcollect_gateway.body.php
index 73a492c..f18cd72 100644
--- a/globalcollect_gateway/globalcollect_gateway.body.php
+++ b/globalcollect_gateway/globalcollect_gateway.body.php
@@ -50,7 +50,7 @@
                                                and 
!$this->adapter->getPaymentSubmethod() ) {
                                        // Synthesize a submethod based on the 
country.
                                        $country_code = strtolower( 
$this->adapter->getData_Unstaged_Escaped( 'country' ) );
-                                       $this->adapter->addData( array(
+                                       $this->adapter->addRequestData( array(
                                                'payment_submethod' => 
"dd_{$country_code}",
                                        ) );
                                }
diff --git a/globalcollect_gateway/scripts/orphan_adapter.php 
b/globalcollect_gateway/scripts/orphan_adapter.php
index 356e3f0..0e0806b 100644
--- a/globalcollect_gateway/scripts/orphan_adapter.php
+++ b/globalcollect_gateway/scripts/orphan_adapter.php
@@ -80,8 +80,8 @@
                $this->revalidate();
        }
 
-       public function addData( $dataArray, $pipelineStage = 'request' ) {
-               parent::addData( $dataArray, $pipelineStage );
+       public function addRequestData( $dataArray ) {
+               parent::addRequestData( $dataArray );
                $this->reAddHardData();
        }
 
diff --git a/paypal_gateway/paypal.adapter.php 
b/paypal_gateway/paypal.adapter.php
index a6fbed2..7017308 100644
--- a/paypal_gateway/paypal.adapter.php
+++ b/paypal_gateway/paypal.adapter.php
@@ -26,7 +26,7 @@
                parent::__construct( $options );
 
                if ($this->getData_Unstaged_Escaped( 'payment_method' ) == null 
) {
-                       $this->addData(
+                       $this->addRequestData(
                                array( 'payment_method' => 'paypal' )
                        );
                }
@@ -224,13 +224,13 @@
                );
        }
 
-       protected function stage_recurring_length( $mode = 'request' ) {
+       protected function stage_recurring_length() {
                if ( array_key_exists( 'recurring_length', $this->staged_data ) 
&& !$this->staged_data['recurring_length'] ) {
                        unset( $this->staged_data['recurring_length'] );
                }
        }
 
-       protected function stage_locale( $mode = 'request' ) {
+       protected function stage_locale() {
                $supported_countries = array(
                        'AU',
                        'AT',
@@ -266,13 +266,13 @@
                        'zh_TW',
                );
 
-               if ( in_array( $this->staged_data['country'], 
$supported_countries ) ) {
-                       $this->staged_data['locale'] = 
$this->staged_data['country'];
+               if ( in_array( $this->unstaged_data['country'], 
$supported_countries ) ) {
+                       $this->staged_data['locale'] = 
$this->unstaged_data['country'];
                }
 
-               $fallbacks = Language::getFallbacksFor( strtolower( 
$this->staged_data['language'] ) );
+               $fallbacks = Language::getFallbacksFor( strtolower( 
$this->unstaged_data['language'] ) );
                foreach ( $fallbacks as $lang ) {
-                       $locale = 
"{$this->staged_data['language']}_{$this->staged_data['country']}";
+                       $locale = 
"{$this->unstaged_data['language']}_{$this->unstaged_data['country']}";
                        if ( in_array( $locale, $supported_full_locales ) ) {
                                $this->staged_data['locale'] = $locale;
                                return;
diff --git a/tests/Adapter/WorldPay/WorldPayTest.php 
b/tests/Adapter/WorldPay/WorldPayTest.php
index e0eb25f..2d79890 100644
--- a/tests/Adapter/WorldPay/WorldPayTest.php
+++ b/tests/Adapter/WorldPay/WorldPayTest.php
@@ -243,7 +243,7 @@
                $this->assertTrue( $gateway->getCVVResult(), 'getCVVResult not 
passing somebody with a match.' );
 
                //and now, for fun, test a wrong code.
-               $gateway->addData( array ( 'cvv_result' => '2' ), 'response' );
+               $gateway->addResponseData( array ( 'cvv_result' => '2' ) );
                $this->assertFalse( $gateway->getCVVResult(), 'getCVVResult not 
failing somebody with garbage.' );
        }
 
diff --git a/tests/GatewayPageTest.php b/tests/GatewayPageTest.php
index ba1f3bc..6050855 100644
--- a/tests/GatewayPageTest.php
+++ b/tests/GatewayPageTest.php
@@ -29,7 +29,7 @@
        public function setUp() {
                $this->page = new TestingGatewayPage();
                $this->adapter = new TestingGenericAdapter();
-               $this->adapter->addData( array(
+               $this->adapter->addRequestData( array(
                        'amount' => '120',
                        'currency_code' => 'SGD' ) );
                $this->adapter->errorsForRevalidate[0] = array( 'currency_code' 
=> 'blah' );
diff --git a/tests/GatewayValidationTest.php b/tests/GatewayValidationTest.php
index 77315a4..7b22ba1 100644
--- a/tests/GatewayValidationTest.php
+++ b/tests/GatewayValidationTest.php
@@ -44,7 +44,7 @@
        }
 
        public function testPassesValidation() {
-               $this->adapter->addData( array(
+               $this->adapter->addRequestData( array(
                        'amount' => '2.00',
                        'country' => 'US',
                        'currency' => 'USD',
@@ -57,7 +57,7 @@
        }
 
        public function testLowAmountError() {
-               $this->adapter->addData( array(
+               $this->adapter->addRequestData( array(
                        'amount' => '1.99',
                        'country' => 'US',
                        'currency' => 'USD',
@@ -72,7 +72,7 @@
        }
 
        public function testHighAmountError() {
-               $this->adapter->addData( array(
+               $this->adapter->addRequestData( array(
                        'amount' => '100.99',
                        'country' => 'US',
                        'currency' => 'USD',
@@ -87,7 +87,7 @@
        }
 
        public function testCurrencyCodeError() {
-               $this->adapter->addData( array(
+               $this->adapter->addRequestData( array(
                        'amount' => '2.99',
                        'country' => 'US',
                        'currency' => 'ZZZ',
@@ -108,7 +108,7 @@
                        'wgDonationInterfaceForbiddenCountries' => array( 'XX' )
                ) );
 
-               $this->adapter->addData( array(
+               $this->adapter->addRequestData( array(
                        'amount' => '2.99',
                        'country' => 'XX',
                        'currency' => 'USD',
@@ -123,7 +123,7 @@
        }
 
        public function testEmailError() {
-               $this->adapter->addData( array(
+               $this->adapter->addRequestData( array(
                        'amount' => '2.99',
                        'currency' => 'USD',
                        'email' => 'foo',
@@ -138,7 +138,7 @@
        }
 
        public function testSpuriousCcError() {
-               $this->adapter->addData( array(
+               $this->adapter->addRequestData( array(
                        'amount' => '2.99',
                        'currency' => 'USD',
                        'fname' => '4111111111111111',
@@ -153,7 +153,7 @@
        }
 
        public function testMissingFieldError() {
-               $this->adapter->addData( array(
+               $this->adapter->addRequestData( array(
                        'amount' => '2.99',
                ) );
 
diff --git a/worldpay_gateway/worldpay.adapter.php 
b/worldpay_gateway/worldpay.adapter.php
index 6e167fd..7d30a17 100644
--- a/worldpay_gateway/worldpay.adapter.php
+++ b/worldpay_gateway/worldpay.adapter.php
@@ -300,7 +300,7 @@
        }
 
        function setGatewayDefaults() {
-               $this->addData( array(
+               $this->addRequestData( array(
                        'region_code'  => 0  // TODO: geolocating this into the 
right region...
                ));
        }
@@ -762,7 +762,7 @@
                                break;
 
                        case 'AuthorizePaymentForFraud':
-                               $this->addData( array( 'cvv' => 
$this->get_cvv() ) );
+                               $this->addRequestData( array( 'cvv' => 
$this->get_cvv() ) );
                                $this->store_cvv_in_session( null ); // Remove 
the CVV from the session
                                return parent::do_transaction( $transaction );
                                break;
@@ -843,7 +843,7 @@
                                                $emptyVars[] = $theirs;
                                        }
                                }
-                               $self->addData( $addme, 'response' );
+                               $self->addResponseData( $addme );
                                return $emptyVars;
                        };
                $setFailOnEmpty = function( $emptyVars ) use ( $response, $self 
) {
@@ -935,7 +935,7 @@
                return $headers;
        }
 
-       protected function stage_returnto( $type = 'request' ) {
+       protected function stage_returnto() {
                global $wgServer, $wgArticlePath;
 
                $this->staged_data['returnto'] = str_replace(
@@ -945,39 +945,37 @@
                );
        }
 
-       protected function stage_wp_acctname( $type = 'request' ) {
+       protected function stage_wp_acctname() {
                $this->staged_data['wp_acctname'] = implode( ' ', array(
                        $this->getData_Unstaged_Escaped( 'fname' ),
                        $this->getData_Unstaged_Escaped( 'lname' )
                ));
        }
 
-       protected function stage_iso_currency_id( $type = 'request' ) {
+       protected function stage_iso_currency_id() {
                $currency = $this->getData_Unstaged_Escaped( 'currency_code' );
                if ( array_key_exists( $currency, self::$CURRENCY_CODES ) ) {
                        $this->staged_data['iso_currency_id'] = 
self::$CURRENCY_CODES[$currency];
                }
        }
 
-       protected function stage_payment_submethod( $type = 'request' ) {
-               if ( $type == 'response' ) {
-                       $paymentMethod = $this->getData_Unstaged_Escaped( 
'payment_method' );
-                       $paymentSubmethod = $this->getData_Unstaged_Escaped( 
'payment_submethod' );
-                       if ( $paymentMethod == 'cc' ) {
-                               if ( array_key_exists( $paymentSubmethod, 
self::$CARD_TYPES ) ) {
-                                       
$this->unstaged_data['payment_submethod'] = 
self::$CARD_TYPES[$paymentSubmethod];
-                               }
+       protected function unstage_payment_submethod() {
+               $paymentMethod = $this->getData_Staged( 'payment_method' );
+               $paymentSubmethod = $this->getData_Staged( 'payment_submethod' 
);
+               if ( $paymentMethod == 'cc' ) {
+                       if ( array_key_exists( $paymentSubmethod, 
self::$CARD_TYPES ) ) {
+                               $this->unstaged_data['payment_submethod'] = 
self::$CARD_TYPES[$paymentSubmethod];
                        }
                }
        }
 
-       protected function stage_merchant_reference_2( $type = 'request' ) {
+       protected function stage_merchant_reference_2() {
                $email = $this->getData_Unstaged_Escaped( 'email' );
                $alphanumeric = preg_replace('/[^0-9a-zA-Z]/', ' ', $email);
                $this->staged_data['merchant_reference_2'] = $alphanumeric;
        }
 
-       protected function stage_narrative_statement_1( $type = 'request' ) {
+       protected function stage_narrative_statement_1() {
                $this->staged_data['narrative_statement_1'] = 
WmfFramework::formatMessage(
                        'donate_interface-statement',
                        $this->getData_Unstaged_Escaped( 
'contribution_tracking_id' )

-- 
To view, visit https://gerrit.wikimedia.org/r/189615
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: If2d241fed6cc055123abfc97d58e25bdd34ae0f8
Gerrit-PatchSet: 7
Gerrit-Project: mediawiki/extensions/DonationInterface
Gerrit-Branch: master
Gerrit-Owner: Awight <awi...@wikimedia.org>
Gerrit-Reviewer: AndyRussG <andrew.green...@gmail.com>
Gerrit-Reviewer: Awight <awi...@wikimedia.org>
Gerrit-Reviewer: Ejegg <eeggles...@wikimedia.org>
Gerrit-Reviewer: Katie Horn <kh...@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

Reply via email to