Skip to content

Commit

Permalink
Add tests showing cache performance
Browse files Browse the repository at this point in the history
  • Loading branch information
bitwiseman committed Sep 12, 2019
1 parent 9e1f16b commit f0b7e17
Show file tree
Hide file tree
Showing 5 changed files with 325 additions and 48 deletions.
11 changes: 10 additions & 1 deletion src/main/java/org/kohsuke/github/GitHubBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
*
* @since 1.59
*/
public class GitHubBuilder {
public class GitHubBuilder implements Cloneable {

// default scoped so unit tests can read them.
/* private */ String endpoint = GitHub.GITHUB_URL;
Expand Down Expand Up @@ -206,4 +206,13 @@ public HttpURLConnection connect(URL url) throws IOException {
public GitHub build() throws IOException {
return new GitHub(endpoint, user, oauthToken, password, connector, rateLimitHandler, abuseLimitHandler);
}

@Override
public GitHubBuilder clone() {
try {
return (GitHubBuilder) super.clone();
} catch (CloneNotSupportedException e) {
throw new RuntimeException("Clone should be supported", e);
}
}
}
23 changes: 21 additions & 2 deletions src/main/java/org/kohsuke/github/extras/OkHttpConnector.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,22 +34,41 @@
public class OkHttpConnector implements HttpConnector {
private final OkUrlFactory urlFactory;

private final String maxAgeHeaderValue;

public OkHttpConnector(OkUrlFactory urlFactory) {
this(urlFactory, 0);
}

/**
* package private for tests to be able to change max-age for cache.
* @param urlFactory
* @param cacheMaxAge
*/
OkHttpConnector(OkUrlFactory urlFactory, int cacheMaxAge) {
urlFactory.client().setSslSocketFactory(TlsSocketFactory());
urlFactory.client().setConnectionSpecs(TlsConnectionSpecs());
this.urlFactory = urlFactory;

if (cacheMaxAge >= 0 && urlFactory.client() != null) {
maxAgeHeaderValue = "max-age=" + cacheMaxAge;
} else {
maxAgeHeaderValue = null;
}
}


public HttpURLConnection connect(URL url) throws IOException {
HttpURLConnection urlConnection = urlFactory.open(url);
if (urlFactory.client() != null && urlFactory.client().getCache() != null) {
// Cache can be added after client is created so we have to check it for each call
if (maxAgeHeaderValue != null && urlFactory.client().getCache() != null) {
// By default OkHttp honors max-age, meaning it will use local cache
// without checking the network within that time frame.
// However, that can result in stale data being returned during that time so
// we force network-based checking no matter how often the query is made.
// OkHttp still automatically does ETag checking and returns cached data when
// GitHub reports 304, but those do not count against rate limit.
urlConnection.setRequestProperty("Cache-Control", "max-age=0");
urlConnection.setRequestProperty("Cache-Control", maxAgeHeaderValue);
}

return urlConnection;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,7 @@ private static GitHubBuilder createGitHubBuilder() {
}

protected GitHubBuilder getGitHubBuilder() {
return githubBuilder;
}

@Before
public void wireMockSetup() throws Exception {
GitHubBuilder builder = getGitHubBuilder();
GitHubBuilder builder = githubBuilder.clone();

if (!githubApi.isUseProxy()) {
// This sets the user and password to a placeholder for wiremock testing
Expand All @@ -97,12 +92,19 @@ public void wireMockSetup() throws Exception {
builder.withPassword(STUBBED_USER_LOGIN, STUBBED_USER_PASSWORD);
}

return builder;
}

@Before
public void wireMockSetup() throws Exception {
GitHubBuilder builder = getGitHubBuilder()
.withEndpoint(githubApi.baseUrl());

gitHub = builder
.withEndpoint("http://localhost:" + githubApi.port())
.build();

if (githubApi.isUseProxy()) {
gitHubBeforeAfter = builder
gitHubBeforeAfter = getGitHubBuilder()
.withEndpoint("https://api.github.com/")
.build();
} else {
Expand Down
37 changes: 0 additions & 37 deletions src/test/java/org/kohsuke/github/OkHttpConnectorTest.java

This file was deleted.

Loading

0 comments on commit f0b7e17

Please sign in to comment.