From 019463952f407ef2b007555801a5bfb3f9da8eeb Mon Sep 17 00:00:00 2001 From: BenWhitehead Date: Fri, 21 Oct 2022 13:32:07 -0400 Subject: [PATCH] chore: refactor and cleanup translation between gRPC status codes and http codes (#1713) Move all mapping from GrpcStorageImpl and BackwardCompatibilityUtils to GrpcToHttpStatusCodeTranslation. Define new GrpcToHttpStatusCodeTranslation.StatusCodeMapping to explicitly define a mapping between io.grpc.Status.Code and our http code Update tests to be explicit and not depend on utility methods which can also be used by the code under test. Add new tests to ensure expected status codes are returned when evaluating DefaultStorageRetryStrategy --- .../storage/BackwardCompatibilityUtils.java | 65 -------- .../google/cloud/storage/GrpcStorageImpl.java | 102 +----------- .../GrpcToHttpStatusCodeTranslation.java | 156 ++++++++++++++++++ .../cloud/storage/StorageException.java | 3 +- .../BackwardCompatibilityUtilsTest.java | 31 ---- .../GrpcToHttpStatusCodeTranslationTest.java | 86 ++++++++++ ...ResultRetryAlgorithmCompatibilityTest.java | 3 +- ...StorageExceptionGrpcCompatibilityTest.java | 147 +++++++++++------ 8 files changed, 350 insertions(+), 243 deletions(-) create mode 100644 google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcToHttpStatusCodeTranslation.java create mode 100644 google-cloud-storage/src/test/java/com/google/cloud/storage/GrpcToHttpStatusCodeTranslationTest.java diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/BackwardCompatibilityUtils.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/BackwardCompatibilityUtils.java index 6dc5575d5..33f317562 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/BackwardCompatibilityUtils.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/BackwardCompatibilityUtils.java @@ -18,8 +18,6 @@ import static java.util.Objects.requireNonNull; -import com.google.api.gax.grpc.GrpcStatusCode; -import com.google.api.gax.rpc.StatusCode; import com.google.api.services.storage.model.Bucket.Lifecycle.Rule; import com.google.cloud.storage.BucketInfo.AgeDeleteRule; import com.google.cloud.storage.BucketInfo.CreatedBeforeDeleteRule; @@ -31,8 +29,6 @@ import com.google.cloud.storage.BucketInfo.NumNewerVersionsDeleteRule; import com.google.cloud.storage.BucketInfo.RawDeleteRule; import com.google.cloud.storage.Conversions.Codec; -import com.google.common.annotations.VisibleForTesting; -import io.grpc.Status.Code; import java.time.Duration; import java.time.Instant; import java.time.OffsetDateTime; @@ -78,67 +74,6 @@ final class BackwardCompatibilityUtils { private BackwardCompatibilityUtils() {} - /** - * When translating from gRPC Status Codes to the HTTP codes all of our middle ware expects, we - * must take care to translate in accordance with the expected retry semantics already outlined - * and validated for the JSON implementation. This is why we do not simply use {@link - * GrpcStatusCode#of(Code)}{@link GrpcStatusCode#getCode() .getCode}{@link - * StatusCode.Code#getHttpStatusCode() .getHttpStatusCode()} as it sometimes returns conflicting - * HTTP codes for our retry handling. - */ - @VisibleForTesting - static int grpcCodeToHttpStatusCode(Code code) { - switch (code) { - // 200 Ok - case OK: - return 200; - // 400 Bad Request - case DATA_LOSS: - case INVALID_ARGUMENT: - case OUT_OF_RANGE: - return 400; - // 401 Unauthorized - case UNAUTHENTICATED: - return 401; - // 403 Forbidden - case PERMISSION_DENIED: - return 403; - // 404 Not Found - case NOT_FOUND: - return 404; - // 408 Request Timeout - // TODO - // 412 Precondition Failed - case FAILED_PRECONDITION: - return 412; - // 409 Conflict - case ALREADY_EXISTS: - return 409; - // 429 Too Many Requests - case RESOURCE_EXHAUSTED: - return 429; - // 500 Internal Server Error - case INTERNAL: - return 500; - // 501 Not Implemented - case UNIMPLEMENTED: - return 501; - // 503 Service Unavailable - case UNAVAILABLE: - return 503; - // 504 Gateway Timeout - case DEADLINE_EXCEEDED: - return 504; - // TODO - - case ABORTED: // ? - case CANCELLED: // ? - case UNKNOWN: // ? - default: - return 0; - } - } - @SuppressWarnings("deprecation") private static LifecycleRule deleteRuleEncode(DeleteRule from) { if (from instanceof RawDeleteRule) { diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java index 0855a0ced..e947e4da7 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java @@ -18,6 +18,7 @@ import static com.google.cloud.storage.ByteSizeConstants._16MiB; import static com.google.cloud.storage.ByteSizeConstants._256KiB; +import static com.google.cloud.storage.GrpcToHttpStatusCodeTranslation.resultRetryAlgorithmToCodes; import static com.google.cloud.storage.Utils.bucketNameCodec; import static com.google.cloud.storage.Utils.ifNonNull; import static com.google.cloud.storage.Utils.projectNameCodec; @@ -32,7 +33,6 @@ import com.google.api.gax.paging.Page; import com.google.api.gax.retrying.ResultRetryAlgorithm; import com.google.api.gax.rpc.ApiException; -import com.google.api.gax.rpc.ApiExceptionFactory; import com.google.api.gax.rpc.ApiExceptions; import com.google.api.gax.rpc.NotFoundException; import com.google.api.gax.rpc.StatusCode; @@ -147,20 +147,6 @@ final class GrpcStorageImpl extends BaseService implements Stora StandardOpenOption.WRITE, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING); - /** - * For use in {@link #resultRetryAlgorithmToCodes(ResultRetryAlgorithm)}. Resolve all codes and - * construct corresponding ApiExceptions. - * - *

Constructing the exceptions will walk the stack for each one. In order to avoid the stack - * walking overhead for every Code for every invocation of read, construct the set of exceptions - * only once and keep in this value. - */ - private static final Set CODE_API_EXCEPTIONS = - Arrays.stream(StatusCode.Code.values()) - .map(GrpcStorageImpl::statusCodeFor) - .map(c -> ApiExceptionFactory.createException(null, c, false)) - .map(StorageException::asStorageException) - .collect(Collectors.toSet()); final StorageClient storageClient; final GrpcConversions codecs; @@ -696,8 +682,7 @@ public GrpcBlobReadChannel reader(String bucket, String blob, BlobSourceOption.. public GrpcBlobReadChannel reader(BlobId blob, BlobSourceOption... options) { Opts opts = Opts.unwrap(options).resolveFrom(blob); ReadObjectRequest request = getReadObjectRequest(blob, opts); - Set codes = - GrpcStorageImpl.resultRetryAlgorithmToCodes(retryAlgorithmManager.getFor(request)); + Set codes = resultRetryAlgorithmToCodes(retryAlgorithmManager.getFor(request)); GrpcCallContext grpcCallContext = GrpcCallContext.createDefault().withRetryableCodes(codes); return new GrpcBlobReadChannel( storageClient.readObjectCallable().withDefaultCallContext(grpcCallContext), @@ -1318,8 +1303,7 @@ static T throwNotYetImplemented(String methodName) { String.format( "%s#%s is not yet implemented for GRPC transport. Please use StorageOptions.http() to construct a compatible instance in the interim.", Storage.class.getName(), methodName); - throw new UnimplementedException( - message, null, statusCodeFor(StatusCode.Code.UNIMPLEMENTED), false); + throw new UnimplementedException(message, null, GrpcStatusCode.of(Code.UNIMPLEMENTED), false); } private static String fmtMethodName(String name, Class... args) { @@ -1368,8 +1352,7 @@ private UnbufferedReadableByteChannelSession unbufferedReadSession( Opts opts = Opts.unwrap(options).resolveFrom(blob); ReadObjectRequest readObjectRequest = getReadObjectRequest(blob, opts); Set codes = - GrpcStorageImpl.resultRetryAlgorithmToCodes( - retryAlgorithmManager.getFor(readObjectRequest)); + resultRetryAlgorithmToCodes(retryAlgorithmManager.getFor(readObjectRequest)); GrpcCallContext grpcCallContext = GrpcCallContext.createDefault().withRetryableCodes(codes); return ResumableMedia.gapic() .read() @@ -1383,8 +1366,7 @@ private UnbufferedReadableByteChannelSession unbufferedReadSession( @VisibleForTesting ApiFuture startResumableWrite( GrpcCallContext grpcCallContext, WriteObjectRequest req) { - Set codes = - GrpcStorageImpl.resultRetryAlgorithmToCodes(retryAlgorithmManager.getFor(req)); + Set codes = resultRetryAlgorithmToCodes(retryAlgorithmManager.getFor(req)); return ResumableMedia.gapic() .write() .resumableWrite( @@ -1394,80 +1376,6 @@ ApiFuture startResumableWrite( req); } - /** - * When using the retry features of the Gapic client, we are only allowed to provide a {@link - * Set}{@code <}{@link StatusCode.Code}{@code >}. Given {@link StatusCode.Code} is an enum, we can - * resolve the set of values from a given {@link ResultRetryAlgorithm} by evaluating each one as - * an {@link ApiException}. - */ - static Set resultRetryAlgorithmToCodes(ResultRetryAlgorithm alg) { - return CODE_API_EXCEPTIONS.stream() - .filter(e -> alg.shouldRetry(e, null)) - .map(e -> e.apiExceptionCause.getStatusCode().getCode()) - .collect(Collectors.toSet()); - } - - private static GrpcStatusCode statusCodeFor(StatusCode.Code code) { - switch (code) { - case OK: - return GrpcStatusCode.of(Code.OK); - case CANCELLED: - return GrpcStatusCode.of(Code.CANCELLED); - case UNKNOWN: - return GrpcStatusCode.of(Code.UNKNOWN); - case INVALID_ARGUMENT: - return GrpcStatusCode.of(Code.INVALID_ARGUMENT); - case DEADLINE_EXCEEDED: - return GrpcStatusCode.of(Code.DEADLINE_EXCEEDED); - case NOT_FOUND: - return GrpcStatusCode.of(Code.NOT_FOUND); - case ALREADY_EXISTS: - return GrpcStatusCode.of(Code.ALREADY_EXISTS); - case PERMISSION_DENIED: - return GrpcStatusCode.of(Code.PERMISSION_DENIED); - case RESOURCE_EXHAUSTED: - return GrpcStatusCode.of(Code.RESOURCE_EXHAUSTED); - case FAILED_PRECONDITION: - return GrpcStatusCode.of(Code.FAILED_PRECONDITION); - case ABORTED: - return GrpcStatusCode.of(Code.ABORTED); - case OUT_OF_RANGE: - return GrpcStatusCode.of(Code.OUT_OF_RANGE); - case UNIMPLEMENTED: - return GrpcStatusCode.of(Code.UNIMPLEMENTED); - case INTERNAL: - return GrpcStatusCode.of(Code.INTERNAL); - case UNAVAILABLE: - return GrpcStatusCode.of(Code.UNAVAILABLE); - case DATA_LOSS: - return GrpcStatusCode.of(Code.DATA_LOSS); - case UNAUTHENTICATED: - return GrpcStatusCode.of(Code.UNAUTHENTICATED); - default: - throw new IllegalStateException("Unrecognized status code: " + code); - } - } - - private static ImmutableList updateFields() { - return ImmutableList.of( - "cors", - "default_event_based_hold", - "retention_policy", - "versioning", - "billing", - "iam_config", - "encryption", - "lifecycle", - "logging", - "website", - "acl", - "default_object_acl", - "storage_class", - "rpo", - "labels", - "event_based_hold"); - } - private SourceObject sourceObjectEncode(SourceBlob from) { SourceObject.Builder to = SourceObject.newBuilder(); to.setName(from.getName()); diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcToHttpStatusCodeTranslation.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcToHttpStatusCodeTranslation.java new file mode 100644 index 000000000..feda6e70f --- /dev/null +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcToHttpStatusCodeTranslation.java @@ -0,0 +1,156 @@ +/* + * Copyright 2022 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.cloud.storage; + +import com.google.api.gax.grpc.GrpcStatusCode; +import com.google.api.gax.retrying.ResultRetryAlgorithm; +import com.google.api.gax.rpc.ApiException; +import com.google.api.gax.rpc.ApiExceptionFactory; +import com.google.api.gax.rpc.StatusCode; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import io.grpc.Status.Code; +import java.util.Map; +import java.util.Set; +import java.util.function.Function; +import java.util.stream.Collectors; + +/** + * A set of compatibility tools to map between grpc and HTTP status codes while trying to keep our + * retry behavior in consideration. + */ +final class GrpcToHttpStatusCodeTranslation { + + /** + * Mappings between gRPC status codes and their corresponding HTTP status codes, which match our + * existing definitions and retry bucketing. + * + *

+ * + *

{@code UNAVAILABLE} covers connection reset + * + * 

+   *   io.grpc.netty.shaded.io.netty.channel.unix.Errors$NativeIoException: readAddress(..) failed: Connection reset by peer
+   *
+ */ + private static final ImmutableList STATUS_CODE_MAPPINGS = + ImmutableList.of( + StatusCodeMapping.of(200, Code.OK), + StatusCodeMapping.of(400, Code.DATA_LOSS), + StatusCodeMapping.of(400, Code.INVALID_ARGUMENT), + StatusCodeMapping.of(400, Code.OUT_OF_RANGE), + StatusCodeMapping.of(401, Code.UNAUTHENTICATED), + StatusCodeMapping.of(403, Code.PERMISSION_DENIED), + StatusCodeMapping.of(404, Code.NOT_FOUND), + StatusCodeMapping.of(409, Code.ALREADY_EXISTS), + StatusCodeMapping.of(412, Code.FAILED_PRECONDITION), + StatusCodeMapping.of(429, Code.RESOURCE_EXHAUSTED), + StatusCodeMapping.of(500, Code.INTERNAL), + StatusCodeMapping.of(501, Code.UNIMPLEMENTED), + StatusCodeMapping.of(503, Code.UNAVAILABLE), + StatusCodeMapping.of(504, Code.DEADLINE_EXCEEDED), + StatusCodeMapping.of(409, Code.ABORTED), + StatusCodeMapping.of(0, Code.CANCELLED), + StatusCodeMapping.of(0, Code.UNKNOWN)); + + /** Index our {@link StatusCodeMapping} for constant time lookup by {@link Code} */ + private static final Map GRPC_CODE_INDEX = + STATUS_CODE_MAPPINGS.stream() + .collect( + ImmutableMap.toImmutableMap(StatusCodeMapping::getGrpcCode, Function.identity())); + + /** + * For use in {@link #resultRetryAlgorithmToCodes(ResultRetryAlgorithm)}. Resolve all codes and + * construct corresponding ApiExceptions. + * + *

Constructing the exceptions will walk the stack for each one. In order to avoid the stack + * walking overhead for every Code for every invocation, construct the set of exceptions only once + * and keep in this value. + */ + private static final Set CODE_API_EXCEPTIONS = + STATUS_CODE_MAPPINGS.stream() + .map(StatusCodeMapping::getGrpcStatusCode) + .map(c -> ApiExceptionFactory.createException(null, c, false)) + .map(StorageException::asStorageException) + .collect(Collectors.toSet()); + + /** + * When translating from gRPC Status Codes to the HTTP codes all of our middle ware expects, we + * must take care to translate in accordance with the expected retry semantics already outlined + * and validated for the JSON implementation. This is why we do not simply use {@link + * GrpcStatusCode#of(Code)}{@link GrpcStatusCode#getCode() .getCode}{@link + * StatusCode.Code#getHttpStatusCode() .getHttpStatusCode()} as it sometimes returns conflicting + * HTTP codes for our retry handling. + */ + static int grpcCodeToHttpStatusCode(Code code) { + StatusCodeMapping found = GRPC_CODE_INDEX.get(code); + // theoretically it's possible for gRPC to add a new code we haven't mapped here, if this + // happens fall through to our default of 0 + if (found != null) { + return found.getHttpStatus(); + } else { + return 0; + } + } + + /** + * When using the retry features of the Gapic client, we are only allowed to provide a {@link + * Set}{@code <}{@link StatusCode.Code}{@code >}. Given {@link StatusCode.Code} is an enum, we can + * resolve the set of values from a given {@link ResultRetryAlgorithm} by evaluating each one as + * an {@link ApiException}. + */ + static Set resultRetryAlgorithmToCodes(ResultRetryAlgorithm alg) { + return CODE_API_EXCEPTIONS.stream() + .filter(e -> alg.shouldRetry(e, null)) + .map(e -> e.apiExceptionCause.getStatusCode().getCode()) + .collect(Collectors.toSet()); + } + + /** + * Simple tuple class to bind together our corresponding http status code and {@link Code} while + * providing easy access to the correct {@link GrpcStatusCode} where necessary. + */ + private static final class StatusCodeMapping { + + private final int httpStatus; + + private final Code grpcCode; + private final GrpcStatusCode grpcStatusCode; + + private StatusCodeMapping(int httpStatus, Code grpcCode, GrpcStatusCode grpcStatusCode) { + this.httpStatus = httpStatus; + this.grpcCode = grpcCode; + this.grpcStatusCode = grpcStatusCode; + } + + public int getHttpStatus() { + return httpStatus; + } + + public Code getGrpcCode() { + return grpcCode; + } + + public GrpcStatusCode getGrpcStatusCode() { + return grpcStatusCode; + } + + static StatusCodeMapping of(int httpStatus, Code grpcCode) { + return new StatusCodeMapping(httpStatus, grpcCode, GrpcStatusCode.of(grpcCode)); + } + } +} diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageException.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageException.java index 61cbbf5b0..61c100219 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageException.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageException.java @@ -129,7 +129,8 @@ static StorageException asStorageException(ApiException apiEx) { StatusCode statusCode = apiEx.getStatusCode(); if (statusCode instanceof GrpcStatusCode) { GrpcStatusCode gsc = (GrpcStatusCode) statusCode; - httpStatusCode = BackwardCompatibilityUtils.grpcCodeToHttpStatusCode(gsc.getTransportCode()); + httpStatusCode = + GrpcToHttpStatusCodeTranslation.grpcCodeToHttpStatusCode(gsc.getTransportCode()); } // If there is a gRPC exception in our cause change pull it's error message up to be our // message otherwise, create a generic error message with the status code. diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/BackwardCompatibilityUtilsTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/BackwardCompatibilityUtilsTest.java index 43e2dda0f..46cec8e0d 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/BackwardCompatibilityUtilsTest.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/BackwardCompatibilityUtilsTest.java @@ -30,8 +30,6 @@ import com.google.cloud.storage.BucketInfo.NumNewerVersionsDeleteRule; import com.google.cloud.storage.BucketInfo.RawDeleteRule; import com.google.cloud.storage.Conversions.Codec; -import io.grpc.Status.Code; -import java.util.EnumMap; import org.junit.Test; public final class BackwardCompatibilityUtilsTest { @@ -94,35 +92,6 @@ public void testRuleMappingIsCorrect_setDeleteRules_nonEmpty() { assertThat(lifecycleRule.getCondition().getAge()).isEqualTo(5); } - @Test - public void grpcCodeToHttpStatusCode_expectedMapping() { - EnumMap expected = new EnumMap<>(Code.class); - expected.put(Code.OK, 200); - expected.put(Code.INVALID_ARGUMENT, 400); - expected.put(Code.OUT_OF_RANGE, 400); - expected.put(Code.UNAUTHENTICATED, 401); - expected.put(Code.PERMISSION_DENIED, 403); - expected.put(Code.NOT_FOUND, 404); - expected.put(Code.FAILED_PRECONDITION, 412); - expected.put(Code.ALREADY_EXISTS, 409); - expected.put(Code.RESOURCE_EXHAUSTED, 429); - expected.put(Code.INTERNAL, 500); - expected.put(Code.UNIMPLEMENTED, 501); - expected.put(Code.UNAVAILABLE, 503); - expected.put(Code.ABORTED, 0); - expected.put(Code.CANCELLED, 0); - expected.put(Code.UNKNOWN, 0); - expected.put(Code.DEADLINE_EXCEEDED, 504); - expected.put(Code.DATA_LOSS, 400); - - EnumMap actual = new EnumMap<>(Code.class); - for (Code c : Code.values()) { - actual.put(c, BackwardCompatibilityUtils.grpcCodeToHttpStatusCode(c)); - } - - assertThat(actual).isEqualTo(expected); - } - private void verifyConversionRoundTrip(DeleteRule delRule) { Rule encode = codec.encode(delRule); DeleteRule decode = codec.decode(encode); diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/GrpcToHttpStatusCodeTranslationTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/GrpcToHttpStatusCodeTranslationTest.java new file mode 100644 index 000000000..4a7136acf --- /dev/null +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/GrpcToHttpStatusCodeTranslationTest.java @@ -0,0 +1,86 @@ +/* + * Copyright 2022 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.cloud.storage; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.api.gax.rpc.StatusCode; +import com.google.common.collect.ImmutableSet; +import io.grpc.Status.Code; +import java.util.EnumMap; +import java.util.Set; +import org.junit.Test; + +public final class GrpcToHttpStatusCodeTranslationTest { + + @Test + public void grpcCodeToHttpStatusCode_expectedMapping() { + EnumMap expected = new EnumMap<>(Code.class); + expected.put(Code.OK, 200); + expected.put(Code.INVALID_ARGUMENT, 400); + expected.put(Code.OUT_OF_RANGE, 400); + expected.put(Code.UNAUTHENTICATED, 401); + expected.put(Code.PERMISSION_DENIED, 403); + expected.put(Code.NOT_FOUND, 404); + expected.put(Code.FAILED_PRECONDITION, 412); + expected.put(Code.ALREADY_EXISTS, 409); + expected.put(Code.RESOURCE_EXHAUSTED, 429); + expected.put(Code.INTERNAL, 500); + expected.put(Code.UNIMPLEMENTED, 501); + expected.put(Code.UNAVAILABLE, 503); + expected.put(Code.ABORTED, 409); + expected.put(Code.CANCELLED, 0); + expected.put(Code.UNKNOWN, 0); + expected.put(Code.DEADLINE_EXCEEDED, 504); + expected.put(Code.DATA_LOSS, 400); + + EnumMap actual = new EnumMap<>(Code.class); + for (Code c : Code.values()) { + actual.put(c, GrpcToHttpStatusCodeTranslation.grpcCodeToHttpStatusCode(c)); + } + + assertThat(actual).isEqualTo(expected); + } + + @Test + public void resultRetryAlgorithmToCodes_idempotent() { + StorageRetryStrategy strategy = StorageRetryStrategy.getDefaultStorageRetryStrategy(); + + Set actual = + GrpcToHttpStatusCodeTranslation.resultRetryAlgorithmToCodes( + strategy.getIdempotentHandler()); + + ImmutableSet expected = + ImmutableSet.of( + StatusCode.Code.UNAVAILABLE, + StatusCode.Code.DEADLINE_EXCEEDED, + StatusCode.Code.INTERNAL, + StatusCode.Code.RESOURCE_EXHAUSTED); + assertThat(actual).isEqualTo(expected); + } + + @Test + public void resultRetryAlgorithmToCodes_nonIdempotent() { + StorageRetryStrategy strategy = StorageRetryStrategy.getDefaultStorageRetryStrategy(); + + Set actual = + GrpcToHttpStatusCodeTranslation.resultRetryAlgorithmToCodes( + strategy.getNonidempotentHandler()); + + assertThat(actual).isEqualTo(ImmutableSet.of()); + } +} diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/ResultRetryAlgorithmCompatibilityTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/ResultRetryAlgorithmCompatibilityTest.java index 9c907175f..40aebc94a 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/ResultRetryAlgorithmCompatibilityTest.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/ResultRetryAlgorithmCompatibilityTest.java @@ -30,7 +30,8 @@ public void validateDefaultStorageRetryStrategy_idempotent() { ResultRetryAlgorithm idempotentHandler = StorageRetryStrategy.getDefaultStorageRetryStrategy().getIdempotentHandler(); - Set codes = GrpcStorageImpl.resultRetryAlgorithmToCodes(idempotentHandler); + Set codes = + GrpcToHttpStatusCodeTranslation.resultRetryAlgorithmToCodes(idempotentHandler); ImmutableSet expected = ImmutableSet.of( Code.INTERNAL, Code.UNAVAILABLE, Code.RESOURCE_EXHAUSTED, Code.DEADLINE_EXCEEDED); diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageExceptionGrpcCompatibilityTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageExceptionGrpcCompatibilityTest.java index 21f6ee395..a877890c9 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageExceptionGrpcCompatibilityTest.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageExceptionGrpcCompatibilityTest.java @@ -16,7 +16,6 @@ package com.google.cloud.storage; -import static com.google.cloud.storage.BackwardCompatibilityUtils.grpcCodeToHttpStatusCode; import static com.google.common.truth.Truth.assertThat; import com.google.api.gax.grpc.GrpcStatusCode; @@ -28,28 +27,115 @@ import com.google.protobuf.Any; import com.google.rpc.DebugInfo; import com.google.rpc.ErrorInfo; +import io.grpc.Status; import io.grpc.Status.Code; import io.grpc.StatusRuntimeException; -import java.util.Arrays; -import java.util.List; import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.Parameterized; -import org.junit.runners.Parameterized.Parameters; -@RunWith(Parameterized.class) public final class StorageExceptionGrpcCompatibilityTest { - private final ApiException x; + @Test + public void testCoalesce_CANCELLED() { + doTestCoalesce(0, Code.CANCELLED); + } - public StorageExceptionGrpcCompatibilityTest(Code ignore, ApiException x) { - this.x = x; + @Test + public void testCoalesce_UNKNOWN() { + doTestCoalesce(0, Code.UNKNOWN); } @Test - public void testCoalesce() { - GrpcStatusCode grpcStatusCode = (GrpcStatusCode) x.getStatusCode(); - int expectedCode = grpcCodeToHttpStatusCode(grpcStatusCode.getTransportCode()); + public void testCoalesce_INVALID_ARGUMENT() { + doTestCoalesce(400, Code.INVALID_ARGUMENT); + } + + @Test + public void testCoalesce_DEADLINE_EXCEEDED() { + doTestCoalesce(504, Code.DEADLINE_EXCEEDED); + } + + @Test + public void testCoalesce_NOT_FOUND() { + doTestCoalesce(404, Code.NOT_FOUND); + } + + @Test + public void testCoalesce_ALREADY_EXISTS() { + doTestCoalesce(409, Code.ALREADY_EXISTS); + } + + @Test + public void testCoalesce_PERMISSION_DENIED() { + doTestCoalesce(403, Code.PERMISSION_DENIED); + } + + @Test + public void testCoalesce_RESOURCE_EXHAUSTED() { + doTestCoalesce(429, Code.RESOURCE_EXHAUSTED); + } + + @Test + public void testCoalesce_FAILED_PRECONDITION() { + doTestCoalesce(412, Code.FAILED_PRECONDITION); + } + + @Test + public void testCoalesce_ABORTED() { + doTestCoalesce(409, Code.ABORTED); + } + + @Test + public void testCoalesce_OUT_OF_RANGE() { + doTestCoalesce(400, Code.OUT_OF_RANGE); + } + + @Test + public void testCoalesce_UNIMPLEMENTED() { + doTestCoalesce(501, Code.UNIMPLEMENTED); + } + + @Test + public void testCoalesce_INTERNAL() { + doTestCoalesce(500, Code.INTERNAL); + } + + @Test + public void testCoalesce_UNAVAILABLE() { + doTestCoalesce(503, Code.UNAVAILABLE); + } + + @Test + public void testCoalesce_DATA_LOSS() { + doTestCoalesce(400, Code.DATA_LOSS); + } + + @Test + public void testCoalesce_UNAUTHENTICATED() { + doTestCoalesce(401, Code.UNAUTHENTICATED); + } + + private void doTestCoalesce(int expectedCode, Code code) { + Status status = code.toStatus(); + GrpcStatusCode statusCode = GrpcStatusCode.of(code); + ErrorInfo errorInfo = + ErrorInfo.newBuilder() + .setReason("reason") + .setDomain("global") + .putMetadata("errors", "x") + .build(); + + DebugInfo debugInfo = + DebugInfo.newBuilder() + .setDetail( + "bw-storage-dev-region-fine@default-223119.iam.gserviceaccount.com does not have storage.hmacKeys.list access to the Google Cloud project.") + .build(); + + ImmutableList anys = ImmutableList.of(Any.pack(errorInfo), Any.pack(debugInfo)); + ErrorDetails errorDetails = ErrorDetails.builder().setRawErrorMessages(anys).build(); + + StatusRuntimeException cause = + new StatusRuntimeException(status.withDescription(debugInfo.getDetail())); + ApiException x = ApiExceptionFactory.createException(cause, statusCode, false, errorDetails); BaseServiceException ex = StorageException.coalesce(x); assertThat(ex.getCode()).isEqualTo(expectedCode); @@ -57,39 +143,4 @@ public void testCoalesce() { assertThat(ex.getMessage()).contains(x.getErrorDetails().getDebugInfo().getDetail()); assertThat(ex).hasCauseThat().isEqualTo(x); } - - @Parameters(name = "{0}") - public static List args() { - return Arrays.stream(Code.values()) - .map(Code::toStatus) - .filter(s -> !s.isOk()) - .map( - status -> { - Code code = status.getCode(); - - GrpcStatusCode statusCode = GrpcStatusCode.of(code); - ErrorInfo errorInfo = - ErrorInfo.newBuilder() - .setReason("reason") - .setDomain("global") - .putMetadata("errors", "x") - .build(); - - DebugInfo debugInfo = - DebugInfo.newBuilder() - .setDetail( - "bw-storage-dev-region-fine@default-223119.iam.gserviceaccount.com does not have storage.hmacKeys.list access to the Google Cloud project.") - .build(); - - ImmutableList anys = ImmutableList.of(Any.pack(errorInfo), Any.pack(debugInfo)); - ErrorDetails errorDetails = ErrorDetails.builder().setRawErrorMessages(anys).build(); - - StatusRuntimeException cause = - new StatusRuntimeException(status.withDescription(debugInfo.getDetail())); - return new Object[] { - code, ApiExceptionFactory.createException(cause, statusCode, false, errorDetails) - }; - }) - .collect(ImmutableList.toImmutableList()); - } }