HDDS-12539. Enable some String-related rules in PMD

dev-support/pmd/pmd-ruleset.xml

@@ -33,7 +33,12 @@
   <rule ref="category/java/bestpractices.xml/UseCollectionIsEmpty" />
   <rule ref="category/java/bestpractices.xml/SimplifiableTestAssertion" />
 
+  <rule ref="category/java/performance.xml/AddEmptyString"/>
   <rule ref="category/java/performance.xml/AvoidFileStream"/>
+  <rule ref="category/java/performance.xml/InefficientEmptyStringCheck"/>
+  <rule ref="category/java/performance.xml/InefficientStringBuffering"/>
+  <rule ref="category/java/performance.xml/StringInstantiation"/>
+  <rule ref="category/java/performance.xml/UseStringBufferLength"/>
 
   <exclude-pattern>.*/generated-sources/.*</exclude-pattern>
 </ruleset>

hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NodeSchemaManager.java

@@ -124,7 +124,7 @@ public String complete(String path) {
     int i, j;
     for (i = 1, j = 1; i < subPath.length && j < (allSchema.size() - 1);) {
       if (allSchema.get(j).matchPrefix(subPath[i])) {
-        newPath.append(NetConstants.PATH_SEPARATOR_STR + subPath[i]);
+        newPath.append(NetConstants.PATH_SEPARATOR_STR).append(subPath[i]);
         i++;
         j++;
       } else {
@@ -133,7 +133,7 @@ public String complete(String path) {
       }
     }
     if (i == (subPath.length - 1)) {
-      newPath.append(NetConstants.PATH_SEPARATOR_STR + subPath[i]);
+      newPath.append(NetConstants.PATH_SEPARATOR_STR).append(subPath[i]);
       return newPath.toString();
     }
     return null;

hadoop-hdds/config/src/main/java/org/apache/hadoop/hdds/conf/ConfigurationSource.java

@@ -98,7 +98,7 @@ default String getTrimmed(String key, String defaultValue) {
 
   default String[] getTrimmedStrings(String name) {
     String valueString = get(name);
-    if (null == valueString || valueString.trim().isEmpty()) {
+    if (null == valueString) {
       return EMPTY_STRING_ARRAY;
     }
 

hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/helpers/BlockDeletingServiceMetrics.java

@@ -205,34 +205,23 @@ public long getProcessedTransactionFailCount() {
 
   @Override
   public String toString() {
-    StringBuffer buffer = new StringBuffer();
-    buffer.append("successCount = " + successCount.value()).append("\t")
-        .append("successBytes = " + successBytes.value()).append("\t")
-        .append("failureCount = " + failureCount.value()).append("\t")
-        .append("outOfOrderDeleteBlockTransactionCount = "
-            + outOfOrderDeleteBlockTransactionCount.value()).append("\t")
-        .append("totalPendingBlockCount = "
-            + totalPendingBlockCount.value()).append("\t")
-        .append("totalBlockChosenCount = "
-            + totalBlockChosenCount.value()).append("\t")
-        .append("totalContainerChosenCount = "
-            + totalContainerChosenCount.value()).append("\t")
-        .append("receivedTransactionCount = "
-            + receivedTransactionCount.value()).append("\t")
-        .append("receivedRetryTransactionCount = "
-            + receivedRetryTransactionCount.value()).append("\t")
-        .append("processedTransactionSuccessCount = "
-            + processedTransactionSuccessCount.value()).append("\t")
-        .append("processedTransactionFailCount = "
-            + processedTransactionFailCount.value()).append("\t")
-        .append("receivedContainerCount = "
-            + receivedContainerCount.value()).append("\t")
-        .append("receivedBlockCount = "
-            + receivedBlockCount.value()).append("\t")
-        .append("markedBlockCount = "
-            + markedBlockCount.value()).append("\t")
-        .append("totalLockTimeoutTransactionCount = "
-            + totalLockTimeoutTransactionCount.value()).append("\t");
+    StringBuilder buffer = new StringBuilder()
+        .append("successCount = ").append(successCount.value()).append("\t")
+        .append("successBytes = ").append(successBytes.value()).append("\t")
+        .append("failureCount = ").append(failureCount.value()).append("\t")
+        .append("outOfOrderDeleteBlockTransactionCount = ")
+        .append(outOfOrderDeleteBlockTransactionCount.value()).append("\t")
+        .append("totalPendingBlockCount = ").append(totalPendingBlockCount.value()).append("\t")
+        .append("totalBlockChosenCount = ").append(totalBlockChosenCount.value()).append("\t")
+        .append("totalContainerChosenCount = ").append(totalContainerChosenCount.value()).append("\t")
+        .append("receivedTransactionCount = ").append(receivedTransactionCount.value()).append("\t")
+        .append("receivedRetryTransactionCount = ").append(receivedRetryTransactionCount.value()).append("\t")
+        .append("processedTransactionSuccessCount = ").append(processedTransactionSuccessCount.value()).append("\t")
+        .append("processedTransactionFailCount = ").append(processedTransactionFailCount.value()).append("\t")
+        .append("receivedContainerCount = ").append(receivedContainerCount.value()).append("\t")
+        .append("receivedBlockCount = ").append(receivedBlockCount.value()).append("\t")
+        .append("markedBlockCount = ").append(markedBlockCount.value()).append("\t")
+        .append("totalLockTimeoutTransactionCount = ").append(totalLockTimeoutTransactionCount.value()).append("\t");
     return buffer.toString();
   }
 }

hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/TarContainerPacker.java

@@ -305,7 +305,7 @@ private byte[] innerUnpack(InputStream input, Path dbRoot, Path chunksRoot)
           descriptorFileContent = readEntry(archiveInput, size);
         } else {
           throw new IllegalArgumentException(
-              "Unknown entry in the tar file: " + "" + name);
+              "Unknown entry in the tar file: " + name);
         }
         entry = archiveInput.getNextEntry();
       }

hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/helpers/TestContainerUtils.java

@@ -95,7 +95,7 @@ public void testDatanodeIDPersistent(@TempDir File tempDir) throws Exception {
     assertWriteRead(tempDir, id1);
 
     // Add certificate serial  id.
-    id1.setCertSerialId("" + RandomUtils.nextLong());
+    id1.setCertSerialId(String.valueOf(RandomUtils.secure().randomLong()));
     assertWriteRead(tempDir, id1);
 
     // Read should return an empty value if file doesn't exist

hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainerMetadataInspector.java

@@ -29,6 +29,7 @@
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.hadoop.hdds.JsonTestUtils;
 import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.DeletedBlocksTransaction;
 import org.apache.hadoop.hdds.utils.db.BatchOperation;
@@ -511,7 +512,7 @@ private JsonNode runInspectorAndGetReport(
     String output = capturer.getOutput();
     capturer.clearOutput();
     // Check if the output is effectively empty
-    if (output.trim().isEmpty()) {
+    if (StringUtils.isBlank(output)) {
       return null;
     }
     return JsonTestUtils.readTree(output);

hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/server/http/ProfileServlet.java

@@ -37,6 +37,7 @@
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang3.StringUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -162,7 +163,7 @@ private Integer getPid() {
 
     // in case if it is not set correctly used fallback from mxbean which is
     // implementation specific
-    if (pidStr == null || pidStr.trim().isEmpty()) {
+    if (StringUtils.isBlank(pidStr)) {
       String name = ManagementFactory.getRuntimeMXBean().getName();
       if (name != null) {
         int idx = name.indexOf("@");
@@ -217,7 +218,7 @@ protected static String validateFileName(String filename) {
   protected void doGet(final HttpServletRequest req,
       final HttpServletResponse resp) throws IOException {
     // make sure async profiler home is set
-    if (asyncProfilerHome == null || asyncProfilerHome.trim().isEmpty()) {
+    if (StringUtils.isBlank(asyncProfilerHome)) {
       resp.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
       setResponseHeader(resp);
       resp.getWriter().write("ASYNC_PROFILER_HOME env is not set.");
@@ -272,7 +273,7 @@ protected void doGet(final HttpServletRequest req,
             cmd.add("-e");
             cmd.add(event.getInternalName());
             cmd.add("-d");
-            cmd.add("" + duration);
+            cmd.add(String.valueOf(duration));
             cmd.add("-o");
             cmd.add(output.name().toLowerCase());
             cmd.add("-f");
@@ -469,7 +470,7 @@ static String getAsyncProfilerHome() {
     String asyncProfilerHome = System.getenv(ASYNC_PROFILER_HOME_ENV);
     // if ENV is not set, see if -Dasync.profiler
     // .home=/path/to/async/profiler/home is set
-    if (asyncProfilerHome == null || asyncProfilerHome.trim().isEmpty()) {
+    if (StringUtils.isBlank(asyncProfilerHome)) {
       asyncProfilerHome =
           System.getProperty(ASYNC_PROFILER_HOME_SYSTEM_PROPERTY);
     }

hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java

@@ -329,25 +329,17 @@ public long getNumCommandsDatanodeFailed() {
 
   @Override
   public String toString() {
-    StringBuffer buffer = new StringBuffer();
-    buffer.append("numBlockDeletionTransactionCreated = "
-        + numBlockDeletionTransactionCreated.value()).append("\t")
-        .append("numBlockDeletionTransactionCompleted = "
-            + numBlockDeletionTransactionCompleted.value()).append("\t")
-        .append("numBlockDeletionCommandSent = "
-            + numBlockDeletionCommandSent.value()).append("\t")
-        .append("numBlockDeletionCommandSuccess = "
-            + numBlockDeletionCommandSuccess.value()).append("\t")
-        .append("numBlockDeletionCommandFailure = "
-            + numBlockDeletionCommandFailure.value()).append("\t")
-        .append("numBlockDeletionTransactionSent = "
-            + numBlockDeletionTransactionSent.value()).append("\t")
-        .append("numBlockDeletionTransactionSuccess = "
-            + numBlockDeletionTransactionSuccess.value()).append("\t")
-        .append("numBlockDeletionTransactionFailure = "
-            + numBlockDeletionTransactionFailure.value()).append("\t")
-        .append("numDeletionCommandsPerDatanode = "
-            + numCommandsDatanode);
+    StringBuilder buffer = new StringBuilder()
+        .append("numBlockDeletionTransactionCreated = ").append(numBlockDeletionTransactionCreated.value()).append("\t")
+        .append("numBlockDeletionTransactionCompleted = ")
+        .append(numBlockDeletionTransactionCompleted.value()).append("\t")
+        .append("numBlockDeletionCommandSent = ").append(numBlockDeletionCommandSent.value()).append("\t")
+        .append("numBlockDeletionCommandSuccess = ").append(numBlockDeletionCommandSuccess.value()).append("\t")
+        .append("numBlockDeletionCommandFailure = ").append(numBlockDeletionCommandFailure.value()).append("\t")
+        .append("numBlockDeletionTransactionSent = ").append(numBlockDeletionTransactionSent.value()).append("\t")
+        .append("numBlockDeletionTransactionSuccess = ").append(numBlockDeletionTransactionSuccess.value()).append("\t")
+        .append("numBlockDeletionTransactionFailure = ").append(numBlockDeletionTransactionFailure.value()).append("\t")
+        .append("numDeletionCommandsPerDatanode = ").append(numCommandsDatanode);
     return buffer.toString();
   }
 }

hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/InfoSubcommand.java

@@ -216,14 +216,13 @@ private static String buildDatanodeDetails(DatanodeDetails details) {
   }
 
   private static String buildReplicaDetails(ContainerReplicaInfo replica) {
-    StringBuilder sb = new StringBuilder();
-    sb.append("State: " + replica.getState() + ";");
+    StringBuilder sb = new StringBuilder()
+        .append("State: ").append(replica.getState()).append(";");
     if (replica.getReplicaIndex() != -1) {
-      sb.append(" ReplicaIndex: " + replica.getReplicaIndex() + ";");
+      sb.append(" ReplicaIndex: ").append(replica.getReplicaIndex()).append(";");
     }
-    sb.append(" Origin: " + replica.getPlaceOfBirth().toString() + ";");
-    sb.append(" Location: "
-        + buildDatanodeDetails(replica.getDatanodeDetails()));
+    sb.append(" Origin: ").append(replica.getPlaceOfBirth().toString()).append(";")
+        .append(" Location: ").append(buildDatanodeDetails(replica.getDatanodeDetails()));
     return sb.toString();
   }
 

hadoop-hdds/tools/src/test/java/org/apache/hadoop/hdds/scm/cli/container/TestReportSubCommand.java

@@ -177,7 +177,7 @@ private String containerList(int start, int end) {
       if (i != start) {
         sb.append(", ");
       }
-      sb.append("#" + i);
+      sb.append("#").append(i);
     }
     return sb.toString();
   }

hadoop-ozone/cli-shell/src/test/java/org/apache/hadoop/ozone/shell/TestOzoneAddress.java

@@ -52,7 +52,7 @@ public void checkRootUrlType(String prefix) throws OzoneClientException {
     address = new OzoneAddress("");
     address.ensureRootAddress();
 
-    address = new OzoneAddress(prefix + "");
+    address = new OzoneAddress(prefix);
     address.ensureRootAddress();
   }
 

hadoop-ozone/httpfsgateway/src/main/java/org/apache/ozone/lib/wsrs/Param.java

@@ -18,6 +18,7 @@
 package org.apache.ozone.lib.wsrs;
 
 import java.text.MessageFormat;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.hadoop.hdds.annotation.InterfaceAudience;
 
 /**
@@ -42,7 +43,7 @@ public String getName() {
 
   public T parseParam(String str) {
     try {
-      value = (str != null && !str.trim().isEmpty()) ? parse(str) : value;
+      value = StringUtils.isNotBlank(str) ? parse(str) : value;
     } catch (Exception ex) {
       throw new IllegalArgumentException(
         MessageFormat.format("Parameter [{0}], invalid value [{1}], " +

hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java

@@ -3040,7 +3040,7 @@ public String getNamespace() {
 
   @Override
   public String getRpcPort() {
-    return "" + omRpcAddress.getPort();
+    return String.valueOf(omRpcAddress.getPort());
   }
 
   private static List<List<String>> getRatisRolesException(String exceptionString) {

hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/helpers/OMAuditLogger.java

@@ -120,7 +120,7 @@ public static void log(OMAuditLogger.Builder builder, TermIndex termIndex) {
       if (null == builder.getAuditMap()) {
         builder.setAuditMap(new HashMap<>());
       }
-      builder.getAuditMap().put("Transaction", "" + termIndex.getIndex());
+      builder.getAuditMap().put("Transaction", String.valueOf(termIndex.getIndex()));
       builder.getMessageBuilder().withParams(builder.getAuditMap());
       builder.getAuditLogger().logWrite(builder.getMessageBuilder().build());
     }
@@ -150,7 +150,7 @@ public static void log(OMAuditLogger.Builder builder, OMClientRequest request, O
     }
     try {
       builder.getAuditMap().put("Command", request.getOmRequest().getCmdType().name());
-      builder.getAuditMap().put("Transaction", "" + termIndex.getIndex());
+      builder.getAuditMap().put("Transaction", String.valueOf(termIndex.getIndex()));
       request.buildAuditMessage(action, builder.getAuditMap(),
           th, request.getUserInfo());
       builder.setLog(true);

hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerDoubleBuffer.java

@@ -515,9 +515,8 @@ private void terminate(Throwable t, int status) {
   }
 
   private void terminate(Throwable t, int status, OMResponse omResponse) {
-    StringBuilder message = new StringBuilder(
-        "During flush to DB encountered error in " +
-        "OMDoubleBuffer flush thread " + Thread.currentThread().getName());
+    StringBuilder message = new StringBuilder("During flush to DB encountered error in OMDoubleBuffer flush thread ")
+        .append(Thread.currentThread().getName());
     if (omResponse != null) {
       message.append(" when handling OMRequest: ").append(omResponse);
     }

hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/OpenKeyCleanupService.java

@@ -204,7 +204,8 @@ public BackgroundTaskResult call() throws Exception {
           if (LOG.isDebugEnabled()) {
             StringBuilder sb = new StringBuilder();
             for (OpenKeyBucket.Builder openKey : openKeyBuckets) {
-              sb.append(openKey.getVolumeName() + OZONE_URI_DELIMITER +  openKey.getBucketName() + ": ")
+              sb.append(openKey.getVolumeName()).append(OZONE_URI_DELIMITER).append(openKey.getBucketName())
+                  .append(": ")
                   .append(openKey.getKeysList().stream().map(OzoneManagerProtocolProtos.OpenKey::getName)
                       .collect(Collectors.toList()))
                   .append("\n");
@@ -226,8 +227,8 @@ public BackgroundTaskResult call() throws Exception {
             if (LOG.isDebugEnabled()) {
               StringBuilder sb = new StringBuilder();
               for (CommitKeyRequest.Builder openKey : hsyncKeys) {
-                sb.append(openKey.getKeyArgs().getVolumeName() + OZONE_URI_DELIMITER +
-                        openKey.getKeyArgs().getBucketName() + ": ")
+                sb.append(openKey.getKeyArgs().getVolumeName()).append(OZONE_URI_DELIMITER)
+                    .append(openKey.getKeyArgs().getBucketName()).append(": ")
                     .append(openKey.getKeyArgs().getKeyName())
                     .append(", ");
               }

hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/ratis_snapshot/TestOmRatisSnapshotProvider.java

@@ -81,7 +81,7 @@ public void setup(@TempDir File snapshotDir,
             false, connectionFactory);
 
     sb = new StringBuilder();
-    sb.append("--" + MULTIPART_FORM_DATA_BOUNDARY + CR_NL);
+    sb.append("--").append(MULTIPART_FORM_DATA_BOUNDARY).append(CR_NL);
     sb.append(CONTENT_DISPOSITION);
   }
 
@@ -105,7 +105,7 @@ public void testDownloadSnapshot() throws IOException,
 
     omRatisSnapshotProvider.downloadSnapshot(leaderNodeId, targetFile);
 
-    sb.append("--" + MULTIPART_FORM_DATA_BOUNDARY + "--" + CR_NL);
+    sb.append("--").append(MULTIPART_FORM_DATA_BOUNDARY).append("--").append(CR_NL);
     assertEquals(sb.toString(),
         new String(outputStream.toByteArray(), StandardCharsets.UTF_8));
   }
@@ -123,7 +123,7 @@ public void testWriteFormDataWithSstFile() throws IOException {
     OmRatisSnapshotProvider.writeFormData(connection, sstFiles);
 
     sb.append(fileName).append(CR_NL);
-    sb.append("--" + MULTIPART_FORM_DATA_BOUNDARY + "--" + CR_NL);
+    sb.append("--").append(MULTIPART_FORM_DATA_BOUNDARY).append("--").append(CR_NL);
     assertEquals(sb.toString(),
         new String(outputStream.toByteArray(), StandardCharsets.UTF_8));
   }
@@ -136,7 +136,7 @@ public void testWriteFormDataWithoutSstFile() throws IOException {
 
     OmRatisSnapshotProvider.writeFormData(connection, new ArrayList<>());
 
-    sb.append("--" + MULTIPART_FORM_DATA_BOUNDARY + "--" + CR_NL);
+    sb.append("--").append(MULTIPART_FORM_DATA_BOUNDARY).append("--").append(CR_NL);
     assertEquals(sb.toString(),
         new String(outputStream.toByteArray(), StandardCharsets.UTF_8));
   }

hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyCommitRequest.java

@@ -21,7 +21,6 @@
 import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.OK;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
@@ -703,9 +702,11 @@ public void testValidateAndUpdateCacheOnOverwrite() throws Exception {
     List<? extends Table.KeyValue<String, RepeatedOmKeyInfo>> rangeKVs
         = omMetadataManager.getDeletedTable().getRangeKVs(null, 100, deletedKey);
     assertThat(rangeKVs.size()).isGreaterThan(0);
-    assertEquals(1, rangeKVs.get(0).getValue().getOmKeyInfoList().size());
-    assertFalse(rangeKVs.get(0).getKey().endsWith(rangeKVs.get(0).getValue().getOmKeyInfoList().get(0).getObjectID()
-        + ""));
+    Table.KeyValue<String, RepeatedOmKeyInfo> keyValue = rangeKVs.get(0);
+    String key = keyValue.getKey();
+    List<OmKeyInfo> omKeyInfoList = keyValue.getValue().getOmKeyInfoList();
+    assertEquals(1, omKeyInfoList.size());
+    assertThat(key).doesNotEndWith(String.valueOf(omKeyInfoList.get(0).getObjectID()));
   }
 
   /**