Merge pull request #27 from bc-luke/big-31502-super-reflection-cache-p

PHPMNT-100 Ensure CachingClassInspector works with no-op cache
bigcommerce · Jul 1, 2024 · d4df050 · d4df050
2 parents 635b1e6 + 50cbc7e
commit d4df050
Show file tree

Hide file tree

Showing 2 changed files with 87 additions and 19 deletions.
diff --git a/src/Reflection/CachingClassInspector.php b/src/Reflection/CachingClassInspector.php
@@ -5,7 +5,6 @@
 namespace Bigcommerce\Injector\Reflection;
 
 use Bigcommerce\Injector\Cache\ServiceCacheInterface;
-use ReflectionClass;
 use ReflectionException;
 
 class CachingClassInspector implements ClassInspectorInterface
@@ -44,14 +43,14 @@ public function warmCache(string $class, string $method): void
     public function classHasMethod(string $class, string $method): bool
     {
         $key = "$class::{$method}::exists";
-        if (!$this->serviceCache->has($key)) {
-            $this->serviceCache->set(
-                $key,
-                $this->classInspector->classHasMethod($class, $method),
-            );
+        if ($this->serviceCache->has($key)) {
+            $value = $this->serviceCache->get($key);
+        } else {
+            $value = $this->classInspector->classHasMethod($class, $method);
+            $this->serviceCache->set($key, $value);
         }
 
-        return $this->serviceCache->get($key);
+        return $value;
     }
 
     /**
@@ -65,14 +64,14 @@ public function classHasMethod(string $class, string $method): bool
     public function methodIsPublic(string $class, string $method): bool
     {
         $key = "$class::{$method}::is_public";
-        if (!$this->serviceCache->has($key)) {
-            $this->serviceCache->set(
-                $key,
-                $this->classInspector->methodIsPublic($class, $method),
-            );
+        if ($this->serviceCache->has($key)) {
+            $value = $this->serviceCache->get($key);
+        } else {
+            $value = $this->classInspector->methodIsPublic($class, $method);
+            $this->serviceCache->set($key, $value);
         }
 
-        return $this->serviceCache->get($key);
+        return $value;
     }
 
     /**
@@ -86,14 +85,14 @@ public function methodIsPublic(string $class, string $method): bool
     public function getMethodSignature(string $class, string $method): array
     {
         $key = "$class::{$method}::signature";
-        if (!$this->serviceCache->has($key)) {
-            $this->serviceCache->set(
-                $key,
-                $this->classInspector->getMethodSignature($class, $method),
-            );
+        if ($this->serviceCache->has($key)) {
+            $value = $this->serviceCache->get($key);
+        } else {
+            $value = $this->classInspector->getMethodSignature($class, $method);
+            $this->serviceCache->set($key, $value);
         }
 
-        return $this->serviceCache->get($key);
+        return $value;
     }
 
     public function getStats(): ClassInspectorStats

diff --git a/tests/Reflection/NoOpCachingClassInspectorTest.php b/tests/Reflection/NoOpCachingClassInspectorTest.php
@@ -0,0 +1,69 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Reflection;
+
+use Bigcommerce\Injector\Cache\ArrayServiceCache;
+use Bigcommerce\Injector\Cache\ServiceCacheInterface;
+use Bigcommerce\Injector\Reflection\CachingClassInspector;
+use Bigcommerce\Injector\Reflection\ClassInspector;
+use PHPUnit\Framework\TestCase;
+use Prophecy\Argument;
+use Prophecy\PhpUnit\ProphecyTrait;
+use Prophecy\Prophecy\ObjectProphecy;
+use Tests\Dummy\DummyDependency;
+
+class NoOpCachingClassInspectorTest extends TestCase
+{
+    use ProphecyTrait;
+
+    private CachingClassInspector $subject;
+    private ArrayServiceCache|ObjectProphecy $serviceCache;
+    private ClassInspector|ObjectProphecy $classInspector;
+
+    protected function setUp(): void
+    {
+        parent::setUp();
+        $serviceCache = $this->prophesize(ServiceCacheInterface::class);
+        $serviceCache->has(Argument::cetera())->willReturn(false);
+        $serviceCache->get(Argument::cetera())->willReturn(false);
+
+        $this->serviceCache = $serviceCache;
+        $this->classInspector = $this->prophesize(ClassInspector::class);
+        $this->subject = new CachingClassInspector(
+            $this->classInspector->reveal(),
+            $this->serviceCache->reveal(),
+        );
+    }
+
+    public function testClassHasMethodWorksWithNoOpCache(): void
+    {
+        $this->classInspector->classHasMethod(Argument::cetera())->willReturn(true);
+        $this->serviceCache->set(Argument::cetera())->shouldBeCalled();
+
+        $hasMethod = $this->subject->classHasMethod(DummyDependency::class, 'isEnabled');
+
+        $this->assertTrue($hasMethod);
+    }
+
+    public function testMethodIsPublicWorksWithNoOpCache(): void
+    {
+        $this->classInspector->methodIsPublic(Argument::cetera())->willReturn(true);
+        $this->serviceCache->set(Argument::cetera())->shouldBeCalled();
+
+        $public = $this->subject->methodIsPublic(DummyDependency::class, 'isEnabled');
+
+        $this->assertTrue($public);
+    }
+
+    public function testGetMethodSignatureWorksWithNoOpCache(): void
+    {
+        $this->classInspector->getMethodSignature(Argument::cetera())->willReturn([]);
+        $this->serviceCache->set(Argument::cetera())->shouldBeCalled();
+
+        $signature = $this->subject->getMethodSignature(DummyDependency::class, 'isEnabled');
+
+        $this->assertEquals([], $signature);
+    }
+}