This is an automated email from the ASF dual-hosted git repository.
lizhimins pushed a commit to branch rocketmq-studio
in repository https://gitbox.apache.org/repos/asf/rocketmq-dashboard.git
The following commit(s) were added to refs/heads/rocketmq-studio by this push:
new 5e4c39b05 fix(alert): preserve rule state without orphaning disabled
rules (#5546)
5e4c39b05 is described below
commit 5e4c39b053c35a5598cc79ff331b8a54e649d7b1
Author: zmuxuny <[email protected]>
AuthorDate: Fri Oct 9 23:39:49 2026 -0700
fix(alert): preserve rule state without orphaning disabled rules (#5546)
Disabling a rule left its alert-state rows behind, so re-enabling it
resurrected stale state. Five of
the nine `deleteByRuleId` call sites - the ones on mutation paths - are now
guarded, while the four on
genuine deletion paths still clean up.
`AlertServiceTest` + `AlertRuleStateLifecycleTest`: 104 tests green, 0
checkstyle violations.
---
.../rocketmq/studio/ops/alert/AlertService.java | 25 +++-
.../ops/alert/AlertRuleStateLifecycleTest.java | 143 +++++++++++++++++++++
.../studio/ops/alert/AlertServiceTest.java | 28 +++-
3 files changed, 187 insertions(+), 9 deletions(-)
diff --git
a/server/src/main/java/org/apache/rocketmq/studio/ops/alert/AlertService.java
b/server/src/main/java/org/apache/rocketmq/studio/ops/alert/AlertService.java
index 4d8b7ef8c..0308397d4 100644
---
a/server/src/main/java/org/apache/rocketmq/studio/ops/alert/AlertService.java
+++
b/server/src/main/java/org/apache/rocketmq/studio/ops/alert/AlertService.java
@@ -199,12 +199,19 @@ public class AlertService {
Long id = rule.getId();
log.info("Updating alert rule: {}", id);
validateRuleId(id);
+ String previousSemanticFingerprint = alertRepository.findRuleById(id)
+ .map(AlertRuleSemanticFingerprint::of)
+ .orElse(null);
NativeAlertRulePolicy.validate(rule);
rejectDuplicateSemanticRule(rule, id);
if (!replaceRuleWithoutDuplicate(rule)) {
throw ruleNotFound(id);
}
- alertStateRepository.deleteByRuleId(id);
+ // Disabled rules are excluded from evaluation/reconciliation; their
runtime must be purged.
+ if (!rule.isEnabled() || previousSemanticFingerprint == null
+ ||
!previousSemanticFingerprint.equals(AlertRuleSemanticFingerprint.of(rule))) {
+ alertStateRepository.deleteByRuleId(id);
+ }
auditRule("UPDATE_ALERT_RULE", rule, null);
return rule;
}
@@ -282,7 +289,9 @@ public class AlertService {
if (!alertRepository.replaceRule(rule)) {
throw ruleNotFound(id);
}
- alertStateRepository.deleteByRuleId(id);
+ if (!enabled) {
+ alertStateRepository.deleteByRuleId(id);
+ }
auditRule("TOGGLE_ALERT_RULE", rule, "enabled=" + enabled);
return rule;
}
@@ -295,7 +304,9 @@ public class AlertService {
if (!alertRepository.replaceRule(rule)) {
throw ruleNotFound(id);
}
- alertStateRepository.deleteByRuleId(id);
+ if (!enabled) {
+ alertStateRepository.deleteByRuleId(id);
+ }
auditRule("TOGGLE_ALERT_RULE", rule, "enabled=" + enabled);
return rule;
}
@@ -344,7 +355,9 @@ public class AlertService {
failures.put(id, "Alert rule not found");
continue;
}
- alertStateRepository.deleteByRuleId(id);
+ if (!enabled) {
+ alertStateRepository.deleteByRuleId(id);
+ }
auditRule("TOGGLE_ALERT_RULE", rule, "enabled=" + enabled + ",
bulk=true");
succeeded.add(id);
updated.add(rule);
@@ -433,7 +446,9 @@ public class AlertService {
failures.put(id, "Alert rule not found");
continue;
}
- alertStateRepository.deleteByRuleId(id);
+ if (!rule.isEnabled()) {
+ alertStateRepository.deleteByRuleId(id);
+ }
auditRule(auditOperation, rule, auditDetail);
succeeded.add(id);
updated.add(rule);
diff --git
a/server/src/test/java/org/apache/rocketmq/studio/ops/alert/AlertRuleStateLifecycleTest.java
b/server/src/test/java/org/apache/rocketmq/studio/ops/alert/AlertRuleStateLifecycleTest.java
new file mode 100644
index 000000000..1c2bbeb03
--- /dev/null
+++
b/server/src/test/java/org/apache/rocketmq/studio/ops/alert/AlertRuleStateLifecycleTest.java
@@ -0,0 +1,143 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.rocketmq.studio.ops.alert;
+
+import org.apache.rocketmq.studio.audit.OperationAuditService;
+import org.apache.rocketmq.studio.cluster.metrics.MetricProfileService;
+import org.apache.rocketmq.studio.cluster.metrics.PrometheusProperties;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+import java.time.LocalDateTime;
+import java.util.List;
+import java.util.Optional;
+import java.util.concurrent.atomic.AtomicReference;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+class AlertRuleStateLifecycleTest {
+ private final AlertRepository rules = mock(AlertRepository.class);
+ private final AlertStateRepository states =
mock(AlertStateRepository.class);
+ private final AtomicReference<AlertRuleVO> storedRule = new
AtomicReference<>();
+ private final AtomicReference<AlertRuleRuntimeVO> runtime = new
AtomicReference<>();
+ private AlertService service;
+
+ @BeforeEach
+ void setUpTest() {
+ service = new AlertService(rules, states, new AlertRuleAssetService(),
+ mock(OperationAuditService.class), new
MetricProfileService(new PrometheusProperties()));
+ storedRule.set(rule(true, 100, "Lag"));
+
runtime.set(AlertRuleRuntimeVO.builder().ruleId(4L).fingerprint("active-episode")
+
.status(AlertStateStatus.FIRING).lastNotifiedAt(LocalDateTime.of(2026, 9, 1,
12, 0))
+ .nextReminderAt(LocalDateTime.of(2026, 9, 1, 12, 5)).build());
+ when(rules.findRuleById(4L)).thenAnswer(invocation ->
Optional.ofNullable(storedRule.get()));
+ when(rules.findRulesByIds(List.of(4L))).thenAnswer(invocation ->
List.of(storedRule.get()));
+ when(rules.findAllRules()).thenAnswer(invocation -> storedRule.get()
== null
+ ? List.of() : List.of(storedRule.get()));
+ when(rules.replaceRule(any())).thenAnswer(invocation -> {
+ storedRule.set(invocation.getArgument(0));
+ return true;
+ });
+ when(rules.deleteRule(4L)).thenAnswer(invocation -> {
+ storedRule.set(null);
+ return true;
+ });
+ // Keep runtime visible even for disabled rules, like the real runtime
query.
+ when(states.findRuntimeByRuleIds(any())).thenAnswer(invocation ->
runtime.get() == null
+ ? List.of() : List.of(runtime.get()));
+ doAnswer(invocation -> {
+ runtime.set(null);
+ return null;
+ }).when(states).deleteByRuleId(4L);
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"toggle", "domainToggle", "bulk", "domainBulk",
"edit", "domainEdit"})
+ void disablingRulePurgesFiringRuntimeAndReminderTimesTest(String path) {
+
assertThat(service.listRuleRuntime(AlertDomain.BUSINESS)).singleElement()
+
.extracting(AlertRuleRuntimeVO::getStatus).isEqualTo(AlertStateStatus.FIRING);
+ mutate(path, false, 100, "Lag");
+ assertThat(storedRule.get().isEnabled()).isFalse();
+ assertThat(service.listRuleRuntime(AlertDomain.BUSINESS)).isEmpty();
+ verify(states).deleteByRuleId(4L);
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"toggle", "domainToggle", "bulk", "domainBulk",
"edit", "domainEdit"})
+ void enablingRulePreservesExistingRuntimeTest(String path) {
+ storedRule.get().setEnabled(false);
+ AlertRuleRuntimeVO before = runtime.get();
+ mutate(path, true, 100, "Lag");
+
assertThat(service.listRuleRuntime(AlertDomain.BUSINESS)).containsExactly(before);
+ verify(states, never()).deleteByRuleId(4L);
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"edit", "domainEdit"})
+ void semanticChangePurgesRuntimeTest(String path) {
+ mutate(path, true, 200, "Lag");
+ assertThat(service.listRuleRuntime(AlertDomain.BUSINESS)).isEmpty();
+ verify(states).deleteByRuleId(4L);
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"edit", "domainEdit"})
+ void presentationOnlyEditPreservesRuntimeTest(String path) {
+ AlertRuleRuntimeVO before = runtime.get();
+ mutate(path, true, 100, "Renamed");
+
assertThat(service.listRuleRuntime(AlertDomain.BUSINESS)).containsExactly(before);
+ verify(states, never()).deleteByRuleId(4L);
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"delete", "domainDelete", "bulkDelete",
"domainBulkDelete"})
+ void deletingRuleStillPurgesRuntimeTest(String path) {
+ switch (path) {
+ case "delete" -> service.deleteRule(4L);
+ case "domainDelete" -> service.deleteRule(AlertDomain.BUSINESS,
4L);
+ case "bulkDelete" -> service.bulkDeleteRules(List.of(4L));
+ case "domainBulkDelete" ->
service.bulkDeleteRules(AlertDomain.BUSINESS, List.of(4L));
+ default -> throw new IllegalArgumentException(path);
+ }
+ assertThat(runtime.get()).isNull();
+ verify(states).deleteByRuleId(4L);
+ }
+
+ private void mutate(String path, boolean enabled, double threshold, String
name) {
+ switch (path) {
+ case "toggle" -> service.toggleRule(4L, enabled);
+ case "domainToggle" -> service.toggleRule(AlertDomain.BUSINESS,
4L, enabled);
+ case "bulk" -> service.bulkToggleRules(List.of(4L), enabled);
+ case "domainBulk" -> service.bulkToggleRules(AlertDomain.BUSINESS,
List.of(4L), enabled);
+ case "edit" -> service.updateRule(rule(enabled, threshold, name));
+ case "domainEdit" -> service.updateRule(AlertDomain.BUSINESS,
rule(enabled, threshold, name));
+ default -> throw new IllegalArgumentException(path);
+ }
+ }
+
+ private static AlertRuleVO rule(boolean enabled, double threshold, String
name) {
+ return
AlertRuleVO.builder().id(4L).name(name).domain(AlertDomain.BUSINESS)
+
.metric("consumer.lag.total").operator(">").threshold(threshold)
+ .instanceId("local").enabled(enabled).build();
+ }
+}
diff --git
a/server/src/test/java/org/apache/rocketmq/studio/ops/alert/AlertServiceTest.java
b/server/src/test/java/org/apache/rocketmq/studio/ops/alert/AlertServiceTest.java
index 02a32d34b..b128cf2f7 100644
---
a/server/src/test/java/org/apache/rocketmq/studio/ops/alert/AlertServiceTest.java
+++
b/server/src/test/java/org/apache/rocketmq/studio/ops/alert/AlertServiceTest.java
@@ -184,13 +184,16 @@ class AlertServiceTest {
assertThat(updated.isEnabled()).isTrue();
verify(alertRepository).replaceRule(cluster);
verify(alertRepository, never()).findAllRules();
- verify(alertStateRepository).deleteByRuleId(2L);
+ verify(alertStateRepository, never()).deleteByRuleId(2L);
}
@Test
void updatingRuleShouldResetItsPreviousEvaluationStateTest() {
+ AlertRuleVO existing =
AlertRuleVO.builder().id(4L).name("Lag").metric("consumer.lag.total")
+ .operator(">").threshold(50).instanceId("local").build();
AlertRuleVO rule =
AlertRuleVO.builder().id(4L).name("Lag").metric("consumer.lag.total")
.operator(">").threshold(100).instanceId("local").build();
+
when(alertRepository.findRuleById(4L)).thenReturn(Optional.of(existing));
when(alertRepository.replaceRule(rule)).thenReturn(true);
alertService.updateRule(rule);
@@ -198,6 +201,23 @@ class AlertServiceTest {
verify(alertStateRepository).deleteByRuleId(4L);
}
+ @Test
+ void updatingOnlyNonSemanticFieldsShouldPreserveEvaluationStateTest() {
+ AlertRuleVO existing = AlertRuleVO.builder().id(4L).name("Lag old")
+ .metric("consumer.lag.total").operator(">").threshold(100)
+ .instanceId("local").enabled(true).severity("warning").build();
+ AlertRuleVO updated = AlertRuleVO.builder().id(4L).name("Lag renamed")
+ .metric("consumer.lag.total").operator(">").threshold(100)
+ .instanceId("local").enabled(true).severity("critical")
+ .channels(List.of("dingtalk")).build();
+
when(alertRepository.findRuleById(4L)).thenReturn(Optional.of(existing));
+ when(alertRepository.replaceRule(updated)).thenReturn(true);
+
+ alertService.updateRule(updated);
+
+ verify(alertStateRepository, never()).deleteByRuleId(4L);
+ }
+
@Test
void deletingRuleShouldRemoveItsStoredEvaluationStateTest() {
when(alertRepository.deleteRule(4L)).thenReturn(true);
@@ -1212,18 +1232,18 @@ class AlertServiceTest {
.extracting(AlertRuleVO::isEnabled).isEqualTo(true);
verify(alertRepository).replaceRule(rule);
verify(alertRepository, never()).findAllRules();
- verify(alertStateRepository).deleteByRuleId(1L);
+ verify(alertStateRepository, never()).deleteByRuleId(1L);
}
@Test
- void domainBulkOperationsShouldResetEachMutatedRuleStateTest() {
+ void domainBulkDisableAndDeleteShouldResetEachMutatedRuleStateTest() {
AlertRuleVO rule = AlertRuleVO.builder().id(1L).name("Broker
unavailable")
.domain(AlertDomain.CLUSTER).enabled(false).build();
when(alertRepository.findRulesByIds(List.of(1L))).thenReturn(List.of(rule));
when(alertRepository.replaceRule(rule)).thenReturn(true);
when(alertRepository.deleteRule(1L)).thenReturn(true);
- alertService.bulkToggleRules(AlertDomain.CLUSTER, List.of(1L), true);
+ alertService.bulkToggleRules(AlertDomain.CLUSTER, List.of(1L), false);
alertService.bulkDeleteRules(AlertDomain.CLUSTER, List.of(1L));
verify(alertStateRepository,
org.mockito.Mockito.times(2)).deleteByRuleId(1L);
