From 28c60468e8890e204eb1ad6d91e5c95c5d05d5b0 Mon Sep 17 00:00:00 2001 From: ruanwenjun Date: Fri, 8 May 2026 13:13:07 +0800 Subject: [PATCH] [Improvement-18224][API] Migrate SchedulerService Map returns to typed returns Refactor 5 SchedulerService methods from Map to typed return + ServiceException, with cascading updates to the controller, controller test and PythonGateway: - insertSchedule(...): Schedule - updateSchedule(...): Schedule - queryScheduleList(User, long): List - previewSchedule(User, String): List - updateScheduleByWorkflowDefinitionCode(...): Schedule HTTP wire format is preserved: ApiExceptionHandler converts ServiceException to the same Result(code, msg) shape that BaseController.returnDataList(map) produced; success paths use Result.success(data) matching the prior JSON body byte-for-byte. The private updateSchedule helper now throws on validation failure and returns the persisted Schedule on success, replacing the Map-mutation contract. Py4J boundary is preserved: PythonGateway.createOrUpdateSchedule reads the new scheduleId via Schedule#getId() instead of result.get("scheduleId"), so Python SDK clients see no behavior change. Part of the migration series tracked by #18224. --- .../api/controller/SchedulerController.java | 100 ++++----- .../api/python/PythonGateway.java | 4 +- .../api/service/SchedulerService.java | 75 ++++--- .../service/impl/SchedulerServiceImpl.java | 198 ++++++------------ .../controller/SchedulerControllerTest.java | 17 +- 5 files changed, 163 insertions(+), 231 deletions(-) diff --git a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/SchedulerController.java b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/SchedulerController.java index ec5ce9f15c57..27b38b669280 100644 --- a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/SchedulerController.java +++ b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/SchedulerController.java @@ -33,13 +33,15 @@ import org.apache.dolphinscheduler.api.exceptions.ApiException; import org.apache.dolphinscheduler.api.service.SchedulerService; import org.apache.dolphinscheduler.api.utils.Result; +import org.apache.dolphinscheduler.api.vo.ScheduleVO; import org.apache.dolphinscheduler.common.enums.FailureStrategy; import org.apache.dolphinscheduler.common.enums.Priority; import org.apache.dolphinscheduler.common.enums.WarningType; +import org.apache.dolphinscheduler.dao.entity.Schedule; import org.apache.dolphinscheduler.dao.entity.User; import org.apache.dolphinscheduler.plugin.task.api.utils.ParameterUtils; -import java.util.Map; +import java.util.List; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.HttpStatus; @@ -104,18 +106,18 @@ public class SchedulerController extends BaseController { @ResponseStatus(HttpStatus.CREATED) @ApiException(CREATE_SCHEDULE_ERROR) @OperatorLog(auditType = AuditType.SCHEDULE_CREATE) - public Result createSchedule(@Parameter(hidden = true) @RequestAttribute(value = SESSION_USER) User loginUser, - @Parameter(name = "projectCode", description = "PROJECT_CODE", required = true) @PathVariable long projectCode, - @RequestParam(value = "workflowDefinitionCode") long workflowDefinitionCode, - @RequestParam(value = "schedule") String schedule, - @RequestParam(value = "warningType", required = false, defaultValue = DEFAULT_WARNING_TYPE) WarningType warningType, - @RequestParam(value = "warningGroupId", required = false, defaultValue = DEFAULT_NOTIFY_GROUP_ID) int warningGroupId, - @RequestParam(value = "failureStrategy", required = false, defaultValue = DEFAULT_FAILURE_POLICY) FailureStrategy failureStrategy, - @RequestParam(value = "workerGroup", required = false, defaultValue = "default") String workerGroup, - @RequestParam(value = "tenantCode", required = false, defaultValue = "default") String tenantCode, - @RequestParam(value = "environmentCode", required = false, defaultValue = "-1") Long environmentCode, - @RequestParam(value = "workflowInstancePriority", required = false, defaultValue = DEFAULT_WORKFLOW_INSTANCE_PRIORITY) Priority workflowInstancePriority) { - Map result = schedulerService.insertSchedule( + public Result createSchedule(@Parameter(hidden = true) @RequestAttribute(value = SESSION_USER) User loginUser, + @Parameter(name = "projectCode", description = "PROJECT_CODE", required = true) @PathVariable long projectCode, + @RequestParam(value = "workflowDefinitionCode") long workflowDefinitionCode, + @RequestParam(value = "schedule") String schedule, + @RequestParam(value = "warningType", required = false, defaultValue = DEFAULT_WARNING_TYPE) WarningType warningType, + @RequestParam(value = "warningGroupId", required = false, defaultValue = DEFAULT_NOTIFY_GROUP_ID) int warningGroupId, + @RequestParam(value = "failureStrategy", required = false, defaultValue = DEFAULT_FAILURE_POLICY) FailureStrategy failureStrategy, + @RequestParam(value = "workerGroup", required = false, defaultValue = "default") String workerGroup, + @RequestParam(value = "tenantCode", required = false, defaultValue = "default") String tenantCode, + @RequestParam(value = "environmentCode", required = false, defaultValue = "-1") Long environmentCode, + @RequestParam(value = "workflowInstancePriority", required = false, defaultValue = DEFAULT_WORKFLOW_INSTANCE_PRIORITY) Priority workflowInstancePriority) { + Schedule createdSchedule = schedulerService.insertSchedule( loginUser, projectCode, workflowDefinitionCode, @@ -127,8 +129,7 @@ public Result createSchedule(@Parameter(hidden = true) @RequestAttribute(value = workerGroup, tenantCode, environmentCode); - - return returnDataList(result); + return Result.success(createdSchedule); } /** @@ -162,22 +163,21 @@ public Result createSchedule(@Parameter(hidden = true) @RequestAttribute(value = @ResponseStatus(HttpStatus.OK) @ApiException(UPDATE_SCHEDULE_ERROR) @OperatorLog(auditType = AuditType.SCHEDULE_UPDATE) - public Result updateSchedule(@Parameter(hidden = true) @RequestAttribute(value = SESSION_USER) User loginUser, - @Parameter(name = "projectCode", description = "PROJECT_CODE", required = true) @PathVariable long projectCode, - @PathVariable(value = "id") Integer id, - @RequestParam(value = "schedule") String schedule, - @RequestParam(value = "warningType", required = false, defaultValue = DEFAULT_WARNING_TYPE) WarningType warningType, - @RequestParam(value = "warningGroupId", required = false, defaultValue = DEFAULT_NOTIFY_GROUP_ID) int warningGroupId, - @RequestParam(value = "failureStrategy", required = false, defaultValue = "END") FailureStrategy failureStrategy, - @RequestParam(value = "workerGroup", required = false, defaultValue = "default") String workerGroup, - @RequestParam(value = "tenantCode", required = false, defaultValue = "default") String tenantCode, - @RequestParam(value = "environmentCode", required = false, defaultValue = "-1") Long environmentCode, - @RequestParam(value = "workflowInstancePriority", required = false, defaultValue = DEFAULT_WORKFLOW_INSTANCE_PRIORITY) Priority workflowInstancePriority) { - - Map result = schedulerService.updateSchedule(loginUser, projectCode, id, schedule, + public Result updateSchedule(@Parameter(hidden = true) @RequestAttribute(value = SESSION_USER) User loginUser, + @Parameter(name = "projectCode", description = "PROJECT_CODE", required = true) @PathVariable long projectCode, + @PathVariable(value = "id") Integer id, + @RequestParam(value = "schedule") String schedule, + @RequestParam(value = "warningType", required = false, defaultValue = DEFAULT_WARNING_TYPE) WarningType warningType, + @RequestParam(value = "warningGroupId", required = false, defaultValue = DEFAULT_NOTIFY_GROUP_ID) int warningGroupId, + @RequestParam(value = "failureStrategy", required = false, defaultValue = "END") FailureStrategy failureStrategy, + @RequestParam(value = "workerGroup", required = false, defaultValue = "default") String workerGroup, + @RequestParam(value = "tenantCode", required = false, defaultValue = "default") String tenantCode, + @RequestParam(value = "environmentCode", required = false, defaultValue = "-1") Long environmentCode, + @RequestParam(value = "workflowInstancePriority", required = false, defaultValue = DEFAULT_WORKFLOW_INSTANCE_PRIORITY) Priority workflowInstancePriority) { + Schedule updatedSchedule = schedulerService.updateSchedule(loginUser, projectCode, id, schedule, warningType, warningGroupId, failureStrategy, workflowInstancePriority, workerGroup, tenantCode, environmentCode); - return returnDataList(result); + return Result.success(updatedSchedule); } @Operation(summary = "online", description = "ONLINE_SCHEDULE_NOTES") @@ -274,10 +274,10 @@ public Result deleteScheduleById(@RequestAttribute(value = SESSION_USER) User lo @Operation(summary = "queryScheduleList", description = "QUERY_SCHEDULE_LIST_NOTES") @PostMapping("/list") @ApiException(QUERY_SCHEDULE_LIST_ERROR) - public Result queryScheduleList(@Parameter(hidden = true) @RequestAttribute(value = SESSION_USER) User loginUser, - @Parameter(name = "projectCode", description = "PROJECT_CODE", required = true) @PathVariable long projectCode) { - Map result = schedulerService.queryScheduleList(loginUser, projectCode); - return returnDataList(result); + public Result> queryScheduleList(@Parameter(hidden = true) @RequestAttribute(value = SESSION_USER) User loginUser, + @Parameter(name = "projectCode", description = "PROJECT_CODE", required = true) @PathVariable long projectCode) { + List scheduleList = schedulerService.queryScheduleList(loginUser, projectCode); + return Result.success(scheduleList); } /** @@ -294,10 +294,10 @@ public Result queryScheduleList(@Parameter(hidden = true) @RequestAttribute(valu @PostMapping("/preview") @ResponseStatus(HttpStatus.CREATED) @ApiException(PREVIEW_SCHEDULE_ERROR) - public Result previewSchedule(@Parameter(hidden = true) @RequestAttribute(value = SESSION_USER) User loginUser, - @RequestParam(value = "schedule") String schedule) { - Map result = schedulerService.previewSchedule(loginUser, schedule); - return returnDataList(result); + public Result> previewSchedule(@Parameter(hidden = true) @RequestAttribute(value = SESSION_USER) User loginUser, + @RequestParam(value = "schedule") String schedule) { + List previewDateList = schedulerService.previewSchedule(loginUser, schedule); + return Result.success(previewDateList); } /** @@ -330,21 +330,21 @@ public Result previewSchedule(@Parameter(hidden = true) @RequestAttribute(value @ResponseStatus(HttpStatus.OK) @ApiException(UPDATE_SCHEDULE_ERROR) @OperatorLog(auditType = AuditType.SCHEDULE_UPDATE) - public Result updateScheduleByWorkflowDefinitionCode(@Parameter(hidden = true) @RequestAttribute(value = SESSION_USER) User loginUser, - @Parameter(name = "projectCode", description = "PROJECT_CODE", required = true) @PathVariable long projectCode, - @PathVariable(value = "code") long workflowDefinitionCode, - @RequestParam(value = "schedule") String schedule, - @RequestParam(value = "warningType", required = false, defaultValue = DEFAULT_WARNING_TYPE) WarningType warningType, - @RequestParam(value = "warningGroupId", required = false) int warningGroupId, - @RequestParam(value = "failureStrategy", required = false, defaultValue = "END") FailureStrategy failureStrategy, - @RequestParam(value = "workerGroup", required = false, defaultValue = "default") String workerGroup, - @RequestParam(value = "tenantCode", required = false, defaultValue = "default") String tenantCode, - @RequestParam(value = "environmentCode", required = false, defaultValue = "-1") long environmentCode, - @RequestParam(value = "workflowInstancePriority", required = false) Priority workflowInstancePriority) { - Map result = schedulerService.updateScheduleByWorkflowDefinitionCode(loginUser, projectCode, + public Result updateScheduleByWorkflowDefinitionCode(@Parameter(hidden = true) @RequestAttribute(value = SESSION_USER) User loginUser, + @Parameter(name = "projectCode", description = "PROJECT_CODE", required = true) @PathVariable long projectCode, + @PathVariable(value = "code") long workflowDefinitionCode, + @RequestParam(value = "schedule") String schedule, + @RequestParam(value = "warningType", required = false, defaultValue = DEFAULT_WARNING_TYPE) WarningType warningType, + @RequestParam(value = "warningGroupId", required = false) int warningGroupId, + @RequestParam(value = "failureStrategy", required = false, defaultValue = "END") FailureStrategy failureStrategy, + @RequestParam(value = "workerGroup", required = false, defaultValue = "default") String workerGroup, + @RequestParam(value = "tenantCode", required = false, defaultValue = "default") String tenantCode, + @RequestParam(value = "environmentCode", required = false, defaultValue = "-1") long environmentCode, + @RequestParam(value = "workflowInstancePriority", required = false) Priority workflowInstancePriority) { + Schedule updatedSchedule = schedulerService.updateScheduleByWorkflowDefinitionCode(loginUser, projectCode, workflowDefinitionCode, schedule, warningType, warningGroupId, failureStrategy, workflowInstancePriority, workerGroup, tenantCode, environmentCode); - return returnDataList(result); + return Result.success(updatedSchedule); } } diff --git a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/python/PythonGateway.java b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/python/PythonGateway.java index 6635d97356d8..b69f6862b72e 100644 --- a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/python/PythonGateway.java +++ b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/python/PythonGateway.java @@ -331,11 +331,11 @@ private void createOrUpdateSchedule(User user, int scheduleId; if (scheduleObj == null) { workflowDefinitionService.onlineWorkflowDefinition(user, projectCode, workflowCode); - Map result = schedulerService.insertSchedule(user, projectCode, workflowCode, + Schedule createdSchedule = schedulerService.insertSchedule(user, projectCode, workflowCode, schedule, WarningType.valueOf(warningType), warningGroupId, DEFAULT_FAILURE_STRATEGY, DEFAULT_PRIORITY, workerGroup, user.getTenantCode(), DEFAULT_ENVIRONMENT_CODE); - scheduleId = (int) result.get("scheduleId"); + scheduleId = createdSchedule.getId(); } else { scheduleId = scheduleObj.getId(); workflowDefinitionService.offlineWorkflowDefinition(user, projectCode, workflowCode); diff --git a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/SchedulerService.java b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/SchedulerService.java index aad32f1f1f61..bb1b9d4b9194 100644 --- a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/SchedulerService.java +++ b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/SchedulerService.java @@ -18,6 +18,7 @@ package org.apache.dolphinscheduler.api.service; import org.apache.dolphinscheduler.api.utils.Result; +import org.apache.dolphinscheduler.api.vo.ScheduleVO; import org.apache.dolphinscheduler.common.enums.FailureStrategy; import org.apache.dolphinscheduler.common.enums.Priority; import org.apache.dolphinscheduler.common.enums.WarningType; @@ -25,7 +26,6 @@ import org.apache.dolphinscheduler.dao.entity.User; import java.util.List; -import java.util.Map; public interface SchedulerService { @@ -43,19 +43,18 @@ public interface SchedulerService { * @param workerGroup worker group * @param tenantCode tenant code * @param environmentCode environment code - * @return create result code */ - Map insertSchedule(User loginUser, - long projectCode, - long workflowDefinitionCode, - String schedule, - WarningType warningType, - int warningGroupId, - FailureStrategy failureStrategy, - Priority workflowInstancePriority, - String workerGroup, - String tenantCode, - Long environmentCode); + Schedule insertSchedule(User loginUser, + long projectCode, + long workflowDefinitionCode, + String schedule, + WarningType warningType, + int warningGroupId, + FailureStrategy failureStrategy, + Priority workflowInstancePriority, + String workerGroup, + String tenantCode, + Long environmentCode); /** * updateWorkflowInstance schedule @@ -71,19 +70,18 @@ Map insertSchedule(User loginUser, * @param tenantCode tenant code * @param environmentCode environment code * @param workflowInstancePriority workflow instance priority - * @return update result code */ - Map updateSchedule(User loginUser, - long projectCode, - Integer id, - String scheduleExpression, - WarningType warningType, - int warningGroupId, - FailureStrategy failureStrategy, - Priority workflowInstancePriority, - String workerGroup, - String tenantCode, - Long environmentCode); + Schedule updateSchedule(User loginUser, + long projectCode, + Integer id, + String scheduleExpression, + WarningType warningType, + int warningGroupId, + FailureStrategy failureStrategy, + Priority workflowInstancePriority, + String workerGroup, + String tenantCode, + Long environmentCode); /** * query schedule @@ -108,7 +106,7 @@ Result querySchedule(User loginUser, long projectCode, long workflowDefinitionCo * @param projectCode project code * @return schedule list */ - Map queryScheduleList(User loginUser, long projectCode); + List queryScheduleList(User loginUser, long projectCode); /** * delete schedule by id @@ -125,7 +123,7 @@ Result querySchedule(User loginUser, long projectCode, long workflowDefinitionCo * @param schedule schedule expression * @return the next five fire time */ - Map previewSchedule(User loginUser, String schedule); + List previewSchedule(User loginUser, String schedule); /** * update workflow definition schedule @@ -140,19 +138,18 @@ Result querySchedule(User loginUser, long projectCode, long workflowDefinitionCo * @param workerGroup worker group * @param tenantCode tenant code * @param workflowInstancePriority workflow instance priority - * @return update result code */ - Map updateScheduleByWorkflowDefinitionCode(User loginUser, - long projectCode, - long workflowDefinitionCode, - String scheduleExpression, - WarningType warningType, - int warningGroupId, - FailureStrategy failureStrategy, - Priority workflowInstancePriority, - String workerGroup, - String tenantCode, - long environmentCode); + Schedule updateScheduleByWorkflowDefinitionCode(User loginUser, + long projectCode, + long workflowDefinitionCode, + String scheduleExpression, + WarningType warningType, + int warningGroupId, + FailureStrategy failureStrategy, + Priority workflowInstancePriority, + String workerGroup, + String tenantCode, + long environmentCode); /** * Online the scheduler by scheduler id, if the related workflow definition is not online will throw exception. diff --git a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/SchedulerServiceImpl.java b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/SchedulerServiceImpl.java index fc073e79a31c..772456ac24c6 100644 --- a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/SchedulerServiceImpl.java +++ b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/SchedulerServiceImpl.java @@ -57,9 +57,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.Date; -import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.TimeZone; import java.util.stream.Collectors; @@ -113,31 +111,25 @@ public class SchedulerServiceImpl extends BaseServiceImpl implements SchedulerSe * @param workerGroup worker group * @param tenantCode tenant code * @param environmentCode environment code - * @return create result code */ @Override @Transactional - public Map insertSchedule(User loginUser, - long projectCode, - long workflowDefinitionCode, - String schedule, - WarningType warningType, - int warningGroupId, - FailureStrategy failureStrategy, - Priority workflowInstancePriority, - String workerGroup, - String tenantCode, - Long environmentCode) { - - Map result = new HashMap<>(); + public Schedule insertSchedule(User loginUser, + long projectCode, + long workflowDefinitionCode, + String schedule, + WarningType warningType, + int warningGroupId, + FailureStrategy failureStrategy, + Priority workflowInstancePriority, + String workerGroup, + String tenantCode, + Long environmentCode) { Project project = projectMapper.queryByCode(projectCode); // check project auth - boolean hasProjectAndPerm = projectService.hasProjectAndPerm(loginUser, project, result, null); - if (!hasProjectAndPerm) { - return result; - } + projectService.checkProjectAndAuthThrowException(loginUser, project, null); // check workflow define release state WorkflowDefinition workflowDefinition = workflowDefinitionMapper.queryByCode(workflowDefinitionCode); @@ -149,8 +141,8 @@ public Map insertSchedule(User loginUser, if (scheduleExists != null) { log.error("Schedule already exist, scheduleId:{}, workflowDefinitionCode:{}", scheduleExists.getId(), workflowDefinitionCode); - putMsg(result, Status.SCHEDULE_ALREADY_EXISTS, workflowDefinitionCode, scheduleExists.getId()); - return result; + throw new ServiceException(Status.SCHEDULE_ALREADY_EXISTS, workflowDefinitionCode, + scheduleExists.getId()); } Schedule scheduleObj = new Schedule(); @@ -166,21 +158,18 @@ public Map insertSchedule(User loginUser, ScheduleParam scheduleParam = JSONUtils.parseObject(schedule, ScheduleParam.class); if (DateUtils.differSec(scheduleParam.getStartTime(), scheduleParam.getEndTime()) == 0) { log.warn("The start time must not be the same as the end or time can not be null."); - putMsg(result, Status.SCHEDULE_START_TIME_END_TIME_SAME); - return result; + throw new ServiceException(Status.SCHEDULE_START_TIME_END_TIME_SAME); } if (scheduleParam.getStartTime().getTime() > scheduleParam.getEndTime().getTime()) { log.warn("The start time must smaller than end time"); - putMsg(result, Status.START_TIME_BIGGER_THAN_END_TIME_ERROR); - return result; + throw new ServiceException(Status.START_TIME_BIGGER_THAN_END_TIME_ERROR); } scheduleObj.setStartTime(scheduleParam.getStartTime()); scheduleObj.setEndTime(scheduleParam.getEndTime()); if (!CronUtils.isValidExpression(scheduleParam.getCrontab())) { log.error("Schedule crontab verify failure, crontab:{}.", scheduleParam.getCrontab()); - putMsg(result, Status.REQUEST_PARAMS_NOT_VALID_ERROR, scheduleParam.getCrontab()); - return result; + throw new ServiceException(Status.REQUEST_PARAMS_NOT_VALID_ERROR, scheduleParam.getCrontab()); } scheduleObj.setCrontab(scheduleParam.getCrontab()); scheduleObj.setTimezoneId(scheduleParam.getTimezoneId()); @@ -203,13 +192,9 @@ public Map insertSchedule(User loginUser, workflowDefinition.setWarningGroupId(warningGroupId); workflowDefinitionMapper.updateById(workflowDefinition); - // return scheduler object with ID - result.put(Constants.DATA_LIST, scheduleMapper.selectById(scheduleObj.getId())); - putMsg(result, Status.SUCCESS); log.info("Schedule create complete, projectCode:{}, workflowDefinitionCode:{}, scheduleId:{}.", projectCode, workflowDefinitionCode, scheduleObj.getId()); - result.put("scheduleId", scheduleObj.getId()); - return result; + return scheduleMapper.selectById(scheduleObj.getId()); } protected void projectPermCheckByWorkflowCode(User loginUser, long workflowDefinitionCode) { @@ -236,38 +221,32 @@ protected void projectPermCheckByWorkflowCode(User loginUser, long workflowDefin * @param tenantCode tenant code * @param environmentCode environment code * @param workflowInstancePriority workflow instance priority - * @return update result code */ @Override @Transactional - public Map updateSchedule(User loginUser, - long projectCode, - Integer id, - String scheduleExpression, - WarningType warningType, - int warningGroupId, - FailureStrategy failureStrategy, - Priority workflowInstancePriority, - String workerGroup, - String tenantCode, - Long environmentCode) { - Map result = new HashMap<>(); + public Schedule updateSchedule(User loginUser, + long projectCode, + Integer id, + String scheduleExpression, + WarningType warningType, + int warningGroupId, + FailureStrategy failureStrategy, + Priority workflowInstancePriority, + String workerGroup, + String tenantCode, + Long environmentCode) { Project project = projectMapper.queryByCode(projectCode); // check project auth - boolean hasProjectAndPerm = projectService.hasProjectAndPerm(loginUser, project, result, null); - if (!hasProjectAndPerm) { - return result; - } + projectService.checkProjectAndAuthThrowException(loginUser, project, null); // check schedule exists Schedule schedule = scheduleMapper.selectById(id); if (schedule == null) { log.error("Schedule does not exist, scheduleId:{}.", id); - putMsg(result, Status.SCHEDULE_NOT_EXISTS, id); - return result; + throw new ServiceException(Status.SCHEDULE_NOT_EXISTS, id); } WorkflowDefinition workflowDefinition = @@ -275,13 +254,12 @@ public Map updateSchedule(User loginUser, if (workflowDefinition == null || projectCode != workflowDefinition.getProjectCode()) { log.error("workflow definition does not exist, workflowDefinitionCode:{}.", schedule.getWorkflowDefinitionCode()); - putMsg(result, Status.WORKFLOW_DEFINITION_NOT_EXIST, String.valueOf(schedule.getWorkflowDefinitionCode())); - return result; + throw new ServiceException(Status.WORKFLOW_DEFINITION_NOT_EXIST, + String.valueOf(schedule.getWorkflowDefinitionCode())); } - updateSchedule(result, schedule, workflowDefinition, scheduleExpression, warningType, warningGroupId, + return updateSchedule(schedule, workflowDefinition, scheduleExpression, warningType, warningGroupId, failureStrategy, workflowInstancePriority, workerGroup, tenantCode, environmentCode); - return result; } /** @@ -351,43 +329,18 @@ public List queryScheduleByWorkflowDefinitionCodes(@NonNull List * @return schedule list */ @Override - public Map queryScheduleList(User loginUser, long projectCode) { - Map result = new HashMap<>(); + public List queryScheduleList(User loginUser, long projectCode) { Project project = projectMapper.queryByCode(projectCode); // check project auth - boolean hasProjectAndPerm = projectService.hasProjectAndPerm(loginUser, project, result, null); - if (!hasProjectAndPerm) { - return result; - } + projectService.checkProjectAndAuthThrowException(loginUser, project, null); List schedules = scheduleMapper.querySchedulerListByProjectName(project.getName()); List scheduleList = new ArrayList<>(); for (Schedule schedule : schedules) { scheduleList.add(new ScheduleVO(schedule)); } - - result.put(Constants.DATA_LIST, scheduleList); - putMsg(result, Status.SUCCESS); - - return result; - } - - /** - * check valid - * - * @param result result - * @param bool bool - * @param status status - * @return check result code - */ - private boolean checkValid(Map result, boolean bool, Status status) { - // timeout is valid - if (bool) { - putMsg(result, status); - return true; - } - return false; + return scheduleList; } /** @@ -426,8 +379,7 @@ public void deleteSchedulesById(User loginUser, Integer scheduleId) { * @return the next five fire time */ @Override - public Map previewSchedule(User loginUser, String schedule) { - Map result = new HashMap<>(); + public List previewSchedule(User loginUser, String schedule) { Cron cron; ScheduleParam scheduleParam = JSONUtils.parseObject(schedule, ScheduleParam.class); @@ -442,16 +394,13 @@ public Map previewSchedule(User loginUser, String schedule) { cron = CronUtils.parse2Cron(scheduleParam.getCrontab()); } catch (CronParseException e) { log.error("Parse cron to cron expression error, crontab:{}.", scheduleParam.getCrontab(), e); - putMsg(result, Status.PARSE_TO_CRON_EXPRESSION_ERROR); - return result; + throw new ServiceException(Status.PARSE_TO_CRON_EXPRESSION_ERROR); } List selfFireDateList = CronUtils.getSelfFireDateList(startTime, endTime, cron, Constants.PREVIEW_SCHEDULE_EXECUTE_COUNT); - List previewDateList = - selfFireDateList.stream().map(t -> DateUtils.dateToString(t, zoneId)).collect(Collectors.toList()); - result.put(Constants.DATA_LIST, previewDateList); - putMsg(result, Status.SUCCESS); - return result; + return selfFireDateList.stream() + .map(t -> DateUtils.dateToString(t, zoneId)) + .collect(Collectors.toList()); } /** @@ -467,44 +416,40 @@ public Map previewSchedule(User loginUser, String schedule) { * @param workerGroup worker group * @param tenantCode tenant code * @param workflowInstancePriority workflow instance priority - * @return update result code */ @Override - public Map updateScheduleByWorkflowDefinitionCode(User loginUser, - long projectCode, - long workflowDefinitionCode, - String scheduleExpression, - WarningType warningType, - int warningGroupId, - FailureStrategy failureStrategy, - Priority workflowInstancePriority, - String workerGroup, - String tenantCode, - long environmentCode) { + public Schedule updateScheduleByWorkflowDefinitionCode(User loginUser, + long projectCode, + long workflowDefinitionCode, + String scheduleExpression, + WarningType warningType, + int warningGroupId, + FailureStrategy failureStrategy, + Priority workflowInstancePriority, + String workerGroup, + String tenantCode, + long environmentCode) { Project project = projectMapper.queryByCode(projectCode); // check user access for project projectService.checkProjectAndAuthThrowException(loginUser, project, null); - Map result = new HashMap<>(); // check schedule exists Schedule schedule = scheduleMapper.queryByWorkflowDefinitionCode(workflowDefinitionCode); if (schedule == null) { log.error("Schedule of workflow definition does not exist, workflowDefinitionCode:{}.", workflowDefinitionCode); - putMsg(result, Status.SCHEDULE_CRON_NOT_EXISTS, workflowDefinitionCode); - return result; + throw new ServiceException(Status.SCHEDULE_CRON_NOT_EXISTS, workflowDefinitionCode); } WorkflowDefinition workflowDefinition = workflowDefinitionMapper.queryByCode(workflowDefinitionCode); if (workflowDefinition == null || projectCode != workflowDefinition.getProjectCode()) { log.error("workflow definition does not exist, workflowDefinitionCode:{}.", workflowDefinitionCode); - putMsg(result, Status.WORKFLOW_DEFINITION_NOT_EXIST, String.valueOf(workflowDefinitionCode)); - return result; + throw new ServiceException(Status.WORKFLOW_DEFINITION_NOT_EXIST, + String.valueOf(workflowDefinitionCode)); } - updateSchedule(result, schedule, workflowDefinition, scheduleExpression, warningType, warningGroupId, + return updateSchedule(schedule, workflowDefinition, scheduleExpression, warningType, warningGroupId, failureStrategy, workflowInstancePriority, workerGroup, tenantCode, environmentCode); - return result; } @Transactional @@ -576,16 +521,14 @@ private void doOfflineScheduler(Schedule schedule) { schedulerApi.deleteScheduleTask(project.getId(), schedule.getId()); } - private void updateSchedule(Map result, Schedule schedule, WorkflowDefinition workflowDefinition, - String scheduleExpression, WarningType warningType, int warningGroupId, - FailureStrategy failureStrategy, Priority workflowInstancePriority, String workerGroup, - String tenantCode, - long environmentCode) { - if (checkValid(result, schedule.getReleaseState() == ReleaseState.ONLINE, - Status.SCHEDULE_CRON_ONLINE_FORBID_UPDATE)) { + private Schedule updateSchedule(Schedule schedule, WorkflowDefinition workflowDefinition, + String scheduleExpression, WarningType warningType, int warningGroupId, + FailureStrategy failureStrategy, Priority workflowInstancePriority, + String workerGroup, String tenantCode, long environmentCode) { + if (schedule.getReleaseState() == ReleaseState.ONLINE) { log.warn("Schedule can not be updated due to schedule is {}, scheduleId:{}.", ReleaseState.ONLINE.getDescp(), schedule.getId()); - return; + throw new ServiceException(Status.SCHEDULE_CRON_ONLINE_FORBID_UPDATE); } Date now = new Date(); @@ -598,26 +541,22 @@ private void updateSchedule(Map result, Schedule schedule, Workf ScheduleParam scheduleParam = JSONUtils.parseObject(scheduleExpression, ScheduleParam.class); if (scheduleParam == null) { log.warn("Parameter scheduleExpression is invalid, so parse cron error."); - putMsg(result, Status.PARSE_TO_CRON_EXPRESSION_ERROR); - return; + throw new ServiceException(Status.PARSE_TO_CRON_EXPRESSION_ERROR); } if (DateUtils.differSec(scheduleParam.getStartTime(), scheduleParam.getEndTime()) == 0) { log.warn("The start time must not be the same as the end or time can not be null."); - putMsg(result, Status.SCHEDULE_START_TIME_END_TIME_SAME); - return; + throw new ServiceException(Status.SCHEDULE_START_TIME_END_TIME_SAME); } if (scheduleParam.getStartTime().getTime() > scheduleParam.getEndTime().getTime()) { log.warn("The start time must smaller than end time"); - putMsg(result, Status.START_TIME_BIGGER_THAN_END_TIME_ERROR); - return; + throw new ServiceException(Status.START_TIME_BIGGER_THAN_END_TIME_ERROR); } schedule.setStartTime(scheduleParam.getStartTime()); schedule.setEndTime(scheduleParam.getEndTime()); if (!CronUtils.isValidExpression(scheduleParam.getCrontab())) { log.error("Schedule crontab verify failure, crontab:{}.", scheduleParam.getCrontab()); - putMsg(result, Status.SCHEDULE_CRON_CHECK_FAILED, scheduleParam.getCrontab()); - return; + throw new ServiceException(Status.SCHEDULE_CRON_CHECK_FAILED, scheduleParam.getCrontab()); } schedule.setCrontab(scheduleParam.getCrontab()); schedule.setTimezoneId(scheduleParam.getTimezoneId()); @@ -645,8 +584,7 @@ private void updateSchedule(Map result, Schedule schedule, Workf log.info("Schedule update complete, projectCode:{}, workflowDefinitionCode:{}, scheduleId:{}.", workflowDefinition.getProjectCode(), workflowDefinition.getCode(), schedule.getId()); - result.put(Constants.DATA_LIST, schedule); - putMsg(result, Status.SUCCESS); + return schedule; } } diff --git a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/controller/SchedulerControllerTest.java b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/controller/SchedulerControllerTest.java index 36a1163898f9..4508934c30f8 100644 --- a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/controller/SchedulerControllerTest.java +++ b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/controller/SchedulerControllerTest.java @@ -30,7 +30,6 @@ import org.apache.dolphinscheduler.api.service.SchedulerService; import org.apache.dolphinscheduler.api.utils.PageInfo; import org.apache.dolphinscheduler.api.utils.Result; -import org.apache.dolphinscheduler.common.constants.Constants; import org.apache.dolphinscheduler.common.enums.FailureStrategy; import org.apache.dolphinscheduler.common.enums.Priority; import org.apache.dolphinscheduler.common.enums.WarningType; @@ -38,6 +37,8 @@ import org.apache.dolphinscheduler.dao.entity.Schedule; import org.apache.dolphinscheduler.dao.entity.User; +import java.util.Collections; + import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -50,17 +51,12 @@ import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; -import com.google.common.collect.ImmutableMap; - public class SchedulerControllerTest extends AbstractControllerTest { private static Schedule scheduleObj = new Schedule(); private static final Logger logger = LoggerFactory.getLogger(SchedulerControllerTest.class); - final ImmutableMap result = - ImmutableMap.of(Constants.STATUS, Status.SUCCESS, Constants.DATA_LIST, scheduleObj); - @MockBean(name = "schedulerService") private SchedulerService schedulerService; @@ -86,7 +82,7 @@ public void testCreateSchedule() throws Exception { Mockito.when(schedulerService.insertSchedule(isA(User.class), isA(Long.class), isA(Long.class), isA(String.class), isA(WarningType.class), isA(int.class), isA(FailureStrategy.class), - isA(Priority.class), isA(String.class), isA(String.class), isA(Long.class))).thenReturn(result); + isA(Priority.class), isA(String.class), isA(String.class), isA(Long.class))).thenReturn(scheduleObj); MvcResult mvcResult = mockMvc.perform(post("/projects/{projectCode}/schedules/", 123) .header(SESSION_ID, sessionId) @@ -117,7 +113,7 @@ public void testUpdateSchedule() throws Exception { Mockito.when(schedulerService.updateSchedule(isA(User.class), isA(Long.class), isA(Integer.class), isA(String.class), isA(WarningType.class), isA(Integer.class), isA(FailureStrategy.class), - isA(Priority.class), isA(String.class), isA(String.class), isA(Long.class))).thenReturn(result); + isA(Priority.class), isA(String.class), isA(String.class), isA(Long.class))).thenReturn(scheduleObj); MvcResult mvcResult = mockMvc.perform(put("/projects/{projectCode}/schedules/{id}", 123, 37) .header(SESSION_ID, sessionId) @@ -197,7 +193,8 @@ public void testQueryScheduleListPaging() throws Exception { @Test public void testQueryScheduleList() throws Exception { - Mockito.when(schedulerService.queryScheduleList(isA(User.class), isA(Long.class))).thenReturn(success()); + Mockito.when(schedulerService.queryScheduleList(isA(User.class), isA(Long.class))) + .thenReturn(Collections.emptyList()); MvcResult mvcResult = mockMvc.perform(post("/projects/{projectCode}/schedules/list", 123) .header(SESSION_ID, sessionId)) @@ -213,7 +210,7 @@ public void testQueryScheduleList() throws Exception { @Test public void testPreviewSchedule() throws Exception { Mockito.when(schedulerService.previewSchedule(isA(User.class), isA(String.class))) - .thenReturn(success()); + .thenReturn(Collections.emptyList()); MvcResult mvcResult = mockMvc.perform(post("/projects/{projectCode}/schedules/preview", 123) .header(SESSION_ID, sessionId)