From 051948d97b000108cf44098add2e2efdc5f0401b Mon Sep 17 00:00:00 2001 From: colemanw Date: Fri, 25 Aug 2023 23:37:45 -0400 Subject: [PATCH] API - Soft-deprecate civicrm_api() wrapper, cleanup code comments, remove unused vars --- CRM/Core/Form.php | 2 +- Civi/API/Kernel.php | 31 +++++++++---------------------- api/api.php | 23 +++++++++++------------ 3 files changed, 21 insertions(+), 35 deletions(-) diff --git a/CRM/Core/Form.php b/CRM/Core/Form.php index 76a7baaf79..3e21e9391c 100644 --- a/CRM/Core/Form.php +++ b/CRM/Core/Form.php @@ -2467,7 +2467,7 @@ class CRM_Core_Form extends HTML_QuickForm_Page { * @return int * @throws \CRM_Core_Exception */ - public function getRequestedContactID(): ?int { + public function getRequestedContactID(): ?int { if (isset($this->_params) && !empty($this->_params['select_contact_id'])) { return (int) $this->_params['select_contact_id']; } diff --git a/Civi/API/Kernel.php b/Civi/API/Kernel.php index 8e67c3142c..9630e8c60b 100644 --- a/Civi/API/Kernel.php +++ b/Civi/API/Kernel.php @@ -34,9 +34,7 @@ class Kernel { /** * @param \Civi\Core\CiviEventDispatcherInterface $dispatcher - * The event dispatcher which receives kernel events. - * @param array $apiProviders - * Array of ProviderInterface. + * @param \Civi\API\Provider\ProviderInterface[] $apiProviders */ public function __construct($dispatcher, $apiProviders = []) { $this->apiProviders = $apiProviders; @@ -271,14 +269,12 @@ class Kernel { /** * @param int $version * API version. - * @return array - * Array. + * @return string[] */ public function getEntityNames($version) { // Question: Would it better to eliminate $this->apiProviders and just use $this->dispatcher? $entityNames = []; foreach ($this->getApiProviders() as $provider) { - /** @var \Civi\API\Provider\ProviderInterface $provider */ $entityNames = array_merge($entityNames, $provider->getEntityNames($version)); } $entityNames = array_unique($entityNames); @@ -291,14 +287,12 @@ class Kernel { * API version. * @param string $entity * API entity. - * @return array - * Array + * @return string[] */ public function getActionNames($version, $entity) { // Question: Would it better to eliminate $this->apiProviders and just use $this->dispatcher? $actionNames = []; foreach ($this->getApiProviders() as $provider) { - /** @var \Civi\API\Provider\ProviderInterface $provider */ $actionNames = array_merge($actionNames, $provider->getActionNames($version, $entity)); } $actionNames = array_unique($actionNames); @@ -321,7 +315,7 @@ class Kernel { if (!empty($apiRequest['params']['debug'])) { $data['trace'] = $e->getTraceAsString(); } - return $this->createError($e->getMessage(), $data, $apiRequest, $e->getCode()); + return $this->createError($e->getMessage(), $data, $apiRequest); } /** @@ -336,9 +330,7 @@ class Kernel { */ public function formatApiException($e, $apiRequest) { $data = $e->getExtraParams(); - $errorCode = $e->getCode(); if (($data['exception'] ?? NULL) instanceof \DB_Error) { - $errorCode = $e->getDBErrorMessage(); $data['sql'] = $e->getSQL(); $data['debug_info'] = $e->getUserInfo(); } @@ -346,14 +338,14 @@ class Kernel { $data['entity'] = $apiRequest['entity'] ?? NULL; $data['action'] = $apiRequest['action'] ?? NULL; - if (\CRM_Utils_Array::value('debug', \CRM_Utils_Array::value('params', $apiRequest)) + if (!empty($apiRequest['params']['debug']) // prevent recursion && empty($data['trace']) ) { $data['trace'] = $e->getTraceAsString(); } - return $this->createError($e->getMessage(), $data, $apiRequest, $errorCode); + return $this->createError($e->getMessage(), $data, $apiRequest); } /** @@ -397,15 +389,11 @@ class Kernel { * Error data. * @param array $apiRequest * The full description of the API request. - * @param mixed $code - * Doesn't appear to be used. * * @throws \CRM_Core_Exception * @return array - * Array. */ - public function createError($msg, $data, $apiRequest, $code = NULL) { - // FIXME what to do with $code? + public function createError($msg, $data, $apiRequest) { if ($msg === 'DB Error: constraint violation' || substr($msg, 0, 9) == 'DB Error:' || $msg == 'DB Error: already exists') { try { $fields = _civicrm_api3_api_getfields($apiRequest); @@ -448,15 +436,14 @@ class Kernel { } /** - * @return array + * @return \Civi\API\Provider\ProviderInterface[] */ public function getApiProviders() { return $this->apiProviders; } /** - * @param array $apiProviders - * Array. + * @param \Civi\API\Provider\ProviderInterface[] $apiProviders * @return Kernel */ public function setApiProviders($apiProviders) { diff --git a/api/api.php b/api/api.php index 670e6ee5fe..42ebc7e36d 100644 --- a/api/api.php +++ b/api/api.php @@ -7,14 +7,20 @@ */ /** - * CiviCRM API wrapper function. + * The original API wrapper. + * + * @deprecated + * Not recommended for new code but ok for existing code to continue using. + * + * Calling `civicrm_api()` is functionally identical to `civicrm_api3()` or `civicrm_api4()` except: + * 1. It requires `$params['version']`. + * 2. It catches exceptions and returns an array like `['is_error' => 1, 'error_message' => ...]`. + * This is disfavored for typical business-logic/hooks/forms/etc. + * However, if an existing caller handles `civicrm_api()`-style errors, then there is no functional benefit to reworking it. * * @param string $entity - * type of entities to deal with * @param string $action - * create, get, delete or some special action name. * @param array $params - * array to be passed to function * * @return array|int|Civi\Api4\Generic\Result */ @@ -117,16 +123,13 @@ function civicrm_api4(string $entity, string $action, array $params = [], $index * Throws exception. * * @param string $entity - * Type of entities to deal with. * @param string $action - * Create, get, delete or some special action name. * @param array $params - * Array to be passed to function. * * @throws CRM_Core_Exception * * @return array|int - * Dependant on the $action + * Dependent on the $action */ function civicrm_api3(string $entity, string $action, array $params = []) { $params['version'] = 3; @@ -159,10 +162,6 @@ function _civicrm_api3_api_getfields(&$apiRequest) { if (strtolower($apiRequest['action'] == 'getfields')) { // the main param getfields takes is 'action' - however this param is not compatible with REST // so we accept 'api_action' as an alias of action on getfields - if (!empty($apiRequest['params']['api_action'])) { - // $apiRequest['params']['action'] = $apiRequest['params']['api_action']; - // unset($apiRequest['params']['api_action']); - } return ['action' => ['api.aliases' => ['api_action']]]; } $getFieldsParams = ['action' => $apiRequest['action']]; -- 2.25.1