From 77b6e5ac3377cd1bfb1ebbc18bc9925caf06ad38 Mon Sep 17 00:00:00 2001 From: colemanw Date: Sun, 17 Sep 2023 21:56:12 -0400 Subject: [PATCH] DAO/APIv4 - Optimize ACL clauses Before: no values passed to BAO::addSelectWhereClause After: $entityName and $conditions passed. $entityName is rarely needed, but useful for dynamic entities like ECK. $conditions contains field/value pairs gleaned from the WHERE clause or ON clause (depending on how the entity was added to the query) --- CRM/Activity/BAO/Activity.php | 13 +++- CRM/Case/BAO/Case.php | 4 +- CRM/Case/BAO/CaseContact.php | 4 +- CRM/Contact/BAO/Contact.php | 4 +- CRM/Contact/BAO/Group.php | 4 +- CRM/Contact/BAO/RelationshipCache.php | 4 +- CRM/Contribute/BAO/ContributionSoft.php | 4 +- CRM/Core/BAO/CustomValue.php | 9 ++- CRM/Core/BAO/File.php | 6 +- CRM/Core/BAO/Note.php | 4 +- CRM/Core/BAO/UFJoin.php | 4 +- CRM/Core/BAO/UFMatch.php | 4 +- CRM/Core/BAO/UserJob.php | 4 +- CRM/Core/DAO.php | 63 ++++++++++++++----- CRM/Price/BAO/LineItem.php | 4 +- CRM/Queue/BAO/Queue.php | 2 +- CRM/Utils/Hook.php | 2 +- CRM/Utils/SQL.php | 8 +-- Civi/Api4/Query/Api4SelectQuery.php | 49 +++++++++++---- .../CRM/OAuth/BAO/OAuthContactToken.php | 4 +- tests/phpunit/api/v4/Entity/ActivityTest.php | 15 +++++ 21 files changed, 164 insertions(+), 51 deletions(-) diff --git a/CRM/Activity/BAO/Activity.php b/CRM/Activity/BAO/Activity.php index 3d320dafb8..821034f9a5 100644 --- a/CRM/Activity/BAO/Activity.php +++ b/CRM/Activity/BAO/Activity.php @@ -801,17 +801,24 @@ class CRM_Activity_BAO_Activity extends CRM_Activity_DAO_Activity { } /** + * @param string|null $entityName + * @param array $conditions * @inheritDoc */ - public function addSelectWhereClause(): array { + public function addSelectWhereClause(string $entityName = NULL, array $conditions = []): array { $clauses = []; $permittedActivityTypeIDs = self::getPermittedActivityTypes(); - $allActivityTypes = self::buildOptions('activity_type_id', 'validate'); + if (!empty($conditions['activity_type_id'])) { + $allActivityTypes = (array) $conditions['activity_type_id']; + } + else { + $allActivityTypes = self::buildOptions('activity_type_id', 'validate'); + } if (empty($permittedActivityTypeIDs)) { // This just prevents a mysql fail if they have no access - should be extremely edge case. $permittedActivityTypeIDs = [0]; } - if (array_diff_key($allActivityTypes, $permittedActivityTypeIDs)) { + if (array_diff($allActivityTypes, $permittedActivityTypeIDs)) { $clauses['activity_type_id'] = ['IN (' . implode(', ', $permittedActivityTypeIDs) . ')']; } diff --git a/CRM/Case/BAO/Case.php b/CRM/Case/BAO/Case.php index 68cc1c1a7f..8ffae031ab 100644 --- a/CRM/Case/BAO/Case.php +++ b/CRM/Case/BAO/Case.php @@ -3013,9 +3013,11 @@ WHERE id IN (' . implode(',', $copiedActivityIds) . ')'; } /** + * @param string|null $entityName + * @param array $conditions * @inheritDoc */ - public function addSelectWhereClause(): array { + public function addSelectWhereClause(string $entityName = NULL, array $conditions = []): array { // We always return an array with these keys, even if they are empty, // because this tells the query builder that we have considered these fields for acls $clauses = [ diff --git a/CRM/Case/BAO/CaseContact.php b/CRM/Case/BAO/CaseContact.php index d1fa6ceb25..5cdd8aa6d3 100644 --- a/CRM/Case/BAO/CaseContact.php +++ b/CRM/Case/BAO/CaseContact.php @@ -71,9 +71,11 @@ class CRM_Case_BAO_CaseContact extends CRM_Case_DAO_CaseContact implements \Civi } /** + * @param string|null $entityName + * @param array $conditions * @inheritDoc */ - public function addSelectWhereClause(): array { + public function addSelectWhereClause(string $entityName = NULL, array $conditions = []): array { return [ // Reuse case acls 'case_id' => CRM_Utils_SQL::mergeSubquery('Case'), diff --git a/CRM/Contact/BAO/Contact.php b/CRM/Contact/BAO/Contact.php index f5635b1e5e..7fcef7ca85 100644 --- a/CRM/Contact/BAO/Contact.php +++ b/CRM/Contact/BAO/Contact.php @@ -3409,9 +3409,11 @@ LEFT JOIN civicrm_address ON ( civicrm_address.contact_id = civicrm_contact.id ) } /** + * @param string|null $entityName + * @param array $conditions * @inheritDoc */ - public function addSelectWhereClause(): array { + public function addSelectWhereClause(string $entityName = NULL, array $conditions = []): array { // We always return an array with these keys, even if they are empty, // because this tells the query builder that we have considered these fields for acls $clauses = [ diff --git a/CRM/Contact/BAO/Group.php b/CRM/Contact/BAO/Group.php index 617d09e4ef..cf25ab4c65 100644 --- a/CRM/Contact/BAO/Group.php +++ b/CRM/Contact/BAO/Group.php @@ -310,9 +310,11 @@ class CRM_Contact_BAO_Group extends CRM_Contact_DAO_Group { } /** + * @param string|null $entityName + * @param array $conditions * @inheritDoc */ - public function addSelectWhereClause(): array { + public function addSelectWhereClause(string $entityName = NULL, array $conditions = []): array { $clauses = []; if (!CRM_Core_Permission::check([['edit all contacts', 'view all contacts']])) { $allowedGroups = CRM_Core_Permission::group(NULL, FALSE); diff --git a/CRM/Contact/BAO/RelationshipCache.php b/CRM/Contact/BAO/RelationshipCache.php index b7fbaa2690..37bace2c9d 100644 --- a/CRM/Contact/BAO/RelationshipCache.php +++ b/CRM/Contact/BAO/RelationshipCache.php @@ -156,9 +156,11 @@ class CRM_Contact_BAO_RelationshipCache extends CRM_Contact_DAO_RelationshipCach } /** + * @param string|null $entityName + * @param array $conditions * @return array */ - public function addSelectWhereClause(): array { + public function addSelectWhereClause(string $entityName = NULL, array $conditions = []): array { // Permission for this entity depends on access to the two related contacts. $contactClause = CRM_Utils_SQL::mergeSubquery('Contact'); $clauses = [ diff --git a/CRM/Contribute/BAO/ContributionSoft.php b/CRM/Contribute/BAO/ContributionSoft.php index 5843115465..d5203ea5b0 100644 --- a/CRM/Contribute/BAO/ContributionSoft.php +++ b/CRM/Contribute/BAO/ContributionSoft.php @@ -646,9 +646,11 @@ class CRM_Contribute_BAO_ContributionSoft extends CRM_Contribute_DAO_Contributio } /** + * @param string|null $entityName + * @param array $conditions * @inheritDoc */ - public function addSelectWhereClause(): array { + public function addSelectWhereClause(string $entityName = NULL, array $conditions = []): array { $clauses['contribution_id'] = CRM_Utils_SQL::mergeSubquery('Contribution'); CRM_Utils_Hook::selectWhereClause($this, $clauses); return $clauses; diff --git a/CRM/Core/BAO/CustomValue.php b/CRM/Core/BAO/CustomValue.php index 9a00a61b68..c225665baa 100644 --- a/CRM/Core/BAO/CustomValue.php +++ b/CRM/Core/BAO/CustomValue.php @@ -212,13 +212,18 @@ class CRM_Core_BAO_CustomValue extends CRM_Core_DAO { /** * ACL clause for an APIv4 custom pseudo-entity (aka multi-record custom group extending Contact). + * @param string|null $entityName + * @param array $conditions * @return array */ - public function addSelectWhereClause(): array { + public function addSelectWhereClause(string $entityName = NULL, array $conditions = []): array { + // To-date, custom-value-based entities are only supported for contacts. + // If this changes, $entityName variable contains the name of this custom group, + // and could be used to lookup the type of entity this custom group joins to. $clauses = [ 'entity_id' => CRM_Utils_SQL::mergeSubquery('Contact'), ]; - CRM_Utils_Hook::selectWhereClause($this, $clauses); + CRM_Utils_Hook::selectWhereClause($entityName ?? $this, $clauses); return $clauses; } diff --git a/CRM/Core/BAO/File.php b/CRM/Core/BAO/File.php index 6c24a6078d..555f15d00c 100644 --- a/CRM/Core/BAO/File.php +++ b/CRM/Core/BAO/File.php @@ -835,14 +835,16 @@ HEREDOC; } /** + * @param string|null $entityName + * @param array $conditions * @inheritDoc */ - public function addSelectWhereClause(): array { + public function addSelectWhereClause(string $entityName = NULL, array $conditions = []): array { // TODO: This seemded like a good idea... piggybacking off the ACL clause of EntityFile // however that's too restrictive because entityFile ACLs are limited to just attachments, // so this would prevent access to other file fields (e.g. custom fields) // Disabling this function for now by calling the parent instead. - return parent::addSelectWhereClause(); + return parent::addSelectWhereClause($entityName, $conditions); $clauses = [ 'id' => [], ]; diff --git a/CRM/Core/BAO/Note.php b/CRM/Core/BAO/Note.php index d84e9b320f..bcb71d37ec 100644 --- a/CRM/Core/BAO/Note.php +++ b/CRM/Core/BAO/Note.php @@ -526,9 +526,9 @@ WHERE participant.contact_id = %1 AND note.entity_table = 'civicrm_participant' } } - public function addSelectWhereClause(): array { + public function addSelectWhereClause(string $entityName = NULL, array $conditions = []): array { $clauses = []; - $relatedClauses = self::getDynamicFkAclClauses('entity_table', 'entity_id'); + $relatedClauses = self::getDynamicFkAclClauses('entity_table', 'entity_id', $conditions['entity_table'] ?? NULL); if ($relatedClauses) { // Nested array will be joined with OR $clauses['entity_table'] = [$relatedClauses]; diff --git a/CRM/Core/BAO/UFJoin.php b/CRM/Core/BAO/UFJoin.php index d89e2cd46b..422e6d2e45 100644 --- a/CRM/Core/BAO/UFJoin.php +++ b/CRM/Core/BAO/UFJoin.php @@ -178,9 +178,11 @@ class CRM_Core_BAO_UFJoin extends CRM_Core_DAO_UFJoin { /** * Override base method which assumes permissions should be based on entity_table. * + * @param string|null $entityName + * @param array $conditions * @return array */ - public function addSelectWhereClause(): array { + public function addSelectWhereClause(string $entityName = NULL, array $conditions = []): array { $clauses = []; CRM_Utils_Hook::selectWhereClause($this, $clauses); return $clauses; diff --git a/CRM/Core/BAO/UFMatch.php b/CRM/Core/BAO/UFMatch.php index 4c2317222c..3cffbc2260 100644 --- a/CRM/Core/BAO/UFMatch.php +++ b/CRM/Core/BAO/UFMatch.php @@ -616,9 +616,11 @@ AND domain_id = %4 } /** + * @param string|null $entityName + * @param array $conditions * @inheritDoc */ - public function addSelectWhereClause(): array { + public function addSelectWhereClause(string $entityName = NULL, array $conditions = []): array { // Prevent default behavior of joining ACLs onto the contact_id field. $clauses = []; CRM_Utils_Hook::selectWhereClause($this, $clauses); diff --git a/CRM/Core/BAO/UserJob.php b/CRM/Core/BAO/UserJob.php index 9979e6c73e..7e99bf6f75 100644 --- a/CRM/Core/BAO/UserJob.php +++ b/CRM/Core/BAO/UserJob.php @@ -147,9 +147,11 @@ class CRM_Core_BAO_UserJob extends CRM_Core_DAO_UserJob implements HookInterface * use an existing permission? a new permission ? do they require * 'view all contacts' etc. * + * @param string|null $entityName + * @param array $conditions * @inheritDoc */ - public function addSelectWhereClause(): array { + public function addSelectWhereClause(string $entityName = NULL, array $conditions = []): array { $clauses = []; if (!\CRM_Core_Permission::check('administer queues')) { // @todo - the is_template should really be prefixed. We need to add support diff --git a/CRM/Core/DAO.php b/CRM/Core/DAO.php index 7568c03f6e..289ede21b5 100644 --- a/CRM/Core/DAO.php +++ b/CRM/Core/DAO.php @@ -3070,11 +3070,19 @@ SELECT contact_id * ``` * * Note that all array keys must be actual field names in this entity. Use subqueries to filter on other tables e.g. custom values. - * The query strings MAY reference other fields in this entity; but they must be enclosed in {curly_braces}. - * + * The query strings MAY reference other fields in this entity; they must be enclosed in {curly_braces}. + * + * @param string|null $entityName + * Name of the entity being queried (for normal BAO files implementing this method, this variable is redundant + * as there is a 1-1 relationship between most entities and most BAOs. However the variable is passed in to support + * dynamic entities such as ECK). + * @param array $conditions + * Contains field/value pairs gleaned from the WHERE clause or ON clause + * (depending on how the entity was added to the query). + * Can be used for optimization/deduping of clauses. * @return array */ - public function addSelectWhereClause(): array { + public function addSelectWhereClause(string $entityName = NULL, array $conditions = []): array { $clauses = []; $fields = $this::getSupportedFields(); foreach ($fields as $fieldName => $field) { @@ -3087,21 +3095,44 @@ SELECT contact_id } // Clause for an entity_table/entity_id combo if ($fieldName === 'entity_id' && isset($fields['entity_table'])) { - $relatedClauses = self::getDynamicFkAclClauses('entity_table', 'entity_id'); + $relatedClauses = self::getDynamicFkAclClauses('entity_table', 'entity_id', $conditions['entity_table'] ?? NULL); if ($relatedClauses) { // Nested array will be joined with OR $clauses['entity_table'] = [$relatedClauses]; } } } - CRM_Utils_Hook::selectWhereClause($this, $clauses); + CRM_Utils_Hook::selectWhereClause($entityName ?? $this, $clauses); return $clauses; } - protected static function getDynamicFkAclClauses($entityTableField, $entityIdField): array { + /** + * Get an array of ACL clauses for a dynamic FK (entity_id/entity_table combo) + * + * @param string $entityTableField + * @param string $entityIdField + * @param mixed|NULL $entityTableValues + * @return array + */ + protected static function getDynamicFkAclClauses(string $entityTableField, string $entityIdField, $entityTableValues = NULL): array { + // If entity_table is specified in the WHERE clause, use that instead of the entity_table pseudoconstant + if ($entityTableValues && is_string($entityTableValues) || is_array($entityTableValues)) { + // Ideally we would validate table names against the entity_table pseudoconstant, + // but some entities have missing/incomplete metadata and it's better to generate an ACL + // clause for what we have than no ACL clause at all, so validate against all known tables. + $allTableNames = CRM_Core_DAO_AllCoreTables::tables(); + $relatedEntities = array_intersect_key(array_flip((array) $entityTableValues), $allTableNames); + } + // No valid entity_table in WHERE clause so build an ACL case for every possible entity type + if (empty($relatedEntities)) { + $relatedEntities = static::buildOptions($entityTableField, 'get'); + } + // Hmm, this entity is missing entity_table pseudoconstant. We really should fix that. + if (!$relatedEntities) { + return []; + } $relatedClauses = []; - $relatedEntities = static::buildOptions($entityTableField, 'get'); - foreach ((array) $relatedEntities as $table => $ent) { + foreach ($relatedEntities as $table => $ent) { // Ensure $ent is the machine name of the entity not a translated title $ent = CRM_Core_DAO_AllCoreTables::getEntityNameForTable($table); // Prevent infinite recursion @@ -3109,7 +3140,8 @@ SELECT contact_id if ($subquery) { $relatedClauses[] = "= '$table' AND {{$entityIdField}} " . implode(" AND {{$entityIdField}} ", $subquery); } - else { + // If it's the only value with no conditions, don't need to add it + elseif (!$entityTableValues || count($relatedEntities) > 1) { $relatedClauses[] = "= '$table'"; } } @@ -3122,16 +3154,19 @@ SELECT contact_id * With acls from related entities + additional clauses from hook_civicrm_selectWhereClause * * @param string $tableAlias + * @param string $entityName + * @param array $conditions + * Values from WHERE or ON clause * @return array */ - public static function getSelectWhereClause($tableAlias = NULL) { + public static function getSelectWhereClause($tableAlias = NULL, $entityName = NULL, $conditions = []) { $bao = new static(); - if ($tableAlias === NULL) { - $tableAlias = $bao->tableName(); - } + $tableAlias = $tableAlias ?? $bao->tableName(); + $entityName = $entityName ?? CRM_Core_DAO_AllCoreTables::getBriefName($bao::class); $finalClauses = []; $fields = static::getSupportedFields(); - foreach ((array) $bao->addSelectWhereClause() as $fieldName => $fieldClauses) { + $selectWhereClauses = $bao->addSelectWhereClause($entityName, $conditions); + foreach ($selectWhereClauses as $fieldName => $fieldClauses) { $finalClauses[$fieldName] = NULL; if ($fieldClauses) { if (!is_array($fieldClauses)) { diff --git a/CRM/Price/BAO/LineItem.php b/CRM/Price/BAO/LineItem.php index 4bfbaebdfd..fbc37be9fa 100644 --- a/CRM/Price/BAO/LineItem.php +++ b/CRM/Price/BAO/LineItem.php @@ -1279,9 +1279,11 @@ WHERE li.contribution_id = %1"; * clauses being added. Additional filters joining on the participant * and membership tables just seem too non-performant. * + * @param string|null $entityName + * @param array $conditions * @inheritDoc */ - public function addSelectWhereClause(): array { + public function addSelectWhereClause(string $entityName = NULL, array $conditions = []): array { $clauses['contribution_id'] = CRM_Utils_SQL::mergeSubquery('Contribution'); CRM_Utils_Hook::selectWhereClause($this, $clauses); return $clauses; diff --git a/CRM/Queue/BAO/Queue.php b/CRM/Queue/BAO/Queue.php index 665c9908eb..af942b9a90 100644 --- a/CRM/Queue/BAO/Queue.php +++ b/CRM/Queue/BAO/Queue.php @@ -20,7 +20,7 @@ */ class CRM_Queue_BAO_Queue extends CRM_Queue_DAO_Queue implements \Civi\Core\HookInterface { - public function addSelectWhereClause(): array { + public function addSelectWhereClause(string $entityName = NULL, array $conditions = []): array { $clauses = []; if (!\CRM_Core_Permission::check('administer queues')) { $cid = (int) CRM_Core_Session::getLoggedInContactID(); diff --git a/CRM/Utils/Hook.php b/CRM/Utils/Hook.php index 71d75c3fc2..b263f0e6fe 100644 --- a/CRM/Utils/Hook.php +++ b/CRM/Utils/Hook.php @@ -630,7 +630,7 @@ abstract class CRM_Utils_Hook { * @return mixed */ public static function selectWhereClause($entity, &$clauses) { - $entityName = is_object($entity) ? _civicrm_api_get_entity_name_from_dao($entity) : $entity; + $entityName = is_object($entity) ? CRM_Core_DAO_AllCoreTables::getBriefName($entity::class) : $entity; $null = NULL; return self::singleton()->invoke(['entity', 'clauses'], $entityName, $clauses, $null, $null, $null, $null, diff --git a/CRM/Utils/SQL.php b/CRM/Utils/SQL.php index 0c50dd795a..5f7311d609 100644 --- a/CRM/Utils/SQL.php +++ b/CRM/Utils/SQL.php @@ -52,16 +52,16 @@ class CRM_Utils_SQL { /** * Helper function for adding the permissioned subquery from one entity onto another * - * @param string $entity + * @param string $entityName * @param string $joinColumn * @return array */ - public static function mergeSubquery($entity, $joinColumn = 'id') { - $baoName = CRM_Core_DAO_AllCoreTables::getBAOClassName(CRM_Core_DAO_AllCoreTables::getFullName($entity)); + public static function mergeSubquery($entityName, $joinColumn = 'id') { + $baoName = CRM_Core_DAO_AllCoreTables::getBAOClassName(CRM_Core_DAO_AllCoreTables::getFullName($entityName)); $bao = new $baoName(); $fields = $bao::getSupportedFields(); $mergeClauses = $subClauses = []; - foreach ((array) $bao->addSelectWhereClause() as $fieldName => $fieldClauses) { + foreach ((array) $bao->addSelectWhereClause($entityName) as $fieldName => $fieldClauses) { if ($fieldClauses) { foreach ((array) $fieldClauses as $fieldClause) { $formattedClause = CRM_Utils_SQL::prefixFieldNames($fieldClause, array_keys($fields), $bao->tableName()); diff --git a/Civi/Api4/Query/Api4SelectQuery.php b/Civi/Api4/Query/Api4SelectQuery.php index 2b5cf1c0a1..2d42a34960 100644 --- a/Civi/Api4/Query/Api4SelectQuery.php +++ b/Civi/Api4/Query/Api4SelectQuery.php @@ -85,8 +85,7 @@ class Api4SelectQuery extends Api4Query { $this->entityAccess[$this->getEntity()] = TRUE; // Add ACLs first to avoid redundant subclauses - $baoName = CoreUtil::getBAOFromApiName($this->getEntity()); - $this->query->where($this->getAclClause(self::MAIN_TABLE_ALIAS, $baoName)); + $this->query->where($this->getAclClause(self::MAIN_TABLE_ALIAS, $this->getEntity(), [], $this->getWhere())); // Add explicit joins. Other joins implied by dot notation may be added later $this->addExplicitJoins(); @@ -323,11 +322,12 @@ class Api4SelectQuery extends Api4Query { * Get acl clause for an entity * * @param string $tableAlias - * @param \CRM_Core_DAO|string $baoName + * @param string $entityName * @param array $stack + * @param array[] $conditions * @return array */ - public function getAclClause($tableAlias, $baoName, $stack = []) { + public function getAclClause($tableAlias, $entityName, $stack = [], $conditions = []) { if (!$this->getCheckPermissions()) { return []; } @@ -337,7 +337,28 @@ class Api4SelectQuery extends Api4Query { if (count($stack) === 1 && in_array(reset($stack), $this->aclFields, TRUE)) { return []; } - $clauses = $baoName::getSelectWhereClause($tableAlias); + // Glean entity values from the WHERE or ON clause conditions + $entityValues = []; + foreach ($conditions as $condition) { + [$fieldExpr, $operator, $valueExpr, $isExpr] = array_pad((array) $condition, 4, NULL); + if (in_array($operator, ['=', 'IN'], TRUE)) { + // If flag is set then value must be parsed as an expression + if ($isExpr) { + $expr = SqlExpression::convert($valueExpr); + $valueExpr = in_array($expr->getType(), ['SqlString', 'SqlNumber'], TRUE) ? $expr->getExpr() : NULL; + } + if (isset($valueExpr)) { + [$fieldPath] = explode(':', $fieldExpr); + $fieldSpec = $this->getField($fieldPath); + $entityValues[$fieldPath] = $valueExpr; + if ($fieldSpec) { + FormattingUtil::formatInputValue($entityValues[$fieldPath], $fieldExpr, $fieldSpec, $entityValues, $operator); + } + } + } + } + $baoName = CoreUtil::getBAOFromApiName($entityName); + $clauses = $baoName::getSelectWhereClause($tableAlias, $entityName, $entityValues); if (!$stack) { // Track field clauses added to the main entity $this->aclFields = array_keys($clauses); @@ -542,8 +563,15 @@ class Api4SelectQuery extends Api4Query { $conditions = []; } } - $baoName = CoreUtil::getBAOFromApiName($joinEntity); - $acls = array_values($this->getAclClause($alias, $baoName, $aclStack)); + // Gather join conditions to help optimize aclClause + $joinOn = []; + foreach ($joinTree as $clause) { + if (is_array($clause) && isset($clause[2])) { + // Set 4th param ($isExpr) default to TRUE because this is an ON clause + $joinOn[] = array_pad($clause, 4, TRUE); + } + } + $acls = array_values($this->getAclClause($alias, $joinEntity, $aclStack, $joinOn)); return array_merge($acls, $conditions); } @@ -575,7 +603,7 @@ class Api4SelectQuery extends Api4Query { $bridgeConditions = $this->getBridgeJoinConditions($joinTree, $baseRef, $alias, $bridgeAlias, $bridgeEntity, $aclStack); - $acls = array_values($this->getAclClause($alias, CoreUtil::getBAOFromApiName($joinEntity), $aclStack)); + $acls = array_values($this->getAclClause($alias, $joinEntity, $aclStack)); $outerConditions = []; foreach (array_filter($joinTree) as $clause) { @@ -825,10 +853,9 @@ class Api4SelectQuery extends Api4Query { // Serialized joins are rendered by this::renderSerializedJoin. Don't add their tables. if (!$virtualField) { - $bao = $joinEntity ? CoreUtil::getBAOFromApiName($joinEntity) : NULL; $conditions = $link->getConditionsForJoin($baseTableAlias, $tableAlias); - if ($bao) { - $conditions = array_merge($conditions, $this->getAclClause($tableAlias, $bao, $joinPath)); + if ($joinEntity) { + $conditions = array_merge($conditions, $this->getAclClause($tableAlias, $joinEntity, $joinPath)); } $this->addJoin('LEFT', $target, $tableAlias, $baseTableAlias, $conditions); } diff --git a/ext/oauth-client/CRM/OAuth/BAO/OAuthContactToken.php b/ext/oauth-client/CRM/OAuth/BAO/OAuthContactToken.php index 9786a68c1b..e9f633ea74 100644 --- a/ext/oauth-client/CRM/OAuth/BAO/OAuthContactToken.php +++ b/ext/oauth-client/CRM/OAuth/BAO/OAuthContactToken.php @@ -97,9 +97,11 @@ class CRM_OAuth_BAO_OAuthContactToken extends CRM_OAuth_DAO_OAuthContactToken { } /** + * @param string|null $entityName + * @param array $conditions * @inheritDoc */ - public function addSelectWhereClause(): array { + public function addSelectWhereClause(string $entityName = NULL, array $conditions = []): array { $clauses = []; $loggedInContactID = CRM_Core_Session::getLoggedInContactID(); diff --git a/tests/phpunit/api/v4/Entity/ActivityTest.php b/tests/phpunit/api/v4/Entity/ActivityTest.php index 56625cdcea..ece27a3ae7 100644 --- a/tests/phpunit/api/v4/Entity/ActivityTest.php +++ b/tests/phpunit/api/v4/Entity/ActivityTest.php @@ -86,7 +86,22 @@ class ActivityTest extends Api4TestBase implements TransactionalInterface { $this->assertEquals($sourceContactId, $contactGet['activity.source_contact_id']); $this->assertEquals($targetContactIds, $contactGet['activity.target_contact_id']); $this->assertEquals($assigneeContactIds, $contactGet['activity.assignee_contact_id']); + } + public function testAllowedActivityTypes(): void { + // No access to CiviCase, etc will result in a limited number of activity types + \CRM_Core_Config::singleton()->userPermissionClass->permissions = ['access CiviCRM', 'view all contacts', 'view debug output']; + $result = Activity::get() + ->setDebug(TRUE) + ->execute(); + // SQL includes a constraint listing some activity type ids + $this->assertMatchesRegularExpression('/activity_type_id[` ]*IN[ ]*\([ \d,]{9}/', $result->debug['sql'][0]); + $result = Activity::get() + ->addWhere('activity_type_id:name', '=', 'Meeting') + ->setDebug(TRUE) + ->execute(); + // Constraint is redundant with WHERE clause so should not have been included + $this->assertDoesNotMatchRegularExpression('/activity_type_id[` ]*IN[ ]*\([ \d,]{9}/', $result->debug['sql'][0]); } } -- 2.25.1