tflobbe commented on code in PR #1921:
URL: https://github.com/apache/solr/pull/1921#discussion_r1325142428
##########
solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java:
##########
@@ -485,34 +487,75 @@ private Optional<String> getUser() {
}
}
+ private static class CachedToken {
+ Instant generatedAt;
+ String token;
+
+ private CachedToken(Instant generatedAt, String token) {
+ this.generatedAt = generatedAt;
+ this.token = token;
+ }
+ }
+
+ private volatile ConcurrentHashMap<String, AtomicReference<CachedToken>>
cachedV1Tokens =
+ new ConcurrentHashMap<>();
+ private volatile ConcurrentHashMap<String, AtomicReference<CachedToken>>
cachedV2Tokens =
+ new ConcurrentHashMap<>();
+
+ private static final Duration cacheExpiryTime = Duration.ofSeconds(1);
+
+ private String getToken(String usr) {
+ AtomicReference<CachedToken> tokenRef =
+ cachedV1Tokens.computeIfAbsent(usr, u -> new
AtomicReference<>(generateToken(u)));
Review Comment:
I'm wondering if we need to consider limiting the size of this map in some
way? can the different users be ever growing (probably not with the OOTB
authentication, but maybe with plugins?)
##########
solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java:
##########
@@ -485,34 +487,75 @@ private Optional<String> getUser() {
}
}
+ private static class CachedToken {
+ Instant generatedAt;
+ String token;
+
+ private CachedToken(Instant generatedAt, String token) {
+ this.generatedAt = generatedAt;
+ this.token = token;
+ }
+ }
+
+ private volatile ConcurrentHashMap<String, AtomicReference<CachedToken>>
cachedV1Tokens =
+ new ConcurrentHashMap<>();
+ private volatile ConcurrentHashMap<String, AtomicReference<CachedToken>>
cachedV2Tokens =
+ new ConcurrentHashMap<>();
+
+ private static final Duration cacheExpiryTime = Duration.ofSeconds(1);
Review Comment:
How about we make this relative to `MAX_VALIDITY`? like `MAX_VALIDITY *
0.5`, or `MAX_VALIDITY * 0.1` by default?
##########
solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java:
##########
@@ -485,34 +487,75 @@ private Optional<String> getUser() {
}
}
+ private static class CachedToken {
+ Instant generatedAt;
+ String token;
+
+ private CachedToken(Instant generatedAt, String token) {
+ this.generatedAt = generatedAt;
Review Comment:
not a big deal, but if we just store the expiration time instead of the
generation time we don't have to subtract every time
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
