From 016502d22e0490ed8b1b4951f5e12be01c4a224c Mon Sep 17 00:00:00 2001
From: Valery Kharseko <vharseko@3a-systems.ru>
Date: Mon, 03 Aug 2026 16:13:03 +0000
Subject: [PATCH] [#807] Do not drop persistent search notifications through search-phase dedup (#812)
---
opendj-server-legacy/src/test/java/org/openidentityplatform/opendj/AliasTestCase.java | 143 +++++++++++++++++++++++++++++++++++++++++++++--
1 files changed, 137 insertions(+), 6 deletions(-)
diff --git a/opendj-server-legacy/src/test/java/org/openidentityplatform/opendj/AliasTestCase.java b/opendj-server-legacy/src/test/java/org/openidentityplatform/opendj/AliasTestCase.java
index b3360fe..ef32c41 100644
--- a/opendj-server-legacy/src/test/java/org/openidentityplatform/opendj/AliasTestCase.java
+++ b/opendj-server-legacy/src/test/java/org/openidentityplatform/opendj/AliasTestCase.java
@@ -21,6 +21,7 @@
import org.forgerock.opendj.ldap.controls.PersistentSearchRequestControl;
import org.forgerock.opendj.ldap.requests.Requests;
import org.forgerock.opendj.ldap.requests.SearchRequest;
+import org.forgerock.opendj.ldap.responses.Result;
import org.forgerock.opendj.ldap.responses.SearchResultEntry;
import org.forgerock.opendj.ldap.responses.SearchResultReference;
import org.forgerock.opendj.ldif.ConnectionEntryReader;
@@ -32,6 +33,9 @@
import org.opends.server.api.LocalBackend;
import org.opends.server.backends.MemoryBackend;
import org.opends.server.core.DirectoryServer;
+import org.opends.server.core.PersistentSearch;
+import org.opends.server.protocols.internal.InternalClientConnection;
+import org.opends.server.protocols.internal.InternalSearchOperation;
import org.opends.server.types.AcceptRejectWarn;
import org.opends.server.types.Entry;
import org.testng.annotations.AfterClass;
@@ -562,7 +566,9 @@
psearch.searchAsync(request, new SearchResultHandler() {
@Override
public boolean handleEntry(SearchResultEntry entry) {
- notified.add(entry.getName().toString());
+ // Every notification carries the same DN, so the DN alone cannot tell a lost
+ // notification from a duplicated one: record which change is being reported.
+ notified.add(entry.getName() + " " + entry.parseAttribute("description").asString());
return true;
}
@@ -574,25 +580,150 @@
// searchAsync returns before the server has registered the persistent search, so wait
// until the backend reports it; otherwise the first modification below can be notified
- // before the search is listening and be missed.
+ // before the search is listening and be missed. A failed test is rerun in the same JVM
+ // (rerunFailingTestsCount), which can leave the persistent search of the previous run
+ // behind, hence the wait for a persistent search on our own base DN.
final LocalBackend<?> backend = TestCaseUtils.getServerContext()
.getBackendConfigManager().getLocalBackendById(TestCaseUtils.TEST_BACKEND_ID);
- for (int i = 0; backend.getPersistentSearches().isEmpty() && i < 500; i++) {
+ for (int i = 0; !isPersistentSearchRegistered(backend, "ou=psearch,o=test") && i < 500; i++) {
Thread.sleep(10);
}
- assertThat(backend.getPersistentSearches()).isNotEmpty();
+ assertThat(isPersistentSearchRegistered(backend, "ou=psearch,o=test"))
+ .as("the persistent search was never registered with backend %s", backend.getBackendID())
+ .isTrue();
// The same entry is modified repeatedly: each change must reach the persistent search.
for (int i = 1; i <= 3; i++) {
connection.modify(Requests.newModifyRequest("cn=changing,ou=psearch,o=test")
.addModification(ModificationType.REPLACE, "description", "change " + i));
assertThat(notified.poll(30, TimeUnit.SECONDS))
- .as("notification for change " + i)
- .isEqualTo("cn=changing,ou=psearch,o=test");
+ .as("notification for change %d, persistent search still registered: %s, "
+ + "notifications received afterwards: %s",
+ i, isPersistentSearchRegistered(backend, "ou=psearch,o=test"), notified)
+ .isEqualTo("cn=changing,ou=psearch,o=test change " + i);
}
}
}
+ // A persistent search notification is not a search result: it must be reported whether or not
+ // the entry was returned before, and it is not bound by the size and time limits of the search.
+ // In a real persistent search the search phase is only open for a few instructions after the
+ // search is registered with the backend, so the notification path is driven directly here.
+ @Test
+ public void test_persistent_search_notification_ignores_search_phase_state() throws Exception {
+ TestCaseUtils.addEntries(
+ "dn: ou=psearch-notify,o=test",
+ "objectClass: top",
+ "objectClass: organizationalUnit",
+ "ou: psearch-notify",
+ ""
+ );
+ final Entry entry = DirectoryServer.getEntry(DN.valueOf("ou=psearch-notify,o=test"));
+
+ final InternalSearchOperation search = new InternalSearchOperation(
+ InternalClientConnection.getRootConnection(),
+ InternalClientConnection.nextOperationID(),
+ InternalClientConnection.nextMessageID(),
+ org.opends.server.protocols.internal.Requests
+ .newSearchRequest(DN.valueOf("o=test"), SearchScope.WHOLE_SUBTREE)
+ .setDereferenceAliasesPolicy(DereferenceAliasesPolicy.ALWAYS));
+
+ // The search phase returns the entry once, and drops it when it reaches it a second time
+ // through an alias ...
+ assertThat(search.returnEntry(entry, null)).isTrue();
+ assertThat(search.returnEntry(entry, null)).isTrue();
+ assertThat(search.getSearchEntries()).hasSize(1);
+
+ // ... but a change reported to a persistent search is never a duplicate, whether the search
+ // phase is still open (the entry was just returned by it) or already over.
+ assertThat(search.returnPersistentSearchEntry(entry, null)).isTrue();
+ search.endSearchPhase();
+ assertThat(search.returnPersistentSearchEntry(entry, null)).isTrue();
+ assertThat(search.getSearchEntries()).hasSize(3);
+
+ // The size limit of the search does not bound a notification: it only bounds the search
+ // phase, and is lifted for the rest of a persistent search once that phase is over.
+ search.setSizeLimit(1);
+ assertThat(search.returnPersistentSearchEntry(entry, null)).isTrue();
+ assertThat(search.getSearchEntries()).hasSize(4);
+ // The search phase itself is still bound by it.
+ assertThat(search.returnEntry(entry, null)).isFalse();
+ assertThat(search.getResultCode()).isEqualTo(ResultCode.SIZE_LIMIT_EXCEEDED);
+
+ // Same for the time limit, checked on its own: with a size limit left in the way the search
+ // phase would stop on that one and the time limit would never be reached.
+ search.setSizeLimit(0);
+ search.setTimeLimit(1);
+ search.setTimeLimitExpiration(0);
+ assertThat(search.returnPersistentSearchEntry(entry, null)).isTrue();
+ assertThat(search.getSearchEntries()).hasSize(5);
+ assertThat(search.returnEntry(entry, null)).isFalse();
+ assertThat(search.getResultCode()).isEqualTo(ResultCode.TIME_LIMIT_EXCEEDED);
+ }
+
+ // A persistent search which is cancelled because its backend goes away, as happens when the
+ // backend is disabled or re-initialized, must be told so: without a search result done the
+ // client waits forever for changes on a search which no longer exists.
+ @Test
+ public void test_persistent_search_is_told_when_its_backend_goes_away() throws Exception {
+ final String backendID = "psearchUnavailable";
+ final String baseDN = "o=psearch-unavailable";
+ TestCaseUtils.initializeMemoryBackend(backendID, baseDN, true);
+ final MemoryBackend backend = (MemoryBackend) TestCaseUtils.getServerContext()
+ .getBackendConfigManager().getLocalBackendById(backendID);
+
+ final SearchRequest request =
+ Requests.newSearchRequest(baseDN, SearchScope.WHOLE_SUBTREE, "(objectclass=*)")
+ .addControl(PersistentSearchRequestControl.newControl(
+ true, true, false, PersistentSearchChangeType.MODIFY));
+
+ final LDAPConnectionFactory factory =
+ new LDAPConnectionFactory("localhost", TestCaseUtils.getServerLdapPort());
+ try (Connection psearch = factory.getConnection()) {
+ psearch.bind("cn=Directory Manager", "password".toCharArray());
+ final LdapPromise<Result> searchDone = psearch.searchAsync(request, new SearchResultHandler() {
+ @Override
+ public boolean handleEntry(SearchResultEntry entry) {
+ return true;
+ }
+
+ @Override
+ public boolean handleReference(SearchResultReference reference) {
+ return true;
+ }
+ });
+
+ for (int i = 0; !isPersistentSearchRegistered(backend, baseDN) && i < 500; i++) {
+ Thread.sleep(10);
+ }
+ assertThat(isPersistentSearchRegistered(backend, baseDN))
+ .as("the persistent search was never registered with backend %s", backendID)
+ .isTrue();
+
+ backend.finalizeBackend();
+
+ try {
+ final Result result = searchDone.getOrThrow(30, TimeUnit.SECONDS);
+ fail("the persistent search should have been terminated, it returned " + result);
+ } catch (LdapException e) {
+ assertThat(e.getResult().getResultCode()).isEqualTo(ResultCode.UNAVAILABLE);
+ assertThat(e.getResult().getDiagnosticMessage()).contains(backendID);
+ }
+ } finally {
+ TestCaseUtils.getServerContext().getBackendConfigManager().deregisterLocalBackend(backend);
+ }
+ }
+
+ /** Whether the provided backend has a persistent search registered for the provided base DN. */
+ private static boolean isPersistentSearchRegistered(LocalBackend<?> backend, String baseDN) {
+ for (PersistentSearch psearch : backend.getPersistentSearches()) {
+ if (psearch.getSearchOperation().getBaseDN().equals(DN.valueOf(baseDN))) {
+ return true;
+ }
+ }
+ return false;
+ }
+
// An alias is dereferenced before its target is reached on its own: the target must still be
// returned, and exactly once. The original regression was order-sensitive, dropping the target
// when the alias reached it first, so this pins the alias-before-target order specifically. The
--
Gitblit v1.10.0