diff --git a/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/module/handler/GenericCronTriggerHandler.java b/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/module/handler/GenericCronTriggerHandler.java index 6988f6f9310..f289dcab5f3 100644 --- a/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/module/handler/GenericCronTriggerHandler.java +++ b/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/module/handler/GenericCronTriggerHandler.java @@ -68,9 +68,13 @@ public synchronized void setCallback(ModuleHandlerCallback callback) { } private void scheduleJob() { - schedule = scheduler.schedule(this, expression); - logger.debug("Scheduled cron job '{}' for trigger '{}'.", module.getConfiguration().get(CFG_CRON_EXPRESSION), - module.getId()); + try { + schedule = scheduler.schedule(this, expression); + logger.debug("Scheduled cron job '{}' for trigger '{}'.", + module.getConfiguration().get(CFG_CRON_EXPRESSION), module.getId()); + } catch (IllegalArgumentException e) { // Catch exception from CronAdjuster + logger.warn("Failed to schedule job for trigger '{}'. {}", module.getId(), e.getMessage()); + } } @Override diff --git a/bundles/org.openhab.core/src/main/java/org/openhab/core/scheduler/CronAdjuster.java b/bundles/org.openhab.core/src/main/java/org/openhab/core/scheduler/CronAdjuster.java index 15f25a717b8..2798f27d543 100644 --- a/bundles/org.openhab.core/src/main/java/org/openhab/core/scheduler/CronAdjuster.java +++ b/bundles/org.openhab.core/src/main/java/org/openhab/core/scheduler/CronAdjuster.java @@ -12,6 +12,7 @@ */ package org.openhab.core.scheduler; +import java.time.DateTimeException; import java.time.DayOfWeek; import java.time.temporal.ChronoField; import java.time.temporal.ChronoUnit; @@ -71,7 +72,6 @@ interface Checker { .collect(Collectors.toMap(Entry::getKey, Entry::getValue)); private final List fields = new ArrayList<>(7); - private String cronExpression; private final Map environmentMap; private final boolean reboot; @@ -83,7 +83,6 @@ public CronAdjuster(final String specification) { environmentMap = parseEnvironment(entries); String cronExpression = entries[entries.length - 1].trim(); - this.cronExpression = cronExpression; reboot = "@reboot".equals(cronExpression); @@ -108,6 +107,14 @@ public CronAdjuster(final String specification) { parseAndAdd(cronExpression, parts[2], ChronoField.HOUR_OF_DAY); parseAndAdd(cronExpression, parts[1], ChronoField.MINUTE_OF_HOUR); parseAndAdd(cronExpression, parts[0], ChronoField.SECOND_OF_MINUTE); + + try { + // Test the cron expression in action to make sure it won't cause too many restarts + adjustInto(java.time.ZonedDateTime.now()); + } catch (final DateTimeException e) { + throw new IllegalArgumentException( + String.format("Invalid cron expression '%s': %s", cronExpression, e.getMessage())); + } } /** @@ -529,8 +536,7 @@ public Temporal adjustInto(@Nullable final Temporal temporal) { index++; } else { if (restarts++ > 1000) { - throw new IllegalArgumentException( - String.format("Cron expression '%s' is not valid, too many restarts", cronExpression)); + throw new DateTimeException("Conditions not satisfied."); } ret = out; index = 0; diff --git a/bundles/org.openhab.core/src/test/java/org/openhab/core/internal/scheduler/CronAdjusterTest.java b/bundles/org.openhab.core/src/test/java/org/openhab/core/internal/scheduler/CronAdjusterTest.java index 0c3cbaa227a..095d064a34e 100644 --- a/bundles/org.openhab.core/src/test/java/org/openhab/core/internal/scheduler/CronAdjusterTest.java +++ b/bundles/org.openhab.core/src/test/java/org/openhab/core/internal/scheduler/CronAdjusterTest.java @@ -203,9 +203,6 @@ public void testCronExpression(String in, String cron, String[] outs) { @ParameterizedTest @ValueSource(strings = { "0 0 0 31 2 *", "* * *", "80 * * * * *" }) public void testInvalidCronExpression(String cron) { - assertThrows(IllegalArgumentException.class, () -> { - final CronAdjuster cronAdjuster = new CronAdjuster(cron); - cronAdjuster.adjustInto(java.time.ZonedDateTime.now()); - }); + assertThrows(IllegalArgumentException.class, () -> new CronAdjuster(cron)); } }