From e0d526a1e5ba5d49914f14b647feee9de1f7133b Mon Sep 17 00:00:00 2001 From: colemanw Date: Mon, 11 Sep 2023 21:04:30 -0400 Subject: [PATCH] APIv4 - Fix conformance test for entities with multiple primary keys --- .../phpunit/api/v4/Entity/ConformanceTest.php | 218 +++++++++--------- 1 file changed, 111 insertions(+), 107 deletions(-) diff --git a/tests/phpunit/api/v4/Entity/ConformanceTest.php b/tests/phpunit/api/v4/Entity/ConformanceTest.php index 8fd04eaf73..596d53254c 100644 --- a/tests/phpunit/api/v4/Entity/ConformanceTest.php +++ b/tests/phpunit/api/v4/Entity/ConformanceTest.php @@ -143,20 +143,20 @@ class ConformanceTest extends Api4TestBase implements HookInterface { $this->markTestSkipped("The API \"$entityName\" does not implement CRUD actions"); } - $this->checkFields($entityClass, $entityName); + $this->checkFields($entityName); $this->checkCreationDenied($entityName, $entityClass); - $id = $this->checkCreation($entityName, $entityClass); - $getResult = $this->checkGet($entityName, $id); + $entityKeys = $this->checkCreation($entityName); + $getResult = $this->checkGet($entityName, $entityKeys); // civi.api4.authorizeRecord does not work on `get` actions // $this->checkGetAllowed($entityClass, $id, $entityName); - $this->checkGetCount($entityClass, $id, $entityName); - $this->checkUpdateFailsFromCreate($entityClass, $id); - $this->checkUpdate($entityName, $getResult); + $this->checkGetCount($entityClass, $entityKeys, $entityName); + $this->checkUpdateFailsFromCreate($entityClass, $entityKeys); + $this->checkUpdate($entityName, $entityKeys, $getResult); $this->checkWrongParamType($entityClass); $this->checkDeleteWithNoId($entityClass); - $this->checkDeletionDenied($entityClass, $id, $entityName); - $this->checkDeletionAllowed($entityClass, $id, $entityName); - $this->checkPostDelete($entityClass, $id, $entityName); + $this->checkDeletionDenied($entityClass, $entityKeys, $entityName); + $this->checkDeletionAllowed($entityClass, $entityKeys, $entityName); + $this->checkPostDelete($entityClass, $entityKeys, $entityName); } /** @@ -176,22 +176,22 @@ class ConformanceTest extends Api4TestBase implements HookInterface { } /** - * @param \Civi\Api4\Generic\AbstractEntity|string $entityClass - * @param string $entity + * @param string $entityName * * @throws \CRM_Core_Exception */ - protected function checkFields($entityClass, $entity) { - $fields = $entityClass::getFields(FALSE) - ->addWhere('type', '=', 'Field') - ->execute() - ->indexBy('name'); + protected function checkFields($entityName) { + $fields = civicrm_api4($entityName, 'getFields', [ + 'checkPermissions' => FALSE, + 'where' => [['type', '=', 'Field']], + ])->indexBy('name'); - $idField = CoreUtil::getIdFieldName($entity); + $idField = CoreUtil::getIdFieldName($entityName); - $errMsg = sprintf('%s getfields is missing primary key field', $entity); + $errMsg = sprintf('%s getfields is missing primary key field', $entityName); $this->assertArrayHasKey($idField, $fields, $errMsg); + // Hmm, not true of every primary key... what about Afform.name? $this->assertEquals('Integer', $fields[$idField]['data_type']); // Ensure that the getFields (FieldSpec) format is generally consistent. @@ -225,84 +225,82 @@ class ConformanceTest extends Api4TestBase implements HookInterface { } /** - * @param string $entity - * @param \Civi\Api4\Generic\AbstractEntity|string $entityClass + * @param string $entityName * - * @return mixed + * @return array */ - protected function checkCreation($entity, $entityClass) { - $isReadOnly = $this->isReadOnly($entityClass); + protected function checkCreation(string $entityName): array { + $isReadOnly = $this->isReadOnly($entityName); $hookLog = []; $onValidate = function(ValidateValuesEvent $e) use (&$hookLog) { $hookLog[$e->getEntityName()][$e->getActionName()] = 1 + ($hookLog[$e->getEntityName()][$e->getActionName()] ?? 0); }; \Civi::dispatcher()->addListener('civi.api4.validate', $onValidate); - \Civi::dispatcher()->addListener('civi.api4.validate::' . $entity, $onValidate); + \Civi::dispatcher()->addListener('civi.api4.validate::' . $entityName, $onValidate); - $this->setCheckAccessGrants(["{$entity}::create" => TRUE]); - $this->assertEquals(0, $this->checkAccessCounts["{$entity}::create"]); + $this->setCheckAccessGrants(["{$entityName}::create" => TRUE]); + $this->assertEquals(0, $this->checkAccessCounts["{$entityName}::create"]); - $requiredParams = $this->getRequiredValuesToCreate($entity); - $createResult = $entityClass::create() - ->setValues($requiredParams) - ->setCheckPermissions(!$isReadOnly) - ->execute() - ->first(); + $requiredParams = $this->getRequiredValuesToCreate($entityName); + $createResult = civicrm_api4($entityName, 'create', [ + 'values' => $requiredParams, + 'checkPermissions' => !$isReadOnly, + ])->single(); - $idField = CoreUtil::getIdFieldName($entity); + $primaryKeys = CoreUtil::getInfoItem($entityName, 'primary_key'); - $this->assertArrayHasKey($idField, $createResult, "create missing ID"); - $id = $createResult[$idField]; - $this->assertGreaterThanOrEqual(1, $id, "$entity ID not positive"); + foreach ($primaryKeys as $idField) { + $this->assertArrayHasKey($idField, $createResult, "create missing $idField"); + } + $id = $createResult[$primaryKeys[0]]; + $this->assertGreaterThanOrEqual(1, $id, "$entityName ID not positive"); if (!$isReadOnly) { - $this->assertEquals(1, $this->checkAccessCounts["{$entity}::create"]); + $this->assertEquals(1, $this->checkAccessCounts["{$entityName}::create"]); } $this->resetCheckAccess(); - $this->assertEquals(2, $hookLog[$entity]['create']); + $this->assertEquals(2, $hookLog[$entityName]['create']); \Civi::dispatcher()->removeListener('civi.api4.validate', $onValidate); - \Civi::dispatcher()->removeListener('civi.api4.validate::' . $entity, $onValidate); + \Civi::dispatcher()->removeListener('civi.api4.validate::' . $entityName, $onValidate); - return $id; + return array_intersect_key($createResult, array_flip($primaryKeys)); } /** - * @param string $entity + * @param string $entityName * @param \Civi\Api4\Generic\AbstractEntity|string $entityClass */ - protected function checkCreationDenied(string $entity, $entityClass): void { - $this->setCheckAccessGrants(["{$entity}::create" => FALSE]); - $this->assertEquals(0, $this->checkAccessCounts["{$entity}::create"]); + protected function checkCreationDenied(string $entityName, $entityClass): void { + $this->setCheckAccessGrants(["{$entityName}::create" => FALSE]); + $this->assertEquals(0, $this->checkAccessCounts["{$entityName}::create"]); - $requiredParams = $this->getRequiredValuesToCreate($entity); + $requiredParams = $this->getRequiredValuesToCreate($entityName); try { $entityClass::create() ->setValues($requiredParams) - ->setCheckPermissions(TRUE) - ->execute() - ->first(); + ->execute(); $this->fail("{$entityClass}::create() should throw an authorization failure."); } catch (UnauthorizedException $e) { // OK, expected exception } - if (!$this->isReadOnly($entityClass)) { - $this->assertEquals(1, $this->checkAccessCounts["{$entity}::create"]); + if (!$this->isReadOnly($entityName)) { + $this->assertEquals(1, $this->checkAccessCounts["{$entityName}::create"]); } $this->resetCheckAccess(); } /** * @param \Civi\Api4\Generic\AbstractEntity|string $entityClass - * @param int $id + * @param array $entityKeys */ - protected function checkUpdateFailsFromCreate($entityClass, int $id): void { + protected function checkUpdateFailsFromCreate($entityClass, array $entityKeys): void { $exceptionThrown = ''; try { $entityClass::create(FALSE) - ->addValue('id', $id) + ->addValue('id', reset($entityKeys)) ->execute(); } catch (\CRM_Core_Exception $e) { @@ -313,36 +311,36 @@ class ConformanceTest extends Api4TestBase implements HookInterface { /** * @param string $entityName - * @param int $id + * @param array $entityKeys */ - protected function checkGet(string $entityName, int $id): array { - $idField = CoreUtil::getIdFieldName($entityName); + protected function checkGet(string $entityName, array $entityKeys): array { $getResult = civicrm_api4($entityName, 'get', [ 'checkPermissions' => FALSE, - 'where' => [[$idField, '=', $id]], - ]); - $errMsg = sprintf('Failed to fetch a %s after creation', $entityName); - $this->assertEquals($id, $getResult->first()[$idField], $errMsg); - return $getResult->single(); + 'where' => self::valsToClause($entityKeys), + ])->single(); + foreach ($entityKeys as $key => $val) { + $this->assertEquals($val, $getResult[$key]); + } + return $getResult; } /** * Ensure updating an entity does not alter it * * @param string $entityName + * @param array $entityKeys * @param array $getResult * @throws \CRM_Core_Exception */ - protected function checkUpdate(string $entityName, array $getResult): void { - $idField = CoreUtil::getIdFieldName($entityName); + protected function checkUpdate(string $entityName, array $entityKeys, array $getResult): void { civicrm_api4($entityName, 'update', [ 'checkPermissions' => FALSE, - 'where' => [[$idField, '=', $getResult[$idField]]], - 'values' => [$idField, $getResult[$idField]], + 'where' => self::valsToClause($entityKeys), + 'values' => $entityKeys, ]); $getResult2 = civicrm_api4($entityName, 'get', [ 'checkPermissions' => FALSE, - 'where' => [[$idField, '=', $getResult[$idField]]], + 'where' => self::valsToClause($entityKeys), ]); $this->assertEquals($getResult, $getResult2->single()); } @@ -350,14 +348,14 @@ class ConformanceTest extends Api4TestBase implements HookInterface { /** * FIXME: Not working. `civi.api4.authorizeRecord` does not work on `get` actions. */ - protected function checkGetAllowed($entityClass, $id, $entity) { - $this->setCheckAccessGrants(["{$entity}::get" => TRUE]); + protected function checkGetAllowed($entityClass, $id, $entityName) { + $this->setCheckAccessGrants(["{$entityName}::get" => TRUE]); $getResult = $entityClass::get() ->addWhere('id', '=', $id) ->execute(); - $errMsg = sprintf('Failed to fetch a %s after creation', $entity); - $idField = CoreUtil::getIdFieldName($entity); + $errMsg = sprintf('Failed to fetch a %s after creation', $entityName); + $idField = CoreUtil::getIdFieldName($entityName); $this->assertEquals($id, $getResult->first()[$idField], $errMsg); $this->assertEquals(1, $getResult->count(), $errMsg); $this->resetCheckAccess(); @@ -365,16 +363,15 @@ class ConformanceTest extends Api4TestBase implements HookInterface { /** * @param \Civi\Api4\Generic\AbstractEntity|string $entityClass - * @param int $id - * @param string $entity + * @param array $entityKeys + * @param string $entityName */ - protected function checkGetCount($entityClass, $id, $entity): void { - $idField = CoreUtil::getIdFieldName($entity); + protected function checkGetCount(string $entityClass, array $entityKeys, string $entityName): void { $getResult = $entityClass::get(FALSE) - ->addWhere($idField, '=', $id) + ->setWhere(self::valsToClause($entityKeys)) ->selectRowCount() ->execute(); - $errMsg = sprintf('%s getCount failed', $entity); + $errMsg = sprintf('%s getCount failed', $entityName); $this->assertEquals(1, $getResult->count(), $errMsg); $getResult = $entityClass::get(FALSE) @@ -418,18 +415,17 @@ class ConformanceTest extends Api4TestBase implements HookInterface { * Delete an entity - while having a targeted grant (hook_civirm_checkAccess). * * @param \Civi\Api4\Generic\AbstractEntity|string $entityClass - * @param int $id - * @param string $entity + * @param array $entityKeys + * @param string $entityName */ - protected function checkDeletionAllowed($entityClass, $id, $entity) { - $this->setCheckAccessGrants(["{$entity}::delete" => TRUE]); - $this->assertEquals(0, $this->checkAccessCounts["{$entity}::delete"]); - $isReadOnly = $this->isReadOnly($entityClass); + protected function checkDeletionAllowed($entityClass, $entityKeys, $entityName) { + $this->setCheckAccessGrants(["{$entityName}::delete" => TRUE]); + $this->assertEquals(0, $this->checkAccessCounts["{$entityName}::delete"]); + $isReadOnly = $this->isReadOnly($entityName); - $idField = CoreUtil::getIdFieldName($entity); $deleteAction = $entityClass::delete() ->setCheckPermissions(!$isReadOnly) - ->addWhere($idField, '=', $id); + ->setWhere(self::valsToClause($entityKeys)); if (property_exists($deleteAction, 'useTrash')) { $deleteAction->setUseTrash(FALSE); @@ -439,16 +435,16 @@ class ConformanceTest extends Api4TestBase implements HookInterface { $deleteResult = $deleteAction->execute(); }); - if (in_array('DAOEntity', CoreUtil::getInfoItem($entity, 'type'))) { + if (in_array('DAOEntity', CoreUtil::getInfoItem($entityName, 'type'))) { // We should have emitted an event. - $hookEntity = ($entity === 'Contact') ? 'Individual' : $entity;/* ooph */ - $this->assertContains("pre.{$hookEntity}.delete", $log, "$entity should emit hook_civicrm_pre() for deletions"); - $this->assertContains("post.{$hookEntity}.delete", $log, "$entity should emit hook_civicrm_post() for deletions"); + $hookEntity = ($entityName === 'Contact') ? 'Individual' : $entityName;/* ooph */ + $this->assertContains("pre.{$hookEntity}.delete", $log, "$entityName should emit hook_civicrm_pre() for deletions"); + $this->assertContains("post.{$hookEntity}.delete", $log, "$entityName should emit hook_civicrm_post() for deletions"); // should get back an array of deleted id - $this->assertEquals([['id' => $id]], (array) $deleteResult); + $this->assertEquals([$entityKeys], (array) $deleteResult); if (!$isReadOnly) { - $this->assertEquals(1, $this->checkAccessCounts["{$entity}::delete"]); + $this->assertEquals(1, $this->checkAccessCounts["{$entityName}::delete"]); } } $this->resetCheckAccess(); @@ -458,40 +454,40 @@ class ConformanceTest extends Api4TestBase implements HookInterface { * Attempt to delete an entity while having explicitly denied permission (hook_civicrm_checkAccess). * * @param \Civi\Api4\Generic\AbstractEntity|string $entityClass - * @param int $id - * @param string $entity + * @param array $entityKeys + * @param string $entityName */ - protected function checkDeletionDenied($entityClass, $id, $entity) { - $this->setCheckAccessGrants(["{$entity}::delete" => FALSE]); - $this->assertEquals(0, $this->checkAccessCounts["{$entity}::delete"]); + protected function checkDeletionDenied($entityClass, array $entityKeys, $entityName) { + $this->setCheckAccessGrants(["{$entityName}::delete" => FALSE]); + $this->assertEquals(0, $this->checkAccessCounts["{$entityName}::delete"]); try { $entityClass::delete() - ->addWhere('id', '=', $id) + ->setWhere(self::valsToClause($entityKeys)) ->execute(); - $this->fail("{$entity}::delete should throw an authorization failure."); + $this->fail("{$entityName}::delete should throw an authorization failure."); } catch (UnauthorizedException $e) { // OK } - if (!$this->isReadOnly($entityClass)) { - $this->assertEquals(1, $this->checkAccessCounts["{$entity}::delete"]); + if (!$this->isReadOnly($entityName)) { + $this->assertEquals(1, $this->checkAccessCounts["{$entityName}::delete"]); } $this->resetCheckAccess(); } /** * @param \Civi\Api4\Generic\AbstractEntity|string $entityClass - * @param int $id - * @param string $entity + * @param array $entityKeys + * @param string $entityName */ - protected function checkPostDelete($entityClass, $id, $entity) { + protected function checkPostDelete($entityClass, array $entityKeys, $entityName) { $getDeletedResult = $entityClass::get(FALSE) - ->addWhere('id', '=', $id) + ->setWhere(self::valsToClause($entityKeys)) ->execute(); - $errMsg = sprintf('Entity "%s" was not deleted', $entity); + $errMsg = sprintf('Entity "%s" was not deleted', $entityName); $this->assertEquals(0, count($getDeletedResult), $errMsg); } @@ -514,11 +510,11 @@ class ConformanceTest extends Api4TestBase implements HookInterface { } /** - * @param \Civi\Api4\Generic\AbstractEntity|string $entityClass + * @param string $entityName * @return bool */ - protected function isReadOnly($entityClass) { - return in_array('ReadOnlyEntity', $entityClass::getInfo()['type'], TRUE); + protected function isReadOnly($entityName) { + return in_array('ReadOnlyEntity', CoreUtil::getInfoItem($entityName, 'type'), TRUE); } /** @@ -555,4 +551,12 @@ class ConformanceTest extends Api4TestBase implements HookInterface { return $log; } + private static function valsToClause(array $vals) { + $clause = []; + foreach ($vals as $key => $val) { + $clause[] = [$key, '=', $val]; + } + return $clause; + } + } -- 2.25.1