Skip to content

Commit

Permalink
APIv4 - Add support for HAVING clause
Browse files Browse the repository at this point in the history
  • Loading branch information
colemanw committed Apr 8, 2020
1 parent 0ff7d95 commit d80a98e
Show file tree
Hide file tree
Showing 8 changed files with 146 additions and 37 deletions.
27 changes: 27 additions & 0 deletions Civi/Api4/Generic/DAOGetAction.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@
* Use the `select` param to determine which fields are returned, defaults to `[*]`.
*
* Perform joins on other related entities using a dot notation.
*
* @method $this setHaving(array $clauses)
* @method array getHaving()
*/
class DAOGetAction extends AbstractGetAction {
use Traits\DAOActionTrait;
Expand All @@ -51,6 +54,15 @@ class DAOGetAction extends AbstractGetAction {
*/
protected $groupBy = [];

/**
* Clause for filtering results after grouping and filters are applied.
*
* Each expression should correspond to an item from the SELECT array.
*
* @var array
*/
protected $having = [];

public function _run(Result $result) {
$this->setDefaultWhereClause();
$this->expandSelectClauseWildcards();
Expand Down Expand Up @@ -95,4 +107,19 @@ public function addGroupBy(string $field) {
return $this;
}

/**
* @param string $expr
* @param string $op
* @param mixed $value
* @return $this
* @throws \API_Exception
*/
public function addHaving(string $expr, string $op, $value = NULL) {
if (!in_array($op, \CRM_Core_DAO::acceptedSQLOperators())) {
throw new \API_Exception('Unsupported operator');
}
$this->having[] = [$expr, $op, $value];
return $this;
}

}
73 changes: 55 additions & 18 deletions Civi/Api4/Query/Api4SelectQuery.php
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,11 @@ class Api4SelectQuery extends SelectQuery {
*/
public $groupBy = [];

/**
* @var array
*/
public $having = [];

/**
* @param \Civi\Api4\Generic\DAOGetAction $apiGet
*/
Expand All @@ -76,6 +81,7 @@ public function __construct($apiGet) {
$this->orderBy = $apiGet->getOrderBy();
$this->limit = $apiGet->getLimit();
$this->offset = $apiGet->getOffset();
$this->having = $apiGet->getHaving();
if ($apiGet->getDebug()) {
$this->debugOutput =& $apiGet->_debugOutput;
}
Expand Down Expand Up @@ -106,6 +112,7 @@ public function getSql() {
$this->buildOrderBy();
$this->buildLimit();
$this->buildGroupBy();
$this->buildHavingClause();
return $this->query->toSQL();
}

Expand All @@ -129,7 +136,7 @@ public function run() {
break;
}
$results[$id] = [];
foreach ($this->selectAliases as $alias) {
foreach ($this->selectAliases as $alias => $expr) {
$returnName = $alias;
$alias = str_replace('.', '_', $alias);
$results[$id][$returnName] = property_exists($query, $alias) ? $query->$alias : NULL;
Expand Down Expand Up @@ -186,7 +193,8 @@ protected function buildSelectClause() {
}
}
if ($valid) {
$alias = $this->selectAliases[] = $expr->getAlias();
$alias = $expr->getAlias();
$this->selectAliases[$alias] = $expr->getExpr();
$this->query->select($expr->render($this->apiFieldSpec) . " AS `$alias`");
}
}
Expand All @@ -197,8 +205,18 @@ protected function buildSelectClause() {
*/
protected function buildWhereClause() {
foreach ($this->where as $clause) {
$sql_clause = $this->treeWalkWhereClause($clause);
$this->query->where($sql_clause);
$this->query->where($this->treeWalkClauses($clause, 'WHERE'));
}
}

/**
* Build HAVING clause.
*
* Every expression referenced must also be in the SELECT clause.
*/
protected function buildHavingClause() {
foreach ($this->having as $clause) {
$this->query->having($this->treeWalkClauses($clause, 'HAVING'));
}
}

Expand Down Expand Up @@ -244,25 +262,26 @@ protected function buildGroupBy() {
* Recursively validate and transform a branch or leaf clause array to SQL.
*
* @param array $clause
* @param string $type
* WHERE|HAVING
* @return string SQL where clause
*
* @uses validateClauseAndComposeSql() to generate the SQL etc.
* @todo if an 'and' is nested within and 'and' (or or-in-or) then should
* flatten that to be a single list of clauses.
* @throws \API_Exception
* @uses composeClause() to generate the SQL etc.
*/
protected function treeWalkWhereClause($clause) {
protected function treeWalkClauses($clause, $type) {
switch ($clause[0]) {
case 'OR':
case 'AND':
// handle branches
if (count($clause[1]) === 1) {
// a single set so AND|OR is immaterial
return $this->treeWalkWhereClause($clause[1][0]);
return $this->treeWalkClauses($clause[1][0], $type);
}
else {
$sql_subclauses = [];
foreach ($clause[1] as $subclause) {
$sql_subclauses[] = $this->treeWalkWhereClause($subclause);
$sql_subclauses[] = $this->treeWalkClauses($subclause, $type);
}
return '(' . implode("\n" . $clause[0], $sql_subclauses) . ')';
}
Expand All @@ -272,30 +291,48 @@ protected function treeWalkWhereClause($clause) {
if (!is_string($clause[1][0])) {
$clause[1] = ['AND', $clause[1]];
}
return 'NOT (' . $this->treeWalkWhereClause($clause[1]) . ')';
return 'NOT (' . $this->treeWalkClauses($clause[1], $type) . ')';

default:
return $this->validateClauseAndComposeSql($clause);
return $this->composeClause($clause, $type);
}
}

/**
* Validate and transform a leaf clause array to SQL.
* @param array $clause [$fieldName, $operator, $criteria]
* @param string $type
* WHERE|HAVING
* @return string SQL
* @throws \API_Exception
* @throws \Exception
*/
protected function validateClauseAndComposeSql($clause) {
protected function composeClause(array $clause, string $type) {
// Pad array for unary operators
list($fieldName, $operator, $value) = array_pad($clause, 3, NULL);
$field = $this->getField($fieldName, TRUE);
list($expr, $operator, $value) = array_pad($clause, 3, NULL);

FormattingUtil::formatInputValue($value, $field, $this->getEntity());
// For WHERE clause, expr must be the name of a field.
if ($type === 'WHERE') {
$field = $this->getField($expr, TRUE);
FormattingUtil::formatInputValue($value, $field, $this->getEntity());
$fieldAlias = $field['sql_name'];
}
// For HAVING, expr must be an item in the SELECT clause
else {
if (isset($this->selectAliases[$expr])) {
$fieldAlias = $expr;
}
elseif (in_array($expr, $this->selectAliases)) {
$fieldAlias = array_search($expr, $this->selectAliases);
}
else {
throw new \API_Exception("Invalid expression in $type clause: '$expr'. Must use a value from SELECT clause.");
}
}

$sql_clause = \CRM_Core_DAO::createSQLFilter($field['sql_name'], [$operator => $value]);
$sql_clause = \CRM_Core_DAO::createSQLFilter($fieldAlias, [$operator => $value]);
if ($sql_clause === NULL) {
throw new \API_Exception("Invalid value in where clause for field '$fieldName'");
throw new \API_Exception("Invalid value in $type clause for '$expr'");
}
return $sql_clause;
}
Expand Down
23 changes: 15 additions & 8 deletions Civi/Api4/Query/SqlExpression.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,23 +26,24 @@ abstract class SqlExpression {
protected $fields = [];

/**
* The SELECT alias (if null it will be calculated by getAlias)
* @var string|null
*/
protected $alias;

/**
* The argument string.
* The raw expression, minus the alias.
* @var string
*/
protected $arg = '';
protected $expr = '';

/**
* SqlFunction constructor.
* @param string $arg
* @param string $expr
* @param string|null $alias
*/
public function __construct(string $arg, $alias = NULL) {
$this->arg = $arg;
public function __construct(string $expr, $alias = NULL) {
$this->expr = $expr;
$this->alias = $alias;
$this->initialize();
}
Expand All @@ -68,14 +69,13 @@ public static function convert(string $expression, $parseAlias = FALSE, $mustBe
$bracketPos = strpos($expr, '(');
$firstChar = substr($expr, 0, 1);
$lastChar = substr($expr, -1);
// Function
// If there are brackets but not the first character, we have a function
if ($bracketPos && $lastChar === ')') {
$fnName = substr($expr, 0, $bracketPos);
if ($fnName !== strtoupper($fnName)) {
throw new \API_Exception('Sql function must be uppercase.');
}
$className = 'SqlFunction' . $fnName;
$expr = substr($expr, $bracketPos + 1, -1);
}
// String expression
elseif ($firstChar === $lastChar && in_array($firstChar, ['"', "'"], TRUE)) {
Expand Down Expand Up @@ -132,13 +132,20 @@ public function getFields(): array {
*/
abstract public function render(array $fieldList): string;

/**
* @return string
*/
public function getExpr(): string {
return $this->expr;
}

/**
* Returns the alias to use for SELECT AS.
*
* @return string
*/
public function getAlias(): string {
return $this->alias ?? $this->fields[0] ?? \CRM_Utils_String::munge($this->arg);
return $this->alias ?? $this->fields[0] ?? \CRM_Utils_String::munge($this->expr);
}

}
8 changes: 4 additions & 4 deletions Civi/Api4/Query/SqlField.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,14 @@
class SqlField extends SqlExpression {

protected function initialize() {
$this->fields[] = $this->arg;
$this->fields[] = $this->expr;
}

public function render(array $fieldList): string {
if (empty($fieldList[$this->arg])) {
throw new \API_Exception("Invalid field '{$this->arg}'");
if (empty($fieldList[$this->expr])) {
throw new \API_Exception("Invalid field '{$this->expr}'");
}
return $fieldList[$this->arg]['sql_name'];
return $fieldList[$this->expr]['sql_name'];
}

}
2 changes: 1 addition & 1 deletion Civi/Api4/Query/SqlFunction.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ abstract class SqlFunction extends SqlExpression {
* Parse the argument string into an array of function arguments
*/
protected function initialize() {
$arg = $this->arg;
$arg = trim(substr($this->expr, strpos($this->expr, '(') + 1, -1));
foreach ($this->getParams() as $param) {
$prefix = $this->captureKeyword($param['prefix'], $arg);
if ($param['expr'] && isset($prefix) || in_array('', $param['prefix']) || !$param['optional']) {
Expand Down
4 changes: 2 additions & 2 deletions Civi/Api4/Query/SqlNumber.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@
class SqlNumber extends SqlExpression {

protected function initialize() {
\CRM_Utils_Type::validate($this->arg, 'Float');
\CRM_Utils_Type::validate($this->expr, 'Float');
}

public function render(array $fieldList): string {
return $this->arg;
return $this->expr;
}

}
8 changes: 4 additions & 4 deletions Civi/Api4/Query/SqlString.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,15 @@ class SqlString extends SqlExpression {

protected function initialize() {
// Remove surrounding quotes
$str = substr($this->arg, 1, -1);
$str = substr($this->expr, 1, -1);
// Unescape the outer quote character inside the string to prevent double-escaping in render()
$quot = substr($this->arg, 0, 1);
$quot = substr($this->expr, 0, 1);
$backslash = chr(0) . 'backslash' . chr(0);
$this->arg = str_replace(['\\\\', "\\$quot", $backslash], [$backslash, $quot, '\\\\'], $str);
$this->expr = str_replace(['\\\\', "\\$quot", $backslash], [$backslash, $quot, '\\\\'], $str);
}

public function render(array $fieldList): string {
return '"' . \CRM_Core_DAO::escapeString($this->arg) . '"';
return '"' . \CRM_Core_DAO::escapeString($this->expr) . '"';
}

}
38 changes: 38 additions & 0 deletions tests/phpunit/api/v4/Action/SqlFunctionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,4 +59,42 @@ public function testGroupAggregates() {
$this->assertEquals(4, $agg['count']);
}

public function testGroupHaving() {
$cid = Contact::create()->setCheckPermissions(FALSE)->addValue('first_name', 'donor')->execute()->first()['id'];
Contribution::save()
->setCheckPermissions(FALSE)
->setDefaults(['contact_id' => $cid, 'financial_type_id' => 1])
->setRecords([
['total_amount' => 100, 'receive_date' => '2020-02-02'],
['total_amount' => 200, 'receive_date' => '2020-02-02'],
['total_amount' => 300, 'receive_date' => '2020-03-03'],
['total_amount' => 400, 'receive_date' => '2020-04-04'],
])
->execute();
$result = Contribution::get()
->setCheckPermissions(FALSE)
->addGroupBy('contact_id')
->addGroupBy('receive_date')
->addSelect('contact_id')
->addSelect('receive_date')
->addSelect('AVG(total_amount) AS average')
->addSelect('SUM(total_amount)')
->addSelect('MAX(total_amount)')
->addSelect('MIN(total_amount)')
->addSelect('COUNT(*) AS count')
->addOrderBy('receive_date')
->addHaving('contact_id', '=', $cid)
->addHaving('receive_date', '<', '2020-04-01')
->execute();
$this->assertCount(2, $result);
$this->assertEquals(150, $result[0]['average']);
$this->assertEquals(300, $result[1]['average']);
$this->assertEquals(300, $result[0]['SUM:total_amount']);
$this->assertEquals(300, $result[1]['SUM:total_amount']);
$this->assertEquals(200, $result[0]['MAX:total_amount']);
$this->assertEquals(100, $result[0]['MIN:total_amount']);
$this->assertEquals(2, $result[0]['count']);
$this->assertEquals(1, $result[1]['count']);
}

}

0 comments on commit d80a98e

Please sign in to comment.