Merge pull request #19825 from colemanw/api4BridgeJoinSubquery

APIv4 - Use subquery to LEFT JOIN via a bridge entity
civicrm · Mar 21, 2021 · 5271fc9 · 5271fc9
2 parents b5aefc2 + 266e8de
commit 5271fc9
Show file tree

Hide file tree

Showing 7 changed files with 192 additions and 56 deletions.
diff --git a/Civi/Api4/Generic/DAOGetAction.php b/Civi/Api4/Generic/DAOGetAction.php
@@ -164,16 +164,16 @@ public function addHaving(string $expr, string $op, $value = NULL) {
 
   /**
    * @param string $entity
-   * @param bool $required
+   * @param string|bool $type
    * @param string $bridge
    * @param array ...$conditions
    * @return DAOGetAction
    */
-  public function addJoin(string $entity, bool $required = FALSE, $bridge = NULL, ...$conditions): DAOGetAction {
+  public function addJoin(string $entity, $type = 'LEFT', $bridge = NULL, ...$conditions): DAOGetAction {
     if ($bridge) {
       array_unshift($conditions, $bridge);
     }
-    array_unshift($conditions, $entity, $required);
+    array_unshift($conditions, $entity, $type);
     $this->join[] = $conditions;
     return $this;
   }

diff --git a/Civi/Api4/Query/Api4SelectQuery.php b/Civi/Api4/Query/Api4SelectQuery.php
@@ -595,7 +595,18 @@ private function addExplicitJoins() {
       $alias = $alias ? \CRM_Utils_String::munge($alias, '_', 256) : strtolower($entity);
       // First item in the array is a boolean indicating if the join is required (aka INNER or LEFT).
       // The rest are join conditions.
-      $side = array_shift($join) ? 'INNER' : 'LEFT';
+      $side = array_shift($join);
+      // If omitted, supply default (LEFT); and legacy support for boolean values
+      if (!is_string($side)) {
+        $side = $side ? 'INNER' : 'LEFT';
+      }
+      if (!in_array($side, ['INNER', 'LEFT', 'EXCLUDE'])) {
+        throw new \API_Exception("Illegal value for join side: '$side'.");
+      }
+      if ($side === 'EXCLUDE') {
+        $side = 'LEFT';
+        $this->api->addWhere("$alias.id", 'IS NULL');
+      }
       // Add all fields from joined entity to spec
       $joinEntityGet = \Civi\API\Request::create($entity, 'get', ['version' => 4, 'checkPermissions' => $this->getCheckPermissions()]);
       $joinEntityFields = $joinEntityGet->entityFields();
@@ -613,15 +624,15 @@ private function addExplicitJoins() {
       // If the first condition is a string, it's the name of a bridge entity
       if (!empty($join[0]) && is_string($join[0]) && \CRM_Utils_Rule::alphanumeric($join[0])) {
         $this->explicitJoins[$alias]['bridge'] = $join[0];
-        $conditions = $this->getBridgeJoin($join, $entity, $alias);
+        $this->addBridgeJoin($join, $entity, $alias, $side);
       }
       else {
         $conditions = $this->getJoinConditions($join, $entity, $alias, $joinEntityFields);
+        foreach (array_filter($join) as $clause) {
+          $conditions[] = $this->treeWalkClauses($clause, 'ON');
+        }
+        $this->join($side, $tableName, $alias, $conditions);
       }
-      foreach (array_filter($join) as $clause) {
-        $conditions[] = $this->treeWalkClauses($clause, 'ON');
-      }
-      $this->join($side, $tableName, $alias, $conditions);
     }
   }
 
@@ -680,17 +691,67 @@ private function getJoinConditions($joinTree, $joinEntity, $alias, $joinEntityFi
    *
    * This creates a double-join in sql that appears to the API user like a single join.
    *
+   * LEFT joins use a subquery so that the bridge + joined-entity can be treated like a single table.
+   *
    * @param array $joinTree
    * @param string $joinEntity
    * @param string $alias
-   * @return array
+   * @param string $side
    * @throws \API_Exception
    */
-  protected function getBridgeJoin(&$joinTree, $joinEntity, $alias) {
+  protected function addBridgeJoin($joinTree, $joinEntity, $alias, $side) {
     $bridgeEntity = array_shift($joinTree);
+
+    // INNER joins require unique aliases, whereas left joins will be inside a subquery and short aliases are more readable
+    $bridgeAlias = $side === 'INNER' ? $alias . '_via_' . strtolower($bridgeEntity) : 'b';
+    $joinAlias = $side === 'INNER' ? $alias : 'c';
+
+    $joinTable = CoreUtil::getTableName($joinEntity);
+    [$bridgeTable, $baseRef, $joinRef] = $this->getBridgeRefs($bridgeEntity, $joinEntity);
+
+    $bridgeFields = $this->registerBridgeJoinFields($bridgeEntity, $joinRef, $baseRef, $alias, $bridgeAlias, $side);
+
+    $linkConditions = $this->getBridgeLinkConditions($bridgeAlias, $joinAlias, $joinTable, $joinRef);
+
+    $bridgeConditions = $this->getBridgeJoinConditions($joinTree, $baseRef, $alias, $bridgeAlias, $bridgeEntity, $side);
+
+    $acls = array_values($this->getAclClause($joinAlias, CoreUtil::getBAOFromApiName($joinEntity), [NULL, NULL]));
+
+    $joinConditions = [];
+    foreach (array_filter($joinTree) as $clause) {
+      $joinConditions[] = $this->treeWalkClauses($clause, 'ON');
+    }
+
+    // INNER joins are done with 2 joins
+    if ($side === 'INNER') {
+      $this->join('INNER', $bridgeTable, $bridgeAlias, $bridgeConditions);
+      $this->join('INNER', $joinTable, $alias, array_merge($linkConditions, $acls, $joinConditions));
+    }
+    // For LEFT joins, construct a subquery to link the bridge & join tables as one
+    else {
+      $joinEntityClass = '\Civi\Api4\\' . $joinEntity;
+      foreach ($joinEntityClass::get($this->getCheckPermissions())->entityFields() as $name => $field) {
+        $bridgeFields[$field['column_name']] = '`' . $joinAlias . '`.`' . $field['column_name'] . '`';
+      }
+      $select = implode(',', $bridgeFields);
+      $joinConditions = array_merge($joinConditions, $bridgeConditions);
+      $innerConditions = array_merge($linkConditions, $acls);
+      $subquery = "SELECT $select FROM `$bridgeTable` `$bridgeAlias`, `$joinTable` `$joinAlias` WHERE " . implode(' AND ', $innerConditions);
+      $this->query->join($alias, "$side JOIN ($subquery) `$alias` ON " . implode(' AND ', $joinConditions));
+    }
+  }
+
+  /**
+   * Get the table name and 2 reference columns from a bridge entity
+   *
+   * @param string $bridgeEntity
+   * @param string $joinEntity
+   * @return array
+   * @throws \API_Exception
+   */
+  private function getBridgeRefs(string $bridgeEntity, string $joinEntity): array {
     /* @var \Civi\Api4\Generic\DAOEntity $bridgeEntityClass */
     $bridgeEntityClass = '\Civi\Api4\\' . $bridgeEntity;
-    $bridgeAlias = $alias . '_via_' . strtolower($bridgeEntity);
     $bridgeInfo = $bridgeEntityClass::getInfo();
     $bridgeFields = $bridgeInfo['bridge'] ?? [];
     // Sanity check - bridge entity should declare exactly 2 FK fields
@@ -701,8 +762,6 @@ protected function getBridgeJoin(&$joinTree, $joinEntity, $alias) {
     $bridgeDAO = $bridgeInfo['dao'];
     $bridgeTable = $bridgeDAO::getTableName();
 
-    $joinTable = CoreUtil::getTableName($joinEntity);
-    $bridgeEntityGet = $bridgeEntityClass::get($this->getCheckPermissions());
     // Get the 2 bridge reference columns as CRM_Core_Reference_* objects
     $joinRef = $baseRef = NULL;
     foreach ($bridgeDAO::getReferenceColumns() as $ref) {
@@ -718,29 +777,74 @@ protected function getBridgeJoin(&$joinTree, $joinEntity, $alias) {
     if (!$joinRef || !$baseRef) {
       throw new \API_Exception("Unable to join $bridgeEntity to $joinEntity");
     }
-    // Create link between bridge entity and join entity
-    $joinConditions = [
-      "`$bridgeAlias`.`{$joinRef->getReferenceKey()}` = `$alias`.`{$joinRef->getTargetKey()}`",
+    return [$bridgeTable, $baseRef, $joinRef];
+  }
+
+  /**
+   * Get the clause to link bridge entity with join entity
+   *
+   * @param string $bridgeAlias
+   * @param string $joinAlias
+   * @param string $joinTable
+   * @param $joinRef
+   * @return array
+   */
+  private function getBridgeLinkConditions(string $bridgeAlias, string $joinAlias, string $joinTable, $joinRef): array {
+    $linkConditions = [
+      "`$bridgeAlias`.`{$joinRef->getReferenceKey()}` = `$joinAlias`.`{$joinRef->getTargetKey()}`",
     ];
     // For dynamic references, also add the type column (e.g. `entity_table`)
     if ($joinRef->getTypeColumn()) {
-      $joinConditions[] = "`$bridgeAlias`.`{$joinRef->getTypeColumn()}` = '$joinTable'";
+      $linkConditions[] = "`$bridgeAlias`.`{$joinRef->getTypeColumn()}` = '$joinTable'";
     }
-    // Register fields (other than bridge FK fields) from the bridge entity as if they belong to the join entity
+    return $linkConditions;
+  }
+
+  /**
+   * Register fields (other than bridge FK fields) from the bridge entity as if they belong to the join entity
+   *
+   * @param $bridgeEntity
+   * @param $joinRef
+   * @param $baseRef
+   * @param string $alias
+   * @param string $bridgeAlias
+   * @param string $side
+   * @return array
+   */
+  private function registerBridgeJoinFields($bridgeEntity, $joinRef, $baseRef, string $alias, string $bridgeAlias, string $side): array {
     $fakeFields = [];
-    foreach ($bridgeEntityGet->entityFields() as $name => $field) {
-      if ($name === 'id' || $name === $joinRef->getReferenceKey() || $name === $joinRef->getTypeColumn() || $name === $baseRef->getReferenceKey() || $name === $baseRef->getTypeColumn()) {
+    $bridgeFkFields = [$joinRef->getReferenceKey(), $joinRef->getTypeColumn(), $baseRef->getReferenceKey(), $baseRef->getTypeColumn()];
+    $bridgeEntityClass = '\Civi\Api4\\' . $bridgeEntity;
+    foreach ($bridgeEntityClass::get($this->getCheckPermissions())->entityFields() as $name => $field) {
+      if ($name === 'id' || ($side === 'INNER' && in_array($name, $bridgeFkFields, TRUE))) {
         continue;
       }
-      // Note these fields get a sql alias pointing to the bridge entity, but an api alias pretending they belong to the join entity
-      $field['sql_name'] = '`' . $bridgeAlias . '`.`' . $field['column_name'] . '`';
-      $this->addSpecField($alias . '.' . $field['name'], $field);
-      $fakeFields[] = $alias . '.' . $field['name'];
+      // For INNER joins, these fields get a sql alias pointing to the bridge entity,
+      // but an api alias pretending they belong to the join entity.
+      $field['sql_name'] = '`' . ($side === 'LEFT' ? $alias : $bridgeAlias) . '`.`' . $field['column_name'] . '`';
+      $this->addSpecField($alias . '.' . $name, $field);
+      $fakeFields[$field['column_name']] = '`' . $bridgeAlias . '`.`' . $field['column_name'] . '`';
     }
-    // Move conditions for the bridge join out of the joinTree
+    return $fakeFields;
+  }
+
+  /**
+   * Extract bridge join conditions from the joinTree if any, else supply default conditions for join to base entity
+   *
+   * @param array $joinTree
+   * @param $baseRef
+   * @param string $alias
+   * @param string $bridgeAlias
+   * @param string $bridgeEntity
+   * @param string $side
+   * @return string[]
+   * @throws \API_Exception
+   */
+  private function getBridgeJoinConditions(array &$joinTree, $baseRef, string $alias, string $bridgeAlias, string $bridgeEntity, string $side): array {
     $bridgeConditions = [];
-    $isExplicit = FALSE;
-    $joinTree = array_filter($joinTree, function($clause) use ($baseRef, $alias, $bridgeAlias, $fakeFields, &$bridgeConditions, &$isExplicit) {
+    $bridgeAlias = $side === 'INNER' ? $bridgeAlias : $alias;
+    // Find explicit bridge join conditions and move them out of the joinTree
+    $joinTree = array_filter($joinTree, function ($clause) use ($baseRef, $alias, $bridgeAlias, &$bridgeConditions) {
       list($sideA, $op, $sideB) = array_pad((array) $clause, 3, NULL);
       // Skip AND/OR/NOT branches
       if (!$sideB) {
@@ -750,27 +854,18 @@ protected function getBridgeJoin(&$joinTree, $joinEntity, $alias) {
       if ($op === '=' && $sideB && ($sideA === "$alias.{$baseRef->getReferenceKey()}" || $sideB === "$alias.{$baseRef->getReferenceKey()}")) {
         $expr = $sideA === "$alias.{$baseRef->getReferenceKey()}" ? $sideB : $sideA;
         $bridgeConditions[] = "`$bridgeAlias`.`{$baseRef->getReferenceKey()}` = " . $this->getExpression($expr)->render($this->apiFieldSpec);
-        $isExplicit = TRUE;
         return FALSE;
       }
       // Explicit link with dynamic "entity_table" column
       elseif ($op === '=' && $baseRef->getTypeColumn() && ($sideA === "$alias.{$baseRef->getTypeColumn()}" || $sideB === "$alias.{$baseRef->getTypeColumn()}")) {
         $expr = $sideA === "$alias.{$baseRef->getTypeColumn()}" ? $sideB : $sideA;
         $bridgeConditions[] = "`$bridgeAlias`.`{$baseRef->getTypeColumn()}` = " . $this->getExpression($expr)->render($this->apiFieldSpec);
-        $isExplicit = TRUE;
         return FALSE;
       }
-      // Other conditions that apply only to the bridge table should be
-      foreach ([$sideA, $sideB] as $expr) {
-        if (is_string($expr) && in_array(explode(':', $expr)[0], $fakeFields)) {
-          $bridgeConditions[] = $this->composeClause($clause, 'ON');
-          return FALSE;
-        }
-      }
       return TRUE;
     });
     // If no bridge conditions were specified, link it to the base entity
-    if (!$isExplicit) {
+    if (!$bridgeConditions) {
       if (!in_array($this->getEntity(), $baseRef->getTargetEntities())) {
         throw new \API_Exception("Unable to join $bridgeEntity to " . $this->getEntity());
       }
@@ -779,12 +874,7 @@ protected function getBridgeJoin(&$joinTree, $joinEntity, $alias) {
         $bridgeConditions[] = "`$bridgeAlias`.`{$baseRef->getTypeColumn()}` = '" . $this->getFrom() . "'";
       }
     }
-
-    $this->join('LEFT', $bridgeTable, $bridgeAlias, $bridgeConditions);
-
-    $baoName = CoreUtil::getBAOFromApiName($joinEntity);
-    $acls = array_values($this->getAclClause($alias, $baoName, [NULL, NULL]));
-    return array_merge($acls, $joinConditions);
+    return $bridgeConditions;
   }
 
   /**

diff --git a/ang/api4Explorer/Explorer.js b/ang/api4Explorer/Explorer.js
@@ -56,7 +56,7 @@
     $scope.loading = false;
     $scope.controls = {};
     $scope.langs = ['php', 'js', 'ang', 'cli'];
-    $scope.joinTypes = [{k: false, v: 'FALSE (LEFT JOIN)'}, {k: true, v: 'TRUE (INNER JOIN)'}];
+    $scope.joinTypes = [{k: 'LEFT', v: 'LEFT JOIN'}, {k: 'INNER', v: 'INNER JOIN'}, {k: 'EXCLUDE', v: 'EXCLUDE'}];
     $scope.bridgeEntities = _.filter(schema, function(entity) {return _.includes(entity.type, 'EntityBridge');});
     $scope.code = {
       php: [
@@ -529,7 +529,7 @@
               $timeout(function() {
                 if (field) {
                   if (name === 'join') {
-                    $scope.params[name].push([field + ' AS ' + _.snakeCase(field), false]);
+                    $scope.params[name].push([field + ' AS ' + _.snakeCase(field), 'LEFT']);
                     ctrl.buildFieldList();
                   }
                   else if (typeof objectParams[name] === 'undefined') {

diff --git a/ext/search/CRM/Search/Upgrader.php b/ext/search/CRM/Search/Upgrader.php
@@ -110,4 +110,27 @@ public function upgrade_1002() {
     return TRUE;
   }
 
+  /**
+   * Upgrade 1003 - update APIv4 join syntax in saved searches
+   * @return bool
+   */
+  public function upgrade_1003() {
+    $this->ctx->log->info('Applying 1003 - update APIv4 join syntax in saved searches.');
+    $savedSearches = \Civi\Api4\SavedSearch::get(FALSE)
+      ->addSelect('id', 'api_params')
+      ->addWhere('api_params', 'IS NOT NULL')
+      ->execute();
+    foreach ($savedSearches as $savedSearch) {
+      foreach ($savedSearch['api_params']['join'] ?? [] as $i => $join) {
+        $savedSearch['api_params']['join'][$i][1] = empty($join[1]) ? 'LEFT' : 'INNER';
+      }
+      if (!empty($savedSearch['api_params']['join'])) {
+        \Civi\Api4\SavedSearch::update(FALSE)
+          ->setValues($savedSearch)
+          ->execute();
+      }
+    }
+    return TRUE;
+  }
+
 }
diff --git a/ext/search/ang/crmSearchAdmin/compose/criteria.html b/ext/search/ang/crmSearchAdmin/compose/criteria.html
@@ -3,9 +3,8 @@
     <div ng-if=":: $ctrl.paramExists('join')">
       <fieldset ng-repeat="join in $ctrl.savedSearch.api_params.join">
         <div class="form-inline">
-          <label for="crm-search-join-{{ $index }}">{{:: ts('With') }}</label>
+          <select class="form-control" ng-model="join[1]" ng-change="$ctrl.changeJoinType(join)" ng-options="o.k as o.v for o in ::joinTypes" ></select>
           <input id="crm-search-join-{{ $index }}" class="form-control huge" ng-model="join[0]" crm-ui-select="{placeholder: ' ', data: getJoinEntities}" disabled >
-          <select class="form-control" ng-model="join[1]" ng-options="o.k as o.v for o in ::joinTypes" ></select>
           <button type="button" class="btn btn-xs btn-danger-outline" ng-click="$ctrl.removeJoin($index)" title="{{:: ts('Remove join') }}">
             <i class="crm-i fa-trash" aria-hidden="true"></i>
           </button>
@@ -16,7 +15,8 @@
       </fieldset>
       <fieldset>
         <div class="form-inline">
-          <input id="crm-search-add-join" class="form-control crm-action-menu fa-plus huge" ng-model="controls.join" crm-ui-select="{placeholder: ts('With'), data: getJoinEntities}" ng-change="addJoin()"/>
+          <select class="form-control" ng-model="controls.joinType" ng-options="o.k as o.v for o in ::joinTypes" ></select>
+          <input id="crm-search-add-join" class="form-control crm-action-menu fa-plus huge" ng-model="controls.join" crm-ui-select="{placeholder: ts('Entity'), data: getJoinEntities}" ng-change="addJoin()"/>
         </div>
       </fieldset>
     </div>