This is an automated email from the ASF dual-hosted git repository.
caishunfeng pushed a commit to branch dev
in repository https://gitbox.apache.org/repos/asf/dolphinscheduler.git
The following commit(s) were added to refs/heads/dev by this push:
new 61637cc0a3 [Bug] [Cron] Parse corn expression error (#13841)
61637cc0a3 is described below
commit 61637cc0a377007e093a9c0f92eb5956bff6ecad
Author: 旺阳 <[email protected]>
AuthorDate: Sun Apr 9 12:50:42 2023 +0800
[Bug] [Cron] Parse corn expression error (#13841)
* fix-cron
* update catch exception
---
.../service/impl/ProcessDefinitionServiceImpl.java | 3 ++-
.../api/service/impl/SchedulerServiceImpl.java | 7 +++----
.../api/service/SchedulerServiceTest.java | 8 ++++++++
.../dolphinscheduler/service/cron/CronUtils.java | 20 +++++++++++++++++++-
.../dolphinscheduler/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 * * ? *"));
+ }
}
