From 4846df9154866b0a33ee8ec7a1f48c65c77b670b Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Thu, 19 Feb 2015 15:02:05 -0500 Subject: [PATCH] CRM-15988 - Cleanup internal use of entity names --- CRM/Core/DAO/permissions.php | 3 ++- CRM/Utils/Api.php | 26 ++++++++++---------- Civi/API/Provider/ReflectionProvider.php | 3 --- Civi/API/Subscriber/ChainSubscriber.php | 20 ++++++++-------- api/api.php | 8 +++---- api/v3/CustomField.php | 2 +- api/v3/CustomGroup.php | 2 +- api/v3/Generic.php | 16 ++++++------- api/v3/utils.php | 30 +++++++++++++----------- tests/phpunit/api/v3/APIWrapperTest.php | 4 ++-- 10 files changed, 57 insertions(+), 57 deletions(-) diff --git a/CRM/Core/DAO/permissions.php b/CRM/Core/DAO/permissions.php index 5db8394876..bd31720c82 100644 --- a/CRM/Core/DAO/permissions.php +++ b/CRM/Core/DAO/permissions.php @@ -37,8 +37,9 @@ * Array of permissions to check for this entity-action combo */ function _civicrm_api3_permissions($entity, $action, &$params) { + // FIXME: Lowercase entity_names are nonstandard but difficult to fix here + // because this function invokes hook_civicrm_alterAPIPermissions $entity = _civicrm_api_get_entity_name_from_camel($entity); - $action = strtolower($action); /** * @var array of permissions diff --git a/CRM/Utils/Api.php b/CRM/Utils/Api.php index f5b1d02c6a..c2a1595060 100644 --- a/CRM/Utils/Api.php +++ b/CRM/Utils/Api.php @@ -44,41 +44,41 @@ class CRM_Utils_Api { // First try the obvious replacements $daoName = str_replace(array('_BAO_', '_Form_', '_Page_'), '_DAO_', $className); - $shortName = CRM_Core_DAO_AllCoreTables::getBriefName($daoName); + $entityName = CRM_Core_DAO_AllCoreTables::getBriefName($daoName); // If that didn't work, try a different pattern - if (!$shortName) { + if (!$entityName) { list(, $parent, , $child) = explode('_', $className); $daoName = "CRM_{$parent}_DAO_$child"; - $shortName = CRM_Core_DAO_AllCoreTables::getBriefName($daoName); + $entityName = CRM_Core_DAO_AllCoreTables::getBriefName($daoName); } // If that didn't work, try a different pattern - if (!$shortName) { + if (!$entityName) { $daoName = "CRM_{$parent}_DAO_$parent"; - $shortName = CRM_Core_DAO_AllCoreTables::getBriefName($daoName); + $entityName = CRM_Core_DAO_AllCoreTables::getBriefName($daoName); } // If that didn't work, try a different pattern - if (!$shortName) { + if (!$entityName) { $daoName = "CRM_Core_DAO_$child"; - $shortName = CRM_Core_DAO_AllCoreTables::getBriefName($daoName); + $entityName = CRM_Core_DAO_AllCoreTables::getBriefName($daoName); } // If that didn't work, try using just the trailing name - if (!$shortName) { - $shortName = CRM_Core_DAO_AllCoreTables::getFullName($child) ? $child : NULL; + if (!$entityName) { + $entityName = CRM_Core_DAO_AllCoreTables::getFullName($child) ? $child : NULL; } // If that didn't work, try using just the leading name - if (!$shortName) { - $shortName = CRM_Core_DAO_AllCoreTables::getFullName($parent) ? $parent : NULL; + if (!$entityName) { + $entityName = CRM_Core_DAO_AllCoreTables::getFullName($parent) ? $parent : NULL; } - if (!$shortName) { + if (!$entityName) { throw new CRM_Core_Exception('Could not find api name for supplied class'); } - return _civicrm_api_get_entity_name_from_camel($shortName); + return $entityName; } } diff --git a/Civi/API/Provider/ReflectionProvider.php b/Civi/API/Provider/ReflectionProvider.php index 32e4bc96b6..bd56c65ddd 100644 --- a/Civi/API/Provider/ReflectionProvider.php +++ b/Civi/API/Provider/ReflectionProvider.php @@ -67,10 +67,7 @@ class ReflectionProvider implements EventSubscriberInterface, ProviderInterface public function __construct($apiKernel) { $this->apiKernel = $apiKernel; $this->actions = array( - // FIXME: We really need to deal with the api's lack of uniformity wrt - // case (Entity or entity). 'Entity' => array('get', 'getactions'), - 'entity' => array('get', 'getactions'), '*' => array('getactions'), // 'getfields' ); } diff --git a/Civi/API/Subscriber/ChainSubscriber.php b/Civi/API/Subscriber/ChainSubscriber.php index 0a230cdea1..e0a6209d45 100644 --- a/Civi/API/Subscriber/ChainSubscriber.php +++ b/Civi/API/Subscriber/ChainSubscriber.php @@ -86,15 +86,15 @@ class ChainSubscriber implements EventSubscriberInterface { * @throws \Exception */ protected function callNestedApi(&$params, &$result, $action, $entity, $version) { - $entity = _civicrm_api_get_entity_name_from_camel($entity); + $lowercase_entity = _civicrm_api_get_entity_name_from_camel($entity); // We don't need to worry about nested api in the getfields/getoptions // actions, so just return immediately. - if (in_array(strtolower($action), array('getfields', 'getoptions'))) { + if (in_array($action, array('getfields', 'getoptions'))) { return; } - if (strtolower($action) == 'getsingle') { + if ($action == 'getsingle') { // I don't understand the protocol here, but we don't want // $result to be a recursive array // $result['values'][0] = $result; @@ -120,11 +120,11 @@ class ChainSubscriber implements EventSubscriberInterface { $subParams = array( 'debug' => \CRM_Utils_Array::value('debug', $params), ); - $subEntity = $subAPI[1]; + $subEntity = _civicrm_api_get_entity_name_from_camel($subAPI[1]); foreach ($result['values'] as $idIndex => $parentAPIValues) { - if (strtolower($subEntity) != 'contact') { + if ($subEntity != 'contact') { //contact spits the dummy at activity_id so what else won't it like? //set entity_id & entity table based on the parent's id & entity. //e.g for something like note if the parent call is contact @@ -132,10 +132,10 @@ class ChainSubscriber implements EventSubscriberInterface { //from the parent call. in this case 'contact_id' will also be //set to the parent's id $subParams["entity_id"] = $parentAPIValues['id']; - $subParams['entity_table'] = 'civicrm_' . _civicrm_api_get_entity_name_from_camel($entity); - $subParams[strtolower($entity) . "_id"] = $parentAPIValues['id']; + $subParams['entity_table'] = 'civicrm_' . $lowercase_entity; + $subParams[$lowercase_entity . "_id"] = $parentAPIValues['id']; } - if (strtolower($entity) != 'contact' && \CRM_Utils_Array::value(strtolower($subEntity . "_id"), $parentAPIValues)) { + if ($entity != 'Contact' && \CRM_Utils_Array::value(strtolower($subEntity . "_id"), $parentAPIValues)) { //e.g. if event_id is in the values returned & subentity is event //then pass in event_id as 'id' don't do this for contact as it //does some wierd things like returning primary email & @@ -149,7 +149,7 @@ class ChainSubscriber implements EventSubscriberInterface { } // if we are dealing with the same entity pass 'id' through // (useful for get + delete for example) - if (strtolower($entity) == strtolower($subEntity)) { + if ($lowercase_entity == $subEntity) { $subParams['id'] = $result['values'][$idIndex]['id']; } @@ -183,7 +183,7 @@ class ChainSubscriber implements EventSubscriberInterface { } } } - if (strtolower($action) == 'getsingle') { + if ($action == 'getsingle') { $result = $result['values'][0]; } } diff --git a/api/api.php b/api/api.php index fa1d345e8b..45a46e0b56 100644 --- a/api/api.php +++ b/api/api.php @@ -107,12 +107,12 @@ function civicrm_error($result) { /** * Get camel case version of entity or action name. * - * @param $entity + * @param string|null $entity * - * @return string + * @return string|null */ function _civicrm_api_get_camel_name($entity) { - return CRM_Utils_String::convertStringToCamel($entity); + return is_string($entity) ? CRM_Utils_String::convertStringToCamel($entity) : NULL; } /** @@ -196,5 +196,5 @@ function _civicrm_api_get_entity_name_from_camel($entity) { */ function _civicrm_api_get_entity_name_from_dao($bao) { $daoName = str_replace("BAO", "DAO", get_class($bao)); - return _civicrm_api_get_entity_name_from_camel(CRM_Core_DAO_AllCoreTables::getBriefName($daoName)); + return CRM_Core_DAO_AllCoreTables::getBriefName($daoName); } diff --git a/api/v3/CustomField.php b/api/v3/CustomField.php index 068258ce55..e9e05d5001 100644 --- a/api/v3/CustomField.php +++ b/api/v3/CustomField.php @@ -253,7 +253,7 @@ SELECT count(*) */ function civicrm_api3_custom_field_setvalue($params) { require_once 'api/v3/Generic/Setvalue.php'; - $result = civicrm_api3_generic_setValue(array("entity" => 'custom_field', 'params' => $params)); + $result = civicrm_api3_generic_setValue(array("entity" => 'CustomField', 'params' => $params)); if (empty($result['is_error'])) { CRM_Utils_System::flushCache(); } diff --git a/api/v3/CustomGroup.php b/api/v3/CustomGroup.php index 0774fa8bcd..a5826fb522 100644 --- a/api/v3/CustomGroup.php +++ b/api/v3/CustomGroup.php @@ -114,7 +114,7 @@ function civicrm_api3_custom_group_get($params) { */ function civicrm_api3_custom_group_setvalue($params) { require_once 'api/v3/Generic/Setvalue.php'; - $result = civicrm_api3_generic_setValue(array("entity" => 'custom_group', 'params' => $params)); + $result = civicrm_api3_generic_setValue(array("entity" => 'CustomGroup', 'params' => $params)); if (empty($result['is_error'])) { CRM_Utils_System::flushCache(); } diff --git a/api/v3/Generic.php b/api/v3/Generic.php index 635ad69b40..7344f1b2d1 100644 --- a/api/v3/Generic.php +++ b/api/v3/Generic.php @@ -71,10 +71,10 @@ function civicrm_api3_generic_getfields($apiRequest) { } } } - $entity = _civicrm_api_get_camel_name($apiRequest['entity']); - $lcase_entity = _civicrm_api_get_entity_name_from_camel($entity); + $entity = $apiRequest['entity']; + $lowercase_entity = _civicrm_api_get_entity_name_from_camel($entity); $subentity = CRM_Utils_Array::value('contact_type', $apiRequest['params']); - $action = strtolower(CRM_Utils_Array::value('action', $apiRequest['params'])); + $action = CRM_Utils_Array::value('action', $apiRequest['params']); $sequential = empty($apiRequest['params']) ? 0 : 1; $apiOptions = CRM_Utils_Array::value('options', $apiRequest['params'], array()); if (!$action || $action == 'getvalue' || $action == 'getcount') { @@ -103,9 +103,9 @@ function civicrm_api3_generic_getfields($apiRequest) { if (empty($metadata['id'])) { // if id is not set we will set it eg. 'id' from 'case_id', case_id will be an alias if (!empty($metadata[strtolower($apiRequest['entity']) . '_id'])) { - $metadata['id'] = $metadata[$lcase_entity . '_id']; - unset($metadata[$lcase_entity . '_id']); - $metadata['id']['api.aliases'] = array($lcase_entity . '_id'); + $metadata['id'] = $metadata[$lowercase_entity . '_id']; + unset($metadata[$lowercase_entity . '_id']); + $metadata['id']['api.aliases'] = array($lowercase_entity . '_id'); } } else { @@ -114,7 +114,7 @@ function civicrm_api3_generic_getfields($apiRequest) { // (note) or setting for all api where fields is returning 'id' & we want to accept 'note_id' @ the api layer // nb we don't officially accept note_id anyway - rationale here is more about centralising a now-tested // inconsistency - $metadata['id']['api.aliases'] = array($lcase_entity . '_id'); + $metadata['id']['api.aliases'] = array($lowercase_entity . '_id'); } break; @@ -124,7 +124,7 @@ function civicrm_api3_generic_getfields($apiRequest) { 'title' => $entity . ' ID', 'name' => 'id', 'api.required' => 1, - 'api.aliases' => array($lcase_entity . '_id'), + 'api.aliases' => array($lowercase_entity . '_id'), 'type' => CRM_Utils_Type::T_INT, )); break; diff --git a/api/v3/utils.php b/api/v3/utils.php index b41db295d8..74c4755620 100644 --- a/api/v3/utils.php +++ b/api/v3/utils.php @@ -163,12 +163,15 @@ function civicrm_api3_create_error($msg, $data = array()) { */ function civicrm_api3_create_success($values = 1, $params = array(), $entity = NULL, $action = NULL, &$dao = NULL, $extraReturnValues = array()) { $result = array(); + $lowercase_entity = _civicrm_api_get_entity_name_from_camel($entity); + // TODO: This shouldn't be necessary but this fn sometimes gets called with lowercase entity + $entity = _civicrm_api_get_camel_name($entity); $result['is_error'] = 0; //lets set the ['id'] field if it's not set & we know what the entity is - if (is_array($values) && !empty($entity) && $action != 'getfields') { + if (is_array($values) && $entity && $action != 'getfields') { foreach ($values as $key => $item) { - if (empty($item['id']) && !empty($item[$entity . "_id"])) { - $values[$key]['id'] = $item[$entity . "_id"]; + if (empty($item['id']) && !empty($item[$lowercase_entity . "_id"])) { + $values[$key]['id'] = $item[$lowercase_entity . "_id"]; } if (!empty($item['financial_type_id'])) { //4.3 legacy handling @@ -746,8 +749,7 @@ function _civicrm_api3_apply_filters_to_dao($filterField, $filterValue, &$dao) { * options extracted from params */ function _civicrm_api3_get_options_from_params(&$params, $queryObject = FALSE, $entity = '', $action = '') { - // Entity should be l-case so it can be concatenated into field names - $entity = _civicrm_api_get_entity_name_from_camel($entity); + $lowercase_entity = _civicrm_api_get_entity_name_from_camel($entity); $is_count = FALSE; $sort = CRM_Utils_Array::value('sort', $params, 0); $sort = CRM_Utils_Array::value('option.sort', $params, $sort); @@ -783,14 +785,14 @@ function _civicrm_api3_get_options_from_params(&$params, $queryObject = FALSE, $ } if ($entity && $action == 'get') { if (!empty($returnProperties['id'])) { - $returnProperties[$entity . '_id'] = 1; + $returnProperties[$lowercase_entity . '_id'] = 1; unset($returnProperties['id']); } switch (trim(strtolower($sort))) { case 'id': case 'id desc': case 'id asc': - $sort = str_replace('id', $entity . '_id', $sort); + $sort = str_replace('id', $lowercase_entity . '_id', $sort); } } @@ -821,7 +823,7 @@ function _civicrm_api3_get_options_from_params(&$params, $queryObject = FALSE, $ $legacyreturnProperties[substr($n, 7)] = $v; } elseif ($n == 'id') { - $inputParams[$entity . '_id'] = $v; + $inputParams[$lowercase_entity . '_id'] = $v; } elseif (in_array($n, $otherVars)) { } @@ -874,9 +876,9 @@ function _civicrm_api3_build_fields_array(&$bao, $unique = TRUE) { $fields = $bao->fields(); if ($unique) { if (empty($fields['id'])) { - $entity = _civicrm_api_get_entity_name_from_dao($bao); - $fields['id'] = $fields[$entity . '_id']; - unset($fields[$entity . '_id']); + $lowercase_entity = _civicrm_api_get_entity_name_from_camel(_civicrm_api_get_entity_name_from_dao($bao)); + $fields['id'] = $fields[$lowercase_entity . '_id']; + unset($fields[$lowercase_entity . '_id']); } return $fields; } @@ -2197,12 +2199,12 @@ function _civicrm_api3_api_resolve_alias($entity, $fieldName) { */ function _civicrm_api3_deprecation_check($entity, $result = array()) { if ($entity) { - $apiFile = 'api/v3/' . _civicrm_api_get_camel_name($entity) . '.php'; + $apiFile = "api/v3/$entity.php"; if (CRM_Utils_File::isIncludable($apiFile)) { require_once $apiFile; } - $entity = _civicrm_api_get_entity_name_from_camel($entity); - $fnName = "_civicrm_api3_{$entity}_deprecation"; + $lowercase_entity = _civicrm_api_get_entity_name_from_camel($entity); + $fnName = "_civicrm_api3_{$lowercase_entity}_deprecation"; if (function_exists($fnName)) { return $fnName($result); } diff --git a/tests/phpunit/api/v3/APIWrapperTest.php b/tests/phpunit/api/v3/APIWrapperTest.php index ed1b74ea35..d61cb29549 100644 --- a/tests/phpunit/api/v3/APIWrapperTest.php +++ b/tests/phpunit/api/v3/APIWrapperTest.php @@ -94,7 +94,7 @@ class api_v3_APIWrapperTest_Impl implements API_Wrapper { * @inheritDoc */ public function fromApiInput($apiRequest) { - if ($apiRequest['entity'] == 'contact' && $apiRequest['action'] == 'create') { + if ($apiRequest['entity'] == 'Contact' && $apiRequest['action'] == 'create') { if ('Invalid' == CRM_Utils_Array::value('contact_type', $apiRequest['params'])) { $apiRequest['params']['contact_type'] = 'Individual'; } @@ -106,7 +106,7 @@ class api_v3_APIWrapperTest_Impl implements API_Wrapper { * @inheritDoc */ public function toApiOutput($apiRequest, $result) { - if ($apiRequest['entity'] == 'contact' && $apiRequest['action'] == 'create') { + if ($apiRequest['entity'] == 'Contact' && $apiRequest['action'] == 'create') { if (isset($result['id'], $result['values'][$result['id']]['display_name'])) { $result['values'][$result['id']]['display_name_munged'] = 'MUNGE! ' . $result['values'][$result['id']]['display_name']; unset($result['values'][$result['id']]['display_name']); -- 2.25.1