demus/openhab-core
1
0
Fork 0
mirror of https://github.com/danieldemus/openhab-core.git synced 2026-07-29 12:34:22 +02:00
Code Issues 1 Packages Projects Releases Wiki Activity

Fix AddonSuggestionService configuration (#5042)

Browse Source 
AddonSuggestionService has had a delayed application of configuration because it was based on a timer that refreshed it every minute. This led to various issue where "wrong decision" were made based on stale information. This addresses the root cause for why the configuration wasn't delivered in a timely manner by OSGi, and removes the scheduled configuration refresh.

For the configuration to be accessible to multiple bundles, its "location" must be "a region". This must be configured before any bundle tries to use the configuration. To do this very early, it has been attached to the Activator of the "main core bundle". It doesn't have to be there, as long as it will always run very early during startup.

Signed-off-by: Ravi Nadahar <nadahar@rediffmail.com>
This commit is contained in:
Nadahar
2025-10-08 20:30:06 +02:00
committed by GitHub
parent 9aa597249a
commit 9d679718ed
3 changed files with 76 additions and 113 deletions
Download Patch File Download Diff File Expand all files Collapse all files
bundles/org.openhab.core.config.discovery.addon/src/main/java/org/openhab/core/config/discovery/addon/AddonSuggestionService.java
+59 -78
View File
@@ -14,30 +14,23 @@ package org.openhab.core.config.discovery.addon;
import static org.openhab.core.config.discovery.addon.AddonFinderConstants.*;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Dictionary;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Map.Entry;
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.function.Function;
import java.util.stream.Collectors;
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.openhab.core.OpenHAB;
import org.openhab.core.addon.AddonInfo;
import org.openhab.core.addon.AddonInfoProvider;
import org.openhab.core.common.ThreadPoolManager;
import org.openhab.core.config.core.ConfigParser;
import org.openhab.core.i18n.LocaleProvider;
import org.osgi.service.cm.ConfigurationAdmin;
import org.osgi.service.component.annotations.Activate;
import org.osgi.service.component.annotations.Component;
import org.osgi.service.component.annotations.Deactivate;
@@ -56,51 +49,40 @@ import org.slf4j.LoggerFactory;
* @author Mark Herwege - Install/remove finders
*/
@NonNullByDefault
@Component(immediate = true, service = AddonSuggestionService.class, name = AddonSuggestionService.SERVICE_NAME, configurationPid = AddonSuggestionService.CONFIG_PID)
public class AddonSuggestionService implements AutoCloseable {
@Component(immediate = true, service = AddonSuggestionService.class, name = AddonSuggestionService.SERVICE_NAME, configurationPid = OpenHAB.ADDONS_SERVICE_PID)
public class AddonSuggestionService {
public static final String SERVICE_NAME = "addon-suggestion-service";
public static final String CONFIG_PID = "org.openhab.addons";
private final Logger logger = LoggerFactory.getLogger(AddonSuggestionService.class);
private final Set<AddonInfoProvider> addonInfoProviders = ConcurrentHashMap.newKeySet();
private final List<AddonFinder> addonFinders = Collections.synchronizedList(new ArrayList<>());
private final ConfigurationAdmin configurationAdmin;
// All access must be guarded by "addonFinders"
private final List<AddonFinder> addonFinders = new ArrayList<>();
private final LocaleProvider localeProvider;
private @Nullable AddonFinderService addonFinderService;
private @Nullable Map<String, Object> config;
private final ScheduledExecutorService scheduler;
private volatile @Nullable AddonFinderService addonFinderService;
private final Map<String, Boolean> baseFinderConfig = new ConcurrentHashMap<>();
private final ScheduledFuture<?> cfgRefreshTask;
@Activate
public AddonSuggestionService(final @Reference ConfigurationAdmin configurationAdmin,
@Reference LocaleProvider localeProvider) {
this.configurationAdmin = configurationAdmin;
public AddonSuggestionService(@Reference LocaleProvider localeProvider, Map<String, Object> config) {
this.localeProvider = localeProvider;
SUGGESTION_FINDERS.forEach(f -> baseFinderConfig.put(f, false));
// Changes to the configuration are expected to call the {@link modified} method. This works well when running
// in Eclipse. Running in Karaf, the method was not consistently called. Therefore regularly check for changes
// in configuration.
// This pattern and code was re-used from {@link org.openhab.core.karaf.internal.FeatureInstaller}
scheduler = ThreadPoolManager.getScheduledPool(ThreadPoolManager.THREAD_POOL_NAME_COMMON);
cfgRefreshTask = scheduler.scheduleWithFixedDelay(this::syncConfiguration, 1, 1, TimeUnit.MINUTES);
modified(config);
}
@Deactivate
protected void deactivate() {
cfgRefreshTask.cancel(true);
public void deactivate() {
synchronized (addonFinders) {
addonFinders.clear();
}
addonInfoProviders.clear();
}
@Reference(cardinality = ReferenceCardinality.OPTIONAL, policy = ReferencePolicy.DYNAMIC, policyOption = ReferencePolicyOption.GREEDY)
protected void addAddonFinderService(AddonFinderService addonFinderService) {
this.addonFinderService = addonFinderService;
// We retrieve the configuration at this time so that it contains all the user settings defining what finder to
// enable. When the service is started, the valid configuration is not yet loaded.
modified(getConfiguration());
initAddonFinderService();
}
protected void removeAddonFinderService(AddonFinderService addonFinderService) {
@@ -112,49 +94,56 @@ public class AddonSuggestionService implements AutoCloseable {
@Modified
public void modified(@Nullable final Map<String, Object> config) {
baseFinderConfig.forEach((finder, currentEnabled) -> {
String cfgParam = SUGGESTION_FINDER_CONFIGS.get(finder);
if (cfgParam != null) {
boolean newEnabled = (config != null)
? ConfigParser.valueAsOrElse(config.get(cfgParam), Boolean.class, true)
: currentEnabled;
if (currentEnabled != newEnabled) {
String type = SUGGESTION_FINDER_TYPES.get(finder);
AddonFinderService finderService = addonFinderService;
if (type != null && finderService != null) {
logger.debug("baseFinderConfig {} {} = {} => updating from {} to {}", finder, cfgParam,
config == null ? "null config" : config.get(cfgParam), currentEnabled, newEnabled);
baseFinderConfig.put(finder, newEnabled);
if (newEnabled) {
finderService.install(type);
if (config != null) {
AddonFinderService finderService = addonFinderService;
baseFinderConfig.forEach((finder, currentEnabled) -> {
String cfgParam = SUGGESTION_FINDER_CONFIGS.get(finder);
if (cfgParam != null) {
boolean newEnabled = ConfigParser.valueAsOrElse(config.get(cfgParam), Boolean.class, true);
if (currentEnabled != newEnabled) {
String type = SUGGESTION_FINDER_TYPES.get(finder);
if (type != null) {
logger.debug("baseFinderConfig {} {} = {} => updating from {} to {}", finder, cfgParam,
config.get(cfgParam), currentEnabled, newEnabled);
baseFinderConfig.put(finder, newEnabled);
if (finderService != null) {
if (newEnabled) {
finderService.install(type);
} else {
finderService.uninstall(type);
}
}
} else {
finderService.uninstall(type);
logger.warn("Failed to resolve addon suggestion finder type for suggestion finder {}",
finder);
}
}
}
}
});
this.config = config;
}
private void syncConfiguration() {
final Map<String, Object> cfg = getConfiguration();
if (cfg != null && !cfg.equals(config)) {
modified(cfg);
});
}
}
private @Nullable Map<String, Object> getConfiguration() {
try {
Dictionary<String, Object> cfg = configurationAdmin.getConfiguration(CONFIG_PID).getProperties();
if (cfg != null) {
List<String> keys = Collections.list(cfg.keys());
return keys.stream().collect(Collectors.toMap(Function.identity(), cfg::get));
private void initAddonFinderService() {
AddonFinderService finderService = this.addonFinderService;
if (finderService == null) {
return;
}
String type;
for (Entry<String, Boolean> entry : baseFinderConfig.entrySet()) {
type = SUGGESTION_FINDER_TYPES.get(entry.getKey());
if (type != null) {
if (entry.getValue() instanceof Boolean enabled) {
if (enabled) {
finderService.install(type);
} else {
finderService.uninstall(type);
}
}
} else {
logger.warn("Failed to resolve addon suggestion finder type for suggestion finder {}", entry.getKey());
}
} catch (IOException | IllegalStateException e) {
logger.debug("Exception occurred while trying to get the configuration: {}", e.getMessage());
}
return null;
}
private boolean isFinderEnabled(AddonFinder finder) {
@@ -191,22 +180,14 @@ public class AddonSuggestionService implements AutoCloseable {
}
private void changed() {
List<AddonInfo> candidates = addonInfoProviders.stream().map(p -> p.getAddonInfos(localeProvider.getLocale()))
Locale locale = localeProvider.getLocale();
List<AddonInfo> candidates = addonInfoProviders.stream().map(p -> p.getAddonInfos(locale))
.flatMap(Collection::stream).toList();
synchronized (addonFinders) {
addonFinders.stream().filter(this::isFinderEnabled).forEach(f -> f.setAddonCandidates(candidates));
}
}
@Deactivate
@Override
public void close() throws Exception {
synchronized (addonFinders) {
addonFinders.clear();
}
addonInfoProviders.clear();
}
public Set<AddonInfo> getSuggestedAddons(@Nullable Locale locale) {
synchronized (addonFinders) {
return addonFinders.stream().filter(this::isFinderEnabled).map(f -> f.getSuggestedAddons())
bundles/org.openhab.core.config.discovery.addon/src/test/java/org/openhab/core/config/discovery/addon/tests/AddonSuggestionServiceTests.java
+4 -35
View File
@@ -17,10 +17,8 @@ import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.*;
import static org.openhab.core.config.discovery.addon.AddonFinderConstants.*;
import java.io.IOException;
import java.util.Dictionary;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Hashtable;
import java.util.List;
import java.util.Locale;
import java.util.Map;
@@ -42,8 +40,6 @@ import org.openhab.core.config.discovery.addon.AddonFinder;
import org.openhab.core.config.discovery.addon.AddonFinderConstants;
import org.openhab.core.config.discovery.addon.AddonSuggestionService;
import org.openhab.core.i18n.LocaleProvider;
import org.osgi.service.cm.Configuration;
import org.osgi.service.cm.ConfigurationAdmin;
/**
* JUnit tests for the {@link AddonSuggestionService}.
@@ -57,21 +53,20 @@ public class AddonSuggestionServiceTests {
public static final String MDNS_SERVICE_TYPE = "mdnsServiceType";
private @NonNullByDefault({}) ConfigurationAdmin configurationAdmin;
private @NonNullByDefault({}) LocaleProvider localeProvider;
private @NonNullByDefault({}) AddonInfoProvider addonInfoProvider;
private @NonNullByDefault({}) AddonFinder mdnsAddonFinder;
private @NonNullByDefault({}) AddonFinder upnpAddonFinder;
private @NonNullByDefault({}) AddonSuggestionService addonSuggestionService;
private final Hashtable<String, Object> config = new Hashtable<>(
private final HashMap<String, Object> config = new HashMap<>(
Map.of(AddonFinderConstants.CFG_FINDER_MDNS, true, AddonFinderConstants.CFG_FINDER_UPNP, true));
@AfterAll
public void cleanUp() {
assertNotNull(addonSuggestionService);
try {
addonSuggestionService.close();
addonSuggestionService.deactivate();
} catch (Exception e) {
fail(e);
}
@@ -79,7 +74,6 @@ public class AddonSuggestionServiceTests {
@BeforeAll
public void setup() {
setupMockConfigurationAdmin();
setupMockLocaleProvider();
setupMockAddonInfoProvider();
setupMockMdnsAddonFinder();
@@ -88,7 +82,7 @@ public class AddonSuggestionServiceTests {
}
private AddonSuggestionService createAddonSuggestionService() {
AddonSuggestionService addonSuggestionService = new AddonSuggestionService(configurationAdmin, localeProvider);
AddonSuggestionService addonSuggestionService = new AddonSuggestionService(localeProvider, config);
assertNotNull(addonSuggestionService);
addonSuggestionService.addAddonFinder(mdnsAddonFinder);
@@ -97,31 +91,6 @@ public class AddonSuggestionServiceTests {
return addonSuggestionService;
}
private void setupMockConfigurationAdmin() {
// create the mock
configurationAdmin = mock(ConfigurationAdmin.class);
Configuration configuration = mock(Configuration.class);
try {
when(configurationAdmin.getConfiguration(any())).thenReturn(configuration);
} catch (IOException e) {
}
when(configuration.getProperties()).thenReturn(config);
// check that it works
assertNotNull(configurationAdmin);
try {
Dictionary<String, Object> cfg = configurationAdmin.getConfiguration(AddonSuggestionService.CONFIG_PID)
.getProperties();
assertNotNull(cfg);
assertTrue(cfg.get(AddonFinderConstants.CFG_FINDER_MDNS) instanceof Boolean);
assertTrue((Boolean) cfg.get(AddonFinderConstants.CFG_FINDER_MDNS));
assertTrue(cfg.get(AddonFinderConstants.CFG_FINDER_UPNP) instanceof Boolean);
assertTrue((Boolean) cfg.get(AddonFinderConstants.CFG_FINDER_UPNP));
assertNull(cfg.get(AddonFinderConstants.CFG_FINDER_USB));
} catch (IOException e) {
}
}
private void setupMockLocaleProvider() {
// create the mock
localeProvider = mock(LocaleProvider.class);
bundles/org.openhab.core/src/main/java/org/openhab/core/Activator.java
+13
View File
@@ -18,6 +18,9 @@ import org.osgi.annotation.bundle.Header;
import org.osgi.framework.BundleActivator;
import org.osgi.framework.BundleContext;
import org.osgi.framework.Constants;
import org.osgi.framework.ServiceReference;
import org.osgi.service.cm.Configuration;
import org.osgi.service.cm.ConfigurationAdmin;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -35,6 +38,16 @@ public final class Activator implements BundleActivator {
@Override
public void start(@Nullable BundleContext bc) throws Exception {
logger.info("Starting openHAB {} ({})", OpenHAB.getVersion(), OpenHAB.buildString());
ServiceReference<ConfigurationAdmin> ref;
if (bc != null && (ref = bc.getServiceReference(ConfigurationAdmin.class)) != null) {
ConfigurationAdmin ca = bc.getService(ref);
Configuration conf = ca.getConfiguration(OpenHAB.ADDONS_SERVICE_PID);
conf.setBundleLocation("?openhab");
bc.ungetService(ref);
} else {
logger.warn("Could not acquire ConfigurationAdmin instance, configuration \"{}\" might not work correctly",
OpenHAB.ADDONS_SERVICE_PID);
}
}
@Override