Various YAML fixes by Nadahar · Pull Request #5588 · openhab/openhab-core

Nadahar · 2026-05-22T16:53:52Z

While working on an upcoming PR to handle "the new YAML format" from the community marketplace, I encountered some issues with the current implementation:

The Widget (and UI Page) YAML definitions use DTOs made for GSON, not Jackson, which leads to annotations not working, among other things. Importing example widgets of mine therefore failed, e.g. because default isn't mapped to defaultValue.
The "list/array" form of the configuration description parameters is inconsistent with the way they are specified in rule templates (where they have "map/object" form). That's very confusing when it is in fact the same data otherwise.
ObjectParser.getParsedObjects() returns a collection that is "unstable" from all but one implementation, which means that when finishParsingFormat() is called, the already retrieved collection is magically emptied (because it's linked to the underlyinig Map). This is very impractical, and surprising, when parsing.

I've addressed all these issues in the PR. I've created new YAML specific DTOs YamlConfigDescriptionDTO and YamlConfigDescriptionParameterGroupDTO. YamlConfigDescriptionDTO can deserialize parameter and parameter groups in either "list/array" or "map/object" form, but will serialize to "map/object" form. I've replaced the DTOs used for configuration description for both UI Widgets and UI Pages with this new implementation.

I've created a YamlWidgetParser, which is one step on the way to fully integrate UI Widgets in the REST API "file conversion" endpoints. I did not make the serialization part because I don't need it for the marketplace handling, and I don't know whether there are pitfalls there that I'm not familiar with.

It should be said that there are some "holes" in the logic regarding UIComponents and RootUIComponents. Currently, a RootUIComponent is just a UIComponent that supports a configuration. It makes no sense to me, it should rather be a ConfigurableUIComponent or similar, if at all needed. Perhaps all UIComponent implementations could support having a configuration, I don't really know the consequences. In addition, there are no "implementation types" here, so "everything" is pretty much a RootUIComponent. That means that we can't differentiate, and the current widget: tag can, as far as I can tell, be used to import all kind of UI components. Whether that "loophole" matters or not, I don't know, but it's there anyway.

Finally, I tried to make sure that ObjectParser.getParsedObjects() don't return live views, and create copies instead. I've also added JavaDoc comments to try to make it clear that future implementations should do the same.

Nadahar · 2026-05-22T16:54:24Z

@lolodomo @mherwege FYI

lolodomo · 2026-05-22T17:25:41Z

Note that I have also added List.copyOf in getAllFromModel in that other PR #5573 , at least for all YAML providers.

Nadahar · 2026-05-22T17:28:30Z

Note that I have also added List.copyOf in getAllFromModel in that other PR #5573 , at least for YAML providers.

Hehe, at least we agree that it should be that way 😉 It doesn't really matter where we do it, as long as it's done. I need to adapt #5572 to this as well.

lolodomo · 2026-05-22T17:29:48Z

Hehe, at least we agree that it should be that way

It was a comment from CoPilot also.

I hope my PR can be merged first ;)

Nadahar · 2026-05-22T17:33:05Z

I hope my PR can be merged first ;)

I'm not afraid of doing the rebasing, if that's what you're thinking of, but I was hoping that this could be a bit quick, since my work on the marketplace is "halted" waiting on this now. The PR is quite small and simple, so it shouldn't be too bad to review.

lolodomo · 2026-05-22T17:37:58Z

My PR is even smaller than yours and its review is I believe very well advanced ;)

Nadahar · 2026-05-22T17:41:24Z

My PR is even smaller than yours and its review is I believe very well advanced ;)

As long as things move forward, there's no problem for me to rebase this to fit "on top of" yours - but I don't know sematic tags very well, so I can't quite understand if you have fully resolved the "complications" or not.

lolodomo · 2026-05-28T10:30:49Z

You will have to adjust your PR after the merge of #5573 handling a part of your changes.

Nadahar · 2026-05-28T11:51:25Z

You will have to adjust your PR after the merge of #5573 handling a part of your changes.

I know, but just to make it simpler: I already know that we both changed getAllFromModel(). Are there any other changes you're aware of where we have conflicts?

…upDTO Signed-off-by: Ravi Nadahar <nadahar@rediffmail.com>

Signed-off-by: Ravi Nadahar <nadahar@rediffmail.com>

…er.java Signed-off-by: Ravi Nadahar <nadahar@rediffmail.com>

Signed-off-by: Ravi Nadahar <nadahar@rediffmail.com>

