Laravel-permission: slow hasPermissionTo

[andrubeldie]

Hi,

I have the following context:
142 permissions attached to 27 roles, 2000 users - vast majority with one or more roles, only few with direct permissions, and a blade template that use @can for 101 times in order to generate a menu according to current user's permissions.

rendering this menu is very time consuming (~4 seconds) and I discovered that problem is with hasPermissionTo.

in order to not touch laravel-permissions files the solution was to overwrite the method in the my model and to do some cache:

public function hasPermissionTo($permission, $guardName = null): bool
    {
        if (is_string($permission)) {
            $permission = \Cache::remember('spatie.permissions.'.$permission.'.'.$guardName, 3600, function()use($permission, $guardName){
                return app(Permission::class)->findByName(
                    $permission,
                    $guardName ?? $this->getDefaultGuardName()
                );
            });
        }
        return $this->hasDirectPermission($permission) || $this->hasPermissionViaRole($permission);
    }

what I achieved with this cache is to render that menu in 0.03 seconds.

I write you in hope you'll find a way to optimize this method inside this awesome laravel module :)

[andrubeldie]

in order to test what is slow in the process, I used a console command like this:

    public function handle()
    {
        Auth::loginUsingId(SOME_VALID_USER_ID);
        for ($i=0; $i<100; $i++){
            $time_start = microtime(true);
            auth()->user()->can('some_existent_permission');
            $time_end = microtime(true);
            $t = $time_end - $time_start;
            $time[] = $t;
        }
        dd($time, array_sum($time));
    }

[AlexVanderbist]

Hi,

Thanks for all your work looking into this. Ideally these cached permissions would come from app(PermissionRegistrar::class)->getPermissions() as they're already being cached there.

This is definitely something we'll address in the next version of the package!

Thanks again!

[andrubeldie]

lovely!

I would also strongly recommend use ->filter() instead of ->where() on collections (i.e. findByName method of Permission::class) as is 10-15 times faster.

    public function handle()
    {
        $collection_size = 1000;
        $faker = Faker::create();
        for ($i=0; $i < $collection_size; $i++) {
            $a[] = ['firstname' => $faker->firstNameMale, 'lastname' => $faker->lastName];
        }
        $a[] = ['firstname' => 'John', 'lastname' => 'Doe'];
        $c = collect($a);

        $iterations = 1000;
        $tstartwhere = microtime(true);
        for($i=0; $i < $iterations; $i++){
            $c->where('firstname', 'John')->where('lastname', 'Doe');
        }
        $tendwhere = microtime(true);
        $twhere = $tendwhere - $tstartwhere;
        $tstartfilter = microtime(true);
        for($i=0; $i < $iterations; $i++){
            $c->filter(function($value, $key){
                return $value['firstname'] == 'John' && $value['lastname'] == 'Doe';
            });
        }
        $tendfilter = microtime(true);
        $tfilter = $tendfilter - $tstartfilter;

        dd($twhere, $tfilter);
    }

[AlexVanderbist]

Good catch as well, very interesting how these methods differ so much in performance... Thanks a lot!

[drbyte]

@andrubeldie is this what you envisioned, converting where to filter ?

    public static function findByName(string $name, $guardName = null): PermissionContract
    {
        $guardName = $guardName ?? config('auth.defaults.guard');

-        $permission = static::getPermissions()->where('name', $name)->where('guard_name', $guardName)->first();
+        $permission = static::getPermissions()
+            ->filter(function ($value, $key) use ($name, $guardName) {
+                if (! is_null($guardName)) {
+                    return $value['name'] === $name && $value['guard_name'] === $guardName;
+                }
+
+                return $value['name'] === $name;
+            })->first();

        if (! $permission) {
            throw PermissionDoesNotExist::create($name, $guardName);
        }

        return $permission;
    }

[ctf0]

@drbyte atm any of the hasPermission methods call the permission relation but as we already have the permission cached , cant we use the cached copy instead ?

[drbyte]

atm any of the hasPermission methods call the permission relation but as we already have the permission cached , cant we use the cached copy instead ?

This package registers all permissions with Laravel's Gate, using the cache. So, when you call can() on the user, you're leveraging the cache.

[ctf0]

@drbyte i was talking about hasAnyPermission which is used in a middleware, which is why you probably updated the package middleware to
https://github.com/spatie/laravel-permission/blob/94940d81c2436b4f211990236c8f0ab1b7de9f37/src/Middlewares/PermissionMiddleware.php#L21-L25
instead

[thibaultlavoisey]

Hello @AlexVanderbist,

Is this issue fixed in the latest version? Our use case is the same as @andrubeldie and we have an overhead of 200% :/

Thank you!

[andrubeldie]

@thibaultlavoisey seems to be still there ...
https://github.com/spatie/laravel-permission/blob/4d47ef1644ed451f1ea99d1c44443a3ad9bacfd4/src/Models/Permission.php#L87

[thibaultlavoisey]

@andrubeldie Arf! :/

@AlexVanderbist @freekmurze Is it possible to reopen this issue? Can we help you with a PR to fix this bug ASAP?

Thank you!

[AlexVanderbist]

Hey, sorry for not checking in on this issue earlier. Feel free to PR a fix and add me as a reviewer and we'll take a look 👍

[thibaultlavoisey]

@andrubeldie Do you want to make the PR as you're the first person that has reported this issue?

[andrubeldie]

@thibaultlavoisey: arf! it's all yours, go for it. :)

[drbyte]

Fixed in v2.11.0 -- Thanks to everyone for their input and collaboration!