(rocketmq-dashboard) branch rocketmq-studio updated: fix(cluster): return defensive copies and atomically update cluster config (#1140)

[lizhimin]

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 d84b330c fix(cluster): return defensive copies and atomically update 
cluster config (#1140)
d84b330c is described below

commit d84b330c20291abbb50c4e25499e73dfdd63160a
Author: yyqdbngt <btl...@gmail.com>
AuthorDate: Fri Aug 7 15:27:12 2026 +0800

    fix(cluster): return defensive copies and atomically update cluster config 
(#1140)
    
    Co-authored-by: yyqdbngt <300715189+yyqdb...@users.noreply.github.com>
---
 .../cluster/broker/ClusterRepositoryImpl.java      | 45 +++++++++++++++++++---
 .../cluster/broker/ClusterRepositoryImplTest.java  | 14 +++++++
 2 files changed, 53 insertions(+), 6 deletions(-)

diff --git 
a/server/src/main/java/org/apache/rocketmq/studio/cluster/broker/ClusterRepositoryImpl.java
 
b/server/src/main/java/org/apache/rocketmq/studio/cluster/broker/ClusterRepositoryImpl.java
index e4252219..ea33574e 100644
--- 
a/server/src/main/java/org/apache/rocketmq/studio/cluster/broker/ClusterRepositoryImpl.java
+++ 
b/server/src/main/java/org/apache/rocketmq/studio/cluster/broker/ClusterRepositoryImpl.java
@@ -29,6 +29,7 @@ import org.springframework.beans.factory.annotation.Value;
 import org.springframework.stereotype.Repository;
 
 import java.time.LocalDateTime;
+import java.util.ArrayList;
 import java.util.Comparator;
 import java.util.List;
 import java.util.Map;
@@ -51,6 +52,7 @@ public class ClusterRepositoryImpl implements 
ClusterRepository {
     @Override
     public List<ClusterVO> findAll() {
         return store.values().stream()
+                .map(this::defensiveCopy)
                 .sorted(Comparator
                         .comparing(ClusterVO::getName, 
Comparator.nullsLast(String::compareToIgnoreCase))
                         .thenComparing(ClusterVO::getId, 
Comparator.nullsLast(String::compareTo)))
@@ -59,17 +61,48 @@ public class ClusterRepositoryImpl implements 
ClusterRepository {
 
     @Override
     public Optional<ClusterVO> findById(String id) {
-        return Optional.ofNullable(store.get(id));
+        return Optional.ofNullable(store.get(id)).map(this::defensiveCopy);
     }
 
     @Override
     public void updateConfig(String clusterId, ClusterConfigVO config) {
-        ClusterVO cluster = store.get(clusterId);
-        if (cluster != null) {
-            cluster.setConfig(config);
-            cluster.setUpdatedAt(LocalDateTime.now());
+        // Atomically replace the stored cluster with a fresh copy so 
concurrent readers never
+        // observe a partially updated snapshot (new config, old updatedAt).
+        store.computeIfPresent(clusterId, (id, cluster) -> {
+            ClusterVO updated = defensiveCopy(cluster);
+            updated.setConfig(config);
+            updated.setUpdatedAt(LocalDateTime.now());
             log.info("Config updated for cluster: {}", clusterId);
-        }
+            return updated;
+        });
+    }
+
+    /**
+     * Returns an independent copy so callers cannot mutate the cached cluster 
and concurrent
+     * readers never share a partially-updated object. Nested lists are copied 
as immutable lists.
+     */
+    private ClusterVO defensiveCopy(ClusterVO cluster) {
+        ClusterVO copy = ClusterVO.builder()
+                .name(cluster.getName())
+                .nsClusterName(cluster.getNsClusterName())
+                .type(cluster.getType())
+                .endpoint(cluster.getEndpoint())
+                .status(cluster.getStatus())
+                .version(cluster.getVersion())
+                // new ArrayList forces a fresh list even when the source is 
already immutable
+                // (List.copyOf would return the source reference for 
immutable inputs).
+                .brokers(cluster.getBrokers() == null ? null : new 
ArrayList<>(cluster.getBrokers()))
+                .proxies(cluster.getProxies() == null ? null : new 
ArrayList<>(cluster.getProxies()))
+                .nameServers(cluster.getNameServers() == null ? null : new 
ArrayList<>(cluster.getNameServers()))
+                .config(cluster.getConfig())
+                .topicCount(cluster.getTopicCount())
+                .groupCount(cluster.getGroupCount())
+                .tpsHistory(cluster.getTpsHistory() == null ? null : new 
ArrayList<>(cluster.getTpsHistory()))
+                .build();
+        copy.setId(cluster.getId());
+        copy.setCreatedAt(cluster.getCreatedAt());
+        copy.setUpdatedAt(cluster.getUpdatedAt());
+        return copy;
     }
 
     private void initStubData() {
diff --git 
a/server/src/test/java/org/apache/rocketmq/studio/cluster/broker/ClusterRepositoryImplTest.java
 
b/server/src/test/java/org/apache/rocketmq/studio/cluster/broker/ClusterRepositoryImplTest.java
index ac074a44..8077addb 100644
--- 
a/server/src/test/java/org/apache/rocketmq/studio/cluster/broker/ClusterRepositoryImplTest.java
+++ 
b/server/src/test/java/org/apache/rocketmq/studio/cluster/broker/ClusterRepositoryImplTest.java
@@ -40,4 +40,18 @@ class ClusterRepositoryImplTest {
 
         assertThat(repository.findAll()).isEmpty();
     }
+
+    @Test
+    void findByIdShouldReturnIndependentCopy() {
+        ClusterRepositoryImpl repository = new ClusterRepositoryImpl(true);
+
+        ClusterVO first = repository.findById("cluster-001").orElseThrow();
+        first.setName("mutated");
+
+        ClusterVO second = repository.findById("cluster-001").orElseThrow();
+
+        // Mutating the returned copy must not affect the cached cluster.
+        assertThat(second.getName()).isEqualTo("rmq-cluster-prod");
+        assertThat(first.getBrokers()).isNotSameAs(second.getBrokers());
+    }
 }