Added expire functionality as core framework feature #1803

kaikreuzer · 2020-11-08T22:33:04Z

This adds the functionality to let item states expire after a configuration duration as a core feature.
It follows the same syntax as it was defined by the 1.x expire binding, so that this feature is fully backward compatible.
The configurations are done as item metadata with the namespace expire.

Implements #1620

Signed-off-by: Kai Kreuzer kai@openhab.org

Signed-off-by: Kai Kreuzer <kai@openhab.org>

openhab-bot · 2020-11-09T08:53:48Z

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/openhab-3-0-milestone-2-discussion/107564/100

Signed-off-by: Kai Kreuzer <kai@openhab.org>

kaikreuzer · 2020-11-15T20:27:23Z

@openhab/core-maintainers I'd say this is now ready to be reviewed.

Signed-off-by: Kai Kreuzer <kai@openhab.org>

cweitkamp

Thanks you for this implementation. On the first glance it looks really promising but I have some questions.

We are listening to each command / state update on all items. Aren't you worried about performance issues?

According to the code one can enable / disable this feature. But when disabling it the scheduler job will be stopped and the listener will be removed. The ExpireManager is still listening on the event bus and tries to parse the metadata and so on all the time. Should we try to skip that too?

cweitkamp · 2020-11-16T18:19:07Z

bundles/org.openhab.core/src/main/java/org/openhab/core/internal/items/ExpireManager.java

