Merge pull request #16889 from colemanw/api4ref

Make api4 select query object more sane
civicrm · Mar 29, 2020 · f932111 · f932111
2 parents 115ab1c + a689294
commit f932111
Show file tree

Hide file tree

Showing 10 changed files with 158 additions and 204 deletions.
diff --git a/Civi/Api4/Event/Subscriber/PostSelectQuerySubscriber.php b/Civi/Api4/Event/Subscriber/PostSelectQuerySubscriber.php
@@ -210,7 +210,7 @@ private function formatSelects($alias, $selects, Api4SelectQuery $query) {
     $selectFields = [];
 
     foreach ($selects as $select) {
-      $selectAlias = $query->getFkSelectAliases()[$select];
+      $selectAlias = str_replace('`', '', $query->getField($select)['sql_name']);
       $fieldAlias = substr($select, strrpos($select, '.') + 1);
       $selectFields[$fieldAlias] = $selectAlias;
     }

diff --git a/Civi/Api4/Query/Api4SelectQuery.php b/Civi/Api4/Query/Api4SelectQuery.php
@@ -77,6 +77,9 @@ public function __construct($apiGet) {
     $baoName = CoreUtil::getBAOFromApiName($this->entity);
     $this->entityFieldNames = array_column($baoName::fields(), 'name');
     $this->apiFieldSpec = $apiGet->entityFields();
+    foreach ($this->apiFieldSpec as $key => $field) {
+      $this->apiFieldSpec[$key]['sql_name'] = '`' . self::MAIN_TABLE_ALIAS . '`.`' . $field['column_name'] . '`';
+    }
 
     $this->constructQueryObject($baoName);
 
@@ -93,26 +96,10 @@ public function __construct($apiGet) {
    * @throws \Civi\API\Exception\UnauthorizedException
    */
   public function getSql() {
-    $this->addJoins();
-    $this->buildSelectFields();
+    $this->buildSelectClause();
     $this->buildWhereClause();
-
-    // Select
-    if (in_array('row_count', $this->select)) {
-      $this->query->select("count(*) as c");
-    }
-    else {
-      foreach ($this->selectFields as $column => $alias) {
-        $this->query->select("$column as `$alias`");
-      }
-      // Order by
-      $this->buildOrderBy();
-    }
-
-    // Limit
-    if (!empty($this->limit) || !empty($this->offset)) {
-      $this->query->limit($this->limit, $this->offset);
-    }
+    $this->buildOrderBy();
+    $this->buildLimit();
     return $this->query->toSQL();
   }
 
@@ -135,10 +122,12 @@ public function run() {
         break;
       }
       $results[$query->id] = [];
-      foreach ($this->selectFields as $column => $alias) {
+      foreach ($this->select as $alias) {
         $returnName = $alias;
-        $alias = str_replace('.', '_', $alias);
-        $results[$query->id][$returnName] = property_exists($query, $alias) ? $query->$alias : NULL;
+        if ($this->isOneToOneField($alias)) {
+          $alias = str_replace('.', '_', $alias);
+          $results[$query->id][$returnName] = property_exists($query, $alias) ? $query->$alias : NULL;
+        }
       };
     }
     $event = new PostSelectQueryEvent($results, $this);
@@ -147,59 +136,44 @@ public function run() {
     return $event->getResults();
   }
 
-  /**
-   * Gets all FK fields and does the required joins
-   */
-  protected function addJoins() {
-    $allFields = array_merge($this->select, array_keys($this->orderBy));
-    $recurse = function($clauses) use (&$allFields, &$recurse) {
-      foreach ($clauses as $clause) {
-        if ($clause[0] === 'NOT' && is_string($clause[1][0])) {
-          $recurse($clause[1][1]);
-        }
-        elseif (in_array($clause[0], ['AND', 'OR', 'NOT'])) {
-          $recurse($clause[1]);
-        }
-        elseif (is_array($clause[0])) {
-          array_walk($clause, $recurse);
-        }
-        else {
-          $allFields[] = $clause[0];
-        }
-      }
-    };
-    $recurse($this->where);
-    $dotFields = array_unique(array_filter($allFields, function ($field) {
-      return strpos($field, '.') !== FALSE;
-    }));
-
-    foreach ($dotFields as $dotField) {
-      $this->joinFK($dotField);
+  protected function buildSelectClause() {
+    if (empty($this->select)) {
+      $this->select = $this->entityFieldNames;
     }
-  }
-
-  /**
-   * Populate $this->selectFields
-   *
-   * @throws \Civi\API\Exception\UnauthorizedException
-   */
-  protected function buildSelectFields() {
-    $selectAll = (empty($this->select) || in_array('*', $this->select));
-    $select = $selectAll ? $this->entityFieldNames : $this->select;
-
-    // Always select the ID if the table has one.
-    if (array_key_exists('id', $this->apiFieldSpec) || strstr($this->entity, 'Custom_')) {
-      $this->selectFields[self::MAIN_TABLE_ALIAS . ".id"] = "id";
+    elseif (in_array('row_count', $this->select)) {
+      $this->query->select("COUNT(*) AS `c`");
+      return;
     }
-
-    // core return fields
-    foreach ($select as $fieldName) {
+    else {
+      // Always select id field
+      $this->select = array_merge(['id'], $this->select);
+
+      // Expand wildcards in joins (the api wrapper already expanded non-joined wildcards)
+      $wildFields = array_filter($this->select, function($item) {
+        return strpos($item, '*') !== FALSE && strpos($item, '.') !== FALSE;
+      });
+      foreach ($wildFields as $item) {
+        $pos = array_search($item, array_values($this->select));
+        $this->joinFK($item);
+        $matches = SelectUtil::getMatchingFields($item, array_keys($this->apiFieldSpec));
+        array_splice($this->select, $pos, 1, $matches);
+      }
+      $this->select = array_unique($this->select);
+    }
+    foreach ($this->select as $fieldName) {
       $field = $this->getField($fieldName);
-      if (strpos($fieldName, '.') && !empty($this->fkSelectAliases[$fieldName]) && !array_filter($this->getPathJoinTypes($fieldName))) {
-        $this->selectFields[$this->fkSelectAliases[$fieldName]] = $fieldName;
+      if (!$this->isOneToOneField($fieldName)) {
+        continue;
       }
-      elseif ($field && in_array($field['name'], $this->entityFieldNames)) {
-        $this->selectFields[self::MAIN_TABLE_ALIAS . "." . ($field['column_name'] ?? $field['name'])] = $field['name'];
+      elseif ($field) {
+        $this->query->select($field['sql_name'] . " AS `$fieldName`");
+      }
+      // Remove unknown fields without raising an error
+      else {
+        $this->select = array_diff($this->select, [$fieldName]);
+        if (is_array($this->debugOutput)) {
+          $this->debugOutput['undefined_fields'][] = $fieldName;
+        }
       }
     }
   }
@@ -218,16 +192,20 @@ protected function buildWhereClause() {
    * @inheritDoc
    */
   protected function buildOrderBy() {
-    foreach ($this->orderBy as $field => $dir) {
+    foreach ($this->orderBy as $fieldName => $dir) {
       if ($dir !== 'ASC' && $dir !== 'DESC') {
-        throw new \API_Exception("Invalid sort direction. Cannot order by $field $dir");
-      }
-      if ($this->getField($field)) {
-        $this->query->orderBy(self::MAIN_TABLE_ALIAS . '.' . $field . " $dir");
-      }
-      else {
-        throw new \API_Exception("Invalid sort field. Cannot order by $field $dir");
+        throw new \API_Exception("Invalid sort direction. Cannot order by $fieldName $dir");
       }
+      $this->query->orderBy($this->getField($fieldName, TRUE)['sql_name'] . " $dir");
+    }
+  }
+
+  /**
+   * @throws \CRM_Core_Exception
+   */
+  protected function buildLimit() {
+    if (!empty($this->limit) || !empty($this->offset)) {
+      $this->query->limit($this->limit, $this->offset);
     }
   }
 
@@ -279,28 +257,14 @@ protected function treeWalkWhereClause($clause) {
    */
   protected function validateClauseAndComposeSql($clause) {
     // Pad array for unary operators
-    list($key, $operator, $value) = array_pad($clause, 3, NULL);
-    $fieldSpec = $this->getField($key);
-    // derive table and column:
-    $table_name = NULL;
-    $column_name = NULL;
-    if (in_array($key, $this->entityFieldNames)) {
-      $table_name = self::MAIN_TABLE_ALIAS;
-      $column_name = $key;
-    }
-    elseif (strpos($key, '.') && isset($this->fkSelectAliases[$key])) {
-      list($table_name, $column_name) = explode('.', $this->fkSelectAliases[$key]);
-    }
-
-    if (!$table_name || !$column_name) {
-      throw new \API_Exception("Invalid field '$key' in where clause.");
-    }
+    list($fieldName, $operator, $value) = array_pad($clause, 3, NULL);
+    $field = $this->getField($fieldName, TRUE);
 
-    FormattingUtil::formatInputValue($value, $fieldSpec, $this->getEntity());
+    FormattingUtil::formatInputValue($value, $field, $this->getEntity());
 
-    $sql_clause = \CRM_Core_DAO::createSQLFilter("`$table_name`.`$column_name`", [$operator => $value]);
+    $sql_clause = \CRM_Core_DAO::createSQLFilter($field['sql_name'], [$operator => $value]);
     if ($sql_clause === NULL) {
-      throw new \API_Exception("Invalid value in where clause for field '$key'");
+      throw new \API_Exception("Invalid value in where clause for field '$fieldName'");
     }
     return $sql_clause;
   }
@@ -316,88 +280,70 @@ protected function getFields() {
    * Fetch a field from the getFields list
    *
    * @param string $fieldName
+   * @param bool $strict
    *
    * @return string|null
+   * @throws \API_Exception
    */
-  protected function getField($fieldName) {
-    if ($fieldName) {
-      $fieldPath = explode('.', $fieldName);
-      if (count($fieldPath) > 1) {
-        $fieldName = implode('.', array_slice($fieldPath, -2));
-      }
-      return $this->apiFieldSpec[$fieldName] ?? NULL;
+  public function getField($fieldName, $strict = FALSE) {
+    // Perform join if field not yet available - this will add it to apiFieldSpec
+    if (!isset($this->apiFieldSpec[$fieldName]) && strpos($fieldName, '.')) {
+      $this->joinFK($fieldName);
+    }
+    $field = $this->apiFieldSpec[$fieldName] ?? NULL;
+    // Check if field exists and we have permission to view it
+    if ($field && (!$this->checkPermissions || empty($field['permission']) || \CRM_Core_Permission::check($field['permission']))) {
+      return $field;
+    }
+    elseif ($strict) {
+      throw new \API_Exception("Invalid field '$fieldName'");
     }
     return NULL;
   }
 
   /**
+   * Joins a path and adds all fields in the joined eneity to apiFieldSpec
+   *
    * @param $key
+   * @return bool
    * @throws \API_Exception
+   * @throws \Exception
    */
   protected function joinFK($key) {
-    $pathArray = explode('.', $key);
-
-    if (count($pathArray) < 2) {
-      return;
+    if (isset($this->apiFieldSpec[$key])) {
+      return TRUE;
     }
 
+    $pathArray = explode('.', $key);
+
     /** @var \Civi\Api4\Service\Schema\Joiner $joiner */
     $joiner = \Civi::container()->get('joiner');
-    $field = array_pop($pathArray);
+    // 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);
     $pathString = implode('.', $pathArray);
 
     if (!$joiner->canJoin($this, $pathString)) {
-      return;
+      return FALSE;
     }
 
     $joinPath = $joiner->join($this, $pathString);
     /** @var \Civi\Api4\Service\Schema\Joinable\Joinable $lastLink */
     $lastLink = array_pop($joinPath);
 
-    $isWild = strpos($field, '*') !== FALSE;
-    if ($isWild) {
-      if (!in_array($key, $this->select)) {
-        throw new \API_Exception('Wildcards can only be used in the SELECT clause.');
-      }
-      $this->select = array_diff($this->select, [$key]);
+    // Custom field names are already prefixed
+    if ($lastLink instanceof CustomGroupJoinable) {
+      array_pop($pathArray);
     }
-
+    $prefix = $pathArray ? implode('.', $pathArray) . '.' : '';
     // Cache field info for retrieval by $this->getField()
-    $prefix = array_pop($pathArray) . '.';
-    if (!isset($this->apiFieldSpec[$prefix . $field])) {
-      $joinEntity = $lastLink->getEntity();
-      // Custom fields are already prefixed
-      if ($lastLink instanceof CustomGroupJoinable) {
-        $prefix = '';
-      }
-      foreach ($lastLink->getEntityFields() as $fieldObject) {
-        $this->apiFieldSpec[$prefix . $fieldObject->getName()] = $fieldObject->toArray() + ['entity' => $joinEntity];
-      }
-    }
-
-    if (!$isWild && !$lastLink->getField($field)) {
-      throw new \API_Exception('Invalid join');
-    }
-
-    $fields = $isWild ? [] : [$field];
-    // Expand wildcard and add matching fields to $this->select
-    if ($isWild) {
-      $fields = SelectUtil::getMatchingFields($field, $lastLink->getEntityFieldNames());
-      foreach ($fields as $field) {
-        $this->select[] = $pathString . '.' . $field;
-      }
-      $this->select = array_unique($this->select);
+    $joinEntity = $lastLink->getEntity();
+    foreach ($lastLink->getEntityFields() as $fieldObject) {
+      $fieldArray = ['entity' => $joinEntity] + $fieldObject->toArray();
+      $fieldArray['sql_name'] = '`' . $lastLink->getAlias() . '`.`' . $fieldArray['column_name'] . '`';
+      $this->apiFieldSpec[$prefix . $fieldArray['name']] = $fieldArray;
     }
 
-    foreach ($fields as $field) {
-      // custom groups use aliases for field names
-      $col = ($lastLink instanceof CustomGroupJoinable) ? $lastLink->getSqlColumn($field) : $field;
-      // Check Permission on field.
-      if ($this->checkPermissions && !empty($this->apiFieldSpec[$prefix . $field]['permission']) && !\CRM_Core_Permission::check($this->apiFieldSpec[$prefix . $field]['permission'])) {
-        continue;
-      }
-      $this->fkSelectAliases[$pathString . '.' . $field] = sprintf('%s.%s', $lastLink->getAlias(), $col);
-    }
+    return TRUE;
   }
 
   /**
@@ -523,13 +469,6 @@ public function getApiVersion() {
     return $this->apiVersion;
   }
 
-  /**
-   * @return array
-   */
-  public function getFkSelectAliases() {
-    return $this->fkSelectAliases;
-  }
-
   /**
    * @return \Civi\Api4\Service\Schema\Joinable\Joinable[]
    */
@@ -566,6 +505,19 @@ public function constructQueryObject($baoName) {
     }
   }
 
+  /**
+   * Checks if a field either belongs to the main entity or is joinable 1-to-1.
+   *
+   * Used to determine if a field can be added to the SELECT of the main query,
+   * or if it must be fetched post-query.
+   *
+   * @param string $fieldPath
+   * @return bool
+   */
+  public function isOneToOneField(string $fieldPath) {
+    return strpos($fieldPath, '.') === FALSE || !array_filter($this->getPathJoinTypes($fieldPath));
+  }
+
   /**
    * Separates a string like 'emails.location_type.label' into an array, where
    * each value in the array tells whether it is 1-1 or 1-n join type

diff --git a/Civi/Api4/Service/Schema/Joinable/CustomGroupJoinable.php b/Civi/Api4/Service/Schema/Joinable/CustomGroupJoinable.php
@@ -59,7 +59,7 @@ public function getEntityFields() {
     if (!$this->entityFields) {
       $fields = CustomField::get()
         ->setCheckPermissions(FALSE)
-        ->setSelect(['custom_group.name', 'custom_group_id', 'name', 'label', 'data_type', 'html_type', 'is_required', 'is_searchable', 'is_search_range', 'weight', 'is_active', 'is_view', 'option_group_id', 'default_value', 'date_format', 'time_format', 'start_date_years', 'end_date_years'])
+        ->setSelect(['custom_group.name', '*'])
         ->addWhere('custom_group.table_name', '=', $this->getTargetTable())
         ->execute();
       foreach ($fields as $field) {