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 85efb638f220fd72692d57076886454d5812903a
Author: Jiale He <35652389+jial...@users.noreply.github.com>
AuthorDate: Fri Dec 30 15:28:47 2022 +0800
KYLIN-5457 Optimize NKylinUserManager and NUserGroupManager
* KYLIN-5457 Optimize NKylinUserManager and NUserGroupManager
* KYLIN-5457 fix ut
* KYLIN-5457 fix ut
---
.../service/CaseInsensitiveKylinUserService.java | 62 ++---------------
.../kylin/rest/service/KylinUserService.java | 80 ++++++++++++++--------
.../kylin/rest/service/NUserGroupService.java | 63 ++++++++---------
.../CaseInsensitiveKylinUserServiceTest.java | 12 ++--
.../kylin/rest/service/KylinUserServiceTest.java | 58 +++++++++++++---
.../kylin/rest/service/NUserGroupServiceTest.java | 19 ++---
.../kylin/rest/service/UserAclServiceTest.java | 5 +-
.../kylin/metadata/user/NKylinUserManager.java | 32 +++++++--
.../metadata/usergroup/NUserGroupManager.java | 26 +++++--
.../kylin/metadata/user/NKylinUserManagerTest.java | 71 +++++++++++++++++++
.../metadata/usergroup/NUserGroupManagerTest.java | 44 ++++--------
11 files changed, 289 insertions(+), 183 deletions(-)
diff --git
a/src/common-service/src/main/java/org/apache/kylin/rest/service/CaseInsensitiveKylinUserService.java
b/src/common-service/src/main/java/org/apache/kylin/rest/service/CaseInsensitiveKylinUserService.java
index a0cf392dca..ad8b54acea 100644
---
a/src/common-service/src/main/java/org/apache/kylin/rest/service/CaseInsensitiveKylinUserService.java
+++
b/src/common-service/src/main/java/org/apache/kylin/rest/service/CaseInsensitiveKylinUserService.java
@@ -18,32 +18,19 @@
package org.apache.kylin.rest.service;
-import static org.apache.kylin.rest.constant.Constant.ROLE_ADMIN;
-
-import java.io.IOException;
-import java.util.HashSet;
import java.util.List;
-import java.util.Set;
import java.util.stream.Collectors;
import org.apache.kylin.rest.constant.Constant;
+
import org.apache.kylin.metadata.user.ManagedUser;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-import org.springframework.security.core.userdetails.UserDetails;
+import lombok.extern.slf4j.Slf4j;
+@Slf4j
public class CaseInsensitiveKylinUserService extends KylinUserService {
- private final Logger logger =
LoggerFactory.getLogger(CaseInsensitiveKylinUserService.class);
-
- @Override
- public boolean userExists(String userName) {
- logger.trace("judge user exist: {}", userName);
- return getKylinUserManager().exists(userName);
- }
-
@Override
- public List<String> listAdminUsers() throws IOException {
+ public List<String> listAdminUsers() {
return listUsers().parallelStream()
.filter(managedUser ->
managedUser.getAuthorities().parallelStream().anyMatch(
simpleGrantedAuthority ->
Constant.ROLE_ADMIN.equals(simpleGrantedAuthority.getAuthority())))
@@ -51,49 +38,10 @@ public class CaseInsensitiveKylinUserService extends
KylinUserService {
}
@Override
- public List<String> listNormalUsers() throws IOException {
+ public List<String> listNormalUsers() {
return listUsers().parallelStream()
.filter(managedUser ->
managedUser.getAuthorities().parallelStream().noneMatch(
simpleGrantedAuthority ->
Constant.ROLE_ADMIN.equals(simpleGrantedAuthority.getAuthority())))
.map(ManagedUser::getUsername).collect(Collectors.toList());
}
-
- @Override
- public boolean isGlobalAdmin(String username) throws IOException {
- try {
- UserDetails userDetails = loadUserByUsername(username);
- return isGlobalAdmin(userDetails);
- } catch (Exception e) {
- logger.warn("Cat not load user by username {}", username, e);
- }
-
- return false;
- }
-
- @Override
- public boolean isGlobalAdmin(UserDetails userDetails) throws IOException {
- return userDetails != null && userDetails.getAuthorities().stream()
- .anyMatch(grantedAuthority ->
grantedAuthority.getAuthority().equals(ROLE_ADMIN));
- }
-
- @Override
- public Set<String> retainsNormalUser(Set<String> usernames) throws
IOException {
- Set<String> results = new HashSet<>();
- for (String username : usernames) {
- if (!isGlobalAdmin(username)) {
- results.add(username);
- }
- }
- return results;
- }
-
- @Override
- public boolean containsGlobalAdmin(Set<String> usernames) throws
IOException {
- for (String username : usernames) {
- if (isGlobalAdmin(username)) {
- return true;
- }
- }
- return false;
- }
}
diff --git
a/src/common-service/src/main/java/org/apache/kylin/rest/service/KylinUserService.java
b/src/common-service/src/main/java/org/apache/kylin/rest/service/KylinUserService.java
index 8e0b99e18c..3f9511ca09 100644
---
a/src/common-service/src/main/java/org/apache/kylin/rest/service/KylinUserService.java
+++
b/src/common-service/src/main/java/org/apache/kylin/rest/service/KylinUserService.java
@@ -21,15 +21,15 @@ package org.apache.kylin.rest.service;
import static
org.apache.kylin.common.exception.ServerErrorCode.DUPLICATE_USER_NAME;
import static
org.apache.kylin.common.exception.ServerErrorCode.PERMISSION_DENIED;
import static
org.apache.kylin.common.exception.code.ErrorCodeServer.USER_LOGIN_FAILED;
+import static org.apache.kylin.rest.constant.Constant.ROLE_ADMIN;
-import java.io.IOException;
-import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Locale;
+import java.util.Objects;
+import java.util.Set;
import java.util.stream.Collectors;
-import org.apache.commons.lang.StringUtils;
import org.apache.kylin.common.KylinConfig;
import org.apache.kylin.common.exception.KylinException;
import org.apache.kylin.common.msg.Message;
@@ -40,8 +40,6 @@ import org.apache.kylin.rest.aspect.Transaction;
import org.apache.kylin.rest.constant.Constant;
import org.apache.kylin.rest.exception.InternalErrorException;
import org.apache.kylin.rest.security.AclPermission;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.security.core.authority.SimpleGrantedAuthority;
@@ -55,11 +53,11 @@ import org.apache.kylin.metadata.user.ManagedUser;
import org.apache.kylin.metadata.user.NKylinUserManager;
import lombok.SneakyThrows;
import lombok.val;
+import lombok.extern.slf4j.Slf4j;
+@Slf4j
public class KylinUserService implements UserService {
- private Logger logger = LoggerFactory.getLogger(KylinUserService.class);
-
public static final String DIR_PREFIX = "/user/";
public static final Serializer<ManagedUser> SERIALIZER = new
JsonSerializer<>(ManagedUser.class);
@@ -91,7 +89,7 @@ public class KylinUserService implements UserService {
}
getKylinUserManager().update(managedUser);
userAclService.updateUserAclPermission(user, AclPermission.DATA_QUERY);
- logger.trace("update user : {}", user.getUsername());
+ log.trace("update user : {}", user.getUsername());
}
@SneakyThrows
@@ -106,7 +104,7 @@ public class KylinUserService implements UserService {
userAclService.deleteUserAcl(userName);
getKylinUserManager().delete(userName);
- logger.trace("delete user : {}", userName);
+ log.trace("delete user : {}", userName);
}
@Override
@@ -116,14 +114,8 @@ public class KylinUserService implements UserService {
@Override
public boolean userExists(String userName) {
- logger.trace("judge user exist: {}", userName);
- val users = listUsers();
- for (val user : users) {
- if (StringUtils.equalsIgnoreCase(userName, user.getUsername())) {
- return true;
- }
- }
- return false;
+ log.trace("judge user exist: {}", userName);
+ return getKylinUserManager().exists(userName);
}
/**
@@ -133,17 +125,17 @@ public class KylinUserService implements UserService {
@Override
public UserDetails loadUserByUsername(String userName) throws
UsernameNotFoundException {
Message msg = MsgPicker.getMsg();
- ManagedUser managedUser = null;
+ ManagedUser managedUser;
try {
managedUser = getKylinUserManager().get(userName);
} catch (IllegalArgumentException e) {
- logger.error("exception: ", e);
+ log.error("exception: ", e);
throw new UsernameNotFoundException(USER_LOGIN_FAILED.getMsg());
}
if (managedUser == null) {
throw new UsernameNotFoundException(String.format(Locale.ROOT,
msg.getUserNotFound(), userName));
}
- logger.trace("load user : {}", userName);
+ log.trace("load user : {}", userName);
return managedUser;
}
@@ -158,14 +150,47 @@ public class KylinUserService implements UserService {
}
@Override
- public List<String> listAdminUsers() throws IOException {
- List<String> adminUsers = new ArrayList<>();
- for (ManagedUser managedUser : listUsers()) {
- if (managedUser.getAuthorities().contains(new
SimpleGrantedAuthority(Constant.ROLE_ADMIN))) {
- adminUsers.add(managedUser.getUsername());
- }
+ public List<String> listAdminUsers() {
+ SimpleGrantedAuthority adminAuthority = new
SimpleGrantedAuthority(Constant.ROLE_ADMIN);
+ return listUsers().stream().filter(user ->
user.getAuthorities().contains(adminAuthority))
+ .map(ManagedUser::getUsername).collect(Collectors.toList());
+ }
+
+ @Override
+ public boolean isGlobalAdmin(String username) {
+ try {
+ UserDetails userDetails = loadUserByUsername(username);
+ return isGlobalAdmin(userDetails);
+ } catch (Exception e) {
+ logger.debug("Cat not load user by username {}", username, e);
+ }
+ return false;
+ }
+
+ @Override
+ public boolean isGlobalAdmin(UserDetails userDetails) {
+ if (Objects.isNull(userDetails)) {
+ return false;
}
- return adminUsers;
+ return userDetails.getAuthorities().stream()
+ .anyMatch(grantedAuthority ->
grantedAuthority.getAuthority().equals(ROLE_ADMIN));
+ }
+
+ @Override
+ public boolean containsGlobalAdmin(Set<String> usernames) {
+ return usernames.stream().anyMatch(this::isGlobalAdmin);
+ }
+
+ @Override
+ public Set<String> retainsNormalUser(Set<String> usernames) {
+ return usernames.stream().filter(username ->
!isGlobalAdmin(username)).collect(Collectors.toSet());
+ }
+
+ @Override
+ public List<String> listNormalUsers() {
+ SimpleGrantedAuthority adminAuthority = new
SimpleGrantedAuthority(Constant.ROLE_ADMIN);
+ return listUsers().stream().filter(user ->
!user.getAuthorities().contains(adminAuthority))
+ .map(ManagedUser::getUsername).collect(Collectors.toList());
}
@Override
@@ -184,5 +209,4 @@ public class KylinUserService implements UserService {
protected NKylinUserManager getKylinUserManager() {
return NKylinUserManager.getInstance(KylinConfig.getInstanceFromEnv());
}
-
}
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 7ecd9297c8..c8a50fdbb4 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
@@ -29,22 +29,25 @@ import static
org.apache.kylin.rest.constant.Constant.ROLE_ADMIN;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
+import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Locale;
import java.util.Map;
+import java.util.Objects;
import java.util.Set;
import java.util.function.Function;
import java.util.stream.Collectors;
import org.apache.commons.collections.CollectionUtils;
-import org.apache.commons.lang.StringUtils;
+import org.apache.commons.lang3.StringUtils;
import org.apache.kylin.common.KylinConfig;
import org.apache.kylin.common.exception.KylinException;
import org.apache.kylin.common.msg.MsgPicker;
import org.apache.kylin.common.persistence.ResourceStore;
import org.apache.kylin.metadata.MetadataConstants;
import org.apache.kylin.metadata.user.ManagedUser;
+import org.apache.kylin.metadata.user.NKylinUserManager;
import org.apache.kylin.metadata.usergroup.NUserGroupManager;
import org.apache.kylin.metadata.usergroup.UserGroup;
import org.apache.kylin.rest.aspect.Transaction;
@@ -54,6 +57,7 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
+import org.springframework.security.core.GrantedAuthority;
import org.springframework.security.core.authority.SimpleGrantedAuthority;
import org.springframework.stereotype.Component;
@@ -173,8 +177,9 @@ public class NUserGroupService implements IUserGroupService
{
public List<String> getAuthoritiesFilterByGroupName(String userGroupName) {
aclEvaluate.checkIsGlobalAdmin();
return StringUtils.isEmpty(userGroupName) ? getAllUserGroups()
- : getAllUserGroups().stream().filter(userGroup ->
userGroup.toUpperCase(Locale.ROOT)
-
.contains(userGroupName.toUpperCase(Locale.ROOT))).collect(Collectors.toList());
+ : getAllUserGroups().stream()
+ .filter(userGroup ->
StringUtils.containsIgnoreCase(userGroup, userGroupName))
+ .collect(Collectors.toList());
}
@Override
@@ -185,10 +190,14 @@ public class NUserGroupService implements
IUserGroupService {
@Override
public List<UserGroup> getUserGroupsFilterByGroupName(String
userGroupName) {
aclEvaluate.checkIsGlobalAdmin();
- return StringUtils.isEmpty(userGroupName) ? listUserGroups()
- :
getUserGroupManager().getAllGroups().stream().filter(userGroup ->
userGroup.getGroupName()
-
.toUpperCase(Locale.ROOT).contains(userGroupName.toUpperCase(Locale.ROOT)))
- .collect(Collectors.toList());
+ if (StringUtils.isEmpty(userGroupName)) {
+ return listUserGroups();
+ }
+ return getUserGroupManager().getAllUsers(path -> {
+ val pathPair = StringUtils.split(path, "/");
+ String groupName = pathPair[pathPair.length - 1];
+ return StringUtils.containsIgnoreCase(groupName, userGroupName);
+ });
}
@Override
@@ -205,18 +214,21 @@ public class NUserGroupService implements
IUserGroupService {
@Override
public String getUuidByGroupName(String groupName) {
- val groups = getUserGroupManager().getAllGroups();
- for (val group : groups) {
- if (StringUtils.equalsIgnoreCase(groupName, group.getGroupName()))
{
- return group.getUuid();
- }
+ if (StringUtils.isEmpty(groupName)) {
+ throw new KylinException(USERGROUP_NOT_EXIST,
+ String.format(Locale.ROOT,
MsgPicker.getMsg().getUserGroupNotExist(), groupName));
}
- 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()) {
+ throw new KylinException(USERGROUP_NOT_EXIST,
+ String.format(Locale.ROOT,
MsgPicker.getMsg().getUserGroupNotExist(), groupName));
+ }
+ return userGroups.get(0).getUuid();
}
public boolean exists(String name) {
- return getAllUserGroups().contains(name);
+ return getUserGroupManager().exists(name);
}
public ResourceStore getStore() {
@@ -244,23 +256,12 @@ public class NUserGroupService implements
IUserGroupService {
}
public Set<String> listUserGroups(String username) {
- try {
- List<String> groups = getAllUserGroups();
- Set<String> result = new HashSet<>();
- for (String group : groups) {
- val users = getGroupMembersByName(group);
- for (val user : users) {
- if (StringUtils.equalsIgnoreCase(username,
user.getUsername())) {
- result.add(group);
- break;
- }
- }
- }
- return result;
- } catch (IOException e) {
- logger.error("List user groups failed...", e);
- throw new RuntimeException(e);
+ ManagedUser user =
NKylinUserManager.getInstance(KylinConfig.getInstanceFromEnv()).get(username);
+ if (Objects.isNull(user)) {
+ return Collections.emptySet();
}
+ return
user.getAuthorities().stream().map(GrantedAuthority::getAuthority).filter(this::exists)
+ .collect(Collectors.toSet());
}
private NUserGroupManager getUserGroupManager() {
diff --git
a/src/common-service/src/test/java/org/apache/kylin/rest/service/CaseInsensitiveKylinUserServiceTest.java
b/src/common-service/src/test/java/org/apache/kylin/rest/service/CaseInsensitiveKylinUserServiceTest.java
index b160c7f9d3..818f8c8c3a 100644
---
a/src/common-service/src/test/java/org/apache/kylin/rest/service/CaseInsensitiveKylinUserServiceTest.java
+++
b/src/common-service/src/test/java/org/apache/kylin/rest/service/CaseInsensitiveKylinUserServiceTest.java
@@ -18,7 +18,6 @@
package org.apache.kylin.rest.service;
-import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
@@ -49,9 +48,6 @@ public class CaseInsensitiveKylinUserServiceTest extends
NLocalFileMetadataTestC
@Mock
UserAclService userAclService = Mockito.spy(UserAclService.class);
- @InjectMocks
- private KylinUserService kylinUserService1;
-
@InjectMocks
@Spy
private CaseInsensitiveKylinUserService kylinUserService;
@@ -117,14 +113,14 @@ public class CaseInsensitiveKylinUserServiceTest extends
NLocalFileMetadataTestC
}
@Test
- public void testListAdminUsers() throws IOException {
+ public void testListAdminUsers() {
List<String> adminUsers = kylinUserService.listAdminUsers();
Assert.assertEquals(1, adminUsers.size());
Assert.assertTrue(adminUsers.contains("ADMIN"));
}
@Test
- public void testIsGlobalAdmin() throws IOException {
+ public void testIsGlobalAdmin() {
Assert.assertTrue(kylinUserService.isGlobalAdmin("ADMIN"));
Assert.assertTrue(kylinUserService.isGlobalAdmin("AdMIN"));
@@ -132,14 +128,14 @@ public class CaseInsensitiveKylinUserServiceTest extends
NLocalFileMetadataTestC
}
@Test
- public void testRetainsNormalUser() throws IOException {
+ public void testRetainsNormalUser() {
Set<String> normalUsers =
kylinUserService.retainsNormalUser(Sets.newHashSet("ADMIN", "adMIN",
"NOTEXISTS"));
Assert.assertEquals(1, normalUsers.size());
Assert.assertTrue(normalUsers.contains("NOTEXISTS"));
}
@Test
- public void testContainsGlobalAdmin() throws IOException {
+ public void testContainsGlobalAdmin() {
Assert.assertTrue(kylinUserService.containsGlobalAdmin(Sets.newHashSet("ADMIN")));
Assert.assertTrue(kylinUserService.containsGlobalAdmin(Sets.newHashSet("adMIN")));
Assert.assertFalse(kylinUserService.containsGlobalAdmin(Sets.newHashSet("adMI
N")));
diff --git
a/src/common-service/src/test/java/org/apache/kylin/rest/service/KylinUserServiceTest.java
b/src/common-service/src/test/java/org/apache/kylin/rest/service/KylinUserServiceTest.java
index d039577498..da5e20f211 100644
---
a/src/common-service/src/test/java/org/apache/kylin/rest/service/KylinUserServiceTest.java
+++
b/src/common-service/src/test/java/org/apache/kylin/rest/service/KylinUserServiceTest.java
@@ -18,9 +18,9 @@
package org.apache.kylin.rest.service;
-import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
+import java.util.Set;
import org.apache.kylin.common.util.NLocalFileMetadataTestCase;
import org.apache.kylin.rest.constant.Constant;
@@ -36,6 +36,9 @@ import
org.springframework.security.core.userdetails.UserDetails;
import org.springframework.security.core.userdetails.UsernameNotFoundException;
import org.springframework.test.util.ReflectionTestUtils;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Sets;
+
import org.apache.kylin.metadata.user.ManagedUser;
import org.apache.kylin.metadata.user.NKylinUserManager;
import lombok.extern.slf4j.Slf4j;
@@ -98,18 +101,55 @@ public class KylinUserServiceTest extends
NLocalFileMetadataTestCase {
@Test
public void testUserExists() {
- ManagedUser user = new ManagedUser();
- user.setUsername("tTtUser");
- List<SimpleGrantedAuthority> roles = new ArrayList<>();
- roles.add(new SimpleGrantedAuthority("ALL_USERS"));
- user.setGrantedAuthorities(roles);
-
Mockito.doNothing().when(userAclService).updateUserAclPermission(Mockito.any(UserDetails.class),
- Mockito.any(Permission.class));
- kylinUserService.createUser(user);
+ createNormalUser("tTtUser");
Assert.assertTrue(kylinUserService.userExists("tTtUser"));
Assert.assertTrue(kylinUserService.userExists("tttuser"));
Assert.assertTrue(kylinUserService.userExists("TTTUSER"));
Assert.assertFalse(kylinUserService.userExists("NOTEXIST"));
}
+ @Test
+ public void testIsGlobalAdmin() {
+ Assert.assertFalse(kylinUserService.isGlobalAdmin((UserDetails) null));
+
+ UserDetails adminUser = kylinUserService.loadUserByUsername("ADMIN");
+ Assert.assertTrue(kylinUserService.isGlobalAdmin(adminUser));
+ Assert.assertFalse(kylinUserService.isGlobalAdmin("notexist"));
+ }
+
+ @Test
+ public void testContainsGlobalAdmin() {
+
Assert.assertTrue(kylinUserService.containsGlobalAdmin(Sets.newHashSet("ADMIN")));
+ createNormalUser("normalUser1");
+
Assert.assertFalse(kylinUserService.containsGlobalAdmin(Sets.newHashSet("normalUser1")));
+
Assert.assertTrue(kylinUserService.containsGlobalAdmin(Sets.newHashSet("normalUser1",
"ADMIN")));
+ }
+
+ @Test
+ public void testRetainsNormalUser() {
+ createNormalUser("normalUser2");
+ createNormalUser("normalUser3");
+
+ Set<String> normalUserSet = kylinUserService
+ .retainsNormalUser(Sets.newHashSet("normalUser2",
"normalUser3", "ADMIN"));
+ Assert.assertFalse(normalUserSet.isEmpty());
+ Assert.assertEquals(2, normalUserSet.size());
+ }
+
+ @Test
+ public void testListNormalUsers() {
+ createNormalUser("normalUser4");
+ List<String> normalUsers = kylinUserService.listNormalUsers();
+ Assert.assertFalse(normalUsers.isEmpty());
+ Assert.assertTrue(normalUsers.contains("normalUser4"));
+ }
+
+ private void createNormalUser(String userName) {
+ ManagedUser user = new ManagedUser();
+ user.setUsername(userName);
+ user.setGrantedAuthorities(Lists.newArrayList(new
SimpleGrantedAuthority("ALL_USERS")));
+
Mockito.doNothing().when(userAclService).updateUserAclPermission(Mockito.any(UserDetails.class),
+ Mockito.any(Permission.class));
+ kylinUserService.createUser(user);
+ }
}
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 d231dfa8b8..cda761931c 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
@@ -24,10 +24,11 @@ import static
org.apache.kylin.rest.constant.Constant.ROLE_ADMIN;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.Collections;
import java.util.List;
import java.util.Map;
-import org.apache.commons.lang.StringUtils;
+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;
@@ -62,7 +63,6 @@ public class NUserGroupServiceTest extends ServiceTestBase {
userGroupService.deleteGroup(group);
}
//test group add and get
- // userGroupService.addGroup(GROUP_ALL_USERS);
userGroupService.addGroup("g1");
userGroupService.addGroup("g2");
userGroupService.addGroup("g3");
@@ -72,6 +72,7 @@ public class NUserGroupServiceTest extends ServiceTestBase {
Assert.assertEquals(Lists.newArrayList("g1"),
userGroupService.getAuthoritiesFilterByGroupName("g1"));
val groups = userGroupService.getUserGroupsFilterByGroupName("G");
Assert.assertEquals(3, groups.size());
+ Assert.assertThrows(KylinException.class, () ->
userGroupService.getUuidByGroupName("noexist_group"));
for (val group : groups) {
Assert.assertNotNull(group.getUuid());
Assert.assertTrue(group.getGroupName().contains("g"));
@@ -144,9 +145,9 @@ public class NUserGroupServiceTest extends ServiceTestBase {
}
@Test
- public void testAddUserToNotExistGroup() throws Exception {
+ public void testAddUserToNotExistGroup() {
try {
- userGroupService.modifyGroupUsers("UNKNOWN",
Arrays.asList("ADMIN"));
+ userGroupService.modifyGroupUsers("UNKNOWN",
Collections.singletonList("ADMIN"));
} catch (TransactionException e) {
Assert.assertTrue(e.getCause().getCause() instanceof
KylinException);
Assert.assertTrue(StringUtils.equals(e.getCause().getCause().getMessage(),
@@ -160,19 +161,21 @@ public class NUserGroupServiceTest extends
ServiceTestBase {
public void testListUserGroups() throws IOException {
userGroupService.addGroup("t1");
userGroupService.addGroup("t2");
- userGroupService.modifyGroupUsers("t1", Arrays.asList("MODELER"));
- userGroupService.modifyGroupUsers("t2", Arrays.asList("MODELER"));
+ userGroupService.modifyGroupUsers("t1",
Collections.singletonList("MODELER"));
+ userGroupService.modifyGroupUsers("t2",
Collections.singletonList("MODELER"));
+ var emptyGroups = userGroupService.listUserGroups("notexist");
+ Assert.assertTrue(emptyGroups.isEmpty());
var groups = userGroupService.listUserGroups("MODELER");
Assert.assertEquals(2, groups.size());
Assert.assertTrue(groups.contains("t1"));
Assert.assertTrue(groups.contains("t2"));
userGroupService.addGroup("t3");
- userGroupService.modifyGroupUsers("t3", Arrays.asList("MODELER"));
+ userGroupService.modifyGroupUsers("t3",
Collections.singletonList("MODELER"));
groups = userGroupService.listUserGroups("MODELER");
Assert.assertEquals(3, groups.size());
Assert.assertTrue(groups.contains("t3"));
- List<String> userList = Arrays.asList("ADMIN");
+ List<String> userList = Collections.singletonList("ADMIN");
Assert.assertThrows(RuntimeException.class, () ->
userGroupService.modifyGroupUsers("t1", userList));
}
diff --git
a/src/common-service/src/test/java/org/apache/kylin/rest/service/UserAclServiceTest.java
b/src/common-service/src/test/java/org/apache/kylin/rest/service/UserAclServiceTest.java
index 3814e8bc39..c1c67df8a5 100644
---
a/src/common-service/src/test/java/org/apache/kylin/rest/service/UserAclServiceTest.java
+++
b/src/common-service/src/test/java/org/apache/kylin/rest/service/UserAclServiceTest.java
@@ -53,6 +53,8 @@ import
org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.security.core.userdetails.UserDetails;
import org.springframework.test.util.ReflectionTestUtils;
+import lombok.SneakyThrows;
+
public class UserAclServiceTest extends ServiceTestBase {
@Mock
@@ -121,8 +123,9 @@ public class UserAclServiceTest extends ServiceTestBase {
@Test
public void testGetAllUsersHasGlobalPermission() {
KylinUserService kylinUserService = new KylinUserService() {
+ @SneakyThrows
@Override
- public List<String> listAdminUsers() throws IOException {
+ public List<String> listAdminUsers() {
throw new IOException("test");
}
};
diff --git
a/src/core-metadata/src/main/java/org/apache/kylin/metadata/user/NKylinUserManager.java
b/src/core-metadata/src/main/java/org/apache/kylin/metadata/user/NKylinUserManager.java
index f268ea69a6..4b487a6841 100644
---
a/src/core-metadata/src/main/java/org/apache/kylin/metadata/user/NKylinUserManager.java
+++
b/src/core-metadata/src/main/java/org/apache/kylin/metadata/user/NKylinUserManager.java
@@ -23,13 +23,16 @@ import static
org.apache.kylin.common.persistence.ResourceStore.USER_ROOT;
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
+import java.util.NavigableSet;
+import java.util.Objects;
import java.util.Set;
import java.util.stream.Collectors;
+import org.apache.commons.lang3.StringUtils;
import org.apache.kylin.common.KylinConfig;
import org.apache.kylin.common.persistence.ResourceStore;
-import org.apache.kylin.metadata.cachesync.CachedCrudAssist;
import org.apache.kylin.common.persistence.transaction.UnitOfWork;
+import org.apache.kylin.metadata.cachesync.CachedCrudAssist;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.security.core.authority.SimpleGrantedAuthority;
@@ -81,12 +84,15 @@ public class NKylinUserManager {
}
public ManagedUser get(String name) {
+ if (StringUtils.isEmpty(name)) {
+ return null;
+ }
+ ManagedUser user = crud.get(name);
if (getConfig().isMetadataKeyCaseInSensitiveEnabled()) {
- return crud.get(name);
- } else {
- return crud.listAll().stream().filter(managedUser ->
managedUser.getUsername().equalsIgnoreCase(name))
- .findAny().orElse(null);
+ return user;
}
+ return Objects.nonNull(user) ? user
+ : crud.listPartial(path ->
StringUtils.endsWithIgnoreCase(path, name)).stream().findAny().orElse(null);
}
public List<ManagedUser> list() {
@@ -116,7 +122,21 @@ public class NKylinUserManager {
}
public boolean exists(String username) {
- return get(username) != null;
+ if (StringUtils.isEmpty(username)) {
+ return false;
+ }
+ ManagedUser user = crud.get(username);
+ if (getConfig().isMetadataKeyCaseInSensitiveEnabled()) {
+ return Objects.nonNull(user);
+ }
+ if (Objects.nonNull(user)) {
+ return true;
+ }
+ NavigableSet<String> users = getStore().listResources(USER_ROOT);
+ if (Objects.isNull(users)) {
+ return false;
+ }
+ return users.stream().anyMatch(path ->
StringUtils.endsWithIgnoreCase(path, username));
}
public Set<String> getUserGroups(String userName) {
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 1c51457eee..f9c5115f66 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
@@ -22,17 +22,21 @@ import static
org.apache.kylin.common.exception.ServerErrorCode.DUPLICATE_USERGR
import static
org.apache.kylin.common.exception.ServerErrorCode.USERGROUP_NOT_EXIST;
import static
org.apache.kylin.common.persistence.ResourceStore.USER_GROUP_ROOT;
+import java.util.Collections;
import java.util.List;
import java.util.Locale;
+import java.util.NavigableSet;
+import java.util.Objects;
+import java.util.function.Predicate;
import java.util.stream.Collectors;
-import org.apache.commons.lang.StringUtils;
+import org.apache.commons.lang3.StringUtils;
import org.apache.kylin.common.KylinConfig;
import org.apache.kylin.common.exception.KylinException;
import org.apache.kylin.common.msg.MsgPicker;
import org.apache.kylin.common.persistence.ResourceStore;
-import org.apache.kylin.metadata.cachesync.CachedCrudAssist;
import org.apache.kylin.common.persistence.transaction.UnitOfWork;
+import org.apache.kylin.metadata.cachesync.CachedCrudAssist;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -73,15 +77,29 @@ public class NUserGroupManager {
}
public List<String> getAllGroupNames() {
- return
ImmutableList.copyOf(crud.listAll().stream().map(UserGroup::getGroupName).collect(Collectors.toList()));
+ NavigableSet<String> userGroups =
getStore().listResources(USER_GROUP_ROOT);
+ if (Objects.isNull(userGroups)) {
+ return Collections.emptyList();
+ }
+ return userGroups.stream().map(path -> {
+ String[] pathArray = StringUtils.split(path, "/");
+ return pathArray[pathArray.length - 1];
+ }).collect(Collectors.toList());
}
public List<UserGroup> getAllGroups() {
return ImmutableList.copyOf(crud.listAll());
}
+ public List<UserGroup> getAllUsers(Predicate<String> predicate) {
+ return ImmutableList.copyOf(crud.listPartial(predicate));
+ }
+
public boolean exists(String name) {
- return getAllGroupNames().contains(name);
+ if (StringUtils.isEmpty(name)) {
+ return false;
+ }
+ return Objects.nonNull(crud.get(name));
}
public UserGroup copyForWrite(UserGroup userGroup) {
diff --git
a/src/core-metadata/src/test/java/org/apache/kylin/metadata/user/NKylinUserManagerTest.java
b/src/core-metadata/src/test/java/org/apache/kylin/metadata/user/NKylinUserManagerTest.java
new file mode 100644
index 0000000000..c5246228dd
--- /dev/null
+++
b/src/core-metadata/src/test/java/org/apache/kylin/metadata/user/NKylinUserManagerTest.java
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kylin.metadata.user;
+
+import java.util.Arrays;
+
+import org.apache.kylin.common.util.NLocalFileMetadataTestCase;
+import org.apache.kylin.rest.constant.Constant;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.springframework.security.core.authority.SimpleGrantedAuthority;
+
+class NKylinUserManagerTest extends NLocalFileMetadataTestCase {
+
+ @BeforeEach
+ void setUp() {
+ this.createTestMetadata();
+ }
+
+ @AfterEach
+ void tearDown() {
+ this.cleanupTestMetadata();
+ }
+
+ @Test
+ void testGetAndExist() {
+ NKylinUserManager manager =
NKylinUserManager.getInstance(getTestConfig());
+
+ // no user
+ Assertions.assertFalse(manager.exists("noexist"));
+ Assertions.assertFalse(manager.exists(null));
+
+ // has ADMIN
+ ManagedUser adminUser = new ManagedUser("ADMIN", "KYLIN", false,
Arrays.asList(//
+ new SimpleGrantedAuthority(Constant.ROLE_ADMIN), new
SimpleGrantedAuthority(Constant.ROLE_ANALYST),
+ new SimpleGrantedAuthority(Constant.ROLE_MODELER)));
+ manager.update(adminUser);
+ Assertions.assertFalse(manager.exists("noexist"));
+
+ // admin exists
+ Assertions.assertTrue(manager.exists("ADMIN"));
+
+ getTestConfig().setProperty("kylin.metadata.key-case-insensitive",
"true");
+ Assertions.assertTrue(manager.exists("ADMIN"));
+
+ // get
+ Assertions.assertNotNull(manager.get("ADMIN"));
+ getTestConfig().setProperty("kylin.metadata.key-case-insensitive",
"false");
+ Assertions.assertNotNull(manager.get("ADMIN"));
+ Assertions.assertNull(manager.get("notexist"));
+ Assertions.assertNull(manager.get(null));
+ }
+}
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 1ebbdd337b..96647be4f0 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
@@ -18,7 +18,10 @@
package org.apache.kylin.metadata.usergroup;
+import java.util.Locale;
+
import org.apache.kylin.common.exception.KylinException;
+import org.apache.kylin.common.msg.MsgPicker;
import org.apache.kylin.common.util.NLocalFileMetadataTestCase;
import org.junit.After;
import org.junit.Assert;
@@ -45,27 +48,18 @@ public class NUserGroupManagerTest extends
NLocalFileMetadataTestCase {
group.add("g1");
group.add("g2");
group.add("g3");
+ Assert.assertFalse(group.exists(null));
Assert.assertTrue(group.exists("g1"));
Assert.assertFalse(group.exists("g4"));
Assert.assertEquals(Lists.newArrayList("g1", "g2", "g3"),
group.getAllGroupNames());
- try {
- group.add("g1");
- Assert.fail("expecting some AlreadyExistsException here");
- } catch (KylinException e) {
- Assert.assertEquals("The user group \"g1\" already exists. Please
check and try again.", e.getMessage());
- }
+ Assert.assertEquals("g1", group.getAllUsers(path ->
path.endsWith("g1")).get(0).getGroupName());
+ Assert.assertThrows(String.format(Locale.ROOT,
MsgPicker.getMsg().getUserGroupExist(), "g1"),
+ KylinException.class, () -> group.add("g1"));
group.delete("g1");
Assert.assertFalse(group.exists("g1"));
-
- try {
- group.delete("g1");
- Assert.fail("expecting some AlreadyExistsException here");
- } catch (Exception e) {
- Assert.assertTrue(e instanceof KylinException);
- Assert.assertEquals("Invalid values in parameter “group_name“. The
value g1 doesn’t exist.",
- e.getMessage());
- }
+ Assert.assertThrows(String.format(Locale.ROOT,
MsgPicker.getMsg().getUserGroupNotExist(), "g1"),
+ KylinException.class, () -> group.delete("g1"));
}
@Test
@@ -74,23 +68,11 @@ public class NUserGroupManagerTest extends
NLocalFileMetadataTestCase {
group.add("test1");
group.add("test2");
group.add("test3");
- try {
- group.add("TEST1");
- Assert.fail("expecting some AlreadyExistsException here");
- } catch (KylinException e) {
- Assert.assertEquals("The user group \"test1\" already exists.
Please check and try again.", e.getMessage());
- }
-
+ Assert.assertThrows(String.format(Locale.ROOT,
MsgPicker.getMsg().getUserGroupExist(), "TEST1"),
+ KylinException.class, () -> group.add("TEST1"));
group.delete("Test1");
Assert.assertFalse(group.exists("test1"));
-
- try {
- group.delete("test1");
- Assert.fail("expecting some AlreadyExistsException here");
- } catch (Exception e) {
- Assert.assertTrue(e instanceof KylinException);
- Assert.assertEquals("Invalid values in parameter “group_name“. The
value test1 doesn’t exist.",
- e.getMessage());
- }
+ Assert.assertThrows(String.format(Locale.ROOT,
MsgPicker.getMsg().getUserGroupNotExist(), "test1"),
+ KylinException.class, () -> group.delete("test1"));
}
}
