Add optional rule UID to DSL Rule file syntax

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

@@ -12,6 +12,8 @@
  */
 package org.openhab.core.model.rule.runtime.internal;
 
+import static org.openhab.core.model.core.ModelCoreConstants.isIsolatedModel;
+
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Iterator;
@@ -105,6 +107,7 @@
  *
  * @author Kai Kreuzer - Initial contribution
  * @author Laurent Garnier - Add support for conditions
+ * @author Laurent Garnier - Add optional rule UID + registry notification refactoring
  */
 @NonNullByDefault
 @Component(immediate = true, service = { DSLRuleProvider.class, RuleProvider.class, DSLScriptContextProvider.class })
@@ -115,7 +118,7 @@ public class DSLRuleProvider
 
     private final Logger logger = LoggerFactory.getLogger(DSLRuleProvider.class);
     private final Collection<ProviderChangeListener<Rule>> listeners = new ArrayList<>();
-    private final Map<String, Rule> rules = new ConcurrentHashMap<>();
+    private final Map<String, List<Rule>> rulesMap = new ConcurrentHashMap<>();
     private final Map<String, IEvaluationContext> contexts = new ConcurrentHashMap<>();
     private final Map<String, XExpression> xExpressions = new ConcurrentHashMap<>();
     private final ReadyMarker marker = new ReadyMarker("rules", "dslprovider");
