Mismatch with disabled rules when using config files

[lolodomo]

I am not 100% sure if the bug is in core framewrok or in Main UI.
When you disable a rule in Main UI, the data is saved in the file userdata/jsondb/automation_rules_disabled.json.
When only one rule is disabled, the file content is:

{
  "maison-10": {
    "class": "java.lang.Boolean",
    "value": true
  }

What is saved seems to be a rule ID (maison-10) and not a rule name.
In config file, what we define are rule names (and not rule ID).
What happens is that if your remove a rule from this config file, the disabled rule will be another rule, not your original rule. There is a mismatch between disabled rules.

[lolodomo]

openhab-core/bundles/org.openhab.core.model.rule.runtime/src/org/openhab/core/model/rule/runtime/internal/DSLRuleProvider.java

Line 260 in 926304f

String uid = modelName + "-" + index;

Rule UID is built from the model name and with the index of the rule in the file.
With this way of doing, of course we can encounter the bug I declared.
Using the model name + rule name as rule UID would avoid the problem but at the same time that also means we can't have several rules with the same name in the same model. Maybe consider an index only if several rules in the model have the same name.

[lolodomo]

That's being said, by not considering the rule name in the rule UID, you can change rule name without creating a mismatch with disabled rules.
The mismatch only happens if you add or remove a rule in the model.

[lolodomo]

Maybe we should add a rule UID to the syntax for users wanting a proper handling of enabled/disabled rules.

[rkoshak]

Maybe we should add a rule UID to the syntax for users wanting a proper handling of enabled/disabled rules.

This would be my preferred approach. I believe only Rules DSL lacks the ability for users to set the UID of a rule. Not only would this solve this problem, but it will make it easier to map errors in the log to the rule that generated the rule.

[Nadahar]

There are some other issues here too, like if you remove a disabled rule, the "disabled entry" isn't removed (I believe) and is basically left there as "remains" that might strike another day if a rule with the same UID is created. Also, I think there's an issue with disabling rules created by automation add-ons (SimpleRule), they tend to run again the next time you start OH. Perhaps some of them create a random UID, so it won't be the same the next time - I don't know - I've just observed that it happens.

[jimtng]

There are some other issues here too, like if you remove a disabled rule, the "disabled entry" isn't removed

I encountered this long ago. My automation_rules_disabled.json file was huge.

[lolodomo]

Maybe something to add to Main UI "diagnostics" (sorry I don't remember the used terminology) like orphaned links and wrong tags ?
@mherwege for information

[mherwege]

Not sure if this is anything for diagnostics. If the rule does not exist anymore, I would think this should be removed from the storage, at time of removal, and maybe with a cleanup on upgrade (upgrade tool?). While you could put it in UI diagnostics, the UI would not allow you to do anything about it as the rule is gone. If the uid has changed, the UI cannot now about that.

[jimtng]

In a file-based ruby rule, when you save the file, the rule is removed, then re-created. So you shouldn't remove the "disabled" entry based on when the rule is "removed", otherwise a previously disabled rule gets re-enabled.

File based rules can be disabled/enabled through the UI.

[Nadahar]

@mherwege I think that managed rules already "clean up" when they are deleted, the problem is all the file-based ones. They are "deleted" and "created" all the time, when modified, during OH restart etc. So, removing them from the "disabled list" upon deletion would cause these to be impossible to disable. I think that they might be a good candidate for the "UI diagnostics", but probably with some kind of delay.