Merge pull request #56 from treasure-data/fix_hang_when_validate_task

Fix hang when validate task
treasure-data · Apr 8, 2019 · c3ed36f · c3ed36f
2 parents 3a8158b + f4602e9
commit c3ed36f
Show file tree

Hide file tree

Showing 8 changed files with 173 additions and 165 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,7 @@
+## 0.2.8 - 2019-04-08
+
+* [fixed] Process is hang forever when validate task [#56](https://github.com/treasure-data/embulk-input-jira/pull/56)
+
 ## 0.2.7 - 2019-03-05
 
 * [enhancement] Support empty JQL [#55](https://github.com/treasure-data/embulk-input-jira/pull/55)

diff --git a/build.gradle b/build.gradle
@@ -9,23 +9,19 @@ import com.github.jrubygradle.JRubyExec
 repositories {
     mavenCentral()
     jcenter()
-    maven {
-        url "https://dl.bintray.com/embulk-base-restclient/maven"
-    }
 }
 configurations {
     provided
 }
 
-version = "0.2.7"
+version = "0.2.8"
 
 sourceCompatibility = 1.8
 targetCompatibility = 1.8
 
 dependencies {
     compile  "org.embulk:embulk-core:0.9.11"
     provided "org.embulk:embulk-core:0.9.11"
-    compile  "org.embulk.base.restclient:embulk-util-retryhelper-jetty92:0.6.0"
     compile group: 'org.apache.httpcomponents', name: 'httpclient', version: '4.5.6'
     compile group: 'org.glassfish.jersey.core', name: 'jersey-client', version: '2.27'
     compile group: 'com.google.code.gson', name: 'gson', version: '2.8.5'

diff --git a/src/main/java/org/embulk/input/jira/Constant.java b/src/main/java/org/embulk/input/jira/Constant.java
@@ -7,6 +7,7 @@ public final class Constant
     public static final int GUESS_RECORDS_COUNT = 50;
     public static final int PREVIEW_RECORDS_COUNT = 10;
     public static final int GUESS_BUFFER_SIZE = 5 * 1024 * 1024;
+    public static final int HTTP_TIMEOUT = 300 * 1000;
 
     public static final String DEFAULT_TIMESTAMP_PATTERN = "%Y-%m-%dT%H:%M:%S.%L%z";
 

diff --git a/src/main/java/org/embulk/input/jira/client/JiraClient.java b/src/main/java/org/embulk/input/jira/client/JiraClient.java
@@ -8,14 +8,15 @@
 import com.google.gson.JsonParser;
 import com.google.gson.JsonPrimitive;
 
-import org.apache.http.HttpResponse;
 import org.apache.http.HttpStatus;
-import org.apache.http.client.HttpClient;
+import org.apache.http.client.config.CookieSpecs;
 import org.apache.http.client.config.RequestConfig;
+import org.apache.http.client.methods.CloseableHttpResponse;
 import org.apache.http.client.methods.HttpGet;
 import org.apache.http.client.methods.HttpPost;
 import org.apache.http.client.methods.HttpRequestBase;
 import org.apache.http.entity.StringEntity;
+import org.apache.http.impl.client.CloseableHttpClient;
 import org.apache.http.impl.client.HttpClientBuilder;
 import org.apache.http.util.EntityUtils;
 import org.embulk.config.ConfigException;
@@ -40,15 +41,14 @@
 import static org.apache.http.HttpHeaders.ACCEPT;
 import static org.apache.http.HttpHeaders.AUTHORIZATION;
 import static org.apache.http.HttpHeaders.CONTENT_TYPE;
+import static org.embulk.input.jira.Constant.HTTP_TIMEOUT;
 import static org.embulk.input.jira.Constant.MIN_RESULTS;
 import static org.embulk.spi.util.RetryExecutor.retryExecutor;
 
 public class JiraClient
 {
     public JiraClient() {}
 
-    private static final int CONNECTION_TIME_OUT = 300000;
-
     private static final Logger LOGGER = Exec.getLogger(JiraClient.class);
 
     public void checkUserCredentials(final PluginTask task)
@@ -157,22 +157,22 @@ public void onGiveup(Exception firstException, Exception lastException) throws R
 
     private String authorizeAndRequest(final PluginTask task, String url, String body) throws JiraException
     {
-        try {
-            HttpClient client = createHttpClient();
+        try (CloseableHttpClient client = createHttpClient()) {
             HttpRequestBase request;
             if (body == null) {
                 request = createGetRequest(task, url);
             }
             else {
                 request = createPostRequest(task, url, body);
             }
-            HttpResponse response = client.execute(request);
-            // Check for HTTP response code : 200 : SUCCESS
-            int statusCode = response.getStatusLine().getStatusCode();
-            if (statusCode != HttpStatus.SC_OK) {
-                throw new JiraException(statusCode, extractErrorMessages(EntityUtils.toString(response.getEntity())));
+            try (CloseableHttpResponse response = client.execute(request)) {
+             // Check for HTTP response code : 200 : SUCCESS
+                int statusCode = response.getStatusLine().getStatusCode();
+                if (statusCode != HttpStatus.SC_OK) {
+                    throw new JiraException(statusCode, extractErrorMessages(EntityUtils.toString(response.getEntity())));
+                }
+                return EntityUtils.toString(response.getEntity());
             }
-            return EntityUtils.toString(response.getEntity());
         }
         catch (IOException e) {
             throw new JiraException(-1, e.getMessage());
@@ -195,13 +195,16 @@ private String extractErrorMessages(String errorResponse)
     }
 
     @VisibleForTesting
-    public HttpClient createHttpClient()
+    public CloseableHttpClient createHttpClient()
     {
-        RequestConfig config = RequestConfig.custom()
-                .setConnectTimeout(CONNECTION_TIME_OUT)
-                .setConnectionRequestTimeout(CONNECTION_TIME_OUT)
-                .build();
-        return HttpClientBuilder.create().setDefaultRequestConfig(config).build();
+        return HttpClientBuilder.create()
+                    .setDefaultRequestConfig(RequestConfig.custom()
+                                                        .setConnectTimeout(HTTP_TIMEOUT)
+                                                        .setConnectionRequestTimeout(HTTP_TIMEOUT)
+                                                        .setSocketTimeout(HTTP_TIMEOUT)
+                                                        .setCookieSpec(CookieSpecs.STANDARD)
+                                                        .build())
+                    .build();
     }
 
     private HttpRequestBase createPostRequest(PluginTask task, String url, String body) throws IOException

diff --git a/src/main/java/org/embulk/input/jira/util/JiraUtil.java b/src/main/java/org/embulk/input/jira/util/JiraUtil.java
@@ -2,6 +2,12 @@
 
 import com.google.gson.JsonElement;
 
+import org.apache.http.client.config.CookieSpecs;
+import org.apache.http.client.config.RequestConfig;
+import org.apache.http.client.methods.CloseableHttpResponse;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.impl.client.CloseableHttpClient;
+import org.apache.http.impl.client.HttpClientBuilder;
 import org.embulk.config.ConfigException;
 import org.embulk.input.jira.Issue;
 import org.embulk.input.jira.JiraInputPlugin.PluginTask;
@@ -17,15 +23,14 @@
 import javax.ws.rs.core.UriBuilder;
 
 import java.io.IOException;
-import java.net.HttpURLConnection;
-import java.net.URL;
 import java.util.List;
 import java.util.stream.Collectors;
 import java.util.stream.StreamSupport;
 
 import static com.google.common.base.Strings.isNullOrEmpty;
 import static org.embulk.input.jira.Constant.CREDENTIAL_URI_PATH;
 import static org.embulk.input.jira.Constant.DEFAULT_TIMESTAMP_PATTERN;
+import static org.embulk.input.jira.Constant.HTTP_TIMEOUT;
 import static org.embulk.input.jira.Constant.SEARCH_URI_PATH;
 
 public final class JiraUtil
@@ -61,21 +66,22 @@ public static void validateTaskConfig(final PluginTask task)
         if (isNullOrEmpty(uri)) {
             throw new ConfigException("JIRA API endpoint could not be empty");
         }
-        HttpURLConnection connection = null;
-        try {
-            URL u = new URL(uri);
-            connection = (HttpURLConnection) u.openConnection();
-            connection.setRequestMethod("GET");
-            connection.getResponseCode();
+        try (CloseableHttpClient client = HttpClientBuilder.create()
+                                            .setDefaultRequestConfig(RequestConfig.custom()
+                                                                                .setConnectTimeout(HTTP_TIMEOUT)
+                                                                                .setConnectionRequestTimeout(HTTP_TIMEOUT)
+                                                                                .setSocketTimeout(HTTP_TIMEOUT)
+                                                                                .setCookieSpec(CookieSpecs.STANDARD)
+                                                                                .build())
+                                            .build()) {
+            HttpGet request = new HttpGet(uri);
+            try (CloseableHttpResponse response = client.execute(request)) {
+                response.getStatusLine().getStatusCode();
+            }
         }
-        catch (IOException e) {
+        catch (IOException | IllegalArgumentException e) {
             throw new ConfigException("JIRA API endpoint is incorrect or not available");
         }
-        finally {
-            if (connection != null) {
-                connection.disconnect();
-            }
-        }
         int retryInitialWaitSec = task.getInitialRetryIntervalMillis();
         if (retryInitialWaitSec < 1) {
             throw new ConfigException("Initial retry delay should be equal or greater than 1");

diff --git a/src/test/java/org/embulk/input/jira/JiraInputPluginTest.java b/src/test/java/org/embulk/input/jira/JiraInputPluginTest.java
@@ -3,11 +3,11 @@
 import com.google.gson.JsonElement;
 import com.google.gson.JsonObject;
 import com.google.gson.JsonParser;
-import org.apache.http.HttpResponse;
 import org.apache.http.StatusLine;
-import org.apache.http.client.HttpClient;
+import org.apache.http.client.methods.CloseableHttpResponse;
 import org.apache.http.client.methods.HttpUriRequest;
 import org.apache.http.entity.StringEntity;
+import org.apache.http.impl.client.CloseableHttpClient;
 import org.embulk.config.ConfigDiff;
 import org.embulk.config.ConfigSource;
 import org.embulk.config.TaskReport;
@@ -42,8 +42,8 @@ public class JiraInputPluginTest
     private JiraClient jiraClient;
     private JsonObject data;
     private ConfigSource config;
-    private HttpClient client = Mockito.mock(HttpClient.class);
-    private HttpResponse response = Mockito.mock(HttpResponse.class);
+    private CloseableHttpClient client = Mockito.mock(CloseableHttpClient.class);
+    private CloseableHttpResponse response = Mockito.mock(CloseableHttpResponse.class);
     private StatusLine statusLine = Mockito.mock(StatusLine.class);
 
     private MockPageOutput output = new MockPageOutput();

diff --git a/src/test/java/org/embulk/input/jira/client/JiraClientTest.java b/src/test/java/org/embulk/input/jira/client/JiraClientTest.java
@@ -2,10 +2,10 @@
 
 import com.google.gson.JsonObject;
 
-import org.apache.http.HttpResponse;
 import org.apache.http.StatusLine;
-import org.apache.http.client.HttpClient;
+import org.apache.http.client.methods.CloseableHttpResponse;
 import org.apache.http.entity.StringEntity;
+import org.apache.http.impl.client.CloseableHttpClient;
 import org.embulk.EmbulkTestRuntime;
 import org.embulk.config.ConfigException;
 import org.embulk.config.ConfigSource;
@@ -34,8 +34,8 @@ public class JiraClientTest
     private JiraClient jiraClient;
     private PluginTask task;
 
-    private HttpClient client = Mockito.mock(HttpClient.class);
-    private HttpResponse response = Mockito.mock(HttpResponse.class);
+    private CloseableHttpClient client = Mockito.mock(CloseableHttpClient.class);
+    private CloseableHttpResponse response = Mockito.mock(CloseableHttpResponse.class);
     private StatusLine statusLine = Mockito.mock(StatusLine.class);
     private JsonObject data;
 
@@ -44,7 +44,7 @@ public void setUp() throws IOException
     {
         if (jiraClient == null) {
             jiraClient = Mockito.spy(new JiraClient());
-            response = Mockito.mock(HttpResponse.class);
+            response = Mockito.mock(CloseableHttpResponse.class);
             task = TestHelpers.config().loadConfig(PluginTask.class);
             data = TestHelpers.getJsonFromFile("jira_client.json");
         }