HDDS-10367. Fix possible NullPointerException in listKeysLight method

hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java

@@ -254,7 +254,6 @@ public final class OzoneManagerProtocolClientSideTranslatorPB
   private OmTransport transport;
   private ThreadLocal<S3Auth> threadLocalS3Auth
       = new ThreadLocal<>();
-
   private boolean s3AuthCheck;
 
   public static final int BLOCK_ALLOCATION_RETRY_COUNT = 5;
@@ -1033,7 +1032,7 @@ public ListKeysLightResult listKeysLight(String volumeName,
     reqBuilder.setBucketName(bucketName);
     reqBuilder.setCount(maxKeys);
 
-    if (StringUtils.isNotEmpty(startKey)) {
+    if (startKey != null) {
       reqBuilder.setStartKey(startKey);
     }
 
@@ -2257,16 +2256,9 @@ public List<OzoneFileStatus> listStatus(OmKeyArgs args, boolean recursive,
         .setSortDatanodes(args.getSortDatanodes())
         .setLatestVersionLocation(args.getLatestVersionLocation())
         .build();
-    ListStatusRequest.Builder listStatusRequestBuilder =
-        ListStatusRequest.newBuilder()
-            .setKeyArgs(keyArgs)
-            .setRecursive(recursive)
-            .setStartKey(startKey)
-            .setNumEntries(numEntries);
 
-    if (allowPartialPrefixes) {
-      listStatusRequestBuilder.setAllowPartialPrefix(allowPartialPrefixes);
-    }
+    ListStatusRequest.Builder listStatusRequestBuilder = createListStatusRequestBuilder(keyArgs, recursive, startKey,
+        numEntries, allowPartialPrefixes);
 
     OMRequest omRequest = createOMRequest(Type.ListStatus)
         .setListStatusRequest(listStatusRequestBuilder.build())
@@ -2293,16 +2285,9 @@ public List<OzoneFileStatusLight> listStatusLight(OmKeyArgs args,
         .setSortDatanodes(false)
         .setLatestVersionLocation(true)
         .build();
-    ListStatusRequest.Builder listStatusRequestBuilder =
-        ListStatusRequest.newBuilder()
-            .setKeyArgs(keyArgs)
-            .setRecursive(recursive)
-            .setStartKey(startKey)
-            .setNumEntries(numEntries);
 
-    if (allowPartialPrefixes) {
-      listStatusRequestBuilder.setAllowPartialPrefix(allowPartialPrefixes);
-    }
+    ListStatusRequest.Builder listStatusRequestBuilder = createListStatusRequestBuilder(keyArgs, recursive, startKey,
+        numEntries, allowPartialPrefixes);
 
     OMRequest omRequest = createOMRequest(Type.ListStatusLight)
         .setListStatusRequest(listStatusRequestBuilder.build())
@@ -2319,6 +2304,26 @@ public List<OzoneFileStatusLight> listStatusLight(OmKeyArgs args,
     return statusList;
   }
 
+  private ListStatusRequest.Builder createListStatusRequestBuilder(KeyArgs keyArgs, boolean recursive, String startKey,
+      long numEntries, boolean allowPartialPrefixes) {
+    ListStatusRequest.Builder listStatusRequestBuilder =
+        ListStatusRequest.newBuilder()
+            .setKeyArgs(keyArgs)
+            .setRecursive(recursive)
+            .setNumEntries(numEntries);
+
+    if (startKey != null) {
+      listStatusRequestBuilder.setStartKey(startKey);
+    } else {
+      listStatusRequestBuilder.setStartKey("");
+    }
+
+    if (allowPartialPrefixes) {
+      listStatusRequestBuilder.setAllowPartialPrefix(allowPartialPrefixes);
+    }
+    return listStatusRequestBuilder;
+  }
+
   @Override
   public List<OzoneFileStatus> listStatus(OmKeyArgs args, boolean recursive,
       String startKey, long numEntries) throws IOException {

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestListKeysWithFSO.java

@@ -63,6 +63,8 @@ public class TestListKeysWithFSO {
   private static OzoneBucket fsoOzoneBucket;
   private static OzoneBucket legacyOzoneBucket2;
   private static OzoneBucket fsoOzoneBucket2;
+  private static OzoneBucket emptyLegacyOzoneBucket;
+  private static OzoneBucket emptyFsoOzoneBucket;
   private static OzoneClient client;
 
   /**
@@ -105,6 +107,10 @@ public static void init() throws Exception {
     ozoneVolume.createBucket(fsoBucketName, omBucketArgs);
     fsoOzoneBucket2 = ozoneVolume.getBucket(fsoBucketName);
 
+    fsoBucketName = "bucket" + RandomStringUtils.randomNumeric(5);
+    ozoneVolume.createBucket(fsoBucketName, omBucketArgs);
+    emptyFsoOzoneBucket = ozoneVolume.getBucket(fsoBucketName);
+
     builder = BucketArgs.newBuilder();
     builder.setStorageType(StorageType.DISK);
     builder.setBucketLayout(BucketLayout.LEGACY);
@@ -113,6 +119,10 @@ public static void init() throws Exception {
     ozoneVolume.createBucket(legacyBucketName, omBucketArgs);
     legacyOzoneBucket2 = ozoneVolume.getBucket(legacyBucketName);
 
+    legacyBucketName = "bucket" + RandomStringUtils.randomNumeric(5);
+    ozoneVolume.createBucket(legacyBucketName, omBucketArgs);
+    emptyLegacyOzoneBucket = ozoneVolume.getBucket(legacyBucketName);
+
     initFSNameSpace();
   }
 
@@ -479,6 +489,23 @@ public void testShallowListKeys() throws Exception {
     expectedKeys =
         getExpectedKeyShallowList(keyPrefix, startKey, legacyOzoneBucket);
     checkKeyShallowList(keyPrefix, startKey, expectedKeys, fsoOzoneBucket);
+
+    // case-7: keyPrefix corresponds to multiple existing keys and
+    // startKey is null in empty bucket
+    keyPrefix = "a1/b1/c12";
+    startKey = null;
+    // a1/b1/c1222.tx
+    expectedKeys =
+        getExpectedKeyShallowList(keyPrefix, startKey, emptyLegacyOzoneBucket);
+    checkKeyShallowList(keyPrefix, startKey, expectedKeys, emptyFsoOzoneBucket);
+
+    // case-8: keyPrefix corresponds to multiple existing keys and
+    // startKey is null
+    keyPrefix = "a1/b1/c12";
+    // a1/b1/c1222.tx
+    expectedKeys =
+        getExpectedKeyShallowList(keyPrefix, startKey, legacyOzoneBucket);
+    checkKeyShallowList(keyPrefix, startKey, expectedKeys, fsoOzoneBucket);
   }
 
   /**

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestListStatus.java

@@ -16,10 +16,10 @@
  */
 package org.apache.hadoop.ozone.om;
 
-import org.apache.hadoop.hdds.utils.IOUtils;
 import org.apache.hadoop.hdds.client.RatisReplicationConfig;
 import org.apache.hadoop.hdds.conf.OzoneConfiguration;
 import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.hdds.utils.IOUtils;
 import org.apache.hadoop.ozone.MiniOzoneCluster;
 import org.apache.hadoop.ozone.TestDataUtil;
 import org.apache.hadoop.ozone.client.OzoneBucket;
@@ -29,36 +29,42 @@
 import org.apache.hadoop.ozone.om.helpers.OzoneFileStatus;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeAll;
-import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.Timeout;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
 import java.util.List;
+import java.util.stream.Stream;
+
+import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_FS_ITERATE_BATCH_SIZE;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.apache.hadoop.ozone.OzoneConfigKeys.
-    OZONE_FS_ITERATE_BATCH_SIZE;
+import static org.junit.jupiter.params.provider.Arguments.arguments;
 
 /**
  * A simple test that asserts that list status output is sorted.
  */
 @Timeout(1200)
 public class TestListStatus {
+  private static final Logger LOG = LoggerFactory.getLogger(TestListStatus.class);
 
   private static MiniOzoneCluster cluster = null;
-  private static OzoneConfiguration conf;
   private static OzoneBucket fsoOzoneBucket;
   private static OzoneClient client;
 
   /**
    * Create a MiniDFSCluster for testing.
    * <p>
    *
-   * @throws IOException
+   * @throws IOException in case of I/O error
    */
   @BeforeAll
   public static void init() throws Exception {
-    conf = new OzoneConfiguration();
+    OzoneConfiguration conf = new OzoneConfiguration();
     conf.setBoolean(OMConfigKeys.OZONE_OM_ENABLE_FILESYSTEM_PATHS,
         true);
     cluster = MiniOzoneCluster.newBuilder(conf).build();
@@ -69,7 +75,7 @@ public static void init() throws Exception {
     fsoOzoneBucket = TestDataUtil
         .createVolumeAndBucket(client, BucketLayout.FILE_SYSTEM_OPTIMIZED);
 
-    // Set the number of keys to be processed during batch operate.
+    // Set the number of keys to be processed during batch operated.
     conf.setInt(OZONE_FS_ITERATE_BATCH_SIZE, 5);
 
     buildNameSpaceTree(fsoOzoneBucket);
@@ -83,44 +89,30 @@ public static void teardownClass() {
     }
   }
 
-  @Test
-  public void testSortedListStatus() throws Exception {
-    // a) test if output is sorted
-    checkKeyList("", "", 1000, 10, false);
-
-    // b) number of keys returns is expected
-    checkKeyList("", "", 2, 2, false);
-
-    // c) check if full prefix works
-    checkKeyList("a1", "", 100, 3, false);
-
-    //  d) check if full prefix with numEntries work
-    checkKeyList("a1", "", 2, 2, false);
-
-    // e) check if existing start key >>>
-    checkKeyList("a1", "a1/a12", 100, 2, false);
-
-    // f) check with non-existing start key
-    checkKeyList("", "a7", 100, 6, false);
-
-    // g) check if half prefix works
-    checkKeyList("b", "", 100, 4, true);
-
-    // h) check half prefix with non-existing start key
-    checkKeyList("b", "b5", 100, 2, true);
-
-    // i) check half prefix with non-existing parent in start key
-    checkKeyList("b", "c", 100, 0, true);
-
-    // i) check half prefix with non-existing parent in start key
-    checkKeyList("b", "b/g5", 100, 4, true);
-
-    // i) check half prefix with non-existing parent in start key
-    checkKeyList("b", "c/g5", 100, 0, true);
+  @MethodSource("sortedListStatusParametersSource")
+  @ParameterizedTest(name = "{index} {5}")
+  public void testSortedListStatus(String keyPrefix, String startKey, int numEntries, int expectedNumKeys,
+                                   boolean isPartialPrefix, String testName) throws Exception {
+    checkKeyList(keyPrefix, startKey, numEntries, expectedNumKeys, isPartialPrefix);
+  }
 
-    // j) check prefix with non-existing prefix key
-    //    and non-existing parent in start key
-    checkKeyList("a1/a111", "a1/a111/a100", 100, 0, true);
+  private static Stream<Arguments> sortedListStatusParametersSource() {
+    return Stream.of(
+        arguments("", "", 1000, 10, false, "Test if output is sorted"),
+        arguments("", "", 2, 2, false, "Number of keys returns is expected"),
+        arguments("a1", "", 100, 3, false, "Check if the full prefix works"),
+        arguments("a1", "", 2, 2, false, "Check if full prefix with numEntries work"),
+        arguments("a1", "a1/a12", 100, 2, false, "Check if existing start key >>>"),
+        arguments("", "a7", 100, 6, false, "Check with a non-existing start key"),
+        arguments("b", "", 100, 4, true, "Check if half-prefix works"),
+        arguments("b", "b5", 100, 2, true, "Check half prefix with non-existing start key"),
+        arguments("b", "c", 100, 0, true, "Check half prefix with non-existing parent in a start key"),
+        arguments("b", "b/g5", 100, 4, true, "Check half prefix with non-existing parent in a start key"),
+        arguments("b", "c/g5", 100, 0, true, "Check half prefix with non-existing parent in a start key"),
+        arguments("a1/a111", "a1/a111/a100", 100, 0, true, "Check prefix with a non-existing prefix key\n" +
+            " and non-existing parent in a start key"),
+        arguments("a1/a111", null, 100, 0, true, "Check start key is null")
+    );
   }
 
   private static void createFile(OzoneBucket bucket, String keyName)
@@ -131,6 +123,7 @@ private static void createFile(OzoneBucket bucket, String keyName)
       oos.flush();
     }
   }
+
   private static void buildNameSpaceTree(OzoneBucket ozoneBucket)
       throws Exception {
     /*
@@ -172,33 +165,29 @@ private static void buildNameSpaceTree(OzoneBucket ozoneBucket)
     createFile(ozoneBucket, "/b8");
   }
 
-  private void checkKeyList(String keyPrefix, String startKey,
-                            long numEntries, int expectedNumKeys,
-                            boolean isPartialPrefix)
-      throws Exception {
+  private void checkKeyList(String keyPrefix, String startKey, long numEntries, int expectedNumKeys,
+                            boolean isPartialPrefix) throws Exception {
 
     List<OzoneFileStatus> statuses =
         fsoOzoneBucket.listStatus(keyPrefix, false, startKey,
             numEntries, isPartialPrefix);
     assertEquals(expectedNumKeys, statuses.size());
 
-    System.out.println("BEGIN:::keyPrefix---> " + keyPrefix + ":::---> " +
-        startKey);
+    LOG.info("BEGIN:::keyPrefix---> {} :::---> {}", keyPrefix, startKey);
 
     for (int i = 0; i < statuses.size() - 1; i++) {
       OzoneFileStatus stCurr = statuses.get(i);
       OzoneFileStatus stNext = statuses.get(i + 1);
 
-      System.out.println("status:"  + stCurr);
+      LOG.info("status: {}", stCurr);
       assertThat(stCurr.getPath().compareTo(stNext.getPath())).isLessThan(0);
     }
 
     if (!statuses.isEmpty()) {
       OzoneFileStatus stNext = statuses.get(statuses.size() - 1);
-      System.out.println("status:" + stNext);
+      LOG.info("status: {}", stNext);
     }
 
-    System.out.println("END:::keyPrefix---> " + keyPrefix + ":::---> " +
-        startKey);
+    LOG.info("END:::keyPrefix---> {}:::---> {}", keyPrefix, startKey);
   }
 }