From c8aa607b8439fc6b80737eee97fceacf69760c91 Mon Sep 17 00:00:00 2001 From: eileen Date: Tue, 30 Jul 2013 21:28:16 +1200 Subject: [PATCH] CRM-12108 refactore PayPalProIPN.php so that it can be called outside of a webtest & can be tested --- CRM/Core/Payment/BaseIPN.php | 24 ++++- CRM/Core/Payment/PayPalIPN.php | 2 +- CRM/Core/Payment/PayPalProIPN.php | 161 +++++++++++++++++++++++------- extern/ipn.php | 15 ++- 4 files changed, 160 insertions(+), 42 deletions(-) diff --git a/CRM/Core/Payment/BaseIPN.php b/CRM/Core/Payment/BaseIPN.php index d122bbd967..0457bf7bf6 100644 --- a/CRM/Core/Payment/BaseIPN.php +++ b/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); diff --git a/CRM/Core/Payment/PayPalIPN.php b/CRM/Core/Payment/PayPalIPN.php index b4a55472bf..6d2ee45c7d 100644 --- a/CRM/Core/Payment/PayPalIPN.php +++ b/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); diff --git a/CRM/Core/Payment/PayPalProIPN.php b/CRM/Core/Payment/PayPalProIPN.php index 0bab2a4a92..327c7ffacc 100644 --- a/CRM/Core/Payment/PayPalProIPN.php +++ b/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

"; - 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 + */ + 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

"; - 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 + */ 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'); + } } diff --git a/extern/ipn.php b/extern/ipn.php index 4aa7f0e174..5aa519c857 100644 --- a/extern/ipn.php +++ b/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(); -- 2.25.1