Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix UI defined sitemaps #3850

Merged
merged 4 commits into from
Oct 26, 2023
Merged

Fix UI defined sitemaps #3850

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0f56c22
fix UI defined sitemaps
mherwege Oct 18, 2023
a0ca197
allow AND conditions
mherwege Oct 20, 2023
05df117
label not required
mherwege Oct 21, 2023
61774ed
review adjustments
mherwege Oct 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
151 changes: 97 additions & 54 deletions ....ui/src/main/java/org/openhab/core/ui/internal/components/UIComponentSitemapProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@
package org.openhab.core.ui.internal.components;

import java.math.BigDecimal;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.CopyOnWriteArraySet;
Expand Down Expand Up @@ -82,6 +84,7 @@
* @author Laurent Garnier - icon color support for all widgets
* @author Laurent Garnier - Added support for new element Buttongrid
* @author Laurent Garnier - Added icon field for mappings
* @author Mark Herwege - Make UI provided sitemaps compatible with enhanced syntax in conditions
*/
@NonNullByDefault
@Component(service = SitemapProvider.class)
Expand All @@ -93,11 +96,8 @@ public class UIComponentSitemapProvider implements SitemapProvider, RegistryChan
private static final String SITEMAP_PREFIX = "uicomponents_";
private static final String SITEMAP_SUFFIX = ".sitemap";

private static final Pattern VISIBILITY_PATTERN = Pattern
.compile("(?<item>[A-Za-z]\\w*)\\s*(?<condition>==|!=|<=|>=|<|>)\\s*(?<sign>\\+|-)?(?<state>.+)");
private static final Pattern COLOR_PATTERN = Pattern.compile(
"((?<item>[A-Za-z]\\w*)?\\s*((?<condition>==|!=|<=|>=|<|>)\\s*(?<sign>\\+|-)?(?<state>[^=]*[^= ]+))?\\s*=)?\\s*(?<arg>\\S+)");
private static final Pattern ICON_PATTERN = COLOR_PATTERN;
private static final Pattern CONDITION_PATTERN = Pattern
.compile("(?<item>[A-Za-z]\\w*)?\\s*(?<condition>==|!=|<=|>=|<|>)?\\s*(?<sign>\\+|-)?(?<state>.+)");

