Refactor ThingHandlerService to an OSGi component prototype

bom/runtime/pom.xml

@@ -61,6 +61,12 @@
       <version>1.5.0</version>
       <scope>compile</scope>
     </dependency>
+    <dependency>
+      <groupId>org.osgi</groupId>
+      <artifactId>org.osgi.service.component.annotations</artifactId>
+      <version>1.5.0</version>
+      <scope>compile</scope>
+    </dependency>
     <dependency>
       <groupId>org.apache.felix</groupId>
       <artifactId>org.apache.felix.scr</artifactId>

bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/AbstractDiscoveryService.java

@@ -27,6 +27,7 @@
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
 import org.openhab.core.common.ThreadPoolManager;
+import org.openhab.core.config.core.ConfigParser;
 import org.openhab.core.i18n.I18nUtil;
 import org.openhab.core.i18n.LocaleProvider;
 import org.openhab.core.i18n.TranslationProvider;
@@ -347,10 +348,9 @@ protected void removeOlderResults(long timestamp, @Nullable Collection<ThingType
      */
     protected void activate(@Nullable Map<String, Object> configProperties) {
         if (configProperties != null) {
-            Object property = configProperties.get(DiscoveryService.CONFIG_PROPERTY_BACKGROUND_DISCOVERY);
-            if (property != null) {
-                backgroundDiscoveryEnabled = getAutoDiscoveryEnabled(property);
-            }
+            backgroundDiscoveryEnabled = ConfigParser.valueAsOrElse(
+                    configProperties.get(DiscoveryService.CONFIG_PROPERTY_BACKGROUND_DISCOVERY), Boolean.class,
+                    backgroundDiscoveryEnabled);
         }
         if (backgroundDiscoveryEnabled) {
             startBackgroundDiscovery();
@@ -370,20 +370,18 @@ protected void activate(@Nullable Map<String, Object> configProperties) {
      */
     protected void modified(@Nullable Map<String, Object> configProperties) {
         if (configProperties != null) {
-            Object property = configProperties.get(DiscoveryService.CONFIG_PROPERTY_BACKGROUND_DISCOVERY);
-            if (property != null) {
-                boolean enabled = getAutoDiscoveryEnabled(property);
-
-                if (backgroundDiscoveryEnabled && !enabled) {
-                    stopBackgroundDiscovery();
-                    logger.debug("Background discovery for discovery service '{}' disabled.",
-                            this.getClass().getName());
-                } else if (!backgroundDiscoveryEnabled && enabled) {
-                    startBackgroundDiscovery();
-                    logger.debug("Background discovery for discovery service '{}' enabled.", this.getClass().getName());
-                }
-                backgroundDiscoveryEnabled = enabled;
+            boolean enabled = ConfigParser.valueAsOrElse(
+                    configProperties.get(DiscoveryService.CONFIG_PROPERTY_BACKGROUND_DISCOVERY), Boolean.class,
+                    backgroundDiscoveryEnabled);
+
+            if (backgroundDiscoveryEnabled && !enabled) {
+                stopBackgroundDiscovery();
+                logger.debug("Background discovery for discovery service '{}' disabled.", this.getClass().getName());
+            } else if (!backgroundDiscoveryEnabled && enabled) {
+                startBackgroundDiscovery();
+                logger.debug("Background discovery for discovery service '{}' enabled.", this.getClass().getName());
             }
+            backgroundDiscoveryEnabled = enabled;
         }
     }
 
@@ -426,14 +424,6 @@ protected long getTimestampOfLastScan() {
         return timestampOfLastScan;
     }
 
-    private boolean getAutoDiscoveryEnabled(Object autoDiscoveryEnabled) {
-        if (autoDiscoveryEnabled instanceof String string) {
-            return Boolean.parseBoolean(string);
-        } else {
-            return Boolean.TRUE.equals(autoDiscoveryEnabled);
-        }
-    }
-
     private String inferKey(DiscoveryResult discoveryResult, String lastSegment) {
         return "discovery." + discoveryResult.getThingUID().getAsString().replace(":", ".") + "." + lastSegment;
     }

bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/AbstractThingHandlerDiscoveryService.java

@@ -0,0 +1,129 @@
+/**
+ * Copyright (c) 2010-2023 Contributors to the openHAB project
+ *
+ * See the NOTICE file(s) distributed with this work for additional
+ * information.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ */
+package org.openhab.core.config.discovery;
+
+import java.util.Map;
+import java.util.Set;
+
+import org.eclipse.jdt.annotation.NonNull;
+import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.eclipse.jdt.annotation.Nullable;
+import org.openhab.core.config.core.ConfigParser;
+import org.openhab.core.thing.ThingTypeUID;
+import org.openhab.core.thing.binding.ThingHandler;
+import org.openhab.core.thing.binding.ThingHandlerService;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * The {@link AbstractThingHandlerDiscoveryService} extends the {@link AbstractDiscoveryService} for thing-based
+ * discovery services.
+ *
+ * It handles the injection of the {@link ThingHandler}
+ *
+ * @author Jan N. Klug - Initial contribution
+ */
+@NonNullByDefault
+public abstract class AbstractThingHandlerDiscoveryService<T extends ThingHandler> extends AbstractDiscoveryService
+        implements ThingHandlerService {
+    private final Logger logger = LoggerFactory.getLogger(AbstractThingHandlerDiscoveryService.class);
+    private final Class<T> thingClazz;
+    private boolean backgroundDiscoveryEnabled = false;
+
+    // this works around a bug in ecj: @NonNullByDefault({}) complains about the field not being
+    // initialized when the type is generic, so we have to initialize it with "something"
+    protected @NonNullByDefault({}) T thingHandler = (@NonNull T) null;
+
+    protected AbstractThingHandlerDiscoveryService(Class<T> thingClazz, @Nullable Set<ThingTypeUID> supportedThingTypes,
+            int timeout, boolean backgroundDiscoveryEnabledByDefault) throws IllegalArgumentException {
+        super(supportedThingTypes, timeout, backgroundDiscoveryEnabledByDefault);
+        this.thingClazz = thingClazz;
+    }
+
+    protected AbstractThingHandlerDiscoveryService(Class<T> thingClazz, @Nullable Set<ThingTypeUID> supportedThingTypes,
+            int timeout) throws IllegalArgumentException {
+        super(supportedThingTypes, timeout);
+        this.thingClazz = thingClazz;
+    }
+
+    protected AbstractThingHandlerDiscoveryService(Class<T> thingClazz, int timeout) throws IllegalArgumentException {
+        super(timeout);
+        this.thingClazz = thingClazz;
+    }
+
+    @Override
+    protected abstract void startScan();
+
+    @Override
+    @SuppressWarnings("unchecked")
+    public void setThingHandler(ThingHandler handler) {
+        if (thingClazz.isAssignableFrom(handler.getClass())) {
+            this.thingHandler = (T) handler;
+        } else {
+            throw new IllegalArgumentException(
+                    "Expected class is " + thingClazz + " but the parameter has class " + handler.getClass());
+        }
+    }
+
+    @Override
+    public @Nullable ThingHandler getThingHandler() {
+        return thingHandler;
+    }
+
+    @Override
+    public void activate(@Nullable Map<String, Object> config) {
+        // do not call super.activate here, otherwise the scan might be background scan might be started before the
+        // thing handler is set. This is correctly handled in initialize
+        if (config != null) {
+            backgroundDiscoveryEnabled = ConfigParser.valueAsOrElse(
+                    config.get(DiscoveryService.CONFIG_PROPERTY_BACKGROUND_DISCOVERY), Boolean.class, false);
+        }
+    }
+
+    @Override
+    public void modified(@Nullable Map<String, Object> config) {
+        if (config != null) {
+            boolean enabled = ConfigParser.valueAsOrElse(
+                    config.get(DiscoveryService.CONFIG_PROPERTY_BACKGROUND_DISCOVERY), Boolean.class, false);
+
+            if (backgroundDiscoveryEnabled && !enabled) {
+                stopBackgroundDiscovery();
+                logger.debug("Background discovery for discovery service '{}' disabled.", getClass().getName());
+            } else if (!backgroundDiscoveryEnabled && enabled) {
+                startBackgroundDiscovery();
+                logger.debug("Background discovery for discovery service '{}' enabled.", getClass().getName());
+            }
+            backgroundDiscoveryEnabled = enabled;
+        }
+    }
+
+    @Override
+    public void deactivate() {
+        // do not call super.deactivate here, background scan is already handled in dispose
+    }
+
+    @Override
+    public void initialize() {
+        if (backgroundDiscoveryEnabled) {
+            startBackgroundDiscovery();
+            logger.debug("Background discovery for discovery service '{}' enabled.", getClass().getName());
+        }
+    }
+
+    @Override
+    public void dispose() {
+        if (backgroundDiscoveryEnabled) {
+            stopBackgroundDiscovery();
+        }
+    }
+}