Rule conversion preparation

[Nadahar]

I'm working on a PR that will handle conversion of rules and rule templates to/from YAML and DSL. The upcoming PR will be somewhat comprehensive, so I've separated out things that I've come across during the work that isn't really related to it, in collected it in this PR.

This means that there is no real "theme" to this PR, it's a bit of this and a bit of that. It doesn't really contain any real changes, it's mostly minor bug fixes and "general cleanup".

If you think this must be split into even smaller PRs, please say so, but I'm hoping this is acceptable like this.

I've written relatively detailed commit messages that should explain what each change does.

[Nadahar]

@lolodomo I don't know if there's anything here you find interesting, but it touches the YAML stuff, so you'll probably want to take a look.

[mherwege]

@Nadahar There is an integration test failure with these changes.

[Copilot]

Pull request overview

Preparatory refactor/cleanup changes in openHAB Core ahead of upcoming YAML/DSL rule + template conversion work, focusing on isolated-model handling and some internal API/utility improvements.

Changes:

• Filter YAML rule/rule-template providers to ignore “isolated” models and suppress listener notifications for those models; add per-model accessors.
• Refine module type alias mapping (including additional alias support) and align rule/template handling utilities (RuleBuilder, RuleImpl, RuleRegistryImpl).
• Minor bugfixes/docs cleanup (DTO list sizing, config parameter Javadoc, scoping provider cleanup, gitignore).

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/rules/YamlRuleTemplateProvider.java	Ignore isolated models for template exposure/notifications; add per-model retrieval.
bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/rules/YamlRuleProvider.java	Ignore isolated models for rule exposure/notifications; add per-model retrieval.
bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/rules/ModuleTypeAliases.java	Switch alias table to UID constants; expand mappings; improve typeToAlias for implementations.
bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/config/YamlConfigDescriptionParameterDTO.java	Fix list pre-sizing bug (filterCriterias.size()).
bundles/org.openhab.core.model.yaml/pom.xml	Add dependency needed for script module UID constants.
bundles/org.openhab.core.model.script/src/org/openhab/core/model/script/scoping/StateAndCommandProvider.java	Replace mutable static sets with immutable sets; minor API cleanup.
bundles/org.openhab.core.model.script/.gitignore	Ignore /src.moved/.
bundles/org.openhab.core.config.core/src/main/java/org/openhab/core/config/core/ConfigDescriptionParameter.java	Fix incorrect Javadoc for isReadOnly().
bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/util/RuleBuilder.java	Add builder overloads/docs for cloning and template preparation.
bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/RuleRegistryImpl.java	Refactor configuration resolution/validation; tighten template-related docs/logic.
bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/RuleImpl.java	Rename template status field/setter to templateState.
bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/ModuleImpl.java	Add toString() implementation.
bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/Rule.java	Clarify getTemplateUID() Javadoc wording.

---

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

[Nadahar]

There is an integration test failure with these changes.

Yeah, I don't run those locally because it takes ages, so I guess that's expected. I'll have to see what the problem is.

[Nadahar]

There is an integration test failure with these changes.

I reverted some of the changes, because resolveConfigurations() turns out to be used for more than I thought when I changed it. The goal was to avoid needlessly inserting default values "polluting" the rules (basically the same issue that has been discussed for Things), but since the population of default values is so intertwined with other logic, the "fix" isn't this simple. It will have to be its own project to figure that out, and I'm not up for that at the moment, thus the reversal.

[dilyanpalauzov]

Looking at the branch https://github.com/Nadahar/openhab-core.git:yaml-rules:

Set<String> enums = new LinkedHashSet<>();
for (State state : StateAndCommandProvider.getAllStates())
    enums.add(state.toString());
this.enumStates = Set.copyOf(enums);

enums = new LinkedHashSet<>();
for (Command command : StateAndCommandProvider.getAllCommands())
    enums.add(command.toString());
this.enumCommands = Set.copyOf(enums);

I guess you want actually that the return type of StateAndCommandProvider::getAllStates() and StateAndCommandProvider.getAllCommands() is an unmodifiable collection, so that you can do here just

