From c349027b90750623fb91f92f175fccd9ebae7f2a Mon Sep 17 00:00:00 2001 From: Sami Salonen Date: Sun, 8 May 2022 13:59:00 +0300 Subject: [PATCH 1/4] [expiry] [expire] bool configuration parameters parsing as util method Signed-off-by: Sami Salonen --- .../core/internal/items/ExpireManager.java | 30 ++++++++++++++----- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/bundles/org.openhab.core/src/main/java/org/openhab/core/internal/items/ExpireManager.java b/bundles/org.openhab.core/src/main/java/org/openhab/core/internal/items/ExpireManager.java index fa63e8e65e2..601f5cb39f2 100644 --- a/bundles/org.openhab.core/src/main/java/org/openhab/core/internal/items/ExpireManager.java +++ b/bundles/org.openhab.core/src/main/java/org/openhab/core/internal/items/ExpireManager.java @@ -330,14 +330,7 @@ public ExpireConfig(Item item, String configString, Map configur ? configString.substring(commaPos + 1).trim() : null; - Object ignoreStateUpdatesConfigObject = configuration.get(CONFIG_IGNORE_STATE_UPDATES); - if (ignoreStateUpdatesConfigObject instanceof String) { - ignoreStateUpdates = Boolean.parseBoolean((String) ignoreStateUpdatesConfigObject); - } else if (ignoreStateUpdatesConfigObject instanceof Boolean) { - ignoreStateUpdates = (Boolean) ignoreStateUpdatesConfigObject; - } else { - ignoreStateUpdates = false; - } + ignoreStateUpdates = getBooleanConfigValue(configuration, CONFIG_IGNORE_STATE_UPDATES); if ((stateOrCommand != null) && (stateOrCommand.length() > 0)) { if (stateOrCommand.startsWith(COMMAND_PREFIX)) { @@ -375,6 +368,27 @@ public ExpireConfig(Item item, String configString, Map configur } } + /** + * Parse configuration value as primitive boolean. Supports parsing of String and Boolean values. + * + * @param configuration map of configuration keys and values + * @param configKey configuration key to lookup configuration map + * @return configuration value parsed as boolean. Defaults to false when configKey is not present in + * configuration + */ + private boolean getBooleanConfigValue(Map configuration, String configKey) { + boolean configValue; + Object configValueObject = configuration.get(configKey); + if (configValueObject instanceof String) { + configValue = Boolean.parseBoolean((String) configValueObject); + } else if (configValueObject instanceof Boolean) { + configValue = (Boolean) configValueObject; + } else { + configValue = false; + } + return configValue; + } + private Duration parseDuration(String durationString) throws IllegalArgumentException { Matcher m = DURATION_PATTERN.matcher(durationString); if (!m.matches() || (m.group(1) == null && m.group(2) == null && m.group(3) == null)) { From 74f2088fb542ab90e29c312070677faadc56497d Mon Sep 17 00:00:00 2001 From: Sami Salonen Date: Sun, 8 May 2022 14:20:51 +0300 Subject: [PATCH 2/4] [expiry] [expire] extend ExpiryManager to allow ignoring commands Similar to ignoreStateUpdates parameter, this introduces new optional parameter ignoreCommands. Signed-off-by: Sami Salonen --- .../core/internal/items/ExpireManager.java | 44 +++++++++-- .../internal/items/ExpireManagerTest.java | 74 +++++++++++++++++++ 2 files changed, 110 insertions(+), 8 deletions(-) diff --git a/bundles/org.openhab.core/src/main/java/org/openhab/core/internal/items/ExpireManager.java b/bundles/org.openhab.core/src/main/java/org/openhab/core/internal/items/ExpireManager.java index 601f5cb39f2..fa666e8ab50 100644 --- a/bundles/org.openhab.core/src/main/java/org/openhab/core/internal/items/ExpireManager.java +++ b/bundles/org.openhab.core/src/main/java/org/openhab/core/internal/items/ExpireManager.java @@ -141,11 +141,13 @@ protected void deactivate() { itemExpireMap.clear(); } - private void processEvent(String itemName, Type type, boolean isStateUpdate) { - logger.trace("Received '{}' for item {}, state update = {}", type, itemName, isStateUpdate); + private void processEvent(String itemName, Type type, EventType eventType) { + logger.trace("Received '{}' for item {}, event type= {}", type, itemName, eventType); ExpireConfig expireConfig = getExpireConfig(itemName); if (expireConfig != null) { - if (isStateUpdate && expireConfig.ignoreStateUpdates) { + if (eventType == EventType.StateUpdate && expireConfig.ignoreStateUpdates) { + return; + } else if (eventType == EventType.Command && expireConfig.ignoreCommands) { return; } Command expireCommand = expireConfig.expireCommand; @@ -244,13 +246,13 @@ public void receive(Event event) { } if (event instanceof ItemStateEvent) { ItemStateEvent isEvent = (ItemStateEvent) event; - processEvent(isEvent.getItemName(), isEvent.getItemState(), true); + processEvent(isEvent.getItemName(), isEvent.getItemState(), EventType.StateUpdate); } else if (event instanceof ItemCommandEvent) { ItemCommandEvent icEvent = (ItemCommandEvent) event; - processEvent(icEvent.getItemName(), icEvent.getItemCommand(), false); + processEvent(icEvent.getItemName(), icEvent.getItemCommand(), EventType.Command); } else if (event instanceof ItemStateChangedEvent) { ItemStateChangedEvent icEvent = (ItemStateChangedEvent) event; - processEvent(icEvent.getItemName(), icEvent.getItemState(), false); + processEvent(icEvent.getItemName(), icEvent.getItemState(), EventType.StateChanged); } } @@ -289,6 +291,7 @@ public void updated(Metadata oldElement, Metadata element) { static class ExpireConfig { static final String CONFIG_IGNORE_STATE_UPDATES = "ignoreStateUpdates"; + static final String CONFIG_IGNORE_COMMANDS = "ignoreCommands"; private static final StringType STRING_TYPE_NULL_HYPHEN = new StringType("'NULL'"); private static final StringType STRING_TYPE_NULL = new StringType("NULL"); @@ -306,13 +309,14 @@ static class ExpireConfig { final String durationString; final Duration duration; final boolean ignoreStateUpdates; + final boolean ignoreCommands; /** * Construct an ExpireConfig from the config string. * * Valid syntax: * - * {@code <duration>[,(state=|command=|)<stateOrCommand>][,ignoreStateUpdates]}
+ * {@code <duration>[,(state=|command=|)<stateOrCommand>][,ignoreStateUpdates][,ignoreCommands]}
* if neither state= or command= is present, assume state * * @param item the item to which we are bound @@ -331,6 +335,7 @@ public ExpireConfig(Item item, String configString, Map configur : null; ignoreStateUpdates = getBooleanConfigValue(configuration, CONFIG_IGNORE_STATE_UPDATES); + ignoreCommands = getBooleanConfigValue(configuration, CONFIG_IGNORE_COMMANDS); if ((stateOrCommand != null) && (stateOrCommand.length() > 0)) { if (stateOrCommand.startsWith(COMMAND_PREFIX)) { @@ -412,7 +417,30 @@ private Duration parseDuration(String durationString) throws IllegalArgumentExce @Override public String toString() { return "duration='" + durationString + "', s=" + duration.toSeconds() + ", state='" + expireState - + "', command='" + expireCommand + "', ignoreStateUpdates=" + ignoreStateUpdates; + + "', command='" + expireCommand + "', ignoreStateUpdates=" + ignoreStateUpdates + + ", ignoreCommands=" + ignoreCommands; } } + + /** + * Type of event received + * + * @author Sami Salonen - Initial contribution + * + */ + static enum EventType { + /** + * State updated (but not changed) + */ + StateUpdate, + /** + * State changed + */ + StateChanged, + /** + * Command + */ + Command + + } } diff --git a/bundles/org.openhab.core/src/test/java/org/openhab/core/internal/items/ExpireManagerTest.java b/bundles/org.openhab.core/src/test/java/org/openhab/core/internal/items/ExpireManagerTest.java index 8a9fcfb00a4..13073c86f77 100644 --- a/bundles/org.openhab.core/src/test/java/org/openhab/core/internal/items/ExpireManagerTest.java +++ b/bundles/org.openhab.core/src/test/java/org/openhab/core/internal/items/ExpireManagerTest.java @@ -182,6 +182,58 @@ void testIgnoreStateUpdateDoesNotExtendExpiryOnStateUpdate() throws InterruptedE verify(eventPublisherMock, times(1)).post(any()); } + @Test + void testIgnoreCommandsExtendsExpiryOnStateChange() throws InterruptedException, ItemNotFoundException { + Item testItem = new NumberItem(ITEMNAME); + when(itemRegistryMock.getItem(ITEMNAME)).thenReturn(testItem); + when(metadataRegistryMock.get(METADATA_KEY)) + .thenReturn(config("2s", Map.of(ExpireConfig.CONFIG_IGNORE_COMMANDS, true))); + + Event event = ItemEventFactory.createStateEvent(ITEMNAME, new DecimalType(1)); + expireManager.receive(event); + Thread.sleep(1500L); + event = ItemEventFactory.createStateChangedEvent(ITEMNAME, new DecimalType(2), new DecimalType(1)); + expireManager.receive(event); + Thread.sleep(1500L); + verify(eventPublisherMock, never()).post(any()); + Thread.sleep(2000L); + verify(eventPublisherMock, times(1)).post(any()); + } + + @Test + void testIgnoreCommandsExtendsExpiryOnStateUpdate() throws InterruptedException, ItemNotFoundException { + Item testItem = new NumberItem(ITEMNAME); + when(itemRegistryMock.getItem(ITEMNAME)).thenReturn(testItem); + when(metadataRegistryMock.get(METADATA_KEY)) + .thenReturn(config("2s", Map.of(ExpireConfig.CONFIG_IGNORE_COMMANDS, true))); + + Event event = ItemEventFactory.createStateEvent(ITEMNAME, new DecimalType(1)); + expireManager.receive(event); + Thread.sleep(1500L); + event = ItemEventFactory.createStateEvent(ITEMNAME, new DecimalType(1)); + expireManager.receive(event); + Thread.sleep(1500L); + verify(eventPublisherMock, never()).post(any()); + Thread.sleep(2000L); + verify(eventPublisherMock, times(1)).post(any()); + } + + @Test + void testIgnoreCommandDoesNotExtendExpiryOnStateUpdate() throws InterruptedException, ItemNotFoundException { + Item testItem = new NumberItem(ITEMNAME); + when(itemRegistryMock.getItem(ITEMNAME)).thenReturn(testItem); + when(metadataRegistryMock.get(METADATA_KEY)) + .thenReturn(config("2s", Map.of(ExpireConfig.CONFIG_IGNORE_COMMANDS, true))); + + Event event = ItemEventFactory.createStateEvent(ITEMNAME, new DecimalType(1)); + expireManager.receive(event); + Thread.sleep(1500L); + event = ItemEventFactory.createCommandEvent(ITEMNAME, new DecimalType(1)); + expireManager.receive(event); + Thread.sleep(1500L); + verify(eventPublisherMock, times(1)).post(any()); + } + @Test void testMetadataChange() throws InterruptedException, ItemNotFoundException { Metadata md = new Metadata(METADATA_KEY, "1s", null); @@ -214,30 +266,35 @@ void testExpireConfig() { assertEquals(UnDefType.UNDEF, cfg.expireState); assertNull(cfg.expireCommand); assertFalse(cfg.ignoreStateUpdates); + assertFalse(cfg.ignoreCommands); cfg = new ExpireManager.ExpireConfig(testItem, "5h 3m 2s", Map.of()); assertEquals(Duration.ofHours(5).plusMinutes(3).plusSeconds(2), cfg.duration); assertEquals(UnDefType.UNDEF, cfg.expireState); assertNull(cfg.expireCommand); assertFalse(cfg.ignoreStateUpdates); + assertFalse(cfg.ignoreCommands); cfg = new ExpireManager.ExpireConfig(testItem, "1h,OFF", Map.of()); assertEquals(Duration.ofHours(1), cfg.duration); assertEquals(OnOffType.OFF, cfg.expireState); assertNull(cfg.expireCommand); assertFalse(cfg.ignoreStateUpdates); + assertFalse(cfg.ignoreCommands); cfg = new ExpireManager.ExpireConfig(testItem, "1h,state=OFF", Map.of()); assertEquals(Duration.ofHours(1), cfg.duration); assertEquals(OnOffType.OFF, cfg.expireState); assertNull(cfg.expireCommand); assertFalse(cfg.ignoreStateUpdates); + assertFalse(cfg.ignoreCommands); cfg = new ExpireManager.ExpireConfig(testItem, "1h,command=OFF", Map.of()); assertEquals(Duration.ofHours(1), cfg.duration); assertNull(cfg.expireState); assertEquals(OnOffType.OFF, cfg.expireCommand); assertFalse(cfg.ignoreStateUpdates); + assertFalse(cfg.ignoreCommands); cfg = new ExpireManager.ExpireConfig(testItem, "1h,command=OFF", Map.of(ExpireConfig.CONFIG_IGNORE_STATE_UPDATES, true)); @@ -245,6 +302,7 @@ void testExpireConfig() { assertNull(cfg.expireState); assertEquals(OnOffType.OFF, cfg.expireCommand); assertTrue(cfg.ignoreStateUpdates); + assertFalse(cfg.ignoreCommands); cfg = new ExpireManager.ExpireConfig(testItem, "5h 3m 2s", Map.of(ExpireConfig.CONFIG_IGNORE_STATE_UPDATES, "true")); @@ -252,6 +310,22 @@ void testExpireConfig() { assertEquals(UnDefType.UNDEF, cfg.expireState); assertNull(cfg.expireCommand); assertTrue(cfg.ignoreStateUpdates); + assertFalse(cfg.ignoreCommands); + + cfg = new ExpireManager.ExpireConfig(testItem, "1h,command=OFF", + Map.of(ExpireConfig.CONFIG_IGNORE_COMMANDS, true)); + assertEquals(Duration.ofHours(1), cfg.duration); + assertNull(cfg.expireState); + assertEquals(OnOffType.OFF, cfg.expireCommand); + assertFalse(cfg.ignoreStateUpdates); + assertTrue(cfg.ignoreCommands); + + cfg = new ExpireManager.ExpireConfig(testItem, "5h 3m 2s", Map.of(ExpireConfig.CONFIG_IGNORE_COMMANDS, "true")); + assertEquals(Duration.ofHours(5).plusMinutes(3).plusSeconds(2), cfg.duration); + assertEquals(UnDefType.UNDEF, cfg.expireState); + assertNull(cfg.expireCommand); + assertFalse(cfg.ignoreStateUpdates); + assertTrue(cfg.ignoreCommands); try { cfg = new ExpireManager.ExpireConfig(testItem, "1h,command=OPEN", Map.of()); From fa9493626981d37a3eb7af6c6bac7782dfc1c2f4 Mon Sep 17 00:00:00 2001 From: Sami Salonen Date: Mon, 9 May 2022 07:50:08 +0300 Subject: [PATCH 3/4] [expire] [expiry] Code cleaning Signed-off-by: Sami Salonen --- .../core/internal/items/ExpireManager.java | 84 ++++++++----------- 1 file changed, 35 insertions(+), 49 deletions(-) diff --git a/bundles/org.openhab.core/src/main/java/org/openhab/core/internal/items/ExpireManager.java b/bundles/org.openhab.core/src/main/java/org/openhab/core/internal/items/ExpireManager.java index fa666e8ab50..fadb3d3a13a 100644 --- a/bundles/org.openhab.core/src/main/java/org/openhab/core/internal/items/ExpireManager.java +++ b/bundles/org.openhab.core/src/main/java/org/openhab/core/internal/items/ExpireManager.java @@ -40,6 +40,7 @@ import org.openhab.core.items.MetadataRegistry; import org.openhab.core.items.events.GroupItemStateChangedEvent; import org.openhab.core.items.events.ItemCommandEvent; +import org.openhab.core.items.events.ItemEvent; import org.openhab.core.items.events.ItemEventFactory; import org.openhab.core.items.events.ItemStateChangedEvent; import org.openhab.core.items.events.ItemStateEvent; @@ -141,31 +142,23 @@ protected void deactivate() { itemExpireMap.clear(); } - private void processEvent(String itemName, Type type, EventType eventType) { - logger.trace("Received '{}' for item {}, event type= {}", type, itemName, eventType); - ExpireConfig expireConfig = getExpireConfig(itemName); - if (expireConfig != null) { - if (eventType == EventType.StateUpdate && expireConfig.ignoreStateUpdates) { - return; - } else if (eventType == EventType.Command && expireConfig.ignoreCommands) { - return; - } - Command expireCommand = expireConfig.expireCommand; - State expireState = expireConfig.expireState; - - if ((expireCommand != null && expireCommand.equals(type)) - || (expireState != null && expireState.equals(type))) { - // New event is expired command or state -> no further action needed - itemExpireMap.remove(itemName); // remove expire trigger until next update or command - logger.debug("Item {} received '{}'; stopping any future expiration.", itemName, type); - } else { - // New event is not the expired command or state, so add the trigger to the map - Duration duration = expireConfig.duration; - itemExpireMap.put(itemName, Instant.now().plus(duration)); - logger.debug("Item {} will expire (with '{}' {}) in {} ms", itemName, - expireCommand == null ? expireState : expireCommand, - expireCommand == null ? "state" : "command", duration); - } + private void processEvent(String itemName, Type stateOrCommand, ExpireConfig expireConfig, Class eventClz) { + logger.trace("Received '{}' for item {}, event type= {}", stateOrCommand, itemName, eventClz.getSimpleName()); + Command expireCommand = expireConfig.expireCommand; + State expireState = expireConfig.expireState; + + if ((expireCommand != null && expireCommand.equals(stateOrCommand)) + || (expireState != null && expireState.equals(stateOrCommand))) { + // New event is expired command or state -> no further action needed + itemExpireMap.remove(itemName); // remove expire trigger until next update or command + logger.debug("Item {} received '{}'; stopping any future expiration.", itemName, stateOrCommand); + } else { + // New event is not the expired command or state, so add the trigger to the map + Duration duration = expireConfig.duration; + itemExpireMap.put(itemName, Instant.now().plus(duration)); + logger.debug("Item {} will expire (with '{}' {}) in {} ms", itemName, + expireCommand == null ? expireState : expireCommand, expireCommand == null ? "state" : "command", + duration); } } @@ -244,15 +237,29 @@ public void receive(Event event) { if (!enabled) { return; } + if (!(event instanceof ItemEvent)) { + return; + } + + ItemEvent itemEvent = (ItemEvent) event; + String itemName = itemEvent.getItemName(); + ExpireConfig expireConfig = getExpireConfig(itemName); + if (event instanceof ItemStateEvent) { ItemStateEvent isEvent = (ItemStateEvent) event; - processEvent(isEvent.getItemName(), isEvent.getItemState(), EventType.StateUpdate); + if (expireConfig != null && !expireConfig.ignoreStateUpdates) { + processEvent(itemName, isEvent.getItemState(), expireConfig, event.getClass()); + } } else if (event instanceof ItemCommandEvent) { ItemCommandEvent icEvent = (ItemCommandEvent) event; - processEvent(icEvent.getItemName(), icEvent.getItemCommand(), EventType.Command); + if (expireConfig != null && !expireConfig.ignoreCommands) { + processEvent(itemName, icEvent.getItemCommand(), expireConfig, event.getClass()); + } } else if (event instanceof ItemStateChangedEvent) { ItemStateChangedEvent icEvent = (ItemStateChangedEvent) event; - processEvent(icEvent.getItemName(), icEvent.getItemState(), EventType.StateChanged); + if (expireConfig != null) { + processEvent(itemName, icEvent.getItemState(), expireConfig, event.getClass()); + } } } @@ -422,25 +429,4 @@ public String toString() { } } - /** - * Type of event received - * - * @author Sami Salonen - Initial contribution - * - */ - static enum EventType { - /** - * State updated (but not changed) - */ - StateUpdate, - /** - * State changed - */ - StateChanged, - /** - * Command - */ - Command - - } } From fc4152578214eb6a3a0c6cc179d34e8a4a56a517 Mon Sep 17 00:00:00 2001 From: Sami Salonen Date: Wed, 11 May 2022 18:41:53 +0300 Subject: [PATCH 4/4] [expire] [expiry] code cleanup + spotless Signed-off-by: Sami Salonen --- .../openhab/core/internal/items/ExpireManager.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/bundles/org.openhab.core/src/main/java/org/openhab/core/internal/items/ExpireManager.java b/bundles/org.openhab.core/src/main/java/org/openhab/core/internal/items/ExpireManager.java index fadb3d3a13a..72a68615330 100644 --- a/bundles/org.openhab.core/src/main/java/org/openhab/core/internal/items/ExpireManager.java +++ b/bundles/org.openhab.core/src/main/java/org/openhab/core/internal/items/ExpireManager.java @@ -244,22 +244,23 @@ public void receive(Event event) { ItemEvent itemEvent = (ItemEvent) event; String itemName = itemEvent.getItemName(); ExpireConfig expireConfig = getExpireConfig(itemName); + if (expireConfig == null) { + return; + } if (event instanceof ItemStateEvent) { ItemStateEvent isEvent = (ItemStateEvent) event; - if (expireConfig != null && !expireConfig.ignoreStateUpdates) { + if (!expireConfig.ignoreStateUpdates) { processEvent(itemName, isEvent.getItemState(), expireConfig, event.getClass()); } } else if (event instanceof ItemCommandEvent) { ItemCommandEvent icEvent = (ItemCommandEvent) event; - if (expireConfig != null && !expireConfig.ignoreCommands) { + if (!expireConfig.ignoreCommands) { processEvent(itemName, icEvent.getItemCommand(), expireConfig, event.getClass()); } } else if (event instanceof ItemStateChangedEvent) { ItemStateChangedEvent icEvent = (ItemStateChangedEvent) event; - if (expireConfig != null) { - processEvent(itemName, icEvent.getItemState(), expireConfig, event.getClass()); - } + processEvent(itemName, icEvent.getItemState(), expireConfig, event.getClass()); } } @@ -428,5 +429,4 @@ public String toString() { + ", ignoreCommands=" + ignoreCommands; } } - }