Skip to content

Commit f5a6041

Browse files
krishnamuttevikrishnamuttevi
committed
RANGER-5623: Addressed the review comments to optimise and add proper unit testcases
1 parent f69bb31 commit f5a6041

2 files changed

Lines changed: 111 additions & 51 deletions

File tree

‎security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1657,15 +1657,10 @@ public VXGroupUser updateXGroupUser(VXGroupUser vXGroupUser) {
16571657
throw restErrorUtil.createRESTException(HttpServletResponse.SC_BAD_REQUEST, "Groupuser not found", true);
16581658
}
16591659

1660-
XXGroupUser xxGroupUser = null;
16611660

1662-
if (vXGroupUser.getId() != null) {
1663-
xxGroupUser = daoManager.getXXGroupUser().getById(vXGroupUser.getId());
1664-
if (xxGroupUser == null) {
1665-
throw restErrorUtil.createRESTException(
1666-
"Group-User mapping not found with ID: " + vXGroupUser.getId(),
1667-
MessageEnums.DATA_NOT_FOUND);
1668-
}
1661+
if (vXGroupUser.getId() != null && daoManager.getXXGroupUser().getById(vXGroupUser.getId()) == null) {
1662+
throw restErrorUtil.createRESTException("Group-User mapping not found with ID: " + vXGroupUser.getId(),
1663+
MessageEnums.DATA_NOT_FOUND);
16691664
}
16701665

16711666
XXGroup xGroup = daoManager.getXXGroup().findByGroupName(vXGroupUser.getName());

‎security-admin/src/test/java/org/apache/ranger/biz/TestXUserMgr.java‎

Lines changed: 108 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -2026,32 +2026,49 @@ public void test54modifyUserActiveStatus() {
20262026
}
20272027

20282028
@Test
2029-
public void test55updateXGroupUser() {
2029+
public void test55updateXGroupUser_success() {
20302030
setup();
2031-
VXUser vxUser = vxUser();
2032-
vxUser.setUserSource(RangerCommonEnums.USER_EXTERNAL);
2031+
20332032
VXGroupUser vxGroupUser = vxGroupUser();
2034-
// Mock XXGroupUser lookup
2033+
2034+
// Mock GroupUser
20352035
XXGroupUserDao xxGroupUserDao = Mockito.mock(XXGroupUserDao.class);
20362036
XXGroupUser xxGroupUser = new XXGroupUser();
2037+
20372038
Mockito.when(daoManager.getXXGroupUser()).thenReturn(xxGroupUserDao);
20382039
Mockito.when(xxGroupUserDao.getById(vxGroupUser.getId())).thenReturn(xxGroupUser);
2039-
// Mock XXGroup lookup
2040+
2041+
// Mock Group
20402042
XXGroupDao xxGroupDao = Mockito.mock(XXGroupDao.class);
20412043
XXGroup xxGroup = new XXGroup();
20422044
xxGroup.setId(1L);
2045+
20432046
Mockito.when(daoManager.getXXGroup()).thenReturn(xxGroupDao);
20442047
Mockito.when(xxGroupDao.findByGroupName(vxGroupUser.getName())).thenReturn(xxGroup);
2045-
// Mock XXUser lookup
2048+
2049+
// Mock User
20462050
XXUserDao xxUserDao = Mockito.mock(XXUserDao.class);
20472051
XXUser xxUser = new XXUser();
2052+
20482053
Mockito.when(daoManager.getXXUser()).thenReturn(xxUserDao);
20492054
Mockito.when(xxUserDao.getById(vxGroupUser.getUserId())).thenReturn(xxUser);
2055+
2056+
// Mock update
20502057
Mockito.when(xGroupUserService.updateResource(Mockito.any())).thenReturn(vxGroupUser);
2051-
VXGroupUser dbvxUser = xUserMgr.updateXGroupUser(vxGroupUser);
2052-
Assertions.assertNotNull(dbvxUser);
2053-
Assertions.assertEquals(dbvxUser.getId(), vxGroupUser.getId());
2054-
Assertions.assertEquals(dbvxUser.getName(), vxGroupUser.getName());
2058+
2059+
VXGroupUser result = xUserMgr.updateXGroupUser(vxGroupUser);
2060+
2061+
Assertions.assertNotNull(result);
2062+
Assertions.assertEquals(vxGroupUser.getId(), result.getId());
2063+
Assertions.assertEquals(vxGroupUser.getName(), result.getName());
2064+
2065+
// addition of parent group id
2066+
Assertions.assertEquals(1L, result.getParentGroupId());
2067+
2068+
Mockito.verify(xxGroupUserDao).getById(vxGroupUser.getId());
2069+
Mockito.verify(xxGroupDao).findByGroupName(vxGroupUser.getName());
2070+
Mockito.verify(xxUserDao).getById(vxGroupUser.getUserId());
2071+
20552072
Mockito.verify(xGroupUserService).updateResource(Mockito.any());
20562073
}
20572074

@@ -2494,40 +2511,14 @@ public void test81checkAdminAccess() {
24942511
}
24952512

24962513
@Test
2497-
public void test82updateXgroupUserForGroupUpdate() {
2514+
public void test56updateXGroupUser_nullVXGroupUser() {
24982515
setup();
2499-
XXGroupUserDao xxGroupUserDao = Mockito.mock(XXGroupUserDao.class);
2500-
VXGroup vXGroup = vxGroup();
2501-
List<XXGroupUser> xXGroupUserList = new ArrayList<>();
2502-
VXGroupUser vxGroupUser = vxGroupUser();
2503-
XXGroupUser xXGroupUser = new XXGroupUser();
2504-
xXGroupUser.setId(vxGroupUser.getId());
2505-
xXGroupUser.setName(vxGroupUser.getName());
2506-
xXGroupUser.setParentGroupId(vxGroupUser.getParentGroupId());
2507-
xXGroupUser.setUserId(vxGroupUser.getUserId());
2508-
xXGroupUserList.add(xXGroupUser);
2509-
Mockito.when(daoManager.getXXGroupUser()).thenReturn(xxGroupUserDao);
2510-
Mockito.when(xxGroupUserDao.findByGroupId(vXGroup.getId())).thenReturn(xXGroupUserList);
2511-
// CHANGED: Use anyLong() so it matches regardless of internal ID state
2512-
Mockito.when(xxGroupUserDao.getById(Mockito.anyLong())).thenReturn(xXGroupUser);
2513-
// Mock XXGroup lookup
2514-
XXGroupDao xxGroupDao = Mockito.mock(XXGroupDao.class);
2515-
XXGroup xxGroup = new XXGroup();
2516-
xxGroup.setId(vXGroup.getId());
2517-
Mockito.when(daoManager.getXXGroup()).thenReturn(xxGroupDao);
2518-
// CHANGED: Use anyString() because the group name likely gets altered internally during the loop
2519-
Mockito.when(xxGroupDao.findByGroupName(Mockito.anyString())).thenReturn(xxGroup);
2520-
// Mock XXUser lookup
2521-
XXUserDao xxUserDao = Mockito.mock(XXUserDao.class);
2522-
XXUser xxUser = new XXUser();
2523-
Mockito.when(daoManager.getXXUser()).thenReturn(xxUserDao);
2524-
// CHANGED: Use anyLong() for user ID lookup
2525-
Mockito.when(xxUserDao.getById(Mockito.anyLong())).thenReturn(xxUser);
2526-
Mockito.when(xGroupUserService.populateViewBean(xXGroupUser)).thenReturn(vxGroupUser);
2527-
xUserMgr.updateXgroupUserForGroupUpdate(vXGroup);
2528-
// Verify changes
2529-
Mockito.verify(daoManager, Mockito.atLeastOnce()).getXXGroupUser();
2530-
Mockito.verify(xxGroupUserDao).findByGroupId(vXGroup.getId());
2516+
2517+
Mockito.when(restErrorUtil.createRESTException(HttpServletResponse.SC_BAD_REQUEST, "Groupuser not found", true)).thenThrow(new WebApplicationException());
2518+
2519+
Assertions.assertThrows(WebApplicationException.class, () -> {
2520+
xUserMgr.updateXGroupUser(null);
2521+
});
25312522
}
25322523

25332524
@Test
@@ -4489,6 +4480,80 @@ public void test01CreateXUser_federated() {
44894480
});
44904481
}
44914482

4483+
@Test
4484+
public void test133updateXGroupUser_groupUserMappingNotFound() {
4485+
setup();
4486+
4487+
VXGroupUser vxGroupUser = vxGroupUser();
4488+
4489+
XXGroupUserDao xxGroupUserDao = Mockito.mock(XXGroupUserDao.class);
4490+
4491+
Mockito.when(daoManager.getXXGroupUser()).thenReturn(xxGroupUserDao);
4492+
Mockito.when(xxGroupUserDao.getById(vxGroupUser.getId())).thenReturn(null);
4493+
4494+
Mockito.when(restErrorUtil.createRESTException(Mockito.contains("Group-User mapping not found"), Mockito.eq(MessageEnums.DATA_NOT_FOUND))).thenThrow(new WebApplicationException());
4495+
4496+
Assertions.assertThrows(WebApplicationException.class, () -> {
4497+
xUserMgr.updateXGroupUser(vxGroupUser);
4498+
});
4499+
}
4500+
4501+
@Test
4502+
public void test134updateXGroupUser_groupNotFound() {
4503+
setup();
4504+
4505+
VXGroupUser vxGroupUser = vxGroupUser();
4506+
4507+
XXGroupUserDao xxGroupUserDao = Mockito.mock(XXGroupUserDao.class);
4508+
XXGroupUser xxGroupUser = new XXGroupUser();
4509+
4510+
Mockito.when(daoManager.getXXGroupUser()).thenReturn(xxGroupUserDao);
4511+
Mockito.when(xxGroupUserDao.getById(vxGroupUser.getId())).thenReturn(xxGroupUser);
4512+
4513+
XXGroupDao xxGroupDao = Mockito.mock(XXGroupDao.class);
4514+
4515+
Mockito.when(daoManager.getXXGroup()).thenReturn(xxGroupDao);
4516+
Mockito.when(xxGroupDao.findByGroupName(vxGroupUser.getName())).thenReturn(null);
4517+
4518+
Mockito.when(restErrorUtil.createRESTException(Mockito.contains("Group not found"), Mockito.eq(MessageEnums.DATA_NOT_FOUND))).thenThrow(new WebApplicationException());
4519+
4520+
Assertions.assertThrows(WebApplicationException.class, () -> {
4521+
xUserMgr.updateXGroupUser(vxGroupUser);
4522+
});
4523+
}
4524+
4525+
@Test
4526+
public void test135updateXGroupUser_userNotFound() {
4527+
setup();
4528+
4529+
VXGroupUser vxGroupUser = vxGroupUser();
4530+
4531+
XXGroupUserDao xxGroupUserDao = Mockito.mock(XXGroupUserDao.class);
4532+
XXGroupUser xxGroupUser = new XXGroupUser();
4533+
4534+
Mockito.when(daoManager.getXXGroupUser()).thenReturn(xxGroupUserDao);
4535+
Mockito.when(xxGroupUserDao.getById(vxGroupUser.getId())).thenReturn(xxGroupUser);
4536+
4537+
XXGroupDao xxGroupDao = Mockito.mock(XXGroupDao.class);
4538+
XXGroup xxGroup = new XXGroup();
4539+
xxGroup.setId(1L);
4540+
4541+
Mockito.when(daoManager.getXXGroup()).thenReturn(xxGroupDao);
4542+
Mockito.when(xxGroupDao.findByGroupName(vxGroupUser.getName())).thenReturn(xxGroup);
4543+
4544+
XXUserDao xxUserDao = Mockito.mock(XXUserDao.class);
4545+
4546+
Mockito.when(daoManager.getXXUser()).thenReturn(xxUserDao);
4547+
Mockito.when(xxUserDao.getById(vxGroupUser.getUserId())).thenReturn(null);
4548+
4549+
Mockito.when(restErrorUtil.createRESTException(Mockito.contains("User not found"), Mockito.eq(MessageEnums.DATA_NOT_FOUND))).thenThrow(new WebApplicationException());
4550+
4551+
Assertions.assertThrows(WebApplicationException.class, () -> {
4552+
xUserMgr.updateXGroupUser(vxGroupUser);
4553+
});
4554+
}
4555+
4556+
44924557
private VXUser vxUser() {
44934558
Collection<String> userRoleList = new ArrayList<>();
44944559
userRoleList.add("ROLE_USER");

0 commit comments

Comments
 (0)