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
Role Strategy Fix + Cleanup + PermissionAssert #131
Role Strategy Fix + Cleanup + PermissionAssert #131
Conversation
oleg-nenashev
commented
Feb 23, 2018
•
edited
edited
- Move PermissionFinder to an utility package and restrict it. I was about using this API in Role Strategy, so I have addressed the TODO comment in the code
- Fix Role Strategy integration - test is not fully correct #99
- Introduce API for checking permissions
- Add tests for Role Strategy integration - test is not fully correct #99
Tests are in progress |
@lanwen may want to criticize my old-style asserts instead of matchers, adding to Cc just in case |
HashSet<String> resolvedIds = new HashSet<>(); | ||
for (String id : permissions) { | ||
String resolvedId = PermissionFinder.findPermissionId(id); | ||
if (resolvedId != null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Objects.requireNonNull?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!