+    }
+
+    private @Nullable ExpireConfig getExpireConfig(String itemName) {
+        // the value might be null, so we explicitly check if an entry for that key exists


I do not understand this comment. Where do we add a null value to the itemExpireConfig map? I guess your idea was to prevent repeated interaction with the metadata registry but currently it looks like we are trying to get and parse the metadata all the time unless an item has the new namespace.

Good catch, this was indeed the intention.
With 79c256b in place, it should now indeed work as planned.

That is good. A follow-up question. metadata can be added, edited or removed during runtime e.g. via console or REST API. Will it be taken I to account too?

You're always posing the right question... Indeed it wasn't taken into account, but it now is with d7d71fb.

Signed-off-by: Kai Kreuzer <kai@openhab.org>

kaikreuzer · 2020-11-16T18:48:26Z

We are listening to each command / state update on all items. Aren't you worried about performance issues?

Not really - being an event listener itself isn't costly and I tried to return as quickly as possible when there's nothing to do.

Should we try to skip that too?

Good point, I have added d8a6a14 for that.

Signed-off-by: Kai Kreuzer <kai@openhab.org>

cweitkamp

Great. Now I am feeling comfortable with the provided features. I finally left some comments on the code.

bundles/org.openhab.core/src/main/java/org/openhab/core/internal/items/ExpireManager.java

Signed-off-by: Kai Kreuzer <kai@openhab.org>

kaikreuzer · 2020-11-17T20:34:02Z

Thanks again, @cweitkamp! I have addressed all your comments, please have another look!

cweitkamp

LGTM. Thank you.

cweitkamp · 2020-11-19T13:14:37Z

Lets merge it now. We have some days for testing until next milestone.

cweitkamp · 2020-11-19T17:50:02Z

...rg.openhab.core.model.item/src/org/openhab/core/model/item/internal/GenericItemProvider.java

@@ -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()))) {


Hm, ... I immediately ran into an NPE. I did not yet figure why.

java.lang.NullPointerException: null at java.util.Objects.requireNonNull(Unknown Source) ~[?:?] at org.openhab.core.items.ItemUtil.getMainItemType(ItemUtil.java:91) ~[?:?] at org.openhab.core.model.item.internal.GenericItemProvider.dispatchBindingsPerItemType(GenericItemProvider.java:303) ~[?:?] at org.openhab.core.model.item.internal.GenericItemProvider.addItemFactory(GenericItemProvider.java:134) ~[?:?] at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:?] at jdk.internal.reflect.NativeMethodAccessorImpl.invoke(Unknown Source) ~[?:?] at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source) ~[?:?] at java.lang.reflect.Method.invoke(Unknown Source) ~[?:?] at org.apache.felix.scr.impl.inject.methods.BaseMethod.invokeMethod(BaseMethod.java:228) ~[bundleFile:?] at org.apache.felix.scr.impl.inject.methods.BaseMethod.access$500(BaseMethod.java:41) ~[bundleFile:?] at org.apache.felix.scr.impl.inject.methods.BaseMethod$Resolved.invoke(BaseMethod.java:664) ~[bundleFile:?] at org.apache.felix.scr.impl.inject.methods.BaseMethod.invoke(BaseMethod.java:510) [bundleFile:?] at org.apache.felix.scr.impl.inject.methods.BindMethod.invoke(BindMethod.java:42) [bundleFile:?] at org.apache.felix.scr.impl.manager.DependencyManager.doInvokeBindMethod(DependencyManager.java:1813) [bundleFile:?] at org.apache.felix.scr.impl.manager.DependencyManager.bindDependency(DependencyManager.java:1637) [bundleFile:?] at org.apache.felix.scr.impl.manager.DependencyManager.bind(DependencyManager.java:1624) [bundleFile:?] at org.apache.felix.scr.impl.manager.SingleComponentManager.createImplementationObject(SingleComponentManager.java:307) [bundleFile:?] at org.apache.felix.scr.impl.manager.SingleComponentManager.createComponent(SingleComponentManager.java:114) [bundleFile:?] at org.apache.felix.scr.impl.manager.SingleComponentManager.getService(SingleComponentManager.java:982) [bundleFile:?] at org.apache.felix.scr.impl.manager.SingleComponentManager.getServiceInternal(SingleComponentManager.java:955) [bundleFile:?] at org.apache.felix.scr.impl.manager.SingleComponentManager.getService(SingleComponentManager.java:900) [bundleFile:?] at org.eclipse.osgi.internal.serviceregistry.ServiceFactoryUse$1.run(ServiceFactoryUse.java:212) [org.eclipse.osgi-3.12.100.jar:?] at java.security.AccessController.doPrivileged(Native Method) [?:?] at org.eclipse.osgi.internal.serviceregistry.ServiceFactoryUse.factoryGetService(ServiceFactoryUse.java:210) [org.eclipse.osgi-3.12.100.jar:?] at org.eclipse.osgi.internal.serviceregistry.ServiceFactoryUse.getService(ServiceFactoryUse.java:111) [org.eclipse.osgi-3.12.100.jar:?] at org.eclipse.osgi.internal.serviceregistry.ServiceConsumer$2.getService(ServiceConsumer.java:45) [org.eclipse.osgi-3.12.100.jar:?] at org.eclipse.osgi.internal.serviceregistry.ServiceRegistrationImpl.getService(ServiceRegistrationImpl.java:508) [org.eclipse.osgi-3.12.100.jar:?] at org.eclipse.osgi.internal.serviceregistry.ServiceRegistry.getService(ServiceRegistry.java:461) [org.eclipse.osgi-3.12.100.jar:?] at org.eclipse.osgi.internal.framework.BundleContextImpl.getService(BundleContextImpl.java:624) [org.eclipse.osgi-3.12.100.jar:?] at org.openhab.core.common.registry.AbstractRegistry$ProviderTracker.addingService(AbstractRegistry.java:145) [bundleFile:?] at org.openhab.core.common.registry.AbstractRegistry$ProviderTracker.addingService(AbstractRegistry.java:1) [bundleFile:?] at org.osgi.util.tracker.ServiceTracker$Tracked.customizerAdding(ServiceTracker.java:941) [osgi.core-6.0.0.jar:?] at org.osgi.util.tracker.ServiceTracker$Tracked.customizerAdding(ServiceTracker.java:870) [osgi.core-6.0.0.jar:?] at org.osgi.util.tracker.AbstractTracked.trackAdding(AbstractTracked.java:256) [osgi.core-6.0.0.jar:?] at org.osgi.util.tracker.AbstractTracked.track(AbstractTracked.java:229) [osgi.core-6.0.0.jar:?] at org.osgi.util.tracker.ServiceTracker$Tracked.serviceChanged(ServiceTracker.java:901) [osgi.core-6.0.0.jar:?] at org.eclipse.osgi.internal.serviceregistry.FilteredServiceListener.serviceChanged(FilteredServiceListener.java:109) [org.eclipse.osgi-3.12.100.jar:?] at org.eclipse.osgi.internal.framework.BundleContextImpl.dispatchEvent(BundleContextImpl.java:920) [org.eclipse.osgi-3.12.100.jar:?] at org.eclipse.osgi.framework.eventmgr.EventManager.dispatchEvent(EventManager.java:230) [org.eclipse.osgi-3.12.100.jar:?] at org.eclipse.osgi.framework.eventmgr.ListenerQueue.dispatchEventSynchronous(ListenerQueue.java:148) [org.eclipse.osgi-3.12.100.jar:?] at org.eclipse.osgi.internal.serviceregistry.ServiceRegistry.publishServiceEventPrivileged(ServiceRegistry.java:862) [org.eclipse.osgi-3.12.100.jar:?] at org.eclipse.osgi.internal.serviceregistry.ServiceRegistry.publishServiceEvent(ServiceRegistry.java:801) [org.eclipse.osgi-3.12.100.jar:?] at org.eclipse.osgi.internal.serviceregistry.ServiceRegistrationImpl.register(ServiceRegistrationImpl.java:127) [org.eclipse.osgi-3.12.100.jar:?] at org.eclipse.osgi.internal.serviceregistry.ServiceRegistry.registerService(ServiceRegistry.java:225) [org.eclipse.osgi-3.12.100.jar:?] at org.eclipse.osgi.internal.framework.BundleContextImpl.registerService(BundleContextImpl.java:469) [org.eclipse.osgi-3.12.100.jar:?] at org.apache.felix.scr.impl.manager.AbstractComponentManager$3.register(AbstractComponentManager.java:906) [bundleFile:?] at org.apache.felix.scr.impl.manager.AbstractComponentManager$3.register(AbstractComponentManager.java:892) [bundleFile:?] at org.apache.felix.scr.impl.manager.RegistrationManager.changeRegistration(RegistrationManager.java:128) [bundleFile:?] at org.apache.felix.scr.impl.manager.AbstractComponentManager.registerService(AbstractComponentManager.java:959) [bundleFile:?] at org.apache.felix.scr.impl.manager.AbstractComponentManager.activateInternal(AbstractComponentManager.java:732) [bundleFile:?] at org.apache.felix.scr.impl.manager.DependencyManager$MultipleDynamicCustomizer.addedService(DependencyManager.java:338) [bundleFile:?] at org.apache.felix.scr.impl.manager.DependencyManager$MultipleDynamicCustomizer.addedService(DependencyManager.java:294) [bundleFile:?] at org.apache.felix.scr.impl.manager.ServiceTracker$Tracked.customizerAdded(ServiceTracker.java:1216) [bundleFile:?] at org.apache.felix.scr.impl.manager.ServiceTracker$Tracked.customizerAdded(ServiceTracker.java:1137) [bundleFile:?] at org.apache.felix.scr.impl.manager.ServiceTracker$AbstractTracked.trackAdding(ServiceTracker.java:944) [bundleFile:?] at org.apache.felix.scr.impl.manager.ServiceTracker$AbstractTracked.track(ServiceTracker.java:880) [bundleFile:?] at org.apache.felix.scr.impl.manager.ServiceTracker$Tracked.serviceChanged(ServiceTracker.java:1168) [bundleFile:?] at org.apache.felix.scr.impl.BundleComponentActivator$ListenerInfo.serviceChanged(BundleComponentActivator.java:125) [bundleFile:?] at org.eclipse.osgi.internal.serviceregistry.FilteredServiceListener.serviceChanged(FilteredServiceListener.java:109) [org.eclipse.osgi-3.12.100.jar:?] at org.eclipse.osgi.internal.framework.BundleContextImpl.dispatchEvent(BundleContextImpl.java:920) [org.eclipse.osgi-3.12.100.jar:?] at org.eclipse.osgi.framework.eventmgr.EventManager.dispatchEvent(EventManager.java:230) [org.eclipse.osgi-3.12.100.jar:?] at org.eclipse.osgi.framework.eventmgr.ListenerQueue.dispatchEventSynchronous(ListenerQueue.java:148) [org.eclipse.osgi-3.12.100.jar:?] at org.eclipse.osgi.internal.serviceregistry.ServiceRegistry.publishServiceEventPrivileged(ServiceRegistry.java:862) [org.eclipse.osgi-3.12.100.jar:?] at org.eclipse.osgi.internal.serviceregistry.ServiceRegistry.publishServiceEvent(ServiceRegistry.java:801) [org.eclipse.osgi-3.12.100.jar:?] at org.eclipse.osgi.internal.serviceregistry.ServiceRegistrationImpl.register(ServiceRegistrationImpl.java:127) [org.eclipse.osgi-3.12.100.jar:?] at org.eclipse.osgi.internal.serviceregistry.ServiceRegistry.registerService(ServiceRegistry.java:225) [org.eclipse.osgi-3.12.100.jar:?] at org.eclipse.osgi.internal.framework.BundleContextImpl.registerService(BundleContextImpl.java:469) [org.eclipse.osgi-3.12.100.jar:?] at org.apache.felix.scr.impl.manager.AbstractComponentManager$3.register(AbstractComponentManager.java:906) [bundleFile:?] at org.apache.felix.scr.impl.manager.AbstractComponentManager$3.register(AbstractComponentManager.java:892) [bundleFile:?] at org.apache.felix.scr.impl.manager.RegistrationManager.changeRegistration(RegistrationManager.java:128) [bundleFile:?] at org.apache.felix.scr.impl.manager.AbstractComponentManager.registerService(AbstractComponentManager.java:959) [bundleFile:?] at org.apache.felix.scr.impl.manager.AbstractComponentManager.activateInternal(AbstractComponentManager.java:732) [bundleFile:?] at org.apache.felix.scr.impl.manager.AbstractComponentManager.enableInternal(AbstractComponentManager.java:666) [bundleFile:?] at org.apache.felix.scr.impl.manager.AbstractComponentManager.enable(AbstractComponentManager.java:432) [bundleFile:?] at org.apache.felix.scr.impl.manager.ConfigurableComponentHolder.enableComponents(ConfigurableComponentHolder.java:665) [bundleFile:?] at org.apache.felix.scr.impl.BundleComponentActivator.initialEnable(BundleComponentActivator.java:338) [bundleFile:?] at org.apache.felix.scr.impl.Activator.loadComponents(Activator.java:382) [bundleFile:?] at org.apache.felix.scr.impl.Activator.access$200(Activator.java:49) [bundleFile:?] at org.apache.felix.scr.impl.Activator$ScrExtension.start(Activator.java:264) [bundleFile:?] at org.apache.felix.scr.impl.AbstractExtender.createExtension(AbstractExtender.java:196) [bundleFile:?] at org.apache.felix.scr.impl.AbstractExtender.modifiedBundle(AbstractExtender.java:169) [bundleFile:?] at org.apache.felix.scr.impl.AbstractExtender.modifiedBundle(AbstractExtender.java:49) [bundleFile:?] at org.osgi.util.tracker.BundleTracker$Tracked.customizerModified(BundleTracker.java:482) [osgi.core-6.0.0.jar:?] at org.osgi.util.tracker.BundleTracker$Tracked.customizerModified(BundleTracker.java:415) [osgi.core-6.0.0.jar:?] at org.osgi.util.tracker.AbstractTracked.track(AbstractTracked.java:232) [osgi.core-6.0.0.jar:?] at org.osgi.util.tracker.BundleTracker$Tracked.bundleChanged(BundleTracker.java:444) [osgi.core-6.0.0.jar:?] at org.eclipse.osgi.internal.framework.BundleContextImpl.dispatchEvent(BundleContextImpl.java:908) [org.eclipse.osgi-3.12.100.jar:?] at org.eclipse.osgi.framework.eventmgr.EventManager.dispatchEvent(EventManager.java:230) [org.eclipse.osgi-3.12.100.jar:?] at org.eclipse.osgi.framework.eventmgr.ListenerQueue.dispatchEventSynchronous(ListenerQueue.java:148) [org.eclipse.osgi-3.12.100.jar:?] at org.eclipse.osgi.internal.framework.EquinoxEventPublisher.publishBundleEventPrivileged(EquinoxEventPublisher.java:213) [org.eclipse.osgi-3.12.100.jar:?] at org.eclipse.osgi.internal.framework.EquinoxEventPublisher.publishBundleEvent(EquinoxEventPublisher.java:120) [org.eclipse.osgi-3.12.100.jar:?] at org.eclipse.osgi.internal.framework.EquinoxEventPublisher.publishBundleEvent(EquinoxEventPublisher.java:112) [org.eclipse.osgi-3.12.100.jar:?] at org.eclipse.osgi.internal.framework.EquinoxContainerAdaptor.publishModuleEvent(EquinoxContainerAdaptor.java:168) [org.eclipse.osgi-3.12.100.jar:?] at org.eclipse.osgi.container.Module.publishEvent(Module.java:476) [org.eclipse.osgi-3.12.100.jar:?] at org.eclipse.osgi.container.Module.start(Module.java:467) [org.eclipse.osgi-3.12.100.jar:?] at org.eclipse.osgi.internal.framework.EquinoxBundle.start(EquinoxBundle.java:383) [org.eclipse.osgi-3.12.100.jar:?] at org.eclipse.osgi.internal.framework.EquinoxBundle.start(EquinoxBundle.java:402) [org.eclipse.osgi-3.12.100.jar:?] at org.apache.karaf.features.internal.service.BundleInstallSupportImpl.startBundle(BundleInstallSupportImpl.java:165) [bundleFile:?] at org.apache.karaf.features.internal.service.FeaturesServiceImpl.startBundle(FeaturesServiceImpl.java:1153) [bundleFile:?] at org.apache.karaf.features.internal.service.Deployer.deploy(Deployer.java:1036) [bundleFile:?] at org.apache.karaf.features.internal.service.FeaturesServiceImpl.doProvision(FeaturesServiceImpl.java:1062) [bundleFile:?] at org.apache.karaf.features.internal.service.FeaturesServiceImpl.lambda$doProvisionInThread$13(FeaturesServiceImpl.java:998) [bundleFile:?] at java.util.concurrent.FutureTask.run(Unknown Source) [?:?] at java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source) [?:?] at java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source) [?:?] at java.lang.Thread.run(Unknown Source) [?:?]

