Prune orphaned entries in automation_rules_disabled.json on start up (#5514)

* Prune orphaned entries in automation_rules_disabled.json on start up * Configurable startup delay * Wait for removal in case rules are being reloaded Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>
2026-07-29 12:34:22 +02:00 · 2026-04-27 12:36:12 +02:00
parent fb8e12efce
commit 64253e54a8
2 changed files with 253 additions and 4 deletions
@@ -0,0 +1,167 @@
+/*
+ * Copyright (c) 2010-2026 Contributors to the openHAB project
+ *
+ * See the NOTICE file(s) distributed with this work for additional
+ * information.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ */
+package org.openhab.core.automation.internal;
+
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.Future;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+import java.util.function.Supplier;
+
+import org.openhab.core.automation.RuleRegistry;
+import org.openhab.core.storage.Storage;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Encapsulates the disabled-rules cleanup logic: tracking missing-since timestamps, scheduling
+ * rescans and removing orphaned entries from storage.
+ *
+ * @author Jimmy Tanagra - Initial contribution
+ */
+public class DisabledRulesCleaner {
+
+    private final Logger logger = LoggerFactory.getLogger(DisabledRulesCleaner.class);
+
+    private final RuleRegistry ruleRegistry;
+    private final Storage<Boolean> disabledRulesStorage;
+    private final Supplier<ScheduledExecutorService> executorSupplier;
+    private static final long ORPHAN_THRESHOLD_MS = TimeUnit.MINUTES.toMillis(2);
+    private static final long RECHECK_INTERVAL_MS = TimeUnit.MINUTES.toMillis(1);
+
+    private final Object lock = new Object();
+
+    // allocated when the cleaner itself is created
+    private final Map<String, Long> missingSince = new ConcurrentHashMap<>();
+    private volatile Future<?> scheduledFuture;
+    private volatile Runnable onFinished;
+
+    public DisabledRulesCleaner(RuleRegistry ruleRegistry, Storage<Boolean> disabledRulesStorage,
+            Supplier<ScheduledExecutorService> executorSupplier) {
+        this.ruleRegistry = ruleRegistry;
+        this.disabledRulesStorage = disabledRulesStorage;
+        this.executorSupplier = executorSupplier;
+    }
+
+    /**
+     * Set a callback that will be invoked when the cleaner finishes its scan-rescan sequence
+     * and has no remaining candidates. The callback is executed outside the cleaner lock.
+     */
+    public void setOnFinished(Runnable onFinished) {
+        this.onFinished = onFinished;
+    }
+
+    public void start(long initialDelayMinutes) {
+        synchronized (lock) {
+            cancelInternal();
+            if (initialDelayMinutes <= 0) {
+                logger.debug("Disabled rules cleanup is disabled ({} minutes)", initialDelayMinutes);
+                return;
+            }
+            scheduledFuture = executorSupplier.get().schedule(this::runCleanupScan, initialDelayMinutes,
+                    TimeUnit.MINUTES);
+        }
+    }
+
+    public void stop() {
+        synchronized (lock) {
+            cancelInternal();
+            // clear tracked candidates to free memory
+            missingSince.clear();
+        }
+    }
+
+    public void onRuleAdded(String uid) {
+        Map<String, Long> ms = missingSince;
+        if (ms != null) {
+            ms.remove(uid);
+        }
+    }
+
+    private void cancelInternal() {
+        Future<?> f = scheduledFuture;
+        if (f != null && !f.isDone()) {
+            f.cancel(true);
+        }
+        scheduledFuture = null;
+    }
+
+    private void runCleanupScan() {
+        logger.debug("Starting cleanup scan of disabled rules in storage '{}'", "automation_rules_disabled");
+        try {
+            long now = System.currentTimeMillis();
+
+            final Set<String> keys = new HashSet<>(disabledRulesStorage.getKeys());
+
+            Map<String, Long> ms = missingSince;
+
+            for (String uid : keys) {
+                if (ruleRegistry.get(uid) == null) {
+                    ms.putIfAbsent(uid, now);
+                } else {
+                    ms.remove(uid);
+                }
+            }
+
+            int removed = 0;
+            for (Iterator<Entry<String, Long>> it = ms.entrySet().iterator(); it.hasNext();) {
+                Entry<String, Long> e = it.next();
+                String uid = e.getKey();
+                long since = e.getValue();
+                if (now - since >= ORPHAN_THRESHOLD_MS) {
+                    if (disabledRulesStorage.remove(uid) != null) {
+                        removed++;
+                        logger.debug("Removed disabled-rules entry for non-existing rule '{}'", uid);
+                    }
+                    it.remove();
+                }
+            }
+
+            if (removed > 0) {
+                logger.info("Disabled rules cleanup: removed {} orphan entries.", removed);
+            } else {
+                logger.debug("Disabled rules cleanup: no orphan entries removed in this scan.");
+            }
+
+            if (!ms.isEmpty()) {
+                logger.debug("Remaining {} potential orphan entries; scheduling re-scan in {} ms", ms.size(),
+                        RECHECK_INTERVAL_MS);
+                synchronized (lock) {
+                    scheduledFuture = executorSupplier.get().schedule(this::runCleanupScan, RECHECK_INTERVAL_MS,
+                            TimeUnit.MILLISECONDS);
+                }
+            } else {
+                Runnable notify = null;
+                synchronized (lock) {
+                    scheduledFuture = null;
+                    ms.clear();
+                    notify = this.onFinished;
+                }
+                if (notify != null) {
+                    try {
+                        notify.run();
+                    } catch (Throwable t) {
+                        logger.warn("DisabledRulesCleaner onFinished callback threw: {}", t.getMessage());
+                    }
+                }
+            }
+        } catch (Throwable t) {
+            logger.warn("Exception during disabled rules cleanup: {}", t.getMessage());
+        }
+    }
+}
@@ -30,6 +30,7 @@ import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
 import java.util.stream.Stream;

 import org.eclipse.jdt.annotation.NonNullByDefault;
@@ -88,6 +89,7 @@ import org.openhab.core.storage.StorageService;
 import org.osgi.service.component.annotations.Activate;
 import org.osgi.service.component.annotations.Component;
 import org.osgi.service.component.annotations.Deactivate;
+import org.osgi.service.component.annotations.Modified;
 import org.osgi.service.component.annotations.Reference;
 import org.osgi.service.component.annotations.ReferenceCardinality;
 import org.osgi.service.component.annotations.ReferencePolicy;
@@ -113,7 +115,7 @@ import org.slf4j.LoggerFactory;
 * @author Ana Dimova - new reference syntax: list[index], map["key"], bean.field
 * @author Florian Hotze - add support for script condition/action compilation
 */
-@Component(immediate = true, service = { RuleManager.class })
+@Component(immediate = true, service = { RuleManager.class }, configurationPid = RuleEngineImpl.SERVICE_PID)
@NonNullByDefault
 public class RuleEngineImpl implements RuleManager, RegistryChangeListener<ModuleType>, ReadyTracker {

@@ -128,6 +130,14 @@ public class RuleEngineImpl implements RuleManager, RegistryChangeListener<Modul

    private static final ReadyMarker MARKER = new ReadyMarker("ruleengine", "start");

+    static final String SERVICE_PID = "org.openhab.ruleengine";
+    private static final String DISABLED_RULES_CLEANUP_DELAY_PROP = "disabledRules.cleanupDelayMinutes";
+
+    // Delay (in minutes) after reaching startlevel rules to run cleanup. 0 = disabled.
+    private volatile long disabledRulesCleanupDelayMinutes = 30L;
+
+    private final AtomicReference<@Nullable DisabledRulesCleaner> disabledRulesCleaner = new AtomicReference<>();
+
    private final Map<String, WrappedRule> managedRules = new ConcurrentHashMap<>();

    /**
@@ -256,9 +266,10 @@ public class RuleEngineImpl implements RuleManager, RegistryChangeListener<Modul
     * Constructor of {@link RuleEngineImpl}.
     */
    @Activate
-    public RuleEngineImpl(final @Reference ModuleTypeRegistry moduleTypeRegistry,
-            final @Reference RuleRegistry ruleRegistry, final @Reference StorageService storageService,
-            final @Reference ReadyService readyService, final @Reference StartLevelService startLevelService) {
+    public RuleEngineImpl(final Map<String, Object> configuration,
+            final @Reference ModuleTypeRegistry moduleTypeRegistry, final @Reference RuleRegistry ruleRegistry,
+            final @Reference StorageService storageService, final @Reference ReadyService readyService,
+            final @Reference StartLevelService startLevelService) {
        this.disabledRulesStorage = storageService.getStorage(DISABLED_RULE_STORAGE, this.getClass().getClassLoader());

        mtRegistry = moduleTypeRegistry;
@@ -292,10 +303,29 @@ public class RuleEngineImpl implements RuleManager, RegistryChangeListener<Modul
            addRule(rule);
        }

+        updateDisabledRulesCleanupDelay(configuration);
+
        readyService.registerTracker(this, new ReadyMarkerFilter().withType(StartLevelService.STARTLEVEL_MARKER_TYPE)
                .withIdentifier(Integer.toString(StartLevelService.STARTLEVEL_RULES)));
    }

+    private DisabledRulesCleaner getOrCreateDisabledRulesCleaner() {
+        for (;;) {
+            DisabledRulesCleaner cur = disabledRulesCleaner.get();
+            if (cur != null) {
+                return cur;
+            }
+            DisabledRulesCleaner created = new DisabledRulesCleaner(this.ruleRegistry, this.disabledRulesStorage,
+                    this::getScheduledExecutor);
+            // when the cleaner naturally finishes, clear the reference so it can be GC'ed
+            created.setOnFinished(() -> disabledRulesCleaner.compareAndSet(created, null));
+            if (disabledRulesCleaner.compareAndSet(null, created)) {
+                return created;
+            }
+            // CAS failed; another thread installed a cleaner — retry
+        }
+    }
+
    /**
     * The method cleans used resources by rule engine when it is deactivated.
     */
@@ -310,6 +340,11 @@ public class RuleEngineImpl implements RuleManager, RegistryChangeListener<Modul

        compositeFactory.deactivate();

+        DisabledRulesCleaner prev = disabledRulesCleaner.getAndSet(null);
+        if (prev != null) {
+            prev.stop();
+        }
+
        for (Future<?> f : scheduleTasks.values()) {
            f.cancel(true);
        }
@@ -452,6 +487,11 @@ public class RuleEngineImpl implements RuleManager, RegistryChangeListener<Modul
        final String rUID = newRule.getUID();
        final WrappedRule rule = new WrappedRule(newRule);
        managedRules.put(rUID, rule);
+        // Inform cleaner that the rule is present again (clears missing timestamp)
+        DisabledRulesCleaner c = disabledRulesCleaner.get();
+        if (c != null) {
+            c.onRuleAdded(rUID);
+        }
        RuleStatusInfo initStatusInfo = disabledRulesStorage.get(rUID) == null
                ? new RuleStatusInfo(RuleStatus.INITIALIZING)
                : new RuleStatusInfo(RuleStatus.UNINITIALIZED, RuleStatusDetail.DISABLED);
@@ -1572,6 +1612,9 @@ public class RuleEngineImpl implements RuleManager, RegistryChangeListener<Modul
    @Override
    public void onReadyMarkerAdded(ReadyMarker readyMarker) {
        compileRules();
+        if (disabledRulesCleanupDelayMinutes > 0) {
+            getOrCreateDisabledRulesCleaner().start(disabledRulesCleanupDelayMinutes);
+        }
    }

    @Override
@@ -1608,6 +1651,45 @@ public class RuleEngineImpl implements RuleManager, RegistryChangeListener<Modul
        });
    }

