Skip to content

Commit 68a2371

Browse files
jimtngjimtng
committed
DSL Items Parser: Fix incorrect parsing of tags as start of item definition
When specifying tag names that match one of the valid item types, e.g. `Switch`, the parser incorrectly treated it as a new start of an item definition. Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>
1 parent a4c6b49 commit 68a2371

10 files changed

Lines changed: 133 additions & 58 deletions

File tree

‎bundles/org.openhab.core.model.item/bnd.bnd‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ Import-Package: javax.measure,\
2222
org.openhab.core.items,\
2323
org.openhab.core.items.dto,\
2424
org.openhab.core.items.fileconverter,\
25+
org.openhab.core.library,\
2526
org.openhab.core.library.items,\
2627
org.openhab.core.library.types,\
2728
org.openhab.core.thing.util,\

‎bundles/org.openhab.core.model.item/src/org/openhab/core/model/Items.xtext‎

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,28 +17,30 @@ ModelItem:
1717
(ModelNormalItem | ModelGroupItem) name=ID
1818
(label=STRING)?
1919
('<' icon=Icon '>')?
20-
('(' groups+=ID (',' groups+=ID)* ')')?
20+
('(' groups+=ID (',' groups+=ID)* ')')?
2121
('[' tags+=(ID|STRING) (',' tags+=(ID|STRING))* ']')?
22-
('{' bindings+=ModelBinding (',' bindings+=ModelBinding)* '}')?
22+
('{' bindings+=ModelBinding (',' bindings+=ModelBinding)* '}')?
2323
;
2424

2525
ModelGroupItem:
26-
{ModelGroupItem} 'Group' (':' type=ModelItemType ( ':' function=ModelGroupFunction ('(' args+=(ID|STRING) (',' args+=(ID|STRING))* ')')?)?)?
27-
;
28-
29-
enum ModelGroupFunction:
30-
EQUALITY='EQUALITY' | AND='AND' | OR='OR' | NAND='NAND' | NOR='NOR' | XOR='XOR' | AVG='AVG' | MEDIAN='MEDIAN' | SUM='SUM' | MAX='MAX' | MIN='MIN' | COUNT='COUNT' | LATEST='LATEST' | EARLIEST='EARLIEST'
26+
{ModelGroupItem} 'Group' (':' type=ModelItemType ( ':' function=ID ('(' args+=(ID|STRING) (',' args+=(ID|STRING))* ')'))?)?
3127
;
3228

3329
ModelNormalItem:
34-
type=ModelItemType
30+
type=ModelItemType
3531
;
3632

33+
// Avoid using item type and group function literals in xtext grammar.
34+
// They cause misparsing when encountered in a different context (tag, icon, metadata, item name).
35+
// e.g. if `Switch` is defined here as a literal, "Switch MySwitch [Switch]" or "Switch AVERAGE"
36+
// will cause a parsing error.
37+
//
38+
// Instead, the validation is performed in ItemsValidator.xtend
39+
//
40+
// The downside: group function may be misread as a dimension in some cases, e.g. "Group:Number:MAX".
41+
// However, this is resolved in GenericItemProvider.
3742
ModelItemType:
38-
BaseModelItemType | ('Number' (':' ID)?)
39-
;
40-
BaseModelItemType:
41-
'Switch' | 'Rollershutter' | 'String' | 'Dimmer' | 'Contact' | 'DateTime' | 'Color' | 'Player' | 'Location' | 'Call' | 'Image'
43+
ID (':' ID)?
4244
;
4345

4446
ModelBinding:
@@ -56,7 +58,7 @@ ValueType returns ecore::EJavaObject:
5658
STRING | NUMBER | BOOLEAN
5759
;
5860

59-
BOOLEAN returns ecore::EBoolean:
61+
BOOLEAN returns ecore::EBoolean:
6062
'true' | 'false'
6163
;
6264

