Skip to content

Commit

Permalink
Fix caching of the Mono requesting an accessToken
Browse files Browse the repository at this point in the history
  • Loading branch information
blacelle committed Sep 23, 2024
1 parent 6f55bcd commit e889763
Show file tree
Hide file tree
Showing 14 changed files with 69 additions and 106 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public void registerBoard(UUID contestId, IKumiteBoard initialBoard) {
}

public IHasBoard makeDynamicBoardHolder(UUID contestId) {
if (!boardRepository.containsKey(contestId)) {
if (!boardRepository.hasContest(contestId)) {
throw new IllegalArgumentException("Unknown contestId=" + contestId);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public interface IBoardRepository {
*/
Optional<IKumiteBoard> putIfAbsent(UUID contestId, IKumiteBoard initialBoard);

boolean containsKey(UUID contestId);
boolean hasContest(UUID contestId);

Optional<IKumiteBoard> getBoard(UUID contestId);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public Optional<IKumiteBoard> putIfAbsent(UUID contestId, IKumiteBoard initialBo
}

@Override
public boolean containsKey(UUID contestId) {
public boolean hasContest(UUID contestId) {
return contestIdToBoard.containsKey(contestId);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public boolean isContender(UUID contestId, UUID playerId) {

@Override
public IHasPlayers makeDynamicHasPlayers(UUID contestId) {
if (!boardRepository.containsKey(contestId)) {
if (!boardRepository.hasContest(contestId)) {
throw new IllegalArgumentException("Unknown contestId=" + contestId);
}
return () -> requireBoard(contestId).snapshotPlayers().stream().map(playerId -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@
webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
@ActiveProfiles({ IKumiteSpringProfiles.P_UNSAFE, IKumiteSpringProfiles.P_INMEMORY, IKumiteSpringProfiles.P_FAKEUSER })
@TestPropertySource(properties = { "kumite.random.seed=123",
"kumite.player.wait_duration_if_no_move" + "=PT0.001S",

KumiteWebclientServerProperties.KEY_PLAYER_CONTESTBASEURL + "=http://localhost:LocalServerPort" })
@Slf4j
public class TestRandomGamingLogic {
Expand All @@ -60,13 +62,16 @@ public void testOptimization() {
UUID playerId = FakePlayerTokens.FAKE_PLAYER_ID1;

KumiteWebclientServerProperties properties = KumiteWebclientServerProperties.forTests(env, randomServerPort);
IKumiteServer kumiteServer = KumiteWebclientServer.fromProperties(properties);
KumiteWebclientServer kumiteServer = KumiteWebclientServer.fromProperties(properties);

IGamingLogic kumitePlayer = new RandomGamingLogic(kumiteServer);
RandomGamingLogic kumitePlayer = new RandomGamingLogic(env, kumiteServer);

Set<UUID> contestIds = kumitePlayer.playOptimizationGames(playerId);

Assertions.assertThat(contestIds).hasSizeGreaterThanOrEqualTo(1);

// This should have its dedicated unitTest
Assertions.assertThat(kumiteServer.getNbAccessTokens()).isEqualTo(1);
}

@Test
Expand All @@ -85,7 +90,7 @@ public void test1v1TurnBased() throws InterruptedException {

KumiteWebclientServerProperties properties = KumiteWebclientServerProperties.forTests(env, randomServerPort);
IKumiteServer kumiteServer = KumiteWebclientServer.fromProperties(properties);
IGamingLogic kumitePlayer = new RandomGamingLogic(kumiteServer);
IGamingLogic kumitePlayer = new RandomGamingLogic(env, kumiteServer);

for (int iPlayer = 0; iPlayer < nbPlayers; iPlayer++) {
UUID playerId = FakePlayerTokens.fakePlayerId(iPlayer);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
@ActiveProfiles({ IKumiteSpringProfiles.P_UNSAFE, IKumiteSpringProfiles.P_REDIS, IKumiteSpringProfiles.P_FAKEUSER })
@TestPropertySource(properties = { "kumite.random.seed=123",
"kumite.player.wait_duration_if_no_move" + "=PT0.001S",
KumiteWebclientServerProperties.KEY_PLAYER_CONTESTBASEURL + "=http://localhost:LocalServerPort",

"rediscloud.url" + "=redis://${spring.data.redis.host}:${spring.data.redis.port}",
Expand All @@ -69,7 +70,7 @@ public void testOptimization() {
KumiteWebclientServerProperties properties = KumiteWebclientServerProperties.forTests(env, randomServerPort);
IKumiteServer kumiteServer = KumiteWebclientServer.fromProperties(properties);

IGamingLogic kumitePlayer = new RandomGamingLogic(kumiteServer);
IGamingLogic kumitePlayer = new RandomGamingLogic(env, kumiteServer);

Set<UUID> contestIds = kumitePlayer.playOptimizationGames(playerId);

Expand All @@ -92,7 +93,7 @@ public void test1v1TurnBased() throws InterruptedException {

KumiteWebclientServerProperties properties = KumiteWebclientServerProperties.forTests(env, randomServerPort);
IKumiteServer kumiteServer = KumiteWebclientServer.fromProperties(properties);
IGamingLogic kumitePlayer = new RandomGamingLogic(kumiteServer);
IGamingLogic kumitePlayer = new RandomGamingLogic(env, kumiteServer);

for (int iPlayer = 0; iPlayer < nbPlayers; iPlayer++) {
UUID playerId = FakePlayerTokens.fakePlayerId(iPlayer);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
@ActiveProfiles({ IKumiteSpringProfiles.P_UNSAFE, IKumiteSpringProfiles.P_INMEMORY, IKumiteSpringProfiles.P_FAKEUSER })
@TestPropertySource(properties = { "kumite.random.seed=123",
"kumite.player.wait_duration_if_no_move" + "=PT0.001S",
KumiteWebclientServerProperties.KEY_PLAYER_CONTESTBASEURL + "=http://localhost:LocalServerPort" })
@Slf4j
public class TestTSPLifecycleThroughRouter {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
KumitePlayerRandomConfiguration.class,
JdkUuidGenerator.class,

RandomGamingLogic.class,

})
@Slf4j
public class KumitePlayerComponentsConfiguration {
Expand All @@ -52,11 +54,6 @@ public IKumiteServer kumiteServer(KumiteWebclientServerProperties kumiteWebclien
kumiteWebclientServerProperties.getRefreshToken());
}

@Bean
public IGamingLogic kumitePlayer(IKumiteServer kumiteServer) {
return new RandomGamingLogic(kumiteServer);
}

@Bean
public Void playGames(IGamingLogic kumitePlayer, KumiteWebclientServerProperties kumiteWebclientServerProperties) {
Set<UUID> playerIds = playerIdFromRefreshToken(kumiteWebclientServerProperties);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
import java.util.UUID;
import java.util.concurrent.ConcurrentSkipListSet;

import org.springframework.core.env.Environment;

import eu.solven.kumite.app.server.IKumiteServer;
import eu.solven.kumite.contest.ContestSearchParameters;
import eu.solven.kumite.contest.ContestView;
Expand All @@ -28,6 +30,7 @@
@AllArgsConstructor
@Slf4j
public class RandomGamingLogic implements IGamingLogic {
final Environment env;
final IKumiteServer kumiteServer;

@Override
Expand Down Expand Up @@ -216,7 +219,7 @@ public Set<UUID> play1v1TurnBasedGames(UUID playerId) {
* @return the duration to sleep before polling again for exampleMoves.
*/
protected Duration waitDurationIfNoMove() {
return Duration.ofMillis(10);
return Duration.parse(env.getProperty("kumite.player.wait_duration_if_no_move", "PT15S"));
}

/**
Expand All @@ -230,4 +233,5 @@ protected Duration waitDurationIfNoMove() {
// WaitForPlayersMove and WaitForSignups would have a `wait:true` flag
return moves.getMoves().values().stream().filter(m -> !Boolean.TRUE.equals(m.get("wait"))).findAny();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@
import java.util.TreeSet;
import java.util.UUID;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Function;
import java.util.function.Supplier;
import java.util.stream.Collectors;

import org.springframework.http.HttpHeaders;
Expand All @@ -34,7 +34,6 @@
import eu.solven.kumite.player.PlayerRawMovesHolder;
import eu.solven.kumite.player.PlayerSearchParameters;
import lombok.extern.slf4j.Slf4j;
import oshi.util.MemoizedSupplier;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;

Expand All @@ -57,7 +56,10 @@ public class KumiteWebclientServer implements IKumiteServer {

final AtomicReference<WebClient> webClientRef = new AtomicReference<>();

final Map<UUID, Supplier<Mono<AccessTokenWrapper>>> playerIdToAccessTokenSupplier = new ConcurrentHashMap<>();
// @VisibleForTesting
final AtomicInteger countAccessToken = new AtomicInteger();

final Map<UUID, Mono<AccessTokenWrapper>> playerIdToAccessTokenSupplier = new ConcurrentHashMap<>();

public static KumiteWebclientServer fromProperties(KumiteWebclientServerProperties properties) {
return new KumiteWebclientServer(properties.getBaseUrl(), properties.getRefreshToken());
Expand Down Expand Up @@ -120,6 +122,8 @@ private Mono<AccessTokenWrapper> requestAccessToken(UUID playerId) {
.header(HttpHeaders.AUTHORIZATION, "Bearer " + refreshToken);

return spec.exchangeToMono(r -> {
countAccessToken.incrementAndGet();

if (!r.statusCode().is2xxSuccessful()) {
throw new IllegalArgumentException("Request rejected: " + r.statusCode());
}
Expand All @@ -131,14 +135,13 @@ private Mono<AccessTokenWrapper> requestAccessToken(UUID playerId) {
// https://stackoverflow.com/questions/65972564/spring-reactive-web-client-rest-request-with-oauth-token-in-case-of-401-response
Mono<AccessTokenWrapper> accessToken(UUID playerId) {
// Return the same Mono for 45 minutes
Function<? super UUID, ? extends Supplier<Mono<AccessTokenWrapper>>> mappingFunction =
k -> MemoizedSupplier.memoize(() -> requestAccessToken(k), Duration.ofMinutes(45));
Supplier<Mono<AccessTokenWrapper>> supplier =
playerIdToAccessTokenSupplier.computeIfAbsent(playerId, mappingFunction);

return supplier.get()
Function<? super UUID, ? extends Mono<AccessTokenWrapper>> mappingFunction = k -> requestAccessToken(k)
// Given Mono will see its value cache for 45 minutes
// BEWARE This will not automatically discard the cached value if we get (for any reason) a 401
.cache(Duration.ofMinutes(45));
Mono<AccessTokenWrapper> supplier = playerIdToAccessTokenSupplier.computeIfAbsent(playerId, mappingFunction);

return supplier;
}

// see GameSearchHandler
Expand Down Expand Up @@ -325,4 +328,8 @@ public Mono<LeaderboardRaw> loadLeaderboard(UUID contestId) {
});
}

public int getNbAccessTokens() {
return countAccessToken.get();
}

}
73 changes: 0 additions & 73 deletions player/src/main/java/oshi/util/MemoizedSupplier.java

This file was deleted.

3 changes: 3 additions & 0 deletions player/src/main/resources/application.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,7 @@ kumite.player:
# One need to generate a refresh tken from the `js` app, on route `/html/me`
refresh_token: "NEEDS_A_PROPER_VALUE"

# https://stackoverflow.com/questions/30571319/spring-boot-logging-pattern
# https://github.com/spring-projects/spring-boot/blob/main/spring-boot-project/spring-boot/src/main/resources/org/springframework/boot/logging/logback/defaults.xml#L15C48-L15C67
logging.pattern.console: "%clr(%d{yyyy-MM-dd'T'HH:mm:ss}){faint} %clr(%5p) %clr(){faint}%clr(%-40.40logger{39}){cyan}|%method\\(%line\\)%clr(:){faint} %m%n%wEx"

Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public Optional<IKumiteBoard> putIfAbsent(UUID contestId, IKumiteBoard initialBo
}

@Override
public boolean containsKey(UUID contestId) {
public boolean hasContest(UUID contestId) {
return redisTemplate.hasKey(key(contestId));
}

Expand All @@ -59,7 +59,7 @@ public Optional<IKumiteBoard> getBoard(UUID contestId) {

@Override
public void updateBoard(UUID contestId, IKumiteBoard currentBoard) {
valueOp(contestId).setIfPresent(currentBoard);
valueOp(contestId).setIfPresent(currentBoard, getTtl());
}

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
package eu.solven.kumite.app.webflux.api;

import java.text.ParseException;
import java.util.Map;
import java.util.Map.Entry;
import java.util.UUID;

import org.springframework.http.MediaType;
Expand All @@ -11,6 +14,8 @@
import org.springframework.web.reactive.function.server.ServerRequest;
import org.springframework.web.reactive.function.server.ServerResponse;

import com.nimbusds.jwt.SignedJWT;

import eu.solven.kumite.account.KumiteUser;
import eu.solven.kumite.account.KumiteUsersRegistry;
import eu.solven.kumite.login.AccessTokenWrapper;
Expand All @@ -35,22 +40,35 @@ public Mono<ServerResponse> getAccessToken(ServerRequest request) {
// The playerId authenticated by the accessToken
UUID queryPlayerId = KumiteHandlerHelper.uuid(request, "player_id");

return ReactiveSecurityContextHolder.getContext().map(securityContext -> {
return ReactiveSecurityContextHolder.getContext().flatMap(securityContext -> {
Authentication authentication = securityContext.getAuthentication();

KumiteUser user = userFromRefreshTokenJwt(authentication);
Entry<Jwt, KumiteUser> user = userFromRefreshTokenJwt(authentication);

AccessTokenWrapper tokenWrapper = kumiteTokenService.wrapInJwtAccessToken(user.getValue(), queryPlayerId);

String accessTokenJti = getJti(tokenWrapper);

return user;
}).flatMap(user -> {
AccessTokenWrapper tokenWrapper = kumiteTokenService.wrapInJwtAccessToken(user, queryPlayerId);
log.info("playerId={} Generating access_token.kid={} given refresh_token.kid={}",
queryPlayerId,
accessTokenJti,
user.getKey().getId());

return ServerResponse.ok()
.contentType(MediaType.APPLICATION_JSON)
.body(BodyInserters.fromValue(tokenWrapper));
});
}

private KumiteUser userFromRefreshTokenJwt(Authentication authentication) {
private String getJti(AccessTokenWrapper tokenWrapper) {
try {
return SignedJWT.parse(tokenWrapper.getAccessToken()).getJWTClaimsSet().getJWTID();
} catch (ParseException e) {
throw new IllegalStateException("Issue parsing our own access_token", e);
}
}

private Map.Entry<Jwt, KumiteUser> userFromRefreshTokenJwt(Authentication authentication) {
JwtAuthenticationToken jwtAuthentication = (JwtAuthenticationToken) authentication;

Jwt jwt = jwtAuthentication.getToken();
Expand All @@ -65,7 +83,7 @@ private KumiteUser userFromRefreshTokenJwt(Authentication authentication) {

KumiteUser user = usersRegistry.getUser(accountId);
log.debug("We loaded {} from jti={}", user, jwt.getId());
return user;
return Map.entry(jwt, user);
}

}

0 comments on commit e889763

Please sign in to comment.