Nadahar · 2026-05-28T13:06:45Z

Rebased on latest main.

@lolodomo I didn't find anything else that clashed.

Nadahar · 2026-05-28T13:57:40Z

@mherwege Now that #5573 has been merged, it would be nice if this got some priority. This PR is basically bugfixes, the YAML widget and page implementations currently in main are actually broken. They must have received very minimal testing before merging, because it started failing immediately when I tried to use it to import "real world widgets".

…core/model/yaml/internal/config/YamlConfigDescriptionDTO.java Co-authored-by: Mark Herwege <mherwege@users.noreply.github.com> Signed-off-by: Nadahar <Nadahar@users.noreply.github.com>

mherwege · 2026-05-29T06:45:32Z

+ *
+ * SPDX-License-Identifier: EPL-2.0
+ */
+package org.openhab.core.model.yaml.internal.widgets.converter;


In line with the other converters, put it in a package org.openhab.core.model.yaml.internal.widgets.fileconverter.

None of those I have made uses "fileconverter" - because I think "file" is misleading here. When marketplace add-ons are being converted, there is no "file" involved. The same when the UI converts between JSON and DSL/YAML when you switch tabs.

We did agree to remove "file" from various places for this reason, but @lolodomo didn't want to rename the packages that already existed. My idea then has been that I at least won't make new ones with that name.

I understand. I am fine either way, but I don't like the inconsistency. @lolodomo what is your view?

I don't like too.

I'm no fan of inconsistency either, but I'm not a fan of letting one "mistake" cause an issue to spread either. Ideally, I would have preferred to rename the existing packages, but I understand that breaking the API for a mere "consistency issue" probably isn't worth it.

It is the same for rules and rule templates by the way.

I would have preferred to rename the existing packages, but I understand that breaking the API for a mere "consistency issue" probably isn't worth it.

How much a breaking change is this? To me knowledge, this is only used in core by the REST API and therefore fairly easy to change. But let's keep that separate from this PR.

Agreed. I also see that "converter" might be overly generic, and perhaps e.g. "formatconverter" or something like that would be a better fit.

Copilot

Pull request overview

This PR improves YAML handling in openHAB’s model/file-converter layer by introducing YAML-specific DTOs for config descriptions, normalizing config-description parameter forms, and making parsed-object retrieval stable (not tied to live underlying maps) across parsers/providers.

Changes:

Update ObjectParser#getParsedObjects contract/signature to return a copy and use Collection<? extends T> to support safer/covariant returns.
Introduce YAML-specific config description DTOs + Jackson deserializers that accept both list/array and map/object representations (while standardizing internal representation).
Add YAML widget parsing support via a new YamlWidgetParser, and update widget/page providers/DTOs to use the new YAML config-description DTOs.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
bundles/org.openhab.core/src/main/java/org/openhab/core/items/fileconverter/ItemParser.java	Updates parsed-object contract/Javadoc to require returning copies.
bundles/org.openhab.core/src/main/java/org/openhab/core/converter/ObjectParser.java	Adjusts API contract + return type to avoid live-view collections.
bundles/org.openhab.core.ui/src/main/java/org/openhab/core/ui/components/converter/UIComponentParser.java	Adds a parser interface for UI components aligned with the updated `ObjectParser` contract.
bundles/org.openhab.core.ui/src/main/java/org/openhab/core/ui/components/converter/RootUIComponentParser.java	Adds a parser interface for root UI components (noted doc/import issues).
bundles/org.openhab.core.thing/src/main/java/org/openhab/core/thing/fileconverter/ThingParser.java	Updates parsed-object contract/Javadoc to require returning copies.
bundles/org.openhab.core.sitemap/src/main/java/org/openhab/core/sitemap/fileconverter/SitemapParser.java	Updates parsed-object contract/Javadoc to require returning copies.
bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/widgets/YamlWidgetProvider.java	Ensures per-model widget retrieval returns a stable copy; adapts props mapping to new YAML DTO.
bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/widgets/YamlWidgetDTO.java	Switches widget `props` to YAML-specific config-description DTO.
bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/widgets/converter/YamlWidgetParser.java	Adds YAML widget parser using isolated models (noted charset issue).
bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/util/ConfigDescriptionParametersDeserializer.java	Adds Jackson deserializer accepting list or map forms for config parameters.
bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/util/ConfigDescriptionParameterGroupsDeserializer.java	Adds Jackson deserializer accepting list or map forms for parameter groups.
bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/pages/YamlPageProvider.java	Adapts page props mapping to new YAML config-description DTO.
bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/pages/YamlPageDTO.java	Switches page `props` to YAML-specific config-description DTO.
bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/config/YamlConfigDescriptionParameterGroupDTO.java	Adds YAML DTO for parameter groups (noted Javadoc syntax issue).
bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/config/YamlConfigDescriptionParameterDTO.java	Extends YAML parameter DTO with mapping helpers + list-entry variant (noted Javadoc syntax issue).
bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/config/YamlConfigDescriptionDTO.java	Adds YAML config-description DTO bridging to core DTO/model (noted Javadoc field name issue).
bundles/org.openhab.core.model.thing/src/org/openhab/core/model/thing/internal/GenericThingProvider.xtend	Returns stable per-model thing collections via `List.copyOf`.
bundles/org.openhab.core.model.item/src/org/openhab/core/model/item/internal/GenericItemProvider.java	Returns stable per-model item collections via `List.copyOf`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Signed-off-by: Ravi Nadahar <nadahar@rediffmail.com>

