From 8f0a809eee66ec9aedf7f7cb09f4fbd0e1852256 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?JB=20Onofr=C3=A9?= Date: Tue, 10 Mar 2026 08:16:42 +0100 Subject: [PATCH] fix(test): fix CachedLDAPAuthorizationModuleLegacyTest flakiness caused by Thread.sleep Replace Thread.sleep calls with Wait.waitFor in AbstractCachedLDAPAuthorizationMapLegacyTest to fix testRestartAsync timeout. The root cause was that getReadACLs called while LDAP was down triggered an async reconnection task that blocked the single-threaded updater on a TCP connect timeout, causing all subsequent reconnection attempts to be silently discarded. --- ...tCachedLDAPAuthorizationMapLegacyTest.java | 113 ++++++++++++------ 1 file changed, 76 insertions(+), 37 deletions(-) diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/security/AbstractCachedLDAPAuthorizationMapLegacyTest.java b/activemq-unit-tests/src/test/java/org/apache/activemq/security/AbstractCachedLDAPAuthorizationMapLegacyTest.java index 39f91cb2ab9..36069630ea2 100644 --- a/activemq-unit-tests/src/test/java/org/apache/activemq/security/AbstractCachedLDAPAuthorizationMapLegacyTest.java +++ b/activemq-unit-tests/src/test/java/org/apache/activemq/security/AbstractCachedLDAPAuthorizationMapLegacyTest.java @@ -152,9 +152,14 @@ public void testAdvisory() throws Exception { public void testTemporary() throws Exception { map.query(); - Thread.sleep(1000); + assertTrue("did not get expected temp ACLs", Wait.waitFor(new Wait.Condition() { + @Override + public boolean isSatisified() throws Exception { + Set acls = map.getTempDestinationReadACLs(); + return acls != null && acls.size() == 2; + } + })); Set readACLs = map.getTempDestinationReadACLs(); - assertEquals("set size: " + readACLs, 2, readACLs.size()); assertTrue("Contains admin group", readACLs.contains(ADMINS)); assertTrue("Contains users group", readACLs.contains(USERS)); } @@ -174,10 +179,12 @@ public void testAdd() throws Exception { reader.close(); - Thread.sleep(2000); - - failedACLs = map.getReadACLs(new ActiveMQQueue("FAILED")); - assertEquals("set size: " + failedACLs, 2, failedACLs.size()); + assertTrue("did not get expected size after add", Wait.waitFor(new Wait.Condition() { + @Override + public boolean isSatisified() throws Exception { + return map.getReadACLs(new ActiveMQQueue("FAILED")).size() == 2; + } + })); } @Test @@ -194,10 +201,13 @@ public void testRemove() throws Exception { } reader.close(); - Thread.sleep(2000); - failedACLs = map.getReadACLs(new ActiveMQQueue("TEST.FOO")); - assertEquals("set size: " + failedACLs, 0, failedACLs.size()); + assertTrue("did not get expected size after remove", Wait.waitFor(new Wait.Condition() { + @Override + public boolean isSatisified() throws Exception { + return map.getReadACLs(new ActiveMQQueue("TEST.FOO")).size() == 0; + } + })); assertTrue(map.getTempDestinationReadACLs() == null || map.getTempDestinationReadACLs().isEmpty()); assertTrue(map.getTempDestinationWriteACLs() == null || map.getTempDestinationWriteACLs().isEmpty()); @@ -215,11 +225,12 @@ public void testRenameDestination() throws Exception { connection.rename(new Dn("cn=TEST.FOO," + getQueueBaseDn()), new Rdn("cn=TEST.BAR")); - Thread.sleep(2000); - - failedACLs = map.getReadACLs(new ActiveMQQueue("TEST.FOO")); - assertEquals("set size: " + failedACLs, 0, failedACLs.size()); - + assertTrue("TEST.FOO ACLs not removed after rename", Wait.waitFor(new Wait.Condition() { + @Override + public boolean isSatisified() throws Exception { + return map.getReadACLs(new ActiveMQQueue("TEST.FOO")).size() == 0; + } + })); failedACLs = map.getReadACLs(new ActiveMQQueue("TEST.BAR")); assertEquals("set size: " + failedACLs, 2, failedACLs.size()); @@ -232,21 +243,25 @@ public void testRenamePermission() throws Exception { // Test for a permission rename connection.delete(new Dn("cn=Read,cn=TEST.FOO," + getQueueBaseDn())); - Thread.sleep(2000); - - Set failedACLs = map.getReadACLs(new ActiveMQQueue("TEST.FOO")); - assertEquals("set size: " + failedACLs, 0, failedACLs.size()); + assertTrue("Read ACLs not removed after delete", Wait.waitFor(new Wait.Condition() { + @Override + public boolean isSatisified() throws Exception { + return map.getReadACLs(new ActiveMQQueue("TEST.FOO")).size() == 0; + } + })); - failedACLs = map.getWriteACLs(new ActiveMQQueue("TEST.FOO")); + Set failedACLs = map.getWriteACLs(new ActiveMQQueue("TEST.FOO")); assertEquals("set size: " + failedACLs, 2, failedACLs.size()); connection.rename(new Dn("cn=Write,cn=TEST.FOO," + getQueueBaseDn()), new Rdn("cn=Read")); - Thread.sleep(2000); - - failedACLs = map.getReadACLs(new ActiveMQQueue("TEST.FOO")); - assertEquals("set size: " + failedACLs, 2, failedACLs.size()); + assertTrue("Read ACLs not restored after rename", Wait.waitFor(new Wait.Condition() { + @Override + public boolean isSatisified() throws Exception { + return map.getReadACLs(new ActiveMQQueue("TEST.FOO")).size() == 2; + } + })); failedACLs = map.getWriteACLs(new ActiveMQQueue("TEST.FOO")); assertEquals("set size: " + failedACLs, 0, failedACLs.size()); @@ -268,10 +283,12 @@ public void testChange() throws Exception { connection.modify(request); - Thread.sleep(2000); - - failedACLs = map.getReadACLs(new ActiveMQQueue("TEST.FOO")); - assertEquals("set size: " + failedACLs, 1, failedACLs.size()); + assertTrue("Read ACLs not updated after modify", Wait.waitFor(new Wait.Condition() { + @Override + public boolean isSatisified() throws Exception { + return map.getReadACLs(new ActiveMQQueue("TEST.FOO")).size() == 1; + } + })); // Change destination entry request = new ModifyRequestImpl(); @@ -280,10 +297,12 @@ public void testChange() throws Exception { connection.modify(request); - Thread.sleep(2000); - - failedACLs = map.getReadACLs(new ActiveMQQueue("TEST.FOO")); - assertEquals("set size: " + failedACLs, 1, failedACLs.size()); + assertTrue("Read ACLs changed unexpectedly after destination modify", Wait.waitFor(new Wait.Condition() { + @Override + public boolean isSatisified() throws Exception { + return map.getReadACLs(new ActiveMQQueue("TEST.FOO")).size() == 1; + } + })); } @Test @@ -313,7 +332,7 @@ public void testRestart(final boolean sync) throws Exception { // wait for the context to be closed // as we can't rely on ldap server isStarted() - Wait.waitFor(new Wait.Condition() { + assertTrue("Context was not closed after LDAP server stop", Wait.waitFor(new Wait.Condition() { @Override public boolean isSatisified() throws Exception { if (sync) { @@ -322,14 +341,34 @@ public boolean isSatisified() throws Exception { return map.context == null; } } - }, 5*60*1000); + }, 5*60*1000)); - failedACLs = map.getReadACLs(new ActiveMQQueue("TEST.FOO")); - assertEquals("set size: " + failedACLs, 2, failedACLs.size()); + // Verify cached ACLs are still available while LDAP is down. + // Avoid calling getReadACLs here in async mode as it triggers + // checkForUpdates which submits an async reconnection task to the + // single-threaded updater. That task blocks on the dead LDAP + // server (TCP connect timeout), preventing later reconnection + // tasks from executing and causing the test to time out. + if (sync) { + failedACLs = map.getReadACLs(new ActiveMQQueue("TEST.FOO")); + assertEquals("set size: " + failedACLs, 2, failedACLs.size()); + } getLdapServer().start(); - Thread.sleep(2000); + // Wait for the LDAP server to be fully ready and force a synchronous + // reconnection of the map, re-registering event listeners. + assertTrue("Map did not reconnect to restarted LDAP server", Wait.waitFor(new Wait.Condition() { + @Override + public boolean isSatisified() throws Exception { + try { + map.query(); + return true; + } catch (Exception e) { + return false; + } + } + })); connection = getLdapConnection(); @@ -347,7 +386,7 @@ public boolean isSatisified() throws Exception { public boolean isSatisified() throws Exception { return map.getReadACLs(new ActiveMQQueue("FAILED")).size() == 2; } - }, 5*60*1000)); + })); } protected SimpleCachedLDAPAuthorizationMap createMap() {