APIv4 - Fix conformance test for entities with multiple primary keys

diff --git a/tests/phpunit/api/v4/Entity/ConformanceTest.php b/tests/phpunit/api/v4/Entity/ConformanceTest.php
index 8fd04eaf73326fd86bf5e1d240653f52345a1141..596d53254ce79bae15bc2aa2c06e98f11c525976 100644 (file)
--- 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;
+  }
+
 }