Skip to content

Commit

Permalink
A single flow for all outgoing HTTP requests
Browse files Browse the repository at this point in the history 
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 <[email protected]>
Change-Id: I2f334b7bb133b3f136d2b8e5b18301602f427355
  • Loading branch information
@tareksha
tareksha committed Aug 24, 2016
1 parent 988c2c8 commit f385a76
Show file tree
Hide file tree
Showing 18 changed files with 380 additions and 224 deletions.
45 changes: 0 additions & 45 deletions platform-api/che-core-api-builder/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -223,49 +223,4 @@
</plugin>
</plugins>
</build>
<profiles>
<profile>
<id>default</id>
<activation>
<activeByDefault>true</activeByDefault>
</activation>
<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
<excludes>
<exclude>**/*Test.java</exclude>
</excludes>
</configuration>
</plugin>
</plugins>
</build>
</profile>
<profile>
<id>unix</id>
<activation>
<os>
<family>unix</family>
</os>
</activation>
<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
<systemPropertyVariables>
<workDir>${project.build.directory}</workDir>
</systemPropertyVariables>
<includes>
<include>**/*Test.java</include>
</includes>
</configuration>
</plugin>
</plugins>
</build>
</profile>
</profiles>
</project>
4 changes: 2 additions & 2 deletions ...form-api/che-core-api-builder/src/main/java/org/eclipse/che/api/builder/BuilderUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down
18 changes: 2 additions & 16 deletions platform-api/che-core-api-builder/src/main/java/org/eclipse/che/api/builder/RemoteTask.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand All @@ -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();
}
}
}
100 changes: 51 additions & 49 deletions ...re-api-builder/src/main/java/org/eclipse/che/api/builder/internal/SourcesManagerImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -82,19 +88,21 @@ public class SourcesManagerImpl implements SourcesManager {
private final AtomicReference<String> projectKeyHolder;
private final Set<SourceManagerListener> 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
Expand Down Expand Up @@ -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<java.io.File> q = new LinkedList<>();
q.add(downloadTo);
final long start = System.currentTimeMillis();
final List<Pair<String, String>> 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<java.io.File> q = new LinkedList<>();
q.add(downloadTo);
final long start = System.currentTimeMillis();
final List<Pair<String, String>> 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<String, String> 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<String, String> 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");
Expand Down Expand Up @@ -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();
}
}
}

Expand Down
12 changes: 5 additions & 7 deletions platform-api/che-core-api-builder/src/test/java/org/eclipse/che/api/builder/BuilderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
}
}
Expand Down
Loading

0 comments on commit f385a76

Please sign in to comment.