You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ranger.apache.org by Qiang Zhang <zh...@zte.com.cn> on 2017/03/08 03:43:31 UTC
Review Request 57409: RANGER-1432:Do some code improvement in
UserMgr.java
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57409/
-----------------------------------------------------------
Review request for ranger, Alok Lal, Ankita Sinha, Don Bosco Durai, Colm O hEigeartaigh, Gautam Borad, Madhan Neethiraj, Ramesh Mani, Selvamohan Neethiraj, and Velmurugan Periasamy.
Bugs: RANGER-1432
https://issues.apache.org/jira/browse/RANGER-1432
Repository: ranger
Description
-------
In UserMgr.java, there are some duplicate logic.
1. At the beginning of method gjUserToUserProfile, we have already checked if sess is null, so we don't need to check it again in below codes.
UserSessionBase sess = ContextUtil.getCurrentUserSession();
if (sess == null) {
return;
}
if (sess != null) {
userProfile.setUserSource(sess.getAuthProvider());
}
2. In method setUserRoles, it should be 'vStringRolesList' instead of 'vString' in comment.
/**
* @param userId
* @param vStrings
*/
public void setUserRoles(Long userId, List<VXString> vStringRolesList)
3. In method deactivateUser, it should be 'gjUser' instead of 'userId' in comment.
/**
* @param userId
*/
public VXPortalUser deactivateUser(XXPortalUser gjUser)
4. In method gjUserToUserProfile, below validation appears twice.
if (sess.isUserAdmin() || sess.isKeyAdmin()
|| sess.getXXPortalUser().getId().equals(user.getId())) {
userProfile.setLoginId(user.getLoginId());
}
if (sess.isUserAdmin() || sess.isKeyAdmin()
|| sess.getXXPortalUser().getId().equals(user.getId())) {
userProfile.setId(user.getId());
}
IMO, we can put them together.
Diffs
-----
security-admin/src/main/java/org/apache/ranger/biz/UserMgr.java be16f75
Diff: https://reviews.apache.org/r/57409/diff/1/
Testing
-------
Thanks,
Qiang Zhang
Re: Review Request 57409: RANGER-1432:Do some code improvement in
UserMgr.java
Posted by Colm O hEigeartaigh <co...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57409/#review168260
-----------------------------------------------------------
Ship it!
Ship It!
- Colm O hEigeartaigh
On March 8, 2017, 3:43 a.m., Qiang Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57409/
> -----------------------------------------------------------
>
> (Updated March 8, 2017, 3:43 a.m.)
>
>
> Review request for ranger, Alok Lal, Ankita Sinha, Don Bosco Durai, Colm O hEigeartaigh, Gautam Borad, Madhan Neethiraj, Ramesh Mani, Selvamohan Neethiraj, and Velmurugan Periasamy.
>
>
> Bugs: RANGER-1432
> https://issues.apache.org/jira/browse/RANGER-1432
>
>
> Repository: ranger
>
>
> Description
> -------
>
> In UserMgr.java, there are some duplicate logic.
> 1. At the beginning of method gjUserToUserProfile, we have already checked if sess is null, so we don't need to check it again in below codes.
> UserSessionBase sess = ContextUtil.getCurrentUserSession();
> if (sess == null) {
> return;
> }
> if (sess != null) {
> userProfile.setUserSource(sess.getAuthProvider());
> }
> 2. In method setUserRoles, it should be 'vStringRolesList' instead of 'vString' in comment.
> /**
> * @param userId
> * @param vStrings
> */
> public void setUserRoles(Long userId, List<VXString> vStringRolesList)
> 3. In method deactivateUser, it should be 'gjUser' instead of 'userId' in comment.
> /**
> * @param userId
> */
> public VXPortalUser deactivateUser(XXPortalUser gjUser)
> 4. In method gjUserToUserProfile, below validation appears twice.
> if (sess.isUserAdmin() || sess.isKeyAdmin()
> || sess.getXXPortalUser().getId().equals(user.getId())) {
> userProfile.setLoginId(user.getLoginId());
> }
> if (sess.isUserAdmin() || sess.isKeyAdmin()
> || sess.getXXPortalUser().getId().equals(user.getId())) {
> userProfile.setId(user.getId());
>
> }
> IMO, we can put them together.
>
>
> Diffs
> -----
>
> security-admin/src/main/java/org/apache/ranger/biz/UserMgr.java be16f75
>
>
> Diff: https://reviews.apache.org/r/57409/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qiang Zhang
>
>