Add optional rule UID to DSL Rule file syntax

[lolodomo]

Closes #5437

Allow avoiding iussue described in #5415 by setting an UID.

Also avoid notifying the rule registry for isolated models.

[lolodomo]

Withis rule file:

rule "Initialize demo items"
when
  System started
then
  DemoLocation.postUpdate("52,9")
  DemoContact.postUpdate(OPEN)
  DemoString.postUpdate("Hello SmartHome!")
  DemoDateTime.postUpdate(new DateTimeType())
  DemoNumber.postUpdate(12.34)
end

rule "Test time of day trigger" myTestRule [ Test, "my test" ]
when
  Time is 13:26
then
  logInfo("Rule","Test")
end

The result in Main UI:

[lolodomo]

Going into draft mode until I check what happens if several same UID are used.

[openhab-bot]

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/rules-and-rule-templates-yaml-integration/168568/143

[lolodomo]

Going into draft mode until I check what happens if several same UID are used.

No problem, it is detected by the registry (as for other stuff - items, things, ...).

I realized that the syntax did not accept hyphen inside the UID and not a number at the beginning of the ID, I will have to extend that and implement the same control on the UID as what @Nadahar implemented in the YAML provider (":" also accepted).

@florian-h05 or @jsjames : can you please tell me what is the syntax accepted in Main UI for the rule UID ? We need the same to allow "conversions".

[florian-h05]

Main UI validates against this RegEx:

https://github.com/openhab/openhab-webui/blob/723626067bd8eafaec88b1737df5d91a77a8736e/bundles/org.openhab.ui/web/src/components/rule/rule-general-settings.vue#L17

[lolodomo]

So Main UI accepts leading hypen and does not accept colon

[Nadahar]

So Main UI accepts leading hypen and does not accept colon

It might be that it's time to formalize the format for the UID. I think it's probably "a bit random" what has been done until now, when I made "my choice", I did so by looking around and trying to "sum it up". It probably shouldn't be very restricted though, to avoid breaking existing rules.

[lolodomo]

For compatibility reason, I propose to at least cover Main UI pattern.

[florian-h05]

It might be that it's time to formalize the format for the UID.

I support that.
If we only apply the new format for new rules, and don’t validate existing UIDs against it, it shouldn’t be a problem to be more strict in Main UI.

[lolodomo]

I plan to create a new RuleUID extending AbstractUID.
Edit: or simply add a method isValidRuleUID.
The allowed syntax will be exactly the one already used by the YAML provider.
This will cause problem only for existing UI rules with UID starting with a hyphen, conversion to DSL or YAML will then fail. We could then remove the leading hyphens as a workaround. But it should not concern a lot of users as it is not natural to do that I believe.

[Nadahar]

But it should not concern a lot of users as it is not natural to do that I believe.

Unless you're a C programmer 😝

[florian-h05]

BTW, also see https://github.com/openhab/openhab-core/blob/4d9cacc3ae14bef89409949a2235bd1a9c414e1e/bundles/org.openhab.core/src/main/java/org/openhab/core/util/UIDUtils.java and what it does.

[Nadahar]

BTW, also see https://github.com/openhab/openhab-core/blob/4d9cacc3ae14bef89409949a2235bd1a9c414e1e/bundles/org.openhab.core/src/main/java/org/openhab/core/util/UIDUtils.java and what it does.

I'm not sure what the benefit is of such encoding. I mean, as far as I know, "any string" will work here (as that's basically what we have today). We have URL encoding to apply if used as a URL parameter, JSON has its way to encode this.

I think we should ideally only restrict what can cause problems somewhere. I'm not sure what that is, but some of the "symbols" can probably collide with something somewhere..?

[lolodomo]

I propose to be consistent with other UID like thing UID.

[Nadahar]

I propose to be consistent with other UID like thing UID.

Isn't that much more restrictive than it is now, and will break a lot of existing rules. Is that really "worth it"?

[Nadahar]

Suggested change

	return rulesMap.keySet().stream().filter(name -> !isIsolatedModel(name))
	.map(name -> rulesMap.getOrDefault(name, List.of())).flatMap(list -> list.stream()).toList();
	return rulesMap.entrySet().stream().filter(e -> !isIsolatedModel(e.getKey()))
	.flatMap(e -> e.getValue().stream()).toList();

[Nadahar]

This is more efficient and prevents lookup in the map from inside the iteration. Neither of these solutions are thread-safe, but that goes for the whole logic around rules/rulesMap and is nothing new.

[Nadahar]

It would be better to use a List than a Collection, then you could just call first().

[lolodomo]

Good suggestion

[Nadahar]

Suggested change

	Iterator<Entry<String, XExpression>> it = xExpressions.entrySet().iterator();
	while (it.hasNext()) {
	for (Iterator<Entry<String, XExpression>> it = xExpressions.entrySet().iterator(); it.hasNext();) {

[Nadahar]

I think there's a thing you have overlooked (unless I have overlooked your solution in the code): The xExpressions map.

The rule actions are stored as "scripts" in this map, using the "indexed UID scheme". To let the rules continue to work, non-generated rule UIDs must be handled here (and in RuleEngine) as well. It seems like RuleEngine "extracts" the index. It seems awfully complicated, and I'm not sure if I understand the point. Wasn't it easier if the xExpressions map were simply indexed with the rule UID, regardless of the "index system". The rule has a UID, and then xExpression should be possible to retrieve using the same UID IMO. Regardless, this needs to be taken into account.