cherry-pick Fix-12832][API] Fix update worker group exception group name already exists. #12874
This commit is contained in:
Kerwin
zhuangchong
parent
0a88528deb
commit
c592bcfbb6
|
|
@ -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<WorkerGroup> 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<WorkerGroup> 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;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -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<String, Object> 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<String, Object> 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<WorkerGroup> workerGroupList = new ArrayList<WorkerGroup>();
|
||||
workerGroupList.add(getWorkerGroup(1));
|
||||
Mockito.when(workerGroupMapper.queryWorkerGroupByName(GROUP_NAME)).thenReturn(workerGroupList);
|
||||
|
||||
Map<String, Object> 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<String, String> serverMaps = new HashMap<>();
|
||||
serverMaps.put("localhost1:0000", "");
|
||||
Mockito.when(registryClient.getServerMaps(NodeType.WORKER)).thenReturn(serverMaps);
|
||||
|
||||
Map<String, Object> 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<String, String> serverMaps = new HashMap<>();
|
||||
serverMaps.put("localhost:0000", "");
|
||||
Mockito.when(registryClient.getServerMaps(NodeType.WORKER)).thenReturn(serverMaps);
|
||||
Mockito.when(workerGroupMapper.insert(any())).thenReturn(1);
|
||||
|
||||
Map<String, Object> 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<Integer> ids = new HashSet<>();
|
||||
ids.add(1);
|
||||
List<WorkerGroup> 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<String> 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<String, Object> result = workerGroupService.queryAllGroup(loginUSer);
|
||||
Map<String, Object> result = workerGroupService.queryAllGroup(getLoginUser());
|
||||
List<String> workerGroups = (List<String>) 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<String, Object> 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<String, Object> notExistResult = workerGroupService.deleteWorkerGroupById(loginUser, 1);
|
||||
Assertions.assertEquals(Status.DELETE_WORKER_GROUP_NOT_EXIST.getCode(),
|
||||
((Status) notExistResult.get(Constants.STATUS)).getCode());
|
||||
}
|
||||
|
||||
/**
|
||||
* get processInstances
|
||||
*/
|
||||
private List<ProcessInstance> getProcessInstanceList() {
|
||||
List<ProcessInstance> 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<ProcessInstance> processInstances = new ArrayList<ProcessInstance>();
|
||||
processInstances.add(processInstance);
|
||||
Mockito.when(processInstanceMapper.queryByWorkerGroupNameAndStatus(workerGroup.getName(),
|
||||
org.apache.dolphinscheduler.service.utils.Constants.NOT_TERMINATED_STATES))
|
||||
.thenReturn(processInstances);
|
||||
|
||||
Map<String, Object> 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<String, Object> successResult = workerGroupService.deleteWorkerGroupById(loginUser, 1);
|
||||
Assertions.assertEquals(Status.SUCCESS.getCode(),
|
||||
((Status) successResult.get(Constants.STATUS)).getCode());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testQueryAllGroupWithDefault() {
|
||||
Map<String, Object> result = workerGroupService.queryAllGroup(loginUSer);
|
||||
Map<String, Object> result = workerGroupService.queryAllGroup(getLoginUser());
|
||||
List<String> workerGroups = (List<String>) 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<WorkerGroup> getList() {
|
||||
List<WorkerGroup> list = new ArrayList<>();
|
||||
list.add(getWorkerGroup());
|
||||
return list;
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue