Improved OkHttpConnector caching behavior

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

@@ -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;
@@ -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);
+        }
+    }
 }

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

@@ -1,5 +1,6 @@
 package org.kohsuke.github.extras;
 
+import com.squareup.okhttp.CacheControl;
 import com.squareup.okhttp.ConnectionSpec;
 import com.squareup.okhttp.OkHttpClient;
 import com.squareup.okhttp.OkUrlFactory;
@@ -16,6 +17,7 @@
 
 import java.util.Arrays;
 import java.util.List;
+import java.util.concurrent.TimeUnit;
 
 import javax.net.ssl.SSLContext;
 import javax.net.ssl.SSLSocketFactory;
@@ -32,16 +34,49 @@
  * @author Kohsuke Kawaguchi
  */
 public class OkHttpConnector implements HttpConnector {
+    private static final String HEADER_NAME = "Cache-Control";
     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 && urlFactory.client().getCache() != null) {
+            maxAgeHeaderValue =  new CacheControl.Builder()
+                .maxAge(cacheMaxAge, TimeUnit.SECONDS)
+                .build()
+                .toString();
+        } else {
+            maxAgeHeaderValue = null;
+        }
     }
 
+
     public HttpURLConnection connect(URL url) throws IOException {
-        return urlFactory.open(url);
+        HttpURLConnection urlConnection = urlFactory.open(url);
+        if (maxAgeHeaderValue != 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(HEADER_NAME, maxAgeHeaderValue);
+        }
+
+        return urlConnection;
     }
 
     /** Returns TLSv1.2 only SSL Socket Factory. */

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

@@ -4,6 +4,7 @@
 import com.github.tomakehurst.wiremock.core.WireMockConfiguration;
 import com.github.tomakehurst.wiremock.extension.Parameters;
 import com.github.tomakehurst.wiremock.extension.ResponseTransformer;
+import com.github.tomakehurst.wiremock.extension.responsetemplating.ResponseTemplateTransformer;
 import com.github.tomakehurst.wiremock.http.Request;
 import com.github.tomakehurst.wiremock.http.Response;
 import org.apache.commons.io.IOUtils;
@@ -35,6 +36,8 @@ public abstract class AbstractGitHubApiWireMockTest extends Assert {
     final static String STUBBED_USER_LOGIN = "placeholder-user";
     final static String STUBBED_USER_PASSWORD = "placeholder-password";
 
+    protected boolean useDefaultGitHub = true;
+
     /**
      * {@link GitHub} instance for use during test.
      * Traffic will be part of snapshot when taken.
@@ -52,11 +55,20 @@ public abstract class AbstractGitHubApiWireMockTest extends Assert {
     protected final String baseRecordPath = "src/test/resources/" + baseFilesClassPath + "/wiremock";
 
     @Rule
-    public GitHubApiWireMockRule githubApi = new GitHubApiWireMockRule(
-        WireMockConfiguration.options()
+    public final GitHubApiWireMockRule githubApi;
+
+    public AbstractGitHubApiWireMockTest() {
+        githubApi = new GitHubApiWireMockRule(
+            this.getWireMockOptions()
+        );
+    }
+
+    protected WireMockConfiguration getWireMockOptions() {
+        return WireMockConfiguration.options()
             .dynamicPort()
-            .usingFilesUnderDirectory(baseRecordPath)
-    );
+            .usingFilesUnderDirectory(baseRecordPath);
+    };
+
 
     private static GitHubBuilder createGitHubBuilder() {
 
@@ -86,12 +98,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
@@ -100,12 +107,21 @@ public void wireMockSetup() throws Exception {
             builder.withPassword(STUBBED_USER_LOGIN, STUBBED_USER_PASSWORD);
         }
 
-        gitHub = builder
-            .withEndpoint("http://localhost:" + githubApi.port())
-            .build();
+        return builder;
+    }
+
+    @Before
+    public void wireMockSetup() throws Exception {
+        GitHubBuilder builder = getGitHubBuilder()
+            .withEndpoint(githubApi.baseUrl());
+
+        if (useDefaultGitHub) {
+            gitHub = builder
+                .build();
+        }
 
         if (githubApi.isUseProxy()) {
-            gitHubBeforeAfter = builder
+            gitHubBeforeAfter = getGitHubBuilder()
                 .withEndpoint("https://api.github.com/")
                 .build();
         } else {