Closed mcolominas closed 1 year ago
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
https://github.com/spatie/laravel-permission/pull/1833 Here is an idea of what you propose, you could do it in your implementation
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.
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
Instead of short it's the joins, I think it's faster to do the sql without join and do 2 loops.
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.
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.
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: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: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:To Reproduce I have the class Admin
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.