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 f75951052 fix(studio): persist cleared optional instance and registry
fields (#4466)
f75951052 is described below
commit f75951052c4ba3290ce4459ef032f6d48a27e0fd
Author: 0 <[email protected]>
AuthorDate: Mon Sep 21 12:19:36 2026 +0800
fix(studio): persist cleared optional instance and registry fields (#4466)
Both updates replace every editable field of the row, but MyBatis-Plus
updateById omits null entity fields, so clearing an optional field silently
kept
its previous value: an instance's admin credential reference survived being
removed, and a NameServer registry entry kept an omitted k8s namespace, k8s
id or
description.
Assign those columns explicitly when the request omits them —
MybatisPlusInstanceRepository.save for the credential reference, and a new
NameserverRegistryService.clearOmittedOptionalColumns for the three registry
columns. Both paths carry a red/green test asserting the column is written
NULL.
Maintainer edit on top of the contribution: the two new test methods in
MybatisPlusInstanceRepositoryTest were renamed to end with Test, matching
the
project convention and the sibling methods this PR added to
NameserverRegistryServiceTest.
---
.../nameserver/NameserverRegistryService.java | 29 ++++++++++
.../instance/MybatisPlusInstanceRepository.java | 9 ++++
.../nameserver/NameserverRegistryServiceTest.java | 63 ++++++++++++++++++++++
.../MybatisPlusInstanceRepositoryTest.java | 30 +++++++++++
4 files changed, 131 insertions(+)
diff --git
a/server/src/main/java/org/apache/rocketmq/studio/cluster/nameserver/NameserverRegistryService.java
b/server/src/main/java/org/apache/rocketmq/studio/cluster/nameserver/NameserverRegistryService.java
index 615499813..908cb4095 100644
---
a/server/src/main/java/org/apache/rocketmq/studio/cluster/nameserver/NameserverRegistryService.java
+++
b/server/src/main/java/org/apache/rocketmq/studio/cluster/nameserver/NameserverRegistryService.java
@@ -17,6 +17,7 @@
package org.apache.rocketmq.studio.cluster.nameserver;
import com.baomidou.mybatisplus.core.conditions.query.QueryWrapper;
+import com.baomidou.mybatisplus.core.conditions.update.UpdateWrapper;
import lombok.RequiredArgsConstructor;
import org.apache.rocketmq.studio.common.exception.BusinessException;
import org.apache.rocketmq.studio.persistence.entity.RmqNameserver;
@@ -90,6 +91,7 @@ public class NameserverRegistryService {
// The unique index is the final guard against concurrent rename
collisions.
throw duplicateName(name);
}
+ clearOmittedOptionalColumns(entity);
RmqNameserver stored = nameserverMapper.selectById(entity.getId());
if (stored == null) {
// The row vanished between the update and the reload; do not
convert null to a VO.
@@ -141,4 +143,31 @@ public class NameserverRegistryService {
.gmtModified(entity.getGmtModified())
.build();
}
+
+ /**
+ * The registry update replaces every editable field of the entry, but
MyBatis-Plus
+ * {@code updateById} omits null entity fields. An omitted k8s namespace,
k8s id or
+ * description would therefore silently keep its previous value even
though the request
+ * submitted no value for it; assign those columns explicitly so the
stored entry matches
+ * what the request asked for.
+ */
+ private void clearOmittedOptionalColumns(RmqNameserver entity) {
+ UpdateWrapper<RmqNameserver> cleared = new UpdateWrapper<>();
+ boolean anyCleared = false;
+ if (entity.getK8sNamespace() == null) {
+ cleared.set("k8s_namespace", null);
+ anyCleared = true;
+ }
+ if (entity.getK8sId() == null) {
+ cleared.set("k8s_id", null);
+ anyCleared = true;
+ }
+ if (entity.getDescription() == null) {
+ cleared.set("description", null);
+ anyCleared = true;
+ }
+ if (anyCleared) {
+ nameserverMapper.update(null, cleared.eq("id", entity.getId()));
+ }
+ }
}
diff --git
a/server/src/main/java/org/apache/rocketmq/studio/instance/MybatisPlusInstanceRepository.java
b/server/src/main/java/org/apache/rocketmq/studio/instance/MybatisPlusInstanceRepository.java
index 63b3e4d05..f63f9b4e7 100644
---
a/server/src/main/java/org/apache/rocketmq/studio/instance/MybatisPlusInstanceRepository.java
+++
b/server/src/main/java/org/apache/rocketmq/studio/instance/MybatisPlusInstanceRepository.java
@@ -18,6 +18,7 @@
package org.apache.rocketmq.studio.instance;
import com.baomidou.mybatisplus.core.conditions.query.QueryWrapper;
+import com.baomidou.mybatisplus.core.conditions.update.UpdateWrapper;
import org.apache.rocketmq.studio.common.domain.enums.InstanceType;
import org.apache.rocketmq.studio.common.domain.enums.InstanceVendor;
import org.apache.rocketmq.studio.common.exception.BusinessException;
@@ -123,6 +124,14 @@ public class MybatisPlusInstanceRepository implements
InstanceRepository {
throw new BusinessException(409,
"Instance update was not applied: " + entity.getId());
}
+ if (instance.getAdminCredentialRef() == null) {
+ // updateById omits null entity fields, so a cleared reference
has to be
+ // assigned explicitly; otherwise the stored reference
survives an update that
+ // removed it.
+ instanceMapper.update(null, new UpdateWrapper<RmqInstance>()
+ .eq("id", entity.getId())
+ .set("admin_credential_ref", null));
+ }
} else {
instanceMapper.insert(entity);
instance.setId(entity.getId());
diff --git
a/server/src/test/java/org/apache/rocketmq/studio/cluster/nameserver/NameserverRegistryServiceTest.java
b/server/src/test/java/org/apache/rocketmq/studio/cluster/nameserver/NameserverRegistryServiceTest.java
index e253677b1..29146acf5 100644
---
a/server/src/test/java/org/apache/rocketmq/studio/cluster/nameserver/NameserverRegistryServiceTest.java
+++
b/server/src/test/java/org/apache/rocketmq/studio/cluster/nameserver/NameserverRegistryServiceTest.java
@@ -16,6 +16,7 @@
*/
package org.apache.rocketmq.studio.cluster.nameserver;
+import com.baomidou.mybatisplus.core.conditions.update.UpdateWrapper;
import org.apache.rocketmq.studio.common.exception.BusinessException;
import org.apache.rocketmq.studio.persistence.entity.RmqNameserver;
import org.apache.rocketmq.studio.persistence.mapper.RmqNameserverMapper;
@@ -34,6 +35,7 @@ import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyLong;
+import static org.mockito.ArgumentMatchers.isNull;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -339,6 +341,67 @@ class NameserverRegistryServiceTest {
.hasMessageContaining("deleted concurrently");
}
+ @Test
+ void updateShouldClearOptionalColumnsThatTheRequestOmitsTest() {
+ // The Studio form submits a blanked optional field as an absent one,
and updateById
+ // skips null entity fields, so the cleared columns must be assigned
explicitly.
+ RmqNameserver existing = new RmqNameserver();
+ existing.setId(1L);
+ existing.setName("rocketmq1");
+ existing.setNamesrvAddr("rocketmq1-nameserver.svc:9876");
+ existing.setK8sNamespace("rocketmq1");
+ existing.setK8sId("k8s-1");
+ existing.setDescription("community chart cluster");
+
when(nameserverMapper.selectById(1L)).thenReturn(existing).thenReturn(existing);
+ when(nameserverMapper.selectCount(any())).thenReturn(0L);
+
when(nameserverMapper.updateById(any(RmqNameserver.class))).thenReturn(1);
+ when(nameserverMapper.update(isNull(),
any(UpdateWrapper.class))).thenReturn(1);
+
+ NameserverRegistryVO updated =
service.update(UpdateNameserverRegistryDTO.builder()
+ .id(1L)
+ .name("rocketmq1")
+ .namesrvAddr("rocketmq1-nameserver.svc:9876")
+ .build());
+
+ @SuppressWarnings("rawtypes")
+ ArgumentCaptor<UpdateWrapper> captor =
ArgumentCaptor.forClass(UpdateWrapper.class);
+ verify(nameserverMapper).update(isNull(), captor.capture());
+ assertThat(captor.getValue().getSqlSet())
+ .contains("k8s_namespace")
+ .contains("k8s_id")
+ .contains("description");
+
assertThat(captor.getValue().getParamNameValuePairs()).containsValue(null);
+ assertThat(existing.getK8sNamespace()).isNull();
+ assertThat(existing.getK8sId()).isNull();
+ assertThat(updated.getDescription()).isNull();
+ }
+
+ @Test
+ void updateShouldWriteOptionalColumnsSubmittedByTheRequestTest() {
+ RmqNameserver existing = new RmqNameserver();
+ existing.setId(1L);
+ existing.setK8sNamespace("old-namespace");
+ existing.setK8sId("old-k8s");
+ existing.setDescription("old description");
+
when(nameserverMapper.selectById(1L)).thenReturn(existing).thenReturn(existing);
+ when(nameserverMapper.selectCount(any())).thenReturn(0L);
+
when(nameserverMapper.updateById(any(RmqNameserver.class))).thenReturn(1);
+
+ service.update(UpdateNameserverRegistryDTO.builder()
+ .id(1L)
+ .name("rocketmq1")
+ .namesrvAddr("x:9876")
+ .k8sNamespace("new-namespace")
+ .k8sId("new-k8s")
+ .description("new description")
+ .build());
+
+ verify(nameserverMapper, never()).update(any(), any());
+ assertThat(existing.getK8sNamespace()).isEqualTo("new-namespace");
+ assertThat(existing.getK8sId()).isEqualTo("new-k8s");
+ assertThat(existing.getDescription()).isEqualTo("new description");
+ }
+
@Test
void updateShouldThrowWhenEntryVanishesBeforeReloadTest() {
RmqNameserver existing = new RmqNameserver();
diff --git
a/server/src/test/java/org/apache/rocketmq/studio/instance/MybatisPlusInstanceRepositoryTest.java
b/server/src/test/java/org/apache/rocketmq/studio/instance/MybatisPlusInstanceRepositoryTest.java
index 40cbe0f14..c01d64b74 100644
---
a/server/src/test/java/org/apache/rocketmq/studio/instance/MybatisPlusInstanceRepositoryTest.java
+++
b/server/src/test/java/org/apache/rocketmq/studio/instance/MybatisPlusInstanceRepositoryTest.java
@@ -18,6 +18,7 @@
package org.apache.rocketmq.studio.instance;
import com.baomidou.mybatisplus.core.conditions.query.QueryWrapper;
+import com.baomidou.mybatisplus.core.conditions.update.UpdateWrapper;
import org.apache.rocketmq.studio.common.domain.enums.InstanceType;
import org.apache.rocketmq.studio.common.domain.enums.InstanceVendor;
import org.apache.rocketmq.studio.common.exception.BusinessException;
@@ -39,6 +40,7 @@ import java.util.Optional;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.isNull;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoInteractions;
@@ -236,6 +238,34 @@ class MybatisPlusInstanceRepositoryTest {
assertThat(repository.deleteById(2L)).isFalse();
}
+ @Test
+ void saveShouldClearTheAdminCredentialRefWhenTheUpdateRemovesItTest() {
+ // updateById omits null entity fields, so a cleared reference has to
be assigned
+ // explicitly or the stored value survives the update.
+ InstanceVO vo = vo(5L, "instance-proxy-2", InstanceType.PROXY_CLUSTER);
+ vo.setAdminCredentialRef(null);
+ when(instanceMapper.updateById(any(RmqInstance.class))).thenReturn(1);
+ when(instanceMapper.update(isNull(),
any(UpdateWrapper.class))).thenReturn(1);
+
+ repository.save(vo);
+
+ @SuppressWarnings("rawtypes")
+ ArgumentCaptor<UpdateWrapper> captor =
ArgumentCaptor.forClass(UpdateWrapper.class);
+ verify(instanceMapper).update(isNull(), captor.capture());
+
assertThat(captor.getValue().getSqlSet()).contains("admin_credential_ref");
+
assertThat(captor.getValue().getParamNameValuePairs()).containsValue(null);
+ }
+
+ @Test
+ void saveShouldKeepTheAdminCredentialRefWhenTheUpdateCarriesOneTest() {
+ InstanceVO vo = vo(5L, "instance-proxy-2", InstanceType.PROXY_CLUSTER);
+ when(instanceMapper.updateById(any(RmqInstance.class))).thenReturn(1);
+
+ repository.save(vo);
+
+ verify(instanceMapper, never()).update(any(), any());
+ }
+
private RmqInstance entity(Long id, String name, InstanceType type) {
RmqInstance entity = new RmqInstance();
entity.setId(id);
