Copilot commented on code in PR #873:
URL:
https://github.com/apache/rocketmq-dashboard/pull/873#discussion_r3705195554
##########
server/src/test/java/org/apache/rocketmq/studio/instance/topic/TopicControllerTest.java:
##########
@@ -106,6 +108,47 @@ void createTopicShouldReturnCreatedTopic() throws
Exception {
.andExpect(jsonPath("$.code").value(200))
.andExpect(jsonPath("$.data.name").value("new-topic"))
.andExpect(jsonPath("$.data.writeQueues").value(16));
+
+ ArgumentCaptor<TopicVO> captor =
ArgumentCaptor.forClass(TopicVO.class);
+ verify(metadataService).createTopic(captor.capture());
+ assertThat(captor.getValue().getName()).isEqualTo("new-topic");
+ assertThat(captor.getValue().getWriteQueues()).isEqualTo(16);
+ assertThat(captor.getValue().getReadQueues()).isEqualTo(16);
+ }
+
+ @Test
+ void createTopicShouldRejectMissingName() throws Exception {
+ mockMvc.perform(post("/api/topics/create")
+ .contentType(MediaType.APPLICATION_JSON)
+ .content("""
+ {
+ "writeQueues": 8,
+ "readQueues": 8
+ }
+ """))
+ .andExpect(status().isBadRequest())
+ .andExpect(jsonPath("$.code").value(400))
+ .andExpect(jsonPath("$.message").value("name is required"));
+
+ verifyNoInteractions(metadataService);
+ }
Review Comment:
The PR description calls out rejecting “blank topic names”, but the added
test only covers a missing `name` (null). Add a test case where `name` is
present but blank/whitespace (e.g., `"name": " "`) to ensure `@NotBlank` is
enforced as intended (and still results in no interactions with
`metadataService`).
##########
server/src/test/java/org/apache/rocketmq/studio/instance/topic/TopicControllerTest.java:
##########
@@ -106,6 +108,47 @@ void createTopicShouldReturnCreatedTopic() throws
Exception {
.andExpect(jsonPath("$.code").value(200))
.andExpect(jsonPath("$.data.name").value("new-topic"))
.andExpect(jsonPath("$.data.writeQueues").value(16));
+
+ ArgumentCaptor<TopicVO> captor =
ArgumentCaptor.forClass(TopicVO.class);
+ verify(metadataService).createTopic(captor.capture());
+ assertThat(captor.getValue().getName()).isEqualTo("new-topic");
+ assertThat(captor.getValue().getWriteQueues()).isEqualTo(16);
+ assertThat(captor.getValue().getReadQueues()).isEqualTo(16);
+ }
+
+ @Test
+ void createTopicShouldRejectMissingName() throws Exception {
+ mockMvc.perform(post("/api/topics/create")
+ .contentType(MediaType.APPLICATION_JSON)
+ .content("""
+ {
+ "writeQueues": 8,
+ "readQueues": 8
+ }
+ """))
+ .andExpect(status().isBadRequest())
+ .andExpect(jsonPath("$.code").value(400))
+ .andExpect(jsonPath("$.message").value("name is required"));
+
+ verifyNoInteractions(metadataService);
+ }
+
+ @Test
+ void createTopicShouldRejectNegativeQueueCount() throws Exception {
+ mockMvc.perform(post("/api/topics/create")
+ .contentType(MediaType.APPLICATION_JSON)
+ .content("""
+ {
+ "name": "new-topic",
+ "writeQueues": -1,
+ "readQueues": 8
+ }
+ """))
+ .andExpect(status().isBadRequest())
+ .andExpect(jsonPath("$.code").value(400))
+ .andExpect(jsonPath("$.message").value("writeQueues must be
zero or positive"));
+
+ verifyNoInteractions(metadataService);
}
Review Comment:
Only `writeQueues` is tested for negative values, but `CreateTopicDTO` also
validates `readQueues` via `@PositiveOrZero`. Add a symmetric test that sends
`"readQueues": -1` (with a valid `writeQueues`) and asserts the expected 400 +
message, plus `verifyNoInteractions(metadataService)`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
