From: colemanw <coleman@civicrm.org>
Date: Mon, 2 Oct 2023 18:36:05 +0000 (-0400)
Subject: APIv4 - ensure action names get camelCase properly
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=c26a7a076cb21df9bb79ff7cf57796be49184f4b;p=civicrm-core.git

APIv4 - ensure action names get camelCase properly
---

diff --git a/Civi/Api4/Generic/AbstractAction.php b/Civi/Api4/Generic/AbstractAction.php
index 43563bae3e..ade0f637b7 100644
--- a/Civi/Api4/Generic/AbstractAction.php
+++ b/Civi/Api4/Generic/AbstractAction.php
@@ -153,12 +153,19 @@ abstract class AbstractAction implements \ArrayAccess {
    * @param string $actionName
    */
   public function __construct($entityName, $actionName) {
-    // If a namespaced class name is passed in
-    if (strpos($entityName, '\\') !== FALSE) {
-      $entityName = substr($entityName, strrpos($entityName, '\\') + 1);
+    // If a namespaced class name is passed in, convert to entityName
+    $this->_entityName = CoreUtil::stripNamespace($entityName);
+    // Normalize action name case (because PHP is case-insensitive, we have to do an extra check)
+    $thisClassName = CoreUtil::stripNamespace(get_class($this));
+    // If this was called via magic method, $actionName won't necessarily have the
+    // correct case because PHP doesn't care about case when calling methods.
+    if (strtolower($thisClassName) === strtolower($actionName)) {
+      $this->_actionName = lcfirst($thisClassName);
+    }
+    // If called via static method, case should already be correct.
+    else {
+      $this->_actionName = $actionName;
     }
-    $this->_entityName = $entityName;
-    $this->_actionName = $actionName;
     $this->_id = \Civi\API\Request::getNextId();
   }
 
diff --git a/Civi/Api4/Generic/AbstractEntity.php b/Civi/Api4/Generic/AbstractEntity.php
index 5caca47384..b6681d52be 100644
--- a/Civi/Api4/Generic/AbstractEntity.php
+++ b/Civi/Api4/Generic/AbstractEntity.php
@@ -12,6 +12,7 @@
 namespace Civi\Api4\Generic;
 
 use Civi\API\Exception\NotImplementedException;
+use Civi\Api4\Utils\CoreUtil;
 use Civi\Api4\Utils\ReflectionUtils;
 
 /**
@@ -75,7 +76,7 @@ abstract class AbstractEntity {
    * @return string
    */
   public static function getEntityName(): string {
-    return self::stripNamespace(static::class);
+    return CoreUtil::stripNamespace(static::class);
   }
 
   /**
@@ -128,7 +129,7 @@ abstract class AbstractEntity {
       'name' => $entityName,
       'title' => static::getEntityTitle(),
       'title_plural' => static::getEntityTitle(TRUE),
-      'type' => [self::stripNamespace(get_parent_class(static::class))],
+      'type' => [CoreUtil::stripNamespace(get_parent_class(static::class))],
       'paths' => [],
       'class' => static::class,
       'primary_key' => ['id'],
@@ -148,7 +149,7 @@ abstract class AbstractEntity {
       $info['icon_field'] = (array) ($dao::fields()['icon']['name'] ?? NULL);
     }
     foreach (ReflectionUtils::getTraits(static::class) as $trait) {
-      $info['type'][] = self::stripNamespace($trait);
+      $info['type'][] = CoreUtil::stripNamespace($trait);
     }
     // Get DocBlock from APIv4 Entity class
     $reflection = new \ReflectionClass(static::class);
@@ -179,14 +180,4 @@ abstract class AbstractEntity {
     return $info;
   }
 
-  /**
-   * Remove namespace prefix from a class name
-   *
-   * @param string $className
-   * @return string
-   */
-  private static function stripNamespace(string $className): string {
-    return substr($className, strrpos($className, '\\') + 1);
-  }
-
 }
diff --git a/Civi/Api4/Query/SqlExpression.php b/Civi/Api4/Query/SqlExpression.php
index 9555152db6..de05876e48 100644
--- a/Civi/Api4/Query/SqlExpression.php
+++ b/Civi/Api4/Query/SqlExpression.php
@@ -11,6 +11,8 @@
 
 namespace Civi\Api4\Query;
 
+use Civi\Api4\Utils\CoreUtil;
+
 /**
  * Base class for SqlColumn, SqlString, SqlBool, and SqlFunction classes.
  *
@@ -174,8 +176,7 @@ abstract class SqlExpression {
    * @return string
    */
   public function getType(): string {
-    $className = get_class($this);
-    return substr($className, strrpos($className, '\\') + 1);
+    return CoreUtil::stripNamespace(get_class($this));
   }
 
   /**
diff --git a/Civi/Api4/Utils/CoreUtil.php b/Civi/Api4/Utils/CoreUtil.php
index fb5df8e6d3..bbace033ee 100644
--- a/Civi/Api4/Utils/CoreUtil.php
+++ b/Civi/Api4/Utils/CoreUtil.php
@@ -437,4 +437,14 @@ class CoreUtil {
     }
   }
 
+  /**
+   * Strips leading namespace from a classname
+   * @param string $className
+   * @return string
+   */
+  public static function stripNamespace(string $className): string {
+    $slashPos = strrpos($className, '\\');
+    return $slashPos === FALSE ? $className : substr($className, $slashPos + 1);
+  }
+
 }
