From 798dabe9c09d4afcc8e9aaa8106d6205c807ded2 Mon Sep 17 00:00:00 2001 From: Kai Kreuzer Date: Sun, 13 Sep 2020 20:30:27 +0200 Subject: [PATCH 01/10] added expire functionality Signed-off-by: Kai Kreuzer --- .../item/internal/GenericItemProvider.java | 3 +- .../core/internal/items/ExpireManager.java | 358 ++++++++++++++++++ 2 files changed, 360 insertions(+), 1 deletion(-) create mode 100644 bundles/org.openhab.core/src/main/java/org/openhab/core/internal/items/ExpireManager.java diff --git a/bundles/org.openhab.core.model.item/src/org/openhab/core/model/item/internal/GenericItemProvider.java b/bundles/org.openhab.core.model.item/src/org/openhab/core/model/item/internal/GenericItemProvider.java index 5d3880d469b..f4f5b885f95 100644 --- a/bundles/org.openhab.core.model.item/src/org/openhab/core/model/item/internal/GenericItemProvider.java +++ b/bundles/org.openhab.core.model.item/src/org/openhab/core/model/item/internal/GenericItemProvider.java @@ -36,6 +36,7 @@ import org.openhab.core.items.Item; import org.openhab.core.items.ItemFactory; import org.openhab.core.items.ItemProvider; +import org.openhab.core.items.ItemUtil; import org.openhab.core.items.dto.GroupFunctionDTO; import org.openhab.core.items.dto.ItemDTOMapper; import org.openhab.core.model.core.EventType; @@ -299,7 +300,7 @@ private void dispatchBindingsPerItemType(@Nullable BindingConfigReader reader, S if (model != null) { for (ModelItem modelItem : model.getItems()) { for (String itemType : itemTypes) { - if (itemType.equals(modelItem.getType())) { + if (itemType.equals(ItemUtil.getMainItemType(modelItem.getType()))) { Item item = createItemFromModelItem(modelItem); if (item != null) { internalDispatchBindings(reader, modelName, item, modelItem.getBindings()); 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 new file mode 100644 index 00000000000..6cf8901546a --- /dev/null +++ b/bundles/org.openhab.core/src/main/java/org/openhab/core/internal/items/ExpireManager.java @@ -0,0 +1,358 @@ +/** + * Copyright (c) 2010-2020 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.internal.items; + +import java.time.Duration; +import java.time.Instant; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.TimeUnit; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; +import org.openhab.core.common.ThreadPoolManager; +import org.openhab.core.common.registry.RegistryChangeListener; +import org.openhab.core.events.Event; +import org.openhab.core.events.EventFilter; +import org.openhab.core.events.EventPublisher; +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.Metadata; +import org.openhab.core.items.MetadataKey; +import org.openhab.core.items.MetadataRegistry; +import org.openhab.core.items.events.ItemCommandEvent; +import org.openhab.core.items.events.ItemEventFactory; +import org.openhab.core.items.events.ItemStateEvent; +import org.openhab.core.types.Command; +import org.openhab.core.types.State; +import org.openhab.core.types.TypeParser; +import org.openhab.core.types.UnDefType; +import org.osgi.service.component.annotations.Activate; +import org.osgi.service.component.annotations.Component; +import org.osgi.service.component.annotations.ConfigurationPolicy; +import org.osgi.service.component.annotations.Modified; +import org.osgi.service.component.annotations.Reference; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Component which takes care of sending item state expiry events. + * + * @author Kai Kreuzer - Initial contribution + * @author Michael Wyraz - Author of the 1.x expire binding, which this class is based on + */ +@NonNullByDefault +@Component(immediate = true, service = { ExpireManager.class, + EventSubscriber.class }, configurationPid = "org.openhab.expire", configurationPolicy = ConfigurationPolicy.OPTIONAL) +public class ExpireManager implements EventSubscriber, RegistryChangeListener { + + protected static final String EVENT_SOURCE = "org.openhab.core.expire"; + protected static final String METADATA_NAMESPACE = "expire"; + protected static final String PROPERTY_ENABLED = "enabled"; + + private static final Set SUBSCRIBED_EVENT_TYPES = Set.of(ItemStateEvent.TYPE, ItemCommandEvent.TYPE); + + private final Logger logger = LoggerFactory.getLogger(ExpireManager.class); + + private Map itemExpireConfig = new ConcurrentHashMap<>(); + private Map itemExpireMap = new ConcurrentHashMap<>(); + + private ScheduledExecutorService threadPool = ThreadPoolManager + .getScheduledPool(ThreadPoolManager.THREAD_POOL_NAME_COMMON); + + private final EventPublisher eventPublisher; + private final MetadataRegistry metadataRegistry; + private final ItemRegistry itemRegistry; + + private boolean enabled = true; + + private @Nullable ScheduledFuture expireJob; + + @Activate + public ExpireManager(Map configuration, final @Reference EventPublisher eventPublisher, + final @Reference MetadataRegistry metadataRegistry, final @Reference ItemRegistry itemRegistry) { + this.eventPublisher = eventPublisher; + this.metadataRegistry = metadataRegistry; + this.itemRegistry = itemRegistry; + + modified(configuration); + } + + @Modified + protected void modified(Map configuration) { + Object valueEnabled = configuration.get(PROPERTY_ENABLED); + if (valueEnabled != null) { + enabled = Boolean.parseBoolean(valueEnabled.toString()); + } + if (enabled) { + if (expireJob == null) { + expireJob = threadPool.scheduleWithFixedDelay(() -> { + if (!itemExpireMap.isEmpty()) { + for (String itemName : itemExpireConfig.keySet()) { + if (isReadyToExpire(itemName)) { + expire(itemName); + } + } + } + }, 1, 1, TimeUnit.SECONDS); + } + itemRegistry.addRegistryChangeListener(this); + } else { + itemRegistry.removeRegistryChangeListener(this); + if (expireJob != null) { + expireJob.cancel(true); + expireJob = null; + } + } + } + + public void receiveCommand(ItemCommandEvent commandEvent) { + Command command = commandEvent.getItemCommand(); + String itemName = commandEvent.getItemName(); + logger.trace("Received command '{}' for item {}", command, itemName); + ExpireConfig expireConfig = getExpireConfig(itemName); + if (expireConfig != null) { + Command expireCommand = expireConfig.expireCommand; + State expireState = expireConfig.expireState; + + if ((expireCommand != null && expireCommand.equals(command)) + || (expireState != null && expireState.equals(command))) { + // New command is expired command or state -> no further action needed + itemExpireMap.remove(itemName); // remove expire trigger until next update or command + logger.debug("Item {} received command '{}'; stopping any future expiration.", itemName, command); + } else { + // New command 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); + } + } + } + + protected void receiveUpdate(ItemStateEvent stateEvent) { + State state = stateEvent.getItemState(); + String itemName = stateEvent.getItemName(); + logger.trace("Received update '{}' for item {}", state, itemName); + ExpireConfig expireConfig = getExpireConfig(itemName); + if (expireConfig != null) { + Command expireCommand = expireConfig.expireCommand; + State expireState = expireConfig.expireState; + + if ((expireCommand != null && expireCommand.equals(state)) + || (expireState != null && expireState.equals(state))) { + // New state is expired command or state -> no further action needed + itemExpireMap.remove(itemName); // remove expire trigger until next update or command + logger.debug("Item {} received update '{}'; stopping any future expiration.", itemName, state); + } else { + // New state 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 @Nullable ExpireConfig getExpireConfig(String itemName) { + // the value might be null, so we explicitly check if an entry for that key exists + if (itemExpireConfig.containsKey(itemName)) { + return itemExpireConfig.get(itemName); + } else { + Metadata metadata = metadataRegistry.get(new MetadataKey(METADATA_NAMESPACE, itemName)); + if (metadata != null) { + try { + Item item = itemRegistry.getItem(itemName); + try { + ExpireConfig cfg = new ExpireConfig(item, metadata.getValue()); + itemExpireConfig.put(itemName, cfg); + return cfg; + } catch (IllegalArgumentException e) { + logger.warn("Expire config '{}' of item '{}' is invalid: {}", metadata.getValue(), itemName, + e.getMessage()); + } + } catch (ItemNotFoundException e) { + logger.debug("Item '{}' does not exist.", itemName); + } + } + return null; + } + } + + private void postCommand(String itemName, Command command) { + eventPublisher.post(ItemEventFactory.createCommandEvent(itemName, command, EVENT_SOURCE)); + } + + private void postUpdate(String itemName, State state) { + eventPublisher.post(ItemEventFactory.createStateEvent(itemName, state, EVENT_SOURCE)); + } + + private boolean isReadyToExpire(String itemName) { + Instant nextExpiry = itemExpireMap.get(itemName); + return (nextExpiry != null) && Instant.now().isAfter(nextExpiry); + } + + private void expire(String itemName) { + itemExpireMap.remove(itemName); // disable expire trigger until next update or command + ExpireConfig expireConfig = itemExpireConfig.get(itemName); + + if (expireConfig != null) { + Command expireCommand = expireConfig.expireCommand; + State expireState = expireConfig.expireState; + + if (expireCommand != null) { + logger.debug("Item {} received no command or update for {} - posting command '{}'", itemName, + expireConfig.duration, expireCommand); + postCommand(itemName, expireCommand); + } else if (expireState != null) { + logger.debug("Item {} received no command or update for {} - posting state '{}'", itemName, + expireConfig.duration, expireState); + postUpdate(itemName, expireState); + } + } + } + + @Override + public Set getSubscribedEventTypes() { + return SUBSCRIBED_EVENT_TYPES; + } + + @Override + public @Nullable EventFilter getEventFilter() { + return null; + } + + @Override + public void receive(Event event) { + if (event instanceof ItemStateEvent) { + receiveUpdate((ItemStateEvent) event); + } else if (event instanceof ItemCommandEvent) { + receiveCommand((ItemCommandEvent) event); + } + } + + private class ExpireConfig { + + protected static final String COMMAND_PREFIX = "command="; + protected static final String STATE_PREFIX = "state="; + + protected final Pattern durationPattern = Pattern.compile("(?:([0-9]+)H)?\\s*(?:([0-9]+)M)?\\s*(?:([0-9]+)S)?", + Pattern.CASE_INSENSITIVE); + + @Nullable + final Command expireCommand; + @Nullable + final State expireState; + final String durationString; + final Duration duration; + + /** + * Construct an ExpireConfig from the config string. + * + * Valid syntax: + * + * {@code <duration>[,[state=|command=|]<stateOrCommand>]}
+ * if neither state= or command= is present, assume state + * + * @param item the item to which we are bound + * @param configString the string that the user specified in the metadate + * @throws IllegalArgumentException if it is ill-formatted + */ + public ExpireConfig(Item item, String configString) throws IllegalArgumentException { + int commaPos = configString.indexOf(','); + + durationString = (commaPos >= 0) ? configString.substring(0, commaPos).trim() : configString.trim(); + duration = parseDuration(durationString); + + String stateOrCommand = ((commaPos >= 0) && (configString.length() - 1) > commaPos) + ? configString.substring(commaPos + 1).trim() + : null; + + if ((stateOrCommand != null) && (stateOrCommand.length() > 0)) { + if (stateOrCommand.startsWith(COMMAND_PREFIX)) { + String commandString = stateOrCommand.substring(COMMAND_PREFIX.length()); + expireCommand = TypeParser.parseCommand(item.getAcceptedCommandTypes(), commandString); + expireState = null; + if (expireCommand == null) { + throw new IllegalArgumentException("The string '" + commandString + + "' does not represent a valid command for item " + item.getName()); + } + } else { + if (stateOrCommand.startsWith(STATE_PREFIX)) { + stateOrCommand = stateOrCommand.substring(STATE_PREFIX.length()); + } + String stateString = stateOrCommand; + expireState = TypeParser.parseState(item.getAcceptedDataTypes(), stateString); + expireCommand = null; + if (expireState == null) { + throw new IllegalArgumentException("The string '" + stateString + + "' does not represent a valid state for item " + item.getName()); + } + } + } else { + // default is to post Undefined state + expireCommand = null; + expireState = UnDefType.UNDEF; + } + } + + private Duration parseDuration(String durationString) throws IllegalArgumentException { + Matcher m = durationPattern.matcher(durationString); + if (!m.matches() || (m.group(1) == null && m.group(2) == null && m.group(3) == null)) { + throw new IllegalArgumentException( + "Invalid duration: " + durationString + ". Expected something like: '1h 15m 30s'"); + } + + Duration duration = Duration.ZERO; + if (m.group(1) != null) { + duration = duration.plus(Duration.ofHours(Long.parseLong(m.group(1)))); + } + if (m.group(2) != null) { + duration = duration.plus(Duration.ofMinutes(Long.parseLong(m.group(2)))); + } + if (m.group(3) != null) { + duration = duration.plus(Duration.ofSeconds(Long.parseLong(m.group(3)))); + } + return duration; + } + + @Override + public String toString() { + return "duration='" + durationString + "', s=" + duration.toSeconds() + ", state='" + expireState + + "', command='" + expireCommand + "'"; + } + } + + @Override + public void added(Item item) { + } + + @Override + public void removed(Item item) { + itemExpireConfig.remove(item.getName()); + } + + @Override + public void updated(Item oldItem, Item item) { + itemExpireConfig.remove(item.getName()); + } +} From c50ab78f7000690f215673e3616f71b2c7af2184 Mon Sep 17 00:00:00 2001 From: Kai Kreuzer Date: Sat, 14 Nov 2020 21:13:58 +0100 Subject: [PATCH 02/10] added unit tests Signed-off-by: Kai Kreuzer --- .../openhab/core/events/AbstractEvent.java | 47 +++++ .../core/internal/items/ExpireManager.java | 2 +- .../internal/items/ExpireManagerTest.java | 181 ++++++++++++++++++ 3 files changed, 229 insertions(+), 1 deletion(-) create mode 100644 bundles/org.openhab.core/src/test/java/org/openhab/core/internal/items/ExpireManagerTest.java diff --git a/bundles/org.openhab.core/src/main/java/org/openhab/core/events/AbstractEvent.java b/bundles/org.openhab.core/src/main/java/org/openhab/core/events/AbstractEvent.java index 99a1b079a07..bad9162d7b7 100644 --- a/bundles/org.openhab.core/src/main/java/org/openhab/core/events/AbstractEvent.java +++ b/bundles/org.openhab.core/src/main/java/org/openhab/core/events/AbstractEvent.java @@ -56,4 +56,51 @@ public String getPayload() { public @Nullable String getSource() { return source; } + + @Override + public int hashCode() { + final int prime = 31; + int result = 1; + result = prime * result + ((payload == null) ? 0 : payload.hashCode()); + result = prime * result + ((source == null) ? 0 : source.hashCode()); + result = prime * result + ((topic == null) ? 0 : topic.hashCode()); + return result; + } + + @Override + public boolean equals(@Nullable Object obj) { + if (this == obj) { + return true; + } + if (obj == null) { + return false; + } + if (getClass() != obj.getClass()) { + return false; + } + AbstractEvent other = (AbstractEvent) obj; + if (payload == null) { + if (other.payload != null) { + return false; + } + } else if (!payload.equals(other.payload)) { + return false; + } + if (source == null) { + if (other.source != null) { + return false; + } + } else if (!source.equals(other.source)) { + return false; + } + if (topic == null) { + if (other.topic != null) { + return false; + } + } else if (!topic.equals(other.topic)) { + return false; + } + return true; + } + } 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 6cf8901546a..870dec805cf 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 @@ -250,7 +250,7 @@ public void receive(Event event) { } } - private class ExpireConfig { + class ExpireConfig { protected static final String COMMAND_PREFIX = "command="; protected static final String STATE_PREFIX = "state="; 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 new file mode 100644 index 00000000000..55480cb8847 --- /dev/null +++ b/bundles/org.openhab.core/src/test/java/org/openhab/core/internal/items/ExpireManagerTest.java @@ -0,0 +1,181 @@ +/** + * Copyright (c) 2010-2020 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.internal.items; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.*; +import static org.mockito.Mockito.*; + +import java.time.Duration; +import java.util.HashMap; + +import javax.measure.quantity.Temperature; + +import org.eclipse.jdt.annotation.Nullable; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.openhab.core.events.Event; +import org.openhab.core.events.EventPublisher; +import org.openhab.core.internal.items.ExpireManager.ExpireConfig; +import org.openhab.core.items.Item; +import org.openhab.core.items.ItemNotFoundException; +import org.openhab.core.items.ItemRegistry; +import org.openhab.core.items.Metadata; +import org.openhab.core.items.MetadataKey; +import org.openhab.core.items.MetadataRegistry; +import org.openhab.core.items.events.ItemEventFactory; +import org.openhab.core.library.items.NumberItem; +import org.openhab.core.library.items.SwitchItem; +import org.openhab.core.library.types.OnOffType; +import org.openhab.core.library.types.QuantityType; +import org.openhab.core.types.UnDefType; + +/** + * The {@link ExpireManagerTest} tests the {@link ExpireManager}. + * + * @author Kai Kreuzer - Initial contribution + */ +@ExtendWith(MockitoExtension.class) +class ExpireManagerTest { + + private static final String ITEMNAME = "Test"; + private static final MetadataKey METADATA_KEY = new MetadataKey(ExpireManager.METADATA_NAMESPACE, ITEMNAME); + + private ExpireManager expireManager; + private @Mock EventPublisher eventPublisher; + private @Mock MetadataRegistry metadataRegistry; + private @Mock ItemRegistry itemRegistry; + + @BeforeEach + public void setup() { + expireManager = new ExpireManager(new HashMap(), eventPublisher, metadataRegistry, + itemRegistry); + } + + @Test + void testDefaultStateExpiry() throws InterruptedException { + when(metadataRegistry.get(METADATA_KEY)).thenReturn(new Metadata(METADATA_KEY, "1s", null)); + + Event event = ItemEventFactory.createCommandEvent(ITEMNAME, OnOffType.ON); + + expireManager.receive(event); + + verify(eventPublisher, never()).post(any()); + Thread.sleep(2500L); + verify(eventPublisher) + .post(eq(ItemEventFactory.createStateEvent(ITEMNAME, UnDefType.UNDEF, ExpireManager.EVENT_SOURCE))); + } + + @Test + void testStateExpiryWithCustomState() throws InterruptedException, ItemNotFoundException { + Item testItem = new SwitchItem(ITEMNAME); + when(itemRegistry.getItem(ITEMNAME)).thenReturn(testItem); + when(metadataRegistry.get(METADATA_KEY)).thenReturn(config("1s,state=OFF")); + + Event event = ItemEventFactory.createCommandEvent(ITEMNAME, OnOffType.ON); + + expireManager.receive(event); + + verify(eventPublisher, never()).post(any()); + Thread.sleep(2500L); + verify(eventPublisher) + .post(eq(ItemEventFactory.createStateEvent(ITEMNAME, OnOffType.OFF, ExpireManager.EVENT_SOURCE))); + } + + @Test + void testStateExpiryWithCustomCommand() throws InterruptedException, ItemNotFoundException { + Item testItem = new SwitchItem(ITEMNAME); + when(itemRegistry.getItem(ITEMNAME)).thenReturn(testItem); + when(metadataRegistry.get(METADATA_KEY)).thenReturn(config("1s,command=ON")); + + Event event = ItemEventFactory.createCommandEvent(ITEMNAME, OnOffType.OFF); + + expireManager.receive(event); + + verify(eventPublisher, never()).post(any()); + Thread.sleep(2500L); + verify(eventPublisher) + .post(eq(ItemEventFactory.createCommandEvent(ITEMNAME, OnOffType.ON, ExpireManager.EVENT_SOURCE))); + } + + @Test + void testCancelExpiry() throws InterruptedException, ItemNotFoundException { + Item testItem = new SwitchItem(ITEMNAME); + when(itemRegistry.getItem(ITEMNAME)).thenReturn(testItem); + when(metadataRegistry.get(METADATA_KEY)).thenReturn(config("1s,ON")); + + Event event = ItemEventFactory.createCommandEvent(ITEMNAME, OnOffType.OFF); + expireManager.receive(event); + Thread.sleep(500L); + event = ItemEventFactory.createCommandEvent(ITEMNAME, OnOffType.ON); + expireManager.receive(event); + Thread.sleep(2000L); + verify(eventPublisher, never()).post(any()); + } + + @Test + void testExpireConfig() { + Item testItem = new SwitchItem(ITEMNAME); + ExpireConfig cfg = expireManager.new ExpireConfig(testItem, "1s"); + assertEquals(Duration.ofSeconds(1), cfg.duration); + assertEquals(UnDefType.UNDEF, cfg.expireState); + assertEquals(null, cfg.expireCommand); + + cfg = expireManager.new ExpireConfig(testItem, "5h 3m 2s"); + assertEquals(Duration.ofHours(5).plusMinutes(3).plusSeconds(2), cfg.duration); + assertEquals(UnDefType.UNDEF, cfg.expireState); + assertEquals(null, cfg.expireCommand); + + cfg = expireManager.new ExpireConfig(testItem, "1h,OFF"); + assertEquals(Duration.ofHours(1), cfg.duration); + assertEquals(OnOffType.OFF, cfg.expireState); + assertEquals(null, cfg.expireCommand); + + cfg = expireManager.new ExpireConfig(testItem, "1h,state=OFF"); + assertEquals(Duration.ofHours(1), cfg.duration); + assertEquals(OnOffType.OFF, cfg.expireState); + assertEquals(null, cfg.expireCommand); + + cfg = expireManager.new ExpireConfig(testItem, "1h,command=OFF"); + assertEquals(Duration.ofHours(1), cfg.duration); + assertEquals(null, cfg.expireState); + assertEquals(OnOffType.OFF, cfg.expireCommand); + + try { + cfg = expireManager.new ExpireConfig(testItem, "1h,command=OPEN"); + fail(); + } catch (IllegalArgumentException e) { + // expected as command is invalid + } + + try { + cfg = expireManager.new ExpireConfig(testItem, "1h,OPEN"); + fail(); + } catch (IllegalArgumentException e) { + // expected as state is invalid + } + + testItem = new NumberItem("Number:Temperature", ITEMNAME); + cfg = expireManager.new ExpireConfig(testItem, "1h,15 °C"); + assertEquals(Duration.ofHours(1), cfg.duration); + assertEquals(new QuantityType("15 °C"), cfg.expireState); + assertEquals(null, cfg.expireCommand); + } + + private Metadata config(String cfg) { + return new Metadata(METADATA_KEY, cfg, null); + } +} From 0644e0dc28c5cf8655f2e16c1c9623fd29e80dda Mon Sep 17 00:00:00 2001 From: Kai Kreuzer Date: Sun, 15 Nov 2020 10:55:03 +0100 Subject: [PATCH 03/10] fixed spotless problem Signed-off-by: Kai Kreuzer --- .../src/main/java/org/openhab/core/events/AbstractEvent.java | 1 - 1 file changed, 1 deletion(-) diff --git a/bundles/org.openhab.core/src/main/java/org/openhab/core/events/AbstractEvent.java b/bundles/org.openhab.core/src/main/java/org/openhab/core/events/AbstractEvent.java index bad9162d7b7..ea906a431f1 100644 --- a/bundles/org.openhab.core/src/main/java/org/openhab/core/events/AbstractEvent.java +++ b/bundles/org.openhab.core/src/main/java/org/openhab/core/events/AbstractEvent.java @@ -102,5 +102,4 @@ public boolean equals(@Nullable Object obj) { } return true; } - } From 93a8510508b32c0a0ee65699b7ec26192f7c3a99 Mon Sep 17 00:00:00 2001 From: Kai Kreuzer Date: Sat, 14 Nov 2020 11:29:53 +0100 Subject: [PATCH 04/10] Allow NULL and UNDEF as strings Signed-off-by: Kai Kreuzer --- .../core/internal/items/ExpireManager.java | 11 ++++++++++- .../core/internal/items/ExpireManagerTest.java | 18 ++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) 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 870dec805cf..f01bd4caf87 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.events.ItemCommandEvent; import org.openhab.core.items.events.ItemEventFactory; import org.openhab.core.items.events.ItemStateEvent; +import org.openhab.core.library.types.StringType; import org.openhab.core.types.Command; import org.openhab.core.types.State; import org.openhab.core.types.TypeParser; @@ -301,7 +302,15 @@ public ExpireConfig(Item item, String configString) throws IllegalArgumentExcept stateOrCommand = stateOrCommand.substring(STATE_PREFIX.length()); } String stateString = stateOrCommand; - expireState = TypeParser.parseState(item.getAcceptedDataTypes(), stateString); + State state = TypeParser.parseState(item.getAcceptedDataTypes(), stateString); + // do special handling to allow NULL and UNDEF as strings when being put in single quotes + if (new StringType("'NULL'").equals(state)) { + expireState = new StringType("NULL"); + } else if (new StringType("'UNDEF'").equals(state)) { + expireState = new StringType("UNDEF"); + } else { + expireState = state; + } expireCommand = null; if (expireState == null) { throw new IllegalArgumentException("The string '" + stateString 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 55480cb8847..a18155f3251 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 @@ -38,9 +38,11 @@ import org.openhab.core.items.MetadataRegistry; import org.openhab.core.items.events.ItemEventFactory; import org.openhab.core.library.items.NumberItem; +import org.openhab.core.library.items.StringItem; import org.openhab.core.library.items.SwitchItem; import org.openhab.core.library.types.OnOffType; import org.openhab.core.library.types.QuantityType; +import org.openhab.core.library.types.StringType; import org.openhab.core.types.UnDefType; /** @@ -173,6 +175,22 @@ void testExpireConfig() { assertEquals(Duration.ofHours(1), cfg.duration); assertEquals(new QuantityType("15 °C"), cfg.expireState); assertEquals(null, cfg.expireCommand); + + testItem = new StringItem(ITEMNAME); + cfg = expireManager.new ExpireConfig(testItem, "1h,NULL"); + assertEquals(Duration.ofHours(1), cfg.duration); + assertEquals(UnDefType.NULL, cfg.expireState); + assertEquals(null, cfg.expireCommand); + + cfg = expireManager.new ExpireConfig(testItem, "1h,'NULL'"); + assertEquals(Duration.ofHours(1), cfg.duration); + assertEquals(new StringType("NULL"), cfg.expireState); + assertEquals(null, cfg.expireCommand); + + cfg = expireManager.new ExpireConfig(testItem, "1h,'UNDEF'"); + assertEquals(Duration.ofHours(1), cfg.duration); + assertEquals(new StringType("UNDEF"), cfg.expireState); + assertEquals(null, cfg.expireCommand); } private Metadata config(String cfg) { From d8a6a14867be51a9b908198d84eea4064a58f8e9 Mon Sep 17 00:00:00 2001 From: Kai Kreuzer Date: Mon, 16 Nov 2020 19:42:01 +0100 Subject: [PATCH 05/10] directly return when being disabled Signed-off-by: Kai Kreuzer --- .../java/org/openhab/core/internal/items/ExpireManager.java | 3 +++ 1 file changed, 3 insertions(+) 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 f01bd4caf87..45f56e45e41 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,6 +244,9 @@ public Set getSubscribedEventTypes() { @Override public void receive(Event event) { + if (!enabled) { + return; + } if (event instanceof ItemStateEvent) { receiveUpdate((ItemStateEvent) event); } else if (event instanceof ItemCommandEvent) { From 79c256be3779703d783cae5a6afe597dd24b1752 Mon Sep 17 00:00:00 2001 From: Kai Kreuzer Date: Mon, 16 Nov 2020 19:55:12 +0100 Subject: [PATCH 06/10] do not retry config parsing Signed-off-by: Kai Kreuzer --- .../java/org/openhab/core/internal/items/ExpireManager.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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 45f56e45e41..7b23b97123a 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 @@ -72,7 +72,7 @@ public class ExpireManager implements EventSubscriber, RegistryChangeListener itemExpireConfig = new ConcurrentHashMap<>(); + private Map itemExpireConfig = new ConcurrentHashMap<>(); private Map itemExpireMap = new ConcurrentHashMap<>(); private ScheduledExecutorService threadPool = ThreadPoolManager @@ -195,6 +195,8 @@ protected void receiveUpdate(ItemStateEvent stateEvent) { logger.debug("Item '{}' does not exist.", itemName); } } + // also fill the map when there is no config, so that we do not retry to find one + itemExpireConfig.put(itemName, null); return null; } } @@ -356,6 +358,7 @@ public String toString() { @Override public void added(Item item) { + itemExpireConfig.remove(item.getName()); } @Override From 073a2e4245e4813dcb2a95c626a677b6b5514b39 Mon Sep 17 00:00:00 2001 From: Kai Kreuzer Date: Mon, 16 Nov 2020 22:43:46 +0100 Subject: [PATCH 07/10] fixed potential NPE when no metadata is available in cache Signed-off-by: Kai Kreuzer --- .../core/internal/items/ExpireManager.java | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 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 7b23b97123a..939f5147e78 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 @@ -15,6 +15,7 @@ import java.time.Duration; import java.time.Instant; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ScheduledExecutorService; @@ -175,9 +176,9 @@ protected void receiveUpdate(ItemStateEvent stateEvent) { } private @Nullable ExpireConfig getExpireConfig(String itemName) { - // the value might be null, so we explicitly check if an entry for that key exists - if (itemExpireConfig.containsKey(itemName)) { - return itemExpireConfig.get(itemName); + Optional itemConfig = itemExpireConfig.get(itemName); + if (itemConfig != null) { + return itemConfig.isPresent() ? itemConfig.get() : null; } else { Metadata metadata = metadataRegistry.get(new MetadataKey(METADATA_NAMESPACE, itemName)); if (metadata != null) { @@ -185,7 +186,7 @@ protected void receiveUpdate(ItemStateEvent stateEvent) { Item item = itemRegistry.getItem(itemName); try { ExpireConfig cfg = new ExpireConfig(item, metadata.getValue()); - itemExpireConfig.put(itemName, cfg); + itemExpireConfig.put(itemName, Optional.of(cfg)); return cfg; } catch (IllegalArgumentException e) { logger.warn("Expire config '{}' of item '{}' is invalid: {}", metadata.getValue(), itemName, @@ -196,7 +197,7 @@ protected void receiveUpdate(ItemStateEvent stateEvent) { } } // also fill the map when there is no config, so that we do not retry to find one - itemExpireConfig.put(itemName, null); + itemExpireConfig.put(itemName, Optional.empty()); return null; } } @@ -216,19 +217,19 @@ private boolean isReadyToExpire(String itemName) { private void expire(String itemName) { itemExpireMap.remove(itemName); // disable expire trigger until next update or command - ExpireConfig expireConfig = itemExpireConfig.get(itemName); + Optional expireConfig = itemExpireConfig.get(itemName); - if (expireConfig != null) { - Command expireCommand = expireConfig.expireCommand; - State expireState = expireConfig.expireState; + if (expireConfig != null && expireConfig.isPresent()) { + Command expireCommand = expireConfig.get().expireCommand; + State expireState = expireConfig.get().expireState; if (expireCommand != null) { logger.debug("Item {} received no command or update for {} - posting command '{}'", itemName, - expireConfig.duration, expireCommand); + expireConfig.get().duration, expireCommand); postCommand(itemName, expireCommand); } else if (expireState != null) { logger.debug("Item {} received no command or update for {} - posting state '{}'", itemName, - expireConfig.duration, expireState); + expireConfig.get().duration, expireState); postUpdate(itemName, expireState); } } From d7d71fbc903b4570d85d986573ba89c79cc08b18 Mon Sep 17 00:00:00 2001 From: Kai Kreuzer Date: Mon, 16 Nov 2020 22:44:09 +0100 Subject: [PATCH 08/10] listen to changes in metadata registry Signed-off-by: Kai Kreuzer --- .../core/internal/items/ExpireManager.java | 54 +++++++++++++------ .../internal/items/ExpireManagerTest.java | 24 +++++++++ 2 files changed, 62 insertions(+), 16 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 939f5147e78..edb1a50488c 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 @@ -73,7 +73,7 @@ public class ExpireManager implements EventSubscriber, RegistryChangeListener itemExpireConfig = new ConcurrentHashMap<>(); + private Map> itemExpireConfig = new ConcurrentHashMap<>(); private Map itemExpireMap = new ConcurrentHashMap<>(); private ScheduledExecutorService threadPool = ThreadPoolManager @@ -82,6 +82,7 @@ public class ExpireManager implements EventSubscriber, RegistryChangeListener configuration) { }, 1, 1, TimeUnit.SECONDS); } itemRegistry.addRegistryChangeListener(this); + metadataRegistry.addRegistryChangeListener(metadataChangeListener); } else { itemRegistry.removeRegistryChangeListener(this); + metadataRegistry.removeRegistryChangeListener(metadataChangeListener); if (expireJob != null) { expireJob.cancel(true); expireJob = null; @@ -257,6 +260,40 @@ public void receive(Event event) { } } + @Override + public void added(Item item) { + itemExpireConfig.remove(item.getName()); + } + + @Override + public void removed(Item item) { + itemExpireConfig.remove(item.getName()); + } + + @Override + public void updated(Item oldItem, Item item) { + itemExpireConfig.remove(item.getName()); + } + + class MetadataChangeListener implements RegistryChangeListener { + + @Override + public void added(Metadata element) { + itemExpireConfig.remove(element.getUID().getItemName()); + } + + @Override + public void removed(Metadata element) { + itemExpireConfig.remove(element.getUID().getItemName()); + } + + @Override + public void updated(Metadata oldElement, Metadata element) { + itemExpireConfig.remove(element.getUID().getItemName()); + } + + } + class ExpireConfig { protected static final String COMMAND_PREFIX = "command="; @@ -356,19 +393,4 @@ public String toString() { + "', command='" + expireCommand + "'"; } } - - @Override - public void added(Item item) { - itemExpireConfig.remove(item.getName()); - } - - @Override - public void removed(Item item) { - itemExpireConfig.remove(item.getName()); - } - - @Override - public void updated(Item oldItem, Item item) { - itemExpireConfig.remove(item.getName()); - } } 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 a18155f3251..822db948e9f 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 @@ -128,6 +128,30 @@ void testCancelExpiry() throws InterruptedException, ItemNotFoundException { verify(eventPublisher, never()).post(any()); } + @Test + void testMetadataChange() throws InterruptedException, ItemNotFoundException { + Metadata md = new Metadata(METADATA_KEY, "1s", null); + when(metadataRegistry.get(METADATA_KEY)).thenReturn(md); + + Event event = ItemEventFactory.createCommandEvent(ITEMNAME, OnOffType.ON); + expireManager.receive(event); + + verify(eventPublisher, never()).post(any()); + Thread.sleep(2500L); + verify(eventPublisher) + .post(eq(ItemEventFactory.createStateEvent(ITEMNAME, UnDefType.UNDEF, ExpireManager.EVENT_SOURCE))); + + when(metadataRegistry.get(METADATA_KEY)).thenReturn(null); + expireManager.metadataChangeListener.removed(md); + reset(eventPublisher); + + event = ItemEventFactory.createCommandEvent(ITEMNAME, OnOffType.ON); + expireManager.receive(event); + verify(eventPublisher, never()).post(any()); + Thread.sleep(2500L); + verify(eventPublisher, never()).post(any()); + } + @Test void testExpireConfig() { Item testItem = new SwitchItem(ITEMNAME); From 0570b8407ef159e4747ebbe03f2b65e450a2527d Mon Sep 17 00:00:00 2001 From: Kai Kreuzer Date: Tue, 17 Nov 2020 08:15:06 +0100 Subject: [PATCH 09/10] spotless fix Signed-off-by: Kai Kreuzer --- .../main/java/org/openhab/core/internal/items/ExpireManager.java | 1 - 1 file changed, 1 deletion(-) 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 edb1a50488c..57d6d27df5c 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 @@ -291,7 +291,6 @@ public void removed(Metadata element) { public void updated(Metadata oldElement, Metadata element) { itemExpireConfig.remove(element.getUID().getItemName()); } - } class ExpireConfig { From eb88b0e2f5746b2081046cbccd194157125ed4c6 Mon Sep 17 00:00:00 2001 From: Kai Kreuzer Date: Tue, 17 Nov 2020 21:33:08 +0100 Subject: [PATCH 10/10] addressed review comments Signed-off-by: Kai Kreuzer --- .../core/internal/items/ExpireManager.java | 84 ++++++++----------- .../internal/items/ExpireManagerTest.java | 22 ++--- 2 files changed, 48 insertions(+), 58 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 57d6d27df5c..308e1678f0c 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 @@ -44,11 +44,13 @@ import org.openhab.core.library.types.StringType; import org.openhab.core.types.Command; import org.openhab.core.types.State; +import org.openhab.core.types.Type; import org.openhab.core.types.TypeParser; import org.openhab.core.types.UnDefType; import org.osgi.service.component.annotations.Activate; import org.osgi.service.component.annotations.Component; import org.osgi.service.component.annotations.ConfigurationPolicy; +import org.osgi.service.component.annotations.Deactivate; import org.osgi.service.component.annotations.Modified; import org.osgi.service.component.annotations.Reference; import org.slf4j.Logger; @@ -105,7 +107,8 @@ protected void modified(Map configuration) { enabled = Boolean.parseBoolean(valueEnabled.toString()); } if (enabled) { - if (expireJob == null) { + ScheduledFuture localExpireJob = expireJob; + if (localExpireJob == null) { expireJob = threadPool.scheduleWithFixedDelay(() -> { if (!itemExpireMap.isEmpty()) { for (String itemName : itemExpireConfig.keySet()) { @@ -119,56 +122,36 @@ protected void modified(Map configuration) { itemRegistry.addRegistryChangeListener(this); metadataRegistry.addRegistryChangeListener(metadataChangeListener); } else { - itemRegistry.removeRegistryChangeListener(this); - metadataRegistry.removeRegistryChangeListener(metadataChangeListener); - if (expireJob != null) { - expireJob.cancel(true); - expireJob = null; - } + deactivate(); } } - public void receiveCommand(ItemCommandEvent commandEvent) { - Command command = commandEvent.getItemCommand(); - String itemName = commandEvent.getItemName(); - logger.trace("Received command '{}' for item {}", command, itemName); - ExpireConfig expireConfig = getExpireConfig(itemName); - if (expireConfig != null) { - Command expireCommand = expireConfig.expireCommand; - State expireState = expireConfig.expireState; - - if ((expireCommand != null && expireCommand.equals(command)) - || (expireState != null && expireState.equals(command))) { - // New command is expired command or state -> no further action needed - itemExpireMap.remove(itemName); // remove expire trigger until next update or command - logger.debug("Item {} received command '{}'; stopping any future expiration.", itemName, command); - } else { - // New command 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); - } + @Deactivate + protected void deactivate() { + ScheduledFuture localExpireJob = expireJob; + if (localExpireJob != null) { + localExpireJob.cancel(true); + expireJob = null; } + itemRegistry.removeRegistryChangeListener(this); + metadataRegistry.removeRegistryChangeListener(metadataChangeListener); + itemExpireMap.clear(); } - protected void receiveUpdate(ItemStateEvent stateEvent) { - State state = stateEvent.getItemState(); - String itemName = stateEvent.getItemName(); - logger.trace("Received update '{}' for item {}", state, itemName); + private void processEvent(String itemName, Type type) { + logger.trace("Received '{}' for item {}", type, itemName); ExpireConfig expireConfig = getExpireConfig(itemName); if (expireConfig != null) { Command expireCommand = expireConfig.expireCommand; State expireState = expireConfig.expireState; - if ((expireCommand != null && expireCommand.equals(state)) - || (expireState != null && expireState.equals(state))) { - // New state is expired command or state -> no further action needed + 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 update '{}'; stopping any future expiration.", itemName, state); + logger.debug("Item {} received '{}'; stopping any future expiration.", itemName, type); } else { - // New state is not the expired command or state, so add the trigger to the map + // 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, @@ -254,9 +237,11 @@ public void receive(Event event) { return; } if (event instanceof ItemStateEvent) { - receiveUpdate((ItemStateEvent) event); + ItemStateEvent isEvent = (ItemStateEvent) event; + processEvent(isEvent.getItemName(), isEvent.getItemState()); } else if (event instanceof ItemCommandEvent) { - receiveCommand((ItemCommandEvent) event); + ItemCommandEvent icEvent = (ItemCommandEvent) event; + processEvent(icEvent.getItemName(), icEvent.getItemCommand()); } } @@ -293,13 +278,18 @@ public void updated(Metadata oldElement, Metadata element) { } } - class ExpireConfig { + static class ExpireConfig { + + private static final StringType STRING_TYPE_NULL_HYPHEN = new StringType("'NULL'"); + private static final StringType STRING_TYPE_NULL = new StringType("NULL"); + private static final StringType STRING_TYPE_UNDEF_HYPHEN = new StringType("'UNDEF'"); + private static final StringType STRING_TYPE_UNDEF = new StringType("UNDEF"); protected static final String COMMAND_PREFIX = "command="; protected static final String STATE_PREFIX = "state="; - protected final Pattern durationPattern = Pattern.compile("(?:([0-9]+)H)?\\s*(?:([0-9]+)M)?\\s*(?:([0-9]+)S)?", - Pattern.CASE_INSENSITIVE); + protected static final Pattern durationPattern = Pattern + .compile("(?:([0-9]+)H)?\\s*(?:([0-9]+)M)?\\s*(?:([0-9]+)S)?", Pattern.CASE_INSENSITIVE); @Nullable final Command expireCommand; @@ -346,10 +336,10 @@ public ExpireConfig(Item item, String configString) throws IllegalArgumentExcept String stateString = stateOrCommand; State state = TypeParser.parseState(item.getAcceptedDataTypes(), stateString); // do special handling to allow NULL and UNDEF as strings when being put in single quotes - if (new StringType("'NULL'").equals(state)) { - expireState = new StringType("NULL"); - } else if (new StringType("'UNDEF'").equals(state)) { - expireState = new StringType("UNDEF"); + if (STRING_TYPE_NULL_HYPHEN.equals(state)) { + expireState = STRING_TYPE_NULL; + } else if (STRING_TYPE_UNDEF_HYPHEN.equals(state)) { + expireState = STRING_TYPE_UNDEF; } else { expireState = state; } 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 822db948e9f..816063d3a4b 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 @@ -155,63 +155,63 @@ void testMetadataChange() throws InterruptedException, ItemNotFoundException { @Test void testExpireConfig() { Item testItem = new SwitchItem(ITEMNAME); - ExpireConfig cfg = expireManager.new ExpireConfig(testItem, "1s"); + ExpireConfig cfg = new ExpireManager.ExpireConfig(testItem, "1s"); assertEquals(Duration.ofSeconds(1), cfg.duration); assertEquals(UnDefType.UNDEF, cfg.expireState); assertEquals(null, cfg.expireCommand); - cfg = expireManager.new ExpireConfig(testItem, "5h 3m 2s"); + cfg = new ExpireManager.ExpireConfig(testItem, "5h 3m 2s"); assertEquals(Duration.ofHours(5).plusMinutes(3).plusSeconds(2), cfg.duration); assertEquals(UnDefType.UNDEF, cfg.expireState); assertEquals(null, cfg.expireCommand); - cfg = expireManager.new ExpireConfig(testItem, "1h,OFF"); + cfg = new ExpireManager.ExpireConfig(testItem, "1h,OFF"); assertEquals(Duration.ofHours(1), cfg.duration); assertEquals(OnOffType.OFF, cfg.expireState); assertEquals(null, cfg.expireCommand); - cfg = expireManager.new ExpireConfig(testItem, "1h,state=OFF"); + cfg = new ExpireManager.ExpireConfig(testItem, "1h,state=OFF"); assertEquals(Duration.ofHours(1), cfg.duration); assertEquals(OnOffType.OFF, cfg.expireState); assertEquals(null, cfg.expireCommand); - cfg = expireManager.new ExpireConfig(testItem, "1h,command=OFF"); + cfg = new ExpireManager.ExpireConfig(testItem, "1h,command=OFF"); assertEquals(Duration.ofHours(1), cfg.duration); assertEquals(null, cfg.expireState); assertEquals(OnOffType.OFF, cfg.expireCommand); try { - cfg = expireManager.new ExpireConfig(testItem, "1h,command=OPEN"); + cfg = new ExpireManager.ExpireConfig(testItem, "1h,command=OPEN"); fail(); } catch (IllegalArgumentException e) { // expected as command is invalid } try { - cfg = expireManager.new ExpireConfig(testItem, "1h,OPEN"); + cfg = new ExpireManager.ExpireConfig(testItem, "1h,OPEN"); fail(); } catch (IllegalArgumentException e) { // expected as state is invalid } testItem = new NumberItem("Number:Temperature", ITEMNAME); - cfg = expireManager.new ExpireConfig(testItem, "1h,15 °C"); + cfg = new ExpireManager.ExpireConfig(testItem, "1h,15 °C"); assertEquals(Duration.ofHours(1), cfg.duration); assertEquals(new QuantityType("15 °C"), cfg.expireState); assertEquals(null, cfg.expireCommand); testItem = new StringItem(ITEMNAME); - cfg = expireManager.new ExpireConfig(testItem, "1h,NULL"); + cfg = new ExpireManager.ExpireConfig(testItem, "1h,NULL"); assertEquals(Duration.ofHours(1), cfg.duration); assertEquals(UnDefType.NULL, cfg.expireState); assertEquals(null, cfg.expireCommand); - cfg = expireManager.new ExpireConfig(testItem, "1h,'NULL'"); + cfg = new ExpireManager.ExpireConfig(testItem, "1h,'NULL'"); assertEquals(Duration.ofHours(1), cfg.duration); assertEquals(new StringType("NULL"), cfg.expireState); assertEquals(null, cfg.expireCommand); - cfg = expireManager.new ExpireConfig(testItem, "1h,'UNDEF'"); + cfg = new ExpireManager.ExpireConfig(testItem, "1h,'UNDEF'"); assertEquals(Duration.ofHours(1), cfg.duration); assertEquals(new StringType("UNDEF"), cfg.expireState); assertEquals(null, cfg.expireCommand);