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 0c1f04d235..285c5e82eb 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 @@ -102,15 +102,11 @@ public class WorkerGroupServiceImpl extends BaseServiceImpl implements WorkerGro return result; } Date now = new Date(); - WorkerGroup workerGroup; + WorkerGroup workerGroup = null; if (id != 0) { workerGroup = workerGroupMapper.selectById(id); - // check exist - if (workerGroup == null) { - workerGroup = new WorkerGroup(); - workerGroup.setCreateTime(now); - } - } else { + } + if (workerGroup == null) { workerGroup = new WorkerGroup(); workerGroup.setCreateTime(now); } @@ -151,23 +147,21 @@ public class WorkerGroupServiceImpl extends BaseServiceImpl implements WorkerGro * @return boolean */ private boolean checkWorkerGroupNameExists(WorkerGroup workerGroup) { + // check database List workerGroupList = workerGroupMapper.queryWorkerGroupByName(workerGroup.getName()); if (CollectionUtils.isNotEmpty(workerGroupList)) { - // new group has same name + // create group, the same group name exists in the database if (workerGroup.getId() == null) { return true; } - // check group id - for (WorkerGroup group : workerGroupList) { - if (Objects.equals(group.getId(), workerGroup.getId())) { - return true; - } + // update group, the database exists with the same group name except itself + Optional sameNameWorkGroupOptional = workerGroupList.stream() + .filter(group -> !Objects.equals(group.getId(), workerGroup.getId())).findFirst(); + if (sameNameWorkGroupOptional.isPresent()) { + return true; } } - // check zookeeper - String workerGroupPath = - Constants.REGISTRY_DOLPHINSCHEDULER_WORKERS + Constants.SINGLE_SLASH + workerGroup.getName(); - return registryClient.exists(workerGroupPath); + return false; } /** diff --git a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/WorkerGroupServiceTest.java b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/WorkerGroupServiceTest.java index 7b199183a6..a219a40188 100644 --- a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/WorkerGroupServiceTest.java +++ b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/WorkerGroupServiceTest.java @@ -17,11 +17,18 @@ package org.apache.dolphinscheduler.api.service; -import org.apache.dolphinscheduler.api.ApiApplicationServer; +import static org.apache.dolphinscheduler.api.constants.ApiFuncIdentificationConstant.WORKER_GROUP_CREATE; +import static org.apache.dolphinscheduler.api.constants.ApiFuncIdentificationConstant.WORKER_GROUP_DELETE; +import static org.mockito.ArgumentMatchers.any; + import org.apache.dolphinscheduler.api.enums.Status; +import org.apache.dolphinscheduler.api.permission.ResourcePermissionCheckService; +import org.apache.dolphinscheduler.api.service.impl.BaseServiceImpl; import org.apache.dolphinscheduler.api.service.impl.WorkerGroupServiceImpl; +import org.apache.dolphinscheduler.api.utils.Result; import org.apache.dolphinscheduler.common.constants.Constants; -import org.apache.dolphinscheduler.common.enums.ProfileType; +import org.apache.dolphinscheduler.common.enums.AuthorizationType; +import org.apache.dolphinscheduler.common.enums.NodeType; import org.apache.dolphinscheduler.common.enums.UserType; import org.apache.dolphinscheduler.dao.entity.ProcessInstance; import org.apache.dolphinscheduler.dao.entity.User; @@ -32,95 +39,235 @@ import org.apache.dolphinscheduler.dao.mapper.WorkerGroupMapper; import org.apache.dolphinscheduler.service.registry.RegistryClient; import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; -import org.junit.Assert; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.Mockito; -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.boot.test.context.SpringBootTest; -import org.springframework.boot.test.mock.mockito.MockBean; -import org.springframework.test.context.ActiveProfiles; -import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import org.mockito.junit.jupiter.MockitoExtension; +import org.mockito.junit.jupiter.MockitoSettings; +import org.mockito.quality.Strictness; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; -@ActiveProfiles(value = {ProfileType.H2}) -@RunWith(SpringJUnit4ClassRunner.class) -@SpringBootTest(classes = ApiApplicationServer.class) +@ExtendWith(MockitoExtension.class) +@MockitoSettings(strictness = Strictness.LENIENT) public class WorkerGroupServiceTest { - @MockBean(name = "registryClient") - private RegistryClient registryClient; + private static final Logger logger = LoggerFactory.getLogger(WorkerGroupServiceTest.class); - @Autowired + private static final Logger baseServiceLogger = LoggerFactory.getLogger(BaseServiceImpl.class); + + private static final Logger serviceLogger = LoggerFactory.getLogger(WorkerGroupService.class); + + @InjectMocks private WorkerGroupServiceImpl workerGroupService; - @MockBean(name = "workerGroupMapper") + @Mock private WorkerGroupMapper workerGroupMapper; - @MockBean(name = "processInstanceMapper") + @Mock private ProcessInstanceMapper processInstanceMapper; - private String groupName = "groupName000001"; + @Mock + private RegistryClient registryClient; + + @Mock + private ResourcePermissionCheckService resourcePermissionCheckService; - private User loginUSer; @Mock private EnvironmentWorkerGroupRelationMapper environmentWorkerGroupRelationMapper; - @Before - public void init(){ - loginUSer = new User(); - loginUSer.setUserType(UserType.ADMIN_USER); + private final String GROUP_NAME = "testWorkerGroup"; + + private User getLoginUser() { + User loginUser = new User(); + loginUser.setUserType(UserType.GENERAL_USER); + loginUser.setUserName("workerGroupTestUser"); + loginUser.setId(1); + return loginUser; + } + + @Test + public void giveNoPermission_whenSaveWorkerGroup_expectNoOperation() { + User loginUser = getLoginUser(); + Mockito.when(resourcePermissionCheckService.operationPermissionCheck(AuthorizationType.WORKER_GROUP, null, loginUser.getId(), + WORKER_GROUP_CREATE, baseServiceLogger)).thenReturn(false); + Mockito.when(resourcePermissionCheckService.resourcePermissionCheck(AuthorizationType.WORKER_GROUP, null, 1, + baseServiceLogger)).thenReturn(false); + Map result = + workerGroupService.saveWorkerGroup(loginUser, 1, GROUP_NAME, "localhost:0000", "test group", ""); + Assertions.assertEquals(Status.USER_NO_OPERATION_PERM.getCode(), + ((Status) result.get(Constants.STATUS)).getCode()); + } + + @Test + public void giveNullName_whenSaveWorkerGroup_expectNAME_NULL() { + User loginUser = getLoginUser(); + Mockito.when(resourcePermissionCheckService.operationPermissionCheck(AuthorizationType.WORKER_GROUP, null, loginUser.getId(), + WORKER_GROUP_CREATE, baseServiceLogger)).thenReturn(true); + Mockito.when(resourcePermissionCheckService.resourcePermissionCheck(AuthorizationType.WORKER_GROUP, null, 1, + baseServiceLogger)).thenReturn(true); + Map result = + workerGroupService.saveWorkerGroup(loginUser, 1, "", "localhost:0000", "test group", ""); + Assertions.assertEquals(Status.NAME_NULL.getCode(), + ((Status) result.get(Constants.STATUS)).getCode()); + } + + @Test + public void giveSameUserName_whenSaveWorkerGroup_expectNAME_EXIST() { + User loginUser = getLoginUser(); + Mockito.when(resourcePermissionCheckService.operationPermissionCheck(AuthorizationType.WORKER_GROUP, null, loginUser.getId(), + WORKER_GROUP_CREATE, baseServiceLogger)).thenReturn(true); + Mockito.when(resourcePermissionCheckService.resourcePermissionCheck(AuthorizationType.WORKER_GROUP, null, 1, + baseServiceLogger)).thenReturn(true); + Mockito.when(workerGroupMapper.selectById(1)).thenReturn(null); + List workerGroupList = new ArrayList(); + workerGroupList.add(getWorkerGroup(1)); + Mockito.when(workerGroupMapper.queryWorkerGroupByName(GROUP_NAME)).thenReturn(workerGroupList); + + Map result = + workerGroupService.saveWorkerGroup(loginUser, 1, GROUP_NAME, "localhost:0000", "test group", ""); + Assertions.assertEquals(Status.NAME_EXIST.getCode(), + ((Status) result.get(Constants.STATUS)).getCode()); + } + + @Test + public void giveInvalidAddress_whenSaveWorkerGroup_expectADDRESS_INVALID() { + User loginUser = getLoginUser(); + Mockito.when(resourcePermissionCheckService.operationPermissionCheck(AuthorizationType.WORKER_GROUP, null, loginUser.getId(), + WORKER_GROUP_CREATE, baseServiceLogger)).thenReturn(true); + Mockito.when(resourcePermissionCheckService.resourcePermissionCheck(AuthorizationType.WORKER_GROUP, null, 1, + baseServiceLogger)).thenReturn(true); + Mockito.when(workerGroupMapper.selectById(1)).thenReturn(null); + Mockito.when(workerGroupMapper.queryWorkerGroupByName(GROUP_NAME)).thenReturn(null); + Map serverMaps = new HashMap<>(); + serverMaps.put("localhost1:0000", ""); + Mockito.when(registryClient.getServerMaps(NodeType.WORKER)).thenReturn(serverMaps); + + Map result = + workerGroupService.saveWorkerGroup(loginUser, 1, GROUP_NAME, "localhost:0000", "test group", ""); + Assertions.assertEquals(Status.WORKER_ADDRESS_INVALID.getCode(), + ((Status) result.get(Constants.STATUS)).getCode()); + } + + @Test + public void giveValidWorkerGroup_whenSaveWorkerGroup_expectSuccess() { + User loginUser = getLoginUser(); + Mockito.when(resourcePermissionCheckService.operationPermissionCheck(AuthorizationType.WORKER_GROUP, null, loginUser.getId(), + WORKER_GROUP_CREATE, baseServiceLogger)).thenReturn(true); + Mockito.when(resourcePermissionCheckService.resourcePermissionCheck(AuthorizationType.WORKER_GROUP, null, 1, + baseServiceLogger)).thenReturn(true); + + Mockito.when(workerGroupMapper.selectById(1)).thenReturn(null); + Mockito.when(workerGroupMapper.queryWorkerGroupByName(GROUP_NAME)).thenReturn(null); + Map serverMaps = new HashMap<>(); + serverMaps.put("localhost:0000", ""); + Mockito.when(registryClient.getServerMaps(NodeType.WORKER)).thenReturn(serverMaps); + Mockito.when(workerGroupMapper.insert(any())).thenReturn(1); + + Map result = + workerGroupService.saveWorkerGroup(loginUser, 1, GROUP_NAME, "localhost:0000", "test group", ""); + Assertions.assertEquals(Status.SUCCESS.getCode(), + ((Status) result.get(Constants.STATUS)).getCode()); + } + + @Test + public void giveValidParams_whenQueryAllGroupPaging_expectSuccess() { + User loginUser = getLoginUser(); + Set ids = new HashSet<>(); + ids.add(1); + List workerGroups = new ArrayList<>(); + workerGroups.add(getWorkerGroup(1)); + Mockito.when(resourcePermissionCheckService.userOwnedResourceIdsAcquisition(AuthorizationType.WORKER_GROUP, + loginUser.getId(), serviceLogger)).thenReturn(ids); + Mockito.when(workerGroupMapper.selectBatchIds(ids)).thenReturn(workerGroups); + Set activeWorkerNodes = new HashSet<>(); + activeWorkerNodes.add("localhost:12345"); + activeWorkerNodes.add("localhost:23456"); + Mockito.when(registryClient.getServerNodeSet(NodeType.WORKER)).thenReturn(activeWorkerNodes); + + Result result = workerGroupService.queryAllGroupPaging(loginUser, 1, 1, null); + Assertions.assertEquals(result.getCode(), Status.SUCCESS.getCode()); } @Test public void testQueryAllGroup() { - Map result = workerGroupService.queryAllGroup(loginUSer); + Map result = workerGroupService.queryAllGroup(getLoginUser()); List workerGroups = (List) result.get(Constants.DATA_LIST); - Assert.assertEquals(workerGroups.size(), 1); + Assertions.assertEquals(workerGroups.size(), 1); } - /** - * delete group by id - */ @Test - public void testDeleteWorkerGroupById() { - User user = new User(); - user.setId(1); - user.setUserType(UserType.ADMIN_USER); - WorkerGroup wg2 = getWorkerGroup(2); - Mockito.when(workerGroupMapper.selectById(2)).thenReturn(wg2); - Mockito.when(processInstanceMapper.queryByWorkerGroupNameAndStatus(wg2.getName(), org.apache.dolphinscheduler.service.utils.Constants.NOT_TERMINATED_STATES)).thenReturn(getProcessInstanceList()); - Map result = workerGroupService.deleteWorkerGroupById(user, 1); - Assert.assertEquals(Status.DELETE_WORKER_GROUP_NOT_EXIST.getCode(), ((Status) result.get(Constants.STATUS)).getCode()); - result = workerGroupService.deleteWorkerGroupById(user, 2); - Assert.assertEquals(Status.DELETE_WORKER_GROUP_BY_ID_FAIL.getCode(), ((Status) result.get(Constants.STATUS)).getCode()); - // correct - WorkerGroup wg3 = getWorkerGroup(3); - Mockito.when(workerGroupMapper.selectById(3)).thenReturn(wg3); - Mockito.when(processInstanceMapper.queryByWorkerGroupNameAndStatus(wg3.getName(), org.apache.dolphinscheduler.service.utils.Constants.NOT_TERMINATED_STATES)).thenReturn(new ArrayList<>()); - result = workerGroupService.deleteWorkerGroupById(user, 3); - Assert.assertEquals(Status.SUCCESS.getMsg(), result.get(Constants.MSG)); + public void giveNotExistsWorkerGroup_whenDeleteWorkerGroupById_expectNotExists() { + User loginUser = getLoginUser(); + Mockito.when(resourcePermissionCheckService.operationPermissionCheck(AuthorizationType.WORKER_GROUP, null, loginUser.getId(), + WORKER_GROUP_DELETE, baseServiceLogger)).thenReturn(true); + Mockito.when(resourcePermissionCheckService.resourcePermissionCheck(AuthorizationType.WORKER_GROUP, null, 1, + baseServiceLogger)).thenReturn(true); + Mockito.when(workerGroupMapper.selectById(1)).thenReturn(null); + + Map notExistResult = workerGroupService.deleteWorkerGroupById(loginUser, 1); + Assertions.assertEquals(Status.DELETE_WORKER_GROUP_NOT_EXIST.getCode(), + ((Status) notExistResult.get(Constants.STATUS)).getCode()); } - /** - * get processInstances - */ - private List getProcessInstanceList() { - List processInstances = new ArrayList<>(); - processInstances.add(new ProcessInstance()); - return processInstances; + @Test + public void giveRunningProcess_whenDeleteWorkerGroupById_expectFailed() { + User loginUser = getLoginUser(); + Mockito.when(resourcePermissionCheckService.operationPermissionCheck(AuthorizationType.WORKER_GROUP, null, loginUser.getId(), + WORKER_GROUP_DELETE, baseServiceLogger)).thenReturn(true); + Mockito.when(resourcePermissionCheckService.resourcePermissionCheck(AuthorizationType.WORKER_GROUP, null, 1, + baseServiceLogger)).thenReturn(true); + WorkerGroup workerGroup = getWorkerGroup(1); + Mockito.when(workerGroupMapper.selectById(1)).thenReturn(workerGroup); + ProcessInstance processInstance = new ProcessInstance(); + processInstance.setId(1); + List processInstances = new ArrayList(); + processInstances.add(processInstance); + Mockito.when(processInstanceMapper.queryByWorkerGroupNameAndStatus(workerGroup.getName(), + org.apache.dolphinscheduler.service.utils.Constants.NOT_TERMINATED_STATES)) + .thenReturn(processInstances); + + Map deleteFailed = workerGroupService.deleteWorkerGroupById(loginUser, 1); + Assertions.assertEquals(Status.DELETE_WORKER_GROUP_BY_ID_FAIL.getCode(), + ((Status) deleteFailed.get(Constants.STATUS)).getCode()); + } + + @Test + public void giveValidParams_whenDeleteWorkerGroupById_expectSuccess() { + User loginUser = getLoginUser(); + Mockito.when(resourcePermissionCheckService.operationPermissionCheck(AuthorizationType.WORKER_GROUP, null, loginUser.getId(), + WORKER_GROUP_DELETE, baseServiceLogger)).thenReturn(true); + Mockito.when(resourcePermissionCheckService.resourcePermissionCheck(AuthorizationType.WORKER_GROUP, null, 1, + baseServiceLogger)).thenReturn(true); + WorkerGroup workerGroup = getWorkerGroup(1); + Mockito.when(workerGroupMapper.selectById(1)).thenReturn(workerGroup); + Mockito.when(processInstanceMapper.queryByWorkerGroupNameAndStatus(workerGroup.getName(), + org.apache.dolphinscheduler.service.utils.Constants.NOT_TERMINATED_STATES)).thenReturn(null); + Mockito.when(workerGroupMapper.deleteById(1)).thenReturn(1); + Mockito.when(processInstanceMapper.updateProcessInstanceByWorkerGroupName(workerGroup.getName(), "")) + .thenReturn(1); + Mockito.when(environmentWorkerGroupRelationMapper.queryByWorkerGroupName(workerGroup.getName())) + .thenReturn(null); + Map successResult = workerGroupService.deleteWorkerGroupById(loginUser, 1); + Assertions.assertEquals(Status.SUCCESS.getCode(), + ((Status) successResult.get(Constants.STATUS)).getCode()); } @Test public void testQueryAllGroupWithDefault() { - Map result = workerGroupService.queryAllGroup(loginUSer); + Map result = workerGroupService.queryAllGroup(getLoginUser()); List workerGroups = (List) result.get(Constants.DATA_LIST); - Assert.assertEquals(1, workerGroups.size()); - Assert.assertEquals("default", workerGroups.toArray()[0]); + Assertions.assertEquals(1, workerGroups.size()); + Assertions.assertEquals("default", workerGroups.toArray()[0]); } /** @@ -128,19 +275,9 @@ public class WorkerGroupServiceTest { */ private WorkerGroup getWorkerGroup(int id) { WorkerGroup workerGroup = new WorkerGroup(); - workerGroup.setName(groupName); + workerGroup.setName(GROUP_NAME); workerGroup.setId(id); return workerGroup; } - private WorkerGroup getWorkerGroup() { - return getWorkerGroup(1); - } - - private List getList() { - List list = new ArrayList<>(); - list.add(getWorkerGroup()); - return list; - } - }