From 87c582648088a6ee86eea61032be00a1132d0121 Mon Sep 17 00:00:00 2001 From: liningrui Date: Sun, 6 Jan 2019 20:13:39 +0800 Subject: [PATCH] Let RestClient can reuse connections Implement #14 Change-Id: I49c0675566145e233e40f25fe0c30c9614bf2c14 --- pom.xml | 19 +- .../com/baidu/hugegraph/rest/RestClient.java | 117 ++++++-- .../com/baidu/hugegraph/rest/RestResult.java | 6 +- .../com/baidu/hugegraph/util/VersionUtil.java | 2 +- .../hugegraph/version/CommonVersion.java | 2 +- .../baidu/hugegraph/unit/UnitTestSuite.java | 4 + .../hugegraph/unit/rest/RestClientTest.java | 265 ++++++++++++++++++ .../hugegraph/unit/rest/RestResultTest.java | 144 ++++++++++ 8 files changed, 521 insertions(+), 38 deletions(-) create mode 100644 src/test/java/com/baidu/hugegraph/unit/rest/RestClientTest.java create mode 100644 src/test/java/com/baidu/hugegraph/unit/rest/RestResultTest.java diff --git a/pom.xml b/pom.xml index 079239dd9f..6ead7db1fd 100644 --- a/pom.xml +++ b/pom.xml @@ -6,7 +6,7 @@ com.baidu.hugegraph hugegraph-common - 1.5.8 + 1.5.9 hugegraph-common https://github.com/hugegraph/hugegraph-common @@ -66,9 +66,11 @@ 1.0 3.0.1 3.21.0-GA - 2.25.1 + + 2.22 2.27 4.12 + 2.25.1 @@ -78,6 +80,12 @@ junit ${junit.version} + + org.mockito + mockito-core + ${mockito.version} + test + @@ -157,6 +165,11 @@ jersey-media-json-jackson ${jersey.version} + + org.glassfish.jersey.connectors + jersey-apache-connector + ${jersey.version} + org.glassfish.jersey.inject jersey-hk2 @@ -199,7 +212,7 @@ - 1.5.8.0 + 1.5.9.0 diff --git a/src/main/java/com/baidu/hugegraph/rest/RestClient.java b/src/main/java/com/baidu/hugegraph/rest/RestClient.java index cce2d141e6..4a519b0427 100644 --- a/src/main/java/com/baidu/hugegraph/rest/RestClient.java +++ b/src/main/java/com/baidu/hugegraph/rest/RestClient.java @@ -19,6 +19,8 @@ package com.baidu.hugegraph.rest; +import static org.glassfish.jersey.apache.connector.ApacheClientProperties.CONNECTION_MANAGER; + import java.util.Collection; import java.util.Map; import java.util.concurrent.Callable; @@ -33,6 +35,9 @@ import javax.ws.rs.core.Response; import javax.ws.rs.core.Variant; +import org.apache.http.config.SocketConfig; +import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; +import org.glassfish.jersey.apache.connector.ApacheConnectorProvider; import org.glassfish.jersey.client.ClientConfig; import org.glassfish.jersey.client.ClientProperties; import org.glassfish.jersey.client.authentication.HttpAuthenticationFeature; @@ -45,24 +50,42 @@ public abstract class RestClient { - private Client client; - private WebTarget target; + private final ClientConfig config; + private final Client client; + private final WebTarget target; public RestClient(String url, int timeout) { - this(url, buildConfig(timeout)); + this(url, new ConfigBuilder().config(timeout).build()); + } + + public RestClient(String url, int timeout, int maxTotal, int maxPerRoute) { + this(url, new ConfigBuilder().config(timeout) + .config(timeout, maxTotal, maxPerRoute) + .build()); } public RestClient(String url, String user, String password, int timeout) { - this(url, buildConfigWithBasicAuth(user, password, timeout)); + this(url, new ConfigBuilder().config(timeout) + .config(user, password) + .build()); } + public RestClient(String url, String user, String password, int timeout, + int maxTotal, int maxPerRoute) { + this(url, new ConfigBuilder().config(timeout) + .config(user, password) + .config(timeout, maxTotal, maxPerRoute) + .build()); + } + public RestClient(String url, ClientConfig config) { - this.client = ClientBuilder.newClient(config); + this.config = config; + this.client = ClientBuilder.newClient(this.config); this.client.register(GZipEncoder.class); this.target = this.client.target(url); } - private Response request(Callable method) { + protected Response request(Callable method) { try { return method.call(); } catch (Exception e) { @@ -208,30 +231,64 @@ private static String encode(String raw) { return UriComponent.encode(raw, UriComponent.Type.PATH_SEGMENT); } - private static ClientConfig buildConfigWithBasicAuth(String username, - String password, - int timeout) { - ClientConfig config = buildConfig(timeout); - /* - * NOTE: don't use non-preemptive mode - * In non-preemptive mode the authentication information is added - * only when server refuses the request with 401 status code and - * then the request is repeated. - * Non-preemptive has negative impact on the performance. The advantage - * is that it does not send credentials when they are not needed. - * https://jersey.github.io/documentation/latest/client.html#d0e5461 - */ - config.register(HttpAuthenticationFeature.basic(username, password)); - return config; - } - - private static ClientConfig buildConfig(int timeout) { - ClientConfig config = new ClientConfig(); - config.property(ClientProperties.CONNECT_TIMEOUT, timeout); - config.property(ClientProperties.READ_TIMEOUT, timeout); - return config; - } - protected abstract void checkStatus(Response response, Response.Status... statuses); + + private static class ConfigBuilder { + + private final ClientConfig config; + + public ConfigBuilder() { + this.config = new ClientConfig(); + } + + public ConfigBuilder config(int timeout) { + this.config.property(ClientProperties.CONNECT_TIMEOUT, timeout); + this.config.property(ClientProperties.READ_TIMEOUT, timeout); + return this; + } + + public ConfigBuilder config(String username, String password) { + /* + * NOTE: don't use non-preemptive mode + * In non-preemptive mode the authentication information is added + * only when server refuses the request with 401 status code and + * then the request is repeated. + * Non-preemptive has negative impact on the performance. The + * advantage is it doesn't send credentials when they are not needed + * https://jersey.github.io/documentation/latest/client.html#d0e5461 + */ + this.config.register(HttpAuthenticationFeature.basic(username, + password)); + return this; + } + + public ConfigBuilder config(int timeout, int maxTotal, + int maxPerRoute) { + /* + * Using httpclient with connection pooling, and configuring the + * jersey connector, reference: + * http://www.theotherian.com/2013/08/jersey-client-2.0-httpclient-timeouts-max-connections.html + * https://stackoverflow.com/questions/43228051/memory-issue-with-jax-rs-using-jersey/46175943#46175943 + * + * But the jersey that has been released in the maven central + * repository seems to have a bug. + * https://github.com/jersey/jersey/pull/3752 + */ + PoolingHttpClientConnectionManager pcm = + new PoolingHttpClientConnectionManager(); + pcm.setDefaultSocketConfig(SocketConfig.custom() + .setSoTimeout(timeout) + .build()); + pcm.setMaxTotal(maxTotal); + pcm.setDefaultMaxPerRoute(maxPerRoute); + this.config.property(CONNECTION_MANAGER, pcm); + this.config.connectorProvider(new ApacheConnectorProvider()); + return this; + } + + public ClientConfig build() { + return this.config; + } + } } diff --git a/src/main/java/com/baidu/hugegraph/rest/RestResult.java b/src/main/java/com/baidu/hugegraph/rest/RestResult.java index 3be29b8cae..39abf6ca56 100644 --- a/src/main/java/com/baidu/hugegraph/rest/RestResult.java +++ b/src/main/java/com/baidu/hugegraph/rest/RestResult.java @@ -34,9 +34,9 @@ public class RestResult { private static final ObjectMapper mapper = new ObjectMapper(); - private int status; - private MultivaluedMap headers; - private String content; + private final int status; + private final MultivaluedMap headers; + private final String content; public RestResult(Response response) { this.status = response.getStatus(); diff --git a/src/main/java/com/baidu/hugegraph/util/VersionUtil.java b/src/main/java/com/baidu/hugegraph/util/VersionUtil.java index 6ac64f20f7..92bef3da23 100644 --- a/src/main/java/com/baidu/hugegraph/util/VersionUtil.java +++ b/src/main/java/com/baidu/hugegraph/util/VersionUtil.java @@ -44,7 +44,7 @@ public static boolean match(Version version, String begin, String end) { /** * Compare if a version is greater than the other one (inclusive) * @param version The version to be compared - * @param begin The lower bound of the range + * @param other The lower bound of the range * @return true if it's greater than the other, otherwise false */ public static boolean gte(String version, String other) { diff --git a/src/main/java/com/baidu/hugegraph/version/CommonVersion.java b/src/main/java/com/baidu/hugegraph/version/CommonVersion.java index b7bafdefa0..a34e52abf6 100644 --- a/src/main/java/com/baidu/hugegraph/version/CommonVersion.java +++ b/src/main/java/com/baidu/hugegraph/version/CommonVersion.java @@ -27,5 +27,5 @@ public class CommonVersion { // The second parameter of Version.of() is for all-in-one JAR public static final Version VERSION = Version.of(CommonVersion.class, - "1.5.8"); + "1.5.9"); } diff --git a/src/test/java/com/baidu/hugegraph/unit/UnitTestSuite.java b/src/test/java/com/baidu/hugegraph/unit/UnitTestSuite.java index c8f6e4cfb3..6850815ff9 100644 --- a/src/test/java/com/baidu/hugegraph/unit/UnitTestSuite.java +++ b/src/test/java/com/baidu/hugegraph/unit/UnitTestSuite.java @@ -30,6 +30,8 @@ import com.baidu.hugegraph.unit.iterator.FlatMapperIteratorTest; import com.baidu.hugegraph.unit.iterator.MapperIteratorTest; import com.baidu.hugegraph.unit.perf.PerfUtilTest; +import com.baidu.hugegraph.unit.rest.RestClientTest; +import com.baidu.hugegraph.unit.rest.RestResultTest; import com.baidu.hugegraph.unit.util.BytesTest; import com.baidu.hugegraph.unit.util.CollectionUtilTest; import com.baidu.hugegraph.unit.util.HashUtilTest; @@ -46,6 +48,8 @@ HugeConfigTest.class, EventHubTest.class, PerfUtilTest.class, + RestClientTest.class, + RestResultTest.class, VersionTest.class, ExtendableIteratorTest.class, diff --git a/src/test/java/com/baidu/hugegraph/unit/rest/RestClientTest.java b/src/test/java/com/baidu/hugegraph/unit/rest/RestClientTest.java new file mode 100644 index 0000000000..0b097682f6 --- /dev/null +++ b/src/test/java/com/baidu/hugegraph/unit/rest/RestClientTest.java @@ -0,0 +1,265 @@ +/* + * Copyright 2017 HugeGraph Authors + * + * 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.baidu.hugegraph.unit.rest; + +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.Callable; + +import javax.ws.rs.core.MultivaluedHashMap; +import javax.ws.rs.core.MultivaluedMap; +import javax.ws.rs.core.Response; + +import org.glassfish.jersey.internal.util.collection.ImmutableMultivaluedMap; +import org.junit.Test; +import org.mockito.Mockito; + +import com.baidu.hugegraph.rest.ClientException; +import com.baidu.hugegraph.rest.RestClient; +import com.baidu.hugegraph.rest.RestResult; +import com.baidu.hugegraph.testutil.Assert; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; + +public class RestClientTest { + + private static class RestClientImpl extends RestClient { + + private final int status; + private final MultivaluedMap headers; + private final String content; + + public RestClientImpl(String url, int timeout, + int maxTotal, int maxPerRoute, int status) { + super(url, timeout, maxTotal, maxPerRoute); + this.status = status; + this.headers = ImmutableMultivaluedMap.empty(); + this.content = ""; + } + + public RestClientImpl(String url, String user, String password, + int timeout, int status) { + super(url, user, password, timeout); + this.status = status; + this.headers = ImmutableMultivaluedMap.empty(); + this.content = ""; + } + + public RestClientImpl(String url, String user, String password, + int timeout, int maxTotal, int maxPerRoute, + int status) { + super(url, user, password, timeout, maxTotal, maxPerRoute); + this.status = status; + this.headers = ImmutableMultivaluedMap.empty(); + this.content = ""; + } + + public RestClientImpl(String url, int timeout, int status) { + this(url, timeout, status, ImmutableMultivaluedMap.empty(), ""); + } + + public RestClientImpl(String url, int timeout, int status, + MultivaluedMap headers) { + this(url, timeout, status, headers, ""); + } + + public RestClientImpl(String url, int timeout, int status, + String content) { + this(url, timeout, status, ImmutableMultivaluedMap.empty(), content); + } + + public RestClientImpl(String url, int timeout, int status, + MultivaluedMap headers, + String content) { + super(url, timeout); + this.status = status; + this.headers = headers; + this.content = content; + } + + @Override + protected Response request(Callable method) { + Response response = Mockito.mock(Response.class); + Mockito.when(response.getStatus()).thenReturn(this.status); + Mockito.when(response.getHeaders()).thenReturn(this.headers); + Mockito.when(response.readEntity(String.class)) + .thenReturn(this.content); + return response; + } + + @Override + protected void checkStatus(Response response, + Response.Status... statuses) { + boolean match = false; + for (Response.Status status : statuses) { + if (status.getStatusCode() == response.getStatus()) { + match = true; + break; + } + } + if (!match) { + throw new ClientException("Invalid response '%s'", response); + } + } + } + + @Test + public void testPost() { + RestClient client = new RestClientImpl("/test", 1000, 200); + RestResult restResult = client.post("path", "body"); + Assert.assertEquals(200, restResult.status()); + } + + @Test + // TODO: How to verify it? + public void testPostWithMaxTotalAndPerRoute() { + RestClient client = new RestClientImpl("/test", 1000, 10, 5, 200); + RestResult restResult = client.post("path", "body"); + Assert.assertEquals(200, restResult.status()); + } + + @Test + public void testPostWithUserAndPassword() { + RestClient client = new RestClientImpl("/test", "user", "", 1000, 200); + RestResult restResult = client.post("path", "body"); + Assert.assertEquals(200, restResult.status()); + } + + @Test + public void testPostWithAllParams() { + RestClient client = new RestClientImpl("/test", "user", "", 1000, + 10, 5, 200); + RestResult restResult = client.post("path", "body"); + Assert.assertEquals(200, restResult.status()); + } + + @Test + public void testPostWithHeaderAndContent() { + MultivaluedMap headers = new MultivaluedHashMap<>(); + headers.add("key1", "value1-1"); + headers.add("key1", "value1-2"); + headers.add("Content-Encoding", "gzip"); + String content = "{\"names\": [\"marko\", \"josh\", \"lop\"]}"; + RestClient client = new RestClientImpl("/test", 1000, 200, + headers, content); + RestResult restResult = client.post("path", "body"); + Assert.assertEquals(200, restResult.status()); + Assert.assertEquals(headers, restResult.headers()); + Assert.assertEquals(content, restResult.content()); + Assert.assertEquals(ImmutableList.of("marko", "josh", "lop"), + restResult.readList("names", String.class)); + } + + @Test + public void testPostWithException() { + RestClient client = new RestClientImpl("/test", 1000, 400); + Assert.assertThrows(ClientException.class, () -> { + client.post("path", "body"); + }); + } + + @Test + public void testPostWithParams() { + RestClient client = new RestClientImpl("/test", 1000, 200); + MultivaluedMap headers = ImmutableMultivaluedMap.empty(); + Map params = ImmutableMap.of("param1", "value1"); + RestResult restResult = client.post("path", "body", headers, + params); + Assert.assertEquals(200, restResult.status()); + } + + @Test + public void testPut() { + RestClient client = new RestClientImpl("/test", 1000, 200); + RestResult restResult = client.put("path", "id1", "body"); + Assert.assertEquals(200, restResult.status()); + } + + @Test + public void testPutWithParams() { + RestClient client = new RestClientImpl("/test", 1000, 200); + Map params = ImmutableMap.of("param1", "value1"); + RestResult restResult = client.put("path", "id1", "body", params); + Assert.assertEquals(200, restResult.status()); + } + + @Test + public void testPutWithException() { + RestClient client = new RestClientImpl("/test", 1000, 400); + Assert.assertThrows(ClientException.class, () -> { + client.put("path", "id1", "body"); + }); + } + + @Test + public void testGet() { + RestClient client = new RestClientImpl("/test", 1000, 200); + RestResult restResult = client.get("path"); + Assert.assertEquals(200, restResult.status()); + } + + @Test + public void testGetWithId() { + RestClient client = new RestClientImpl("/test", 1000, 200); + RestResult restResult = client.get("path", "id1"); + Assert.assertEquals(200, restResult.status()); + } + + @Test + public void testGetWithParams() { + RestClient client = new RestClientImpl("/test", 1000, 200); + Map params = new HashMap<>(); + params.put("key1", ImmutableList.of("value1-1", "value1-2")); + params.put("key2", "value2"); + RestResult restResult = client.get("path", params); + Assert.assertEquals(200, restResult.status()); + } + + @Test + public void testGetWithException() { + RestClient client = new RestClientImpl("/test", 1000, 400); + Assert.assertThrows(ClientException.class, () -> { + client.get("path", "id1"); + }); + } + + @Test + public void testDeleteWithId() { + RestClient client = new RestClientImpl("/test", 1000, 204); + RestResult restResult = client.delete("path", "id1"); + Assert.assertEquals(204, restResult.status()); + } + + @Test + public void testDeleteWithParams() { + RestClient client = new RestClientImpl("/test", 1000, 204); + Map params = ImmutableMap.of("param1", "value1"); + RestResult restResult = client.delete("path", params); + Assert.assertEquals(204, restResult.status()); + } + + @Test + public void testDeleteWithException() { + RestClient client = new RestClientImpl("/test", 1000, 400); + Assert.assertThrows(ClientException.class, () -> { + client.delete("path", "id1"); + }); + } +} diff --git a/src/test/java/com/baidu/hugegraph/unit/rest/RestResultTest.java b/src/test/java/com/baidu/hugegraph/unit/rest/RestResultTest.java new file mode 100644 index 0000000000..357c4a06d7 --- /dev/null +++ b/src/test/java/com/baidu/hugegraph/unit/rest/RestResultTest.java @@ -0,0 +1,144 @@ +/* + * Copyright 2017 HugeGraph Authors + * + * 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.baidu.hugegraph.unit.rest; + +import java.util.Map; + +import javax.ws.rs.core.MultivaluedHashMap; +import javax.ws.rs.core.MultivaluedMap; +import javax.ws.rs.core.Response; + +import org.glassfish.jersey.internal.util.collection.ImmutableMultivaluedMap; +import org.junit.Test; +import org.mockito.Mockito; + +import com.baidu.hugegraph.rest.RestResult; +import com.baidu.hugegraph.rest.SerializeException; +import com.baidu.hugegraph.testutil.Assert; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; + +public class RestResultTest { + + @Test + public void testStatus() { + RestResult result = newRestResult(200); + Assert.assertEquals(200, result.status()); + } + + @Test + public void testHeaders() { + MultivaluedMap headers = new MultivaluedHashMap<>(); + headers.add("key1", "value1-1"); + headers.add("key1", "value1-2"); + headers.add("key2", "value2"); + RestResult result = newRestResult(200, headers); + Assert.assertEquals(200, result.status()); + Assert.assertEquals(headers, result.headers()); + } + + @Test + public void testContent() { + String content = "{\"name\": \"marko\"}"; + RestResult result = newRestResult(200, content); + Assert.assertEquals(200, result.status()); + Assert.assertEquals(content, result.content()); + Assert.assertEquals(ImmutableMap.of("name", "marko"), + result.readObject(Map.class)); + } + + @Test + public void testContentWithException() { + String content = "{illegal key: \"marko\"}"; + RestResult result = newRestResult(200, content); + Assert.assertEquals(200, result.status()); + Assert.assertEquals(content, result.content()); + Assert.assertThrows(SerializeException.class, () -> { + result.readObject(Map.class); + }); + } + + @Test + public void testContentList() { + String content = "{\"names\": [\"marko\", \"josh\", \"lop\"]}"; + RestResult result = newRestResult(200, content); + Assert.assertEquals(200, result.status()); + Assert.assertEquals(content, result.content()); + Assert.assertEquals(ImmutableList.of("marko", "josh", "lop"), + result.readList("names", String.class)); + + content = "[\"marko\", \"josh\", \"lop\"]"; + result = newRestResult(200, content); + Assert.assertEquals(200, result.status()); + Assert.assertEquals(content, result.content()); + Assert.assertEquals(ImmutableList.of("marko", "josh", "lop"), + result.readList(String.class)); + } + + @Test + public void testContentListWithException() { + String content = "{\"names\": [\"marko\", \"josh\", \"lop\"]}"; + RestResult result = newRestResult(200, content); + Assert.assertEquals(200, result.status()); + Assert.assertEquals(content, result.content()); + Assert.assertThrows(SerializeException.class, () -> { + result.readList("unexitsed key", String.class); + }); + + content = "{\"names\": [marko, josh, \"lop\"]}"; + RestResult result2 = newRestResult(200, content); + Assert.assertEquals(200, result2.status()); + Assert.assertEquals(content, result2.content()); + Assert.assertThrows(SerializeException.class, () -> { + result2.readList("names", String.class); + }); + + content = "[marko, josh, \"lop\"]"; + RestResult result3 = newRestResult(200, content); + Assert.assertEquals(200, result3.status()); + Assert.assertEquals(content, result3.content()); + Assert.assertThrows(SerializeException.class, () -> { + result3.readList(String.class); + }); + } + + private static RestResult newRestResult(int status) { + return newRestResult(status, "", ImmutableMultivaluedMap.empty()); + } + + private static RestResult newRestResult(int status, String content) { + return newRestResult(status, content, ImmutableMultivaluedMap.empty()); + } + + private static RestResult newRestResult(int status, + MultivaluedMap h) { + return newRestResult(status, "", h); + } + + private static RestResult newRestResult(int status, String content, + MultivaluedMap h) { + Response response = Mockito.mock(Response.class); + Mockito.when(response.getStatus()).thenReturn(status); + Mockito.when(response.getHeaders()).thenReturn(h); + Mockito.when(response.readEntity(String.class)) + .thenReturn(content); + return new RestResult(response); + } +}