Cache reflection lookups in ExtendableTrait magic method handling

src/Extension/ExtendableTrait.php

@@ -49,6 +49,21 @@ trait ExtendableTrait
      */
     protected static $extendableClassLoader = null;
 
+    /**
+     * @var array Cache of parent class reflections (or `false` when there is none), keyed by class name.
+     */
+    protected static $extensionParentClassCache = [];
+
+    /**
+     * @var array Cache of magic method reflections (or `false` when unavailable), keyed by "Class::method".
+     */
+    protected static $extensionMethodReflectionCache = [];
+
+    /**
+     * @var array Cache of property accessibility checks, keyed by class name and property name.
+     */
+    protected static $extensionAccessiblePropertyCache = [];
+
     /**
      * This method should be called as part of the constructor.
      */
@@ -349,9 +364,10 @@ public function propertyExists($name)
      */
     protected function extendableIsAccessible($class, $propertyName)
     {
-        $reflector = new ReflectionClass($class);
-        $property = $reflector->getProperty($propertyName);
-        return $property->isPublic();
+        $className = is_object($class) ? get_class($class) : $class;
+
+        return self::$extensionAccessiblePropertyCache[$className][$propertyName]
+            ??= (new ReflectionClass($class))->getProperty($propertyName)->isPublic();
     }
 
     /**
@@ -554,12 +570,32 @@ protected function extensionGetParentClass(?object $instance = null)
             return false;
         }
 
-        // Find if any parent uses the Extendable trait
+        // Intermediate steps of the recursive resolution are not cached.
         if (!is_null($instance)) {
-            $reflector = $instance;
-        } else {
-            $reflector = new ReflectionClass($this);
+            return $this->extensionResolveParentClass($instance);
         }
+
+        // The resolution depends only on the class, so cache it per class to avoid repeating the
+        // reflection walk on every magic method call.
+        $className = static::class;
+
+        if (!array_key_exists($className, self::$extensionParentClassCache)) {
+            self::$extensionParentClassCache[$className] = $this->extensionResolveParentClass(
+                new ReflectionClass($this)
+            );
+        }
+
+        return self::$extensionParentClassCache[$className];
+    }
+
+    /**
+     * Resolves the first parent class that does not use the Extendable trait.
+     *
+     * @return ReflectionClass|false
+     */
+    protected function extensionResolveParentClass(object $reflector)
+    {
+        // Find if any parent uses the Extendable trait
         $parent = $reflector->getParentClass();
 
         // If there's no parent, stop here.
@@ -583,33 +619,53 @@ protected function extensionGetParentClass(?object $instance = null)
         }
 
         // Otherwise, we need to loop through until we find the parent class that doesn't use the Extendable trait
-        return $this->extensionGetParentClass($parent);
+        return $this->extensionResolveParentClass($parent);
     }
 
     /**
      * Determines if the given class reflection contains the given method.
      */
     protected function extensionMethodExists(ReflectionClass $class, string $methodName): bool
     {
-        try {
-            $method = $class->getMethod($methodName);
+        return $this->extensionGetMethodReflection($class, $methodName) !== false;
+    }
 
-            if (!$method->isPublic()) {
-                return false;
+    /**
+     * Gets the reflection for a public method on the given class reflection, or `false` if the method is
+     * unavailable. The result is cached per class, as it cannot change at runtime.
+     *
+     * @return ReflectionMethod|false
+     */
+    protected function extensionGetMethodReflection(ReflectionClass $class, string $methodName)
+    {
+        $cacheKey = $class->getName() . '::' . $methodName;
+
+        if (!array_key_exists($cacheKey, self::$extensionMethodReflectionCache)) {
+            try {
+                $method = $class->getMethod($methodName);
+
+                self::$extensionMethodReflectionCache[$cacheKey] = $method->isPublic() ? $method : false;
+            } catch (ReflectionException $e) {
+                self::$extensionMethodReflectionCache[$cacheKey] = false;
             }
-        } catch (ReflectionException $e) {
-            return false;
         }
 
-        return true;
+        return self::$extensionMethodReflectionCache[$cacheKey];
     }
 
     /**
      * Calls a method through reflection.
      */
     protected function extensionCallMethod(ReflectionClass $class, string $method, array $params)
     {
-        $method = $class->getMethod($method);
-        return $method->invokeArgs($this, $params);
+        $reflection = $this->extensionGetMethodReflection($class, $method);
+
+        // Fall back to an uncached reflection for non-public methods, retaining the previous behaviour
+        // of this method for direct calls that are not guarded by extensionMethodExists().
+        if ($reflection === false) {
+            $reflection = $class->getMethod($method);
+        }
+
+        return $reflection->invokeArgs($this, $params);
     }
 }

tests/Extension/ExtendableTraitTest.php

@@ -42,6 +42,58 @@ public function testGetExtensionParentClassOnExtendableTrait()
         $this->assertEquals(Level1NonExtendable::class, $this->callProtectedMethod($level2NonExtendable, 'extensionGetParentClass')->getName());
         $this->assertEquals(Level1NonExtendable::class, $this->callProtectedMethod($level3NonExtendable, 'extensionGetParentClass')->getName());
     }
+
+    /**
+     * @testdox will reuse the parent class reflection across calls and instances of the same class
+     *
+     * The parent class resolution depends only on the class, so the reflection should be resolved once per class
+     * instead of being rebuilt on every magic method call.
+     */
+    public function testParentClassReflectionIsMemoized()
+    {
+        $level2 = new Level2NonExtendable;
+
+        $first = $this->callProtectedMethod($level2, 'extensionGetParentClass');
+        $second = $this->callProtectedMethod($level2, 'extensionGetParentClass');
+
+        $this->assertInstanceOf(\ReflectionClass::class, $first);
+        $this->assertSame($first, $second);
+
+        // Instances of the same class share the resolved reflection
+        $third = $this->callProtectedMethod(new Level2NonExtendable, 'extensionGetParentClass');
+        $this->assertSame($first, $third);
+
+        // Subclasses are memoized under their own class and still resolve correctly
+        $level3First = $this->callProtectedMethod(new Level3NonExtendable, 'extensionGetParentClass');
+        $level3Second = $this->callProtectedMethod(new Level3NonExtendable, 'extensionGetParentClass');
+
+        $this->assertEquals(Level1NonExtendable::class, $level3First->getName());
+        $this->assertSame($level3First, $level3Second);
+    }
+
+    /**
+     * @testdox routes magic methods consistently across repeated calls and instances
+     */
+    public function testRepeatedMagicAccessBehavesConsistently()
+    {
+        $a = new Level2NonExtendable;
+        $b = new Level2NonExtendable;
+
+        // No parent __get exists, so undefined properties resolve to null - repeatedly
+        $this->assertNull($a->undefinedProperty);
+        $this->assertNull($a->undefinedProperty);
+        $this->assertNull($b->undefinedProperty);
+
+        // Undefined methods throw consistently on every call
+        foreach ([$a, $b, $a] as $instance) {
+            try {
+                $instance->undefinedMethod();
+                $this->fail('Expected BadMethodCallException was not thrown');
+            } catch (\BadMethodCallException $e) {
+                $this->assertStringContainsString('undefinedMethod', $e->getMessage());
+            }
+        }
+    }
 }
 
 class ExtendableRoot extends Extendable