Skip to content

Commit

Permalink
Merge pull request #1680 from emrebasar/prepare_request_on_retries
Browse files Browse the repository at this point in the history
Re-Prepare request for every retry
  • Loading branch information
bitwiseman authored Aug 15, 2023
2 parents a84a000 + 3544ce6 commit 238331d
Show file tree
Hide file tree
Showing 11 changed files with 523 additions and 1 deletion.
18 changes: 17 additions & 1 deletion src/main/java/org/kohsuke/github/GitHubClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,6 @@ public <T> GitHubResponse<T> sendRequest(GitHubRequest request, @CheckForNull Bo
throws IOException {
int retries = CONNECTION_ERROR_RETRIES;
GitHubConnectorRequest connectorRequest = prepareConnectorRequest(request);

do {
GitHubConnectorResponse connectorResponse = null;
try {
Expand Down Expand Up @@ -461,6 +460,7 @@ private void detectKnownErrors(GitHubConnectorResponse connectorResponse,
boolean detectStatusCodeError) throws IOException {
detectOTPRequired(connectorResponse);
detectInvalidCached404Response(connectorResponse, request);
detectExpiredToken(connectorResponse, request);
detectRedirect(connectorResponse);
if (rateLimitHandler.isError(connectorResponse)) {
rateLimitHandler.onError(connectorResponse);
Expand All @@ -474,6 +474,22 @@ private void detectKnownErrors(GitHubConnectorResponse connectorResponse,
}
}

private void detectExpiredToken(GitHubConnectorResponse connectorResponse, GitHubRequest request)
throws IOException {
if (connectorResponse.statusCode() != HTTP_UNAUTHORIZED) {
return;
}
String originalAuthorization = connectorResponse.request().header("Authorization");
if (Objects.isNull(originalAuthorization) || originalAuthorization.isEmpty()) {
return;
}
GitHubConnectorRequest updatedRequest = prepareConnectorRequest(request);
String updatedAuthorization = updatedRequest.header("Authorization");
if (!originalAuthorization.equals(updatedAuthorization)) {
throw new RetryRequestException(updatedRequest);
}
}

private void detectRedirect(GitHubConnectorResponse connectorResponse) throws IOException {
if (connectorResponse.statusCode() == HTTP_MOVED_PERM || connectorResponse.statusCode() == HTTP_MOVED_TEMP) {
// GitHubClient depends on GitHubConnector implementations to follow any redirects automatically
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
package org.kohsuke.github.extras.authorization;

import com.github.tomakehurst.wiremock.core.WireMockConfiguration;
import org.junit.Test;
import org.kohsuke.github.AbstractGitHubWireMockTest;
import org.kohsuke.github.GHUser;
import org.kohsuke.github.RateLimitHandler;
import org.kohsuke.github.authorization.AuthorizationProvider;

import java.io.IOException;

/**
* Test authorization token refresh.
*/
public class AuthorizationTokenRefreshTest extends AbstractGitHubWireMockTest {

/**
* Instantiates a new test.
*/
public AuthorizationTokenRefreshTest() {
useDefaultGitHub = false;
}

/**
* Gets the wire mock options.
*
* @return the wire mock options
*/
@Override
protected WireMockConfiguration getWireMockOptions() {
return super.getWireMockOptions().extensions(templating.newResponseTransformer());
}

/**
* Retried request should get new token when the old one expires.
*
* @throws IOException
* the exception
*/
@Test
public void testRetriedRequestGetsNewAuthorizationTokenWhenOldOneExpires() throws IOException {
snapshotNotAllowed();
gitHub = getGitHubBuilder().withAuthorizationProvider(new RefreshingAuthorizationProvider())
.withEndpoint(mockGitHub.apiServer().baseUrl())
.withRateLimitHandler(RateLimitHandler.WAIT)
.build();
final GHUser kohsuke = gitHub.getUser("kohsuke");
assertThat("Usernames match", "kohsuke".equals(kohsuke.getLogin()));
}

/**
* Retried request should not get new token when the old one is still valid.
*
* @throws IOException
* the exception
*/
@Test
public void testRetriedRequestDoesNotGetNewAuthorizationTokenWhenOldOneIsStillValid() throws IOException {
gitHub = getGitHubBuilder().withAuthorizationProvider(() -> "original token")
.withEndpoint(mockGitHub.apiServer().baseUrl())
.withRateLimitHandler(RateLimitHandler.WAIT)
.build();
final GHUser kohsuke = gitHub.getUser("kohsuke");
assertThat("Usernames match", "kohsuke".equals(kohsuke.getLogin()));
}

static class RefreshingAuthorizationProvider implements AuthorizationProvider {
private boolean used = false;

@Override
public String getEncodedAuthorization() {
if (used) {
return "refreshed token";
}
used = true;
return "original token";
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
{
"login": "kohsuke",
"id": 50003,
"node_id": "MDQ6VXNlcjUwMDAz",
"avatar_url": "https://avatars.githubusercontent.com/u/50003?v=4",
"gravatar_id": "",
"url": "https://api.github.com/users/kohsuke",
"html_url": "https://github.com/kohsuke",
"followers_url": "https://api.github.com/users/kohsuke/followers",
"following_url": "https://api.github.com/users/kohsuke/following{/other_user}",
"gists_url": "https://api.github.com/users/kohsuke/gists{/gist_id}",
"starred_url": "https://api.github.com/users/kohsuke/starred{/owner}{/repo}",
"subscriptions_url": "https://api.github.com/users/kohsuke/subscriptions",
"organizations_url": "https://api.github.com/users/kohsuke/orgs",
"repos_url": "https://api.github.com/users/kohsuke/repos",
"events_url": "https://api.github.com/users/kohsuke/events{/privacy}",
"received_events_url": "https://api.github.com/users/kohsuke/received_events",
"type": "User",
"site_admin": false,
"name": "Kohsuke Kawaguchi",
"company": "@launchableinc ",
"blog": "https://www.kohsuke.org/",
"location": "San Jose, California",
"email": null,
"hireable": null,
"bio": null,
"twitter_username": null,
"public_repos": 263,
"public_gists": 113,
"followers": 2110,
"following": 3,
"created_at": "2009-01-28T18:53:21Z",
"updated_at": "2023-08-09T13:37:19Z"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
{
"id": "d37116f2-558c-4543-9d1d-e2241cfcaf54",
"name": "users_kohsuke",
"scenarioName": "Retry with valid credentials",
"requiredScenarioState": "Started",
"newScenarioState": "Retry with the same credentials",
"request": {
"url": "/users/kohsuke",
"method": "GET",
"headers": {
"Accept": {
"equalTo": "application/vnd.github.v3+json"
},
"Authorization": {
"equalTo": "original token"
}
}
},
"response": {
"status": 403,
"headers": {
"Server": "GitHub.com",
"Date": "Thu, 10 Aug 2023 09:12:37 GMT",
"Content-Type": "application/json; charset=utf-8",
"Cache-Control": "public, max-age=60, s-maxage=60",
"Vary": [
"Accept",
"Accept-Encoding, Accept, X-Requested-With"
],
"ETag": "W/\"e2b462e6fe85486beb06033fe196d7c7b7669a96ddbb8f411b91f56288b31b3b\"",
"Last-Modified": "Wed, 09 Aug 2023 13:37:19 GMT",
"X-GitHub-Media-Type": "github.v3; format=json",
"x-github-api-version-selected": "2022-11-28",
"X-RateLimit-Limit": "60",
"X-RateLimit-Remaining": "0",
"X-RateLimit-Reset": "{{testStartDate offset='3 seconds' format='unix'}}",
"X-RateLimit-Used": "1",
"X-RateLimit-Resource": "core",
"Access-Control-Expose-Headers": "ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Used, X-RateLimit-Resource, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type, X-GitHub-SSO, X-GitHub-Request-Id, Deprecation, Sunset",
"Access-Control-Allow-Origin": "*",
"Strict-Transport-Security": "max-age=31536000; includeSubdomains; preload",
"X-Frame-Options": "deny",
"X-Content-Type-Options": "nosniff",
"X-XSS-Protection": "0",
"Referrer-Policy": "origin-when-cross-origin, strict-origin-when-cross-origin",
"Content-Security-Policy": "default-src 'none'",
"X-GitHub-Request-Id": "BDE4:73C9:BF78362:C13B808:64D4AA05"
}
},
"uuid": "d37116f2-558c-4543-9d1d-e2241cfcaf54",
"persistent": true,
"insertionIndex": 1
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
{
"id": "d37116f2-558c-4543-9d1d-e2241cfcaf54",
"name": "users_kohsuke",
"requiredScenarioState": "Retry with the same credentials",
"request": {
"url": "/users/kohsuke",
"method": "GET",
"headers": {
"Accept": {
"equalTo": "application/vnd.github.v3+json"
},
"Authorization": {
"equalTo": "original token"
}
}
},
"response": {
"status": 200,
"bodyFileName": "users_kohsuke-1.json",
"headers": {
"Server": "GitHub.com",
"Date": "Thu, 10 Aug 2023 09:12:37 GMT",
"Content-Type": "application/json; charset=utf-8",
"Cache-Control": "public, max-age=60, s-maxage=60",
"Vary": [
"Accept",
"Accept-Encoding, Accept, X-Requested-With"
],
"ETag": "W/\"e2b462e6fe85486beb06033fe196d7c7b7669a96ddbb8f411b91f56288b31b3b\"",
"Last-Modified": "Wed, 09 Aug 2023 13:37:19 GMT",
"X-GitHub-Media-Type": "github.v3; format=json",
"x-github-api-version-selected": "2022-11-28",
"X-RateLimit-Limit": "60",
"X-RateLimit-Remaining": "59",
"X-RateLimit-Reset": "{{testStartDate offset='1 hours' format='unix'}}",
"X-RateLimit-Used": "1",
"X-RateLimit-Resource": "core",
"Access-Control-Expose-Headers": "ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Used, X-RateLimit-Resource, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type, X-GitHub-SSO, X-GitHub-Request-Id, Deprecation, Sunset",
"Access-Control-Allow-Origin": "*",
"Strict-Transport-Security": "max-age=31536000; includeSubdomains; preload",
"X-Frame-Options": "deny",
"X-Content-Type-Options": "nosniff",
"X-XSS-Protection": "0",
"Referrer-Policy": "origin-when-cross-origin, strict-origin-when-cross-origin",
"Content-Security-Policy": "default-src 'none'",
"X-GitHub-Request-Id": "BDE4:73C9:BF78362:C13B808:64D4AA05"
}
},
"uuid": "d37116f2-558c-4543-9d1d-e2241cfcaf54",
"persistent": true,
"insertionIndex": 1
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
{
"login": "emrebasar",
"id": 3469498,
"node_id": "MDQ6VXNlcjM0Njk0OTg=",
"avatar_url": "https://avatars.githubusercontent.com/u/3469498?v=4",
"gravatar_id": "",
"url": "https://api.github.com/users/emrebasar",
"html_url": "https://github.com/emrebasar",
"followers_url": "https://api.github.com/users/emrebasar/followers",
"following_url": "https://api.github.com/users/emrebasar/following{/other_user}",
"gists_url": "https://api.github.com/users/emrebasar/gists{/gist_id}",
"starred_url": "https://api.github.com/users/emrebasar/starred{/owner}{/repo}",
"subscriptions_url": "https://api.github.com/users/emrebasar/subscriptions",
"organizations_url": "https://api.github.com/users/emrebasar/orgs",
"repos_url": "https://api.github.com/users/emrebasar/repos",
"events_url": "https://api.github.com/users/emrebasar/events{/privacy}",
"received_events_url": "https://api.github.com/users/emrebasar/received_events",
"type": "User",
"site_admin": false,
"name": "R. Emre Basar",
"company": null,
"blog": "",
"location": null,
"email": null,
"hireable": null,
"bio": null,
"twitter_username": null,
"public_repos": 3,
"public_gists": 0,
"followers": 2,
"following": 0,
"created_at": "2013-02-04T08:26:33Z",
"updated_at": "2023-06-29T21:22:45Z"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
{
"login": "kohsuke",
"id": 50003,
"node_id": "MDQ6VXNlcjUwMDAz",
"avatar_url": "https://avatars.githubusercontent.com/u/50003?v=4",
"gravatar_id": "",
"url": "https://api.github.com/users/kohsuke",
"html_url": "https://github.com/kohsuke",
"followers_url": "https://api.github.com/users/kohsuke/followers",
"following_url": "https://api.github.com/users/kohsuke/following{/other_user}",
"gists_url": "https://api.github.com/users/kohsuke/gists{/gist_id}",
"starred_url": "https://api.github.com/users/kohsuke/starred{/owner}{/repo}",
"subscriptions_url": "https://api.github.com/users/kohsuke/subscriptions",
"organizations_url": "https://api.github.com/users/kohsuke/orgs",
"repos_url": "https://api.github.com/users/kohsuke/repos",
"events_url": "https://api.github.com/users/kohsuke/events{/privacy}",
"received_events_url": "https://api.github.com/users/kohsuke/received_events",
"type": "User",
"site_admin": false,
"name": "Kohsuke Kawaguchi",
"company": "@launchableinc ",
"blog": "https://www.kohsuke.org/",
"location": "San Jose, California",
"email": "[email protected]",
"hireable": null,
"bio": null,
"twitter_username": null,
"public_repos": 263,
"public_gists": 113,
"followers": 2098,
"following": 3,
"created_at": "2009-01-28T18:53:21Z",
"updated_at": "2023-05-16T02:35:24Z"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
{
"id": "34cadfa1-f969-4d8d-a855-d4073878d0d8",
"name": "user",
"request": {
"url": "/user",
"method": "GET",
"headers": {
"Accept": {
"equalTo": "application/vnd.github.v3+json"
}
}
},
"response": {
"status": 200,
"bodyFileName": "user-1.json",
"headers": {
"Server": "GitHub.com",
"Date": "Tue, 04 Jul 2023 09:27:51 GMT",
"Content-Type": "application/json; charset=utf-8",
"Cache-Control": "private, max-age=60, s-maxage=60",
"Vary": [
"Accept, Authorization, Cookie, X-GitHub-OTP",
"Accept-Encoding, Accept, X-Requested-With"
],
"ETag": "W/\"1ccca51d2d215d0aa63c9c0e7912237f2e1f42bd17ac8e9ab30f539d9673fd4e\"",
"Last-Modified": "Thu, 29 Jun 2023 21:22:45 GMT",
"X-OAuth-Scopes": "gist, read:org, repo",
"X-Accepted-OAuth-Scopes": "",
"x-oauth-client-id": "178c6fc778ccc68e1d6a",
"X-GitHub-Media-Type": "github.v3; format=json",
"x-github-api-version-selected": "2022-11-28",
"X-RateLimit-Limit": "5000",
"X-RateLimit-Remaining": "4989",
"X-RateLimit-Reset": "1688466174",
"X-RateLimit-Used": "11",
"X-RateLimit-Resource": "core",
"Access-Control-Expose-Headers": "ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Used, X-RateLimit-Resource, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type, X-GitHub-SSO, X-GitHub-Request-Id, Deprecation, Sunset",
"Access-Control-Allow-Origin": "*",
"Strict-Transport-Security": "max-age=31536000; includeSubdomains; preload",
"X-Frame-Options": "deny",
"X-Content-Type-Options": "nosniff",
"X-XSS-Protection": "0",
"Referrer-Policy": "origin-when-cross-origin, strict-origin-when-cross-origin",
"Content-Security-Policy": "default-src 'none'",
"X-GitHub-Request-Id": "BC5B:03C1:15BF2AD:1603E7D:64A3E617"
}
},
"uuid": "34cadfa1-f969-4d8d-a855-d4073878d0d8",
"persistent": true,
"insertionIndex": 1
}
Loading

0 comments on commit 238331d

Please sign in to comment.