Improve FeatureInstaller

[J-N-K]

There are some flaws in the FeatureInstaller which this PR tries to solve. While debugging #3025 I found that the periodic sync job sets the config cache before the configuration is applied. This might lead to issues if another call to modified re-sets the configuration to something else but does not update the configuration cache. It is also not ensured that all configuration updates are processed in the order they arrive, so the asynchronous processing of the updates might result in an older configuration overwriting a newer one.

This PR

• introduces a queue that holds the configuration supplied to modified and processes them in the correct order
• ensures that configuration cache is properly updated when an update is processed (independent from the configuration source)
• removed nearly all synchronized as they are no longer needed
• some other code clean-up

Signed-off-by: Jan N. Klug [email protected]

[J-N-K]

I think I found it (or at least one reason). This is the tree for org.openhab.core before any add-on is installed:

Bundle org.openhab.core [148] is currently ACTIVE

org.openhab.core [148]
+- tech.units.indriya [230]
|  +- javax.measure.unit-api [52]
|  +- org.apache.servicemix.bundles.javax-inject [86]
|  +- uom-lib-common [231]
|     +- javax.measure.unit-api [52]
+- org.apache.karaf.services.eventadmin [2]
|  +- org.apache.felix.configadmin [12]
|  |  +- org.apache.felix.coordinator [10]
|  +- org.apache.felix.metatype [4]
+- uom-lib-common [231]
+- si-units [227]
|  +- javax.measure.unit-api [52]
|  +- tech.units.indriya [230]
|  +- si.uom.si-quantity [228]
|  |  +- javax.measure.unit-api [52]
|  +- org.apache.servicemix.bundles.javax-inject [86]
+- javax.measure.unit-api [52]
+- com.google.gson [35]
+- org.apache.felix.scr [68]
|  +- org.apache.felix.configadmin [12]
|  +- org.apache.felix.metatype [4]
|  +- org.osgi.util.promise [9]
|  |  +- org.osgi.util.function [8]
|  +- org.apache.karaf.shell.core [23]
|     +- org.apache.karaf.services.eventadmin [2]
|     +- org.apache.felix.configadmin [12]
|     +- org.ops4j.pax.logging.pax-logging-api [5]
|     |  +- org.apache.karaf.services.eventadmin [2]
|     +- org.jline [25]
|        +- org.apache.sshd.sftp [93]
|        |  +- org.apache.sshd.osgi [91]
|        |  |  +- bcpkix [26]
|        |  |  |  +- bcprov [27]
|        |  |  |  +- bcutil [28]
|        |  |  |     +- bcprov [27]
|        |  |  +- org.ops4j.pax.logging.pax-logging-api [5]
|        |  |  +- bcprov [27]
|        |  +- org.ops4j.pax.logging.pax-logging-api [5]
|        +- org.apache.sshd.scp [92]
|        |  +- org.apache.sshd.osgi [91]
|        |  +- org.ops4j.pax.logging.pax-logging-api [5]
|        +- org.fusesource.jansi [6]
|        +- org.apache.sshd.osgi [91]
+- org.ops4j.pax.logging.pax-logging-api [5]

When installing binding-amazondashbutton the bundle reloads. This is because the binding also installs the feature openhab-runtime-jna and after the installation the tree looks like that:

Bundle org.openhab.core [148] is currently ACTIVE

org.openhab.core [148]
+- tech.units.indriya [230]
|  +- javax.measure.unit-api [52]
|  +- org.apache.servicemix.bundles.javax-inject [86]
|  +- uom-lib-common [231]
|     +- javax.measure.unit-api [52]
+- org.apache.karaf.services.eventadmin [2]
|  +- org.apache.felix.configadmin [12]
|  |  +- org.apache.felix.coordinator [10]
|  +- org.apache.felix.metatype [4]
+- uom-lib-common [231]
+- si-units [227]
|  +- javax.measure.unit-api [52]
|  +- tech.units.indriya [230]
|  +- si.uom.si-quantity [228]
|  |  +- javax.measure.unit-api [52]
|  +- org.apache.servicemix.bundles.javax-inject [86]
+- javax.measure.unit-api [52]
+- com.google.gson [35]
+- org.apache.felix.scr [68]
|  +- org.apache.felix.configadmin [12]
|  +- org.apache.felix.metatype [4]
|  +- org.osgi.util.promise [9]
|  |  +- org.osgi.util.function [8]
|  +- org.apache.karaf.shell.core [23]
|     +- org.apache.karaf.services.eventadmin [2]
|     +- org.apache.felix.configadmin [12]
|     +- org.ops4j.pax.logging.pax-logging-api [5]
|     |  +- org.apache.karaf.services.eventadmin [2]
|     +- org.jline [25]
|        +- org.apache.sshd.sftp [93]
|        |  +- org.apache.sshd.osgi [91]
|        |  |  +- bcpkix [26]
|        |  |  |  +- bcprov [27]
|        |  |  |  +- bcutil [28]
|        |  |  |     +- bcprov [27]
|        |  |  +- org.ops4j.pax.logging.pax-logging-api [5]
|        |  |  +- bcprov [27]
|        |  +- org.ops4j.pax.logging.pax-logging-api [5]
|        +- org.apache.sshd.scp [92]
|        |  +- org.apache.sshd.osgi [91]
|        |  +- org.ops4j.pax.logging.pax-logging-api [5]
|        +- org.apache.sshd.osgi [91]
|        +- com.sun.jna [256]
|        +- org.fusesource.jansi [6]
+- org.ops4j.pax.logging.pax-logging-api [5]

So an optional dependency is fulfilled and because of (2) the core bundle is refreshed. This causes a lot of other bundles to refresh (including all UI bundles). I believe there are similar issues with other bundles/features and this might also be the reason for a lot of strange behavior when installing add-ons, especially if several add-ons are installed at the same time (e.g. after an upgrade). This not only affects the FeatureInstaller but also other services (e.g. the KarafAddonService.

I'm not sure how to address that. We could add openhab-runtime-jna to openhab-runtime-base, this will solve the issue I found here. It does not prevent similar issues and while there may be a general benefit if we provide JNA by default, this might not be the case for other optional dependencies. Maybe also creating an override that blacklists the JNA bundle might work. Another option is to prevent bundle refreshes during add-on-installation and refresh after all configuration changes have been processed.

[openhab-bot]

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

https://community.openhab.org/t/featureinstaller-failed-installing-a-list-of-every-binding-ui-and-so-on-null/138242/2

[wborn]

We could add openhab-runtime-jna to openhab-runtime-base, this will solve the issue I found here.

Looks like this is a required dependency for https://github.com/openhab/openhab-core/pull/3004 anyhow.

[J-N-K]

I moved JNA to an openhab.tp-jna feature and included that in the openhab-core-base feature. This needs an adjustment in openhab-addons: https://github.com/openhab/openhab-addons/pull/13298