Merge pull request #27783 from colemanw/afformPerformance

Afform - improve loading performance
civicrm · Nov 6, 2023 · e625909 · e625909
2 parents 9efb910 + 6af23f8
commit e625909
Show file tree

Hide file tree

Showing 12 changed files with 195 additions and 215 deletions.
diff --git a/Civi/Angular/Manager.php b/Civi/Angular/Manager.php
@@ -14,29 +14,13 @@ class Manager {
   protected $res = NULL;
 
   /**
-   * Modules.
+   * Static cache of html partials.
    *
-   * @var array|null
-   *   Each item has some combination of these keys:
-   *   - ext: string
-   *     The Civi extension which defines the Angular module.
-   *   - js: array(string $relativeFilePath)
-   *     List of JS files (relative to the extension).
-   *   - css: array(string $relativeFilePath)
-   *     List of CSS files (relative to the extension).
-   *   - partials: array(string $relativeFilePath)
-   *     A list of partial-HTML folders (relative to the extension).
-   *     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.
-   *   - permissions: array
-   *     List of permissions to make available client-side
-   *   - requires: array
-   *     List of other modules required
+   * Stashing it here because it's too big to store in SqlCache
+   * FIXME: So that probably means we shouldn't be storing in memory either!
+   * @var array
    */
-  protected $modules = NULL;
+  private $partials = [];
 
   /**
    * @var \CRM_Utils_Cache_Interface
@@ -68,7 +52,7 @@ public function __construct($res, \CRM_Utils_Cache_Interface $cache = NULL) {
    */
   public function clear() {
     $this->cache->clear();
-    $this->modules = NULL;
+    $this->partials = [];
     $this->changeSets = NULL;
     // Force-refresh assetBuilder files
     \Civi::container()->get('asset_builder')->clear(FALSE);
@@ -93,14 +77,15 @@ public function clear() {
    *     List of settings to preload.
    */
   public function getModules() {
-    if ($this->modules === NULL) {
+    $moduleNames = $this->cache->get('moduleNames');
+    $angularModules = [];
+    // Cache not set, fetch fresh list of modules and store in cache
+    if (!$moduleNames) {
       $config = \CRM_Core_Config::singleton();
       global $civicrm_root;
 
       // Note: It would be nice to just glob("$civicrm_root/ang/*.ang.php"), but at time
       // of writing CiviMail and CiviCase have special conditionals.
-
-      $angularModules = [];
       $angularModules['angularFileUpload'] = include "$civicrm_root/ang/angularFileUpload.ang.php";
       $angularModules['checklist-model'] = include "$civicrm_root/ang/checklist-model.ang.php";
       $angularModules['crmApp'] = include "$civicrm_root/ang/crmApp.ang.php";
@@ -150,17 +135,26 @@ public function getModules() {
           }
         }
       }
-      $this->modules = $this->resolvePatterns($angularModules);
+      $angularModules = $this->resolvePatterns($angularModules);
+      $this->cache->set('moduleNames', array_keys($angularModules));
+      foreach ($angularModules as $moduleName => $moduleInfo) {
+        $this->cache->set("module $moduleName", $moduleInfo);
+      }
+    }
+    // Rehydrate modules from cache
+    else {
+      foreach ($moduleNames as $moduleName) {
+        $angularModules[$moduleName] = $this->cache->get("module $moduleName");
+      }
     }
 
-    return $this->modules;
+    return $angularModules;
   }
 
   /**
    * Get the descriptor for an Angular module.
    *
-   * @param string $name
-   *   Module name.
+   * @param string $moduleName
    * @return array
    *   Details about the module:
    *   - ext: string, the name of the Civi extension which defines the module
@@ -169,12 +163,12 @@ public function getModules() {
    *   - partials: array(string $relativeFilePath).
    * @throws \Exception
    */
-  public function getModule($name) {
-    $modules = $this->getModules();
-    if (!isset($modules[$name])) {
+  public function getModule($moduleName) {
+    $module = $this->cache->get("module $moduleName") ?? $this->getModules()[$moduleName] ?? NULL;
+    if (!isset($module)) {
       throw new \Exception("Unrecognized Angular module");
     }
-    return $modules[$name];
+    return $module;
   }
 
   /**
@@ -292,13 +286,10 @@ public function getRawPartials($name) {
    *   Invalid partials configuration.
    */
   public function getPartials($name) {
-    $cacheKey = "angular-partials_$name";
-    $cacheValue = $this->cache->get($cacheKey);
-    if ($cacheValue === NULL) {
-      $cacheValue = ChangeSet::applyResourceFilters($this->getChangeSets(), 'partials', $this->getRawPartials($name));
-      $this->cache->set($cacheKey, $cacheValue);
+    if (!isset($this->partials[$name])) {
+      $this->partials[$name] = ChangeSet::applyResourceFilters($this->getChangeSets(), 'partials', $this->getRawPartials($name));
     }
-    return $cacheValue;
+    return $this->partials[$name];
   }
 
   /**

diff --git a/Civi/Api4/Utils/CoreUtil.php b/Civi/Api4/Utils/CoreUtil.php
@@ -239,6 +239,7 @@ public static function isCustomEntity($customGroupName): bool {
    */
   public static function checkAccessRecord(AbstractAction $apiRequest, array $record, int $userID = NULL): ?bool {
     $userID = $userID ?? \CRM_Core_Session::getLoggedInContactID() ?? 0;
+    $idField = self::getIdFieldName($apiRequest->getEntityName());
 
     // Super-admins always have access to everything
     if (\CRM_Core_Permission::check('all CiviCRM permissions and ACLs', $userID)) {
@@ -249,7 +250,7 @@ public static function checkAccessRecord(AbstractAction $apiRequest, array $reco
     // It's a cheap trick and not as efficient as not running the query at all,
     // but BAO::checkAccess doesn't consistently check permissions for the "get" action.
     if (is_a($apiRequest, '\Civi\Api4\Generic\AbstractGetAction')) {
-      return (bool) $apiRequest->addSelect('id')->addWhere('id', '=', $record['id'])->execute()->count();
+      return (bool) $apiRequest->addSelect($idField)->addWhere($idField, '=', $record[$idField])->execute()->count();
     }
 
     $event = new \Civi\Api4\Event\AuthorizeRecordEvent($apiRequest, $record, $userID);

diff --git a/Civi/Core/Container.php b/Civi/Core/Container.php
@@ -427,7 +427,14 @@ public function createContainer() {
    * @return \Civi\Angular\Manager
    */
   public function createAngularManager() {
-    return new \Civi\Angular\Manager(\CRM_Core_Resources::singleton());
+    $moduleEnvId = md5(\CRM_Core_Config_Runtime::getId());
+    $angCache = \CRM_Utils_Cache::create([
+      'name' => substr('angular_' . $moduleEnvId, 0, 32),
+      'type' => ['*memory*', 'SqlGroup', 'ArrayCache'],
+      'withArray' => 'fast',
+      'prefetch' => TRUE,
+    ]);
+    return new \Civi\Angular\Manager(\CRM_Core_Resources::singleton(), $angCache);
   }
 
   /**

diff --git a/ext/afform/core/CRM/Afform/AfformScanner.php b/ext/afform/core/CRM/Afform/AfformScanner.php
@@ -129,54 +129,50 @@ public function clear(): void {
   }
 
   /**
-   * Get the effective metadata for a form.
+   * Get metadata and optionally the layout for a file-based Afform.
    *
    * @param string $name
    *   Ex: 'afformViewIndividual'
-   * @return array
-   *   An array with some mix of the following keys: name, title, description, server_route, requires, is_public.
-   *   NOTE: This is only data available in *.aff.json. It does *NOT* include layout.
-   *   Ex: [
-   *     'name' => 'afformViewIndividual',
-   *     'title' => 'View an individual contact',
-   *     'server_route' => 'civicrm/view-individual',
-   *     'requires' => ['afform'],
-   *   ]
+   * @param bool $getLayout
+   *   Whether to fetch 'layout' from the related html file.
+   * @return array|null
+   *   An array with some mix of the keys supported by getFields
+   * @see \Civi\Api4\Afform::getFields
    */
-  public function getMeta(string $name): ?array {
-    // FIXME error checking
-
-    $defaults = [
-      'requires' => [],
-      'title' => '',
-      'description' => '',
-      'is_public' => FALSE,
-      'permission' => ['access CiviCRM'],
-      'type' => 'system',
-    ];
+  public function getMeta(string $name, bool $getLayout = FALSE): ?array {
     $defn = [];
+    $mtime = NULL;
 
-    // If there is a local file it will be json - read from that first
     $jsonFile = $this->findFilePath($name, self::METADATA_JSON);
+    $htmlFile = $this->findFilePath($name, self::LAYOUT_FILE);
+
+    // Meta file can be either php or json format.
+    // Json takes priority because local overrides are always saved in that format.
     if ($jsonFile !== NULL) {
       $defn = json_decode(file_get_contents($jsonFile), 1);
+      $mtime = filemtime($jsonFile);
     }
     // Extensions may provide afform definitions in php files
     else {
       $phpFile = $this->findFilePath($name, self::METADATA_PHP);
       if ($phpFile !== NULL) {
         $defn = include $phpFile;
+        $mtime = filemtime($phpFile);
       }
     }
-    // A form must have at least a layout file (if no metadata file, just use defaults)
-    if (!$defn && !$this->findFilePath($name, self::LAYOUT_FILE)) {
-      return NULL;
+    if ($htmlFile !== NULL) {
+      $mtime = max($mtime, filemtime($htmlFile));
+      if ($getLayout) {
+        // If the defn file included a layout, the html file overrides
+        $defn['layout'] = file_get_contents($htmlFile);
+      }
     }
-    $defn = array_merge($defaults, $defn, ['name' => $name]);
-    // Previous revisions of GUI allowed permission==''. array_merge() doesn't catch all forms of missing-ness.
-    if (empty($defn['permission'])) {
-      $defn['permission'] = $defaults['permission'];
+    // All 3 files don't exist!
+    elseif (!$defn) {
+      return NULL;
     }
+    $defn['name'] = $name;
+    $defn['modified_date'] = date('Y-m-d H:i:s', $mtime);
     return $defn;
   }
 
@@ -198,13 +194,10 @@ public function addComputedFields(array &$record) {
   }
 
   /**
-   * @param string $formName
-   *   Ex: 'view-individual'
-   * @return string|NULL
-   *   Ex: '<em>Hello world!</em>'
-   *   NULL if no layout exists
+   * @deprecated unused function
    */
   public function getLayout($formName) {
+    CRM_Core_Error::deprecatedFunctionWarning('APIv4');
     $filePath = $this->findFilePath($formName, self::LAYOUT_FILE);
     return $filePath === NULL ? NULL : file_get_contents($filePath);
   }
@@ -214,7 +207,7 @@ public function getLayout($formName) {
    *
    * @return array
    *   A list of all forms, keyed by form name.
-   *   NOTE: This is only data available in metadata files. It does *NOT* include layout.
+   *   NOTE: This is only data available in *.aff.(json|php) files. It does *NOT* include layout.
    *   Ex: ['afformViewIndividual' => ['title' => 'View an individual contact', ...]]
    */
   public function getMetas(): array {

diff --git a/ext/afform/core/Civi/Afform/AngularDependencyMapper.php b/ext/afform/core/Civi/Afform/AngularDependencyMapper.php
@@ -18,89 +18,37 @@
 class AngularDependencyMapper {
 
   /**
-   * Scan the list of Angular modules and inject automatic-requirements.
+   * @var array{attr: array, el: array}
+   */
+  private $revMap;
+
+  public function __construct(array $angularModules) {
+    $this->revMap = $this->getRevMap($angularModules);
+  }
+
+  /**
+   * Adds angular dependencies based on the html contents of an afform.
    *
    * TLDR: if an afform uses element "<other-el/>", and if another module defines
    * `$angularModules['otherMod']['exports']['el'][0] === 'other-el'`, then
    * the 'otherMod' is automatically required.
    *
-   * @param \Civi\Core\Event\GenericHookEvent $e
+   * @param array $afform
    * @see CRM_Utils_Hook::angularModules()
    */
-  public static function autoReq($e) {
-    /** @var \CRM_Afform_AfformScanner $scanner */
-    $scanner = \Civi::service('afform_scanner');
-    $moduleEnvId = md5(\CRM_Core_Config_Runtime::getId() . implode(',', array_keys($e->angularModules)));
-    $depCache = \CRM_Utils_Cache::create([
-      'name' => 'afdep_' . substr($moduleEnvId, 0, 32 - 6),
-      'type' => ['*memory*', 'SqlGroup', 'ArrayCache'],
-      'withArray' => 'fast',
-      'prefetch' => TRUE,
-    ]);
-    $depCacheTtl = 2 * 60 * 60;
-
-    $revMap = self::reverseDeps($e->angularModules);
-
-    $formNames = array_keys($scanner->findFilePaths());
-    foreach ($formNames as $formName) {
-      $angModule = _afform_angular_module_name($formName, 'camel');
-      $cacheLine = $depCache->get($formName, NULL);
-
-      $jFile = $scanner->findFilePath($formName, 'aff.json');
-      $hFile = $scanner->findFilePath($formName, 'aff.html');
-
-      if (!$hFile) {
-        \Civi::log()->warning("Missing html file for Afform: '$jFile'");
-        continue;
-      }
-      $jStat = $jFile ? stat($jFile) : FALSE;
-      $hStat = stat($hFile);
-
-      if ($cacheLine === NULL) {
-        $needsUpdate = TRUE;
-      }
-      elseif ($jStat !== FALSE && $jStat['size'] !== $cacheLine['js']) {
-        $needsUpdate = TRUE;
-      }
-      elseif ($jStat !== FALSE && $jStat['mtime'] > $cacheLine['jm']) {
-        $needsUpdate = TRUE;
-      }
-      elseif ($hStat !== FALSE && $hStat['size'] !== $cacheLine['hs']) {
-        $needsUpdate = TRUE;
-      }
-      elseif ($hStat !== FALSE && $hStat['mtime'] > $cacheLine['hm']) {
-        $needsUpdate = TRUE;
-      }
-      else {
-        $needsUpdate = FALSE;
-      }
-
-      if ($needsUpdate) {
-        $cacheLine = [
-          'js' => $jStat['size'] ?? NULL,
-          'jm' => $jStat['mtime'] ?? NULL,
-          'hs' => $hStat['size'] ?? NULL,
-          'hm' => $hStat['mtime'] ?? NULL,
-          'r' => array_values(array_unique(array_merge(
-            [\CRM_Afform_AfformScanner::DEFAULT_REQUIRES],
-            $e->angularModules[$angModule]['requires'] ?? [],
-            self::reverseDepsFind(file_get_contents($hFile), $revMap)
-          ))),
-        ];
-        $depCache->set($formName, $cacheLine, $depCacheTtl);
-      }
-
-      $e->angularModules[$angModule]['requires'] = $cacheLine['r'];
-    }
+  public function autoReq(array $afform) {
+    $afform['requires'][] = \CRM_Afform_AfformScanner::DEFAULT_REQUIRES;
+    $dependencies = empty($afform['layout']) ? [] : $this->reverseDepsFind($afform['layout']);
+    return array_values(array_unique(array_merge($afform['requires'], $dependencies)));
   }
 
   /**
-   * @param $angularModules
-   * @return array
-   *   'attr': array(string $attrName => string $angModuleName)
-   *   'el': array(string $elementName => string $angModuleName)
+   * @param array $angularModules
+   * @return array{attr: array, el: array}
+   *   'attr': [string $attrName => string $angModuleName]
+   *   'el': [string $elementName => string $angModuleName]
    */
-  private static function reverseDeps($angularModules):array {
+  private function getRevMap(array $angularModules): array {
     $revMap = ['attr' => [], 'el' => []];
     foreach (array_keys($angularModules) as $module) {
       if (!isset($angularModules[$module]['exports'])) {
@@ -120,15 +68,13 @@ private static function reverseDeps($angularModules):array {
 
   /**
    * @param string $html
-   * @param array $revMap
-   *   The reverse-dependencies map from reverseDeps().
    * @return array
    */
-  private static function reverseDepsFind($html, $revMap):array {
+  private function reverseDepsFind(string $html): array {
     $symbols = \Civi\Afform\Symbols::scan($html);
-    $elems = array_intersect_key($revMap['el'], $symbols->elements);
-    $attrs = array_intersect_key($revMap['attr'], $symbols->attributes);
-    return array_values(array_unique(array_merge($elems, $attrs)));
+    $elems = array_intersect_key($this->revMap['el'], $symbols->elements);
+    $attrs = array_intersect_key($this->revMap['attr'], $symbols->attributes);
+    return array_merge($elems, $attrs);
   }
 
 }