private Map<String, Sitemap> sitemaps = new HashMap<>();
private @Nullable UIComponentRegistryFactory componentRegistryFactory;
Expand Down Expand Up @@ -160,7 +160,10 @@ protected Sitemap buildSitemap(RootUIComponent rootComponent) {

SitemapImpl sitemap = (SitemapImpl) SitemapFactory.eINSTANCE.createSitemap();
sitemap.setName(SITEMAP_PREFIX + rootComponent.getUID());
sitemap.setLabel(rootComponent.getConfig().get("label").toString());
Object label = rootComponent.getConfig().get("label");
if (label != null) {
sitemap.setLabel(label.toString());
}

if (rootComponent.getSlots() != null && rootComponent.getSlots().containsKey("widgets")) {
for (UIComponent component : rootComponent.getSlot("widgets")) {
Expand Down Expand Up @@ -280,8 +283,7 @@ protected Sitemap buildSitemap(RootUIComponent rootComponent) {

if (widget != null) {
setWidgetPropertyFromComponentConfig(widget, component, "label", SitemapPackage.WIDGET__LABEL);
setWidgetPropertyFromComponentConfig(widget, component, "icon", SitemapPackage.WIDGET__ICON);
setWidgetPropertyFromComponentConfig(widget, component, "staticIcon", SitemapPackage.WIDGET__STATIC_ICON);
setWidgetIconPropertyFromComponentConfig(widget, component);
setWidgetPropertyFromComponentConfig(widget, component, "item", SitemapPackage.WIDGET__ITEM);

if (widget instanceof LinkableWidget linkableWidget) {
Expand Down Expand Up @@ -331,10 +333,28 @@ private void setWidgetPropertyFromComponentConfig(Widget widget, @Nullable UICom
}
}

private void setWidgetIconPropertyFromComponentConfig(Widget widget, @Nullable UIComponent component) {
if (component == null || component.getConfig() == null) {
return;
}
Object staticIcon = component.getConfig().get("staticIcon");
if (staticIcon != null && Boolean.valueOf(ConfigUtil.normalizeType(staticIcon).toString())) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure to understand: staticIcon is not a boolean but a string containing the icon.

Copy link
Contributor Author

@mherwege mherwege Oct 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not in the JSON db and JSON config. It is only in the grammar. The UI simply shows a switch turning staticIcon on or off. The state of that switch is what goes into the field. For the UI that is a lot nicer than showing 2 icon fields and having to move the value between them if switching to static. It could obviously be solved differently in the UI, but I see no reason to make it more complex there.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok

setWidgetPropertyFromComponentConfig(widget, component, "icon", SitemapPackage.WIDGET__STATIC_ICON);
return;
}

Object icon = component.getConfig().get("icon");
if (icon == null) {
return;
}
setWidgetPropertyFromComponentConfig(widget, component, "icon", SitemapPackage.WIDGET__ICON);
}

private void addWidgetMappings(EList<Mapping> mappings, UIComponent component) {
if (component.getConfig() != null && component.getConfig().containsKey("mappings")) {
if (component.getConfig().get("mappings") instanceof Collection<?>) {
for (Object sourceMapping : (Collection<?>) component.getConfig().get("mappings")) {
Object sourceMappings = component.getConfig().get("mappings");
if (sourceMappings instanceof Collection<?>) {
for (Object sourceMapping : (Collection<?>) sourceMappings) {
if (sourceMapping instanceof String) {
String[] splitMapping = sourceMapping.toString().split("=");
String cmd = splitMapping[0].trim();
Expand All @@ -353,8 +373,9 @@ private void addWidgetMappings(EList<Mapping> mappings, UIComponent component) {

private void addWidgetButtons(EList<Button> buttons, UIComponent component) {
if (component.getConfig() != null && component.getConfig().containsKey("buttons")) {
if (component.getConfig().get("buttons") instanceof Collection<?>) {
for (Object sourceButton : (Collection<?>) component.getConfig().get("buttons")) {
Object sourceButtons = component.getConfig().get("buttons");
if (sourceButtons instanceof Collection<?>) {
for (Object sourceButton : (Collection<?>) sourceButtons) {
if (sourceButton instanceof String) {
String[] splitted1 = sourceButton.toString().split(":");
int idx = Integer.parseInt(splitted1[0].trim());
Expand All @@ -376,22 +397,16 @@ private void addWidgetButtons(EList<Button> buttons, UIComponent component) {

private void addWidgetVisibility(EList<VisibilityRule> visibility, UIComponent component) {
if (component.getConfig() != null && component.getConfig().containsKey("visibility")) {
for (Object sourceVisibility : (Collection<?>) component.getConfig().get("visibility")) {
if (sourceVisibility instanceof String) {
Matcher matcher = VISIBILITY_PATTERN.matcher(sourceVisibility.toString());
if (matcher.matches()) {
Object sourceVisibilities = component.getConfig().get("visibility");
if (sourceVisibilities instanceof Collection<?>) {
for (Object sourceVisibility : (Collection<?>) sourceVisibilities) {
if (sourceVisibility instanceof String) {
List<String> conditionsString = getRuleConditions(sourceVisibility.toString(), null);
VisibilityRuleImpl visibilityRule = (VisibilityRuleImpl) SitemapFactory.eINSTANCE
.createVisibilityRule();
ConditionImpl condition = (ConditionImpl) SitemapFactory.eINSTANCE.createCondition();
condition.setItem(matcher.group("item"));
condition.setCondition(matcher.group("condition"));
condition.setSign(matcher.group("sign"));
condition.setState(matcher.group("state"));
visibilityRule.eSet(SitemapPackage.VISIBILITY_RULE__CONDITIONS, condition);
List<ConditionImpl> conditions = getConditions(conditionsString, component, "visibility");
visibilityRule.eSet(SitemapPackage.VISIBILITY_RULE__CONDITIONS, conditions);
visibility.add(visibilityRule);
} else {
logger.warn("Syntax error in visibility rule '{}' for widget {}", sourceVisibility,
component.getType());
}
}
}
Expand All @@ -412,50 +427,78 @@ private void addIconColor(EList<ColorArray> iconColor, UIComponent component) {

private void addColor(EList<ColorArray> color, UIComponent component, String key) {
if (component.getConfig() != null && component.getConfig().containsKey(key)) {
for (Object sourceColor : (Collection<?>) component.getConfig().get(key)) {
if (sourceColor instanceof String) {
Matcher matcher = COLOR_PATTERN.matcher(sourceColor.toString());
if (matcher.matches()) {
Object sourceColors = component.getConfig().get(key);
if (sourceColors instanceof Collection<?>) {
for (Object sourceColor : (Collection<?>) sourceColors) {
if (sourceColor instanceof String) {
String argument = getRuleArgument(sourceColor.toString());
List<String> conditionsString = getRuleConditions(sourceColor.toString(), argument);
ColorArrayImpl colorArray = (ColorArrayImpl) SitemapFactory.eINSTANCE.createColorArray();
ConditionImpl condition = (ConditionImpl) SitemapFactory.eINSTANCE.createCondition();
condition.setItem(matcher.group("item"));
condition.setCondition(matcher.group("condition"));
condition.setSign(matcher.group("sign"));
condition.setState(matcher.group("state"));
colorArray.eSet(SitemapPackage.COLOR_ARRAY__CONDITIONS, condition);
colorArray.setArg(argument);
List<ConditionImpl> conditions = getConditions(conditionsString, component, key);
colorArray.eSet(SitemapPackage.COLOR_ARRAY__CONDITIONS, conditions);
color.add(colorArray);
} else {
logger.warn("Syntax error in {} rule '{}' for widget {}", key, sourceColor,
component.getType());
}
}
}
}
}

private void addIconRules(EList<IconRule> iconDef, UIComponent component) {
if (component.getConfig() != null && component.getConfig().containsKey("icon")) {
for (Object sourceIcon : (Collection<?>) component.getConfig().get("icon")) {
if (sourceIcon instanceof String) {
Matcher matcher = ICON_PATTERN.matcher(sourceIcon.toString());
if (matcher.matches()) {
private void addIconRules(EList<IconRule> icon, UIComponent component) {
if (component.getConfig() != null && component.getConfig().containsKey("iconrules")) {
Object sourceIcons = component.getConfig().get("iconrules");
if (sourceIcons instanceof Collection<?>) {
for (Object sourceIcon : (Collection<?>) sourceIcons) {
if (sourceIcon instanceof String) {
String argument = getRuleArgument(sourceIcon.toString());
List<String> conditionsString = getRuleConditions(sourceIcon.toString(), argument);
IconRuleImpl iconRule = (IconRuleImpl) SitemapFactory.eINSTANCE.createIconRule();
ConditionImpl condition = (ConditionImpl) SitemapFactory.eINSTANCE.createCondition();
condition.setItem(matcher.group("item"));
condition.setCondition(matcher.group("condition"));
condition.setSign(matcher.group("sign"));
condition.setState(matcher.group("state"));
iconRule.eSet(SitemapPackage.ICON_RULE__CONDITIONS, condition);
iconRule.setArg(matcher.group("arg"));
iconDef.add(iconRule);
} else {
logger.warn("Syntax error in icon rule '{}' for widget {}", sourceIcon, component.getType());
iconRule.setArg(argument);
List<ConditionImpl> conditions = getConditions(conditionsString, component, "iconrules");
iconRule.eSet(SitemapPackage.ICON_RULE__CONDITIONS, conditions);
icon.add(iconRule);
}
}
}
}
}

private List<ConditionImpl> getConditions(List<String> conditionsString, UIComponent component, String key) {
List<ConditionImpl> conditions = new ArrayList<>();
for (String conditionString : conditionsString) {
Matcher matcher = CONDITION_PATTERN.matcher(conditionString);
if (matcher.matches()) {
ConditionImpl condition = (ConditionImpl) SitemapFactory.eINSTANCE.createCondition();
condition.setItem(matcher.group("item"));
condition.setCondition(matcher.group("condition"));
condition.setSign(matcher.group("sign"));
condition.setState(matcher.group("state"));
conditions.add(condition);
} else {
logger.warn("Syntax error in {} rule condition '{}' for widget {}", key, conditionString,
component.getType());
}
}
return conditions;
}

private String getRuleArgument(String rule) {
int argIndex = rule.lastIndexOf("=") + 1;
return rule.substring(argIndex).trim();
}

private List<String> getRuleConditions(String rule, @Nullable String argument) {
String conditions = rule;
if (argument != null) {
conditions = rule.substring(0, rule.lastIndexOf(argument)).trim();
if (conditions.endsWith("=")) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And if there is one or several spaces between = and the "argument" ?

Copy link
Contributor Author

@mherwege mherwege Oct 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The full rule will be something like:

Item>=5 = red

The passed in argument would be red.
If there is an argument (!= null), it is split off and conditions should end with = (after trim). If there is a second = before, it should actually be part of the state string (5 in the example). So I think this is right.

Spaces after = are removed using trim.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not take attention that the previous line ended with a call to trim(). So, no problem, you can ignore this comment.

conditions = conditions.substring(0, conditions.length() - 1);
}
}
List<String> conditionsList = List.of(conditions.split(" AND "));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does UI systematically adds space around the AND ?

Copy link
Contributor Author

@mherwege mherwege Oct 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it does. And the chance of having an erronuous match is lower when adding the spaces in the separator string.

return conditionsList.stream().map(String::trim).toList();
}

@Override
public void addModelChangeListener(ModelRepositoryChangeListener listener) {
modelChangeListeners.add(listener);
Expand Down