From d2fe16d252cd511e16049b6a88e4147bc9304ff6 Mon Sep 17 00:00:00 2001 From: WangJPLeo <103574007+WangJPLeo@users.noreply.github.com> Date: Tue, 5 Jul 2022 15:51:23 +0800 Subject: [PATCH] [Optimization] Config module resource relation fix. (#10718) * Config module resource relation fix. * Boolean judge fix * e2e rerun * e2e rerun * e2e rerun * ArrayList fix * change variable name. * Add transaction processing on create data resource method --- .../service/impl/AlertGroupServiceImpl.java | 3 ++ .../service/impl/DataSourceServiceImpl.java | 3 ++ .../service/impl/EnvironmentServiceImpl.java | 2 +- .../api/service/impl/ProjectServiceImpl.java | 2 ++ .../api/service/impl/QueueServiceImpl.java | 28 ++++++++++--------- .../service/impl/TaskGroupServiceImpl.java | 2 ++ .../api/service/impl/TenantServiceImpl.java | 2 ++ .../api/service/impl/UdfFuncServiceImpl.java | 1 + .../service/impl/WorkerGroupServiceImpl.java | 27 +++++++++--------- .../ResourcePermissionCheckServiceTest.java | 14 ++++++---- .../api/service/QueueServiceTest.java | 2 +- .../dolphinscheduler/common/Constants.java | 2 ++ .../dao/mapper/QueueMapper.java | 2 +- .../dao/mapper/QueueMapper.xml | 6 ++++ .../dao/mapper/QueueMapperTest.java | 7 ++--- 15 files changed, 63 insertions(+), 40 deletions(-) diff --git a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/AlertGroupServiceImpl.java b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/AlertGroupServiceImpl.java index 918fc7a680..74eb186a19 100644 --- a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/AlertGroupServiceImpl.java +++ b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/AlertGroupServiceImpl.java @@ -160,6 +160,7 @@ public class AlertGroupServiceImpl extends BaseServiceImpl implements AlertGroup * @return create result code */ @Override + @Transactional(rollbackFor = Exception.class) public Map createAlertgroup(User loginUser, String groupName, String desc, String alertInstanceIds) { Map result = new HashMap<>(); //only admin can operate @@ -191,6 +192,8 @@ public class AlertGroupServiceImpl extends BaseServiceImpl implements AlertGroup } catch (DuplicateKeyException ex) { logger.error("Create alert group error.", ex); putMsg(result, Status.ALERT_GROUP_EXIST); + } catch (RuntimeException e) { + throw new RuntimeException(e.getMessage()); } return result; diff --git a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/DataSourceServiceImpl.java b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/DataSourceServiceImpl.java index 0cea4cf5a6..b12d9e9904 100644 --- a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/DataSourceServiceImpl.java +++ b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/DataSourceServiceImpl.java @@ -100,6 +100,7 @@ public class DataSourceServiceImpl extends BaseServiceImpl implements DataSource * @return create result code */ @Override + @Transactional(rollbackFor = Exception.class) public Result createDataSource(User loginUser, BaseDataSourceParamDTO datasourceParam) { DataSourceUtils.checkDatasourceParam(datasourceParam); Result result = new Result<>(); @@ -139,6 +140,8 @@ public class DataSourceServiceImpl extends BaseServiceImpl implements DataSource } catch (DuplicateKeyException ex) { logger.error("Create datasource error.", ex); putMsg(result, Status.DATASOURCE_EXIST); + } catch (RuntimeException e) { + throw new RuntimeException(e.getMessage()); } return result; diff --git a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/EnvironmentServiceImpl.java b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/EnvironmentServiceImpl.java index 5ee992d16b..67def53674 100644 --- a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/EnvironmentServiceImpl.java +++ b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/EnvironmentServiceImpl.java @@ -92,8 +92,8 @@ public class EnvironmentServiceImpl extends BaseServiceImpl implements Environme * @param desc environment desc * @param workerGroups worker groups */ - @Transactional(rollbackFor = RuntimeException.class) @Override + @Transactional(rollbackFor = Exception.class) public Map createEnvironment(User loginUser, String name, String config, String desc, String workerGroups) { Map result = new HashMap<>(); if (!canOperatorPermissions(loginUser, null, AuthorizationType.ENVIRONMENT, ENVIRONMENT_CREATE)) { diff --git a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProjectServiceImpl.java b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProjectServiceImpl.java index b10ee78392..12ddafaa42 100644 --- a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProjectServiceImpl.java +++ b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProjectServiceImpl.java @@ -41,6 +41,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; import java.util.ArrayList; import java.util.Collections; @@ -86,6 +87,7 @@ public class ProjectServiceImpl extends BaseServiceImpl implements ProjectServic * @return returns an error if it exists */ @Override + @Transactional(rollbackFor = Exception.class) public Map createProject(User loginUser, String name, String desc) { Map result = checkDesc(desc); diff --git a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/QueueServiceImpl.java b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/QueueServiceImpl.java index 1301540a46..465f2243f4 100644 --- a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/QueueServiceImpl.java +++ b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/QueueServiceImpl.java @@ -17,22 +17,26 @@ package org.apache.dolphinscheduler.api.service.impl; +import com.baomidou.mybatisplus.core.metadata.IPage; +import com.baomidou.mybatisplus.extension.plugins.pagination.Page; +import org.apache.commons.lang3.StringUtils; import org.apache.dolphinscheduler.api.enums.Status; import org.apache.dolphinscheduler.api.service.QueueService; import org.apache.dolphinscheduler.api.utils.PageInfo; import org.apache.dolphinscheduler.api.utils.Result; import org.apache.dolphinscheduler.common.Constants; import org.apache.dolphinscheduler.common.enums.AuthorizationType; +import org.apache.dolphinscheduler.common.enums.UserType; import org.apache.dolphinscheduler.dao.entity.Queue; import org.apache.dolphinscheduler.dao.entity.User; import org.apache.dolphinscheduler.dao.mapper.QueueMapper; import org.apache.dolphinscheduler.dao.mapper.UserMapper; -import org.apache.commons.lang3.StringUtils; - +import java.util.ArrayList; import java.util.Collections; import java.util.Date; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -41,11 +45,10 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; -import com.baomidou.mybatisplus.core.metadata.IPage; -import com.baomidou.mybatisplus.extension.plugins.pagination.Page; - -import static org.apache.dolphinscheduler.api.constants.ApiFuncIdentificationConstant.*; +import static org.apache.dolphinscheduler.api.constants.ApiFuncIdentificationConstant.YARN_QUEUE_CREATE; +import static org.apache.dolphinscheduler.api.constants.ApiFuncIdentificationConstant.YARN_QUEUE_UPDATE; /** * queue service impl @@ -71,15 +74,13 @@ public class QueueServiceImpl extends BaseServiceImpl implements QueueService { public Map queryList(User loginUser) { Map result = new HashMap<>(); Set ids = resourcePermissionCheckService.userOwnedResourceIdsAcquisition(AuthorizationType.QUEUE, loginUser.getId(), logger); - if (ids.isEmpty()) { - result.put(Constants.DATA_LIST, Collections.emptyList()); - putMsg(result, Status.SUCCESS); - return result; + if (loginUser.getUserType().equals(UserType.GENERAL_USER)) { + ids = ids.isEmpty() ? new HashSet<>() : ids; + ids.add(Constants.DEFAULT_QUEUE_ID); } List queueList = queueMapper.selectBatchIds(ids); result.put(Constants.DATA_LIST, queueList); putMsg(result, Status.SUCCESS); - return result; } @@ -103,7 +104,7 @@ public class QueueServiceImpl extends BaseServiceImpl implements QueueService { return result; } Page page = new Page<>(pageNo, pageSize); - IPage queueList = queueMapper.queryQueuePaging(page, searchVal); + IPage queueList = queueMapper.queryQueuePaging(page, new ArrayList<>(ids), searchVal); Integer count = (int) queueList.getTotal(); pageInfo.setTotal(count); pageInfo.setTotalList(queueList.getRecords()); @@ -122,6 +123,7 @@ public class QueueServiceImpl extends BaseServiceImpl implements QueueService { * @return create result */ @Override + @Transactional(rollbackFor = Exception.class) public Map createQueue(User loginUser, String queue, String queueName) { Map result = new HashMap<>(); if (!canOperatorPermissions(loginUser,null, AuthorizationType.QUEUE,YARN_QUEUE_CREATE)) { @@ -160,7 +162,7 @@ public class QueueServiceImpl extends BaseServiceImpl implements QueueService { queueMapper.insert(queueObj); result.put(Constants.DATA_LIST, queueObj); putMsg(result, Status.SUCCESS); - + permissionPostHandle(AuthorizationType.QUEUE, loginUser.getId(), Collections.singletonList(queueObj.getId()), logger); return result; } diff --git a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/TaskGroupServiceImpl.java b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/TaskGroupServiceImpl.java index e02fbca658..7d8d1128d9 100644 --- a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/TaskGroupServiceImpl.java +++ b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/TaskGroupServiceImpl.java @@ -39,6 +39,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; import java.util.ArrayList; import java.util.Collections; @@ -78,6 +79,7 @@ public class TaskGroupServiceImpl extends BaseServiceImpl implements TaskGroupSe * @return the result code and msg */ @Override + @Transactional(rollbackFor = Exception.class) public Map createTaskGroup(User loginUser, Long projectCode, String name, String description, int groupSize) { Map result = new HashMap<>(); diff --git a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/TenantServiceImpl.java b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/TenantServiceImpl.java index a3c4e30ef6..3d315ee3cf 100644 --- a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/TenantServiceImpl.java +++ b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/TenantServiceImpl.java @@ -46,6 +46,7 @@ import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.Date; import java.util.HashMap; @@ -132,6 +133,7 @@ public class TenantServiceImpl extends BaseServiceImpl implements TenantService if (PropertyUtils.getResUploadStartupState()) { storageOperate.createTenantDirIfNotExists(tenantCode); } + permissionPostHandle(AuthorizationType.TENANT, loginUser.getId(), Collections.singletonList(tenant.getId()), logger); result.put(Constants.DATA_LIST, tenant); putMsg(result, Status.SUCCESS); return result; diff --git a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/UdfFuncServiceImpl.java b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/UdfFuncServiceImpl.java index 010629756e..4542b648b8 100644 --- a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/UdfFuncServiceImpl.java +++ b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/UdfFuncServiceImpl.java @@ -77,6 +77,7 @@ public class UdfFuncServiceImpl extends BaseServiceImpl implements UdfFuncServic * @return create result code */ @Override + @Transactional(rollbackFor = Exception.class) public Result createUdfFunction(User loginUser, String funcName, String className, diff --git a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/WorkerGroupServiceImpl.java b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/WorkerGroupServiceImpl.java index bf678d3dcc..7878d94220 100644 --- a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/WorkerGroupServiceImpl.java +++ b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/WorkerGroupServiceImpl.java @@ -84,6 +84,7 @@ public class WorkerGroupServiceImpl extends BaseServiceImpl implements WorkerGro * @return create or update result code */ @Override + @Transactional(rollbackFor = Exception.class) public Map saveWorkerGroup(User loginUser, int id, String name, String addrList) { Map result = new HashMap<>(); if (!canOperatorPermissions(loginUser,null, AuthorizationType.WORKER_GROUP, WORKER_GROUP_CREATE)) { @@ -191,14 +192,12 @@ public class WorkerGroupServiceImpl extends BaseServiceImpl implements WorkerGro int toIndex = (pageNo - 1) * pageSize + pageSize; Result result = new Result(); - List workerGroups = new ArrayList<>(); + List workerGroups; if (loginUser.getUserType().equals(UserType.ADMIN_USER)) { - workerGroups = getWorkerGroups(true); + workerGroups = getWorkerGroups(true, null); } else { Set ids = resourcePermissionCheckService.userOwnedResourceIdsAcquisition(AuthorizationType.WORKER_GROUP, loginUser.getId(), logger); - if (!ids.isEmpty()) { - workerGroups = workerGroupMapper.selectBatchIds(ids); - } + workerGroups = getWorkerGroups(true, ids.isEmpty() ? Collections.emptyList() : new ArrayList<>(ids)); } List resultDataList = new ArrayList<>(); int total = 0; @@ -244,15 +243,10 @@ public class WorkerGroupServiceImpl extends BaseServiceImpl implements WorkerGro Map result = new HashMap<>(); List workerGroups; if (loginUser.getUserType().equals(UserType.ADMIN_USER)) { - workerGroups = getWorkerGroups(false); + workerGroups = getWorkerGroups(false, null); } else { Set ids = resourcePermissionCheckService.userOwnedResourceIdsAcquisition(AuthorizationType.WORKER_GROUP, loginUser.getId(), logger); - if (ids.isEmpty()) { - result.put(Constants.DATA_LIST, Collections.emptyList()); - putMsg(result, Status.SUCCESS); - return result; - } - workerGroups = workerGroupMapper.selectBatchIds(ids); + workerGroups = getWorkerGroups(false, ids.isEmpty() ? Collections.emptyList() : new ArrayList<>(ids)); } List availableWorkerGroupList = workerGroups.stream() .map(WorkerGroup::getName) @@ -273,9 +267,14 @@ public class WorkerGroupServiceImpl extends BaseServiceImpl implements WorkerGro * @param isPaging whether paging * @return WorkerGroup list */ - private List getWorkerGroups(boolean isPaging) { + private List getWorkerGroups(boolean isPaging, List ids) { // worker groups from database - List workerGroups = workerGroupMapper.queryAllWorkerGroup(); + List workerGroups; + if (ids != null) { + workerGroups = ids.isEmpty() ? new ArrayList<>() : workerGroupMapper.selectBatchIds(ids); + } else { + workerGroups = workerGroupMapper.queryAllWorkerGroup(); + } // worker groups from zookeeper String workerPath = Constants.REGISTRY_DOLPHINSCHEDULER_WORKERS; Collection workerGroupList = null; diff --git a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/permission/ResourcePermissionCheckServiceTest.java b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/permission/ResourcePermissionCheckServiceTest.java index cf6b4e71cc..68e5e74196 100644 --- a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/permission/ResourcePermissionCheckServiceTest.java +++ b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/permission/ResourcePermissionCheckServiceTest.java @@ -17,13 +17,19 @@ package org.apache.dolphinscheduler.api.permission; -import com.google.common.collect.Lists; import org.apache.dolphinscheduler.common.enums.AuthorizationType; import org.apache.dolphinscheduler.common.enums.UserType; import org.apache.dolphinscheduler.dao.entity.Project; import org.apache.dolphinscheduler.dao.entity.User; import org.apache.dolphinscheduler.dao.mapper.ProjectMapper; import org.apache.dolphinscheduler.service.process.ProcessService; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; + import org.junit.Assert; import org.junit.Test; import org.junit.runner.RunWith; @@ -34,11 +40,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.context.ApplicationContext; -import java.util.ArrayList; -import java.util.List; -import java.util.Map; -import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; +import com.google.common.collect.Lists; /** * permission service test diff --git a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/QueueServiceTest.java b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/QueueServiceTest.java index a2a7957a23..30eaeb52e1 100644 --- a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/QueueServiceTest.java +++ b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/QueueServiceTest.java @@ -110,7 +110,7 @@ public class QueueServiceTest { Set ids = new HashSet<>(); ids.add(1); Mockito.when(resourcePermissionCheckService.userOwnedResourceIdsAcquisition(AuthorizationType.QUEUE, getLoginUser().getId(), queueServiceImplLogger)).thenReturn(ids); - Mockito.when(queueMapper.queryQueuePaging(Mockito.any(Page.class), Mockito.eq(queueName))).thenReturn(page); + Mockito.when(queueMapper.queryQueuePaging(Mockito.any(Page.class), Mockito.anyList(), Mockito.eq(queueName))).thenReturn(page); Result result = queueService.queryList(getLoginUser(), queueName, 1, 10); logger.info(result.toString()); PageInfo pageInfo = (PageInfo) result.getData(); diff --git a/dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/Constants.java b/dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/Constants.java index a081570f4a..ea54e362cc 100644 --- a/dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/Constants.java +++ b/dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/Constants.java @@ -833,4 +833,6 @@ public final class Constants { public static final int USER_PASSWORD_MIN_LENGTH = 2; public static final String FUNCTION_START_WITH = "$"; + + public static final Integer DEFAULT_QUEUE_ID = 1; } diff --git a/dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/QueueMapper.java b/dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/QueueMapper.java index e48607007d..8a3942a738 100644 --- a/dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/QueueMapper.java +++ b/dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/QueueMapper.java @@ -34,7 +34,7 @@ public interface QueueMapper extends BaseMapper { * @param searchVal searchVal * @return queue IPage */ - IPage queryQueuePaging(IPage page, + IPage queryQueuePaging(IPage page, @Param("ids")List ids, @Param("searchVal") String searchVal); /** diff --git a/dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/QueueMapper.xml b/dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/QueueMapper.xml index cf381e494a..3705365543 100644 --- a/dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/QueueMapper.xml +++ b/dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/QueueMapper.xml @@ -29,6 +29,12 @@ and queue_name like concat('%', #{searchVal}, '%') + + and id in + + #{i} + + order by update_time desc