From d62279a8cf5497a8251290b357726b0630d57a84 Mon Sep 17 00:00:00 2001 From: Mitchell Hentges <110673802+mitchhentgesspotify@users.noreply.github.com> Date: Thu, 12 Sep 2024 14:22:27 +0200 Subject: [PATCH] Add `asAppScopedClient()` function for convenience (#196) * Add `asAppScopedClient()` function for convenience * Add tests, fix missing `getCause()` --- .../java/com/spotify/github/async/Async.java | 17 ++++++ .../github/v3/clients/GitHubClient.java | 35 +++++++++++ .../github/v3/clients/GitHubClientTest.java | 59 +++++++++++++++++++ 3 files changed, 111 insertions(+) diff --git a/src/main/java/com/spotify/github/async/Async.java b/src/main/java/com/spotify/github/async/Async.java index b35ef7bb..cb49608b 100644 --- a/src/main/java/com/spotify/github/async/Async.java +++ b/src/main/java/com/spotify/github/async/Async.java @@ -20,6 +20,8 @@ package com.spotify.github.async; +import java.util.concurrent.CompletableFuture; +import java.util.function.Function; import java.util.stream.Stream; import static java.util.stream.StreamSupport.stream; @@ -34,4 +36,19 @@ public static Stream streamFromPaginatingIterable(final Iterable stream(page.spliterator(), false)); } + + public static CompletableFuture exceptionallyCompose( + final CompletableFuture future, final Function> handler) { + + return future + .handle( + (result, throwable) -> { + if (throwable != null) { + return handler.apply(throwable); + } else { + return CompletableFuture.completedFuture(result); + } + }) + .thenCompose(Function.identity()); + } } diff --git a/src/main/java/com/spotify/github/v3/clients/GitHubClient.java b/src/main/java/com/spotify/github/v3/clients/GitHubClient.java index 8f41df5e..803bfe51 100644 --- a/src/main/java/com/spotify/github/v3/clients/GitHubClient.java +++ b/src/main/java/com/spotify/github/v3/clients/GitHubClient.java @@ -25,10 +25,12 @@ import com.fasterxml.jackson.core.type.TypeReference; import com.spotify.github.Tracer; +import com.spotify.github.async.Async; import com.spotify.github.jackson.Json; import com.spotify.github.v3.Team; import com.spotify.github.v3.User; import com.spotify.github.v3.checks.AccessToken; +import com.spotify.github.v3.checks.Installation; import com.spotify.github.v3.comment.Comment; import com.spotify.github.v3.exceptions.ReadOnlyRepositoryException; import com.spotify.github.v3.exceptions.RequestNotOkException; @@ -53,6 +55,7 @@ import java.util.Objects; import java.util.Optional; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionStage; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Consumer; @@ -71,6 +74,7 @@ public class GitHubClient { private static final int EXPIRY_MARGIN_IN_MINUTES = 5; + private static final int HTTP_NOT_FOUND = 404; private Tracer tracer = NoopTracer.INSTANCE; @@ -367,6 +371,37 @@ public GitHubClient withScopeForInstallationId(final int installationId) { installationId); } + /** + * This is for clients authenticated as a GitHub App: when performing operations, + * the "installation" of the App must be specified. + * This returns a {@code GitHubClient} that has been scoped to the + * user's/organization's installation of the app, if any. + */ + public CompletionStage> asAppScopedClient(final String owner) { + return Async.exceptionallyCompose(this + .createOrganisationClient(owner) + .createGithubAppClient() + .getInstallation() + .thenApply(Installation::id), e -> { + if (e.getCause() instanceof RequestNotOkException && ((RequestNotOkException) e.getCause()).statusCode() == HTTP_NOT_FOUND) { + return this + .createUserClient(owner) + .createGithubAppClient() + .getUserInstallation() + .thenApply(Installation::id); + } + return CompletableFuture.failedFuture(e); + }) + .thenApply(id -> Optional.of(this.withScopeForInstallationId(id))) + .exceptionally( + e -> { + if (e.getCause() instanceof RequestNotOkException && ((RequestNotOkException) e.getCause()).statusCode() == HTTP_NOT_FOUND) { + return Optional.empty(); + } + throw new RuntimeException(e); + }); + } + public GitHubClient withTracer(final Tracer tracer) { this.tracer = tracer; return this; diff --git a/src/test/java/com/spotify/github/v3/clients/GitHubClientTest.java b/src/test/java/com/spotify/github/v3/clients/GitHubClientTest.java index 1492d126..53541217 100644 --- a/src/test/java/com/spotify/github/v3/clients/GitHubClientTest.java +++ b/src/test/java/com/spotify/github/v3/clients/GitHubClientTest.java @@ -22,6 +22,8 @@ import static com.google.common.io.Resources.getResource; import static java.nio.charset.Charset.defaultCharset; +import static java.util.concurrent.CompletableFuture.completedFuture; +import static java.util.concurrent.CompletableFuture.failedFuture; import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.collection.IsMapContaining.hasEntry; @@ -33,6 +35,7 @@ import com.google.common.io.Resources; import com.spotify.github.Tracer; import com.spotify.github.v3.checks.CheckSuiteResponseList; +import com.spotify.github.v3.checks.Installation; import com.spotify.github.v3.exceptions.ReadOnlyRepositoryException; import com.spotify.github.v3.exceptions.RequestNotOkException; import com.spotify.github.v3.repos.CommitItem; @@ -42,6 +45,7 @@ import java.io.IOException; import java.net.URI; import java.net.URISyntaxException; +import java.util.HashMap; import java.util.List; import java.util.Optional; import java.util.concurrent.CompletableFuture; @@ -228,4 +232,59 @@ public void testGetCheckSuites() throws Throwable { assertThat(result.checkSuites().get(0).app().get().slug().get(), is("octoapp")); } + + @Test + void asAppScopedClientGetsUserClientIfOrgClientNotFound() { + var appGithub = GitHubClient.create(client, URI.create("http://bogus"), new byte[] {}, 1); + var githubSpy = spy(appGithub); + + var orgClientMock = mock(OrganisationClient.class); + when(githubSpy.createOrganisationClient("owner")).thenReturn(orgClientMock); + + var appClientMock = mock(GithubAppClient.class); + when(orgClientMock.createGithubAppClient()).thenReturn(appClientMock); + when(appClientMock.getInstallation()).thenReturn(failedFuture(new RequestNotOkException("", "", 404, "", new HashMap<>()))); + + var userClientMock = mock(UserClient.class); + when(githubSpy.createUserClient("owner")).thenReturn(userClientMock); + + var appClientMock2 = mock(GithubAppClient.class); + when(userClientMock.createGithubAppClient()).thenReturn(appClientMock2); + + var installationMock = mock(Installation.class); + when(appClientMock2.getUserInstallation()).thenReturn(completedFuture(installationMock)); + when(installationMock.id()).thenReturn(1); + + var maybeScopedClient = githubSpy.asAppScopedClient("owner").toCompletableFuture().join(); + + Assertions.assertTrue(maybeScopedClient.isPresent()); + verify(githubSpy, times(1)).createOrganisationClient("owner"); + verify(githubSpy, times(1)).createUserClient("owner"); + } + + @Test + void asAppScopedClientReturnsEmptyIfNoInstallation() { + var appGithub = GitHubClient.create(client, URI.create("http://bogus"), new byte[] {}, 1); + var githubSpy = spy(appGithub); + + var orgClientMock = mock(OrganisationClient.class); + when(githubSpy.createOrganisationClient("owner")).thenReturn(orgClientMock); + + var appClientMock = mock(GithubAppClient.class); + when(orgClientMock.createGithubAppClient()).thenReturn(appClientMock); + when(appClientMock.getInstallation()).thenReturn(failedFuture(new RequestNotOkException("", "", 404, "", new HashMap<>()))); + + var userClientMock = mock(UserClient.class); + when(githubSpy.createUserClient("owner")).thenReturn(userClientMock); + + var appClientMock2 = mock(GithubAppClient.class); + when(userClientMock.createGithubAppClient()).thenReturn(appClientMock2); + + var installationMock = mock(Installation.class); + when(appClientMock2.getUserInstallation()).thenReturn(failedFuture(new RequestNotOkException("", "", 404, "", new HashMap<>()))); + when(installationMock.id()).thenReturn(1); + + var maybeScopedClient = githubSpy.asAppScopedClient("owner").toCompletableFuture().join(); + Assertions.assertTrue(maybeScopedClient.isEmpty()); + } }