- remove onerror from authxmanager

- fix flaky tests
redis · Dec 6, 2024 · 1ec5239 · 1ec5239
1 parent b1ab1db
commit 1ec5239
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 37 deletions.
diff --git a/src/main/java/redis/clients/jedis/authentication/AuthXManager.java b/src/main/java/redis/clients/jedis/authentication/AuthXManager.java
@@ -75,7 +75,6 @@ public void onTokenRenewed(Token token) {
             @Override
             public void onError(Exception reason) {
                 listener.onIdentityProviderError(reason);
-                AuthXManager.this.onError(reason);
             }
         }, true);
     }
@@ -93,12 +92,6 @@ public void authenticateConnections(Token token) {
         postAuthenticateHooks.forEach(hook -> hook.accept(token));
     }
 
-    public void onError(Exception reason) {
-        log.error("Token manager failed to acquire new token!", reason);
-        throw new JedisAuthenticationException("Token manager failed to acquire new token!",
-                reason);
-    }
-
     public Connection addConnection(Connection connection) {
         connections.add(new WeakReference<>(connection));
         return connection;

diff --git a/src/test/java/redis/clients/jedis/authentication/TokenBasedAuthenticationUnitTests.java b/src/test/java/redis/clients/jedis/authentication/TokenBasedAuthenticationUnitTests.java
@@ -11,12 +11,12 @@
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
-import static org.mockito.Mockito.atLeast;
 import static org.awaitility.Awaitility.await;
 import static org.awaitility.Durations.*;
 import static org.hamcrest.CoreMatchers.either;
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.lessThanOrEqualTo;
 
 import java.util.Collections;
 import java.util.concurrent.CountDownLatch;
@@ -38,7 +38,6 @@
 import redis.clients.authentication.core.TokenListener;
 import redis.clients.authentication.core.TokenManager;
 import redis.clients.authentication.core.TokenManagerConfig;
-import redis.clients.authentication.core.TokenRequestException;
 import redis.clients.jedis.ConnectionPool;
 import redis.clients.jedis.EndpointConfig;
 import redis.clients.jedis.HostAndPort;
@@ -154,8 +153,8 @@ public void testCalculateRenewalDelay() {
 
     delay = manager.calculateRenewalDelay(expireDate, issueDate);
 
-    assertThat(delay, Matchers
-        .greaterThanOrEqualTo(Math.min(duration - config.lower, (long) (duration * config.ratio))));
+    assertThat(delay,
+      lessThanOrEqualTo(Math.min(duration - config.lower, (long) (duration * config.ratio))));
 
     duration = 10000;
     config.lower = 8000;
@@ -165,8 +164,8 @@ public void testCalculateRenewalDelay() {
 
     delay = manager.calculateRenewalDelay(expireDate, issueDate);
 
-    assertThat(delay, Matchers
-        .greaterThanOrEqualTo(Math.min(duration - config.lower, (long) (duration * config.ratio))));
+    assertThat(delay,
+      lessThanOrEqualTo(Math.min(duration - config.lower, (long) (duration * config.ratio))));
 
     duration = 10000;
     config.lower = 10000;
@@ -234,7 +233,7 @@ public void testAuthXManagerReceivesNewToken()
   }
 
   @Test
-  public void testBlockForInitialToken() {
+  public void testBlockForInitialTokenWhenException() {
     String exceptionMessage = "Test exception from identity provider!";
     IdentityProvider identityProvider = () -> {
       throw new RuntimeException(exceptionMessage);
@@ -244,41 +243,33 @@ public void testBlockForInitialToken() {
         new TokenManagerConfig(0.7F, 200, 2000, new TokenManagerConfig.RetryPolicy(5, 100)));
 
     AuthXManager manager = new AuthXManager(tokenManager);
-    TokenRequestException e = assertThrows(TokenRequestException.class, () -> manager.start());
+    JedisAuthenticationException e = assertThrows(JedisAuthenticationException.class,
+      () -> manager.start());
 
-    assertEquals(exceptionMessage, e.getCause().getCause().getCause().getMessage());
+    assertEquals(exceptionMessage, e.getCause().getCause().getMessage());
   }
 
   @Test
-  public void testNoBlockForInitialToken()
-      throws InterruptedException, ExecutionException, TimeoutException {
-    int numberOfRetries = 1;
-    CountDownLatch requesLatch = new CountDownLatch(numberOfRetries);
+  public void testBlockForInitialTokenWhenHangs() {
+    String exceptionMessage = "AuthXManager failed to start!";
+    CountDownLatch latch = new CountDownLatch(1);
     IdentityProvider identityProvider = () -> {
       try {
-        requesLatch.await();
+        latch.await();
       } catch (InterruptedException e) {
-        e.printStackTrace();
       }
-      throw new RuntimeException("Test exception from identity provider!");
+      return null;
     };
 
     TokenManager tokenManager = new TokenManager(identityProvider,
-        new TokenManagerConfig(0.7F, 200, 500, new TokenManagerConfig.RetryPolicy(5, 0)));
-
-    AuthXManager manager = spy(new AuthXManager(tokenManager));
-    manager.start();
+        new TokenManagerConfig(0.7F, 200, 1000, new TokenManagerConfig.RetryPolicy(2, 100)));
 
-    await().during(FIVE_HUNDRED_MILLISECONDS).until(tokenManager::getCurrentToken,
-      Matchers.nullValue());
-    verify(manager, never()).onError(any());
-    verify(manager, never()).authenticateConnections(any());
-    requesLatch.countDown();
+    AuthXManager manager = new AuthXManager(tokenManager);
+    JedisAuthenticationException e = assertThrows(JedisAuthenticationException.class,
+      () -> manager.start());
 
-    await().during(FIVE_HUNDRED_MILLISECONDS).until(tokenManager::getCurrentToken,
-      Matchers.nullValue());
-    verify(manager, atLeast(1)).onError(any());
-    verify(manager, never()).authenticateConnections(any());
+    latch.countDown();
+    assertEquals(exceptionMessage, e.getMessage());
   }
 
   @Test
@@ -337,9 +328,13 @@ public void testTokenManagerWithHangingTokenRequest()
         executionTimeout, new TokenManagerConfig.RetryPolicy(numberOfRetries, 100)));
 
     AuthXManager manager = spy(new AuthXManager(tokenManager));
+    AuthXEventListener listener = mock(AuthXEventListener.class);
+    manager.setListener(listener);
     manager.start();
     requesLatch.await();
-    verify(manager, never()).onError(any());
+    verify(listener, never()).onIdentityProviderError(any());
+    verify(listener, never()).onConnectionAuthenticationError(any());
+
     await().atMost(2, TimeUnit.SECONDS).untilAsserted(() -> {
       verify(manager, times(1)).authenticateConnections(any());
     });