That's weird indeed. I have already upgraded my system and didn't have any such issues.
Can you run in debug mode and remotely connect to see what this modelItem without a type is?

In any case, I have created #1839, which should solve the issue.

* Added expire functionality as core framework feature Closes openhab#1620 Signed-off-by: Kai Kreuzer <kai@openhab.org> GitOrigin-RevId: 63ec434

added expire functionality

798dabe

Signed-off-by: Kai Kreuzer <kai@openhab.org>

kaikreuzer added the enhancement An enhancement or new feature of the Core label Nov 8, 2020

kaikreuzer added 2 commits November 14, 2020 21:13

added unit tests

c50ab78

Signed-off-by: Kai Kreuzer <kai@openhab.org>

fixed spotless problem

0644e0d

Signed-off-by: Kai Kreuzer <kai@openhab.org>

kaikreuzer added rebuild Triggers the Jenkins PR build and removed rebuild Triggers the Jenkins PR build labels Nov 15, 2020

kaikreuzer requested a review from a team November 15, 2020 20:26

kaikreuzer marked this pull request as ready for review November 15, 2020 20:26

kaikreuzer changed the title ~~[WIP] Added expire functionality~~ Added expire functionality as core framework feature Nov 15, 2020

Allow NULL and UNDEF as strings

93a8510

