From c9e3ae2e6a1e52d56e4a81993f015710f19f1cff Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Tue, 7 Apr 2020 20:43:12 -0400 Subject: [PATCH] APIv4 - Add support for HAVING clause --- Civi/Api4/Generic/DAOGetAction.php | 27 +++++++ Civi/Api4/Query/Api4SelectQuery.php | 73 ++++++++++++++----- Civi/Api4/Query/SqlExpression.php | 23 ++++-- Civi/Api4/Query/SqlField.php | 8 +- Civi/Api4/Query/SqlFunction.php | 2 +- Civi/Api4/Query/SqlNumber.php | 4 +- Civi/Api4/Query/SqlString.php | 8 +- .../phpunit/api/v4/Action/SqlFunctionTest.php | 38 ++++++++++ .../api/v4/Query/SqlExpressionParserTest.php | 2 +- 9 files changed, 147 insertions(+), 38 deletions(-) diff --git a/Civi/Api4/Generic/DAOGetAction.php b/Civi/Api4/Generic/DAOGetAction.php index ae29e3992a..ab7d85b966 100644 --- a/Civi/Api4/Generic/DAOGetAction.php +++ b/Civi/Api4/Generic/DAOGetAction.php @@ -29,6 +29,9 @@ use Civi\Api4\Query\Api4SelectQuery; * 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; @@ -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(); @@ -95,4 +107,19 @@ class DAOGetAction extends AbstractGetAction { 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; + } + } diff --git a/Civi/Api4/Query/Api4SelectQuery.php b/Civi/Api4/Query/Api4SelectQuery.php index 09e95f147e..34c27ee671 100644 --- a/Civi/Api4/Query/Api4SelectQuery.php +++ b/Civi/Api4/Query/Api4SelectQuery.php @@ -64,6 +64,11 @@ class Api4SelectQuery extends SelectQuery { */ public $groupBy = []; + /** + * @var array + */ + public $having = []; + /** * @param \Civi\Api4\Generic\DAOGetAction $apiGet */ @@ -76,6 +81,7 @@ class Api4SelectQuery extends SelectQuery { $this->orderBy = $apiGet->getOrderBy(); $this->limit = $apiGet->getLimit(); $this->offset = $apiGet->getOffset(); + $this->having = $apiGet->getHaving(); if ($apiGet->getDebug()) { $this->debugOutput =& $apiGet->_debugOutput; } @@ -106,6 +112,7 @@ class Api4SelectQuery extends SelectQuery { $this->buildOrderBy(); $this->buildLimit(); $this->buildGroupBy(); + $this->buildHavingClause(); return $this->query->toSQL(); } @@ -129,7 +136,7 @@ class Api4SelectQuery extends SelectQuery { 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; @@ -186,7 +193,8 @@ class Api4SelectQuery extends SelectQuery { } } if ($valid) { - $alias = $this->selectAliases[] = $expr->getAlias(); + $alias = $expr->getAlias(); + $this->selectAliases[$alias] = $expr->getExpr(); $this->query->select($expr->render($this->apiFieldSpec) . " AS `$alias`"); } } @@ -197,8 +205,18 @@ class Api4SelectQuery extends SelectQuery { */ 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')); } } @@ -244,25 +262,26 @@ class Api4SelectQuery extends SelectQuery { * 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) . ')'; } @@ -272,30 +291,48 @@ class Api4SelectQuery extends SelectQuery { 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; } diff --git a/Civi/Api4/Query/SqlExpression.php b/Civi/Api4/Query/SqlExpression.php index 198ce1c51c..e21f3880f4 100644 --- a/Civi/Api4/Query/SqlExpression.php +++ b/Civi/Api4/Query/SqlExpression.php @@ -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(); } @@ -68,14 +69,13 @@ abstract class SqlExpression { $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)) { @@ -132,13 +132,20 @@ abstract class SqlExpression { */ 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); } } diff --git a/Civi/Api4/Query/SqlField.php b/Civi/Api4/Query/SqlField.php index 488e3b052c..712bd7b8cc 100644 --- a/Civi/Api4/Query/SqlField.php +++ b/Civi/Api4/Query/SqlField.php @@ -17,14 +17,14 @@ namespace Civi\Api4\Query; 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']; } } diff --git a/Civi/Api4/Query/SqlFunction.php b/Civi/Api4/Query/SqlFunction.php index df93b66603..e10bbcb23b 100644 --- a/Civi/Api4/Query/SqlFunction.php +++ b/Civi/Api4/Query/SqlFunction.php @@ -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']) { diff --git a/Civi/Api4/Query/SqlNumber.php b/Civi/Api4/Query/SqlNumber.php index e8ee2550b3..064121bfa9 100644 --- a/Civi/Api4/Query/SqlNumber.php +++ b/Civi/Api4/Query/SqlNumber.php @@ -17,11 +17,11 @@ namespace Civi\Api4\Query; 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; } } diff --git a/Civi/Api4/Query/SqlString.php b/Civi/Api4/Query/SqlString.php index 12c85ab393..8ea9c00137 100644 --- a/Civi/Api4/Query/SqlString.php +++ b/Civi/Api4/Query/SqlString.php @@ -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) . '"'; } } diff --git a/tests/phpunit/api/v4/Action/SqlFunctionTest.php b/tests/phpunit/api/v4/Action/SqlFunctionTest.php index 9b33c594c3..ab9b09dd8b 100644 --- a/tests/phpunit/api/v4/Action/SqlFunctionTest.php +++ b/tests/phpunit/api/v4/Action/SqlFunctionTest.php @@ -59,4 +59,42 @@ class SqlFunctionTest extends UnitTestCase { $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']); + } + } diff --git a/tests/phpunit/api/v4/Query/SqlExpressionParserTest.php b/tests/phpunit/api/v4/Query/SqlExpressionParserTest.php index cf43dbd6c0..4be10cefec 100644 --- a/tests/phpunit/api/v4/Query/SqlExpressionParserTest.php +++ b/tests/phpunit/api/v4/Query/SqlExpressionParserTest.php @@ -49,7 +49,7 @@ class SqlExpressionParserTest extends UnitTestCase { $this->assertNotEmpty($params[0]['prefix']); $this->assertEmpty($params[0]['suffix']); - $sqlFn = new $className('total'); + $sqlFn = new $className($fnName . '(total)'); $this->assertEquals($fnName, $sqlFn->getName()); $this->assertEquals(['total'], $sqlFn->getFields()); $this->assertCount(1, $this->getArgs($sqlFn)); -- 2.25.1