Fixes to extension class following on from removal of singleton function in CRM-16808...

AND enforcing enotice compliance on this test class

diff --git a/CRM/Extension/Manager/Payment.php b/CRM/Extension/Manager/Payment.php
index 36d9a8ad92db56e0ab718fb579cffbd0e59e2ea1..cf60b4d1d4ef5fc9f3f21b944aa9b2d927b3d16a 100644 (file)
--- a/CRM/Extension/Manager/Payment.php
+++ b/CRM/Extension/Manager/Payment.php
@@ -213,13 +213,12 @@ class CRM_Extension_Manager_Payment extends CRM_Extension_Manager_Base {
       return;
     }
 
-    // See if we have any instances of this PP defined ..
-    if ($processor_id = CRM_Core_DAO::singleValueQuery("
-                SELECT pp.id
+    $processorDAO = CRM_Core_DAO::executeQuery(
+      "                SELECT pp.id, ppt.class_name
                   FROM civicrm_extension ext
             INNER JOIN civicrm_payment_processor_type ppt
                     ON ext.name = ppt.name
-            INNER JOIN civicrm_payment_processor pp
+            LEFT JOIN civicrm_payment_processor pp
                     ON ppt.id = pp.payment_processor_type_id
                  WHERE ext.type = 'payment'
                    AND ext.full_name = %1
@@ -227,52 +226,20 @@ class CRM_Extension_Manager_Payment extends CRM_Extension_Manager_Base {
       array(
         1 => array($info->key, 'String'),
       )
-    )
-    ) {
-      // If so, load params in the usual way ..
-      $paymentProcessor = CRM_Financial_BAO_PaymentProcessor::getPayment($processor_id, NULL);
+    );
+
+    while ($processorDAO->fetch()) {
+      $class_name = $processorDAO->class_name;
+      $processor_id = $processorDAO->id;
     }
-    else {
-      // Otherwise, do the best we can to construct some ..
-      $dao = CRM_Core_DAO::executeQuery("
-                    SELECT ppt.*
-                      FROM civicrm_extension ext
-                INNER JOIN civicrm_payment_processor_type ppt
-                        ON ppt.name = ext.name
-                     WHERE ext.name = %1
-                       AND ext.type = 'payment'
-            ",
-        array(
-          1 => array($info->name, 'String'),
-        )
-      );
-      if ($dao->fetch()) {
-        $paymentProcessor = array(
-          'id' => -1,
-          'name' => $dao->title,
-          'payment_processor_type_id' => $dao->id,
-          'user_name' => 'nothing',
-          'password' => 'nothing',
-          'signature' => '',
-          'url_site' => $dao->url_site_default,
-          'url_api' => $dao->url_api_default,
-          'url_recur' => $dao->url_recur_default,
-          'url_button' => $dao->url_button_default,
-          'subject' => '',
-          'class_name' => $dao->class_name,
-          'is_recur' => $dao->is_recur,
-          'billing_mode' => $dao->billing_mode,
-          'payment_type' => $dao->payment_type,
-        );
-      }
-      else {
-        CRM_Core_Error::fatal("Unable to find payment processor in " . __CLASS__ . '::' . __METHOD__);
-      }
+
+    if (empty($class_name)) {
+      CRM_Core_Error::fatal("Unable to find payment processor in " . __CLASS__ . '::' . __METHOD__);
     }
 
     // In the case of uninstall, check for instances of PP first.
     // Don't run hook if any are found.
-    if ($method == 'uninstall' && $paymentProcessor['id'] > 0) {
+    if ($method == 'uninstall' && $processor_id > 0) {
       return;
     }
 
@@ -281,9 +248,8 @@ class CRM_Extension_Manager_Payment extends CRM_Extension_Manager_Base {
       case 'uninstall':
       case 'enable':
       case 'disable':
-
-        // Instantiate PP
-        $processorInstance = $paymentProcessor['object'];
+        // Instantiate PP - the getClass function allows us to do this when no payment processor instances exist.
+        $processorInstance = Civi\Payment\System::singleton()->getByClass($class_name);
 
         // Does PP implement this method, and can we call it?
         if (method_exists($processorInstance, $method) && is_callable(array(

diff --git a/Civi/Payment/System.php b/Civi/Payment/System.php
index c9ba4bc9514809804264823e9dd26779417d48bd..2e69ad907c143b66572ae2c9c899f725f35a3661 100644 (file)
--- a/Civi/Payment/System.php
+++ b/Civi/Payment/System.php
@@ -34,41 +34,40 @@ class System {
    * If there is no valid configuration it will not be retrieved.
    *
    * @param array $processor
+   * @param bool $force
+   *   Override the config check. This is required in uninstall as no valid instances exist
+   *   but will deliberately not work with any valid processors.
    *
    * @return CRM_Core_Payment|NULL
    *
    * @throws \CRM_Core_Exception
    */
-  public function getByProcessor($processor) {
-    $id = $processor['id'];
+  public function getByProcessor($processor, $force = FALSE) {
+    $id = $force ? 0 : $processor['id'];
 
-    if (!isset($this->cache[$id])) {
-      if (!isset($this->cache[$id])) {
-        //does this config need to be called?
-        $config = \CRM_Core_Config::singleton();
-        $ext = \CRM_Extension_System::singleton()->getMapper();
-        if ($ext->isExtensionKey($processor['class_name'])) {
-          $paymentClass = $ext->keyToClass($processor['class_name'], 'payment');
-          require_once $ext->classToPath($paymentClass);
-        }
-        else {
-          $paymentClass = 'CRM_Core_' . $processor['class_name'];
-          if (empty($paymentClass)) {
-            throw new \CRM_Core_Exception('no class provided');
-          }
-          require_once str_replace('_', DIRECTORY_SEPARATOR, $paymentClass) . '.php';
+    if (!isset($this->cache[$id]) || $force) {
+      $ext = \CRM_Extension_System::singleton()->getMapper();
+      if ($ext->isExtensionKey($processor['class_name'])) {
+        $paymentClass = $ext->keyToClass($processor['class_name'], 'payment');
+        require_once $ext->classToPath($paymentClass);
+      }
+      else {
+        $paymentClass = 'CRM_Core_' . $processor['class_name'];
+        if (empty($paymentClass)) {
+          throw new \CRM_Core_Exception('no class provided');
         }
+        require_once str_replace('_', DIRECTORY_SEPARATOR, $paymentClass) . '.php';
+      }
 
-        $processorObject = new $paymentClass(!empty($processor['is_test']) ? 'test' : 'live', $processor);
-        if ($processorObject->checkConfig()) {
-          $processorObject = NULL;
-        }
-        else {
-          $processorObject->setPaymentProcessor($processor);
-        }
+      $processorObject = new $paymentClass(!empty($processor['is_test']) ? 'test' : 'live', $processor);
+      if (!$force && !$processorObject->checkConfig()) {
+        $processorObject = NULL;
+      }
+      else {
+        $processorObject->setPaymentProcessor($processor);
       }
-      $this->cache[$id] = $processorObject;
     }
+    $this->cache[$id] = $processorObject;
     return $this->cache[$id];
   }
 
@@ -107,4 +106,24 @@ class System {
     \CRM_Financial_BAO_PaymentProcessor::getAllPaymentProcessors('test', TRUE);
   }
 
+  /**
+   * Sometimes we want to instantiate a processor object when no valid instance exists (eg. when uninstalling a
+   * processor).
+   *
+   * This function does not load instance specific details for the processor.
+   *
+   * @param string $className
+   *
+   * @return \Civi\Payment\CRM_Core_Payment|NULL
+   * @throws \CiviCRM_API3_Exception
+   */
+  public function getByClass($className) {
+    return $this->getByProcessor(array(
+      'class_name' => $className,
+      'id' => 0,
+      'is_test' => 0,
+    ),
+    TRUE);
+  }
+
 }

diff --git a/tests/extensions/test.extension.manager.paymenttest/main.php b/tests/extensions/test.extension.manager.paymenttest/main.php
index e28abf9c707181bc564c6120b8d0aca87356e180..dac3dc64e3d6f44bd94b9e78c78880f562745a6a 100644 (file)
--- a/tests/extensions/test.extension.manager.paymenttest/main.php
+++ b/tests/extensions/test.extension.manager.paymenttest/main.php
@@ -30,4 +30,15 @@ class test_extension_manager_paymenttest extends CRM_Core_Payment {
   public function checkConfig() {
   }
 
+  /**
+   * Get the desired value from $counts.
+   *
+   * @param string $type
+   *
+   * @return int
+   */
+  public static function getCount($type) {
+    return isset(self::$counts[$type]) ? self::$counts[$type] : 0;
+  }
+
 }

diff --git a/tests/phpunit/CRM/Extension/Manager/PaymentTest.php b/tests/phpunit/CRM/Extension/Manager/PaymentTest.php
index eb65b99feb1d675c7daa632f4e6d1c65eae47248..edfae39feba6b40b50f3856f5e181bb81918504d 100644 (file)
--- a/tests/phpunit/CRM/Extension/Manager/PaymentTest.php
+++ b/tests/phpunit/CRM/Extension/Manager/PaymentTest.php
@@ -122,14 +122,14 @@ class CRM_Extension_Manager_PaymentTest extends CiviUnitTestCase {
     }
     catch (CRM_Extension_Exception_DependencyException $e) {
     }
-    $this->assertEquals(0, test_extension_manager_paymenttest::$counts['uninstall']);
+    $this->assertEquals(0, test_extension_manager_paymenttest::getCount('uninstall'));
     $this->assertDBQuery(1, 'SELECT count(*) FROM civicrm_payment_processor_type WHERE class_name = "test.extension.manager.paymenttest"');
 
     $ppDAO->delete();
 
     // second attempt to uninstall -- ok
     $manager->uninstall(array('test.extension.manager.paymenttest'));
-    $this->assertEquals(1, test_extension_manager_paymenttest::$counts['uninstall']);
+    $this->assertEquals(1, test_extension_manager_paymenttest::getCount('uninstall'));
     $this->assertDBQuery(0, 'SELECT count(*) FROM civicrm_payment_processor_type WHERE class_name = "test.extension.manager.paymenttest"');
   }