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/08/27 14:30:42 UTC
[GitHub] [dolphinscheduler] HomminLee opened a new pull request, #11682: [Improvement-11678][API] Improvement the error message when batch delete workflow
HomminLee opened a new pull request, #11682:
URL: https://github.com/apache/dolphinscheduler/pull/11682
this close #11678
The effect is as follows：
<img width="1147" alt="image" src="https://user-images.githubusercontent.com/25881185/187034531-cd7ea4ac-7c3f-411f-9792-b5e8f7b69d33.png">
<img width="1124" alt="image" src="https://user-images.githubusercontent.com/25881185/187034554-83b03fbd-18f2-4a7e-aac9-147ef348f090.png">
notice: another issue wraps error messages to a new line, improve readability.
--
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 #11682: [Improvement-11678][API] Improvement the error message when batch delete workflow
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #11682:
URL: https://github.com/apache/dolphinscheduler/pull/11682#issuecomment-1237902341
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=11682)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=CODE_SMELL) [5 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11682&metric=new_coverage&view=list) [7.1% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11682&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11682&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11682&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] github-code-scanning[bot] commented on a diff in pull request #11682: [Improvement-11678][API] Improvement the error message when batch delete workflow
Posted by GitBox <gi...@apache.org>.
github-code-scanning[bot] commented on code in PR #11682:
URL: https://github.com/apache/dolphinscheduler/pull/11682#discussion_r976646380
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProcessDefinitionServiceImpl.java:
##########
@@ -787,6 +788,49 @@
return result;
}
+ @Override
+ @Transactional
+ public Map<String, Object> batchDeleteProcessDefinitionByCodes(User loginUser, long projectCode, String codes) {
+ Map<String, Object> result = new HashMap<>();
+ if (StringUtils.isEmpty(codes)) {
+ putMsg(result, Status.SUCCESS);
+ return result;
+ }
+
+ Set<Long> definitionCodes = Arrays.stream(codes.split(Constants.COMMA))
+ .map(Long::parseLong)
Review Comment:
## Missing catch of NumberFormatException
Potential uncaught 'java.lang.NumberFormatException'.
[Show more details](https://github.com/apache/dolphinscheduler/security/code-scanning/1512)
--
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] HomminLee commented on a diff in pull request #11682: [Improvement-11678][API] Improvement the error message when batch delete workflow
Posted by GitBox <gi...@apache.org>.
HomminLee commented on code in PR #11682:
URL: https://github.com/apache/dolphinscheduler/pull/11682#discussion_r956906155
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProcessDefinitionServiceImpl.java:
##########
@@ -698,6 +699,43 @@ public Map<String, Object> verifyProcessDefinitionName(User loginUser, long proj
return result;
}
+ @Override
+ public Map<String, Object> batchDeleteProcessDefinitionByCodes(User loginUser,
+ long projectCode,
+ String codes){
+ Map<String, Object> result = new HashMap<>();
+ Set<String> deleteFailedCodeSet = new HashSet<>();
+ if (!StringUtils.isEmpty(codes)) {
+ String[] processDefinitionCodeArray = codes.split(",");
+ for (String strProcessDefinitionCode : processDefinitionCodeArray) {
+ long code = Long.parseLong(strProcessDefinitionCode);
+ ProcessDefinition processDefinition = processDefinitionMapper.queryByCode(code);
+ // check workflow exists, avoid null exception
+ if (processDefinition == null || projectCode != processDefinition.getProjectCode()) {
+ deleteFailedCodeSet.add(MessageFormat.format(Status.PROCESS_DEFINE_NOT_EXIST.getMsg(), String.valueOf(code)));
+ continue;
+ }
+ try {
+ Map<String, Object> deleteResult = this.deleteProcessDefinitionByCode(loginUser, projectCode, code);
+ if (!Status.SUCCESS.equals(deleteResult.get(Constants.STATUS))) {
+ String errorMsg = MessageFormat.format(Status.DELETE_PROCESS_DEFINE_BY_CODES_ERROR.getMsg(), processDefinition.getName(), deleteResult.get(Constants.MSG));
+ deleteFailedCodeSet.add(errorMsg);
+ logger.error(errorMsg);
+ }
+ } catch (Exception e) {
+ deleteFailedCodeSet.add(MessageFormat.format(Status.DELETE_PROCESS_DEFINE_BY_CODES_ERROR.getMsg(), processDefinition.getName(), e.getMessage()));
+ }
+ }
+ }
+
+ if (!deleteFailedCodeSet.isEmpty()) {
+ putMsg(result, Status.BATCH_DELETE_PROCESS_DEFINE_BY_CODES_ERROR, "\n " + String.join("\n ", deleteFailedCodeSet));
Review Comment:
You mean should write another interface which receive the names, instead of query the name by codes?
--
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] HomminLee commented on a diff in pull request #11682: [Improvement-11678][API] Improvement the error message when batch delete workflow
Posted by GitBox <gi...@apache.org>.
HomminLee commented on code in PR #11682:
URL: https://github.com/apache/dolphinscheduler/pull/11682#discussion_r956906424
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProcessDefinitionServiceImpl.java:
##########
@@ -698,6 +699,43 @@ public Map<String, Object> verifyProcessDefinitionName(User loginUser, long proj
return result;
}
+ @Override
+ public Map<String, Object> batchDeleteProcessDefinitionByCodes(User loginUser,
+ long projectCode,
+ String codes){
+ Map<String, Object> result = new HashMap<>();
+ Set<String> deleteFailedCodeSet = new HashSet<>();
+ if (!StringUtils.isEmpty(codes)) {
+ String[] processDefinitionCodeArray = codes.split(",");
+ for (String strProcessDefinitionCode : processDefinitionCodeArray) {
+ long code = Long.parseLong(strProcessDefinitionCode);
+ ProcessDefinition processDefinition = processDefinitionMapper.queryByCode(code);
+ // check workflow exists, avoid null exception
+ if (processDefinition == null || projectCode != processDefinition.getProjectCode()) {
+ deleteFailedCodeSet.add(MessageFormat.format(Status.PROCESS_DEFINE_NOT_EXIST.getMsg(), String.valueOf(code)));
+ continue;
+ }
+ try {
+ Map<String, Object> deleteResult = this.deleteProcessDefinitionByCode(loginUser, projectCode, code);
+ if (!Status.SUCCESS.equals(deleteResult.get(Constants.STATUS))) {
+ String errorMsg = MessageFormat.format(Status.DELETE_PROCESS_DEFINE_BY_CODES_ERROR.getMsg(), processDefinition.getName(), deleteResult.get(Constants.MSG));
+ deleteFailedCodeSet.add(errorMsg);
+ logger.error(errorMsg);
+ }
+ } catch (Exception e) {
+ deleteFailedCodeSet.add(MessageFormat.format(Status.DELETE_PROCESS_DEFINE_BY_CODES_ERROR.getMsg(), processDefinition.getName(), e.getMessage()));
+ }
+ }
+ }
+
+ if (!deleteFailedCodeSet.isEmpty()) {
+ putMsg(result, Status.BATCH_DELETE_PROCESS_DEFINE_BY_CODES_ERROR, "\n " + String.join("\n ", deleteFailedCodeSet));
Review Comment:
> In fact, it's better to return the error process name rather than code, but the interface receive codes...
You mean should write another interface which receive the names, instead of query the name by codes?
--
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] github-code-scanning[bot] commented on a diff in pull request #11682: [Improvement-11678][API] Improvement the error message when batch delete workflow
Posted by GitBox <gi...@apache.org>.
github-code-scanning[bot] commented on code in PR #11682:
URL: https://github.com/apache/dolphinscheduler/pull/11682#discussion_r977796992
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProcessDefinitionServiceImpl.java:
##########
@@ -805,6 +805,52 @@
return result;
}
+ @Override
+ @Transactional
+ public Map<String, Object> batchDeleteProcessDefinitionByCodes(User loginUser, long projectCode, String codes) {
+ Map<String, Object> result = new HashMap<>();
+ if (StringUtils.isEmpty(codes)) {
+ logger.error("Parameter processDefinitionCodes is empty, projectCode is {}.", projectCode);
+ putMsg(result, Status.PROCESS_DEFINITION_CODES_IS_EMPTY);
+ return result;
+ }
+
+ Set<Long> definitionCodes = Lists.newArrayList(codes.split(Constants.COMMA)).stream().map(Long::parseLong)
Review Comment:
## Missing catch of NumberFormatException
Potential uncaught 'java.lang.NumberFormatException'.
[Show more details](https://github.com/apache/dolphinscheduler/security/code-scanning/1535)
--
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 pull request #11682: [Improvement-11678][API] Improvement the error message when batch delete workflow
Posted by GitBox <gi...@apache.org>.
caishunfeng commented on PR #11682:
URL: https://github.com/apache/dolphinscheduler/pull/11682#issuecomment-1261926621
related pr: #12177
--
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] ruanwenjun commented on a diff in pull request #11682: [Improvement-11678][API] Improvement the error message when batch delete workflow
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on code in PR #11682:
URL: https://github.com/apache/dolphinscheduler/pull/11682#discussion_r956725532
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProcessDefinitionServiceImpl.java:
##########
@@ -698,6 +699,43 @@ public Map<String, Object> verifyProcessDefinitionName(User loginUser, long proj
return result;
}
+ @Override
Review Comment:
We need to add `@Transaction` to rollback the data.
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProcessDefinitionServiceImpl.java:
##########
@@ -698,6 +699,43 @@ public Map<String, Object> verifyProcessDefinitionName(User loginUser, long proj
return result;
}
+ @Override
+ public Map<String, Object> batchDeleteProcessDefinitionByCodes(User loginUser,
+ long projectCode,
+ String codes){
+ Map<String, Object> result = new HashMap<>();
+ Set<String> deleteFailedCodeSet = new HashSet<>();
+ if (!StringUtils.isEmpty(codes)) {
+ String[] processDefinitionCodeArray = codes.split(",");
+ for (String strProcessDefinitionCode : processDefinitionCodeArray) {
+ long code = Long.parseLong(strProcessDefinitionCode);
+ ProcessDefinition processDefinition = processDefinitionMapper.queryByCode(code);
+ // check workflow exists, avoid null exception
+ if (processDefinition == null || projectCode != processDefinition.getProjectCode()) {
+ deleteFailedCodeSet.add(MessageFormat.format(Status.PROCESS_DEFINE_NOT_EXIST.getMsg(), String.valueOf(code)));
+ continue;
+ }
+ try {
+ Map<String, Object> deleteResult = this.deleteProcessDefinitionByCode(loginUser, projectCode, code);
+ if (!Status.SUCCESS.equals(deleteResult.get(Constants.STATUS))) {
+ String errorMsg = MessageFormat.format(Status.DELETE_PROCESS_DEFINE_BY_CODES_ERROR.getMsg(), processDefinition.getName(), deleteResult.get(Constants.MSG));
+ deleteFailedCodeSet.add(errorMsg);
+ logger.error(errorMsg);
+ }
+ } catch (Exception e) {
+ deleteFailedCodeSet.add(MessageFormat.format(Status.DELETE_PROCESS_DEFINE_BY_CODES_ERROR.getMsg(), processDefinition.getName(), e.getMessage()));
+ }
+ }
+ }
+
+ if (!deleteFailedCodeSet.isEmpty()) {
+ putMsg(result, Status.BATCH_DELETE_PROCESS_DEFINE_BY_CODES_ERROR, "\n " + String.join("\n ", deleteFailedCodeSet));
Review Comment:
In fact, it's better to return the error process name rather than code, but the interface receive codes...
--
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 #11682: [Improvement-11678][API] Improvement the error message when batch delete workflow
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #11682:
URL: https://github.com/apache/dolphinscheduler/pull/11682#issuecomment-1245685935
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=11682)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=CODE_SMELL) [5 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11682&metric=new_coverage&view=list) [90.9% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11682&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11682&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11682&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] sonarcloud[bot] commented on pull request #11682: [Improvement-11678][API] Improvement the error message when batch delete workflow
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #11682:
URL: https://github.com/apache/dolphinscheduler/pull/11682#issuecomment-1260672946
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=11682)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=CODE_SMELL) [7 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11682&metric=new_coverage&view=list) [86.8% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11682&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11682&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11682&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] github-code-scanning[bot] commented on a diff in pull request #11682: [Improvement-11678][API] Improvement the error message when batch delete workflow
Posted by GitBox <gi...@apache.org>.
github-code-scanning[bot] commented on code in PR #11682:
URL: https://github.com/apache/dolphinscheduler/pull/11682#discussion_r979406248
##########
dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/ProcessDefinitionServiceTest.java:
##########
@@ -504,6 +505,76 @@
Assertions.assertDoesNotThrow(() -> processDefinitionService.deleteProcessDefinitionByCode(user, 46L));
}
+ @Test
+ public void batchDeleteProcessDefinitionByCodeTest() {
+ Mockito.when(projectMapper.queryByCode(projectCode)).thenReturn(getProject(projectCode));
+
+ Project project = getProject(projectCode);
+
+ // process check exists
+ final String twoCodes = "11,12";
+ Set<Long> definitionCodes = Lists.newArrayList(twoCodes.split(Constants.COMMA)).stream()
+ .map(Long::parseLong).collect(Collectors.toSet());
Review Comment:
## Missing catch of NumberFormatException
Potential uncaught 'java.lang.NumberFormatException'.
[Show more details](https://github.com/apache/dolphinscheduler/security/code-scanning/1540)
##########
dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/ProcessDefinitionServiceTest.java:
##########
@@ -504,6 +505,76 @@
Assertions.assertDoesNotThrow(() -> processDefinitionService.deleteProcessDefinitionByCode(user, 46L));
}
+ @Test
+ public void batchDeleteProcessDefinitionByCodeTest() {
+ Mockito.when(projectMapper.queryByCode(projectCode)).thenReturn(getProject(projectCode));
+
+ Project project = getProject(projectCode);
+
+ // process check exists
+ final String twoCodes = "11,12";
+ Set<Long> definitionCodes = Lists.newArrayList(twoCodes.split(Constants.COMMA)).stream()
+ .map(Long::parseLong).collect(Collectors.toSet());
+ ProcessDefinition process = getProcessDefinition();
+ List<ProcessDefinition> processDefinitionList = new ArrayList<>();
+ processDefinitionList.add(process);
+ Mockito.when(processDefinitionMapper.queryByCodes(definitionCodes)).thenReturn(processDefinitionList);
+ Throwable exception = Assertions.assertThrows(ServiceException.class,
+ () -> processDefinitionService.batchDeleteProcessDefinitionByCodes(user, projectCode, twoCodes));
+ String formatter = MessageFormat.format(Status.BATCH_DELETE_PROCESS_DEFINE_BY_CODES_ERROR.getMsg(),
+ "12[process definition not exist]");
+ Assertions.assertEquals(formatter, exception.getMessage());
+
+ // project check auth fail
+ Map<String, Object> result = new HashMap<>();
+ final String singleCodes = "11";
+ putMsg(result, Status.PROJECT_NOT_FOUND, projectCode);
+ definitionCodes = Lists.newArrayList(singleCodes.split(Constants.COMMA)).stream().map(Long::parseLong)
Review Comment:
## Missing catch of NumberFormatException
Potential uncaught 'java.lang.NumberFormatException'.
[Show more details](https://github.com/apache/dolphinscheduler/security/code-scanning/1541)
--
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 #11682: [Improvement-11678][API] Improvement the error message when batch delete workflow
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #11682:
URL: https://github.com/apache/dolphinscheduler/pull/11682#issuecomment-1261806834
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=11682)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=CODE_SMELL) [7 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11682&metric=new_coverage&view=list) [86.8% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11682&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11682&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11682&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] sonarcloud[bot] commented on pull request #11682: [Improvement-11678][API] Improvement the error message when batch delete workflow
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #11682:
URL: https://github.com/apache/dolphinscheduler/pull/11682#issuecomment-1261807418
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=11682)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=CODE_SMELL) [7 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11682&metric=new_coverage&view=list) [86.8% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11682&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11682&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11682&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] github-code-scanning[bot] commented on a diff in pull request #11682: [Improvement-11678][API] Improvement the error message when batch delete workflow
Posted by GitBox <gi...@apache.org>.
github-code-scanning[bot] commented on code in PR #11682:
URL: https://github.com/apache/dolphinscheduler/pull/11682#discussion_r969869620
##########
dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/ProcessDefinitionServiceTest.java:
##########
@@ -504,6 +504,76 @@
Assert.assertEquals(Status.SUCCESS, deleteSuccess.get(Constants.STATUS));
}
+ @Test
+ public void batchDeleteProcessDefinitionByCodeTest() {
+ long projectCode = 1L;
+ Mockito.when(projectMapper.queryByCode(projectCode)).thenReturn(getProject(projectCode));
+
+ Project project = getProject(projectCode);
+ User loginUser = new User();
+ loginUser.setId(-1);
+ loginUser.setUserType(UserType.GENERAL_USER);
+
+ // process check exists
+ Set<Long> definitionCodes =
+ Arrays.stream("46,47".split(Constants.COMMA)).map(Long::parseLong).collect(Collectors.toSet());
Review Comment:
## Missing catch of NumberFormatException
Potential uncaught 'java.lang.NumberFormatException'.
[Show more details](https://github.com/apache/dolphinscheduler/security/code-scanning/1232)
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProcessDefinitionServiceImpl.java:
##########
@@ -796,6 +797,53 @@
return result;
}
+ @Override
+ @Transactional
+ public Map<String, Object> batchDeleteProcessDefinitionByCodes(User loginUser,
+ long projectCode,
+ String codes) {
+ Map<String, Object> result = new HashMap<>();
+ if (StringUtils.isEmpty(codes)) {
+ putMsg(result, Status.SUCCESS);
+ return result;
+ }
+
+ Set<Long> definitionCodes = Arrays.stream(codes.split(Constants.COMMA)).map(Long::parseLong)
Review Comment:
## Missing catch of NumberFormatException
Potential uncaught 'java.lang.NumberFormatException'.
[Show more details](https://github.com/apache/dolphinscheduler/security/code-scanning/1231)
##########
dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/ProcessDefinitionServiceTest.java:
##########
@@ -504,6 +504,76 @@
Assert.assertEquals(Status.SUCCESS, deleteSuccess.get(Constants.STATUS));
}
+ @Test
+ public void batchDeleteProcessDefinitionByCodeTest() {
+ long projectCode = 1L;
+ Mockito.when(projectMapper.queryByCode(projectCode)).thenReturn(getProject(projectCode));
+
+ Project project = getProject(projectCode);
+ User loginUser = new User();
+ loginUser.setId(-1);
+ loginUser.setUserType(UserType.GENERAL_USER);
+
+ // process check exists
+ Set<Long> definitionCodes =
+ Arrays.stream("46,47".split(Constants.COMMA)).map(Long::parseLong).collect(Collectors.toSet());
+ ProcessDefinition process = getProcessDefinition();
+ List<ProcessDefinition> processDefinitionList = new ArrayList<>();
+ processDefinitionList.add(process);
+ Mockito.when(processDefineMapper.queryByCodes(definitionCodes)).thenReturn(processDefinitionList);
+ Map<String, Object> map = processDefinitionService.batchDeleteProcessDefinitionByCodes(loginUser, projectCode, "46,47");
+ Assert.assertEquals(Status.BATCH_DELETE_PROCESS_DEFINE_BY_CODES_ERROR, map.get(Constants.STATUS));
+
+ // project check auth fail
+ Map<String, Object> result = new HashMap<>();
+ putMsg(result, Status.PROJECT_NOT_FOUND, projectCode);
+ definitionCodes = Arrays.stream("46".split(Constants.COMMA)).map(Long::parseLong).collect(Collectors.toSet());
Review Comment:
## Missing catch of NumberFormatException
Potential uncaught 'java.lang.NumberFormatException'.
[Show more details](https://github.com/apache/dolphinscheduler/security/code-scanning/1233)
--
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 #11682: [Improvement-11678][API] Improvement the error message when batch delete workflow
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #11682:
URL: https://github.com/apache/dolphinscheduler/pull/11682#issuecomment-1245688879
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=11682)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=CODE_SMELL) [5 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11682&metric=new_coverage&view=list) [90.9% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11682&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11682&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11682&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] HomminLee commented on a diff in pull request #11682: [Improvement-11678][API] Improvement the error message when batch delete workflow
Posted by GitBox <gi...@apache.org>.
HomminLee commented on code in PR #11682:
URL: https://github.com/apache/dolphinscheduler/pull/11682#discussion_r963446599
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProcessDefinitionServiceImpl.java:
##########
@@ -698,6 +699,43 @@ public Map<String, Object> verifyProcessDefinitionName(User loginUser, long proj
return result;
}
+ @Override
+ public Map<String, Object> batchDeleteProcessDefinitionByCodes(User loginUser,
+ long projectCode,
+ String codes){
+ Map<String, Object> result = new HashMap<>();
+ Set<String> deleteFailedCodeSet = new HashSet<>();
+ if (!StringUtils.isEmpty(codes)) {
+ String[] processDefinitionCodeArray = codes.split(",");
+ for (String strProcessDefinitionCode : processDefinitionCodeArray) {
+ long code = Long.parseLong(strProcessDefinitionCode);
+ ProcessDefinition processDefinition = processDefinitionMapper.queryByCode(code);
+ // check workflow exists, avoid null exception
+ if (processDefinition == null || projectCode != processDefinition.getProjectCode()) {
+ deleteFailedCodeSet.add(MessageFormat.format(Status.PROCESS_DEFINE_NOT_EXIST.getMsg(), String.valueOf(code)));
+ continue;
+ }
+ try {
+ Map<String, Object> deleteResult = this.deleteProcessDefinitionByCode(loginUser, projectCode, code);
+ if (!Status.SUCCESS.equals(deleteResult.get(Constants.STATUS))) {
+ String errorMsg = MessageFormat.format(Status.DELETE_PROCESS_DEFINE_BY_CODES_ERROR.getMsg(), processDefinition.getName(), deleteResult.get(Constants.MSG));
+ deleteFailedCodeSet.add(errorMsg);
+ logger.error(errorMsg);
+ }
+ } catch (Exception e) {
+ deleteFailedCodeSet.add(MessageFormat.format(Status.DELETE_PROCESS_DEFINE_BY_CODES_ERROR.getMsg(), processDefinition.getName(), e.getMessage()));
+ }
+ }
+ }
+
+ if (!deleteFailedCodeSet.isEmpty()) {
+ putMsg(result, Status.BATCH_DELETE_PROCESS_DEFINE_BY_CODES_ERROR, "\n " + String.join("\n ", deleteFailedCodeSet));
Review Comment:
I modified the implementation: if any workflow fails to delete, stop subsequent actions and rollback.
--
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 #11682: [Improvement-11678][API] Improvement the error message when batch delete workflow
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #11682:
URL: https://github.com/apache/dolphinscheduler/pull/11682#issuecomment-1260673464
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=11682)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=CODE_SMELL) [7 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11682&metric=new_coverage&view=list) [86.8% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11682&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11682&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11682&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] ruanwenjun merged pull request #11682: [Improvement-11678][API] Improvement the error message when batch delete workflow
Posted by GitBox <gi...@apache.org>.
ruanwenjun merged PR #11682:
URL: https://github.com/apache/dolphinscheduler/pull/11682
--
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] ruanwenjun commented on a diff in pull request #11682: [Improvement-11678][API] Improvement the error message when batch delete workflow
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on code in PR #11682:
URL: https://github.com/apache/dolphinscheduler/pull/11682#discussion_r959656768
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProcessDefinitionServiceImpl.java:
##########
@@ -698,6 +699,43 @@ public Map<String, Object> verifyProcessDefinitionName(User loginUser, long proj
return result;
}
+ @Override
Review Comment:
If the operation failed, but some workflow has been deleted, this will make the data incorrect.
--
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] ruanwenjun commented on a diff in pull request #11682: [Improvement-11678][API] Improvement the error message when batch delete workflow
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on code in PR #11682:
URL: https://github.com/apache/dolphinscheduler/pull/11682#discussion_r959659510
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProcessDefinitionServiceImpl.java:
##########
@@ -698,6 +699,43 @@ public Map<String, Object> verifyProcessDefinitionName(User loginUser, long proj
return result;
}
+ @Override
+ public Map<String, Object> batchDeleteProcessDefinitionByCodes(User loginUser,
+ long projectCode,
+ String codes){
+ Map<String, Object> result = new HashMap<>();
+ Set<String> deleteFailedCodeSet = new HashSet<>();
+ if (!StringUtils.isEmpty(codes)) {
+ String[] processDefinitionCodeArray = codes.split(",");
+ for (String strProcessDefinitionCode : processDefinitionCodeArray) {
+ long code = Long.parseLong(strProcessDefinitionCode);
+ ProcessDefinition processDefinition = processDefinitionMapper.queryByCode(code);
+ // check workflow exists, avoid null exception
+ if (processDefinition == null || projectCode != processDefinition.getProjectCode()) {
+ deleteFailedCodeSet.add(MessageFormat.format(Status.PROCESS_DEFINE_NOT_EXIST.getMsg(), String.valueOf(code)));
+ continue;
+ }
+ try {
+ Map<String, Object> deleteResult = this.deleteProcessDefinitionByCode(loginUser, projectCode, code);
+ if (!Status.SUCCESS.equals(deleteResult.get(Constants.STATUS))) {
+ String errorMsg = MessageFormat.format(Status.DELETE_PROCESS_DEFINE_BY_CODES_ERROR.getMsg(), processDefinition.getName(), deleteResult.get(Constants.MSG));
+ deleteFailedCodeSet.add(errorMsg);
+ logger.error(errorMsg);
+ }
+ } catch (Exception e) {
+ deleteFailedCodeSet.add(MessageFormat.format(Status.DELETE_PROCESS_DEFINE_BY_CODES_ERROR.getMsg(), processDefinition.getName(), e.getMessage()));
+ }
+ }
+ }
+
+ if (!deleteFailedCodeSet.isEmpty()) {
+ putMsg(result, Status.BATCH_DELETE_PROCESS_DEFINE_BY_CODES_ERROR, "\n " + String.join("\n ", deleteFailedCodeSet));
Review Comment:
No, the interface need to receive code. But we need to return process name in UI, since the processCode is not friendly to user, but if someone use openAPI, it's better to return code, since process name is not friendly for openAPI.
--
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] HomminLee commented on a diff in pull request #11682: [Improvement-11678][API] Improvement the error message when batch delete workflow
Posted by GitBox <gi...@apache.org>.
HomminLee commented on code in PR #11682:
URL: https://github.com/apache/dolphinscheduler/pull/11682#discussion_r956912706
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProcessDefinitionServiceImpl.java:
##########
@@ -698,6 +699,43 @@ public Map<String, Object> verifyProcessDefinitionName(User loginUser, long proj
return result;
}
+ @Override
Review Comment:
Batch deletion, implemented as a loop call `deleteProcessDefinitionByCode` which add `@Transaction`. In my original idea, the delete operation of each workflow does not affect each other.
So, I would like to ask, it is necessary to add `@Transaction` here?
--
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 #11682: [Improvement-11678][API] Improvement the error message when batch delete workflow
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #11682:
URL: https://github.com/apache/dolphinscheduler/pull/11682#issuecomment-1237830210
# [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/11682?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#11682](https://codecov.io/gh/apache/dolphinscheduler/pull/11682?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (dfab4f4) into [dev](https://codecov.io/gh/apache/dolphinscheduler/commit/45ebd528d6f70df8f23d930a075cc0d7a451946d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (45ebd52) will **decrease** coverage by `0.03%`.
> The diff coverage is `9.37%`.
```diff
@@ Coverage Diff @@
## dev #11682 +/- ##
============================================
- Coverage 39.60% 39.56% -0.04%
+ Complexity 4696 4694 -2
============================================
Files 1014 1014
Lines 37959 37971 +12
Branches 4243 4243
============================================
- Hits 15032 15024 -8
- Misses 21318 21341 +23
+ Partials 1609 1606 -3
```
| [Impacted Files](https://codecov.io/gh/apache/dolphinscheduler/pull/11682?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...er/api/controller/ProcessDefinitionController.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11682/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2NvbnRyb2xsZXIvUHJvY2Vzc0RlZmluaXRpb25Db250cm9sbGVyLmphdmE=) | `58.33% <0.00%> (+9.33%)` | :arrow_up: |
| [...api/service/impl/ProcessDefinitionServiceImpl.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11682/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL3NlcnZpY2UvaW1wbC9Qcm9jZXNzRGVmaW5pdGlvblNlcnZpY2VJbXBsLmphdmE=) | `31.64% <0.00%> (-0.67%)` | :arrow_down: |
| [.../org/apache/dolphinscheduler/api/enums/Status.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11682/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2VudW1zL1N0YXR1cy5qYXZh) | `100.00% <100.00%> (ø)` | |
| [...olphinscheduler/rpc/remote/NettyServerHandler.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11682/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1yZW1vdGUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcnBjL3JlbW90ZS9OZXR0eVNlcnZlckhhbmRsZXIuamF2YQ==) | `69.04% <0.00%> (-11.91%)` | :arrow_down: |
| [...eduler/server/worker/task/WorkerHeartBeatTask.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11682/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci13b3JrZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL3dvcmtlci90YXNrL1dvcmtlckhlYXJ0QmVhdFRhc2suamF2YQ==) | `68.08% <0.00%> (-8.52%)` | :arrow_down: |
| [...che/dolphinscheduler/api/python/PythonGateway.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11682/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL3B5dGhvbi9QeXRob25HYXRld2F5LmphdmE=) | `17.22% <0.00%> (ø)` | |
| [...er/master/dispatch/host/assign/RandomSelector.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11682/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1tYXN0ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9kaXNwYXRjaC9ob3N0L2Fzc2lnbi9SYW5kb21TZWxlY3Rvci5qYXZh) | `83.33% <0.00%> (+5.55%)` | :arrow_up: |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?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 #11682: [Improvement-11678][API] Improvement the error message when batch delete workflow
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #11682:
URL: https://github.com/apache/dolphinscheduler/pull/11682#issuecomment-1237905915
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=11682)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=CODE_SMELL) [5 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11682&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11682&metric=new_coverage&view=list) [7.1% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11682&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11682&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11682&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