From 96fa15ae8740eb447c4d3acebf6a6d6fecb00f1f Mon Sep 17 00:00:00 2001 From: Christoph Weitkamp Date: Thu, 22 Jul 2021 13:38:21 +0200 Subject: [PATCH 1/2] Allow UoM in ItemStateConditions Signed-off-by: Christoph Weitkamp --- .../core/automation/internal/ActionImpl.java | 5 +- .../automation/internal/ConditionImpl.java | 5 +- .../handler/ItemStateConditionHandler.java | 118 +++++++-- .../ItemStateConditionHandlerTest.java | 233 ++++++++++++++++++ ...er.java => MagicRollershutterHandler.java} | 0 5 files changed, 335 insertions(+), 26 deletions(-) create mode 100644 bundles/org.openhab.core.automation/src/test/java/org/openhab/core/automation/internal/module/handler/ItemStateConditionHandlerTest.java rename bundles/org.openhab.core.test.magic/src/main/java/org/openhab/core/magic/binding/handler/{MagicRolllershutterHandler.java => MagicRollershutterHandler.java} (100%) diff --git a/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/ActionImpl.java b/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/ActionImpl.java index df411344043..2dec47583b2 100644 --- a/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/ActionImpl.java +++ b/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/ActionImpl.java @@ -12,7 +12,6 @@ */ package org.openhab.core.automation.internal; -import java.util.Collections; import java.util.Map; import org.eclipse.jdt.annotation.NonNullByDefault; @@ -33,7 +32,7 @@ @NonNullByDefault public class ActionImpl extends ModuleImpl implements Action { - private Map inputs = Collections.emptyMap(); + private Map inputs = Map.of(); /** * Constructor of Action object. @@ -48,7 +47,7 @@ public class ActionImpl extends ModuleImpl implements Action { public ActionImpl(String UID, String typeUID, @Nullable Configuration configuration, @Nullable String label, @Nullable String description, @Nullable Map inputs) { super(UID, typeUID, configuration, label, description); - this.inputs = inputs == null ? Collections.emptyMap() : Collections.unmodifiableMap(inputs); + this.inputs = inputs == null ? Map.of() : Map.copyOf(inputs); } /** diff --git a/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/ConditionImpl.java b/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/ConditionImpl.java index 77aaa8ad912..da9cd2225ac 100644 --- a/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/ConditionImpl.java +++ b/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/ConditionImpl.java @@ -12,7 +12,6 @@ */ package org.openhab.core.automation.internal; -import java.util.Collections; import java.util.Map; import org.eclipse.jdt.annotation.NonNullByDefault; @@ -31,7 +30,7 @@ @NonNullByDefault public class ConditionImpl extends ModuleImpl implements Condition { - private Map inputs = Collections.emptyMap(); + private Map inputs = Map.of(); /** * Constructor of {@link Condition} module object. @@ -46,7 +45,7 @@ public class ConditionImpl extends ModuleImpl implements Condition { public ConditionImpl(String id, String typeUID, @Nullable Configuration configuration, @Nullable String label, @Nullable String description, @Nullable Map inputs) { super(id, typeUID, configuration, label, description); - this.inputs = inputs == null ? Collections.emptyMap() : Collections.unmodifiableMap(inputs); + this.inputs = inputs == null ? Map.of() : Map.copyOf(inputs); } /** diff --git a/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/module/handler/ItemStateConditionHandler.java b/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/module/handler/ItemStateConditionHandler.java index 058d5dc61ac..bbc459f6248 100644 --- a/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/module/handler/ItemStateConditionHandler.java +++ b/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/module/handler/ItemStateConditionHandler.java @@ -14,12 +14,15 @@ import java.util.Map; +import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; import org.openhab.core.automation.Condition; import org.openhab.core.automation.handler.BaseConditionModuleHandler; import org.openhab.core.items.Item; import org.openhab.core.items.ItemNotFoundException; import org.openhab.core.items.ItemRegistry; import org.openhab.core.library.types.DecimalType; +import org.openhab.core.library.types.QuantityType; import org.openhab.core.types.State; import org.openhab.core.types.TypeParser; import org.slf4j.Logger; @@ -31,21 +34,21 @@ * @author Benedikt Niehues - Initial contribution * @author Kai Kreuzer - refactored and simplified customized module handling */ +@NonNullByDefault public class ItemStateConditionHandler extends BaseConditionModuleHandler { /** - * Constants for Config-Parameters corresponding to Definition in - * ItemModuleTypeDefinition.json + * Constants for Config-Parameters corresponding to Definition in ItemModuleTypeDefinition.json */ - private static final String ITEM_NAME = "itemName"; - private static final String OPERATOR = "operator"; - private static final String STATE = "state"; + public static final String ITEM_NAME = "itemName"; + public static final String OPERATOR = "operator"; + public static final String STATE = "state"; private final Logger logger = LoggerFactory.getLogger(ItemStateConditionHandler.class); public static final String ITEM_STATE_CONDITION = "core.ItemStateCondition"; - private ItemRegistry itemRegistry; + private @Nullable ItemRegistry itemRegistry; public ItemStateConditionHandler(Condition condition) { super(condition); @@ -74,6 +77,7 @@ public void dispose() { itemRegistry = null; } + @SuppressWarnings({ "rawtypes", "unchecked" }) @Override public boolean isSatisfied(Map inputs) { String itemName = (String) module.getConfiguration().get(ITEM_NAME); @@ -92,44 +96,118 @@ public boolean isSatisfied(Map inputs) { Item item = itemRegistry.getItem(itemName); State compareState = TypeParser.parseState(item.getAcceptedDataTypes(), state); State itemState = item.getState(); - DecimalType decimalState = itemState.as(DecimalType.class); logger.debug("ItemStateCondition '{}'checking if {} (State={}) {} {}", module.getId(), itemName, itemState, operator, compareState); switch (operator) { case "=": - logger.debug("ConditionSatisfied --> {}", itemState.equals(compareState)); return itemState.equals(compareState); case "!=": return !itemState.equals(compareState); case "<": - if (null != decimalState && compareState instanceof DecimalType) { - return decimalState.compareTo((DecimalType) compareState) < 0; + if (itemState instanceof QuantityType) { + QuantityType qtState = (QuantityType) itemState; + if (compareState instanceof DecimalType) { + // allow compareState without unit -> implicitly assume its the same as the one from the + // state, but warn the user + logger.warn( + "Received a QuantityType state '{}' with unit, but the condition is defined as a plain number without unit ({}), please consider adding a unit to the condition.", + qtState, state); + return qtState.compareTo(new QuantityType<>(((DecimalType) compareState).toBigDecimal(), + qtState.getUnit())) < 0; + } else if (compareState instanceof QuantityType) { + return qtState.compareTo((QuantityType) compareState) < 0; + } else { + logger.warn( + "Condition '{}' cannot be compared to the incompatible state '{}' from the item.", + state, qtState); + } + } else if (itemState instanceof DecimalType && null != compareState) { + DecimalType decimalState = compareState.as(DecimalType.class); + if (null != decimalState) { + return ((DecimalType) itemState).compareTo(decimalState) < 0; + } } - break; case "<=": case "=<": - if (null != decimalState && compareState instanceof DecimalType) { - return decimalState.compareTo((DecimalType) compareState) <= 0; + if (itemState instanceof QuantityType) { + QuantityType qtState = (QuantityType) itemState; + if (compareState instanceof DecimalType) { + // allow compareState without unit -> implicitly assume its the same as the one from the + // state, but warn the user + logger.warn( + "Received a QuantityType state '{}' with unit, but the condition is defined as a plain number without unit ({}), please consider adding a unit to the condition.", + qtState, state); + return qtState.compareTo(new QuantityType<>(((DecimalType) compareState).toBigDecimal(), + qtState.getUnit())) <= 0; + } else if (compareState instanceof QuantityType) { + return qtState.compareTo((QuantityType) compareState) <= 0; + } else { + logger.warn( + "Condition '{}' cannot be compared to the incompatible state '{}' from the item.", + state, qtState); + } + } else if (itemState instanceof DecimalType && null != compareState) { + DecimalType decimalState = compareState.as(DecimalType.class); + if (null != decimalState) { + return ((DecimalType) itemState).compareTo(decimalState) <= 0; + } } break; case ">": - if (null != decimalState && compareState instanceof DecimalType) { - return decimalState.compareTo((DecimalType) compareState) > 0; + if (itemState instanceof QuantityType) { + QuantityType qtState = (QuantityType) itemState; + if (compareState instanceof DecimalType) { + // allow compareState without unit -> implicitly assume its the same as the one from the + // state, but warn the user + logger.warn( + "Received a QuantityType state '{}' with unit, but the condition is defined as a plain number without unit ({}), please consider adding a unit to the condition.", + qtState, state); + return qtState.compareTo(new QuantityType<>(((DecimalType) compareState).toBigDecimal(), + qtState.getUnit())) > 0; + } else if (compareState instanceof QuantityType) { + return qtState.compareTo((QuantityType) compareState) > 0; + } else { + logger.warn( + "Condition '{}' cannot be compared to the incompatible state '{}' from the item.", + state, qtState); + } + } else if (itemState instanceof DecimalType && null != compareState) { + DecimalType decimalState = compareState.as(DecimalType.class); + if (null != decimalState) { + return ((DecimalType) itemState).compareTo(decimalState) > 0; + } } break; case ">=": case "=>": - if (null != decimalState && compareState instanceof DecimalType) { - return decimalState.compareTo((DecimalType) compareState) >= 0; + if (itemState instanceof QuantityType) { + QuantityType qtState = (QuantityType) itemState; + if (compareState instanceof DecimalType) { + // allow compareState without unit -> implicitly assume its the same as the one from the + // state, but warn the user + logger.warn( + "Received a QuantityType state '{}' with unit, but the condition is defined as a plain number without unit ({}), please consider adding a unit to the condition.", + qtState, state); + return qtState.compareTo(new QuantityType<>(((DecimalType) compareState).toBigDecimal(), + qtState.getUnit())) >= 0; + } else if (compareState instanceof QuantityType) { + return qtState.compareTo((QuantityType) compareState) >= 0; + } else { + logger.warn( + "Condition '{}' cannot be compared to the incompatible state '{}' from the item.", + state, qtState); + } + } else if (itemState instanceof DecimalType && null != compareState) { + DecimalType decimalState = compareState.as(DecimalType.class); + if (null != decimalState) { + return ((DecimalType) itemState).compareTo(decimalState) >= 0; + } } break; - default: - break; } } catch (ItemNotFoundException e) { logger.error("Item with Name {} not found in itemRegistry", itemName); - return false; } return false; } diff --git a/bundles/org.openhab.core.automation/src/test/java/org/openhab/core/automation/internal/module/handler/ItemStateConditionHandlerTest.java b/bundles/org.openhab.core.automation/src/test/java/org/openhab/core/automation/internal/module/handler/ItemStateConditionHandlerTest.java new file mode 100644 index 00000000000..2a7f9de76f5 --- /dev/null +++ b/bundles/org.openhab.core.automation/src/test/java/org/openhab/core/automation/internal/module/handler/ItemStateConditionHandlerTest.java @@ -0,0 +1,233 @@ +/** + * Copyright (c) 2010-2021 Contributors to the openHAB project + * + * See the NOTICE file(s) distributed with this work for additional + * information. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0 + * + * SPDX-License-Identifier: EPL-2.0 + */ +package org.openhab.core.automation.internal.module.handler; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.when; + +import java.util.Arrays; +import java.util.Collection; +import java.util.Map; + +import org.eclipse.jdt.annotation.NonNullByDefault; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.mockito.junit.jupiter.MockitoSettings; +import org.mockito.quality.Strictness; +import org.openhab.core.automation.util.ConditionBuilder; +import org.openhab.core.config.core.Configuration; +import org.openhab.core.items.ItemNotFoundException; +import org.openhab.core.items.ItemRegistry; +import org.openhab.core.library.items.NumberItem; +import org.openhab.core.library.types.DecimalType; +import org.openhab.core.library.types.QuantityType; +import org.openhab.core.library.unit.SIUnits; +import org.openhab.core.types.State; + +/** + * Basic unit tests for {@link ItemStateConditionHandler}. + * + * @author Christoph Weitkamp - Initial contribution + */ +@ExtendWith(MockitoExtension.class) +@MockitoSettings(strictness = Strictness.WARN) +public class ItemStateConditionHandlerTest { + + @NonNullByDefault + public static class ParameterSet { + public final String comparisonState; + public final State itemState; + public final boolean expectedResult; + + public ParameterSet(String comparisonState, State itemState, boolean expectedResult) { + this.comparisonState = comparisonState; + this.itemState = itemState; + this.expectedResult = expectedResult; + } + } + + public static Collection equalsParameters() { + return Arrays.asList(new Object[][] { // + { new ParameterSet("5", new DecimalType(23), false) }, // + { new ParameterSet("5", new DecimalType(5), true) }, // + { new ParameterSet("5 °C", new DecimalType(23), false) }, // + { new ParameterSet("5 °C", new DecimalType(5), false) }, // + { new ParameterSet("5", new QuantityType<>(23, SIUnits.CELSIUS), false) }, // + { new ParameterSet("5", new QuantityType<>(5, SIUnits.CELSIUS), false) }, // + { new ParameterSet("5 °C", new QuantityType<>(23, SIUnits.CELSIUS), false) }, // + { new ParameterSet("5 °C", new QuantityType<>(5, SIUnits.CELSIUS), true) } }); + } + + public static Collection greaterThanParameters() { + return Arrays.asList(new Object[][] { // + { new ParameterSet("5", new DecimalType(23), true) }, // + { new ParameterSet("5", new DecimalType(5), false) }, // + { new ParameterSet("5 °C", new DecimalType(23), true) }, // + { new ParameterSet("5 °C", new DecimalType(5), false) }, // + { new ParameterSet("5", new QuantityType<>(23, SIUnits.CELSIUS), true) }, // + { new ParameterSet("5", new QuantityType<>(5, SIUnits.CELSIUS), false) }, // + { new ParameterSet("5 °C", new QuantityType<>(23, SIUnits.CELSIUS), true) }, // + { new ParameterSet("5 °C", new QuantityType<>(5, SIUnits.CELSIUS), false) } }); + } + + public static Collection greaterThanOrEqualsParameters() { + return Arrays.asList(new Object[][] { // + { new ParameterSet("5", new DecimalType(23), true) }, // + { new ParameterSet("5", new DecimalType(5), true) }, // + { new ParameterSet("5", new DecimalType(4), false) }, // + { new ParameterSet("5 °C", new DecimalType(23), true) }, // + { new ParameterSet("5 °C", new DecimalType(5), true) }, // + { new ParameterSet("5 °C", new DecimalType(4), false) }, // + { new ParameterSet("5", new QuantityType<>(23, SIUnits.CELSIUS), true) }, // + { new ParameterSet("5", new QuantityType<>(5, SIUnits.CELSIUS), true) }, // + { new ParameterSet("5", new QuantityType<>(4, SIUnits.CELSIUS), false) }, // + { new ParameterSet("5 °C", new QuantityType<>(23, SIUnits.CELSIUS), true) }, // + { new ParameterSet("5 °C", new QuantityType<>(5, SIUnits.CELSIUS), true) }, // + { new ParameterSet("5 °C", new QuantityType<>(4, SIUnits.CELSIUS), false) } }); + } + + public static Collection lessThanParameters() { + return Arrays.asList(new Object[][] { // + { new ParameterSet("5", new DecimalType(23), false) }, // + { new ParameterSet("5", new DecimalType(4), true) }, // + { new ParameterSet("5 °C", new DecimalType(23), false) }, // + { new ParameterSet("5 °C", new DecimalType(4), true) }, // + { new ParameterSet("5", new QuantityType<>(23, SIUnits.CELSIUS), false) }, // + { new ParameterSet("5", new QuantityType<>(4, SIUnits.CELSIUS), true) }, // + { new ParameterSet("5 °C", new QuantityType<>(23, SIUnits.CELSIUS), false) }, // + { new ParameterSet("5 °C", new QuantityType<>(4, SIUnits.CELSIUS), true) } }); + } + + public static Collection lessThanOrEqualsParameters() { + return Arrays.asList(new Object[][] { // + { new ParameterSet("5", new DecimalType(23), false) }, // + { new ParameterSet("5", new DecimalType(5), true) }, // + { new ParameterSet("5", new DecimalType(4), true) }, // + { new ParameterSet("5 °C", new DecimalType(23), false) }, // + { new ParameterSet("5 °C", new DecimalType(5), true) }, // + { new ParameterSet("5 °C", new DecimalType(4), true) }, // + { new ParameterSet("5", new QuantityType<>(23, SIUnits.CELSIUS), false) }, // + { new ParameterSet("5", new QuantityType<>(5, SIUnits.CELSIUS), true) }, // + { new ParameterSet("5", new QuantityType<>(4, SIUnits.CELSIUS), true) }, // + { new ParameterSet("5 °C", new QuantityType<>(23, SIUnits.CELSIUS), false) }, // + { new ParameterSet("5 °C", new QuantityType<>(5, SIUnits.CELSIUS), true) }, // + { new ParameterSet("5 °C", new QuantityType<>(4, SIUnits.CELSIUS), true) } }); + } + + private static final String ITEM_NAME = "myItem"; + + private final NumberItem item = new NumberItem(ITEM_NAME); + + private @Mock ItemRegistry mockItemRegistry; + + @BeforeEach + public void setup() throws ItemNotFoundException { + when(mockItemRegistry.getItem(ITEM_NAME)).thenReturn(item); + } + + @ParameterizedTest + @MethodSource("equalsParameters") + public void testEqualsCondition(ParameterSet parameterSet) { + ItemStateConditionHandler handler = initItemStateConditionHandler("=", parameterSet.comparisonState); + + item.setState(parameterSet.itemState); + if (parameterSet.expectedResult) { + assertTrue(handler.isSatisfied(Map.of())); + } else { + assertFalse(handler.isSatisfied(Map.of())); + } + } + + @ParameterizedTest + @MethodSource("equalsParameters") + public void testNotEqualsCondition(ParameterSet parameterSet) { + ItemStateConditionHandler handler = initItemStateConditionHandler("!=", parameterSet.comparisonState); + + item.setState(parameterSet.itemState); + if (parameterSet.expectedResult) { + assertFalse(handler.isSatisfied(Map.of())); + } else { + assertTrue(handler.isSatisfied(Map.of())); + } + } + + @ParameterizedTest + @MethodSource("greaterThanParameters") + public void testGreaterThanCondition(ParameterSet parameterSet) { + ItemStateConditionHandler handler = initItemStateConditionHandler(">", parameterSet.comparisonState); + + item.setState(parameterSet.itemState); + if (parameterSet.expectedResult) { + assertTrue(handler.isSatisfied(Map.of())); + } else { + assertFalse(handler.isSatisfied(Map.of())); + } + } + + @ParameterizedTest + @MethodSource("greaterThanOrEqualsParameters") + public void testGreaterThanOrEqualsCondition(ParameterSet parameterSet) { + ItemStateConditionHandler handler = initItemStateConditionHandler(">=", parameterSet.comparisonState); + + item.setState(parameterSet.itemState); + if (parameterSet.expectedResult) { + assertTrue(handler.isSatisfied(Map.of())); + } else { + assertFalse(handler.isSatisfied(Map.of())); + } + } + + @ParameterizedTest + @MethodSource("lessThanParameters") + public void testLessThanCondition(ParameterSet parameterSet) { + ItemStateConditionHandler handler = initItemStateConditionHandler("<", parameterSet.comparisonState); + + item.setState(parameterSet.itemState); + if (parameterSet.expectedResult) { + assertTrue(handler.isSatisfied(Map.of())); + } else { + assertFalse(handler.isSatisfied(Map.of())); + } + } + + @ParameterizedTest + @MethodSource("lessThanOrEqualsParameters") + public void testLessThanOrEqualsCondition(ParameterSet parameterSet) { + ItemStateConditionHandler handler = initItemStateConditionHandler("<=", parameterSet.comparisonState); + + item.setState(parameterSet.itemState); + if (parameterSet.expectedResult) { + assertTrue(handler.isSatisfied(Map.of())); + } else { + assertFalse(handler.isSatisfied(Map.of())); + } + } + + private ItemStateConditionHandler initItemStateConditionHandler(String operator, String state) { + Configuration configuration = new Configuration(); + configuration.put(ItemStateConditionHandler.ITEM_NAME, ITEM_NAME); + configuration.put(ItemStateConditionHandler.OPERATOR, operator); + configuration.put(ItemStateConditionHandler.STATE, state); + ConditionBuilder builder = ConditionBuilder.create() // + .withId("conditionId") // + .withTypeUID(ItemStateConditionHandler.ITEM_STATE_CONDITION) // + .withConfiguration(configuration); + ItemStateConditionHandler handler = new ItemStateConditionHandler(builder.build()); + handler.setItemRegistry(mockItemRegistry); + return handler; + } +} diff --git a/bundles/org.openhab.core.test.magic/src/main/java/org/openhab/core/magic/binding/handler/MagicRolllershutterHandler.java b/bundles/org.openhab.core.test.magic/src/main/java/org/openhab/core/magic/binding/handler/MagicRollershutterHandler.java similarity index 100% rename from bundles/org.openhab.core.test.magic/src/main/java/org/openhab/core/magic/binding/handler/MagicRolllershutterHandler.java rename to bundles/org.openhab.core.test.magic/src/main/java/org/openhab/core/magic/binding/handler/MagicRollershutterHandler.java From daa73c81dbfeda7d29b1655d1c006307297fd6af Mon Sep 17 00:00:00 2001 From: Christoph Weitkamp Date: Thu, 22 Jul 2021 21:31:03 +0200 Subject: [PATCH 2/2] Revert renamed file Signed-off-by: Christoph Weitkamp --- ...cRollershutterHandler.java => MagicRolllershutterHandler.java} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename bundles/org.openhab.core.test.magic/src/main/java/org/openhab/core/magic/binding/handler/{MagicRollershutterHandler.java => MagicRolllershutterHandler.java} (100%) diff --git a/bundles/org.openhab.core.test.magic/src/main/java/org/openhab/core/magic/binding/handler/MagicRollershutterHandler.java b/bundles/org.openhab.core.test.magic/src/main/java/org/openhab/core/magic/binding/handler/MagicRolllershutterHandler.java similarity index 100% rename from bundles/org.openhab.core.test.magic/src/main/java/org/openhab/core/magic/binding/handler/MagicRollershutterHandler.java rename to bundles/org.openhab.core.test.magic/src/main/java/org/openhab/core/magic/binding/handler/MagicRolllershutterHandler.java