This is an automated email from the ASF dual-hosted git repository.
xxyu pushed a commit to branch kylin5
in repository https://gitbox.apache.org/repos/asf/kylin.git
commit beabe662fd745466f0e8b0ece9f2e96b0eaa9fea
Author: Jiale He <35652389+jial...@users.noreply.github.com>
AuthorDate: Wed Jan 4 21:33:17 2023 +0800
KYLIN-5457 fix user group delete
---
.../kylin/rest/service/NUserGroupService.java | 33 ++++++++++------------
.../kylin/rest/service/NUserGroupServiceTest.java | 21 ++++++++++++++
.../metadata/usergroup/NUserGroupManager.java | 2 +-
.../metadata/usergroup/NUserGroupManagerTest.java | 2 +-
4 files changed, 38 insertions(+), 20 deletions(-)
diff --git
a/src/common-service/src/main/java/org/apache/kylin/rest/service/NUserGroupService.java
b/src/common-service/src/main/java/org/apache/kylin/rest/service/NUserGroupService.java
index c8a50fdbb4..7983384e18 100644
---
a/src/common-service/src/main/java/org/apache/kylin/rest/service/NUserGroupService.java
+++
b/src/common-service/src/main/java/org/apache/kylin/rest/service/NUserGroupService.java
@@ -106,16 +106,15 @@ public class NUserGroupService implements
IUserGroupService {
aclEvaluate.checkIsGlobalAdmin();
checkGroupCanBeDeleted(name);
// remove retained user group in all users
- List<ManagedUser> managedUsers = userService.listUsers();
- for (ManagedUser managedUser : managedUsers) {
- if (managedUser.getAuthorities().contains(new
SimpleGrantedAuthority(name))) {
- managedUser.removeAuthorities(name);
- userService.updateUser(managedUser);
- }
- }
+ SimpleGrantedAuthority simpleAuthority = new
SimpleGrantedAuthority(name);
+ userService.listUsers(false).stream().filter(
+ user ->
user.getAuthorities().parallelStream().anyMatch(authority ->
authority.equals(simpleAuthority)))
+ .forEach(user -> {
+ user.removeAuthorities(name);
+ userService.updateUser(user);
+ });
//delete group's project ACL
accessService.revokeProjectPermission(name,
MetadataConstants.TYPE_GROUP);
-
getUserGroupManager().delete(name);
}
@@ -126,10 +125,8 @@ public class NUserGroupService implements
IUserGroupService {
aclEvaluate.checkIsGlobalAdmin();
checkGroupNameExist(groupName);
- List<String> groupUsers = new ArrayList<>();
- for (ManagedUser user : getGroupMembersByName(groupName)) {
- groupUsers.add(user.getUsername());
- }
+ List<String> groupUsers =
getGroupMembersByName(groupName).stream().map(ManagedUser::getUsername)
+ .collect(Collectors.toList());
List<String> moveInUsers = Lists.newArrayList(users);
List<String> moveOutUsers = Lists.newArrayList(groupUsers);
moveInUsers.removeAll(groupUsers);
@@ -139,7 +136,7 @@ public class NUserGroupService implements IUserGroupService
{
String currentUser = aclEvaluate.getCurrentUserName();
- List<String> moveList = new ArrayList<String>();
+ List<String> moveList = Lists.newArrayList();
moveList.addAll(moveInUsers);
moveList.addAll(moveOutUsers);
val superAdminList = userService.listSuperAdminUsers();
@@ -193,7 +190,7 @@ public class NUserGroupService implements IUserGroupService
{
if (StringUtils.isEmpty(userGroupName)) {
return listUserGroups();
}
- return getUserGroupManager().getAllUsers(path -> {
+ return getUserGroupManager().getAllGroups(path -> {
val pathPair = StringUtils.split(path, "/");
String groupName = pathPair[pathPair.length - 1];
return StringUtils.containsIgnoreCase(groupName, userGroupName);
@@ -218,13 +215,13 @@ public class NUserGroupService implements
IUserGroupService {
throw new KylinException(USERGROUP_NOT_EXIST,
String.format(Locale.ROOT,
MsgPicker.getMsg().getUserGroupNotExist(), groupName));
}
- List<UserGroup> userGroups = getUserGroupManager()
- .getAllUsers(path -> StringUtils.endsWithIgnoreCase(path,
groupName));
- if (userGroups.isEmpty()) {
+ val optional = getUserGroupManager().getAllGroups(path ->
StringUtils.endsWithIgnoreCase(path, groupName))
+ .stream().filter(group ->
StringUtils.equalsIgnoreCase(group.getGroupName(), groupName)).findFirst();
+ if (!optional.isPresent()) {
throw new KylinException(USERGROUP_NOT_EXIST,
String.format(Locale.ROOT,
MsgPicker.getMsg().getUserGroupNotExist(), groupName));
}
- return userGroups.get(0).getUuid();
+ return optional.get().getUuid();
}
public boolean exists(String name) {
diff --git
a/src/common-service/src/test/java/org/apache/kylin/rest/service/NUserGroupServiceTest.java
b/src/common-service/src/test/java/org/apache/kylin/rest/service/NUserGroupServiceTest.java
index cda761931c..93de1a097b 100644
---
a/src/common-service/src/test/java/org/apache/kylin/rest/service/NUserGroupServiceTest.java
+++
b/src/common-service/src/test/java/org/apache/kylin/rest/service/NUserGroupServiceTest.java
@@ -32,6 +32,7 @@ import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.exception.ExceptionUtils;
import org.apache.kylin.common.exception.KylinException;
import org.apache.kylin.common.persistence.transaction.TransactionException;
+import org.apache.kylin.metadata.usergroup.NUserGroupManager;
import org.apache.kylin.metadata.user.ManagedUser;
import org.apache.kylin.metadata.usergroup.UserGroup;
import org.apache.kylin.rest.response.UserGroupResponseKI;
@@ -217,6 +218,26 @@ public class NUserGroupServiceTest extends ServiceTestBase
{
Assert.assertEquals(Lists.newArrayList("g1", "g2", "g3"),
userGroupService.getAllUserGroups());
}
+ @Test
+ public void testDeleteAdminNameGroup() throws IOException {
+ String adminGroupName = "admin";
+ NUserGroupManager manager =
NUserGroupManager.getInstance(getTestConfig());
+ Assert.assertFalse(userGroupService.exists(adminGroupName));
+ Assert.assertFalse(manager.exists(adminGroupName));
+ // add 'admin' group
+ userGroupService.addGroup(adminGroupName);
+ Assert.assertTrue(userGroupService.exists(adminGroupName));
+ Assert.assertTrue(manager.exists(adminGroupName));
+ // check 'admin' group uuid
+ String adminUUID = userGroupService.getUuidByGroupName(adminGroupName);
+ manager.getAllGroups().stream().filter(group ->
group.getGroupName().equalsIgnoreCase(adminGroupName))
+ .findFirst().ifPresent(userGroup ->
Assert.assertEquals(userGroup.getUuid(), adminUUID));
+ // delete 'admin' group
+ userGroupService.deleteGroup(adminGroupName);
+ Assert.assertFalse(userGroupService.exists(adminGroupName));
+ Assert.assertFalse(manager.exists(adminGroupName));
+ }
+
private void checkDelUserGroupWithException(String groupName) {
try {
userGroupService.deleteGroup(groupName);
diff --git
a/src/core-metadata/src/main/java/org/apache/kylin/metadata/usergroup/NUserGroupManager.java
b/src/core-metadata/src/main/java/org/apache/kylin/metadata/usergroup/NUserGroupManager.java
index f9c5115f66..32fd77f381 100644
---
a/src/core-metadata/src/main/java/org/apache/kylin/metadata/usergroup/NUserGroupManager.java
+++
b/src/core-metadata/src/main/java/org/apache/kylin/metadata/usergroup/NUserGroupManager.java
@@ -91,7 +91,7 @@ public class NUserGroupManager {
return ImmutableList.copyOf(crud.listAll());
}
- public List<UserGroup> getAllUsers(Predicate<String> predicate) {
+ public List<UserGroup> getAllGroups(Predicate<String> predicate) {
return ImmutableList.copyOf(crud.listPartial(predicate));
}
diff --git
a/src/core-metadata/src/test/java/org/apache/kylin/metadata/usergroup/NUserGroupManagerTest.java
b/src/core-metadata/src/test/java/org/apache/kylin/metadata/usergroup/NUserGroupManagerTest.java
index 96647be4f0..58bf84d092 100644
---
a/src/core-metadata/src/test/java/org/apache/kylin/metadata/usergroup/NUserGroupManagerTest.java
+++
b/src/core-metadata/src/test/java/org/apache/kylin/metadata/usergroup/NUserGroupManagerTest.java
@@ -52,7 +52,7 @@ public class NUserGroupManagerTest extends
NLocalFileMetadataTestCase {
Assert.assertTrue(group.exists("g1"));
Assert.assertFalse(group.exists("g4"));
Assert.assertEquals(Lists.newArrayList("g1", "g2", "g3"),
group.getAllGroupNames());
- Assert.assertEquals("g1", group.getAllUsers(path ->
path.endsWith("g1")).get(0).getGroupName());
+ Assert.assertEquals("g1", group.getAllGroups(path ->
path.endsWith("g1")).get(0).getGroupName());
Assert.assertThrows(String.format(Locale.ROOT,
MsgPicker.getMsg().getUserGroupExist(), "g1"),
KylinException.class, () -> group.add("g1"));
