Skip to content

Commit 8ac4d0a

Browse files
jimtngjimtng
committed
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 6f7d46c commit 8ac4d0a

12 files changed

Lines changed: 309 additions & 124 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: 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: 71 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,7 @@
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;
49-
import org.openhab.core.model.items.ModelGroupItem;
5048
import org.openhab.core.model.items.ModelItem;
51-
import org.openhab.core.model.items.ModelNormalItem;
5249
import org.openhab.core.types.StateDescriptionFragment;
5350
import org.openhab.core.types.StateDescriptionFragmentBuilder;
5451
import org.openhab.core.types.StateDescriptionFragmentProvider;
@@ -222,48 +219,36 @@ private void processBindingConfigsFromModel(String modelName, EventType type) {
222219
}
223220

224221
private @Nullable Item createItemFromModelItem(ModelItem modelItem) {
225-
Item item;
226-
if (modelItem instanceof ModelGroupItem modelGroupItem) {
227-
Item baseItem;
228-
try {
229-
baseItem = createItemOfType(modelGroupItem.getType(), modelGroupItem.getName());
230-
} catch (IllegalArgumentException e) {
231-
logger.debug("Error creating base item for group item '{}', item will be ignored: {}",
232-
modelGroupItem.getName(), e.getMessage());
233-
return null;
234-
}
235-
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();
238-
item = applyGroupFunction(baseItem, modelGroupItem, function);
222+
String itemType = modelItem.getType();
223+
224+
try {
225+
String[] itemTypeSegments = itemType.split(ItemUtil.EXTENSION_SEPARATOR);
226+
String mainItemType = itemTypeSegments[0];
227+
228+
Item item = switch (mainItemType) {
229+
case "Group" -> createGroupItem(modelItem, itemTypeSegments);
230+
default -> createItemOfType(itemType, modelItem.getName());
231+
};
232+
233+
if (item instanceof ActiveItem activeItem) {
234+
String label = modelItem.getLabel();
235+
String format = extractFormat(label);
236+
if (format != null) {
237+
label = label.substring(0, label.indexOf("[")).trim();
238+
stateDescriptionFragments.put(modelItem.getName(),
239+
StateDescriptionFragmentBuilder.create().withPattern(format).build());
240+
} else {
241+
stateDescriptionFragments.remove(modelItem.getName());
242+
}
243+
activeItem.setLabel(label);
244+
activeItem.setCategory(modelItem.getIcon());
245+
assignTags(modelItem, activeItem);
246+
return item;
239247
} else {
240-
item = new GroupItem(modelGroupItem.getName());
241-
}
242-
} else {
243-
ModelNormalItem normalItem = (ModelNormalItem) modelItem;
244-
try {
245-
item = createItemOfType(normalItem.getType(), normalItem.getName());
246-
} catch (IllegalArgumentException e) {
247-
logger.debug("Error creating item '{}', item will be ignored: {}", normalItem.getName(),
248-
e.getMessage());
249248
return null;
250249
}
251-
}
252-
if (item instanceof ActiveItem activeItem) {
253-
String label = modelItem.getLabel();
254-
String format = extractFormat(label);
255-
if (format != null) {
256-
label = label.substring(0, label.indexOf("[")).trim();
257-
stateDescriptionFragments.put(modelItem.getName(),
258-
StateDescriptionFragmentBuilder.create().withPattern(format).build());
259-
} else {
260-
stateDescriptionFragments.remove(modelItem.getName());
261-
}
262-
activeItem.setLabel(label);
263-
activeItem.setCategory(modelItem.getIcon());
264-
assignTags(modelItem, activeItem);
265-
return item;
266-
} else {
250+
} catch (IllegalArgumentException e) {
251+
logger.debug("Error creating item '{}', item will be ignored: {}", modelItem.getName(), e.getMessage());
267252
return null;
268253
}
269254
}
@@ -286,14 +271,14 @@ private void assignTags(ModelItem modelItem, ActiveItem item) {
286271
}
287272
}
288273

