YAML things: Fix way to request models refresh to avoid a circular ref

Fix the circular reference.

YamlModelRepositoryImpl was referencing YamlThingProvider through YamlModelListener and YamlThingProvider was referencing YamlModelRepository. So circular reference.

The reference between YamlThingProvider and YamlModelRepository is now optional.
The refresh now only update things attached to a new loaded binding.

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

Author: lolodomo

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

@@ -25,6 +25,7 @@
  *
  * @author Laurent Garnier - Initial contribution
  * @author Laurent Garnier - Added basic version management
+ * @author Laurent Garnier - Added method refreshedModel
  */
 @NonNullByDefault
 public interface YamlModelListener<T extends YamlElement> {
@@ -56,6 +57,15 @@ public interface YamlModelListener<T extends YamlElement> {
      */
     void removedModel(String modelName, Collection<T> elements);
 
+    /**
+     * Method called by the model repository when elements from a model are refreshed. All elements are contained
+     * in the collection.
+     *
+     * @param modelName the name of the model
+     * @param elements the collection of refreshed elements
+     */
+    void refreshedModel(String modelName, Collection<T> elements);
+
     /**
      * Get the DTO class to be used for each object of this model type. The DTO class MUST implement {@link YamlElement}
      * and fulfill all requirements defined for the interface.

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

@@ -568,7 +568,6 @@ public void updateElementInModel(String modelName, YamlElement element) {
     @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);
@@ -589,7 +588,7 @@ public void refreshModelElements(String modelName, String elementName) {
             List elements = parseJsonNodes(modelNodes != null ? modelNodes : List.of(), modelMapNode, elementClass,
                     null, null);
             if (!elements.isEmpty()) {
-                listener.updatedModel(modelName, elements);
+                listener.refreshedModel(modelName, elements);
             }
         });
     }

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

@@ -105,6 +105,10 @@ public void removedModel(String modelName, Collection<YamlSemanticTagDTO> elemen
         });
     }
 
+    @Override
+    public void refreshedModel(String modelName, Collection<YamlSemanticTagDTO> elements) {
+    }
+
     private SemanticTag mapSemanticTag(YamlSemanticTagDTO tagDTO) {
         return new SemanticTagImpl(tagDTO.uid, tagDTO.label, tagDTO.description, tagDTO.synonyms);
     }

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

@@ -84,7 +84,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;
@@ -93,6 +92,7 @@ public class YamlThingProvider extends AbstractProvider<Thing>
 
     private final List<ThingHandlerFactory> thingHandlerFactories = new CopyOnWriteArrayList<>();
     private final Set<String> loadedXmlThingTypes = new CopyOnWriteArraySet<>();
+    private final List<ThingHandlerFactory> thingHandlerFactoriesToProcess = new CopyOnWriteArrayList<>();
 
     private final Map<String, Collection<Thing>> thingsMap = new ConcurrentHashMap<>();
 
@@ -148,17 +148,18 @@ public void run() {
 
     private @Nullable Thread lazyRetryThread;
 
+    private @Nullable YamlModelRepository modelRepository;
+
     private record QueueContent(ThingHandlerFactory thingHandlerFactory, ThingTypeUID thingTypeUID,
             Configuration configuration, ThingUID thingUID, @Nullable ThingUID bridgeUID) {
     }
 
     @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;
@@ -240,14 +241,29 @@ public void removedModel(String modelName, Collection<YamlThingDTO> elements) {
         }
     }
 
+    @Override
+    public void refreshedModel(String modelName, Collection<YamlThingDTO> elements) {
+        List<YamlThingDTO> elementsNeedingRefresh = elements.stream().filter(this::refreshNeeded).toList();
+        if (!elementsNeedingRefresh.isEmpty()) {
+            logger.info("Refreshing {} {} from YAML model {}", elementsNeedingRefresh.size(),
+                    getElementClass().getAnnotation(YamlElementName.class).value(), modelName);
+            updatedModel(modelName, elementsNeedingRefresh);
+        } else {
+            logger.debug("No refresh needed from YAML model {}", modelName);
+        }
+    }
+
     @Reference(cardinality = ReferenceCardinality.MULTIPLE, policy = ReferencePolicy.DYNAMIC)
     protected void addThingHandlerFactory(final ThingHandlerFactory thingHandlerFactory) {
+        logger.debug("addThingHandlerFactory {}", thingHandlerFactory.getClass().getSimpleName());
         thingHandlerFactories.add(thingHandlerFactory);
-        thingHandlerFactoryAdded(thingHandlerFactory);
+        thingHandlerFactoriesToProcess.add(thingHandlerFactory);
+        processAddedthingHandlerFactory(thingHandlerFactory);
     }
 
     protected void removeThingHandlerFactory(final ThingHandlerFactory thingHandlerFactory) {
         thingHandlerFactories.remove(thingHandlerFactory);
+        thingHandlerFactoriesToProcess.remove(thingHandlerFactory);
     }
 
     @Reference
@@ -267,26 +283,56 @@ public void onReadyMarkerAdded(ReadyMarker readyMarker) {
         } else if (XML_THING_TYPE.equals(type)) {
             String bsn = readyMarker.getIdentifier();
             loadedXmlThingTypes.add(bsn);
-            thingHandlerFactories.stream().filter(factory -> bsn.equals(getBundleName(factory))).forEach(factory -> {
-                thingHandlerFactoryAdded(factory);
-            });
+            thingHandlerFactoriesToProcess.stream().filter(factory -> bsn.equals(getBundleName(factory)))
+                    .forEach(factory -> {
+                        processAddedthingHandlerFactory(factory);
+                    });
         }
     }
 
+    @Reference(cardinality = ReferenceCardinality.OPTIONAL, policy = ReferencePolicy.DYNAMIC)
+    public void setModelRepository(final YamlModelRepository modelRepository) {
+        logger.debug("setModelRepository");
+        this.modelRepository = modelRepository;
+        thingHandlerFactoriesToProcess.forEach(factory -> {
+            processAddedthingHandlerFactory(factory);
+        });
+    }
+
+    public void unsetModelRepository(final YamlModelRepository modelRepository) {
+        this.modelRepository = null;
+    }
+
     @Override
     public void onReadyMarkerRemoved(ReadyMarker readyMarker) {
         loadedXmlThingTypes.remove(readyMarker.getIdentifier());
     }
 
-    private void thingHandlerFactoryAdded(ThingHandlerFactory thingHandlerFactory) {
+    private boolean refreshNeeded(YamlThingDTO thingDto) {
+        ThingUID thingUID = new ThingUID(thingDto.uid);
+        String[] segments = thingUID.getAsString().split(AbstractUID.SEPARATOR);
+        ThingTypeUID thingTypeUID = new ThingTypeUID(thingUID.getBindingId(), segments[1]);
+        ThingHandlerFactory handlerFactory = thingHandlerFactoriesToProcess.stream()
+                .filter(thf -> thf.supportsThingType(thingTypeUID)).findFirst().orElse(null);
+        return handlerFactory != null;
+    }
+
+    private void processAddedthingHandlerFactory(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());
-            });
+        YamlModelRepository repository = modelRepository;
+        if (repository != null && bundleName != null && loadedXmlThingTypes.contains(bundleName)) {
+            if (!thingsMap.isEmpty()) {
+                logger.debug("Refreshing models due to new thing handler factory {}",
+                        thingHandlerFactory.getClass().getSimpleName());
+                thingsMap.keySet().forEach(modelName -> {
+                    repository.refreshModelElements(modelName,
+                            getElementClass().getAnnotation(YamlElementName.class).value());
+                });
+            } else {
+                logger.debug("No things yet loaded; no need to trigger a refresh due to new thing handler factory {}",
+                        thingHandlerFactory.getClass().getSimpleName());
+            }
+            thingHandlerFactoriesToProcess.remove(thingHandlerFactory);
         }
     }