You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dolphinscheduler.apache.org by GitBox <gi...@apache.org> on 2022/01/25 08:06:01 UTC
[GitHub] [dolphinscheduler] JinyLeeChina opened a new pull request #8190: [Bug] [API] relation delete api bug
JinyLeeChina opened a new pull request #8190:
URL: https://github.com/apache/dolphinscheduler/pull/8190
close by #8146
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] caishunfeng merged pull request #8190: [Bug] [API] relation delete api bug
Posted by GitBox <gi...@apache.org>.
caishunfeng merged pull request #8190:
URL: https://github.com/apache/dolphinscheduler/pull/8190
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] codecov-commenter commented on pull request #8190: [Bug] [API] relation delete api bug
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #8190:
URL: https://github.com/apache/dolphinscheduler/pull/8190#issuecomment-1020916149
# [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/8190?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> :exclamation: No coverage uploaded for pull request base (`2.0.4-prepare@f5241e5`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#section-missing-base-commit).
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/dolphinscheduler/pull/8190?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## 2.0.4-prepare #8190 +/- ##
================================================
Coverage ? 21.03%
Complexity ? 1580
================================================
Files ? 536
Lines ? 22444
Branches ? 2647
================================================
Hits ? 4721
Misses ? 17250
Partials ? 473
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/8190?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/8190?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [f5241e5...bb6a76f](https://codecov.io/gh/apache/dolphinscheduler/pull/8190?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #8190: [Bug] [API] relation delete api bug
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #8190:
URL: https://github.com/apache/dolphinscheduler/pull/8190#issuecomment-1020922731
SonarCloud Quality Gate failed. 
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=8190&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=8190&resolved=false&types=BUG) [24 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=8190&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=8190&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=8190&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=8190&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=8190&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=8190&resolved=false&types=SECURITY_HOTSPOT) [3 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=8190&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=8190&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=8190&resolved=false&types=CODE_SMELL) [352 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=8190&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=8190&metric=new_coverage&view=list) [15.0% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=8190&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=8190&metric=new_duplicated_lines_density&view=list) [1.1% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=8190&metric=new_duplicated_lines_density&view=list)
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] caishunfeng merged pull request #8190: [Bug] [API] relation delete api bug
Posted by GitBox <gi...@apache.org>.
caishunfeng merged pull request #8190:
URL: https://github.com/apache/dolphinscheduler/pull/8190
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] caishunfeng commented on a change in pull request #8190: [Bug] [API] relation delete api bug
Posted by GitBox <gi...@apache.org>.
caishunfeng commented on a change in pull request #8190:
URL: https://github.com/apache/dolphinscheduler/pull/8190#discussion_r791543873
##########
File path: dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProcessTaskRelationServiceImpl.java
##########
@@ -555,47 +474,44 @@ private void updateProcessDefiniteVersion(User loginUser, Map<String, Object> re
putMsg(result, Status.PROCESS_DEFINE_NOT_EXIST, processDefinitionCode);
return result;
}
- List<ProcessTaskRelation> processTaskRelationList = processTaskRelationMapper.queryByCode(projectCode, processDefinitionCode, preTaskCode, postTaskCode);
+ List<ProcessTaskRelation> processTaskRelationList = processTaskRelationMapper.queryByProcessCode(projectCode, processDefinitionCode);
if (CollectionUtils.isEmpty(processTaskRelationList)) {
putMsg(result, Status.DATA_IS_NULL, "processTaskRelationList");
return result;
}
- if (processTaskRelationList.size() > 1) {
- putMsg(result, Status.DATA_IS_NOT_VALID, "processTaskRelationList");
- return result;
+ Map<Long, List<ProcessTaskRelation>> taskRelationMap = new HashMap<>();
+ for (ProcessTaskRelation processTaskRelation : processTaskRelationList) {
+ taskRelationMap.compute(processTaskRelation.getPostTaskCode(), (k, v) -> {
+ if (v == null) {
+ v = new ArrayList<>();
+ }
+ v.add(processTaskRelation);
+ return v;
+ });
}
- ProcessTaskRelation processTaskRelation = processTaskRelationList.get(0);
- int upstreamCount = processTaskRelationMapper.countByCode(projectCode, processTaskRelation.getProcessDefinitionCode(),
- 0L, processTaskRelation.getPostTaskCode());
-
- if (upstreamCount == 0) {
- putMsg(result, Status.DATA_IS_NULL, "upstreamCount");
+ if (!taskRelationMap.containsKey(postTaskCode)) {
+ putMsg(result, Status.DATA_IS_NULL, "postTaskCode");
return result;
}
- if (upstreamCount > 1) {
- int delete = processTaskRelationMapper.deleteById(processTaskRelation.getId());
- if (delete == 0) {
- putMsg(result, Status.DELETE_EDGE_ERROR);
+ if (taskRelationMap.get(postTaskCode).size() > 1) {
+ for (ProcessTaskRelation processTaskRelation : taskRelationMap.get(postTaskCode)) {
+ if (processTaskRelation.getPreTaskCode() == preTaskCode) {
+ int delete = processTaskRelationMapper.deleteById(processTaskRelation.getId());
+ if (delete == 0) {
+ putMsg(result, Status.DELETE_EDGE_ERROR);
+ throw new ServiceException(Status.DELETE_EDGE_ERROR);
+ }
+ processTaskRelationList.remove(processTaskRelation);
+ }
}
- return result;
- }
- updateProcessDefiniteVersion(loginUser, result, processDefinition);
- processTaskRelation.setProcessDefinitionVersion(processDefinition.getVersion());
- processTaskRelation.setPreTaskVersion(0);
- processTaskRelation.setPreTaskCode(0L);
- Date now = new Date();
- processTaskRelation.setUpdateTime(now);
- int update = processTaskRelationMapper.updateById(processTaskRelation);
- processTaskRelation.setId(0);
- ProcessTaskRelationLog processTaskRelationLog = new ProcessTaskRelationLog(processTaskRelation);
- processTaskRelationLog.setCreateTime(now);
- processTaskRelationLog.setOperator(loginUser.getId());
- processTaskRelationLog.setOperateTime(now);
- int insert = processTaskRelationLogMapper.insert(processTaskRelationLog);
- if ((update & insert) == 0) {
- putMsg(result, Status.DELETE_EDGE_ERROR);
- throw new ServiceException(Status.DELETE_EDGE_ERROR);
+ } else {
+ ProcessTaskRelation processTaskRelation = taskRelationMap.get(postTaskCode).get(0);
+ processTaskRelationList.remove(processTaskRelation);
+ processTaskRelation.setPreTaskVersion(0);
+ processTaskRelation.setPreTaskCode(0L);
+ processTaskRelationList.add(processTaskRelation);
Review comment:
why to add a empty preTask here?
##########
File path: dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/ProcessTaskRelationController.java
##########
@@ -101,46 +101,6 @@ public Result createProcessTaskRelation(@ApiIgnore @RequestAttribute(value = Con
return returnDataList(result);
}
- /**
- * move task to other processDefinition
- *
- * @param loginUser login user info
- * @param projectCode project code
- * @param processDefinitionCode process definition code
- * @param targetProcessDefinitionCode target process definition code
- * @param taskCode the current task code (the post task code)
- * @return move result code
- */
- @ApiOperation(value = "moveRelation", notes = "MOVE_TASK_TO_OTHER_PROCESS_DEFINITION_NOTES")
- @ApiImplicitParams({
- @ApiImplicitParam(name = "projectCode", value = "PROJECT_CODE", required = true, type = "Long"),
- @ApiImplicitParam(name = "processDefinitionCode", value = "PROCESS_DEFINITION_CODE", required = true, type = "Long"),
- @ApiImplicitParam(name = "targetProcessDefinitionCode", value = "TARGET_PROCESS_DEFINITION_CODE", required = true, type = "Long"),
- @ApiImplicitParam(name = "taskCode", value = "TASK_CODE", required = true, type = "Long")
- })
- @PostMapping(value = "/move")
- @ResponseStatus(HttpStatus.OK)
- @ApiException(MOVE_PROCESS_TASK_RELATION_ERROR)
- @AccessLogAnnotation(ignoreRequestArgs = "loginUser")
- public Result moveTaskProcessRelation(@ApiIgnore @RequestAttribute(value = Constants.SESSION_USER) User loginUser,
Review comment:
Is it OK to delete the controller function directly?
##########
File path: dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProcessTaskRelationServiceImpl.java
##########
@@ -372,6 +259,21 @@ private void updateProcessDefiniteVersion(User loginUser, Map<String, Object> re
return result;
}
+ private void updateRelation(User loginUser, Map<String, Object> result, ProcessDefinition processDefinition,
+ List<ProcessTaskRelation> processTaskRelationList) {
+ updateProcessDefiniteVersion(loginUser, result, processDefinition);
+ List<ProcessTaskRelationLog> relationLogs = processTaskRelationList.stream().map(ProcessTaskRelationLog::new).collect(Collectors.toList());
+ int insertResult = processService.saveTaskRelation(loginUser, processDefinition.getProjectCode(), processDefinition.getCode(),
+ processDefinition.getVersion(), relationLogs, Lists.newArrayList(), Boolean.TRUE);
+ if (insertResult == Constants.EXIT_CODE_SUCCESS) {
Review comment:
It's better to use another const, but not `EXIT_CODE_SUCCESS`
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] caishunfeng commented on a change in pull request #8190: [Bug] [API] relation delete api bug
Posted by GitBox <gi...@apache.org>.
caishunfeng commented on a change in pull request #8190:
URL: https://github.com/apache/dolphinscheduler/pull/8190#discussion_r791543873
##########
File path: dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProcessTaskRelationServiceImpl.java
##########
@@ -555,47 +474,44 @@ private void updateProcessDefiniteVersion(User loginUser, Map<String, Object> re
putMsg(result, Status.PROCESS_DEFINE_NOT_EXIST, processDefinitionCode);
return result;
}
- List<ProcessTaskRelation> processTaskRelationList = processTaskRelationMapper.queryByCode(projectCode, processDefinitionCode, preTaskCode, postTaskCode);
+ List<ProcessTaskRelation> processTaskRelationList = processTaskRelationMapper.queryByProcessCode(projectCode, processDefinitionCode);
if (CollectionUtils.isEmpty(processTaskRelationList)) {
putMsg(result, Status.DATA_IS_NULL, "processTaskRelationList");
return result;
}
- if (processTaskRelationList.size() > 1) {
- putMsg(result, Status.DATA_IS_NOT_VALID, "processTaskRelationList");
- return result;
+ Map<Long, List<ProcessTaskRelation>> taskRelationMap = new HashMap<>();
+ for (ProcessTaskRelation processTaskRelation : processTaskRelationList) {
+ taskRelationMap.compute(processTaskRelation.getPostTaskCode(), (k, v) -> {
+ if (v == null) {
+ v = new ArrayList<>();
+ }
+ v.add(processTaskRelation);
+ return v;
+ });
}
- ProcessTaskRelation processTaskRelation = processTaskRelationList.get(0);
- int upstreamCount = processTaskRelationMapper.countByCode(projectCode, processTaskRelation.getProcessDefinitionCode(),
- 0L, processTaskRelation.getPostTaskCode());
-
- if (upstreamCount == 0) {
- putMsg(result, Status.DATA_IS_NULL, "upstreamCount");
+ if (!taskRelationMap.containsKey(postTaskCode)) {
+ putMsg(result, Status.DATA_IS_NULL, "postTaskCode");
return result;
}
- if (upstreamCount > 1) {
- int delete = processTaskRelationMapper.deleteById(processTaskRelation.getId());
- if (delete == 0) {
- putMsg(result, Status.DELETE_EDGE_ERROR);
+ if (taskRelationMap.get(postTaskCode).size() > 1) {
+ for (ProcessTaskRelation processTaskRelation : taskRelationMap.get(postTaskCode)) {
+ if (processTaskRelation.getPreTaskCode() == preTaskCode) {
+ int delete = processTaskRelationMapper.deleteById(processTaskRelation.getId());
+ if (delete == 0) {
+ putMsg(result, Status.DELETE_EDGE_ERROR);
+ throw new ServiceException(Status.DELETE_EDGE_ERROR);
+ }
+ processTaskRelationList.remove(processTaskRelation);
+ }
}
- return result;
- }
- updateProcessDefiniteVersion(loginUser, result, processDefinition);
- processTaskRelation.setProcessDefinitionVersion(processDefinition.getVersion());
- processTaskRelation.setPreTaskVersion(0);
- processTaskRelation.setPreTaskCode(0L);
- Date now = new Date();
- processTaskRelation.setUpdateTime(now);
- int update = processTaskRelationMapper.updateById(processTaskRelation);
- processTaskRelation.setId(0);
- ProcessTaskRelationLog processTaskRelationLog = new ProcessTaskRelationLog(processTaskRelation);
- processTaskRelationLog.setCreateTime(now);
- processTaskRelationLog.setOperator(loginUser.getId());
- processTaskRelationLog.setOperateTime(now);
- int insert = processTaskRelationLogMapper.insert(processTaskRelationLog);
- if ((update & insert) == 0) {
- putMsg(result, Status.DELETE_EDGE_ERROR);
- throw new ServiceException(Status.DELETE_EDGE_ERROR);
+ } else {
+ ProcessTaskRelation processTaskRelation = taskRelationMap.get(postTaskCode).get(0);
+ processTaskRelationList.remove(processTaskRelation);
+ processTaskRelation.setPreTaskVersion(0);
+ processTaskRelation.setPreTaskCode(0L);
+ processTaskRelationList.add(processTaskRelation);
Review comment:
why to add a empty preTask here?
##########
File path: dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/ProcessTaskRelationController.java
##########
@@ -101,46 +101,6 @@ public Result createProcessTaskRelation(@ApiIgnore @RequestAttribute(value = Con
return returnDataList(result);
}
- /**
- * move task to other processDefinition
- *
- * @param loginUser login user info
- * @param projectCode project code
- * @param processDefinitionCode process definition code
- * @param targetProcessDefinitionCode target process definition code
- * @param taskCode the current task code (the post task code)
- * @return move result code
- */
- @ApiOperation(value = "moveRelation", notes = "MOVE_TASK_TO_OTHER_PROCESS_DEFINITION_NOTES")
- @ApiImplicitParams({
- @ApiImplicitParam(name = "projectCode", value = "PROJECT_CODE", required = true, type = "Long"),
- @ApiImplicitParam(name = "processDefinitionCode", value = "PROCESS_DEFINITION_CODE", required = true, type = "Long"),
- @ApiImplicitParam(name = "targetProcessDefinitionCode", value = "TARGET_PROCESS_DEFINITION_CODE", required = true, type = "Long"),
- @ApiImplicitParam(name = "taskCode", value = "TASK_CODE", required = true, type = "Long")
- })
- @PostMapping(value = "/move")
- @ResponseStatus(HttpStatus.OK)
- @ApiException(MOVE_PROCESS_TASK_RELATION_ERROR)
- @AccessLogAnnotation(ignoreRequestArgs = "loginUser")
- public Result moveTaskProcessRelation(@ApiIgnore @RequestAttribute(value = Constants.SESSION_USER) User loginUser,
Review comment:
Is it OK to delete the controller function directly?
##########
File path: dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProcessTaskRelationServiceImpl.java
##########
@@ -372,6 +259,21 @@ private void updateProcessDefiniteVersion(User loginUser, Map<String, Object> re
return result;
}
+ private void updateRelation(User loginUser, Map<String, Object> result, ProcessDefinition processDefinition,
+ List<ProcessTaskRelation> processTaskRelationList) {
+ updateProcessDefiniteVersion(loginUser, result, processDefinition);
+ List<ProcessTaskRelationLog> relationLogs = processTaskRelationList.stream().map(ProcessTaskRelationLog::new).collect(Collectors.toList());
+ int insertResult = processService.saveTaskRelation(loginUser, processDefinition.getProjectCode(), processDefinition.getCode(),
+ processDefinition.getVersion(), relationLogs, Lists.newArrayList(), Boolean.TRUE);
+ if (insertResult == Constants.EXIT_CODE_SUCCESS) {
Review comment:
It's better to use another const, but not `EXIT_CODE_SUCCESS`
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #8190: [Bug] [API] relation delete api bug
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #8190:
URL: https://github.com/apache/dolphinscheduler/pull/8190#issuecomment-1020922731
SonarCloud Quality Gate failed. 
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=8190&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=8190&resolved=false&types=BUG) [24 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=8190&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=8190&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=8190&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=8190&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=8190&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=8190&resolved=false&types=SECURITY_HOTSPOT) [3 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=8190&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=8190&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=8190&resolved=false&types=CODE_SMELL) [352 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=8190&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=8190&metric=new_coverage&view=list) [15.0% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=8190&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=8190&metric=new_duplicated_lines_density&view=list) [1.1% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=8190&metric=new_duplicated_lines_density&view=list)
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] JinyLeeChina commented on a change in pull request #8190: [Bug] [API] relation delete api bug
Posted by GitBox <gi...@apache.org>.
JinyLeeChina commented on a change in pull request #8190:
URL: https://github.com/apache/dolphinscheduler/pull/8190#discussion_r791564067
##########
File path: dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProcessTaskRelationServiceImpl.java
##########
@@ -555,47 +474,44 @@ private void updateProcessDefiniteVersion(User loginUser, Map<String, Object> re
putMsg(result, Status.PROCESS_DEFINE_NOT_EXIST, processDefinitionCode);
return result;
}
- List<ProcessTaskRelation> processTaskRelationList = processTaskRelationMapper.queryByCode(projectCode, processDefinitionCode, preTaskCode, postTaskCode);
+ List<ProcessTaskRelation> processTaskRelationList = processTaskRelationMapper.queryByProcessCode(projectCode, processDefinitionCode);
if (CollectionUtils.isEmpty(processTaskRelationList)) {
putMsg(result, Status.DATA_IS_NULL, "processTaskRelationList");
return result;
}
- if (processTaskRelationList.size() > 1) {
- putMsg(result, Status.DATA_IS_NOT_VALID, "processTaskRelationList");
- return result;
+ Map<Long, List<ProcessTaskRelation>> taskRelationMap = new HashMap<>();
+ for (ProcessTaskRelation processTaskRelation : processTaskRelationList) {
+ taskRelationMap.compute(processTaskRelation.getPostTaskCode(), (k, v) -> {
+ if (v == null) {
+ v = new ArrayList<>();
+ }
+ v.add(processTaskRelation);
+ return v;
+ });
}
- ProcessTaskRelation processTaskRelation = processTaskRelationList.get(0);
- int upstreamCount = processTaskRelationMapper.countByCode(projectCode, processTaskRelation.getProcessDefinitionCode(),
- 0L, processTaskRelation.getPostTaskCode());
-
- if (upstreamCount == 0) {
- putMsg(result, Status.DATA_IS_NULL, "upstreamCount");
+ if (!taskRelationMap.containsKey(postTaskCode)) {
+ putMsg(result, Status.DATA_IS_NULL, "postTaskCode");
return result;
}
- if (upstreamCount > 1) {
- int delete = processTaskRelationMapper.deleteById(processTaskRelation.getId());
- if (delete == 0) {
- putMsg(result, Status.DELETE_EDGE_ERROR);
+ if (taskRelationMap.get(postTaskCode).size() > 1) {
+ for (ProcessTaskRelation processTaskRelation : taskRelationMap.get(postTaskCode)) {
+ if (processTaskRelation.getPreTaskCode() == preTaskCode) {
+ int delete = processTaskRelationMapper.deleteById(processTaskRelation.getId());
+ if (delete == 0) {
+ putMsg(result, Status.DELETE_EDGE_ERROR);
+ throw new ServiceException(Status.DELETE_EDGE_ERROR);
+ }
+ processTaskRelationList.remove(processTaskRelation);
+ }
}
- return result;
- }
- updateProcessDefiniteVersion(loginUser, result, processDefinition);
- processTaskRelation.setProcessDefinitionVersion(processDefinition.getVersion());
- processTaskRelation.setPreTaskVersion(0);
- processTaskRelation.setPreTaskCode(0L);
- Date now = new Date();
- processTaskRelation.setUpdateTime(now);
- int update = processTaskRelationMapper.updateById(processTaskRelation);
- processTaskRelation.setId(0);
- ProcessTaskRelationLog processTaskRelationLog = new ProcessTaskRelationLog(processTaskRelation);
- processTaskRelationLog.setCreateTime(now);
- processTaskRelationLog.setOperator(loginUser.getId());
- processTaskRelationLog.setOperateTime(now);
- int insert = processTaskRelationLogMapper.insert(processTaskRelationLog);
- if ((update & insert) == 0) {
- putMsg(result, Status.DELETE_EDGE_ERROR);
- throw new ServiceException(Status.DELETE_EDGE_ERROR);
+ } else {
+ ProcessTaskRelation processTaskRelation = taskRelationMap.get(postTaskCode).get(0);
+ processTaskRelationList.remove(processTaskRelation);
+ processTaskRelation.setPreTaskVersion(0);
+ processTaskRelation.setPreTaskCode(0L);
+ processTaskRelationList.add(processTaskRelation);
Review comment:
This method is to delete the dependency between two tasks. When the node is not clustered, just set the prefix to 0, which means to delete a line
##########
File path: dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/ProcessTaskRelationController.java
##########
@@ -101,46 +101,6 @@ public Result createProcessTaskRelation(@ApiIgnore @RequestAttribute(value = Con
return returnDataList(result);
}
- /**
- * move task to other processDefinition
- *
- * @param loginUser login user info
- * @param projectCode project code
- * @param processDefinitionCode process definition code
- * @param targetProcessDefinitionCode target process definition code
- * @param taskCode the current task code (the post task code)
- * @return move result code
- */
- @ApiOperation(value = "moveRelation", notes = "MOVE_TASK_TO_OTHER_PROCESS_DEFINITION_NOTES")
- @ApiImplicitParams({
- @ApiImplicitParam(name = "projectCode", value = "PROJECT_CODE", required = true, type = "Long"),
- @ApiImplicitParam(name = "processDefinitionCode", value = "PROCESS_DEFINITION_CODE", required = true, type = "Long"),
- @ApiImplicitParam(name = "targetProcessDefinitionCode", value = "TARGET_PROCESS_DEFINITION_CODE", required = true, type = "Long"),
- @ApiImplicitParam(name = "taskCode", value = "TASK_CODE", required = true, type = "Long")
- })
- @PostMapping(value = "/move")
- @ResponseStatus(HttpStatus.OK)
- @ApiException(MOVE_PROCESS_TASK_RELATION_ERROR)
- @AccessLogAnnotation(ignoreRequestArgs = "loginUser")
- public Result moveTaskProcessRelation(@ApiIgnore @RequestAttribute(value = Constants.SESSION_USER) User loginUser,
Review comment:
There is a logic bug that cannot be bypassed. Providing this method will lead to inconsistent workflow versions
##########
File path: dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProcessTaskRelationServiceImpl.java
##########
@@ -372,6 +259,21 @@ private void updateProcessDefiniteVersion(User loginUser, Map<String, Object> re
return result;
}
+ private void updateRelation(User loginUser, Map<String, Object> result, ProcessDefinition processDefinition,
+ List<ProcessTaskRelation> processTaskRelationList) {
+ updateProcessDefiniteVersion(loginUser, result, processDefinition);
+ List<ProcessTaskRelationLog> relationLogs = processTaskRelationList.stream().map(ProcessTaskRelationLog::new).collect(Collectors.toList());
+ int insertResult = processService.saveTaskRelation(loginUser, processDefinition.getProjectCode(), processDefinition.getCode(),
+ processDefinition.getVersion(), relationLogs, Lists.newArrayList(), Boolean.TRUE);
+ if (insertResult == Constants.EXIT_CODE_SUCCESS) {
Review comment:
This comes from processservice, which is unified with others
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] JinyLeeChina commented on a change in pull request #8190: [Bug] [API] relation delete api bug
Posted by GitBox <gi...@apache.org>.
JinyLeeChina commented on a change in pull request #8190:
URL: https://github.com/apache/dolphinscheduler/pull/8190#discussion_r791564067
##########
File path: dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProcessTaskRelationServiceImpl.java
##########
@@ -555,47 +474,44 @@ private void updateProcessDefiniteVersion(User loginUser, Map<String, Object> re
putMsg(result, Status.PROCESS_DEFINE_NOT_EXIST, processDefinitionCode);
return result;
}
- List<ProcessTaskRelation> processTaskRelationList = processTaskRelationMapper.queryByCode(projectCode, processDefinitionCode, preTaskCode, postTaskCode);
+ List<ProcessTaskRelation> processTaskRelationList = processTaskRelationMapper.queryByProcessCode(projectCode, processDefinitionCode);
if (CollectionUtils.isEmpty(processTaskRelationList)) {
putMsg(result, Status.DATA_IS_NULL, "processTaskRelationList");
return result;
}
- if (processTaskRelationList.size() > 1) {
- putMsg(result, Status.DATA_IS_NOT_VALID, "processTaskRelationList");
- return result;
+ Map<Long, List<ProcessTaskRelation>> taskRelationMap = new HashMap<>();
+ for (ProcessTaskRelation processTaskRelation : processTaskRelationList) {
+ taskRelationMap.compute(processTaskRelation.getPostTaskCode(), (k, v) -> {
+ if (v == null) {
+ v = new ArrayList<>();
+ }
+ v.add(processTaskRelation);
+ return v;
+ });
}
- ProcessTaskRelation processTaskRelation = processTaskRelationList.get(0);
- int upstreamCount = processTaskRelationMapper.countByCode(projectCode, processTaskRelation.getProcessDefinitionCode(),
- 0L, processTaskRelation.getPostTaskCode());
-
- if (upstreamCount == 0) {
- putMsg(result, Status.DATA_IS_NULL, "upstreamCount");
+ if (!taskRelationMap.containsKey(postTaskCode)) {
+ putMsg(result, Status.DATA_IS_NULL, "postTaskCode");
return result;
}
- if (upstreamCount > 1) {
- int delete = processTaskRelationMapper.deleteById(processTaskRelation.getId());
- if (delete == 0) {
- putMsg(result, Status.DELETE_EDGE_ERROR);
+ if (taskRelationMap.get(postTaskCode).size() > 1) {
+ for (ProcessTaskRelation processTaskRelation : taskRelationMap.get(postTaskCode)) {
+ if (processTaskRelation.getPreTaskCode() == preTaskCode) {
+ int delete = processTaskRelationMapper.deleteById(processTaskRelation.getId());
+ if (delete == 0) {
+ putMsg(result, Status.DELETE_EDGE_ERROR);
+ throw new ServiceException(Status.DELETE_EDGE_ERROR);
+ }
+ processTaskRelationList.remove(processTaskRelation);
+ }
}
- return result;
- }
- updateProcessDefiniteVersion(loginUser, result, processDefinition);
- processTaskRelation.setProcessDefinitionVersion(processDefinition.getVersion());
- processTaskRelation.setPreTaskVersion(0);
- processTaskRelation.setPreTaskCode(0L);
- Date now = new Date();
- processTaskRelation.setUpdateTime(now);
- int update = processTaskRelationMapper.updateById(processTaskRelation);
- processTaskRelation.setId(0);
- ProcessTaskRelationLog processTaskRelationLog = new ProcessTaskRelationLog(processTaskRelation);
- processTaskRelationLog.setCreateTime(now);
- processTaskRelationLog.setOperator(loginUser.getId());
- processTaskRelationLog.setOperateTime(now);
- int insert = processTaskRelationLogMapper.insert(processTaskRelationLog);
- if ((update & insert) == 0) {
- putMsg(result, Status.DELETE_EDGE_ERROR);
- throw new ServiceException(Status.DELETE_EDGE_ERROR);
+ } else {
+ ProcessTaskRelation processTaskRelation = taskRelationMap.get(postTaskCode).get(0);
+ processTaskRelationList.remove(processTaskRelation);
+ processTaskRelation.setPreTaskVersion(0);
+ processTaskRelation.setPreTaskCode(0L);
+ processTaskRelationList.add(processTaskRelation);
Review comment:
This method is to delete the dependency between two tasks. When the node is not clustered, just set the prefix to 0, which means to delete a line
##########
File path: dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/ProcessTaskRelationController.java
##########
@@ -101,46 +101,6 @@ public Result createProcessTaskRelation(@ApiIgnore @RequestAttribute(value = Con
return returnDataList(result);
}
- /**
- * move task to other processDefinition
- *
- * @param loginUser login user info
- * @param projectCode project code
- * @param processDefinitionCode process definition code
- * @param targetProcessDefinitionCode target process definition code
- * @param taskCode the current task code (the post task code)
- * @return move result code
- */
- @ApiOperation(value = "moveRelation", notes = "MOVE_TASK_TO_OTHER_PROCESS_DEFINITION_NOTES")
- @ApiImplicitParams({
- @ApiImplicitParam(name = "projectCode", value = "PROJECT_CODE", required = true, type = "Long"),
- @ApiImplicitParam(name = "processDefinitionCode", value = "PROCESS_DEFINITION_CODE", required = true, type = "Long"),
- @ApiImplicitParam(name = "targetProcessDefinitionCode", value = "TARGET_PROCESS_DEFINITION_CODE", required = true, type = "Long"),
- @ApiImplicitParam(name = "taskCode", value = "TASK_CODE", required = true, type = "Long")
- })
- @PostMapping(value = "/move")
- @ResponseStatus(HttpStatus.OK)
- @ApiException(MOVE_PROCESS_TASK_RELATION_ERROR)
- @AccessLogAnnotation(ignoreRequestArgs = "loginUser")
- public Result moveTaskProcessRelation(@ApiIgnore @RequestAttribute(value = Constants.SESSION_USER) User loginUser,
Review comment:
There is a logic bug that cannot be bypassed. Providing this method will lead to inconsistent workflow versions
##########
File path: dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProcessTaskRelationServiceImpl.java
##########
@@ -372,6 +259,21 @@ private void updateProcessDefiniteVersion(User loginUser, Map<String, Object> re
return result;
}
+ private void updateRelation(User loginUser, Map<String, Object> result, ProcessDefinition processDefinition,
+ List<ProcessTaskRelation> processTaskRelationList) {
+ updateProcessDefiniteVersion(loginUser, result, processDefinition);
+ List<ProcessTaskRelationLog> relationLogs = processTaskRelationList.stream().map(ProcessTaskRelationLog::new).collect(Collectors.toList());
+ int insertResult = processService.saveTaskRelation(loginUser, processDefinition.getProjectCode(), processDefinition.getCode(),
+ processDefinition.getVersion(), relationLogs, Lists.newArrayList(), Boolean.TRUE);
+ if (insertResult == Constants.EXIT_CODE_SUCCESS) {
Review comment:
This comes from processservice, which is unified with others
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] codecov-commenter commented on pull request #8190: [Bug] [API] relation delete api bug
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #8190:
URL: https://github.com/apache/dolphinscheduler/pull/8190#issuecomment-1020916149
# [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/8190?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> :exclamation: No coverage uploaded for pull request base (`2.0.4-prepare@f5241e5`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#section-missing-base-commit).
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/dolphinscheduler/pull/8190?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## 2.0.4-prepare #8190 +/- ##
================================================
Coverage ? 21.03%
Complexity ? 1580
================================================
Files ? 536
Lines ? 22444
Branches ? 2647
================================================
Hits ? 4721
Misses ? 17250
Partials ? 473
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/8190?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/8190?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [f5241e5...bb6a76f](https://codecov.io/gh/apache/dolphinscheduler/pull/8190?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org