Skip to content

use getResponseBodyAsStream for potential large response #42

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

Closed
fengxx wants to merge 1 commit into from
Closed

use getResponseBodyAsStream for potential large response #42

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
103 changes: 31 additions & 72 deletions .../java/com/cloudbees/jenkins/plugins/bitbucket/server/client/BitbucketServerAPIClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
import java.net.Proxy;
import java.net.URL;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.logging.Level;
import java.util.logging.Logger;
Expand All @@ -60,7 +59,6 @@
import com.cloudbees.jenkins.plugins.bitbucket.api.BitbucketTeam;
import com.cloudbees.jenkins.plugins.bitbucket.api.BitbucketWebHook;
import com.cloudbees.jenkins.plugins.bitbucket.client.repository.UserRoleInRepository;
import com.cloudbees.jenkins.plugins.bitbucket.hooks.BitbucketSCMSourcePushHookReceiver;
import com.cloudbees.jenkins.plugins.bitbucket.server.client.branch.BitbucketServerBranch;
import com.cloudbees.jenkins.plugins.bitbucket.server.client.branch.BitbucketServerBranches;
import com.cloudbees.jenkins.plugins.bitbucket.server.client.branch.BitbucketServerCommit;
Expand All @@ -72,23 +70,9 @@
import hudson.util.Secret;
import jenkins.model.Jenkins;
import net.sf.json.JSONObject;
import org.apache.commons.httpclient.*;
import org.apache.commons.httpclient.auth.AuthScope;
import org.apache.commons.httpclient.methods.*;
import org.apache.commons.io.IOUtils;
import org.apache.commons.lang.StringUtils;
import org.codehaus.jackson.map.ObjectMapper;

import java.io.IOException;
import java.io.InputStream;
import java.io.UnsupportedEncodingException;
import java.net.InetSocketAddress;
import java.net.Proxy;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.logging.Level;
import java.util.logging.Logger;

/**
* Bitbucket API client.
Expand Down Expand Up @@ -118,8 +102,7 @@ public class BitbucketServerAPIClient implements BitbucketApi {
private static final int MAX_PAGES = 100;

/**
* Repository owner.
* This must be null if {@link #project} is not null.
* Repository owner or Project name.
*/
private String owner;

