From 61637cc0a377007e093a9c0f92eb5956bff6ecad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=97=BA=E9=98=B3?= Date: Sun, 9 Apr 2023 12:50:42 +0800 Subject: [PATCH] [Bug] [Cron] Parse corn expression error (#13841) * fix-cron * update catch exception --- .../impl/ProcessDefinitionServiceImpl.java | 3 ++- .../service/impl/SchedulerServiceImpl.java | 7 +++---- .../api/service/SchedulerServiceTest.java | 8 ++++++++ .../service/cron/CronUtils.java | 20 ++++++++++++++++++- .../service/cron/CronUtilsTest.java | 6 ++++++ 5 files changed, 38 insertions(+), 6 deletions(-) diff --git a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProcessDefinitionServiceImpl.java b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProcessDefinitionServiceImpl.java index fde462e9e9..4e7e750ba0 100644 --- a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProcessDefinitionServiceImpl.java +++ b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProcessDefinitionServiceImpl.java @@ -122,6 +122,7 @@ import org.apache.dolphinscheduler.plugin.task.api.enums.TaskTimeoutStrategy; import org.apache.dolphinscheduler.plugin.task.api.model.Property; import org.apache.dolphinscheduler.plugin.task.api.parameters.ParametersNode; import org.apache.dolphinscheduler.plugin.task.api.parameters.SqlParameters; +import org.apache.dolphinscheduler.service.cron.CronUtils; import org.apache.dolphinscheduler.service.model.TaskNode; import org.apache.dolphinscheduler.service.process.ProcessService; @@ -2607,7 +2608,7 @@ public class ProcessDefinitionServiceImpl extends BaseServiceImpl implements Pro putMsg(result, Status.SCHEDULE_START_TIME_END_TIME_SAME); return result; } - if (!org.quartz.CronExpression.isValidExpression(scheduleObj.getCrontab())) { + if (!CronUtils.isValidExpression(scheduleObj.getCrontab())) { log.error("CronExpression verify failure, cron:{}.", scheduleObj.getCrontab()); putMsg(result, Status.REQUEST_PARAMS_NOT_VALID_ERROR, scheduleObj.getCrontab()); return result; 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 1251add8c1..32d4821f60 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 @@ -75,7 +75,6 @@ import java.util.stream.Collectors; import lombok.NonNull; import lombok.extern.slf4j.Slf4j; -import org.quartz.CronExpression; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; @@ -182,7 +181,7 @@ public class SchedulerServiceImpl extends BaseServiceImpl implements SchedulerSe scheduleObj.setStartTime(scheduleParam.getStartTime()); scheduleObj.setEndTime(scheduleParam.getEndTime()); - if (!org.quartz.CronExpression.isValidExpression(scheduleParam.getCrontab())) { + 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; @@ -238,7 +237,7 @@ public class SchedulerServiceImpl extends BaseServiceImpl implements SchedulerSe if (scheduleParam.getStartTime().getTime() > scheduleParam.getEndTime().getTime()) { throw new ServiceException(Status.START_TIME_BIGGER_THAN_END_TIME_ERROR); } - if (!CronExpression.isValidExpression(scheduleParam.getCrontab())) { + if (!CronUtils.isValidExpression(scheduleParam.getCrontab())) { throw new ServiceException(Status.SCHEDULE_CRON_CHECK_FAILED, scheduleParam.getCrontab()); } } @@ -828,7 +827,7 @@ public class SchedulerServiceImpl extends BaseServiceImpl implements SchedulerSe schedule.setStartTime(scheduleParam.getStartTime()); schedule.setEndTime(scheduleParam.getEndTime()); - if (!org.quartz.CronExpression.isValidExpression(scheduleParam.getCrontab())) { + if (!CronUtils.isValidExpression(scheduleParam.getCrontab())) { log.error("Schedule crontab verify failure, crontab:{}.", scheduleParam.getCrontab()); putMsg(result, Status.SCHEDULE_CRON_CHECK_FAILED, scheduleParam.getCrontab()); return; diff --git a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/SchedulerServiceTest.java b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/SchedulerServiceTest.java index 48ddefc6f6..a510f42cd4 100644 --- a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/SchedulerServiceTest.java +++ b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/SchedulerServiceTest.java @@ -242,6 +242,14 @@ public class SchedulerServiceTest extends BaseServiceTestTool { () -> schedulerService.createSchedulesV2(user, scheduleCreateRequest)); Assertions.assertEquals(Status.SCHEDULE_CRON_CHECK_FAILED.getCode(), ((ServiceException) exception).getCode()); + // error schedule crontab + String badCrontab2 = "0 0 13/0 * * ? *"; + scheduleCreateRequest.setStartTime(startTime); + scheduleCreateRequest.setCrontab(badCrontab2); + exception = Assertions.assertThrows(ServiceException.class, + () -> schedulerService.createSchedulesV2(user, scheduleCreateRequest)); + Assertions.assertEquals(Status.SCHEDULE_CRON_CHECK_FAILED.getCode(), ((ServiceException) exception).getCode()); + // error create error scheduleCreateRequest.setCrontab(crontab); Mockito.when(scheduleMapper.insert(isA(Schedule.class))).thenReturn(0); diff --git a/dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/cron/CronUtils.java b/dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/cron/CronUtils.java index 0ffb483a1a..c76bb773ef 100644 --- a/dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/cron/CronUtils.java +++ b/dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/cron/CronUtils.java @@ -79,11 +79,29 @@ public class CronUtils { public static Cron parse2Cron(String cronExpression) throws CronParseException { try { return QUARTZ_CRON_PARSER.parse(cronExpression); - } catch (Exception ex) { + } catch (IllegalArgumentException ex) { throw new CronParseException(String.format("Parse corn expression: [%s] error", cronExpression), ex); } } + /** + * Indicates whether the specified cron expression can be parsed into a + * valid cron expression + * + * @param cronExpression the expression to evaluate + * @return a boolean indicating whether the given expression is a valid cron + * expression + */ + public static boolean isValidExpression(String cronExpression) { + try { + parse2Cron(cronExpression); + } catch (CronParseException e) { + return false; + } + + return true; + } + /** * get max cycle * diff --git a/dolphinscheduler-service/src/test/java/org/apache/dolphinscheduler/service/cron/CronUtilsTest.java b/dolphinscheduler-service/src/test/java/org/apache/dolphinscheduler/service/cron/CronUtilsTest.java index 6be6d95459..5aec41f93e 100644 --- a/dolphinscheduler-service/src/test/java/org/apache/dolphinscheduler/service/cron/CronUtilsTest.java +++ b/dolphinscheduler-service/src/test/java/org/apache/dolphinscheduler/service/cron/CronUtilsTest.java @@ -245,4 +245,10 @@ public class CronUtilsTest { expirationTime = CronUtils.getExpirationTime(startTime, CycleEnum.YEAR); Assertions.assertEquals("2020-02-07 18:30:00", DateUtils.dateToString(expirationTime)); } + + @Test + public void testValid() { + Assertions.assertFalse(CronUtils.isValidExpression("0 0 13/0 * * ? *")); + Assertions.assertTrue(CronUtils.isValidExpression("0 0 13-0 * * ? *")); + } }