From d4561f2071fdf4ae023ae26a37e74f3d4d607bc8 Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Sat, 12 Sep 2026 09:17:37 +0300 Subject: [PATCH 1/4] [#1030] Keep the setup log out of the way of start-ds, and say when it is gone Since #576 the launcher scripts put java.io.tmpdir at /tmp and start-ds sweeps that directory clean before starting the server. Setup starts the server through start-ds, so its own log went with the sweep on every run; a start that failed afterwards named a file that was no longer there and printed a NoSuchFileException stack instead of the log. - start-ds / start-ds.bat: remove only the bc-fips-jni_* directories the BC FIPS native loader extracts on every start, which is what the sweep was introduced for; other files in tmp belong to tools that may still be running. - TempLogFile can be placed in a directory of the caller's choosing; SetupLauncher puts the setup log under /logs, next to server.out, falling back to the temporary directory when the launcher does not run from an installation or the directory cannot be used. - Installer checks that the log is readable before promising it, and reports a missing or unreadable log to the listeners instead of printStackTrace(). - build.yml: on a failed test step, print logs/server.out and logs/errors of the instances left behind - the server-side reason of a failed start is nowhere else. Fixes #1030 --- .github/workflows/build.yml | 13 ++ opendj-server-legacy/resource/bin/start-ds | 10 +- .../resource/bin/start-ds.bat | 15 +- .../org/opends/quicksetup/Installation.java | 2 +- .../java/org/opends/quicksetup/Launcher.java | 19 ++- .../org/opends/quicksetup/TempLogFile.java | 64 ++++++++ .../quicksetup/installer/Installer.java | 37 +++-- .../quicksetup/installer/SetupLauncher.java | 26 ++- .../org/opends/messages/quickSetup.properties | 4 + .../opends/quicksetup/TempLogFileTest.java | 151 ++++++++++++++++++ 10 files changed, 311 insertions(+), 30 deletions(-) create mode 100644 opendj-server-legacy/src/test/java/org/opends/quicksetup/TempLogFileTest.java diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index ad914f09e6..1d33a3110c 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -412,6 +412,19 @@ jobs: if ($LASTEXITCODE -ne 0) { throw "net stop 'OpenDJ Server' failed with exit code $LASTEXITCODE" } opendj-server-legacy\target\package\opendj\bat\windows-service.bat --disableService + # A test step that fails leaves its instances behind. The server-side story of a + # failed start lives in logs/server.out and logs/errors, and nothing else prints it + # (setup only has the client-side view, see issue #1030). + - name: Dump the server logs of a failed test + if: failure() + shell: bash + run: | + for f in opendj-server-legacy/target/package/opendj*/logs/server.out opendj-server-legacy/target/package/opendj*/logs/errors; do + [ -f "$f" ] || continue + echo "===== $f" + cat "$f" + done + - name: Upload artifacts OpenDJ Server uses: actions/upload-artifact@v7 with: diff --git a/opendj-server-legacy/resource/bin/start-ds b/opendj-server-legacy/resource/bin/start-ds index 0c965b6ad0..91805e8376 100644 --- a/opendj-server-legacy/resource/bin/start-ds +++ b/opendj-server-legacy/resource/bin/start-ds @@ -14,7 +14,7 @@ # # Copyright 2006-2009 Sun Microsystems, Inc. # Portions Copyright 2011-2014 ForgeRock AS. -# Portions Copyright 2025 3A Systems LLC. +# Portions Copyright 2025-2026 3A Systems LLC. # Capture the current working directory so that we can change to it later. @@ -73,10 +73,14 @@ PID_FILE=${INSTANCE_ROOT}/logs/server.pid LOG_FILE=${INSTANCE_ROOT}/logs/server.out STARTING_FILE=${INSTANCE_ROOT}/logs/server.starting -# Cleanup the tmp directory +# Remove the native libraries the BC FIPS loader extracts into the tmp directory +# on every start, so that they do not pile up. Nothing else in there is ours to +# remove: the tmp directory is java.io.tmpdir for every tool, and a tool may +# still be running - setup starts the server through this script and keeps its +# own log open (issue #1030). OPENDJ_TMP_DIR="${INSTANCE_ROOT}/tmp" if [ -d "${OPENDJ_TMP_DIR}" ]; then - rm -rf ${OPENDJ_TMP_DIR}/* + rm -rf "${OPENDJ_TMP_DIR}"/bc-fips-jni_* fi # See if the provided set of arguments were sufficient for us to be able to diff --git a/opendj-server-legacy/resource/bin/start-ds.bat b/opendj-server-legacy/resource/bin/start-ds.bat index f591502ba2..de3b6791e6 100644 --- a/opendj-server-legacy/resource/bin/start-ds.bat +++ b/opendj-server-legacy/resource/bin/start-ds.bat @@ -58,16 +58,13 @@ echo %SCRIPT%: CLASSPATH=%CLASSPATH% >> %LOG% echo %SCRIPT%: PATH=%PATH% >> %LOG% -rem cleanup the tmp directory -set CUR_DIR=%CD% +rem Remove the native libraries the BC FIPS loader extracts into the tmp +rem directory on every start, so that they do not pile up. Nothing else in +rem there is ours to remove: the tmp directory is java.io.tmpdir for every +rem tool, and a tool may still be running - setup starts the server through +rem this script and keeps its own log open (issue #1030). set OPENDJ_TMP_DIR=%INSTANCE_ROOT%\tmp -rem The paths must be quoted: an unquoted parenthesis (e.g. from -rem "C:\Program Files (x86)") terminates the ( ) block at parse time. -dir /b /s /a "%OPENDJ_TMP_DIR%" | findstr .>nul && ( - cd /d "%OPENDJ_TMP_DIR%" - for /F "delims=" %%i in ('dir /b') do (rmdir "%%i" /s/q>NUL 2>&1 || del "%%i" /s/q>NUL 2>&1) - cd /d "%CUR_DIR%" -) +for /D %%i in ("%OPENDJ_TMP_DIR%\bc-fips-jni_*") do rmdir "%%i" /s/q>NUL 2>&1 "%OPENDJ_JAVA_BIN%" -client %SCRIPT_NAME_ARG% org.opends.server.core.DirectoryServer --configFile "%INSTANCE_ROOT%\config\config.ldif" --checkStartability %* diff --git a/opendj-server-legacy/src/main/java/org/opends/quicksetup/Installation.java b/opendj-server-legacy/src/main/java/org/opends/quicksetup/Installation.java index dbf983fa1a..0c54f02822 100644 --- a/opendj-server-legacy/src/main/java/org/opends/quicksetup/Installation.java +++ b/opendj-server-legacy/src/main/java/org/opends/quicksetup/Installation.java @@ -64,7 +64,7 @@ public final class Installation /** The relative path where the database files are. */ private static final String DATABASES_PATH_RELATIVE = "db"; /** The relative path where the log files are. */ - private static final String LOGS_PATH_RELATIVE = "logs"; + public static final String LOGS_PATH_RELATIVE = "logs"; /** The relative path where the LDIF files are. */ private static final String LDIFS_PATH_RELATIVE = "ldif"; /** The relative path where the backup files are. */ diff --git a/opendj-server-legacy/src/main/java/org/opends/quicksetup/Launcher.java b/opendj-server-legacy/src/main/java/org/opends/quicksetup/Launcher.java index 1fc34e76f5..432d2d5097 100644 --- a/opendj-server-legacy/src/main/java/org/opends/quicksetup/Launcher.java +++ b/opendj-server-legacy/src/main/java/org/opends/quicksetup/Launcher.java @@ -13,6 +13,7 @@ * * Copyright 2008-2009 Sun Microsystems, Inc. * Portions Copyright 2013-2016 ForgeRock AS. + * Portions Copyright 2026 3A Systems, LLC. */ package org.opends.quicksetup; @@ -21,6 +22,7 @@ import static org.opends.messages.QuickSetupMessages.*; import static org.opends.server.util.DynamicConstants.*; +import java.io.File; import java.io.PrintStream; import org.forgerock.i18n.LocalizableMessage; @@ -52,11 +54,26 @@ public abstract class Launcher { * temporary log file path where messages will be logged */ public Launcher(final String[] args, final String tempLogFilePrefix) { + this(args, tempLogFilePrefix, null); + } + + /** + * Creates a Launcher whose temporary log file lives in the given directory. + * + * @param args + * String[] of argument passes from the command line + * @param tempLogFilePrefix + * temporary log file path where messages will be logged + * @param tempLogFileDirectory + * the directory to create the temporary log file in, or {@code null} for the OS + * temporary directory + */ + public Launcher(final String[] args, final String tempLogFilePrefix, final File tempLogFileDirectory) { if (args == null) { throw new IllegalArgumentException("args cannot be null"); } this.args = args; - this.tempLogFile = TempLogFile.newTempLogFile(tempLogFilePrefix); + this.tempLogFile = TempLogFile.newTempLogFile(tempLogFilePrefix, tempLogFileDirectory); } /** diff --git a/opendj-server-legacy/src/main/java/org/opends/quicksetup/TempLogFile.java b/opendj-server-legacy/src/main/java/org/opends/quicksetup/TempLogFile.java index ccb1c9e887..259afe8f66 100644 --- a/opendj-server-legacy/src/main/java/org/opends/quicksetup/TempLogFile.java +++ b/opendj-server-legacy/src/main/java/org/opends/quicksetup/TempLogFile.java @@ -20,6 +20,7 @@ import java.io.File; import java.io.FileOutputStream; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.util.Date; import java.text.DateFormat; @@ -51,6 +52,39 @@ public class TempLogFile */ public static TempLogFile newTempLogFile(final String prefix) { + return newTempLogFile(prefix, null); + } + + /** + * Creates a new temporary log file in the given directory. + *

+ * The directory is created if it does not exist yet. When it is {@code null} or cannot be + * used, the log file goes to the OS temporary directory instead, as with + * {@link #newTempLogFile(String)}. The name of the file follows the pattern + * prefix-[RANDOM_NUMBER_STRING].log either way. + * + * @param prefix + * log file prefix to which log messages will be written. + * @param directory + * the directory to create the log file in, or {@code null} for the OS temporary + * directory. + * @return a new temporary log file. + */ + public static TempLogFile newTempLogFile(final String prefix, final File directory) + { + if (directory != null) + { + try + { + Files.createDirectories(directory.toPath()); + return new TempLogFile(Files.createTempFile(directory.toPath(), prefix, ".log").toFile()); + } + catch (final IOException e) + { + localizedLogger.warn(LocalizableMessage.raw("Unable to create temp log file in " + directory + + " because: " + e.getMessage() + ", falling back to the temporary directory"), e); + } + } try { return new TempLogFile(Files.createTempFile(prefix, ".log").toFile()); @@ -121,6 +155,36 @@ public boolean isEnabled() return logFile != null; } + /** + * Return {@code true} if the temp log file is still on disk and can be read. + *

+ * Unlike {@link #isEnabled()} this is about the file, not the logger: something else may have + * removed the file while the logger still writes to it (see issue #1030), and then there is + * nothing to hand over to whoever needs the log. + * + * @return {@code true} if the temp log file is there and readable. + */ + public boolean isReadable() + { + return logFile != null && Files.isReadable(logFile.toPath()) && Files.isRegularFile(logFile.toPath()); + } + + /** + * Reads the whole temp log file. + * + * @return the contents of the temp log file, decoded as UTF-8. + * @throws IOException + * if the file cannot be read, for instance because it is no longer there. + */ + public String readContents() throws IOException + { + if (logFile == null) + { + throw new IOException("No temp log file"); + } + return new String(Files.readAllBytes(logFile.toPath()), StandardCharsets.UTF_8); + } + /** * Return the absolute path of the temp log file. * @return the absolute path of the temp log file. diff --git a/opendj-server-legacy/src/main/java/org/opends/quicksetup/installer/Installer.java b/opendj-server-legacy/src/main/java/org/opends/quicksetup/installer/Installer.java index 0d3096cfc2..009105dd54 100644 --- a/opendj-server-legacy/src/main/java/org/opends/quicksetup/installer/Installer.java +++ b/opendj-server-legacy/src/main/java/org/opends/quicksetup/installer/Installer.java @@ -42,9 +42,6 @@ import java.io.FileWriter; import java.io.IOException; import java.io.PrintStream; -import java.io.UnsupportedEncodingException; -import java.nio.file.Files; -import java.nio.file.Paths; import java.security.KeyStore; import java.security.KeyStoreException; import java.security.NoSuchAlgorithmException; @@ -619,21 +616,31 @@ public String getInstancePath() private void notifyListenersOfExistingLogFile() { - if (tempLogFile.isEnabled()) + if (!tempLogFile.isEnabled()) { - final String tempLogFilePath = tempLogFile.getPath(); - notifyListeners(getFormattedProgress(INFO_GENERAL_PROVIDE_LOG_IN_ERROR.get(tempLogFilePath))); - //write log - try { - notifyListeners(getLineBreak()); - notifyListeners(LocalizableMessage.valueOf(new String(Files.readAllBytes(Paths.get(tempLogFilePath)),"UTF-8"))); - } catch (UnsupportedEncodingException e) { - e.printStackTrace(); - } catch (IOException e) { - e.printStackTrace(); - } + return; + } + final String tempLogFilePath = tempLogFile.getPath(); + if (!tempLogFile.isReadable()) + { + // Something removed the log while it was being written (issue #1030): say so + // rather than ask for a file that is not there. + notifyListeners(getFormattedWarning(INFO_GENERAL_LOG_IN_ERROR_MISSING.get(tempLogFilePath))); notifyListeners(getLineBreak()); + return; + } + notifyListeners(getFormattedProgress(INFO_GENERAL_PROVIDE_LOG_IN_ERROR.get(tempLogFilePath))); + notifyListeners(getLineBreak()); + // Write the log out as well, so that a report has it even when the file is not attached. + try + { + notifyListeners(LocalizableMessage.raw(tempLogFile.readContents())); } + catch (final IOException e) + { + notifyListeners(getFormattedWarning(INFO_GENERAL_LOG_IN_ERROR_UNREADABLE.get(tempLogFilePath, e))); + } + notifyListeners(getLineBreak()); } /** Creates a default instance. */ diff --git a/opendj-server-legacy/src/main/java/org/opends/quicksetup/installer/SetupLauncher.java b/opendj-server-legacy/src/main/java/org/opends/quicksetup/installer/SetupLauncher.java index ff6820bb20..94446ae875 100644 --- a/opendj-server-legacy/src/main/java/org/opends/quicksetup/installer/SetupLauncher.java +++ b/opendj-server-legacy/src/main/java/org/opends/quicksetup/installer/SetupLauncher.java @@ -13,6 +13,7 @@ * * Copyright 2008-2010 Sun Microsystems, Inc. * Portions Copyright 2014-2016 ForgeRock AS. + * Portions Copyright 2026 3A Systems, LLC. */ package org.opends.quicksetup.installer; @@ -22,6 +23,8 @@ import static com.forgerock.opendj.util.StaticUtils.registerBcProvider; +import java.io.File; + import org.forgerock.i18n.LocalizableMessage; import org.opends.quicksetup.CliApplication; import org.opends.quicksetup.Installation; @@ -65,7 +68,7 @@ public static void main(String[] args) { * @param args the arguments passed by the command lines. */ public SetupLauncher(String[] args) { - super(args, LOG_FILE_PREFIX); + super(args, LOG_FILE_PREFIX, instanceLogsDirectory()); if (System.getProperty(PROPERTY_SCRIPT_NAME) == null) { System.setProperty(PROPERTY_SCRIPT_NAME, Installation.getSetupFileName()); @@ -73,6 +76,27 @@ public SetupLauncher(String[] args) { initializeParser(); } + /** + * The {@code logs/} directory of the instance being set up, where the setup log is kept. + *

+ * The launcher scripts point {@code java.io.tmpdir} at {@code /tmp}, the scratch + * space of every tool, which {@code start-ds} - run by setup itself to start the server - + * used to sweep clean (issue #1030). The log of a failed setup belongs next to the server's + * own logs instead, where {@code server.out} tells the other half of the story. + * + * @return the logs directory of the instance, or {@code null} when the launcher is not + * running from an installation and the OS temporary directory has to do. + */ + private static File instanceLogsDirectory() + { + final String installPath = Utils.getInstallPathFromClasspath(); + if (installPath == null) + { + return null; + } + return new File(Utils.getInstancePathFromInstallPath(installPath), Installation.LOGS_PATH_RELATIVE); + } + /** Initialize the contents of the argument parser. */ protected void initializeParser() { diff --git a/opendj-server-legacy/src/messages/org/opends/messages/quickSetup.properties b/opendj-server-legacy/src/messages/org/opends/messages/quickSetup.properties index 9a304f60e2..f5d02c8bbc 100644 --- a/opendj-server-legacy/src/messages/org/opends/messages/quickSetup.properties +++ b/opendj-server-legacy/src/messages/org/opends/messages/quickSetup.properties @@ -369,6 +369,10 @@ INFO_GENERAL_LOADING=Loading... INFO_GENERAL_SEE_FOR_DETAILS=See %s for a detailed log of this operation. INFO_GENERAL_PROVIDE_LOG_IN_ERROR=See %s for a detailed log of the failed operation.\ \nPlease report this error and provide the log file mentioned above. +INFO_GENERAL_LOG_IN_ERROR_MISSING=The log file %s of the failed operation is no \ + longer there, so it cannot be provided with the error report. +INFO_GENERAL_LOG_IN_ERROR_UNREADABLE=The log file %s of the failed operation \ + could not be read: %s INFO_GENERAL_SERVER_STARTED=started INFO_GENERAL_SERVER_STOPPED=stopped INFO_GENERAL_WARNING=Warning diff --git a/opendj-server-legacy/src/test/java/org/opends/quicksetup/TempLogFileTest.java b/opendj-server-legacy/src/test/java/org/opends/quicksetup/TempLogFileTest.java new file mode 100644 index 0000000000..3b68ebaea8 --- /dev/null +++ b/opendj-server-legacy/src/test/java/org/opends/quicksetup/TempLogFileTest.java @@ -0,0 +1,151 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2026 3A Systems, LLC. + */ +package org.opends.quicksetup; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static java.nio.file.StandardOpenOption.APPEND; +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertNotEquals; +import static org.testng.Assert.assertTrue; +import static org.testng.Assert.fail; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.util.ArrayList; +import java.util.List; + +import org.opends.server.DirectoryServerTestCase; +import org.opends.server.TestCaseUtils; +import org.testng.annotations.AfterClass; +import org.testng.annotations.BeforeClass; +import org.testng.annotations.Test; + +/** + * Tests that a {@link TempLogFile} can be placed in a directory of the caller's choosing (the + * instance {@code logs/} directory for setup, so that {@code start-ds} does not sweep it away + * with the rest of {@code tmp/}, see issue #1030), and that it tells whether the file is still + * there to be read. + */ +@SuppressWarnings("javadoc") +@Test(groups = { "precommit", "quicksetup" }, sequential = true) +public class TempLogFileTest extends DirectoryServerTestCase +{ + private static final String PREFIX = "opendj-setup-"; + + private File tempDir; + private final List created = new ArrayList<>(); + + @BeforeClass + public void setUp() throws IOException + { + tempDir = TestCaseUtils.createTemporaryDirectory("tempLogFileTest"); + } + + @AfterClass + public void tearDown() throws IOException + { + for (TempLogFile logFile : created) + { + logFile.deleteLogFileAfterSuccess(); + } + TestCaseUtils.deleteDirectory(tempDir); + } + + private TempLogFile track(TempLogFile logFile) + { + created.add(logFile); + return logFile; + } + + private static File parentOf(TempLogFile logFile) throws IOException + { + return logFile.getLogFile().getCanonicalFile().getParentFile(); + } + + /** The directory does not exist before setup lays the instance down: it has to be created. */ + @Test + public void testLogFileIsCreatedInTheRequestedDirectory() throws Exception + { + final File logs = new File(tempDir, "not-yet-laid-down/logs"); + assertFalse(logs.exists()); + + final TempLogFile logFile = track(TempLogFile.newTempLogFile(PREFIX, logs)); + + assertTrue(logFile.isEnabled()); + assertTrue(logFile.isReadable()); + assertEquals(parentOf(logFile), logs.getCanonicalFile()); + assertTrue(logFile.getLogFile().getName().startsWith(PREFIX), logFile.getPath()); + assertTrue(logFile.getLogFile().getName().endsWith(".log"), logFile.getPath()); + } + + @Test + public void testReadContentsReturnsWhatIsInTheFile() throws Exception + { + final TempLogFile logFile = track(TempLogFile.newTempLogFile(PREFIX, new File(tempDir, "logs"))); + final String marker = "the last line written before the failure"; + Files.write(logFile.getLogFile().toPath(), (marker + "\n").getBytes(UTF_8), APPEND); + + assertTrue(logFile.readContents().endsWith(marker + "\n")); + } + + /** Being enabled means messages are logged; being readable means the file is there to hand over. */ + @Test + public void testIsReadableFollowsTheFileNotTheLogger() throws Exception + { + final TempLogFile logFile = track(TempLogFile.newTempLogFile(PREFIX, new File(tempDir, "logs"))); + assertTrue(logFile.isReadable()); + + assertTrue(logFile.getLogFile().delete()); + + assertTrue(logFile.isEnabled()); + assertFalse(logFile.isReadable()); + try + { + logFile.readContents(); + fail("reading a deleted log must fail"); + } + catch (IOException expected) + { + // the caller reports it instead of promising the file + } + } + + @Test + public void testNoDirectoryMeansTheTemporaryDirectory() throws Exception + { + final TempLogFile logFile = track(TempLogFile.newTempLogFile(PREFIX, null)); + + assertTrue(logFile.isEnabled()); + assertEquals(parentOf(logFile), new File(System.getProperty("java.io.tmpdir")).getCanonicalFile()); + } + + /** A directory that cannot be used must not cost the log: fall back to the temporary directory. */ + @Test + public void testUnusableDirectoryFallsBackToTheTemporaryDirectory() throws Exception + { + final File notADirectory = new File(tempDir, "not-a-directory"); + assertTrue(notADirectory.createNewFile()); + + final TempLogFile logFile = track(TempLogFile.newTempLogFile(PREFIX, notADirectory)); + + assertTrue(logFile.isEnabled()); + assertTrue(logFile.isReadable()); + assertNotEquals(parentOf(logFile), notADirectory.getCanonicalFile()); + assertEquals(parentOf(logFile), new File(System.getProperty("java.io.tmpdir")).getCanonicalFile()); + } +} From 7624f9496f918d6c7a56636294d98fbb4f1fe93c Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Mon, 21 Sep 2026 20:07:50 +0300 Subject: [PATCH 2/4] [#1030] Create the setup log only where an install is at stake Round 2 of the review. The log was built in the launcher constructor, before any argument was parsed, and only a successful install removed it, so --help, --version, a usage error, "already installed", a refused licence and a cancel each left an opendj-setup-*.log - and, on a package not yet set up, the logs/ directory itself - behind for good. Launcher now creates it on the first road that can fail an operation: SetupLauncher hands InstallDS a Supplier called where the installer is built, hasTempLogFile() answers whether there is a log to name without making one, and a cancelled install deletes the log that road never names. Also from the review: the error and debug publishers installed with the log are taken off the logger singletons when it is deleted; the fallback warning is logged once there is a log to carry it; readContents() decodes with the charset TextWriter.STREAM wrote with; start-ds.bat uses %%~i; InstallerTest pins the arms of the failure report and LauncherTest pins when the log is created; TempLogFileTest removes the log through deleteLogFileAfterSuccess() (Windows will not delete an open file) and shuts the writer before appending its marker; the FIPS and Windows CI steps check that the tmp/ sweep takes the bc-fips-jni_* directories and nothing else. --- .github/workflows/build.yml | 16 ++ .../resource/bin/start-ds.bat | 4 +- .../uninstaller/UninstallLauncher.java | 5 +- .../java/org/opends/quicksetup/Launcher.java | 52 +++++- .../org/opends/quicksetup/TempLogFile.java | 48 ++++- .../quicksetup/installer/Installer.java | 7 +- .../quicksetup/installer/SetupLauncher.java | 13 +- .../org/opends/server/tools/InstallDS.java | 41 ++++- .../org/opends/quicksetup/LauncherTest.java | 133 ++++++++++++++ .../opends/quicksetup/TempLogFileTest.java | 25 ++- .../quicksetup/installer/InstallerTest.java | 168 ++++++++++++++++++ 11 files changed, 480 insertions(+), 32 deletions(-) create mode 100644 opendj-server-legacy/src/test/java/org/opends/quicksetup/LauncherTest.java create mode 100644 opendj-server-legacy/src/test/java/org/opends/quicksetup/installer/InstallerTest.java diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 1d33a3110c..ab39c9043f 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -241,8 +241,17 @@ jobs: --baseDN dc=example,dc=com --sampleData 5000 --cli --acceptLicense --no-prompt \ --propertiesFilePath /tmp/opendj-setup.properties.bcfks --doNotStart + # start-ds sweeps the native libraries the BC FIPS loader leaves in tmp/ and nothing + # else: the rest of tmp/ belongs to tools that may still be running, setup among them + # (issue #1030). Without these two probes a sweep back to "tmp/*" passes every cell. + mkdir -p opendj-server-legacy/target/package/opendj/tmp/bc-fips-jni_123 + touch opendj-server-legacy/target/package/opendj/tmp/keep.me + opendj-server-legacy/target/package/opendj/bin/start-ds + test ! -e opendj-server-legacy/target/package/opendj/tmp/bc-fips-jni_123 + test -e opendj-server-legacy/target/package/opendj/tmp/keep.me + opendj-server-legacy/target/package/opendj/bin/status --hostname localhost --bindDN "cn=Directory Manager" --bindPassword password --trustAll opendj-server-legacy/target/package/opendj/bin/ldapsearch --hostname localhost --port 1636 --bindDN "cn=Directory Manager" --bindPassword password --useSsl --trustAll --baseDN "dc=example,dc=com" --searchScope base "(objectClass=*)" 1.1 opendj-server-legacy/target/package/opendj/bin/ldapsearch --hostname localhost --port 1636 --bindDN "cn=Directory Manager" --bindPassword password --useSsl --trustAll --baseDN "ou=people,dc=example,dc=com" --searchScope sub "(uid=user.*)" dn | grep ^dn: | wc -l | grep -q 5000 @@ -397,7 +406,14 @@ jobs: opendj-server-legacy\target\package\opendj\upgrade.bat opendj-server-legacy\target\package\opendj\bat\import-ldif.bat --offline --ldifFile test.ldif --backendID=example2 opendj-server-legacy\target\package\opendj\bat\rebuild-index.bat --offline --bindDN "cn=Directory Manager" --bindPassword password --baseDN "dc=example2,dc=com" --rebuildAll + # The same probe as the Unix FIPS step, for start-ds.bat: the sweep takes the BC FIPS + # directories and leaves everything else in tmp\ alone (issue #1030). The runner's + # install path has no space in it, so this does not settle the quoting of the for /D line. + New-Item -ItemType Directory -Force opendj-server-legacy\target\package\opendj\tmp\bc-fips-jni_123 | Out-Null + New-Item -ItemType File -Force opendj-server-legacy\target\package\opendj\tmp\keep.me | Out-Null opendj-server-legacy\target\package\opendj\bat\start-ds.bat + if (Test-Path opendj-server-legacy\target\package\opendj\tmp\bc-fips-jni_123) { throw "start-ds.bat left a bc-fips-jni_* directory behind" } + if (-not (Test-Path opendj-server-legacy\target\package\opendj\tmp\keep.me)) { throw "start-ds.bat removed a tmp file that is not its own" } opendj-server-legacy\target\package\opendj\bat\rebuild-index.bat --bindDN "cn=Directory Manager" --bindPassword password --baseDN "dc=example2,dc=com" --rebuildAll --trustAll opendj-server-legacy\target\package\opendj\bat\ldapsearch.bat --hostname localhost --port 1636 --bindDN "cn=Directory Manager" --bindPassword password --useSsl --trustAll --baseDN "dc=example2,dc=com" --searchScope sub "(uid=user.*)" dn | find /c '"dn:"' | findstr "10000" opendj-server-legacy\target\package\opendj\bat\stop-ds.bat diff --git a/opendj-server-legacy/resource/bin/start-ds.bat b/opendj-server-legacy/resource/bin/start-ds.bat index de3b6791e6..46c5f31f95 100644 --- a/opendj-server-legacy/resource/bin/start-ds.bat +++ b/opendj-server-legacy/resource/bin/start-ds.bat @@ -64,7 +64,9 @@ rem there is ours to remove: the tmp directory is java.io.tmpdir for every rem tool, and a tool may still be running - setup starts the server through rem this script and keeps its own log open (issue #1030). set OPENDJ_TMP_DIR=%INSTANCE_ROOT%\tmp -for /D %%i in ("%OPENDJ_TMP_DIR%\bc-fips-jni_*") do rmdir "%%i" /s/q>NUL 2>&1 +rem %%~i drops the quotes cmd would hand back if it ever kept them: an install path with a +rem space would otherwise split the rmdir argument, and the redirect would hide it. +for /D %%i in ("%OPENDJ_TMP_DIR%\bc-fips-jni_*") do rmdir "%%~i" /s/q>NUL 2>&1 "%OPENDJ_JAVA_BIN%" -client %SCRIPT_NAME_ARG% org.opends.server.core.DirectoryServer --configFile "%INSTANCE_ROOT%\config\config.ldif" --checkStartability %* diff --git a/opendj-server-legacy/src/main/java/org/opends/guitools/uninstaller/UninstallLauncher.java b/opendj-server-legacy/src/main/java/org/opends/guitools/uninstaller/UninstallLauncher.java index 0f714af2f6..301b48b3a8 100644 --- a/opendj-server-legacy/src/main/java/org/opends/guitools/uninstaller/UninstallLauncher.java +++ b/opendj-server-legacy/src/main/java/org/opends/guitools/uninstaller/UninstallLauncher.java @@ -13,6 +13,7 @@ * * Copyright 2006-2010 Sun Microsystems, Inc. * Portions Copyright 2011-2016 ForgeRock AS. + * Portions Copyright 2026 3A Systems, LLC. */ package org.opends.guitools.uninstaller; @@ -129,8 +130,8 @@ protected void initializeParser() @Override protected void guiLaunchFailed() { System.err.println( - tempLogFile.isEnabled() ? ERR_UNINSTALL_LAUNCHER_GUI_LAUNCHED_FAILED_DETAILS.get(tempLogFile.getPath()) - : ERR_UNINSTALL_LAUNCHER_GUI_LAUNCHED_FAILED.get()); + hasTempLogFile() ? ERR_UNINSTALL_LAUNCHER_GUI_LAUNCHED_FAILED_DETAILS.get(getTempLogFile().getPath()) + : ERR_UNINSTALL_LAUNCHER_GUI_LAUNCHED_FAILED.get()); } @Override diff --git a/opendj-server-legacy/src/main/java/org/opends/quicksetup/Launcher.java b/opendj-server-legacy/src/main/java/org/opends/quicksetup/Launcher.java index 432d2d5097..53171a6ea9 100644 --- a/opendj-server-legacy/src/main/java/org/opends/quicksetup/Launcher.java +++ b/opendj-server-legacy/src/main/java/org/opends/quicksetup/Launcher.java @@ -42,8 +42,14 @@ public abstract class Launcher { /** Arguments with which this launcher was invoked. */ protected final String[] args; - /** The temporary log file which will be kept if an error occurs. */ - protected final TempLogFile tempLogFile; + private final String tempLogFilePrefix; + private final File tempLogFileDirectory; + /** + * The temporary log file which will be kept if an error occurs; see {@link #getTempLogFile()}. + * Volatile because the GUI road asks for it from the thread that runs the splash screen and + * the roads after it read it from the main thread. + */ + private volatile TempLogFile tempLogFile; /** * Creates a Launcher. @@ -73,7 +79,36 @@ public Launcher(final String[] args, final String tempLogFilePrefix, final File throw new IllegalArgumentException("args cannot be null"); } this.args = args; - this.tempLogFile = TempLogFile.newTempLogFile(tempLogFilePrefix, tempLogFileDirectory); + this.tempLogFilePrefix = tempLogFilePrefix; + this.tempLogFileDirectory = tempLogFileDirectory; + } + + /** + * The temporary log file of this launcher, created the first time it is asked for. + *

+ * Creating it costs a file - and, with a directory of the caller's choosing, the directory + * as well - that nothing removes afterwards unless the operation succeeds. So it is created + * on the first road that can fail an operation and not before: {@code --help}, + * {@code --version}, a usage error and the other roads that attempt nothing leave no log + * behind (issue #1030). + * + * @return the temporary log file, creating it if this is the first call. + */ + protected synchronized TempLogFile getTempLogFile() { + if (tempLogFile == null) { + tempLogFile = TempLogFile.newTempLogFile(tempLogFilePrefix, tempLogFileDirectory); + } + return tempLogFile; + } + + /** + * Whether there is a log to name, without creating one to answer. + * + * @return {@code true} if a temporary log file has been created and can be used to log + * messages. + */ + protected boolean hasTempLogFile() { + return tempLogFile != null && tempLogFile.isEnabled(); } /** @@ -211,12 +246,12 @@ public void run() { try { - SplashScreen.main(tempLogFile, args); + SplashScreen.main(getTempLogFile(), args); returnValue[0] = 0; } catch (Throwable t) { - if (tempLogFile.isEnabled()) + if (hasTempLogFile()) { logger.warn(LocalizableMessage.raw("Error launching GUI: "+t)); StringBuilder buf = new StringBuilder(); @@ -354,6 +389,9 @@ else if (shouldPrintUsage()) { } System.exit(ReturnCode.SUCCESSFUL.getReturnCode()); } else if (isCli()) { + // An operation is about to run: from here on there is something worth logging, and + // preExit() names the file. The roads above attempt nothing and leave no log behind. + getTempLogFile(); CliApplication cliApp = createCliApplication(); int exitCode = launchCli(cliApp); preExit(cliApp); @@ -378,8 +416,8 @@ private void preExit(CliApplication cliApp) { // Add an extra space systematically System.out.println(); - if (tempLogFile.isEnabled()) { - System.out.println(INFO_GENERAL_SEE_FOR_DETAILS.get(tempLogFile.getPath())); + if (hasTempLogFile()) { + System.out.println(INFO_GENERAL_SEE_FOR_DETAILS.get(getTempLogFile().getPath())); } } } diff --git a/opendj-server-legacy/src/main/java/org/opends/quicksetup/TempLogFile.java b/opendj-server-legacy/src/main/java/org/opends/quicksetup/TempLogFile.java index 259afe8f66..6f193b8803 100644 --- a/opendj-server-legacy/src/main/java/org/opends/quicksetup/TempLogFile.java +++ b/opendj-server-legacy/src/main/java/org/opends/quicksetup/TempLogFile.java @@ -20,7 +20,7 @@ import java.io.File; import java.io.FileOutputStream; import java.io.IOException; -import java.nio.charset.StandardCharsets; +import java.nio.charset.Charset; import java.nio.file.Files; import java.util.Date; import java.text.DateFormat; @@ -72,6 +72,7 @@ public static TempLogFile newTempLogFile(final String prefix) */ public static TempLogFile newTempLogFile(final String prefix, final File directory) { + IOException fallbackReason = null; if (directory != null) { try @@ -81,13 +82,21 @@ public static TempLogFile newTempLogFile(final String prefix, final File directo } catch (final IOException e) { - localizedLogger.warn(LocalizableMessage.raw("Unable to create temp log file in " + directory - + " because: " + e.getMessage() + ", falling back to the temporary directory"), e); + // Nothing can be logged yet: the first publisher is the one the constructor installs + // below, so the warning has to wait until there is a log to write it to. + fallbackReason = e; } } try { - return new TempLogFile(Files.createTempFile(prefix, ".log").toFile()); + final TempLogFile tempLogFile = new TempLogFile(Files.createTempFile(prefix, ".log").toFile()); + if (fallbackReason != null) + { + localizedLogger.warn(LocalizableMessage.raw("Unable to create temp log file in " + directory + + " because: " + fallbackReason.getMessage() + ", falling back to the temporary directory"), + fallbackReason); + } + return tempLogFile; } catch (final IOException e) { @@ -102,9 +111,14 @@ private TempLogFile() { this.logFile = null; this.writer=null; + this.startupErrorLogPublisher = null; + this.startupDebugLogPublisher = null; } final TextWriter writer; + /** Kept so that they can be taken off the logger singletons again, see {@link #deleteLogFileAfterSuccess()}. */ + private final ErrorLogPublisher startupErrorLogPublisher; + private final DebugLogPublisher startupDebugLogPublisher; private TempLogFile(final File file) throws IOException { @@ -117,9 +131,9 @@ private TempLogFile(final File file) throws IOException }else { writer=new TextWriter.STREAM(new FileOutputStream(file)); } - ErrorLogPublisher startupErrorLogPublisher = TextErrorLogPublisher.getServerStartupTextErrorPublisher(writer); + startupErrorLogPublisher = TextErrorLogPublisher.getServerStartupTextErrorPublisher(writer); ErrorLogger.getInstance().addLogPublisher(startupErrorLogPublisher); - DebugLogger.getInstance().addPublisherIfRequired(writer); + startupDebugLogPublisher = DebugLogger.getInstance().addPublisherIfRequired(writer); localizedLogger.info(LocalizableMessage.raw("QuickSetup application launched " + DateFormat.getDateTimeInstance(DateFormat.LONG, DateFormat.LONG).format(new Date()), null)); } @@ -134,11 +148,23 @@ public File getLogFile() return logFile; } - /** Closes the log file handler and delete the temp log file . */ + /** + * Closes the log file handler and delete the temp log file . + *

+ * The publishers installed by the constructor go with it: they are held by the logger + * singletons, which outlive this object, and once the writer is shut everything they are + * handed is written to a closed stream and swallowed. + */ public void deleteLogFileAfterSuccess() { if (isEnabled()) { + if (startupErrorLogPublisher != null) { + ErrorLogger.getInstance().removeLogPublisher(startupErrorLogPublisher); + } + if (startupDebugLogPublisher != null) { + DebugLogger.getInstance().removeLogPublisher(startupDebugLogPublisher); + } if (writer!=null) { writer.shutdown(); } @@ -171,8 +197,12 @@ public boolean isReadable() /** * Reads the whole temp log file. + *

+ * The file is decoded with the default charset of the JVM, which is the one + * {@link TextWriter.STREAM} wrote it with: reader and writer are the same JVM, so a + * non-ASCII path or base DN in a report comes back as it was logged. * - * @return the contents of the temp log file, decoded as UTF-8. + * @return the contents of the temp log file. * @throws IOException * if the file cannot be read, for instance because it is no longer there. */ @@ -182,7 +212,7 @@ public String readContents() throws IOException { throw new IOException("No temp log file"); } - return new String(Files.readAllBytes(logFile.toPath()), StandardCharsets.UTF_8); + return new String(Files.readAllBytes(logFile.toPath()), Charset.defaultCharset()); } /** diff --git a/opendj-server-legacy/src/main/java/org/opends/quicksetup/installer/Installer.java b/opendj-server-legacy/src/main/java/org/opends/quicksetup/installer/Installer.java index 009105dd54..17b8266156 100644 --- a/opendj-server-legacy/src/main/java/org/opends/quicksetup/installer/Installer.java +++ b/opendj-server-legacy/src/main/java/org/opends/quicksetup/installer/Installer.java @@ -314,6 +314,10 @@ public void run() uninstall(); setCurrentProgressStep(InstallProgressStep.FINISHED_CANCELED); notifyListeners(null); + // Nothing names this log on this road - notifyListenersOfExistingLogFile() belongs to + // handleInstallationError() below - and uninstall() has just taken the installation + // back, so keeping the file would leave a report nobody is told about (issue #1030). + tempLogFile.deleteLogFileAfterSuccess(); } else { handleInstallationError(ex); } @@ -614,7 +618,8 @@ public String getInstancePath() return Utils.getInstancePathFromInstallPath(installPath); } - private void notifyListenersOfExistingLogFile() + /** Package-private so that {@code InstallerTest} can drive every road of this report. */ + void notifyListenersOfExistingLogFile() { if (!tempLogFile.isEnabled()) { diff --git a/opendj-server-legacy/src/main/java/org/opends/quicksetup/installer/SetupLauncher.java b/opendj-server-legacy/src/main/java/org/opends/quicksetup/installer/SetupLauncher.java index 94446ae875..99c0a981c6 100644 --- a/opendj-server-legacy/src/main/java/org/opends/quicksetup/installer/SetupLauncher.java +++ b/opendj-server-legacy/src/main/java/org/opends/quicksetup/installer/SetupLauncher.java @@ -83,6 +83,11 @@ public SetupLauncher(String[] args) { * space of every tool, which {@code start-ds} - run by setup itself to start the server - * used to sweep clean (issue #1030). The log of a failed setup belongs next to the server's * own logs instead, where {@code server.out} tells the other half of the story. + *

+ * Neither the directory nor the log is created here: {@link Launcher#getTempLogFile()} + * creates both when an install is about to run, so that a road which installs nothing - + * {@code setup --help} on a package whose instance directory is not laid down yet, for one - + * leaves nothing behind. * * @return the logs directory of the instance, or {@code null} when the launcher is not * running from an installation and the OS temporary directory has to do. @@ -133,7 +138,7 @@ else if (argParser.isUsageArgumentPresent() || else if (isCli()) { Utils.checkJavaVersion(); - System.exit(InstallDS.mainCLI(args, tempLogFile)); + System.exit(InstallDS.mainCLI(args, this::getTempLogFile)); } else { @@ -145,7 +150,7 @@ else if (isCli()) if (exitCode != 0) { guiLaunchFailed(); Utils.checkJavaVersion(); - System.exit(InstallDS.mainCLI(args, tempLogFile)); + System.exit(InstallDS.mainCLI(args, this::getTempLogFile)); } } } @@ -169,8 +174,8 @@ public ArgumentParser getArgumentParser() { @Override protected void guiLaunchFailed() { System.err.println( - tempLogFile.isEnabled() ? INFO_SETUP_LAUNCHER_GUI_LAUNCHED_FAILED_DETAILS.get(tempLogFile.getPath()) - : INFO_SETUP_LAUNCHER_GUI_LAUNCHED_FAILED.get()); + hasTempLogFile() ? INFO_SETUP_LAUNCHER_GUI_LAUNCHED_FAILED_DETAILS.get(getTempLogFile().getPath()) + : INFO_SETUP_LAUNCHER_GUI_LAUNCHED_FAILED.get()); } @Override diff --git a/opendj-server-legacy/src/main/java/org/opends/server/tools/InstallDS.java b/opendj-server-legacy/src/main/java/org/opends/server/tools/InstallDS.java index b652428599..a0a2fdc546 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/tools/InstallDS.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/tools/InstallDS.java @@ -40,6 +40,7 @@ import java.util.Collections; import java.util.LinkedList; import java.util.List; +import java.util.function.Supplier; import com.forgerock.opendj.util.FipsStaticUtils; import org.forgerock.i18n.LocalizableMessage; @@ -202,7 +203,11 @@ public int getReturnCode() private Integer lastResetAdminConnectorPort; private Integer lastResetJmxPort; - private final TempLogFile tempLogFile; + /** + * The temporary log file where messages will be logged, asked for only once an install is + * about to run: the roads which return before that one leave no log behind (issue #1030). + */ + private final Supplier tempLogFile; private static final LocalizedLogger logger = LocalizedLogger.getLoggerForThisClass(); @@ -216,7 +221,7 @@ public int getReturnCode() * @param tempLogFile * the temporary log file where messages will be logged. */ - private InstallDS(PrintStream out, PrintStream err, TempLogFile tempLogFile) + private InstallDS(PrintStream out, PrintStream err, Supplier tempLogFile) { super(out, err); this.tempLogFile = tempLogFile; @@ -229,10 +234,12 @@ private InstallDS(PrintStream out, PrintStream err, TempLogFile tempLogFile) * @param args * the command-line arguments provided to this program. * @param tempLogFile - * the temporary log file where messages will be logged. + * supplies the temporary log file where messages will be logged. It is called on + * the road that runs the install and not on the roads which return before it, so + * that a run which installs nothing creates no log. * @return The error code. */ - public static int mainCLI(String[] args, final TempLogFile tempLogFile) + public static int mainCLI(String[] args, final Supplier tempLogFile) { return mainCLI(args, System.out, System.err, tempLogFile); } @@ -259,7 +266,29 @@ public static int mainCLI( // // *NOTE* this method has been kept public because it is used by OpenAM. // + return mainCLI(args, outStream, errStream, () -> tempLogFile); + } + /** + * Parses the provided command-line arguments and uses that information to run + * the setup tool. + * + * @param args + * The command-line arguments provided to this program. + * @param outStream + * The output stream to use for standard output, or null + * if standard output is not needed. + * @param errStream + * The output stream to use for standard error, or null + * if standard error is not needed. + * @param tempLogFile + * supplies the temporary log file where messages will be logged, see + * {@link #mainCLI(String[], Supplier)}. + * @return The error code. + */ + public static int mainCLI( + String[] args, OutputStream outStream, OutputStream errStream, Supplier tempLogFile) + { final PrintStream out = NullOutputStream.wrapOrNullStream(outStream); System.setProperty(Constants.CLI_JAVA_PROPERTY, "true"); @@ -349,7 +378,9 @@ private int execute(String[] args) System.setProperty(Constants.CLI_JAVA_PROPERTY, "true"); final Installer installer = new Installer(); - installer.setTempLogFile(tempLogFile); + // The first point where an install can fail: from here on there is a log to keep, and + // this is where it gets created. + installer.setTempLogFile(tempLogFile.get()); installer.setUserData(uData); installer.setProgressMessageFormatter(formatter); installer.addProgressUpdateListener( diff --git a/opendj-server-legacy/src/test/java/org/opends/quicksetup/LauncherTest.java b/opendj-server-legacy/src/test/java/org/opends/quicksetup/LauncherTest.java new file mode 100644 index 0000000000..e429c84503 --- /dev/null +++ b/opendj-server-legacy/src/test/java/org/opends/quicksetup/LauncherTest.java @@ -0,0 +1,133 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2026 3A Systems, LLC. + */ +package org.opends.quicksetup; + +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertSame; +import static org.testng.Assert.assertTrue; + +import java.io.File; +import java.io.IOException; + +import org.forgerock.i18n.LocalizableMessage; +import org.opends.server.DirectoryServerTestCase; +import org.opends.server.TestCaseUtils; +import org.testng.annotations.AfterClass; +import org.testng.annotations.BeforeClass; +import org.testng.annotations.Test; + +import com.forgerock.opendj.cli.ArgumentParser; + +/** + * Tests when a launcher creates its temporary log file. + *

+ * Nothing removes that file unless the operation it belongs to succeeds, so a road which + * attempts nothing - {@code setup --help}, {@code --version}, a usage error - must not create + * one, nor the directory it would live in (issue #1030). + */ +@SuppressWarnings("javadoc") +@Test(groups = { "precommit", "quicksetup" }, sequential = true) +public class LauncherTest extends DirectoryServerTestCase +{ + private static final String PREFIX = "opendj-setup-"; + + private File tempDir; + private TempLogFile created; + + @BeforeClass + public void setUp() throws IOException + { + tempDir = TestCaseUtils.createTemporaryDirectory("launcherTest"); + } + + @AfterClass + public void tearDown() throws IOException + { + if (created != null) + { + created.deleteLogFileAfterSuccess(); + } + TestCaseUtils.deleteDirectory(tempDir); + } + + @Test + public void testBuildingALauncherLeavesNothingOnDisk() throws Exception + { + final File instance = new File(tempDir, "not-yet-laid-down"); + final File logs = new File(instance, "logs"); + + final TestLauncher launcher = new TestLauncher(logs); + + assertFalse(launcher.hasTempLogFile(), "the log must wait for a road that can fail"); + assertFalse(logs.exists(), logs.getPath()); + assertFalse(instance.exists(), instance.getPath()); + } + + @Test + public void testAskingForTheLogCreatesItOnce() throws Exception + { + final File logs = new File(tempDir, "asked-for/logs"); + final TestLauncher launcher = new TestLauncher(logs); + + final TempLogFile logFile = launcher.getTempLogFile(); + created = logFile; + + assertTrue(logFile.isReadable(), logFile.getPath()); + assertEquals(logFile.getLogFile().getCanonicalFile().getParentFile(), logs.getCanonicalFile()); + assertTrue(launcher.hasTempLogFile()); + assertSame(launcher.getTempLogFile(), logFile, "a second ask must not create a second log"); + } + + /** A launcher with nothing in it but the log file behaviour under test. */ + private static final class TestLauncher extends Launcher + { + TestLauncher(final File tempLogFileDirectory) + { + super(new String[0], PREFIX, tempLogFileDirectory); + } + + @Override + public ArgumentParser getArgumentParser() + { + return null; + } + + @Override + protected LocalizableMessage getFrameTitle() + { + return LocalizableMessage.raw("test"); + } + + @Override + protected CliApplication createCliApplication() + { + return null; + } + + @Override + protected void willLaunchGui() + { + // nothing is launched here + } + + @Override + protected void guiLaunchFailed() + { + // nothing is launched here + } + } +} diff --git a/opendj-server-legacy/src/test/java/org/opends/quicksetup/TempLogFileTest.java b/opendj-server-legacy/src/test/java/org/opends/quicksetup/TempLogFileTest.java index 3b68ebaea8..c3add16ebc 100644 --- a/opendj-server-legacy/src/test/java/org/opends/quicksetup/TempLogFileTest.java +++ b/opendj-server-legacy/src/test/java/org/opends/quicksetup/TempLogFileTest.java @@ -15,7 +15,7 @@ */ package org.opends.quicksetup; -import static java.nio.charset.StandardCharsets.UTF_8; +import static java.nio.charset.Charset.defaultCharset; import static java.nio.file.StandardOpenOption.APPEND; import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertFalse; @@ -97,8 +97,12 @@ public void testLogFileIsCreatedInTheRequestedDirectory() throws Exception public void testReadContentsReturnsWhatIsInTheFile() throws Exception { final TempLogFile logFile = track(TempLogFile.newTempLogFile(PREFIX, new File(tempDir, "logs"))); + // The log's own stream is not in append mode and sits at the end of its own bytes, so a + // record written after the marker would be written over it: shut the writer first, and + // the marker is the last thing in the file whatever else the JVM logs. + logFile.writer.shutdown(); final String marker = "the last line written before the failure"; - Files.write(logFile.getLogFile().toPath(), (marker + "\n").getBytes(UTF_8), APPEND); + Files.write(logFile.getLogFile().toPath(), (marker + "\n").getBytes(defaultCharset()), APPEND); assertTrue(logFile.readContents().endsWith(marker + "\n")); } @@ -110,7 +114,10 @@ public void testIsReadableFollowsTheFileNotTheLogger() throws Exception final TempLogFile logFile = track(TempLogFile.newTempLogFile(PREFIX, new File(tempDir, "logs"))); assertTrue(logFile.isReadable()); - assertTrue(logFile.getLogFile().delete()); + // Not File.delete(): the writer still holds the file, and Windows does not delete a file + // that is open. deleteLogFileAfterSuccess() shuts the writer first, as setup does. + logFile.deleteLogFileAfterSuccess(); + assertFalse(logFile.getLogFile().exists()); assertTrue(logFile.isEnabled()); assertFalse(logFile.isReadable()); @@ -125,6 +132,18 @@ public void testIsReadableFollowsTheFileNotTheLogger() throws Exception } } + /** A directory where the log was is not a log: there is nothing to hand over either. */ + @Test + public void testADirectoryAtTheLogPathIsNotReadable() throws Exception + { + final TempLogFile logFile = track(TempLogFile.newTempLogFile(PREFIX, new File(tempDir, "logs"))); + logFile.deleteLogFileAfterSuccess(); + assertTrue(logFile.getLogFile().mkdir()); + + assertTrue(Files.isReadable(logFile.getLogFile().toPath())); + assertFalse(logFile.isReadable()); + } + @Test public void testNoDirectoryMeansTheTemporaryDirectory() throws Exception { diff --git a/opendj-server-legacy/src/test/java/org/opends/quicksetup/installer/InstallerTest.java b/opendj-server-legacy/src/test/java/org/opends/quicksetup/installer/InstallerTest.java new file mode 100644 index 0000000000..6e9f462188 --- /dev/null +++ b/opendj-server-legacy/src/test/java/org/opends/quicksetup/installer/InstallerTest.java @@ -0,0 +1,168 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2026 3A Systems, LLC. + */ +package org.opends.quicksetup.installer; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.opends.messages.QuickSetupMessages.*; +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertTrue; + +import java.io.File; +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; + +import org.forgerock.i18n.LocalizableMessage; +import org.opends.quicksetup.TempLogFile; +import org.opends.quicksetup.event.ProgressUpdateEvent; +import org.opends.quicksetup.event.ProgressUpdateListener; +import org.opends.quicksetup.util.PlainTextProgressMessageFormatter; +import org.opends.server.DirectoryServerTestCase; +import org.opends.server.TestCaseUtils; +import org.testng.annotations.AfterClass; +import org.testng.annotations.BeforeClass; +import org.testng.annotations.Test; + +/** + * Tests what a failed installation tells its listeners about the log file. + *

+ * This report is the one place the diagnosis of a failed setup lives, and it used to promise + * the file without looking at it - printing a {@code NoSuchFileException} stack when the file + * was gone (issue #1030). The roads below are the whole of that decision. + */ +@SuppressWarnings("javadoc") +@Test(groups = { "precommit", "quicksetup" }, sequential = true) +public class InstallerTest extends DirectoryServerTestCase +{ + private static final String PREFIX = "opendj-setup-"; + + private File tempDir; + private final List created = new ArrayList<>(); + + @BeforeClass + public void setUp() throws IOException + { + tempDir = TestCaseUtils.createTemporaryDirectory("installerTest"); + } + + @AfterClass + public void tearDown() throws IOException + { + for (TempLogFile logFile : created) + { + logFile.deleteLogFileAfterSuccess(); + } + TestCaseUtils.deleteDirectory(tempDir); + } + + /** A log that is there is named and written out, as before. */ + @Test + public void testAReadableLogIsHandedOver() throws Exception + { + final TempLogFile logFile = newLogFile(); + + final String report = reportOf(logFile); + + assertContains(report, INFO_GENERAL_PROVIDE_LOG_IN_ERROR.get(logFile.getPath())); + // The line the constructor logs: the contents of the file reach the report. + assertTrue(report.contains("QuickSetup application launched"), report); + assertDoesNotContain(report, INFO_GENERAL_LOG_IN_ERROR_MISSING.get(logFile.getPath())); + } + + /** A log something else removed is reported as gone, instead of being asked for. */ + @Test + public void testAMissingLogIsReportedAsMissing() throws Exception + { + final TempLogFile logFile = newLogFile(); + final String path = logFile.getPath(); + logFile.deleteLogFileAfterSuccess(); + assertFalse(logFile.isReadable()); + + final String report = reportOf(logFile); + + assertContains(report, INFO_GENERAL_LOG_IN_ERROR_MISSING.get(path)); + // The line that asks for the file must not go out when there is no file to provide. + assertDoesNotContain(report, INFO_GENERAL_PROVIDE_LOG_IN_ERROR.get(path)); + } + + /** A log which is there but cannot be read costs the report its contents, not its diagnosis. */ + @Test + public void testAnUnreadableLogIsReportedAsUnreadable() throws Exception + { + final String path = new File(tempDir, "unreadable.log").getAbsolutePath(); + final IOException failure = new IOException("Input/output error"); + final TempLogFile logFile = mock(TempLogFile.class); + when(logFile.isEnabled()).thenReturn(true); + when(logFile.isReadable()).thenReturn(true); + when(logFile.getPath()).thenReturn(path); + when(logFile.readContents()).thenThrow(failure); + + final String report = reportOf(logFile); + + assertContains(report, INFO_GENERAL_LOG_IN_ERROR_UNREADABLE.get(path, failure)); + assertDoesNotContain(report, INFO_GENERAL_LOG_IN_ERROR_MISSING.get(path)); + } + + /** Nothing is said about a log that was never created. */ + @Test + public void testNoLogMeansNoReport() throws Exception + { + final TempLogFile logFile = mock(TempLogFile.class); + when(logFile.isEnabled()).thenReturn(false); + + assertTrue(reportOf(logFile).isEmpty(), "a launcher without a log has nothing to report"); + } + + private TempLogFile newLogFile() + { + final TempLogFile logFile = TempLogFile.newTempLogFile(PREFIX, new File(tempDir, "logs")); + created.add(logFile); + return logFile; + } + + /** What the listeners of a failed installation are told about the log file. */ + private static String reportOf(final TempLogFile logFile) + { + final Installer installer = new Installer(); + installer.setProgressMessageFormatter(new PlainTextProgressMessageFormatter()); + final StringBuilder report = new StringBuilder(); + installer.addProgressUpdateListener(new ProgressUpdateListener() + { + @Override + public void progressUpdate(final ProgressUpdateEvent ev) + { + if (ev.getNewLogs() != null) + { + report.append(ev.getNewLogs()); + } + } + }); + installer.setTempLogFile(logFile); + installer.notifyListenersOfExistingLogFile(); + return report.toString(); + } + + private static void assertContains(final String report, final LocalizableMessage expected) + { + assertTrue(report.contains(expected.toString()), "expected <" + expected + "> in <" + report + ">"); + } + + private static void assertDoesNotContain(final String report, final LocalizableMessage unexpected) + { + assertFalse(report.contains(unexpected.toString()), "unexpected <" + unexpected + "> in <" + report + ">"); + } +} From 03a03d3e1935bdb54673ed5085535376535f2bd3 Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Wed, 23 Sep 2026 09:06:58 +0300 Subject: [PATCH 3/4] [#1030] Create the setup log where the install begins, not where the wizard opens Round 3 of the review. The lazy log of round 2 closed the CLI road only. On the GUI road getTempLogFile() was evaluated as the argument of SplashScreen.main, so the log - and the instance logs/ directory - was created before the wizard existed, and every exit that installs nothing (a quit at any step, "Server Already Configured", the failed java version check, the headless fallback) kept it for good. Launcher now hands the splash screen a Supplier, which SplashScreen, QuickSetup.initialize and Application pass on untouched, and Installer.run() resolves it as its first statement - the first point where an install can fail. InstallDS hands the installer the same supplier instead of the file. The reason a GUI launch failed is kept in the launcher and written to the log as soon as something asks for one, so nothing is created to hold it, and Launcher.launch()'s fallback from the GUI to the command line asks for the log itself, since the operation runs there. Also from the review: InstallDSTest pins that a run which installs nothing never asks for a log; InstallerTest pins the cancel road, marks the arm each line of the report goes out on and pins the contents of the log through a mock, so OPENDJ_LOG_TO_STDOUT cannot empty it; TempLogFileTest pins the deferred fallback warning and that the log's publisher comes off the error logger; the tmp/ sweep comment hands the logs of the other tools to the operator. --- opendj-server-legacy/resource/bin/start-ds | 4 +- .../resource/bin/start-ds.bat | 5 +- .../org/opends/quicksetup/Application.java | 46 +++++++- .../java/org/opends/quicksetup/Launcher.java | 75 +++++++++--- .../org/opends/quicksetup/SplashScreen.java | 14 ++- .../quicksetup/installer/Installer.java | 5 + .../org/opends/quicksetup/ui/QuickSetup.java | 7 +- .../org/opends/server/tools/InstallDS.java | 8 +- .../org/opends/quicksetup/LauncherTest.java | 47 +++++++- .../opends/quicksetup/TempLogFileTest.java | 50 ++++++++ .../quicksetup/installer/InstallerTest.java | 109 ++++++++++++++++-- .../opends/server/tools/InstallDSTest.java | 89 ++++++++++++++ 12 files changed, 415 insertions(+), 44 deletions(-) create mode 100644 opendj-server-legacy/src/test/java/org/opends/server/tools/InstallDSTest.java diff --git a/opendj-server-legacy/resource/bin/start-ds b/opendj-server-legacy/resource/bin/start-ds index 91805e8376..8e3d0325bd 100644 --- a/opendj-server-legacy/resource/bin/start-ds +++ b/opendj-server-legacy/resource/bin/start-ds @@ -77,7 +77,9 @@ STARTING_FILE=${INSTANCE_ROOT}/logs/server.starting # on every start, so that they do not pile up. Nothing else in there is ours to # remove: the tmp directory is java.io.tmpdir for every tool, and a tool may # still be running - setup starts the server through this script and keeps its -# own log open (issue #1030). +# own log open (issue #1030). The logs a failed dsreplication or status run +# leaves there (opendj-replication-*.log, opendj-status-*.log) are the +# operator's to read and to remove; starting the server no longer takes them. OPENDJ_TMP_DIR="${INSTANCE_ROOT}/tmp" if [ -d "${OPENDJ_TMP_DIR}" ]; then rm -rf "${OPENDJ_TMP_DIR}"/bc-fips-jni_* diff --git a/opendj-server-legacy/resource/bin/start-ds.bat b/opendj-server-legacy/resource/bin/start-ds.bat index 46c5f31f95..905e7e884e 100644 --- a/opendj-server-legacy/resource/bin/start-ds.bat +++ b/opendj-server-legacy/resource/bin/start-ds.bat @@ -62,7 +62,10 @@ rem Remove the native libraries the BC FIPS loader extracts into the tmp rem directory on every start, so that they do not pile up. Nothing else in rem there is ours to remove: the tmp directory is java.io.tmpdir for every rem tool, and a tool may still be running - setup starts the server through -rem this script and keeps its own log open (issue #1030). +rem this script and keeps its own log open (issue #1030). The logs a failed +rem dsreplication or status run leaves there (opendj-replication-*.log, +rem opendj-status-*.log) are the operator's to read and to remove; starting +rem the server no longer takes them. set OPENDJ_TMP_DIR=%INSTANCE_ROOT%\tmp rem %%~i drops the quotes cmd would hand back if it ever kept them: an install path with a rem space would otherwise split the rmdir argument, and the redirect would hide it. diff --git a/opendj-server-legacy/src/main/java/org/opends/quicksetup/Application.java b/opendj-server-legacy/src/main/java/org/opends/quicksetup/Application.java index 0a6faf14a7..5ba329a51e 100644 --- a/opendj-server-legacy/src/main/java/org/opends/quicksetup/Application.java +++ b/opendj-server-legacy/src/main/java/org/opends/quicksetup/Application.java @@ -26,6 +26,7 @@ import java.lang.reflect.InvocationTargetException; import java.util.Map; import java.util.Set; +import java.util.function.Supplier; import org.forgerock.i18n.LocalizableMessage; import org.forgerock.i18n.LocalizableMessageBuilder; @@ -76,9 +77,17 @@ public abstract class Application implements ProgressNotifier, Runnable { private final ErrorPrintStream err = new ErrorPrintStream(); private final OutputPrintStream out = new OutputPrintStream(); - /** Temporary log file where messages will be logged. */ + /** + * Temporary log file where messages will be logged, once {@link #openTempLogFile()} has + * asked for it. It stays {@code null} until then: creating it costs a file - and the + * directory it lives in - that nothing removes unless the operation succeeds, so the roads + * which attempt nothing must not create one (issue #1030). + */ protected TempLogFile tempLogFile; + /** Supplies the temporary log file, see {@link #openTempLogFile()}. */ + private Supplier tempLogFileSupplier; + /** * Creates an application by instantiating the Application class * denoted by the System property @@ -693,6 +702,41 @@ protected void applicationPrintStreamReceived(String message) public void setTempLogFile(final TempLogFile tempLogFile) { this.tempLogFile = tempLogFile; + this.tempLogFileSupplier = () -> tempLogFile; + } + + /** + * Sets where the temporary log file of this application comes from, without creating it. + *

+ * The wizard and the CLI hand the supplier over as they start: the file itself is created by + * {@link #openTempLogFile()}, on the road that runs the operation, so that a run which does + * nothing - a quit at any step of the wizard, a server which turns out to be installed + * already - leaves no log behind (issue #1030). + * + * @param tempLogFile + * supplies the temporary log file where messages will be logged. + */ + public void setTempLogFile(final Supplier tempLogFile) + { + this.tempLogFile = null; + this.tempLogFileSupplier = tempLogFile; + } + + /** + * Creates the temporary log file of this application, unless it has one already. + *

+ * Called where the operation begins and not before: from that point on the log is worth + * keeping, because there is something which can fail and be reported. + * + * @return the temporary log file of this application. + */ + protected TempLogFile openTempLogFile() + { + if (tempLogFile == null) + { + tempLogFile = tempLogFileSupplier.get(); + } + return tempLogFile; } /** diff --git a/opendj-server-legacy/src/main/java/org/opends/quicksetup/Launcher.java b/opendj-server-legacy/src/main/java/org/opends/quicksetup/Launcher.java index 53171a6ea9..41e132332e 100644 --- a/opendj-server-legacy/src/main/java/org/opends/quicksetup/Launcher.java +++ b/opendj-server-legacy/src/main/java/org/opends/quicksetup/Launcher.java @@ -24,6 +24,7 @@ import java.io.File; import java.io.PrintStream; +import java.util.function.Supplier; import org.forgerock.i18n.LocalizableMessage; import org.forgerock.i18n.slf4j.LocalizedLogger; @@ -50,6 +51,8 @@ public abstract class Launcher { * the roads after it read it from the main thread. */ private volatile TempLogFile tempLogFile; + /** Why the GUI did not come up, kept until there is a log to write it to. */ + private volatile Throwable guiLaunchFailure; /** * Creates a Launcher. @@ -97,6 +100,7 @@ public Launcher(final String[] args, final String tempLogFilePrefix, final File protected synchronized TempLogFile getTempLogFile() { if (tempLogFile == null) { tempLogFile = TempLogFile.newTempLogFile(tempLogFilePrefix, tempLogFileDirectory); + logGuiLaunchFailure(); } return tempLogFile; } @@ -111,6 +115,29 @@ protected boolean hasTempLogFile() { return tempLogFile != null && tempLogFile.isEnabled(); } + /** Writes the reason the GUI did not come up, now that there is a log to hold it. */ + private void logGuiLaunchFailure() { + Throwable failure = guiLaunchFailure; + if (failure == null) { + return; + } + logger.warn(LocalizableMessage.raw("Error launching GUI: " + failure)); + StringBuilder buf = new StringBuilder(); + while (failure != null) + { + for (StackTraceElement aStack : failure.getStackTrace()) { + buf.append(aStack).append("\n"); + } + + failure = failure.getCause(); + if (failure != null) + { + buf.append("Root cause:\n"); + } + } + logger.warn(LocalizableMessage.raw(buf)); + } + /** * Gets the arguments with which this launcher was invoked. * @return String[] args from the CLI invocation @@ -246,29 +273,16 @@ public void run() { try { - SplashScreen.main(getTempLogFile(), args); + startSplashScreen(Launcher.this::getTempLogFile, args); returnValue[0] = 0; } catch (Throwable t) { - if (hasTempLogFile()) - { - logger.warn(LocalizableMessage.raw("Error launching GUI: "+t)); - StringBuilder buf = new StringBuilder(); - while (t != null) - { - for (StackTraceElement aStack : t.getStackTrace()) { - buf.append(aStack).append("\n"); - } - - t = t.getCause(); - if (t != null) - { - buf.append("Root cause:\n"); - } - } - logger.warn(LocalizableMessage.raw(buf)); - } + // Kept rather than logged: a GUI which does not come up is not by itself an + // operation that failed, and creating a log here would leave one behind on every + // headless road that installs nothing (issue #1030). It goes into the log as soon + // as something asks for one. + guiLaunchFailure = t; } } }); @@ -291,6 +305,26 @@ public void run() return returnValue[0]; } + /** + * Shows the splash screen and, behind it, builds the wizard. + *

+ * The log file is handed over as a supplier and not as a file: the splash screen comes up + * before the user has said anything, and a log created there outlives every road that + * installs nothing - a quit at any wizard step, a server which is configured already + * (issue #1030). The application asks for it when it starts the operation. + *

+ * Package-private so that a test can drive {@link #launchGui(String[])} without a display. + * + * @param tempLogFile + * supplies the temporary log file of the application + * @param args + * the arguments to pass to the splash screen + */ + void startSplashScreen(final Supplier tempLogFile, final String[] args) + { + SplashScreen.main(tempLogFile, args); + } + /** * Gets the frame title of the GUI application that will be used * in some operating systems. @@ -401,6 +435,9 @@ else if (shouldPrintUsage()) { int exitCode = launchGui(args); if (exitCode != 0) { guiLaunchFailed(); + // The GUI did not come up and the operation runs on the command line after all: from + // here on there is something worth logging, the reason the GUI failed included. + getTempLogFile(); CliApplication cliApp = createCliApplication(); exitCode = launchCli(cliApp); preExit(cliApp); diff --git a/opendj-server-legacy/src/main/java/org/opends/quicksetup/SplashScreen.java b/opendj-server-legacy/src/main/java/org/opends/quicksetup/SplashScreen.java index e16a3bbe72..b2c0d015f2 100644 --- a/opendj-server-legacy/src/main/java/org/opends/quicksetup/SplashScreen.java +++ b/opendj-server-legacy/src/main/java/org/opends/quicksetup/SplashScreen.java @@ -26,6 +26,7 @@ import java.awt.MediaTracker; import java.awt.Toolkit; import java.awt.Window; +import java.util.function.Supplier; import javax.swing.SwingUtilities; @@ -51,7 +52,12 @@ public class SplashScreen extends Window private Object quickSetup; private Class quickSetupClass; - private TempLogFile tempLogFile; + /** + * Supplies the temporary log file of the application behind this splash screen. Nothing + * here asks for it: the splash screen shows before anything is attempted, and a log created + * at that point outlives every road that attempts nothing (issue #1030). + */ + private Supplier tempLogFile; /** Constant for the display of the splash screen. */ private static final int MIN_SPLASH_DISPLAY = 3000; @@ -61,11 +67,11 @@ public class SplashScreen extends Window * It can be called from the event thread and outside the event thread. * * @param tempLogFile - * temporary log file where messages will be logged + * supplies the temporary log file where messages will be logged * @param args * arguments to be passed to the method QuickSetup.initialize */ - public static void main(final TempLogFile tempLogFile, String[] args) + public static void main(final Supplier tempLogFile, String[] args) { SplashScreen screen = new SplashScreen(); screen.tempLogFile = tempLogFile; @@ -196,7 +202,7 @@ protected void constructApplication(String[] args) { quickSetupClass = Class.forName("org.opends.quicksetup.ui.QuickSetup"); quickSetup = quickSetupClass.getDeclaredConstructor().newInstance(); - quickSetupClass.getMethod("initialize", new Class[] { TempLogFile.class, String[].class }) + quickSetupClass.getMethod("initialize", new Class[] { Supplier.class, String[].class }) .invoke(quickSetup, tempLogFile, args); } catch (Exception e) { diff --git a/opendj-server-legacy/src/main/java/org/opends/quicksetup/installer/Installer.java b/opendj-server-legacy/src/main/java/org/opends/quicksetup/installer/Installer.java index 17b8266156..a51294136b 100644 --- a/opendj-server-legacy/src/main/java/org/opends/quicksetup/installer/Installer.java +++ b/opendj-server-legacy/src/main/java/org/opends/quicksetup/installer/Installer.java @@ -241,6 +241,11 @@ public class Installer extends GuiApplication @Override public void run() { + // The install begins here: this is the first point where something can fail and be worth + // a report, so this is where the log file is created. Every road which stops before this + // one - a quit at any step of the wizard, "already configured", a refused licence, a + // cancel at the prompt - leaves neither the log nor its directory behind (issue #1030). + openTempLogFile(); applicationException = null; PrintStream origErr = System.err; PrintStream origOut = System.out; diff --git a/opendj-server-legacy/src/main/java/org/opends/quicksetup/ui/QuickSetup.java b/opendj-server-legacy/src/main/java/org/opends/quicksetup/ui/QuickSetup.java index 5f122f50a6..0dc21c98f0 100644 --- a/opendj-server-legacy/src/main/java/org/opends/quicksetup/ui/QuickSetup.java +++ b/opendj-server-legacy/src/main/java/org/opends/quicksetup/ui/QuickSetup.java @@ -27,6 +27,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.function.Supplier; import java.util.logging.Handler; import javax.swing.SwingUtilities; @@ -91,12 +92,14 @@ public class QuickSetup implements ButtonActionListener, ProgressUpdateListener * blocked. * * @param tempLogFile - * temporary log file where messages will be logged. + * supplies the temporary log file where messages will be logged. The wizard does + * not create it: the application does, when it starts the operation, so that a + * wizard the user quits leaves no log behind (issue #1030). * @param args * for the moment this parameter is not used but we keep it in order * to (in case of need) pass parameters through the command line. */ - public void initialize(final TempLogFile tempLogFile, String[] args) + public void initialize(final Supplier tempLogFile, String[] args) { ProgressMessageFormatter formatter = new HtmlProgressMessageFormatter(); diff --git a/opendj-server-legacy/src/main/java/org/opends/server/tools/InstallDS.java b/opendj-server-legacy/src/main/java/org/opends/server/tools/InstallDS.java index a0a2fdc546..49931c0bb9 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/tools/InstallDS.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/tools/InstallDS.java @@ -378,9 +378,11 @@ private int execute(String[] args) System.setProperty(Constants.CLI_JAVA_PROPERTY, "true"); final Installer installer = new Installer(); - // The first point where an install can fail: from here on there is a log to keep, and - // this is where it gets created. - installer.setTempLogFile(tempLogFile.get()); + // The supplier is handed over, not the file: the installer creates it when it starts to + // run, which is the first point where something can fail. The roads above - a usage + // error, "already installed", a refused licence, a cancel at the prompt - return without + // asking for it and leave no log behind (issue #1030). + installer.setTempLogFile(tempLogFile); installer.setUserData(uData); installer.setProgressMessageFormatter(formatter); installer.addProgressUpdateListener( diff --git a/opendj-server-legacy/src/test/java/org/opends/quicksetup/LauncherTest.java b/opendj-server-legacy/src/test/java/org/opends/quicksetup/LauncherTest.java index e429c84503..da185ad06e 100644 --- a/opendj-server-legacy/src/test/java/org/opends/quicksetup/LauncherTest.java +++ b/opendj-server-legacy/src/test/java/org/opends/quicksetup/LauncherTest.java @@ -22,6 +22,9 @@ import java.io.File; import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import java.util.function.Supplier; import org.forgerock.i18n.LocalizableMessage; import org.opends.server.DirectoryServerTestCase; @@ -46,7 +49,7 @@ public class LauncherTest extends DirectoryServerTestCase private static final String PREFIX = "opendj-setup-"; private File tempDir; - private TempLogFile created; + private final List created = new ArrayList<>(); @BeforeClass public void setUp() throws IOException @@ -57,9 +60,9 @@ public void setUp() throws IOException @AfterClass public void tearDown() throws IOException { - if (created != null) + for (TempLogFile logFile : created) { - created.deleteLogFileAfterSuccess(); + logFile.deleteLogFileAfterSuccess(); } TestCaseUtils.deleteDirectory(tempDir); } @@ -84,7 +87,7 @@ public void testAskingForTheLogCreatesItOnce() throws Exception final TestLauncher launcher = new TestLauncher(logs); final TempLogFile logFile = launcher.getTempLogFile(); - created = logFile; + created.add(logFile); assertTrue(logFile.isReadable(), logFile.getPath()); assertEquals(logFile.getLogFile().getCanonicalFile().getParentFile(), logs.getCanonicalFile()); @@ -92,14 +95,50 @@ public void testAskingForTheLogCreatesItOnce() throws Exception assertSame(launcher.getTempLogFile(), logFile, "a second ask must not create a second log"); } + /** + * The wizard road: the splash screen comes up before the user has said anything, so the log + * cannot be created on the way to it - it is the application that asks for one, when it + * starts the install. A wizard quit at any step leaves nothing behind. + */ + @Test + public void testLaunchingTheGuiCreatesNoLog() throws Exception + { + final File instance = new File(tempDir, "quit-at-the-first-step"); + final File logs = new File(instance, "logs"); + final TestLauncher launcher = new TestLauncher(logs); + + // The wizard behind the splash screen quits without installing anything. + launcher.launchGui(new String[0]); + + assertFalse(launcher.hasTempLogFile(), "the splash screen must not cost a log"); + assertFalse(logs.exists(), logs.getPath()); + assertFalse(instance.exists(), instance.getPath()); + + // What the wizard was handed is the launcher's own log, made on the first ask. + final TempLogFile logFile = launcher.splashLogFile.get(); + created.add(logFile); + assertTrue(logFile.isReadable(), logFile.getPath()); + assertSame(logFile, launcher.getTempLogFile()); + } + /** A launcher with nothing in it but the log file behaviour under test. */ private static final class TestLauncher extends Launcher { + /** What {@link Launcher#launchGui(String[])} handed the splash screen. */ + private Supplier splashLogFile; + TestLauncher(final File tempLogFileDirectory) { super(new String[0], PREFIX, tempLogFileDirectory); } + @Override + void startSplashScreen(final Supplier tempLogFile, final String[] args) + { + // No display here, and no install either: the wizard is quit at its first step. + splashLogFile = tempLogFile; + } + @Override public ArgumentParser getArgumentParser() { diff --git a/opendj-server-legacy/src/test/java/org/opends/quicksetup/TempLogFileTest.java b/opendj-server-legacy/src/test/java/org/opends/quicksetup/TempLogFileTest.java index c3add16ebc..8ff7ea5890 100644 --- a/opendj-server-legacy/src/test/java/org/opends/quicksetup/TempLogFileTest.java +++ b/opendj-server-legacy/src/test/java/org/opends/quicksetup/TempLogFileTest.java @@ -25,12 +25,16 @@ import java.io.File; import java.io.IOException; +import java.lang.reflect.Method; import java.nio.file.Files; import java.util.ArrayList; +import java.util.Collection; import java.util.List; import org.opends.server.DirectoryServerTestCase; import org.opends.server.TestCaseUtils; +import org.opends.server.loggers.ErrorLogger; +import org.testng.SkipException; import org.testng.annotations.AfterClass; import org.testng.annotations.BeforeClass; import org.testng.annotations.Test; @@ -166,5 +170,51 @@ public void testUnusableDirectoryFallsBackToTheTemporaryDirectory() throws Excep assertTrue(logFile.isReadable()); assertNotEquals(parentOf(logFile), notADirectory.getCanonicalFile()); assertEquals(parentOf(logFile), new File(System.getProperty("java.io.tmpdir")).getCanonicalFile()); + + // Why the log is not where it was asked for is warned about once there is a log to carry + // the warning: at the point the directory failed, no publisher was installed yet. + assumeTheLogGoesToTheFile(); + logFile.writer.shutdown(); + final String contents = logFile.readContents(); + assertTrue(contents.contains("falling back to the temporary directory"), contents); + assertTrue(contents.contains(notADirectory.toString()), contents); + } + + /** + * The publishers the constructor puts on the logger singletons come off again with the log: + * the singletons outlive the file, and what a leaked publisher is handed goes to a closed + * stream and is swallowed. + */ + @Test + public void testDeletingTheLogTakesItsPublisherOffTheLogger() throws Exception + { + final int before = errorLogPublishers(); + + final TempLogFile logFile = track(TempLogFile.newTempLogFile(PREFIX, new File(tempDir, "logs"))); + assertEquals(errorLogPublishers(), before + 1, "the log logs through a publisher of its own"); + + logFile.deleteLogFileAfterSuccess(); + + assertEquals(errorLogPublishers(), before, "the publisher must not outlive the log"); + } + + /** How many publishers the error logger holds; {@code getLogPublishers()} is protected. */ + private static int errorLogPublishers() throws Exception + { + final Method getLogPublishers = ErrorLogger.class.getDeclaredMethod("getLogPublishers"); + getLogPublishers.setAccessible(true); + return ((Collection) getLogPublishers.invoke(ErrorLogger.getInstance())).size(); + } + + /** + * {@code OPENDJ_LOG_TO_STDOUT} sends the records to stdout and leaves the file empty, so a + * case which reads the log back has nothing to look at. + */ + private static void assumeTheLogGoesToTheFile() + { + if ("true".equalsIgnoreCase(System.getenv("OPENDJ_LOG_TO_STDOUT"))) + { + throw new SkipException("OPENDJ_LOG_TO_STDOUT writes the log to stdout, not to the file"); + } } } diff --git a/opendj-server-legacy/src/test/java/org/opends/quicksetup/installer/InstallerTest.java b/opendj-server-legacy/src/test/java/org/opends/quicksetup/installer/InstallerTest.java index 6e9f462188..c0275a788b 100644 --- a/opendj-server-legacy/src/test/java/org/opends/quicksetup/installer/InstallerTest.java +++ b/opendj-server-legacy/src/test/java/org/opends/quicksetup/installer/InstallerTest.java @@ -18,6 +18,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import static org.opends.messages.QuickSetupMessages.*; +import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertFalse; import static org.testng.Assert.assertTrue; @@ -28,6 +29,7 @@ import org.forgerock.i18n.LocalizableMessage; import org.opends.quicksetup.TempLogFile; +import org.opends.quicksetup.UserData; import org.opends.quicksetup.event.ProgressUpdateEvent; import org.opends.quicksetup.event.ProgressUpdateListener; import org.opends.quicksetup.util.PlainTextProgressMessageFormatter; @@ -49,6 +51,10 @@ public class InstallerTest extends DirectoryServerTestCase { private static final String PREFIX = "opendj-setup-"; + /** Marks a line the report sends out as a warning, see {@link MarkedArmsFormatter}. */ + private static final String WARNING = "[W]"; + /** Marks a line the report sends out as progress. */ + private static final String PROGRESS = "[P]"; private File tempDir; private final List created = new ArrayList<>(); @@ -69,7 +75,7 @@ public void tearDown() throws IOException TestCaseUtils.deleteDirectory(tempDir); } - /** A log that is there is named and written out, as before. */ + /** A log that is there is named, as before, and named as progress rather than as a warning. */ @Test public void testAReadableLogIsHandedOver() throws Exception { @@ -77,12 +83,32 @@ public void testAReadableLogIsHandedOver() throws Exception final String report = reportOf(logFile); - assertContains(report, INFO_GENERAL_PROVIDE_LOG_IN_ERROR.get(logFile.getPath())); - // The line the constructor logs: the contents of the file reach the report. - assertTrue(report.contains("QuickSetup application launched"), report); + assertContains(report, PROGRESS, INFO_GENERAL_PROVIDE_LOG_IN_ERROR.get(logFile.getPath())); assertDoesNotContain(report, INFO_GENERAL_LOG_IN_ERROR_MISSING.get(logFile.getPath())); } + /** + * The contents of the log reach the report: attaching the file is up to whoever reads it, + * so the report carries the log itself. + *

+ * What the file holds is not the point here and is mocked away - with + * {@code OPENDJ_LOG_TO_STDOUT} set, a real log is empty and would pin nothing. + */ + @Test + public void testTheContentsOfTheLogReachTheReport() throws Exception + { + final String contents = "QuickSetup application launched\nthe last line before the failure\n"; + final TempLogFile logFile = mock(TempLogFile.class); + when(logFile.isEnabled()).thenReturn(true); + when(logFile.isReadable()).thenReturn(true); + when(logFile.getPath()).thenReturn(new File(tempDir, "readable.log").getAbsolutePath()); + when(logFile.readContents()).thenReturn(contents); + + final String report = reportOf(logFile); + + assertTrue(report.contains(contents), report); + } + /** A log something else removed is reported as gone, instead of being asked for. */ @Test public void testAMissingLogIsReportedAsMissing() throws Exception @@ -94,7 +120,7 @@ public void testAMissingLogIsReportedAsMissing() throws Exception final String report = reportOf(logFile); - assertContains(report, INFO_GENERAL_LOG_IN_ERROR_MISSING.get(path)); + assertContains(report, WARNING, INFO_GENERAL_LOG_IN_ERROR_MISSING.get(path)); // The line that asks for the file must not go out when there is no file to provide. assertDoesNotContain(report, INFO_GENERAL_PROVIDE_LOG_IN_ERROR.get(path)); } @@ -113,7 +139,7 @@ public void testAnUnreadableLogIsReportedAsUnreadable() throws Exception final String report = reportOf(logFile); - assertContains(report, INFO_GENERAL_LOG_IN_ERROR_UNREADABLE.get(path, failure)); + assertContains(report, WARNING, INFO_GENERAL_LOG_IN_ERROR_UNREADABLE.get(path, failure)); assertDoesNotContain(report, INFO_GENERAL_LOG_IN_ERROR_MISSING.get(path)); } @@ -127,6 +153,49 @@ public void testNoLogMeansNoReport() throws Exception assertTrue(reportOf(logFile).isEmpty(), "a launcher without a log has nothing to report"); } + /** + * A cancelled install takes its log with it. + *

+ * Nothing names the log on that road - the report belongs to the failure road - and the + * cancel has just taken the installation back, so a log kept there is a report nobody is + * ever pointed at (issue #1030). + */ + @Test + public void testACancelledInstallTakesItsLogWithIt() throws Exception + { + final TempLogFile logFile = newLogFile(); + final File instance = new File(tempDir, "cancelled"); + // A locks directory the lock file can be taken in: without it the cancel road reads the + // server as running and goes off to stop it. + assertTrue(new File(instance, "locks").mkdirs()); + + // An installation under the temporary directory rather than the one the class path names: + // the installer takes both paths from there, and in a test run there is none. + final Installer installer = new Installer() + { + @Override + public String getInstallationPath() + { + return instance.getAbsolutePath(); + } + + @Override + public String getInstancePath() + { + return instance.getAbsolutePath(); + } + }; + installer.setProgressMessageFormatter(new MarkedArmsFormatter()); + installer.setTempLogFile(logFile); + installer.setUserData(new UserData()); + installer.cancel(); + + installer.run(); + + assertEquals(installer.getCurrentProgressStep(), InstallProgressStep.FINISHED_CANCELED); + assertFalse(logFile.getLogFile().exists(), logFile.getPath()); + } + private TempLogFile newLogFile() { final TempLogFile logFile = TempLogFile.newTempLogFile(PREFIX, new File(tempDir, "logs")); @@ -138,7 +207,7 @@ private TempLogFile newLogFile() private static String reportOf(final TempLogFile logFile) { final Installer installer = new Installer(); - installer.setProgressMessageFormatter(new PlainTextProgressMessageFormatter()); + installer.setProgressMessageFormatter(new MarkedArmsFormatter()); final StringBuilder report = new StringBuilder(); installer.addProgressUpdateListener(new ProgressUpdateListener() { @@ -156,13 +225,35 @@ public void progressUpdate(final ProgressUpdateEvent ev) return report.toString(); } - private static void assertContains(final String report, final LocalizableMessage expected) + private static void assertContains(final String report, final String arm, final LocalizableMessage expected) { - assertTrue(report.contains(expected.toString()), "expected <" + expected + "> in <" + report + ">"); + assertTrue(report.contains(arm + expected), "expected <" + arm + expected + "> in <" + report + ">"); } private static void assertDoesNotContain(final String report, final LocalizableMessage unexpected) { assertFalse(report.contains(unexpected.toString()), "unexpected <" + unexpected + "> in <" + report + ">"); } + + /** + * A formatter that marks the arm each line goes out on. + *

+ * The plain text formatter returns warnings and progress messages unchanged - only the + * wizard's HTML formatter tells them apart - so without a mark a report which says "the log + * is gone" as an ordinary progress line reads exactly like one which warns about it. + */ + private static final class MarkedArmsFormatter extends PlainTextProgressMessageFormatter + { + @Override + public LocalizableMessage getFormattedWarning(final LocalizableMessage text, final boolean applyMargin) + { + return LocalizableMessage.raw(WARNING + super.getFormattedWarning(text, applyMargin)); + } + + @Override + public LocalizableMessage getFormattedProgress(final LocalizableMessage text) + { + return LocalizableMessage.raw(PROGRESS + super.getFormattedProgress(text)); + } + } } diff --git a/opendj-server-legacy/src/test/java/org/opends/server/tools/InstallDSTest.java b/opendj-server-legacy/src/test/java/org/opends/server/tools/InstallDSTest.java new file mode 100644 index 0000000000..4f6db14f47 --- /dev/null +++ b/opendj-server-legacy/src/test/java/org/opends/server/tools/InstallDSTest.java @@ -0,0 +1,89 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2026 3A Systems, LLC. + */ +package org.opends.server.tools; + +import static org.testng.Assert.assertEquals; + +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Supplier; + +import org.opends.quicksetup.Constants; +import org.opends.quicksetup.TempLogFile; +import org.opends.server.DirectoryServerTestCase; +import org.testng.annotations.Test; + +/** + * Tests when the CLI setup asks for its log file. + *

+ * Nothing removes that file unless the install succeeds, so the roads which install nothing - + * a usage request, a usage error, a server which is configured already, a refused licence, a + * cancel at the prompt - must not ask for one: asking creates it, and the instance + * {@code logs/} directory with it (issue #1030). + */ +@SuppressWarnings("javadoc") +@Test(groups = { "precommit", "tools" }, sequential = true) +public class InstallDSTest extends DirectoryServerTestCase +{ + /** {@code InstallReturnCode.SUCCESSFUL_NOP}: the usage was displayed and nothing was done. */ + private static final int SUCCESSFUL_NOP = 0; + /** {@code InstallReturnCode.ERROR_USER_DATA}: the arguments could not be parsed. */ + private static final int ERROR_USER_DATA = 2; + + /** {@code setup --help} displays the usage and returns before anything can fail. */ + @Test + public void testAUsageRequestAsksForNoLog() throws Exception + { + assertAsksForNoLog(SUCCESSFUL_NOP, "--help"); + } + + /** An argument the parser does not know is reported, and no install is attempted. */ + @Test + public void testAUsageErrorAsksForNoLog() throws Exception + { + assertAsksForNoLog(ERROR_USER_DATA, "--no-such-option"); + } + + /** + * Runs the CLI setup with a supplier which counts the asks and hands out nothing, so that a + * road which asks for the log is red here rather than a file left behind on a real run. + */ + private static void assertAsksForNoLog(final int expectedReturnCode, final String... args) + { + final AtomicInteger asked = new AtomicInteger(); + final Supplier countingSupplier = () -> { + asked.incrementAndGet(); + return null; + }; + // mainCLI() sets this property for the run; put back what the rest of the JVM had. + final String cliProperty = System.getProperty(Constants.CLI_JAVA_PROPERTY); + try + { + assertEquals(InstallDS.mainCLI(args, null, null, countingSupplier), expectedReturnCode); + assertEquals(asked.get(), 0, "a run which installs nothing must not create a log"); + } + finally + { + if (cliProperty != null) + { + System.setProperty(Constants.CLI_JAVA_PROPERTY, cliProperty); + } + else + { + System.clearProperty(Constants.CLI_JAVA_PROPERTY); + } + } + } +} From 4845f2fbe6b15c8469af91898dceab849c9d91fe Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Wed, 23 Sep 2026 12:31:16 +0300 Subject: [PATCH 4/4] [#1030] Give the uninstall its log back, and name the log only where there is one The uninstall wizard was left with no log at all: only Installer.run() resolved the supplier, and the uninstaller's errors reach the file only through the publisher the log puts on the error logger. Uninstaller.run() now resolves it too, and Application.openTempLogFile() tolerates the command line uninstaller, which is handed no supplier. Application no longer takes the file itself, only the supplier, so no caller on the way can resolve it early again. guiLaunchFailed() could never name a log: none existed yet when it ran. The uninstall road now asks for the log first; the setup road cannot without bringing the headless leftover back, so its DETAILS arm and INFO_SETUP_LAUNCHER_GUI_LAUNCHED_FAILED_DETAILS are removed. Tests pin the supplier road of the installer, the null guard, the GUI failure written once a log exists, and the "already installed" road of the CLI setup. --- .../guitools/uninstaller/Uninstaller.java | 5 ++ .../org/opends/quicksetup/Application.java | 23 +++--- .../java/org/opends/quicksetup/Launcher.java | 5 +- .../quicksetup/installer/SetupLauncher.java | 7 +- .../org/opends/messages/quickSetup.properties | 3 - .../opends/messages/quickSetup_de.properties | 2 +- .../opends/messages/quickSetup_es.properties | 2 +- .../opends/messages/quickSetup_fr.properties | 2 +- .../opends/messages/quickSetup_ja.properties | 2 +- .../opends/messages/quickSetup_ko.properties | 2 +- .../opends/messages/quickSetup_pl.properties | 2 +- .../messages/quickSetup_zh_CN.properties | 2 +- .../messages/quickSetup_zh_TW.properties | 2 +- .../org/opends/quicksetup/LauncherTest.java | 39 ++++++++- .../quicksetup/installer/InstallerTest.java | 81 ++++++++++++++----- .../opends/server/tools/InstallDSTest.java | 42 ++++++++++ 16 files changed, 169 insertions(+), 52 deletions(-) diff --git a/opendj-server-legacy/src/main/java/org/opends/guitools/uninstaller/Uninstaller.java b/opendj-server-legacy/src/main/java/org/opends/guitools/uninstaller/Uninstaller.java index 808f87be21..f5265b8cb1 100644 --- a/opendj-server-legacy/src/main/java/org/opends/guitools/uninstaller/Uninstaller.java +++ b/opendj-server-legacy/src/main/java/org/opends/guitools/uninstaller/Uninstaller.java @@ -696,6 +696,11 @@ private void putSummary(UninstallProgressStep status, Arg0 msg) /** Actually performs the uninstall in this thread. The thread is blocked. */ @Override public void run() { + // The uninstall begins here, as the install begins in Installer.run(): the errors below go + // to the error logger, and only the log file puts a publisher on it. Without it a failed + // uninstall in the wizard leaves no record anywhere (issue #1030). The command line + // uninstaller is given no supplier - its launcher has created the log already. + openTempLogFile(); status = STARTED; logger.info(LocalizableMessage.raw("run of the Uninstaller started")); diff --git a/opendj-server-legacy/src/main/java/org/opends/quicksetup/Application.java b/opendj-server-legacy/src/main/java/org/opends/quicksetup/Application.java index 5ba329a51e..4d7745a2d5 100644 --- a/opendj-server-legacy/src/main/java/org/opends/quicksetup/Application.java +++ b/opendj-server-legacy/src/main/java/org/opends/quicksetup/Application.java @@ -693,18 +693,6 @@ protected void applicationPrintStreamReceived(String message) // no-op } - /** - * Sets the temporary log file where messages will be logged. - * - * @param tempLogFile - * temporary log file where messages will be logged. - */ - public void setTempLogFile(final TempLogFile tempLogFile) - { - this.tempLogFile = tempLogFile; - this.tempLogFileSupplier = () -> tempLogFile; - } - /** * Sets where the temporary log file of this application comes from, without creating it. *

@@ -712,6 +700,10 @@ public void setTempLogFile(final TempLogFile tempLogFile) * {@link #openTempLogFile()}, on the road that runs the operation, so that a run which does * nothing - a quit at any step of the wizard, a server which turns out to be installed * already - leaves no log behind (issue #1030). + *

+ * There is deliberately no setter taking the file itself: a caller holding a supplier could + * then resolve it on the spot, and the log would be created before anything is attempted + * again. * * @param tempLogFile * supplies the temporary log file where messages will be logged. @@ -727,12 +719,15 @@ public void setTempLogFile(final Supplier tempLogFile) *

* Called where the operation begins and not before: from that point on the log is worth * keeping, because there is something which can fail and be reported. + *

+ * An application nobody handed a supplier to has no log of its own: the command line + * uninstaller is built that way, and its launcher creates the log before running it. * - * @return the temporary log file of this application. + * @return the temporary log file of this application, or {@code null} if it was given none. */ protected TempLogFile openTempLogFile() { - if (tempLogFile == null) + if (tempLogFile == null && tempLogFileSupplier != null) { tempLogFile = tempLogFileSupplier.get(); } diff --git a/opendj-server-legacy/src/main/java/org/opends/quicksetup/Launcher.java b/opendj-server-legacy/src/main/java/org/opends/quicksetup/Launcher.java index 41e132332e..c0622eef6c 100644 --- a/opendj-server-legacy/src/main/java/org/opends/quicksetup/Launcher.java +++ b/opendj-server-legacy/src/main/java/org/opends/quicksetup/Launcher.java @@ -434,10 +434,11 @@ else if (shouldPrintUsage()) { willLaunchGui(); int exitCode = launchGui(args); if (exitCode != 0) { - guiLaunchFailed(); // The GUI did not come up and the operation runs on the command line after all: from - // here on there is something worth logging, the reason the GUI failed included. + // here on there is something worth logging, the reason the GUI failed included. The + // log comes first, so that guiLaunchFailed() can name the file holding that reason. getTempLogFile(); + guiLaunchFailed(); CliApplication cliApp = createCliApplication(); exitCode = launchCli(cliApp); preExit(cliApp); diff --git a/opendj-server-legacy/src/main/java/org/opends/quicksetup/installer/SetupLauncher.java b/opendj-server-legacy/src/main/java/org/opends/quicksetup/installer/SetupLauncher.java index 99c0a981c6..32f91fe6de 100644 --- a/opendj-server-legacy/src/main/java/org/opends/quicksetup/installer/SetupLauncher.java +++ b/opendj-server-legacy/src/main/java/org/opends/quicksetup/installer/SetupLauncher.java @@ -173,9 +173,10 @@ public ArgumentParser getArgumentParser() { @Override protected void guiLaunchFailed() { - System.err.println( - hasTempLogFile() ? INFO_SETUP_LAUNCHER_GUI_LAUNCHED_FAILED_DETAILS.get(getTempLogFile().getPath()) - : INFO_SETUP_LAUNCHER_GUI_LAUNCHED_FAILED.get()); + // No log is named here: none exists yet, and creating one to name would leave it behind on + // every command line road that installs nothing (issue #1030). The reason the GUI failed + // goes into the log of the install that follows, and a failed install names that log. + System.err.println(INFO_SETUP_LAUNCHER_GUI_LAUNCHED_FAILED.get()); } @Override diff --git a/opendj-server-legacy/src/messages/org/opends/messages/quickSetup.properties b/opendj-server-legacy/src/messages/org/opends/messages/quickSetup.properties index f5d02c8bbc..ccc934d958 100644 --- a/opendj-server-legacy/src/messages/org/opends/messages/quickSetup.properties +++ b/opendj-server-legacy/src/messages/org/opends/messages/quickSetup.properties @@ -735,9 +735,6 @@ INFO_SERVER_SETTINGS_PANEL_TITLE=Server Settings INFO_SERVER_SETTINGS_STEP=Server Settings INFO_SETUP_LAUNCHER_GUI_LAUNCHED_FAILED=%n%nThe graphical Setup launch \ failed.%n%nLaunching command line setup... -INFO_SETUP_LAUNCHER_GUI_LAUNCHED_FAILED_DETAILS=%n%nThe graphical Setup \ - launch failed. Check file %s for more details.%n%nLaunching command line \ - setup... INFO_SETUP_LAUNCHER_LAUNCHING_GUI=Launching graphical setup... INFO_SHOW_EXCEPTION_DETAILS=Show Details # Only translate if the image is specific to the local diff --git a/opendj-server-legacy/src/messages/org/opends/messages/quickSetup_de.properties b/opendj-server-legacy/src/messages/org/opends/messages/quickSetup_de.properties index 1e095a237f..7817c927f5 100644 --- a/opendj-server-legacy/src/messages/org/opends/messages/quickSetup_de.properties +++ b/opendj-server-legacy/src/messages/org/opends/messages/quickSetup_de.properties @@ -12,6 +12,7 @@ # # Copyright 2006-2010 Sun Microsystems, Inc. # Portions Copyright 2010-2016 ForgeRock AS. +# Portions Copyright 2026 3A Systems, LLC. @@ -491,7 +492,6 @@ INFO_SERVER_SETTINGS_PANEL_INSTRUCTIONS=Geben Sie einen Port an, der auf LDAP-An INFO_SERVER_SETTINGS_PANEL_TITLE=Servereinstellungen INFO_SERVER_SETTINGS_STEP=Servereinstellungen INFO_SETUP_LAUNCHER_GUI_LAUNCHED_FAILED=%n%nSetup-Start \u00fcber grafische Benutzeroberfl\u00e4che ist fehlgeschlagen.%n%nSetup \u00fcber Befehlszeile wird gestartet... -INFO_SETUP_LAUNCHER_GUI_LAUNCHED_FAILED_DETAILS=%n%nSetup-Start \u00fcber grafische Benutzeroberfl\u00e4che ist fehlgeschlagen. Weitere Details finden Sie in der Datei %s.%n%nSetup \u00fcber Befehlszeile wird gestartet... INFO_SETUP_LAUNCHER_LAUNCHING_GUI=Setup \u00fcber grafische Benutzeroberfl\u00e4che wird gestartet... INFO_SHOW_EXCEPTION_DETAILS=Details anzeigen # Only translate if the image is specific to the local diff --git a/opendj-server-legacy/src/messages/org/opends/messages/quickSetup_es.properties b/opendj-server-legacy/src/messages/org/opends/messages/quickSetup_es.properties index c97b569911..221a263581 100644 --- a/opendj-server-legacy/src/messages/org/opends/messages/quickSetup_es.properties +++ b/opendj-server-legacy/src/messages/org/opends/messages/quickSetup_es.properties @@ -12,6 +12,7 @@ # # Copyright 2006-2010 Sun Microsystems, Inc. # Portions Copyright 2010-2016 ForgeRock AS. +# Portions Copyright 2026 3A Systems, LLC. @@ -491,7 +492,6 @@ INFO_SERVER_SETTINGS_PANEL_INSTRUCTIONS=Escriba un puerto de escucha para las so INFO_SERVER_SETTINGS_PANEL_TITLE=Configuraci\u00f3n del servidor INFO_SERVER_SETTINGS_STEP=Configuraci\u00f3n del servidor INFO_SETUP_LAUNCHER_GUI_LAUNCHED_FAILED=%n%nError al iniciar la instalaci\u00f3n gr\u00e1fica.%n%nIniciando la instalaci\u00f3n mediante l\u00ednea de comandos... -INFO_SETUP_LAUNCHER_GUI_LAUNCHED_FAILED_DETAILS=%n%nError al iniciar la instalaci\u00f3n gr\u00e1fica. Consulte el archivo %s para obtener m\u00e1s informaci\u00f3n.%n%nIniciando la instalaci\u00f3n mediante l\u00ednea de comandos... INFO_SETUP_LAUNCHER_LAUNCHING_GUI=Iniciando configuraci\u00f3n gr\u00e1fica... INFO_SHOW_EXCEPTION_DETAILS=Mostrar detalles # Only translate if the image is specific to the local diff --git a/opendj-server-legacy/src/messages/org/opends/messages/quickSetup_fr.properties b/opendj-server-legacy/src/messages/org/opends/messages/quickSetup_fr.properties index b7d4817a87..2c361dc3e9 100644 --- a/opendj-server-legacy/src/messages/org/opends/messages/quickSetup_fr.properties +++ b/opendj-server-legacy/src/messages/org/opends/messages/quickSetup_fr.properties @@ -12,6 +12,7 @@ # # Copyright 2006-2010 Sun Microsystems, Inc. # Portions Copyright 2010-2016 ForgeRock AS. +# Portions Copyright 2026 3A Systems, LLC. @@ -492,7 +493,6 @@ INFO_SERVER_SETTINGS_PANEL_INSTRUCTIONS=Saisir un port pour \u00e9couter les dem INFO_SERVER_SETTINGS_PANEL_TITLE=Param\u00e8tres du serveur INFO_SERVER_SETTINGS_STEP=Param\u00e8tres du serveur INFO_SETUP_LAUNCHER_GUI_LAUNCHED_FAILED=%n%n\u00c9chec du lancement de la configuration graphique.%n%nD\u00e9marrage de la configuration via la ligne de commande... -INFO_SETUP_LAUNCHER_GUI_LAUNCHED_FAILED_DETAILS=%n%n\u00c9chec du lancement de la configuration graphique. Reportez-vous au fichier %s pour plus d'informations.%n%nD\u00e9marrage de la configuration via la ligne de commande... INFO_SETUP_LAUNCHER_LAUNCHING_GUI=D\u00e9marrage de la configuration graphique... INFO_SHOW_EXCEPTION_DETAILS=Afficher les d\u00e9tails # Only translate if the image is specific to the local diff --git a/opendj-server-legacy/src/messages/org/opends/messages/quickSetup_ja.properties b/opendj-server-legacy/src/messages/org/opends/messages/quickSetup_ja.properties index 7c3edf37b4..221a882ff8 100644 --- a/opendj-server-legacy/src/messages/org/opends/messages/quickSetup_ja.properties +++ b/opendj-server-legacy/src/messages/org/opends/messages/quickSetup_ja.properties @@ -12,6 +12,7 @@ # # Copyright 2006-2010 Sun Microsystems, Inc. # Portions Copyright 2010-2016 ForgeRock AS. +# Portions Copyright 2026 3A Systems, LLC. @@ -491,7 +492,6 @@ INFO_SERVER_SETTINGS_PANEL_INSTRUCTIONS=LDAP \u8981\u6c42\u3092\u5f85\u6a5f\u305 INFO_SERVER_SETTINGS_PANEL_TITLE=\u30b5\u30fc\u30d0\u30fc\u8a2d\u5b9a INFO_SERVER_SETTINGS_STEP=\u30b5\u30fc\u30d0\u30fc\u8a2d\u5b9a INFO_SETUP_LAUNCHER_GUI_LAUNCHED_FAILED=%n%n\u30b0\u30e9\u30d5\u30a3\u30ab\u30eb\u30bb\u30c3\u30c8\u30a2\u30c3\u30d7\u306e\u8d77\u52d5\u306b\u5931\u6557\u3057\u307e\u3057\u305f\u3002%n%n\u30b3\u30de\u30f3\u30c9\u884c\u30bb\u30c3\u30c8\u30a2\u30c3\u30d7\u3092\u8d77\u52d5\u3057\u3066\u3044\u307e\u3059... -INFO_SETUP_LAUNCHER_GUI_LAUNCHED_FAILED_DETAILS=%n%n\u30b0\u30e9\u30d5\u30a3\u30ab\u30eb\u30bb\u30c3\u30c8\u30a2\u30c3\u30d7\u306e\u8d77\u52d5\u306b\u5931\u6557\u3057\u307e\u3057\u305f\u3002 \u8a73\u7d30\u306f\u30d5\u30a1\u30a4\u30eb %s \u3092\u78ba\u8a8d\u3057\u3066\u304f\u3060\u3055\u3044\u3002%n%n\u30b3\u30de\u30f3\u30c9\u884c\u30bb\u30c3\u30c8\u30a2\u30c3\u30d7\u3092\u8d77\u52d5\u3057\u3066\u3044\u307e\u3059... INFO_SETUP_LAUNCHER_LAUNCHING_GUI=\u30b0\u30e9\u30d5\u30a3\u30ab\u30eb\u30bb\u30c3\u30c8\u30a2\u30c3\u30d7\u3092\u8d77\u52d5\u3057\u3066\u3044\u307e\u3059... INFO_SHOW_EXCEPTION_DETAILS=\u8a73\u7d30\u3092\u8868\u793a # Only translate if the image is specific to the local diff --git a/opendj-server-legacy/src/messages/org/opends/messages/quickSetup_ko.properties b/opendj-server-legacy/src/messages/org/opends/messages/quickSetup_ko.properties index 809a5aedd9..b58c01eb6b 100644 --- a/opendj-server-legacy/src/messages/org/opends/messages/quickSetup_ko.properties +++ b/opendj-server-legacy/src/messages/org/opends/messages/quickSetup_ko.properties @@ -12,6 +12,7 @@ # # Copyright 2006-2009 Sun Microsystems, Inc. # Portions Copyright 2010-2016 ForgeRock AS. +# Portions Copyright 2026 3A Systems, LLC. @@ -470,7 +471,6 @@ INFO_SERVER_SETTINGS_PANEL_INSTRUCTIONS=Enter a port to listen for LDAP requests INFO_SERVER_SETTINGS_PANEL_TITLE=\uc11c\ubc84 \uc124\uc815 INFO_SERVER_SETTINGS_STEP=\uc11c\ubc84 \uc124\uc815 INFO_SETUP_LAUNCHER_GUI_LAUNCHED_FAILED=%n%n\uadf8\ub798\ud53d \uc124\uce58\ub97c \uc2dc\uc791\ud558\uc9c0 \ubabb\ud588\uc2b5\ub2c8\ub2e4.%n%n\uba85\ub839\uc904 \uc124\uce58\ub97c \uc2dc\uc791\ud558\ub294 \uc911... -INFO_SETUP_LAUNCHER_GUI_LAUNCHED_FAILED_DETAILS=%n%n\uadf8\ub798\ud53d \uc124\uce58\ub97c \uc2dc\uc791\ud558\uc9c0 \ubabb\ud588\uc2b5\ub2c8\ub2e4. \uc790\uc138\ud55c \ub0b4\uc6a9\uc740 %s \ud30c\uc77c\uc744 \ucc38\uc870\ud558\uc2ed\uc2dc\uc624.%n%n\uba85\ub839\uc904 \uc124\uce58\ub97c \uc2dc\uc791\ud558\ub294 \uc911... INFO_SETUP_LAUNCHER_LAUNCHING_GUI=\uadf8\ub798\ud53d \uc124\uce58\ub97c \uc2dc\uc791\ud558\ub294 \uc911... INFO_SHOW_EXCEPTION_DETAILS=\uc138\ubd80 \uc815\ubcf4 \ud45c\uc2dc # Only translate if the image is specific to the local diff --git a/opendj-server-legacy/src/messages/org/opends/messages/quickSetup_pl.properties b/opendj-server-legacy/src/messages/org/opends/messages/quickSetup_pl.properties index 5b2d8283de..d2690bc4e4 100644 --- a/opendj-server-legacy/src/messages/org/opends/messages/quickSetup_pl.properties +++ b/opendj-server-legacy/src/messages/org/opends/messages/quickSetup_pl.properties @@ -12,6 +12,7 @@ # # Copyright 2006-2009 Sun Microsystems, Inc. # Portions Copyright 2010-2016 ForgeRock AS. +# Portions Copyright 2026 3A Systems, LLC. @@ -63,7 +64,6 @@ INFO_CERTIFICATE_ISSUED_BY_LABEL=Wydany przez: INFO_CERTIFICATE_LABEL=Certyfikat: ERR_CANNOT_CONNECT_TO_LOCAL_GENERIC=Could not connect to the server. Check that the server is running and that the provided credentials are valid.%nSzczeg\u00f3\u0142y b\u0142\u0119du:%n%s INFO_CERTIFICATE_DIALOG_ACCEPT_FOR_SESSION_BUTTON_TOOLTIP=Zamknij to okienko i zaakceptuj certyfikat tylko dla tej sesji. -INFO_SETUP_LAUNCHER_GUI_LAUNCHED_FAILED_DETAILS=%n%nThe graphical Setup launch failed. Sprawd\u017a plik %s aby uzyska\u0107 szczeg\u00f3\u0142owe informacje.%n%nLaunching command line setup... INFO_CANCEL_BUTTON_LABEL=Anuluj INFO_CERTIFICATE_DIALOG_ACCEPT_PERMANENTLY_BUTTON_TOOLTIP=Zamknij to okienko i zaakceptuj certyfikat na sta\u0142e. INFO_CANNOT_USE_DEFAULT_ADMIN_CONNECTOR_PORT=Nie mo\u017cna u\u017cy\u0107 4444. Port w u\u017cyciu lub nieautoryzowany u\u017cytkownik. diff --git a/opendj-server-legacy/src/messages/org/opends/messages/quickSetup_zh_CN.properties b/opendj-server-legacy/src/messages/org/opends/messages/quickSetup_zh_CN.properties index c1cd700fe0..c4cbc1413b 100644 --- a/opendj-server-legacy/src/messages/org/opends/messages/quickSetup_zh_CN.properties +++ b/opendj-server-legacy/src/messages/org/opends/messages/quickSetup_zh_CN.properties @@ -12,6 +12,7 @@ # # Copyright 2006-2010 Sun Microsystems, Inc. # Portions Copyright 2010-2016 ForgeRock AS. +# Portions Copyright 2026 3A Systems, LLC. @@ -491,7 +492,6 @@ INFO_SERVER_SETTINGS_PANEL_INSTRUCTIONS=\u8f93\u5165\u7528\u4e8e\u4fa6\u542c LDA INFO_SERVER_SETTINGS_PANEL_TITLE=\u670d\u52a1\u5668\u8bbe\u7f6e INFO_SERVER_SETTINGS_STEP=\u670d\u52a1\u5668\u8bbe\u7f6e INFO_SETUP_LAUNCHER_GUI_LAUNCHED_FAILED=%n%n\u542f\u52a8\u56fe\u5f62\u5316\u5b89\u88c5\u5931\u8d25\u3002%n%n\u6b63\u5728\u542f\u52a8\u547d\u4ee4\u884c\u5b89\u88c5... -INFO_SETUP_LAUNCHER_GUI_LAUNCHED_FAILED_DETAILS=%n%n\u542f\u52a8\u56fe\u5f62\u5316\u5b89\u88c5\u5931\u8d25\u3002\u8bf7\u68c0\u67e5\u6587\u4ef6 %s\uff0c\u4ee5\u83b7\u53d6\u66f4\u591a\u7684\u8be6\u7ec6\u4fe1\u606f\u3002%n%n\u6b63\u5728\u542f\u52a8\u547d\u4ee4\u884c\u5b89\u88c5... INFO_SETUP_LAUNCHER_LAUNCHING_GUI=\u6b63\u5728\u542f\u52a8\u56fe\u5f62\u5316\u5b89\u88c5... INFO_SHOW_EXCEPTION_DETAILS=\u663e\u793a\u8be6\u7ec6\u8d44\u6599 # Only translate if the image is specific to the local diff --git a/opendj-server-legacy/src/messages/org/opends/messages/quickSetup_zh_TW.properties b/opendj-server-legacy/src/messages/org/opends/messages/quickSetup_zh_TW.properties index 23086687e9..6de08a8f34 100644 --- a/opendj-server-legacy/src/messages/org/opends/messages/quickSetup_zh_TW.properties +++ b/opendj-server-legacy/src/messages/org/opends/messages/quickSetup_zh_TW.properties @@ -12,6 +12,7 @@ # # Copyright 2006-2009 Sun Microsystems, Inc. # Portions Copyright 2010-2016 ForgeRock AS. +# Portions Copyright 2026 3A Systems, LLC. @@ -471,7 +472,6 @@ INFO_SERVER_SETTINGS_PANEL_INSTRUCTIONS=Enter a port to listen for LDAP requests INFO_SERVER_SETTINGS_PANEL_TITLE=\u4f3a\u670d\u5668\u8a2d\u5b9a INFO_SERVER_SETTINGS_STEP=\u4f3a\u670d\u5668\u8a2d\u5b9a INFO_SETUP_LAUNCHER_GUI_LAUNCHED_FAILED=%n%n\u5716\u5f62\u5316\u8a2d\u5b9a\u555f\u52d5\u5931\u6557\u3002%n%n\u6b63\u5728\u555f\u52d5\u6307\u4ee4\u884c\u8a2d\u5b9a... -INFO_SETUP_LAUNCHER_GUI_LAUNCHED_FAILED_DETAILS=%n%n\u5716\u5f62\u5316\u8a2d\u5b9a\u555f\u52d5\u5931\u6557\u3002\u6aa2\u67e5\u6a94\u6848 %s \u4ee5\u53d6\u5f97\u66f4\u591a\u8a73\u7d30\u8cc7\u6599\u3002%n%n\u6b63\u5728\u555f\u52d5\u6307\u4ee4\u884c\u8a2d\u5b9a... INFO_SETUP_LAUNCHER_LAUNCHING_GUI=\u6b63\u5728\u555f\u52d5\u5716\u5f62\u5316\u8a2d\u5b9a... INFO_SHOW_EXCEPTION_DETAILS=\u986f\u793a\u8a73\u7d30\u8cc7\u8a0a # Only translate if the image is specific to the local diff --git a/opendj-server-legacy/src/test/java/org/opends/quicksetup/LauncherTest.java b/opendj-server-legacy/src/test/java/org/opends/quicksetup/LauncherTest.java index da185ad06e..1e0de53663 100644 --- a/opendj-server-legacy/src/test/java/org/opends/quicksetup/LauncherTest.java +++ b/opendj-server-legacy/src/test/java/org/opends/quicksetup/LauncherTest.java @@ -29,6 +29,7 @@ import org.forgerock.i18n.LocalizableMessage; import org.opends.server.DirectoryServerTestCase; import org.opends.server.TestCaseUtils; +import org.testng.SkipException; import org.testng.annotations.AfterClass; import org.testng.annotations.BeforeClass; import org.testng.annotations.Test; @@ -121,11 +122,42 @@ public void testLaunchingTheGuiCreatesNoLog() throws Exception assertSame(logFile, launcher.getTempLogFile()); } + /** + * A GUI which does not come up costs no log either, and its reason is not lost for that: it + * goes into the log as soon as something asks for one - on the headless road, the operation + * which runs on the command line instead. + */ + @Test + public void testAGuiWhichDoesNotComeUpIsLoggedOnceThereIsALog() throws Exception + { + if ("true".equalsIgnoreCase(System.getenv("OPENDJ_LOG_TO_STDOUT"))) + { + // The reason is written to the log, which then goes to stdout and leaves the file empty. + throw new SkipException("OPENDJ_LOG_TO_STDOUT writes the log to stdout, not to the file"); + } + final File logs = new File(tempDir, "headless/logs"); + final TestLauncher launcher = new TestLauncher(logs); + launcher.splashFailure = new IllegalStateException("no display at all"); + + assertTrue(launcher.launchGui(new String[0]) != 0, "a GUI which does not come up is reported"); + + assertFalse(launcher.hasTempLogFile(), "a GUI which does not come up must not cost a log"); + assertFalse(logs.exists(), logs.getPath()); + + final TempLogFile logFile = launcher.getTempLogFile(); + created.add(logFile); + logFile.writer.shutdown(); + final String contents = logFile.readContents(); + assertTrue(contents.contains("no display at all"), contents); + } + /** A launcher with nothing in it but the log file behaviour under test. */ private static final class TestLauncher extends Launcher { /** What {@link Launcher#launchGui(String[])} handed the splash screen. */ private Supplier splashLogFile; + /** What the splash screen throws, if it is not to come up. */ + private RuntimeException splashFailure; TestLauncher(final File tempLogFileDirectory) { @@ -135,8 +167,13 @@ private static final class TestLauncher extends Launcher @Override void startSplashScreen(final Supplier tempLogFile, final String[] args) { - // No display here, and no install either: the wizard is quit at its first step. + // No display here, and no install either: the wizard is quit at its first step - or it + // does not come up at all. splashLogFile = tempLogFile; + if (splashFailure != null) + { + throw splashFailure; + } } @Override diff --git a/opendj-server-legacy/src/test/java/org/opends/quicksetup/installer/InstallerTest.java b/opendj-server-legacy/src/test/java/org/opends/quicksetup/installer/InstallerTest.java index c0275a788b..5f6b6242fd 100644 --- a/opendj-server-legacy/src/test/java/org/opends/quicksetup/installer/InstallerTest.java +++ b/opendj-server-legacy/src/test/java/org/opends/quicksetup/installer/InstallerTest.java @@ -20,6 +20,7 @@ import static org.opends.messages.QuickSetupMessages.*; import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertNull; import static org.testng.Assert.assertTrue; import java.io.File; @@ -154,11 +155,12 @@ public void testNoLogMeansNoReport() throws Exception } /** - * A cancelled install takes its log with it. + * A cancelled install takes its log with it - and asks for it only when it begins. *

* Nothing names the log on that road - the report belongs to the failure road - and the * cancel has just taken the installation back, so a log kept there is a report nobody is - * ever pointed at (issue #1030). + * ever pointed at (issue #1030). The installer is handed the log the way the wizard and the + * command line hand it over, as a supplier: nothing asks for it before {@code run()}. */ @Test public void testACancelledInstallTakesItsLogWithIt() throws Exception @@ -169,33 +171,34 @@ public void testACancelledInstallTakesItsLogWithIt() throws Exception // server as running and goes off to stop it. assertTrue(new File(instance, "locks").mkdirs()); - // An installation under the temporary directory rather than the one the class path names: - // the installer takes both paths from there, and in a test run there is none. - final Installer installer = new Installer() - { - @Override - public String getInstallationPath() - { - return instance.getAbsolutePath(); - } - - @Override - public String getInstancePath() - { - return instance.getAbsolutePath(); - } - }; + final TestInstaller installer = new TestInstaller(instance); installer.setProgressMessageFormatter(new MarkedArmsFormatter()); - installer.setTempLogFile(logFile); + final int[] asked = { 0 }; + installer.setTempLogFile(() -> { + asked[0]++; + return logFile; + }); installer.setUserData(new UserData()); installer.cancel(); + assertEquals(asked[0], 0, "nothing is logged before the install begins"); installer.run(); + assertEquals(asked[0], 1, "the install asks for its log once, as it begins"); assertEquals(installer.getCurrentProgressStep(), InstallProgressStep.FINISHED_CANCELED); assertFalse(logFile.getLogFile().exists(), logFile.getPath()); } + /** + * An application nobody handed a log to has none, and asking for it is not a failure: the + * command line uninstaller is built that way, its launcher having created the log already. + */ + @Test + public void testAnApplicationGivenNoLogHasNone() throws Exception + { + assertNull(new TestInstaller(null).openTempLogFile()); + } + private TempLogFile newLogFile() { final TempLogFile logFile = TempLogFile.newTempLogFile(PREFIX, new File(tempDir, "logs")); @@ -206,7 +209,7 @@ private TempLogFile newLogFile() /** What the listeners of a failed installation are told about the log file. */ private static String reportOf(final TempLogFile logFile) { - final Installer installer = new Installer(); + final TestInstaller installer = new TestInstaller(null); installer.setProgressMessageFormatter(new MarkedArmsFormatter()); final StringBuilder report = new StringBuilder(); installer.addProgressUpdateListener(new ProgressUpdateListener() @@ -220,7 +223,9 @@ public void progressUpdate(final ProgressUpdateEvent ev) } } }); - installer.setTempLogFile(logFile); + installer.setTempLogFile(() -> logFile); + // The report is made by an install which has begun, and so has asked for its log. + installer.openTempLogFile(); installer.notifyListenersOfExistingLogFile(); return report.toString(); } @@ -235,6 +240,40 @@ private static void assertDoesNotContain(final String report, final LocalizableM assertFalse(report.contains(unexpected.toString()), "unexpected <" + unexpected + "> in <" + report + ">"); } + /** An installer which can be told where it lives, and whose log can be asked for. */ + private static final class TestInstaller extends Installer + { + /** The installation, or {@code null} for the one the class path names. */ + private final File instance; + + TestInstaller(final File instance) + { + this.instance = instance; + } + + /** + * An installation under the temporary directory rather than the one the class path names: + * the installer takes both paths from there, and in a test run there is none. + */ + @Override + public String getInstallationPath() + { + return instance != null ? instance.getAbsolutePath() : super.getInstallationPath(); + } + + @Override + public String getInstancePath() + { + return instance != null ? instance.getAbsolutePath() : super.getInstancePath(); + } + + @Override + protected TempLogFile openTempLogFile() + { + return super.openTempLogFile(); + } + } + /** * A formatter that marks the arm each line goes out on. *

diff --git a/opendj-server-legacy/src/test/java/org/opends/server/tools/InstallDSTest.java b/opendj-server-legacy/src/test/java/org/opends/server/tools/InstallDSTest.java index 4f6db14f47..ce4127ebf2 100644 --- a/opendj-server-legacy/src/test/java/org/opends/server/tools/InstallDSTest.java +++ b/opendj-server-legacy/src/test/java/org/opends/server/tools/InstallDSTest.java @@ -16,13 +16,18 @@ package org.opends.server.tools; import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertTrue; +import java.io.File; +import java.lang.reflect.Field; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Supplier; import org.opends.quicksetup.Constants; +import org.opends.quicksetup.Installation; import org.opends.quicksetup.TempLogFile; import org.opends.server.DirectoryServerTestCase; +import org.opends.server.TestCaseUtils; import org.testng.annotations.Test; /** @@ -41,6 +46,8 @@ public class InstallDSTest extends DirectoryServerTestCase private static final int SUCCESSFUL_NOP = 0; /** {@code InstallReturnCode.ERROR_USER_DATA}: the arguments could not be parsed. */ private static final int ERROR_USER_DATA = 2; + /** {@code InstallReturnCode.ERROR_SERVER_ALREADY_INSTALLED}: the server is configured already. */ + private static final int ERROR_SERVER_ALREADY_INSTALLED = 3; /** {@code setup --help} displays the usage and returns before anything can fail. */ @Test @@ -56,6 +63,41 @@ public void testAUsageErrorAsksForNoLog() throws Exception assertAsksForNoLog(ERROR_USER_DATA, "--no-such-option"); } + /** + * A server which is configured already is reported as such, and no install is attempted. + *

+ * This is the first road past the argument parser - the licence and the prompts come after + * it - so it is the one which tells a log asked for as soon as the arguments are known from + * one asked for where the install begins. + */ + @Test + public void testAServerWhichIsConfiguredAlreadyAsksForNoLog() throws Exception + { + // The installation the setup looks at: one whose configuration directory holds something. + // Neither the class path of a test run nor the server the tests run against will do - the + // former is no installation, and the latter is not to be taken as one - so the setup is + // pointed at a directory of its own. A locks directory keeps it from reading as running. + final File instance = TestCaseUtils.createTemporaryDirectory("installDSTest"); + assertTrue(new File(instance, "locks").mkdirs()); + assertTrue(new File(instance, "config").mkdirs()); + assertTrue(new File(instance, "config/config.ldif").createNewFile()); + + // Installation.getLocal() keeps what it resolved first in a static field. + final Field local = Installation.class.getDeclaredField("local"); + local.setAccessible(true); + final Object previous = local.get(null); + local.set(null, new Installation(instance, instance)); + try + { + assertAsksForNoLog(ERROR_SERVER_ALREADY_INSTALLED); + } + finally + { + local.set(null, previous); + TestCaseUtils.deleteDirectory(instance); + } + } + /** * Runs the CLI setup with a supplier which counts the asks and hands out nothing, so that a * road which asks for the log is red here rather than a file left behind on a real run.