[#1026] Close the import config on every path an import task can end on

opendj-server-legacy/src/main/java/org/opends/server/tasks/ImportTask.java

@@ -617,6 +617,14 @@ else if(backend != locatedBackend)
       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 @@ else if(backend != locatedBackend)
     }
     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 @@ else if(backend != locatedBackend)
       }
     }
 
-    // Clean up after the import by closing the import config.
-    importConfig.close();
     return getFinalTaskState();
   }
 

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 class TestTaskListener
   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 processImportBegin(LocalBackend<?> backend, LDIFImportConfig config)
   public void processImportEnd(LocalBackend<?> backend, LDIFImportConfig config, boolean successful)
   {
     importEndCount.incrementAndGet();
+    lastImportEndConfig.set(config);
   }
 }

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 @@ public void testImportExport(Entry taskEntry, TaskState expectedState)
   /**
    * 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 @@ public void testImportEndsWhenTheBackendCannotBeDisabled() throws Exception
      */
     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 @@ public void testImportEndsWhenTheBackendCannotBeDisabled() throws Exception
           "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 @@ public void testFailedImportEndsOnlyOnce() throws Exception
     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();