]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-19875 Make direct calls to GitHub to fetch repository team permissions
authorWojtek Wajerowicz <115081248+wojciech-wajerowicz-sonarsource@users.noreply.github.com>
Wed, 12 Jul 2023 12:56:33 +0000 (14:56 +0200)
committersonartech <sonartech@sonarsource.com>
Tue, 18 Jul 2023 20:03:21 +0000 (20:03 +0000)
server/sonar-alm-client/src/main/java/org/sonar/alm/client/github/GithubApplicationHttpClient.java
server/sonar-alm-client/src/main/java/org/sonar/alm/client/github/GithubApplicationHttpClientImpl.java
server/sonar-alm-client/src/test/java/org/sonar/alm/client/github/GithubApplicationClientImplTest.java
server/sonar-alm-client/src/test/java/org/sonar/alm/client/github/GithubApplicationHttpClientImplTest.java

index 86b277ec3659f692328dab82525e10f1fd30fd3d..10c99ee84c32204bddb5197df67b56ccba93f855 100644 (file)
@@ -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();
   }
+
 }
index 65f7984a9ca6514c8fcdab196ae2f4f4cc4ea620..5ab475e6e461fa8b60db45200df53e563846f3e7 100644 (file)
@@ -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;
     }
 
index 2c5e4a4726b3bdd1044f1aae1f108a9def8b4e74..14ab8ce0654d62c6b0ce6100594aac95e9e61b25 100644 (file)
@@ -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;
@@ -979,6 +993,11 @@ public class GithubApplicationClientImplTest {
       return Optional.ofNullable(content);
     }
 
+    @Override
+    public RateLimit getRateLimit() {
+      return RATE_LIMIT;
+    }
+
     @Override
     public Optional<String> getNextEndPoint() {
       return Optional.ofNullable(nextEndPoint);
index 1e90d5b127b3456ac008b7d5adee21aef1d12fbf..55e6a4cc207941b6f114dda90e87b234dcb64252 100644 (file)
@@ -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));
+  }
 }