mherwege

LGTM

Nadahar requested a review from a team as a code owner May 22, 2026 16:53

Nadahar force-pushed the yaml-config-description branch from c0219d9 to 7e2fe9b Compare May 22, 2026 17:12

Nadahar mentioned this pull request May 22, 2026

Support "new YAML" on the marketplace for rule templates and widgets #5590

Draft

Ravi Nadahar added 4 commits May 28, 2026 14:55

Create YamlConfigDescriptionDTO and YamlConfigDescriptionParameterGro…

ab03b57

…upDTO Signed-off-by: Ravi Nadahar <nadahar@rediffmail.com>

Use YamlConfigDescriptionDTO for YAML pages and widgets

f730910

Signed-off-by: Ravi Nadahar <nadahar@rediffmail.com>

Create YamlWidgetParser and add getAllFromModel() to YamlWidgetProvid…

51a9114

…er.java Signed-off-by: Ravi Nadahar <nadahar@rediffmail.com>

Make sure that ObjectParser.getParsedObjects() returns a copy

0e6c1a8

Signed-off-by: Ravi Nadahar <nadahar@rediffmail.com>

Nadahar force-pushed the yaml-config-description branch from 7e2fe9b to 0e6c1a8 Compare May 28, 2026 13:05

Nadahar mentioned this pull request May 28, 2026

Add semantic tags to conversion REST API #5573

Merged

mherwege reviewed May 28, 2026

View reviewed changes

Comment thread ...yaml/src/main/java/org/openhab/core/model/yaml/internal/config/YamlConfigDescriptionDTO.java Outdated

Update bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/…

20ccc4e

…core/model/yaml/internal/config/YamlConfigDescriptionDTO.java Co-authored-by: Mark Herwege <mherwege@users.noreply.github.com> Signed-off-by: Nadahar <Nadahar@users.noreply.github.com>

mherwege reviewed May 29, 2026

View reviewed changes

mherwege requested a review from Copilot May 29, 2026 07:29

Copilot started reviewing on behalf of mherwege May 29, 2026 07:29 View session

Copilot AI reviewed May 29, 2026

View reviewed changes

Address review feedback and apply explicit charset to other "converters"

8f070dd

Signed-off-by: Ravi Nadahar <nadahar@rediffmail.com>

mherwege approved these changes May 29, 2026

View reviewed changes

mherwege merged commit 1132515 into openhab:main May 29, 2026
5 checks passed

mherwege added this to the 5.2 milestone May 29, 2026

mherwege added the bug An unexpected problem or unintended behavior of the Core label May 29, 2026

Nadahar deleted the yaml-config-description branch May 29, 2026 14:52

Uh oh!

Conversation

Nadahar commented May 22, 2026

Uh oh!

Nadahar commented May 22, 2026

Uh oh!

lolodomo commented May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Nadahar commented May 22, 2026

Uh oh!

lolodomo commented May 22, 2026

Uh oh!

Nadahar commented May 22, 2026

Uh oh!

lolodomo commented May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Nadahar commented May 22, 2026

Uh oh!

lolodomo commented May 28, 2026

Uh oh!

Nadahar commented May 28, 2026

Uh oh!

Nadahar commented May 28, 2026

Uh oh!

Nadahar commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mherwege left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

lolodomo commented May 22, 2026 •

edited

Loading

lolodomo commented May 22, 2026 •

edited

Loading

Nadahar commented May 28, 2026 •

edited

Loading