Merge pull request #1170 from tdrwenski/fix-mfileS3-exists

Fix MFileS3::exists to work for "directories"
Unidata · Apr 26, 2023 · 3524a0e · 3524a0e
2 parents c185740 + bf0a581
commit 3524a0e
Show file tree

Hide file tree

Showing 2 changed files with 104 additions and 6 deletions.
diff --git a/cdm/s3/src/main/java/thredds/inventory/s3/MFileS3.java b/cdm/s3/src/main/java/thredds/inventory/s3/MFileS3.java
@@ -103,6 +103,19 @@ private HeadObjectResponse getHeadObjectResponse() {
     return response;
   }
 
+  @Nullable
+  private HeadBucketResponse getHeadBucketResponse() {
+    HeadBucketResponse response = null;
+    S3Client client = getClient();
+
+    if (client != null) {
+      HeadBucketRequest headBucketRequest = HeadBucketRequest.builder().bucket(cdmS3Uri.getBucket()).build();
+      response = client.headBucket(headBucketRequest);
+    }
+
+    return response;
+  }
+
   @Nullable
   private S3Client getClient() {
     S3Client client = null;
@@ -285,13 +298,44 @@ public boolean exists() {
     return exists;
   }
 
-  // Update file exists by fetching from a head request
   private void updateExists() {
+    if (isDirectory()) {
+      exists = directoryExists();
+    } else {
+      exists = key == null ? bucketExists() : objectExists();
+    }
+  }
+
+  private boolean directoryExists() {
+    final S3Client client = getClient();
+    if (client == null) {
+      return false;
+    }
+
+    try {
+      final ListObjectsV2Response listObjects =
+          client.listObjectsV2(ListObjectsV2Request.builder().bucket(cdmS3Uri.getBucket()).prefix(key).build());
+      return listObjects.sdkHttpResponse().isSuccessful() && !listObjects.contents().isEmpty();
+    } catch (NoSuchBucketException e) {
+      return false;
+    }
+  }
+
+  private boolean bucketExists() {
     try {
-      headObjectResponse.get();
-      exists = true;
+      final HeadBucketResponse response = getHeadBucketResponse();
+      return response != null && response.sdkHttpResponse().isSuccessful();
+    } catch (NoSuchBucketException e) {
+      return false;
+    }
+  }
+
+  private boolean objectExists() {
+    try {
+      final HeadObjectResponse response = headObjectResponse.get();
+      return response != null && response.sdkHttpResponse().isSuccessful();
     } catch (NoSuchKeyException e) {
-      exists = false;
+      return false;
     }
   }
 

diff --git a/cdm/s3/src/test/java/thredds/inventory/s3/TestMFileS3.java b/cdm/s3/src/test/java/thredds/inventory/s3/TestMFileS3.java
@@ -229,7 +229,7 @@ public void shouldNotWriteDirectoryToStream() throws IOException {
 
   @Test
   public void shouldNotWriteNonExistingObjectToStream() throws IOException {
-    final MFile mFile = new MFileS3(AWS_G16_S3_URI_DIR + "?NotARealKey");
+    final MFile mFile = new MFileS3(AWS_G16_S3_URI_DIR + "/NotARealKey");
 
     final ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
     assertThrows(NoSuchKeyException.class, () -> mFile.writeToStream(outputStream));
@@ -243,10 +243,64 @@ public void shouldReturnTrueForExistingFile() throws IOException {
 
   @Test
   public void shouldReturnFalseForNonExistingFile() throws IOException {
-    final MFile mFile = new MFileS3(AWS_G16_S3_URI_DIR + "?NotARealKey");
+    final MFile mFile = new MFileS3(AWS_G16_S3_URI_DIR + "/NotARealKey");
     assertThat(mFile.exists()).isEqualTo(false);
   }
 
+  @Test
+  public void shouldCheckExistsForExistingDirectory() throws IOException {
+    final MFile withFragmentWithSlash = new MFileS3(AWS_G16_S3_URI_DIR + "/" + DELIMITER_FRAGMENT);
+    assertThat(withFragmentWithSlash.exists()).isEqualTo(true);
+
+    final MFile withoutFragmentWithSlash = new MFileS3(AWS_G16_S3_URI_DIR + "/");
+    assertThat(withoutFragmentWithSlash.exists()).isEqualTo(false);
+
+    final MFile withFragmentWithoutSlash = new MFileS3(AWS_G16_S3_URI_DIR + DELIMITER_FRAGMENT);
+    assertThat(withFragmentWithoutSlash.exists()).isEqualTo(false);
+
+    final MFile withoutFragmentWithoutSlash = new MFileS3(AWS_G16_S3_URI_DIR);
+    assertThat(withoutFragmentWithoutSlash.exists()).isEqualTo(false);
+  }
+
+  @Test
+  public void shouldReturnFalseForNonExistingDirectory() throws IOException {
+    final MFile withFragmentWithSlash = new MFileS3(AWS_G16_S3_URI_DIR + "/notADirectory/" + DELIMITER_FRAGMENT);
+    assertThat(withFragmentWithSlash.exists()).isEqualTo(false);
+
+    final MFile withoutFragmentWithSlash = new MFileS3(AWS_G16_S3_URI_DIR + "/notADirectory/");
+    assertThat(withoutFragmentWithSlash.exists()).isEqualTo(false);
+
+    final MFile withFragmentWithoutSlash = new MFileS3(AWS_G16_S3_URI_DIR + "/notADirectory" + DELIMITER_FRAGMENT);
+    assertThat(withFragmentWithoutSlash.exists()).isEqualTo(false);
+
+    final MFile withoutFragmentWithoutSlash = new MFileS3(AWS_G16_S3_URI_DIR + "/notADirectory");
+    assertThat(withoutFragmentWithoutSlash.exists()).isEqualTo(false);
+  }
+
+  @Test
+  public void shouldReturnFalseForKeyPrefixMatch() throws IOException {
+    final MFile mFile = new MFileS3(AWS_G16_S3_OBJECT_1.substring(0, AWS_G16_S3_OBJECT_1.length() - 5));
+    assertThat(mFile.exists()).isEqualTo(false);
+  }
+
+  @Test
+  public void shouldReturnTrueForBucket() throws IOException {
+    final MFile bucketWithDelimiter = new MFileS3(S3TestsCommon.TOP_LEVEL_AWS_BUCKET + DELIMITER_FRAGMENT);
+    assertThat(bucketWithDelimiter.exists()).isEqualTo(true);
+
+    final MFile bucketWithoutDelimiter = new MFileS3(S3TestsCommon.TOP_LEVEL_AWS_BUCKET);
+    assertThat(bucketWithoutDelimiter.exists()).isEqualTo(true);
+  }
+
+  @Test
+  public void shouldReturnFalseForNonExistentBucket() throws IOException {
+    final MFile bucketWithDelimiter = new MFileS3("cdms3:notABucket" + DELIMITER_FRAGMENT);
+    assertThat(bucketWithDelimiter.exists()).isEqualTo(false);
+
+    final MFile bucketWithoutDelimiter = new MFileS3("cdms3:notABucket");
+    assertThat(bucketWithoutDelimiter.exists()).isEqualTo(false);
+  }
+
   @Test
   public void shouldGetInputStream() throws IOException {
     final MFile mFile = new MFileS3(AWS_G16_S3_OBJECT_1);