The cache system is not working properly

[mcolominas]

Describe the bug
I know this package caches queries to improve performance, but by adding laravel's laravel debugbar, I saw queries being made to the roles and permissions tables.

How the permissions are used, for example, create a "Super Admin" role and assign the necessary permissions to the Role, in this case they are all the permissions.

Being something strange, I investigated a bit and I found the problem, I don't know if it's something expected or not, but in my opinion I wouldn't have to make queries to the database, since supposedly, they are caching, the problem originates from:

In the file vendor/spatie/laravel-permission/src/Traits/HasPermissions.php on line 305:

public function hasDirectPermission($permission): bool
{
    $permission = $this->filterPermission($permission);
    
    return $this->permissions->contains($permission->getKeyName(), $permission->getKey()); // <-- Line 305
}

What happens in this case is that the permissions relationship does not exist and when using $this->permissions it goes to the database to load the relationship, instead of getting the cached data.

On the other side where the role query is performed is in the file vendor/spatie/laravel-permission/src/Traits/HasRoles.php on the line 188:

public function hasRole($roles, string $guard = null): bool
{
    $this->loadMissing('roles');  // <-- Line 188
...

Exactly the same thing happens as in the previous case, the roles relationship is not loaded and goes to the database to obtain the data.

Versions
You can use composer show to get the version numbers of:

• spatie/laravel-permission package version: 5.10.2
• illuminate/framework package: v10.18.0

To Reproduce
I have the class Admin

class Admin extends Authenticatable implements MustVerifyEmail
{
    use HasRoles, HasPermissions;
...

I have a route that needs a permission using the middleware can:permission_name and having the laravel debugbar active and accessing the route, you can see how the queries are made to the database.

[erikn69]

That is an expected behavior, cache is for permissions/roles relation, user/permissions and user/roles queries run on every call, the same happens for the logged user query
It is not a bug

[erikn69]

#1833
Here is an idea of what you propose, you could do it in your implementation

[mcolominas]

Ok thanks,I have been reviewing and it could be optimize the sql queries for users/permissions and users/roles?

I explain, this is the SQL used to get the roles of a user, it is long and has joins:

select `roles`.*, `model_has_roles`.`model_id` as `pivot_model_id`, `model_has_roles`.`role_id` as `pivot_role_id`, `model_has_roles`.`model_type` as `pivot_model_type` from `roles` inner join `model_has_roles` on `roles`.`id` = `model_has_roles`.`role_id` where `model_has_roles`.`model_id` in (1) and `model_has_roles`.`model_type` = 'Models\Admin'

Reviewing the cache, all the information of the roles, permissions and their relationship between them is stored.

Wouldn't it be more optimal to perform the following query?

SELECT role_id FROM model_has_roles WHERE model_id in (1) AND model_type = 'Models\Admin';

And since you already have all the roles cached, you can manually assign the cache roles relation to user model.

EDIT: With the user/permissions, the exact same thing happens as in the previous code.

[erikn69]

In short, that is how Laravel handles polymorphic relationships, and overriding the default behavior could be hard to maintain.

In your own implementation you can do it the way you see fit.

PD: I also don't think it will improve much by making a shorter sql, possibly doing it as you mention will lower performance

[mcolominas]

Instead of short it's the joins, I think it's faster to do the sql without join and do 2 loops.

[baselbj]

I will try to be short but I faced the same issue and I tried to understand the issue as we don't use Direct Permissions and we use the roles permissions but as @mcolominas says it calls the hasDirectPermission function and hits the database.

the issue is in this function

    public function hasPermissionTo($permission, $guardName = null): bool
    {
        if ($this->getWildcardClass()) {
            return $this->hasWildcardPermission($permission, $guardName);
        }

        $permission = $this->filterPermission($permission, $guardName);

        return $this->hasDirectPermission($permission) || $this->hasPermissionViaRole($permission);
    }

To give the option for using direct permissions to be over the roles permissions if the user has both then the code calls the direct permissions function and then, if no permissions found, it will call the roles permissions function. This make an unnecessary call to the database.

To solve this i will try making my own class implementation but I think it will be good if you can offer a settings that can avoid this and choose only roles permissions.

[drbyte]

While not a "complete" solution, if you're not using Direct Permissions, is it enough to simply reverse the order of the checks?

ie:

-        return $this->hasDirectPermission($permission) || $this->hasPermissionViaRole($permission);
+        return $this->hasPermissionViaRole($permission) || $this->hasDirectPermission($permission);

Or, since this public method can be overridden in your model, you could just drop the call to hasDirectPermission since you're not using that feature.