Skip to content

Commit ac5aa1c

Browse files
healchowhealchow
authored
[INLONG-12075][Manager] Allow admin or in-charge delete InlongGroup (#12076)
1 parent c6a949e commit ac5aa1c

10 files changed

Lines changed: 214 additions & 93 deletions

File tree

‎inlong-manager/manager-common/src/main/java/org/apache/inlong/manager/common/util/Preconditions.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,9 @@
2525

2626
import java.util.Arrays;
2727
import java.util.Collection;
28-
import java.util.HashSet;
2928
import java.util.List;
3029
import java.util.Map;
3130
import java.util.Objects;
32-
import java.util.Set;
3331
import java.util.function.Supplier;
3432

3533
/**
@@ -197,7 +195,9 @@ public static boolean inSeparatedString(String target, String separatedStr, Stri
197195
if (StringUtils.isBlank(target) || StringUtils.isBlank(separatedStr)) {
198196
return false;
199197
}
200-
Set<String> set = new HashSet<>(Arrays.asList(separatedStr.split(separator)));
201-
return set.contains(target);
198+
// For a single lookup, using a List avoids the extra cost of building a Set;
199+
// prefer Set only for repeated lookups.
200+
List<String> list = Arrays.asList(separatedStr.split(separator));
201+
return list.contains(target);
202202
}
203203
}

‎inlong-manager/manager-service/src/main/java/org/apache/inlong/manager/service/cluster/InlongClusterServiceImpl.java‎

Lines changed: 12 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@
3434
import org.apache.inlong.manager.common.enums.ErrorCodeEnum;
3535
import org.apache.inlong.manager.common.enums.GroupStatus;
3636
import org.apache.inlong.manager.common.enums.NodeStatus;
37-
import org.apache.inlong.manager.common.enums.TenantUserTypeEnum;
3837
import org.apache.inlong.manager.common.exceptions.BusinessException;
3938
import org.apache.inlong.manager.common.util.CommonBeanUtils;
4039
import org.apache.inlong.manager.common.util.Preconditions;
@@ -85,6 +84,7 @@
8584
import org.apache.inlong.manager.service.tenant.InlongTenantService;
8685
import org.apache.inlong.manager.service.user.InlongRoleService;
8786
import org.apache.inlong.manager.service.user.TenantRoleService;
87+
import org.apache.inlong.manager.service.user.UserService;
8888

8989
import com.github.pagehelper.Page;
9090
import com.github.pagehelper.PageHelper;
@@ -168,6 +168,8 @@ public class InlongClusterServiceImpl implements InlongClusterService {
168168
@Autowired
169169
private TenantClusterTagEntityMapper tenantClusterTagMapper;
170170
@Autowired
171+
private UserService userService;
172+
@Autowired
171173
private InlongTenantService tenantService;
172174
@Autowired
173175
private InlongRoleService inlongRoleService;
@@ -283,12 +285,9 @@ public List<ClusterTagResponse> listTag(ClusterTagPageRequest request, UserInfo
283285
List<InlongClusterTagEntity> clusterTagEntities = clusterTagMapper.selectByCondition(request);
284286
if (CollectionUtils.isNotEmpty(clusterTagEntities)) {
285287
for (InlongClusterTagEntity tagEntity : clusterTagEntities) {
286-
// only the person in charges can query
287-
if (!opInfo.getAccountType().equals(TenantUserTypeEnum.TENANT_ADMIN.getCode())) {
288-
List<String> inCharges = Arrays.asList(tagEntity.getInCharges().split(InlongConstants.COMMA));
289-
if (!inCharges.contains(opInfo.getName())) {
290-
continue;
291-
}
288+
// only the person is admin or in charges can query
289+
if (!userService.isAdminOrInCharge(opInfo, null, tagEntity.getInCharges())) {
290+
continue;
292291
}
293292
filterResult.add(tagEntity);
294293
}
@@ -503,12 +502,9 @@ public List<ClusterInfo> list(ClusterPageRequest request, UserInfo opInfo) {
503502
List<InlongClusterEntity> clusterEntities = clusterMapper.selectByCondition(request);
504503
List<InlongClusterEntity> filterResult = new ArrayList<>();
505504
for (InlongClusterEntity entity : clusterEntities) {
506-
// only the person in charges can query
507-
if (!opInfo.getAccountType().equals(TenantUserTypeEnum.TENANT_ADMIN.getCode())) {
508-
List<String> inCharges = Arrays.asList(entity.getInCharges().split(InlongConstants.COMMA));
509-
if (!inCharges.contains(opInfo.getName())) {
510-
continue;
511-
}
505+
// only the person is admin or in charges can query
506+
if (!userService.isAdminOrInCharge(opInfo, null, entity.getInCharges())) {
507+
continue;
512508
}
513509
filterResult.add(entity);
514510
}
@@ -797,12 +793,9 @@ public List<ClusterNodeResponse> listNode(ClusterPageRequest request, UserInfo o
797793
List<InlongClusterEntity> clusterList =
798794
clusterMapper.selectByKey(request.getClusterTag(), request.getName(), request.getType());
799795
for (InlongClusterEntity cluster : clusterList) {
800-
// only the person in charges can query
801-
if (!opInfo.getAccountType().equals(TenantUserTypeEnum.TENANT_ADMIN.getCode())) {
802-
List<String> inCharges = Arrays.asList(cluster.getInCharges().split(InlongConstants.COMMA));
803-
if (!inCharges.contains(opInfo.getName())) {
804-
continue;
805-
}
796+
// only the person is admin or in charges can query
797+
if (!userService.isAdminOrInCharge(opInfo, null, cluster.getInCharges())) {
798+
continue;
806799
}
807800
allNodeList.addAll(clusterNodeMapper.selectByParentId(cluster.getId(), null));
808801
}

‎inlong-manager/manager-service/src/main/java/org/apache/inlong/manager/service/group/InlongGroupProcessService.java‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,11 @@
1717

1818
package org.apache.inlong.manager.service.group;
1919

20-
import org.apache.inlong.manager.common.consts.InlongConstants;
2120
import org.apache.inlong.manager.common.enums.ErrorCodeEnum;
2221
import org.apache.inlong.manager.common.enums.GroupOperateType;
2322
import org.apache.inlong.manager.common.enums.GroupStatus;
2423
import org.apache.inlong.manager.common.enums.ProcessName;
2524
import org.apache.inlong.manager.common.enums.TaskStatus;
26-
import org.apache.inlong.manager.common.enums.TenantUserTypeEnum;
2725
import org.apache.inlong.manager.common.exceptions.BusinessException;
2826
import org.apache.inlong.manager.common.exceptions.WorkflowListenerException;
2927
import org.apache.inlong.manager.common.threadPool.VisiableThreadPoolTaskExecutor;
@@ -54,7 +52,6 @@
5452
import org.springframework.stereotype.Service;
5553

5654
import java.util.ArrayList;
57-
import java.util.Arrays;
5855
import java.util.Comparator;
5956
import java.util.List;
6057
import java.util.concurrent.ExecutorService;
@@ -274,15 +271,10 @@ public Boolean deleteProcess(String groupId, String operator) {
274271
public Boolean deleteProcess(String groupId, UserInfo opInfo) {
275272
InlongGroupEntity entity = groupMapper.selectByGroupId(groupId);
276273
Preconditions.expectNotNull(entity, ErrorCodeEnum.GROUP_NOT_FOUND, ErrorCodeEnum.GROUP_NOT_FOUND.getMessage());
277-
// only the person in charges can delete
278-
if (!opInfo.getAccountType().equals(TenantUserTypeEnum.TENANT_ADMIN.getCode())) {
279-
List<String> inCharges = Arrays.asList(entity.getInCharges().split(InlongConstants.COMMA));
280-
if (!inCharges.contains(opInfo.getName())) {
281-
throw new BusinessException(ErrorCodeEnum.GROUP_PERMISSION_DENIED);
282-
}
283-
}
274+
284275
// check can be deleted
285276
InlongGroupInfo groupInfo = groupService.doDeleteCheck(groupId, opInfo.getName());
277+
286278
// start to delete group process
287279
GroupResourceProcessForm form = genGroupResourceProcessForm(groupInfo, GroupOperateType.DELETE);
288280
WorkflowResult result = workflowService.start(ProcessName.DELETE_GROUP_PROCESS, opInfo.getName(), form);

‎inlong-manager/manager-service/src/main/java/org/apache/inlong/manager/service/group/InlongGroupServiceImpl.java‎

Lines changed: 12 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
import org.apache.inlong.manager.common.enums.GroupStatus;
2828
import org.apache.inlong.manager.common.enums.OperationTarget;
2929
import org.apache.inlong.manager.common.enums.ProcessName;
30-
import org.apache.inlong.manager.common.enums.TenantUserTypeEnum;
3130
import org.apache.inlong.manager.common.exceptions.BusinessException;
3231
import org.apache.inlong.manager.common.util.CommonBeanUtils;
3332
import org.apache.inlong.manager.common.util.JsonUtils;
@@ -108,7 +107,6 @@
108107
import java.sql.Timestamp;
109108
import java.time.LocalDateTime;
110109
import java.util.ArrayList;
111-
import java.util.Arrays;
112110
import java.util.HashMap;
113111
import java.util.List;
114112
import java.util.Map;
@@ -463,12 +461,9 @@ public List<InlongGroupBriefInfo> listBrief(InlongGroupPageRequest request, User
463461
OrderFieldEnum.checkOrderField(request);
464462
OrderTypeEnum.checkOrderType(request);
465463
for (InlongGroupEntity groupEntity : groupMapper.selectByCondition(request)) {
466-
// only the person in charges can query
467-
if (!opInfo.getAccountType().equals(TenantUserTypeEnum.TENANT_ADMIN.getCode())) {
468-
List<String> inCharges = Arrays.asList(groupEntity.getInCharges().split(InlongConstants.COMMA));
469-
if (!inCharges.contains(opInfo.getName())) {
470-
continue;
471-
}
464+
// only the person is admin or in charges can query
465+
if (!userService.isAdminOrInCharge(opInfo, null, groupEntity.getInCharges())) {
466+
continue;
472467
}
473468
filterGroupEntities.add(groupEntity);
474469
}
@@ -635,12 +630,10 @@ public Map<String, Object> detail(String groupId) {
635630
@Override
636631
public InlongGroupInfo doDeleteCheck(String groupId, String operator) {
637632
InlongGroupInfo groupInfo = this.get(groupId);
638-
// only the person in charges can update
639-
List<String> inCharges = Arrays.asList(groupInfo.getInCharges().split(InlongConstants.COMMA));
640-
if (!inCharges.contains(operator)) {
641-
throw new BusinessException(ErrorCodeEnum.GROUP_PERMISSION_DENIED,
642-
String.format("user [%s] has no privilege for the inlong group", operator));
643-
}
633+
// only the person is admin or in charges can delete
634+
userService.checkUser(groupInfo.getInCharges(), operator,
635+
"Current user does not have permission to delete group info");
636+
644637
// determine whether the current status can be deleted
645638
GroupStatus curState = GroupStatus.forCode(groupInfo.getStatus());
646639
if (GroupStatus.notAllowedTransition(curState, GroupStatus.CONFIG_DELETING)) {
@@ -762,14 +755,12 @@ private UserDefinedSortConf createUserDefinedSortConfig(Map<String, String> extM
762755
}
763756

764757
private void chkUnmodifiableParams(InlongGroupEntity entity, InlongGroupRequest request) {
765-
// check mqType
766-
Preconditions.expectTrue(
767-
Objects.equals(entity.getMqType(), request.getMqType())
768-
|| Objects.equals(entity.getStatus(), GroupStatus.TO_BE_SUBMIT.getCode()),
758+
// check mqType, the mqType must not null in entity
759+
Preconditions.expectTrue(entity.getMqType().equals(request.getMqType())
760+
|| GroupStatus.TO_BE_SUBMIT.getCode().equals(entity.getStatus()),
769761
"mqType not allowed modify");
770762
// check record version
771-
Preconditions.expectEquals(entity.getVersion(), request.getVersion(),
772-
ErrorCodeEnum.CONFIG_EXPIRED,
763+
Preconditions.expectEquals(entity.getVersion(), request.getVersion(), ErrorCodeEnum.CONFIG_EXPIRED,
773764
String.format("record has expired with record version=%d, request version=%d",
774765
entity.getVersion(), request.getVersion()));
775766
}
@@ -783,7 +774,7 @@ public Boolean startTagSwitch(String groupId, String clusterTag) {
783774

784775
// check if the group mode is data sync mode
785776
if (InlongConstants.DATASYNC_REALTIME_MODE.equals(groupInfo.getInlongGroupMode())) {
786-
String errMSg = String.format("no need to switch sync mode group = {}", groupId);
777+
String errMSg = String.format("no need to switch sync mode group = %s", groupId);
787778
LOGGER.error(errMSg);
788779
throw new BusinessException(errMSg);
789780
}

‎inlong-manager/manager-service/src/main/java/org/apache/inlong/manager/service/sink/StreamSinkServiceImpl.java‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import org.apache.inlong.manager.common.enums.OperationTarget;
2626
import org.apache.inlong.manager.common.enums.SinkStatus;
2727
import org.apache.inlong.manager.common.enums.StreamStatus;
28-
import org.apache.inlong.manager.common.enums.TenantUserTypeEnum;
2928
import org.apache.inlong.manager.common.exceptions.BusinessException;
3029
import org.apache.inlong.manager.common.util.CommonBeanUtils;
3130
import org.apache.inlong.manager.common.util.JsonUtils;
@@ -96,7 +95,6 @@
9695

9796
import java.io.StringReader;
9897
import java.util.ArrayList;
99-
import java.util.Arrays;
10098
import java.util.Collections;
10199
import java.util.HashMap;
102100
import java.util.List;
@@ -435,17 +433,13 @@ public List<? extends StreamSink> listByCondition(SinkPageRequest request, UserI
435433
StreamSinkOperator sinkOperator = operatorFactory.getInstance(entry.getKey());
436434
PageResult<? extends StreamSink> pageInfo = sinkOperator.getPageInfo(entry.getValue());
437435
for (StreamSink streamSink : pageInfo.getList()) {
438-
InlongGroupEntity groupEntity =
439-
groupMapper.selectByGroupId(streamSink.getInlongGroupId());
436+
InlongGroupEntity groupEntity = groupMapper.selectByGroupId(streamSink.getInlongGroupId());
440437
if (groupEntity == null) {
441438
continue;
442439
}
443-
// only the person in charges can query
444-
if (!opInfo.getAccountType().equals(TenantUserTypeEnum.TENANT_ADMIN.getCode())) {
445-
List<String> inCharges = Arrays.asList(groupEntity.getInCharges().split(InlongConstants.COMMA));
446-
if (!inCharges.contains(opInfo.getName())) {
447-
continue;
448-
}
440+
// only the person is admin or in charges can query
441+
if (!userService.isAdminOrInCharge(opInfo, null, groupEntity.getInCharges())) {
442+
continue;
449443
}
450444
filterResult.add(streamSink);
451445
}

‎inlong-manager/manager-service/src/main/java/org/apache/inlong/manager/service/source/StreamSourceServiceImpl.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,7 @@ public PageResult<? extends StreamSource> listByCondition(SourcePageRequest requ
251251
OrderTypeEnum.checkOrderType(request);
252252
List<StreamSourceEntity> entityList = sourceMapper.selectByCondition(request);
253253
List<StreamSourceEntity> filteredEntitys = Lists.newArrayList();
254-
if (opInfo.getAccountType().equals(TenantUserTypeEnum.TENANT_ADMIN.getCode())) {
254+
if (TenantUserTypeEnum.TENANT_ADMIN.getCode().equals(opInfo.getAccountType())) {
255255
filteredEntitys.addAll(entityList);
256256
} else {
257257
Set<String> totalGroupIds = new HashSet<>();

‎inlong-manager/manager-service/src/main/java/org/apache/inlong/manager/service/user/UserService.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
import org.apache.inlong.manager.pojo.user.UserLoginRequest;
2323
import org.apache.inlong.manager.pojo.user.UserRequest;
2424

25+
import javax.annotation.Nullable;
26+
2527
/**
2628
* User service interface
2729
*/
@@ -91,4 +93,13 @@ public interface UserService {
9193
*/
9294
void checkUser(String inCharges, String user, String errMsg);
9395

96+
/**
97+
* Check whether the user is admin or in charge.
98+
* @param userInfo user info
99+
* @param username username, if it is null, will use userInfo.getName()
100+
* @param inCharges in charge list, if it is null, will ignore check in charge
101+
* @return true if the user is admin or in charge
102+
*/
103+
boolean isAdminOrInCharge(UserInfo userInfo, @Nullable String username, @Nullable String inCharges);
104+
94105
}

0 commit comments

Comments
 (0)