Fix syntax for time in trigger/condition in DSL rule (#5591)

* Fix syntax for time in trigger/condition in DSL rule

Avoid conflict with switch-case statement

Time as "7:0" is now also supported.

Fix #5586

Signed-off-by: Laurent Garnier <lg.hc@free.fr>

* Optimize method buildMsgWithLineNb

Signed-off-by: Laurent Garnier <lg.hc@free.fr>

* Consider Copilot review comments

Signed-off-by: Laurent Garnier <lg.hc@free.fr>

* Consider human comments

Signed-off-by: Laurent Garnier <lg.hc@free.fr>

---------

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
This commit is contained in:
lolodomo
2026-05-23 21:19:28 +02:00
committed by GitHub
parent 83881e6ab3
commit 2169fe0323
5 changed files with 106 additions and 19 deletions
@@ -353,7 +353,7 @@ public class ModelRepositoryImpl implements ModelRepository {
case "rules":
if (d instanceof AbstractValidationDiagnostic vd
&& d.getSeverity() == org.eclipse.emf.common.util.Diagnostic.ERROR
&& "uid".equals(vd.getIssueCode())) {
&& ("uid".equals(vd.getIssueCode()) || "time".equals(vd.getIssueCode()))) {
errors.add(d.getMessage());
} else {
warnings.add(d.getMessage());
@@ -457,7 +457,7 @@ public class DSLRuleProvider
} else if ("midnight".equals(id)) {
cfg.put(TimeOfDayTriggerHandler.CFG_TIME, "00:00");
} else {
cfg.put(TimeOfDayTriggerHandler.CFG_TIME, id);
cfg.put(TimeOfDayTriggerHandler.CFG_TIME, formatTime(id));
}
yield TriggerBuilder.create().withId(Integer.toString(triggerId++))
.withTypeUID(TimeOfDayTriggerHandler.MODULE_TYPE_ID).withConfiguration(cfg).build();
@@ -505,8 +505,8 @@ public class DSLRuleProvider
Configuration cfg = new Configuration();
return switch (condition) {
case TimeOfDayCondition todCond -> {
cfg.put(TimeOfDayConditionHandler.CFG_START_TIME, todCond.getStart());
cfg.put(TimeOfDayConditionHandler.CFG_END_TIME, todCond.getEnd());
cfg.put(TimeOfDayConditionHandler.CFG_START_TIME, formatTime(todCond.getStart()));
cfg.put(TimeOfDayConditionHandler.CFG_END_TIME, formatTime(todCond.getEnd()));
yield ConditionBuilder.create().withId(Integer.toString(triggerId++))
.withTypeUID(TimeOfDayConditionHandler.MODULE_TYPE_ID).withConfiguration(cfg).build();
}
@@ -588,6 +588,21 @@ public class DSLRuleProvider
};
}
/**
* Format a time to have hour and minute on two characters with leading 0.
* "8:5" => "08:05"
* "10:25" => "10:25"
*
* @param time a valid time with hour and minute having the format "<int>:<int>"
* @return the time "<hour>:<minute>" with <hour> and <minute> on two characters with leading 0
*/
private String formatTime(String time) {
String[] splittedTime = time.split(":", 2);
int hour = Integer.parseInt(splittedTime[0]);
int minute = Integer.parseInt(splittedTime[1]);
return "%02d:%02d".formatted(hour, minute);
}
@Override
public void onReadyMarkerAdded(ReadyMarker readyMarker) {
for (String modelFileName : modelRepository.getAllModelNamesOfType("rules")) {
@@ -64,7 +64,7 @@ EventEmittedTrigger:
TimerTrigger:
'Time' 'cron' cron=STRING |
'Time' 'is' time=('midnight' | 'noon' | TIME)
'Time' 'is' time=('midnight' | 'noon' | Time)
;
DateTimeTrigger:
@@ -112,7 +112,7 @@ Condition:
;
TimeOfDayCondition:
'Time' 'is' 'between' start=TIME 'and' end=TIME
'Time' 'is' 'between' start=Time 'and' end=Time
;
DayOfWeekCondition:
@@ -207,8 +207,8 @@ Operator:
'=' | '!=' | '>' | '>=' | '<' | '<' '='
;
terminal TIME:
(('0'..'1') ('0'..'9') ':' ('0'..'5') ('0'..'9')) | ('2' ('0'..'3') ':' ('0'..'5') ('0'..'9'))
Time:
INT ':' INT
;
SIGNED_INT:
@@ -12,15 +12,22 @@
*/
package org.openhab.core.model.rule.validation;
import java.util.List;
import java.util.Set;
import org.eclipse.emf.ecore.EAttribute;
import org.eclipse.emf.ecore.EObject;
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.eclipse.xtext.nodemodel.ICompositeNode;
import org.eclipse.xtext.nodemodel.INode;
import org.eclipse.xtext.nodemodel.util.NodeModelUtils;
import org.eclipse.xtext.validation.Check;
import org.openhab.core.automation.util.RuleUtil;
import org.openhab.core.model.rule.rules.Rule;
import org.openhab.core.model.rule.rules.RulesPackage;
import org.openhab.core.model.rule.rules.TimeOfDayCondition;
import org.openhab.core.model.rule.rules.TimerTrigger;
/**
* This class contains custom validation rules.
@@ -32,6 +39,8 @@ import org.openhab.core.model.rule.rules.RulesPackage;
@NonNullByDefault
public class RulesValidator extends AbstractRulesValidator {
private static final Set<String> ALLOWED_TIME_NAMES = Set.of("midnight", "noon");
@Check
public void checkRuleUID(@Nullable Rule rule) {
String uid;
@@ -39,17 +48,80 @@ public class RulesValidator extends AbstractRulesValidator {
return;
}
if (!RuleUtil.isValidRuleUID(uid)) {
error(buildMsgWithLineNb(rule, "Rule UID '" + uid
+ "' is invalid. A rule UID can't contain '/', '\\' or have leading or trailing whitespace."), rule,
RulesPackage.Literals.RULE__UID, "uid");
error(buildMsgWithLineNb("Rule UID '" + uid
+ "' is invalid. A rule UID can't contain '/', '\\' or have leading or trailing whitespace.", rule,
RulesPackage.Literals.RULE__UID),
RulesPackage.Literals.RULE.getEStructuralFeature(RulesPackage.RULE__UID), "uid");
}
}
private static String buildMsgWithLineNb(EObject object, String msg) {
ICompositeNode node = NodeModelUtils.getNode(object);
if (node == null) {
return msg;
@Check
public void checkTimerTrigger(@Nullable TimerTrigger timeTrigger) {
String time;
if (timeTrigger == null || (time = timeTrigger.getTime()) == null) {
return;
}
if (ALLOWED_TIME_NAMES.contains(time)) {
return;
}
if (!isValidTime(time)) {
error(buildMsgWithLineNb("time '" + time
+ "' in trigger is invalid. It must comply with (H)H:MM format with a range of 0 to 23 for the hour and a range of 0 to 59 for the minute.",
timeTrigger, RulesPackage.Literals.TIMER_TRIGGER__TIME),
RulesPackage.Literals.TIMER_TRIGGER.getEStructuralFeature(RulesPackage.TIMER_TRIGGER__TIME),
"time");
}
}
@Check
public void checkTimeOfDayCondition(@Nullable TimeOfDayCondition timeOfDayCondition) {
String start, end;
if (timeOfDayCondition == null || (start = timeOfDayCondition.getStart()) == null
|| (end = timeOfDayCondition.getEnd()) == null) {
return;
}
if (!isValidTime(start)) {
error(buildMsgWithLineNb("start time '" + start
+ "' in condition is invalid. It must comply with (H)H:MM format with a range of 0 to 23 for the hour and a range of 0 to 59 for the minute.",
timeOfDayCondition, RulesPackage.Literals.TIME_OF_DAY_CONDITION__START),
RulesPackage.Literals.TIME_OF_DAY_CONDITION
.getEStructuralFeature(RulesPackage.TIME_OF_DAY_CONDITION__START),
"time");
}
if (!isValidTime(end)) {
error(buildMsgWithLineNb("end time '" + end
+ "' in condition is invalid. It must comply with (H)H:MM format with a range of 0 to 23 for the hour and a range of 0 to 59 for the minute.",
timeOfDayCondition, RulesPackage.Literals.TIME_OF_DAY_CONDITION__END),
RulesPackage.Literals.TIME_OF_DAY_CONDITION
.getEStructuralFeature(RulesPackage.TIME_OF_DAY_CONDITION__END),
"time");
}
}
private static boolean isValidTime(String time) {
String[] splittedTime = time.split(":", 3);
if (splittedTime.length != 2) {
return false;
}
try {
int hour = Integer.parseInt(splittedTime[0]);
int minute = Integer.parseInt(splittedTime[1]);
return hour >= 0 && hour <= 23 && minute >= 0 && minute <= 59;
} catch (NumberFormatException e) {
return false;
}
}
private static String buildMsgWithLineNb(String msg, EObject object, EAttribute attribute) {
List<INode> nodes = NodeModelUtils.findNodesForFeature(object, attribute);
if (nodes != null && nodes.size() >= 1) {
return buildMsgWithLineNb(msg, nodes.getFirst());
}
ICompositeNode node = NodeModelUtils.getNode(object);
return node != null ? buildMsgWithLineNb(msg, node) : msg;
}
private static String buildMsgWithLineNb(String msg, INode node) {
int startLine = node.getStartLine();
int endLine = node.getEndLine();
return (startLine == endLine) ? "Line " + startLine + ": " + msg
@@ -194,7 +194,7 @@ public class DSLRuleProviderTest extends JavaOSGiTest {
" System started or\n" + //
" Time is noon or\n" + //
" Time is midnight or\n" + //
" Time is 20:57 or\n" + //
" Time is 8:57 or\n" + //
" Time cron \"0 0/1 * * * ?\" or\n" + //
" Item X received command ON or\n" + //
" Item Y received update \"A\" or\n" + //
@@ -233,7 +233,7 @@ public class DSLRuleProviderTest extends JavaOSGiTest {
assertThat(rule.getTriggers().get(2).getConfiguration().get(TimeOfDayTriggerHandler.CFG_TIME), is("00:00"));
assertThat(rule.getTriggers().get(3).getTypeUID(), is(TimeOfDayTriggerHandler.MODULE_TYPE_ID));
assertThat(rule.getTriggers().get(3).getConfiguration().get(TimeOfDayTriggerHandler.CFG_TIME), is("20:57"));
assertThat(rule.getTriggers().get(3).getConfiguration().get(TimeOfDayTriggerHandler.CFG_TIME), is("08:57"));
assertThat(rule.getTriggers().get(4).getTypeUID(), is(GenericCronTriggerHandler.MODULE_TYPE_ID));
assertThat(rule.getTriggers().get(4).getConfiguration().get(GenericCronTriggerHandler.CFG_CRON_EXPRESSION),
@@ -298,7 +298,7 @@ public class DSLRuleProviderTest extends JavaOSGiTest {
"when\n" + //
" Item X received command ON\n" + //
"but only if\n" + //
" Time is between 19:20 and 22:10 and\n" + //
" Time is between 7:20 and 22:10 and\n" + //
" Day is Thursday, Tuesday, Sunday and\n" + //
" Day is weekday and\n" + //
" Day is weekend and\n" + //
@@ -332,7 +332,7 @@ public class DSLRuleProviderTest extends JavaOSGiTest {
assertThat(rule.getConditions().getFirst().getTypeUID(), is(TimeOfDayConditionHandler.MODULE_TYPE_ID));
assertThat(rule.getConditions().getFirst().getConfiguration().get(TimeOfDayConditionHandler.CFG_START_TIME),
is("19:20"));
is("07:20"));
assertThat(rule.getConditions().getFirst().getConfiguration().get(TimeOfDayConditionHandler.CFG_END_TIME),
is("22:10"));