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

Optimize the non-cached clipboard to only fetch relevant abilities #276

Merged
merged 1 commit into from
Apr 9, 2018

Conversation

JosephSilber
Copy link
Owner

@JosephSilber JosephSilber commented Apr 9, 2018

Current situation

Currently, the clipboard will always fetch all abilities when checking anything at the gate. The abilities are cached (by default for the current request, but can also be cached forever by calling the cache() method on Bouncer).

This is great if you only use a handful of abilities, since pulling them all down once and then running all checks against that set of abilities is very efficient.

However, if you use abilities for specific models, like:

Bouncer::allow($user)->to('view', $listing);

...you can quickly end up with users who have hundreds of abilities. Pulling down all abilities to run a single check becomes a performance penalty instead of an improvement.

Improvement in this PR

With this PR, the non-caching clipboard now only pulls down the single relevant ability for the given check. So if you configure Bouncer not to cache your queries:

Bouncer::dontCache();

...it now will no longer pull all abilities from the database at once.


After merging this and having people try it out, I plan to create an additional caching clipboard based on this one, that only caches the results of the checks at the gate.


References #263

Repository owner deleted a comment from Nitpick-CI Apr 9, 2018
Repository owner deleted a comment from Nitpick-CI Apr 9, 2018
Repository owner deleted a comment from Nitpick-CI Apr 9, 2018
Repository owner deleted a comment from Nitpick-CI Apr 9, 2018
Repository owner deleted a comment from Nitpick-CI Apr 9, 2018
Repository owner deleted a comment from Nitpick-CI Apr 9, 2018
Repository owner deleted a comment from Nitpick-CI Apr 9, 2018
Repository owner deleted a comment from Nitpick-CI Apr 9, 2018
Repository owner deleted a comment from Nitpick-CI Apr 9, 2018
Repository owner deleted a comment from Nitpick-CI Apr 9, 2018
Repository owner deleted a comment from Nitpick-CI Apr 9, 2018
Repository owner deleted a comment from Nitpick-CI Apr 9, 2018
Repository owner deleted a comment from Nitpick-CI Apr 9, 2018
Repository owner deleted a comment from Nitpick-CI Apr 9, 2018
@JosephSilber JosephSilber merged commit 0c7b4fa into master Apr 9, 2018
@JosephSilber JosephSilber deleted the optimize-clipboard branch April 9, 2018 12:27
@JosephSilber
Copy link
Owner Author

Since this is a pretty big change that required refactoring almost all tests, I would really appreciate if any of you could try this out in your apps to see if it actually works as expected.

How to try it

  1. Update the Bouncer version in composer.json to master:

    "silber/bouncer": "dev-master"
    
  2. Run composer update.

  3. Configure Bouncer not to cache anything, by putting the following in your AppServiceProvider's boot method:

    \Bouncer::dontCache();
  4. Run your tests and/or click around in your app to make sure everything works as expected.

Please report back with your findings ❤️

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

Successfully merging this pull request may close these issues.

None yet

1 participant