API Kernel - cleanup deprecated fn & unused param
authorColeman Watts <coleman@civicrm.org>
Tue, 11 Feb 2020 22:29:06 +0000 (17:29 -0500)
committerColeman Watts <coleman@civicrm.org>
Wed, 12 Feb 2020 01:11:05 +0000 (20:11 -0500)
CRM/Admin/Form/Job.php
Civi/API/Kernel.php
Civi/API/Request.php
Civi/API/Subscriber/ChainSubscriber.php
Civi/API/Subscriber/DynamicFKAuthorization.php
tests/phpunit/Civi/API/Event/PrepareEventTest.php
tests/phpunit/Civi/API/KernelTest.php
tests/phpunit/Civi/API/RequestTest.php
tests/phpunit/Civi/API/Subscriber/DynamicFKAuthorizationTest.php
tests/phpunit/Civi/API/Subscriber/WhitelistSubscriberTest.php
tests/phpunit/api/v3/UtilsTest.php

index f0bc3fefd2f9985941679dfa6dd717ede425a4ca..8a8679b5b444a2ce54091a2faffcefc887ae2d58 100644 (file)
@@ -107,7 +107,7 @@ class CRM_Admin_Form_Job extends CRM_Admin_Form {
 
     /** @var \Civi\API\Kernel $apiKernel */
     $apiKernel = \Civi::service('civi_api_kernel');
-    $apiRequest = \Civi\API\Request::create($fields['api_entity'], $fields['api_action'], ['version' => 3], NULL);
+    $apiRequest = \Civi\API\Request::create($fields['api_entity'], $fields['api_action'], ['version' => 3]);
     try {
       $apiKernel->resolve($apiRequest);
     }
index 9aaa03a274845fbebd510227aa71387f34c5d8a2..f7a1d5c7289bfb6f1cf72f0644f9118b1e55628b 100644 (file)
@@ -44,20 +44,20 @@ class Kernel {
   }
 
   /**
-   * @deprecated
    * @param string $entity
-   *   Type of entities to deal with.
+   *   Name of entity: e.g. Contact, Activity, Event
    * @param string $action
-   *   Create, get, delete or some special action name.
+   *   Name of action: e.g. create, get, delete
    * @param array $params
    *   Array to be passed to API function.
-   * @param mixed $extra
-   *   Unused/deprecated.
+   *
    * @return array|int
+   * @throws \API_Exception
    * @see runSafe
+   * @deprecated
    */
-  public function run($entity, $action, $params, $extra = NULL) {
-    return $this->runSafe($entity, $action, $params, $extra);
+  public function run($entity, $action, $params) {
+    return $this->runSafe($entity, $action, $params);
   }
 
   /**
@@ -65,19 +65,17 @@ class Kernel {
    * normal format.
    *
    * @param string $entity
-   *   Type of entities to deal with.
+   *   Name of entity: e.g. Contact, Activity, Event
    * @param string $action
-   *   Create, get, delete or some special action name.
+   *   Name of action: e.g. create, get, delete
    * @param array $params
    *   Array to be passed to API function.
-   * @param mixed $extra
-   *   Unused/deprecated.
    *
    * @return array|int
    * @throws \API_Exception
    */
-  public function runSafe($entity, $action, $params, $extra = NULL) {
-    $apiRequest = Request::create($entity, $action, $params, $extra);
+  public function runSafe($entity, $action, $params) {
+    $apiRequest = Request::create($entity, $action, $params);
 
     try {
       $apiResponse = $this->runRequest($apiRequest);
@@ -109,16 +107,14 @@ class Kernel {
    *   Create, get, delete or some special action name.
    * @param array $params
    *   Array to be passed to function.
-   * @param mixed $extra
-   *   Unused/deprecated.
    *
    * @return bool
    *   TRUE if authorization would succeed.
    * @throws \Exception
    */
-  public function runAuthorize($entity, $action, $params, $extra = NULL) {
+  public function runAuthorize($entity, $action, $params) {
     $apiProvider = NULL;
-    $apiRequest = Request::create($entity, $action, $params, $extra);
+    $apiRequest = Request::create($entity, $action, $params);
 
     try {
       $this->boot($apiRequest);
index d3379ab437285ac6880e4d6964e4530f134b5c4c..11c32ffc5e0adfc2bd810826535653a5b354b51a 100644 (file)
@@ -26,8 +26,6 @@ class Request {
    *   API action name.
    * @param array $params
    *   API parameters.
-   * @param mixed $extra
-   *   Who knows? ...
    *
    * @throws \API_Exception
    * @return array
@@ -36,13 +34,12 @@ class Request {
    *   - entity: string
    *   - action: string
    *   - params: array (string $key => mixed $value) [deprecated in v4]
-   *   - extra: unspecified
    *   - fields: NULL|array (string $key => array $fieldSpec)
    *   - options: \CRM_Utils_OptionBag derived from params [v4-only]
    *   - data: \CRM_Utils_OptionBag derived from params [v4-only]
    *   - chains: unspecified derived from params [v4-only]
    */
-  public static function create($entity, $action, $params, $extra = NULL) {
+  public static function create($entity, $action, $params) {
     $version = \CRM_Utils_Array::value('version', $params);
     switch ($version) {
       default:
@@ -50,10 +47,9 @@ class Request {
         $apiRequest['id'] = self::$nextId++;
         $apiRequest['version'] = (int) $version;
         $apiRequest['params'] = $params;
-        $apiRequest['extra'] = $extra;
         $apiRequest['fields'] = NULL;
-        $apiRequest['entity'] = self::normalizeEntityName($entity, $apiRequest['version']);
-        $apiRequest['action'] = self::normalizeActionName($action, $apiRequest['version']);
+        $apiRequest['entity'] = self::normalizeEntityName($entity);
+        $apiRequest['action'] = self::normalizeActionName($action);
         return $apiRequest;
 
       case 4:
@@ -79,10 +75,9 @@ class Request {
    * APIv1-v3 munges entity/action names, and accepts any mixture of case and underscores.
    *
    * @param string $entity
-   * @param int $version
    * @return string
    */
-  public static function normalizeEntityName($entity, $version) {
+  public static function normalizeEntityName($entity) {
     return \CRM_Utils_String::convertStringToCamel(\CRM_Utils_String::munge($entity));
   }
 
@@ -95,7 +90,7 @@ class Request {
    * @param $version
    * @return string
    */
-  public static function normalizeActionName($action, $version) {
+  public static function normalizeActionName($action) {
     return strtolower(\CRM_Utils_String::munge($action));
   }
 
index 42baa4062ec4a65e5b9c1c7546dac044de3d29de..37c8688a4c86b73fec14eb660cc01ef497c28634 100644 (file)
@@ -183,7 +183,7 @@ class ChainSubscriber implements EventSubscriberInterface {
             foreach ($newparams as $entityparams) {
               $subParams = array_merge($genericParams, $entityparams);
               _civicrm_api_replace_variables($subParams, $result['values'][$idIndex], $separator);
-              $result['values'][$idIndex][$field][] = $apiKernel->run($subEntity, $subaction, $subParams);
+              $result['values'][$idIndex][$field][] = $apiKernel->runSafe($subEntity, $subaction, $subParams);
               if ($result['is_error'] === 1) {
                 throw new \Exception($subEntity . ' ' . $subaction . 'call failed with' . $result['error_message']);
               }
@@ -193,7 +193,7 @@ class ChainSubscriber implements EventSubscriberInterface {
 
             $subParams = array_merge($subParams, $newparams);
             _civicrm_api_replace_variables($subParams, $result['values'][$idIndex], $separator);
-            $result['values'][$idIndex][$field] = $apiKernel->run($subEntity, $subaction, $subParams);
+            $result['values'][$idIndex][$field] = $apiKernel->runSafe($subEntity, $subaction, $subParams);
             if (!empty($result['is_error'])) {
               throw new \Exception($subEntity . ' ' . $subaction . 'call failed with' . $result['error_message']);
             }
index 7747d0a6285ecfbdcfbbf9e256c27bfb7dda31ce..ecaf5ff428ecb8719090530fead9281366a65b5d 100644 (file)
@@ -230,7 +230,7 @@ class DynamicFKAuthorization implements EventSubscriberInterface {
         'id' => $entityId,
       ];
 
-      $result = $self->kernel->run($entity, $self->getDelegatedAction($action), $params);
+      $result = $self->kernel->runSafe($entity, $self->getDelegatedAction($action), $params);
       if ($result['is_error'] || empty($result['values'])) {
         $exception = new \Civi\API\Exception\UnauthorizedException("Authorization failed on ($entity,$entityId)", [
           'cause' => $result,
index 824e03d59f88677dfb9f27e23bbc4703979ea022..258a7bf642126dd3ff1d666626df53b1a8d616a2 100644 (file)
@@ -47,7 +47,7 @@ class PrepareEventTest extends \CiviUnitTestCase {
   public function testOnPrepare($onPrepare, $inputApiCall, $expectResult) {
     $this->dispatcher->addListener(Events::PREPARE, [$this, $onPrepare]);
     $this->kernel->registerApiProvider($this->createWidgetFrobnicateProvider());
-    $result = call_user_func_array([$this->kernel, 'run'], $inputApiCall);
+    $result = call_user_func_array([$this->kernel, 'runSafe'], $inputApiCall);
     $this->assertEquals($expectResult, $result['values']);
   }
 
index 63944f2e6da10f67420c3420f819d6972915bef9..6d8829b01d652c0100b7d89a2b6493ccaa45c010 100644 (file)
@@ -34,7 +34,7 @@ class KernelTest extends \CiviUnitTestCase {
 
   public function testNormalEvents() {
     $this->kernel->registerApiProvider($this->createWidgetFrobnicateProvider());
-    $result = $this->kernel->run('Widget', 'frobnicate', [
+    $result = $this->kernel->runSafe('Widget', 'frobnicate', [
       'version' => self::MOCK_VERSION,
     ]);
 
@@ -58,7 +58,7 @@ class KernelTest extends \CiviUnitTestCase {
     });
 
     $this->kernel->registerApiProvider($this->createWidgetFrobnicateProvider());
-    $result = $this->kernel->run('Widget', 'frobnicate', [
+    $result = $this->kernel->runSafe('Widget', 'frobnicate', [
       'version' => self::MOCK_VERSION,
     ]);
 
index be0e6821c9657faebfa018a28c25d65581b42294..3434d2cad7ec43f9fe5e8cd365d1a12269439daa 100644 (file)
@@ -37,7 +37,7 @@ class RequestTest extends \CiviUnitTestCase {
    */
   public function testCreateRequest_EntityActionMunging($input, $expected) {
     list ($inEntity, $inAction, $inVersion) = $input;
-    $apiRequest = Request::create($inEntity, $inAction, ['version' => $inVersion], NULL);
+    $apiRequest = Request::create($inEntity, $inAction, ['version' => $inVersion]);
     $this->assertEquals($expected, [$apiRequest['entity'], $apiRequest['action'], $apiRequest['version']]);
   }
 
index e1a4902218518c7762f1c6c94848b1f3d3808fa0..0934a626d2f8f5de63cd342c95473ecbbc043d00 100644 (file)
@@ -186,7 +186,7 @@ class DynamicFKAuthorizationTest extends \CiviUnitTestCase {
     $params['version'] = 3;
     $params['debug'] = 1;
     $params['check_permissions'] = 1;
-    $result = $this->kernel->run($entity, $action, $params);
+    $result = $this->kernel->runSafe($entity, $action, $params);
     $this->assertFalse((bool) $result['is_error'], print_r([
       '$entity' => $entity,
       '$action' => $action,
@@ -206,7 +206,7 @@ class DynamicFKAuthorizationTest extends \CiviUnitTestCase {
     $params['version'] = 3;
     $params['debug'] = 1;
     $params['check_permissions'] = 1;
-    $result = $this->kernel->run($entity, $action, $params);
+    $result = $this->kernel->runSafe($entity, $action, $params);
     $this->assertTrue((bool) $result['is_error'], print_r([
       '$entity' => $entity,
       '$action' => $action,
@@ -231,12 +231,12 @@ class DynamicFKAuthorizationTest extends \CiviUnitTestCase {
       'check_permissions' => 1,
     ];
     // run with permission check
-    $result = $this->kernel->run('FakeFile', 'create', $params);
+    $result = $this->kernel->runSafe('FakeFile', 'create', $params);
     $this->assertTrue((bool) $result['is_error'], 'Undelegated entity with check_permissions = 1 should fail');
     $this->assertRegExp('/Unrecognized target entity table \(civicrm_membership\)/', $result['error_message']);
     // repeat without permission check
     $params['check_permissions'] = 0;
-    $result = $this->kernel->run('FakeFile', 'create', $params);
+    $result = $this->kernel->runSafe('FakeFile', 'create', $params);
     $this->assertFalse((bool) $result['is_error'], 'Undelegated entity with check_permissions = 0 should succeed');
   }
 
index 73407752866c7224d42027aac9697e6033b8d349..d7fcd2dc624e061f4a784e25fca39e0341943cf3 100644 (file)
@@ -386,7 +386,7 @@ class WhitelistSubscriberTest extends \CiviUnitTestCase {
 
     $apiRequest['params']['debug'] = 1;
     $apiRequest['params']['check_permissions'] = 'whitelist';
-    $result = $kernel->run($apiRequest['entity'], $apiRequest['action'], $apiRequest['params']);
+    $result = $kernel->runSafe($apiRequest['entity'], $apiRequest['action'], $apiRequest['params']);
 
     if ($expectSuccess) {
       $this->assertAPISuccess($result);
index f11fa585ccb053b54d26d1d9efba7b00bb562308..e6f5a525308609ac21f096d826673b5c398f06bd 100644 (file)
@@ -98,7 +98,7 @@ class api_v3_UtilsTest extends CiviUnitTestCase {
     $dispatcher = new \Symfony\Component\EventDispatcher\EventDispatcher();
     $dispatcher->addSubscriber(new \Civi\API\Subscriber\PermissionCheck());
     $kernel = new \Civi\API\Kernel($dispatcher);
-    $apiRequest = \Civi\API\Request::create($entity, $action, $params, NULL);
+    $apiRequest = \Civi\API\Request::create($entity, $action, $params);
     try {
       $kernel->authorize(NULL, $apiRequest);
       return TRUE;
@@ -366,18 +366,18 @@ class api_v3_UtilsTest extends CiviUnitTestCase {
     });
     $kernel->registerApiProvider($provider);
 
-    $r1 = $kernel->run('Widget', 'get', $params);
+    $r1 = $kernel->runSafe('Widget', 'get', $params);
     $this->assertEquals(count($resultIds), $r1['count']);
     $this->assertEquals($resultIds, array_keys($r1['values']));
     $this->assertEquals($resultIds, array_values(CRM_Utils_Array::collect('snack_id', $r1['values'])));
     $this->assertEquals($resultIds, array_values(CRM_Utils_Array::collect('id', $r1['values'])));
 
-    $r2 = $kernel->run('Widget', 'get', $params + ['sequential' => 1]);
+    $r2 = $kernel->runSafe('Widget', 'get', $params + ['sequential' => 1]);
     $this->assertEquals(count($resultIds), $r2['count']);
     $this->assertEquals($resultIds, array_values(CRM_Utils_Array::collect('snack_id', $r2['values'])));
     $this->assertEquals($resultIds, array_values(CRM_Utils_Array::collect('id', $r2['values'])));
 
-    $r3 = $kernel->run('Widget', 'get', $params + ['options' => ['offset' => 1, 'limit' => 2]]);
+    $r3 = $kernel->runSafe('Widget', 'get', $params + ['options' => ['offset' => 1, 'limit' => 2]]);
     $slice = array_slice($resultIds, 1, 2);
     $this->assertEquals(count($slice), $r3['count']);
     $this->assertEquals($slice, array_values(CRM_Utils_Array::collect('snack_id', $r3['values'])));
@@ -398,7 +398,7 @@ class api_v3_UtilsTest extends CiviUnitTestCase {
     });
     $kernel->registerApiProvider($provider);
 
-    $r1 = $kernel->run('Widget', 'get', [
+    $r1 = $kernel->runSafe('Widget', 'get', [
       'version' => 3,
       'snack_id' => 'b',
       'return' => 'fruit',
@@ -406,7 +406,7 @@ class api_v3_UtilsTest extends CiviUnitTestCase {
     $this->assertAPISuccess($r1);
     $this->assertEquals(['b' => ['id' => 'b', 'fruit' => 'grape']], $r1['values']);
 
-    $r2 = $kernel->run('Widget', 'get', [
+    $r2 = $kernel->runSafe('Widget', 'get', [
       'version' => 3,
       'snack_id' => 'b',
       'return' => ['fruit', 'cheese'],
@@ -414,7 +414,7 @@ class api_v3_UtilsTest extends CiviUnitTestCase {
     $this->assertAPISuccess($r2);
     $this->assertEquals(['b' => ['id' => 'b', 'fruit' => 'grape', 'cheese' => 'cheddar']], $r2['values']);
 
-    $r3 = $kernel->run('Widget', 'get', [
+    $r3 = $kernel->runSafe('Widget', 'get', [
       'version' => 3,
       'cheese' => 'cheddar',
       'return' => ['fruit'],