diff --git a/bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/YamlModelRepository.java b/bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/YamlModelRepository.java index bec0466c5b1..eaa33edbc30 100644 --- a/bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/YamlModelRepository.java +++ b/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. * diff --git a/bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/YamlModelRepositoryImpl.java b/bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/YamlModelRepositoryImpl.java index 9407c154c3b..111eda169fc 100644 --- a/bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/YamlModelRepositoryImpl.java +++ b/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 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 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) { diff --git a/bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/things/YamlThingProvider.java b/bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/things/YamlThingProvider.java index 153c00391b6..868d0dadc5f 100644 --- a/bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/things/YamlThingProvider.java +++ b/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 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> entry : thingsMap.entrySet()) { - oldThing = entry.getValue().stream().filter(t -> t.getUID().equals(newThing.getUID())) - .findFirst().orElse(null); - if (oldThing != null) { - mergeThing(newThing, oldThing); - Collection 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 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 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 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 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); } } @@ -415,7 +431,12 @@ private List createChannels(ThingTypeUID thingTypeUID, ThingUID thingUI } private void mergeThing(Thing target, Thing source) { - target.setLabel(source.getLabel()); + String label = source.getLabel(); + if (label == null) { + ThingType thingType = thingTypeRegistry.getThingType(target.getThingTypeUID(), localeProvider.getLocale()); + label = thingType != null ? thingType.getLabel() : null; + } + target.setLabel(label); target.setLocation(source.getLocation()); target.setBridgeUID(source.getBridgeUID()); @@ -439,4 +460,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(); + } + } }