this.enumStates = StateAndCommandProvider.getAllStates()
this.enumCommands = StateAndCommandProvider.getAllCommands()

or

this.enumStates = Set.copyOf(StateAndCommandProvider.getAllStates());
this.enumCommands = Set.copyOf(StateAndCommandProvider.getAllCommands());

where Set.copyOf() just returns its parameter, generally without creating a copy, if the parameter is an unmodifiable Set.

Or you might even want do not keep members enumStates and enumCommands but use instead StateAndCommandProvider.getAllStates() and StateAndCommandProvider.getAllCommands() directly.

Given that, you might want in the current changeset to alter the return types of StateAndCommandProvider::getAllStates and StateAndCommandProvider::getAllCommands.

[Nadahar]

@dilyanpalauzov I wrote that code when the methods weren't static and the returned collections were mutable - which is why I iterate and copy. The "retrieving code" should be simplified now that these changes have been made. But, I don't want to switch to that branch until this PR has gone through, in case I need to do more changes here, so I'll just have to try to remember to do that when I can switch back to the "working branch".

Switching branches takes hours of building and updating before everything is resolved and Eclipse is ready to go, which is why I don't want to do it. This is particularly bad when working with the org.openhab.core.model.* bundles, which can make getting everything back working after a switch a nightmare.

[dilyanpalauzov]

For the record, #5509 proposes removing all usages of StateAndCommandProvider::getAllTypes() (and therefore all usages of the classStateAndCommandProvider). If it gets integrated, the right thing to do would be to delete the method StateAndCommandProvider::getAllTypes(), move the remaining code, to where it is nedded, and make it package-private.

[lolodomo]

This looks good to me

[mherwege]

LGTM

[Nadahar]

Thanks, now I can go back to the "work branch" and continue 😉

[Nadahar]

@mherwege I'm trying to rebase the sitemap changes into my work branch, FileFormatResource is full of conflicts because we have changed all the same things (obviously). But, it seems to be, that I'm missing some sitemap code when trying to resolve this: In the parse() method, in the application/yaml section, I can't find any if (sitemapParser != null) section, which will probably mean that if a combined JSON is parsed, containing sitemaps and other types, the resulting YAML won't contain the sitemap(s).

Is that intentional or an oversight?

[mherwege]

The merged code for sitemaps only covers DSL. @lolodomo is working on YAML support.

[Nadahar]

The merged code for sitemaps only covers DSL. @lolodomo is working on YAML support.

Ah, ok, that explains it. I don't have to worry that I botched something rebasing then 👍

[dilyanpalauzov]

It was not clarified here if OpenClosedType.CLOSED is as COMMAND.

[Nadahar]

It was not clarified here if OpenClosedType.CLOSED is as COMMAND.

Probably because nobody here knows for sure. As far as I can tell, it's not a command. I think it's been this way from the start: eclipse-archived/smarthome@108712e

Maybe it was initially intended to also be a command, but that it ended up not being implemented or needed. What sense would it make to send a command CLOSED to something?

This is probably where the mistake happened: openhab/openhab1-addons@10c02b0

As you can see, it was initially intended to be both a state and a command, and then changes to being just a state, but implements Command was never removed. I'm pretty sure that the right "fix" is to remove that.

[lolodomo]

It was not clarified here if OpenClosedType.CLOSED is as COMMAND.

It is a state but not a command.

[mherwege]

It was not clarified here if OpenClosedType.CLOSED is as COMMAND.

It is a state but not a command.

That's also my understanding. But removing it as a Command should be in a separate PR. This has been around for to long and we may not see all consequences in the code of removing this.

[dilyanpalauzov]

As demonstrated at #5515, org.openhab.core.io.transport.modbus assumes that OpenClosedType can be a command:

[kaikreuzer]

This is probably where the mistake happened: openhab/openhab1-addons@10c02b0

Wow, that's some digging in the past. 😄
At that time I was the only committer, so I guess the culprit is clear.
Indeed my assumption was also always that it isn't a command, so it's really weird that 16 years later the enum still implements the command interface... Good catch!