From 16f5a13d62017fa30190a477b9f580b4eabd23a2 Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Thu, 7 May 2020 16:51:52 -0400 Subject: [PATCH] APIv4 - Add explicit joins --- Civi/API/SelectQuery.php | 2 +- Civi/Api4/Generic/DAOGetAction.php | 35 ++++++ Civi/Api4/Query/Api4SelectQuery.php | 125 ++++++++++++++++++--- Civi/Api4/Query/SqlExpression.php | 12 +- Civi/Api4/Utils/FormattingUtil.php | 1 + tests/phpunit/api/v4/Action/FkJoinTest.php | 85 ++++++++++++++ 6 files changed, 245 insertions(+), 15 deletions(-) diff --git a/Civi/API/SelectQuery.php b/Civi/API/SelectQuery.php index 92e09ad8d1..22577b92c3 100644 --- a/Civi/API/SelectQuery.php +++ b/Civi/API/SelectQuery.php @@ -361,7 +361,7 @@ abstract class SelectQuery { * Get acl clause for an entity * * @param string $tableAlias - * @param string $baoName + * @param \CRM_Core_DAO|string $baoName * @param array $stack * @return array */ diff --git a/Civi/Api4/Generic/DAOGetAction.php b/Civi/Api4/Generic/DAOGetAction.php index 73098f5a5c..d922781ada 100644 --- a/Civi/Api4/Generic/DAOGetAction.php +++ b/Civi/Api4/Generic/DAOGetAction.php @@ -45,6 +45,13 @@ class DAOGetAction extends AbstractGetAction { */ protected $select = []; + /** + * Joins to other entities. + * + * @var array + */ + protected $join = []; + /** * Field(s) by which to group the results. * @@ -120,4 +127,32 @@ class DAOGetAction extends AbstractGetAction { return $this; } + /** + * @param string $entity + * @param bool $required + * @param array ...$conditions + * @return DAOGetAction + */ + public function addJoin(string $entity, bool $required = FALSE, ...$conditions): DAOGetAction { + array_unshift($conditions, $entity, $required); + $this->join[] = $conditions; + return $this; + } + + /** + * @param array $join + * @return DAOGetAction + */ + public function setJoin(array $join): DAOGetAction { + $this->join = $join; + return $this; + } + + /** + * @return array + */ + public function getJoin(): array { + return $this->join; + } + } diff --git a/Civi/Api4/Query/Api4SelectQuery.php b/Civi/Api4/Query/Api4SelectQuery.php index 0e91377545..80208522b4 100644 --- a/Civi/Api4/Query/Api4SelectQuery.php +++ b/Civi/Api4/Query/Api4SelectQuery.php @@ -92,6 +92,9 @@ class Api4SelectQuery extends SelectQuery { // Add ACLs first to avoid redundant subclauses $this->query->where($this->getAclClause(self::MAIN_TABLE_ALIAS, $baoName)); + + // Add explicit joins. Other joins implied by dot notation may be added later + $this->addExplicitJoins($apiGet->getJoin()); } /** @@ -220,11 +223,7 @@ class Api4SelectQuery extends SelectQuery { if ($dir !== 'ASC' && $dir !== 'DESC') { throw new \API_Exception("Invalid sort direction. Cannot order by $item $dir"); } - $expr = SqlExpression::convert($item); - foreach ($expr->getFields() as $fieldName) { - $this->getField($fieldName, TRUE); - } - $this->query->orderBy($expr->render($this->apiFieldSpec) . " $dir"); + $this->query->orderBy($this->renderExpression($item) . " $dir"); } } @@ -243,11 +242,7 @@ class Api4SelectQuery extends SelectQuery { */ protected function buildGroupBy() { foreach ($this->groupBy as $item) { - $expr = SqlExpression::convert($item); - foreach ($expr->getFields() as $fieldName) { - $this->getField($fieldName, TRUE); - } - $this->query->groupBy($expr->render($this->apiFieldSpec)); + $this->query->groupBy($this->renderExpression($item)); } } @@ -256,7 +251,7 @@ class Api4SelectQuery extends SelectQuery { * * @param array $clause * @param string $type - * WHERE|HAVING + * WHERE|HAVING|ON * @return string SQL where clause * * @throws \API_Exception @@ -295,7 +290,7 @@ class Api4SelectQuery extends SelectQuery { * Validate and transform a leaf clause array to SQL. * @param array $clause [$fieldName, $operator, $criteria] * @param string $type - * WHERE|HAVING + * WHERE|HAVING|ON * @return string SQL * @throws \API_Exception * @throws \Exception @@ -303,6 +298,9 @@ class Api4SelectQuery extends SelectQuery { protected function composeClause(array $clause, string $type) { // Pad array for unary operators list($expr, $operator, $value) = array_pad($clause, 3, NULL); + if (!in_array($operator, \CRM_Core_DAO::acceptedSQLOperators(), TRUE)) { + throw new \API_Exception('Illegal operator'); + } // For WHERE clause, expr must be the name of a field. if ($type === 'WHERE') { @@ -311,7 +309,7 @@ class Api4SelectQuery extends SelectQuery { $fieldAlias = $field['sql_name']; } // For HAVING, expr must be an item in the SELECT clause - else { + elseif ($type === 'HAVING') { // Expr references a fieldName or alias if (isset($this->selectAliases[$expr])) { $fieldAlias = $expr; @@ -341,6 +339,21 @@ class Api4SelectQuery extends SelectQuery { } $fieldAlias = '`' . $fieldAlias . '`'; } + elseif ($type === 'ON') { + $expr = $this->getExpression($expr); + $fieldName = count($expr->getFields()) === 1 ? $expr->getFields()[0] : NULL; + $fieldAlias = $expr->render($this->apiFieldSpec); + if (is_string($value)) { + $valExpr = $this->getExpression($value); + if ($fieldName && $valExpr->getType() === 'SqlString') { + FormattingUtil::formatInputValue($valExpr->expr, $fieldName, $this->apiFieldSpec[$fieldName]); + } + return sprintf('%s %s %s', $fieldAlias, $operator, $valExpr->render($this->apiFieldSpec)); + } + elseif ($fieldName) { + FormattingUtil::formatInputValue($value, $fieldName, $this->apiFieldSpec[$fieldName]); + } + } $sql_clause = \CRM_Core_DAO::createSQLFilter($fieldAlias, [$operator => $value]); if ($sql_clause === NULL) { @@ -349,6 +362,29 @@ class Api4SelectQuery extends SelectQuery { return $sql_clause; } + /** + * @param string $expr + * @return SqlExpression + * @throws \API_Exception + */ + protected function getExpression(string $expr) { + $sqlExpr = SqlExpression::convert($expr); + foreach ($sqlExpr->getFields() as $fieldName) { + $this->getField($fieldName, TRUE); + } + return $sqlExpr; + } + + /** + * @param string $expr + * @return string + * @throws \API_Exception + */ + protected function renderExpression(string $expr) { + $sqlExpr = $this->getExpression($expr); + return $sqlExpr->render($this->apiFieldSpec); + } + /** * @inheritDoc */ @@ -383,6 +419,69 @@ class Api4SelectQuery extends SelectQuery { return $field; } + /** + * Join onto other entities as specified by the api call. + * + * @param $joins + * @throws \API_Exception + * @throws \Civi\API\Exception\NotImplementedException + */ + private function addExplicitJoins($joins) { + foreach ($joins as $join) { + // First item in the array is the entity name + $entity = array_shift($join); + // Which might contain an alias. Split on the keyword "AS" + list($entity, $alias) = array_pad(explode(' AS ', $entity), 2, NULL); + // Ensure alias is a safe string, and supply default if not given + $alias = $alias ? \CRM_Utils_String::munge($alias) : 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'; + $joinEntityGet = \Civi\API\Request::create($entity, 'get', ['version' => 4, 'checkPermissions' => $this->checkPermissions]); + foreach ($joinEntityGet->entityFields() as $field) { + $field['sql_name'] = '`' . $alias . '`.`' . $field['column_name'] . '`'; + $field['is_join'] = TRUE; + $this->addSpecField($alias . '.' . $field['name'], $field); + } + $conditions = []; + foreach (array_merge($join, $this->getJoinConditions($entity, $alias)) as $clause) { + $conditions[] = $this->treeWalkClauses($clause, 'ON'); + } + $tableName = AllCoreTables::getTableForEntityName($entity); + $this->join($side, $tableName, $alias, $conditions); + } + } + + /** + * Supply conditions for an explicit join. + * + * @param $entity + * @param $alias + * @return array + */ + private function getJoinConditions($entity, $alias) { + $conditions = []; + // getAclClause() expects a stack of 1-to-1 join fields to help it dedupe, but this is more flexible, + // so unless this is a direct 1-to-1 join with the main entity, we'll just hack it + // with a padded empty stack to bypass its deduping. + $stack = [NULL, NULL]; + foreach ($this->apiFieldSpec as $name => $field) { + if ($field['entity'] !== $entity && $field['fk_entity'] === $entity) { + $conditions[] = [$name, '=', "$alias.id"]; + $stack = [$name]; + } + elseif (strpos($name, "$alias.") === 0 && substr_count($name, '.') === 1 && $field['fk_entity'] === $this->entity) { + $conditions[] = [$name, '=', 'id']; + } + } + // Hmm, if we came up with > 1 condition, then it's ambiguous how it should be joined so we won't return anything but the generic ACLs + if (count($conditions) > 1) { + return $this->getAclClause($alias, AllCoreTables::getFullName($entity), [NULL, NULL]); + } + $acls = $this->getAclClause($alias, AllCoreTables::getFullName($entity), $stack); + return array_merge($acls, $conditions); + } + /** * Joins a path and adds all fields in the joined entity to apiFieldSpec * diff --git a/Civi/Api4/Query/SqlExpression.php b/Civi/Api4/Query/SqlExpression.php index e21f3880f4..2759d3baff 100644 --- a/Civi/Api4/Query/SqlExpression.php +++ b/Civi/Api4/Query/SqlExpression.php @@ -35,7 +35,7 @@ abstract class SqlExpression { * The raw expression, minus the alias. * @var string */ - protected $expr = ''; + public $expr = ''; /** * SqlFunction constructor. @@ -148,4 +148,14 @@ abstract class SqlExpression { return $this->alias ?? $this->fields[0] ?? \CRM_Utils_String::munge($this->expr); } + /** + * Returns the name of this sql expression class. + * + * @return string + */ + public function getType(): string { + $className = get_class($this); + return substr($className, strrpos($className, '\\') + 1); + } + } diff --git a/Civi/Api4/Utils/FormattingUtil.php b/Civi/Api4/Utils/FormattingUtil.php index be91ce41e1..b3cd5ecd05 100644 --- a/Civi/Api4/Utils/FormattingUtil.php +++ b/Civi/Api4/Utils/FormattingUtil.php @@ -88,6 +88,7 @@ class FormattingUtil { if ($suffix) { $options = self::getPseudoconstantList($fieldSpec['entity'], $fieldSpec['name'], substr($fieldName, $suffix + 1)); $value = self::replacePseudoconstant($options, $value, TRUE); + return; } elseif (is_array($value)) { foreach ($value as &$val) { diff --git a/tests/phpunit/api/v4/Action/FkJoinTest.php b/tests/phpunit/api/v4/Action/FkJoinTest.php index f23109a479..602942f93d 100644 --- a/tests/phpunit/api/v4/Action/FkJoinTest.php +++ b/tests/phpunit/api/v4/Action/FkJoinTest.php @@ -24,6 +24,8 @@ namespace api\v4\Action; use api\v4\UnitTestCase; use Civi\Api4\Activity; use Civi\Api4\Contact; +use Civi\Api4\Email; +use Civi\Api4\Phone; /** * @group headless @@ -55,4 +57,87 @@ class FkJoinTest extends UnitTestCase { $this->assertCount(1, $results); } + public function testOptionalJoin() { + // DefaultDataSet includes 2 phones for contact 1, 0 for contact 2. + // We'll add one for contact 2 as a red herring to make sure we only get back the correct ones. + Phone::create()->setCheckPermissions(FALSE) + ->setValues(['contact_id' => $this->getReference('test_contact_2')['id'], 'phone' => '123456']) + ->execute(); + $contacts = Contact::get() + ->setCheckPermissions(FALSE) + ->addJoin('Phone', FALSE) + ->addSelect('id', 'phone.phone') + ->addWhere('id', 'IN', [$this->getReference('test_contact_1')['id']]) + ->addOrderBy('phone.id') + ->execute(); + $this->assertCount(2, $contacts); + $this->assertEquals($this->getReference('test_contact_1')['id'], $contacts[0]['id']); + $this->assertEquals($this->getReference('test_contact_1')['id'], $contacts[1]['id']); + } + + public function testRequiredJoin() { + // Joining with no condition + $contacts = Contact::get() + ->setCheckPermissions(FALSE) + ->addSelect('id', 'phone.phone') + ->addJoin('Phone', TRUE) + ->addWhere('id', 'IN', [$this->getReference('test_contact_1')['id'], $this->getReference('test_contact_2')['id']]) + ->addOrderBy('phone.id') + ->execute(); + $this->assertCount(2, $contacts); + $this->assertEquals($this->getReference('test_contact_1')['id'], $contacts[0]['id']); + $this->assertEquals($this->getReference('test_contact_1')['id'], $contacts[1]['id']); + + // Add is_primary condition, should result in only one record + $contacts = Contact::get() + ->setCheckPermissions(FALSE) + ->addSelect('id', 'phone.phone', 'phone.location_type_id') + ->addJoin('Phone', TRUE, ['phone.is_primary', '=', TRUE]) + ->addWhere('id', 'IN', [$this->getReference('test_contact_1')['id'], $this->getReference('test_contact_2')['id']]) + ->addOrderBy('phone.id') + ->execute(); + $this->assertCount(1, $contacts); + $this->assertEquals($this->getReference('test_contact_1')['id'], $contacts[0]['id']); + $this->assertEquals('+35355439483', $contacts[0]['phone.phone']); + $this->assertEquals('1', $contacts[0]['phone.location_type_id']); + } + + public function testJoinToTheSameTableTwice() { + $cid1 = Contact::create()->setCheckPermissions(FALSE) + ->addValue('first_name', 'Aaa') + ->addChain('email1', Email::create()->setValues(['email' => 'yoohoo@yahoo.test', 'contact_id' => '$id', 'location_type_id:name' => 'Home'])) + ->addChain('email2', Email::create()->setValues(['email' => 'yahoo@yoohoo.test', 'contact_id' => '$id', 'location_type_id:name' => 'Work'])) + ->execute() + ->first()['id']; + + $cid2 = Contact::create()->setCheckPermissions(FALSE) + ->addValue('first_name', 'Bbb') + ->addChain('email1', Email::create()->setValues(['email' => '1@test.test', 'contact_id' => '$id', 'location_type_id:name' => 'Home'])) + ->addChain('email2', Email::create()->setValues(['email' => '2@test.test', 'contact_id' => '$id', 'location_type_id:name' => 'Work'])) + ->addChain('email3', Email::create()->setValues(['email' => '3@test.test', 'contact_id' => '$id', 'location_type_id:name' => 'Other'])) + ->execute() + ->first()['id']; + + $cid3 = Contact::create()->setCheckPermissions(FALSE) + ->addValue('first_name', 'Ccc') + ->execute() + ->first()['id']; + + $contacts = Contact::get() + ->setCheckPermissions(FALSE) + ->addSelect('id', 'first_name', 'any_email.email', 'any_email.location_type_id:name', 'any_email.is_primary', 'primary_email.email') + ->addJoin('Email AS any_email', TRUE) + ->addJoin('Email AS primary_email', FALSE, ['primary_email.is_primary', '=', TRUE]) + ->addWhere('id', 'IN', [$cid1, $cid2, $cid3]) + ->addOrderBy('any_email.id') + ->setDebug(TRUE) + ->execute(); + $this->assertCount(5, $contacts); + $this->assertEquals('Home', $contacts[0]['any_email.location_type_id:name']); + $this->assertEquals('yoohoo@yahoo.test', $contacts[1]['primary_email.email']); + $this->assertEquals('1@test.test', $contacts[2]['primary_email.email']); + $this->assertEquals('1@test.test', $contacts[3]['primary_email.email']); + $this->assertEquals('1@test.test', $contacts[4]['primary_email.email']); + } + } -- 2.25.1