Fix code warnings in `SimpleLogHandler` and test. Use `ProcessHandle` to get the pid, cleaning up a `TODO`. PiperOrigin-RevId: 407454858
diff --git a/src/main/java/com/google/devtools/build/lib/util/SimpleLogHandler.java b/src/main/java/com/google/devtools/build/lib/util/SimpleLogHandler.java index e03b871..6f0edb9 100644 --- a/src/main/java/com/google/devtools/build/lib/util/SimpleLogHandler.java +++ b/src/main/java/com/google/devtools/build/lib/util/SimpleLogHandler.java
@@ -28,7 +28,6 @@ import java.io.IOException; import java.io.InterruptedIOException; import java.io.OutputStreamWriter; -import java.lang.management.ManagementFactory; import java.nio.file.DirectoryStream; import java.nio.file.Files; import java.nio.file.LinkOption; @@ -90,7 +89,7 @@ */ private final Optional<Path> symlinkPath; /** Absolute path to common base name of log files. */ - @VisibleForTesting final Path baseFilePath; + private final Path baseFilePath; /** Log file currently in use. */ @GuardedBy("this") private final Output output = new Output(); @@ -204,8 +203,8 @@ * <p>If unset, the value of "symlink" from the JVM logging configuration for {@link * SimpleLogHandler} will be used; and if that's unset, the prefix will be used. * - * @param symlink either symlink filename without a directory part, or an absolute path whose - * directory part matches the prefix + * @param symlinkName either symlink filename without a directory part, or an absolute path + * whose directory part matches the prefix * @return this {@code Builder} object */ public Builder setSymlinkName(String symlinkName) { @@ -224,7 +223,7 @@ * @return this {@code Builder} object */ public Builder setCreateSymlink(boolean createSymlink) { - this.createSymlink = Boolean.valueOf(createSymlink); + this.createSymlink = createSymlink; return this; } @@ -238,7 +237,7 @@ * @return this {@code Builder} object */ public Builder setRotateLimitBytes(int rotateLimitBytes) { - this.rotateLimitBytes = Integer.valueOf(rotateLimitBytes); + this.rotateLimitBytes = rotateLimitBytes; return this; } @@ -256,7 +255,7 @@ * @return this {@code Builder} object */ public Builder setTotalLimitBytes(int totalLimitBytes) { - this.totalLimitBytes = Integer.valueOf(totalLimitBytes); + this.totalLimitBytes = totalLimitBytes; return this; } @@ -367,7 +366,9 @@ ? Optional.of( getSymlinkAbsolutePath(this.baseFilePath.getParent(), configuredSymlinkName)) : Optional.empty(); - this.extension = getConfiguredStringProperty(extension, "extension", getPidString()); + this.extension = + getConfiguredStringProperty( + extension, "extension", Long.toString(ProcessHandle.current().pid())); this.isStaticExtension = (getConfiguredStringProperty(extension, "extension", null) != null); this.rotateLimitBytes = getConfiguredIntProperty(rotateLimitBytes, "rotate_limit_bytes", 0); checkArgument(this.rotateLimitBytes >= 0, "File size limits cannot be negative"); @@ -524,7 +525,7 @@ /** Matches java.logging.* configuration behavior; configured strings are trimmed. */ private static String getConfiguredStringProperty( String builderValue, String configuredName, String fallbackValue) { - return getConfiguredProperty(builderValue, configuredName, val -> val.trim(), fallbackValue); + return getConfiguredProperty(builderValue, configuredName, String::trim, fallbackValue); } /** @@ -545,13 +546,13 @@ return true; } else if ("false".equals(val) || "0".equals(val)) { return false; - } else if (val.length() == 0) { + } else if (val.isEmpty()) { return null; } throw new IllegalArgumentException("Cannot parse boolean property value"); }, null); - return value != null ? value.booleanValue() : fallbackValue; + return value != null ? value : fallbackValue; } /** @@ -567,10 +568,10 @@ configuredName, val -> { val = val.trim(); - return val.length() > 0 ? Integer.parseInt(val) : null; + return !val.isEmpty() ? Integer.parseInt(val) : null; }, null); - return value != null ? value.intValue() : fallbackValue; + return value != null ? value : fallbackValue; } /** @@ -586,7 +587,7 @@ configuredName, val -> { val = val.trim(); - return val.length() > 0 ? Level.parse(val) : null; + return !val.isEmpty() ? Level.parse(val) : null; }, null); return value != null ? value : fallbackValue; @@ -605,7 +606,7 @@ configuredName, val -> { val = val.trim(); - if (val.length() > 0) { + if (!val.isEmpty()) { try { return (Formatter) ClassLoader.getSystemClassLoader() @@ -623,19 +624,6 @@ } @VisibleForTesting - static String getPidString() { - long pid; - try { - // TODO(b/78168359): Replace with ProcessHandle.current().pid() in Java 9 - pid = Long.parseLong(ManagementFactory.getRuntimeMXBean().getName().split("@", -1)[0]); - } catch (NumberFormatException e) { - // getRuntimeMXBean().getName() output is implementation-specific, may be unparseable. - pid = 0; - } - return Long.toString(pid); - } - - @VisibleForTesting static String getLocalHostnameFirstComponent() { String name = NetUtil.getCachedShortHostName(); if (!InetAddresses.isInetAddress(name)) { @@ -707,7 +695,7 @@ */ private static Path getSymlinkAbsolutePath(Path logDir, String symlink) { checkNotNull(symlink); - checkArgument(symlink.length() > 0); + checkArgument(!symlink.isEmpty()); File symlinkFile = new File(symlink); if (!symlinkFile.isAbsolute()) { symlinkFile = new File(logDir + File.separator + symlink); @@ -800,7 +788,7 @@ * @throws NullPointerException if not open * @throws IOException if an underlying IO operation failed */ - public void closeIfByteCountAtleast(int limit) throws IOException { + void closeIfByteCountAtleast(int limit) throws IOException { if (stream.getCount() < limit && stream.getCount() + 8192L >= limit) { // The writer and its internal encoder buffer output before writing to the output stream. // The default size of the encoder's buffer is 8192 bytes. To count the bytes in the output @@ -892,9 +880,7 @@ return timestamp; } - /** - * File path ordered by timestamp. - */ + /** File path ordered by timestamp. */ private static final class PathByTimestamp implements Comparable<PathByTimestamp> { private final Path path; private final Date timestamp;
diff --git a/src/test/java/com/google/devtools/build/lib/util/SimpleLogHandlerTest.java b/src/test/java/com/google/devtools/build/lib/util/SimpleLogHandlerTest.java index 5d7f49a..112afc6 100644 --- a/src/test/java/com/google/devtools/build/lib/util/SimpleLogHandlerTest.java +++ b/src/test/java/com/google/devtools/build/lib/util/SimpleLogHandlerTest.java
@@ -101,7 +101,7 @@ SimpleLogHandler.builder().setPrefix(tmp.getRoot() + File.separator + "hello").build(); handler.publish(new LogRecord(Level.SEVERE, "Hello world")); // To open the log file. assertThat(handler.getCurrentLogFilePath().get().toString()) - .endsWith("." + SimpleLogHandler.getPidString()); + .endsWith("." + ProcessHandle.current().pid()); } @Test @@ -179,7 +179,7 @@ Path logPath = handler.getCurrentLogFilePath().get(); handler.close(); - assertThat(new String(Files.readAllBytes(logPath), UTF_8)).isEqualTo("Hello world\n"); + assertThat(Files.readString(logPath)).isEqualTo("Hello world\n"); } @Test @@ -238,7 +238,7 @@ } @Test - public void testSymlinkDisabling() throws Exception { + public void testSymlinkDisabling() { SimpleLogHandler handler = SimpleLogHandler.builder() .setPrefix(tmp.getRoot() + File.separator + "hello") @@ -256,7 +256,7 @@ .setPrefix(tmp.getRoot() + File.separator + "hello") .setSymlinkName("bye" + File.separator + "bye") .setCreateSymlink(true); - assertThrows(IllegalArgumentException.class, () -> builder.build()); + assertThrows(IllegalArgumentException.class, builder::build); } @Test