Skip to content

Commit 9a91cc4

Browse files
bitwisemanbitwiseman
authored
Merge pull request hub4j#542 from bitwiseman/cache-fix
Improved OkHttpConnector caching behavior
2 parents 57b58cf + f4cbab5 commit 9a91cc4

108 files changed

Lines changed: 12340 additions & 16 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎src/main/java/org/kohsuke/github/GitHubBuilder.java‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
*
2020
* @since 1.59
2121
*/
22-
public class GitHubBuilder {
22+
public class GitHubBuilder implements Cloneable {
2323

2424
// default scoped so unit tests can read them.
2525
/* private */ String endpoint = GitHub.GITHUB_URL;
@@ -206,4 +206,13 @@ public HttpURLConnection connect(URL url) throws IOException {
206206
public GitHub build() throws IOException {
207207
return new GitHub(endpoint, user, oauthToken, password, connector, rateLimitHandler, abuseLimitHandler);
208208
}
209+
210+
@Override
211+
public GitHubBuilder clone() {
212+
try {
213+
return (GitHubBuilder) super.clone();
214+
} catch (CloneNotSupportedException e) {
215+
throw new RuntimeException("Clone should be supported", e);
216+
}
217+
}
209218
}

‎src/main/java/org/kohsuke/github/extras/OkHttpConnector.java‎

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package org.kohsuke.github.extras;
22

3+
import com.squareup.okhttp.CacheControl;
34
import com.squareup.okhttp.ConnectionSpec;
45
import com.squareup.okhttp.OkHttpClient;
56
import com.squareup.okhttp.OkUrlFactory;
@@ -16,6 +17,7 @@
1617

1718
import java.util.Arrays;
1819
import java.util.List;
20+
import java.util.concurrent.TimeUnit;
1921

2022
import javax.net.ssl.SSLContext;
2123
import javax.net.ssl.SSLSocketFactory;
@@ -32,16 +34,49 @@
3234
* @author Kohsuke Kawaguchi
3335
*/
3436
public class OkHttpConnector implements HttpConnector {
37+
private static final String HEADER_NAME = "Cache-Control";
3538
private final OkUrlFactory urlFactory;
3639

40+
private final String maxAgeHeaderValue;
41+
3742
public OkHttpConnector(OkUrlFactory urlFactory) {
43+
this(urlFactory, 0);
44+
}
45+
46+
/**
47+
* package private for tests to be able to change max-age for cache.
48+
* @param urlFactory
49+
* @param cacheMaxAge
50+
*/
51+
OkHttpConnector(OkUrlFactory urlFactory, int cacheMaxAge) {
3852
urlFactory.client().setSslSocketFactory(TlsSocketFactory());
3953
urlFactory.client().setConnectionSpecs(TlsConnectionSpecs());
4054
this.urlFactory = urlFactory;
55+
56+
if (cacheMaxAge >= 0 && urlFactory.client() != null && urlFactory.client().getCache() != null) {
57+
maxAgeHeaderValue = new CacheControl.Builder()
58+
.maxAge(cacheMaxAge, TimeUnit.SECONDS)
59+
.build()
60+
.toString();
61+
} else {
62+
maxAgeHeaderValue = null;
63+
}
4164
}
4265

66+
4367
public HttpURLConnection connect(URL url) throws IOException {
44-
return urlFactory.open(url);
68+
HttpURLConnection urlConnection = urlFactory.open(url);
69+
if (maxAgeHeaderValue != null) {
70+
// By default OkHttp honors max-age, meaning it will use local cache
71+
// without checking the network within that time frame.
72+
// However, that can result in stale data being returned during that time so
73+
// we force network-based checking no matter how often the query is made.
74+
// OkHttp still automatically does ETag checking and returns cached data when
75+
// GitHub reports 304, but those do not count against rate limit.
76+
urlConnection.setRequestProperty(HEADER_NAME, maxAgeHeaderValue);
77+
}
78+
79+
return urlConnection;
4580
}
4681

4782
/** Returns TLSv1.2 only SSL Socket Factory. */

‎src/test/java/org/kohsuke/github/AbstractGitHubApiWireMockTest.java‎

Lines changed: 30 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import com.github.tomakehurst.wiremock.core.WireMockConfiguration;
55
import com.github.tomakehurst.wiremock.extension.Parameters;
66
import com.github.tomakehurst.wiremock.extension.ResponseTransformer;
7+
import com.github.tomakehurst.wiremock.extension.responsetemplating.ResponseTemplateTransformer;
78
import com.github.tomakehurst.wiremock.http.Request;
89
import com.github.tomakehurst.wiremock.http.Response;
910
import org.apache.commons.io.IOUtils;
@@ -35,6 +36,8 @@ public abstract class AbstractGitHubApiWireMockTest extends Assert {
3536
final static String STUBBED_USER_LOGIN = "placeholder-user";
3637
final static String STUBBED_USER_PASSWORD = "placeholder-password";
3738

39+
protected boolean useDefaultGitHub = true;
40+
3841
/**
3942
* {@link GitHub} instance for use during test.
4043
* Traffic will be part of snapshot when taken.
@@ -52,11 +55,20 @@ public abstract class AbstractGitHubApiWireMockTest extends Assert {
5255
protected final String baseRecordPath = "src/test/resources/" + baseFilesClassPath + "/wiremock";
5356

5457
@Rule
55-
public GitHubApiWireMockRule githubApi = new GitHubApiWireMockRule(
56-
WireMockConfiguration.options()
58+
public final GitHubApiWireMockRule githubApi;
59+
60+
public AbstractGitHubApiWireMockTest() {
61+
githubApi = new GitHubApiWireMockRule(
62+
this.getWireMockOptions()
63+
);
64+
}
65+
66+
protected WireMockConfiguration getWireMockOptions() {
67+
return WireMockConfiguration.options()
5768
.dynamicPort()
58-
.usingFilesUnderDirectory(baseRecordPath)
59-
);
69+
.usingFilesUnderDirectory(baseRecordPath);
70+
};
71+
6072

6173
private static GitHubBuilder createGitHubBuilder() {
6274

@@ -86,12 +98,7 @@ private static GitHubBuilder createGitHubBuilder() {
8698
}
8799

88100
protected GitHubBuilder getGitHubBuilder() {
89-
return githubBuilder;
90-
}
91-
92-
@Before
93-
public void wireMockSetup() throws Exception {
94-
GitHubBuilder builder = getGitHubBuilder();
101+
GitHubBuilder builder = githubBuilder.clone();
95102

96103
if (!githubApi.isUseProxy()) {
97104
// This sets the user and password to a placeholder for wiremock testing
@@ -100,12 +107,21 @@ public void wireMockSetup() throws Exception {
100107
builder.withPassword(STUBBED_USER_LOGIN, STUBBED_USER_PASSWORD);
101108
}
102109

103-
gitHub = builder
104-
.withEndpoint("http://localhost:" + githubApi.port())
105-
.build();
110+
return builder;
111+
}
112+
113+
@Before
114+
public void wireMockSetup() throws Exception {
115+
GitHubBuilder builder = getGitHubBuilder()
116+
.withEndpoint(githubApi.baseUrl());
117+
118+
if (useDefaultGitHub) {
119+
gitHub = builder
120+
.build();
121+
}
106122

107123
if (githubApi.isUseProxy()) {
108-
gitHubBeforeAfter = builder
124+
gitHubBeforeAfter = getGitHubBuilder()
109125
.withEndpoint("https://api.github.com/")
110126
.build();
111127
} else {

0 commit comments

Comments
 (0)