From 9ba02e3e25fe10e8d1eb46c07b77b7fa407eda0d Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Wed, 13 Mar 2019 00:35:54 -0700 Subject: [PATCH] (ops#878) Enforce common signature for CRM_Utils_System::loadBootStrap(). Fix WP E2E error. 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 | 2 ++ CRM/Utils/System/Soap.php | 5 +++++ CRM/Utils/System/UnitTests.php | 9 +-------- CRM/Utils/System/WordPress.php | 21 +++++++++++++++------ 4 files changed, 23 insertions(+), 14 deletions(-) diff --git a/CRM/Utils/System/Base.php b/CRM/Utils/System/Base.php index c2978ecf94..01ff909499 100644 --- a/CRM/Utils/System/Base.php +++ b/CRM/Utils/System/Base.php @@ -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. * diff --git a/CRM/Utils/System/Soap.php b/CRM/Utils/System/Soap.php index ddb3eb3588..df688a033c 100644 --- a/CRM/Utils/System/Soap.php +++ b/CRM/Utils/System/Soap.php @@ -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"); + } + } diff --git a/CRM/Utils/System/UnitTests.php b/CRM/Utils/System/UnitTests.php index 7d4426f6ab..ea220a4169 100644 --- a/CRM/Utils/System/UnitTests.php +++ b/CRM/Utils/System/UnitTests.php @@ -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; } diff --git a/CRM/Utils/System/WordPress.php b/CRM/Utils/System/WordPress.php index f450056bdb..08e286e053 100644 --- a/CRM/Utils/System/WordPress.php +++ b/CRM/Utils/System/WordPress.php @@ -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); } -- 2.25.1