Signed-off-by: Kai Kreuzer <kai@openhab.org>

kaikreuzer mentioned this pull request Nov 15, 2020

Introduce "expire" functionality for item states #1620

Closed

cweitkamp reviewed Nov 16, 2020

View reviewed changes

directly return when being disabled

d8a6a14

Signed-off-by: Kai Kreuzer <kai@openhab.org>

kaikreuzer added 4 commits November 16, 2020 19:55

do not retry config parsing

79c256b

Signed-off-by: Kai Kreuzer <kai@openhab.org>

fixed potential NPE when no metadata is available in cache

073a2e4

Signed-off-by: Kai Kreuzer <kai@openhab.org>

listen to changes in metadata registry

d7d71fb

Signed-off-by: Kai Kreuzer <kai@openhab.org>

spotless fix

0570b84

Signed-off-by: Kai Kreuzer <kai@openhab.org>

cweitkamp requested changes Nov 17, 2020

View reviewed changes

addressed review comments

eb88b0e

Signed-off-by: Kai Kreuzer <kai@openhab.org>

kaikreuzer requested a review from cweitkamp November 17, 2020 20:34

cweitkamp approved these changes Nov 19, 2020

View reviewed changes

cweitkamp added this to the 3.0.0.M3 milestone Nov 19, 2020