+    @Modified
+    protected void modified(Map<String, Object> configuration) {
+        long old = disabledRulesCleanupDelayMinutes;
+        updateDisabledRulesCleanupDelay(configuration);
+        if (old != disabledRulesCleanupDelayMinutes) {
+            DisabledRulesCleaner prev = disabledRulesCleaner.getAndSet(null);
+            if (prev != null) {
+                prev.stop();
+            }
+            if (disabledRulesCleanupDelayMinutes > 0
+                    && startLevelService.getStartLevel() >= StartLevelService.STARTLEVEL_RULES) {
+                getOrCreateDisabledRulesCleaner().start(disabledRulesCleanupDelayMinutes);
+            }
+        }
+    }
+
+    private void updateDisabledRulesCleanupDelay(Map<String, Object> configuration) {
+        if (configuration == null) {
+            disabledRulesCleanupDelayMinutes = 30L;
+            return;
+        }
+        Object v = configuration.get(DISABLED_RULES_CLEANUP_DELAY_PROP);
+        if (v == null) {
+            disabledRulesCleanupDelayMinutes = 30L;
+            return;
+        }
+        try {
+            if (v instanceof Number) {
+                disabledRulesCleanupDelayMinutes = ((Number) v).longValue();
+            } else {
+                disabledRulesCleanupDelayMinutes = Long.parseLong(v.toString());
+            }
+        } catch (Exception e) {
+            logger.warn("Invalid configuration for {}: {} - using default 30 minutes",
+                    DISABLED_RULES_CLEANUP_DELAY_PROP, v);
+            disabledRulesCleanupDelayMinutes = 30L;
+        }
+    }
+
    private boolean mustTrigger(Rule r) {
        for (Trigger t : r.getTriggers()) {
            if (SystemTriggerHandler.STARTLEVEL_MODULE_TYPE_ID.equals(t.getTypeUID())) {