Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

OAuth2Client: defer initial token fetch when user interaction is required #8354

Merged
merged 3 commits into from
Apr 18, 2024
Merged

OAuth2Client: defer initial token fetch when user interaction is required #8354

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
36d8519
OAUth2Client: defer initial token fetch when user interaction is requ…
adutra Apr 18, 2024
aebe261
cancel used future on close
adutra Apr 18, 2024
04ed47e
move to finally
adutra Apr 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 13 additions & 3 deletions api/client/src/main/java/org/projectnessie/client/auth/oauth2/OAuth2Client.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ class OAuth2Client implements OAuth2Authenticator, Closeable {
private final HttpClient httpClient;
private final ScheduledExecutorService executor;
private final CompletableFuture<Void> started = new CompletableFuture<>();
private final CompletableFuture<Void> used = new CompletableFuture<>();
/* Visible for testing. */ final AtomicBoolean sleeping = new AtomicBoolean();
private final AtomicBoolean closing = new AtomicBoolean();

Expand All @@ -70,14 +71,21 @@ class OAuth2Client implements OAuth2Authenticator, Closeable {
.getExecutor()
.orElseGet(
() -> new OAuth2TokenRefreshExecutor(config.getBackgroundThreadIdleTimeout()));
currentTokensStage = started.thenApplyAsync((v) -> fetchNewTokens(), this.executor);
// when user interaction is not required, token fetch can happen immediately upon start();
// otherwise, it will be deferred until authenticate() is called the first time.
CompletableFuture<?> ready =
config.getGrantType().requiresUserInteraction()
? CompletableFuture.allOf(started, used)
: started;
currentTokensStage = ready.thenApplyAsync((v) -> fetchNewTokens(), executor);
currentTokensStage
.whenComplete((tokens, error) -> log(error))
.whenComplete((tokens, error) -> maybeScheduleTokensRenewal(tokens));
}

@Override
public AccessToken authenticate() {
used.complete(null);
Instant now = config.getClock().get();
lastAccess = now;
if (sleeping.compareAndSet(true, false)) {
Expand Down Expand Up @@ -270,10 +278,12 @@ private void maybeWarn(String message, Throwable error) {
boolean shouldWarn =
lastWarn == null || Duration.between(lastWarn, now).compareTo(MIN_WARN_INTERVAL) > 0;
if (shouldWarn) {
// defer logging until the client is used to avoid confusing log messages appearing
// before the client is actually used
if (error instanceof HttpClientException) {
LOGGER.warn("{}: {}", message, error.toString());
used.thenRun(() -> LOGGER.warn("{}: {}", message, error.toString()));
dimas-b marked this conversation as resolved.
Show resolved Hide resolved
} else {
LOGGER.warn(message, error);
used.thenRun(() -> LOGGER.warn(message, error));
}
lastWarn = now;
} else {
Expand Down