From 01df9735ef3ab7300a080daece63a5a611897071 Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Tue, 21 Jul 2026 09:26:21 +0300 Subject: [PATCH 1/2] Deterministic changelog wait in SunDSChangeLogSyncStrategyTests (#116) The stabilization heuristic (two equal lastChangeNumber reads 2s apart) is blind to changelog records OpenDJ has not written yet, so on slow runners a cleanup delete could land after the "stable" verdict and leak into the next test's sync window (expected:<1> but was:<2>). cleanupBaseContext() and LdapModifyForTests.modify() now report how many changes they made, and the tests wait until lastChangeNumber reaches the value read beforehand plus that count (plus two equal consecutive reads as a guard), with a 60s timeout instead of hanging forever. --- .../ldap/SunDSTestBase.java | 7 ++- .../ldap/sync/sunds/LdapModifyForTests.java | 7 ++- .../SunDSChangeLogSyncStrategyTests.java | 58 +++++++++++++------ 3 files changed, 52 insertions(+), 20 deletions(-) diff --git a/OpenICF-ldap-connector/src/test/java/org/identityconnectors/ldap/SunDSTestBase.java b/OpenICF-ldap-connector/src/test/java/org/identityconnectors/ldap/SunDSTestBase.java index 45109ab3..b20b3271 100644 --- a/OpenICF-ldap-connector/src/test/java/org/identityconnectors/ldap/SunDSTestBase.java +++ b/OpenICF-ldap-connector/src/test/java/org/identityconnectors/ldap/SunDSTestBase.java @@ -75,7 +75,11 @@ public static LdapConfiguration newConfiguration() { return config; } - public static void cleanupBaseContext(LdapConnection conn) throws NamingException { + /** + * Deletes everything under the base contexts and returns the number of entries deleted, so + * callers tracking the change log can tell how many delete records they are owed. + */ + public static int cleanupBaseContext(LdapConnection conn) throws NamingException { SearchControls controls = LdapInternalSearch.createDefaultSearchControls(); controls.setSearchScope(SearchControls.SUBTREE_SCOPE); LdapInternalSearch search = new LdapInternalSearch(conn, null, Arrays.asList(conn.getConfiguration().getBaseContexts()), @@ -93,6 +97,7 @@ public boolean handle(String baseDN, SearchResult result) throws NamingException for (LdapName entryDN : entryDNs) { conn.getInitialContext().destroySubcontext(entryDN); } + return entryDNs.size(); } public static ConnectorFacade newFacade(LdapConfiguration cfg) { diff --git a/OpenICF-ldap-connector/src/test/java/org/identityconnectors/ldap/sync/sunds/LdapModifyForTests.java b/OpenICF-ldap-connector/src/test/java/org/identityconnectors/ldap/sync/sunds/LdapModifyForTests.java index a785ad4d..4d1c4cf4 100644 --- a/OpenICF-ldap-connector/src/test/java/org/identityconnectors/ldap/sync/sunds/LdapModifyForTests.java +++ b/OpenICF-ldap-connector/src/test/java/org/identityconnectors/ldap/sync/sunds/LdapModifyForTests.java @@ -19,6 +19,7 @@ * enclosed by brackets [] replaced by your own identifying information: * "Portions Copyrighted [year] [name of copyright owner]" * ==================== + * Portions Copyrighted 2026 3A Systems, LLC */ package org.identityconnectors.ldap.sync.sunds; @@ -58,10 +59,12 @@ public class LdapModifyForTests { private static final Log log = Log.getLog(LdapModifyForTests.class); - public static void modify(LdapConnection conn, String ldif) throws NamingException { + /** Applies the changes in the LDIF and returns how many were performed. */ + public static int modify(LdapConnection conn, String ldif) throws NamingException { LdifParser parser = new LdifParser(ldif); Iterator lines = parser.iterator(); + int changes = 0; String dn = null; String changeType = null; @@ -76,6 +79,7 @@ public static void modify(LdapConnection conn, String ldif) throws NamingExcepti Line line = lines.next(); if (line instanceof ChangeSeparator && dn != null) { performChange(conn, dn, changeType, added, deleted, newRdn, deleteOldRdn); + changes++; dn = null; changeType = null; added.clear(); @@ -144,6 +148,7 @@ public static void modify(LdapConnection conn, String ldif) throws NamingExcepti } } } + return changes; } private static void performChange(LdapConnection conn, String dn, String changeType, diff --git a/OpenICF-ldap-connector/src/test/java/org/identityconnectors/ldap/sync/sunds/SunDSChangeLogSyncStrategyTests.java b/OpenICF-ldap-connector/src/test/java/org/identityconnectors/ldap/sync/sunds/SunDSChangeLogSyncStrategyTests.java index 72628a76..52efe8b4 100644 --- a/OpenICF-ldap-connector/src/test/java/org/identityconnectors/ldap/sync/sunds/SunDSChangeLogSyncStrategyTests.java +++ b/OpenICF-ldap-connector/src/test/java/org/identityconnectors/ldap/sync/sunds/SunDSChangeLogSyncStrategyTests.java @@ -19,6 +19,7 @@ * enclosed by brackets [] replaced by your own identifying information: * "Portions Copyrighted [year] [name of copyright owner]" * ==================== + * Portions Copyrighted 2026 3A Systems, LLC */ package org.identityconnectors.ldap.sync.sunds; @@ -57,38 +58,59 @@ public class SunDSChangeLogSyncStrategyTests extends SunDSTestBase { private static final Log log = Log.getLog(SunDSChangeLogSyncStrategyTests.class); - private static final int STABLE_CHANGELOG_INTERVAL = 2000; /* milliseconds */ + private static final int CHANGELOG_POLL_INTERVAL = 100; /* milliseconds */ + private static final int CHANGELOG_WAIT_TIMEOUT = 60000; /* milliseconds */ private static LdapConnection newConnection(LdapConfiguration config) throws NamingException { LdapConnection conn = new LdapConnection(config); - cleanupBaseContext(conn); - waitForChangeLogToStabilize(conn); + int lastChangeNumber = getLastChangeNumber(conn); + int deleted = cleanupBaseContext(conn); + waitForChangeLogToReach(conn, lastChangeNumber + deleted); return conn; } - private static void waitForChangeLogToStabilize(LdapConnection conn) { - int lastChangeNumber = -1; - int previousLastChangeNumber; - do { - if (lastChangeNumber > 0) { - log.ok("Waiting for change log to stabilize (last change number: {0})", lastChangeNumber); - try { - Thread.sleep(STABLE_CHANGELOG_INTERVAL); - } catch (InterruptedException e) { - // Ignore. - } + private static int getLastChangeNumber(LdapConnection conn) { + // A fresh strategy instance every time: getChangeLogAttributes() caches its first read. + return new SunDSChangeLogSyncStrategy(conn, ObjectClass.ACCOUNT).getChangeLogAttributes().getLastChangeNumber(); + } + + /** + * Waits until the change log has caught up with the caller's own changes. OpenDJ writes the + * retro change log asynchronously, so an LDAP operation can return before its change log + * record exists; waiting for two equal consecutive reads alone is blind to such records, and + * a straggler then leaks into the next sync window (issue #116). The caller therefore passes + * the change number the log is known to owe it: the last change number read before its + * operations plus the number of operations performed. + */ + private static void waitForChangeLogToReach(LdapConnection conn, int expectedChangeNumber) { + long deadline = System.currentTimeMillis() + CHANGELOG_WAIT_TIMEOUT; + int previousLastChangeNumber = -1; + int lastChangeNumber = getLastChangeNumber(conn); + // On top of the expected number, require two equal consecutive reads, as a guard against + // records the caller did not account for still trickling in. + while (lastChangeNumber < expectedChangeNumber || lastChangeNumber != previousLastChangeNumber) { + if (System.currentTimeMillis() >= deadline) { + throw new AssertionError("Change log did not stabilize at change number " + + expectedChangeNumber + " or later within " + CHANGELOG_WAIT_TIMEOUT + + " ms (last change number: " + lastChangeNumber + ")"); + } + log.ok("Waiting for change log to reach {0} (last change number: {1})", expectedChangeNumber, lastChangeNumber); + try { + Thread.sleep(CHANGELOG_POLL_INTERVAL); + } catch (InterruptedException e) { + // Ignore. } previousLastChangeNumber = lastChangeNumber; - lastChangeNumber = new SunDSChangeLogSyncStrategy(conn, ObjectClass.ACCOUNT).getChangeLogAttributes().getLastChangeNumber(); - } while (lastChangeNumber != previousLastChangeNumber); + lastChangeNumber = getLastChangeNumber(conn); + } } private List doTest(LdapConnection conn, String ldif, int expected) throws NamingException { SunDSChangeLogSyncStrategy sync = new SunDSChangeLogSyncStrategy(conn, ObjectClass.ACCOUNT); SyncToken token = sync.getLatestSyncToken(); - LdapModifyForTests.modify(conn, ldif); - waitForChangeLogToStabilize(conn); + int changes = LdapModifyForTests.modify(conn, ldif); + waitForChangeLogToReach(conn, (Integer) token.getValue() + changes); OperationOptionsBuilder builder = new OperationOptionsBuilder(); builder.setAttributesToGet("cn", "sn", "givenName", "uid"); From 751f81306856a4e0b493c68b3d530b3b7e548a45 Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Tue, 21 Jul 2026 13:52:00 +0300 Subject: [PATCH 2/2] Fail fast on unsupported changeType in LdapModifyForTests (#116) Apply review suggestion: performChange() now throws IllegalArgumentException for an unsupported changeType instead of silently doing nothing, so a bad LDIF fails immediately rather than inflating the expected change number and timing out in waitForChangeLogToReach(). Also pass the missing dn argument to the "Deleting context" log placeholder. --- .../ldap/sync/sunds/LdapModifyForTests.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/OpenICF-ldap-connector/src/test/java/org/identityconnectors/ldap/sync/sunds/LdapModifyForTests.java b/OpenICF-ldap-connector/src/test/java/org/identityconnectors/ldap/sync/sunds/LdapModifyForTests.java index 4d1c4cf4..3b742278 100644 --- a/OpenICF-ldap-connector/src/test/java/org/identityconnectors/ldap/sync/sunds/LdapModifyForTests.java +++ b/OpenICF-ldap-connector/src/test/java/org/identityconnectors/ldap/sync/sunds/LdapModifyForTests.java @@ -176,7 +176,7 @@ private static void performChange(LdapConnection conn, String dn, String changeT log.ok("Modifying context {0} with attributes {1}", dn, modItems); conn.getInitialContext().modifyAttributes(dn, modItems.toArray(new ModificationItem[modItems.size()])); } else if ("delete".equalsIgnoreCase(changeType)) { - log.ok("Deleting context {0}"); + log.ok("Deleting context {0}", dn); conn.getInitialContext().destroySubcontext(dn); } else if ("modrdn".equalsIgnoreCase(changeType)) { LdapName oldName = quietCreateLdapName(dn); @@ -190,6 +190,8 @@ private static void performChange(LdapConnection conn, String dn, String changeT } finally { ctx.close(); } + } else { + throw new IllegalArgumentException("Unsupported changeType: " + changeType); } }