APIv4 - Support multiple implicit joins to the same table

Before: APIv4 always used the name of the table when adding an implicit join, meaning it could only be added once. After: Creates a unique alias for each instance of an implicit join, allowing the same table to be joined multiple times. Fixes dev/report#73
civicrm · Aug 10, 2021 · 93163f7 · 93163f7
1 parent a424ae9
commit 93163f7
Show file tree

Hide file tree

Showing 4 changed files with 140 additions and 104 deletions.
diff --git a/Civi/Api4/Query/Api4SelectQuery.php b/Civi/Api4/Query/Api4SelectQuery.php
@@ -46,6 +46,18 @@ class Api4SelectQuery {
    */
   protected $joins = [];
 
+  /**
+   * Used to keep track of implicit join table aliases
+   * @var array
+   */
+  protected $joinTree = [];
+
+  /**
+   * Used to create a unique table alias for each implicit join
+   * @var int
+   */
+  protected $autoJoinSuffix = 0;
+
   /**
    * @var array[]
    */
@@ -598,7 +610,7 @@ public function getAclClause($tableAlias, $baoName, $stack = []) {
     // Prevent (most) redundant acl sub clauses if they have already been applied to the main entity.
     // FIXME: Currently this only works 1 level deep, but tracking through multiple joins would increase complexity
     // and just doing it for the first join takes care of most acl clause deduping.
-    if (count($stack) === 1 && in_array($stack[0], $this->aclFields, TRUE)) {
+    if (count($stack) === 1 && in_array(reset($stack), $this->aclFields, TRUE)) {
       return [];
     }
     $clauses = $baoName::getSelectWhereClause($tableAlias);
@@ -970,49 +982,96 @@ private function getBridgeJoinConditions(array &$joinTree, $baseRef, string $ali
    * Joins a path and adds all fields in the joined entity to apiFieldSpec
    *
    * @param $key
-   * @throws \API_Exception
-   * @throws \Exception
    */
   protected function autoJoinFK($key) {
     if (isset($this->apiFieldSpec[$key])) {
       return;
     }
-
-    $pathArray = explode('.', $key);
-
     /** @var \Civi\Api4\Service\Schema\Joiner $joiner */
     $joiner = \Civi::container()->get('joiner');
+
+    $pathArray = explode('.', $key);
     // The last item in the path is the field name. We don't care about that; we'll add all fields from the joined entity.
     array_pop($pathArray);
 
+    $baseTableAlias = $this::MAIN_TABLE_ALIAS;
+
+    // If the first item is the name of an explicit join, use it as the base & shift it off the path
+    $explicitJoin = $this->getExplicitJoin($pathArray[0]);
+    if ($explicitJoin) {
+      $baseTableAlias = array_shift($pathArray);
+    }
+
+    // Ensure joinTree array contains base table
+    $this->joinTree[$baseTableAlias]['#table_alias'] = $baseTableAlias;
+    $this->joinTree[$baseTableAlias]['#path'] = $explicitJoin ? $baseTableAlias . '.' : '';
+    // During iteration this variable will refer to the current position in the tree
+    $joinTreeNode =& $this->joinTree[$baseTableAlias];
+
     try {
-      $joinPath = $joiner->autoJoin($this, $pathArray);
+      $joinPath = $joiner->getPath($explicitJoin['table'] ?? $this->getFrom(), $pathArray);
     }
     catch (\Exception $e) {
+      // Because the select clause silently ignores unknown fields, this function shouldn't throw exceptions
       return;
     }
-    $lastLink = array_pop($joinPath);
-    $previousLink = array_pop($joinPath);
 
-    // Custom field names are already prefixed
-    $isCustom = $lastLink instanceof CustomGroupJoinable;
-    if ($isCustom) {
-      array_pop($pathArray);
-    }
-    $prefix = $pathArray ? implode('.', $pathArray) . '.' : '';
-    // Cache field info for retrieval by $this->getField()
-    foreach ($lastLink->getEntityFields() as $fieldObject) {
-      $fieldArray = $fieldObject->toArray();
-      // Set sql name of field, using column name for real joins
-      if (!$lastLink->getSerialize()) {
-        $fieldArray['sql_name'] = '`' . $lastLink->getAlias() . '`.`' . $fieldArray['column_name'] . '`';
-      }
-      // For virtual joins on serialized fields, the callback function will need the sql name of the serialized field
-      // @see self::renderSerializedJoin()
-      else {
-        $fieldArray['sql_name'] = '`' . $previousLink->getAlias() . '`.`' . $lastLink->getBaseColumn() . '`';
+    foreach ($joinPath as $joinName => $link) {
+      if (!isset($joinTreeNode[$joinName])) {
+        $target = $link->getTargetTable();
+        $tableAlias = $link->getAlias() . '_' . ++$this->autoJoinSuffix;
+        $isCustom = $link instanceof CustomGroupJoinable;
+
+        $joinTreeNode[$joinName] = [
+          '#table_alias' => $tableAlias,
+          '#path' => $joinTreeNode['#path'] . $joinName . '.',
+        ];
+        $joinEntity = CoreUtil::getApiNameFromTableName($target);
+
+        if ($joinEntity && !$this->checkEntityAccess($joinEntity)) {
+          throw new UnauthorizedException('Cannot join to ' . $joinEntity);
+        }
+        if ($this->getCheckPermissions() && $isCustom) {
+          // Check access to custom group
+          $groupId = \CRM_Core_DAO::getFieldValue('CRM_Core_DAO_CustomGroup', $link->getTargetTable(), 'id', 'table_name');
+          if (!\CRM_Core_BAO_CustomGroup::checkGroupAccess($groupId, \CRM_Core_Permission::VIEW)) {
+            return;
+          }
+        }
+        if ($link->isDeprecated()) {
+          \CRM_Core_Error::deprecatedWarning("Deprecated join alias '$tableAlias' used in APIv4 get. Should be changed to '{$tableAlias}_id'");
+        }
+        $virtualField = $link->getSerialize();
+
+        // Cache field info for retrieval by $this->getField()
+        foreach ($link->getEntityFields() as $fieldObject) {
+          $fieldArray = $fieldObject->toArray();
+          // Set sql name of field, using column name for real joins
+          if (!$virtualField) {
+            $fieldArray['sql_name'] = '`' . $tableAlias . '`.`' . $fieldArray['column_name'] . '`';
+          }
+          // For virtual joins on serialized fields, the callback function will need the sql name of the serialized field
+          // @see self::renderSerializedJoin()
+          else {
+            $fieldArray['sql_name'] = '`' . $joinTreeNode['#table_alias'] . '`.`' . $link->getBaseColumn() . '`';
+          }
+          // Custom fields will already have the group name prefixed
+          $fieldName = $isCustom ? explode('.', $fieldArray['name'])[1] : $fieldArray['name'];
+          $this->addSpecField($joinTreeNode[$joinName]['#path'] . $fieldName, $fieldArray);
+        }
+
+        // Serialized joins are rendered by this::renderSerializedJoin. Don't add their tables.
+        if (!$virtualField) {
+          $bao = $joinEntity ? CoreUtil::getBAOFromApiName($joinEntity) : NULL;
+          $conditions = $link->getConditionsForJoin($joinTreeNode['#table_alias'], $tableAlias);
+          if ($bao) {
+            $conditions = array_merge($conditions, $this->getAclClause($tableAlias, $bao, $joinPath));
+          }
+          $this->join('LEFT', $target, $tableAlias, $conditions);
+        }
+
       }
-      $this->addSpecField($prefix . $fieldArray['name'], $fieldArray);
+      $joinTreeNode =& $joinTreeNode[$joinName];
     }
   }
 

diff --git a/Civi/Api4/Service/Schema/Joinable/Joinable.php b/Civi/Api4/Service/Schema/Joinable/Joinable.php
@@ -97,17 +97,17 @@ public function __construct($targetTable, $targetColumn, $alias = NULL) {
   /**
    * Gets conditions required when joining to a base table
    *
-   * @param string|null $baseTableAlias
-   *   Name of the base table, if aliased.
+   * @param string $baseTableAlias
+   * @param string $tableAlias
    *
    * @return array
    */
-  public function getConditionsForJoin($baseTableAlias = NULL) {
+  public function getConditionsForJoin(string $baseTableAlias, string $tableAlias) {
     $baseCondition = sprintf(
       '%s.%s =  %s.%s',
-      $baseTableAlias ?: $this->baseTable,
+      $baseTableAlias,
       $this->baseColumn,
-      $this->getAlias(),
+      $tableAlias,
       $this->targetColumn
     );
 

diff --git a/Civi/Api4/Service/Schema/Joiner.php b/Civi/Api4/Service/Schema/Joiner.php
@@ -12,11 +12,6 @@
 
 namespace Civi\Api4\Service\Schema;
 
-use Civi\API\Exception\UnauthorizedException;
-use Civi\Api4\Query\Api4SelectQuery;
-use Civi\Api4\Service\Schema\Joinable\CustomGroupJoinable;
-use Civi\Api4\Utils\CoreUtil;
-
 class Joiner {
   /**
    * @var SchemaMap
@@ -36,79 +31,15 @@ public function __construct(SchemaMap $schemaMap) {
   }
 
   /**
-   * @param \Civi\Api4\Query\Api4SelectQuery $query
-   *   The query object to do the joins on
-   * @param array $joinPath
-   *   A list of aliases, e.g. [contact, phone]
-   * @param string $side
-   *   Can be LEFT or INNER
+   * Get the path used to create an implicit join
    *
-   * @throws \Exception
-   * @return \Civi\Api4\Service\Schema\Joinable\Joinable[]
-   *   The path used to make the join
-   */
-  public function autoJoin(Api4SelectQuery $query, array $joinPath, $side = 'LEFT') {
-    $explicitJoin = $query->getExplicitJoin($joinPath[0]);
-
-    // If the first item is the name of an explicit join, use it as the base & shift it off the path
-    if ($explicitJoin) {
-      $from = $explicitJoin['table'];
-      $baseTableAlias = array_shift($joinPath);
-    }
-    // Otherwise use the api entity as the base
-    else {
-      $from = $query->getFrom();
-      $baseTableAlias = $query::MAIN_TABLE_ALIAS;
-    }
-
-    $fullPath = $this->getPath($from, $joinPath);
-
-    foreach ($fullPath as $link) {
-      $target = $link->getTargetTable();
-      $alias = $link->getAlias();
-      $joinEntity = CoreUtil::getApiNameFromTableName($target);
-
-      if ($joinEntity && !$query->checkEntityAccess($joinEntity)) {
-        throw new UnauthorizedException('Cannot join to ' . $joinEntity);
-      }
-      if ($query->getCheckPermissions() && is_a($link, CustomGroupJoinable::class)) {
-        // Check access to custom group
-        $groupId = \CRM_Core_DAO::getFieldValue('CRM_Core_DAO_CustomGroup', $link->getTargetTable(), 'id', 'table_name');
-        if (!\CRM_Core_BAO_CustomGroup::checkGroupAccess($groupId, \CRM_Core_Permission::VIEW)) {
-          throw new UnauthorizedException('Cannot join to ' . $link->getAlias());
-        }
-      }
-      if ($link->isDeprecated()) {
-        \CRM_Core_Error::deprecatedWarning("Deprecated join alias '$alias' used in APIv4 get. Should be changed to '{$alias}_id'");
-      }
-      // Serialized joins are rendered by Api4SelectQuery::renderSerializedJoin
-      if ($link->getSerialize()) {
-        // Virtual join, don't actually add this table
-        break;
-      }
-
-      $bao = $joinEntity ? CoreUtil::getBAOFromApiName($joinEntity) : NULL;
-      $conditions = $link->getConditionsForJoin($baseTableAlias);
-      if ($bao) {
-        $conditions = array_merge($conditions, $query->getAclClause($alias, $bao, $joinPath));
-      }
-
-      $query->join($side, $target, $alias, $conditions);
-
-      $baseTableAlias = $link->getAlias();
-    }
-
-    return $fullPath;
-  }
-
-  /**
    * @param string $baseTable
    * @param array $joinPath
    *
    * @return \Civi\Api4\Service\Schema\Joinable\Joinable[]
    * @throws \API_Exception
    */
-  protected function getPath(string $baseTable, array $joinPath) {
+  public function getPath(string $baseTable, array $joinPath) {
     $cacheKey = sprintf('%s.%s', $baseTable, implode('.', $joinPath));
     if (!isset($this->cache[$cacheKey])) {
       $fullPath = [];
@@ -120,7 +51,7 @@ protected function getPath(string $baseTable, array $joinPath) {
           throw new \API_Exception(sprintf('Cannot join %s to %s', $baseTable, $targetAlias));
         }
         else {
-          $fullPath[] = $link;
+          $fullPath[$targetAlias] = $link;
           $baseTable = $link->getTargetTable();
         }
       }

diff --git a/tests/phpunit/api/v4/Action/BasicCustomFieldTest.php b/tests/phpunit/api/v4/Action/BasicCustomFieldTest.php
@@ -277,6 +277,52 @@ public function testRelationshipCacheCustomFields() {
     $this->assertEquals('Buddy', $results[0]["$cgName.PetName"]);
   }
 
+  public function testMultipleJoinsToCustomTable() {
+    $cgName = uniqid('My');
+
+    CustomGroup::create(FALSE)
+      ->addValue('name', $cgName)
+      ->addValue('extends', 'Contact')
+      ->addChain('field1', CustomField::create()
+        ->addValue('label', 'FavColor')
+        ->addValue('custom_group_id', '$id')
+        ->addValue('html_type', 'Text')
+        ->addValue('data_type', 'String'))
+      ->execute();
+
+    $parent = Contact::create(FALSE)
+      ->addValue('first_name', 'Parent')
+      ->addValue('last_name', 'Tester')
+      ->addValue("$cgName.FavColor", 'Purple')
+      ->execute()
+      ->first()['id'];
+
+    $child = Contact::create(FALSE)
+      ->addValue('first_name', 'Child')
+      ->addValue('last_name', 'Tester')
+      ->addValue("$cgName.FavColor", 'Cyan')
+      ->execute()
+      ->first()['id'];
+
+    Relationship::create(FALSE)
+      ->addValue('contact_id_a', $parent)
+      ->addValue('contact_id_b', $child)
+      ->addValue('relationship_type_id', 1)
+      ->execute();
+
+    $results = Contact::get(FALSE)
+      ->addSelect('first_name', 'child.first_name', "$cgName.FavColor", "child.$cgName.FavColor")
+      ->addWhere('id', '=', $parent)
+      ->addJoin('Contact AS child', 'INNER', 'RelationshipCache', ['id', '=', 'child.far_contact_id'])
+      ->execute();
+
+    $this->assertCount(1, $results);
+    $this->assertEquals('Parent', $results[0]['first_name']);
+    $this->assertEquals('Child', $results[0]['child.first_name']);
+    $this->assertEquals('Purple', $results[0]["$cgName.FavColor"]);
+    $this->assertEquals('Cyan', $results[0]["child.$cgName.FavColor"]);
+  }
+
   /**
    * Some types are creating a dummy option group even if we don't have
    * any option values.