cweitkamp merged commit 63ec434 into openhab:master Nov 19, 2020

cweitkamp linked an issue Nov 19, 2020 that may be closed by this pull request

Introduce "expire" functionality for item states #1620

Closed

kaikreuzer deleted the exp branch November 19, 2020 16:15

cweitkamp reviewed Nov 19, 2020

View reviewed changes

Hilbrand mentioned this pull request Dec 1, 2020

openHAB 1 addon compatibility overview with openHAB 3 openhab/openhab-addons#6179

Closed

ssalonen mentioned this pull request May 8, 2022

Enhancement: Expire to optionally ignore commands #2952

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added expire functionality as core framework feature #1803

Added expire functionality as core framework feature #1803

kaikreuzer commented Nov 8, 2020

openhab-bot commented Nov 9, 2020

kaikreuzer commented Nov 15, 2020

cweitkamp left a comment

cweitkamp Nov 16, 2020

kaikreuzer Nov 16, 2020

cweitkamp Nov 16, 2020 •

edited

Loading

kaikreuzer Nov 16, 2020

kaikreuzer commented Nov 16, 2020

cweitkamp left a comment

kaikreuzer commented Nov 17, 2020

cweitkamp left a comment

cweitkamp commented Nov 19, 2020

cweitkamp Nov 19, 2020

kaikreuzer Nov 19, 2020

kaikreuzer Nov 19, 2020

Added expire functionality as core framework feature #1803

Added expire functionality as core framework feature #1803

Conversation

kaikreuzer commented Nov 8, 2020

openhab-bot commented Nov 9, 2020

kaikreuzer commented Nov 15, 2020

cweitkamp left a comment

Choose a reason for hiding this comment

cweitkamp Nov 16, 2020

Choose a reason for hiding this comment

kaikreuzer Nov 16, 2020

Choose a reason for hiding this comment

cweitkamp Nov 16, 2020 • edited Loading

Choose a reason for hiding this comment

kaikreuzer Nov 16, 2020

Choose a reason for hiding this comment

kaikreuzer commented Nov 16, 2020

cweitkamp left a comment

Choose a reason for hiding this comment

kaikreuzer commented Nov 17, 2020

cweitkamp left a comment

Choose a reason for hiding this comment

cweitkamp commented Nov 19, 2020

cweitkamp Nov 19, 2020

Choose a reason for hiding this comment

kaikreuzer Nov 19, 2020

Choose a reason for hiding this comment

kaikreuzer Nov 19, 2020

Choose a reason for hiding this comment

cweitkamp Nov 16, 2020 •

edited

Loading