caishunfeng commented on code in PR #12142:
URL: https://github.com/apache/dolphinscheduler/pull/12142#discussion_r980648004
##########
dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/process/ProcessServiceImpl.java:
##########
@@ -2565,7 +2563,7 @@ public int saveProcessDefine(User operator,
ProcessDefinition processDefinition,
@Override
public int saveTaskRelation(User operator, long projectCode, long
processDefinitionCode, int processDefinitionVersion,
List<ProcessTaskRelationLog> taskRelationList,
List<TaskDefinitionLog> taskDefinitionLogs,
- Boolean syncDefine) {
+ Boolean syncDefine, boolean isDelete) {
Review Comment:
We should avoid this way, because `saveTaskRelation` is already a complex
function, expanding the `isDelete` model will exponentially increase the
maintenance cost.
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/TaskDefinitionServiceImpl.java:
##########
@@ -506,28 +516,43 @@ public Map<String, Object> updateTaskWithUpstream(User
loginUser, long projectCo
ProcessTaskRelation taskRelation = upstreamTaskRelations.get(0);
List<ProcessTaskRelation> processTaskRelations =
processTaskRelationMapper.queryByProcessCode(projectCode,
taskRelation.getProcessDefinitionCode());
List<ProcessTaskRelation> processTaskRelationList =
Lists.newArrayList(processTaskRelations);
- List<ProcessTaskRelation> relationList = Lists.newArrayList();
for (ProcessTaskRelation processTaskRelation :
processTaskRelationList) {
if (processTaskRelation.getPostTaskCode() == taskCode) {
if
(queryUpStreamTaskCodeMap.containsKey(processTaskRelation.getPreTaskCode()) &&
processTaskRelation.getPreTaskCode() != 0L) {
queryUpStreamTaskCodeMap.remove(processTaskRelation.getPreTaskCode());
} else {
processTaskRelation.setPreTaskCode(0L);
processTaskRelation.setPreTaskVersion(0);
- relationList.add(processTaskRelation);
}
}
}
- processTaskRelationList.removeAll(relationList);
for (Map.Entry<Long, TaskDefinition> queryUpStreamTask :
queryUpStreamTaskCodeMap.entrySet()) {
- taskRelation.setPreTaskCode(queryUpStreamTask.getKey());
-
taskRelation.setPreTaskVersion(queryUpStreamTask.getValue().getVersion());
- processTaskRelationList.add(taskRelation);
+ ProcessTaskRelation processTaskRelation = new
ProcessTaskRelation();
+ processTaskRelation.setPreTaskCode(queryUpStreamTask.getKey());
+
processTaskRelation.setPreTaskVersion(queryUpStreamTask.getValue().getVersion());
+ processTaskRelation.setPostTaskCode(taskCode);
+
processTaskRelation.setPostTaskVersion(taskDefinitionToUpdate.getVersion());
+ processTaskRelation.setConditionType(ConditionType.NONE);
+ processTaskRelation.setConditionParams("{}");
+ processTaskRelationList.add(processTaskRelation);
}
if (queryUpStreamTaskCodeMap.isEmpty() &&
!processTaskRelationList.isEmpty()) {
processTaskRelationList.add(processTaskRelationList.get(0));
}
- updateDag(loginUser, result,
taskRelation.getProcessDefinitionCode(), processTaskRelations,
Lists.newArrayList(taskDefinitionToUpdate));
+ processTaskRelationList.sort((p1, p2) ->
Long.compare(p2.getPreTaskCode(), p1.getPreTaskCode()));
+ boolean sign = false;
+ for (int i = 0; i < processTaskRelationList.size(); i++) {
+ ProcessTaskRelation processTaskRelation =
processTaskRelationList.get(i);
+ if (processTaskRelation.getPostTaskCode() == taskCode) {
+ if (processTaskRelation.getPreTaskCode() != 0) {
+ sign = true;
+ } else if (sign) {
+ processTaskRelationList.remove(processTaskRelation);
+ }
+ }
+ }
Review Comment:
I think it's better to deal by `Map<PostTaskCode, ProcessTaskRelation>`, due
to the sign after sort is not clear. WDYT?
##########
dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/process/ProcessServiceImpl.java:
##########
@@ -2565,7 +2563,7 @@ public int saveProcessDefine(User operator,
ProcessDefinition processDefinition,
@Override
public int saveTaskRelation(User operator, long projectCode, long
processDefinitionCode, int processDefinitionVersion,
List<ProcessTaskRelationLog> taskRelationList,
List<TaskDefinitionLog> taskDefinitionLogs,
- Boolean syncDefine) {
+ Boolean syncDefine, boolean isDelete) {
Review Comment:
I think it will defaultly delete the old relation list and insert the new
ones.
What's the case will it not be deleted?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
