YAML things provider: create things even if binding is not yet installed

Makes it consistent with managed thing provider.

Removes the OSGi reference between YamlThingProvider and YamlModelRepository and as consequence removes the circular reference.

Signed-off-by: Laurent Garnier <[email protected]>

Author: lolodomo

bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/YamlModelRepository.java

@@ -21,7 +21,7 @@
  * The {@link YamlModelRepository} defines methods to update elements in a YAML model.
  *
  * @author Jan N. Klug - Initial contribution
- * @author Laurent Garnier - Added methods refreshModelElements and generateSyntaxFromElements
+ * @author Laurent Garnier - Added method generateSyntaxFromElements
  */
 @NonNullByDefault
 public interface YamlModelRepository {
@@ -31,14 +31,6 @@ public interface YamlModelRepository {
 
     void updateElementInModel(String modelName, YamlElement element);
 
-    /**
-     * Triggers the refresh of a certain type of elements in a given model.
-     *
-     * @param modelName the model name
-     * @param elementName the type of elements to refresh
-     */
-    void refreshModelElements(String modelName, String elementName);
-
     /**
      * Generate the YAML syntax from a provided list of elements.
      *

bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/YamlModelRepositoryImpl.java

@@ -77,7 +77,7 @@
  * @author Jan N. Klug - Refactored for multiple types per file and add modifying possibility
  * @author Laurent Garnier - Introduce version 2 using map instead of table
  * @author Laurent Garnier - Added basic version management
- * @author Laurent Garnier - Added methods refreshModelElements and generateSyntaxFromElements + new parameters
+ * @author Laurent Garnier - Added method generateSyntaxFromElements + new parameters
  *         for method isValid
  */
 @NonNullByDefault
@@ -565,35 +565,6 @@ public void updateElementInModel(String modelName, YamlElement element) {
         writeModel(modelName);
     }
 
-    @Override
-    @SuppressWarnings({ "rawtypes", "unchecked" })
-    public void refreshModelElements(String modelName, String elementName) {
-        logger.info("Refreshing {} from YAML model {}", elementName, modelName);
-        YamlModelWrapper model = modelCache.get(modelName);
-        if (model == null) {
-            logger.warn("Failed to refresh model {} because it is not known.", modelName);
-            return;
-        }
-
-        List<JsonNode> modelNodes = model.getNodesV1().get(elementName);
-        JsonNode modelMapNode = model.getNodes().get(elementName);
-        if (modelNodes == null && modelMapNode == null) {
-            logger.warn("Failed to refresh model {} because type {} is not known in the model.", modelName,
-                    elementName);
-            return;
-        }
-
-        getElementListeners(elementName, model.getVersion()).forEach(listener -> {
-            Class<? extends YamlElement> elementClass = listener.getElementClass();
-
-            List elements = parseJsonNodes(modelNodes != null ? modelNodes : List.of(), modelMapNode, elementClass,
-                    null, null);
-            if (!elements.isEmpty()) {
-                listener.updatedModel(modelName, elements);
-            }
-        });
-    }
-
     private void writeModel(String modelName) {
         YamlModelWrapper model = modelCache.get(modelName);
         if (model == null) {

bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/things/YamlThingProvider.java

@@ -32,9 +32,7 @@
 import org.openhab.core.config.core.ConfigUtil;
 import org.openhab.core.config.core.Configuration;
 import org.openhab.core.i18n.LocaleProvider;
-import org.openhab.core.model.yaml.YamlElementName;
 import org.openhab.core.model.yaml.YamlModelListener;
-import org.openhab.core.model.yaml.YamlModelRepository;
 import org.openhab.core.service.ReadyMarker;
 import org.openhab.core.service.ReadyService;
 import org.openhab.core.service.StartLevelService;
@@ -84,7 +82,6 @@ public class YamlThingProvider extends AbstractProvider<Thing>
 
     private final Logger logger = LoggerFactory.getLogger(YamlThingProvider.class);
 
-    private final YamlModelRepository modelRepository;
     private final BundleResolver bundleResolver;
     private final ThingTypeRegistry thingTypeRegistry;
     private final ChannelTypeRegistry channelTypeRegistry;
@@ -104,32 +101,8 @@ public void run() {
             logger.debug("Starting lazy retry thread");
             while (!queue.isEmpty()) {
                 for (QueueContent qc : queue) {
-                    logger.trace("Retry creating thing {}", qc.thingUID);
-                    Thing newThing = qc.thingHandlerFactory.createThing(qc.thingTypeUID, qc.configuration, qc.thingUID,
-                            qc.bridgeUID);
-                    if (newThing != null) {
-                        logger.debug("Successfully loaded thing \'{}\' during retry", qc.thingUID);
-                        Thing oldThing = null;
-                        for (Map.Entry<String, Collection<Thing>> entry : thingsMap.entrySet()) {
-                            oldThing = entry.getValue().stream().filter(t -> t.getUID().equals(newThing.getUID()))
-                                    .findFirst().orElse(null);
-                            if (oldThing != null) {
-                                mergeThing(newThing, oldThing);
-                                Collection<Thing> thingsForModel = Objects
-                                        .requireNonNull(thingsMap.get(entry.getKey()));
-                                thingsForModel.remove(oldThing);
-                                thingsForModel.add(newThing);
-                                logger.debug("Refreshing thing \'{}\' after successful retry", newThing.getUID());
-                                if (!ThingHelper.equals(oldThing, newThing)) {
-                                    notifyListenersAboutUpdatedElement(oldThing, newThing);
-                                }
-                                break;
-                            }
-                        }
-                        if (oldThing == null) {
-                            logger.debug("Refreshing thing \'{}\' after retry failed because thing is not found",
-                                    newThing.getUID());
-                        }
+                    if (retryCreateThing(qc.thingHandlerFactory, qc.thingTypeUID, qc.configuration, qc.thingUID,
+                            qc.bridgeUID)) {
                         queue.remove(qc);
                     }
                 }
@@ -153,12 +126,11 @@ private record QueueContent(ThingHandlerFactory thingHandlerFactory, ThingTypeUI
     }
 
     @Activate
-    public YamlThingProvider(final @Reference YamlModelRepository modelRepository,
-            final @Reference BundleResolver bundleResolver, final @Reference ThingTypeRegistry thingTypeRegistry,
+    public YamlThingProvider(final @Reference BundleResolver bundleResolver,
+            final @Reference ThingTypeRegistry thingTypeRegistry,
             final @Reference ChannelTypeRegistry channelTypeRegistry,
             final @Reference ConfigDescriptionRegistry configDescriptionRegistry,
             final @Reference LocaleProvider localeProvider) {
-        this.modelRepository = modelRepository;
         this.bundleResolver = bundleResolver;
         this.thingTypeRegistry = thingTypeRegistry;
         this.channelTypeRegistry = channelTypeRegistry;
@@ -242,6 +214,7 @@ public void removedModel(String modelName, Collection<YamlThingDTO> elements) {
 
     @Reference(cardinality = ReferenceCardinality.MULTIPLE, policy = ReferencePolicy.DYNAMIC)
     protected void addThingHandlerFactory(final ThingHandlerFactory thingHandlerFactory) {
+        logger.debug("addThingHandlerFactory {}", thingHandlerFactory.getClass().getSimpleName());
         thingHandlerFactories.add(thingHandlerFactory);
         thingHandlerFactoryAdded(thingHandlerFactory);
     }
@@ -278,16 +251,73 @@ public void onReadyMarkerRemoved(ReadyMarker readyMarker) {
         loadedXmlThingTypes.remove(readyMarker.getIdentifier());
     }
 
-    private void thingHandlerFactoryAdded(ThingHandlerFactory thingHandlerFactory) {
-        String bundleName = getBundleName(thingHandlerFactory);
-        if (bundleName != null && loadedXmlThingTypes.contains(bundleName)) {
-            logger.debug("Refreshing models due to new thing handler factory {}",
-                    thingHandlerFactory.getClass().getSimpleName());
-            thingsMap.keySet().forEach(modelName -> {
-                modelRepository.refreshModelElements(modelName,
-                        getElementClass().getAnnotation(YamlElementName.class).value());
-            });
+    private void thingHandlerFactoryAdded(ThingHandlerFactory handlerFactory) {
+        logger.debug("thingHandlerFactoryAdded {} isThingHandlerFactoryReady={}",
+                handlerFactory.getClass().getSimpleName(), isThingHandlerFactoryReady(handlerFactory));
+        if (isThingHandlerFactoryReady(handlerFactory)) {
+            if (!thingsMap.isEmpty()) {
+                logger.debug("Refreshing models due to new thing handler factory {}",
+                        handlerFactory.getClass().getSimpleName());
+                thingsMap.keySet().forEach(modelName -> {
+                    List<Thing> things = thingsMap.getOrDefault(modelName, List.of()).stream()
+                            .filter(th -> handlerFactory.supportsThingType(th.getThingTypeUID())).toList();
+                    if (!things.isEmpty()) {
+                        logger.info("Refreshing YAML model {} ({} things with {})", modelName, things.size(),
+                                handlerFactory.getClass().getSimpleName());
+                        things.forEach(thing -> {
+                            if (!retryCreateThing(handlerFactory, thing.getThingTypeUID(), thing.getConfiguration(),
+                                    thing.getUID(), thing.getBridgeUID())) {
+                                // Possible cause: Asynchronous loading of the XML files
+                                // Add the data to the queue in order to retry it later
+                                logger.debug(
+                                        "ThingHandlerFactory \'{}\' claimed it can handle \'{}\' type but actually did not. Queued for later refresh.",
+                                        handlerFactory.getClass().getSimpleName(), thing.getThingTypeUID());
+                                queueRetryThingCreation(handlerFactory, thing.getThingTypeUID(),
+                                        thing.getConfiguration(), thing.getUID(), thing.getBridgeUID());
+                            }
+                        });
+                    } else {
+                        logger.debug("No refresh needed from YAML model {}", modelName);
+                    }
+                });
+            } else {
+                logger.debug("No things yet loaded; no need to trigger a refresh due to new thing handler factory");
+            }
+        }
+    }
+
+    private boolean retryCreateThing(ThingHandlerFactory handlerFactory, ThingTypeUID thingTypeUID,
+            Configuration configuration, ThingUID thingUID, @Nullable ThingUID bridgeUID) {
+        logger.trace("Retry creating thing {}", thingUID);
+        Thing newThing = handlerFactory.createThing(thingTypeUID, configuration, thingUID, bridgeUID);
+        if (newThing != null) {
+            logger.debug("Successfully loaded thing \'{}\' during retry", thingUID);
+            Thing oldThing = null;
+            for (Collection<Thing> modelThings : thingsMap.values()) {
+                oldThing = modelThings.stream().filter(t -> t.getUID().equals(newThing.getUID())).findFirst()
+                        .orElse(null);
+                if (oldThing != null) {
+                    mergeThing(newThing, oldThing);
+                    modelThings.remove(oldThing);
+                    modelThings.add(newThing);
+                    logger.debug("Refreshing thing \'{}\' after successful retry", newThing.getUID());
+                    if (!ThingHelper.equals(oldThing, newThing)) {
+                        notifyListenersAboutUpdatedElement(oldThing, newThing);
+                    }
+                    break;
+                }
+            }
+            if (oldThing == null) {
+                logger.debug("Refreshing thing \'{}\' after retry failed because thing is not found",
+                        newThing.getUID());
+            }
         }
+        return newThing != null;
+    }
+
+    private boolean isThingHandlerFactoryReady(ThingHandlerFactory thingHandlerFactory) {
+        String bundleName = getBundleName(thingHandlerFactory);
+        return bundleName != null && loadedXmlThingTypes.contains(bundleName);
     }
 
     private @Nullable String getBundleName(ThingHandlerFactory thingHandlerFactory) {
@@ -300,20 +330,6 @@ private void thingHandlerFactoryAdded(ThingHandlerFactory thingHandlerFactory) {
         String[] segments = thingUID.getAsString().split(AbstractUID.SEPARATOR);
         ThingTypeUID thingTypeUID = new ThingTypeUID(thingUID.getBindingId(), segments[1]);
 
-        ThingHandlerFactory handlerFactory = thingHandlerFactories.stream()
-                .filter(thf -> thf.supportsThingType(thingTypeUID)).findFirst().orElse(null);
-        if (handlerFactory == null) {
-            if (modelLoaded) {
-                logger.info("No ThingHandlerFactory found for thing {} (thing-type is {}). Deferring initialization.",
-                        thingUID, thingTypeUID);
-            }
-            return null;
-        }
-        String bundleName = getBundleName(handlerFactory);
-        if (bundleName == null || !loadedXmlThingTypes.contains(bundleName)) {
-            return null;
-        }
-
         ThingType thingType = thingTypeRegistry.getThingType(thingTypeUID, localeProvider.getLocale());
         ThingUID bridgeUID = thingDto.bridge != null ? new ThingUID(thingDto.bridge) : null;
         Configuration configuration = new Configuration(thingDto.config);
@@ -333,22 +349,22 @@ private void thingHandlerFactoryAdded(ThingHandlerFactory thingHandlerFactory) {
 
         Thing thing = thingBuilder.build();
 
-        Thing thingFromHandler = handlerFactory.createThing(thingTypeUID, configuration, thingUID, bridgeUID);
-        if (thingFromHandler != null) {
-            mergeThing(thingFromHandler, thing);
-            logger.debug("Successfully loaded thing \'{}\'", thingUID);
-        } else {
-            // Possible cause: Asynchronous loading of the XML files
-            // Add the data to the queue in order to retry it later
-            logger.debug(
-                    "ThingHandlerFactory \'{}\' claimed it can handle \'{}\' type but actually did not. Queued for later refresh.",
-                    handlerFactory.getClass().getSimpleName(), thingTypeUID);
-            queue.add(new QueueContent(handlerFactory, thingTypeUID, configuration, thingUID, bridgeUID));
-            Thread thread = lazyRetryThread;
-            if (thread == null || !thread.isAlive()) {
-                thread = new Thread(lazyRetryRunnable);
-                lazyRetryThread = thread;
-                thread.start();
+        Thing thingFromHandler = null;
+        ThingHandlerFactory handlerFactory = thingHandlerFactories.stream()
+                .filter(thf -> isThingHandlerFactoryReady(thf) && thf.supportsThingType(thingTypeUID)).findFirst()
+                .orElse(null);
+        if (handlerFactory != null) {
+            thingFromHandler = handlerFactory.createThing(thingTypeUID, configuration, thingUID, bridgeUID);
+            if (thingFromHandler != null) {
+                mergeThing(thingFromHandler, thing);
+                logger.debug("Successfully loaded thing \'{}\'", thingUID);
+            } else {
+                // Possible cause: Asynchronous loading of the XML files
+                // Add the data to the queue in order to retry it later
+                logger.debug(
+                        "ThingHandlerFactory \'{}\' claimed it can handle \'{}\' type but actually did not. Queued for later refresh.",
+                        handlerFactory.getClass().getSimpleName(), thingTypeUID);
+                queueRetryThingCreation(handlerFactory, thingTypeUID, configuration, thingUID, bridgeUID);
             }
         }
 
@@ -383,7 +399,7 @@ private List<Channel> createChannels(ThingTypeUID thingTypeUID, ThingUID thingUI
                                 configDescriptionRegistry.getConfigDescription(descUriO));
                     }
                 } else {
-                    logger.warn("Channel type {} could not be found.", channelTypeUID);
+                    logger.warn("Channel type {} could not be found for thing '{}'.", channelTypeUID, thingUID);
                 }
             }
 
@@ -439,4 +455,15 @@ private void mergeThing(Thing target, Thing source) {
         // add the channels only defined in source list to the target list
         ThingHelper.addChannelsToThing(target, channelsToAdd);
     }
+
+    private void queueRetryThingCreation(ThingHandlerFactory handlerFactory, ThingTypeUID thingTypeUID,
+            Configuration configuration, ThingUID thingUID, @Nullable ThingUID bridgeUID) {
+        queue.add(new QueueContent(handlerFactory, thingTypeUID, configuration, thingUID, bridgeUID));
+        Thread thread = lazyRetryThread;
+        if (thread == null || !thread.isAlive()) {
+            thread = new Thread(lazyRetryRunnable);
+            lazyRetryThread = thread;
+            thread.start();
+        }
+    }
 }