(ops#878) Enforce common signature for CRM_Utils_System::loadBootStrap(). Fix WP...
authorTim Otten <totten@civicrm.org>
Wed, 13 Mar 2019 07:35:54 +0000 (00:35 -0700)
committerTim Otten <totten@civicrm.org>
Wed, 13 Mar 2019 18:44:05 +0000 (11:44 -0700)
Before
------

1.  Most `CRM_Utils_System_*` classes implement a method `loadBootStrap($params = array(),...)`.  However, `WordPress`
   and `UnitTests` implement a different signature (`loadBootStrap($name = NULL, $pass = NULL)`), and `Soap` does not
   implement it all.  This is problematic -- because the function is invoked in a centralized way (e.g.
   `CRM_Utils_System::loadBootStrap()` invokes `$config->userSystem->loadBootStrap($params, $loadUser, $throwError,
   $realPath)`), which implies that all UF drivers need to support this signature.

2. When running any of the end-to-end (E2E) tests on CiviCRM-WordPress, the output is polluted with these warnings:

```
bknix-dfl:~/bknix/build/wpmaster/wp-content/plugins/civicrm/civicrm] phpunit5 tests/phpunit/E2E/Cache/FastArrayDecoratorTest.php ; echo "exit=$?"
PHPUnit 5.7.27 by Sebastian Bergmann and contributors.

PHP Warning:  strip_tags() expects parameter 1 to be string, array given in /Users/totten/bknix/build/wpmaster/wp-includes/formatting.php on line 4649
PHP Stack trace:
PHP   1. {main}() /Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar:0
PHP   2. PHPUnit_TextUI_Command::main() /Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar:598
PHP   3. PHPUnit_TextUI_Command->run() phar:///Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar/phpunit/TextUI/Command.php:116
PHP   4. PHPUnit_TextUI_TestRunner->doRun() phar:///Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar/phpunit/TextUI/Command.php:186
PHP   5. PHPUnit_Framework_TestSuite->run() phar:///Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar/phpunit/TextUI/TestRunner.php:517
PHP   6. call_user_func:{phar:///Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar/phpunit/Framework/TestSuite.php:679}() phar:///Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar/phpunit/Framework/TestSuite.php:679
PHP   7. E2E_Cache_CacheTestCase::setUpBeforeClass() phar:///Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar/phpunit/Framework/TestSuite.php:679
PHP   8. CRM_Utils_System::loadBootStrap() /Users/totten/bknix/build/wpmaster/wp-content/plugins/civicrm/civicrm/tests/phpunit/E2E/Cache/CacheTestCase.php:43
PHP   9. CRM_Utils_System_WordPress->loadBootStrap() /Users/totten/bknix/build/wpmaster/wp-content/plugins/civicrm/civicrm/CRM/Utils/System.php:1477
PHP  10. wp_authenticate() /Users/totten/bknix/build/wpmaster/wp-content/plugins/civicrm/civicrm/CRM/Utils/System/WordPress.php:495
PHP  11. sanitize_user() /Users/totten/bknix/build/wpmaster/wp-includes/pluggable.php:515
PHP  12. wp_strip_all_tags() /Users/totten/bknix/build/wpmaster/wp-includes/formatting.php:1850
PHP  13. strip_tags() /Users/totten/bknix/build/wpmaster/wp-includes/formatting.php:4649

Warning: strip_tags() expects parameter 1 to be string, array given in /Users/totten/bknix/build/wpmaster/wp-includes/formatting.php on line 4649

Call Stack:
    0.0026     425040   1. {main}() /Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar:0
    0.0766   14246216   2. PHPUnit_TextUI_Command::main() /Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar:598
    0.0766   14246952   3. PHPUnit_TextUI_Command->run() phar:///Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar/phpunit/TextUI/Command.php:116
    0.4267   23405608   4. PHPUnit_TextUI_TestRunner->doRun() phar:///Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar/phpunit/TextUI/Command.php:186
    0.4336   23511184   5. PHPUnit_Framework_TestSuite->run() phar:///Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar/phpunit/TextUI/TestRunner.php:517
    0.4403   23516840   6. call_user_func:{phar:///Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar/phpunit/Framework/TestSuite.php:679}() phar:///Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar/phpunit/Framework/TestSuite.php:679
    0.4403   23516920   7. E2E_Cache_CacheTestCase::setUpBeforeClass() phar:///Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar/phpunit/Framework/TestSuite.php:679
    0.5065   31975824   8. CRM_Utils_System::loadBootStrap() /Users/totten/bknix/build/wpmaster/wp-content/plugins/civicrm/civicrm/tests/phpunit/E2E/Cache/CacheTestCase.php:43
    0.5065   31976344   9. CRM_Utils_System_WordPress->loadBootStrap() /Users/totten/bknix/build/wpmaster/wp-content/plugins/civicrm/civicrm/CRM/Utils/System.php:1477
    0.6517   57998912  10. wp_authenticate() /Users/totten/bknix/build/wpmaster/wp-content/plugins/civicrm/civicrm/CRM/Utils/System/WordPress.php:495
    0.6517   57998992  11. sanitize_user() /Users/totten/bknix/build/wpmaster/wp-includes/pluggable.php:515
    0.6517   57999088  12. wp_strip_all_tags() /Users/totten/bknix/build/wpmaster/wp-includes/formatting.php:1850
    0.6517   57999736  13. strip_tags() /Users/totten/bknix/build/wpmaster/wp-includes/formatting.php:4649
```

3.  The warning indicates that (on WordPress) the E2E test is not doing what it's trying to do -- specifically,
    `E2E_Cache_CacheTestCase::setUpBeforeClass()` (or, similarly, `CiviEndToEndTestCase::setUpBeforeClass()`) calls
    `CRM_Utils_System::loadBootStrap()` to authenticate as the CMS administrator, but this fails because WP's
    implementation of `loadBootStrap()` does not respect the same signature.

After
-----

1. `CRM_Utils_System_Base` defines an abstract function `loadBootStrap($params = array(),...)`. Any non-compliant driver (which
   uses a different signature or which omits the function) will produce a syntax error.

2. The classes `CRM_Utils_System_{WordPress,UnitTests}` have been updated to comply with the signature.

3. The class `CRM_Utils_System_Soap` nominally complies. However, this class isn't really a UF driver, and it doesn't make sense for
   it to extend this base class, and the implementation is a stub. But we don't need to scope-creep the bugfix.

4. The `loadBootStrap...strip_tags()...` warnings are no longer generated when running E2E tests on WP.

Comments
--------

* I verified that all extant classes in `CRM/Utils/System/*.php` are loadable/compilable (i.e they now comply with the delcared signature).
* The SOAP integration has test-coverage viia `E2E_Extern_SoapTest`.
* In the PR test output, you'll only see E2E results from D7.  *However*, this patch is part of a bigger effort
  (infrastructure/ops#878) which will eventually give some visibility on that.

CRM/Utils/System/Base.php
CRM/Utils/System/Soap.php
CRM/Utils/System/UnitTests.php
CRM/Utils/System/WordPress.php

index c2978ecf942ea80f2eb1682310491d556de86f6f..01ff9094996e378deaa0d6bc85674db936483511 100644 (file)
@@ -56,6 +56,8 @@ abstract class CRM_Utils_System_Base {
     }
   }
 
+  public abstract function loadBootStrap($params = array(), $loadUser = TRUE, $throwError = TRUE, $realPath = NULL);
+
   /**
    * Append an additional breadcrumb tag to the existing breadcrumb.
    *
index ddb3eb358870e18efe68d8a45c92385a50b9c26e..df688a033c0ba6aeef7537773094c8bf490ec861 100644 (file)
@@ -124,4 +124,9 @@ class CRM_Utils_System_Soap extends CRM_Utils_System_Base {
     throw new Exception("Method not implemented: getLoginURL");
   }
 
+  public function loadBootStrap($params = array(), $loadUser = TRUE, $throwError = TRUE, $realPath = NULL) {
+    // It makes zero sense for this class to extend CRM_Utils_System_Base.
+    throw new \RuntimeException("Not implemented");
+  }
+
 }
index 7d4426f6ab43592301188fcb97ee65b015092199..ea220a41694868d0b2c4d2627f4c03d702c03517 100644 (file)
@@ -54,15 +54,8 @@ class CRM_Utils_System_UnitTests extends CRM_Utils_System_Base {
 
   /**
    * Bootstrap the phony CMS.
-   *
-   * @param string $name
-   *   Optional username for login.
-   * @param string $pass
-   *   Optional password for login.
-   *
-   * @return bool
    */
-  public function loadBootStrap($name = NULL, $pass = NULL) {
+  public function loadBootStrap($params = array(), $loadUser = TRUE, $throwError = TRUE, $realPath = NULL) {
     return TRUE;
   }
 
index f450056bdbd65a3671a8863d50d0b437b717df00..08e286e0539c87117819931920ba15ad185100ab 100644 (file)
@@ -338,7 +338,10 @@ class CRM_Utils_System_WordPress extends CRM_Utils_System_Base {
     $config = CRM_Core_Config::singleton();
 
     if ($loadCMSBootstrap) {
-      $config->userSystem->loadBootStrap($name, $password);
+      $config->userSystem->loadBootStrap([
+        'name' => $name,
+        'pass' => $password,
+      ]);
     }
 
     $user = wp_authenticate($name, $password);
@@ -453,16 +456,22 @@ class CRM_Utils_System_WordPress extends CRM_Utils_System_Base {
   /**
    * Load wordpress bootstrap.
    *
-   * @param string $name
-   *   optional username for login.
-   * @param string $pass
-   *   optional password for login.
+   * @param array $params
+   *   Optional credentials
+   *   - name: string, cms username
+   *   - pass: string, cms password
    *
    * @return bool
    */
-  public function loadBootStrap($name = NULL, $pass = NULL) {
+  public function loadBootStrap($params = array(), $loadUser = TRUE, $throwError = TRUE, $realPath = NULL) {
     global $wp, $wp_rewrite, $wp_the_query, $wp_query, $wpdb, $current_site, $current_blog, $current_user;
 
+    $name = CRM_Utils_Array::value('name', $params);
+    $pass = CRM_Utils_Array::value('pass', $params);
+    if (isset($params['uid'])) {
+      throw new \RuntimeException("Not implemented WordPress::loadBootStrap([uid=>\$num]))");
+    }
+
     if (!defined('WP_USE_THEMES')) {
       define('WP_USE_THEMES', FALSE);
     }