aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorWojtek Wajerowicz <115081248+wojciech-wajerowicz-sonarsource@users.noreply.github.com>2023-07-12 14:56:33 +0200
committersonartech <sonartech@sonarsource.com>2023-07-18 20:03:21 +0000
commit5ad57b562df44def539788d25d98e4cb0616ccc7 (patch)
tree2e9711b97a8b2213b3a9bb85c005bac6c0bddf26
parent91090d511b75c9336e9102edffd7e9d7dd36a090 (diff)
downloadsonarqube-5ad57b562df44def539788d25d98e4cb0616ccc7.tar.gz
sonarqube-5ad57b562df44def539788d25d98e4cb0616ccc7.zip
SONAR-19875 Make direct calls to GitHub to fetch repository team permissions
-rw-r--r--server/sonar-alm-client/src/main/java/org/sonar/alm/client/github/GithubApplicationHttpClient.java6
-rw-r--r--server/sonar-alm-client/src/main/java/org/sonar/alm/client/github/GithubApplicationHttpClientImpl.java66
-rw-r--r--server/sonar-alm-client/src/test/java/org/sonar/alm/client/github/GithubApplicationClientImplTest.java75
-rw-r--r--server/sonar-alm-client/src/test/java/org/sonar/alm/client/github/GithubApplicationHttpClientImplTest.java65
4 files changed, 168 insertions, 44 deletions
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<String> getContent();
+
+ RateLimit getRateLimit();
}
interface GetResponse extends Response {
Optional<String> 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> T headerValueOrNull(okhttp3.Response response, String header, Function<String, T> 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<String> 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<String> getNextEndPoint() {
return Optional.empty();
@@ -728,6 +732,11 @@ public class GithubApplicationClientImplTest {
public Optional<String> 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<String> getNextEndPoint() {
return Optional.empty();
@@ -920,6 +929,11 @@ public class GithubApplicationClientImplTest {
public Optional<String> getContent() {
return Optional.of(responseJson);
}
+
+ @Override
+ public RateLimit getRateLimit() {
+ return RATE_LIMIT;
+ }
});
Optional<GithubApplicationClient.Repository> 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;
@@ -980,6 +994,11 @@ public class GithubApplicationClientImplTest {
}
@Override
+ public RateLimit getRateLimit() {
+ return RATE_LIMIT;
+ }
+
+ @Override
public Optional<String> 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<Response> 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<Response> 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));
+ }
}