From: Coleman Watts <coleman@civicrm.org>
Date: Fri, 22 May 2020 20:45:11 +0000 (-0400)
Subject: APIv4 - Fix explicit joins to custom entities
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=5e327f378a3816aae7762841486a3022107d3ef4;hp=77fdabbf64db7fc9342faf1f8c527dec4b461dfd;p=civicrm-core.git

APIv4 - Fix explicit joins to custom entities
---

diff --git a/CRM/Core/BAO/CustomValue.php b/CRM/Core/BAO/CustomValue.php
index 8cbe6b3d08..3a64cdb29d 100644
--- a/CRM/Core/BAO/CustomValue.php
+++ b/CRM/Core/BAO/CustomValue.php
@@ -206,4 +206,16 @@ class CRM_Core_BAO_CustomValue extends CRM_Core_DAO {
     );
   }
 
+  /**
+   * ACL clause for an APIv4 custom pseudo-entity (aka multi-record custom group extending Contact).
+   * @return array
+   */
+  public function addSelectWhereClause() {
+    $clauses = [
+      'entity_id' => CRM_Utils_SQL::mergeSubquery('Contact'),
+    ];
+    CRM_Utils_Hook::selectWhereClause($this, $clauses);
+    return $clauses;
+  }
+
 }
diff --git a/Civi/API/SelectQuery.php b/Civi/API/SelectQuery.php
index 8e91d0f68b..97fac0a133 100644
--- a/Civi/API/SelectQuery.php
+++ b/Civi/API/SelectQuery.php
@@ -63,7 +63,7 @@ abstract class SelectQuery {
   /**
    * @var array
    */
-  protected $entityFieldNames;
+  protected $entityFieldNames = [];
   /**
    * @var array
    */
diff --git a/Civi/Api4/Action/Entity/GetLinks.php b/Civi/Api4/Action/Entity/GetLinks.php
index a5c6c53fbd..e841c566ac 100644
--- a/Civi/Api4/Action/Entity/GetLinks.php
+++ b/Civi/Api4/Action/Entity/GetLinks.php
@@ -33,7 +33,7 @@ class GetLinks extends \Civi\Api4\Generic\BasicGetAction {
     foreach ($schema->getTables() as $table) {
       $entity = CoreUtil::getApiNameFromTableName($table->getName());
       // Since this is an api function, exclude tables that don't have an api
-      if (class_exists('\Civi\Api4\\' . $entity)) {
+      if (strpos($entity, 'Custom_') === 0 || class_exists('\Civi\Api4\\' . $entity)) {
         $item = [
           'entity' => $entity,
           'table' => $table->getName(),
diff --git a/Civi/Api4/Generic/Traits/CustomValueActionTrait.php b/Civi/Api4/Generic/Traits/CustomValueActionTrait.php
index 236cb3fc4d..acf90cdcf5 100644
--- a/Civi/Api4/Generic/Traits/CustomValueActionTrait.php
+++ b/Civi/Api4/Generic/Traits/CustomValueActionTrait.php
@@ -76,7 +76,7 @@ trait CustomValueActionTrait {
    * @inheritDoc
    */
   protected function deleteObjects($items) {
-    $customTable = CoreUtil::getCustomTableByName($this->getCustomGroup());
+    $customTable = CoreUtil::getTableName($this->getEntityName());
     $ids = [];
     foreach ($items as $item) {
       \CRM_Utils_Hook::pre('delete', $this->getEntityName(), $item['id'], \CRM_Core_DAO::$_nullArray);
diff --git a/Civi/Api4/Query/Api4SelectQuery.php b/Civi/Api4/Query/Api4SelectQuery.php
index 80208522b4..9dffe7a734 100644
--- a/Civi/Api4/Query/Api4SelectQuery.php
+++ b/Civi/Api4/Query/Api4SelectQuery.php
@@ -16,7 +16,6 @@ use Civi\Api4\Service\Schema\Joinable\CustomGroupJoinable;
 use Civi\Api4\Utils\FormattingUtil;
 use Civi\Api4\Utils\CoreUtil;
 use Civi\Api4\Utils\SelectUtil;
-use CRM_Core_DAO_AllCoreTables as AllCoreTables;
 
 /**
  * A query `node` may be in one of three formats:
@@ -81,14 +80,14 @@ class Api4SelectQuery extends SelectQuery {
     if ($apiGet->getDebug()) {
       $this->debugOutput =& $apiGet->_debugOutput;
     }
-    $baoName = CoreUtil::getBAOFromApiName($this->entity);
-    $this->entityFieldNames = array_column($baoName::fields(), 'name');
-    foreach ($apiGet->entityFields() as $path => $field) {
+    foreach ($apiGet->entityFields() as $field) {
+      $this->entityFieldNames[] = $field['name'];
       $field['sql_name'] = '`' . self::MAIN_TABLE_ALIAS . '`.`' . $field['column_name'] . '`';
-      $this->addSpecField($path, $field);
+      $this->addSpecField($field['name'], $field);
     }
 
-    $this->constructQueryObject($baoName);
+    $baoName = CoreUtil::getBAOFromApiName($this->entity);
+    $this->constructQueryObject();
 
     // Add ACLs first to avoid redundant subclauses
     $this->query->where($this->getAclClause(self::MAIN_TABLE_ALIAS, $baoName));
@@ -443,11 +442,11 @@ class Api4SelectQuery extends SelectQuery {
         $field['is_join'] = TRUE;
         $this->addSpecField($alias . '.' . $field['name'], $field);
       }
-      $conditions = [];
-      foreach (array_merge($join, $this->getJoinConditions($entity, $alias)) as $clause) {
+      $conditions = $this->getJoinConditions($entity, $alias);
+      foreach (array_filter($join) as $clause) {
         $conditions[] = $this->treeWalkClauses($clause, 'ON');
       }
-      $tableName = AllCoreTables::getTableForEntityName($entity);
+      $tableName = CoreUtil::getTableName($entity);
       $this->join($side, $tableName, $alias, $conditions);
     }
   }
@@ -467,18 +466,20 @@ class Api4SelectQuery extends SelectQuery {
     $stack = [NULL, NULL];
     foreach ($this->apiFieldSpec as $name => $field) {
       if ($field['entity'] !== $entity && $field['fk_entity'] === $entity) {
-        $conditions[] = [$name, '=', "$alias.id"];
-        $stack = [$name];
+        $conditions[] = $this->treeWalkClauses([$name, '=', "$alias.id"], 'ON');
       }
       elseif (strpos($name, "$alias.") === 0 && substr_count($name, '.') === 1 &&  $field['fk_entity'] === $this->entity) {
-        $conditions[] = [$name, '=', 'id'];
+        $conditions[] = $this->treeWalkClauses([$name, '=', 'id'], 'ON');
+        $stack = ['id'];
       }
     }
     // Hmm, if we came up with > 1 condition, then it's ambiguous how it should be joined so we won't return anything but the generic ACLs
     if (count($conditions) > 1) {
-      return $this->getAclClause($alias, AllCoreTables::getFullName($entity), [NULL, NULL]);
+      $stack = [NULL, NULL];
+      $conditions = [];
     }
-    $acls = $this->getAclClause($alias, AllCoreTables::getFullName($entity), $stack);
+    $baoName = CoreUtil::getBAOFromApiName($entity);
+    $acls = array_values($this->getAclClause($alias, $baoName, $stack));
     return array_merge($acls, $conditions);
   }
 
@@ -531,7 +532,7 @@ class Api4SelectQuery extends SelectQuery {
    * @return FALSE|string
    */
   public function getFrom() {
-    return AllCoreTables::getTableForClass(AllCoreTables::getFullName($this->entity));
+    return CoreUtil::getTableName($this->entity);
   }
 
   /**
@@ -635,19 +636,11 @@ class Api4SelectQuery extends SelectQuery {
   /**
    * Get table name on basis of entity
    *
-   * @param string $baoName
-   *
    * @return void
    */
-  public function constructQueryObject($baoName) {
-    if (strstr($this->entity, 'Custom_')) {
-      $this->query = \CRM_Utils_SQL_Select::from(CoreUtil::getCustomTableByName(str_replace('Custom_', '', $this->entity)) . ' ' . self::MAIN_TABLE_ALIAS);
-      $this->entityFieldNames = array_keys($this->apiFieldSpec);
-    }
-    else {
-      $bao = new $baoName();
-      $this->query = \CRM_Utils_SQL_Select::from($bao->tableName() . ' ' . self::MAIN_TABLE_ALIAS);
-    }
+  public function constructQueryObject() {
+    $tableName = CoreUtil::getTableName($this->entity);
+    $this->query = \CRM_Utils_SQL_Select::from($tableName . ' ' . self::MAIN_TABLE_ALIAS);
   }
 
   /**
diff --git a/Civi/Api4/Service/Schema/SchemaMapBuilder.php b/Civi/Api4/Service/Schema/SchemaMapBuilder.php
index 684784f54f..b6393755e0 100644
--- a/Civi/Api4/Service/Schema/SchemaMapBuilder.php
+++ b/Civi/Api4/Service/Schema/SchemaMapBuilder.php
@@ -110,10 +110,13 @@ class SchemaMapBuilder {
       foreach ($table->getTableLinks() as $link) {
         $target = $map->getTableByName($link->getTargetTable());
         $tableName = $link->getBaseTable();
-        $plural = str_replace('civicrm_', '', $this->getPlural($tableName));
-        $joinable = new Joinable($tableName, $link->getBaseColumn(), $plural);
-        $joinable->setJoinType($joinable::JOIN_TYPE_ONE_TO_MANY);
-        $target->addTableLink($link->getTargetColumn(), $joinable);
+        // Exclude custom field tables
+        if (strpos($link->getTargetTable(), 'civicrm_value_') !== 0) {
+          $plural = str_replace('civicrm_', '', $this->getPlural($tableName));
+          $joinable = new Joinable($tableName, $link->getBaseColumn(), $plural);
+          $joinable->setJoinType($joinable::JOIN_TYPE_ONE_TO_MANY);
+          $target->addTableLink($link->getTargetColumn(), $joinable);
+        }
       }
     }
   }
@@ -178,6 +181,12 @@ class SchemaMapBuilder {
       $links[$alias]['tableName'] = $tableName;
       $links[$alias]['isMultiple'] = !empty($fieldData->is_multiple);
       $links[$alias]['columns'][$fieldData->name] = $fieldData->column_name;
+
+      // Add backreference
+      if (!empty($fieldData->is_multiple)) {
+        $joinable = new Joinable($baseTable->getName(), 'id', AllCoreTables::convertEntityNameToLower($entity));
+        $customTable->addTableLink('entity_id', $joinable);
+      }
     }
 
     foreach ($links as $alias => $link) {
diff --git a/Civi/Api4/Service/Spec/SpecGatherer.php b/Civi/Api4/Service/Spec/SpecGatherer.php
index 798779a879..22d730be53 100644
--- a/Civi/Api4/Service/Spec/SpecGatherer.php
+++ b/Civi/Api4/Service/Spec/SpecGatherer.php
@@ -138,6 +138,7 @@ class SpecGatherer {
    */
   private function getCustomGroupFields($customGroup, RequestSpec $specification) {
     $customFields = CustomField::get()
+      ->setCheckPermissions(FALSE)
       ->addWhere('custom_group.name', '=', $customGroup)
       ->setSelect(['custom_group.name', 'custom_group.table_name', '*'])
       ->execute();
diff --git a/Civi/Api4/Utils/CoreUtil.php b/Civi/Api4/Utils/CoreUtil.php
index 9055a63149..1f4c7766a4 100644
--- a/Civi/Api4/Utils/CoreUtil.php
+++ b/Civi/Api4/Utils/CoreUtil.php
@@ -21,7 +21,6 @@
 
 namespace Civi\Api4\Utils;
 
-use Civi\Api4\CustomGroup;
 use CRM_Core_DAO_AllCoreTables as AllCoreTables;
 
 require_once 'api/v3/utils.php';
@@ -39,34 +38,39 @@ class CoreUtil {
    */
   public static function getBAOFromApiName($entityName) {
     if ($entityName === 'CustomValue' || strpos($entityName, 'Custom_') === 0) {
-      return 'CRM_Contact_BAO_Contact';
+      return 'CRM_Core_BAO_CustomValue';
     }
     return \_civicrm_api3_get_BAO($entityName);
   }
 
   /**
-   * Get table name of given Custom group
+   * Get table name of given entity
    *
-   * @param string $customGroupName
+   * @param string $entityName
    *
    * @return string
    */
-  public static function getCustomTableByName($customGroupName) {
-    return CustomGroup::get()
-      ->addSelect('table_name')
-      ->addWhere('name', '=', $customGroupName)
-      ->execute()
-      ->first()['table_name'];
+  public static function getTableName($entityName) {
+    if (strpos($entityName, 'Custom_') === 0) {
+      $customGroup = substr($entityName, 7);
+      return \CRM_Core_DAO::getFieldValue('CRM_Core_DAO_CustomGroup', $customGroup, 'table_name', 'name');
+    }
+    return AllCoreTables::getTableForEntityName($entityName);
   }
 
   /**
    * Given a sql table name, return the name of the api entity.
    *
    * @param $tableName
-   * @return string
+   * @return string|NULL
    */
   public static function getApiNameFromTableName($tableName) {
-    return AllCoreTables::getBriefName(AllCoreTables::getClassForTable($tableName));
+    $entityName = AllCoreTables::getBriefName(AllCoreTables::getClassForTable($tableName));
+    if (!$entityName) {
+      $customGroup = \CRM_Core_DAO::getFieldValue('CRM_Core_DAO_CustomGroup', $tableName, 'name', 'table_name');
+      $entityName = $customGroup ? "Custom_$customGroup" : NULL;
+    }
+    return $entityName;
   }
 
 }
diff --git a/tests/phpunit/api/v3/ACLPermissionTest.php b/tests/phpunit/api/v3/ACLPermissionTest.php
index c5db7e2ecd..3035ac8668 100644
--- a/tests/phpunit/api/v3/ACLPermissionTest.php
+++ b/tests/phpunit/api/v3/ACLPermissionTest.php
@@ -9,6 +9,11 @@
  +--------------------------------------------------------------------+
  */
 
+use Civi\Api4\Contact;
+use Civi\Api4\CustomField;
+use Civi\Api4\CustomGroup;
+use Civi\Api4\CustomValue;
+
 /**
  * This class is intended to test ACL permission using the multisite module
  *
@@ -1001,4 +1006,64 @@ class api_v3_ACLPermissionTest extends CiviUnitTestCase {
     $this->assertFalse(isset($result['values'][$tag2][$createdFirstName]));
   }
 
+  public function testApi4CustomEntityACL() {
+    $group = uniqid('mg');
+    $textField = uniqid('tx');
+
+    CustomGroup::create()
+      ->setCheckPermissions(FALSE)
+      ->addValue('name', $group)
+      ->addValue('extends', 'Contact')
+      ->addValue('is_multiple', TRUE)
+      ->addChain('field', CustomField::create()
+        ->addValue('label', $textField)
+        ->addValue('custom_group_id', '$id')
+        ->addValue('html_type', 'Text')
+        ->addValue('data_type', 'String')
+      )
+      ->execute();
+
+    $this->createLoggedInUser();
+    $c1 = $this->individualCreate(['first_name' => 'C1']);
+    $c2 = $this->individualCreate(['first_name' => 'C2', 'is_deleted' => 1], 1);
+
+    CustomValue::save($group)->setCheckPermissions(FALSE)
+      ->addRecord(['entity_id' => $c1, $textField => '1'])
+      ->addRecord(['entity_id' => $c2, $textField => '2'])
+      ->execute();
+
+    $this->setPermissions(['access CiviCRM', 'view debug output']);
+    $this->hookClass->setHook('civicrm_aclWhereClause', [$this, 'aclWhereHookAllResults']);
+
+    // Without "access deleted contacts" we won't see C2
+    $vals = CustomValue::get($group)->setDebug(TRUE)->execute();
+    $this->assertCount(1, $vals);
+    $this->assertEquals($c1, $vals[0]['entity_id']);
+
+    $this->setPermissions(['access CiviCRM', 'access deleted contacts', 'view debug output']);
+    $this->hookClass->setHook('civicrm_aclWhereClause', [$this, 'aclWhereHookAllResults']);
+    $this->cleanupCachedPermissions();
+
+    $vals = CustomValue::get($group)->execute();
+    $this->assertCount(2, $vals);
+
+    $this->allowedContactId = $c2;
+    $this->hookClass->setHook('civicrm_aclWhereClause', [$this, 'aclWhereOnlyOne']);
+    $this->cleanupCachedPermissions();
+
+    $vals = CustomValue::get($group)->addSelect('*', 'contact.first_name')->execute();
+    $this->assertCount(1, $vals);
+    $this->assertEquals($c2, $vals[0]['entity_id']);
+    $this->assertEquals('C2', $vals[0]['contact.first_name']);
+
+    $vals = Contact::get()
+      ->addJoin('Custom_' . $group . ' AS cf')
+      ->addSelect('first_name', 'cf.' . $textField)
+      ->addWhere('is_deleted', '=', TRUE)
+      ->execute();
+    $this->assertCount(1, $vals);
+    $this->assertEquals('C2', $vals[0]['first_name']);
+    $this->assertEquals('2', $vals[0]['cf.' . $textField]);
+  }
+
 }