From 3c7c8fa6a4eb0fc9ac7d2de6ce7cc6dd686bb759 Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Sun, 22 Mar 2020 22:32:04 -0400 Subject: [PATCH] APIv4 - Cleanup and restructure the way get query objects are constructed --- CRM/Contact/BAO/GroupContactCache.php | 10 ++---- Civi/Api4/Generic/DAODeleteAction.php | 2 +- Civi/Api4/Generic/DAOGetAction.php | 15 ++++++++ Civi/Api4/Generic/DAOUpdateAction.php | 2 +- Civi/Api4/Generic/Traits/DAOActionTrait.php | 21 ------------ Civi/Api4/Query/Api4SelectQuery.php | 34 ++++++++++--------- .../Query/Api4SelectQueryComplexJoinTest.php | 11 +++--- .../api/v4/Query/Api4SelectQueryTest.php | 12 ++++--- .../api/v4/Query/OptionValueJoinTest.php | 3 +- 9 files changed, 54 insertions(+), 56 deletions(-) diff --git a/CRM/Contact/BAO/GroupContactCache.php b/CRM/Contact/BAO/GroupContactCache.php index 3234710a6b..c97afa799b 100644 --- a/CRM/Contact/BAO/GroupContactCache.php +++ b/CRM/Contact/BAO/GroupContactCache.php @@ -726,13 +726,9 @@ ORDER BY gc.contact_id, g.children * @throws CRM_Core_Exception */ protected static function getApiSQL($savedSearchID, array $savedSearch): array { - $api = \Civi\API\Request::create($savedSearch['api_entity'], 'get', $savedSearch['api_params']); - $query = new \Civi\Api4\Query\Api4SelectQuery($api->getEntityName(), FALSE, $api->entityFields()); - $query->select = ['id']; - $query->where = $api->getWhere(); - $query->orderBy = $api->getOrderBy(); - $query->limit = $api->getLimit(); - $query->offset = $api->getOffset(); + $apiParams = ['select' => ['id'], 'checkPermissions' => FALSE] + $savedSearch['api_params']; + $api = \Civi\API\Request::create($savedSearch['api_entity'], 'get', $apiParams); + $query = new \Civi\Api4\Query\Api4SelectQuery($api); $sql = $query->getSql(); return [ 'select' => substr($sql, 0, strpos($sql, 'FROM')), diff --git a/Civi/Api4/Generic/DAODeleteAction.php b/Civi/Api4/Generic/DAODeleteAction.php index 79bc911c4a..666e702993 100644 --- a/Civi/Api4/Generic/DAODeleteAction.php +++ b/Civi/Api4/Generic/DAODeleteAction.php @@ -38,7 +38,7 @@ class DAODeleteAction extends AbstractBatchAction { throw new \API_Exception('Cannot delete ' . $this->getEntityName() . ' with no "where" parameter specified'); } - $items = $this->getObjects(); + $items = $this->getBatchRecords(); if ($items) { $result->exchangeArray($this->deleteObjects($items)); } diff --git a/Civi/Api4/Generic/DAOGetAction.php b/Civi/Api4/Generic/DAOGetAction.php index c7472969f4..100173d962 100644 --- a/Civi/Api4/Generic/DAOGetAction.php +++ b/Civi/Api4/Generic/DAOGetAction.php @@ -21,6 +21,8 @@ namespace Civi\Api4\Generic; +use Civi\Api4\Query\Api4SelectQuery; + /** * Retrieve $ENTITIES based on criteria specified in the `where` parameter. * @@ -48,4 +50,17 @@ class DAOGetAction extends AbstractGetAction { $result->exchangeArray($this->getObjects()); } + /** + * @return array|int + */ + protected function getObjects() { + $query = new Api4SelectQuery($this); + + $result = $query->run(); + if (is_array($result)) { + \CRM_Utils_API_HTMLInputCoder::singleton()->decodeRows($result); + } + return $result; + } + } diff --git a/Civi/Api4/Generic/DAOUpdateAction.php b/Civi/Api4/Generic/DAOUpdateAction.php index b2768ee4d1..5fe1cda280 100644 --- a/Civi/Api4/Generic/DAOUpdateAction.php +++ b/Civi/Api4/Generic/DAOUpdateAction.php @@ -66,7 +66,7 @@ class DAOUpdateAction extends AbstractUpdateAction { } // Batch update 1 or more records based on WHERE clause - $items = $this->getObjects(); + $items = $this->getBatchRecords(); foreach ($items as &$item) { $item = $this->values + $item; } diff --git a/Civi/Api4/Generic/Traits/DAOActionTrait.php b/Civi/Api4/Generic/Traits/DAOActionTrait.php index 065e8a0884..b8cb994feb 100644 --- a/Civi/Api4/Generic/Traits/DAOActionTrait.php +++ b/Civi/Api4/Generic/Traits/DAOActionTrait.php @@ -21,7 +21,6 @@ namespace Civi\Api4\Generic\Traits; use Civi\Api4\Utils\FormattingUtil; -use Civi\Api4\Query\Api4SelectQuery; /** * @method string getLanguage() @@ -79,26 +78,6 @@ trait DAOActionTrait { return $values; } - /** - * @return array|int - */ - protected function getObjects() { - $query = new Api4SelectQuery($this->getEntityName(), $this->getCheckPermissions(), $this->entityFields()); - $query->select = $this->getSelect(); - $query->where = $this->getWhere(); - $query->orderBy = $this->getOrderBy(); - $query->limit = $this->getLimit(); - $query->offset = $this->getOffset(); - if ($this->getDebug()) { - $query->debugOutput =& $this->_debugOutput; - } - $result = $query->run(); - if (is_array($result)) { - \CRM_Utils_API_HTMLInputCoder::singleton()->decodeRows($result); - } - return $result; - } - /** * Fill field defaults which were declared by the api. * diff --git a/Civi/Api4/Query/Api4SelectQuery.php b/Civi/Api4/Query/Api4SelectQuery.php index 1291726b87..db5016e880 100644 --- a/Civi/Api4/Query/Api4SelectQuery.php +++ b/Civi/Api4/Query/Api4SelectQuery.php @@ -62,22 +62,24 @@ class Api4SelectQuery extends SelectQuery { public $debugOutput = NULL; /** - * @param string $entity - * @param bool $checkPermissions - * @param array $fields - */ - public function __construct($entity, $checkPermissions, $fields) { - require_once 'api/v3/utils.php'; - $this->entity = $entity; - $this->checkPermissions = $checkPermissions; - - $baoName = CoreUtil::getBAOFromApiName($entity); - $bao = new $baoName(); - - $this->entityFieldNames = _civicrm_api3_field_names(_civicrm_api3_build_fields_array($bao)); - $this->apiFieldSpec = (array) $fields; + * @param \Civi\Api4\Generic\DAOGetAction $apiGet + */ + public function __construct($apiGet) { + $this->entity = $apiGet->getEntityName(); + $this->checkPermissions = $apiGet->getCheckPermissions(); + $this->select = $apiGet->getSelect(); + $this->where = $apiGet->getWhere(); + $this->orderBy = $apiGet->getOrderBy(); + $this->limit = $apiGet->getLimit(); + $this->offset = $apiGet->getOffset(); + if ($apiGet->getDebug()) { + $this->debugOutput =& $apiGet->_debugOutput; + } + $baoName = CoreUtil::getBAOFromApiName($this->entity); + $this->entityFieldNames = array_column($baoName::fields(), 'name'); + $this->apiFieldSpec = $apiGet->entityFields(); - \CRM_Utils_SQL_Select::from($this->getTableName($baoName) . ' ' . self::MAIN_TABLE_ALIAS); + $this->constructQueryObject($baoName); // Add ACLs first to avoid redundant subclauses $this->query->where($this->getAclClause(self::MAIN_TABLE_ALIAS, $baoName)); @@ -554,7 +556,7 @@ class Api4SelectQuery extends SelectQuery { * * @return void */ - public function getTableName($baoName) { + public function constructQueryObject($baoName) { if (strstr($this->entity, 'Custom_')) { $this->query = \CRM_Utils_SQL_Select::from(CoreUtil::getCustomTableByName(str_replace('Custom_', '', $this->entity)) . ' ' . self::MAIN_TABLE_ALIAS); $this->entityFieldNames = array_keys($this->apiFieldSpec); diff --git a/tests/phpunit/api/v4/Query/Api4SelectQueryComplexJoinTest.php b/tests/phpunit/api/v4/Query/Api4SelectQueryComplexJoinTest.php index 3b08d4577c..c466cee34b 100644 --- a/tests/phpunit/api/v4/Query/Api4SelectQueryComplexJoinTest.php +++ b/tests/phpunit/api/v4/Query/Api4SelectQueryComplexJoinTest.php @@ -46,8 +46,8 @@ class Api4SelectQueryComplexJoinTest extends UnitTestCase { } public function testWithComplexRelatedEntitySelect() { - $query = new Api4SelectQuery('Contact', FALSE, civicrm_api4('Contact', 'getFields', ['includeCustom' => FALSE, 'checkPermissions' => FALSE, 'action' => 'get'], 'name')); - $query->select[] = 'id'; + $api = \Civi\API\Request::create('Contact', 'get', ['version' => 4, 'checkPermissions' => FALSE]); + $query = new Api4SelectQuery($api); $query->select[] = 'id'; $query->select[] = 'display_name'; $query->select[] = 'phones.*_id'; $query->select[] = 'emails.email'; @@ -83,8 +83,8 @@ class Api4SelectQueryComplexJoinTest extends UnitTestCase { } public function testWithSelectOfOrphanDeepValues() { - $query = new Api4SelectQuery('Contact', FALSE, civicrm_api4('Contact', 'getFields', ['includeCustom' => FALSE, 'checkPermissions' => FALSE, 'action' => 'get'], 'name')); - $query->select[] = 'id'; + $api = \Civi\API\Request::create('Contact', 'get', ['version' => 4, 'checkPermissions' => FALSE]); + $query = new Api4SelectQuery($api); $query->select[] = 'id'; $query->select[] = 'first_name'; // emails not selected $query->select[] = 'emails.location_type.name'; @@ -95,7 +95,8 @@ class Api4SelectQueryComplexJoinTest extends UnitTestCase { } public function testOrderDoesNotMatter() { - $query = new Api4SelectQuery('Contact', FALSE, civicrm_api4('Contact', 'getFields', ['includeCustom' => FALSE, 'checkPermissions' => FALSE, 'action' => 'get'], 'name')); + $api = \Civi\API\Request::create('Contact', 'get', ['version' => 4, 'checkPermissions' => FALSE]); + $query = new Api4SelectQuery($api); $query->select[] = 'id'; $query->select[] = 'first_name'; // before emails selection diff --git a/tests/phpunit/api/v4/Query/Api4SelectQueryTest.php b/tests/phpunit/api/v4/Query/Api4SelectQueryTest.php index 85d8d9c413..d82250811a 100644 --- a/tests/phpunit/api/v4/Query/Api4SelectQueryTest.php +++ b/tests/phpunit/api/v4/Query/Api4SelectQueryTest.php @@ -51,7 +51,8 @@ class Api4SelectQueryTest extends UnitTestCase { public function testWithSingleWhereJoin() { $phoneNum = $this->getReference('test_phone_1')['phone']; - $query = new Api4SelectQuery('Contact', FALSE, civicrm_api4('Contact', 'getFields', ['includeCustom' => FALSE, 'checkPermissions' => FALSE, 'action' => 'get'], 'name')); + $api = \Civi\API\Request::create('Contact', 'get', ['version' => 4, 'checkPermissions' => FALSE]); + $query = new Api4SelectQuery($api); $query->where[] = ['phones.phone', '=', $phoneNum]; $results = $query->run(); @@ -61,7 +62,8 @@ class Api4SelectQueryTest extends UnitTestCase { public function testOneToManyJoin() { $phoneNum = $this->getReference('test_phone_1')['phone']; - $query = new Api4SelectQuery('Contact', FALSE, civicrm_api4('Contact', 'getFields', ['includeCustom' => FALSE, 'checkPermissions' => FALSE, 'action' => 'get'], 'name')); + $api = \Civi\API\Request::create('Contact', 'get', ['version' => 4, 'checkPermissions' => FALSE]); + $query = new Api4SelectQuery($api); $query->select[] = 'id'; $query->select[] = 'first_name'; $query->select[] = 'phones.phone'; @@ -79,7 +81,8 @@ class Api4SelectQueryTest extends UnitTestCase { $phoneNum = $this->getReference('test_phone_1')['phone']; $contact = $this->getReference('test_contact_1'); - $query = new Api4SelectQuery('Phone', FALSE, civicrm_api4('Phone', 'getFields', ['includeCustom' => FALSE, 'checkPermissions' => FALSE, 'action' => 'get'], 'name')); + $api = \Civi\API\Request::create('Phone', 'get', ['version' => 4, 'checkPermissions' => FALSE]); + $query = new Api4SelectQuery($api); $query->select[] = 'id'; $query->select[] = 'phone'; $query->select[] = 'contact.display_name'; @@ -93,7 +96,8 @@ class Api4SelectQueryTest extends UnitTestCase { } public function testOneToManyMultipleJoin() { - $query = new Api4SelectQuery('Contact', FALSE, civicrm_api4('Contact', 'getFields', ['includeCustom' => FALSE, 'checkPermissions' => FALSE, 'action' => 'get'], 'name')); + $api = \Civi\API\Request::create('Contact', 'get', ['version' => 4, 'checkPermissions' => FALSE]); + $query = new Api4SelectQuery($api); $query->select[] = 'id'; $query->select[] = 'first_name'; $query->select[] = 'phones.phone'; diff --git a/tests/phpunit/api/v4/Query/OptionValueJoinTest.php b/tests/phpunit/api/v4/Query/OptionValueJoinTest.php index 56131820d7..f9a34f6347 100644 --- a/tests/phpunit/api/v4/Query/OptionValueJoinTest.php +++ b/tests/phpunit/api/v4/Query/OptionValueJoinTest.php @@ -48,7 +48,8 @@ class OptionValueJoinTest extends UnitTestCase { } public function testCommunicationMethodJoin() { - $query = new Api4SelectQuery('Contact', FALSE, civicrm_api4('Contact', 'getFields', ['includeCustom' => FALSE, 'checkPermissions' => FALSE, 'action' => 'get'], 'name')); + $api = \Civi\API\Request::create('Contact', 'get', ['version' => 4, 'checkPermissions' => FALSE]); + $query = new Api4SelectQuery($api); $query->select[] = 'first_name'; $query->select[] = 'preferred_communication_method.label'; $query->where[] = ['preferred_communication_method', 'IS NOT NULL']; -- 2.25.1