From 4c64fa4aa80953d8c28de98d30622f9b820bec40 Mon Sep 17 00:00:00 2001
From: Valery Kharseko <vharseko@3a-systems.ru>
Date: Tue, 22 Sep 2026 09:22:48 +0000
Subject: [PATCH] [#1026] Close the import config on every path an import task can end on (#1028)
---
opendj-server-legacy/src/test/java/org/opends/server/api/TestTaskListener.java | 5 +
opendj-server-legacy/src/test/java/org/opends/server/tasks/TestImportAndExport.java | 112 ++++++++++++++++++++++++++++++++++++
opendj-server-legacy/src/main/java/org/opends/server/tasks/ImportTask.java | 15 ++++
3 files changed, 128 insertions(+), 4 deletions(-)
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/tasks/ImportTask.java b/opendj-server-legacy/src/main/java/org/opends/server/tasks/ImportTask.java
index aaebe55..779a8e1 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/tasks/ImportTask.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/tasks/ImportTask.java
@@ -617,6 +617,14 @@
catch (Exception e)
{
logger.error(ERR_LDIFIMPORT_CANNOT_OPEN_SKIP_FILE, skipFile, getExceptionMessage(e));
+ /*
+ * The reject file is already open and this return is the one which is above the try
+ * whose finally closes it. The two files are not opened inside that try instead: a
+ * listener told an import began puts back what it took offline when it is told the
+ * import ended - a replication domain reloads and rewinds its state - and an import
+ * which never reached a backend has nothing for it to put back.
+ */
+ importConfig.close();
return TaskState.STOPPED_BY_ERROR;
}
}
@@ -720,6 +728,11 @@
}
finally
{
+ // Close the LDIF reader and the reject and skip files whichever way the import ended.
+ // The backend closes them with its reader, but an import which fails before that reader
+ // exists - or before the import is even launched - leaves them to this task.
+ importConfig.close();
+
// Enable the backend, if it was this task which disabled it.
boolean backendLeftDisabled = false;
if (backendDisabled)
@@ -748,8 +761,6 @@
}
}
- // Clean up after the import by closing the import config.
- importConfig.close();
return getFinalTaskState();
}
diff --git a/opendj-server-legacy/src/test/java/org/opends/server/api/TestTaskListener.java b/opendj-server-legacy/src/test/java/org/opends/server/api/TestTaskListener.java
index 8a4f9e9..597cfd3 100644
--- a/opendj-server-legacy/src/test/java/org/opends/server/api/TestTaskListener.java
+++ b/opendj-server-legacy/src/test/java/org/opends/server/api/TestTaskListener.java
@@ -13,10 +13,12 @@
*
* Copyright 2006-2008 Sun Microsystems, Inc.
* Portions Copyright 2015-2016 ForgeRock AS.
+ * Portions Copyright 2026 3A Systems, LLC.
*/
package org.opends.server.api;
import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicReference;
import org.opends.server.core.DirectoryServer;
import org.opends.server.types.BackupConfig;
@@ -42,6 +44,8 @@
public static final AtomicInteger importEndCount = new AtomicInteger(0);
public static final AtomicInteger restoreBeginCount = new AtomicInteger(0);
public static final AtomicInteger restoreEndCount = new AtomicInteger(0);
+ /** The import config the last import end notification carried, {@code null} until the first. */
+ public static final AtomicReference<LDIFImportConfig> lastImportEndConfig = new AtomicReference<>();
/** Registers the task listeners with the Directory Server. */
public static void registerListeners()
@@ -107,5 +111,6 @@
public void processImportEnd(LocalBackend<?> backend, LDIFImportConfig config, boolean successful)
{
importEndCount.incrementAndGet();
+ lastImportEndConfig.set(config);
}
}
diff --git a/opendj-server-legacy/src/test/java/org/opends/server/tasks/TestImportAndExport.java b/opendj-server-legacy/src/test/java/org/opends/server/tasks/TestImportAndExport.java
index f54a243..872ccdc 100644
--- a/opendj-server-legacy/src/test/java/org/opends/server/tasks/TestImportAndExport.java
+++ b/opendj-server-legacy/src/test/java/org/opends/server/tasks/TestImportAndExport.java
@@ -18,17 +18,22 @@
package org.opends.server.tasks;
import java.io.File;
+import java.io.IOException;
+import java.io.Writer;
+import java.lang.reflect.Field;
import java.util.UUID;
import org.forgerock.opendj.ldap.ResultCode;
import org.opends.server.TestCaseUtils;
import org.opends.server.api.LocalBackend;
import org.opends.server.api.TestTaskListener;
+import org.opends.server.backends.task.Task;
import org.opends.server.backends.task.TaskState;
import org.opends.server.core.AddOperation;
import org.opends.server.core.BackendConfigManager;
import org.opends.server.core.DirectoryServer;
import org.opends.server.types.Entry;
+import org.opends.server.types.LDIFImportConfig;
import org.forgerock.opendj.ldap.schema.ObjectClass;
import org.testng.annotations.AfterClass;
import org.testng.annotations.BeforeClass;
@@ -387,7 +392,10 @@
/**
* An import which cannot disable its backend must still tell the import task listeners
* that the import is over: a listener which took something offline when the import began
- * - a replication domain disables itself - has no other chance to put it back.
+ * - a replication domain disables itself - has no other chance to put it back. The reject
+ * and skip files it opened before touching the backend must be closed on that road too:
+ * it is the one which ends furthest from the backend, so a close which rides on the
+ * backend having been disabled, or on the import having been launched, misses it.
*/
@Test
public void testImportEndsWhenTheBackendCannotBeDisabled() throws Exception
@@ -398,6 +406,7 @@
*/
final String backendID = "importTaskUnconfiguredBackend";
TestCaseUtils.initializeMemoryBackend(backendID, "dc=unconfigured,dc=com", true);
+ File skipFile = File.createTempFile("import-test-skipped", ".ldif");
try
{
int importBeginCount = TestTaskListener.importBeginCount.get();
@@ -410,15 +419,24 @@
"objectclass: ds-task-import",
"ds-task-class-name: org.opends.server.tasks.ImportTask",
"ds-task-import-backend-id: " + backendID,
- "ds-task-import-ldif-file: " + ldifFile.getPath());
+ "ds-task-import-ldif-file: " + ldifFile.getPath(),
+ "ds-task-import-reject-file: " + rejectFile.getPath(),
+ "ds-task-import-skip-file: " + skipFile.getPath(),
+ "ds-task-import-overwrite-rejects: TRUE");
+ TestTaskListener.lastImportEndConfig.set(null);
testTask(taskEntry, TaskState.STOPPED_BY_ERROR, 60);
assertEquals(TestTaskListener.importBeginCount.get(), importBeginCount + 1);
assertEquals(TestTaskListener.importEndCount.get(), importEndCount + 1);
+ LDIFImportConfig importConfig = TestTaskListener.lastImportEndConfig.get();
+ assertNotNull(importConfig, "The import end was not notified");
+ assertClosed(importConfig.getRejectWriter(), "reject");
+ assertClosed(importConfig.getSkipWriter(), "skip");
}
finally
{
+ skipFile.delete();
removeMemoryBackend(backendID);
}
}
@@ -450,6 +468,96 @@
assertEquals(TestTaskListener.importEndCount.get(), importEndCount + 1);
}
+ /**
+ * A failed import must close the reject and skip files it opened. A backend closes the
+ * import config together with its LDIF reader, so an import which fails before that reader
+ * exists leaves the closing to the task, whose error returns must not skip it.
+ */
+ @Test
+ public void testFailedImportClosesItsRejectAndSkipFiles() throws Exception
+ {
+ File skipFile = File.createTempFile("import-test-skipped", ".ldif");
+ try
+ {
+ // A directory can be read, so the task accepts it, but it cannot be opened as LDIF:
+ // the import fails before the backend creates the reader which would close the config.
+ Entry taskEntry = TestCaseUtils.makeEntry(
+ "dn: ds-task-id=" + UUID.randomUUID() + ",cn=Scheduled Tasks,cn=Tasks",
+ "objectclass: top",
+ "objectclass: ds-task",
+ "objectclass: ds-task-import",
+ "ds-task-class-name: org.opends.server.tasks.ImportTask",
+ "ds-task-import-backend-id: userRoot",
+ "ds-task-import-ldif-file: " + ldifFile.getParent(),
+ "ds-task-import-reject-file: " + rejectFile.getPath(),
+ "ds-task-import-skip-file: " + skipFile.getPath(),
+ "ds-task-import-overwrite-rejects: TRUE");
+
+ TestTaskListener.lastImportEndConfig.set(null);
+ testTask(taskEntry, TaskState.STOPPED_BY_ERROR, 60);
+
+ LDIFImportConfig importConfig = TestTaskListener.lastImportEndConfig.get();
+ assertNotNull(importConfig, "The import end was not notified");
+ assertClosed(importConfig.getRejectWriter(), "reject");
+ assertClosed(importConfig.getSkipWriter(), "skip");
+ }
+ finally
+ {
+ skipFile.delete();
+ }
+ }
+
+ /**
+ * An import which cannot open its skip file must close the reject file it already opened.
+ * That failure returns before the import is announced, so the config cannot be read through
+ * the listener the other cases use, and is read from the task the scheduler kept instead.
+ */
+ @Test
+ public void testImportWhichCannotOpenItsSkipFileClosesItsRejectFile() throws Exception
+ {
+ Entry taskEntry = TestCaseUtils.makeEntry(
+ "dn: ds-task-id=" + UUID.randomUUID() + ",cn=Scheduled Tasks,cn=Tasks",
+ "objectclass: top",
+ "objectclass: ds-task",
+ "objectclass: ds-task-import",
+ "ds-task-class-name: org.opends.server.tasks.ImportTask",
+ "ds-task-import-backend-id: userRoot",
+ "ds-task-import-ldif-file: " + ldifFile.getPath(),
+ "ds-task-import-reject-file: " + rejectFile.getPath(),
+ // The task stores the path as it is given, and a directory cannot be opened for writing.
+ "ds-task-import-skip-file: " + ldifFile.getParent(),
+ "ds-task-import-overwrite-rejects: TRUE");
+
+ testTask(taskEntry, TaskState.STOPPED_BY_ERROR, 60);
+
+ LDIFImportConfig importConfig = importConfigOf(getDoneTask(taskEntry.getName()));
+ assertNotNull(importConfig, "The task never built an import config");
+ assertClosed(importConfig.getRejectWriter(), "reject");
+ assertNull(importConfig.getSkipWriter(), "The skip writer was opened after all");
+ }
+
+ /** The config an import task worked with, which the task keeps to itself. */
+ private static LDIFImportConfig importConfigOf(Task task) throws Exception
+ {
+ Field importConfig = ImportTask.class.getDeclaredField("importConfig");
+ importConfig.setAccessible(true);
+ return (LDIFImportConfig) importConfig.get(task);
+ }
+
+ private static void assertClosed(Writer writer, String name)
+ {
+ assertNotNull(writer, "The " + name + " writer was never opened");
+ try
+ {
+ writer.write("still open");
+ fail("The " + name + " writer is still open after the import ended");
+ }
+ catch (IOException expected)
+ {
+ // A closed BufferedWriter refuses the write: that is the closed state being checked.
+ }
+ }
+
private void removeMemoryBackend(String backendID) throws Exception
{
BackendConfigManager backendConfigManager = TestCaseUtils.getServerContext().getBackendConfigManager();
--
Gitblit v1.10.0