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
