From: Wojtek Wajerowicz <115081248+wojciech-wajerowicz-sonarsource@users.noreply.github.com> Date: Wed, 12 Jul 2023 12:56:33 +0000 (+0200) Subject: SONAR-19875 Make direct calls to GitHub to fetch repository team permissions X-Git-Tag: 10.2.0.77647~392 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=5ad57b562df44def539788d25d98e4cb0616ccc7;p=sonarqube.git SONAR-19875 Make direct calls to GitHub to fetch repository team permissions --- diff --git a/server/sonar-alm-client/src/main/java/org/sonar/alm/client/github/GithubApplicationHttpClient.java b/server/sonar-alm-client/src/main/java/org/sonar/alm/client/github/GithubApplicationHttpClient.java index 86b277ec365..10c99ee84c3 100644 --- a/server/sonar-alm-client/src/main/java/org/sonar/alm/client/github/GithubApplicationHttpClient.java +++ b/server/sonar-alm-client/src/main/java/org/sonar/alm/client/github/GithubApplicationHttpClient.java @@ -68,7 +68,10 @@ public interface GithubApplicationHttpClient { */ Response delete(String appUrl, AccessToken token, String endPoint) throws IOException; + record RateLimit(int remaining, int limit, long reset) { + } interface Response { + /** * @return the HTTP code of the response. */ @@ -79,9 +82,12 @@ public interface GithubApplicationHttpClient { * HTTP method (see {@link #get(String, AccessToken, String)} and {@link #post(String, AccessToken, String)}). */ Optional getContent(); + + RateLimit getRateLimit(); } interface GetResponse extends Response { Optional getNextEndPoint(); } + } diff --git a/server/sonar-alm-client/src/main/java/org/sonar/alm/client/github/GithubApplicationHttpClientImpl.java b/server/sonar-alm-client/src/main/java/org/sonar/alm/client/github/GithubApplicationHttpClientImpl.java index 65f7984a9ca..5ab475e6e46 100644 --- a/server/sonar-alm-client/src/main/java/org/sonar/alm/client/github/GithubApplicationHttpClientImpl.java +++ b/server/sonar-alm-client/src/main/java/org/sonar/alm/client/github/GithubApplicationHttpClientImpl.java @@ -23,6 +23,7 @@ import java.io.IOException; import java.net.MalformedURLException; import java.net.URL; import java.util.Optional; +import java.util.function.Function; import java.util.regex.Matcher; import java.util.regex.Pattern; import javax.annotation.CheckForNull; @@ -34,10 +35,10 @@ import okhttp3.Request; import okhttp3.RequestBody; import okhttp3.ResponseBody; import org.apache.commons.lang.StringUtils; -import org.sonar.alm.client.TimeoutConfiguration; -import org.sonar.alm.client.github.security.AccessToken; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.sonar.alm.client.TimeoutConfiguration; +import org.sonar.alm.client.github.security.AccessToken; import org.sonarqube.ws.client.OkHttpClientBuilder; import static com.google.common.base.Preconditions.checkArgument; @@ -56,6 +57,10 @@ public class GithubApplicationHttpClientImpl implements GithubApplicationHttpCli private static final String GH_API_VERSION_HEADER = "X-GitHub-Api-Version"; private static final String GH_API_VERSION = "2022-11-28"; + private static final String GH_RATE_LIMIT_REMAINING_HEADER = "x-ratelimit-remaining"; + private static final String GH_RATE_LIMIT_LIMIT_HEADER = "x-ratelimit-limit"; + private static final String GH_RATE_LIMIT_RESET_HEADER = "x-ratelimit-reset"; + private final OkHttpClient client; public GithubApplicationHttpClientImpl(TimeoutConfiguration timeoutConfiguration) { @@ -80,18 +85,18 @@ public class GithubApplicationHttpClientImpl implements GithubApplicationHttpCli validateEndPoint(endPoint); try (okhttp3.Response response = client.newCall(newGetRequest(appUrl, token, endPoint)).execute()) { int responseCode = response.code(); + RateLimit rateLimit = readRateLimit(response); if (responseCode != HTTP_OK) { String content = StringUtils.trimToNull(attemptReadContent(response)); if (withLog) { LOG.warn("GET response did not have expected HTTP code (was {}): {}", responseCode, content); } - return new GetResponseImpl(responseCode, content, null); + return new GetResponseImpl(responseCode, content, null, rateLimit); } - return new GetResponseImpl(responseCode, readContent(response.body()).orElse(null), readNextEndPoint(response)); + return new GetResponseImpl(responseCode, readContent(response.body()).orElse(null), readNextEndPoint(response), rateLimit); } } - private static void validateEndPoint(String endPoint) { checkArgument(endPoint.startsWith("/") || endPoint.startsWith("http") || endPoint.isEmpty(), "endpoint must start with '/' or 'http'"); @@ -124,12 +129,13 @@ public class GithubApplicationHttpClientImpl implements GithubApplicationHttpCli try (okhttp3.Response response = client.newCall(newDeleteRequest(appUrl, token, endPoint)).execute()) { int responseCode = response.code(); + RateLimit rateLimit = readRateLimit(response); if (responseCode != HTTP_NO_CONTENT) { String content = attemptReadContent(response); LOG.warn("DELETE response did not have expected HTTP code (was {}): {}", responseCode, content); - return new ResponseImpl(responseCode, content); + return new ResponseImpl(responseCode, content, rateLimit); } - return new ResponseImpl(responseCode, null); + return new ResponseImpl(responseCode, null, rateLimit); } } @@ -142,14 +148,15 @@ public class GithubApplicationHttpClientImpl implements GithubApplicationHttpCli try (okhttp3.Response response = client.newCall(newPostRequest(appUrl, token, endPoint, body)).execute()) { int responseCode = response.code(); + RateLimit rateLimit = readRateLimit(response); if (responseCode == HTTP_OK || responseCode == HTTP_CREATED || responseCode == HTTP_ACCEPTED) { - return new ResponseImpl(responseCode, readContent(response.body()).orElse(null)); + return new ResponseImpl(responseCode, readContent(response.body()).orElse(null), rateLimit); } else if (responseCode == HTTP_NO_CONTENT) { - return new ResponseImpl(responseCode, null); + return new ResponseImpl(responseCode, null, rateLimit); } String content = attemptReadContent(response); LOG.warn("POST response did not have expected HTTP code (was {}): {}", responseCode, content); - return new ResponseImpl(responseCode, content); + return new ResponseImpl(responseCode, content, rateLimit); } } @@ -158,14 +165,15 @@ public class GithubApplicationHttpClientImpl implements GithubApplicationHttpCli try (okhttp3.Response response = client.newCall(newPatchRequest(token, appUrl, endPoint, body)).execute()) { int responseCode = response.code(); + RateLimit rateLimit = readRateLimit(response); if (responseCode == HTTP_OK) { - return new ResponseImpl(responseCode, readContent(response.body()).orElse(null)); + return new ResponseImpl(responseCode, readContent(response.body()).orElse(null), rateLimit); } else if (responseCode == HTTP_NO_CONTENT) { - return new ResponseImpl(responseCode, null); + return new ResponseImpl(responseCode, null, rateLimit); } String content = attemptReadContent(response); LOG.warn("PATCH response did not have expected HTTP code (was {}): {}", responseCode, content); - return new ResponseImpl(responseCode, content); + return new ResponseImpl(responseCode, content, rateLimit); } } @@ -231,13 +239,32 @@ public class GithubApplicationHttpClientImpl implements GithubApplicationHttpCli return nextLinkMatcher.group(1); } + @CheckForNull + private static RateLimit readRateLimit(okhttp3.Response response) { + Integer remaining = headerValueOrNull(response, GH_RATE_LIMIT_REMAINING_HEADER, Integer::valueOf); + Integer limit = headerValueOrNull(response, GH_RATE_LIMIT_LIMIT_HEADER, Integer::valueOf); + Long reset = headerValueOrNull(response, GH_RATE_LIMIT_RESET_HEADER, Long::valueOf); + if (remaining == null || limit == null || reset == null) { + return null; + } + return new RateLimit(remaining, limit, reset); + } + + @CheckForNull + private static T headerValueOrNull(okhttp3.Response response, String header, Function mapper) { + return ofNullable(response.header(header)).map(mapper::apply).orElse(null); + } + private static class ResponseImpl implements Response { private final int code; private final String content; - private ResponseImpl(int code, @Nullable String content) { + private final RateLimit rateLimit; + + private ResponseImpl(int code, @Nullable String content, @Nullable RateLimit rateLimit) { this.code = code; this.content = content; + this.rateLimit = rateLimit; } @Override @@ -249,13 +276,20 @@ public class GithubApplicationHttpClientImpl implements GithubApplicationHttpCli public Optional getContent() { return ofNullable(content); } + + @Override + @CheckForNull + public RateLimit getRateLimit() { + return rateLimit; + } + } private static final class GetResponseImpl extends ResponseImpl implements GetResponse { private final String nextEndPoint; - private GetResponseImpl(int code, @Nullable String content, @Nullable String nextEndPoint) { - super(code, content); + private GetResponseImpl(int code, @Nullable String content, @Nullable String nextEndPoint, @Nullable RateLimit rateLimit) { + super(code, content, rateLimit); this.nextEndPoint = nextEndPoint; } diff --git a/server/sonar-alm-client/src/test/java/org/sonar/alm/client/github/GithubApplicationClientImplTest.java b/server/sonar-alm-client/src/test/java/org/sonar/alm/client/github/GithubApplicationClientImplTest.java index 2c5e4a4726b..14ab8ce0654 100644 --- a/server/sonar-alm-client/src/test/java/org/sonar/alm/client/github/GithubApplicationClientImplTest.java +++ b/server/sonar-alm-client/src/test/java/org/sonar/alm/client/github/GithubApplicationClientImplTest.java @@ -31,6 +31,7 @@ import org.junit.Before; import org.junit.ClassRule; import org.junit.Test; import org.junit.runner.RunWith; +import org.sonar.alm.client.github.GithubApplicationHttpClient.RateLimit; import org.sonar.alm.client.github.config.GithubAppConfiguration; import org.sonar.alm.client.github.config.GithubAppInstallation; import org.sonar.alm.client.github.security.AccessToken; @@ -54,39 +55,42 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.sonar.alm.client.github.GithubApplicationHttpClient.GetResponse; @RunWith(DataProviderRunner.class) public class GithubApplicationClientImplTest { private static final String APP_JWT_TOKEN = "APP_TOKEN_JWT"; private static final String PAYLOAD_2_ORGS = """ - [ - { - "id": 1, - "account": { - "login": "org1", - "type": "Organization" - }, - "target_type": "Organization", - "permissions": { - "members": "read", - "metadata": "read" - }, - "suspended_at": "2023-05-30T08:40:55Z" + [ + { + "id": 1, + "account": { + "login": "org1", + "type": "Organization" }, - { - "id": 2, - "account": { - "login": "org2", - "type": "Organization" - }, - "target_type": "Organization", - "permissions": { - "members": "read", - "metadata": "read" - } + "target_type": "Organization", + "permissions": { + "members": "read", + "metadata": "read" + }, + "suspended_at": "2023-05-30T08:40:55Z" + }, + { + "id": 2, + "account": { + "login": "org2", + "type": "Organization" + }, + "target_type": "Organization", + "permissions": { + "members": "read", + "metadata": "read" } - ]"""; + } + ]"""; + + private static final RateLimit RATE_LIMIT = new RateLimit(Integer.MAX_VALUE, Integer.MAX_VALUE, 0L); @ClassRule public static LogTester logTester = new LogTester().setLevel(LoggerLevel.WARN); @@ -713,7 +717,7 @@ public class GithubApplicationClientImplTest { + "}"; when(httpClient.get(appUrl, accessToken, String.format("/search/repositories?q=%s&page=%s&per_page=%s", "world+fork:true+org:github", 1, 100))) - .thenReturn(new GithubApplicationHttpClient.GetResponse() { + .thenReturn(new GetResponse() { @Override public Optional getNextEndPoint() { return Optional.empty(); @@ -728,6 +732,11 @@ public class GithubApplicationClientImplTest { public Optional getContent() { return Optional.of(responseJson); } + + @Override + public RateLimit getRateLimit() { + return RATE_LIMIT; + } }); GithubApplicationClient.Repositories repositories = underTest.listRepositories(appUrl, accessToken, "github", "world", 1, 100); @@ -905,7 +914,7 @@ public class GithubApplicationClientImplTest { + "}"; when(httpClient.get(appUrl, accessToken, "/repos/octocat/Hello-World")) - .thenReturn(new GithubApplicationHttpClient.GetResponse() { + .thenReturn(new GetResponse() { @Override public Optional getNextEndPoint() { return Optional.empty(); @@ -920,6 +929,11 @@ public class GithubApplicationClientImplTest { public Optional getContent() { return Optional.of(responseJson); } + + @Override + public RateLimit getRateLimit() { + return RATE_LIMIT; + } }); Optional repository = underTest.getRepository(appUrl, accessToken, "octocat", "octocat/Hello-World"); @@ -954,7 +968,7 @@ public class GithubApplicationClientImplTest { } } - private static class Response implements GithubApplicationHttpClient.GetResponse { + private static class Response implements GetResponse { private final int code; private final String content; private final String nextEndPoint; @@ -979,6 +993,11 @@ public class GithubApplicationClientImplTest { return Optional.ofNullable(content); } + @Override + public RateLimit getRateLimit() { + return RATE_LIMIT; + } + @Override public Optional getNextEndPoint() { return Optional.ofNullable(nextEndPoint); diff --git a/server/sonar-alm-client/src/test/java/org/sonar/alm/client/github/GithubApplicationHttpClientImplTest.java b/server/sonar-alm-client/src/test/java/org/sonar/alm/client/github/GithubApplicationHttpClientImplTest.java index 1e90d5b127b..55e6a4cc207 100644 --- a/server/sonar-alm-client/src/test/java/org/sonar/alm/client/github/GithubApplicationHttpClientImplTest.java +++ b/server/sonar-alm-client/src/test/java/org/sonar/alm/client/github/GithubApplicationHttpClientImplTest.java @@ -24,6 +24,7 @@ import com.tngtech.java.junit.dataprovider.DataProviderRunner; import com.tngtech.java.junit.dataprovider.UseDataProvider; import java.io.IOException; import java.net.SocketTimeoutException; +import java.util.concurrent.Callable; import okhttp3.mockwebserver.MockResponse; import okhttp3.mockwebserver.MockWebServer; import okhttp3.mockwebserver.RecordedRequest; @@ -47,6 +48,7 @@ import static org.apache.commons.lang.RandomStringUtils.randomAlphabetic; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.Assert.fail; +import static org.sonar.alm.client.github.GithubApplicationHttpClient.RateLimit; @RunWith(DataProviderRunner.class) public class GithubApplicationHttpClientImplTest { @@ -405,4 +407,67 @@ public class GithubApplicationHttpClientImplTest { assertThat(response.getContent()).contains(randomBody); } + @Test + public void get_whenRateLimitHeadersArePresent_returnsRateLimit() throws Exception { + testRateLimitHeader(() -> underTest.get(appUrl, accessToken, randomEndPoint)); + } + + private void testRateLimitHeader(Callable request ) throws Exception { + server.enqueue(new MockResponse().setBody(randomBody) + .setHeader("x-ratelimit-remaining", "1") + .setHeader("x-ratelimit-limit", "10") + .setHeader("x-ratelimit-reset", "1000")); + + Response response = request.call(); + + assertThat(response.getRateLimit()) + .isEqualTo(new RateLimit(1, 10, 1000L)); + } + + @Test + public void get_whenRateLimitHeadersAreMissing_returnsNull() throws Exception { + + testMissingRateLimitHeader(() -> underTest.get(appUrl, accessToken, randomEndPoint)); + + } + + private void testMissingRateLimitHeader(Callable request ) throws Exception { + server.enqueue(new MockResponse().setBody(randomBody)); + + Response response = request.call(); + assertThat(response.getRateLimit()) + .isNull(); + } + + @Test + public void delete_whenRateLimitHeadersArePresent_returnsRateLimit() throws Exception { + testRateLimitHeader(() -> underTest.delete(appUrl, accessToken, randomEndPoint)); + + } + + @Test + public void delete_whenRateLimitHeadersAreMissing_returnsNull() throws Exception { + testMissingRateLimitHeader(() -> underTest.delete(appUrl, accessToken, randomEndPoint)); + + } + + @Test + public void patch_whenRateLimitHeadersArePresent_returnsRateLimit() throws Exception { + testRateLimitHeader(() -> underTest.patch(appUrl, accessToken, randomEndPoint, "body")); + } + + @Test + public void patch_whenRateLimitHeadersAreMissing_returnsNull() throws Exception { + testMissingRateLimitHeader(() -> underTest.patch(appUrl, accessToken, randomEndPoint, "body")); + } + + @Test + public void post_whenRateLimitHeadersArePresent_returnsRateLimit() throws Exception { + testRateLimitHeader(() -> underTest.post(appUrl, accessToken, randomEndPoint)); + } + + @Test + public void post_whenRateLimitHeadersAreMissing_returnsNull() throws Exception { + testMissingRateLimitHeader(() -> underTest.post(appUrl, accessToken, randomEndPoint)); + } }