Afform - Improve loading performance
authorcolemanw <coleman@civicrm.org>
Wed, 11 Oct 2023 15:37:57 +0000 (11:37 -0400)
committercolemanw <coleman@civicrm.org>
Mon, 6 Nov 2023 20:16:12 +0000 (15:16 -0500)
Before: Angular modules were not cached, afform loading was slow
After: Cache previously used for afform dependencies now used for all Angular modules

Civi/Angular/Manager.php
Civi/Api4/Utils/CoreUtil.php
Civi/Core/Container.php
ext/afform/core/CRM/Afform/AfformScanner.php
ext/afform/core/Civi/Afform/AngularDependencyMapper.php
ext/afform/core/Civi/Api4/Action/Afform/Get.php
ext/afform/core/afform.php
ext/afform/mock/tests/phpunit/api/v4/AfformContactUsageTest.php
ext/afform/mock/tests/phpunit/api/v4/AfformTest.php
tests/phpunit/Civi/Angular/LoaderTest.php

index d5888cc5d086175d70f06694a72ea322a646ef6d..f2ea6b514f64bc5ea58ffdf3ba074c959b09896a 100644 (file)
@@ -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 @@ class Manager {
    */
   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 @@ class Manager {
    *     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 @@ class Manager {
           }
         }
       }
-      $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 @@ class Manager {
    *   - 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 @@ class Manager {
    *   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];
   }
 
   /**
index bb53f94cb4e1dde642e2dda6b86dc120c1801ebe..d709ac091d392eb6a2a2e226d1b5eb567ec3203d 100644 (file)
@@ -239,6 +239,7 @@ class CoreUtil {
    */
   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 @@ class CoreUtil {
     // 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);
index bbe0527653001260fda51f2678b9c351d08762b9..bdc23796dc8fe4f15c7815b7f46d58826f160924 100644 (file)
@@ -427,7 +427,14 @@ class Container {
    * @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);
   }
 
   /**
index 427c255b5f59756b79c3e3feacfdab7af11f9abc..b93b54d21ed751adc54a2db3004fbeebbc33e242 100644 (file)
@@ -129,35 +129,24 @@ class CRM_Afform_AfformScanner {
   }
 
   /**
-   * 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 = [];
 
-    // 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);
     }
@@ -168,15 +157,17 @@ class CRM_Afform_AfformScanner {
         $defn = include $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) {
+      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;
     return $defn;
   }
 
@@ -198,13 +189,10 @@ class CRM_Afform_AfformScanner {
   }
 
   /**
-   * @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 +202,7 @@ class CRM_Afform_AfformScanner {
    *
    * @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 {
index 692ad8aebf89477df0971732defa875729a888ee..889e318e7588b9b8f997fac43feb5e22b25c319c 100644 (file)
@@ -18,89 +18,37 @@ namespace Civi\Afform;
 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 @@ class AngularDependencyMapper {
 
   /**
    * @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);
   }
 
 }
index 9559b619aeceb52d50acdd0377efadbf63d0e72d..a309fd55107f9ac10457d8252c971f100f921b54 100644 (file)
@@ -16,19 +16,22 @@ class Get extends \Civi\Api4\Generic\BasicGetAction {
   use \Civi\Api4\Utils\AfformFormatTrait;
 
   public function getRecords() {
+    $afforms = [];
+
     /** @var \CRM_Afform_AfformScanner $scanner */
     $scanner = \Civi::service('afform_scanner');
+
+    // Optimization: only fetch extra data if requested
     $getComputed = $this->_isFieldSelected('has_local', 'has_base', 'base_module');
     $getLayout = $this->_isFieldSelected('layout');
     $getSearchDisplays = $this->_isFieldSelected('search_displays');
-    $afforms = [];
-
-    // This helps optimize lookups by file/module/directive name
+    // To optimize lookups by file/module/directive name
     $getNames = array_filter([
       'name' => $this->_itemsToGet('name'),
       'module_name' => $this->_itemsToGet('module_name'),
       'directive_name' => $this->_itemsToGet('directive_name'),
     ]);
+    // To optimize lookups by type
     $getTypes = $this->_itemsToGet('type');
 
     $names = $getNames['name'] ?? array_keys($scanner->findFilePaths());
