Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

isAbleTo function is very slow #645

Open
joh3rd opened this issue Jul 6, 2023 · 6 comments
Open

isAbleTo function is very slow #645

joh3rd opened this issue Jul 6, 2023 · 6 comments

Comments

@joh3rd
Copy link

joh3rd commented Jul 6, 2023

  • 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).

@websitevirtuoso
Copy link

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

@GregPeden
Copy link
Contributor

GregPeden commented Oct 20, 2023

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.

@WillGoldstein
Copy link

WillGoldstein commented Jul 24, 2024

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
Copy link

@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.

@GregPeden
Copy link
Contributor

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.

@joh3rd
Copy link
Author

joh3rd commented Jul 25, 2024

@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 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants