laratrust icon indicating copy to clipboard operation
laratrust copied to clipboard

isAbleTo function is very slow

Open joh3rd opened this issue 1 year ago • 6 comments

  • Laravel Version: 10.0
  • Laratrust Version: 8.0

Describe the bug The isAbleTofunction is very slow. I have many users with many teams and many permissions. In one case, a user has more than 8000 rows in the permission_user table. This is because of the user is assigned to many teams with a lot of permissions in every team. I'm checking permissions like that:

$user->isAbleTo(['permission_one', 'permission_two'], $teamId, true);

The DB query itself is super fast. I logged the raw query and tried that. But the isAbleTo function is very slow (more than two seconds in this case).

joh3rd avatar Jul 06 '23 06:07 joh3rd

Please provide repo with 8000 records and we are able to help to debug and improve code

websitevirtuoso avatar Aug 29 '23 02:08 websitevirtuoso

I happened across this issue while looking for anotherr... I fixed this by re-writing the "isAbleTo" method on User model:

  public function isAbleToOptimized($permissions, $team = null, $excludeGlobalRoles = false, $withDirectPermissions = false)
  {
    $teamId = Helper::getIdFor($team, 'team');
    return $this->roles()->hasByNonDependentSubquery('permissions', function (Builder $q) use ($permissions, $excludeGlobalRoles) {
      if (is_array($permissions)) {
        $q->whereIn('name', $permissions);
      } else {
        $q->where('name', $permissions);
      }
    })->where(function ($q) use ($teamId, $excludeGlobalRoles) {
      $q->where('role_user.team_id', $teamId)
        ->when(!$excludeGlobalRoles && $teamId !== null, function (Builder $q) {
          $q->orWhere('role_user.team_id', null);
        });
    })->exists();
  }

I wrote this to make use of this package: https://github.com/mpyw/eloquent-has-by-non-dependent-subquery

But I think you can do it with built-in Laravel methods now since MySQL has been updated to optimize queries that Laravel generates much better.

This makes better use of the SQL index. I can't remember if I made changes to the indexing on the laratrust tables as well... but the execution time of this is completely fixed and scales with many records.

GregPeden avatar Oct 20 '23 22:10 GregPeden

Is @GregPeden 's fix going to be merged? Or has it been?

Edit: I see that's not feasible now since the package his fix depends on is archived and isn't compatible with Laravel 11. This issue seems significant to me. @santigarcor mind sharing your thoughts?

WillGoldstein avatar Jul 24 '24 11:07 WillGoldstein

@joh3rd question for you: why aren't you using roles instead of permissions in this case? I am testing by creating many users, assigning each user a role, and found the $user->isAbleTo(['permission_one', 'permission_two'], $teamId, true); function to be a negligible 10ms tax. I have a similar scenario where a user could be assigned to many teams, each with lots of permissions, but if you use roles, you don't need to populate the permission_user table. And since permissions can be derived from the role, you can still do the granular check of a permission via isAbleTo.

WillGoldstein avatar Jul 24 '24 11:07 WillGoldstein

Edit: I see that's not feasible now since the package his fix depends on is archived and isn't compatible with Laravel 11. This issue seems significant to me. @santigarcor mind sharing your thoughts?

I think using that solution has been deemed obsolete for anyone running MySQL 8.0.16 or later. It's fine to use Laravel's built-in "whereHas" method.

GregPeden avatar Jul 24 '24 18:07 GregPeden

@joh3rd question for you: why aren't you using roles instead of permissions in this case? I am testing by creating many users, assigning each user a role, and found the $user->isAbleTo(['permission_one', 'permission_two'], $teamId, true); function to be a negligible 10ms tax. I have a similar scenario where a user could be assigned to many teams, each with lots of permissions, but if you use roles, you don't need to populate the permission_user table. And since permissions can be derived from the role, you can still do the granular check of a permission via isAbleTo.

@WillGoldstein Yes i agree that this would make sense but in my case not possible because i created a user admin interface where admins can manage roles and permissions for their users. They have the possibility to make roles dynamically but they also have the possibility to give certain permissions to users without creating a role. And some admins still just assign permissions without having a role in between. Anyway, i'll check again with Laravel 11 and MySQL 8 as soon as i upgraded to this version :)

joh3rd avatar Jul 25 '24 20:07 joh3rd