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

Set autowiring types on services with many alternatives like encoder #257

Merged

Conversation

chalasr
Copy link
Collaborator

@chalasr chalasr commented Oct 20, 2016

See #256

@weaverryan
Copy link

👍 this makes sense (and thanks, that was crazy fast)! The other 2 you added, for JWSProviderInterface and TokenExtractorInterface also seem to make sense, but it just depends on whether or not you want to encourage people to inject these directly into their services (I noticed that they're public=false). That's a subjective call :).

Cheers!

@chalasr chalasr merged commit fd06833 into master Oct 20, 2016
@chalasr chalasr deleted the fix/set_autowire_type_for_tokenauthenticator_encoder branch October 20, 2016 18:15
@chalasr
Copy link
Collaborator Author

chalasr commented Oct 20, 2016

it just depends on whether or not you want to encourage people to inject these directly into their service

For the TokenExtractorInterface, it needs to be injected into the JWTTokenAuthenticator which is intended to be extended by users.

About the JWSProviderInterface, we realized that some needs (more or less specific) may involve to extend our DefaultEncoder with, sometimes, no need for re-writing the logic of the DefaultJWSProvider which is injected into, that's why I thought adding the autowired type onto may be useful.

Thanks for your review!

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

Successfully merging this pull request may close these issues.

2 participants