smolnar82 commented on code in PR #622:
URL: https://github.com/apache/knox/pull/622#discussion_r952457950
##########
gateway-server/src/test/java/org/apache/knox/gateway/session/control/InMemoryConcurrentSessionVerifierTest.java:
##########
@@ -316,25 +296,27 @@ public void testBackgroundThreadRemoveExpiredTokens()
throws ServiceLifecycleExc
verifier.verifySessionForUser("admin", adminToken2);
JWT expiringAdminToken =
tokenAuthority.issueToken(expiringJwtAttributesForAdmin);
verifier.verifySessionForUser("admin", expiringAdminToken);
- Assert.assertEquals(3, verifier.countValidTokensForUser("admin"));
- Thread.sleep(1100);
- Assert.assertEquals(2, verifier.countValidTokensForUser("admin"));
+ Assert.assertEquals(3, verifier.getTokenCountForUser("admin").intValue());
+ Thread.sleep(1050);
+ verifier.removeExpiredTokens();
+ Assert.assertEquals(2, verifier.getTokenCountForUser("admin").intValue());
JWTokenAttributes expiringJwtAttributesForTom = makeJwtAttribute("tom",
true);
verifier.verifySessionForUser("tom", tomToken1);
verifier.verifySessionForUser("tom", tomToken2);
JWT expiringTomToken =
tokenAuthority.issueToken(expiringJwtAttributesForTom);
verifier.verifySessionForUser("tom", expiringTomToken);
- Assert.assertEquals(3, verifier.countValidTokensForUser("tom"));
- Thread.sleep(1100);
- Assert.assertEquals(2, verifier.countValidTokensForUser("tom"));
+ Assert.assertEquals(3, verifier.getTokenCountForUser("tom").intValue());
+ Thread.sleep(1050);
Review Comment:
The same qq. here.
##########
gateway-spi/src/main/java/org/apache/knox/gateway/config/GatewayConfig.java:
##########
@@ -822,7 +822,7 @@ public interface GatewayConfig {
Set<String> getPrivilegedUsers();
- Set<String> getNonPrivilegedUsers();
+ Set<String> getUnlimitedUsers();
Review Comment:
We may rename this to `getSessionVerificationUnlimitiedUsers` and the
previous one to `getSessionVerificationPrivilegedUsers`. What do you think?
##########
gateway-server/src/test/java/org/apache/knox/gateway/session/control/InMemoryConcurrentSessionVerifierTest.java:
##########
@@ -273,41 +255,39 @@ public void testNegativeLimitMeansUnlimited() throws
ServiceLifecycleException {
@Test
public void testExpiredTokensAreNotCounted() throws
ServiceLifecycleException, TokenServiceException, InterruptedException {
- GatewayConfig config = mockConfig(new HashSet<>(Arrays.asList("admin")),
new HashSet<>(Arrays.asList("tom", "guest")), 3, 3);
+ GatewayConfig config = mockConfig(Collections.emptySet(), new
HashSet<>(Arrays.asList("admin")), 3, 3);
verifier.init(config, options);
JWTokenAttributes expiringJwtAttributesForTom = makeJwtAttribute("tom",
true);
- JWT tomToken = tokenAuthority.issueToken(jwtAttributesForTom);
- verifier.verifySessionForUser("tom", tomToken);
+ verifier.verifySessionForUser("tom", tomToken1);
Assert.assertEquals(1, verifier.countValidTokensForUser("tom"));
- tomToken = tokenAuthority.issueToken(expiringJwtAttributesForTom);
- verifier.verifySessionForUser("tom", tomToken);
+ JWT expiringTomToken =
tokenAuthority.issueToken(expiringJwtAttributesForTom);
+ verifier.verifySessionForUser("tom", expiringTomToken);
Assert.assertEquals(2, verifier.countValidTokensForUser("tom"));
- tomToken = tokenAuthority.issueToken(expiringJwtAttributesForTom);
- verifier.verifySessionForUser("tom", tomToken);
+ expiringTomToken = tokenAuthority.issueToken(expiringJwtAttributesForTom);
+ verifier.verifySessionForUser("tom", expiringTomToken);
Assert.assertEquals(3, verifier.countValidTokensForUser("tom"));
- Thread.sleep(1000L);
+ Thread.sleep(1100);
Review Comment:
As discussed offline, we may give a high enough cleaning period, and invoke
the cleaning job manually here to avoid Thread.sleep. Have considered doing
that?
##########
gateway-server/src/test/java/org/apache/knox/gateway/session/control/InMemoryConcurrentSessionVerifierTest.java:
##########
@@ -316,25 +296,27 @@ public void testBackgroundThreadRemoveExpiredTokens()
throws ServiceLifecycleExc
verifier.verifySessionForUser("admin", adminToken2);
JWT expiringAdminToken =
tokenAuthority.issueToken(expiringJwtAttributesForAdmin);
verifier.verifySessionForUser("admin", expiringAdminToken);
- Assert.assertEquals(3, verifier.countValidTokensForUser("admin"));
- Thread.sleep(1100);
- Assert.assertEquals(2, verifier.countValidTokensForUser("admin"));
+ Assert.assertEquals(3, verifier.getTokenCountForUser("admin").intValue());
+ Thread.sleep(1050);
Review Comment:
If you are invoking `verifier.removeExpiredTokens();` anyway, why do we need
the Thread.sleep here?
--
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]