@@ -139,7 +142,7 @@ protected void activate() {
     @Deactivate
     protected void deactivate() {
         modelRepository.removeModelRepositoryChangeListener(this);
-        rules.clear();
+        rulesMap.clear();
         contexts.clear();
         xExpressions.clear();
     }
@@ -150,82 +153,106 @@ public void addProviderChangeListener(ProviderChangeListener<Rule> listener) {
     }
 
     @Override
-    public Collection<Rule> getAll() {
-        return rules.values();
+    public void removeProviderChangeListener(ProviderChangeListener<Rule> listener) {
+        listeners.remove(listener);
     }
 
     @Override
-    public void removeProviderChangeListener(ProviderChangeListener<Rule> listener) {
-        listeners.remove(listener);
+    public Collection<Rule> getAll() {
+        // Ignore isolated models
+        return rulesMap.entrySet().stream().filter(e -> !isIsolatedModel(e.getKey()))
+                .flatMap(e -> e.getValue().stream()).toList();
+    }
+
+    /**
+     * Returns all rules originating from the given model name.
+     *
+     * @param modelFileName the full model file name, including ".rules" or ".script" extension
+     * @return the rules associated with the given model name, or an empty collection if none exist
+     */
+    public Collection<Rule> getAllFromModel(String modelFileName) {
+        return List.copyOf(rulesMap.getOrDefault(modelFileName, List.of()));
     }
 
     @Override
     public void modelChanged(String modelFileName, EventType type) {
         String ruleModelType = modelFileName.substring(modelFileName.lastIndexOf(".") + 1);
+        List<Rule> oldRules;
+        List<Rule> newRules = new ArrayList<>();
         if ("rules".equalsIgnoreCase(ruleModelType)) {
+            boolean isolated = isIsolatedModel(modelFileName);
             String ruleModelName = modelFileName.substring(0, modelFileName.lastIndexOf("."));
-            List<ModelRulePair> modelRules = new ArrayList<>();
             switch (type) {
                 case ADDED:
+                case MODIFIED:
                     EObject model = modelRepository.getModel(modelFileName);
+                    int index = 1;
                     if (model instanceof RuleModel ruleModel) {
-                        int index = 1;
                         for (org.openhab.core.model.rule.rules.Rule rule : ruleModel.getRules()) {
-                            Rule newRule = toRule(ruleModelName, rule, index);
-                            rules.put(newRule.getUID(), newRule);
-                            xExpressions.put(ruleModelName + "-" + index, rule.getScript());
-                            modelRules.add(new ModelRulePair(newRule, null));
+                            newRules.add(toRule(ruleModelName, rule, index));
+                            if (!isolated) {
+                                xExpressions.put(ruleModelName + "-" + index, rule.getScript());
+                            }
                             index++;
                         }
-                        handleVarDeclarations(ruleModelName, ruleModel);
+                        if (!isolated) {
+                            handleVarDeclarations(ruleModelName, ruleModel);
+                        }
                     }
-                    break;
-                case MODIFIED:
-                    removeRuleModel(ruleModelName);
-                    EObject modifiedModel = modelRepository.getModel(modelFileName);
-                    if (modifiedModel instanceof RuleModel ruleModel) {
-                        int index = 1;
-                        for (org.openhab.core.model.rule.rules.Rule rule : ruleModel.getRules()) {
-                            Rule newRule = toRule(ruleModelName, rule, index);
-                            Rule oldRule = rules.remove(ruleModelName);
-                            rules.put(newRule.getUID(), newRule);
-                            xExpressions.put(ruleModelName + "-" + index, rule.getScript());
-                            modelRules.add(new ModelRulePair(newRule, oldRule));
+                    oldRules = rulesMap.put(modelFileName, newRules);
+                    if (!isolated) {
+                        // Cleanup xExpressions for old rules
+                        int nbOldRules = oldRules == null ? 0 : oldRules.size();
+                        while (index <= nbOldRules) {
+                            xExpressions.remove(ruleModelName + "-" + index);
                             index++;
                         }
-                        handleVarDeclarations(ruleModelName, ruleModel);
+                        // Cleanup contexts if no new rules
+                        if (newRules.isEmpty()) {
+                            contexts.remove(ruleModelName);
+                        }
+                        notifyProviderChangeListeners(calcChanges(modelFileName, oldRules, newRules));
                     }
                     break;
                 case REMOVED:
-                    removeRuleModel(ruleModelName);
+                    oldRules = rulesMap.remove(modelFileName);
+                    if (!isolated) {
+                        for (Iterator<Entry<String, XExpression>> it = xExpressions.entrySet().iterator(); it
+                                .hasNext();) {
+                            Entry<String, XExpression> entry = it.next();
+                            if (belongsToModel(entry.getKey(), ruleModelName)) {
+                                it.remove();
+                            }
+                        }
+                        contexts.remove(ruleModelName);
+                        notifyProviderChangeListeners(calcChanges(modelFileName, oldRules, null));
+                    }
                     break;
                 default:
                     logger.debug("Unknown event type.");
             }
-            notifyProviderChangeListeners(modelRules);
         } else if ("script".equals(ruleModelType)) {
-            List<ModelRulePair> modelRules = new ArrayList<>();
             switch (type) {
-                case MODIFIED:
                 case ADDED:
+                case MODIFIED:
                     EObject model = modelRepository.getModel(modelFileName);
                     if (model instanceof Script script) {
-                        Rule oldRule = rules.remove(modelFileName);
-                        Rule newRule = toRule(modelFileName, script);
-                        rules.put(newRule.getUID(), newRule);
-                        modelRules.add(new ModelRulePair(newRule, oldRule));
+                        newRules.add(toRule(modelFileName, script));
+                    }
+                    oldRules = rulesMap.put(modelFileName, newRules);
+                    if (!isIsolatedModel(modelFileName)) {
+                        notifyProviderChangeListeners(calcChanges(modelFileName, oldRules, newRules));
                     }
                     break;
                 case REMOVED:
-                    Rule oldRule = rules.remove(modelFileName);
-                    if (oldRule != null) {
-                        listeners.forEach(listener -> listener.removed(this, oldRule));
+                    oldRules = rulesMap.remove(modelFileName);
+                    if (!isIsolatedModel(modelFileName)) {
+                        notifyProviderChangeListeners(calcChanges(modelFileName, oldRules, null));
                     }
                     break;
                 default:
                     logger.debug("Unknown event type.");
             }
-            notifyProviderChangeListeners(modelRules);
         }
     }
 
@@ -244,24 +271,42 @@ private void handleVarDeclarations(String modelName, RuleModel ruleModel) {
         contexts.put(modelName, context);
     }
 
-    private void removeRuleModel(String modelName) {
-        Iterator<Entry<String, Rule>> it = rules.entrySet().iterator();
-        while (it.hasNext()) {
-            Entry<String, Rule> entry = it.next();
-            if (belongsToModel(entry.getKey(), modelName)) {
-                listeners.forEach(listener -> listener.removed(this, entry.getValue()));
-
-                it.remove();
-            }
+    private Changes calcChanges(String modelFileName, @Nullable List<Rule> oldRules, @Nullable List<Rule> newRules) {
+        if (oldRules == null || oldRules.isEmpty()) {
+            return new Changes(newRules == null ? List.of() : List.copyOf(newRules), List.of(), List.of());
         }
-        Iterator<Entry<String, XExpression>> it2 = xExpressions.entrySet().iterator();
-        while (it2.hasNext()) {
-            Entry<String, XExpression> entry = it2.next();
-            if (belongsToModel(entry.getKey(), modelName)) {
-                it2.remove();
+        List<Rule> oldMutable = new ArrayList<>(oldRules);
+        List<Rule> newMutable = newRules == null ? new ArrayList<>() : new ArrayList<>(newRules);
+        List<RulePair> modified = new ArrayList<>();
+        Rule oldRule, newRule;
+        boolean found;
+        for (Iterator<Rule> iterator = oldMutable.iterator(); iterator.hasNext();) {
+            oldRule = iterator.next();
+            found = false;
+            String uid = oldRule.getUID();
+            for (Iterator<Rule> newIterator = newMutable.iterator(); newIterator.hasNext() && !found;) {
+                newRule = newIterator.next();
+                if (uid.equals(newRule.getUID())) {
+                    modified.add(new RulePair(oldRule, newRule));
+                    newIterator.remove();
+                    found = true;
+                }
+            }
+            if (!found) {
+                // Check if the old rule exists in another model file with the same UID
+                Rule existingRule = rulesMap.entrySet().stream()
+                        .filter(e -> !e.getKey().equals(modelFileName) && !isIsolatedModel(e.getKey()))
+                        .flatMap(e -> e.getValue().stream()).filter(r -> uid.equals(r.getUID())).findAny().orElse(null);
+                if (existingRule != null) {
+                    modified.add(new RulePair(oldRule, existingRule));
+                    found = true;
+                }
+            }
+            if (found) {
+                iterator.remove();
             }
         }
-        contexts.remove(modelName);
+        return new Changes(newMutable, modified, oldMutable);
     }
 
     private boolean belongsToModel(String id, String modelName) {
@@ -287,7 +332,10 @@ private Rule toRule(String modelName, Script script) {
 
     private Rule toRule(String modelName, org.openhab.core.model.rule.rules.Rule rule, int index) {
         String name = rule.getName();
-        String uid = modelName + "-" + index;
+        String uid = rule.getUid();
+        if (uid == null || uid.isBlank()) {
+            uid = modelName + "-" + index;
+        }
 
         // Create Triggers
         triggerId = 0;
@@ -540,46 +588,60 @@ private String removeIndentation(String script) {
 
     @Override
     public void onReadyMarkerAdded(ReadyMarker readyMarker) {
-        for (String ruleFileName : modelRepository.getAllModelNamesOfType("rules")) {
-            EObject model = modelRepository.getModel(ruleFileName);
-            String ruleModelName = ruleFileName.substring(0, ruleFileName.indexOf("."));
+        for (String modelFileName : modelRepository.getAllModelNamesOfType("rules")) {
+            EObject model = modelRepository.getModel(modelFileName);
             if (model instanceof RuleModel ruleModel) {
+                boolean isolated = isIsolatedModel(modelFileName);
+                String ruleModelName = modelFileName.substring(0, modelFileName.lastIndexOf("."));
                 int index = 1;
-                List<ModelRulePair> modelRules = new ArrayList<>();
+                List<Rule> newRules = new ArrayList<>();
                 for (org.openhab.core.model.rule.rules.Rule rule : ruleModel.getRules()) {
-                    Rule newRule = toRule(ruleModelName, rule, index);
-                    xExpressions.put(ruleModelName + "-" + index, rule.getScript());
-                    modelRules.add(new ModelRulePair(newRule, null));
+                    newRules.add(toRule(ruleModelName, rule, index));
+                    if (!isolated) {
+                        xExpressions.put(ruleModelName + "-" + index, rule.getScript());
+                    }
                     index++;
                 }
-                handleVarDeclarations(ruleModelName, ruleModel);
-
-                notifyProviderChangeListeners(modelRules);
+                List<Rule> oldRules = rulesMap.put(modelFileName, newRules);
+                if (!isolated) {
+                    handleVarDeclarations(ruleModelName, ruleModel);
+                    notifyProviderChangeListeners(calcChanges(modelFileName, oldRules, newRules));
+                }
             }
         }
         modelRepository.addModelRepositoryChangeListener(this);
         readyService.markReady(marker);
     }
 
-    private void notifyProviderChangeListeners(List<ModelRulePair> modelRules) {
-        modelRules.forEach(rulePair -> {
-            Rule oldRule = rulePair.oldRule();
-            if (oldRule != null) {
-                rules.remove(oldRule.getUID());
-                rules.put(rulePair.newRule().getUID(), rulePair.newRule());
-                listeners.forEach(listener -> listener.updated(this, oldRule, rulePair.newRule()));
-            } else {
-                rules.put(rulePair.newRule().getUID(), rulePair.newRule());
-                listeners.forEach(listener -> listener.added(this, rulePair.newRule()));
-            }
-        });
+    private void notifyProviderChangeListeners(Changes changes) {
+        if (listeners.isEmpty()) {
+            return;
+        }
+        for (Rule rule : changes.removed) {
+            for (ProviderChangeListener<Rule> listener : listeners) {
+                listener.removed(this, rule);
+            }
+        }
+        for (RulePair pair : changes.modified) {
+            for (ProviderChangeListener<Rule> listener : listeners) {
+                listener.updated(this, pair.oldRule, pair.newRule);
+            }
+        }
+        for (Rule rule : changes.added) {
+            for (ProviderChangeListener<Rule> listener : listeners) {
+                listener.added(this, rule);
+            }
+        }
     }
 
     @Override
     public void onReadyMarkerRemoved(ReadyMarker readyMarker) {
         readyService.unmarkReady(marker);
     }
 
-    private record ModelRulePair(Rule newRule, @Nullable Rule oldRule) {
+    private record RulePair(Rule oldRule, Rule newRule) {
+    }
+
+    private record Changes(List<Rule> added, List<RulePair> modified, List<Rule> removed) {
     }
 }

bundles/org.openhab.core.model.rule/src/org/openhab/core/model/rule/Rules.xtext

@@ -12,7 +12,7 @@ VariableDeclaration:
 ;
 
 Rule:
-	'rule' name=(STRING|ID) ('[' tags+=(ID|STRING) (',' tags+=(ID|STRING))* ']')?
+	'rule' name=(STRING|ID) ('uid' '='? uid=(STRING|ID))? ('[' tags+=(ID|STRING) (',' tags+=(ID|STRING))* ']')?
 	'when' eventtrigger+=EventTrigger ('or' eventtrigger+=EventTrigger)*
 	('but' 'only' 'if' conditions+=Condition ('and' conditions+=Condition)*)?
 	'then' script=XExpressionInClosure

itests/org.openhab.core.model.rule.tests/src/main/java/org/openhab/core/model/rule/runtime/DSLRuleProviderTest.java

@@ -103,7 +103,7 @@ public void testSimpleRules() {
         Collection<Rule> rules = dslRuleProvider.getAll();
         assertThat(rules.size(), is(0));
 
-        String model = "rule RuleNumberOne [ Test, \"my test\" ]\n" + //
+        String model = "rule RuleNumberOne uid = rule1 [ Test, \"my test\" ]\n" + //
                 "when\n" + //
                 "   System started\n" + //
                 "then\n" + //
@@ -125,7 +125,7 @@ public void testSimpleRules() {
 
         Rule firstRule = it.next();
 
-        assertThat(firstRule.getUID(), is("dslruletest-1"));
+        assertThat(firstRule.getUID(), is("rule1"));
         assertThat(firstRule.getName(), is("RuleNumberOne"));
         assertThat(firstRule.getTags(), hasSize(2));
         assertThat(firstRule.getTags(), hasItem("Test"));