Api4 - Improve CachedDAOGetAction to handle simple implicit joins

Civi/Api4/Action/CustomField/Get.php

@@ -17,14 +17,42 @@
  */
 class Get extends \Civi\Api4\Generic\CachedDAOGetAction {
 
+  /**
+   * Return all fields for CustomField entity plus all the ones from
+   * CustomGroup that we are able to get from the cache.
+   *
+   * @return array
+   */
+  protected function getCachedFields(): array {
+    $fields = \Civi::entity('CustomField')->getFields();
+    $groupFields = \Civi::entity('CustomGroup')->getFields();
+    foreach ($groupFields as $name => $groupField) {
+      // CachedDAOGetAction can't handle pseudoconstants across joins
+      if (!isset($groupField['pseudoconstant'])) {
+        $fields["custom_group_id.$name"] = $groupField;
+      }
+    }
+    return $fields;
+  }
+
   /**
    * @inheritdoc
    *
-   * Flatten field records from the CustomGroup cache
+   * Flatten field records from the CustomGroup cache,
+   * formatted as CustomField + implicit joins to CustomGroup
    */
   protected function getCachedRecords(): array {
+    $records = [];
     $groups = \CRM_Core_BAO_CustomGroup::getAll();
-    return array_merge(...array_map(fn ($group) => $group['fields'], $groups));
+    foreach ($groups as $group) {
+      $groupInfo = $group;
+      unset($groupInfo['fields']);
+      $groupInfo = \CRM_Utils_Array::prefixKeys($groupInfo, 'custom_group_id.');
+      foreach ($group['fields'] as $field) {
+        $records[] = $field + $groupInfo;
+      }
+    }
+    return $records;
   }
 
 }

Civi/Api4/Generic/CachedDAOGetAction.php

@@ -13,6 +13,7 @@
 namespace Civi\Api4\Generic;
 
 use Civi\API\Exception\NotImplementedException;
+use Civi\Api4\Utils\CoreUtil;
 
 /**
  * @inheritDoc
@@ -62,9 +63,11 @@ public function __construct($entityName, $actionName, $cacheGetter = NULL) {
    * Decide whether to use self::getFromCache or DAOGetAction::getObjects
    */
   protected function getObjects(Result $result): void {
-    if (is_null($this->useCache)) {
+    // Attempt to use the cache unless explicitly disabled
+    if ($this->useCache !== FALSE) {
       $this->useCache = !$this->needDatabase();
     }
+    $this->_debugOutput['useCache'] = $this->useCache;
     if ($this->useCache) {
       $this->getFromCache($result);
       return;
@@ -130,6 +133,11 @@ protected function getCachedFields(): array {
    * - filter using ArrayQueryActionTrait
    */
   protected function getFromCache($result): void {
+    $idField = CoreUtil::getIdFieldName($this->getEntityName());
+    // For parity with the DAO get action, always select ID.
+    if ($this->select && !in_array($idField, $this->select)) {
+      $this->select[] = $idField;
+    }
     $values = $this->getCachedRecords();
     $this->formatRawValues($values);
     $this->queryArray($values, $result);

tests/phpunit/api/v4/Action/CachedGetTest.php

@@ -66,7 +66,7 @@ public function tearDown(): void {
   }
 
   /**
-   * @return \Civi\Api4\Action\AbstractAction[]
+   * @return \Civi\Api4\Generic\CachedDAOGetAction[]
    */
   protected function cacheableCalls(): array {
     $calls = [];
@@ -80,13 +80,19 @@ protected function cacheableCalls(): array {
 
     // note: pseudoconstant fields should be resolved
     $calls[] = CustomField::get(FALSE)
+      ->addSelect('custom_group_id:label', 'label')
       ->addWhere('custom_group_id:name', '=', 'LemonPreferences');
 
+    // Cache can do simple implicit joins
+    $calls[] = CustomField::get(FALSE)
+      ->addSelect('custom_group_id.name')
+      ->addSelect('custom_group_id.title');
+
     return $calls;
   }
 
   /**
-   * @return \Civi\Api4\Action\AbstractAction[]
+   * @return \Civi\Api4\Generic\CachedDAOGetAction[]
    */
   protected function uncacheableCalls(): array {
     $calls = [];
@@ -100,9 +106,10 @@ protected function uncacheableCalls(): array {
     $calls[] = CustomGroup::get(FALSE)
       ->addSelect('UPPER(name)');
 
-    // cache cant do implicit joins
+    // cache cant do implicit joins with pseudoconstants
     $calls[] = CustomField::get(FALSE)
-      ->addSelect('custom_group_id.name');
+      ->addSelect('custom_group_id.name')
+      ->addSelect('custom_group_id.extends:label');
 
     return $calls;
   }
@@ -113,14 +120,19 @@ protected function uncacheableCalls(): array {
    */
   public function testCachedGetMatchesDatabase(): void {
     foreach ($this->cacheableCalls() as $call) {
+      $call->setDebug(TRUE);
       // we need two copies of the API action object
       $dbCall = clone $call;
 
-      $cacheResult = (array) $call->setUseCache(TRUE)->execute();
+      $cacheResult = $call->setUseCache(TRUE)->execute();
+
+      $dbResult = $dbCall->setUseCache(FALSE)->execute();
 
-      $dbResult = (array) $dbCall->setUseCache(FALSE)->execute();
+      // Assert cache was actually used
+      $this->assertTrue($cacheResult->debug['useCache']);
+      $this->assertFalse($dbResult->debug['useCache']);
 
-      $this->assertEquals($cacheResult, $dbResult);
+      $this->assertEquals((array) $cacheResult, (array) $dbResult);
     }
   }
 
@@ -132,14 +144,15 @@ public function testCachedGetMatchesDatabase(): void {
    */
   public function testHardQueryUsesDatabaseByDefault(): void {
     foreach ($this->uncacheableCalls() as $call) {
-      // we need two copies of the API action object
-      $dbCall = clone $call;
-
-      $defaultResult = (array) $call->execute();
 
-      $dbResult = (array) $dbCall->setUseCache(FALSE)->execute();
+      $dbResult = $call
+        // This setting will be overridden due to the complexity of the api call
+        ->setUseCache(TRUE)
+        ->setDebug(TRUE)
+        ->execute();
 
-      $this->assertEquals($defaultResult, $dbResult);
+      $this->assertFalse($dbResult->debug['useCache']);
+      $this->assertGreaterThanOrEqual(1, $dbResult->count());
     }
   }