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

Valery Kharseko

yesterday 4c64fa4aa80953d8c28de98d30622f9b820bec40

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

 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();
  }


 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);
  }
}

 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();