diff --git a/tests/phpunit/api/v4/Action/ActionNameTest.php b/tests/phpunit/api/v4/Action/ActionNameTest.php
new file mode 100644
index 0000000000..46d27dca45
--- /dev/null
+++ b/tests/phpunit/api/v4/Action/ActionNameTest.php
@@ -0,0 +1,57 @@
+<?php
+
+/*
+ +--------------------------------------------------------------------+
+ | Copyright CiviCRM LLC. All rights reserved.                        |
+ |                                                                    |
+ | This work is published under the GNU AGPLv3 license with some      |
+ | permitted exceptions and without any warranty. For full license    |
+ | and copyright information, see https://civicrm.org/licensing       |
+ +--------------------------------------------------------------------+
+ */
+
+/**
+ *
+ * @package CRM
+ * @copyright CiviCRM LLC https://civicrm.org/licensing
+ */
+
+namespace api\v4\Action;
+
+use api\v4\Api4TestBase;
+use Civi\Api4\MockArrayEntity;
+
+/**
+ * @group headless
+ */
+class ActionNameTest extends Api4TestBase {
+
+  public function testActionCaseSensitive(): void {
+    // This test checks that an action called via STATIC method will internally
+    // be converted to the proper case.
+    // First: ensure the static method DOES exist. If the class is ever refactored to change this,
+    // then this test will no longer be testing what it thinks it's testing!
+    $this->assertTrue(method_exists(MockArrayEntity::class, 'getFields'));
+
+    // PHP is case-insensitive so this will work <sigh>
+    $action = MOCKarrayENTITY::GETFiELDS();
+    // Ensure case was converted internally by the action class
+    $this->assertEquals('getFields', $action->getActionName());
+    $this->assertEquals('MockArrayEntity', $action->getEntityName());
+  }
+
+  public function testActionCaseSensitiveViaMagicMethod(): void {
+    // This test checks that an action called via MAGIC method will internally
+    // be converted to the proper case.
+    // First: ensure the static method does NOT exist. If the class is ever refactored to change this,
+    // then this test will no longer be testing what it thinks it's testing!
+    $this->assertFalse(method_exists(MockArrayEntity::class, 'doNothing'));
+
+    // PHP is case-insensitive so this will work <sigh>
+    $action = moCKarrayENTIty::DOnothING();
+    // Ensure case was converted internally by the action class
+    $this->assertEquals('doNothing', $action->getActionName());
+    $this->assertEquals('MockArrayEntity', $action->getEntityName());
+  }
+
+}
diff --git a/tests/phpunit/api/v4/Custom/CoreUtilTest.php b/tests/phpunit/api/v4/Custom/CoreUtilTest.php
index fbd329f5dc..3418f27d12 100644
--- a/tests/phpunit/api/v4/Custom/CoreUtilTest.php
+++ b/tests/phpunit/api/v4/Custom/CoreUtilTest.php
@@ -77,4 +77,19 @@ class CoreUtilTest extends CustomTestBase {
     $this->assertEquals('Civi\Api4\CustomValue', CoreUtil::getApiClass('Custom_' . $multiGroup['name']));
   }
 
+  public function getNamespaceExamples(): array {
+    return [
+      ['\Foo', 'Foo'],
+      ['\Foo\Bar', 'Bar'],
+      ['Baz', 'Baz'],
+    ];
+  }
+
+  /**
+   * @dataProvider getNamespaceExamples
+   */
+  public function testStripNamespace($input, $expected): void {
+    $this->assertEquals($expected, CoreUtil::stripNamespace($input));
+  }
+
 }
diff --git a/tests/phpunit/api/v4/Mock/Api4/Action/MockArrayEntity/DoNothing.php b/tests/phpunit/api/v4/Mock/Api4/Action/MockArrayEntity/DoNothing.php
new file mode 100644
index 0000000000..a554a8cc55
--- /dev/null
+++ b/tests/phpunit/api/v4/Mock/Api4/Action/MockArrayEntity/DoNothing.php
@@ -0,0 +1,19 @@
+<?php
+
+namespace Civi\Api4\Action\MockArrayEntity;
+
+use Civi\Api4\Generic\Result;
+
+/**
+ * Action that does nothing; called via magic method
+ */
+class DoNothing extends \Civi\Api4\Generic\AbstractAction {
+
+  /**
+   * @inheritDoc
+   */
+  public function _run(Result $result) {
+    // Doing exactly as advertised.
+  }
+
+}
diff --git a/tests/phpunit/api/v4/Mock/Api4/MockArrayEntity.php b/tests/phpunit/api/v4/Mock/Api4/MockArrayEntity.php
index cc8e4cf636..b3d0b99b7a 100644
--- a/tests/phpunit/api/v4/Mock/Api4/MockArrayEntity.php
+++ b/tests/phpunit/api/v4/Mock/Api4/MockArrayEntity.php
@@ -21,7 +21,8 @@ namespace Civi\Api4;
 /**
  * MockArrayEntity entity.
  *
- * @method static Generic\BasicGetAction get()
+ * @method static Action\MockArrayEntity\Get get()
+ * @method static Action\MockArrayEntity\DoNothing doNothing()
  *
  * @package Civi\Api4
  */