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

[RFC] Voter as its own component? #18353

Closed
patrick-mcdougle opened this issue Mar 29, 2016 · 12 comments
Closed

[RFC] Voter as its own component? #18353

patrick-mcdougle opened this issue Mar 29, 2016 · 12 comments
Labels
RFC RFC = Request For Comments (proposals about features that you want to be discussed)

Comments

@patrick-mcdougle
Copy link
Contributor

Sometimes there are other application things that could require voters to make decisions. Decisions about shipping methods or zoning restrictions for certain products. It'd be swell if voter stuff could be abstracted out of the security component and make a new component that the security component uses.

Thoughts, comments, concerns?

At this point, I haven't really considered all the changes that would need to be made, but I could see a change like this being helpful.

@stloyd
Copy link
Contributor

stloyd commented Mar 29, 2016

👍 as I would see some additional case for such "voting" ("filtering" promotions to certain products, "filtering" requests that matches i.e. some parameters etc.), but I'm not sure it could be in fact standalone component, mostly cause it adds "yet more dependency" into Symfony & "yet more code" to maintain.

@lyrixx
Copy link
Member

lyrixx commented Mar 29, 2016

What would be the scope of this lib?

@patrick-mcdougle
Copy link
Contributor Author

@stloyd It would need to be a separate component because it doesn't make sense to leave a reusable voter classes in the security component. And since it would be maintained by symfony (it already is now) it doesn't really fall into the dependency hell because the versions stay in sync. Furthermore, it's not really much more code to maintain than it is already.

@patrick-mcdougle
Copy link
Contributor Author

@lyrixx Basically, it's the VoterInterface (removing the assumptions about access and naming the variables more generally) and a majority of the AccessDecisionManager (in the component it would just be DecisionManager).

@linaori
Copy link
Contributor

linaori commented Mar 30, 2016

I see potential, but isn't it always about permissions/authorization here? You either may or may not do X. The only thing I'm actually missing is "Why can't I do X?".

I'm a bit worried that if this is abstracted away, it would become nothing more than moving the logic for an if statement to a dedicated class. Currently I'm already using this to check states of objects and allow/disallow based on that, which is a bit of a bigger scope than simply authorization: "I may do it, but I cannot because of the state of the object".

@stof
Copy link
Member

stof commented Mar 30, 2016

I agree with @iltar here. Trying to build a generic component here would make it harder to use in the Symfony component (the current signature ships a TokenInterface, but the new component could not), and I don't think it is worth it.

@patrick-mcdougle
Copy link
Contributor Author

@iltar Using Voters can be a part of all sorts of decision making outside of permissions/authorization, that's my point. I think abstracting out voter results in a fairly small change to the security component (probably has some BC, so would be a symfony 4 feature) and would add one more dependency to the security component (the voter component). The security component already has dependencies on other symfony components so this is not unprecedented.

@javiereguiluz javiereguiluz added the RFC RFC = Request For Comments (proposals about features that you want to be discussed) label Mar 31, 2016
@TomasVotruba
Copy link
Contributor

I would really appreciate this.

From my practice, I've seen similar approach for "features flags" - decide if some feature is enabled or not. I told them "oh, that's like voters... did you use them?", and they said: "nope, built from scratch".

@linaori
Copy link
Contributor

linaori commented Apr 3, 2016

@TomasVotruba though it's similar, a feature is either on or off for a given criteria. Usually you just want to know if you have that feature regardless of context. You might be interested in seeing this though: https://github.com/yannickl88/features-bundle

@unkind
Copy link
Contributor

unkind commented Apr 12, 2016

"Voting" sounds like a very domain-specific thing. Abstraction has a number of costs, your domain-specific code would be less explicit and more vague. Different domains have different meaning of "voter", "decision maker/manager", it looks like poor abstraction.

Even if you see similar code, it doesn't mean you have to abstract/generalize it: http://verraes.net/2014/08/dry-is-about-knowledge/

@javiereguiluz
Copy link
Member

I'm 👎 on this for the same reasons explained by @iltar and @stof.

@stof
Copy link
Member

stof commented May 26, 2016

Closing this issue, as it receives several down votes from the core team (and no upvote)

@stof stof closed this as completed May 26, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFC RFC = Request For Comments (proposals about features that you want to be discussed)
Projects
None yet
Development

No branches or pull requests

8 participants