RockteMQ-AI commented on code in PR #3287:
URL:
https://github.com/apache/rocketmq-dashboard/pull/3287#discussion_r3939626095
##########
server/src/test/java/org/apache/rocketmq/studio/provider/credential/CloudCredentialControllerTest.java:
##########
@@ -51,4 +58,61 @@ void getCredentialSecretsShouldDisableResponseCaching()
throws Exception {
.andExpect(status().isOk())
.andExpect(header().string(HttpHeaders.CACHE_CONTROL,
"no-store"));
}
+
+ @Test
+ void createCredentialConvertsAndDelegatesTest() throws Exception {
+ mockMvc.perform(post("/api/cloud-credentials/create")
+ .contentType(MediaType.APPLICATION_JSON)
Review Comment:
The POST /create JSON body is split across two string literals that
concatenate to ...\"vendor\":\"aliyun\",\"\"accessKey\":... - an extra quote
before accessKey. If this is how the source is rendered, the request is
malformed; if tests pass, the diff rendering may be misleading, but please
verify the actual file.
##########
server/src/test/java/org/apache/rocketmq/studio/provider/credential/CloudCredentialControllerTest.java:
##########
@@ -51,4 +58,61 @@ void getCredentialSecretsShouldDisableResponseCaching()
throws Exception {
.andExpect(status().isOk())
.andExpect(header().string(HttpHeaders.CACHE_CONTROL,
"no-store"));
}
+
+ @Test
+ void createCredentialConvertsAndDelegatesTest() throws Exception {
+ mockMvc.perform(post("/api/cloud-credentials/create")
+ .contentType(MediaType.APPLICATION_JSON)
+
.content("{\"name\":\"production\",\"vendor\":\"aliyun\","
+ +
"\"accessKey\":\"cloud-access-key\",\"secretKey\":\"cloud-secret-key\"}"))
+ .andExpect(status().isOk());
+
+ ArgumentCaptor<CloudCredentialVO> view =
ArgumentCaptor.forClass(CloudCredentialVO.class);
+ verify(credentialService).create(view.capture());
+ assertThat(view.getValue().getName()).isEqualTo("production");
+
assertThat(view.getValue().getVendor()).isEqualTo(InstanceVendor.ALIYUN);
+
assertThat(view.getValue().getAccessKey()).isEqualTo("cloud-access-key");
+
assertThat(view.getValue().getSecretKey()).isEqualTo("cloud-secret-key");
Review Comment:
listCredentialsDefaultsThePageWindowTest only verifies HTTP 200 and the
service call; asserting the serialized PageResult body would make the contract
test stronger.
##########
server/src/test/java/org/apache/rocketmq/studio/provider/credential/CloudCredentialControllerTest.java:
##########
@@ -51,4 +58,61 @@ void getCredentialSecretsShouldDisableResponseCaching()
throws Exception {
.andExpect(status().isOk())
Review Comment:
The new tests exercise controller delegation under @WebMvcTest but do not
cover authentication/authorization. Because these endpoints manage cloud AK/SK
pairs, consider adding at least one MockMvc test with Spring Security enabled
to ensure unauthenticated users cannot create/update/delete credentials.
##########
server/src/test/java/org/apache/rocketmq/studio/provider/credential/CloudCredentialControllerTest.java:
##########
@@ -51,4 +58,61 @@ void getCredentialSecretsShouldDisableResponseCaching()
throws Exception {
.andExpect(status().isOk())
.andExpect(header().string(HttpHeaders.CACHE_CONTROL,
"no-store"));
}
+
+ @Test
+ void createCredentialConvertsAndDelegatesTest() throws Exception {
+ mockMvc.perform(post("/api/cloud-credentials/create")
+ .contentType(MediaType.APPLICATION_JSON)
+
.content("{\"name\":\"production\",\"vendor\":\"aliyun\","
+ +
"\"accessKey\":\"cloud-access-key\",\"secretKey\":\"cloud-secret-key\"}"))
+ .andExpect(status().isOk());
+
+ ArgumentCaptor<CloudCredentialVO> view =
ArgumentCaptor.forClass(CloudCredentialVO.class);
+ verify(credentialService).create(view.capture());
+ assertThat(view.getValue().getName()).isEqualTo("production");
+
assertThat(view.getValue().getVendor()).isEqualTo(InstanceVendor.ALIYUN);
+
assertThat(view.getValue().getAccessKey()).isEqualTo("cloud-access-key");
+
assertThat(view.getValue().getSecretKey()).isEqualTo("cloud-secret-key");
+ }
+
+ @Test
+ void listCredentialsDefaultsThePageWindowTest() throws Exception {
+ when(credentialService.listMasked(null, null, 1, 20))
+ .thenReturn(PageResult.empty(1, 20));
+
+ mockMvc.perform(get("/api/cloud-credentials"))
+ .andExpect(status().isOk());
+ verify(credentialService).listMasked(null, null, 1, 20);
+ }
+
+ @Test
+ void updateCredentialDelegatesWithTheIdTest() throws Exception {
+ mockMvc.perform(post("/api/cloud-credentials/update")
+ .contentType(MediaType.APPLICATION_JSON)
+
.content("{\"id\":1,\"secretKey\":\"rotated-cloud-secret\"}"))
+ .andExpect(status().isOk());
+
+ ArgumentCaptor<UpdateCloudCredentialDTO> request =
+ ArgumentCaptor.forClass(UpdateCloudCredentialDTO.class);
+ verify(credentialService).update(request.capture());
+ assertThat(request.getValue().getId()).isEqualTo(1L);
Review Comment:
deleteCredentialParsesTheStringIdTest uses a string id \"7\" to verify
flexible parsing. Consider also testing the numeric-id form to confirm the API
contract accepts both.
--
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]
