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 6ef2f2d85 fix(alert): persist cleared optional columns when a rule
update omits them (#4784)
6ef2f2d85 is described below
commit 6ef2f2d85d2984cebc0faf686c2aef20b2f959a8
Author: 烤化の初雪 <[email protected]>
AuthorDate: Thu Sep 24 18:13:39 2026 +0800
fix(alert): persist cleared optional columns when a rule update omits them
(#4784)
test(alert): pin that a fully-loaded rule update skips the clear pass
The clear pass only runs when a column would otherwise be silently
skipped. Lock the other half of that contract: an update carrying every
optional value issues updateById and nothing else, so the
fully-loaded callers (toggle, bulk toggle, import) are provably
unaffected.
fix(alert): persist a cleared optional column when an alert-rule update
omits it
The rule update replaces every editable field of the rule, but
MybatisPlusAlertRepository.replaceRule built the entity and relied on
updateById, whose NOT_NULL strategy omits null fields from the SET
clause. An update body that omits an optional value therefore passed
validation, the service echoed the submitted VO (field cleared), and
the stored rule silently kept its previous value - the scope filters,
severity, duration, channels, template and description could not be
cleared through the API. clearOmittedOptionalColumns now assigns those
columns explicitly, the same approach NameserverRegistryService uses
for its cleared registry columns (merged #4466). lastTriggered is not
an editable field (owned by markRuleTriggered) and keeps the
skip-on-null behaviour.
---
.../ops/alert/MybatisPlusAlertRepository.java | 69 ++++++++++++++++++-
.../ops/alert/MybatisPlusAlertRepositoryTest.java | 80 ++++++++++++++++++++++
2 files changed, 148 insertions(+), 1 deletion(-)
diff --git
a/server/src/main/java/org/apache/rocketmq/studio/ops/alert/MybatisPlusAlertRepository.java
b/server/src/main/java/org/apache/rocketmq/studio/ops/alert/MybatisPlusAlertRepository.java
index 4b904332b..6c1cd63ba 100644
---
a/server/src/main/java/org/apache/rocketmq/studio/ops/alert/MybatisPlusAlertRepository.java
+++
b/server/src/main/java/org/apache/rocketmq/studio/ops/alert/MybatisPlusAlertRepository.java
@@ -17,6 +17,7 @@
package org.apache.rocketmq.studio.ops.alert;
import com.baomidou.mybatisplus.core.conditions.query.QueryWrapper;
+import com.baomidou.mybatisplus.core.conditions.update.UpdateWrapper;
import com.baomidou.mybatisplus.extension.plugins.pagination.Page;
import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.ObjectMapper;
@@ -142,7 +143,73 @@ public class MybatisPlusAlertRepository implements
AlertRepository {
if (rule.getId() == null || ruleMapper.selectById(rule.getId()) ==
null) {
return false;
}
- return ruleMapper.updateById(toRuleEntity(rule)) > 0;
+ RmqAlertRule entity = toRuleEntity(rule);
+ boolean updated = ruleMapper.updateById(entity) > 0;
+ if (updated) {
+ clearOmittedOptionalColumns(entity);
+ }
+ return updated;
+ }
+
+ /**
+ * The rule update replaces every editable field of the rule, but
MyBatis-Plus
+ * {@code updateById} omits null entity fields. An omitted optional value
would therefore
+ * silently keep its previous stored value even though the request
submitted no value for
+ * it (and the update response echoes the cleared value); assign those
columns explicitly
+ * so the stored rule matches what the request asked for. {@code
lastTriggered} is not an
+ * editable field (it is owned by {@link #markRuleTriggered}) and keeps
the skip-on-null
+ * behaviour.
+ */
+ private void clearOmittedOptionalColumns(RmqAlertRule entity) {
+ UpdateWrapper<RmqAlertRule> cleared = new UpdateWrapper<>();
+ boolean anyCleared = false;
+ if (entity.getThresholdUnit() == null) {
+ cleared.set("threshold_unit", null);
+ anyCleared = true;
+ }
+ if (entity.getDuration() == null) {
+ cleared.set("duration", null);
+ anyCleared = true;
+ }
+ if (entity.getChannels() == null) {
+ cleared.set("channels", null);
+ anyCleared = true;
+ }
+ if (entity.getDescription() == null) {
+ cleared.set("description", null);
+ anyCleared = true;
+ }
+ if (entity.getBrokerName() == null) {
+ cleared.set("broker_name", null);
+ anyCleared = true;
+ }
+ if (entity.getClusterName() == null) {
+ cleared.set("cluster_name", null);
+ anyCleared = true;
+ }
+ if (entity.getSeverity() == null) {
+ cleared.set("severity", null);
+ anyCleared = true;
+ }
+ if (entity.getInstanceId() == null) {
+ cleared.set("instance_id", null);
+ anyCleared = true;
+ }
+ if (entity.getConsumerGroup() == null) {
+ cleared.set("consumer_group", null);
+ anyCleared = true;
+ }
+ if (entity.getTopic() == null) {
+ cleared.set("topic", null);
+ anyCleared = true;
+ }
+ if (entity.getNotificationTemplate() == null) {
+ cleared.set("notification_template", null);
+ anyCleared = true;
+ }
+ if (anyCleared) {
+ ruleMapper.update(null, cleared.eq("id", entity.getId()));
+ }
}
@Override
diff --git
a/server/src/test/java/org/apache/rocketmq/studio/ops/alert/MybatisPlusAlertRepositoryTest.java
b/server/src/test/java/org/apache/rocketmq/studio/ops/alert/MybatisPlusAlertRepositoryTest.java
index 852003124..f4c5ea2cd 100644
---
a/server/src/test/java/org/apache/rocketmq/studio/ops/alert/MybatisPlusAlertRepositoryTest.java
+++
b/server/src/test/java/org/apache/rocketmq/studio/ops/alert/MybatisPlusAlertRepositoryTest.java
@@ -18,6 +18,7 @@ package org.apache.rocketmq.studio.ops.alert;
import com.baomidou.mybatisplus.core.conditions.Wrapper;
import com.baomidou.mybatisplus.core.conditions.query.QueryWrapper;
+import com.baomidou.mybatisplus.core.conditions.update.UpdateWrapper;
import com.baomidou.mybatisplus.core.metadata.IPage;
import com.baomidou.mybatisplus.extension.plugins.pagination.Page;
import org.apache.rocketmq.studio.common.domain.PageResult;
@@ -43,6 +44,7 @@ import java.util.Optional;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.argThat;
+import static org.mockito.ArgumentMatchers.isNull;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -71,6 +73,84 @@ class MybatisPlusAlertRepositoryTest {
assertThat(repository.replaceRule(rule)).isFalse();
}
+ @Test
+ void replaceRuleShouldExplicitlyClearOmittedOptionalColumnsTest() {
+ // The update replaces every editable field, so a null optional field
on the submitted
+ // rule means "cleared". updateById omits null entity fields
(MyBatis-Plus NOT_NULL
+ // strategy), so the repository must assign those columns explicitly
or the stored
+ // values silently survive the update.
+ AlertRuleVO rule = AlertRuleVO.builder()
+ .id(3L)
+ .name("Lag")
+ .metric("consumer.lag.total")
+ .operator(">")
+ .threshold(100.0)
+ .domain(AlertDomain.BUSINESS)
+ .enabled(true)
+ .consecutiveSamples(1)
+ .reminderInterval("30m")
+ .build();
+
+ when(ruleMapper.selectById(3L)).thenReturn(new RmqAlertRule());
+ when(ruleMapper.updateById(any(RmqAlertRule.class))).thenReturn(1);
+
+ assertThat(repository.replaceRule(rule)).isTrue();
+
+ ArgumentCaptor<Wrapper<RmqAlertRule>> clearCaptor =
ArgumentCaptor.forClass(Wrapper.class);
+ verify(ruleMapper).update(isNull(), clearCaptor.capture());
+ UpdateWrapper<RmqAlertRule> cleared = (UpdateWrapper<RmqAlertRule>)
clearCaptor.getValue();
+ String sqlSet = cleared.getSqlSet();
+ // Every omitted optional column must be assigned NULL:
"column=#{ew.paramNameValuePairs.MPGENVALn}"
+ assertThat(sqlSet).isNotBlank();
+ for (String column : new String[] {
+ "threshold_unit", "duration", "channels", "description",
+ "broker_name", "cluster_name", "severity", "instance_id",
+ "consumer_group", "topic", "notification_template"}) {
+ assertThat(sqlSet).contains(column + "=");
+ }
+ // lastTriggered is not an editable field (owned by
markRuleTriggered), so it must
+ // keep the skip-on-null behaviour and stay out of the cleared
assignments.
+ assertThat(sqlSet).doesNotContain("last_triggered=");
+ assertThat(cleared.getParamNameValuePairs()).allSatisfy(
+ (key, value) -> assertThat(value).isNull());
+ }
+
+ @Test
+ void
replaceRuleShouldSkipTheClearPassWhenEveryOptionalFieldIsPresentTest() {
+ // A fully-loaded update (toggle, bulk toggle, import) must not issue
the extra clear
+ // statement at all: it would be redundant work per row and an empty
SET clause is
+ // exactly what MyBatis-Plus guards against.
+ AlertRuleVO rule = AlertRuleVO.builder()
+ .id(4L)
+ .name("Lag")
+ .metric("consumer.lag.total")
+ .operator(">")
+ .threshold(100.0)
+ .thresholdUnit("messages")
+ .duration("5m")
+ .channels(Arrays.asList("email"))
+ .description("checked")
+ .brokerName("broker-a")
+ .clusterName("cluster-a")
+ .severity("warning")
+ .instanceId("inst-1")
+ .consumerGroup("G1")
+ .topic("orders")
+ .notificationTemplate("body")
+ .domain(AlertDomain.BUSINESS)
+ .enabled(true)
+ .consecutiveSamples(1)
+ .reminderInterval("30m")
+ .build();
+
+ when(ruleMapper.selectById(4L)).thenReturn(new RmqAlertRule());
+ when(ruleMapper.updateById(any(RmqAlertRule.class))).thenReturn(1);
+
+ assertThat(repository.replaceRule(rule)).isTrue();
+
+ verify(ruleMapper, never()).update(isNull(), any(Wrapper.class));
+ }
+
@Test
void findRulePageShouldApplyFiltersOrderingAndDatabasePagination() {
RmqAlertRule entity = new RmqAlertRule();