Expand Down Expand Up @@ -252,8 +235,7 @@ public List<BitbucketServerPullRequest> getPullRequests() throws IOException, In
try {
List<BitbucketServerPullRequest> pullRequests = new ArrayList<>();
Integer pageNumber = 1;
String response = getRequest(url);
BitbucketServerPullRequests page = parse(response, BitbucketServerPullRequests.class);
BitbucketServerPullRequests page = getResponse(url,BitbucketServerPullRequests.class);
pullRequests.addAll(page.getValues());
while (!page.isLastPage() && pageNumber < MAX_PAGES) {
if (Thread.interrupted()) {
Expand All @@ -262,8 +244,7 @@ public List<BitbucketServerPullRequest> getPullRequests() throws IOException, In
pageNumber++;
url = String.format(API_PULL_REQUESTS_PATH, getUserCentricOwner(), repositoryName,
page.getNextPageStart());
response = getRequest(url);
page = parse(response, BitbucketServerPullRequests.class);
page = getResponse(url,BitbucketServerPullRequests.class);
pullRequests.addAll(page.getValues());
}
return pullRequests;
Expand All @@ -279,12 +260,7 @@ public List<BitbucketServerPullRequest> getPullRequests() throws IOException, In
@NonNull
public BitbucketPullRequest getPullRequestById(@NonNull Integer id) throws IOException {
String url = String.format(API_PULL_REQUEST_PATH, getUserCentricOwner(), repositoryName, id);
String response = getRequest(url);
try {
return parse(response, BitbucketServerPullRequest.class);
} catch (IOException e) {
throw new IOException("I/O error when accessing URL: " + url, e);
}
return getResponse(url, BitbucketServerPullRequest.class);
}

/**
Expand All @@ -298,12 +274,7 @@ public BitbucketRepository getRepository() throws IOException {
"Cannot get a repository from an API instance that is not associated with a repository");
}
String url = String.format(API_REPOSITORY_PATH, getUserCentricOwner(), repositoryName);
String response = getRequest(url);
try {
return parse(response, BitbucketServerRepository.class);
} catch (IOException e) {
throw new IOException("I/O error when accessing URL: " + url, e);
}
return getResponse(url, BitbucketServerRepository.class);
}

/**
Expand Down Expand Up @@ -334,12 +305,7 @@ public boolean checkPathExists(@NonNull String branch, @NonNull String path) thr
@Override
public String getDefaultBranch() throws IOException {
String url = String.format(API_DEFAULT_BRANCH_PATH, getUserCentricOwner(), repositoryName);
try {
String response = getRequest(url);
return parse(response, BitbucketServerBranch.class).getName();
} catch (IOException e) {
throw new IOException("I/O error when accessing URL: " + url, e);
}
return getResponse(url, BitbucketServerBranch.class).getName();
}

/**
Expand All @@ -353,17 +319,15 @@ public List<BitbucketServerBranch> getBranches() throws IOException, Interrupted
try {
List<BitbucketServerBranch> branches = new ArrayList<>();
Integer pageNumber = 1;
String response = getRequest(url);
BitbucketServerBranches page = parse(response, BitbucketServerBranches.class);
BitbucketServerBranches page = getResponse(url, BitbucketServerBranches.class);
branches.addAll(page.getValues());
while (!page.isLastPage() && pageNumber < MAX_PAGES) {
if (Thread.interrupted()) {
throw new InterruptedException();
}
pageNumber++;
url = String.format(API_BRANCHES_PATH, getUserCentricOwner(), repositoryName, page.getNextPageStart());
response = getRequest(url);
page = parse(response, BitbucketServerBranches.class);
page = getResponse(url, BitbucketServerBranches.class);
branches.addAll(page.getValues());
}
return branches;
Expand All @@ -376,12 +340,7 @@ public List<BitbucketServerBranch> getBranches() throws IOException, Interrupted
@Override
public BitbucketCommit resolveCommit(@NonNull String hash) throws IOException {
String url = String.format(API_COMMITS_PATH, getUserCentricOwner(), repositoryName, hash);
try {
String response = getRequest(url);
return parse(response, BitbucketServerCommit.class);
} catch (IOException e) {
throw new IOException("I/O error when accessing URL: " + url, e);
}
return getResponse(url, BitbucketServerCommit.class);
}

/** {@inheritDoc} */
Expand All @@ -404,8 +363,7 @@ public void removeCommitWebHook(BitbucketWebHook hook) throws IOException, Inter
@NonNull
@Override
public List<? extends BitbucketWebHook> getWebHooks() throws IOException, InterruptedException {
String response = getRequest(String.format(WEBHOOK_REPOSITORY_PATH, getUserCentricOwner(), repositoryName));
return parse(response, BitbucketServerWebhooks.class);
return getResponse(String.format(WEBHOOK_REPOSITORY_PATH, getUserCentricOwner(), repositoryName), BitbucketServerWebhooks.class);
}

/**
Expand All @@ -417,12 +375,7 @@ public BitbucketTeam getTeam() throws IOException {
return null;
} else {
String url = String.format(API_PROJECT_PATH, getOwner());
try {
String response = getRequest(url);
return parse(response, BitbucketServerProject.class);
} catch (IOException e) {
throw new IOException("I/O error when accessing URL: " + url, e);
}
return getResponse(url, BitbucketServerProject.class);
}
}

Expand All @@ -438,17 +391,15 @@ public List<BitbucketServerRepository> getRepositories(@CheckForNull UserRoleInR
try {
List<BitbucketServerRepository> repositories = new ArrayList<>();
Integer pageNumber = 1;
String response = getRequest(url);
BitbucketServerRepositories page = parse(response, BitbucketServerRepositories.class);
BitbucketServerRepositories page = getResponse(url, BitbucketServerRepositories.class);
repositories.addAll(page.getValues());
while (!page.isLastPage() && pageNumber < MAX_PAGES) {
if (Thread.interrupted()) {
throw new InterruptedException();
}
pageNumber++;
url = String.format(API_REPOSITORIES_PATH, getUserCentricOwner(), page.getNextPageStart());
response = getRequest(url);
page = parse(response, BitbucketServerRepositories.class);
page = getResponse(url, BitbucketServerRepositories.class);
repositories.addAll(page.getValues());
}
return repositories;
Expand All @@ -469,28 +420,36 @@ public boolean isPrivate() throws IOException {
return getRepository().isPrivate();
}


private <T> T parse(String response, Class<T> clazz) throws IOException {
ObjectMapper mapper = new ObjectMapper();
return mapper.readValue(response, clazz);
}

private String getRequest(String path) throws IOException {
/**
*
* @param path relative path for bitbucket server api
* @param clazz the class used for deserialization
* @param <T> generic type
* @return the response object
* @throws IOException
*/
private <T> T getResponse(String path, Class<T> clazz) throws IOException {
GetMethod httpget = new GetMethod(this.baseURL + path);
HttpClient client = getHttpClient(getMethodHost(httpget));
try {
Copy link

@kwin kwin Apr 7, 2017

Choose a reason for hiding this comment

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

I would recommend to use the CloseableResponse with try-with-resources (http://stackoverflow.com/questions/21574478/what-is-the-difference-between-closeablehttpclient-and-httpclient-in-apache-http), that way you correctly release the connection even in case of errors

client.executeMethod(httpget);
String response = new String(httpget.getResponseBody(), "UTF-8");
if (httpget.getStatusCode() == HttpStatus.SC_NOT_FOUND) {
throw new FileNotFoundException("URL: " + path);
}
if (httpget.getStatusCode() != HttpStatus.SC_OK) {
throw new BitbucketRequestException(httpget.getStatusCode(), "HTTP request error. Status: " + httpget.getStatusCode() + ": " + httpget.getStatusText() + ".\n" + response);
throw new BitbucketRequestException(httpget.getStatusCode(), "HTTP request error. Status: "
+ httpget.getStatusCode() + ": " + httpget.getStatusText() + ".\n" + httpget.getResponseBodyAsString());
}
return response;
ObjectMapper mapper = new ObjectMapper();
InputStream input = httpget.getResponseBodyAsStream();
T object = mapper.reader(clazz).readValue(input);
input.close();
return object;
} catch (BitbucketRequestException | FileNotFoundException e) {
LOGGER.log(Level.SEVERE, "Request failed for url:" + path, e);
Copy link

@kwin kwin Apr 7, 2017

Choose a reason for hiding this comment

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

Either log or throw but not both

throw e;
} catch (IOException e) {
LOGGER.log(Level.SEVERE, "Communication error for url: " + path, e);
Copy link

@kwin kwin Apr 7, 2017

Choose a reason for hiding this comment

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

Either log or throw but not both

throw new IOException("Communication error for url: " + path, e);
} finally {
httpget.releaseConnection();
Expand Down