From 10d17facf8c04138c4dea5d21a842fd6a486e933 Mon Sep 17 00:00:00 2001 From: Tareq Sharafy Date: Tue, 23 Aug 2016 19:23:36 +0300 Subject: [PATCH] A single flow for all outgoing HTTP requests phase 1 - builder, runner, all json requests Additional methods in HttpJsonRequest allows customizing the HTTP request further by adding headers. The default internal implementation has been refactoring to allow sending general HTTP requests, not necessarily JSON requests and responses. Custom implementations and change the underlying HTTP invocation by simply overriding doRequestGeneral(..), affecting all outgoing HTTP requests issued by the platform. Signed-off-by: Tareq Sharafy Change-Id: I2f334b7bb133b3f136d2b8e5b18301602f427355 --- platform-api/che-core-api-builder/pom.xml | 45 ------ .../eclipse/che/api/builder/BuilderUtils.java | 4 +- .../eclipse/che/api/builder/RemoteTask.java | 18 +-- .../builder/internal/SourcesManagerImpl.java | 100 ++++++------- .../eclipse/che/api/builder/BuilderTest.java | 12 +- .../api/core/rest/DefaultHttpJsonRequest.java | 138 ++++++++++++------ .../che/api/core/rest/HttpJsonRequest.java | 32 ++++ .../che/api/core/rest/HttpResponse.java | 51 +++++++ .../core/rest/URLConnectionHttpResponse.java | 64 ++++++++ .../che/api/core/util/DownloadPlugin.java | 3 + .../che/api/core/util/DownloadPluginImpl.java | 50 +++++++ .../che/api/core/util/HttpDownloadPlugin.java | 26 +--- .../che/api/factory/FactoryServiceTest.java | 1 + .../che/api/runner/RemoteRunnerProcess.java | 23 +-- .../org/eclipse/che/api/runner/RunQueue.java | 20 +-- .../eclipse/che/api/runner/RunnerUtils.java | 4 +- .../che/api/runner/internal/Runner.java | 10 +- .../eclipse/che/api/runner/RunQueueTest.java | 1 + 18 files changed, 378 insertions(+), 224 deletions(-) create mode 100644 platform-api/che-core-api-core/src/main/java/org/eclipse/che/api/core/rest/HttpResponse.java create mode 100644 platform-api/che-core-api-core/src/main/java/org/eclipse/che/api/core/rest/URLConnectionHttpResponse.java create mode 100644 platform-api/che-core-api-core/src/main/java/org/eclipse/che/api/core/util/DownloadPluginImpl.java diff --git a/platform-api/che-core-api-builder/pom.xml b/platform-api/che-core-api-builder/pom.xml index 5220a8233..c9053c04f 100644 --- a/platform-api/che-core-api-builder/pom.xml +++ b/platform-api/che-core-api-builder/pom.xml @@ -223,49 +223,4 @@ - - - default - - true - - - - - org.apache.maven.plugins - maven-surefire-plugin - - - **/*Test.java - - - - - - - - unix - - - unix - - - - - - org.apache.maven.plugins - maven-surefire-plugin - - - ${project.build.directory} - - - **/*Test.java - - - - - - - diff --git a/platform-api/che-core-api-builder/src/main/java/org/eclipse/che/api/builder/BuilderUtils.java b/platform-api/che-core-api-builder/src/main/java/org/eclipse/che/api/builder/BuilderUtils.java index 89df31264..97c3cb417 100644 --- a/platform-api/che-core-api-builder/src/main/java/org/eclipse/che/api/builder/BuilderUtils.java +++ b/platform-api/che-core-api-builder/src/main/java/org/eclipse/che/api/builder/BuilderUtils.java @@ -37,10 +37,10 @@ public static HttpJsonResponse builderRequest(HttpJsonRequest req) throws Builde try { return req.request(); } catch (IOException e) { - throw new BuilderException(e); + throw new BuilderException(e.getMessage(), e); } catch (ServerException | UnauthorizedException | ForbiddenException | NotFoundException | ConflictException | BadRequestException e) { - throw new BuilderException(e.getServiceError()); + throw new BuilderException(e.getMessage(), e); } } diff --git a/platform-api/che-core-api-builder/src/main/java/org/eclipse/che/api/builder/RemoteTask.java b/platform-api/che-core-api-builder/src/main/java/org/eclipse/che/api/builder/RemoteTask.java index 5463f8aea..81399cc47 100644 --- a/platform-api/che-core-api-builder/src/main/java/org/eclipse/che/api/builder/RemoteTask.java +++ b/platform-api/che-core-api-builder/src/main/java/org/eclipse/che/api/builder/RemoteTask.java @@ -17,18 +17,15 @@ import org.eclipse.che.api.core.NotFoundException; import org.eclipse.che.api.core.rest.HttpJsonRequestFactory; import org.eclipse.che.api.core.rest.HttpOutputMessage; +import org.eclipse.che.api.core.rest.HttpResponse; import org.eclipse.che.api.core.rest.shared.dto.Link; -import org.eclipse.che.commons.env.EnvironmentContext; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import javax.ws.rs.HttpMethod; import javax.ws.rs.core.HttpHeaders; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.net.HttpURLConnection; -import java.net.URL; import static com.google.common.base.MoreObjects.firstNonNull; import static org.eclipse.che.api.builder.BuilderUtils.builderRequest; @@ -217,15 +214,7 @@ public void downloadResultArchive(String archType, HttpOutputMessage output) thr } private void readFromUrl(String url, final HttpOutputMessage output) throws IOException { - final HttpURLConnection conn = (HttpURLConnection)new URL(url).openConnection(); - conn.setConnectTimeout(60 * 1000); - conn.setReadTimeout(60 * 1000); - conn.setRequestMethod(HttpMethod.GET); - final EnvironmentContext context = EnvironmentContext.getCurrent(); - if (context.getUser() != null && context.getUser().getToken() != null) { - conn.setRequestProperty(HttpHeaders.AUTHORIZATION, context.getUser().getToken()); - } - try { + try (HttpResponse conn = requestFactory.fromUrl(url).setTimeout(60 * 1000).requestGeneral()) { output.setStatus(conn.getResponseCode()); final String contentType = conn.getContentType(); if (contentType != null) { @@ -241,9 +230,6 @@ private void readFromUrl(String url, final HttpOutputMessage output) throws IOEx OutputStream out = output.getOutputStream()) { ByteStreams.copy(in, out); } - - } finally { - conn.disconnect(); } } } diff --git a/platform-api/che-core-api-builder/src/main/java/org/eclipse/che/api/builder/internal/SourcesManagerImpl.java b/platform-api/che-core-api-builder/src/main/java/org/eclipse/che/api/builder/internal/SourcesManagerImpl.java index 9b2a3c342..f1a6d6870 100644 --- a/platform-api/che-core-api-builder/src/main/java/org/eclipse/che/api/builder/internal/SourcesManagerImpl.java +++ b/platform-api/che-core-api-builder/src/main/java/org/eclipse/che/api/builder/internal/SourcesManagerImpl.java @@ -11,8 +11,11 @@ package org.eclipse.che.api.builder.internal; import org.eclipse.che.api.builder.dto.BaseBuilderRequest; +import org.eclipse.che.api.core.rest.HttpJsonRequest; +import org.eclipse.che.api.core.rest.HttpJsonRequest.BodyWriter; +import org.eclipse.che.api.core.rest.HttpJsonRequestFactory; +import org.eclipse.che.api.core.rest.HttpResponse; import org.eclipse.che.api.core.util.ValueHolder; -import org.eclipse.che.commons.env.EnvironmentContext; import org.eclipse.che.commons.json.JsonHelper; import org.eclipse.che.commons.json.JsonParseException; import org.eclipse.che.commons.lang.IoUtil; @@ -29,6 +32,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.io.BufferedWriter; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.File; @@ -40,7 +44,9 @@ import java.io.StringReader; import java.io.Writer; import java.net.HttpURLConnection; -import java.net.URL; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; import java.text.ParseException; import java.util.HashMap; import java.util.LinkedList; @@ -82,19 +88,21 @@ public class SourcesManagerImpl implements SourcesManager { private final AtomicReference projectKeyHolder; private final Set listeners; private final ScheduledExecutorService executor; + private final HttpJsonRequestFactory provider; private static final long KEEP_PROJECT_TIME = TimeUnit.MINUTES.toMillis(30); private static final int CONNECT_TIMEOUT = (int)TimeUnit.MINUTES.toMillis(4);//This time is chosen empirically and private static final int READ_TIMEOUT = (int)TimeUnit.MINUTES.toMillis(4);//necessary for some large projects. See IDEX-1957. @Inject - public SourcesManagerImpl(@Named(Constants.BASE_DIRECTORY) File rootDirectory) { + public SourcesManagerImpl(@Named(Constants.BASE_DIRECTORY) File rootDirectory, HttpJsonRequestFactory provider) { this.rootDirectory = rootDirectory; tasks = new ConcurrentHashMap<>(); projectKeyHolder = new AtomicReference<>(); executor = Executors.newSingleThreadScheduledExecutor( new ThreadFactoryBuilder().setNameFormat(getClass().getSimpleName() + "-FileCleaner-%d").setDaemon(true).build()); listeners = new CopyOnWriteArraySet<>(); + this.provider = provider; } @PostConstruct @@ -202,53 +210,51 @@ public void write(byte[] b) throws IOException { }; private void download(String downloadUrl, java.io.File downloadTo, File directory) throws IOException { - HttpURLConnection conn = null; - try { - final LinkedList q = new LinkedList<>(); - q.add(downloadTo); - final long start = System.currentTimeMillis(); - final List> md5sums = new LinkedList<>(); - while (!q.isEmpty()) { - java.io.File current = q.pop(); - java.io.File[] list = current.listFiles(); - if (list != null) { - for (java.io.File f : list) { - if (f.isDirectory()) { - q.push(f); - } else { - md5sums.add(Pair.of(com.google.common.io.Files.hash(f, Hashing.md5()).toString(), - downloadTo.toPath().relativize(f.toPath()).toString() - .replace("\\", "/"))); //Replacing of "\" is need for windows support - } + final LinkedList q = new LinkedList<>(); + q.add(downloadTo); + final long start = System.currentTimeMillis(); + final List> md5sums = new LinkedList<>(); + while (!q.isEmpty()) { + java.io.File current = q.pop(); + java.io.File[] list = current.listFiles(); + if (list != null) { + for (java.io.File f : list) { + if (f.isDirectory()) { + q.push(f); + } else { + md5sums.add(Pair.of(com.google.common.io.Files.hash(f, Hashing.md5()).toString(), + downloadTo.toPath().relativize(f.toPath()).toString() + .replace("\\", "/"))); //Replacing of "\" is need for windows support } } } - final long end = System.currentTimeMillis(); - if (md5sums.size() > 0) { - LOG.debug("count md5sums of {} files, time: {}ms", md5sums.size(), (end - start)); - } - conn = (HttpURLConnection)new URL(downloadUrl).openConnection(); - conn.setConnectTimeout(CONNECT_TIMEOUT); - conn.setReadTimeout(READ_TIMEOUT); - final EnvironmentContext context = EnvironmentContext.getCurrent(); - if (context.getUser() != null && context.getUser().getToken() != null) { - conn.setRequestProperty(HttpHeaders.AUTHORIZATION, context.getUser().getToken()); - } - if (!md5sums.isEmpty()) { - conn.setRequestMethod(HttpMethod.POST); - conn.setRequestProperty("Content-type", MediaType.TEXT_PLAIN); - conn.setRequestProperty(HttpHeaders.ACCEPT, MediaType.MULTIPART_FORM_DATA); - conn.setDoOutput(true); - try (OutputStream output = conn.getOutputStream(); - Writer writer = new OutputStreamWriter(output)) { - for (Pair pair : md5sums) { - writer.write(pair.first); - writer.write(' '); - writer.write(pair.second); - writer.write('\n'); + } + final long end = System.currentTimeMillis(); + if (md5sums.size() > 0) { + LOG.debug("count md5sums of {} files, time: {}ms", md5sums.size(), (end - start)); + } + // Create the main request + HttpJsonRequest request = provider.fromUrl(downloadUrl).setTimeout(READ_TIMEOUT); + // handle case where checksums are sent + if (!md5sums.isEmpty()) { + request.usePostMethod(); + request.addHeader(HttpHeaders.CONTENT_TYPE, MediaType.TEXT_PLAIN); + request.addHeader(HttpHeaders.ACCEPT, MediaType.MULTIPART_FORM_DATA); + request.setBodyWriter(new BodyWriter() { + @Override + public void writeTo(OutputStream output) throws IOException { + try (Writer writer = new OutputStreamWriter(output)) { + for (Pair pair : md5sums) { + writer.write(pair.first); + writer.write(' '); + writer.write(pair.second); + writer.write('\n'); + } } } - } + }); + } + try (HttpResponse conn = request.requestGeneral()) { final int responseCode = conn.getResponseCode(); if (responseCode == HttpURLConnection.HTTP_OK) { final String contentType = conn.getHeaderField("content-type"); @@ -314,10 +320,6 @@ private void download(String downloadUrl, java.io.File downloadTo, File director } } catch (ParseException | JsonParseException e) { throw new IOException(e.getMessage(), e); - } finally { - if (conn != null) { - conn.disconnect(); - } } } diff --git a/platform-api/che-core-api-builder/src/test/java/org/eclipse/che/api/builder/BuilderTest.java b/platform-api/che-core-api-builder/src/test/java/org/eclipse/che/api/builder/BuilderTest.java index f7916c4f4..e223472bd 100644 --- a/platform-api/che-core-api-builder/src/test/java/org/eclipse/che/api/builder/BuilderTest.java +++ b/platform-api/che-core-api-builder/src/test/java/org/eclipse/che/api/builder/BuilderTest.java @@ -211,14 +211,12 @@ public void testRemoteBuildSameEnvironment(){ Assert.assertEquals(remoteBuilder.getBuilderEnvironment(), builder.getEnvironments()); } - private void waitForTask(BuildTask task) throws Exception { + private static void waitForTask(BuildTask task) throws Exception { final long end = System.currentTimeMillis() + 5000; - synchronized (this) { - while (!task.isDone()) { - wait(100); - if (System.currentTimeMillis() > end) { - Assert.fail("timeout"); - } + while (!task.isDone()) { + Thread.sleep(100); + if (System.currentTimeMillis() > end) { + Assert.fail("timeout"); } } } diff --git a/platform-api/che-core-api-core/src/main/java/org/eclipse/che/api/core/rest/DefaultHttpJsonRequest.java b/platform-api/che-core-api-core/src/main/java/org/eclipse/che/api/core/rest/DefaultHttpJsonRequest.java index e897b6d28..afd1b2567 100644 --- a/platform-api/che-core-api-core/src/main/java/org/eclipse/che/api/core/rest/DefaultHttpJsonRequest.java +++ b/platform-api/che-core-api-core/src/main/java/org/eclipse/che/api/core/rest/DefaultHttpJsonRequest.java @@ -44,8 +44,10 @@ import java.net.URL; import java.net.URLEncoder; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import static java.util.Objects.requireNonNull; @@ -63,14 +65,15 @@ public class DefaultHttpJsonRequest implements HttpJsonRequest { private static final int DEFAULT_QUERY_PARAMS_LIST_SIZE = 5; - private static final Object[] EMPTY_ARRAY = new Object[0]; private final String url; private int timeout; private String method; private Object body; + private BodyWriter bodyWriter; private List> queryParams; + private Map headers; protected DefaultHttpJsonRequest(String url) { this.url = requireNonNull(url, "Required non-null url"); @@ -91,18 +94,28 @@ public HttpJsonRequest setMethod(@NotNull String method) { @Override public HttpJsonRequest setBody(@NotNull Object body) { this.body = requireNonNull(body, "Required non-null body"); + this.bodyWriter = null; return this; } @Override public HttpJsonRequest setBody(@NotNull Map map) { this.body = new JsonStringMapImpl<>(requireNonNull(map, "Required non-null body")); + this.bodyWriter = null; return this; } @Override public HttpJsonRequest setBody(@NotNull List list) { this.body = new JsonArrayImpl<>(requireNonNull(list, "Required non-null body")); + this.bodyWriter = null; + return this; + } + + @Override + public HttpJsonRequest setBodyWriter(BodyWriter bodyWriter) { + this.bodyWriter = Objects.requireNonNull(bodyWriter, "Required non-null body writer"); + this.body = null; return this; } @@ -116,7 +129,17 @@ public HttpJsonRequest addQueryParam(@NotNull String name, @NotNull Object value queryParams.add(Pair.of(name, value)); return this; } - + + @Override + public HttpJsonRequest addHeader(String name, String value) { + requireNonNull(name, "Required non-null header name"); + if (headers == null) { + headers = new HashMap<>(); + } + headers.put(name, value); + return this; + } + @Override public HttpJsonRequest setTimeout(int timeout) { this.timeout = timeout; @@ -134,9 +157,23 @@ public HttpJsonResponse request() throws IOException, if (method == null) { throw new IllegalStateException("Could not perform request, request method wasn't set"); } + if (bodyWriter != null) { + throw new IllegalStateException("Can not issue a JSON rqeuest in stream mode"); + } return doRequest(timeout, url, method, body, queryParams); } + @Override + public HttpResponse requestGeneral() throws IOException { + if (method == null) { + throw new IllegalStateException("Could not perform request, request method wasn't set"); + } + if (body != null) { + throw new IllegalStateException("Can not issue a stream rqeuest in JSON mode"); + } + return doRequestGeneral(timeout, url, method, bodyWriter, headers, queryParams); + } + /** * Makes this request using {@link HttpURLConnection}. * @@ -181,48 +218,22 @@ protected DefaultHttpJsonResponse doRequest(int timeout, UnauthorizedException, ConflictException, BadRequestException { - final String authToken = getAuthenticationToken(); - final boolean hasQueryParams = parameters != null && !parameters.isEmpty(); - if (hasQueryParams || authToken != null) { - final UriBuilder ub = UriBuilder.fromUri(url); - //remove sensitive information from url. - ub.replaceQueryParam("token", EMPTY_ARRAY); - - if (hasQueryParams) { - for (Pair parameter : parameters) { - String name = URLEncoder.encode(parameter.first, "UTF-8"); - String value = parameter.second == null ? - null : - URLEncoder.encode(String.valueOf(parameter.second), "UTF-8"); - ub.queryParam(name, value); - } - } - url = ub.build().toString(); - } - final HttpURLConnection conn = (HttpURLConnection)new URL(url).openConnection(); - conn.setConnectTimeout(timeout > 0 ? timeout : 60000); - conn.setReadTimeout(timeout > 0 ? timeout : 60000); - try { - conn.setRequestMethod(method); - //drop a hint for server side that we want to receive application/json - conn.addRequestProperty(HttpHeaders.ACCEPT, MediaType.APPLICATION_JSON); - if (authToken != null) { - conn.setRequestProperty(HttpHeaders.AUTHORIZATION, authToken); - } - if (body != null) { - conn.addRequestProperty(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON); - conn.setDoOutput(true); - if (HttpMethod.DELETE.equals(method)) { //to avoid jdk bug described here http://bugs.java.com/view_bug.do?bug_id=7157360 - conn.setRequestMethod(HttpMethod.POST); - conn.setRequestProperty("X-HTTP-Method-Override", HttpMethod.DELETE); - } + Map headers = (this.headers == null ? new HashMap<>() : new HashMap<>(this.headers)); + headers.put(HttpHeaders.ACCEPT, MediaType.APPLICATION_JSON); - try (OutputStream output = conn.getOutputStream()) { - output.write(DtoFactory.getInstance().toJson(body).getBytes()); + BodyWriter bw = null; + if (body != null) { + headers.put(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON); + bw = new BodyWriter() { + @Override + public void writeTo(OutputStream out) throws IOException { + out.write(DtoFactory.getInstance().toJson(body).getBytes()); } - } + }; + } + try (HttpResponse conn = doRequestGeneral(timeout, url, method, bw, headers, parameters)) { final int responseCode = conn.getResponseCode(); if ((responseCode / 100) != 2) { InputStream in = conn.getErrorStream(); @@ -255,22 +266,59 @@ protected DefaultHttpJsonResponse doRequest(int timeout, } // Can't parse content as json or content has format other we expect for error. throw new IOException(String.format("Failed access: %s, method: %s, response code: %d, message: %s", - UriBuilder.fromUri(url).replaceQuery("token").build(), method, responseCode, str)); + url, method, responseCode, str)); } final String contentType = conn.getContentType(); if (contentType != null && !contentType.startsWith(MediaType.APPLICATION_JSON)) { - throw new IOException(conn.getResponseMessage() + " [ Content-Type: " + contentType + " ]"); + throw new IOException(Response.Status.Family.familyOf(conn.getResponseCode()) + " [ Content-Type: " + contentType + " ]"); } try (Reader reader = new InputStreamReader(conn.getInputStream())) { return new DefaultHttpJsonResponse(CharStreams.toString(reader), responseCode); } - } finally { - conn.disconnect(); } } - private String getAuthenticationToken() { + protected HttpResponse doRequestGeneral(int timeout, String url, String method, BodyWriter bodyWriter, + Map headers, List> queryParams) throws IOException { + // Set the query parameters + if (queryParams != null && !queryParams.isEmpty()) { + final UriBuilder ub = UriBuilder.fromUri(url); + for (Pair parameter : queryParams) { + String name = URLEncoder.encode(parameter.first, "UTF-8"); + String value = parameter.second == null ? null + : URLEncoder.encode(String.valueOf(parameter.second), "UTF-8"); + ub.queryParam(name, value); + } + url = ub.build().toString(); + } + // Initialize the connection + URL urlObj = new URL(url); + HttpURLConnection conn = (HttpURLConnection) urlObj.openConnection(); + conn.setConnectTimeout(timeout > 0 ? timeout : DEFAULT_TIMEOUT); + conn.setReadTimeout(timeout > 0 ? timeout : DEFAULT_TIMEOUT); + if (method != null) { + conn.setRequestMethod(method); + } + // Set the authorization header if present + String authToken = getAuthenticationToken(urlObj); + if (authToken != null) { + conn.setRequestProperty(HttpHeaders.AUTHORIZATION, authToken); + } + // Set all the custom headers + if (headers != null) { + headers.forEach(conn::setRequestProperty); + } + // Write the body + if (bodyWriter != null) { + conn.setDoOutput(true); + bodyWriter.writeTo(conn.getOutputStream()); + } + // The result + return new URLConnectionHttpResponse(conn); + } + + protected String getAuthenticationToken(URL urlObj) { final User user = EnvironmentContext.getCurrent().getUser(); if (user != null) { return user.getToken(); diff --git a/platform-api/che-core-api-core/src/main/java/org/eclipse/che/api/core/rest/HttpJsonRequest.java b/platform-api/che-core-api-core/src/main/java/org/eclipse/che/api/core/rest/HttpJsonRequest.java index 45b6b62ff..76b81ff57 100644 --- a/platform-api/che-core-api-core/src/main/java/org/eclipse/che/api/core/rest/HttpJsonRequest.java +++ b/platform-api/che-core-api-core/src/main/java/org/eclipse/che/api/core/rest/HttpJsonRequest.java @@ -24,6 +24,8 @@ import javax.validation.constraints.NotNull; import javax.ws.rs.HttpMethod; import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; import java.util.List; import java.util.Map; import java.util.Objects; @@ -62,6 +64,12 @@ @Beta public interface HttpJsonRequest { + static final int DEFAULT_TIMEOUT = 60 * 1000; + + public interface BodyWriter { + public void writeTo(OutputStream out) throws IOException; + } + /** * Sets http method to use in this request(e.g. {@link javax.ws.rs.HttpMethod#GET GET}). * @@ -108,6 +116,17 @@ public interface HttpJsonRequest { */ HttpJsonRequest setBody(@NotNull List list); + /** + * Copy the given input stream to the request body. + * + * @param bodyWriter + * write data to the request output stream. + * @return this request instance + * @throws NullPointerException + * when {@code body} is null + */ + HttpJsonRequest setBodyWriter(@NotNull BodyWriter bodyWriter); + /** * Adds query parameter to the request. * @@ -121,6 +140,17 @@ public interface HttpJsonRequest { */ HttpJsonRequest addQueryParam(@NotNull String name, @NotNull Object value); + /** + * Adds a header to the request. + * + * @param name + * The name of the header. + * @param value + * The value of the header. + * @return this request instance + */ + HttpJsonRequest addHeader(@NotNull String name, String value); + /** * Sets request timeout. * @@ -160,6 +190,8 @@ HttpJsonResponse request() throws IOException, ConflictException, BadRequestException; + HttpResponse requestGeneral() throws IOException; + /** * Uses {@link HttpMethod#GET} as a request method. * diff --git a/platform-api/che-core-api-core/src/main/java/org/eclipse/che/api/core/rest/HttpResponse.java b/platform-api/che-core-api-core/src/main/java/org/eclipse/che/api/core/rest/HttpResponse.java new file mode 100644 index 000000000..9157bce12 --- /dev/null +++ b/platform-api/che-core-api-core/src/main/java/org/eclipse/che/api/core/rest/HttpResponse.java @@ -0,0 +1,51 @@ +/******************************************************************************* + * Copyright (c) 2012-2016 Codenvy, S.A. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Codenvy, S.A. - initial API and implementation + *******************************************************************************/ +package org.eclipse.che.api.core.rest; + +import java.io.IOException; +import java.io.InputStream; + +import com.google.common.annotations.Beta; + +/** + * Defines response of {@link HttpRequestFactory}. + * + * @author Tareq Sharafy + */ +@Beta +public interface HttpResponse extends AutoCloseable { + + /** + * Returns a response code. + */ + int getResponseCode() throws IOException; + + /** + * The content type of the response data. + */ + String getHeaderField(String name); + + /** + * The value of the Content-Type header. + */ + String getContentType(); + + /** + * Gets a stream of the response body. + */ + InputStream getInputStream() throws IOException; + + InputStream getErrorStream() throws IOException; + + @Override + void close() throws IOException; + +} diff --git a/platform-api/che-core-api-core/src/main/java/org/eclipse/che/api/core/rest/URLConnectionHttpResponse.java b/platform-api/che-core-api-core/src/main/java/org/eclipse/che/api/core/rest/URLConnectionHttpResponse.java new file mode 100644 index 000000000..71c1ea664 --- /dev/null +++ b/platform-api/che-core-api-core/src/main/java/org/eclipse/che/api/core/rest/URLConnectionHttpResponse.java @@ -0,0 +1,64 @@ +/******************************************************************************* + * Copyright (c) 2012-2016 Codenvy, S.A. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Codenvy, S.A. - initial API and implementation + *******************************************************************************/ +package org.eclipse.che.api.core.rest; + +import java.io.IOException; +import java.io.InputStream; +import java.net.HttpURLConnection; + +import javax.net.ssl.HttpsURLConnection; + +/** + * An {@link HttpResponse} implementation that is based on {@link HttpURLConnection} and {@link HttpsURLConnection} + * connections. + * + * @author Tareq Sharafy + * + */ +public class URLConnectionHttpResponse implements HttpResponse { + + private final HttpURLConnection conn; + + public URLConnectionHttpResponse(HttpURLConnection conn) { + this.conn = conn; + } + + @Override + public void close() { + conn.disconnect(); + } + + @Override + public InputStream getInputStream() throws IOException { + return conn.getInputStream(); + } + + @Override + public int getResponseCode() throws IOException { + return conn.getResponseCode(); + } + + @Override + public String getHeaderField(String name) { + return conn.getHeaderField(name); + } + + @Override + public String getContentType() { + return conn.getContentType(); + } + + @Override + public InputStream getErrorStream() throws IOException { + return conn.getErrorStream(); + } + +} diff --git a/platform-api/che-core-api-core/src/main/java/org/eclipse/che/api/core/util/DownloadPlugin.java b/platform-api/che-core-api-core/src/main/java/org/eclipse/che/api/core/util/DownloadPlugin.java index bdc12895e..f6af0164d 100644 --- a/platform-api/che-core-api-core/src/main/java/org/eclipse/che/api/core/util/DownloadPlugin.java +++ b/platform-api/che-core-api-core/src/main/java/org/eclipse/che/api/core/util/DownloadPlugin.java @@ -12,11 +12,14 @@ import java.io.IOException; +import com.google.inject.ImplementedBy; + /** * Downloads remote file. * * @author andrew00x */ +@ImplementedBy(DownloadPluginImpl.class) public interface DownloadPlugin { interface Callback { diff --git a/platform-api/che-core-api-core/src/main/java/org/eclipse/che/api/core/util/DownloadPluginImpl.java b/platform-api/che-core-api-core/src/main/java/org/eclipse/che/api/core/util/DownloadPluginImpl.java new file mode 100644 index 000000000..de46493d2 --- /dev/null +++ b/platform-api/che-core-api-core/src/main/java/org/eclipse/che/api/core/util/DownloadPluginImpl.java @@ -0,0 +1,50 @@ +/******************************************************************************* + * Copyright (c) 2012-2016 Codenvy, S.A. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Codenvy, S.A. - initial API and implementation + *******************************************************************************/ +package org.eclipse.che.api.core.util; + +import java.io.IOException; +import java.util.concurrent.TimeUnit; + +import javax.inject.Singleton; + +import org.eclipse.che.api.core.rest.HttpJsonRequestFactory; +import org.eclipse.che.api.core.rest.HttpResponse; + +/** + * DownloadPlugin that downloads single file. + * + * @author Tareq Sharafy + */ +@Singleton +public class DownloadPluginImpl extends HttpDownloadPlugin { + + private static final int READ_TIMEOUT = (int) TimeUnit.MINUTES.toMillis(3); + + private final HttpJsonRequestFactory provider; + + public DownloadPluginImpl(HttpJsonRequestFactory provider) { + this.provider = provider; + } + + @Override + protected HttpResponse openUrlConnection(String downloadUrl) throws IOException { + // Check it + HttpResponse conn = provider.fromUrl(downloadUrl).setTimeout(READ_TIMEOUT).requestGeneral(); + // Connect + final int responseCode = conn.getResponseCode(); + if (responseCode != 200) { + conn.close(); + throw new IOException(String.format("Invalid response status %d from remote server. ", responseCode)); + } + return conn; + } + +} diff --git a/platform-api/che-core-api-core/src/main/java/org/eclipse/che/api/core/util/HttpDownloadPlugin.java b/platform-api/che-core-api-core/src/main/java/org/eclipse/che/api/core/util/HttpDownloadPlugin.java index 6cc01604e..116c4a8cb 100644 --- a/platform-api/che-core-api-core/src/main/java/org/eclipse/che/api/core/util/HttpDownloadPlugin.java +++ b/platform-api/che-core-api-core/src/main/java/org/eclipse/che/api/core/util/HttpDownloadPlugin.java @@ -10,6 +10,8 @@ *******************************************************************************/ package org.eclipse.che.api.core.util; +import org.eclipse.che.api.core.rest.HttpResponse; +import org.eclipse.che.api.core.rest.URLConnectionHttpResponse; import org.eclipse.che.commons.env.EnvironmentContext; import org.eclipse.che.commons.lang.NameGenerator; import org.slf4j.Logger; @@ -30,17 +32,15 @@ * * @author andrew00x */ -public final class HttpDownloadPlugin implements DownloadPlugin { +public class HttpDownloadPlugin implements DownloadPlugin { private static final Logger LOG = LoggerFactory.getLogger(HttpDownloadPlugin.class); private static final int CONNECT_TIMEOUT = (int)TimeUnit.MINUTES.toMillis(3); - private static final int READ_TIMEOUT = (int)TimeUnit.MINUTES.toMillis(3); + protected static final int READ_TIMEOUT = (int)TimeUnit.MINUTES.toMillis(3); @Override public void download(String downloadUrl, java.io.File downloadTo, Callback callback) { - HttpURLConnection conn = null; - try { - conn = openUrlConnection(downloadUrl); + try (HttpResponse conn = openUrlConnection(downloadUrl)) { final String contentDisposition = conn.getHeaderField(HttpHeaders.CONTENT_DISPOSITION); String fileName = null; if (contentDisposition != null) { @@ -65,18 +65,12 @@ public void download(String downloadUrl, java.io.File downloadTo, Callback callb } catch (IOException e) { LOG.debug(String.format("Failed access: %s, error: %s", downloadUrl, e.getMessage()), e); callback.error(e); - } finally { - if (conn != null) { - conn.disconnect(); - } } } @Override public void download(String downloadUrl, java.io.File downloadTo, String fileName, boolean replaceExisting) throws IOException { - HttpURLConnection conn = null; - try { - conn = openUrlConnection(downloadUrl); + try (HttpResponse conn = openUrlConnection(downloadUrl)) { final java.io.File downloadFile = new java.io.File(downloadTo, fileName); try (InputStream in = conn.getInputStream()) { if (replaceExisting) { @@ -85,14 +79,10 @@ public void download(String downloadUrl, java.io.File downloadTo, String fileNam Files.copy(in, downloadFile.toPath()); } } - } finally { - if (conn != null) { - conn.disconnect(); - } } } - private static HttpURLConnection openUrlConnection(String downloadUrl) throws IOException { + protected HttpResponse openUrlConnection(String downloadUrl) throws IOException { HttpURLConnection conn = (HttpURLConnection)new URL(downloadUrl).openConnection(); // Set timeouts conn.setConnectTimeout(CONNECT_TIMEOUT); @@ -107,6 +97,6 @@ private static HttpURLConnection openUrlConnection(String downloadUrl) throws IO if (responseCode != 200) { throw new IOException(String.format("Invalid response status %d from remote server. ", responseCode)); } - return conn; + return new URLConnectionHttpResponse(conn); } } diff --git a/platform-api/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/FactoryServiceTest.java b/platform-api/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/FactoryServiceTest.java index 4454d10b6..c2135edc5 100644 --- a/platform-api/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/FactoryServiceTest.java +++ b/platform-api/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/FactoryServiceTest.java @@ -123,6 +123,7 @@ public void setUp() throws Exception { editValidator, new LinksHelper(), factoryBuilder, + null, projectManager); when(accountDao.getByMember(anyString())).thenReturn(Arrays.asList(new Member().withRoles(Arrays.asList("account/owner")))); diff --git a/platform-api/che-core-api-runner/src/main/java/org/eclipse/che/api/runner/RemoteRunnerProcess.java b/platform-api/che-core-api-runner/src/main/java/org/eclipse/che/api/runner/RemoteRunnerProcess.java index dd786322a..1923cad79 100644 --- a/platform-api/che-core-api-runner/src/main/java/org/eclipse/che/api/runner/RemoteRunnerProcess.java +++ b/platform-api/che-core-api-runner/src/main/java/org/eclipse/che/api/runner/RemoteRunnerProcess.java @@ -12,20 +12,14 @@ import com.google.common.io.ByteStreams; -import org.eclipse.che.api.core.ConflictException; -import org.eclipse.che.api.core.ForbiddenException; import org.eclipse.che.api.core.NotFoundException; -import org.eclipse.che.api.core.ServerException; -import org.eclipse.che.api.core.UnauthorizedException; -import org.eclipse.che.api.core.rest.HttpJsonHelper; import org.eclipse.che.api.core.rest.HttpJsonRequestFactory; import org.eclipse.che.api.core.rest.HttpOutputMessage; +import org.eclipse.che.api.core.rest.HttpResponse; import org.eclipse.che.api.core.rest.OutputProvider; import org.eclipse.che.api.core.rest.shared.dto.Link; import org.eclipse.che.api.runner.dto.ApplicationProcessDescriptor; import org.eclipse.che.api.runner.internal.Constants; -import org.eclipse.che.commons.env.EnvironmentContext; -import org.eclipse.che.dto.server.DtoFactory; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -37,8 +31,6 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.net.HttpURLConnection; -import java.net.URL; /** * Representation of remote application process. @@ -128,15 +120,7 @@ public void readRecipeFile(OutputProvider output) throws IOException, RunnerExce } private void doRequest(String url, String method, final OutputProvider output) throws IOException { - final HttpURLConnection conn = (HttpURLConnection)new URL(url).openConnection(); - conn.setConnectTimeout(60 * 1000); - conn.setReadTimeout(60 * 1000); - conn.setRequestMethod(method); - final EnvironmentContext context = EnvironmentContext.getCurrent(); - if (context.getUser() != null && context.getUser().getToken() != null) { - conn.setRequestProperty(HttpHeaders.AUTHORIZATION, context.getUser().getToken()); - } - try { + try (HttpResponse conn = requestFactory.fromUrl(url).setTimeout(60 * 1000).setMethod(method).requestGeneral()) { if (output instanceof HttpOutputMessage) { HttpOutputMessage httpOutput = (HttpOutputMessage)output; httpOutput.setStatus(conn.getResponseCode()); @@ -155,9 +139,6 @@ private void doRequest(String url, String method, final OutputProvider output) t OutputStream out = output.getOutputStream()) { ByteStreams.copy(in, out); } - - } finally { - conn.disconnect(); } } } diff --git a/platform-api/che-core-api-runner/src/main/java/org/eclipse/che/api/runner/RunQueue.java b/platform-api/che-core-api-runner/src/main/java/org/eclipse/che/api/runner/RunQueue.java index 7f4a3e930..b465997ba 100644 --- a/platform-api/che-core-api-runner/src/main/java/org/eclipse/che/api/runner/RunQueue.java +++ b/platform-api/che-core-api-runner/src/main/java/org/eclipse/che/api/runner/RunQueue.java @@ -18,15 +18,12 @@ import org.eclipse.che.api.builder.BuilderService; import org.eclipse.che.api.builder.dto.BuildOptions; import org.eclipse.che.api.builder.dto.BuildTaskDescriptor; -import org.eclipse.che.api.core.ConflictException; -import org.eclipse.che.api.core.ForbiddenException; import org.eclipse.che.api.core.NotFoundException; import org.eclipse.che.api.core.ServerException; -import org.eclipse.che.api.core.UnauthorizedException; import org.eclipse.che.api.core.notification.EventService; import org.eclipse.che.api.core.notification.EventSubscriber; -import org.eclipse.che.api.core.rest.HttpJsonHelper; import org.eclipse.che.api.core.rest.HttpJsonRequestFactory; +import org.eclipse.che.api.core.rest.HttpResponse; import org.eclipse.che.api.core.rest.RemoteServiceDescriptor; import org.eclipse.che.api.core.rest.ServiceContext; import org.eclipse.che.api.core.rest.shared.dto.Link; @@ -76,7 +73,6 @@ import static org.eclipse.che.api.runner.RunnerUtils.runnerRequest; import java.io.IOException; -import java.net.HttpURLConnection; import java.net.URL; import java.util.ArrayList; import java.util.HashMap; @@ -1189,7 +1185,7 @@ public String toString() { // >>>>>>>>>>>>>>>>>>>>>>>>>>>>> application start checker - private static class ApplicationUrlChecker implements Runnable { + private class ApplicationUrlChecker implements Runnable { final long taskId; final URL url; final int healthCheckerTimeout; @@ -1215,13 +1211,7 @@ public void run() { } catch (InterruptedException e) { return; } - HttpURLConnection conn = null; - try { - conn = (HttpURLConnection)url.openConnection(); - conn.setRequestMethod(requestMethod); - conn.setConnectTimeout(1000); - conn.setReadTimeout(1000); - + try (HttpResponse conn = requestFactory.fromUrl(url.toString()).setTimeout(1000).setMethod(requestMethod).requestGeneral()) { LOG.debug(String.format("Response code: %d.", conn.getResponseCode())); if (405 == conn.getResponseCode()) { // In case of Method not allowed, we use get instead of HEAD. X-HTTP-Method-Override would be nice but support is @@ -1249,10 +1239,6 @@ public void run() { } } } catch (IOException ignored) { - } finally { - if (conn != null) { - conn.disconnect(); - } } } } diff --git a/platform-api/che-core-api-runner/src/main/java/org/eclipse/che/api/runner/RunnerUtils.java b/platform-api/che-core-api-runner/src/main/java/org/eclipse/che/api/runner/RunnerUtils.java index 60f09461b..a18323512 100644 --- a/platform-api/che-core-api-runner/src/main/java/org/eclipse/che/api/runner/RunnerUtils.java +++ b/platform-api/che-core-api-runner/src/main/java/org/eclipse/che/api/runner/RunnerUtils.java @@ -37,10 +37,10 @@ public static HttpJsonResponse runnerRequest(HttpJsonRequest req) throws RunnerE try { return req.request(); } catch (IOException e) { - throw new RunnerException(e); + throw new RunnerException(e.getMessage(), e); } catch (ServerException | UnauthorizedException | ForbiddenException | NotFoundException | ConflictException | BadRequestException e) { - throw new RunnerException(e.getServiceError()); + throw new RunnerException(e.getMessage(), e); } } diff --git a/platform-api/che-core-api-runner/src/main/java/org/eclipse/che/api/runner/internal/Runner.java b/platform-api/che-core-api-runner/src/main/java/org/eclipse/che/api/runner/internal/Runner.java index fe3859438..ebc545ed3 100644 --- a/platform-api/che-core-api-runner/src/main/java/org/eclipse/che/api/runner/internal/Runner.java +++ b/platform-api/che-core-api-runner/src/main/java/org/eclipse/che/api/runner/internal/Runner.java @@ -35,6 +35,8 @@ import javax.annotation.PostConstruct; import javax.annotation.PreDestroy; +import javax.inject.Inject; + import java.io.IOException; import java.nio.file.Files; import java.util.Collections; @@ -90,6 +92,10 @@ public boolean isValid(DeploymentSources deployment) { private ScheduledExecutorService cleanScheduler; private java.io.File deployDirectory; + @Inject + private DownloadPlugin theDownloadPlugin; + /** @deprecated use {@link #downloadFile(String, java.io.File, String, boolean)} */ + @Deprecated protected final DownloadPlugin downloadPlugin; public Runner(java.io.File deployDirectoryRoot, int cleanupDelay, ResourceAllocators allocators, EventService eventService) { @@ -374,7 +380,7 @@ protected DeploymentSources createDeploymentSources(RunRequest request, java.io. return NO_SOURCES; } final DownloadCallback callback = new DownloadCallback(); - downloadPlugin.download(url, dir, callback); + theDownloadPlugin.download(url, dir, callback); if (callback.getError() != null) { throw callback.getError(); } @@ -413,7 +419,7 @@ public IOException getError() { } protected java.io.File downloadFile(String url, java.io.File downloadDir, String fileName, boolean replaceExisting) throws IOException { - downloadPlugin.download(url, downloadDir, fileName, replaceExisting); + theDownloadPlugin.download(url, downloadDir, fileName, replaceExisting); return new java.io.File(downloadDir, fileName); } diff --git a/platform-api/che-core-api-runner/src/test/java/org/eclipse/che/api/runner/RunQueueTest.java b/platform-api/che-core-api-runner/src/test/java/org/eclipse/che/api/runner/RunQueueTest.java index 10c52f1ca..af4a0bd54 100644 --- a/platform-api/che-core-api-runner/src/test/java/org/eclipse/che/api/runner/RunQueueTest.java +++ b/platform-api/che-core-api-runner/src/test/java/org/eclipse/che/api/runner/RunQueueTest.java @@ -25,6 +25,7 @@ import org.eclipse.che.api.core.rest.DefaultHttpJsonResponse; import org.eclipse.che.api.core.rest.HttpJsonRequest; import org.eclipse.che.api.core.rest.HttpJsonRequestFactory; +import org.eclipse.che.api.core.rest.HttpResponse; import org.eclipse.che.api.core.rest.RemoteServiceDescriptor; import org.eclipse.che.api.core.rest.ServiceContext; import org.eclipse.che.api.core.rest.shared.dto.Link;