HADOOP-16186. S3Guard: NPE in DynamoDBMetadataStore.lambda$listChildren.

Author:    Gabor Bota

hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/DynamoDBMetadataStore.java

@@ -638,22 +638,42 @@ public DirListingMetadata listChildren(final Path path) throws IOException {
             metas.add(meta);
           }
 
+          // Minor race condition here - if the path is deleted between
+          // getting the list of items and the directory metadata we might
+          // get a null in DDBPathMetadata.
           DDBPathMetadata dirPathMeta = get(path);
-          boolean isAuthoritative = false;
-          if(dirPathMeta != null) {
-            isAuthoritative = dirPathMeta.isAuthoritativeDir();
-          }
 
-          LOG.trace("Listing table {} in region {} for {} returning {}",
+          return getDirListingMetadataFromDirMetaAndList(path, metas,
-              tableName, region, path, metas);
+              dirPathMeta);
-
-          return (metas.isEmpty() && dirPathMeta == null)
-              ? null
-              : new DirListingMetadata(path, metas, isAuthoritative,
-              dirPathMeta.getLastUpdated());
         });
   }
 
+  DirListingMetadata getDirListingMetadataFromDirMetaAndList(Path path,
+      List<PathMetadata> metas, DDBPathMetadata dirPathMeta) {
+    boolean isAuthoritative = false;
+    if (dirPathMeta != null) {
+      isAuthoritative = dirPathMeta.isAuthoritativeDir();
+    }
+
+    LOG.trace("Listing table {} in region {} for {} returning {}",
+        tableName, region, path, metas);
+
+    if (!metas.isEmpty() && dirPathMeta == null) {
+      // We handle this case as the directory is deleted.
+      LOG.warn("Directory marker is deleted, but the list of the directory "
+          + "elements is not empty: {}. This case is handled as if the "
+          + "directory was deleted.", metas);
+      return null;
+    }
+
+    if(metas.isEmpty() && dirPathMeta == null) {
+      return null;
+    }
+
+    return new DirListingMetadata(path, metas, isAuthoritative,
+        dirPathMeta.getLastUpdated());
+  }
+
   /**
    * build the list of all parent entries.
    * @param pathsToCreate paths to create

hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestDynamoDBMiscOperations.java

@@ -20,6 +20,7 @@
 
 import java.io.FileNotFoundException;
 import java.io.IOException;
+import java.util.List;
 
 import com.amazonaws.services.dynamodbv2.model.ResourceNotFoundException;
 import com.amazonaws.waiters.WaiterTimedOutException;
@@ -27,6 +28,10 @@
 
 import org.apache.hadoop.fs.s3a.AWSClientIOException;
 import org.apache.hadoop.test.HadoopTestBase;
+import org.apache.hadoop.fs.Path;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 import static org.apache.hadoop.fs.s3a.s3guard.DynamoDBMetadataStore.translateTableWaitFailure;
 import static org.apache.hadoop.test.LambdaTestUtils.intercept;
@@ -91,4 +96,17 @@ public void testTranslateWrappedOtherException() throws Throwable {
     assertEquals(e, ex.getCause());
   }
 
+  @Test
+  public void testInnerListChildrenDirectoryNpe() throws Exception {
+    DynamoDBMetadataStore ddbms = new DynamoDBMetadataStore();
+    Path p = mock(Path.class);
+    List<PathMetadata> metas = mock(List.class);
+
+    when(metas.isEmpty()).thenReturn(false);
+    DDBPathMetadata dirPathMeta = null;
+
+    assertNull("The return value should be null.",
+        ddbms.getDirListingMetadataFromDirMetaAndList(p, metas, dirPathMeta));
+  }
+
 }