Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

A single flow for all outgoing HTTP requests #729

Open
wants to merge 1 commit into
base: 3.x
Choose a base branch
from
Open

A single flow for all outgoing HTTP requests #729

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 0 additions & 45 deletions platform-api/che-core-api-builder/pom.xml
View file
Open in desktop
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
Open in desktop
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
21 changes: 3 additions & 18 deletions platform-api/che-core-api-builder/src/main/java/org/eclipse/che/api/builder/RemoteTask.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,16 @@
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 +213,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.target(url).setTimeout(60 * 1000).request()) {
output.setStatus(conn.getResponseCode());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see where authorization headers is set. Can you point me?

final String contentType = conn.getContentType();
if (contentType != null) {
Expand All @@ -237,13 +225,10 @@ private void readFromUrl(String url, final HttpOutputMessage output) throws IOEx
output.addHttpHeader(HttpHeaders.CONTENT_DISPOSITION, contentDisposition);
}

try (InputStream in = firstNonNull(conn.getErrorStream(), conn.getInputStream());
try (InputStream in = conn.getInputStream();
OutputStream out = output.getOutputStream()) {
ByteStreams.copy(in, out);
}

} finally {
conn.disconnect();
}
}
}
101 changes: 52 additions & 49 deletions ...re-api-builder/src/main/java/org/eclipse/che/api/builder/internal/SourcesManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,12 @@
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.HttpJsonRequestFactory;
import org.eclipse.che.api.core.rest.HttpRequest;
import org.eclipse.che.api.core.rest.HttpRequest.BodyWriter;
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 +33,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 +45,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 +89,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 +211,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
HttpRequest request = provider.target(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.request()) {
final int responseCode = conn.getResponseCode();
if (responseCode == HttpURLConnection.HTTP_OK) {
final String contentType = conn.getHeaderField("content-type");
Expand Down Expand Up @@ -314,10 +321,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
Open in desktop
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