@@ -40,22 +43,12 @@ class Get extends \Civi\Api4\Generic\BasicGetAction {
     \Civi::dispatcher()->dispatch('civi.afform.get', $event);
     // Set defaults for Afforms supplied by hook
     foreach ($hookForms as $afform) {
-      $name = $afform['name'];
-      $afform += [
-        'has_base' => TRUE,
-        'type' => 'form',
-      ];
-      // afCore and af would normally get required by AngularDependencyMapper but that only works on file-based afforms
-      $afform['requires'] = array_unique(array_merge(['afCore', 'af'], $afform['requires'] ?? []));
-      if (!in_array($name, $names)) {
-        $names[] = $name;
-      }
-      $afforms[$name] = $afform;
+      $names[] = $afform['name'];
+      $afform['has_base'] = TRUE;
+      $afforms[$afform['name']] = $afform;
     }
 
-    if ($this->checkPermissions) {
-      $names = array_filter($names, [$this, 'checkPermission']);
-    }
+    $names = array_unique($names);
 
     foreach ($names as $name) {
       $info = [
@@ -66,29 +59,35 @@ class Get extends \Civi\Api4\Generic\BasicGetAction {
       // Skip if afform does not match requested name
       foreach ($getNames as $key => $names) {
         if (!in_array($info[$key], $names)) {
+          unset($afforms[$name]);
           continue 2;
         }
       }
-      $record = $scanner->getMeta($name);
+      $record = $scanner->getMeta($name, $getLayout || $getSearchDisplays);
       // Skip if afform does not exist or is not of requested type(s)
       if (
         (!$record && !isset($afforms[$name])) ||
         ($getTypes && isset($record['type']) && !in_array($record['type'], $getTypes, TRUE))
       ) {
+        unset($afforms[$name]);
         continue;
       }
       $afforms[$name] = array_merge($afforms[$name] ?? [], $record ?? [], $info);
-      if ($getComputed) {
-        $scanner->addComputedFields($afforms[$name]);
-      }
       if (isset($afforms[$name]['permission']) && is_string($afforms[$name]['permission'])) {
         $afforms[$name]['permission'] = explode(',', $afforms[$name]['permission']);
       }
-      if ($getLayout || $getSearchDisplays) {
-        // Autogenerated layouts will already be in values but can be overridden; scanner takes priority
-        $afforms[$name]['layout'] = $scanner->getLayout($name) ?? $afforms[$name]['layout'] ?? '';
+      // No permissions specified, set default.
+      if (empty($afforms[$name]['permission'])) {
+        $afforms[$name]['permission'] = ['access CiviCRM'];
       }
-      if ($getSearchDisplays) {
+      if (!$this->checkPermission($afforms[$name])) {
+        unset($afforms[$name]);
+        continue;
+      }
+      if ($getComputed) {
+        $scanner->addComputedFields($afforms[$name]);
+      }
+      if ($getSearchDisplays && !empty($afforms[$name]['layout'])) {
         $afforms[$name]['search_displays'] = $this->getSearchDisplays($afforms[$name]['layout']);
       }
       if (!isset($afforms[$name]['placement']) && $this->_isFieldSelected('placement')) {
@@ -98,7 +97,7 @@ class Get extends \Civi\Api4\Generic\BasicGetAction {
 
     if ($getLayout && $this->layoutFormat !== 'html') {
       foreach ($afforms as $name => $record) {
-        $afforms[$name]['layout'] = $this->convertHtmlToOutput($record['layout']);
+        $afforms[$name]['layout'] = isset($record['layout']) ? $this->convertHtmlToOutput($record['layout']) : NULL;
       }
     }
 
@@ -124,8 +123,14 @@ class Get extends \Civi\Api4\Generic\BasicGetAction {
    *
    * @return bool
    */
-  protected function checkPermission($name) {
-    return \CRM_Core_Permission::check("@afform:$name");
+  protected function checkPermission($afform) {
+    if (!$this->checkPermissions) {
+      return TRUE;
+    }
+    if (($afform['permission_operator'] ?? NULL) === 'OR') {
+      $afform['permission'] = [$afform['permission']];
+    }
+    return \CRM_Core_Permission::check($afform['permission']);
   }
 
   /**
index ad422e8aa16811ade84ed7dd7ae87a0178a65bbc..18f24071d24a70647cdecc716124f2a466b5bcfa 100644 (file)
@@ -54,7 +54,7 @@ function afform_civicrm_config(&$config) {
   $dispatcher->addListener('civi.afform.submit', ['\Civi\Api4\Action\Afform\Submit', 'processGenericEntity'], 0);
   $dispatcher->addListener('civi.afform.submit', ['\Civi\Api4\Action\Afform\Submit', 'preprocessContact'], 10);
   $dispatcher->addListener('civi.afform.submit', ['\Civi\Api4\Action\Afform\Submit', 'processRelationships'], 1);
-  $dispatcher->addListener('hook_civicrm_angularModules', ['\Civi\Afform\AngularDependencyMapper', 'autoReq'], -1000);
+  $dispatcher->addListener('hook_civicrm_angularModules', '_afform_hook_civicrm_angularModules', -1000);
   $dispatcher->addListener('hook_civicrm_alterAngular', ['\Civi\Afform\AfformMetadataInjector', 'preprocess']);
   $dispatcher->addListener('hook_civicrm_check', ['\Civi\Afform\StatusChecks', 'hook_civicrm_check']);
   $dispatcher->addListener('civi.afform.get', ['\Civi\Api4\Action\Afform\Get', 'getCustomGroupBlocks']);
@@ -135,7 +135,7 @@ function afform_civicrm_managed(&$entities, $modules) {
         'values' => [
           'name' => $afform['name'],
           'label' => $afform['navigation']['label'] ?: $afform['title'],
-          'permission' => $afform['permission'],
+          'permission' => (array) (empty($afform['permission']) ? 'access CiviCRM' : $afform['permission']),
           'permission_operator' => $afform['permission_operator'] ?? 'AND',
           'weight' => $afform['navigation']['weight'] ?? 0,
           'url' => $afform['server_route'],
@@ -301,17 +301,22 @@ function _afform_get_contact_types(array $mixedTypes): array {
 }
 
 /**
- * Implements hook_civicrm_angularModules().
+ * Late-listener for Angular modules: adds all Afforms and their dependencies.
  *
- * Generate a list of Afform Angular modules.
+ * Must run last so that all other modules are present for reverse-dependency mapping.
+ *
+ * @implements CRM_Utils_Hook::angularModules
+ * @param \Civi\Core\Event\GenericHookEvent $e
  */
-function afform_civicrm_angularModules(&$angularModules) {
+function _afform_hook_civicrm_angularModules($e) {
   $afforms = \Civi\Api4\Afform::get(FALSE)
-    ->setSelect(['name', 'requires', 'module_name', 'directive_name'])
+    ->setSelect(['name', 'requires', 'module_name', 'directive_name', 'layout'])
+    ->setLayoutFormat('html')
     ->execute();
 
+  // 1st pass, add each Afform as angular module
   foreach ($afforms as $afform) {
-    $angularModules[$afform['module_name']] = [
+    $e->angularModules[$afform['module_name']] = [
       'ext' => E::LONG_NAME,
       'js' => ['assetBuilder://afform.js?name=' . urlencode($afform['name'])],
       'requires' => $afform['requires'],
@@ -325,6 +330,12 @@ function afform_civicrm_angularModules(&$angularModules) {
       ],
     ];
   }
+
+  // 2nd pass, now that all Angular modules are declared, add reverse dependencies
+  $dependencyMapper = new \Civi\Afform\AngularDependencyMapper($e->angularModules);
+  foreach ($afforms as $afform) {
+    $e->angularModules[$afform['module_name']]['requires'] = $dependencyMapper->autoReq($afform);
+  }
 }
 
 /**
@@ -433,31 +444,18 @@ function afform_civicrm_permission(&$permissions) {
  * @see CRM_Utils_Hook::permission_check()
  */
 function afform_civicrm_permission_check($permission, &$granted, $contactId) {
-  if ($permission[0] !== '@') {
+  if (!str_starts_with($permission, '@afform:') || strlen($permission) < 9) {
     // Micro-optimization - this function may get hit a lot.
     return;
   }
-
-  if (preg_match('/^@afform:(.*)/', $permission, $m)) {
-    $name = $m[1];
-
-    $afform = \Civi\Api4\Afform::get(FALSE)
-      ->addWhere('name', '=', $name)
-      ->addSelect('permission', 'permission_operator')
-      ->execute()
-      ->first();
-    // No permissions found... this shouldn't happen but just in case, set default.
-    if ($afform && empty($afform['permission'])) {
-      $afform['permission'] = ['access CiviCRM'];
-    }
-    if ($afform) {
-      $check = (array) $afform['permission'];
-      if ($afform['permission_operator'] === 'OR') {
-        $check = [$check];
-      }
-      $granted = CRM_Core_Permission::check($check, $contactId);
-    }
-  }
+  [, $name] = explode(':', $permission, 2);
+  // Delegate permission check to APIv4
+  $check = \Civi\Api4\Afform::checkAccess()
+    ->addValue('name', $name)
+    ->setAction('get')
+    ->execute()
+    ->first();
+  $granted = $check['access'];
 }
 
 /**
index b3f8c6307553e756f7f07d88e0424ca3df11e256..0d83ebdfb8c94419bcf2af00d47c3b018b362119 100644 (file)
@@ -3,7 +3,7 @@
 use Civi\Api4\Afform;
 
 /**
- * Test case for Afform.prefill and Afform.submit.
+ * Test case for Afform.checkAccess, Afform.prefill and Afform.submit.
  *
  * @group headless
  */
@@ -198,6 +198,31 @@ EOHTML;
     $this->callAPISuccessGetSingle('ActivityContact', ['contact_id' => $contact['id'], 'activity_id' => $activity['id']]);
   }
 
+  public function testCheckAccess(): void {
+    $this->useValues([
+      'layout' => self::$layouts['aboutMe'],
+      'permission' => ['access CiviCRM'],
+    ]);
+    $this->createLoggedInUser();
+    CRM_Core_Config::singleton()->userPermissionTemp = NULL;
+    CRM_Core_Config::singleton()->userPermissionClass->permissions = [
+      'access Contact Dashboard',
+    ];
+    $check = Afform::checkAccess()
+      ->addValue('name', $this->formName)
+      ->setAction('get')
+      ->execute()->first();
+    $this->assertFalse($check['access']);
+    CRM_Core_Config::singleton()->userPermissionClass->permissions = [
+      'access CiviCRM',
+    ];
+    $check = Afform::checkAccess()
+      ->addValue('name', $this->formName)
+      ->setAction('get')
+      ->execute()->first();
+    $this->assertTrue($check['access']);
+  }
+
   public function testAboutMeForbidden(): void {
     $this->useValues([
       'layout' => self::$layouts['aboutMe'],
index a408249e51d538e47f5b608ac223a69ddbd62fc3..15172613c5890779932b3161ead3b0cbc739aad8 100644 (file)
@@ -279,8 +279,9 @@ class api_v4_AfformTest extends api_v4_AfformTestCase {
     // The default mockPage has 1 explicit requirement + 2 automatic requirements.
     Afform::revert()->addWhere('name', '=', $formName)->execute();
     $angModule = Civi::service('angular')->getModule($formName);
-    $this->assertEquals(['afCore', 'mockBespoke', 'mockBareFile', 'mockFoo'], $angModule['requires']);
+    sort($angModule['requires']);
     $storedRequires = Afform::get()->addWhere('name', '=', $formName)->addSelect('requires')->execute();
+    $this->assertEquals(['afCore', 'mockBareFile', 'mockBespoke', 'mockFoo'], $angModule['requires']);
     $this->assertEquals(['mockBespoke'], $storedRequires[0]['requires']);
 
     // Knock down to 1 explicit + 1 automatic.
@@ -290,8 +291,9 @@ class api_v4_AfformTest extends api_v4_AfformTestCase {
       ->setValues(['layout' => '<div>The bare file says "<mock-bare-file/>"</div>'])
       ->execute();
     $angModule = Civi::service('angular')->getModule($formName);
-    $this->assertEquals(['afCore', 'mockBespoke', 'mockBareFile'], $angModule['requires']);
+    sort($angModule['requires']);
     $storedRequires = Afform::get()->addWhere('name', '=', $formName)->addSelect('requires')->execute();
+    $this->assertEquals(['afCore', 'mockBareFile', 'mockBespoke'], $angModule['requires']);
     $this->assertEquals(['mockBespoke'], $storedRequires[0]['requires']);
 
     // Remove the last explict and implicit requirements.
@@ -310,7 +312,8 @@ class api_v4_AfformTest extends api_v4_AfformTestCase {
 
     Afform::revert()->addWhere('name', '=', $formName)->execute();
     $angModule = Civi::service('angular')->getModule($formName);
-    $this->assertEquals(['afCore', 'mockBespoke', 'mockBareFile', 'mockFoo'], $angModule['requires']);
+    sort($angModule['requires']);
+    $this->assertEquals(['afCore', 'mockBareFile', 'mockBespoke', 'mockFoo'], $angModule['requires']);
   }
 
 }
index 0136ffde8744fa5c1f59879b873ba750f2aea4da..2968a46aab13d73860a13f123d6c926dec74cd04 100644 (file)
@@ -23,6 +23,7 @@ class LoaderTest extends \CiviUnitTestCase {
     $this->hookClass->setHook('civicrm_angularModules', [$this, 'hook_angularModules']);
     self::$dummy_callback_count = 0;
     $this->createLoggedInUser();
+    \Civi::container()->get('angular')->clear();
   }
 
   public function factoryScenarios() {