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

[5.4] Apply policies on subtypes of the configured class #15757

Merged
merged 2 commits into from
Oct 5, 2016

Conversation

guiwoda
Copy link
Contributor

@guiwoda guiwoda commented Oct 4, 2016

This gives the Gate the ability to find the first Policy for a given class / object, where this object is either the configured class, extends from it or implements the configured interface.

Example usage

interface Foo {}
class Bar implements Foo {}
class Baz extends Bar {}

// Previous behavior
Gate::policy(SomePolicy::class, Bar::class);
Gate::check('update', new Bar); // This works, as the policy is bound to Bar::class
Gate::check('update', new Baz); // But not this one!

// Now I can...
Gate::check('update', Baz::class); // as it IS a Bar.

Gate::policy(SomePolicy::class, Foo::class);
Gate::check('update', Bar::class); // It IS a Foo
Gate::check('update', Baz::class); // It IS also a Foo

Backwards compatibility breaks:

  • The public method getPolicyFor($class) used to throw InvalidArgumentException when no policy was found. Now it returns either a Policy or null. This was not being used anywhere else inside the framework, and the Gate would never reach the exception itself, as the argument was always validated beforehand.
  • The protected method firstArgumentCorrespondsToPolicy($arguments) is gone, as it duplicated most of the work done in getPolicyFor($class). Now the Gate will attempt to get the Policy, and if null is returned, will continue trying as before.
  • The protected method resolvePolicyCallback(...) now receives the Policy instead of fetching it.

Pointing at 5.4 because of all this BC breaks. Let me know if you want me to try other approaches to this, or if it's something you don't plan to support.

Cheers!

@GrahamCampbell GrahamCampbell changed the title [5.4][BC] Apply policies on subtypes of the configured class [5.4] Apply policies on subtypes of the configured class Oct 4, 2016
@driesvints
Copy link
Member

Yes yes yes! 👍

Some more context: this is neat when you're working with, let's say Doctrine entities which have Proxies generated for them which are used in the app. Atm. Gate won't find the policy for the proxy class because it isn't an actual instance of the entity but rather an extended class.

Really hope this gets merged 😄

@taylorotwell taylorotwell merged commit 6b09cc9 into laravel:master Oct 5, 2016
@taylorotwell
Copy link
Member

Please send a PR to the master branch of documentation with notes for the 5.4 upgrade guide.

@driesvints
Copy link
Member

Thanks for merging this! :)

@guiwoda guiwoda deleted the apply_policies_on_subtypes branch October 5, 2016 14:57
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.

4 participants