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 aaebe55ee3..779a8e1d60 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 @@ 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(); } 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 8a4f9e9ae9..597cfd3282 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 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 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); } } 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 f54a243a4c..872ccdc488 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 @@ 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();