Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect when the snapshot repo bucket cannot be found and return a clear error message #929

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion RFS/src/main/java/com/rfs/common/S3Repo.java
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ protected S3Uri findRepoFileUri() {
.max(Comparator.comparingInt(s3Object -> extractVersion(s3Object.key())));

String rawUri = highestVersionedIndexFile.map(s3Object -> "s3://" + s3RepoUri.bucketName + "/" + s3Object.key())
.orElse("");
.orElseThrow(() -> new CannotFindSnapshotRepoRoot(s3RepoUri.bucketName, s3RepoUri.key));
return new S3Uri(rawUri);
}

Expand Down Expand Up @@ -215,6 +215,12 @@ public void prepBlobFiles(ShardMetadata shardMetadata) {
completedDirectoryDownload.failedTransfers().forEach(logger::error);
}

public static class CannotFindSnapshotRepoRoot extends RfsException {
public CannotFindSnapshotRepoRoot(String bucket, String prefix) {
super("Cannot find the snapshot repository root in S3 bucket: " + bucket + ", prefix: " + prefix);
}
}

public static class CantCreateS3LocalDir extends RfsException {
public CantCreateS3LocalDir(Path localPath, Throwable cause) {
super("Failed to create the S3 local download directory: " + localPath, cause);
Expand Down
28 changes: 28 additions & 0 deletions RFS/src/test/java/com/rfs/common/S3RepoTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,28 @@
import java.io.IOException;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.List;
import java.util.concurrent.CompletableFuture;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;

import com.rfs.common.S3Repo.CannotFindSnapshotRepoRoot;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.junit.jupiter.MockitoExtension;
import software.amazon.awssdk.core.async.AsyncResponseTransformer;
import software.amazon.awssdk.services.s3.S3AsyncClient;
import software.amazon.awssdk.services.s3.model.GetObjectRequest;
import software.amazon.awssdk.services.s3.model.GetObjectResponse;
import software.amazon.awssdk.services.s3.model.ListObjectsV2Request;
import software.amazon.awssdk.services.s3.model.ListObjectsV2Response;

import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.*;
Expand Down Expand Up @@ -96,6 +103,27 @@ void GetSnapshotRepoDataFilePath_AsExpected() throws IOException {
verify(mockS3Client).getObject(eq(expectedRequest), any(AsyncResponseTransformer.class));
}


@Test
void GetSnapshotRepoDataFilePath_DoesNotExist() throws IOException {
// Set up the test
var response = mock(ListObjectsV2Response.class);
when(response.contents())
.thenReturn(List.of());
when(mockS3Client.listObjectsV2(any(ListObjectsV2Request.class)))
.thenReturn(CompletableFuture.supplyAsync(() -> response));

var nonExistentFileName = "does-not-exist";
var bucket = new S3Uri("s3://bucket-name/directory" + nonExistentFileName);
var testRepo = Mockito.spy(new S3Repo(testDir, bucket, testRegion, mockS3Client));

// Run the test
var thrown = assertThrows(CannotFindSnapshotRepoRoot.class, () -> testRepo.getSnapshotRepoDataFilePath());

// Check the results
assertThat(thrown.getMessage(), containsString(nonExistentFileName));
}

@Test
void GetGlobalMetadataFilePath_AsExpected() throws IOException {
// Set up the test
Expand Down