Skip to content

Commit

Permalink
fix: rely on google core for SSLException's (#188)
Browse files Browse the repository at this point in the history 
* fix: rely on google core for SSLException's

* update requester pays tests to not rely on Project Owner role

* format

* make sure requester pays is cleaned up
  • Loading branch information
@frankyn
frankyn authored Mar 17, 2020
1 parent 3795825 commit 2581f3c
Show file tree
Hide file tree
Showing 5 changed files with 80 additions and 23 deletions.
8 changes: 2 additions & 6 deletions google-cloud-storage/src/main/java/com/google/cloud/storage/StorageException.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
public final class StorageException extends BaseHttpServiceException {
private static final String INTERNAL_ERROR = "internalError";
private static final String CONNECTION_CLOSED_PREMATURELY = "connectionClosedPrematurely";
private static final String CONNECTION_RESET = "connectionReset";

// see: https://cloud.google.com/storage/docs/resumable-uploads-xml#practices
private static final Set<Error> RETRYABLE_ERRORS =
Expand All @@ -47,8 +46,7 @@ public final class StorageException extends BaseHttpServiceException {
new Error(429, null),
new Error(408, null),
new Error(null, INTERNAL_ERROR),
new Error(null, CONNECTION_CLOSED_PREMATURELY),
new Error(null, CONNECTION_RESET));
new Error(null, CONNECTION_CLOSED_PREMATURELY));

private static final long serialVersionUID = -4168430271327813063L;

