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 2020/06/23 15:27:04 UTC
[GitHub] [incubator-dolphinscheduler] samz406 commented on a change in pull request #2884: batch copy or move process #2753
samz406 commented on a change in pull request #2884:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/2884#discussion_r444282276
##########
File path: dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/ProcessDefinitionService.java
##########
@@ -295,38 +294,209 @@ private String getResourceIds(ProcessData processData) {
/**
* copy process definition
*
- * @param loginUser login user
- * @param projectName project name
- * @param processId process definition id
- * @return copy result code
+ * @param loginUser loginUser
+ * @param processId processId
+ * @param targetProject targetProject
+ * @return
+ * @throws JsonProcessingException
*/
- public Map<String, Object> copyProcessDefinition(User loginUser, String projectName, Integer processId) throws JsonProcessingException {
+ private Map<String, Object> copyProcessDefinition(User loginUser,
+ Integer processId,
+ Project targetProject) throws JsonProcessingException {
Map<String, Object> result = new HashMap<>(5);
Review comment:
Hashmap does not recommend the initial value odd，No need to initialize
##########
File path: dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/controller/ProcessDefinitionControllerTest.java
##########
@@ -179,16 +181,33 @@ public void testQueryProcessDefinitionById() throws Exception {
}
@Test
- public void testCopyProcessDefinition() throws Exception {
+ public void testBatchCopyProcessDefinition() throws Exception {
String projectName = "test";
- int id = 1;
+ int targetProjectId = 2;
+ String id = "1";
+
+ Map<String, Object> result = new HashMap<>(5);
+ putMsg(result, Status.SUCCESS);
+
+ Mockito.when(processDefinitionService.batchCopyOrMoveProcessDefinition(user,projectName,id,targetProjectId,true)).thenReturn(result);
+ Result response = processDefinitionController.copyOrMoveProcessDefinition(user, projectName,id,targetProjectId,true);
+
+ Assert.assertEquals(Status.SUCCESS.getCode(),response.getCode().intValue());
+ }
+
+ @Test
+ public void testBatchMoveProcessDefinition() throws Exception {
+
+ String projectName = "test";
+ int targetProjectId = 2;
+ String id = "1";
Map<String, Object> result = new HashMap<>(5);
Review comment:
Hashmap does not recommend the initial value odd，No need to initialize
##########
File path: dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/ProcessDefinitionService.java
##########
@@ -295,38 +294,209 @@ private String getResourceIds(ProcessData processData) {
/**
* copy process definition
*
- * @param loginUser login user
- * @param projectName project name
- * @param processId process definition id
- * @return copy result code
+ * @param loginUser loginUser
+ * @param processId processId
+ * @param targetProject targetProject
+ * @return
+ * @throws JsonProcessingException
*/
- public Map<String, Object> copyProcessDefinition(User loginUser, String projectName, Integer processId) throws JsonProcessingException {
+ private Map<String, Object> copyProcessDefinition(User loginUser,
+ Integer processId,
+ Project targetProject) throws JsonProcessingException {
Map<String, Object> result = new HashMap<>(5);
- Project project = projectMapper.queryByName(projectName);
-
- Map<String, Object> checkResult = projectService.checkProjectAndAuth(loginUser, project, projectName);
- Status resultStatus = (Status) checkResult.get(Constants.STATUS);
- if (resultStatus != Status.SUCCESS) {
- return checkResult;
- }
ProcessDefinition processDefinition = processDefineMapper.selectById(processId);
if (processDefinition == null) {
putMsg(result, Status.PROCESS_DEFINE_NOT_EXIST, processId);
return result;
} else {
+
return createProcessDefinition(
loginUser,
- projectName,
+ targetProject.getName(),
processDefinition.getName() + "_copy_" + System.currentTimeMillis(),
processDefinition.getProcessDefinitionJson(),
processDefinition.getDescription(),
processDefinition.getLocations(),
processDefinition.getConnects());
+
+ }
+ }
+
+ /**
+ * batch copy or move process definition
+ * @param loginUser loginUser
+ * @param projectName projectName
+ * @param processDefinitionIds processDefinitionIds
+ * @param targetProjectId targetProjectId
+ * @param isCopy isCopy
+ * @return
+ */
+ public Map<String, Object> batchCopyOrMoveProcessDefinition(User loginUser,
+ String projectName,
+ String processDefinitionIds,
+ int targetProjectId, boolean isCopy){
+ Map<String, Object> result = new HashMap<>(5);
Review comment:
Hashmap does not recommend the initial value odd，No need to initialize
##########
File path: dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/controller/ProcessDefinitionControllerTest.java
##########
@@ -179,16 +181,33 @@ public void testQueryProcessDefinitionById() throws Exception {
}
@Test
- public void testCopyProcessDefinition() throws Exception {
+ public void testBatchCopyProcessDefinition() throws Exception {
String projectName = "test";
- int id = 1;
+ int targetProjectId = 2;
+ String id = "1";
+
+ Map<String, Object> result = new HashMap<>(5);
Review comment:
Hashmap does not recommend the initial value odd，No need to initialize
##########
File path: dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/ProjectController.java
##########
@@ -226,6 +227,23 @@ public Result queryAuthorizedProject(@ApiIgnore @RequestAttribute(value = Consta
return returnDataList(result);
}
+ /**
+ * query user created project
+ *
+ * @param loginUser login user
+ * @return projects which the user create
+ */
+ @ApiOperation(value = "queryProjectCreatedByUser", notes = "QUERY_USER_CREATED_PROJECT_NOTES")
+
+ @GetMapping(value = "/login-user-created-project")
+ @ResponseStatus(HttpStatus.OK)
+ @ApiException(QUERY_USER_CREATED_PROJECT_ERROR)
+ public Result queryProjectCreatedByUser(@ApiIgnore @RequestAttribute(value = Constants.SESSION_USER) User loginUser) {
+ logger.info("login user {}, query authorized project by user id: {}.", StringUtils.replaceNRTtoUnderline(loginUser.getUserName()), StringUtils.replaceNRTtoUnderline(String.valueOf(loginUser.getId())));
+ Map<String, Object> result = projectService.queryProjectCreatedByUser(loginUser, loginUser.getId());
Review comment:
queryProjectCreatedByUser can define a parameter, you can not userid
##########
File path: dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/ProcessDefinitionServiceTest.java
##########
@@ -224,8 +226,52 @@ public void testCopyProcessDefinition() throws Exception{
definition.getLocations(),
definition.getConnects())).thenReturn(createProcessResult);
- Map<String, Object> successRes = processDefinitionService.copyProcessDefinition(loginUser,
- "project_test1", 46);
+ Map<String, Object> successRes = processDefinitionService.batchCopyOrMoveProcessDefinition(loginUser,"project_test1",
+ "46",1,true);
+
+ Assert.assertEquals(Status.SUCCESS, successRes.get(Constants.STATUS));
+ }
+
+ @Test
+ public void testBatchMoveProcessDefinition() throws Exception{
+ String projectName = "project_test1";
+ Mockito.when(projectMapper.queryByName(projectName)).thenReturn(getProject(projectName));
+
+ String projectName2 = "project_test2";
+ Mockito.when(projectMapper.queryByName(projectName2)).thenReturn(getProject(projectName2));
+
+ int targetProjectId = 2;
+ Mockito.when(projectMapper.queryDetailById(targetProjectId)).thenReturn(getProjectById(targetProjectId));
+
+ Project project = getProject(projectName);
+ Project targetProject = getProjectById(targetProjectId);
+
+ User loginUser = new User();
+ loginUser.setId(-1);
+ loginUser.setUserType(UserType.GENERAL_USER);
+
+ Map<String, Object> result = new HashMap<>(5);
Review comment:
Hashmap does not recommend the initial value odd，No need to initialize
##########
File path: dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/ProcessDefinitionService.java
##########
@@ -295,38 +294,209 @@ private String getResourceIds(ProcessData processData) {
/**
* copy process definition
*
- * @param loginUser login user
- * @param projectName project name
- * @param processId process definition id
- * @return copy result code
+ * @param loginUser loginUser
+ * @param processId processId
+ * @param targetProject targetProject
+ * @return
+ * @throws JsonProcessingException
*/
- public Map<String, Object> copyProcessDefinition(User loginUser, String projectName, Integer processId) throws JsonProcessingException {
+ private Map<String, Object> copyProcessDefinition(User loginUser,
+ Integer processId,
+ Project targetProject) throws JsonProcessingException {
Map<String, Object> result = new HashMap<>(5);
- Project project = projectMapper.queryByName(projectName);
-
- Map<String, Object> checkResult = projectService.checkProjectAndAuth(loginUser, project, projectName);
- Status resultStatus = (Status) checkResult.get(Constants.STATUS);
- if (resultStatus != Status.SUCCESS) {
- return checkResult;
- }
ProcessDefinition processDefinition = processDefineMapper.selectById(processId);
if (processDefinition == null) {
putMsg(result, Status.PROCESS_DEFINE_NOT_EXIST, processId);
return result;
} else {
+
return createProcessDefinition(
loginUser,
- projectName,
+ targetProject.getName(),
processDefinition.getName() + "_copy_" + System.currentTimeMillis(),
processDefinition.getProcessDefinitionJson(),
processDefinition.getDescription(),
processDefinition.getLocations(),
processDefinition.getConnects());
+
+ }
+ }
+
+ /**
+ * batch copy or move process definition
+ * @param loginUser loginUser
+ * @param projectName projectName
+ * @param processDefinitionIds processDefinitionIds
+ * @param targetProjectId targetProjectId
+ * @param isCopy isCopy
+ * @return
+ */
+ public Map<String, Object> batchCopyOrMoveProcessDefinition(User loginUser,
+ String projectName,
+ String processDefinitionIds,
+ int targetProjectId, boolean isCopy){
+ Map<String, Object> result = new HashMap<>(5);
+ List<String> failedIdList = new ArrayList<>();
+
+ if (StringUtils.isEmpty(processDefinitionIds)) {
+ putMsg(result, Status.PROCESS_DEFINITION_IDS_IS_EMPTY, processDefinitionIds);
+ return result;
+ }
+
+ //check src project auth
+ Map<String, Object> checkResult = checkProjectAndAuth(loginUser, projectName);
+ if (checkResult != null) {
+ return checkResult;
+ }
+
+ Project targetProject = projectMapper.queryDetailById(targetProjectId);
+ if(targetProject == null){
+ putMsg(result, Status.PROJECT_NOT_FOUNT, targetProjectId);
+ return result;
+ }
+
+ if(!(targetProject.getName()).equals(projectName)){
+ Map<String, Object> checkTargetProjectResult = checkProjectAndAuth(loginUser, targetProject.getName());
+ if (checkTargetProjectResult != null) {
+ return checkTargetProjectResult;
+ }
+ }
+
+ String[] processDefinitionIdList = processDefinitionIds.split(Constants.COMMA);
+ if(isCopy){
+ doBatchCopyProcessDefinition(loginUser, targetProject, failedIdList, processDefinitionIdList);
+ }else{
+ doBatchMoveProcessDefinition(targetProject, failedIdList, processDefinitionIdList);
+ }
+
+ checkBatchOperateResult(projectName,targetProject.getName(),result, failedIdList,isCopy);
+
+ return result;
+ }
+
+ /**
+ * batch move process definition
+ * @param targetProject targetProject
+ * @param failedIdList failedIdList
+ * @param processDefinitionIdList processDefinitionIdList
+ */
+ private void doBatchMoveProcessDefinition(Project targetProject, List<String> failedIdList, String[] processDefinitionIdList) {
+ for(String processDefinitionId:processDefinitionIdList){
+ try {
+ Map<String, Object> moveProcessDefinitionResult =
+ moveProcessDefinition(Integer.valueOf(processDefinitionId),targetProject);
+ if (!Status.SUCCESS.equals(moveProcessDefinitionResult.get(Constants.STATUS))) {
+ failedIdList.add((String) moveProcessDefinitionResult.get(Constants.MSG));
+ logger.error((String) moveProcessDefinitionResult.get(Constants.MSG));
+ }
+ } catch (Exception e) {
+ failedIdList.add(processDefinitionId);
+ }
+ }
+ }
+
+ /**
+ * batch copy process definition
+ * @param loginUser loginUser
+ * @param targetProject targetProject
+ * @param failedIdList failedIdList
+ * @param processDefinitionIdList processDefinitionIdList
+ */
+ private void doBatchCopyProcessDefinition(User loginUser, Project targetProject, List<String> failedIdList, String[] processDefinitionIdList) {
+ for(String processDefinitionId:processDefinitionIdList){
+ try {
+ Map<String, Object> copyProcessDefinitionResult =
+ copyProcessDefinition(loginUser,Integer.valueOf(processDefinitionId),targetProject);
+ if (!Status.SUCCESS.equals(copyProcessDefinitionResult.get(Constants.STATUS))) {
+ failedIdList.add((String) copyProcessDefinitionResult.get(Constants.MSG));
Review comment:
i think failedIdList should store the id, but there added may be a failed description statement not the id
##########
File path: dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/ProcessDefinitionService.java
##########
@@ -295,38 +294,209 @@ private String getResourceIds(ProcessData processData) {
/**
* copy process definition
*
- * @param loginUser login user
- * @param projectName project name
- * @param processId process definition id
- * @return copy result code
+ * @param loginUser loginUser
+ * @param processId processId
+ * @param targetProject targetProject
+ * @return
+ * @throws JsonProcessingException
*/
- public Map<String, Object> copyProcessDefinition(User loginUser, String projectName, Integer processId) throws JsonProcessingException {
+ private Map<String, Object> copyProcessDefinition(User loginUser,
+ Integer processId,
+ Project targetProject) throws JsonProcessingException {
Map<String, Object> result = new HashMap<>(5);
- Project project = projectMapper.queryByName(projectName);
-
- Map<String, Object> checkResult = projectService.checkProjectAndAuth(loginUser, project, projectName);
- Status resultStatus = (Status) checkResult.get(Constants.STATUS);
- if (resultStatus != Status.SUCCESS) {
- return checkResult;
- }
ProcessDefinition processDefinition = processDefineMapper.selectById(processId);
if (processDefinition == null) {
putMsg(result, Status.PROCESS_DEFINE_NOT_EXIST, processId);
return result;
} else {
+
return createProcessDefinition(
loginUser,
- projectName,
+ targetProject.getName(),
processDefinition.getName() + "_copy_" + System.currentTimeMillis(),
processDefinition.getProcessDefinitionJson(),
processDefinition.getDescription(),
processDefinition.getLocations(),
processDefinition.getConnects());
+
+ }
+ }
+
+ /**
+ * batch copy or move process definition
+ * @param loginUser loginUser
+ * @param projectName projectName
+ * @param processDefinitionIds processDefinitionIds
+ * @param targetProjectId targetProjectId
+ * @param isCopy isCopy
+ * @return
+ */
+ public Map<String, Object> batchCopyOrMoveProcessDefinition(User loginUser,
+ String projectName,
+ String processDefinitionIds,
+ int targetProjectId, boolean isCopy){
+ Map<String, Object> result = new HashMap<>(5);
+ List<String> failedIdList = new ArrayList<>();
+
+ if (StringUtils.isEmpty(processDefinitionIds)) {
+ putMsg(result, Status.PROCESS_DEFINITION_IDS_IS_EMPTY, processDefinitionIds);
+ return result;
+ }
+
+ //check src project auth
+ Map<String, Object> checkResult = checkProjectAndAuth(loginUser, projectName);
+ if (checkResult != null) {
+ return checkResult;
+ }
+
+ Project targetProject = projectMapper.queryDetailById(targetProjectId);
+ if(targetProject == null){
+ putMsg(result, Status.PROJECT_NOT_FOUNT, targetProjectId);
+ return result;
+ }
+
+ if(!(targetProject.getName()).equals(projectName)){
+ Map<String, Object> checkTargetProjectResult = checkProjectAndAuth(loginUser, targetProject.getName());
+ if (checkTargetProjectResult != null) {
+ return checkTargetProjectResult;
+ }
+ }
+
+ String[] processDefinitionIdList = processDefinitionIds.split(Constants.COMMA);
+ if(isCopy){
+ doBatchCopyProcessDefinition(loginUser, targetProject, failedIdList, processDefinitionIdList);
+ }else{
+ doBatchMoveProcessDefinition(targetProject, failedIdList, processDefinitionIdList);
+ }
+
+ checkBatchOperateResult(projectName,targetProject.getName(),result, failedIdList,isCopy);
+
+ return result;
+ }
+
+ /**
+ * batch move process definition
+ * @param targetProject targetProject
+ * @param failedIdList failedIdList
+ * @param processDefinitionIdList processDefinitionIdList
+ */
+ private void doBatchMoveProcessDefinition(Project targetProject, List<String> failedIdList, String[] processDefinitionIdList) {
+ for(String processDefinitionId:processDefinitionIdList){
+ try {
+ Map<String, Object> moveProcessDefinitionResult =
+ moveProcessDefinition(Integer.valueOf(processDefinitionId),targetProject);
+ if (!Status.SUCCESS.equals(moveProcessDefinitionResult.get(Constants.STATUS))) {
+ failedIdList.add((String) moveProcessDefinitionResult.get(Constants.MSG));
+ logger.error((String) moveProcessDefinitionResult.get(Constants.MSG));
+ }
+ } catch (Exception e) {
+ failedIdList.add(processDefinitionId);
+ }
+ }
+ }
+
+ /**
+ * batch copy process definition
+ * @param loginUser loginUser
+ * @param targetProject targetProject
+ * @param failedIdList failedIdList
+ * @param processDefinitionIdList processDefinitionIdList
+ */
+ private void doBatchCopyProcessDefinition(User loginUser, Project targetProject, List<String> failedIdList, String[] processDefinitionIdList) {
+ for(String processDefinitionId:processDefinitionIdList){
+ try {
+ Map<String, Object> copyProcessDefinitionResult =
+ copyProcessDefinition(loginUser,Integer.valueOf(processDefinitionId),targetProject);
+ if (!Status.SUCCESS.equals(copyProcessDefinitionResult.get(Constants.STATUS))) {
+ failedIdList.add((String) copyProcessDefinitionResult.get(Constants.MSG));
+ logger.error((String) copyProcessDefinitionResult.get(Constants.MSG));
+ }
+ } catch (Exception e) {
+ failedIdList.add(processDefinitionId);
+ }
+ }
+ }
+
+ /**
+ * check project and auth
+ * @param loginUser
+ * @param projectName
+ * @return
+ */
+ private Map<String, Object> checkProjectAndAuth(User loginUser, String projectName) {
+ Project project = projectMapper.queryByName(projectName);
+
+ //check user access for project
+ Map<String, Object> checkResult = projectService.checkProjectAndAuth(loginUser, project, projectName);
+ Status resultStatus = (Status) checkResult.get(Constants.STATUS);
+
+ if (resultStatus != Status.SUCCESS) {
+ return checkResult;
+ }
+ return null;
+ }
+
+ /**
+ * move process definition
+ * @param processId processId
+ * @param targetProject targetProject
+ * @return move result code
+ */
+ private Map<String, Object> moveProcessDefinition(Integer processId,
+ Project targetProject) {
+
+ Map<String, Object> result = new HashMap<>(5);
Review comment:
Hashmap does not recommend the initial value odd，No need to initialize
##########
File path: dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/ProjectService.java
##########
@@ -43,7 +43,7 @@
*HttpTask./
Review comment:
Modify note description
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org