Skip to content
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.

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

Rule file provider #4633

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Rule file provider #4633

wants to merge 2 commits into from

Conversation

Nadahar
Copy link
Contributor

@Nadahar Nadahar commented Mar 5, 2025

Rule file provider

  • Create rule file provider for JSON and YAML files
  • Fix DELETE event bug in FolderObserver

Description

The primary task of this PR is to provide file-based YAML and JSON rules. It supports multiple rules in one file if they are supplied within an array, which means enclosed in [ and ] and separated with comma.

This PR is very loosely coupled to #4591 in that they are both split out as PRs from the same "work branch", and this PR use the AbstractJacksonYAMLParser introduced in #4591. The FolderObserver file watcher DELETE fix has been included here, since it was discovered after #4591 was submitted.

There is one issue that might need to be discussed, and that is the location to look for the YAML and JSON files. There are two "obvious candidates", conf/automation/rules and conf/rules. The latter is the folder already in use for Rules DSL files, while the former isn't currently used (or automatically created when OH is installed). In either case, some minor changes to the documentation should be made as well, but it's hard to write that until the final location has been decided.

Currently, this PR uses conf/rules, but that's very easy to change. From a user's perspective, this might be the most logical, as they are still rules, although the format is different. It doesn't seem like this causes any conflicts, but a debug log message is created both from the Rules DSL parser (that it ignores the .yaml or .json file) and from the AbstractFileProvider that it doesn't have a parser for .rules files. None of these are shown by default. The involved classes could be modified to not log these cases, as they are expected, but both are "generic in nature" and introducing exceptions to concrete file extensions doesn't really "fit".

If conf/automation/rules are used instead, the "conflict" with two parsers monitoring the same folder would be avoided, but it might be more confusing for the end user to have to place the rule files in two different locations. It should also be considered if this folder should be created when the installation is set up, like with many of the other folders.

It should also be mentioned that the documentation currently claims support for JSON based rules, but no evidence that this exists, or has existed in previous versions, has been found in the code:

The automation engine reads rule json files from the {openhab-dir}/automation/*.json directory.

If this actually is wrong, the documentation should be corrected.

Ravi Nadahar added 2 commits February 13, 2025 03:53
…server

Signed-off-by: Ravi Nadahar <nadahar@rediffmail.com>
Signed-off-by: Ravi Nadahar <nadahar@rediffmail.com>
@Nadahar Nadahar requested a review from a team as a code owner March 5, 2025 16:24
@Nadahar Nadahar force-pushed the rule-file-provider branch from ba2b127 to 7fde6c3 Compare March 5, 2025 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant