fix Call to undefined method exception (#5)

* fix Call to undefined method exception when calling policy methods that don't exist
InnoGE · Mar 20, 2023 · d19db37 · d19db37
1 parent 9584723
commit d19db37
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 21 deletions.
diff --git a/src/LaravelPolicySoftCache.php b/src/LaravelPolicySoftCache.php
@@ -22,12 +22,6 @@ public static function flushCache(): void
         app()->make(static::class)->cache = [];
     }
 
-    /**
-     * @param  mixed  $user
-     * @param  string  $ability
-     * @param  mixed  $args
-     * @return mixed
-     */
     public function handleGateCall(mixed $user, string $ability, mixed $args): mixed
     {
         if (! is_array($args)) {
@@ -42,28 +36,41 @@ public function handleGateCall(mixed $user, string $ability, mixed $args): mixed
 
         $policy = Gate::getPolicyFor($model);
 
-        if ($model && $this->shouldCache($policy)) {
+        if ($model && $this->shouldCache($policy, $ability)) {
             return $this->callPolicyMethod($user, $policy, $ability, $args);
         }
 
         return null;
     }
 
-    /**
-     * @param  object|null  $policy
-     * @return bool
-     */
-    protected function shouldCache(?object $policy): bool
+    protected function shouldCache(?object $policy, string $ability): bool
     {
-        return $policy && ($policy instanceof SoftCacheable || config('policy-soft-cache.cache_all_policies', false) === true);
+        // when policy is not filled don't cache it
+        if (blank($policy)) {
+            return false;
+        }
+
+        // when policy is not an object don't cache it
+        if (! is_object($policy)) {
+            return false;
+        }
+
+        // when policy doesn't have the ability don't cache it
+        if (! method_exists($policy, $ability)) {
+            return false;
+        }
+
+        // when policy is soft cacheable cache it
+        if ($policy instanceof SoftCacheable) {
+            return true;
+        }
+
+        // when config is set to cache all policies cache it
+        return config('policy-soft-cache.cache_all_policies', false) === true;
     }
 
     /**
-     * @param  Model  $user
-     * @param  object  $policy
-     * @param  string  $ability
      * @param  array<int,mixed>  $args
-     * @return mixed
      */
     protected function callPolicyMethod(Model $user, object $policy, string $ability, array $args): mixed
     {
@@ -80,11 +87,7 @@ protected function callPolicyMethod(Model $user, object $policy, string $ability
     }
 
     /**
-     * @param  Model  $user
-     * @param  object  $policy
      * @param  array<int,mixed>  $args
-     * @param  string  $ability
-     * @return string
      */
     protected function getCacheKey(Model $user, object $policy, array $args, string $ability): string
     {

diff --git a/tests/PolicySoftCacheTest.php b/tests/PolicySoftCacheTest.php
@@ -23,6 +23,24 @@
     Config::set('policy-soft-cache.cache_all_policies', true);
 });
 
+it('does not cache policy calls when ability does not exist', function () {
+    Config::set('policy-soft-cache.cache_all_policies', false);
+
+    $user = new User();
+    $testModel = new TestModel();
+    $testModel->setAttribute('id', 1);
+
+    Gate::policy(TestModel::class, PolicyWithSoftCache::class);
+
+    $user->can('foo', $testModel);
+    $user->can('foo', $testModel);
+
+    expect(PolicyWithSoftCache::$called)->toBe(0);
+
+    PolicyWithSoftCache::$called = 0;
+    Config::set('policy-soft-cache.cache_all_policies', true);
+});
+
 it('does not cache policy calls without SoftCacheable interface', function () {
     Config::set('policy-soft-cache.cache_all_policies', false);