Skip to content

Commit d86b494

Browse files
Abhi347Abhi347
committed
fix: Fix issues with urlFor method
1 parent 083b917 commit d86b494

File tree

7 files changed

+79
-113
lines changed

7 files changed

+79
-113
lines changed

‎src/main/java/com/spotify/github/http/AbstractGitHubApiClient.java‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import java.time.ZonedDateTime;
3333
import java.util.Map;
3434
import java.util.Objects;
35+
import java.util.Optional;
3536
import java.util.concurrent.CompletableFuture;
3637
import java.util.concurrent.atomic.AtomicBoolean;
3738
import javax.ws.rs.core.MediaType;
@@ -119,21 +120,25 @@ private String getInstallationToken(final String jwtToken, final int installatio
119120
}
120121

121122
/**
122-
* Create a URL for a given path to this Github server.
123+
* Create a URL for a given path to this GitHub server.
123124
*
124125
* @param path relative URI
125126
* @return URL to path on this server
126127
*/
127-
String urlFor(final String path) {
128-
return clientConfig().baseUrl().toString().replaceAll("/+$", "")
129-
+ "/"
130-
+ path.replaceAll("^/+", "");
128+
public Optional<String> urlFor(final String path) {
129+
return clientConfig()
130+
.baseUrl()
131+
.map(
132+
baseUrl -> baseUrl.toString().replaceAll("/+$", "") + "/" + path.replaceAll("^/+", ""));
131133
}
132134

133135
private AccessToken generateInstallationToken(final String jwtToken, final int installationId)
134136
throws Exception {
135137
log.info("Got JWT Token. Now getting Github access_token for installation {}", installationId);
136-
final String url = String.format(urlFor(GET_ACCESS_TOKEN_URL), installationId);
138+
final String accessTokenUrl =
139+
urlFor(GET_ACCESS_TOKEN_URL)
140+
.orElseThrow(() -> new IllegalStateException("No baseUrl defined"));
141+
final String url = String.format(accessTokenUrl, installationId);
137142
final Request request =
138143
new Request.Builder()
139144
.addHeader("Accept", "application/vnd.github.machine-man-preview+json")

‎src/main/java/com/spotify/github/v3/clients/GitHubClient.java‎

Lines changed: 2 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@
4949
import java.io.*;
5050
import java.lang.invoke.MethodHandles;
5151
import java.net.URI;
52-
import java.time.ZonedDateTime;
5352
import java.util.HashMap;
5453
import java.util.List;
5554
import java.util.Map;
@@ -709,20 +708,11 @@ CompletableFuture<Response> delete(final String path, final String data) {
709708
return call(request);
710709
}
711710

712-
/**
713-
* Create a URL for a given path to this Github server.
714-
*
715-
* @param path relative URI
716-
* @return URL to path on this server
717-
*/
718-
String urlFor(final String path) {
719-
return baseUrl.toString().replaceAll("/+$", "") + "/" + path.replaceAll("^/+", "");
720-
}
721-
722711
private Request.Builder requestBuilder(final String path) {
712+
String url = urlFor(path).orElseThrow(() -> new IllegalStateException("No baseUrl defined"));
723713
final Request.Builder builder =
724714
new Request.Builder()
725-
.url(urlFor(path))
715+
.url(url)
726716
.addHeader(HttpHeaders.ACCEPT, MediaType.APPLICATION_JSON)
727717
.addHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON);
728718
builder.addHeader(HttpHeaders.AUTHORIZATION, getAuthorizationHeader(path));
@@ -745,62 +735,6 @@ protected OkHttpClient client() {
745735
return client;
746736
}
747737

748-
private boolean isJwtRequest(final String path) {
749-
return path.startsWith("/app/installation") || path.endsWith("installation");
750-
}
751-
752-
private String getInstallationToken(final String jwtToken, final int installationId)
753-
throws Exception {
754-
755-
AccessToken installationToken = installationTokens.get(installationId);
756-
757-
if (installationToken == null || isExpired(installationToken)) {
758-
log.info(
759-
"Github token for installation {} is either expired or null. Trying to get a new one.",
760-
installationId);
761-
installationToken = generateInstallationToken(jwtToken, installationId);
762-
installationTokens.put(installationId, installationToken);
763-
}
764-
return installationToken.token();
765-
}
766-
767-
private boolean isExpired(final AccessToken token) {
768-
// Adds a few minutes to avoid making calls with an expired token due to clock differences
769-
return token.expiresAt().isBefore(ZonedDateTime.now().plusMinutes(EXPIRY_MARGIN_IN_MINUTES));
770-
}
771-
772-
private AccessToken generateInstallationToken(final String jwtToken, final int installationId)
773-
throws Exception {
774-
log.info("Got JWT Token. Now getting Github access_token for installation {}", installationId);
775-
final String url = String.format(urlFor(GET_ACCESS_TOKEN_URL), installationId);
776-
final Request request =
777-
new Request.Builder()
778-
.addHeader("Accept", "application/vnd.github.machine-man-preview+json")
779-
.addHeader("Authorization", "Bearer " + jwtToken)
780-
.url(url)
781-
.method("POST", RequestBody.create(parse(MediaType.APPLICATION_JSON), ""))
782-
.build();
783-
784-
final Response response = client.newCall(request).execute();
785-
786-
if (!response.isSuccessful()) {
787-
throw new Exception(
788-
String.format(
789-
"Got non-2xx status %s when getting an access token from GitHub: %s",
790-
response.code(), response.message()));
791-
}
792-
793-
if (response.body() == null) {
794-
throw new Exception(
795-
String.format(
796-
"Got empty response body when getting an access token from GitHub, HTTP status was: %s",
797-
response.message()));
798-
}
799-
final String text = response.body().string();
800-
response.body().close();
801-
return Json.create().fromJson(text, AccessToken.class);
802-
}
803-
804738
@Override
805739
protected RequestNotOkException mapException(final Response res, final Request request)
806740
throws IOException {

‎src/main/java/com/spotify/github/v3/clients/GithubPage.java‎

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@
77
* Licensed under the Apache License, Version 2.0 (the "License");
88
* you may not use this file except in compliance with the License.
99
* You may obtain a copy of the License at
10-
*
10+
*
1111
* http://www.apache.org/licenses/LICENSE-2.0
12-
*
12+
*
1313
* Unless required by applicable law or agreed to in writing, software
1414
* distributed under the License is distributed on an "AS IS" BASIS,
1515
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -37,7 +37,6 @@
3737
import java.util.NoSuchElementException;
3838
import java.util.Optional;
3939
import java.util.concurrent.CompletableFuture;
40-
import java.util.stream.Stream;
4140
import okhttp3.ResponseBody;
4241

4342
/**
@@ -117,12 +116,14 @@ public CompletableFuture<Pagination> pagination() {
117116
/** {@inheritDoc} */
118117
@Override
119118
public CompletableFuture<AsyncPage<T>> nextPage() {
119+
String url =
120+
github.urlFor("").orElseThrow(() -> new IllegalStateException("No baseUrl defined"));
120121
return linkMapAsync()
121122
.thenApply(
122123
linkMap -> {
123124
final String nextPath =
124125
Optional.ofNullable(linkMap.get("next"))
125-
.map(nextLink -> nextLink.url().toString().replaceAll(github.urlFor(""), ""))
126+
.map(nextLink -> nextLink.url().toString().replaceAll(url, ""))
126127
.orElseThrow(() -> new NoSuchElementException("Page iteration exhausted"));
127128
return new GithubPage<>(github, nextPath, typeReference);
128129
});
@@ -155,18 +156,19 @@ public Iterator<T> iterator() {
155156
}
156157

157158
private CompletableFuture<Map<String, Link>> linkMapAsync() {
158-
return github
159-
.request(path)
160-
.thenApply(
161-
response -> {
162-
Optional.ofNullable(response.body()).ifPresent(ResponseBody::close);
163-
return Optional.ofNullable(response.headers().get("Link"))
164-
.map(linkHeader -> stream(linkHeader.split(",")))
165-
.orElseGet(Stream::empty)
166-
.map(linkString -> Link.from(linkString.split(";")))
167-
.filter(link -> link.rel().isPresent())
168-
.collect(toMap(link -> link.rel().get(), identity()));
169-
});
159+
return Optional.ofNullable(github.request(path))
160+
.map(
161+
responseFuture ->
162+
responseFuture.thenApply(
163+
response -> {
164+
Optional.ofNullable(response.body()).ifPresent(ResponseBody::close);
165+
return Optional.ofNullable(response.headers().get("Link")).stream()
166+
.flatMap(linkHeader -> stream(linkHeader.split(",")))
167+
.map(linkString -> Link.from(linkString.split(";")))
168+
.filter(link -> link.rel().isPresent())
169+
.collect(toMap(link -> link.rel().get(), identity()));
170+
}))
171+
.orElse(CompletableFuture.completedFuture(Map.of()));
170172
}
171173

172174
private Optional<Integer> pageNumberFromUri(final String uri) {

‎src/test/java/com/spotify/github/v3/clients/GitHubClientTest.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ public void setUp() {
7777
GitHubClient.create(
7878
ImmutableGitHubClientConfig.builder()
7979
.client(client)
80-
.baseUrl(URI.create("http://bogus"))
80+
.baseUrl(URI.create("https://bogus"))
8181
.accessToken("token")
8282
.build());
8383
}
@@ -91,7 +91,7 @@ public void withScopedInstallationIdShouldFailWhenMissingPrivateKey() {
9191
public void testWithScopedInstallationId() throws URISyntaxException {
9292
GitHubClient org =
9393
GitHubClient.create(
94-
new URI("http://apa.bepa.cepa"), "some_key_content".getBytes(), null, null);
94+
new URI("https://apa.bepa.cepa"), "some_key_content".getBytes(), null, null);
9595
GitHubClient scoped = org.withScopeForInstallationId(1);
9696
Assertions.assertTrue(scoped.getPrivateKey().isPresent());
9797
Assertions.assertEquals(org.getPrivateKey().get(), scoped.getPrivateKey().get());
@@ -202,7 +202,7 @@ public void testPutConvertsToClass() throws Throwable {
202202

203203
RepositoryInvitation invitation = future.get();
204204
assertThat(requestCapture.getValue().method(), is("PUT"));
205-
assertThat(requestCapture.getValue().url().toString(), is("http://bogus/collaborators/"));
205+
assertThat(requestCapture.getValue().url().toString(), is("https://bogus/collaborators/"));
206206
assertThat(invitation.id(), is(1));
207207
}
208208

‎src/test/java/com/spotify/github/v3/clients/IssueClientTest.java‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@
77
* Licensed under the Apache License, Version 2.0 (the "License");
88
* you may not use this file except in compliance with the License.
99
* You may obtain a copy of the License at
10-
*
10+
*
1111
* http://www.apache.org/licenses/LICENSE-2.0
12-
*
12+
*
1313
* Unless required by applicable law or agreed to in writing, software
1414
* distributed under the License is distributed on an "AS IS" BASIS,
1515
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -32,17 +32,21 @@
3232
import static org.hamcrest.core.Is.is;
3333
import static org.mockito.ArgumentMatchers.anyString;
3434
import static org.mockito.ArgumentMatchers.eq;
35-
import static org.mockito.Mockito.mock;
35+
import static org.mockito.Mockito.*;
3636
import static org.mockito.Mockito.when;
3737

3838
import com.google.common.collect.Lists;
3939
import com.google.common.io.Resources;
4040
import com.spotify.github.async.Async;
4141
import com.spotify.github.async.AsyncPage;
42+
import com.spotify.github.http.GitHubClientConfig;
43+
import com.spotify.github.http.ImmutableGitHubClientConfig;
4244
import com.spotify.github.jackson.Json;
4345
import com.spotify.github.v3.comment.Comment;
4446
import java.io.IOException;
47+
import java.net.URI;
4548
import java.util.List;
49+
import okhttp3.OkHttpClient;
4650
import okhttp3.Response;
4751
import org.junit.jupiter.api.BeforeEach;
4852
import org.junit.jupiter.api.Test;
@@ -54,9 +58,15 @@ public class IssueClientTest {
5458

5559
@BeforeEach
5660
public void setUp() {
61+
GitHubClientConfig config =
62+
ImmutableGitHubClientConfig.builder()
63+
.baseUrl(URI.create("https://github.com/api/v3"))
64+
.client(mock(OkHttpClient.class))
65+
.build();
5766
github = mock(GitHubClient.class);
67+
when(github.clientConfig()).thenReturn(config);
68+
when(github.urlFor(anyString())).thenCallRealMethod();
5869
when(github.json()).thenReturn(Json.create());
59-
when(github.urlFor("")).thenReturn("https://github.com/api/v3");
6070
issueClient = new IssueClient(github, "someowner", "somerepo");
6171
}
6272

@@ -81,7 +91,8 @@ public void testCommentPaginationSpliterator() throws IOException {
8191
.thenReturn(completedFuture(lastPageResponse));
8292

8393
final Iterable<AsyncPage<Comment>> pageIterator = () -> issueClient.listComments(123);
84-
final List<Comment> listComments = Async.streamFromPaginatingIterable(pageIterator).collect(toList());
94+
final List<Comment> listComments =
95+
Async.streamFromPaginatingIterable(pageIterator).collect(toList());
8596

8697
assertThat(listComments.size(), is(30));
8798
assertThat(listComments.get(0).id(), is(1345268));

‎src/test/java/com/spotify/github/v3/clients/RepositoryClientTest.java‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@
4747
import com.google.common.io.Resources;
4848
import com.spotify.github.async.Async;
4949
import com.spotify.github.async.AsyncPage;
50+
import com.spotify.github.http.GitHubClientConfig;
51+
import com.spotify.github.http.ImmutableGitHubClientConfig;
5052
import com.spotify.github.jackson.Json;
5153
import com.spotify.github.v3.comment.Comment;
5254
import com.spotify.github.v3.prs.PullRequestItem;
@@ -64,19 +66,18 @@
6466
import com.spotify.github.v3.repos.RepositoryTest;
6567
import com.spotify.github.v3.repos.Status;
6668
import com.spotify.github.v3.repos.requests.*;
67-
6869
import java.io.IOException;
6970
import java.io.InputStream;
71+
import java.net.URI;
7072
import java.nio.charset.StandardCharsets;
7173
import java.util.List;
7274
import java.util.Optional;
7375
import java.util.concurrent.CompletableFuture;
7476
import java.util.stream.Collectors;
75-
import okhttp3.MediaType;
76-
import okhttp3.Protocol;
77-
import okhttp3.Request;
78-
import okhttp3.Response;
79-
import okhttp3.ResponseBody;
77+
78+
import com.spotify.github.v3.repos.requests.ImmutableAuthenticatedUserRepositoriesFilter;
79+
import com.spotify.github.v3.repos.requests.ImmutableRepositoryUpdate;
80+
import okhttp3.*;
8081
import org.junit.jupiter.api.BeforeEach;
8182
import org.junit.jupiter.api.Test;
8283
import org.mockito.ArgumentCaptor;
@@ -94,7 +95,14 @@ private static String getFixture(String resource) throws IOException {
9495

9596
@BeforeEach
9697
public void setUp() {
98+
GitHubClientConfig config =
99+
ImmutableGitHubClientConfig.builder()
100+
.baseUrl(URI.create("https://github.com/api/v3"))
101+
.client(mock(OkHttpClient.class))
102+
.build();
97103
github = mock(GitHubClient.class);
104+
when(github.clientConfig()).thenReturn(config);
105+
when(github.urlFor(anyString())).thenCallRealMethod();
98106
repoClient = new RepositoryClient(github, "someowner", "somerepo");
99107
json = Json.create();
100108
when(github.json()).thenReturn(json);
@@ -565,7 +573,7 @@ public void testStatusesPaginationForeach() throws Exception {
565573
final String lastPageBody = loadFixture("clients/statuses_page2.json");
566574
final Response lastPageResponse = createMockResponse(lastPageLink, lastPageBody);
567575

568-
when(github.urlFor("")).thenReturn("https://github.com/api/v3");
576+
when(github.urlFor("")).thenReturn(Optional.of("https://github.com/api/v3"));
569577

570578
when(github.request(
571579
format(

0 commit comments

Comments
 (0)