From bce56f297d74c280b2e4da91b42c439d9c5f121a Mon Sep 17 00:00:00 2001 From: "Jan N. Klug" Date: Sun, 19 Jun 2022 10:24:35 +0200 Subject: [PATCH 1/2] Log a warning when item for trigger/condition is missing or removed Signed-off-by: Jan N. Klug --- .../factory/CoreModuleHandlerFactory.java | 10 +-- .../handler/GroupCommandTriggerHandler.java | 32 +++++++- .../handler/GroupStateTriggerHandler.java | 31 +++++++- .../handler/ItemCommandTriggerHandler.java | 39 +++++++--- .../handler/ItemStateConditionHandler.java | 76 ++++++++++++++++++- .../handler/ItemStateTriggerHandler.java | 32 +++++++- .../ItemStateConditionHandlerTest.java | 49 +++++++++++- 7 files changed, 240 insertions(+), 29 deletions(-) diff --git a/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/module/factory/CoreModuleHandlerFactory.java b/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/module/factory/CoreModuleHandlerFactory.java index b7c8163f157..1d6d39f0d06 100644 --- a/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/module/factory/CoreModuleHandlerFactory.java +++ b/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/module/factory/CoreModuleHandlerFactory.java @@ -108,7 +108,7 @@ public Collection getTypes() { } else if (ChannelEventTriggerHandler.MODULE_TYPE_ID.equals(moduleTypeUID)) { return new ChannelEventTriggerHandler((Trigger) module, bundleContext); } else if (ItemCommandTriggerHandler.MODULE_TYPE_ID.equals(moduleTypeUID)) { - return new ItemCommandTriggerHandler((Trigger) module, bundleContext); + return new ItemCommandTriggerHandler((Trigger) module, ruleUID, bundleContext, itemRegistry); } else if (SystemTriggerHandler.STARTLEVEL_MODULE_TYPE_ID.equals(moduleTypeUID)) { return new SystemTriggerHandler((Trigger) module, bundleContext); } else if (ThingStatusTriggerHandler.CHANGE_MODULE_TYPE_ID.equals(moduleTypeUID) @@ -116,17 +116,17 @@ public Collection getTypes() { return new ThingStatusTriggerHandler((Trigger) module, bundleContext); } else if (ItemStateTriggerHandler.CHANGE_MODULE_TYPE_ID.equals(moduleTypeUID) || ItemStateTriggerHandler.UPDATE_MODULE_TYPE_ID.equals(moduleTypeUID)) { - return new ItemStateTriggerHandler((Trigger) module, bundleContext); + return new ItemStateTriggerHandler((Trigger) module, ruleUID, bundleContext, itemRegistry); } else if (GroupCommandTriggerHandler.MODULE_TYPE_ID.equals(moduleTypeUID)) { - return new GroupCommandTriggerHandler((Trigger) module, bundleContext, itemRegistry); + return new GroupCommandTriggerHandler((Trigger) module, ruleUID, bundleContext, itemRegistry); } else if (GroupStateTriggerHandler.CHANGE_MODULE_TYPE_ID.equals(moduleTypeUID) || GroupStateTriggerHandler.UPDATE_MODULE_TYPE_ID.equals(moduleTypeUID)) { - return new GroupStateTriggerHandler((Trigger) module, bundleContext, itemRegistry); + return new GroupStateTriggerHandler((Trigger) module, ruleUID, bundleContext, itemRegistry); } } else if (module instanceof Condition) { // Handle conditions if (ItemStateConditionHandler.ITEM_STATE_CONDITION.equals(moduleTypeUID)) { - return new ItemStateConditionHandler((Condition) module, itemRegistry); + return new ItemStateConditionHandler((Condition) module, ruleUID, bundleContext, itemRegistry); } else if (GenericEventConditionHandler.MODULETYPE_ID.equals(moduleTypeUID)) { return new GenericEventConditionHandler((Condition) module); } else if (CompareConditionHandler.MODULE_TYPE.equals(moduleTypeUID)) { diff --git a/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/module/handler/GroupCommandTriggerHandler.java b/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/module/handler/GroupCommandTriggerHandler.java index 0fb7496ece6..cb5e911091b 100644 --- a/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/module/handler/GroupCommandTriggerHandler.java +++ b/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/module/handler/GroupCommandTriggerHandler.java @@ -28,7 +28,9 @@ import org.openhab.core.events.EventSubscriber; import org.openhab.core.items.Item; import org.openhab.core.items.ItemRegistry; +import org.openhab.core.items.events.ItemAddedEvent; import org.openhab.core.items.events.ItemCommandEvent; +import org.openhab.core.items.events.ItemRemovedEvent; import org.openhab.core.types.Command; import org.osgi.framework.BundleContext; import org.osgi.framework.ServiceRegistration; @@ -59,21 +61,29 @@ public class GroupCommandTriggerHandler extends BaseTriggerModuleHandler impleme public static final String CFG_GROUPNAME = "groupName"; public static final String CFG_COMMAND = "command"; + private final String ruleUID; private ServiceRegistration eventSubscriberRegistration; - public GroupCommandTriggerHandler(Trigger module, BundleContext bundleContext, ItemRegistry itemRegistry) { + public GroupCommandTriggerHandler(Trigger module, String ruleUID, BundleContext bundleContext, + ItemRegistry itemRegistry) { super(module); this.groupName = (String) module.getConfiguration().get(CFG_GROUPNAME); this.command = (String) module.getConfiguration().get(CFG_COMMAND); - this.types = Set.of(ItemCommandEvent.TYPE); + this.types = Set.of(ItemCommandEvent.TYPE, ItemAddedEvent.TYPE, ItemRemovedEvent.TYPE); this.bundleContext = bundleContext; this.itemRegistry = itemRegistry; + this.ruleUID = ruleUID; Dictionary properties = new Hashtable<>(); - this.topic = "openhab/items/"; + this.topic = "openhab/items/*"; properties.put("event.topics", topic); eventSubscriberRegistration = this.bundleContext.registerService(EventSubscriber.class.getName(), this, properties); + + if (itemRegistry.get(groupName) == null) { + logger.warn("Group '{}' needed for rule '{}', trigger '{}' not present in registry. Trigger will not work.", + groupName, ruleUID, module.getId()); + } } @Override @@ -88,6 +98,20 @@ public Set getSubscribedEventTypes() { @Override public void receive(Event event) { + if (event instanceof ItemAddedEvent) { + if (groupName.equals(((ItemAddedEvent) event).getItem().name)) { + logger.info("Group '{}' needed for rule '{}', trigger '{}' added. Trigger will now work.", groupName, + ruleUID, module.getId()); + return; + } + } else if (event instanceof ItemRemovedEvent) { + if (groupName.equals(((ItemRemovedEvent) event).getItem().name)) { + logger.warn("Group '{}' needed for rule '{}', trigger '{}' removed. Trigger will no longer work.", + groupName, ruleUID, module.getId()); + return; + } + } + if (callback instanceof TriggerHandlerCallback) { TriggerHandlerCallback cb = (TriggerHandlerCallback) callback; logger.trace("Received Event: Source: {} Topic: {} Type: {} Payload: {}", event.getSource(), @@ -123,6 +147,6 @@ public void dispose() { @Override public boolean apply(Event event) { logger.trace("->FILTER: {}", event.getTopic()); - return event.getTopic().startsWith(topic); + return event.getTopic().startsWith("openhab/items/"); } } diff --git a/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/module/handler/GroupStateTriggerHandler.java b/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/module/handler/GroupStateTriggerHandler.java index 528ca72274a..ed857485642 100644 --- a/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/module/handler/GroupStateTriggerHandler.java +++ b/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/module/handler/GroupStateTriggerHandler.java @@ -29,6 +29,8 @@ import org.openhab.core.items.Item; import org.openhab.core.items.ItemRegistry; import org.openhab.core.items.events.GroupItemStateChangedEvent; +import org.openhab.core.items.events.ItemAddedEvent; +import org.openhab.core.items.events.ItemRemovedEvent; import org.openhab.core.items.events.ItemStateChangedEvent; import org.openhab.core.items.events.ItemStateEvent; import org.openhab.core.types.State; @@ -59,28 +61,37 @@ public class GroupStateTriggerHandler extends BaseTriggerModuleHandler implement private final String groupName; private final @Nullable String state; private final String previousState; + private final String ruleUID; private Set types; private final BundleContext bundleContext; private ItemRegistry itemRegistry; private ServiceRegistration eventSubscriberRegistration; - public GroupStateTriggerHandler(Trigger module, BundleContext bundleContext, ItemRegistry itemRegistry) { + public GroupStateTriggerHandler(Trigger module, String ruleUID, BundleContext bundleContext, + ItemRegistry itemRegistry) { super(module); this.groupName = (String) module.getConfiguration().get(CFG_GROUPNAME); this.state = (String) module.getConfiguration().get(CFG_STATE); this.previousState = (String) module.getConfiguration().get(CFG_PREVIOUS_STATE); if (UPDATE_MODULE_TYPE_ID.equals(module.getTypeUID())) { - this.types = Set.of(ItemStateEvent.TYPE); + this.types = Set.of(ItemStateEvent.TYPE, ItemAddedEvent.TYPE, ItemRemovedEvent.TYPE); } else { - this.types = Set.of(ItemStateChangedEvent.TYPE, GroupItemStateChangedEvent.TYPE); + this.types = Set.of(ItemStateChangedEvent.TYPE, GroupItemStateChangedEvent.TYPE, ItemAddedEvent.TYPE, + ItemRemovedEvent.TYPE); } this.bundleContext = bundleContext; + this.ruleUID = ruleUID; this.itemRegistry = itemRegistry; Dictionary properties = new Hashtable<>(); properties.put("event.topics", "openhab/items/*"); eventSubscriberRegistration = this.bundleContext.registerService(EventSubscriber.class.getName(), this, properties); + + if (itemRegistry.get(groupName) == null) { + logger.warn("Group '{}' needed for rule '{}', trigger '{}' not present in registry. Trigger will not work.", + groupName, ruleUID, module.getId()); + } } @Override @@ -95,6 +106,20 @@ public Set getSubscribedEventTypes() { @Override public void receive(Event event) { + if (event instanceof ItemAddedEvent) { + if (groupName.equals(((ItemAddedEvent) event).getItem().name)) { + logger.info("Group '{}' needed for rule '{}', trigger '{}' added. Trigger will now work.", groupName, + ruleUID, module.getId()); + return; + } + } else if (event instanceof ItemRemovedEvent) { + if (groupName.equals(((ItemRemovedEvent) event).getItem().name)) { + logger.warn("Group '{}' needed for rule '{}', trigger '{}' removed. Trigger will no longer work.", + groupName, ruleUID, module.getId()); + return; + } + } + if (callback instanceof TriggerHandlerCallback) { TriggerHandlerCallback cb = (TriggerHandlerCallback) callback; logger.trace("Received Event: Source: {} Topic: {} Type: {} Payload: {}", event.getSource(), diff --git a/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/module/handler/ItemCommandTriggerHandler.java b/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/module/handler/ItemCommandTriggerHandler.java index 26cb2b2863f..69501890c21 100644 --- a/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/module/handler/ItemCommandTriggerHandler.java +++ b/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/module/handler/ItemCommandTriggerHandler.java @@ -27,7 +27,10 @@ import org.openhab.core.events.Event; import org.openhab.core.events.EventFilter; import org.openhab.core.events.EventSubscriber; +import org.openhab.core.items.ItemRegistry; +import org.openhab.core.items.events.ItemAddedEvent; import org.openhab.core.items.events.ItemCommandEvent; +import org.openhab.core.items.events.ItemRemovedEvent; import org.openhab.core.types.Command; import org.osgi.framework.BundleContext; import org.osgi.framework.ServiceRegistration; @@ -57,20 +60,27 @@ public class ItemCommandTriggerHandler extends BaseTriggerModuleHandler implemen private final Set types; private final BundleContext bundleContext; + private final String ruleUID; - private @Nullable ServiceRegistration eventSubscriberRegistration; + private final ServiceRegistration eventSubscriberRegistration; - public ItemCommandTriggerHandler(Trigger module, BundleContext bundleContext) { + public ItemCommandTriggerHandler(Trigger module, String ruleUID, BundleContext bundleContext, + ItemRegistry itemRegistry) { super(module); this.itemName = (String) module.getConfiguration().get(CFG_ITEMNAME); this.command = (String) module.getConfiguration().get(CFG_COMMAND); - this.types = Set.of(ItemCommandEvent.TYPE); this.bundleContext = bundleContext; + this.ruleUID = ruleUID; + this.types = Set.of(ItemCommandEvent.TYPE, ItemAddedEvent.TYPE, ItemRemovedEvent.TYPE); Dictionary properties = new Hashtable<>(); - this.topic = "openhab/items/" + itemName + "/command"; + this.topic = "openhab/items/" + itemName + "/*"; properties.put("event.topics", topic); eventSubscriberRegistration = this.bundleContext.registerService(EventSubscriber.class.getName(), this, properties); + if (itemRegistry.get(itemName) == null) { + logger.warn("Item '{}' needed for rule '{}', trigger '{}' not present in registry. Trigger will not work.", + itemName, ruleUID, module.getId()); + } } @Override @@ -85,6 +95,20 @@ public Set getSubscribedEventTypes() { @Override public void receive(Event event) { + if (event instanceof ItemAddedEvent) { + if (itemName.equals(((ItemAddedEvent) event).getItem().name)) { + logger.info("Item '{}' needed for rule '{}', trigger '{}' added. Trigger will now work.", itemName, + ruleUID, module.getId()); + return; + } + } else if (event instanceof ItemRemovedEvent) { + if (itemName.equals(((ItemRemovedEvent) event).getItem().name)) { + logger.warn("Item '{}' needed for rule '{}', trigger '{}' removed. Trigger will no longer work.", + itemName, ruleUID, module.getId()); + return; + } + } + ModuleHandlerCallback callback = this.callback; if (callback != null) { logger.trace("Received Event: Source: {} Topic: {} Type: {} Payload: {}", event.getSource(), @@ -108,15 +132,12 @@ public void receive(Event event) { @Override public void dispose() { super.dispose(); - if (eventSubscriberRegistration != null) { - eventSubscriberRegistration.unregister(); - eventSubscriberRegistration = null; - } + eventSubscriberRegistration.unregister(); } @Override public boolean apply(Event event) { logger.trace("->FILTER: {}:{}", event.getTopic(), itemName); - return event.getTopic().equals(topic); + return event.getTopic().contains("openhab/items/" + itemName + "/"); } } 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 d0ff0d53bea..cd11e3cd033 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 @@ -12,19 +12,30 @@ */ package org.openhab.core.automation.internal.module.handler; +import java.util.Dictionary; +import java.util.Hashtable; import java.util.Map; +import java.util.Set; 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.events.Event; +import org.openhab.core.events.EventFilter; +import org.openhab.core.events.EventSubscriber; import org.openhab.core.items.Item; import org.openhab.core.items.ItemNotFoundException; import org.openhab.core.items.ItemRegistry; +import org.openhab.core.items.events.ItemAddedEvent; +import org.openhab.core.items.events.ItemRemovedEvent; import org.openhab.core.library.types.DecimalType; import org.openhab.core.library.types.QuantityType; import org.openhab.core.library.unit.Units; import org.openhab.core.types.State; import org.openhab.core.types.TypeParser; +import org.osgi.framework.BundleContext; +import org.osgi.framework.ServiceRegistration; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -35,7 +46,7 @@ * @author Kai Kreuzer - refactored and simplified customized module handling */ @NonNullByDefault -public class ItemStateConditionHandler extends BaseConditionModuleHandler { +public class ItemStateConditionHandler extends BaseConditionModuleHandler implements EventSubscriber, EventFilter { /** * Constants for Config-Parameters corresponding to Definition in ItemModuleTypeDefinition.json @@ -49,15 +60,62 @@ public class ItemStateConditionHandler extends BaseConditionModuleHandler { public static final String ITEM_STATE_CONDITION = "core.ItemStateCondition"; private final ItemRegistry itemRegistry; + private final String ruleUID; + private final String itemName; + private final BundleContext bundleContext; + private final Set types; + private final ServiceRegistration eventSubscriberRegistration; - public ItemStateConditionHandler(Condition condition, ItemRegistry itemRegistry) { + public ItemStateConditionHandler(Condition condition, String ruleUID, BundleContext bundleContext, + ItemRegistry itemRegistry) { super(condition); this.itemRegistry = itemRegistry; + this.bundleContext = bundleContext; + this.itemName = (String) module.getConfiguration().get(ITEM_NAME); + this.types = Set.of(ItemAddedEvent.TYPE, ItemRemovedEvent.TYPE); + this.ruleUID = ruleUID; + + Dictionary properties = new Hashtable<>(); + properties.put("event.topics", "openhab/items/" + itemName + "/*"); + eventSubscriberRegistration = this.bundleContext.registerService(EventSubscriber.class.getName(), this, + properties); + + if (itemRegistry.get(itemName) == null) { + logger.warn( + "Item '{}' needed for rule '{}', condition '{}' not present in registry. Condition will not work.", + itemName, ruleUID, module.getId()); + } + } + + @Override + public Set getSubscribedEventTypes() { + return types; + } + + @Override + public @Nullable EventFilter getEventFilter() { + return this; + } + + @Override + public void receive(Event event) { + if (event instanceof ItemAddedEvent) { + if (itemName.equals(((ItemAddedEvent) event).getItem().name)) { + logger.info("Item '{}' needed for rule '{}', condition '{}' added. Condition will now work.", itemName, + ruleUID, module.getId()); + return; + } + } else if (event instanceof ItemRemovedEvent) { + if (itemName.equals(((ItemRemovedEvent) event).getItem().name)) { + logger.warn("Item '{}' needed for rule '{}', condition '{}' removed. Condition will no longer work.", + itemName, ruleUID, module.getId()); + return; + } + } } @Override public boolean isSatisfied(Map inputs) { - String itemName = (String) module.getConfiguration().get(ITEM_NAME); String state = (String) module.getConfiguration().get(STATE); String operator = (String) module.getConfiguration().get(OPERATOR); if (operator == null || state == null || itemName == null) { @@ -167,4 +225,16 @@ private boolean equalsToItemState(String itemName, String state) throws ItemNotF } return itemState.equals(compareState); } + + @Override + public void dispose() { + super.dispose(); + eventSubscriberRegistration.unregister(); + } + + @Override + public boolean apply(Event event) { + logger.trace("->FILTER: {}:{}", event.getTopic(), itemName); + return event.getTopic().contains("openhab/items/" + itemName + "/"); + } } diff --git a/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/module/handler/ItemStateTriggerHandler.java b/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/module/handler/ItemStateTriggerHandler.java index c29c521867f..1867e7b468e 100644 --- a/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/module/handler/ItemStateTriggerHandler.java +++ b/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/module/handler/ItemStateTriggerHandler.java @@ -27,7 +27,10 @@ import org.openhab.core.events.Event; import org.openhab.core.events.EventFilter; import org.openhab.core.events.EventSubscriber; +import org.openhab.core.items.ItemRegistry; import org.openhab.core.items.events.GroupItemStateChangedEvent; +import org.openhab.core.items.events.ItemAddedEvent; +import org.openhab.core.items.events.ItemRemovedEvent; import org.openhab.core.items.events.ItemStateChangedEvent; import org.openhab.core.items.events.ItemStateEvent; import org.openhab.core.types.State; @@ -59,26 +62,35 @@ public class ItemStateTriggerHandler extends BaseTriggerModuleHandler implements private final String itemName; private final @Nullable String state; private final String previousState; + private final String ruleUID; private Set types; private final BundleContext bundleContext; private @Nullable ServiceRegistration eventSubscriberRegistration; - public ItemStateTriggerHandler(Trigger module, BundleContext bundleContext) { + public ItemStateTriggerHandler(Trigger module, String ruleUID, BundleContext bundleContext, + ItemRegistry itemRegistry) { super(module); this.itemName = (String) module.getConfiguration().get(CFG_ITEMNAME); this.state = (String) module.getConfiguration().get(CFG_STATE); this.previousState = (String) module.getConfiguration().get(CFG_PREVIOUS_STATE); + this.ruleUID = ruleUID; if (UPDATE_MODULE_TYPE_ID.equals(module.getTypeUID())) { - this.types = Set.of(ItemStateEvent.TYPE); + this.types = Set.of(ItemStateEvent.TYPE, ItemAddedEvent.TYPE, ItemRemovedEvent.TYPE); } else { - this.types = Set.of(ItemStateChangedEvent.TYPE, GroupItemStateChangedEvent.TYPE); + this.types = Set.of(ItemStateChangedEvent.TYPE, GroupItemStateChangedEvent.TYPE, ItemAddedEvent.TYPE, + ItemRemovedEvent.TYPE); } this.bundleContext = bundleContext; Dictionary properties = new Hashtable<>(); properties.put("event.topics", "openhab/items/" + itemName + "/*"); eventSubscriberRegistration = this.bundleContext.registerService(EventSubscriber.class.getName(), this, properties); + + if (itemRegistry.get(itemName) == null) { + logger.warn("Item '{}' needed for rule '{}', trigger '{}' not present in registry. Trigger will not work.", + itemName, ruleUID, module.getId()); + } } @Override @@ -93,6 +105,20 @@ public Set getSubscribedEventTypes() { @Override public void receive(Event event) { + if (event instanceof ItemAddedEvent) { + if (itemName.equals(((ItemAddedEvent) event).getItem().name)) { + logger.info("Item '{}' needed for rule '{}', trigger '{}' added. Trigger will now work.", itemName, + ruleUID, module.getId()); + return; + } + } else if (event instanceof ItemRemovedEvent) { + if (itemName.equals(((ItemRemovedEvent) event).getItem().name)) { + logger.warn("Item '{}' needed for rule '{}', trigger '{}' removed. Trigger will no longer work.", + itemName, ruleUID, module.getId()); + return; + } + } + ModuleHandlerCallback callback = this.callback; if (callback != null) { logger.trace("Received Event: Source: {} Topic: {} Type: {} Payload: {}", event.getSource(), 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 index 4ca66d1a60c..4df6ccead1c 100644 --- 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 @@ -21,6 +21,7 @@ import org.eclipse.jdt.annotation.NonNullByDefault; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; @@ -28,16 +29,23 @@ import org.mockito.junit.jupiter.MockitoExtension; import org.mockito.junit.jupiter.MockitoSettings; import org.mockito.quality.Strictness; +import org.openhab.core.automation.Condition; 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.items.events.ItemAddedEvent; +import org.openhab.core.items.events.ItemEventFactory; +import org.openhab.core.items.events.ItemRemovedEvent; import org.openhab.core.library.items.NumberItem; +import org.openhab.core.library.items.SwitchItem; import org.openhab.core.library.types.DecimalType; import org.openhab.core.library.types.QuantityType; import org.openhab.core.library.unit.ImperialUnits; import org.openhab.core.library.unit.SIUnits; +import org.openhab.core.test.java.JavaTest; import org.openhab.core.types.State; +import org.osgi.framework.BundleContext; /** * Basic unit tests for {@link ItemStateConditionHandler}. @@ -47,7 +55,7 @@ @ExtendWith(MockitoExtension.class) @MockitoSettings(strictness = Strictness.LENIENT) @NonNullByDefault -public class ItemStateConditionHandlerTest { +public class ItemStateConditionHandlerTest extends JavaTest { public static class ParameterSet { public final String comparisonState; @@ -145,10 +153,12 @@ public static Collection lessThanOrEqualsParameters() { private final NumberItem item = new NumberItem(ITEM_NAME); private @NonNullByDefault({}) @Mock ItemRegistry mockItemRegistry; + private @NonNullByDefault({}) @Mock BundleContext mockBundleContext; @BeforeEach public void setup() throws ItemNotFoundException { when(mockItemRegistry.getItem(ITEM_NAME)).thenReturn(item); + when(mockItemRegistry.get(ITEM_NAME)).thenReturn(item); } @ParameterizedTest @@ -238,6 +248,41 @@ private ItemStateConditionHandler initItemStateConditionHandler(String operator, .withId("conditionId") // .withTypeUID(ItemStateConditionHandler.ITEM_STATE_CONDITION) // .withConfiguration(configuration); - return new ItemStateConditionHandler(builder.build(), mockItemRegistry); + return new ItemStateConditionHandler(builder.build(), "", mockBundleContext, mockItemRegistry); + } + + @Test + public void itemMessagesAreLogged() { + Configuration configuration = new Configuration(); + configuration.put(ItemStateConditionHandler.ITEM_NAME, ITEM_NAME); + configuration.put(ItemStateConditionHandler.OPERATOR, "="); + Condition condition = ConditionBuilder.create() // + .withId("conditionId") // + .withTypeUID(ItemStateConditionHandler.ITEM_STATE_CONDITION) // + .withConfiguration(configuration) // + .build(); + + setupInterceptedLogger(ItemStateConditionHandler.class, LogLevel.INFO); + + // missing on creation + when(mockItemRegistry.get(ITEM_NAME)).thenReturn(null); + ItemStateConditionHandler handler = new ItemStateConditionHandler(condition, "foo", mockBundleContext, + mockItemRegistry); + assertLogMessage(ItemStateConditionHandler.class, LogLevel.WARN, + "Item 'myItem' needed for rule 'foo', condition 'conditionId' not present in registry. Condition will not work."); + + // added later + ItemAddedEvent addedEvent = ItemEventFactory.createAddedEvent(new SwitchItem(ITEM_NAME)); + assertTrue(handler.apply(addedEvent)); + handler.receive(addedEvent); + assertLogMessage(ItemStateConditionHandler.class, LogLevel.INFO, + "Item 'myItem' needed for rule 'foo', condition 'conditionId' added. Condition will now work."); + + // removed later + ItemRemovedEvent removedEvent = ItemEventFactory.createRemovedEvent(new SwitchItem(ITEM_NAME)); + assertTrue(handler.apply(removedEvent)); + handler.receive(removedEvent); + assertLogMessage(ItemStateConditionHandler.class, LogLevel.WARN, + "Item 'myItem' needed for rule 'foo', condition 'conditionId' removed. Condition will no longer work."); } } From 8732e6c2d3516f8dee68650ff628520b2e23084d Mon Sep 17 00:00:00 2001 From: "Jan N. Klug" Date: Sun, 10 Jul 2022 18:45:27 +0200 Subject: [PATCH 2/2] Address review comments. Signed-off-by: Jan N. Klug --- .../module/handler/GroupCommandTriggerHandler.java | 12 ++++++------ .../module/handler/GroupStateTriggerHandler.java | 12 ++++++------ .../module/handler/ItemCommandTriggerHandler.java | 12 ++++++------ .../module/handler/ItemStateConditionHandler.java | 13 ++++++------- .../module/handler/ItemStateTriggerHandler.java | 12 ++++++------ .../handler/ItemStateConditionHandlerTest.java | 6 +++--- 6 files changed, 33 insertions(+), 34 deletions(-) diff --git a/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/module/handler/GroupCommandTriggerHandler.java b/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/module/handler/GroupCommandTriggerHandler.java index cb5e911091b..2cf1c7bb8d9 100644 --- a/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/module/handler/GroupCommandTriggerHandler.java +++ b/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/module/handler/GroupCommandTriggerHandler.java @@ -81,8 +81,8 @@ public GroupCommandTriggerHandler(Trigger module, String ruleUID, BundleContext properties); if (itemRegistry.get(groupName) == null) { - logger.warn("Group '{}' needed for rule '{}', trigger '{}' not present in registry. Trigger will not work.", - groupName, ruleUID, module.getId()); + logger.warn("Group '{}' needed for rule '{}' is missing. Trigger '{}' will not work.", groupName, ruleUID, + module.getId()); } } @@ -100,14 +100,14 @@ public Set getSubscribedEventTypes() { public void receive(Event event) { if (event instanceof ItemAddedEvent) { if (groupName.equals(((ItemAddedEvent) event).getItem().name)) { - logger.info("Group '{}' needed for rule '{}', trigger '{}' added. Trigger will now work.", groupName, - ruleUID, module.getId()); + logger.info("Group '{}' needed for rule '{}' added. Trigger '{}' will now work.", groupName, ruleUID, + module.getId()); return; } } else if (event instanceof ItemRemovedEvent) { if (groupName.equals(((ItemRemovedEvent) event).getItem().name)) { - logger.warn("Group '{}' needed for rule '{}', trigger '{}' removed. Trigger will no longer work.", - groupName, ruleUID, module.getId()); + logger.warn("Group '{}' needed for rule '{}' removed. Trigger '{}' will no longer work.", groupName, + ruleUID, module.getId()); return; } } diff --git a/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/module/handler/GroupStateTriggerHandler.java b/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/module/handler/GroupStateTriggerHandler.java index ed857485642..b52da9668c7 100644 --- a/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/module/handler/GroupStateTriggerHandler.java +++ b/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/module/handler/GroupStateTriggerHandler.java @@ -89,8 +89,8 @@ public GroupStateTriggerHandler(Trigger module, String ruleUID, BundleContext bu properties); if (itemRegistry.get(groupName) == null) { - logger.warn("Group '{}' needed for rule '{}', trigger '{}' not present in registry. Trigger will not work.", - groupName, ruleUID, module.getId()); + logger.warn("Group '{}' needed for rule '{}' is missing. Trigger '{}' will not work.", groupName, ruleUID, + module.getId()); } } @@ -108,14 +108,14 @@ public Set getSubscribedEventTypes() { public void receive(Event event) { if (event instanceof ItemAddedEvent) { if (groupName.equals(((ItemAddedEvent) event).getItem().name)) { - logger.info("Group '{}' needed for rule '{}', trigger '{}' added. Trigger will now work.", groupName, - ruleUID, module.getId()); + logger.info("Group '{}' needed for rule '{}' added. Trigger '{}' will now work.", groupName, ruleUID, + module.getId()); return; } } else if (event instanceof ItemRemovedEvent) { if (groupName.equals(((ItemRemovedEvent) event).getItem().name)) { - logger.warn("Group '{}' needed for rule '{}', trigger '{}' removed. Trigger will no longer work.", - groupName, ruleUID, module.getId()); + logger.warn("Group '{}' needed for rule '{}' removed. Trigger '{}' will no longer work.", groupName, + ruleUID, module.getId()); return; } } diff --git a/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/module/handler/ItemCommandTriggerHandler.java b/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/module/handler/ItemCommandTriggerHandler.java index 69501890c21..edc64971526 100644 --- a/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/module/handler/ItemCommandTriggerHandler.java +++ b/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/module/handler/ItemCommandTriggerHandler.java @@ -78,8 +78,8 @@ public ItemCommandTriggerHandler(Trigger module, String ruleUID, BundleContext b eventSubscriberRegistration = this.bundleContext.registerService(EventSubscriber.class.getName(), this, properties); if (itemRegistry.get(itemName) == null) { - logger.warn("Item '{}' needed for rule '{}', trigger '{}' not present in registry. Trigger will not work.", - itemName, ruleUID, module.getId()); + logger.warn("Item '{}' needed for rule '{}' is missing. Trigger '{}' will not work.", itemName, ruleUID, + module.getId()); } } @@ -97,14 +97,14 @@ public Set getSubscribedEventTypes() { public void receive(Event event) { if (event instanceof ItemAddedEvent) { if (itemName.equals(((ItemAddedEvent) event).getItem().name)) { - logger.info("Item '{}' needed for rule '{}', trigger '{}' added. Trigger will now work.", itemName, - ruleUID, module.getId()); + logger.info("Item '{}' needed for rule '{}' added. Trigger '{}' will now work.", itemName, ruleUID, + module.getId()); return; } } else if (event instanceof ItemRemovedEvent) { if (itemName.equals(((ItemRemovedEvent) event).getItem().name)) { - logger.warn("Item '{}' needed for rule '{}', trigger '{}' removed. Trigger will no longer work.", - itemName, ruleUID, module.getId()); + logger.warn("Item '{}' needed for rule '{}' removed. Trigger '{}' will no longer work.", itemName, + ruleUID, module.getId()); return; } } 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 cd11e3cd033..8f980270b38 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 @@ -81,9 +81,8 @@ public ItemStateConditionHandler(Condition condition, String ruleUID, BundleCont properties); if (itemRegistry.get(itemName) == null) { - logger.warn( - "Item '{}' needed for rule '{}', condition '{}' not present in registry. Condition will not work.", - itemName, ruleUID, module.getId()); + logger.warn("Item '{}' needed for rule '{}' is missing. Condition '{}' will not work.", itemName, ruleUID, + module.getId()); } } @@ -101,14 +100,14 @@ public Set getSubscribedEventTypes() { public void receive(Event event) { if (event instanceof ItemAddedEvent) { if (itemName.equals(((ItemAddedEvent) event).getItem().name)) { - logger.info("Item '{}' needed for rule '{}', condition '{}' added. Condition will now work.", itemName, - ruleUID, module.getId()); + logger.info("Item '{}' needed for rule '{}' added. Condition '{}' will now work.", itemName, ruleUID, + module.getId()); return; } } else if (event instanceof ItemRemovedEvent) { if (itemName.equals(((ItemRemovedEvent) event).getItem().name)) { - logger.warn("Item '{}' needed for rule '{}', condition '{}' removed. Condition will no longer work.", - itemName, ruleUID, module.getId()); + logger.warn("Item '{}' needed for rule '{}' removed. Condition '{}' will no longer work.", itemName, + ruleUID, module.getId()); return; } } diff --git a/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/module/handler/ItemStateTriggerHandler.java b/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/module/handler/ItemStateTriggerHandler.java index 1867e7b468e..5dccd867c46 100644 --- a/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/module/handler/ItemStateTriggerHandler.java +++ b/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/module/handler/ItemStateTriggerHandler.java @@ -88,8 +88,8 @@ public ItemStateTriggerHandler(Trigger module, String ruleUID, BundleContext bun properties); if (itemRegistry.get(itemName) == null) { - logger.warn("Item '{}' needed for rule '{}', trigger '{}' not present in registry. Trigger will not work.", - itemName, ruleUID, module.getId()); + logger.warn("Item '{}' needed for rule '{}' is missing. Trigger '{}' will not work.", itemName, ruleUID, + module.getId()); } } @@ -107,14 +107,14 @@ public Set getSubscribedEventTypes() { public void receive(Event event) { if (event instanceof ItemAddedEvent) { if (itemName.equals(((ItemAddedEvent) event).getItem().name)) { - logger.info("Item '{}' needed for rule '{}', trigger '{}' added. Trigger will now work.", itemName, - ruleUID, module.getId()); + logger.info("Item '{}' needed for rule '{}' added. Trigger '{}' will now work.", itemName, ruleUID, + module.getId()); return; } } else if (event instanceof ItemRemovedEvent) { if (itemName.equals(((ItemRemovedEvent) event).getItem().name)) { - logger.warn("Item '{}' needed for rule '{}', trigger '{}' removed. Trigger will no longer work.", - itemName, ruleUID, module.getId()); + logger.warn("Item '{}' needed for rule '{}' removed. Trigger '{}' will no longer work.", itemName, + ruleUID, module.getId()); return; } } 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 index 4df6ccead1c..6beddedef03 100644 --- 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 @@ -269,20 +269,20 @@ public void itemMessagesAreLogged() { ItemStateConditionHandler handler = new ItemStateConditionHandler(condition, "foo", mockBundleContext, mockItemRegistry); assertLogMessage(ItemStateConditionHandler.class, LogLevel.WARN, - "Item 'myItem' needed for rule 'foo', condition 'conditionId' not present in registry. Condition will not work."); + "Item 'myItem' needed for rule 'foo' is missing. Condition 'conditionId' will not work."); // added later ItemAddedEvent addedEvent = ItemEventFactory.createAddedEvent(new SwitchItem(ITEM_NAME)); assertTrue(handler.apply(addedEvent)); handler.receive(addedEvent); assertLogMessage(ItemStateConditionHandler.class, LogLevel.INFO, - "Item 'myItem' needed for rule 'foo', condition 'conditionId' added. Condition will now work."); + "Item 'myItem' needed for rule 'foo' added. Condition 'conditionId' will now work."); // removed later ItemRemovedEvent removedEvent = ItemEventFactory.createRemovedEvent(new SwitchItem(ITEM_NAME)); assertTrue(handler.apply(removedEvent)); handler.receive(removedEvent); assertLogMessage(ItemStateConditionHandler.class, LogLevel.WARN, - "Item 'myItem' needed for rule 'foo', condition 'conditionId' removed. Condition will no longer work."); + "Item 'myItem' needed for rule 'foo' removed. Condition 'conditionId' will no longer work."); } }