From 463977aa6f78d3a5158bec97b25231dc67634914 Mon Sep 17 00:00:00 2001
From: Tim Otten <totten@civicrm.org>
Date: Mon, 5 Jul 2021 18:54:04 -0700
Subject: [PATCH] (REF) ReflectionUtils - Add findStandardProperties() and
 findMethodHelpers()

---
 Civi/Api4/Utils/ReflectionUtils.php           | 45 +++++++++++++++++++
 Civi/Schema/Traits/MagicGetterSetterTrait.php | 36 +++++++--------
 .../Civi/Schema/MagicGetterSetterTest.php     | 31 +++++++++++++
 3 files changed, 91 insertions(+), 21 deletions(-)

diff --git a/Civi/Api4/Utils/ReflectionUtils.php b/Civi/Api4/Utils/ReflectionUtils.php
index c48f921d0c..a5d59803f1 100644
--- a/Civi/Api4/Utils/ReflectionUtils.php
+++ b/Civi/Api4/Utils/ReflectionUtils.php
@@ -157,4 +157,49 @@ class ReflectionUtils {
     return $traits;
   }
 
+  /**
+   * Get a list of standard properties which can be written+read by outside callers.
+   *
+   * @param string $class
+   */
+  public static function findStandardProperties($class): iterable {
+    try {
+      /** @var \ReflectionClass $clazz */
+      $clazz = new \ReflectionClass($class);
+
+      yield from [];
+      foreach ($clazz->getProperties(\ReflectionProperty::IS_PROTECTED | \ReflectionProperty::IS_PUBLIC) as $property) {
+        if (!$property->isStatic() && $property->getName()[0] !== '_') {
+          yield $property;
+        }
+      }
+    }
+    catch (\ReflectionException $e) {
+      throw new \RuntimeException(sprintf("Cannot inspect class %s.", $class));
+    }
+  }
+
+  /**
+   * Find any methods in this class which match the given prefix.
+   *
+   * @param string $class
+   * @param string $prefix
+   */
+  public static function findMethodHelpers($class, string $prefix): iterable {
+    try {
+      /** @var \ReflectionClass $clazz */
+      $clazz = new \ReflectionClass($class);
+
+      yield from [];
+      foreach ($clazz->getMethods(\ReflectionMethod::IS_PUBLIC | \ReflectionMethod::IS_PROTECTED) as $m) {
+        if (\CRM_Utils_String::startsWith($m->getName(), $prefix)) {
+          yield $m;
+        }
+      }
+    }
+    catch (\ReflectionException $e) {
+      throw new \RuntimeException(sprintf("Cannot inspect class %s.", $class));
+    }
+  }
+
 }
diff --git a/Civi/Schema/Traits/MagicGetterSetterTrait.php b/Civi/Schema/Traits/MagicGetterSetterTrait.php
index cc5e0e325a..12f1ba6e3f 100644
--- a/Civi/Schema/Traits/MagicGetterSetterTrait.php
+++ b/Civi/Schema/Traits/MagicGetterSetterTrait.php
@@ -11,6 +11,8 @@
 
 namespace Civi\Schema\Traits;
 
+use Civi\Api4\Utils\ReflectionUtils;
+
 /**
  * Automatically define getter/setter methods for public and protected fields.
  *
@@ -52,6 +54,9 @@ trait MagicGetterSetterTrait {
           return $this->$prop;
 
         case 'set':
+          if (count($arguments) < 1) {
+            throw new \CRM_Core_Exception(sprintf('Missing required parameter for method %s::%s()', static::CLASS, $method));
+          }
           $this->$prop = $arguments[0];
           return $this;
       }
@@ -68,29 +73,18 @@ trait MagicGetterSetterTrait {
    *   Array(string $propertyName => bool $true).
    */
   protected static function getMagicProperties(): array {
-    // Thread-local cache of class metadata. This is strictly readonly and immutable, and it should ideally be reused across varied test-functions.
-    static $cache = [];
-
-    if (!isset($cache[static::CLASS])) {
-      try {
-        $clazz = new \ReflectionClass(static::CLASS);
-      }
-      catch (\ReflectionException $e) {
-        // This shouldn't happen. Cast to RuntimeException so that we don't have a million `@throws` statements.
-        throw new \RuntimeException(sprintf("Class %s cannot reflect upon itself.", static::CLASS));
-      }
-
-      $fields = [];
-      foreach ($clazz->getProperties(\ReflectionProperty::IS_PROTECTED | \ReflectionProperty::IS_PUBLIC) as $property) {
-        $name = $property->getName();
-        if (!$property->isStatic() && $name[0] !== '_') {
-          $fields[$name] = TRUE;
-        }
+    // Thread-local cache of class metadata. Class metadata is immutable at runtime, so this is strictly write-once. It should ideally be reused across varied test-functions.
+    static $caches = [];
+    $CLASS = static::CLASS;
+    $cache =& $caches[$CLASS];
+    if ($cache === NULL) {
+      $cache = [];
+      foreach (ReflectionUtils::findStandardProperties(static::CLASS) as $property) {
+        /** @var \ReflectionProperty $property */
+        $cache[$property->getName()] = TRUE;
       }
-      unset($clazz);
-      $cache[static::CLASS] = $fields;
     }
-    return $cache[static::CLASS];
+    return $cache;
   }
 
 }
diff --git a/tests/phpunit/Civi/Schema/MagicGetterSetterTest.php b/tests/phpunit/Civi/Schema/MagicGetterSetterTest.php
index 8079d3895c..02a747cfdf 100644
--- a/tests/phpunit/Civi/Schema/MagicGetterSetterTest.php
+++ b/tests/phpunit/Civi/Schema/MagicGetterSetterTest.php
@@ -3,6 +3,7 @@
 namespace Civi\Schema;
 
 use Civi\Schema\Traits\MagicGetterSetterTrait;
+use Civi\Test\Invasive;
 
 class MagicGetterSetterTest extends \CiviUnitTestCase {
 
@@ -38,6 +39,20 @@ class MagicGetterSetterTest extends \CiviUnitTestCase {
     };
   }
 
+  public function createAltExample() {
+    return new class() {
+      use MagicGetterSetterTrait;
+      protected $altField;
+
+    };
+  }
+
+  public function createEmptyExample() {
+    return new class() {
+      use MagicGetterSetterTrait;
+    };
+  }
+
   public function testExample() {
     $ex = $this->createExample();
     $this->assertEquals(NULL, $ex->setProtectedField(NULL)->getProtectedField());
@@ -92,4 +107,20 @@ class MagicGetterSetterTest extends \CiviUnitTestCase {
     }
   }
 
+  public function testImplIndependence() {
+    $ex1 = $this->createExample();
+    $ex2 = $this->createAltExample();
+    $ex3 = $this->createEmptyExample();
+
+    // Multiple alternating calls. Caches remain independent.
+    foreach (range(0, 2) as $i) {
+      $this->assertEquals(
+        ['protectedField', 'publicField', 'overriddenProtectedField', 'set', 'get'],
+        array_keys(Invasive::call([$ex1, 'getMagicProperties']))
+      );
+      $this->assertEquals(['altField'], array_keys(Invasive::call([$ex2, 'getMagicProperties'])));
+      $this->assertEquals([], array_keys(Invasive::call([$ex3, 'getMagicProperties'])));
+    }
+  }
+
 }
-- 
2.25.1