diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcConversions.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcConversions.java index 28c51873b0..63a33b80a1 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcConversions.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcConversions.java @@ -60,11 +60,13 @@ import com.google.storage.v2.Owner; import com.google.type.Date; import com.google.type.Expr; +import java.nio.charset.StandardCharsets; import java.time.Duration; import java.time.Instant; import java.time.LocalDate; import java.time.OffsetDateTime; import java.time.ZoneOffset; +import java.util.Base64; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -118,6 +120,11 @@ final class GrpcConversions { hierarchicalNamespaceCodec = Codec.of(this::hierarchicalNamespaceEncode, this::hierarchicalNamespaceDecode); + private final Codec byteStringB64StringCodec = + Codec.of( + bs -> Base64.getEncoder().encodeToString(bs.toByteArray()), + s -> ByteString.copyFrom(Base64.getDecoder().decode(s.getBytes(StandardCharsets.UTF_8)))); + @VisibleForTesting final Codec timestampCodec = Codec.of( @@ -1048,7 +1055,7 @@ private NotificationInfo notificationDecode(NotificationConfig from) { private com.google.iam.v1.Policy policyEncode(Policy from) { com.google.iam.v1.Policy.Builder to = com.google.iam.v1.Policy.newBuilder(); - ifNonNull(from.getEtag(), ByteString::copyFromUtf8, to::setEtag); + ifNonNull(from.getEtag(), byteStringB64StringCodec::decode, to::setEtag); ifNonNull(from.getVersion(), to::setVersion); from.getBindingsList().stream().map(bindingCodec::encode).forEach(to::addBindings); return to.build(); @@ -1058,7 +1065,7 @@ private Policy policyDecode(com.google.iam.v1.Policy from) { Policy.Builder to = Policy.newBuilder(); ByteString etag = from.getEtag(); if (!etag.isEmpty()) { - to.setEtag(etag.toStringUtf8()); + to.setEtag(byteStringB64StringCodec.encode(etag)); } to.setVersion(from.getVersion()); List bindingsList = from.getBindingsList(); diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcRetryAlgorithmManager.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcRetryAlgorithmManager.java index 3a97c2e952..1178c7c942 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcRetryAlgorithmManager.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcRetryAlgorithmManager.java @@ -20,6 +20,7 @@ import com.google.iam.v1.GetIamPolicyRequest; import com.google.iam.v1.SetIamPolicyRequest; import com.google.iam.v1.TestIamPermissionsRequest; +import com.google.protobuf.ByteString; import com.google.storage.v2.BidiWriteObjectRequest; import com.google.storage.v2.ComposeObjectRequest; import com.google.storage.v2.CreateBucketRequest; @@ -168,8 +169,11 @@ public ResultRetryAlgorithm getFor(RewriteObjectRequest req) { } public ResultRetryAlgorithm getFor(SetIamPolicyRequest req) { - // TODO: etag - return retryStrategy.getNonidempotentHandler(); + if (req.getPolicy().getEtag().equals(ByteString.empty())) { + return retryStrategy.getNonidempotentHandler(); + } else { + return retryStrategy.getIdempotentHandler(); + } } public ResultRetryAlgorithm getFor(StartResumableWriteRequest req) { diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/conformance/retry/CtxFunctions.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/conformance/retry/CtxFunctions.java index 7617be79f9..8a415fd8f6 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/conformance/retry/CtxFunctions.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/conformance/retry/CtxFunctions.java @@ -136,6 +136,10 @@ static final class Local { static final class Rpc { static final CtxFunction createEmptyBlob = (ctx, c) -> ctx.map(state -> state.with(ctx.getStorage().create(state.getBlobInfo()))); + static final CtxFunction bucketIamPolicy = + (ctx, c) -> + ctx.map( + state -> state.with(ctx.getStorage().getIamPolicy(state.getBucket().getName()))); } static final class ResourceSetup { diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/conformance/retry/ITRetryConformanceTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/conformance/retry/ITRetryConformanceTest.java index d8250fcf33..814729f840 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/conformance/retry/ITRetryConformanceTest.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/conformance/retry/ITRetryConformanceTest.java @@ -19,7 +19,6 @@ import static com.google.cloud.storage.PackagePrivateMethodWorkarounds.blobCopyWithStorage; import static com.google.cloud.storage.PackagePrivateMethodWorkarounds.bucketCopyWithStorage; import static com.google.cloud.storage.conformance.retry.Ctx.ctx; -import static com.google.cloud.storage.conformance.retry.ITRetryConformanceTest.RetryTestCaseResolver.lift; import static com.google.cloud.storage.conformance.retry.State.empty; import static com.google.common.truth.Truth.assertThat; import static java.util.Objects.requireNonNull; @@ -168,13 +167,7 @@ public ImmutableList parameters() { .setMappings(new RpcMethodMappings()) .setProjectId("conformance-tests") .setHost(testBench.getBaseUri().replaceAll("https?://", "")) - .setTestAllowFilter( - RetryTestCaseResolver.includeAll() - .and( - (lift(trc -> trc.getTransport() == Transport.GRPC) - .and(RetryTestCaseResolver.scenarioIdIs(2)) - .and((m, trc) -> m == RpcMethod.storage.buckets.setIamPolicy)) - .negate())) + .setTestAllowFilter(RetryTestCaseResolver.includeAll()) .build(); List retryTestCases; diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/conformance/retry/RpcMethodMappings.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/conformance/retry/RpcMethodMappings.java index 3f98a94fa4..a60b1bb984 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/conformance/retry/RpcMethodMappings.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/conformance/retry/RpcMethodMappings.java @@ -685,6 +685,7 @@ private static void setIamPolicy(ArrayList a) { a.add( RpcMethodMapping.newBuilder(240, buckets.setIamPolicy) .withApplicable(TestRetryConformance::isPreconditionsProvided) + .withSetup(ResourceSetup.defaultSetup.andThen(Rpc.bucketIamPolicy)) .withTest( (ctx, c) -> ctx.map( @@ -694,7 +695,7 @@ private static void setIamPolicy(ArrayList a) { .setIamPolicy( state.getBucket().getName(), Policy.newBuilder() - .setEtag("h??") + .setEtag(state.getPolicy().getEtag()) .setVersion(3) .setBindings( ImmutableList.of(