When caching queries, always check roles in the cache

Closes #418

src/BaseClipboard.php

@@ -34,36 +34,73 @@ public function check(Model $authority, $ability, $model = null)
      */
     public function checkRole(Model $authority, $roles, $boolean = 'or')
     {
-        $available = $this->getRoles($authority)
-                          ->intersect(Models::role()->getRoleNames($roles));
+        $count = $this->countMatchingRoles($authority, $roles);
 
         if ($boolean == 'or') {
-            return $available->count() > 0;
+            return $count > 0;
         } elseif ($boolean === 'not') {
-            return $available->count() === 0;
+            return $count === 0;
         }
 
-        return $available->count() == count((array) $roles);
+        return $count == count((array) $roles);
     }
 
     /**
-     * Get the given authority's roles.
+     * Count the authority's roles matching the given roles.
      *
      * @param  \Illuminate\Database\Eloquent\Model  $authority
-     * @return \Illuminate\Support\Collection
+     * @param  array|string  $roles
+     * @return int
      */
-    public function getRoles(Model $authority)
+    protected function countMatchingRoles($authority, $roles)
     {
-        $collection = $authority->roles()->get(['name'])->pluck('name');
+        $lookups = $this->getRolesLookup($authority);
+
+        return count(array_filter($roles, function ($role) use ($lookups) {
+            switch (true) {
+                case is_string($role):
+                    return $lookups['names']->has($role);
+                case is_numeric($role):
+                    return $lookups['ids']->has($role);
+                case $role instanceof Model:
+                    return $lookups['ids']->has($role->getKey());
+            }
+
+            throw new InvalidArgumentException('Invalid model identifier');
+        }));
+    }
+
+    /**
+     * Get the given authority's roles' IDs and names.
+     *
+     * @param  \Illuminate\Database\Eloquent\Model  $authority
+     * @return array
+     */
+    public function getRolesLookup(Model $authority)
+    {
+        $roles = $authority->roles()->pluck(
+            'name', Models::role()->getQualifiedKeyName()
+        );
 
         // In Laravel 5.1, "pluck" returns an Eloquent collection,
         // so we call "toBase" on it. In 5.2, "pluck" returns a
         // base instance, so there is no "toBase" available.
-        if (method_exists($collection, 'toBase')) {
-            $collection = $collection->toBase();
+        if (method_exists($roles, 'toBase')) {
+            $roles = $roles->toBase();
         }
 
-        return $collection;
+        return ['ids' => $roles, 'names' => $roles->flip()];
+    }
+
+    /**
+     * Get the given authority's roles' names.
+     *
+     * @param  \Illuminate\Database\Eloquent\Model  $authority
+     * @return \Illuminate\Support\Collection
+     */
+    public function getRoles(Model $authority)
+    {
+        return $this->getRolesLookup($authority)['names']->keys();
     }
 
     /**

src/CachedClipboard.php

@@ -221,17 +221,17 @@ public function getFreshAbilities(Model $authority, $allowed)
     }
 
     /**
-     * Get the given authority's roles.
+     * Get the given authority's roles' IDs and names.
      *
      * @param  \Illuminate\Database\Eloquent\Model  $authority
-     * @return \Illuminate\Support\Collection
+     * @return array
      */
-    public function getRoles(Model $authority)
+    public function getRolesLookup(Model $authority)
     {
         $key = $this->getCacheKey($authority, 'roles');
 
         return $this->sear($key, function () use ($authority) {
-            return parent::getRoles($authority);
+            return parent::getRolesLookup($authority);
         });
     }
 

src/Database/Concerns/HasRoles.php

@@ -44,7 +44,7 @@ public function roles()
     /**
      * Get all of the model's assigned roles.
      *
-     * @return \Illuminate\Database\Eloquent\Collection
+     * @return \Illuminate\Support\Collection
      */
     public function getRoles()
     {

tests/CachedClipboardTest.php

@@ -60,6 +60,29 @@ function it_caches_roles()
         $this->assertFalse($bouncer->is($user)->a('moderator'));
     }
 
+    /**
+     * @test
+     */
+    function it_always_checks_roles_in_the_cache()
+    {
+        $bouncer = $this->bouncer($user = User::create());
+        $admin = $bouncer->role()->create(['name' => 'admin']);
+
+        $bouncer->assign($admin)->to($user);
+
+        $this->assertTrue($bouncer->is($user)->an('admin'));
+
+        $this->db()->connection()->enableQueryLog();
+
+        $this->assertTrue($bouncer->is($user)->an($admin));
+        $this->assertTrue($bouncer->is($user)->an('admin'));
+        $this->assertTrue($bouncer->is($user)->an($admin->id));
+
+        $this->assertEmpty($this->db()->connection()->getQueryLog());
+
+        $this->db()->connection()->disableQueryLog();
+    }
+
     /**
      * @test
      */