Skip to content

Commit 2a6a24d

Browse files
committed
[Fix-17604] correct logic for assigning and remove worker groups to project
1 parent 2b86561 commit 2a6a24d

File tree

2 files changed

+25
-12
lines changed

2 files changed

+25
-12
lines changed

dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProjectWorkerGroupRelationServiceImpl.java

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -125,26 +125,32 @@ public Result assignWorkerGroupsToProject(User loginUser, Long projectCode, List
125125
// check if assign worker group exists in the project
126126
Set<String> projectWorkerGroupNames =
127127
projectWorkerGroupDao.queryAssignedWorkerGroupNamesByProjectCode(projectCode);
128-
difference = SetUtils.difference(unauthorizedWorkerGroupNames, projectWorkerGroupNames);
128+
Set<String> needDeletedWorkerGroups =
129+
SetUtils.difference(projectWorkerGroupNames, unauthorizedWorkerGroupNames);
129130
Date now = new Date();
130-
if (CollectionUtils.isNotEmpty(difference)) {
131-
Set<String> usedWorkerGroups = getAllUsedWorkerGroups(project);
132131

133-
if (CollectionUtils.isNotEmpty(usedWorkerGroups) && usedWorkerGroups.containsAll(difference)) {
132+
if (CollectionUtils.isNotEmpty(needDeletedWorkerGroups)) {
133+
Set<String> usedWorkerGroups = getAllUsedWorkerGroups(project);
134+
if (CollectionUtils.isNotEmpty(usedWorkerGroups) && usedWorkerGroups.containsAll(needDeletedWorkerGroups)) {
134135
throw new ServiceException(Status.USED_WORKER_GROUP_EXISTS,
135-
SetUtils.intersection(usedWorkerGroups, difference).toSet());
136+
SetUtils.intersection(usedWorkerGroups, needDeletedWorkerGroups).toSet());
136137
}
137-
138138
boolean deleted =
139-
projectWorkerGroupDao.deleteByProjectCodeAndWorkerGroups(projectCode, new ArrayList<>(difference));
139+
projectWorkerGroupDao.deleteByProjectCodeAndWorkerGroups(projectCode,
140+
new ArrayList<>(needDeletedWorkerGroups));
140141
if (deleted) {
141-
log.info("Success to delete worker groups [{}] for the project [{}] .", difference, project.getName());
142+
log.info("Success to delete worker groups [{}] for the project [{}] .", needDeletedWorkerGroups,
143+
project.getName());
142144
} else {
143-
log.error("Failed to delete worker groups [{}] for the project [{}].", difference, project.getName());
145+
log.error("Failed to delete worker groups [{}] for the project [{}].", needDeletedWorkerGroups,
146+
project.getName());
144147
throw new ServiceException(Status.ASSIGN_WORKER_GROUP_TO_PROJECT_ERROR);
145148
}
146-
147-
difference.forEach(workerGroupName -> {
149+
}
150+
Set<String> needAssignedWorkerGroups =
151+
SetUtils.difference(unauthorizedWorkerGroupNames, projectWorkerGroupNames);
152+
if (CollectionUtils.isNotEmpty(needAssignedWorkerGroups)) {
153+
needAssignedWorkerGroups.forEach(workerGroupName -> {
148154
ProjectWorkerGroup projectWorkerGroup = new ProjectWorkerGroup();
149155
projectWorkerGroup.setProjectCode(projectCode);
150156
projectWorkerGroup.setWorkerGroup(workerGroupName);

dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/ProjectWorkerGroupRelationServiceTest.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,11 +146,14 @@ public void testAssignWorkerGroupsToProject() {
146146
getWorkerGroups()));
147147

148148
// fail when wg is referenced by task definition
149+
// test case: project all wg: test, task used wg: test, new wg: test1
149150
Mockito.when(taskDefinitionDao.queryAllTaskDefinitionWorkerGroups(Mockito.anyLong()))
150151
.thenReturn(Collections.singletonList(getProjectWorkerGroup().getWorkerGroup()));
152+
Mockito.when(projectWorkerGroupDao.queryAssignedWorkerGroupNamesByProjectCode(Mockito.any()))
153+
.thenReturn(Sets.newHashSet(getProjectWorkerGroup().getWorkerGroup()));
151154
AssertionsHelper.assertThrowsServiceException(Status.USED_WORKER_GROUP_EXISTS,
152155
() -> projectWorkerGroupRelationService.assignWorkerGroupsToProject(loginUser, projectCode,
153-
getWorkerGroups()));
156+
getUnusedWorkerGroups()));
154157
}
155158

156159
@Test
@@ -192,6 +195,10 @@ private List<String> getWorkerGroups() {
192195
return Lists.newArrayList("test");
193196
}
194197

198+
private List<String> getUnusedWorkerGroups() {
199+
return Lists.newArrayList("test1");
200+
}
201+
195202
private List<String> getDiffWorkerGroups() {
196203
return Lists.newArrayList("test3", "new");
197204
}

0 commit comments

Comments
 (0)