From: Coleman Watts <coleman@civicrm.org>
Date: Sat, 10 Oct 2020 20:58:17 +0000 (-0400)
Subject: AngularLoader: Support 'settingsFactory' callbacks in angular modules.
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=aa882f9b80f49252136748faf036c1ebe6bca7e2;p=civicrm-core.git

AngularLoader: Support 'settingsFactory' callbacks in angular modules.

This allows Angular modules with complex/expensive data to provide it with a callback,
which will only be invoked if the module is actively loaded on the page.

Normally, module settings are calculated on every page request, even if they are not needed.
---

diff --git a/Civi/Angular/AngularLoader.php b/Civi/Angular/AngularLoader.php
index 1dca6bcd9e..be80912a62 100644
--- a/Civi/Angular/AngularLoader.php
+++ b/Civi/Angular/AngularLoader.php
@@ -115,8 +115,13 @@ class AngularLoader {
     }
 
     $res->addSettingsFactory(function () use (&$moduleNames, $angular, $res, $assetParams) {
+      // Merge static settings with the results of settingsFactory functions
+      $settingsByModule = $angular->getResources($moduleNames, 'settings', 'settings');
+      foreach ($angular->getResources($moduleNames, 'settingsFactory', 'settingsFactory') as $moduleName => $factory) {
+        $settingsByModule[$moduleName] = array_merge($settingsByModule[$moduleName] ?? [], $factory());
+      }
       // TODO optimization; client-side caching
-      $result = array_merge($angular->getResources($moduleNames, 'settings', 'settings'), [
+      return array_merge($settingsByModule, [
         'resourceUrls' => \CRM_Extension_System::singleton()->getMapper()->getActiveModuleUrls(),
         'angular' => [
           'modules' => $moduleNames,
@@ -125,7 +130,6 @@ class AngularLoader {
           'bundleUrl' => \Civi::service('asset_builder')->getUrl('angular-modules.json', $assetParams),
         ],
       ]);
-      return $result;
     });
 
     $res->addScriptFile('civicrm', 'bower_components/angular/angular.min.js', 100, $this->getRegion(), FALSE);
diff --git a/Civi/Angular/Manager.php b/Civi/Angular/Manager.php
index a763cf9e71..f925b7d343 100644
--- a/Civi/Angular/Manager.php
+++ b/Civi/Angular/Manager.php
@@ -29,6 +29,10 @@ class Manager {
    *     This will be mapped to "~/moduleName" by crmResource.
    *   - settings: array(string $key => mixed $value)
    *     List of settings to preload.
+   *   - settingsFactory: callable
+   *     Callback function to fetch settings.
+   *   - requires: array
+   *     List of other modules required
    */
   protected $modules = NULL;
 
@@ -120,9 +124,19 @@ class Manager {
         $angularModules = array_merge($angularModules, $component->getAngularModules());
       }
       \CRM_Utils_Hook::angularModules($angularModules);
-      foreach (array_keys($angularModules) as $module) {
-        if (!isset($angularModules[$module]['basePages'])) {
-          $angularModules[$module]['basePages'] = ['civicrm/a'];
+      foreach ($angularModules as $module => $info) {
+        // Merge in defaults
+        $angularModules[$module] += ['basePages' => ['civicrm/a']];
+        // Validate settingsFactory callables
+        if (isset($info['settingsFactory'])) {
+          // To keep the cache small, we want `settingsFactory` to contain the string names of class & function, not an object
+          if (!is_array($info['settingsFactory']) && !is_string($info['settingsFactory'])) {
+            throw new \CRM_Core_Exception($module . ' settingsFactory must be a callable array or string');
+          }
+          // To keep the cache small, convert full object to just the class name
+          if (is_array($info['settingsFactory']) && is_object($info['settingsFactory'][0])) {
+            $angularModules[$module]['settingsFactory'][0] = get_class($info['settingsFactory'][0]);
+          }
         }
       }
       $this->modules = $this->resolvePatterns($angularModules);
@@ -397,6 +411,7 @@ class Manager {
               break;
 
             case 'settings':
+            case 'settingsFactory':
             case 'requires':
               if (!empty($module[$resType])) {
                 $result[$moduleName] = $module[$resType];
diff --git a/tests/phpunit/Civi/Angular/LoaderTest.php b/tests/phpunit/Civi/Angular/LoaderTest.php
new file mode 100644
index 0000000000..07857bd040
--- /dev/null
+++ b/tests/phpunit/Civi/Angular/LoaderTest.php
@@ -0,0 +1,97 @@
+<?php
+/*
+ +--------------------------------------------------------------------+
+ | Copyright CiviCRM LLC. All rights reserved.                        |
+ |                                                                    |
+ | This work is published under the GNU AGPLv3 license with some      |
+ | permitted exceptions and without any warranty. For full license    |
+ | and copyright information, see https://civicrm.org/licensing       |
+ +--------------------------------------------------------------------+
+ */
+
+namespace Civi\Angular;
+
+/**
+ * Test the Angular loader.
+ */
+class LoaderTest extends \CiviUnitTestCase {
+
+  public static $dummy_setting_count = 0;
+  public static $dummy_callback_count = 0;
+
+  public function setUp() {
+    parent::setUp();
+    $this->hookClass->setHook('civicrm_angularModules', [$this, 'hook_angularModules']);
+    self::$dummy_setting_count = 0;
+    self::$dummy_callback_count = 0;
+    $this->createLoggedInUser();
+  }
+
+  public function factoryScenarios() {
+    return [
+      ['dummy1', 2, 1],
+      ['dummy2', 2, 0],
+      ['dummy3', 2, 2],
+    ];
+  }
+
+  /**
+   * Tests that AngularLoader only conditionally loads settings via factory functions for in-use modules.
+   * Our dummy settings callback functions keep a count of the number of times they have been called.
+   *
+   * @dataProvider factoryScenarios
+   * @param $module
+   * @param $expectedSettingCount
+   * @param $expectedCallbackCount
+   */
+  public function testSettingFactory($module, $expectedSettingCount, $expectedCallbackCount) {
+    (new \Civi\Angular\AngularLoader())
+      ->setModules([$module])
+      ->useApp()
+      ->load();
+
+    // Run factory callbacks
+    $factorySettings = \Civi::resources()->getSettings();
+
+    // Dummy1 module's factory setting should be set if it is loaded directly or required by dummy3
+    $this->assertTrue(($expectedCallbackCount > 0) === isset($factorySettings['dummy1']['dummy_setting_factory']));
+    // Dummy3 module's factory setting should be set if it is loaded directly
+    $this->assertTrue(($expectedCallbackCount > 1) === isset($factorySettings['dummy3']['dummy_setting_factory']));
+
+    // Dummy1 module's regular setting should be set if it is loaded directly or required by dummy3
+    $this->assertTrue(($module !== 'dummy2') === isset($factorySettings['dummy1']['dummy_setting']));
+    // Dummy2 module's regular setting should be set if loaded
+    $this->assertTrue(($module === 'dummy2') === isset($factorySettings['dummy2']['dummy_setting']));
+
+    // Assert the callback functions ran the expected number of times
+    $this->assertEquals($expectedSettingCount, self::$dummy_setting_count);
+    $this->assertEquals($expectedCallbackCount, self::$dummy_callback_count);
+  }
+
+  public function hook_angularModules(&$modules) {
+    $modules['dummy1'] = [
+      'ext' => 'civicrm',
+      'settings' => $this->getDummySetting(),
+      'settingsFactory' => [self::class, 'getDummySettingFactory'],
+    ];
+    $modules['dummy2'] = [
+      'ext' => 'civicrm',
+      'settings' => $this->getDummySetting(),
+    ];
+    $modules['dummy3'] = [
+      'ext' => 'civicrm',
+      // The string self::class is preferred but passing object $this should also work
+      'settingsFactory' => [$this, 'getDummySettingFactory'],
+      'requires' => ['dummy1'],
+    ];
+  }
+
+  public function getDummySetting() {
+    return ['dummy_setting' => self::$dummy_setting_count++];
+  }
+
+  public static function getDummySettingFactory() {
+    return ['dummy_setting_factory' => self::$dummy_callback_count++];
+  }
+
+}