From 2c1236d6bbf0440204cb55855ff12af87f876d3c Mon Sep 17 00:00:00 2001 From: liuhy Date: Fri, 24 Jul 2026 07:51:51 -0700 Subject: [PATCH] fix: validate nameserver operation requests --- .../nameserver/CreateNameServerDTO.java | 5 + .../nameserver/DeleteNameServerDTO.java | 4 + .../nameserver/NameServerController.java | 11 +- .../nameserver/RestartNameServerDTO.java | 4 + .../nameserver/UpdateNameServerDTO.java | 5 + .../nameserver/UpgradeNameServerDTO.java | 6 + .../nameserver/NameServerControllerTest.java | 159 ++++++++++++++++++ 7 files changed, 189 insertions(+), 5 deletions(-) create mode 100644 server/src/test/java/com/rocketmq/studio/cluster/nameserver/NameServerControllerTest.java diff --git a/server/src/main/java/com/rocketmq/studio/cluster/nameserver/CreateNameServerDTO.java b/server/src/main/java/com/rocketmq/studio/cluster/nameserver/CreateNameServerDTO.java index 234e4f34..5dde3292 100644 --- a/server/src/main/java/com/rocketmq/studio/cluster/nameserver/CreateNameServerDTO.java +++ b/server/src/main/java/com/rocketmq/studio/cluster/nameserver/CreateNameServerDTO.java @@ -16,6 +16,7 @@ */ package com.rocketmq.studio.cluster.nameserver; +import jakarta.validation.constraints.NotBlank; import lombok.AllArgsConstructor; import lombok.Builder; import lombok.Data; @@ -26,7 +27,11 @@ @NoArgsConstructor @AllArgsConstructor public class CreateNameServerDTO { + @NotBlank(message = "clusterId is required") private String clusterId; + + @NotBlank(message = "addr is required") private String addr; + private String version; } diff --git a/server/src/main/java/com/rocketmq/studio/cluster/nameserver/DeleteNameServerDTO.java b/server/src/main/java/com/rocketmq/studio/cluster/nameserver/DeleteNameServerDTO.java index dd8c8d40..77f1db1b 100644 --- a/server/src/main/java/com/rocketmq/studio/cluster/nameserver/DeleteNameServerDTO.java +++ b/server/src/main/java/com/rocketmq/studio/cluster/nameserver/DeleteNameServerDTO.java @@ -16,6 +16,7 @@ */ package com.rocketmq.studio.cluster.nameserver; +import jakarta.validation.constraints.NotBlank; import lombok.AllArgsConstructor; import lombok.Builder; import lombok.Data; @@ -26,6 +27,9 @@ @NoArgsConstructor @AllArgsConstructor public class DeleteNameServerDTO { + @NotBlank(message = "clusterId is required") private String clusterId; + + @NotBlank(message = "addr is required") private String addr; } diff --git a/server/src/main/java/com/rocketmq/studio/cluster/nameserver/NameServerController.java b/server/src/main/java/com/rocketmq/studio/cluster/nameserver/NameServerController.java index a25e0211..61705739 100644 --- a/server/src/main/java/com/rocketmq/studio/cluster/nameserver/NameServerController.java +++ b/server/src/main/java/com/rocketmq/studio/cluster/nameserver/NameServerController.java @@ -19,6 +19,7 @@ import com.rocketmq.studio.cluster.broker.ClusterService; import com.rocketmq.studio.common.domain.Result; +import jakarta.validation.Valid; import lombok.RequiredArgsConstructor; import org.springframework.web.bind.annotation.PostMapping; import org.springframework.web.bind.annotation.RequestBody; @@ -35,30 +36,30 @@ public class NameServerController { private final ClusterService clusterService; @PostMapping("/create") - public Result createNameServer(@RequestBody CreateNameServerDTO command) { + public Result createNameServer(@Valid @RequestBody CreateNameServerDTO command) { return Result.ok(clusterService.createNameServer(command)); } @PostMapping("/update") - public Result updateNameServer(@RequestBody UpdateNameServerDTO command) { + public Result updateNameServer(@Valid @RequestBody UpdateNameServerDTO command) { clusterService.updateNameServer(command); return Result.ok(); } @PostMapping("/restart") - public Result> restartNameServer(@RequestBody RestartNameServerDTO command) { + public Result> restartNameServer(@Valid @RequestBody RestartNameServerDTO command) { boolean success = clusterService.restartNameServer(command); return Result.ok(Map.of("success", success)); } @PostMapping("/upgrade") - public Result> upgradeNameServer(@RequestBody UpgradeNameServerDTO command) { + public Result> upgradeNameServer(@Valid @RequestBody UpgradeNameServerDTO command) { boolean success = clusterService.upgradeNameServer(command); return Result.ok(Map.of("success", success)); } @PostMapping("/delete") - public Result> deleteNameServer(@RequestBody DeleteNameServerDTO command) { + public Result> deleteNameServer(@Valid @RequestBody DeleteNameServerDTO command) { boolean success = clusterService.deleteNameServer(command); return Result.ok(Map.of("success", success)); } diff --git a/server/src/main/java/com/rocketmq/studio/cluster/nameserver/RestartNameServerDTO.java b/server/src/main/java/com/rocketmq/studio/cluster/nameserver/RestartNameServerDTO.java index 6a04de90..e72644ef 100644 --- a/server/src/main/java/com/rocketmq/studio/cluster/nameserver/RestartNameServerDTO.java +++ b/server/src/main/java/com/rocketmq/studio/cluster/nameserver/RestartNameServerDTO.java @@ -16,6 +16,7 @@ */ package com.rocketmq.studio.cluster.nameserver; +import jakarta.validation.constraints.NotBlank; import lombok.AllArgsConstructor; import lombok.Builder; import lombok.Data; @@ -26,6 +27,9 @@ @NoArgsConstructor @AllArgsConstructor public class RestartNameServerDTO { + @NotBlank(message = "clusterId is required") private String clusterId; + + @NotBlank(message = "addr is required") private String addr; } diff --git a/server/src/main/java/com/rocketmq/studio/cluster/nameserver/UpdateNameServerDTO.java b/server/src/main/java/com/rocketmq/studio/cluster/nameserver/UpdateNameServerDTO.java index a15e62f5..2f9c8154 100644 --- a/server/src/main/java/com/rocketmq/studio/cluster/nameserver/UpdateNameServerDTO.java +++ b/server/src/main/java/com/rocketmq/studio/cluster/nameserver/UpdateNameServerDTO.java @@ -16,6 +16,7 @@ */ package com.rocketmq.studio.cluster.nameserver; +import jakarta.validation.constraints.NotBlank; import lombok.AllArgsConstructor; import lombok.Builder; import lombok.Data; @@ -26,7 +27,11 @@ @NoArgsConstructor @AllArgsConstructor public class UpdateNameServerDTO { + @NotBlank(message = "clusterId is required") private String clusterId; + + @NotBlank(message = "addr is required") private String addr; + private String version; } diff --git a/server/src/main/java/com/rocketmq/studio/cluster/nameserver/UpgradeNameServerDTO.java b/server/src/main/java/com/rocketmq/studio/cluster/nameserver/UpgradeNameServerDTO.java index d9af150b..21fd35fe 100644 --- a/server/src/main/java/com/rocketmq/studio/cluster/nameserver/UpgradeNameServerDTO.java +++ b/server/src/main/java/com/rocketmq/studio/cluster/nameserver/UpgradeNameServerDTO.java @@ -16,6 +16,7 @@ */ package com.rocketmq.studio.cluster.nameserver; +import jakarta.validation.constraints.NotBlank; import lombok.AllArgsConstructor; import lombok.Builder; import lombok.Data; @@ -26,7 +27,12 @@ @NoArgsConstructor @AllArgsConstructor public class UpgradeNameServerDTO { + @NotBlank(message = "clusterId is required") private String clusterId; + + @NotBlank(message = "addr is required") private String addr; + + @NotBlank(message = "targetVersion is required") private String targetVersion; } diff --git a/server/src/test/java/com/rocketmq/studio/cluster/nameserver/NameServerControllerTest.java b/server/src/test/java/com/rocketmq/studio/cluster/nameserver/NameServerControllerTest.java new file mode 100644 index 00000000..261624f9 --- /dev/null +++ b/server/src/test/java/com/rocketmq/studio/cluster/nameserver/NameServerControllerTest.java @@ -0,0 +1,159 @@ +/* + * 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 com.rocketmq.studio.cluster.nameserver; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.rocketmq.studio.cluster.broker.ClusterService; +import com.rocketmq.studio.common.domain.enums.ClusterStatus; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; +import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.http.MediaType; +import org.springframework.test.web.servlet.MockMvc; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +@WebMvcTest(NameServerController.class) +@AutoConfigureMockMvc(addFilters = false) +class NameServerControllerTest { + + @Autowired + private MockMvc mockMvc; + + @Autowired + private ObjectMapper objectMapper; + + @MockBean + private ClusterService clusterService; + + @Test + void createNameServerShouldPassValidatedRequest() throws Exception { + CreateNameServerDTO request = CreateNameServerDTO.builder() + .clusterId("cluster-1") + .addr("127.0.0.1:9876") + .version("5.3.2") + .build(); + NameServerVO created = NameServerVO.builder() + .addr("127.0.0.1:9876") + .status(ClusterStatus.healthy) + .build(); + when(clusterService.createNameServer(any(CreateNameServerDTO.class))).thenReturn(created); + + mockMvc.perform(post("/api/nameservers/create") + .contentType(MediaType.APPLICATION_JSON) + .content(objectMapper.writeValueAsString(request))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.code").value(200)) + .andExpect(jsonPath("$.data.addr").value("127.0.0.1:9876")); + + verify(clusterService).createNameServer(any(CreateNameServerDTO.class)); + } + + @Test + void updateNameServerShouldRejectBlankAddr() throws Exception { + UpdateNameServerDTO request = UpdateNameServerDTO.builder() + .clusterId("cluster-1") + .addr(" ") + .build(); + + mockMvc.perform(post("/api/nameservers/update") + .contentType(MediaType.APPLICATION_JSON) + .content(objectMapper.writeValueAsString(request))) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.code").value(400)) + .andExpect(jsonPath("$.message").value("addr is required")); + + verifyNoInteractions(clusterService); + } + + @Test + void restartNameServerShouldPassValidatedRequest() throws Exception { + RestartNameServerDTO request = RestartNameServerDTO.builder() + .clusterId("cluster-1") + .addr("127.0.0.1:9876") + .build(); + when(clusterService.restartNameServer(any(RestartNameServerDTO.class))).thenReturn(true); + + mockMvc.perform(post("/api/nameservers/restart") + .contentType(MediaType.APPLICATION_JSON) + .content(objectMapper.writeValueAsString(request))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.code").value(200)) + .andExpect(jsonPath("$.data.success").value(true)); + + verify(clusterService).restartNameServer(any(RestartNameServerDTO.class)); + } + + @Test + void restartNameServerShouldRejectMissingClusterId() throws Exception { + RestartNameServerDTO request = RestartNameServerDTO.builder() + .addr("127.0.0.1:9876") + .build(); + + mockMvc.perform(post("/api/nameservers/restart") + .contentType(MediaType.APPLICATION_JSON) + .content(objectMapper.writeValueAsString(request))) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.code").value(400)) + .andExpect(jsonPath("$.message").value("clusterId is required")); + + verifyNoInteractions(clusterService); + } + + @Test + void upgradeNameServerShouldRejectMissingTargetVersion() throws Exception { + UpgradeNameServerDTO request = UpgradeNameServerDTO.builder() + .clusterId("cluster-1") + .addr("127.0.0.1:9876") + .build(); + + mockMvc.perform(post("/api/nameservers/upgrade") + .contentType(MediaType.APPLICATION_JSON) + .content(objectMapper.writeValueAsString(request))) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.code").value(400)) + .andExpect(jsonPath("$.message").value("targetVersion is required")); + + verifyNoInteractions(clusterService); + } + + @Test + void deleteNameServerShouldPassValidatedRequest() throws Exception { + DeleteNameServerDTO request = DeleteNameServerDTO.builder() + .clusterId("cluster-1") + .addr("127.0.0.1:9876") + .build(); + when(clusterService.deleteNameServer(any(DeleteNameServerDTO.class))).thenReturn(true); + + mockMvc.perform(post("/api/nameservers/delete") + .contentType(MediaType.APPLICATION_JSON) + .content(objectMapper.writeValueAsString(request))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.code").value(200)) + .andExpect(jsonPath("$.data.success").value(true)); + + verify(clusterService).deleteNameServer(any(DeleteNameServerDTO.class)); + } +}