From 90908aacb8a72fae294b095c9da95715d7a68bef Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Sat, 11 Jul 2020 14:45:26 -0400 Subject: [PATCH] APIv4 - Specify BridgeEntities to assist with joins --- Civi/Api4/ActivityContact.php | 2 +- Civi/Api4/DashboardContact.php | 2 +- Civi/Api4/Entity.php | 5 + Civi/Api4/EntityTag.php | 2 +- Civi/Api4/Generic/AbstractEntity.php | 13 +- Civi/Api4/Generic/BridgeEntity.php | 21 ++++ Civi/Api4/Generic/DAOGetAction.php | 20 ++- Civi/Api4/GroupContact.php | 2 +- Civi/Api4/Query/Api4SelectQuery.php | 138 ++++++++++++++++++--- Civi/Api4/UFMatch.php | 2 +- ang/api4Explorer/Explorer.html | 7 +- ang/api4Explorer/Explorer.js | 3 +- css/api4-explorer.css | 4 - tests/phpunit/api/v4/Action/FkJoinTest.php | 73 ++++++++++- 14 files changed, 262 insertions(+), 32 deletions(-) create mode 100644 Civi/Api4/Generic/BridgeEntity.php diff --git a/Civi/Api4/ActivityContact.php b/Civi/Api4/ActivityContact.php index 3c306ef139..e0d39cfb27 100644 --- a/Civi/Api4/ActivityContact.php +++ b/Civi/Api4/ActivityContact.php @@ -29,6 +29,6 @@ namespace Civi\Api4; * @see \Civi\Api4\Activity * @package Civi\Api4 */ -class ActivityContact extends Generic\DAOEntity { +class ActivityContact extends Generic\BridgeEntity { } diff --git a/Civi/Api4/DashboardContact.php b/Civi/Api4/DashboardContact.php index 4a4bdc732c..3133272041 100644 --- a/Civi/Api4/DashboardContact.php +++ b/Civi/Api4/DashboardContact.php @@ -26,6 +26,6 @@ namespace Civi\Api4; * @see \Civi\Api4\Dashboard * @package Civi\Api4 */ -class DashboardContact extends Generic\DAOEntity { +class DashboardContact extends Generic\BridgeEntity { } diff --git a/Civi/Api4/Entity.php b/Civi/Api4/Entity.php index a7767eab47..e207172711 100644 --- a/Civi/Api4/Entity.php +++ b/Civi/Api4/Entity.php @@ -51,6 +51,11 @@ class Entity extends Generic\AbstractEntity { 'name' => 'title', 'description' => 'Localized title', ], + [ + 'name' => 'type', + 'description' => 'Base class for this entity', + 'options' => ['DAOEntity' => 'DAOEntity', 'BasicEntity' => 'BasicEntity', 'BridgeEntity' => 'BridgeEntity', 'AbstractEntity' => 'AbstractEntity'], + ], [ 'name' => 'description', 'description' => 'Description from docblock', diff --git a/Civi/Api4/EntityTag.php b/Civi/Api4/EntityTag.php index e2a48276a0..aeefb3d747 100644 --- a/Civi/Api4/EntityTag.php +++ b/Civi/Api4/EntityTag.php @@ -25,6 +25,6 @@ namespace Civi\Api4; * * @package Civi\Api4 */ -class EntityTag extends Generic\DAOEntity { +class EntityTag extends Generic\BridgeEntity { } diff --git a/Civi/Api4/Generic/AbstractEntity.php b/Civi/Api4/Generic/AbstractEntity.php index 254f626a05..75163dc362 100644 --- a/Civi/Api4/Generic/AbstractEntity.php +++ b/Civi/Api4/Generic/AbstractEntity.php @@ -78,7 +78,7 @@ abstract class AbstractEntity { * @return string */ protected static function getEntityName() { - return substr(static::class, strrpos(static::class, '\\') + 1); + return self::stripNamespace(static::class); } /** @@ -121,6 +121,7 @@ abstract class AbstractEntity { $info = [ 'name' => static::getEntityName(), 'title' => static::getEntityTitle(), + 'type' => self::stripNamespace(get_parent_class(static::class)), ]; $reflection = new \ReflectionClass(static::class); $info += ReflectionUtils::getCodeDocs($reflection, NULL, ['entity' => $info['name']]); @@ -128,4 +129,14 @@ abstract class AbstractEntity { return $info; } + /** + * Remove namespace prefix from a class name + * + * @param string $className + * @return string + */ + private static function stripNamespace($className) { + return substr($className, strrpos($className, '\\') + 1); + } + } diff --git a/Civi/Api4/Generic/BridgeEntity.php b/Civi/Api4/Generic/BridgeEntity.php new file mode 100644 index 0000000000..32faea1aa5 --- /dev/null +++ b/Civi/Api4/Generic/BridgeEntity.php @@ -0,0 +1,21 @@ +join[] = $conditions; return $this; diff --git a/Civi/Api4/GroupContact.php b/Civi/Api4/GroupContact.php index 62d1b2dc0f..656f23e764 100644 --- a/Civi/Api4/GroupContact.php +++ b/Civi/Api4/GroupContact.php @@ -30,7 +30,7 @@ namespace Civi\Api4; * * @package Civi\Api4 */ -class GroupContact extends Generic\DAOEntity { +class GroupContact extends Generic\BridgeEntity { /** * @return Action\GroupContact\Create diff --git a/Civi/Api4/Query/Api4SelectQuery.php b/Civi/Api4/Query/Api4SelectQuery.php index b9bedfa86b..ea8cad5f3f 100644 --- a/Civi/Api4/Query/Api4SelectQuery.php +++ b/Civi/Api4/Query/Api4SelectQuery.php @@ -439,7 +439,7 @@ class Api4SelectQuery { // Prevent (most) redundant acl sub clauses if they have already been applied to the main entity. // FIXME: Currently this only works 1 level deep, but tracking through multiple joins would increase complexity // and just doing it for the first join takes care of most acl clause deduping. - if (count($stack) === 1 && in_array($stack[0], $this->aclFields)) { + if (count($stack) === 1 && in_array($stack[0], $this->aclFields, TRUE)) { return []; } $clauses = $baoName::getSelectWhereClause($tableAlias); @@ -494,12 +494,18 @@ class Api4SelectQuery { // 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'; + // Add all fields from joined entity to spec $joinEntityGet = \Civi\API\Request::create($entity, 'get', ['version' => 4, 'checkPermissions' => $this->getCheckPermissions()]); foreach ($joinEntityGet->entityFields() as $field) { $field['sql_name'] = '`' . $alias . '`.`' . $field['column_name'] . '`'; $this->addSpecField($alias . '.' . $field['name'], $field); } - $conditions = $this->getJoinConditions($entity, $alias); + if (!empty($join[0]) && is_string($join[0]) && \CRM_Utils_Rule::alphanumeric($join[0])) { + $conditions = $this->getBridgeJoin($join, $entity, $alias); + } + else { + $conditions = $this->getJoinConditions($join, $entity, $alias); + } foreach (array_filter($join) as $clause) { $conditions[] = $this->treeWalkClauses($clause, 'ON'); } @@ -511,35 +517,133 @@ class Api4SelectQuery { /** * Supply conditions for an explicit join. * - * @param $entity - * @param $alias + * @param array $joinTree + * @param string $joinEntity + * @param string $alias * @return array */ - private function getJoinConditions($entity, $alias) { + private function getJoinConditions($joinTree, $joinEntity, $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[] = $this->treeWalkClauses([$name, '=', "$alias.id"], 'ON'); + // If we're not explicitly referencing the joinEntity ID in the ON clause, search for a default + $explicitId = array_filter($joinTree, function($clause) use ($alias) { + list($sideA, $op, $sideB) = array_pad((array) $clause, 3, NULL); + return $op === '=' && ($sideA === "$alias.id" || $sideB === "$alias.id"); + }); + if (!$explicitId) { + foreach ($this->apiFieldSpec as $name => $field) { + if ($field['entity'] !== $joinEntity && $field['fk_entity'] === $joinEntity) { + $conditions[] = $this->treeWalkClauses([$name, '=', "$alias.id"], 'ON'); + } + elseif (strpos($name, "$alias.") === 0 && substr_count($name, '.') === 1 && $field['fk_entity'] === $this->getEntity()) { + $conditions[] = $this->treeWalkClauses([$name, '=', 'id'], 'ON'); + $stack = ['id']; + } } - elseif (strpos($name, "$alias.") === 0 && substr_count($name, '.') === 1 && $field['fk_entity'] === $this->getEntity()) { - $conditions[] = $this->treeWalkClauses([$name, '=', 'id'], 'ON'); - $stack = ['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) { + $stack = [NULL, NULL]; + $conditions = []; } } - // 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) { - $stack = [NULL, NULL]; - $conditions = []; - } - $baoName = CoreUtil::getBAOFromApiName($entity); + $baoName = CoreUtil::getBAOFromApiName($joinEntity); $acls = array_values($this->getAclClause($alias, $baoName, $stack)); return array_merge($acls, $conditions); } + /** + * Join onto a BridgeEntity table + * + * @param array $joinTree + * @param string $joinEntity + * @param string $alias + * @return array + * @throws \API_Exception + */ + protected function getBridgeJoin(&$joinTree, $joinEntity, $alias) { + $bridgeEntity = array_shift($joinTree); + if (!is_a('\Civi\Api4\\' . $bridgeEntity, '\Civi\Api4\Generic\BridgeEntity', TRUE)) { + throw new \API_Exception("Illegal bridge entity specified: " . $bridgeEntity); + } + $bridgeAlias = $alias . '_via_' . strtolower($bridgeEntity); + $bridgeTable = CoreUtil::getTableName($bridgeEntity); + $joinTable = CoreUtil::getTableName($joinEntity); + $bridgeEntityGet = \Civi\API\Request::create($bridgeEntity, 'get', ['version' => 4, 'checkPermissions' => $this->getCheckPermissions()]); + $fkToJoinField = $fkToBaseField = NULL; + // Find the bridge field that links to the joinEntity (either an explicit FK or an entity_id/entity_table combo) + foreach ($bridgeEntityGet->entityFields() as $name => $field) { + if ($field['fk_entity'] === $joinEntity || (!$fkToJoinField && $name === 'entity_id')) { + $fkToJoinField = $name; + } + } + // Get list of entities allowed for entity_table + if (array_key_exists('entity_id', $bridgeEntityGet->entityFields())) { + $entityTables = (array) civicrm_api4($bridgeEntity, 'getFields', [ + 'checkPermissions' => FALSE, + 'where' => [['name', '=', 'entity_table']], + 'loadOptions' => TRUE, + ], ['options'])->first(); + } + // If bridge field to joinEntity is entity_id, validate entity_table is allowed + if (!$fkToJoinField || ($fkToJoinField === 'entity_id' && !array_key_exists($joinTable, $entityTables))) { + throw new \API_Exception("Unable to join $bridgeEntity to $joinEntity"); + } + // Create link between bridge entity and join entity + $joinConditions = [ + "`$bridgeAlias`.`$fkToJoinField` = `$alias`.`id`", + ]; + if ($fkToJoinField === 'entity_id') { + $joinConditions[] = "`$bridgeAlias`.`entity_table` = '$joinTable'"; + } + // Register fields from the bridge entity as if they belong to the join entity + foreach ($bridgeEntityGet->entityFields() as $name => $field) { + if ($name == 'id' || $name == $fkToJoinField || ($name == 'entity_table' && $fkToJoinField == 'entity_id')) { + continue; + } + if ($field['fk_entity'] || (!$fkToBaseField && $name == 'entity_id')) { + $fkToBaseField = $name; + } + // 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); + } + // Move conditions for the bridge join out of the joinTree + $bridgeConditions = []; + $joinTree = array_filter($joinTree, function($clause) use ($fkToBaseField, $alias, $bridgeAlias, &$bridgeConditions) { + list($sideA, $op, $sideB) = array_pad((array) $clause, 3, NULL); + if ($op === '=' && $sideB && ($sideA === "$alias.$fkToBaseField" || $sideB === "$alias.$fkToBaseField")) { + $expr = $sideA === "$alias.$fkToBaseField" ? $sideB : $sideA; + $bridgeConditions[] = "`$bridgeAlias`.`$fkToBaseField` = " . $this->getExpression($expr)->render($this->apiFieldSpec); + return FALSE; + } + elseif ($op === '=' && $fkToBaseField == 'entity_id' && ($sideA === "$alias.entity_table" || $sideB === "$alias.entity_table")) { + $expr = $sideA === "$alias.entity_table" ? $sideB : $sideA; + $bridgeConditions[] = "`$bridgeAlias`.`entity_table` = " . $this->getExpression($expr)->render($this->apiFieldSpec); + return FALSE; + } + return TRUE; + }); + // If no bridge conditions were specified, link it to the base entity + if (!$bridgeConditions) { + $bridgeConditions[] = "`$bridgeAlias`.`$fkToBaseField` = a.id"; + if ($fkToBaseField == 'entity_id') { + if (!array_key_exists($this->getFrom(), $entityTables)) { + throw new \API_Exception("Unable to join $bridgeEntity to " . $this->getEntity()); + } + $bridgeConditions[] = "`$bridgeAlias`.`entity_table` = '" . $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); + } + /** * Joins a path and adds all fields in the joined entity to apiFieldSpec * diff --git a/Civi/Api4/UFMatch.php b/Civi/Api4/UFMatch.php index d056ebb6f5..ae5129d956 100644 --- a/Civi/Api4/UFMatch.php +++ b/Civi/Api4/UFMatch.php @@ -26,6 +26,6 @@ namespace Civi\Api4; * * @package Civi\Api4 */ -class UFMatch extends Generic\DAOEntity { +class UFMatch extends Generic\BridgeEntity { } diff --git a/ang/api4Explorer/Explorer.html b/ang/api4Explorer/Explorer.html index 5b47978683..b32196dfd5 100644 --- a/ang/api4Explorer/Explorer.html +++ b/ang/api4Explorer/Explorer.html @@ -70,10 +70,15 @@
+ + +
-
+
diff --git a/ang/api4Explorer/Explorer.js b/ang/api4Explorer/Explorer.js index 653ee7bd8f..a4ccbe269e 100644 --- a/ang/api4Explorer/Explorer.js +++ b/ang/api4Explorer/Explorer.js @@ -56,7 +56,8 @@ $scope.loading = false; $scope.controls = {}; $scope.langs = ['php', 'js', 'ang', 'cli']; - $scope.joinTypes = [{k: false, v: ts('Optional')}, {k: true, v: ts('Required')}]; + $scope.joinTypes = [{k: false, v: 'FALSE (LEFT JOIN)'}, {k: true, v: 'TRUE (INNER JOIN)'}]; + $scope.bridgeEntities = _.filter(schema, {type: 'BridgeEntity'}); $scope.code = { php: [ {name: 'oop', label: ts('OOP Style'), code: ''}, diff --git a/css/api4-explorer.css b/css/api4-explorer.css index 18dc679278..fa28828058 100644 --- a/css/api4-explorer.css +++ b/css/api4-explorer.css @@ -54,10 +54,6 @@ margin-bottom: 10px; } -#bootstrap-theme.api4-explorer-page .api4-input.form-inline > label { - margin-right: 12px; -} - #bootstrap-theme.api4-explorer-page .explorer-help-panel .panel-body { word-break: break-word; } diff --git a/tests/phpunit/api/v4/Action/FkJoinTest.php b/tests/phpunit/api/v4/Action/FkJoinTest.php index 602942f93d..4f41170ae4 100644 --- a/tests/phpunit/api/v4/Action/FkJoinTest.php +++ b/tests/phpunit/api/v4/Action/FkJoinTest.php @@ -25,7 +25,9 @@ use api\v4\UnitTestCase; use Civi\Api4\Activity; use Civi\Api4\Contact; use Civi\Api4\Email; +use Civi\Api4\EntityTag; use Civi\Api4\Phone; +use Civi\Api4\Tag; /** * @group headless @@ -126,8 +128,10 @@ class FkJoinTest extends UnitTestCase { $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]) + ->setJoin([ + ['Email AS any_email', TRUE, NULL], + ['Email AS primary_email', FALSE, ['primary_email.is_primary', '=', TRUE]], + ]) ->addWhere('id', 'IN', [$cid1, $cid2, $cid3]) ->addOrderBy('any_email.id') ->setDebug(TRUE) @@ -140,4 +144,69 @@ class FkJoinTest extends UnitTestCase { $this->assertEquals('1@test.test', $contacts[4]['primary_email.email']); } + public function testBridgeJoinTags() { + $tag1 = Tag::create()->setCheckPermissions(FALSE) + ->addValue('name', uniqid('join1')) + ->execute() + ->first()['name']; + $tag2 = Tag::create()->setCheckPermissions(FALSE) + ->addValue('name', uniqid('join2')) + ->execute() + ->first()['name']; + $tag3 = Tag::create()->setCheckPermissions(FALSE) + ->addValue('name', uniqid('join3')) + ->execute() + ->first()['name']; + + $cid1 = Contact::create()->setCheckPermissions(FALSE) + ->addValue('first_name', 'Aaa') + ->addChain('tag1', EntityTag::create()->setValues(['entity_id' => '$id', 'tag_id:name' => $tag1])) + ->addChain('tag2', EntityTag::create()->setValues(['entity_id' => '$id', 'tag_id:name' => $tag2])) + ->execute() + ->first()['id']; + $cid2 = Contact::create()->setCheckPermissions(FALSE) + ->addValue('first_name', 'Bbb') + ->addChain('tag1', EntityTag::create()->setValues(['entity_id' => '$id', 'tag_id:name' => $tag1])) + ->addChain('tag3', EntityTag::create()->setValues(['entity_id' => '$id', 'tag_id:name' => $tag3])) + ->execute() + ->first()['id']; + $cid3 = Contact::create()->setCheckPermissions(FALSE) + ->addValue('first_name', 'Ccc') + ->execute() + ->first()['id']; + + $required = Contact::get()->setCheckPermissions(FALSE) + ->addJoin('Tag', TRUE, 'EntityTag') + ->addSelect('first_name', 'tag.name') + ->addWhere('id', 'IN', [$cid1, $cid2, $cid3]) + ->execute(); + $this->assertCount(4, $required); + + $optional = Contact::get()->setCheckPermissions(FALSE) + ->addJoin('Tag', FALSE, 'EntityTag', ['tag.name', 'IN', [$tag1, $tag2, $tag3]]) + ->addSelect('first_name', 'tag.name') + ->addWhere('id', 'IN', [$cid1, $cid2, $cid3]) + ->execute(); + $this->assertCount(5, $optional); + + $grouped = Contact::get()->setCheckPermissions(FALSE) + ->addJoin('Tag', FALSE, 'EntityTag', ['tag.name', 'IN', [$tag1, $tag3]]) + ->addSelect('first_name', 'COUNT(tag.name) AS tags') + ->addWhere('id', 'IN', [$cid1, $cid2, $cid3]) + ->addGroupBy('id') + ->execute()->indexBy('id'); + $this->assertEquals(1, (int) $grouped[$cid1]['tags']); + $this->assertEquals(2, (int) $grouped[$cid2]['tags']); + $this->assertEquals(0, (int) $grouped[$cid3]['tags']); + + $reverse = Tag::get()->setCheckPermissions(FALSE) + ->addJoin('Contact', FALSE, 'EntityTag', ['contact.id', 'IN', [$cid1, $cid2, $cid3]]) + ->addGroupBy('id') + ->addSelect('name', 'COUNT(contact.id) AS contacts') + ->execute()->indexBy('name'); + $this->assertEquals(2, (int) $reverse[$tag1]['contacts']); + $this->assertEquals(1, (int) $reverse[$tag2]['contacts']); + $this->assertEquals(1, (int) $reverse[$tag3]['contacts']); + } + } -- 2.25.1