[helix] 08/15: Fix http request hanging issue to the SN API

[hu...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

hulee pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/helix.git

commit fe8596bf29b6c289ae04289289610c5846845d38
Author: Yi Wang <yw...@linkedin.com>
AuthorDate: Mon Jun 3 11:15:54 2019 -0700

    Fix http request hanging issue to the SN API
    
    RB=1684758
    G=helix-reviewers
    A=jxue
    
    Signed-off-by: Hunter Lee <hu...@linkedin.com>
---
 .../helix/rest/client/CustomRestClientImpl.java    | 32 ++++++++++++----------
 .../helix/rest/client/TestCustomRestClient.java    | 15 ++++++++++
 .../server/json/instance/TestStoppableCheck.java   |  7 +++--
 3 files changed, 38 insertions(+), 16 deletions(-)

diff --git a/helix-rest/src/main/java/org/apache/helix/rest/client/CustomRestClientImpl.java b/helix-rest/src/main/java/org/apache/helix/rest/client/CustomRestClientImpl.java
index 5d75f6a..a52820b 100644
--- a/helix-rest/src/main/java/org/apache/helix/rest/client/CustomRestClientImpl.java
+++ b/helix-rest/src/main/java/org/apache/helix/rest/client/CustomRestClientImpl.java
@@ -19,26 +19,27 @@ package org.apache.helix.rest.client;
  * under the License.
  */
 
-import com.fasterxml.jackson.databind.JsonNode;
-import com.fasterxml.jackson.databind.ObjectMapper;
-import com.google.common.annotations.VisibleForTesting;
 import java.io.IOException;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.stream.Collectors;
+
 import org.apache.http.HttpEntity;
 import org.apache.http.HttpResponse;
-import org.apache.http.NameValuePair;
+import org.apache.http.HttpStatus;
 import org.apache.http.client.ClientProtocolException;
 import org.apache.http.client.HttpClient;
-import org.apache.http.client.entity.UrlEncodedFormEntity;
 import org.apache.http.client.methods.HttpPost;
-import org.apache.http.message.BasicNameValuePair;
+import org.apache.http.entity.ContentType;
+import org.apache.http.entity.StringEntity;
 import org.apache.http.util.EntityUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.common.annotations.VisibleForTesting;
+
 class CustomRestClientImpl implements CustomRestClient {
   private static final Logger LOG = LoggerFactory.getLogger(CustomRestClient.class);
 
@@ -103,13 +104,15 @@ class CustomRestClientImpl implements CustomRestClient {
   protected JsonNode getJsonObject(HttpResponse httpResponse) throws IOException {
     HttpEntity httpEntity = httpResponse.getEntity();
     String str = EntityUtils.toString(httpEntity);
+    LOG.info("Converting Response Content {} to JsonNode", str);
     return OBJECT_MAPPER.readTree(str);
   }
 
   private Map<String, Boolean> handleResponse(HttpResponse httpResponse,
       JsonConverter jsonConverter) throws IOException {
     int status = httpResponse.getStatusLine().getStatusCode();
-    if (status == 200) {
+    if (status == HttpStatus.SC_OK) {
+      LOG.info("Expected HttpResponse statusCode: {}", HttpStatus.SC_OK);
       return jsonConverter.convert(getJsonObject(httpResponse));
     } else {
       throw new ClientProtocolException("Unexpected response status: " + status + ", reason: "
@@ -117,15 +120,16 @@ class CustomRestClientImpl implements CustomRestClient {
     }
   }
 
-  private HttpResponse post(String url, Map<String, String> payloads) throws IOException {
-    List<NameValuePair> params = payloads.entrySet().stream()
-        .map(entry -> new BasicNameValuePair(entry.getKey(), entry.getValue()))
-        .collect(Collectors.toList());
+  @VisibleForTesting
+  protected HttpResponse post(String url, Map<String, String> payloads) throws IOException {
     try {
       HttpPost postRequest = new HttpPost(url);
       postRequest.setHeader("Accept", ACCEPT_CONTENT_TYPE);
-      postRequest.setEntity(new UrlEncodedFormEntity(params, "UTF-8"));
-      LOG.info("Executing request {}", postRequest.getRequestLine());
+      StringEntity entity = new StringEntity(OBJECT_MAPPER.writeValueAsString(payloads),
+          ContentType.APPLICATION_JSON);
+      postRequest.setEntity(entity);
+      LOG.info("Executing request: {}, headers: {}, entity: {}", postRequest.getRequestLine(),
+          postRequest.getAllHeaders(), postRequest.getEntity());
       return _httpClient.execute(postRequest);
     } catch (IOException e) {
       LOG.error("Failed to perform customized health check. Is participant endpoint {} available?",
diff --git a/helix-rest/src/test/java/org/apache/helix/rest/client/TestCustomRestClient.java b/helix-rest/src/test/java/org/apache/helix/rest/client/TestCustomRestClient.java
index c43578b..812c941 100644
--- a/helix-rest/src/test/java/org/apache/helix/rest/client/TestCustomRestClient.java
+++ b/helix-rest/src/test/java/org/apache/helix/rest/client/TestCustomRestClient.java
@@ -13,6 +13,7 @@ import org.apache.http.StatusLine;
 import org.apache.http.client.ClientProtocolException;
 import org.apache.http.client.HttpClient;
 import org.apache.http.client.methods.HttpPost;
+import org.apache.http.impl.client.HttpClients;
 import org.junit.Assert;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
@@ -130,6 +131,20 @@ public class TestCustomRestClient {
         ImmutableList.of("db0", "db1"), Collections.emptyMap());
   }
 
+  @Test (description = "Validate if the post request has the correct format")
+  public void testPostRequestFormat() throws IOException {
+    // a popular echo server that echos all the inputs
+    // TODO: add a mock rest server
+    final String echoServer = "http://httpbin.org/post";
+    CustomRestClientImpl customRestClient = new CustomRestClientImpl(HttpClients.createDefault());
+    HttpResponse response = customRestClient.post(echoServer, Collections.emptyMap());
+    JsonNode json = customRestClient.getJsonObject(response);
+
+    Assert.assertEquals(response.getStatusLine().getStatusCode(), HttpStatus.SC_OK);
+    Assert.assertEquals(json.get("headers").get("Accept").asText(), "application/json");
+    Assert.assertEquals(json.get("data").asText(), "{}");
+  }
+
   private class MockCustomRestClient extends CustomRestClientImpl {
     private String _jsonResponse = "";
 
diff --git a/helix-rest/src/test/java/org/apache/helix/rest/server/json/instance/TestStoppableCheck.java b/helix-rest/src/test/java/org/apache/helix/rest/server/json/instance/TestStoppableCheck.java
index 0249b8f..43c219b 100644
--- a/helix-rest/src/test/java/org/apache/helix/rest/server/json/instance/TestStoppableCheck.java
+++ b/helix-rest/src/test/java/org/apache/helix/rest/server/json/instance/TestStoppableCheck.java
@@ -31,7 +31,8 @@ public class TestStoppableCheck {
 
   @Test
   public void whenSerializingStoppableCheck() throws JsonProcessingException {
-    StoppableCheck stoppableCheck = new StoppableCheck(false, ImmutableList.of("check"), StoppableCheck.Category.HELIX_OWN_CHECK);
+    StoppableCheck stoppableCheck = new StoppableCheck(false, ImmutableList.of("check"),
+        StoppableCheck.Category.HELIX_OWN_CHECK);
 
     ObjectMapper mapper = new ObjectMapper();
     String result = mapper.writeValueAsString(stoppableCheck);
@@ -41,7 +42,9 @@ public class TestStoppableCheck {
 
   @Test
   public void testConstructorSortingOrder() {
-    StoppableCheck stoppableCheck = new StoppableCheck(ImmutableMap.of("a", true, "c", false, "b", false), StoppableCheck.Category.HELIX_OWN_CHECK);
+    StoppableCheck stoppableCheck =
+        new StoppableCheck(ImmutableMap.of("a", true, "c", false, "b", false),
+            StoppableCheck.Category.HELIX_OWN_CHECK);
     Assert.assertFalse(stoppableCheck.isStoppable());
     Assert.assertEquals(stoppableCheck.getFailedChecks(), ImmutableList.of("Helix:b", "Helix:c"));
   }