From: Coleman Watts <coleman@civicrm.org>
Date: Thu, 19 Feb 2015 20:02:05 +0000 (-0500)
Subject: CRM-15988 - Cleanup internal use of entity names
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=4846df9154866b0a33ee8ec7a1f48c65c77b670b;p=civicrm-core.git

CRM-15988 - Cleanup internal use of entity names
---

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']);