Expand Down Expand Up @@ -86,16 +84,14 @@ public static StorageException translateAndThrow(RetryHelperException ex) {
/**
* Translate IOException to a StorageException representing the cause of the error. This method
* defaults to idempotent always being {@code true}. Additionally, this method translates
* transient issues Connection Closed Prematurely and Connection Reset as retryable errors.
* transient issues Connection Closed Prematurely as a retryable error.
*
* @returns {@code StorageException}
*/
public static StorageException translate(IOException exception) {
if (exception.getMessage().contains("Connection closed prematurely")) {
return new StorageException(
0, exception.getMessage(), CONNECTION_CLOSED_PREMATURELY, exception);
} else if (exception.getMessage().contains("Connection reset")) {
return new StorageException(0, exception.getMessage(), CONNECTION_RESET, exception);
} else {
// default
return new StorageException(exception);
Expand Down
11 changes: 6 additions & 5 deletions google-cloud-storage/src/main/java/com/google/cloud/storage/testing/RemoteStorageHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,9 @@ public static void cleanBuckets(final Storage storage, final long olderThan, lon
@Override
public void run() {
Page<Bucket> buckets =
storage.list(Storage.BucketListOption.prefix(BUCKET_NAME_PREFIX));
storage.list(
Storage.BucketListOption.prefix(BUCKET_NAME_PREFIX),
Storage.BucketListOption.userProject(storage.getOptions().getProjectId()));
for (Bucket bucket : buckets.iterateAll()) {
if (bucket.getCreateTime() < olderThan) {
try {
Expand All @@ -88,10 +90,9 @@ public void run() {
.iterateAll()) {
if (blob.getEventBasedHold() == true || blob.getTemporaryHold() == true) {
storage.update(
blob.toBuilder()
.setTemporaryHold(false)
.setEventBasedHold(false)
.build());
blob.toBuilder().setTemporaryHold(false).setEventBasedHold(false).build(),
Storage.BlobTargetOption.userProject(
storage.getOptions().getProjectId()));
}
}
forceDelete(storage, bucket.getName());
Expand Down
16 changes: 14 additions & 2 deletions google-cloud-storage/src/test/java/com/google/cloud/storage/StorageExceptionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,11 +141,23 @@ public void testStorageException() {
public void testTranslateConnectionReset() {
StorageException exception =
StorageException.translate(
new IOException(
new SSLException(
"Connection has been shutdown: "
+ new SSLException(new SocketException("Connection reset"))));
assertEquals(0, exception.getCode());
assertEquals("connectionReset", exception.getReason());
assertTrue(exception.isRetryable());
}

@Test
public void testTranslateConnectionShutdown() {
StorageException exception =
StorageException.translate(
new SSLException(
"Connection has been shutdown: "
+ new SSLException(new SocketException("Socket closed"))));
String test = exception.getMessage();

assertEquals(0, exception.getCode());
assertTrue(exception.isRetryable());
}

Expand Down
66 changes: 57 additions & 9 deletions google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITStorageTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@
import org.apache.http.impl.client.HttpClients;
import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
import org.junit.AfterClass;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Test;

Expand Down Expand Up @@ -192,6 +193,23 @@ public static void beforeClass() throws IOException {
prepareKmsKeys();
}

@Before
public void beforeEach() {
Bucket remoteBucket =
storage.get(
BUCKET,
Storage.BucketGetOption.fields(BucketField.ID, BucketField.BILLING),
Storage.BucketGetOption.userProject(storage.getOptions().getProjectId()));
// Disable requester pays in case a test fails to clean up.
if (remoteBucket.requesterPays() != null && remoteBucket.requesterPays() == true) {
remoteBucket
.toBuilder()
.setRequesterPays(false)
.build()
.update(Storage.BucketTargetOption.userProject(storage.getOptions().getProjectId()));
}
}

@AfterClass
public static void afterClass() throws ExecutionException, InterruptedException {
if (storage != null) {
Expand Down Expand Up @@ -867,8 +885,9 @@ public void testListBlobRequesterPays() throws InterruptedException {
assertNotNull(storage.create(blob1));

// Test listing a Requester Pays bucket.
Bucket remoteBucket = storage.get(BUCKET, Storage.BucketGetOption.fields(BucketField.ID));
assertNull(remoteBucket.requesterPays());
Bucket remoteBucket =
storage.get(BUCKET, Storage.BucketGetOption.fields(BucketField.ID, BucketField.BILLING));
assertFalse(remoteBucket.requesterPays());
remoteBucket = remoteBucket.toBuilder().setRequesterPays(true).build();
Bucket updatedBucket = storage.update(remoteBucket);
assertTrue(updatedBucket.requesterPays());
Expand Down Expand Up @@ -2138,7 +2157,8 @@ public void testBucketAcl() {

private void testBucketAclRequesterPays(boolean requesterPays) {
if (requesterPays) {
Bucket remoteBucket = storage.get(BUCKET, Storage.BucketGetOption.fields(BucketField.ID));
Bucket remoteBucket =
storage.get(BUCKET, Storage.BucketGetOption.fields(BucketField.ID, BucketField.BILLING));
assertNull(remoteBucket.requesterPays());
remoteBucket = remoteBucket.toBuilder().setRequesterPays(true).build();
Bucket updatedBucket = storage.update(remoteBucket);
Expand All @@ -2164,6 +2184,18 @@ private void testBucketAclRequesterPays(boolean requesterPays) {
assertTrue(acls.contains(updatedAcl));
assertTrue(storage.deleteAcl(BUCKET, User.ofAllAuthenticatedUsers(), bucketOptions));
assertNull(storage.getAcl(BUCKET, User.ofAllAuthenticatedUsers(), bucketOptions));
if (requesterPays) {
Bucket remoteBucket =
storage.get(
BUCKET,
Storage.BucketGetOption.fields(BucketField.ID, BucketField.BILLING),
Storage.BucketGetOption.userProject(projectId));
assertTrue(remoteBucket.requesterPays());
remoteBucket = remoteBucket.toBuilder().setRequesterPays(false).build();
Bucket updatedBucket =
storage.update(remoteBucket, Storage.BucketTargetOption.userProject(projectId));
assertFalse(updatedBucket.requesterPays());
}
}

@Test
Expand Down Expand Up @@ -2347,8 +2379,9 @@ public void testReadCompressedBlob() throws IOException {

@Test
public void testBucketPolicyV1RequesterPays() {
Bucket remoteBucket = storage.get(BUCKET, Storage.BucketGetOption.fields(BucketField.ID));
assertNull(remoteBucket.requesterPays());
Bucket remoteBucket =
storage.get(BUCKET, Storage.BucketGetOption.fields(BucketField.ID, BucketField.BILLING));
assertFalse(remoteBucket.requesterPays());
remoteBucket = remoteBucket.toBuilder().setRequesterPays(true).build();
Bucket updatedBucket = storage.update(remoteBucket);
assertTrue(updatedBucket.requesterPays());
Expand Down Expand Up @@ -2407,6 +2440,9 @@ public void testBucketPolicyV1RequesterPays() {
BUCKET,
ImmutableList.of("storage.buckets.getIamPolicy", "storage.buckets.setIamPolicy"),
bucketOptions));
remoteBucket = remoteBucket.toBuilder().setRequesterPays(false).build();
updatedBucket = storage.update(remoteBucket, Storage.BucketTargetOption.userProject(projectId));
assertFalse(updatedBucket.requesterPays());
}

@Test
Expand Down Expand Up @@ -2624,7 +2660,8 @@ public void testBucketPolicyV3() {

@Test
public void testUpdateBucketLabel() {
Bucket remoteBucket = storage.get(BUCKET, Storage.BucketGetOption.fields(BucketField.ID));
Bucket remoteBucket =
storage.get(BUCKET, Storage.BucketGetOption.fields(BucketField.ID, BucketField.BILLING));
assertNull(remoteBucket.getLabels());
remoteBucket = remoteBucket.toBuilder().setLabels(BUCKET_LABELS).build();
Bucket updatedBucket = storage.update(remoteBucket);
Expand All @@ -2635,8 +2672,9 @@ public void testUpdateBucketLabel() {

@Test
public void testUpdateBucketRequesterPays() {
Bucket remoteBucket = storage.get(BUCKET, Storage.BucketGetOption.fields(BucketField.ID));
assertNull(remoteBucket.requesterPays());
Bucket remoteBucket =
storage.get(BUCKET, Storage.BucketGetOption.fields(BucketField.ID, BucketField.BILLING));
assertFalse(remoteBucket.requesterPays());
remoteBucket = remoteBucket.toBuilder().setRequesterPays(true).build();
Bucket updatedBucket = storage.update(remoteBucket);
assertTrue(updatedBucket.requesterPays());
Expand All @@ -2646,8 +2684,12 @@ public void testUpdateBucketRequesterPays() {
String blobName = "test-create-empty-blob-requester-pays";
Blob remoteBlob = updatedBucket.create(blobName, BLOB_BYTE_CONTENT, option);
assertNotNull(remoteBlob);
byte[] readBytes = storage.readAllBytes(BUCKET, blobName);
byte[] readBytes =
storage.readAllBytes(BUCKET, blobName, Storage.BlobSourceOption.userProject(projectId));
assertArrayEquals(BLOB_BYTE_CONTENT, readBytes);
remoteBucket = remoteBucket.toBuilder().setRequesterPays(false).build();
updatedBucket = storage.update(remoteBucket, Storage.BucketTargetOption.userProject(projectId));
assertFalse(updatedBucket.requesterPays());
}

@Test
Expand Down Expand Up @@ -2786,6 +2828,12 @@ private void retentionPolicyLockRequesterPays(boolean requesterPays)
assertTrue(remoteBucket.retentionPolicyIsLocked());
assertNotNull(remoteBucket.getRetentionEffectiveTime());
} finally {
if (requesterPays) {
bucketInfo = bucketInfo.toBuilder().setRequesterPays(false).build();
Bucket updateBucket =
storage.update(bucketInfo, Storage.BucketTargetOption.userProject(projectId));
assertFalse(updateBucket.requesterPays());
}
RemoteStorageHelper.forceDelete(storage, bucketName, 5, TimeUnit.SECONDS);
}
}
Expand Down
2 changes: 1 addition & 1 deletion pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
<project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
<github.global.server>github</github.global.server>
<site.installationModule>google-cloud-storage-parent</site.installationModule>
<google.core.version>1.93.2</google.core.version>
<google.core.version>1.93.3</google.core.version>
<google.api-common.version>1.8.1</google.api-common.version>
<junit.version>4.13</junit.version>
<threeten.version>1.4.1</threeten.version>
Expand Down

0 comments on commit 2581f3c

Please sign in to comment.