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

Improve AuthFactoryProvider #1163

Merged
merged 1 commit into from
Jul 13, 2015
Merged

Improve AuthFactoryProvider #1163

merged 1 commit into from
Jul 13, 2015

Conversation

arteam
Copy link
Member

@arteam arteam commented Jul 12, 2015

  • Make sure users pass the type inherited from Principal.
  • Remove unchecked cast warnings.
  • Remove unnecessary static class AuthValueFactory, which could be replaced by an anonymous class.
  • Make PrincipalClassProvider a holder class, rather then an interface. Doing so, we don't create a new anonymous class in Binder.
  • Documentation and style improvements.

@arteam
Copy link
Member Author

arteam commented Jul 12, 2015

These are cosmetic changes, but I hope they are useful and make the codebase cleaner.


/**
* Value factory provider supporting the {@link Principal} injection
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you need "the"

@carlo-rtr
Copy link
Member

Besides the one nitpick, this looks great. Thanks

* Make sure users pass the type inherited from `Principal`
* Remove unchecked cast warnings
* Remove unnecessary static class `AuthValueFactory`, which could
be replaced by an anonymous class.
* Make `PrincipalClassProvider` a holder class, rather then an interface.
Doing so, we don't create a new anonymous class in `Binder`.
* Documentation and style improvements.
@arteam
Copy link
Member Author

arteam commented Jul 13, 2015

Ok, thanks for feedback. Pushed a new version, let me know if it's good enough.

@jplock jplock added this to the 0.9.0 milestone Jul 13, 2015
@jplock
Copy link
Member

jplock commented Jul 13, 2015

LGTM!, waiting for Travis to finish

jplock added a commit that referenced this pull request Jul 13, 2015
@jplock jplock merged commit ef71612 into dropwizard:master Jul 13, 2015
@arteam arteam deleted the improve_auth_factory_provider branch January 24, 2016 08:05
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.

3 participants