You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by li...@apache.org on 2026/10/19 10:13:57 UTC

(rocketmq-dashboard) branch rocketmq-studio updated: fix(settings): keep webhooks when the marker echoes (#5724)

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 35fbcd3f8 fix(settings): keep webhooks when the marker echoes (#5724)
35fbcd3f8 is described below

commit 35fbcd3f8be355d43796b8b0f63e3f9cdbf26d0f
Author: 0 <19...@qq.com>
AuthorDate: Mon Oct 19 18:13:50 2026 +0800

    fix(settings): keep webhooks when the marker echoes (#5724)
    
    Verified against `rocketmq-studio` at 86aa49c34 before merging: 缺陷仍在。主干 `server/src/main/java/org/apache/rocketmq/studio/settings/SettingsService.java:113-123` 仍把两个 webhook 脱敏成 `REDACTED_NOTIFICATION_WEBHOOK`(常量 `:
    
    merge-tree clean against the current trunk and no outstanding must-fix items; merged as-is (squash) per the maintainer review of 2026-10-19.
---
 .../rocketmq/studio/settings/SettingsService.java  | 12 ++++++
 .../studio/settings/SettingsServiceTest.java       | 45 ++++++++++++++++++++++
 2 files changed, 57 insertions(+)

diff --git a/server/src/main/java/org/apache/rocketmq/studio/settings/SettingsService.java b/server/src/main/java/org/apache/rocketmq/studio/settings/SettingsService.java
index a9a603ae7..b332ec96e 100644
--- a/server/src/main/java/org/apache/rocketmq/studio/settings/SettingsService.java
+++ b/server/src/main/java/org/apache/rocketmq/studio/settings/SettingsService.java
@@ -159,6 +159,18 @@ public class SettingsService {
         } else if (!StringUtils.hasText(settings.getDingtalkSigningSecret()) && currentSettings != null) {
             settings.setDingtalkSigningSecret(currentSettings.getDingtalkSigningSecret());
         }
+        // Non-admin sessions read the webhooks through redactNotificationWebhooks and every
+        // console save echoes what it loaded, so the marker this service itself emitted must
+        // restore the stored URL instead of being persisted over it (issue #5004). A genuinely
+        // retyped URL — or a blank, which is how the form clears a webhook — still wins.
+        if (currentSettings != null) {
+            if (REDACTED_NOTIFICATION_WEBHOOK.equals(settings.getDingtalkWebhook())) {
+                settings.setDingtalkWebhook(currentSettings.getDingtalkWebhook());
+            }
+            if (REDACTED_NOTIFICATION_WEBHOOK.equals(settings.getSmsWebhook())) {
+                settings.setSmsWebhook(currentSettings.getSmsWebhook());
+            }
+        }
         if (currentSettings != null) {
             if (!StringUtils.hasText(settings.getLlmEngine())) {
                 settings.setLlmEngine(currentSettings.getLlmEngine());
diff --git a/server/src/test/java/org/apache/rocketmq/studio/settings/SettingsServiceTest.java b/server/src/test/java/org/apache/rocketmq/studio/settings/SettingsServiceTest.java
index 3344906bc..244569563 100644
--- a/server/src/test/java/org/apache/rocketmq/studio/settings/SettingsServiceTest.java
+++ b/server/src/test/java/org/apache/rocketmq/studio/settings/SettingsServiceTest.java
@@ -293,6 +293,51 @@ class SettingsServiceTest {
                 null, "General settings updated", "SUCCESS", null);
     }
 
+    @Test
+    void saveGeneralSettingsShouldKeepExistingWebhooksWhenTheRedactedMarkerIsEchoedTest() {
+        // A reader session reads the settings through the non-admin redaction and every save
+        // echoes what it loaded, so the write side must recognise the marker it emitted.
+        GeneralSettingsVO existing = GeneralSettingsVO.builder()
+                .dingtalkWebhook("https://oapi.dingtalk.com/robot/send?access_token=secret")
+                .smsWebhook("https://sms.example.test/notify")
+                .build();
+        GeneralSettingsVO update = GeneralSettingsVO.builder()
+                .theme("light")
+                .dingtalkWebhook("******")
+                .smsWebhook("******")
+                .build();
+        when(settingsRepository.loadGeneralSettings()).thenReturn(existing);
+
+        settingsService.saveGeneralSettings(update);
+
+        assertThat(update.getDingtalkWebhook())
+                .isEqualTo("https://oapi.dingtalk.com/robot/send?access_token=secret");
+        assertThat(update.getSmsWebhook()).isEqualTo("https://sms.example.test/notify");
+        verify(settingsRepository).saveGeneralSettings(update);
+    }
+
+    @Test
+    void saveGeneralSettingsShouldReplaceAWebhookThatWasActuallyRetypedTest() {
+        GeneralSettingsVO existing = GeneralSettingsVO.builder()
+                .dingtalkWebhook("https://oapi.dingtalk.com/robot/send?access_token=old")
+                .smsWebhook("https://sms.example.test/old")
+                .build();
+        GeneralSettingsVO update = GeneralSettingsVO.builder()
+                .theme("light")
+                .dingtalkWebhook("https://oapi.dingtalk.com/robot/send?access_token=new")
+                .smsWebhook("")
+                .build();
+        when(settingsRepository.loadGeneralSettings()).thenReturn(existing);
+
+        settingsService.saveGeneralSettings(update);
+
+        assertThat(update.getDingtalkWebhook())
+                .isEqualTo("https://oapi.dingtalk.com/robot/send?access_token=new");
+        // Clearing a webhook by blanking the field stays a legitimate edit.
+        assertThat(update.getSmsWebhook()).isEmpty();
+        verify(settingsRepository).saveGeneralSettings(update);
+    }
+
     @Test
     void listDataSourcesShouldReturnAllSourcesTest() {
         DataSourceVO ds1 = DataSourceVO.builder().key("ds-1").name("Production").type("rocketmq")