-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/75294/#review227106
-----------------------------------------------------------




agents-common/src/main/java/org/apache/ranger/plugin/store/RoleStore.java
Lines 46 (patched)
<https://reviews.apache.org/r/75294/#comment315379>

    It is suggested to avoid primitive data types.



agents-common/src/main/java/org/apache/ranger/plugin/util/RangerRolesProvider.java
Lines 62 (patched)
<https://reviews.apache.org/r/75294/#comment315380>

    ranger.api.roles.delta.download.enabled is used multiple places, please see 
if it can be kept in a Constants class and referenced.



security-admin/src/main/java/org/apache/ranger/biz/RoleDBStore.java
Lines 287 (patched)
<https://reviews.apache.org/r/75294/#comment315383>

    Suggested to use CollectionUtils.isNotEmpty()



security-admin/src/main/java/org/apache/ranger/biz/RoleDBStore.java
Lines 330 (patched)
<https://reviews.apache.org/r/75294/#comment315384>

    nit: 
    xxRoles = (xxRoles != null) ? xxRoles : new ArrayList<>();



security-admin/src/main/java/org/apache/ranger/biz/RoleDBStore.java
Lines 334 (patched)
<https://reviews.apache.org/r/75294/#comment315386>

    Lambas with stream can be used here as well:
    
    List<Role> ret = xxRoles.stream()
            .map(xxRole -> roleService.read(xxRole.getId()))
            .collect(Collectors.toList());



security-admin/src/main/java/org/apache/ranger/biz/RoleDBStore.java
Lines 340 (patched)
<https://reviews.apache.org/r/75294/#comment315385>

    Lambdas with stream can be used here as well:
    
    Set<RangerRole> deletedRoles = cachedRangerRoles.stream()
            .filter(rangerRole -> !currentRoleIds.contains(rangerRole.getId()))
            .collect(Collectors.toSet());



security-admin/src/main/java/org/apache/ranger/db/XXRoleDao.java
Lines 148 (patched)
<https://reviews.apache.org/r/75294/#comment315381>

    Suggested to use criteriaBuilder as the variable name.



security-admin/src/main/java/org/apache/ranger/db/XXRoleDao.java
Lines 154 (patched)
<https://reviews.apache.org/r/75294/#comment315382>

    What happens if roleIdToVersionMap.get(id) returns null ?


- Abhishek  Kumar


On Dec. 2, 2024, 7:41 a.m., Guru Thejus Arveti wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/75294/
> -----------------------------------------------------------
> 
> (Updated Dec. 2, 2024, 7:41 a.m.)
> 
> 
> Review request for ranger, Pradeep Agrawal and Ramesh Mani.
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> RANGER-5013: Added Support for Ranger Roles Delta Sync
> 
> 
> Diffs
> -----
> 
>   agents-common/src/main/java/org/apache/ranger/plugin/store/RoleStore.java 
> 22e1e6e65 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/util/RangerRolesProvider.java
>  0caf65826 
>   security-admin/src/main/java/org/apache/ranger/biz/RangerPolicyAdmin.java 
> 15a1e7118 
>   
> security-admin/src/main/java/org/apache/ranger/biz/RangerPolicyAdminCache.java
>  5bd3a0934 
>   
> security-admin/src/main/java/org/apache/ranger/biz/RangerPolicyAdminImpl.java 
> 2434db171 
>   security-admin/src/main/java/org/apache/ranger/biz/RoleDBStore.java 
> 930147b06 
>   security-admin/src/main/java/org/apache/ranger/common/RangerRoleCache.java 
> 933104a16 
>   security-admin/src/main/java/org/apache/ranger/db/XXRoleDao.java 4e5b692b5 
>   security-admin/src/main/java/org/apache/ranger/rest/RoleREST.java d8e30b516 
>   
> security-admin/src/main/java/org/apache/ranger/service/RangerRoleServiceBase.java
>  22867cbe5 
>   security-admin/src/main/resources/conf.dist/ranger-admin-default-site.xml 
> 1af339010 
> 
> 
> Diff: https://reviews.apache.org/r/75294/diff/1/
> 
> 
> Testing
> -------
> 
> Testing manually in local but adding/removing/updating ranger roles from 
> ranger admin UI and checking whether the roles download api is returning only 
> the delta and subsequently made changes in the client code
> 
> 
> Thanks,
> 
> Guru Thejus Arveti
> 
>

Reply via email to