‎bundles/org.openhab.core.model.item/src/org/openhab/core/model/item/internal/GenericItemProvider.java‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@
4545
import org.openhab.core.model.item.BindingConfigReader;
4646
import org.openhab.core.model.items.ItemModel;
4747
import org.openhab.core.model.items.ModelBinding;
48-
import org.openhab.core.model.items.ModelGroupFunction;
4948
import org.openhab.core.model.items.ModelGroupItem;
5049
import org.openhab.core.model.items.ModelItem;
5150
import org.openhab.core.model.items.ModelNormalItem;
@@ -224,17 +223,29 @@ private void processBindingConfigsFromModel(String modelName, EventType type) {
224223
private @Nullable Item createItemFromModelItem(ModelItem modelItem) {
225224
Item item;
226225
if (modelItem instanceof ModelGroupItem modelGroupItem) {
226+
logger.warn("modelItem: {}", modelGroupItem);
227+
String itemType = modelGroupItem.getType();
228+
String function = modelGroupItem.getFunction();
229+
if (function == null && itemType != null) {
230+
String itemTypeExtension = ItemUtil.getItemTypeExtension(itemType);
231+
if (itemTypeExtension != null && GroupFunction.VALID_FUNCTIONS.contains(itemTypeExtension)) {
232+
function = itemTypeExtension;
233+
itemType = ItemUtil.getMainItemType(itemType);
234+
}
235+
}
236+
227237
Item baseItem;
228238
try {
229-
baseItem = createItemOfType(modelGroupItem.getType(), modelGroupItem.getName());
239+
baseItem = createItemOfType(itemType, modelGroupItem.getName());
230240
} catch (IllegalArgumentException e) {
231241
logger.debug("Error creating base item for group item '{}', item will be ignored: {}",
232242
modelGroupItem.getName(), e.getMessage());
233243
return null;
234244
}
235245
if (baseItem != null) {
236-
// if the user did not specify a function the first value of the enum in xtext (EQUAL) will be used
237-
ModelGroupFunction function = modelGroupItem.getFunction();
246+
if (function == null || function.isEmpty()) {
247+
function = GroupFunction.DEFAULT;
248+
}
238249
item = applyGroupFunction(baseItem, modelGroupItem, function);
239250
} else {
240251
item = new GroupItem(modelGroupItem.getName());
@@ -286,9 +297,9 @@ private void assignTags(ModelItem modelItem, ActiveItem item) {
286297
}
287298
}
288299

289-
private GroupItem applyGroupFunction(Item baseItem, ModelGroupItem modelGroupItem, ModelGroupFunction function) {
300+
private GroupItem applyGroupFunction(Item baseItem, ModelGroupItem modelGroupItem, String function) {
290301
GroupFunctionDTO dto = new GroupFunctionDTO();
291-
dto.name = function.getName();
302+
dto.name = function;
292303
dto.params = modelGroupItem.getArgs().toArray(new String[0]);
293304

294305
GroupFunction groupFunction = ItemDTOMapper.mapFunction(baseItem, dto);

‎bundles/org.openhab.core.model.item/src/org/openhab/core/model/item/internal/fileconverter/DslItemFileConverter.java‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@
4040
import org.openhab.core.model.items.ItemModel;
4141
import org.openhab.core.model.items.ItemsFactory;
4242
import org.openhab.core.model.items.ModelBinding;
43-
import org.openhab.core.model.items.ModelGroupFunction;
4443
import org.openhab.core.model.items.ModelGroupItem;
4544
import org.openhab.core.model.items.ModelItem;
4645
import org.openhab.core.model.items.ModelProperty;
@@ -104,9 +103,7 @@ private ModelItem buildModelItem(Item item, List<Metadata> channelLinks, List<Me
104103
modelGroup.setType(baseItem.getType());
105104
GroupFunction function = groupItem.getFunction();
106105
if (function != null) {
107-
ModelGroupFunction modelFunction = ModelGroupFunction
108-
.getByName(function.getClass().getSimpleName().toUpperCase());
109-
modelGroup.setFunction(modelFunction);
106+
modelGroup.setFunction(function.getClass().getSimpleName().toUpperCase());
110107
State[] parameters = function.getParameters();
111108
for (int i = 0; i < parameters.length; i++) {
112109
modelGroup.getArgs().add(parameters[i].toString());

‎bundles/org.openhab.core.model.item/src/org/openhab/core/model/validation/ItemsValidator.xtend‎

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,20 @@
1212
*/
1313
package org.openhab.core.model.validation
1414

15-
import org.openhab.core.model.items.ModelItem
15+
import java.util.regex.Pattern
16+
1617
import org.eclipse.xtext.validation.Check
18+
import org.openhab.core.items.GroupFunction
19+
import org.openhab.core.items.ItemUtil
20+
import org.openhab.core.library.CoreItemFactory
1721
import org.openhab.core.model.items.ItemsPackage
22+
import org.openhab.core.model.items.ModelGroupItem
23+
import org.openhab.core.model.items.ModelItem
24+
import org.openhab.core.model.items.ModelNormalItem
1825
import org.openhab.core.types.util.UnitUtils
1926

2027
/**
21-
* Custom validation rules.
28+
* Custom validation rules.
2229
*
2330
* see http://www.eclipse.org/Xtext/documentation.html#validation
2431
*/
@@ -29,23 +36,42 @@ class ItemsValidator extends AbstractItemsValidator {
2936
if (item === null || item.name === null) {
3037
return
3138
}
32-
if (item.name.contains("-")) {
33-
error('Item name must not contain dashes.', ItemsPackage.Literals.MODEL_ITEM__NAME)
34-
}
39+
if (!ItemUtil.isValidItemName(item.name)) {
40+
error('Item name "' + item.name + '" is invalid. It must begin with a letter or underscore, and must not contain symbols or dashes.', ItemsPackage.Literals.MODEL_ITEM__NAME)
41+
}
3542
}
36-
43+
3744
@Check
3845
def checkDimension(ModelItem item) {
3946
if (item === null || item.type === null) {
4047
return
4148
}
4249
if (item.type.startsWith("Number:")) {
4350
var dimension = item.type.substring(item.type.indexOf(":") + 1)
51+
if (item instanceof ModelGroupItem && GroupFunction.VALID_FUNCTIONS.contains(dimension)) {
52+
// The Xtext cannot differentiate between a dimension and a group function
53+
// We'll fix this up in the GenericItemProvider
54+
return
55+
}
4456
try {
45-
UnitUtils.parseDimension(dimension)
57+
UnitUtils.parseDimension(dimension)
4658
} catch (IllegalArgumentException e) {
4759
warning("'" + dimension + "' is not a valid dimension.", ItemsPackage.Literals.MODEL_ITEM__TYPE)
4860
}
4961
}
5062
}
63+
64+
@Check
65+
def checkValidItemType(ModelNormalItem item) {
66+
if (!CoreItemFactory.SUPPORTED_ITEM_TYPES.contains(item.type) && !item.type.startsWith("Number:")) {
67+
error("Invalid item type: " + item.type, ItemsPackage.Literals.MODEL_ITEM__TYPE)
68+
}
69+
}
70+
71+
@Check
72+
def checkValidGroupFunction(ModelGroupItem item) {
73+
if (item.function !== null && !GroupFunction.VALID_FUNCTIONS.contains(item.function)) {
74+
error("Invalid group function: " + item.function, ItemsPackage.Literals.MODEL_GROUP_ITEM__FUNCTION)
75+
}
76+
}
5177
}

‎bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/items/YamlGroupDTO.java‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,10 @@
1414

1515
import java.util.List;
1616
import java.util.Objects;
17-
import java.util.Set;
1817

1918
import org.eclipse.jdt.annotation.NonNull;
2019
import org.eclipse.jdt.annotation.Nullable;
20+
import org.openhab.core.items.GroupFunction;
2121
import org.openhab.core.model.yaml.internal.util.YamlElementUtils;
2222

2323
/**
@@ -28,10 +28,6 @@
2828
*/
2929
public class YamlGroupDTO {
3030

31-
private static final String DEFAULT_FUNCTION = "EQUALITY";
32-
private static final Set<String> VALID_FUNCTIONS = Set.of("AND", "OR", "NAND", "NOR", "XOR", "COUNT", "AVG",
33-
"MEDIAN", "SUM", "MIN", "MAX", "LATEST", "EARLIEST", DEFAULT_FUNCTION);
34-
3531
public String type;
3632
public String dimension;
3733
public String function;
@@ -53,7 +49,7 @@ public boolean isValid(@NonNull List<@NonNull String> errors, @NonNull List<@Non
5349
} else if (dimension != null) {
5450
warnings.add("\"dimension\" field in group ignored as type is not Number");
5551
}
56-
if (!VALID_FUNCTIONS.contains(getFunction())) {
52+
if (!GroupFunction.VALID_FUNCTIONS.contains(getFunction())) {
5753
errors.add("invalid value \"%s\" for \"function\" field".formatted(function));
5854
ok = false;
5955
}
@@ -65,7 +61,7 @@ public boolean isValid(@NonNull List<@NonNull String> errors, @NonNull List<@Non
6561
}
6662

6763
public String getFunction() {
68-
return function != null ? function.toUpperCase() : DEFAULT_FUNCTION;
64+
return function != null ? function.toUpperCase() : GroupFunction.DEFAULT;
6965
}
7066

7167
@Override

‎bundles/org.openhab.core/src/main/java/org/openhab/core/internal/items/GroupFunctionHelper.java‎

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -82,15 +82,15 @@ private GroupFunction createDimensionGroupFunction(GroupFunctionDTO function, Nu
8282
Unit<?> baseItemUnit = baseItem.getUnit();
8383
if (baseItemUnit != null) {
8484
switch (functionName.toUpperCase()) {
85-
case "AVG":
85+
case GroupFunction.AVG:
8686
return new QuantityTypeArithmeticGroupFunction.Avg(baseItemUnit);
87-
case "MEDIAN":
87+
case GroupFunction.MEDIAN:
8888
return new QuantityTypeArithmeticGroupFunction.Median(baseItemUnit);
89-
case "SUM":
89+
case GroupFunction.SUM:
9090
return new QuantityTypeArithmeticGroupFunction.Sum(baseItemUnit);
91-
case "MIN":
91+
case GroupFunction.MIN:
9292
return new QuantityTypeArithmeticGroupFunction.Min(baseItemUnit);
93-
case "MAX":
93+
case GroupFunction.MAX:
9494
return new QuantityTypeArithmeticGroupFunction.Max(baseItemUnit);
9595
default:
9696
}
@@ -102,69 +102,69 @@ private GroupFunction createDefaultGroupFunction(GroupFunctionDTO function, @Nul
102102
final String functionName = function.name;
103103
final List<State> args;
104104
switch (functionName.toUpperCase()) {
105-
case "AND":
105+
case GroupFunction.AND:
106106
args = parseStates(baseItem, function.params);
107107
if (args.size() == 2) {
108108
return new ArithmeticGroupFunction.And(args.getFirst(), args.get(1));
109109
} else {
110110
logger.error("Group function 'AND' requires two arguments. Using Equality instead.");
111111
}
112112
break;
113-
case "OR":
113+
case GroupFunction.OR:
114114
args = parseStates(baseItem, function.params);
115115
if (args.size() == 2) {
116116
return new ArithmeticGroupFunction.Or(args.getFirst(), args.get(1));
117117
} else {
118118
logger.error("Group function 'OR' requires two arguments. Using Equality instead.");
119119
}
120120
break;
121-
case "NAND":
121+
case GroupFunction.NAND:
122122
args = parseStates(baseItem, function.params);
123123
if (args.size() == 2) {
124124
return new ArithmeticGroupFunction.NAnd(args.getFirst(), args.get(1));
125125
} else {
126126
logger.error("Group function 'NOT AND' requires two arguments. Using Equality instead.");
127127
}
128128
break;
129-
case "NOR":
129+
case GroupFunction.NOR:
130130
args = parseStates(baseItem, function.params);
131131
if (args.size() == 2) {
132132
return new ArithmeticGroupFunction.NOr(args.getFirst(), args.get(1));
133133
} else {
134134
logger.error("Group function 'NOT OR' requires two arguments. Using Equality instead.");
135135
}
136136
break;
137-
case "XOR":
137+
case GroupFunction.XOR:
138138
args = parseStates(baseItem, function.params);
139139
if (args.size() == 2) {
140140
return new ArithmeticGroupFunction.Xor(args.getFirst(), args.get(1));
141141
} else {
142142
logger.error("Group function 'XOR' requires two arguments. Using Equality instead.");
143143
}
144144
break;
145-
case "COUNT":
145+
case GroupFunction.COUNT:
146146
if (function.params != null && function.params.length == 1) {
147147
State countParam = new StringType(function.params[0]);
148148
return new ArithmeticGroupFunction.Count(countParam);
149149
} else {
150150
logger.error("Group function 'COUNT' requires one argument. Using Equality instead.");
151151
}
152152
break;
153-
case "AVG":
153+
case GroupFunction.AVG:
154154
return new ArithmeticGroupFunction.Avg();
155-
case "MEDIAN":
155+
case GroupFunction.MEDIAN:
156156
return new ArithmeticGroupFunction.Median();
157-
case "SUM":
157+
case GroupFunction.SUM:
158158
return new ArithmeticGroupFunction.Sum();
159-
case "MIN":
159+
case GroupFunction.MIN:
160160
return new ArithmeticGroupFunction.Min();
161-
case "MAX":
161+
case GroupFunction.MAX:
162162
return new ArithmeticGroupFunction.Max();
163-
case "LATEST":
163+
case GroupFunction.LATEST:
164164
return new DateTimeGroupFunction.Latest();
165-
case "EARLIEST":
165+
case GroupFunction.EARLIEST:
166166
return new DateTimeGroupFunction.Earliest();
167-
case "EQUALITY":
167+
case GroupFunction.EQUALITY:
168168
return new GroupFunction.Equality();
169169
default:
170170
logger.error("Unknown group function '{}'. Using Equality instead.", functionName);

‎bundles/org.openhab.core/src/main/java/org/openhab/core/items/GroupFunction.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,26 @@
2828
*/
2929
@NonNullByDefault
3030
public interface GroupFunction {
31+
String EQUALITY = "EQUALITY";
32+
String AND = "AND";
33+
String OR = "OR";
34+
String NAND = "NAND";
35+
String NOR = "NOR";
36+
String XOR = "XOR";
37+
String COUNT = "COUNT";
38+
String AVG = "AVG";
39+
String MEDIAN = "MEDIAN";
40+
String SUM = "SUM";
41+
String MIN = "MIN";
42+
String MAX = "MAX";
43+
String LATEST = "LATEST";
44+
String EARLIEST = "EARLIEST";
45+
46+
String DEFAULT = EQUALITY;
47+
48+
Set<String> VALID_FUNCTIONS = Set.of( //
49+
EQUALITY, AND, OR, NAND, NOR, XOR, COUNT, AVG, MEDIAN, SUM, MIN, MAX, LATEST, EARLIEST //
50+
);
3151

3252
/**
3353
* Determines the current state of a group based on a list of items

0 commit comments

Comments
 (0)