From 5fea9249a244a1381ef0eb10cade46a4c461ae83 Mon Sep 17 00:00:00 2001 From: colemanw Date: Thu, 5 Dec 2024 10:33:15 -0500 Subject: [PATCH] Api4 - Improve CachedDAOGetAction to handle simple implicit joins --- Civi/Api4/Action/CustomField/Get.php | 32 ++++++++++++++- Civi/Api4/Generic/CachedDAOGetAction.php | 10 ++++- tests/phpunit/api/v4/Action/CachedGetTest.php | 39 ++++++++++++------- 3 files changed, 65 insertions(+), 16 deletions(-) diff --git a/Civi/Api4/Action/CustomField/Get.php b/Civi/Api4/Action/CustomField/Get.php index 507124905e9..b2ad294c7c3 100644 --- a/Civi/Api4/Action/CustomField/Get.php +++ b/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; } } diff --git a/Civi/Api4/Generic/CachedDAOGetAction.php b/Civi/Api4/Generic/CachedDAOGetAction.php index 859bfe04e6b..3d8c20dc004 100644 --- a/Civi/Api4/Generic/CachedDAOGetAction.php +++ b/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); diff --git a/tests/phpunit/api/v4/Action/CachedGetTest.php b/tests/phpunit/api/v4/Action/CachedGetTest.php index 274161b0fb3..047c41c56c3 100644 --- a/tests/phpunit/api/v4/Action/CachedGetTest.php +++ b/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()); } }