CRM-12108 refactore PayPalProIPN.php so that it can be called outside of a webtest & can be tested

CRM/Core/Payment/BaseIPN.php

@@ -36,6 +36,13 @@ class CRM_Core_Payment_BaseIPN {
 
   static $_now = NULL;
 
+  /**
+   * Input parameters from payment processor. Store these so that
+   * the code does not need to keep retrieving from the http request
+   * @var array
+   */
+  protected $_inputParameters = array();
+
   /**
    * Constructor
    */
@@ -43,6 +50,17 @@ class CRM_Core_Payment_BaseIPN {
     self::$_now = date('YmdHis');
   }
 
+  /**
+   * Store input array on the class
+   * @param array $parameters
+   * @throws CRM_Core_Exceptions
+   */
+  function setInputParameters($parameters) {
+    if(!is_array($parameters)) {
+      throw new CRM_Core_Exceptions('Invalid input parameters');
+    }
+    $this->_inputParameters = $parameters;
+  }
   /**
    * Validate incoming data. This function is intended to ensure that incoming data matches
    * It provides a form of pseudo-authentication - by checking the calling fn already knows
@@ -123,7 +141,7 @@ class CRM_Core_Payment_BaseIPN {
     try {
       $success = $contribution->loadRelatedObjects($input, $ids, $required);
     }
-    catch(Exception$e) {
+    catch(Exception $e) {
       if (CRM_Utils_Array::value('log_error', $error_handling)) {
         CRM_Core_Error::debug_log_message($e->getMessage());
       }
@@ -518,8 +536,8 @@ LIMIT 1;";
 
     if ($contribution->id) {
       $contributionStatuses = CRM_Contribute_PseudoConstant::contributionStatus(NULL, 'name');
-      if ((!$input['prevContribution'] && $paymentProcessorId) || (!$input['prevContribution']->is_pay_later &&
-        $input['prevContribution']->contribution_status_id == array_search('Pending', $contributionStatuses))) {
+      if (isset($input['prevContribution']) && !$input['prevContribution']->is_pay_later &&
+        $input['prevContribution']->contribution_status_id == array_search('Pending', $contributionStatuses)) {
         $input['payment_processor'] = $paymentProcessorId;
       }
       $input['contribution_status_id'] = array_search('Completed', $contributionStatuses);

CRM/Core/Payment/PayPalIPN.php

@@ -265,7 +265,7 @@ class CRM_Core_Payment_PayPalIPN extends CRM_Core_Payment_BaseIPN {
   function main() {
     // CRM_Core_Error::debug_var( 'GET' , $_GET , true, true );
     // CRM_Core_Error::debug_var( 'POST', $_POST, true, true );
-
+   //@todo - this could be refactored like PayPalProIPN & a test could be added
 
     $objects = $ids = $input = array();
     $component = CRM_Utils_Array::value('module', $_GET);

CRM/Core/Payment/PayPalProIPN.php

@@ -35,49 +35,116 @@
 class CRM_Core_Payment_PayPalProIPN extends CRM_Core_Payment_BaseIPN {
 
   static $_paymentProcessor = NULL;
-  function __construct() {
+
+  /**
+   * Input parameters from payment processor. Store these so that
+   * the code does not need to keep retrieving from the http request
+   * @var array
+   */
+  protected $_inputParameters = array();
+
+  /**
+   * store for the variables from the invoice string
+   * @var array
+   */
+  protected $_invoiceData = array();
+
+  /**
+   * Is this a payment express transaction
+   */
+  protected $_isPaymentExpress = FALSE;
+
+  /**
+   * Are we dealing with an event an 'anything else' (contribute)
+   * @var string component
+   */
+  protected $_component = 'contribute';
+  /**
+   * constructor function
+   */
+  function __construct($inputData) {
+    $this->setInputParameters($inputData);
+    $this->setInvoiceData();
     parent::__construct();
   }
 
+  /**
+   * function exists to get the values from the rp_invoice_id string
+   * @param string $name e.g. i, values are stored in the string with letter codes
+   * @param boolean $abort fatal if not found?
+   * @return unknown
+   */
   function getValue($name, $abort = TRUE) {
-
-    if (!empty($_POST)) {
-      $rpInvoiceArray = array();
-      $value          = NULL;
-      $rpInvoiceArray = explode('&', $_POST['rp_invoice_id']);
-      foreach ($rpInvoiceArray as $rpInvoiceValue) {
-        $rpValueArray = explode('=', $rpInvoiceValue);
-        if ($rpValueArray[0] == $name) {
-          $value = $rpValueArray[1];
-        }
-      }
-
-      if ($value == NULL && $abort) {
-        echo "Failure: Missing Parameter $name<p>";
-        exit();
-      }
-      else {
-        return $value;
-      }
+    if ($abort && empty($this->_invoiceData[$name])) {
+      throw new CRM_Core_Exception("Failure: Missing Parameter $name");
     }
     else {
-      return NULL;
+      return CRM_Utils_Array::value($name, $this->_invoiceData);
+    }
+  }
+
+  /**
+   * Set $this->_invoiceData from the input array
+   */
+  function setInvoiceData() {
+    if(empty($this->_inputParameters['rp_invoice_id'])) {
+      $this->_isPaymentExpress = TRUE;
+      return;
+    }
+    $rpInvoiceArray = explode('&', $this->_inputParameters['rp_invoice_id']);
+    // for clarify let's also store without the single letter unreadable
+    //@todo after more refactoring we might ditch storing the one letter stuff
+    $mapping = array(
+      'i' => 'invoice_id',
+      'm' => 'component',
+      'c' => 'contact_id',
+      'b' => 'contribution_id',
+      'r' => 'contribution_recur_id',
+      'p' => 'participant_id',
+      'e' => 'event_id',
+    );
+    foreach ($rpInvoiceArray as $rpInvoiceValue) {
+      $rpValueArray = explode('=', $rpInvoiceValue);
+      $this->_invoiceData[$rpValueArray[0]] = $rpValueArray[1];
+      $this->_inputParameters[$mapping[$rpValueArray[0]]] = $rpValueArray[1];
+      // p has been overloaded & could mean contribution page or participant id. Clearly we need an
+      // alphabet with more letters.
+      // the mode will always be resolved before the mystery p is reached
+      if($rpValueArray[1] == 'contribute') {
+        $mapping['p'] = 'contribution_page_id';
+      }
     }
   }
 
-  static function retrieve($name, $type, $location = 'POST', $abort = TRUE) {
-    static $store = NULL;
-    $value = CRM_Utils_Request::retrieve($name, $type, $store,
-      FALSE, NULL, $location
+  /**
+   * @param string $name of variable to return
+   * @param string $type data type
+   *   - String
+   *   - Integer
+   * @param string $location - deprecated
+   * @param boolean $abort abort if empty
+   * @return Ambigous <mixed, NULL, value, unknown, array, number>
+   */
+  function retrieve($name, $type, $location = 'POST', $abort = TRUE) {
+    $value = CRM_Utils_Type::validate(
+      CRM_Utils_Array::value($name, $this->_inputParameters),
+      $type,
+      FALSE
     );
     if ($abort && $value === NULL) {
-      CRM_Core_Error::debug_log_message("Could not find an entry for $name in $location");
-      echo "Failure: Missing Parameter<p>";
-      exit();
+      throw new CRM_Core_Exception("Could not find an entry for $name in $location");
     }
     return $value;
   }
 
+  /**
+   * Process recurring contributions
+   * @param array $input
+   * @param array $ids
+   * @param array $objects
+   * @param boolean $first
+   * @return void|boolean
+   */
   function recur(&$input, &$ids, &$objects, $first) {
     if (!isset($input['txnType'])) {
       CRM_Core_Error::debug_log_message("Could not find txn_type in input request");
@@ -133,14 +200,14 @@ class CRM_Core_Payment_PayPalProIPN extends CRM_Core_Payment_BaseIPN {
 
 
     //set transaction type
-    $txnType = $_POST['txn_type'];
+    $txnType = $this->retrieve('txn_type', 'String');
     //Changes for paypal pro recurring payment
 
     switch ($txnType) {
       case 'recurring_payment_profile_created':
         $recur->create_date = $now;
         $recur->contribution_status_id = 2;
-        $recur->processor_id = $_POST['recurring_payment_id'];
+        $recur->processor_id = $this->retrieve('recurring_payment_id', 'Integer');
         $recur->trxn_id = $recur->processor_id;
         $subscriptionPaymentStatus = CRM_Core_Payment::RECURRING_PAYMENT_START;
         $sendNotification = TRUE;
@@ -155,7 +222,7 @@ class CRM_Core_Payment_PayPalProIPN extends CRM_Core_Payment_BaseIPN {
         }
 
         //contribution installment is completed
-        if ($_POST['profile_status'] == 'Expired') {
+        if ($this->retrieve('profile_status', 'String') == 'Expired') {
           $recur->contribution_status_id = 1;
           $recur->end_date = $now;
           $sendNotification = TRUE;
@@ -279,12 +346,22 @@ class CRM_Core_Payment_PayPalProIPN extends CRM_Core_Payment_BaseIPN {
     $this->completeTransaction($input, $ids, $objects, $transaction, $recur);
   }
 
+  /**
+   * This is the main function to call. It should be sufficient to instantiate the class
+   * (with the input parameters) & call this & all will be done
+   *
+   * @todo the references to POST throughout this class need to be removed
+   * @return void|boolean|Ambigous <void, boolean>
+   */
   function main() {
     CRM_Core_Error::debug_var('GET', $_GET, TRUE, TRUE);
     CRM_Core_Error::debug_var('POST', $_POST, TRUE, TRUE);
-
+    if($this->_isPaymentExpress) {
+      $this->handlePaymentExpress();
+      return;
+    }
     $objects = $ids = $input = array();
-    $input['component'] = self::getValue('m');
+    $this->_component  = $input['component'] = self::getValue('m');
 
     // get the contribution and contact ids from the GET params
     $ids['contact'] = self::getValue('c', TRUE);
@@ -292,13 +369,15 @@ class CRM_Core_Payment_PayPalProIPN extends CRM_Core_Payment_BaseIPN {
 
     $this->getInput($input, $ids);
 
-    if ($component == 'event') {
+    if ($this->_component == 'event') {
       $ids['event'] = self::getValue('e', TRUE);
       $ids['participant'] = self::getValue('p', TRUE);
       $ids['contributionRecur'] = self::getValue('r', FALSE);
     }
     else {
       // get the optional ids
+      //@ how can this not be broken retrieving from GET as we are dealing with a POST request?
+      // copy & paste? Note the retrieve function now uses data from _REQUEST so this will be included
       $ids['membership'] = self::retrieve('membershipID', 'Integer', 'GET', FALSE);
       $ids['contributionRecur'] = self::getValue('r', FALSE);
       $ids['contributionPage'] = self::getValue('p', FALSE);
@@ -330,7 +409,10 @@ INNER JOIN civicrm_membership_payment mp ON m.id = mp.membership_id AND mp.contr
     }
 
     self::$_paymentProcessor = &$objects['paymentProcessor'];
-    if ($component == 'contribute' || $component == 'event') {
+    //?? how on earth would we not have component be one of these?
+    // they are the only valid settings & this IPN file can't even be called without one of them
+    // grepping for this class doesn't find other paths to call this class
+    if ($this->_component == 'contribute' || $this->_component == 'event') {
       if ($ids['contributionRecur']) {
         // check if first contribution is completed, else complete first contribution
         $first = TRUE;
@@ -381,5 +463,14 @@ INNER JOIN civicrm_membership_payment mp ON m.id = mp.membership_id AND mp.contr
     $input['net_amount'] = self::retrieve('settle_amount', 'Money', 'POST', FALSE);
     $input['trxn_id']    = self::retrieve('txn_id', 'String', 'POST', FALSE);
   }
+
+  /**
+   * Handle payment express IPNs
+   * For one off IPNS no actual response is required
+   * Recurring is more difficult as we have limited confirmation material
+   */
+  function handlePaymentExpress() {
+    throw new CRM_Core_Exception('Payment Express IPNS not currently handled');
+  }
 }
 

extern/ipn.php

@@ -40,11 +40,20 @@ require_once '../civicrm.config.php';
 $config = CRM_Core_Config::singleton();
 
 if (empty($_GET)) {
-  $paypalIPN = new CRM_Core_Payment_PayPalProIPN();
+  $paypalIPN = new CRM_Core_Payment_PayPalProIPN($_REQUEST);
 }
 else {
   $paypalIPN = new CRM_Core_Payment_PayPalIPN();
+  // @todo upgrade standard per Pro
+}
+try{
+  $paypalIPN->main();
+}
+catch(CRM_Core_Exception $e) {
+  CRM_Core_Error::debug_log_message($e->getMessage());
+  CRM_Core_Error::debug_var('error data', $e->getErrorData(), TRUE, TRUE);
+  CRM_Core_Error::debug_var('REQUEST', $_REQUEST, TRUE, TRUE);
+  //@todo give better info to logged in user - ie dev
+  echo "The transaction has failed. Please review the log for more detail";
 }
-
-$paypalIPN->main();