From 8882ff5cb3808cb2157cc442e0aaa293983dc869 Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Mon, 29 Dec 2014 07:24:15 -0800 Subject: [PATCH] Civi/API - Code style --- Civi/API/Event/AuthorizeEvent.php | 6 +- Civi/API/Event/Event.php | 12 ++- Civi/API/Event/ExceptionEvent.php | 11 ++- Civi/API/Event/PrepareEvent.php | 1 + Civi/API/Event/ResolveEvent.php | 7 +- Civi/API/Event/RespondEvent.php | 12 ++- Civi/API/Events.php | 7 +- .../API/Exception/NotImplementedException.php | 9 +- Civi/API/Exception/UnauthorizedException.php | 9 +- Civi/API/Kernel.php | 93 +++++++++++++------ Civi/API/Provider/AdhocProvider.php | 18 +++- Civi/API/Provider/MagicFunctionProvider.php | 56 ++++++----- Civi/API/Provider/ProviderInterface.php | 12 ++- Civi/API/Provider/ReflectionProvider.php | 31 +++++-- Civi/API/Provider/StaticProvider.php | 55 +++++++++-- Civi/API/Request.php | 7 +- Civi/API/Subscriber/APIv3SchemaAdapter.php | 14 ++- Civi/API/Subscriber/ChainSubscriber.php | 37 +++++--- .../API/Subscriber/DynamicFKAuthorization.php | 86 +++++++++++------ Civi/API/Subscriber/I18nSubscriber.php | 4 +- Civi/API/Subscriber/PermissionCheck.php | 9 +- Civi/API/Subscriber/TransactionSubscriber.php | 33 +++++-- Civi/API/Subscriber/WrapperAdapter.php | 7 +- Civi/API/Subscriber/XDebugSubscriber.php | 3 +- api/Exception.php | 70 +++++++------- 25 files changed, 421 insertions(+), 188 deletions(-) diff --git a/Civi/API/Event/AuthorizeEvent.php b/Civi/API/Event/AuthorizeEvent.php index 5c123367fc..eea048c608 100644 --- a/Civi/API/Event/AuthorizeEvent.php +++ b/Civi/API/Event/AuthorizeEvent.php @@ -37,12 +37,16 @@ class AuthorizeEvent extends Event { */ private $authorized = FALSE; + /** + * Mark the request as authorized. + */ public function authorize() { $this->authorized = TRUE; } /** - * @return boolean + * @return bool + * TRUE if the request has been authorized. */ public function isAuthorized() { return $this->authorized; diff --git a/Civi/API/Event/Event.php b/Civi/API/Event/Event.php index de29bbabf9..b205a6eaa4 100644 --- a/Civi/API/Event/Event.php +++ b/Civi/API/Event/Event.php @@ -34,19 +34,25 @@ namespace Civi\API\Event; class Event extends \Symfony\Component\EventDispatcher\Event { /** * @var \Civi\API\Provider\ProviderInterface + * The API provider responsible for executing the request. */ protected $apiProvider; /** * @var array + * The full description of the API request. + * + * @see \Civi\API\Request::create */ protected $apiRequest; /** - * @param $apiProvider - * @param $apiRequest + * @param \Civi\API\Provider\ProviderInterface $apiProvider + * The API responsible for executing the request. + * @param array $apiRequest + * The full description of the API request. */ - function __construct($apiProvider, $apiRequest) { + public function __construct($apiProvider, $apiRequest) { $this->apiProvider = $apiProvider; $this->apiRequest = $apiRequest; } diff --git a/Civi/API/Event/ExceptionEvent.php b/Civi/API/Event/ExceptionEvent.php index 0165528334..6575257882 100644 --- a/Civi/API/Event/ExceptionEvent.php +++ b/Civi/API/Event/ExceptionEvent.php @@ -39,11 +39,14 @@ class ExceptionEvent extends Event { private $exception; /** - * @param $exception - * @param $apiProvider - * @param $apiRequest + * @param \Exception $exception + * The exception which arose while processing the API request. + * @param \Civi\API\Provider\ProviderInterface $apiProvider + * The API provider responsible for executing the request. + * @param array $apiRequest + * The full description of the API request. */ - function __construct($exception, $apiProvider, $apiRequest) { + public function __construct($exception, $apiProvider, $apiRequest) { $this->exception = $exception; parent::__construct($apiProvider, $apiRequest); } diff --git a/Civi/API/Event/PrepareEvent.php b/Civi/API/Event/PrepareEvent.php index 482879817d..0512ced78f 100644 --- a/Civi/API/Event/PrepareEvent.php +++ b/Civi/API/Event/PrepareEvent.php @@ -34,6 +34,7 @@ namespace Civi\API\Event; class PrepareEvent extends Event { /** * @param array $apiRequest + * The full description of the API request. * @return RespondEvent */ public function setApiRequest($apiRequest) { diff --git a/Civi/API/Event/ResolveEvent.php b/Civi/API/Event/ResolveEvent.php index c76c795ec0..0629840b42 100644 --- a/Civi/API/Event/ResolveEvent.php +++ b/Civi/API/Event/ResolveEvent.php @@ -33,14 +33,16 @@ namespace Civi\API\Event; */ class ResolveEvent extends Event { /** - * @param $apiRequest + * @param array $apiRequest + * The full description of the API request. */ - function __construct($apiRequest) { + public function __construct($apiRequest) { parent::__construct(NULL, $apiRequest); } /** * @param \Civi\API\Provider\ProviderInterface $apiProvider + * The API provider responsible for executing the request. */ public function setApiProvider($apiProvider) { $this->apiProvider = $apiProvider; @@ -48,6 +50,7 @@ class ResolveEvent extends Event { /** * @param array $apiRequest + * The full description of the API request. */ public function setApiRequest($apiRequest) { $this->apiRequest = $apiRequest; diff --git a/Civi/API/Event/RespondEvent.php b/Civi/API/Event/RespondEvent.php index cf43238c36..0915fcd665 100644 --- a/Civi/API/Event/RespondEvent.php +++ b/Civi/API/Event/RespondEvent.php @@ -38,11 +38,14 @@ class RespondEvent extends Event { private $response; /** - * @param $apiProvider - * @param $apiRequest - * @param $response + * @param \Civi\API\Provider\ProviderInterface $apiProvider + * The API provider responsible for executing the request. + * @param array $apiRequest + * The full description of the API request. + * @param mixed $response + * The response to return to the client. */ - function __construct($apiProvider, $apiRequest, $response) { + public function __construct($apiProvider, $apiRequest, $response) { $this->response = $response; parent::__construct($apiProvider, $apiRequest); } @@ -56,6 +59,7 @@ class RespondEvent extends Event { /** * @param mixed $response + * The response to return to the client. */ public function setResponse($response) { $this->response = $response; diff --git a/Civi/API/Events.php b/Civi/API/Events.php index dc48045216..791739f06b 100644 --- a/Civi/API/Events.php +++ b/Civi/API/Events.php @@ -34,12 +34,14 @@ namespace Civi\API; * * Event subscribers which are concerned about the order of execution should assign * a weight to their subscription (such as W_EARLY, W_MIDDLE, or W_LATE). + * W_LATE). */ class Events { /** * Determine whether the API request is allowed for the current user. - * For successful execution, at least one listener must invoke $event->authorize(). + * For successful execution, at least one listener must invoke + * $event->authorize(). * * @see AuthorizeEvent */ @@ -47,7 +49,8 @@ class Events { /** * Determine which API provider executes the given request. For successful - * execution, at least one listener must invoke $event->setProvider($provider). + * execution, at least one listener must invoke + * $event->setProvider($provider). * * @see ResolveEvent */ diff --git a/Civi/API/Exception/NotImplementedException.php b/Civi/API/Exception/NotImplementedException.php index 68ca54478a..1022fb523a 100644 --- a/Civi/API/Exception/NotImplementedException.php +++ b/Civi/API/Exception/NotImplementedException.php @@ -10,10 +10,15 @@ require_once 'api/Exception.php'; class NotImplementedException extends \API_Exception { /** * @param string $message + * The human friendly error message. * @param array $extraParams - * @param Exception $previous + * Extra params to return. eg an extra array of ids. It is not mandatory, + * but can help the computer using the api. Keep in mind the api consumer + * isn't to be trusted. eg. the database password is NOT a good extra data. + * @param \Exception|NULL $previous + * A previous exception which caused this new exception. */ - public function __construct($message, $extraParams = array(), Exception $previous = NULL) { + public function __construct($message, $extraParams = array(), \Exception $previous = NULL) { parent::__construct($message, \API_Exception::NOT_IMPLEMENTED, $extraParams, $previous); } } diff --git a/Civi/API/Exception/UnauthorizedException.php b/Civi/API/Exception/UnauthorizedException.php index 7162a04f74..55ff1dfcaa 100644 --- a/Civi/API/Exception/UnauthorizedException.php +++ b/Civi/API/Exception/UnauthorizedException.php @@ -10,10 +10,15 @@ require_once 'api/Exception.php'; class UnauthorizedException extends \API_Exception { /** * @param string $message + * The human friendly error message. * @param array $extraParams - * @param Exception $previous + * Extra params to return. eg an extra array of ids. It is not mandatory, + * but can help the computer using the api. Keep in mind the api consumer + * isn't to be trusted. eg. the database password is NOT a good extra data. + * @param \Exception|NULL $previous + * A previous exception which caused this new exception. */ - public function __construct($message, $extraParams = array(), Exception $previous = NULL) { + public function __construct($message, $extraParams = array(), \Exception $previous = NULL) { parent::__construct($message, \API_Exception::UNAUTHORIZED, $extraParams, $previous); } } diff --git a/Civi/API/Kernel.php b/Civi/API/Kernel.php index c641898ab3..0bc3fbd5a8 100644 --- a/Civi/API/Kernel.php +++ b/Civi/API/Kernel.php @@ -34,11 +34,9 @@ use Civi\API\Event\RespondEvent; use Civi\API\Provider\ProviderInterface; /** - * * @package Civi * @copyright CiviCRM LLC (c) 2004-2014 */ - class Kernel { /** @@ -52,22 +50,25 @@ class Kernel { protected $apiProviders; /** - * @param $dispatcher + * @param \Symfony\Component\EventDispatcher\EventDispatcher $dispatcher + * The event dispatcher which receives kernel events. * @param array $apiProviders + * Array of ProviderInterface. */ - function __construct($dispatcher, $apiProviders = array()) { + public function __construct($dispatcher, $apiProviders = array()) { $this->apiProviders = $apiProviders; $this->dispatcher = $dispatcher; } /** * @param string $entity - * type of entities to deal with + * Type of entities to deal with. * @param string $action - * create, get, delete or some special action name. + * Create, get, delete or some special action name. * @param array $params - * array to be passed to function - * @param null $extra + * Array to be passed to API function. + * @param mixed $extra + * Who knows. * * @return array|int */ @@ -102,9 +103,11 @@ class Kernel { if ($e instanceof \PEAR_Exception) { $err = $this->formatPearException($e, $apiRequest); - } elseif ($e instanceof \API_Exception) { + } + elseif ($e instanceof \API_Exception) { $err = $this->formatApiException($e, $apiRequest); - } else { + } + else { $err = $this->formatException($e, $apiRequest); } @@ -116,13 +119,15 @@ class Kernel { * Determine if a hypothetical API call would be authorized. * * @param string $entity - * type of entities to deal with + * Type of entities to deal with. * @param string $action - * create, get, delete or some special action name. + * Create, get, delete or some special action name. * @param array $params - * array to be passed to function - * @param null $extra - * @return bool TRUE if authorization would succeed + * Array to be passed to function. + * @param mixed $extra + * Who knows. + * @return bool + * TRUE if authorization would succeed. * @throws \Exception */ public function runAuthorize($entity, $action, $params, $extra = NULL) { @@ -133,15 +138,18 @@ class Kernel { $this->boot(); list($apiProvider, $apiRequest) = $this->resolve($apiRequest); $this->authorize($apiProvider, $apiRequest); - return true; + return TRUE; } catch (\Civi\API\Exception\UnauthorizedException $e) { - return false; + return FALSE; } } + /** + * Bootstrap - Load basic dependencies. + */ public function boot() { - require_once ('api/v3/utils.php'); + require_once 'api/v3/utils.php'; require_once 'api/Exception.php'; _civicrm_api3_initialize(); } @@ -150,8 +158,10 @@ class Kernel { * Determine which, if any, service will execute the API request. * * @param array $apiRequest + * The full description of the API request. * @throws Exception\NotImplementedException * @return array + * Array(0 => ProviderInterface, 1 => array). */ public function resolve($apiRequest) { /** @var ResolveEvent $resolveEvent */ @@ -167,7 +177,9 @@ class Kernel { * Determine if the API request is allowed (under current policy) * * @param ProviderInterface $apiProvider + * The API provider responsible for executing the request. * @param array $apiRequest + * The full description of the API request. * @throws Exception\UnauthorizedException */ public function authorize($apiProvider, $apiRequest) { @@ -182,7 +194,9 @@ class Kernel { * Allow third-party code to manipulate the API request before execution. * * @param ProviderInterface $apiProvider + * The API provider responsible for executing the request. * @param array $apiRequest + * The full description of the API request. * @return mixed */ public function prepare($apiProvider, $apiRequest) { @@ -195,8 +209,11 @@ class Kernel { * Allow third-party code to manipulate the API response after execution. * * @param ProviderInterface $apiProvider + * The API provider responsible for executing the request. * @param array $apiRequest + * The full description of the API request. * @param array $result + * The response to return to the client. * @return mixed */ public function respond($apiProvider, $apiRequest, $result) { @@ -207,7 +224,9 @@ class Kernel { /** * @param int $version - * @return array + * API version. + * @return array + * Array. */ public function getEntityNames($version) { // Question: Would it better to eliminate $this->apiProviders and just use $this->dispatcher? @@ -223,8 +242,11 @@ class Kernel { /** * @param int $version + * API version. * @param string $entity - * @return array + * API entity. + * @return array + * Array */ public function getActionNames($version, $entity) { // Question: Would it better to eliminate $this->apiProviders and just use $this->dispatcher? @@ -240,8 +262,11 @@ class Kernel { /** * @param \Exception $e + * An unhandled exception. * @param array $apiRequest - * @return array (API response) + * The full description of the API request. + * @return array + * API response. */ public function formatException($e, $apiRequest) { $data = array(); @@ -253,7 +278,9 @@ class Kernel { /** * @param \API_Exception $e + * An unhandled exception. * @param array $apiRequest + * The full description of the API request. * @return array (API response) */ public function formatApiException($e, $apiRequest) { @@ -272,8 +299,11 @@ class Kernel { /** * @param \PEAR_Exception $e + * An unhandled exception. * @param array $apiRequest - * @return array (API response) + * The full description of the API request. + * @return array + * API response. */ public function formatPearException($e, $apiRequest) { $data = array(); @@ -300,20 +330,26 @@ class Kernel { /** * @param string $msg + * Descriptive error message. * @param array $data + * Error data. * @param array $apiRequest - * @param mixed $code doesn't appear to be used + * The full description of the API request. + * @param mixed $code + * Doesn't appear to be used. * * @throws \API_Exception - * @return array + * @return array + * Array. */ - function createError($msg, $data, $apiRequest, $code = NULL) { + public function createError($msg, $data, $apiRequest, $code = NULL) { // FIXME what to do with $code? if ($msg == 'DB Error: constraint violation' || substr($msg, 0, 9) == 'DB Error:' || $msg == 'DB Error: already exists') { try { $fields = _civicrm_api3_api_getfields($apiRequest); _civicrm_api3_validate_fields($apiRequest['entity'], $apiRequest['action'], $apiRequest['params'], $fields, TRUE); - } catch (\Exception $e) { + } + catch (\Exception $e) { $msg = $e->getMessage(); } } @@ -330,7 +366,9 @@ class Kernel { /** * @param array $apiRequest + * The full description of the API request. * @param array $result + * The response to return to the client. * @return mixed */ public function formatResult($apiRequest, $result) { @@ -356,6 +394,7 @@ class Kernel { /** * @param array $apiProviders + * Array. * @return Kernel */ public function setApiProviders($apiProviders) { @@ -365,6 +404,7 @@ class Kernel { /** * @param ProviderInterface $apiProvider + * The API provider responsible for executing the request. * @return Kernel */ public function registerApiProvider($apiProvider) { @@ -384,6 +424,7 @@ class Kernel { /** * @param \Symfony\Component\EventDispatcher\EventDispatcher $dispatcher + * The event dispatcher which receives kernel events. * @return Kernel */ public function setDispatcher($dispatcher) { diff --git a/Civi/API/Provider/AdhocProvider.php b/Civi/API/Provider/AdhocProvider.php index 89c8b5e2bd..c981ef21d3 100644 --- a/Civi/API/Provider/AdhocProvider.php +++ b/Civi/API/Provider/AdhocProvider.php @@ -68,7 +68,9 @@ class AdhocProvider implements EventSubscriberInterface, ProviderInterface { /** * @param int $version + * API version. * @param string $entity + * API entity. */ public function __construct($version, $entity) { $this->entity = $entity; @@ -76,9 +78,14 @@ class AdhocProvider implements EventSubscriberInterface, ProviderInterface { } /** + * Register a new API. + * * @param string $name + * Action name. * @param string $perm - * @param callable $callback + * Permissions required for invoking the action. + * @param mixed $callback + * The function which executes the API. * @return ReflectionProvider */ public function addAction($name, $perm, $callback) { @@ -91,6 +98,7 @@ class AdhocProvider implements EventSubscriberInterface, ProviderInterface { /** * @param \Civi\API\Event\ResolveEvent $event + * API resolution event. */ public function onApiResolve(\Civi\API\Event\ResolveEvent $event) { $apiRequest = $event->getApiRequest(); @@ -103,6 +111,7 @@ class AdhocProvider implements EventSubscriberInterface, ProviderInterface { /** * @param \Civi\API\Event\AuthorizeEvent $event + * API authorization event. */ public function onApiAuthorize(\Civi\API\Event\AuthorizeEvent $event) { $apiRequest = $event->getApiRequest(); @@ -122,14 +131,14 @@ class AdhocProvider implements EventSubscriberInterface, ProviderInterface { /** * {inheritdoc} */ - function getEntityNames($version) { + public function getEntityNames($version) { return array($this->entity); } /** * {inheritdoc} */ - function getActionNames($version, $entity) { + public function getActionNames($version, $entity) { if ($version == $this->version && $entity == $this->entity) { return array_keys($this->actions); } @@ -139,7 +148,8 @@ class AdhocProvider implements EventSubscriberInterface, ProviderInterface { } /** - * @param $apiRequest + * @param array $apiRequest + * The full description of the API request. * * @return bool */ diff --git a/Civi/API/Provider/MagicFunctionProvider.php b/Civi/API/Provider/MagicFunctionProvider.php index fe8fe5d97e..84f83b8005 100644 --- a/Civi/API/Provider/MagicFunctionProvider.php +++ b/Civi/API/Provider/MagicFunctionProvider.php @@ -26,6 +26,7 @@ */ namespace Civi\API\Provider; + use Civi\API\Events; use Symfony\Component\EventDispatcher\EventSubscriberInterface; @@ -53,12 +54,13 @@ class MagicFunctionProvider implements EventSubscriberInterface, ProviderInterfa /** * */ - function __construct() { + public function __construct() { $this->cache = array(); } /** * @param \Civi\API\Event\ResolveEvent $event + * API resolution event. */ public function onApiResolve(\Civi\API\Event\ResolveEvent $event) { $apiRequest = $event->getApiRequest(); @@ -91,13 +93,14 @@ class MagicFunctionProvider implements EventSubscriberInterface, ProviderInterfa /** * {inheritdoc} */ - function getEntityNames($version) { + public function getEntityNames($version) { $entities = array(); $include_dirs = array_unique(explode(PATH_SEPARATOR, get_include_path())); #$include_dirs = array(dirname(__FILE__). '/../../'); foreach ($include_dirs as $include_dir) { - $api_dir = implode(DIRECTORY_SEPARATOR, array($include_dir, 'api', 'v' . $version)); - if (! is_dir($api_dir)) { + $api_dir = implode(DIRECTORY_SEPARATOR, + array($include_dir, 'api', 'v' . $version)); + if (!is_dir($api_dir)) { continue; } $iterator = new \DirectoryIterator($api_dir); @@ -106,12 +109,12 @@ class MagicFunctionProvider implements EventSubscriberInterface, ProviderInterfa // Check for entities with a master file ("api/v3/MyEntity.php") $parts = explode(".", $file); - if (end($parts) == "php" && $file != "utils.php" && !preg_match('/Tests?.php$/', $file) ) { + if (end($parts) == "php" && $file != "utils.php" && !preg_match('/Tests?.php$/', $file)) { // without the ".php" $entities[] = substr($file, 0, -4); } - // Check for entities with standalone action files ("api/v3/MyEntity/MyAction.php") + // Check for entities with standalone action files (eg "api/v3/MyEntity/MyAction.php"). $action_dir = $api_dir . DIRECTORY_SEPARATOR . $file; if (preg_match('/^[A-Z][A-Za-z0-9]*$/', $file) && is_dir($action_dir)) { if (count(glob("$action_dir/[A-Z]*.php")) > 0) { @@ -156,15 +159,17 @@ class MagicFunctionProvider implements EventSubscriberInterface, ProviderInterfa /** * Look up the implementation for a given API request * - * @param $apiRequest array with keys: - * - entity: string, required - * - action: string, required - * - params: array - * - version: scalar, required + * @param array $apiRequest + * Array with keys: + * - entity: string, required. + * - action: string, required. + * - params: array. + * - version: scalar, required. * - * @return array with keys - * - function: callback (mixed) - * - is_generic: boolean + * @return array + * Array with keys: + * - function: callback (mixed) + * - is_generic: boolean */ protected function resolve($apiRequest) { $cachekey = strtolower($apiRequest['entity']) . ':' . strtolower($apiRequest['action']) . ':' . $apiRequest['version']; @@ -179,13 +184,15 @@ class MagicFunctionProvider implements EventSubscriberInterface, ProviderInterfa $stdFunction = $this->getFunctionName($apiRequest['entity'], $apiRequest['action'], $apiRequest['version']); if (function_exists($stdFunction)) { // someone already loaded the appropriate file - // FIXME: This has the affect of masking bugs in load order; this is included to provide bug-compatibility + // FIXME: This has the affect of masking bugs in load order; this is + // included to provide bug-compatibility. $this->cache[$cachekey] = array('function' => $stdFunction, 'is_generic' => FALSE); return $this->cache[$cachekey]; } $stdFiles = array( - // By convention, the $camelName.php is more likely to contain the function, so test it first + // By convention, the $camelName.php is more likely to contain the + // function, so test it first 'api/v' . $apiRequest['version'] . '/' . $camelName . '.php', 'api/v' . $apiRequest['version'] . '/' . $camelName . '/' . $actionCamelName . '.php', ); @@ -204,7 +211,8 @@ class MagicFunctionProvider implements EventSubscriberInterface, ProviderInterfa # $genericFunction = 'civicrm_api3_generic_' . $apiRequest['action']; $genericFunction = $this->getFunctionName('generic', $apiRequest['action'], $apiRequest['version']); $genericFiles = array( - // By convention, the Generic.php is more likely to contain the function, so test it first + // By convention, the Generic.php is more likely to contain the + // function, so test it first 'api/v' . $apiRequest['version'] . '/Generic.php', 'api/v' . $apiRequest['version'] . '/Generic/' . $actionCamelName . '.php', ); @@ -223,9 +231,14 @@ class MagicFunctionProvider implements EventSubscriberInterface, ProviderInterfa } /** + * Determine the function name for a given API request. + * * @param string $entity + * API entity name. * @param string $action - * @param $version + * API action name. + * @param int $version + * API version. * * @return string */ @@ -241,9 +254,9 @@ class MagicFunctionProvider implements EventSubscriberInterface, ProviderInterfa * only appropriate when introspection is really required (eg for "getActions"). * * @param string $entity + * API entity name. * @param int $version - * - * @return void + * API version. */ protected function loadEntity($entity, $version) { $camelName = _civicrm_api_get_camel_name($entity, $version); @@ -259,7 +272,8 @@ class MagicFunctionProvider implements EventSubscriberInterface, ProviderInterfa $include_dirs = array_unique(explode(PATH_SEPARATOR, get_include_path())); foreach ($include_dirs as $include_dir) { foreach (array($camelName, 'Generic') as $name) { - $action_dir = implode(DIRECTORY_SEPARATOR, array($include_dir, 'api', "v${version}", $name)); + $action_dir = implode(DIRECTORY_SEPARATOR, + array($include_dir, 'api', "v${version}", $name)); if (!is_dir($action_dir)) { continue; } diff --git a/Civi/API/Provider/ProviderInterface.php b/Civi/API/Provider/ProviderInterface.php index 31034c70ca..19cc0b4c85 100644 --- a/Civi/API/Provider/ProviderInterface.php +++ b/Civi/API/Provider/ProviderInterface.php @@ -35,22 +35,26 @@ use Symfony\Component\EventDispatcher\EventSubscriberInterface; interface ProviderInterface { /** * @param array $apiRequest + * The full description of the API request. * @return array structured response data (per civicrm_api3_create_success) * @see civicrm_api3_create_success * @throws \API_Exception */ - function invoke($apiRequest); + public function invoke($apiRequest); /** * @param int $version + * API version. * @return array */ - function getEntityNames($version); + public function getEntityNames($version); /** * @param int $version + * API version. * @param string $entity + * API entity. * @return array */ - function getActionNames($version, $entity); -} \ No newline at end of file + public function getActionNames($version, $entity); +} diff --git a/Civi/API/Provider/ReflectionProvider.php b/Civi/API/Provider/ReflectionProvider.php index 4e646faad1..c6cecdec3d 100644 --- a/Civi/API/Provider/ReflectionProvider.php +++ b/Civi/API/Provider/ReflectionProvider.php @@ -26,6 +26,7 @@ */ namespace Civi\API\Provider; + use Civi\API\Events; use Symfony\Component\EventDispatcher\EventSubscriberInterface; @@ -39,10 +40,12 @@ class ReflectionProvider implements EventSubscriberInterface, ProviderInterface public static function getSubscribedEvents() { return array( Events::RESOLVE => array( - array('onApiResolve', Events::W_EARLY), // TODO decide if we really want to override others + // TODO decide if we really want to override others + array('onApiResolve', Events::W_EARLY), ), Events::AUTHORIZE => array( - array('onApiAuthorize', Events::W_EARLY), // TODO decide if we really want to override others + // TODO decide if we really want to override others + array('onApiAuthorize', Events::W_EARLY), ), ); } @@ -59,11 +62,13 @@ class ReflectionProvider implements EventSubscriberInterface, ProviderInterface /** * @param \Civi\API\Kernel $apiKernel + * The API kernel. */ 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) + // 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' @@ -72,6 +77,7 @@ class ReflectionProvider implements EventSubscriberInterface, ProviderInterface /** * @param \Civi\API\Event\ResolveEvent $event + * API resolution event. */ public function onApiResolve(\Civi\API\Event\ResolveEvent $event) { $apiRequest = $event->getApiRequest(); @@ -80,17 +86,20 @@ class ReflectionProvider implements EventSubscriberInterface, ProviderInterface $apiRequest['is_metadata'] = TRUE; $event->setApiRequest($apiRequest); $event->setApiProvider($this); - $event->stopPropagation(); // TODO decide if we really want to override others + $event->stopPropagation(); + // TODO decide if we really want to override others } } /** * @param \Civi\API\Event\AuthorizeEvent $event + * API authorization event. */ public function onApiAuthorize(\Civi\API\Event\AuthorizeEvent $event) { $apiRequest = $event->getApiRequest(); if (isset($apiRequest['is_metadata'])) { - // if (\CRM_Core_Permission::check('access AJAX API') || \CRM_Core_Permission::check('access CiviCRM')) { + // if (\CRM_Core_Permission::check('access AJAX API') + // || \CRM_Core_Permission::check('access CiviCRM')) { $event->authorize(); $event->stopPropagation(); // } @@ -107,26 +116,28 @@ class ReflectionProvider implements EventSubscriberInterface, ProviderInterface switch ($apiRequest['action']) { case 'getactions': return civicrm_api3_create_success($this->apiKernel->getActionNames($apiRequest['version'], $apiRequest['entity']), $apiRequest['params'], $apiRequest['entity'], $apiRequest['action']); -// case 'getfields': -// return $this->doGetFields($apiRequest); + + //case 'getfields': + // return $this->doGetFields($apiRequest); + default: } // We shouldn't get here because onApiResolve() checks $this->actions - throw new \API_Exception("Unsupported action (" . $apiRequest['entity'] . '.' . $apiRequest['action'] . ']'); + throw new \API_Exception("Unsupported action (" . $apiRequest['entity'] . '.' . $apiRequest['action'] . ']'); } /** * {inheritdoc} */ - function getEntityNames($version) { + public function getEntityNames($version) { return array('Entity'); } /** * {inheritdoc} */ - function getActionNames($version, $entity) { + public function getActionNames($version, $entity) { $entity = _civicrm_api_get_camel_name($entity, $version); return isset($this->actions[$entity]) ? $this->actions[$entity] : $this->actions['*']; } diff --git a/Civi/API/Provider/StaticProvider.php b/Civi/API/Provider/StaticProvider.php index 3665d08eda..9572836aec 100644 --- a/Civi/API/Provider/StaticProvider.php +++ b/Civi/API/Provider/StaticProvider.php @@ -30,14 +30,19 @@ namespace Civi\API\Provider; use Civi\API\Events; /** - * A static provider is useful for creating mock API implementations which manages records in-memory. + * A static provider is useful for creating mock API implementations which + * manages records in-memory. * - * TODO Add a static provider to SyntaxConformanceTest to ensure that it's representative. + * TODO Add a static provider to SyntaxConformanceTest to ensure that it's + * representative. */ class StaticProvider extends AdhocProvider { protected $records; protected $fields; + /** + * @return array + */ public static function getSubscribedEvents() { return array( Events::RESOLVE => array( @@ -49,6 +54,18 @@ class StaticProvider extends AdhocProvider { ); } + /** + * @param int $version + * API version. + * @param string $entity + * API entity. + * @param array $fields + * List of fields in this fake entity. + * @param array $perms + * Array(string $action => string $perm). + * @param array $records + * List of mock records to be read/updated by API calls. + */ public function __construct($version, $entity, $fields, $perms = array(), $records = array()) { parent::__construct($version, $entity); @@ -61,9 +78,9 @@ class StaticProvider extends AdhocProvider { $this->records = \CRM_Utils_Array::index(array('id'), $records); $this->fields = $fields; - $this->addAction('create', $perms['create'], array($this, '_create')); - $this->addAction('get', $perms['get'], array($this, '_get')); - $this->addAction('delete', $perms['delete'], array($this, '_delete')); + $this->addAction('create', $perms['create'], array($this, 'doCreate')); + $this->addAction('get', $perms['get'], array($this, 'doGet')); + $this->addAction('delete', $perms['delete'], array($this, 'doDelete')); } /** @@ -75,12 +92,20 @@ class StaticProvider extends AdhocProvider { /** * @param array $records + * List of mock records to be read/updated by API calls. */ public function setRecords($records) { $this->records = $records; } - public function _create($apiRequest) { + /** + * @param array $apiRequest + * The full description of the API request. + * @return array + * Formatted API result + * @throws \API_Exception + */ + public function doCreate($apiRequest) { if (isset($apiRequest['params']['id'])) { $id = $apiRequest['params']['id']; } @@ -102,7 +127,14 @@ class StaticProvider extends AdhocProvider { return civicrm_api3_create_success($this->records[$id]); } - public function _get($apiRequest) { + /** + * @param array $apiRequest + * The full description of the API request. + * @return array + * Formatted API result + * @throws \API_Exception + */ + public function doGet($apiRequest) { $id = @$apiRequest['params']['id']; if ($id && isset($this->records[$id])) { return civicrm_api3_create_success(array($id => $this->records[$id])); @@ -112,7 +144,14 @@ class StaticProvider extends AdhocProvider { } } - public function _delete($apiRequest) { + /** + * @param array $apiRequest + * The full description of the API request. + * @return array + * Formatted API result + * @throws \API_Exception + */ + public function doDelete($apiRequest) { $id = @$apiRequest['params']['id']; if ($id && isset($this->records[$id])) { unset($this->records[$id]); diff --git a/Civi/API/Request.php b/Civi/API/Request.php index 28b2bf2a80..a9678d4941 100644 --- a/Civi/API/Request.php +++ b/Civi/API/Request.php @@ -37,9 +37,13 @@ class Request { * Create a formatted/normalized request object. * * @param string $entity + * API entity name. * @param string $action + * API action name. * @param array $params + * API parameters. * @param mixed $extra + * Who knows? ... * * @throws \API_Exception * @return array the request descriptor; keys: @@ -145,11 +149,12 @@ class Request { * We must be sure that every request uses only one version of the API. * * @param array $params + * API parameters. * @return int */ protected static function parseVersion($params) { $desired_version = empty($params['version']) ? NULL : (int) $params['version']; - if (isset($desired_version) && is_integer($desired_version)) { + if (isset($desired_version) && is_int($desired_version)) { return $desired_version; } else { diff --git a/Civi/API/Subscriber/APIv3SchemaAdapter.php b/Civi/API/Subscriber/APIv3SchemaAdapter.php index 54ec555659..8d25276215 100644 --- a/Civi/API/Subscriber/APIv3SchemaAdapter.php +++ b/Civi/API/Subscriber/APIv3SchemaAdapter.php @@ -26,6 +26,7 @@ */ namespace Civi\API\Subscriber; + use Civi\API\Events; use Symfony\Component\EventDispatcher\EventSubscriberInterface; @@ -48,6 +49,7 @@ class APIv3SchemaAdapter implements EventSubscriberInterface { /** * @param \Civi\API\Event\PrepareEvent $event + * API preparation event. * * @throws \API_Exception */ @@ -64,7 +66,7 @@ class APIv3SchemaAdapter implements EventSubscriberInterface { if (empty($apiRequest['params']['id'])) { $apiRequest['params'] = array_merge($this->getDefaults($apiRequest['fields']), $apiRequest['params']); } - //if 'id' is set then only 'version' will be checked but should still be checked for consistency + // Note: If 'id' is set then verify_mandatory will only check 'version'. civicrm_api3_verify_mandatory($apiRequest['params'], NULL, $this->getRequired($apiRequest['fields'])); } @@ -73,12 +75,14 @@ class APIv3SchemaAdapter implements EventSubscriberInterface { /** * @param \Civi\API\Event\Event $event + * API preparation event. * * @throws \Exception */ public function onApiPrepare_validate(\Civi\API\Event\Event $event) { - $apiRequest = $event->getApiRequest(); - // Not sure why this is omitted for generic actions. It would make sense to omit 'getfields', but that's only one generic action. + $apiRequest = $event->getApiRequest(); + // Not sure why this is omitted for generic actions. It would make sense + // to omit 'getfields', but that's only one generic action. if (isset($apiRequest['function']) && !$apiRequest['is_generic'] && isset($apiRequest['fields'])) { _civicrm_api3_validate_fields($apiRequest['entity'], $apiRequest['action'], $apiRequest['params'], $apiRequest['fields']); @@ -87,7 +91,7 @@ class APIv3SchemaAdapter implements EventSubscriberInterface { } /** - * Return array of defaults for the given API (function is a wrapper on getfields) + * Return array of defaults for the given API (function is a wrapper on getfields). */ public function getDefaults($fields) { $defaults = array(); @@ -101,7 +105,7 @@ class APIv3SchemaAdapter implements EventSubscriberInterface { } /** - * Return array of required fields for the given API (function is a wrapper on getfields) + * Return array of required fields for the given API (function is a wrapper on getfields). */ public function getRequired($fields) { $required = array('version'); diff --git a/Civi/API/Subscriber/ChainSubscriber.php b/Civi/API/Subscriber/ChainSubscriber.php index 1b89222af9..93a2d34540 100644 --- a/Civi/API/Subscriber/ChainSubscriber.php +++ b/Civi/API/Subscriber/ChainSubscriber.php @@ -30,7 +30,8 @@ use Civi\API\Events; use Symfony\Component\EventDispatcher\EventSubscriberInterface; /** - * The ChainSubscriber looks for API parameters which specify a nested or chained API call. For example: + * The ChainSubscriber looks for API parameters which specify a nested or + * chained API call. For example: * * @code * $result = civicrm_api('Contact', 'create', array( @@ -43,8 +44,9 @@ use Symfony\Component\EventDispatcher\EventSubscriberInterface; * )); * @endcode * - * The ChainSubscriber looks for any parameters of the form "api.Email.create"; if found, it issues the nested - * API call (and passes some extra context -- eg Amy's contact_id). + * The ChainSubscriber looks for any parameters of the form "api.Email.create"; + * if found, it issues the nested API call (and passes some extra context -- + * eg Amy's contact_id). */ class ChainSubscriber implements EventSubscriberInterface { /** @@ -58,6 +60,7 @@ class ChainSubscriber implements EventSubscriberInterface { /** * @param \Civi\API\Event\RespondEvent $event + * API response event. * * @throws \Exception */ @@ -65,7 +68,7 @@ class ChainSubscriber implements EventSubscriberInterface { $apiRequest = $event->getApiRequest(); $result = $event->getResponse(); if (\CRM_Utils_Array::value('is_error', $result, 0) == 0) { - $this->_civicrm_api_call_nested_api($apiRequest['params'], $result, $apiRequest['action'], $apiRequest['entity'], $apiRequest['version']); + $this->callNestedApi($apiRequest['params'], $result, $apiRequest['action'], $apiRequest['entity'], $apiRequest['version']); $event->setResponse($result); } } @@ -75,10 +78,11 @@ class ChainSubscriber implements EventSubscriberInterface { * * TODO: We don't really need this to be a separate function. */ - protected function _civicrm_api_call_nested_api(&$params, &$result, $action, $entity, $version) { + protected function callNestedApi(&$params, &$result, $action, $entity, $version) { $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 + // 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'))) { return; } @@ -93,7 +97,8 @@ class ChainSubscriber implements EventSubscriberInterface { foreach ($params as $field => $newparams) { if ((is_array($newparams) || $newparams === 1) && $field <> 'api.has_parent' && substr($field, 0, 3) == 'api') { - // 'api.participant.delete' => 1 is a valid options - handle 1 instead of an array + // 'api.participant.delete' => 1 is a valid options - handle 1 + // instead of an array if ($newparams === 1) { $newparams = array('version' => $version); } @@ -114,17 +119,19 @@ class ChainSubscriber implements EventSubscriberInterface { if (strtolower($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 'entity_table' will be set to 'contact' & 'id' to the contact id from - //the parent call. - //in this case 'contact_id' will also be set to the parent's id + //set entity_id & entity table based on the parent's id & entity. + //e.g for something like note if the parent call is contact + //'entity_table' will be set to 'contact' & 'id' to the contact id + //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']; } if (strtolower($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 & + //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 & //thus limiting the ability to chain email //TODO - this might need the camel treatment $subParams['id'] = $parentAPIValues[$subEntity . "_id"]; @@ -133,12 +140,12 @@ class ChainSubscriber implements EventSubscriberInterface { if (\CRM_Utils_Array::value('entity_table', $result['values'][$idIndex]) == $subEntity) { $subParams['id'] = $result['values'][$idIndex]['entity_id']; } - // if we are dealing with the same entity pass 'id' through (useful for get + delete for example) + // if we are dealing with the same entity pass 'id' through + // (useful for get + delete for example) if (strtolower($entity) == strtolower($subEntity)) { $subParams['id'] = $result['values'][$idIndex]['id']; } - $subParams['version'] = $version; if (!empty($params['check_permissions'])) { $subParams['check_permissions'] = $params['check_permissions']; diff --git a/Civi/API/Subscriber/DynamicFKAuthorization.php b/Civi/API/Subscriber/DynamicFKAuthorization.php index cc7ade15ed..cd6ce4e695 100644 --- a/Civi/API/Subscriber/DynamicFKAuthorization.php +++ b/Civi/API/Subscriber/DynamicFKAuthorization.php @@ -31,16 +31,18 @@ use Civi\API\Events; use Symfony\Component\EventDispatcher\EventSubscriberInterface; /** - * Given an entity which dynamically attaches itself to another entity, determine if one has permission - * to the other entity. + * Given an entity which dynamically attaches itself to another entity, + * determine if one has permission to the other entity. * - * Example: Suppose one tries to manipulate a File which is attached to a Mailing. DynamicFKAuthorization will - * enforce permissions on the File by imitating the permissions of the Mailing. + * Example: Suppose one tries to manipulate a File which is attached to a + * Mailing. DynamicFKAuthorization will enforce permissions on the File by + * imitating the permissions of the Mailing. * - * Note: This enforces a constraint: all matching API calls must define either "id" (e.g. for the file) - * or "entity_table". + * Note: This enforces a constraint: all matching API calls must define + * either "id" (e.g. for the file) or "entity_table". * - * Note: The permission guard does not exactly authorize the request, but it may veto authorization. + * Note: The permission guard does not exactly authorize the request, but it + * may veto authorization. */ class DynamicFKAuthorization implements EventSubscriberInterface { @@ -93,12 +95,19 @@ class DynamicFKAuthorization implements EventSubscriberInterface { /** * @param \Civi\API\Kernel $kernel - * @param string $entityName the entity for which we want to manage permissions (e.g. "File" or "Note") - * @param array $actions the actions for which we want to manage permissions (e.g. "create", "get", "delete") - * @param string $lookupDelegateSql see docblock in DynamicFKAuthorization::$lookupDelegateSql - * @param array|NULL $allowedDelegates e.g. "civicrm_mailing","civicrm_activity"; NULL to allow any + * The API kernel. + * @param string $entityName + * The entity for which we want to manage permissions (e.g. "File" or + * "Note"). + * @param array $actions + * The actions for which we want to manage permissions (e.g. "create", + * "get", "delete"). + * @param string $lookupDelegateSql + * See docblock in DynamicFKAuthorization::$lookupDelegateSql. + * @param array|NULL $allowedDelegates + * e.g. "civicrm_mailing","civicrm_activity"; NULL to allow any. */ - function __construct($kernel, $entityName, $actions, $lookupDelegateSql, $allowedDelegates = NULL) { + public function __construct($kernel, $entityName, $actions, $lookupDelegateSql, $allowedDelegates = NULL) { $this->kernel = $kernel; $this->entityName = $entityName; $this->actions = $actions; @@ -108,6 +117,7 @@ class DynamicFKAuthorization implements EventSubscriberInterface { /** * @param \Civi\API\Event\AuthorizeEvent $event + * API authorization event. * @throws \API_Exception * @throws \Civi\API\Exception\UnauthorizedException */ @@ -129,9 +139,11 @@ class DynamicFKAuthorization implements EventSubscriberInterface { } elseif ($isValidId) { throw new \API_Exception("Failed to match record to related entity"); - } elseif (!$isValidId && strtolower($apiRequest['action']) == 'get') { - // This matches will be an empty set; doesn't make a difference if we reject or accept - // To pass SyntaxConformanceTest, we won't veto "get" on empty-set + } + elseif (!$isValidId && strtolower($apiRequest['action']) == 'get') { + // The matches will be an empty set; doesn't make a difference if we + // reject or accept. + // To pass SyntaxConformanceTest, we won't veto "get" on empty-set. return; } } @@ -151,10 +163,14 @@ class DynamicFKAuthorization implements EventSubscriberInterface { } /** - * @param string $action e.g. "create" - * @param string $entityTable e.g. "civicrm_mailing" + * @param string $action + * The API action (e.g. "create"). + * @param string $entityTable + * The target entity table (e.g. "civicrm_mailing"). * @param int|NULL $entityId + * The target entity ID. * @param array $apiRequest + * The full API request. * @throws \API_Exception * @throws \Civi\API\Exception\UnauthorizedException */ @@ -179,12 +195,17 @@ class DynamicFKAuthorization implements EventSubscriberInterface { } /** - * If the request attempts to change the entity_table/entity_id of an existing record, then generate an error. + * If the request attempts to change the entity_table/entity_id of an + * existing record, then generate an error. * - * @param int $fileId the main record being changed - * @param string $entityTable the saved FK - * @param int $entityId the saved FK + * @param int $fileId + * The main record being changed. + * @param string $entityTable + * The saved FK. + * @param int $entityId + * The saved FK. * @param array $apiRequest + * The full API request. * @throws \API_Exception */ public function preventReassignment($fileId, $entityTable, $entityId, $apiRequest) { @@ -199,8 +220,10 @@ class DynamicFKAuthorization implements EventSubscriberInterface { } /** - * @param string $entityTable e.g. "civicrm_mailing" or "civicrm_activity" - * @return string|NULL e.g. "Mailing" or "Activity" + * @param string $entityTable + * The target entity table (e.g. "civicrm_mailing" or "civicrm_activity"). + * @return string|NULL + * The target entity name (e.g. "Mailing" or "Activity"). */ public function getDelegatedEntityName($entityTable) { if ($this->allowedDelegates === NULL || in_array($entityTable, $this->allowedDelegates)) { @@ -216,19 +239,24 @@ class DynamicFKAuthorization implements EventSubscriberInterface { } /** - * @param string $action e.g. "create" ("When running *create* on a file...") - * @return string e.g. "create" ("Check for *create* permission on the mailing to which it is attached.") + * @param string $action + * API action name -- e.g. "create" ("When running *create* on a file..."). + * @return string + * e.g. "create" ("Check for *create* permission on the mailing to which + * it is attached.") */ public function getDelegatedAction($action) { switch ($action) { case 'get': // reading attachments requires reading the other entity return 'get'; - break; + case 'create': case 'delete': - // creating/updating/deleting attachments requires editing the other entity + // creating/updating/deleting an attachment requires editing + // the other entity return 'create'; + default: return $action; } @@ -236,11 +264,12 @@ class DynamicFKAuthorization implements EventSubscriberInterface { /** * @param int $id + * e.g. file ID. * @return array (0 => bool $isValid, 1 => string $entityTable, 2 => int $entityId) */ public function getDelegate($id) { $query = \CRM_Core_DAO::executeQuery($this->lookupDelegateSql, array( - 1 => array($id, 'Positive') + 1 => array($id, 'Positive'), )); if ($query->fetch()) { return array($query->is_valid, $query->entity_table, $query->entity_id); @@ -252,6 +281,7 @@ class DynamicFKAuthorization implements EventSubscriberInterface { /** * @param array $apiRequest + * The full API request. * @return bool */ public function isTrusted($apiRequest) { diff --git a/Civi/API/Subscriber/I18nSubscriber.php b/Civi/API/Subscriber/I18nSubscriber.php index ec44a3c7a3..2f11eb88dd 100644 --- a/Civi/API/Subscriber/I18nSubscriber.php +++ b/Civi/API/Subscriber/I18nSubscriber.php @@ -26,6 +26,7 @@ */ namespace Civi\API\Subscriber; + use Civi\API\Events; use Symfony\Component\EventDispatcher\EventSubscriberInterface; @@ -39,12 +40,13 @@ class I18nSubscriber implements EventSubscriberInterface { */ public static function getSubscribedEvents() { return array( - Events::PREPARE => array('onApiPrepare', Events::W_MIDDLE) + Events::PREPARE => array('onApiPrepare', Events::W_MIDDLE), ); } /** * @param \Civi\API\Event\Event $event + * API preparation event. * * @throws \API_Exception */ diff --git a/Civi/API/Subscriber/PermissionCheck.php b/Civi/API/Subscriber/PermissionCheck.php index f9fecfe518..ebd080f56d 100644 --- a/Civi/API/Subscriber/PermissionCheck.php +++ b/Civi/API/Subscriber/PermissionCheck.php @@ -30,8 +30,9 @@ use Civi\API\Events; use Symfony\Component\EventDispatcher\EventSubscriberInterface; /** - * For any API requests that correspond to a Doctrine entity ($apiRequest['doctrineClass']), check - * permissions specified in Civi\API\Annotation\Permission. + * For any API requests that correspond to a Doctrine entity + * ($apiRequest['doctrineClass']), check permissions specified in + * Civi\API\Annotation\Permission. */ class PermissionCheck implements EventSubscriberInterface { /** @@ -47,6 +48,7 @@ class PermissionCheck implements EventSubscriberInterface { /** * @param \Civi\API\Event\AuthorizeEvent $event + * API authorization event. * * @throws \Civi\API\Exception\UnauthorizedException */ @@ -74,7 +76,8 @@ class PermissionCheck implements EventSubscriberInterface { if (is_array($permissions)) { $permissions = implode(' and ', $permissions); } - // FIXME: Generating the exception ourselves allows for detailed error but doesn't play well with multiple authz subscribers. + // FIXME: Generating the exception ourselves allows for detailed error + // but doesn't play well with multiple authz subscribers. throw new \Civi\API\Exception\UnauthorizedException("API permission check failed for {$apiRequest['entity']}/{$apiRequest['action']} call; insufficient permission: require $permissions"); } diff --git a/Civi/API/Subscriber/TransactionSubscriber.php b/Civi/API/Subscriber/TransactionSubscriber.php index 0a2939c062..1dd1b71277 100644 --- a/Civi/API/Subscriber/TransactionSubscriber.php +++ b/Civi/API/Subscriber/TransactionSubscriber.php @@ -26,6 +26,7 @@ */ namespace Civi\API\Subscriber; + use Civi\API\Events; use Symfony\Component\EventDispatcher\EventSubscriberInterface; @@ -33,9 +34,12 @@ use Symfony\Component\EventDispatcher\EventSubscriberInterface; * Class TransactionSubscriber * * Implement transaction management for API calls. Two API options are accepted: - * - is_transactional: bool|'nest' - if true, then all work is done inside a transaction. by default, true for mutator actions (C-UD) - * 'nest' will force creation of a nested transaction; otherwise, the default is to re-use any existing transactions - * - options.force_rollback: bool - if true, all work is done in a nested transaction which will be rolled back + * - is_transactional: bool|'nest' - if true, then all work is done inside a + * transaction. By default, true for mutator actions (C-UD). 'nest' will + * force creation of a nested transaction; otherwise, the default is to + * re-use any existing transactions. + * - options.force_rollback: bool - if true, all work is done in a nested + * transaction which will be rolled back. * * @package Civi\API\Subscriber */ @@ -68,12 +72,14 @@ class TransactionSubscriber implements EventSubscriberInterface { * Determine if an API request should be treated as transactional * * @param \Civi\API\Provider\ProviderInterface $apiProvider + * The API provider responsible for this request. * @param array $apiRequest + * The full API request. * @return bool */ public function isTransactional($apiProvider, $apiRequest) { if ($this->isForceRollback($apiProvider, $apiRequest)) { - return true; + return TRUE; } if (isset($apiRequest['params']['is_transactional'])) { return \CRM_Utils_String::strtobool($apiRequest['params']['is_transactional']) || $apiRequest['params']['is_transactional'] == 'nest'; @@ -85,11 +91,13 @@ class TransactionSubscriber implements EventSubscriberInterface { * Determine if caller wants us to *always* rollback. * * @param \Civi\API\Provider\ProviderInterface $apiProvider + * The API provider responsible for this request. * @param array $apiRequest + * The full API request. * @return bool */ public function isForceRollback($apiProvider, $apiRequest) { - // FIXME: When APIv3 uses better parsing, the [params][options][force_rollback] check will be redundant + // FIXME: When APIv3 uses better parsing, only one check will be needed. if (isset($apiRequest['params']['options']['force_rollback'])) { return \CRM_Utils_String::strtobool($apiRequest['params']['options']['force_rollback']); } @@ -103,14 +111,16 @@ class TransactionSubscriber implements EventSubscriberInterface { * Determine if caller wants a nested transaction or a re-used transaction. * * @param \Civi\API\Provider\ProviderInterface $apiProvider + * The API provider responsible for this request. * @param array $apiRequest + * The full API request. * @return bool True if a new nested transaction is required; false if active tx may be used */ public function isNested($apiProvider, $apiRequest) { if ($this->isForceRollback($apiProvider, $apiRequest)) { return TRUE; } - if (isset($apiRequest['params']['is_transactional']) && $apiRequest['params']['is_transactional'] === 'nest') { + if (isset($apiRequest['params']['is_transactional']) && $apiRequest['params']['is_transactional'] === 'nest') { return TRUE; } return FALSE; @@ -120,8 +130,9 @@ class TransactionSubscriber implements EventSubscriberInterface { * Open a new transaction instance (if appropriate in the current policy) * * @param \Civi\API\Event\PrepareEvent $event + * API preparation event. */ - function onApiPrepare(\Civi\API\Event\PrepareEvent $event) { + public function onApiPrepare(\Civi\API\Event\PrepareEvent $event) { $apiRequest = $event->getApiRequest(); if ($this->isTransactional($event->getApiProvider(), $apiRequest)) { $this->transactions[$apiRequest['id']] = new \CRM_Core_Transaction($this->isNested($event->getApiProvider(), $apiRequest)); @@ -133,8 +144,11 @@ class TransactionSubscriber implements EventSubscriberInterface { /** * Close any pending transactions + * + * @param \Civi\API\Event\RespondEvent $event + * API response event. */ - function onApiRespond(\Civi\API\Event\RespondEvent $event) { + public function onApiRespond(\Civi\API\Event\RespondEvent $event) { $apiRequest = $event->getApiRequest(); if (isset($this->transactions[$apiRequest['id']])) { if (civicrm_error($event->getResponse())) { @@ -148,8 +162,9 @@ class TransactionSubscriber implements EventSubscriberInterface { * Rollback the pending transaction * * @param \Civi\API\Event\ExceptionEvent $event + * API exception event. */ - function onApiException(\Civi\API\Event\ExceptionEvent $event) { + public function onApiException(\Civi\API\Event\ExceptionEvent $event) { $apiRequest = $event->getApiRequest(); if (isset($this->transactions[$apiRequest['id']])) { $this->transactions[$apiRequest['id']]->rollback(); diff --git a/Civi/API/Subscriber/WrapperAdapter.php b/Civi/API/Subscriber/WrapperAdapter.php index 312118a4c1..cdf6ff006c 100644 --- a/Civi/API/Subscriber/WrapperAdapter.php +++ b/Civi/API/Subscriber/WrapperAdapter.php @@ -26,6 +26,7 @@ */ namespace Civi\API\Subscriber; + use Civi\API\Events; use Symfony\Component\EventDispatcher\EventSubscriberInterface; @@ -53,13 +54,15 @@ class WrapperAdapter implements EventSubscriberInterface { /** * @param array $defaults + * array(\API_Wrapper). */ - function __construct($defaults = array()) { + public function __construct($defaults = array()) { $this->defaults = $defaults; } /** * @param \Civi\API\Event\PrepareEvent $event + * API preparation event. */ public function onApiPrepare(\Civi\API\Event\PrepareEvent $event) { $apiRequest = $event->getApiRequest(); @@ -74,6 +77,7 @@ class WrapperAdapter implements EventSubscriberInterface { /** * @param \Civi\API\Event\RespondEvent $event + * API response event. */ public function onApiRespond(\Civi\API\Event\RespondEvent $event) { $apiRequest = $event->getApiRequest(); @@ -89,6 +93,7 @@ class WrapperAdapter implements EventSubscriberInterface { /** * @param array $apiRequest + * The full API request. * @return array<\API_Wrapper> */ public function getWrappers($apiRequest) { diff --git a/Civi/API/Subscriber/XDebugSubscriber.php b/Civi/API/Subscriber/XDebugSubscriber.php index 20bf7c7e8c..316a4650df 100644 --- a/Civi/API/Subscriber/XDebugSubscriber.php +++ b/Civi/API/Subscriber/XDebugSubscriber.php @@ -45,8 +45,9 @@ class XDebugSubscriber implements EventSubscriberInterface { /** * @param \Civi\API\Event\RespondEvent $event + * API response event. */ - function onApiRespond(\Civi\API\Event\RespondEvent $event) { + public function onApiRespond(\Civi\API\Event\RespondEvent $event) { $apiRequest = $event->getApiRequest(); $result = $event->getResponse(); if (function_exists('xdebug_time_index') diff --git a/api/Exception.php b/api/Exception.php index 953fafdbe9..1b5d544f8e 100644 --- a/api/Exception.php +++ b/api/Exception.php @@ -11,16 +11,8 @@ /** * This api exception returns more information than the default one. The aim it let the api consumer know better what is exactly the error without having to parse the error message. * If you consume an api that doesn't return an error_code or the extra data you need, consider improving the api and contribute - * @param string $message - * the human friendly error message - * @param string $error_code - * a computer friendly error code. By convention, no space (but underscore allowed) - * ex: mandatory_missing, duplicate, invalid_format - * @param array $data - * extra params to return. eg an extra array of ids. It is not mandatory, but can help the computer using the api. Keep in mind the api consumer isn't to be trusted. eg. the database password is NOT a good extra data */ -class API_Exception extends Exception -{ +class API_Exception extends Exception { const UNAUTHORIZED = 'unauthorized'; const NOT_IMPLEMENTED = 'not-found'; @@ -28,15 +20,25 @@ class API_Exception extends Exception /** * @param string $message - * @param int $error_code + * The human friendly error message. + * @param mixed $error_code + * A computer friendly error code. By convention, no space (but underscore + * allowed) (ex: mandatory_missing, duplicate, invalid_format). * @param array $extraParams - * @param Exception $previous + * Extra params to return. eg an extra array of ids. It is not mandatory, + * but can help the computer using the api. Keep in mind the api consumer + * isn't to be trusted. eg. the database password is NOT a good extra data. + * @param Exception|NULL $previous + * A previous exception which caused this new exception. */ - public function __construct($message, $error_code = 0, $extraParams = array(),Exception $previous = null) { - if (is_numeric ($error_code)) // using int for error code "old way") + public function __construct($message, $error_code = 0, $extraParams = array(), Exception $previous = NULL) { + // Using int for error code "old way") ? + if (is_numeric($error_code)) { $code = $error_code; - else - $code=0; + } + else { + $code = 0; + } parent::__construct(ts($message), $code, $previous); $this->extraParams = $extraParams + array('error_code' => $error_code); } @@ -59,35 +61,38 @@ class API_Exception extends Exception /** * @return array */ - public function getErrorCodes(){ + public function getErrorCodes() { return array( - 2000 => '$params was not an array', - 2001 => 'Invalid Value for Date field', - 2100 => 'String value is longer than permitted length', - self::UNAUTHORIZED => 'Unauthorized', - self::NOT_IMPLEMENTED => 'Entity or method is not implemented', - ); + 2000 => '$params was not an array', + 2001 => 'Invalid Value for Date field', + 2100 => 'String value is longer than permitted length', + self::UNAUTHORIZED => 'Unauthorized', + self::NOT_IMPLEMENTED => 'Entity or method is not implemented', + ); } } + /** * This api exception returns more information than the default one. We are using it rather than * API_Exception from the api wrapper as the namespace is more generic - * @param string $message the human friendly error message - * @param string $error_code a computer friendly error code. By convention, no space (but underscore allowed) - * ex: mandatory_missing, duplicate, invalid_format - * @param array $data extra params to return. eg an extra array of ids. It is not mandatory, but can help the computer using the api. Keep in mind the api consumer isn't to be trusted. eg. the database password is NOT a good extra data */ -class CiviCRM_API3_Exception extends Exception -{ +class CiviCRM_API3_Exception extends Exception { private $extraParams = array(); /** * @param string $message - * @param int $error_code + * The human friendly error message. + * @param mixed $error_code + * A computer friendly error code. By convention, no space (but underscore + * allowed) (ex: mandatory_missing, duplicate, invalid_format). * @param array $extraParams - * @param Exception $previous + * Extra params to return. eg an extra array of ids. It is not mandatory, + * but can help the computer using the api. Keep in mind the api consumer + * isn't to be trusted. eg. the database password is NOT a good extra data. + * @param Exception|NULL $previous + * A previous exception which caused this new exception. */ - public function __construct($message, $error_code, $extraParams = array(),Exception $previous = null) { + public function __construct($message, $error_code, $extraParams = array(), Exception $previous = NULL) { parent::__construct(ts($message)); $this->extraParams = $extraParams + array('error_code' => $error_code); } @@ -100,6 +105,9 @@ class CiviCRM_API3_Exception extends Exception return __CLASS__ . ": [{$this->extraParams['error_code']}: {$this->message}\n"; } + /** + * @return mixed + */ public function getErrorCode() { return $this->extraParams['error_code']; } -- 2.25.1