Skip to content

Commit

Permalink
chore: refactor and cleanup translation between gRPC status codes and…
Browse files Browse the repository at this point in the history 
… 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
  • Loading branch information
@BenWhitehead
BenWhitehead committed Oct 21, 2022
1 parent 5ff8be1 commit 0194639
Show file tree
Hide file tree
Showing 8 changed files with 350 additions and 243 deletions.
65 changes: 0 additions & 65 deletions google-cloud-storage/src/main/java/com/google/cloud/storage/BackwardCompatibilityUtils.java
View file
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
102 changes: 5 additions & 97 deletions google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java
View file
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -696,8 +682,7 @@ public GrpcBlobReadChannel reader(String bucket, String blob, BlobSourceOption..
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),
Expand Down Expand Up @@ -1318,8 +1303,7 @@ static <T> 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) {
Expand Down Expand Up @@ -1368,8 +1352,7 @@ private UnbufferedReadableByteChannelSession<Object> unbufferedReadSession(
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()
Expand All @@ -1383,8 +1366,7 @@ private UnbufferedReadableByteChannelSession<Object> unbufferedReadSession(
@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(
Expand All @@ -1394,80 +1376,6 @@ ApiFuture<ResumableWrite> 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<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());
Expand Down

0 comments on commit 0194639

Please sign in to comment.