Skip to content

Commit 240e363

Browse files
committed
reuse HttpClient in token provider
1 parent fbaf6f4 commit 240e363

File tree

2 files changed

+9
-11
lines changed

2 files changed

+9
-11
lines changed

github-app-installation/src/main/java/com/walmartlabs/concord/github/appinstallation/AccessTokenProvider.java

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44,34 +44,33 @@
4444
import java.net.http.HttpClient;
4545
import java.net.http.HttpRequest;
4646
import java.net.http.HttpResponse;
47+
import java.net.http.HttpResponse.BodyHandlers;
4748
import java.time.Duration;
4849
import java.util.Date;
4950
import java.util.function.BiFunction;
50-
import java.util.function.Supplier;
5151

5252
public class AccessTokenProvider {
5353

5454
private static final Logger log = LoggerFactory.getLogger(AccessTokenProvider.class);
5555

56-
private final Supplier<HttpClient> httpClientSupplier;
56+
private final HttpClient httpClient;
5757
private final Duration httpTimeout;
5858
private final ObjectMapper objectMapper;
5959

6060
public AccessTokenProvider(GitHubAppInstallationConfig cfg, ObjectMapper objectMapper) {
6161
this.httpTimeout = cfg.getHttpClientTimeout();
6262
this.objectMapper = objectMapper;
63-
// TODO: use thread pool for http client. JDK17 HttpClient creates a new thread per client even for synchronous execution
64-
this.httpClientSupplier = () -> HttpClient.newBuilder() // would be nice to re-use client thread-safely
63+
this.httpClient = HttpClient.newBuilder()
6564
.connectTimeout(httpTimeout)
6665
.build();
6766
}
6867

6968
public AccessTokenProvider(GitHubAppInstallationConfig cfg,
7069
ObjectMapper objectMapper,
71-
Supplier<HttpClient> httpClientSupplier) {
70+
HttpClient httpClient) {
7271
this.httpTimeout = cfg.getHttpClientTimeout();
7372
this.objectMapper = objectMapper;
74-
this.httpClientSupplier = httpClientSupplier;
73+
this.httpClient = httpClient;
7574
}
7675

7776
ExternalAuthToken getRepoInstallationToken(GitHubAppAuthConfig app, String orgRepo) throws GitHubAppException {
@@ -162,8 +161,7 @@ private static String generateJWT(GitHubAppAuthConfig auth) throws JOSEException
162161

163162
private <T> T sendRequest(HttpRequest httpRequest, int expectedCode, Class<T> clazz, BiFunction<Integer, String, GitHubAppException> exFun) throws GitHubAppException {
164163
try {
165-
var resp = httpClientSupplier.get()
166-
.send(httpRequest, HttpResponse.BodyHandlers.ofInputStream());
164+
var resp = httpClient.send(httpRequest, BodyHandlers.ofInputStream());
167165
if (resp.statusCode() != expectedCode) {
168166
throw exFun.apply(resp.statusCode(), readBody(resp));
169167
}

github-app-installation/src/test/java/com/walmartlabs/concord/github/appinstallation/AccessTokenProviderTest.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ void test() throws Exception {
7474
when(accessTokenResponse.statusCode()).thenReturn(201);
7575
when(accessTokenResponse.body()).thenReturn(asInputStream(ACCESS_TOKEN_RESPONSE));
7676

77-
var provider = new AccessTokenProvider(CFG, TestConstants.MAPPPER, () -> httpClient);
77+
var provider = new AccessTokenProvider(CFG, TestConstants.MAPPPER, httpClient);
7878

7979
// --
8080

@@ -95,7 +95,7 @@ void testAppNotInstalled() throws Exception {
9595
when(tokenUrlResponse.statusCode()).thenReturn(404);
9696
when(tokenUrlResponse.body()).thenReturn(asInputStream("App is not installed on repo"));
9797

98-
var provider = new AccessTokenProvider(CFG, TestConstants.MAPPPER, () -> httpClient);
98+
var provider = new AccessTokenProvider(CFG, TestConstants.MAPPPER, httpClient);
9999

100100
// --
101101

@@ -118,7 +118,7 @@ void testErrorCreatingToken() throws Exception {
118118
when(accessTokenResponse.statusCode()).thenReturn(500);
119119
when(accessTokenResponse.body()).thenReturn(asInputStream("server error"));
120120

121-
var provider = new AccessTokenProvider(CFG, TestConstants.MAPPPER, () -> httpClient);
121+
var provider = new AccessTokenProvider(CFG, TestConstants.MAPPPER, httpClient);
122122

123123
// --
124124

0 commit comments

Comments
 (0)