Skip to content

[REST] API /file-format/parse: do not extend thing/channel config #5295

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
holgerfriedrich merged 7 commits into from
Jan 27, 2026
Merged

[REST] API /file-format/parse: do not extend thing/channel config #5295

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0d5db33
[REST] API /file-format/parse: do not extend thing/channel config
lolodomo Jan 23, 2026
ed33fc6
Copilot comment considered: rename method parameter name
lolodomo Jan 23, 2026
5f1eb2e
Unit tests added for YAML models containing Thing
lolodomo Jan 24, 2026
9d9d560
Merge branch 'main' into do_not_extend_config
lolodomo Jan 24, 2026
7b06bbe
Remove one channel from test thing to simplify unit tests
lolodomo Jan 24, 2026
341270c
Merge 2x2 unit tests to simplify
lolodomo Jan 24, 2026
0976ae8
Enhanced unit tests + 1 new test
lolodomo Jan 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 32 additions & 27 deletions ...hab.core.model.thing/src/org/openhab/core/model/thing/internal/GenericThingProvider.xtend
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ class GenericThingProvider extends AbstractProviderLazyNullness<Thing> implement
}

def private void createThing(ModelThing modelThing, Collection<Thing> thingList,
ThingHandlerFactory thingHandlerFactory) {
ThingHandlerFactory thingHandlerFactory, boolean isolatedModel) {
val ThingUID thingUID = getThingUID(modelThing, null)
if (thingUID === null) {
// ignore the Thing because its definition is broken
Expand Down Expand Up @@ -249,8 +249,8 @@ class GenericThingProvider extends AbstractProviderLazyNullness<Thing> implement
val location = modelThing.location

val ThingUID bridgeUID = if(modelThing.bridgeUID !== null) new ThingUID(modelThing.bridgeUID)
val thingFromHandler = getThingFromThingHandlerFactories(thingTypeUID, label, configuration, thingUID,
bridgeUID, thingHandlerFactory)
val thingFromHandler = getThingFromThingHandlerFactories(thingTypeUID, label, new Configuration(configuration),
thingUID, bridgeUID, thingHandlerFactory, isolatedModel)

val thingBuilder = if (modelThing instanceof ModelBridge) {
BridgeBuilder.create(thingTypeUID, thingUID)
Expand All @@ -263,7 +263,7 @@ class GenericThingProvider extends AbstractProviderLazyNullness<Thing> implement
thingBuilder.withLabel(label)
thingBuilder.withLocation(location)

val channels = createChannels(thingTypeUID, thingUID, modelThing.channels,
val channels = createChannels(!isolatedModel, thingTypeUID, thingUID, modelThing.channels,
thingType?.channelDefinitions ?: newArrayList)
thingBuilder.withChannels(channels)

Expand All @@ -273,7 +273,7 @@ class GenericThingProvider extends AbstractProviderLazyNullness<Thing> implement
if (thingFromHandler !== null) {

// If a thingHandlerFactory could create a thing, merge the content of the modelThing to it
thingFromHandler.merge(thing)
thingFromHandler.merge(thing, isolatedModel)
}

thingList += thingFromHandler ?: thing
Expand All @@ -292,25 +292,27 @@ class GenericThingProvider extends AbstractProviderLazyNullness<Thing> implement
}

def private Thing getThingFromThingHandlerFactories(ThingTypeUID thingTypeUID, String label,
Configuration configuration, ThingUID thingUID, ThingUID bridgeUID, ThingHandlerFactory specific) {
Configuration configuration, ThingUID thingUID, ThingUID bridgeUID, ThingHandlerFactory specific,
boolean isolatedModel) {
if (specific !== null && specific.supportsThingType(thingTypeUID)) {
logger.trace("Creating thing from specific ThingHandlerFactory {} for thingType {}", specific, thingTypeUID)
return getThingFromThingHandlerFactory(thingTypeUID, label, configuration, thingUID, bridgeUID, specific)
return getThingFromThingHandlerFactory(thingTypeUID, label, configuration, thingUID, bridgeUID,
specific, isolatedModel)
}
for (ThingHandlerFactory thingHandlerFactory : thingHandlerFactories) {
logger.trace("Searching thingHandlerFactory for thingType: {}", thingTypeUID)
if (thingHandlerFactory.supportsThingType(thingTypeUID)) {
return getThingFromThingHandlerFactory(thingTypeUID, label, configuration, thingUID, bridgeUID,
thingHandlerFactory)
thingHandlerFactory, isolatedModel)
}
}
null
}

def private getThingFromThingHandlerFactory(ThingTypeUID thingTypeUID, String label, Configuration configuration,
ThingUID thingUID, ThingUID bridgeUID, ThingHandlerFactory thingHandlerFactory) {
ThingUID thingUID, ThingUID bridgeUID, ThingHandlerFactory thingHandlerFactory, boolean isolatedModel) {
val thing = thingHandlerFactory.createThing(thingTypeUID, configuration, thingUID, bridgeUID)
if (thing === null) {
if (!isolatedModel && thing === null) {
// Apparently the HandlerFactory's eyes were bigger than its stomach...
// Possible cause: Asynchronous loading of the XML files
// Add the data to the queue in order to retry it later
Expand All @@ -322,32 +324,35 @@ class GenericThingProvider extends AbstractProviderLazyNullness<Thing> implement
lazyRetryThread = new Thread(lazyRetryRunnable)
lazyRetryThread.start
}
} else {
} else if (thing !== null) {
thing.label = label
}
return thing
}

def dispatch void merge(Thing targetThing, Thing sourceThing) {
def dispatch void merge(Thing targetThing, Thing sourceThing, boolean keepSourceConfig) {
targetThing.bridgeUID = sourceThing.bridgeUID
targetThing.configuration.merge(sourceThing.configuration)
targetThing.merge(sourceThing.channels)
targetThing.configuration.merge(sourceThing.configuration, keepSourceConfig)
targetThing.merge(sourceThing.channels, keepSourceConfig)
targetThing.location = sourceThing.location
targetThing.label = sourceThing.label
}

def dispatch void merge(Configuration target, Configuration source) {
def dispatch void merge(Configuration target, Configuration source, boolean keepSourceConfig) {
if (keepSourceConfig) {
target.setProperties(Map.of())
}
source.keySet.forEach [
Comment on lines +341 to 345

Copilot AI Jan 23, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add/extend unit tests around the isolated-model parsing flow to cover the new keepSourceConfig merge behavior (clearing the target config before copying source keys). This should verify that /file-format/parse for DSL does not materialize default-valued config parameters that were omitted in the input.

Copilot uses AI. Check for mistakes.
target.put(it, source.get(it))
]
}

def dispatch void merge(Thing targetThing, List<Channel> source) {
def dispatch void merge(Thing targetThing, List<Channel> source, boolean keepSourceConfig) {
val List<Channel> channelsToAdd = newArrayList()
source.forEach [ sourceChannel |
val targetChannels = targetThing.channels.filter[it.UID.equals(sourceChannel.UID)]
targetChannels.forEach [
merge(sourceChannel)
merge(sourceChannel, keepSourceConfig)
]
if (targetChannels.empty) {
channelsToAdd.add(sourceChannel)
Expand All @@ -358,8 +363,8 @@ class GenericThingProvider extends AbstractProviderLazyNullness<Thing> implement
ThingHelper.addChannelsToThing(targetThing, channelsToAdd)
}

def dispatch void merge(Channel target, Channel source) {
target.configuration.merge(source.configuration)
def dispatch void merge(Channel target, Channel source, boolean keepSourceConfig) {
target.configuration.merge(source.configuration, keepSourceConfig)
}

def private getParentPath(ThingUID bridgeUID) {
Expand All @@ -369,8 +374,8 @@ class GenericThingProvider extends AbstractProviderLazyNullness<Thing> implement
return bridgeIds
}

def private List<Channel> createChannels(ThingTypeUID thingTypeUID, ThingUID thingUID,
List<ModelChannel> modelChannels, List<ChannelDefinition> channelDefinitions) {
def private List<Channel> createChannels(boolean applyDefaultConfig, ThingTypeUID thingTypeUID,
ThingUID thingUID, List<ModelChannel> modelChannels, List<ChannelDefinition> channelDefinitions) {
val Set<String> addedChannelIds = newHashSet
val List<Channel> channels = newArrayList
modelChannels.forEach [
Expand All @@ -392,7 +397,7 @@ class GenericThingProvider extends AbstractProviderLazyNullness<Thing> implement
}
autoUpdatePolicy = resolvedChannelType.autoUpdatePolicy
val cfgDescUriOfresolvedChannelType = resolvedChannelType.configDescriptionURI
if (cfgDescUriOfresolvedChannelType !== null) {
if (applyDefaultConfig && cfgDescUriOfresolvedChannelType !== null) {
ConfigUtil.applyDefaultConfiguration(configuration,
configDescriptionRegistry.getConfigDescription(
cfgDescUriOfresolvedChannelType))
Expand Down Expand Up @@ -574,7 +579,7 @@ class GenericThingProvider extends AbstractProviderLazyNullness<Thing> implement
}

def thingHandlerFactoryAdded(ThingHandlerFactory thingHandlerFactory) {
thingsMap.keySet.forEach [
thingsMap.keySet.filter[!isIsolatedModel(it)].forEach [
// create things for this specific thingHandlerFactory from the model.
createThingsFromModelForThingHandlerFactory(it, thingHandlerFactory)
]
Expand Down Expand Up @@ -628,7 +633,7 @@ class GenericThingProvider extends AbstractProviderLazyNullness<Thing> implement
val model = modelRepository.getModel(modelName) as ThingModel
if (model !== null) {
flattenModelThings(model.things).forEach [
createThing(newThings, factory)
createThing(newThings, factory, isIsolatedModel(modelName))
]
}

Expand Down Expand Up @@ -661,8 +666,8 @@ class GenericThingProvider extends AbstractProviderLazyNullness<Thing> implement
val newThings = new ArrayList
queue.forEach [ qc |
logger.trace("Searching thingHandlerFactory for thingType: {}", qc.thingTypeUID)
val thing = qc.thingHandlerFactory.createThing(qc.thingTypeUID, qc.configuration, qc.thingUID,
qc.bridgeUID)
val thing = qc.thingHandlerFactory.createThing(qc.thingTypeUID,
new Configuration(qc.configuration), qc.thingUID, qc.bridgeUID)
if (thing !== null) {
queue.remove(qc)
logger.debug("Successfully loaded '{}' during retry", qc.thingUID)
Expand All @@ -676,7 +681,7 @@ class GenericThingProvider extends AbstractProviderLazyNullness<Thing> implement
]
val oldThing = thingsMap.get(modelName).findFirst[it.UID == newThing.UID]
if (oldThing !== null) {
newThing.merge(oldThing)
newThing.merge(oldThing, false)
thingsMap.get(modelName).remove(oldThing)
thingsMap.get(modelName).add(newThing)
logger.debug("Refreshing thing '{}' after successful retry", newThing.UID)
Expand Down
Loading