zhenyuT commented on code in PR #133:
URL:
https://github.com/apache/incubator-hugegraph-commons/pull/133#discussion_r1306699937
##########
hugegraph-common/src/test/java/org/apache/hugegraph/unit/rest/RestClientTest.java:
##########
@@ -493,137 +514,131 @@ public MockRestClientImpl(String url, int timeout) {
@Override
protected void checkStatus(Response response,
- Response.Status... statuses) {
+ int... statuses) {
// pass
}
}
+ @SneakyThrows
@Test
public void testRequest() {
- MockRestClientImpl client = new MockRestClientImpl("test", 1000);
-
- WebTarget target = Mockito.mock(WebTarget.class);
- Builder builder = Mockito.mock(Builder.class);
-
- Mockito.when(target.path("test")).thenReturn(target);
- Mockito.when(target.path("test")
- .path(AbstractRestClient.encode("id")))
- .thenReturn(target);
- Mockito.when(target.path("test").request()).thenReturn(builder);
- Mockito.when(target.path("test")
- .path(AbstractRestClient.encode("id"))
- .request())
- .thenReturn(builder);
-
- Response response = Mockito.mock(Response.class);
- Mockito.when(response.getStatus()).thenReturn(200);
- Mockito.when(response.getHeaders())
- .thenReturn(new MultivaluedHashMap<>());
- Mockito.when(response.readEntity(String.class)).thenReturn("content");
-
- Mockito.when(builder.delete()).thenReturn(response);
- Mockito.when(builder.get()).thenReturn(response);
- Mockito.when(builder.put(Mockito.any())).thenReturn(response);
- Mockito.when(builder.post(Mockito.any())).thenReturn(response);
-
- Whitebox.setInternalState(client, "target", target);
+ MockRestClientImpl client = new MockRestClientImpl("/test", 1000);
+
+ Response response = Mockito.mock(Response.class,
Mockito.RETURNS_DEEP_STUBS);
+ Mockito.when(response.code()).thenReturn(200);
+ Mockito.when(response.headers())
+ .thenReturn(new Headers.Builder().build());
+ Mockito.when(response.body().string()).thenReturn("content");
+
+ Mockito.when(requestBuilder.delete()).thenReturn(requestBuilder);
+ Mockito.when(requestBuilder.get()).thenReturn(requestBuilder);
+
Mockito.when(requestBuilder.put(Mockito.any())).thenReturn(requestBuilder);
+
Mockito.when(requestBuilder.post(Mockito.any())).thenReturn(requestBuilder);
+
+ OkHttpClient okHttpClient = Mockito.mock(OkHttpClient.class,
Mockito.RETURNS_DEEP_STUBS);
+
Mockito.when(okHttpClient.newCall(Mockito.any()).execute()).thenReturn(response);
+
+ Whitebox.setInternalState(client, "client", okHttpClient);
+ Whitebox.setInternalState(client, "requestBuilder", requestBuilder);
RestResult result;
// Test delete
client.setAuthContext("token1");
result = client.delete("test", ImmutableMap.of());
Assert.assertEquals(200, result.status());
- Mockito.verify(builder).header(HttpHeaders.AUTHORIZATION,
+ Mockito.verify(requestBuilder).addHeader("Authorization",
"token1");
+
client.resetAuthContext();
client.setAuthContext("token2");
result = client.delete("test", "id");
Assert.assertEquals(200, result.status());
- Mockito.verify(builder).header(HttpHeaders.AUTHORIZATION,
+ Mockito.verify(requestBuilder).addHeader(HttpHeaders.AUTHORIZATION,
"token2");
client.resetAuthContext();
// Test get
client.setAuthContext("token3");
result = client.get("test");
Assert.assertEquals(200, result.status());
- Mockito.verify(builder).header(HttpHeaders.AUTHORIZATION,
+ Mockito.verify(requestBuilder).addHeader(HttpHeaders.AUTHORIZATION,
"token3");
client.resetAuthContext();
client.setAuthContext("token4");
result = client.get("test", ImmutableMap.of());
Assert.assertEquals(200, result.status());
- Mockito.verify(builder).header(HttpHeaders.AUTHORIZATION,
+ Mockito.verify(requestBuilder).addHeader(HttpHeaders.AUTHORIZATION,
"token4");
client.resetAuthContext();
client.setAuthContext("token5");
result = client.get("test", "id");
Assert.assertEquals(200, result.status());
- Mockito.verify(builder).header(HttpHeaders.AUTHORIZATION,
+ Mockito.verify(requestBuilder).addHeader(HttpHeaders.AUTHORIZATION,
"token5");
client.resetAuthContext();
// Test put
client.setAuthContext("token6");
- result = client.post("test", new Object());
+// result = client.post("test", new Object()); //why use new Object()
as args here?
+ result = client.post("test", null);
Assert.assertEquals(200, result.status());
- Mockito.verify(builder).header(HttpHeaders.AUTHORIZATION,
+ Mockito.verify(requestBuilder).addHeader(HttpHeaders.AUTHORIZATION,
"token6");
client.resetAuthContext();
client.setAuthContext("token7");
- result = client.post("test", new Object(), new MultivaluedHashMap<>());
+ result = client.post("test", null, new Headers.Builder().build());
Review Comment:
这里原来使用new
Object()作为post的传参，个人觉得本身就不是一种合理的传参方式，默认是json传参，如果没有参数传递，应该使用null或者{}作为参数更合理
##########
hugegraph-common/src/test/java/org/apache/hugegraph/unit/rest/RestClientTest.java:
##########
@@ -493,137 +514,131 @@ public MockRestClientImpl(String url, int timeout) {
@Override
protected void checkStatus(Response response,
- Response.Status... statuses) {
+ int... statuses) {
// pass
}
}
+ @SneakyThrows
@Test
public void testRequest() {
- MockRestClientImpl client = new MockRestClientImpl("test", 1000);
-
- WebTarget target = Mockito.mock(WebTarget.class);
- Builder builder = Mockito.mock(Builder.class);
-
- Mockito.when(target.path("test")).thenReturn(target);
- Mockito.when(target.path("test")
- .path(AbstractRestClient.encode("id")))
- .thenReturn(target);
- Mockito.when(target.path("test").request()).thenReturn(builder);
- Mockito.when(target.path("test")
- .path(AbstractRestClient.encode("id"))
- .request())
- .thenReturn(builder);
-
- Response response = Mockito.mock(Response.class);
- Mockito.when(response.getStatus()).thenReturn(200);
- Mockito.when(response.getHeaders())
- .thenReturn(new MultivaluedHashMap<>());
- Mockito.when(response.readEntity(String.class)).thenReturn("content");
-
- Mockito.when(builder.delete()).thenReturn(response);
- Mockito.when(builder.get()).thenReturn(response);
- Mockito.when(builder.put(Mockito.any())).thenReturn(response);
- Mockito.when(builder.post(Mockito.any())).thenReturn(response);
-
- Whitebox.setInternalState(client, "target", target);
+ MockRestClientImpl client = new MockRestClientImpl("/test", 1000);
+
+ Response response = Mockito.mock(Response.class,
Mockito.RETURNS_DEEP_STUBS);
+ Mockito.when(response.code()).thenReturn(200);
+ Mockito.when(response.headers())
+ .thenReturn(new Headers.Builder().build());
+ Mockito.when(response.body().string()).thenReturn("content");
+
+ Mockito.when(requestBuilder.delete()).thenReturn(requestBuilder);
+ Mockito.when(requestBuilder.get()).thenReturn(requestBuilder);
+
Mockito.when(requestBuilder.put(Mockito.any())).thenReturn(requestBuilder);
+
Mockito.when(requestBuilder.post(Mockito.any())).thenReturn(requestBuilder);
+
+ OkHttpClient okHttpClient = Mockito.mock(OkHttpClient.class,
Mockito.RETURNS_DEEP_STUBS);
+
Mockito.when(okHttpClient.newCall(Mockito.any()).execute()).thenReturn(response);
+
+ Whitebox.setInternalState(client, "client", okHttpClient);
+ Whitebox.setInternalState(client, "requestBuilder", requestBuilder);
RestResult result;
// Test delete
client.setAuthContext("token1");
result = client.delete("test", ImmutableMap.of());
Assert.assertEquals(200, result.status());
- Mockito.verify(builder).header(HttpHeaders.AUTHORIZATION,
+ Mockito.verify(requestBuilder).addHeader("Authorization",
"token1");
+
client.resetAuthContext();
client.setAuthContext("token2");
result = client.delete("test", "id");
Assert.assertEquals(200, result.status());
- Mockito.verify(builder).header(HttpHeaders.AUTHORIZATION,
+ Mockito.verify(requestBuilder).addHeader(HttpHeaders.AUTHORIZATION,
"token2");
client.resetAuthContext();
// Test get
client.setAuthContext("token3");
result = client.get("test");
Assert.assertEquals(200, result.status());
- Mockito.verify(builder).header(HttpHeaders.AUTHORIZATION,
+ Mockito.verify(requestBuilder).addHeader(HttpHeaders.AUTHORIZATION,
"token3");
client.resetAuthContext();
client.setAuthContext("token4");
result = client.get("test", ImmutableMap.of());
Assert.assertEquals(200, result.status());
- Mockito.verify(builder).header(HttpHeaders.AUTHORIZATION,
+ Mockito.verify(requestBuilder).addHeader(HttpHeaders.AUTHORIZATION,
"token4");
client.resetAuthContext();
client.setAuthContext("token5");
result = client.get("test", "id");
Assert.assertEquals(200, result.status());
- Mockito.verify(builder).header(HttpHeaders.AUTHORIZATION,
+ Mockito.verify(requestBuilder).addHeader(HttpHeaders.AUTHORIZATION,
"token5");
client.resetAuthContext();
// Test put
client.setAuthContext("token6");
- result = client.post("test", new Object());
+// result = client.post("test", new Object()); //why use new Object()
as args here?
+ result = client.post("test", null);
Assert.assertEquals(200, result.status());
- Mockito.verify(builder).header(HttpHeaders.AUTHORIZATION,
+ Mockito.verify(requestBuilder).addHeader(HttpHeaders.AUTHORIZATION,
"token6");
client.resetAuthContext();
client.setAuthContext("token7");
- result = client.post("test", new Object(), new MultivaluedHashMap<>());
+ result = client.post("test", null, new Headers.Builder().build());
Review Comment:
这里原来使用new
Object()作为post的传参，个人觉得本身就不是一种合理的传参方式，默认是json传参，如果没有参数传递，应该使用null或者{}作为参数更合理
--
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]
