Skip to content

Commit

Permalink
Consistently use Instant for next trigger computation.
Browse files Browse the repository at this point in the history
Deprecate Date-based variant.

Closes gh-831
  • Loading branch information
mp911de committed Nov 1, 2023
1 parent 95381f3 commit e8b4e42
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -366,8 +366,7 @@ private void scheduleRenewal() {
}

private OneShotTrigger createTrigger(TokenWrapper tokenWrapper) {

return new OneShotTrigger(getRefreshTrigger().nextExecutionTime((LoginToken) tokenWrapper.getToken()));
return new OneShotTrigger(getRefreshTrigger().nextExecution((LoginToken) tokenWrapper.getToken()));
}

private static String format(String message, RuntimeException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package org.springframework.vault.authentication;

import java.time.Clock;
import java.time.Duration;
import java.time.Instant;
import java.util.Date;
Expand Down Expand Up @@ -180,19 +181,20 @@ protected static class OneShotTrigger implements Trigger {

private final AtomicBoolean fired = new AtomicBoolean();

private final Date nextExecutionTime;
@Nullable
private final Instant nextExecutionTime;

public OneShotTrigger(Date nextExecutionTime) {
public OneShotTrigger(@Nullable Date nextExecutionTime) {
this(nextExecutionTime != null ? nextExecutionTime.toInstant() : null);
}

public OneShotTrigger(@Nullable Instant nextExecutionTime) {
this.nextExecutionTime = nextExecutionTime;
}

@Override
public Instant nextExecution(TriggerContext triggerContext) {
if (this.fired.compareAndSet(false, true)) {
return this.nextExecutionTime.toInstant();
}

return null;
return this.fired.compareAndSet(false, true) ? this.nextExecutionTime : null;
}

}
Expand All @@ -207,9 +209,25 @@ public interface RefreshTrigger {
* Determine the next execution time according to the given trigger context.
* @param loginToken login token encapsulating renewability and lease duration.
* @return the next execution time as defined by the trigger, or {@code null} if
* the trigger won't fire anymore
* the trigger won't fire anymore.
* @deprecated since 3.1, use {@link #nextExecution(LoginToken) instead}.
*/
Date nextExecutionTime(LoginToken loginToken);
@Nullable
@Deprecated(since = "3.1")
default Date nextExecutionTime(LoginToken loginToken) {
Instant instant = nextExecution(loginToken);
return instant != null ? Date.from(instant) : null;
}

/**
* Determine the next execution time according to the given trigger context.
* @param loginToken login token encapsulating renewability and lease duration.
* @return the next execution time as defined by the trigger, or {@code null} if
* the trigger won't fire anymore.
* @since 3.1
*/
@Nullable
Instant nextExecution(LoginToken loginToken);

/**
* Returns the minimum TTL duration to consider a token valid after renewal.
Expand All @@ -231,6 +249,8 @@ public interface RefreshTrigger {
*/
public static class FixedTimeoutRefreshTrigger implements RefreshTrigger {

private static final Clock CLOCK = Clock.systemDefaultZone();

private static final Duration ONE_SECOND = Duration.ofSeconds(1);

private final Duration duration;
Expand Down Expand Up @@ -289,12 +309,12 @@ public FixedTimeoutRefreshTrigger(Duration refreshBeforeExpiry, Duration expiryT
}

@Override
public Date nextExecutionTime(LoginToken loginToken) {
public Instant nextExecution(LoginToken loginToken) {

long milliseconds = Math.max(ONE_SECOND.toMillis(),
loginToken.getLeaseDuration().toMillis() - this.duration.toMillis());

return new Date(System.currentTimeMillis() + milliseconds);
return CLOCK.instant().plusMillis(milliseconds);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ private void scheduleRenewal(VaultToken token) {

private OneShotTrigger createTrigger(VaultToken token) {

return new OneShotTrigger(getRefreshTrigger().nextExecutionTime((LoginToken) token));
return new OneShotTrigger(getRefreshTrigger().nextExecution((LoginToken) token));
}

private static Mono<VaultToken> augmentWithSelfLookup(WebClient webClient, VaultToken token) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.Executor;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicIntegerFieldUpdater;
import java.util.concurrent.atomic.AtomicReference;
Expand Down Expand Up @@ -1070,6 +1069,8 @@ public boolean isExpired(Clock clock) {
*/
static class OneShotTrigger implements Trigger {

private static final Clock CLOCK = Clock.systemDefaultZone();

private static final AtomicIntegerFieldUpdater<OneShotTrigger> UPDATER = AtomicIntegerFieldUpdater
.newUpdater(OneShotTrigger.class, "status");

Expand All @@ -1089,11 +1090,8 @@ static class OneShotTrigger implements Trigger {
@Nullable
@Override
public Instant nextExecution(TriggerContext triggerContext) {
if (UPDATER.compareAndSet(this, STATUS_ARMED, STATUS_FIRED)) {
return Instant.ofEpochMilli(System.currentTimeMillis() + TimeUnit.SECONDS.toMillis(this.seconds));
}

return null;
return UPDATER.compareAndSet(this, STATUS_ARMED, STATUS_FIRED) ? CLOCK.instant().plusSeconds(this.seconds)
: null;
}

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package org.springframework.vault.authentication;

import java.time.Duration;
import java.time.Instant;
import java.util.Date;
import java.util.concurrent.TimeUnit;

Expand All @@ -37,19 +38,17 @@ void shouldScheduleNextExecutionTimeCorrectly() {

FixedTimeoutRefreshTrigger trigger = new FixedTimeoutRefreshTrigger(5, TimeUnit.SECONDS);

Date nextExecutionTime = trigger.nextExecutionTime(LoginToken.of("foo".toCharArray(), Duration.ofMinutes(1)));
assertThat(nextExecutionTime).isBetween(new Date(System.currentTimeMillis() + TimeUnit.SECONDS.toMillis(52)),
new Date(System.currentTimeMillis() + TimeUnit.SECONDS.toMillis(56)));
Instant nextExecutionTime = trigger.nextExecution(LoginToken.of("foo".toCharArray(), Duration.ofMinutes(1)));
assertThat(nextExecutionTime).isBetween(Instant.now().plusSeconds(52), Instant.now().plusSeconds(56));
}

@Test
void shouldScheduleNextExecutionIfValidityLessThanTimeout() {

FixedTimeoutRefreshTrigger trigger = new FixedTimeoutRefreshTrigger(5, TimeUnit.SECONDS);

Date nextExecutionTime = trigger.nextExecutionTime(LoginToken.of("foo".toCharArray(), Duration.ofSeconds(2)));
assertThat(nextExecutionTime).isBetween(new Date(System.currentTimeMillis() + TimeUnit.SECONDS.toMillis(0)),
new Date(System.currentTimeMillis() + TimeUnit.SECONDS.toMillis(2)));
Instant nextExecutionTime = trigger.nextExecution(LoginToken.of("foo".toCharArray(), Duration.ofSeconds(2)));
assertThat(nextExecutionTime).isBetween(Instant.now(), Instant.now().plusSeconds(2));
}

}

0 comments on commit e8b4e42

Please sign in to comment.