Fix enabling/disabling of addons suggestors at OH startup (#5020)

Before this fix, the AddonSuggestionService service was started and an incomplete configuration was considered, a configuration containing none of the user settings defining if each suggestion finder should be enabled or disabled.
Later, when AddonFinderService service was added, the same incomplete configuration was reused.

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
This commit is contained in:
lolodomo
2025-09-20 20:54:34 +02:00
committed by GitHub
parent 75ceb0fcd0
commit 9c6d304b7d
2 changed files with 39 additions and 25 deletions
@@ -76,12 +76,11 @@ public class AddonSuggestionService implements AutoCloseable {
@Activate
public AddonSuggestionService(final @Reference ConfigurationAdmin configurationAdmin,
@Reference LocaleProvider localeProvider, @Nullable Map<String, Object> config) {
@Reference LocaleProvider localeProvider) {
this.configurationAdmin = configurationAdmin;
this.localeProvider = localeProvider;
SUGGESTION_FINDERS.forEach(f -> baseFinderConfig.put(f, false));
modified(config);
// 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
@@ -99,7 +98,9 @@ public class AddonSuggestionService implements AutoCloseable {
@Reference(cardinality = ReferenceCardinality.OPTIONAL, policy = ReferencePolicy.DYNAMIC, policyOption = ReferencePolicyOption.GREEDY)
protected void addAddonFinderService(AddonFinderService addonFinderService) {
this.addonFinderService = addonFinderService;
modified(config);
// 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());
}
protected void removeAddonFinderService(AddonFinderService addonFinderService) {
@@ -111,18 +112,20 @@ public class AddonSuggestionService implements AutoCloseable {
@Modified
public void modified(@Nullable final Map<String, Object> config) {
baseFinderConfig.forEach((finder, cfg) -> {
baseFinderConfig.forEach((finder, currentEnabled) -> {
String cfgParam = SUGGESTION_FINDER_CONFIGS.get(finder);
if (cfgParam != null) {
boolean enabled = (config != null)
boolean newEnabled = (config != null)
? ConfigParser.valueAsOrElse(config.get(cfgParam), Boolean.class, true)
: cfg;
if (cfg != enabled) {
: currentEnabled;
if (currentEnabled != newEnabled) {
String type = SUGGESTION_FINDER_TYPES.get(finder);
AddonFinderService finderService = addonFinderService;
if (type != null && finderService != null) {
baseFinderConfig.put(finder, enabled);
if (enabled) {
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);
} else {
finderService.uninstall(type);
@@ -135,19 +138,23 @@ public class AddonSuggestionService implements AutoCloseable {
}
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) {
return;
}
List<String> keys = Collections.list(cfg.keys());
final Map<String, Object> cfgMap = keys.stream().collect(Collectors.toMap(Function.identity(), cfg::get));
if (!cfgMap.equals(config)) {
modified(cfgMap);
if (cfg != null) {
List<String> keys = Collections.list(cfg.keys());
return keys.stream().collect(Collectors.toMap(Function.identity(), cfg::get));
}
} catch (IOException | IllegalStateException e) {
logger.debug("Exception occurred while trying to sync the configuration: {}", e.getMessage());
logger.debug("Exception occurred while trying to get the configuration: {}", e.getMessage());
}
return null;
}
private boolean isFinderEnabled(AddonFinder finder) {
@@ -14,12 +14,13 @@ package org.openhab.core.config.discovery.addon.tests;
import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
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.HashSet;
import java.util.Hashtable;
import java.util.List;
import java.util.Locale;
import java.util.Map;
@@ -63,8 +64,8 @@ public class AddonSuggestionServiceTests {
private @NonNullByDefault({}) AddonFinder upnpAddonFinder;
private @NonNullByDefault({}) AddonSuggestionService addonSuggestionService;
private final Map<String, Object> config = Map.of(AddonFinderConstants.CFG_FINDER_MDNS, true,
AddonFinderConstants.CFG_FINDER_UPNP, true);
private final Hashtable<String, Object> config = new Hashtable<>(
Map.of(AddonFinderConstants.CFG_FINDER_MDNS, true, AddonFinderConstants.CFG_FINDER_UPNP, true));
@AfterAll
public void cleanUp() {
@@ -87,8 +88,7 @@ public class AddonSuggestionServiceTests {
}
private AddonSuggestionService createAddonSuggestionService() {
AddonSuggestionService addonSuggestionService = new AddonSuggestionService(configurationAdmin, localeProvider,
config);
AddonSuggestionService addonSuggestionService = new AddonSuggestionService(configurationAdmin, localeProvider);
assertNotNull(addonSuggestionService);
addonSuggestionService.addAddonFinder(mdnsAddonFinder);
@@ -105,12 +105,19 @@ public class AddonSuggestionServiceTests {
when(configurationAdmin.getConfiguration(any())).thenReturn(configuration);
} catch (IOException e) {
}
when(configuration.getProperties()).thenReturn(null);
when(configuration.getProperties()).thenReturn(config);
// check that it works
assertNotNull(configurationAdmin);
try {
assertNull(configurationAdmin.getConfiguration(AddonSuggestionService.CONFIG_PID).getProperties());
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) {
}
}