289-
private GroupItem applyGroupFunction(Item baseItem, ModelGroupItem modelGroupItem, ModelGroupFunction function) {
274+
private GroupItem applyGroupFunction(Item baseItem, ModelItem modelItem, String function) {
290275
GroupFunctionDTO dto = new GroupFunctionDTO();
291-
dto.name = function.getName();
292-
dto.params = modelGroupItem.getArgs().toArray(new String[0]);
276+
dto.name = function;
277+
dto.params = modelItem.getArgs().toArray(new String[0]);
293278

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

296-
return new GroupItem(modelGroupItem.getName(), baseItem, groupFunction);
281+
return new GroupItem(modelItem.getName(), baseItem, groupFunction);
297282
}
298283

299284
private void dispatchBindingsPerItemType(String[] itemTypes) {
@@ -497,6 +482,46 @@ private Map<String, Item> toItemMap(@Nullable Collection<Item> items) {
497482
return ret;
498483
}
499484

485+
/**
486+
* Creates a new GroupItem based on the given ModelItem and item type segments.
487+
*
488+
* @param modelItem The ModelItem to create the GroupItem from.
489+
* @param itemTypeSegments The segments of the item type.
490+
* @return A new GroupItem or null if the item type is invalid.
491+
*/
492+
private @Nullable GroupItem createGroupItem(ModelItem modelItem, String[] itemTypeSegments) {
493+
if (itemTypeSegments.length == 1) {
494+
// Just plain "Group" with no base type
495+
return new GroupItem(modelItem.getName());
496+
}
497+
498+
String function = GroupFunction.DEFAULT;
499+
500+
String baseItemType = switch (itemTypeSegments.length) {
501+
case 2 -> itemTypeSegments[1];
502+
case 3 -> {
503+
// 3 segments could either be Group:Type:Function, or Group:Number:Dimension -> Find out which one it is
504+
if (!modelItem.getArgs().isEmpty() || GroupFunction.VALID_FUNCTIONS.contains(itemTypeSegments[2])) {
505+
// It's Group:Type:Function because there are arguments or the third segment is a valid function
506+
function = itemTypeSegments[2];
507+
yield itemTypeSegments[1];
508+
} else {
509+
// Otherwise, it must be Group:Number:Dimension
510+
yield itemTypeSegments[1] + ItemUtil.EXTENSION_SEPARATOR + itemTypeSegments[2];
511+
}
512+
}
513+
case 4 -> {
514+
// 4 segments: "Group:Number:Dimension:Function"
515+
function = itemTypeSegments[3];
516+
yield itemTypeSegments[1] + ItemUtil.EXTENSION_SEPARATOR + itemTypeSegments[2];
517+
}
518+
default -> throw new IllegalArgumentException("Invalid group item type: " + modelItem.getType());
519+
};
520+
521+
Item baseItem = createItemOfType(baseItemType, modelItem.getName());
522+
return applyGroupFunction(baseItem, modelItem, function);
523+
}
524+
500525
/**
501526
* Creates a new item of type {@code itemType} by utilizing an appropriate {@link ItemFactory}.
502527
*

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

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -33,15 +33,14 @@
3333
import org.openhab.core.items.GroupFunction;
3434
import org.openhab.core.items.GroupItem;
3535
import org.openhab.core.items.Item;
36+
import org.openhab.core.items.ItemUtil;
3637
import org.openhab.core.items.Metadata;
3738
import org.openhab.core.items.fileconverter.AbstractItemFileGenerator;
3839
import org.openhab.core.items.fileconverter.ItemFileGenerator;
3940
import org.openhab.core.model.core.ModelRepository;
4041
import org.openhab.core.model.items.ItemModel;
4142
import org.openhab.core.model.items.ItemsFactory;
4243
import org.openhab.core.model.items.ModelBinding;
43-
import org.openhab.core.model.items.ModelGroupFunction;
44-
import org.openhab.core.model.items.ModelGroupItem;
4544
import org.openhab.core.model.items.ModelItem;
4645
import org.openhab.core.model.items.ModelProperty;
4746
import org.openhab.core.types.State;
@@ -95,26 +94,25 @@ public synchronized void generateFileFormat(OutputStream out, List<Item> items,
9594

9695
private ModelItem buildModelItem(Item item, List<Metadata> channelLinks, List<Metadata> metadata,
9796
boolean hideDefaultParameters) {
98-
ModelItem model;
97+
ModelItem model = ItemsFactory.eINSTANCE.createModelItem();
9998
if (item instanceof GroupItem groupItem) {
100-
ModelGroupItem modelGroup = ItemsFactory.eINSTANCE.createModelGroupItem();
101-
model = modelGroup;
10299
Item baseItem = groupItem.getBaseItem();
100+
List<String> groupType = new ArrayList<>();
101+
groupType.add(groupItem.getType());
103102
if (baseItem != null) {
104-
modelGroup.setType(baseItem.getType());
103+
groupType.add(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+
groupType.add(function.getClass().getSimpleName().toUpperCase());
110107
State[] parameters = function.getParameters();
111108
for (int i = 0; i < parameters.length; i++) {
112-
modelGroup.getArgs().add(parameters[i].toString());
109+
model.getArgs().add(parameters[i].toString());
113110
}
114111
}
112+
115113
}
114+
model.setType(groupType.stream().collect(Collectors.joining(ItemUtil.EXTENSION_SEPARATOR)));
116115
} else {
117-
model = ItemsFactory.eINSTANCE.createModelNormalItem();
118116
model.setType(item.getType());
119117
}
120118

0 commit comments

Comments
 (0)