Skip to content

Commit 7c848bf

Browse files
jimtngjimtng
authored
DSL Items Parser: Fix incorrect parsing of keywords encountered in the wrong context (#4928)
* DSL Items Parser: Fix incorrect parsing of keywords encountered in the wrong context 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 00b6a47 commit 7c848bf

16 files changed

Lines changed: 359 additions & 169 deletions

File tree

‎bundles/org.openhab.core.model.core/src/main/java/org/openhab/core/model/core/internal/ModelRepositoryImpl.java‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,14 @@ private boolean validateModel(String name, InputStream inputStream, List<String>
330330
final org.eclipse.emf.common.util.Diagnostic diagnostic = safeEmf
331331
.call(() -> Diagnostician.INSTANCE.validate(resource.getContents().getFirst()));
332332
for (org.eclipse.emf.common.util.Diagnostic d : diagnostic.getChildren()) {
333-
warnings.add(d.getMessage());
333+
if (d.getSeverity() == org.eclipse.emf.common.util.Diagnostic.ERROR) {
334+
errors.add(d.getMessage());
335+
} else {
336+
warnings.add(d.getMessage());
337+
}
338+
}
339+
if (!errors.isEmpty()) {
340+
return false;
334341
}
335342
} catch (NullPointerException e) {
336343
// see https://github.com/eclipse/smarthome/issues/3335

‎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: 7 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -14,31 +14,18 @@ ItemModel:
1414
;
1515

1616
ModelItem:
17-
(ModelNormalItem | ModelGroupItem) name=ID
17+
type=ModelItemType ('(' args+=(ID|STRING) (',' args+=(ID|STRING))* ')')?
18+
name=ID
1819
(label=STRING)?
1920
('<' icon=Icon '>')?
20-
('(' groups+=ID (',' groups+=ID)* ')')?
21+
('(' groups+=ID (',' groups+=ID)* ')')?
2122
('[' tags+=(ID|STRING) (',' tags+=(ID|STRING))* ']')?
22-
('{' bindings+=ModelBinding (',' bindings+=ModelBinding)* '}')?
23-
;
24-
25-
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'
31-
;
32-
33-
ModelNormalItem:
34-
type=ModelItemType
23+
('{' bindings+=ModelBinding (',' bindings+=ModelBinding)* '}')?
3524
;
3625

26+
// Supports item types with up to 4 colon-separated segments, e.g., Group:Number:Dimension:MAX
3727
ModelItemType:
38-
BaseModelItemType | ('Number' (':' ID)?)
39-
;
40-
BaseModelItemType:
41-
'Switch' | 'Rollershutter' | 'String' | 'Dimmer' | 'Contact' | 'DateTime' | 'Color' | 'Player' | 'Location' | 'Call' | 'Image'
28+
ID (':' ID (':' ID (':' ID )?)?)?
4229
;
4330

4431
ModelBinding:
@@ -56,7 +43,7 @@ ValueType returns ecore::EJavaObject:
5643
STRING | NUMBER | BOOLEAN
5744
;
5845

59-
BOOLEAN returns ecore::EBoolean:
46+
BOOLEAN returns ecore::EBoolean:
6047
'true' | 'false'
6148
;
6249

‎bundles/org.openhab.core.model.item/src/org/openhab/core/model/formatting/ItemsFormatter.xtend‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,10 @@ class ItemsFormatter extends AbstractDeclarativeFormatter {
2525
@Inject extension ItemsGrammarAccess
2626

2727
override protected void configureFormatting(FormattingConfig c) {
28-
c.setLinewrap(1, 1, 2).before(modelGroupItemRule)
29-
c.setLinewrap(1, 1, 2).before(modelNormalItemRule)
28+
c.setLinewrap(1, 1, 2).before(modelItemRule)
29+
30+
// No space between the item type and the opening parenthesis for the group function arguments
31+
c.setNoSpace().between(modelItemTypeRule, modelItemAccess.leftParenthesisKeyword_1_0)
3032

3133
c.setNoSpace().withinKeywordPairs("<", ">")
3234
c.setNoSpace().withinKeywordPairs("(", ")")

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

Lines changed: 89 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,7 @@
4747
import org.openhab.core.model.item.BindingConfigReader;
4848
import org.openhab.core.model.items.ItemModel;
4949
import org.openhab.core.model.items.ModelBinding;
50-
import org.openhab.core.model.items.ModelGroupFunction;
51-
import org.openhab.core.model.items.ModelGroupItem;
5250
import org.openhab.core.model.items.ModelItem;
53-
import org.openhab.core.model.items.ModelNormalItem;
5451
import org.openhab.core.types.StateDescriptionFragment;
5552
import org.openhab.core.types.StateDescriptionFragmentBuilder;
5653
import org.openhab.core.types.StateDescriptionFragmentProvider;
@@ -234,62 +231,54 @@ private void processBindingConfigsFromModel(String modelName, EventType type) {
234231
}
235232

236233
private @Nullable Item createItemFromModelItem(ModelItem modelItem, String modelName) {
237-
Item item;
238-
if (modelItem instanceof ModelGroupItem modelGroupItem) {
239-
Item baseItem;
240-
try {
241-
baseItem = createItemOfType(modelGroupItem.getType(), modelGroupItem.getName());
242-
} catch (IllegalArgumentException e) {
243-
logger.debug("Error creating base item for group item '{}', item will be ignored: {}",
244-
modelGroupItem.getName(), e.getMessage());
245-
return null;
246-
}
247-
if (baseItem != null) {
248-
// if the user did not specify a function the first value of the enum in xtext (EQUAL) will be used
249-
ModelGroupFunction function = modelGroupItem.getFunction();
250-
item = applyGroupFunction(baseItem, modelGroupItem, function);
251-
} else {
252-
item = new GroupItem(modelGroupItem.getName());
253-
}
254-
} else {
255-
ModelNormalItem normalItem = (ModelNormalItem) modelItem;
256-
try {
257-
item = createItemOfType(normalItem.getType(), normalItem.getName());
258-
} catch (IllegalArgumentException e) {
259-
logger.debug("Error creating item '{}', item will be ignored: {}", normalItem.getName(),
260-
e.getMessage());
261-
return null;
262-
}
234+
String itemType = modelItem.getType();
235+
if (itemType == null || itemType.isBlank()) {
236+
logger.warn("Item '{}' has no type defined, ignoring it.", modelItem.getName());
237+
return null;
263238
}
264-
if (item instanceof ActiveItem activeItem) {
265-
String label = modelItem.getLabel();
266-
String format = extractFormat(label);
267-
if (format != null) {
268-
label = label.substring(0, label.indexOf("[")).trim();
269-
Map<String, String> formatters = Objects
270-
.requireNonNull(stateFormattersMap.computeIfAbsent(modelName, k -> new HashMap<>()));
271-
formatters.put(modelItem.getName(), format);
272-
if (!isIsolatedModel(modelName)) {
273-
stateDescriptionFragments.put(modelItem.getName(),
274-
StateDescriptionFragmentBuilder.create().withPattern(format).build());
275-
}
276-
} else {
277-
Map<String, String> formatters = stateFormattersMap.get(modelName);
278-
if (formatters != null) {
279-
formatters.remove(modelItem.getName());
280-
if (formatters.isEmpty()) {
281-
stateFormattersMap.remove(modelName);
239+
240+
try {
241+
String[] itemTypeSegments = itemType.split(ItemUtil.EXTENSION_SEPARATOR);
242+
String mainItemType = itemTypeSegments[0];
243+
244+
Item item = switch (mainItemType) {
245+
case "Group" -> createGroupItem(modelItem, itemTypeSegments);
246+
default -> createItemOfType(itemType, modelItem.getName());
247+
};
248+
249+
if (item instanceof ActiveItem activeItem) {
250+
String label = modelItem.getLabel();
251+
String format = extractFormat(label);
252+
if (format != null) {
253+
label = label.substring(0, label.indexOf("[")).trim();
254+
Map<String, String> formatters = Objects
255+
.requireNonNull(stateFormattersMap.computeIfAbsent(modelName, k -> new HashMap<>()));
256+
formatters.put(modelItem.getName(), format);
257+
if (!isIsolatedModel(modelName)) {
258+
stateDescriptionFragments.put(modelItem.getName(),
259+
StateDescriptionFragmentBuilder.create().withPattern(format).build());
260+
}
261+
} else {
262+
Map<String, String> formatters = stateFormattersMap.get(modelName);
263+
if (formatters != null) {
264+
formatters.remove(modelItem.getName());
265+
if (formatters.isEmpty()) {
266+
stateFormattersMap.remove(modelName);
267+
}
268+
}
269+
if (!isIsolatedModel(modelName)) {
270+
stateDescriptionFragments.remove(modelItem.getName());
282271
}
283272
}
284-
if (!isIsolatedModel(modelName)) {
285-
stateDescriptionFragments.remove(modelItem.getName());
286-
}
273+
activeItem.setLabel(label);
274+
activeItem.setCategory(modelItem.getIcon());
275+
assignTags(modelItem, activeItem);
276+
return item;
277+
} else {
278+
return null;
287279
}
288-
activeItem.setLabel(label);
289-
activeItem.setCategory(modelItem.getIcon());
290-
assignTags(modelItem, activeItem);
291-
return item;
292-
} else {
280+
} catch (IllegalArgumentException e) {
281+
logger.debug("Error creating item '{}', item will be ignored: {}", modelItem.getName(), e.getMessage());
293282
return null;
294283
}
295284
}
@@ -312,14 +301,14 @@ private void assignTags(ModelItem modelItem, ActiveItem item) {
312301
}
313302
}
314303

315-
private GroupItem applyGroupFunction(Item baseItem, ModelGroupItem modelGroupItem, ModelGroupFunction function) {
304+
private GroupItem applyGroupFunction(Item baseItem, ModelItem modelItem, String function) {
316305
GroupFunctionDTO dto = new GroupFunctionDTO();
317-
dto.name = function.getName();
318-
dto.params = modelGroupItem.getArgs().toArray(new String[0]);
306+
dto.name = function;
307+
dto.params = modelItem.getArgs().toArray(new String[0]);
319308

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

322-
return new GroupItem(modelGroupItem.getName(), baseItem, groupFunction);
311+
return new GroupItem(modelItem.getName(), baseItem, groupFunction);
323312
}
324313

325314
private void dispatchBindingsPerItemType(String[] itemTypes) {
@@ -529,6 +518,47 @@ private Map<String, Item> toItemMap(@Nullable Collection<Item> items) {
529518
return ret;
530519
}
531520

521+
/**
522+
* Creates a new GroupItem based on the given ModelItem and item type segments.
523+
*
524+
* @param modelItem The ModelItem to create the GroupItem from.
525+
* @param itemTypeSegments The segments of the item type.
526+
* @return A new GroupItem or null if the item type is invalid.
527+
*/
528+
private @Nullable GroupItem createGroupItem(ModelItem modelItem, String[] itemTypeSegments) {
529+
if (itemTypeSegments.length == 1) {
530+
// Just plain "Group" with no base type
531+
return new GroupItem(modelItem.getName());
532+
}
533+
534+
String function = GroupFunction.DEFAULT;
535+
536+
String baseItemType = switch (itemTypeSegments.length) {
537+
case 2 -> itemTypeSegments[1];
538+
case 3 -> {
539+
// 3 segments could either be Group:Type:Function, or Group:Number:Dimension -> Find out which one it is
540+
if (!modelItem.getArgs().isEmpty() || GroupFunction.VALID_FUNCTIONS.contains(itemTypeSegments[2])) {
541+
// It's Group:Type:Function because there are arguments or the third segment is a valid function
542+
function = itemTypeSegments[2];
543+
yield itemTypeSegments[1];
544+
} else {
545+
// Otherwise, it must be Group:Number:Dimension
546+
yield itemTypeSegments[1] + ItemUtil.EXTENSION_SEPARATOR + itemTypeSegments[2];
547+
}
548+
}
549+
case 4 -> {
550+
// 4 segments: "Group:Number:Dimension:Function"
551+
function = itemTypeSegments[3];
552+
yield itemTypeSegments[1] + ItemUtil.EXTENSION_SEPARATOR + itemTypeSegments[2];
553+
}
554+
default -> throw new IllegalArgumentException("Invalid group item type: " + modelItem.getType()
555+
+ ". Expected formats are 'Group', 'Group:Type', 'Group:Type:Function', or 'Group:Number:Dimension:Function' with a maximum of 4 segments.");
556+
};
557+
558+
Item baseItem = createItemOfType(baseItemType, modelItem.getName());
559+
return applyGroupFunction(baseItem, modelItem, function);
560+
}
561+
532562
/**
533563
* Creates a new item of type {@code itemType} by utilizing an appropriate {@link ItemFactory}.
534564
*

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

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import org.openhab.core.items.GroupFunction;
3636
import org.openhab.core.items.GroupItem;
3737
import org.openhab.core.items.Item;
38+
import org.openhab.core.items.ItemUtil;
3839
import org.openhab.core.items.Metadata;
3940
import org.openhab.core.items.fileconverter.AbstractItemFileGenerator;
4041
import org.openhab.core.items.fileconverter.ItemFileGenerator;
@@ -45,8 +46,6 @@
4546
import org.openhab.core.model.items.ItemModel;
4647
import org.openhab.core.model.items.ItemsFactory;
4748
import org.openhab.core.model.items.ModelBinding;
48-
import org.openhab.core.model.items.ModelGroupFunction;
49-
import org.openhab.core.model.items.ModelGroupItem;
5049
import org.openhab.core.model.items.ModelItem;
5150
import org.openhab.core.model.items.ModelProperty;
5251
import org.openhab.core.types.State;
@@ -115,26 +114,24 @@ public void generateFileFormat(String id, OutputStream out) {
115114

116115
private ModelItem buildModelItem(Item item, List<Metadata> channelLinks, List<Metadata> metadata,
117116
@Nullable String stateFormatter, boolean hideDefaultParameters) {
118-
ModelItem model;
117+
ModelItem model = ItemsFactory.eINSTANCE.createModelItem();
119118
if (item instanceof GroupItem groupItem) {
120-
ModelGroupItem modelGroup = ItemsFactory.eINSTANCE.createModelGroupItem();
121-
model = modelGroup;
122119
Item baseItem = groupItem.getBaseItem();
120+
List<String> groupType = new ArrayList<>();
121+
groupType.add(groupItem.getType());
123122
if (baseItem != null) {
124-
modelGroup.setType(baseItem.getType());
123+
groupType.add(baseItem.getType());
125124
GroupFunction function = groupItem.getFunction();
126125
if (function != null) {
127-
ModelGroupFunction modelFunction = ModelGroupFunction
128-
.getByName(function.getClass().getSimpleName().toUpperCase());
129-
modelGroup.setFunction(modelFunction);
126+
groupType.add(function.getClass().getSimpleName().toUpperCase());
130127
State[] parameters = function.getParameters();
131128
for (int i = 0; i < parameters.length; i++) {
132-
modelGroup.getArgs().add(parameters[i].toString());
129+
model.getArgs().add(parameters[i].toString());
133130
}
134131
}
135132
}
133+
model.setType(groupType.stream().collect(Collectors.joining(ItemUtil.EXTENSION_SEPARATOR)));
136134
} else {
137-
model = ItemsFactory.eINSTANCE.createModelNormalItem();
138135
model.setType(item.getType());
139136
}
140137

0 commit comments

Comments
 (0)