----------------------------------------------------------- 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 > >