Repository: googleapis/java-storage
Pull request #1713 (2022-10-17 21:47:01)

chore: refactor and cleanup translation between gRPC status codes and http codes
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



Patch:
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 6dc5575d..33f31756 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 @@ package com.google.cloud.storage;
 
 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.LifecycleRule.LifecycleCondition;
 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 0855a0ce..e947e4da 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 @@ package com.google.cloud.storage;
 
 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.AbstractPage;
 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<StorageOptions> implements Stora
           StandardOpenOption.WRITE,
           StandardOpenOption.CREATE,
           StandardOpenOption.TRUNCATE_EXISTING);
-  /**
-   * For use in {@link #resultRetryAlgorithmToCodes(ResultRetryAlgorithm)}. Resolve all codes and
-   * construct corresponding ApiExceptions.
-   *
-   * <p>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<StorageException> 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 @@ final class GrpcStorageImpl extends BaseService<StorageOptions> implements Stora
   public GrpcBlobReadChannel reader(BlobId blob, BlobSourceOption... options) {
     Opts<ObjectSourceOpt> opts = Opts.unwrap(options).resolveFrom(blob);
     ReadObjectRequest request = getReadObjectRequest(blob, opts);
-    Set<StatusCode.Code> codes =
-        GrpcStorageImpl.resultRetryAlgorithmToCodes(retryAlgorithmManager.getFor(request));
+    Set<StatusCode.Code> codes = resultRetryAlgorithmToCodes(retryAlgorithmManager.getFor(request));
     GrpcCallContext grpcCallContext = GrpcCallContext.createDefault().withRetryableCodes(codes);
     return new GrpcBlobReadChannel(
         storageClient.readObjectCallable().withDefaultCallContext(grpcCallContext),
@@ -1318,8 +1303,7 @@ final class GrpcStorageImpl extends BaseService<StorageOptions> implements Stora
         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 @@ final class GrpcStorageImpl extends BaseService<StorageOptions> implements Stora
     Opts<ObjectSourceOpt> opts = Opts.unwrap(options).resolveFrom(blob);
     ReadObjectRequest readObjectRequest = getReadObjectRequest(blob, opts);
     Set<StatusCode.Code> codes =
-        GrpcStorageImpl.resultRetryAlgorithmToCodes(
-            retryAlgorithmManager.getFor(readObjectRequest));
+        resultRetryAlgorithmToCodes(retryAlgorithmManager.getFor(readObjectRequest));
     GrpcCallContext grpcCallContext = GrpcCallContext.createDefault().withRetryableCodes(codes);
     return ResumableMedia.gapic()
         .read()
@@ -1383,8 +1366,7 @@ final class GrpcStorageImpl extends BaseService<StorageOptions> implements Stora
   @VisibleForTesting
   ApiFuture<ResumableWrite> startResumableWrite(
       GrpcCallContext grpcCallContext, WriteObjectRequest req) {
-    Set<StatusCode.Code> codes =
-        GrpcStorageImpl.resultRetryAlgorithmToCodes(retryAlgorithmManager.getFor(req));
+    Set<StatusCode.Code> codes = resultRetryAlgorithmToCodes(retryAlgorithmManager.getFor(req));
     return ResumableMedia.gapic()
         .write()
         .resumableWrite(
@@ -1394,80 +1376,6 @@ final class GrpcStorageImpl extends BaseService<StorageOptions> implements Stora
             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<StatusCode.Code> 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<String> 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 00000000..feda6e70
--- /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.
+   *
+   * <p>
+   *
+   * <p>{@code UNAVAILABLE} covers connection reset
+   *
+   * <pre>
+   *   io.grpc.netty.shaded.io.netty.channel.unix.Errors$NativeIoException: readAddress(..) failed: Connection reset by peer
+   * </pre>
+   */
+  private static final ImmutableList<StatusCodeMapping> 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<Code, StatusCodeMapping> 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.
+   *
+   * <p>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<StorageException> 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<StatusCode.Code> 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 61cbbf5b..61c10021 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 @@ public final class StorageException extends BaseHttpServiceException {
     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.


Test patch:
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 43e2dda0..46cec8e0 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.LifecycleRule.LifecycleAction;
 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 final class BackwardCompatibilityUtilsTest {
     assertThat(lifecycleRule.getCondition().getAge()).isEqualTo(5);
   }
 
-  @Test
-  public void grpcCodeToHttpStatusCode_expectedMapping() {
-    EnumMap<Code, Integer> 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<Code, Integer> 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 00000000..4a7136ac
--- /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<Code, Integer> 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<Code, Integer> 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<StatusCode.Code> actual =
+        GrpcToHttpStatusCodeTranslation.resultRetryAlgorithmToCodes(
+            strategy.getIdempotentHandler());
+
+    ImmutableSet<StatusCode.Code> 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<StatusCode.Code> 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 9c907175..40aebc94 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 final class ResultRetryAlgorithmCompatibilityTest {
     ResultRetryAlgorithm<?> idempotentHandler =
         StorageRetryStrategy.getDefaultStorageRetryStrategy().getIdempotentHandler();
 
-    Set<Code> codes = GrpcStorageImpl.resultRetryAlgorithmToCodes(idempotentHandler);
+    Set<Code> codes =
+        GrpcToHttpStatusCodeTranslation.resultRetryAlgorithmToCodes(idempotentHandler);
     ImmutableSet<Code> 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 21f6ee39..a877890c 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.common.collect.ImmutableList;
 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<Any> 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 final class StorageExceptionGrpcCompatibilityTest {
     assertThat(ex.getMessage()).contains(x.getErrorDetails().getDebugInfo().getDetail());
     assertThat(ex).hasCauseThat().isEqualTo(x);
   }
-
-  @Parameters(name = "{0}")
-  public static List<Object[]> 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<Any> 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());
-  }
 }