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

Duplicate cache look ups #596

Open
siamak2 opened this issue Jun 16, 2022 · 3 comments
Open

Duplicate cache look ups #596

siamak2 opened this issue Jun 16, 2022 · 3 comments

Comments

@siamak2
Copy link

siamak2 commented Jun 16, 2022

I'm using redis as cache driver and enabled cross-request caching:
Bouncer::cache();
I'm also using telescope to monitor everything. I noticed every time I use $user->can('something') Bouncer gets 4 keys from cache, two of them are duplicate. but this isn't the only duplicate. if I use $user->can('something') three times Bouncer gets 8 more keys which is exactly same as first four.
Here is the image of telescope showing duplicate cache look ups for using $user->can('something') three times:
image

I'm planning to use a lot of $user->can() and it's not a good thing to have n * 4 look ups on redis
Is there a way to also cache them for current request?

@JosephSilber
Copy link
Owner

JosephSilber commented Jun 16, 2022

Hmm. That doesn't seem right. Will investigate.

If you want to help, could you create a repository with a minimal reproduction?


I misread before.

This is the way Bouncer is currently designed.

We could probably add another layer of local cache on top the cross-request cache. Will look into it.

@siamak2
Copy link
Author

siamak2 commented Jun 16, 2022

Reproduction is easy. the user has role of superadmin and Bouncer::allow('superadmin')->everything();
I run $user->can( 'view-admin-dashboard' ) and the ability view-admin-dashboard exists in bouncer.
And of course I ran Bouncer::refresh() to clear the cache.

@siamak2
Copy link
Author

siamak2 commented Jun 16, 2022

I used xdebug to trace the duplicate and found it. it's here:

public function getAbilities(Model $authority, $allowed = true)
{
$key = $this->getCacheKey($authority, 'abilities', $allowed);
if (is_array($abilities = $this->cache->get($key))) {
return $this->deserializeAbilities($abilities);
}

Perhaps caching result of $this->cache->get($key) could reduce duplicates.
By reduce duplicates, I mean bouncer still looks up same key with different ending f and a that is decided based on $allowed parameter and getAbilities() is called twice with different $allowed parameter. but this one is OK.
get cache:tag:silber-bouncer:key is used by laravel's cache tag and it's not bouncer's fault.

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

No branches or pull requests

2 participants