Cleanup

Signed-off-by: Ravi Nadahar <[email protected]>

Author: Ravi Nadahar

bundles/org.openhab.core.config.discovery.addon/src/main/java/org/openhab/core/config/discovery/addon/AddonSuggestionService.java

@@ -14,32 +14,23 @@
 
 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;
@@ -66,35 +57,22 @@ public class AddonSuggestionService {
     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 volatile @Nullable AddonFinderService addonFinderService;
-    private @Nullable Map<String, Object> config;
-    private final ScheduledExecutorService scheduler;
     private final Map<String, Boolean> baseFinderConfig = new ConcurrentHashMap<>();
-    private final ScheduledFuture<?> cfgRefreshTask;
 
     @Activate
-    public AddonSuggestionService(final @Reference ConfigurationAdmin configurationAdmin,
-            @Reference LocaleProvider localeProvider, Map<String, Object> config) {
-        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}
         modified(config);
-        scheduler = ThreadPoolManager.getScheduledPool(ThreadPoolManager.THREAD_POOL_NAME_COMMON);
-        cfgRefreshTask = scheduler.scheduleWithFixedDelay(this::syncConfiguration, 1, 1, TimeUnit.MINUTES);
     }
 
     @Deactivate
     public void deactivate() {
-        cfgRefreshTask.cancel(true);
         synchronized (addonFinders) {
             addonFinders.clear();
         }
@@ -142,14 +120,6 @@ public void modified(@Nullable final Map<String, Object> config) {
                     }
                 }
             });
-            this.config = config;
-        }
-    }
-
-    private void syncConfiguration() {
-        final Map<String, Object> cfg = getConfiguration();
-        if (cfg != null && !cfg.equals(config)) {
-            modified(cfg);
         }
     }
 
@@ -176,20 +146,6 @@ private void initAddonFinderService() {
         }
     }
 
-    private @Nullable Map<String, Object> getConfiguration() {
-        try {
-            Dictionary<String, Object> cfg = configurationAdmin.getConfiguration(OpenHAB.ADDONS_SERVICE_PID)
-                    .getProperties();
-            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 get the configuration: {}", e.getMessage());
-        }
-        return null;
-    }
-
     private boolean isFinderEnabled(AddonFinder finder) {
         if (finder instanceof BaseAddonFinder baseFinder) {
             return baseFinderConfig.getOrDefault(baseFinder.getServiceName(), true);
@@ -224,7 +180,8 @@ public void removeAddonFinder(AddonFinder addonFinder) {
     }
 
     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));

bundles/org.openhab.core.config.discovery.addon/src/test/java/org/openhab/core/config/discovery/addon/tests/AddonSuggestionServiceTests.java

@@ -17,10 +17,8 @@
 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;
@@ -33,7 +31,6 @@
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.TestInstance;
 import org.junit.jupiter.api.TestInstance.Lifecycle;
-import org.openhab.core.OpenHAB;
 import org.openhab.core.addon.AddonDiscoveryMethod;
 import org.openhab.core.addon.AddonInfo;
 import org.openhab.core.addon.AddonInfoProvider;
@@ -43,8 +40,6 @@
 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}.
@@ -58,14 +53,13 @@ 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
@@ -80,7 +74,6 @@ public void cleanUp() {
 
     @BeforeAll
     public void setup() {
-        setupMockConfigurationAdmin();
         setupMockLocaleProvider();
         setupMockAddonInfoProvider();
         setupMockMdnsAddonFinder();
@@ -89,8 +82,7 @@ public void setup() {
     }
 
     private AddonSuggestionService createAddonSuggestionService() {
-        AddonSuggestionService addonSuggestionService = new AddonSuggestionService(configurationAdmin, localeProvider,
-                Map.of());
+        AddonSuggestionService addonSuggestionService = new AddonSuggestionService(localeProvider, config);
         assertNotNull(addonSuggestionService);
 
         addonSuggestionService.addAddonFinder(mdnsAddonFinder);
@@ -99,31 +91,6 @@ private AddonSuggestionService createAddonSuggestionService() {
         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(OpenHAB.ADDONS_SERVICE_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);