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.

Sign up for GitHub

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

Jump to bottom

fix: Requester pays to check reason and fallback to error message validation #841

Merged
merged 5 commits into from
Mar 4, 2022
Merged

fix: Requester pays to check reason and fallback to error message validation #841

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
776ee54
fix: Requester pays to check reason and fallback to error message val…
sydney-munro Mar 2, 2022
5149f01
Addressing PR comments
sydney-munro Mar 3, 2022
d4638ce
Adding Tests back for requester pays
sydney-munro Mar 4, 2022
ea0a6f4
Fix autoDetectNoUserProjectTest
sydney-munro Mar 4, 2022
8d596fa
Linter changes
sydney-munro Mar 4, 2022
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
5 changes: 4 additions & 1 deletion ...io/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -967,7 +967,10 @@ public boolean requesterPays(String bucketName) {
Boolean isRP = storage.get(bucketName).requesterPays();
return isRP != null && isRP.booleanValue();
} catch (StorageException ex) {
if (ex.getCode() == 400 && ex.getMessage().contains("Bucket is requester pays")) {
if (ex.getReason().equals("userProjectMissing")) {
return true;
// fallback to checking the error code and error message.
} else if (ex.getCode() == 400 && ex.getMessage().contains("requester pays")) {
return true;
}
throw ex;
Expand Down
27 changes: 9 additions & 18 deletions google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/it/ITGcsNio.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@
import org.junit.AfterClass;
import org.junit.Assert;
import org.junit.BeforeClass;
import org.junit.Ignore;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand Down Expand Up @@ -176,7 +175,6 @@ private static void fillRequesterPaysFile(Storage storage, String fname, int siz

// Start of tests related to the "requester pays" feature
@Test
@Ignore("TODO: https://github.com/googleapis/java-storage-nio/issues/824")
public void testFileExistsRequesterPaysNoUserProject() throws IOException {
CloudStorageFileSystem testBucket = getRequesterPaysBucket(false, "");
Path path = testBucket.getPath(SML_FILE);
Expand All @@ -190,7 +188,6 @@ public void testFileExistsRequesterPaysNoUserProject() throws IOException {
}

@Test
@Ignore("TODO: https://github.com/googleapis/java-storage-nio/issues/824")
public void testFileExistsRequesterPays() throws IOException {
CloudStorageFileSystem testBucket = getRequesterPaysBucket(false, project);
Path path = testBucket.getPath(SML_FILE);
Expand All @@ -199,7 +196,6 @@ public void testFileExistsRequesterPays() throws IOException {
}

@Test
@Ignore("TODO: https://github.com/googleapis/java-storage-nio/issues/824")
public void testFileExistsRequesterPaysWithAutodetect() throws IOException {
CloudStorageFileSystem testBucket = getRequesterPaysBucket(true, project);
Path path = testBucket.getPath(SML_FILE);
Expand All @@ -208,7 +204,6 @@ public void testFileExistsRequesterPaysWithAutodetect() throws IOException {
}

@Test
@Ignore("TODO: https://github.com/googleapis/java-storage-nio/issues/824")
public void testCantCreateWithoutUserProject() throws IOException {
CloudStorageFileSystem testBucket = getRequesterPaysBucket(false, "");
Path path = testBucket.getPath(TMP_FILE);
Expand All @@ -230,7 +225,6 @@ public void testCanCreateWithUserProject() throws IOException {
}

@Test
@Ignore("TODO: https://github.com/googleapis/java-storage-nio/issues/824")
public void testCantReadWithoutUserProject() throws IOException {
CloudStorageFileSystem testBucket = getRequesterPaysBucket(false, "");
Path path = testBucket.getPath(SML_FILE);
Expand All @@ -252,7 +246,6 @@ public void testCanReadWithUserProject() throws IOException {
}

@Test
@Ignore("TODO: https://github.com/googleapis/java-storage-nio/issues/824")
public void testCantCopyWithoutUserProject() throws IOException {
CloudStorageFileSystem testRPBucket = getRequesterPaysBucket(false, "");
CloudStorageFileSystem testBucket = getTestBucket();
Expand Down Expand Up @@ -285,10 +278,7 @@ private void innerTestCantCopyWithoutUserProject(
// normal StorageException.
} catch (HttpResponseException hex) {
Assert.assertEquals(description, hex.getStatusCode(), 400);
Assert.assertTrue(
description,
hex.getMessage()
.contains("Bucket is requester pays bucket but no user project provided"));
Assert.assertTrue(description, hex.getMessage().contains("requester pays"));
} catch (StorageException ex) {
assertIsRequesterPaysException(description, ex);
}
Expand All @@ -314,7 +304,6 @@ public void testCanCopyWithUserProject() throws IOException {
}

@Test
@Ignore("TODO: https://github.com/googleapis/java-storage-nio/issues/824")
public void testAutodetectWhenRequesterPays() throws IOException {
CloudStorageFileSystem testRPBucket = getRequesterPaysBucket(true, project);
Assert.assertEquals(
Expand All @@ -338,18 +327,20 @@ public void testAutodetectWhenNotRequesterPays() throws IOException {
"");
}

@Test
public void testAutoDetectNoUserProject() throws IOException {
CloudStorageFileSystem testBucket = getRequesterPaysBucket(false, "");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid dealing with this check, we set autodetect to false and then just check the provider value for requesterPays directly.

Assert.assertTrue(testBucket.provider().requesterPays(testBucket.bucket()));
}

private void assertIsRequesterPaysException(String message, StorageException ex) {
Assert.assertEquals(message, ex.getCode(), 400);
Assert.assertTrue(
message,
ex.getMessage().contains("Bucket is requester pays bucket but no user project provided"));
Assert.assertTrue(message, ex.getMessage().contains("requester pays"));
}

private void assertIsRequesterPaysException(String message, IOException ioex) {
Assert.assertTrue(message, ioex.getMessage().startsWith("400"));
Assert.assertTrue(
message,
ioex.getMessage().contains("Bucket is requester pays bucket but no user project provided"));
Assert.assertTrue(message, ioex.getMessage().contains("requester pays"));
}
// End of tests related to the "requester pays" feature

Expand Down