From b4e663ab1391b9e506438d0c7e8dc11646f4bbaf Mon Sep 17 00:00:00 2001 From: BenWhitehead Date: Fri, 1 Oct 2021 16:15:31 -0400 Subject: [PATCH] test(retry): fix objects.compose tests * fix incorrect evaluation of idempotency for objects.compose (now generation, previously metageneration) * add mapping for non-idempotent objects.compose invocation * add CtxFunction to construct a ComposeRequest relative to the state * add ComposeRequest handling to state --- .../storage/NewRetryAlgorithmManager.java | 2 +- .../conformance/retry/CtxFunctions.java | 30 ++++++++++++++++++- .../conformance/retry/RpcMethodMappings.java | 23 ++++++++------ .../storage/conformance/retry/State.java | 14 +++++++++ .../retry/testNamesWhichShouldSucceed.txt | 13 +++++--- 5 files changed, 67 insertions(+), 15 deletions(-) diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/NewRetryAlgorithmManager.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/NewRetryAlgorithmManager.java index 8f84990f2b..4dd6390dff 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/NewRetryAlgorithmManager.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/NewRetryAlgorithmManager.java @@ -284,7 +284,7 @@ public ExceptionHandler getForObjectsRewrite(RewriteRequest pb) { @Override public ExceptionHandler getForObjectsCompose( List sources, StorageObject target, Map optionsMap) { - return optionsMap.containsKey(StorageRpc.Option.IF_METAGENERATION_MATCH) + return optionsMap.containsKey(StorageRpc.Option.IF_GENERATION_MATCH) ? IDEMPOTENT_HANDLER : NON_IDEMPOTENT_HANDLER; } 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 5bf0c22bd1..d51de7a668 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 @@ -29,6 +29,8 @@ import com.google.cloud.storage.BucketInfo; import com.google.cloud.storage.HmacKey; import com.google.cloud.storage.ServiceAccount; +import com.google.cloud.storage.Storage.BlobTargetOption; +import com.google.cloud.storage.Storage.ComposeRequest; import com.google.cloud.storage.conformance.retry.Functions.CtxFunction; import com.google.common.base.Joiner; import java.util.HashSet; @@ -46,9 +48,35 @@ */ final class CtxFunctions { - private static final class Util { + static final class Util { private static final CtxFunction blobIdAndBlobInfo = (ctx, c) -> ctx.map(state -> state.with(BlobInfo.newBuilder(state.getBlobId()).build())); + + static final CtxFunction composeRequest = + (ctx, c) -> + ctx.map( + state -> { + Blob blob = state.getBlob(); + String bucket = blob.getBucket(); + final BlobInfo target; + if (c.isPreconditionsProvided()) { + target = BlobInfo.newBuilder(BlobId.of(bucket, "blob-full", 0L)).build(); + } else { + target = BlobInfo.newBuilder(BlobId.of(bucket, "blob-full")).build(); + } + ComposeRequest.Builder builder = + ComposeRequest.newBuilder() + // source bucket is resolved from the target, as compose must be within + // the same bucket + .addSource(blob.getName(), blob.getGeneration()) + .addSource(blob.getName(), blob.getGeneration()) + .setTarget(target); + if (c.isPreconditionsProvided()) { + builder = builder.setTargetOptions(BlobTargetOption.generationMatch()); + } + ComposeRequest r = builder.build(); + return state.with(r); + }); } static final class Local { 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 f2d188a1f8..c76d2d004c 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 @@ -23,7 +23,6 @@ import static com.google.cloud.storage.conformance.retry.CtxFunctions.ResourceSetup.defaultSetup; import static com.google.cloud.storage.conformance.retry.CtxFunctions.ResourceSetup.serviceAccount; import static com.google.common.base.Predicates.not; -import static com.google.common.collect.Lists.newArrayList; import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertTrue; @@ -45,12 +44,12 @@ import com.google.cloud.storage.Storage.BlobWriteOption; import com.google.cloud.storage.Storage.BucketSourceOption; import com.google.cloud.storage.Storage.BucketTargetOption; -import com.google.cloud.storage.Storage.ComposeRequest; import com.google.cloud.storage.Storage.CopyRequest; import com.google.cloud.storage.Storage.SignUrlOption; import com.google.cloud.storage.Storage.UriScheme; import com.google.cloud.storage.conformance.retry.CtxFunctions.Local; import com.google.cloud.storage.conformance.retry.CtxFunctions.Rpc; +import com.google.cloud.storage.conformance.retry.CtxFunctions.Util; import com.google.cloud.storage.conformance.retry.RpcMethod.storage.bucket_acl; import com.google.cloud.storage.conformance.retry.RpcMethod.storage.buckets; import com.google.cloud.storage.conformance.retry.RpcMethod.storage.default_object_acl; @@ -1824,17 +1823,23 @@ private static void update(ArrayList a) {} private static void compose(ArrayList a) { a.add( RpcMethodMapping.newBuilder(35, objects.compose) + .withApplicable(TestRetryConformance::isPreconditionsProvided) + .withSetup(defaultSetup.andThen(Util.composeRequest)) .withTest( (ctx, c) -> ctx.map( state -> - state.with( - ctx.getStorage() - .compose( - ComposeRequest.of( - c.getBucketName(), - newArrayList("blob-part-1", "blob-part-2"), - "blob-full"))))) + state.with(ctx.getStorage().compose(state.getComposeRequest())))) + .build()); + a.add( + RpcMethodMapping.newBuilder(241, objects.compose) + .withApplicable(not(TestRetryConformance::isPreconditionsProvided)) + .withSetup(defaultSetup.andThen(Util.composeRequest)) + .withTest( + (ctx, c) -> + ctx.map( + state -> + state.with(ctx.getStorage().compose(state.getComposeRequest())))) .build()); } diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/conformance/retry/State.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/conformance/retry/State.java index 8dafa7c17f..4f7552977e 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/conformance/retry/State.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/conformance/retry/State.java @@ -30,6 +30,7 @@ import com.google.cloud.storage.HmacKey; import com.google.cloud.storage.HmacKey.HmacKeyMetadata; import com.google.cloud.storage.ServiceAccount; +import com.google.cloud.storage.Storage.ComposeRequest; import com.google.cloud.storage.conformance.retry.Functions.VoidFunction; import com.google.common.collect.ImmutableMap; import com.google.errorprone.annotations.Immutable; @@ -75,6 +76,7 @@ final class State { new Key<>("testIamPermissionsResults"); private static final Key> KEY_ACLS = new Key<>("acls"); private static final Key KEY_BYTES = new Key<>("bytes"); + private static final Key KEY_COMPOSE_REQUEST = new Key<>("composeRequest"); private final ImmutableMap, Object> data; @@ -293,6 +295,18 @@ public State consume(Page page) { return newStateWith(KEY_LIST_OBJECTS, collect); } + public State with(ComposeRequest composeRequest) { + return newStateWith(KEY_COMPOSE_REQUEST, composeRequest); + } + + public ComposeRequest getComposeRequest() { + return getValue(KEY_COMPOSE_REQUEST); + } + + public boolean hasComposeRequest() { + return hasValue(KEY_COMPOSE_REQUEST); + } + private T getValue(Key key) { Object o = data.get(key); requireNonNull(o, () -> String.format("%s was not found in state", key.name)); diff --git a/google-cloud-storage/src/test/resources/com/google/cloud/storage/conformance/retry/testNamesWhichShouldSucceed.txt b/google-cloud-storage/src/test/resources/com/google/cloud/storage/conformance/retry/testNamesWhichShouldSucceed.txt index c631fba3b9..7a183b7eaa 100644 --- a/google-cloud-storage/src/test/resources/com/google/cloud/storage/conformance/retry/testNamesWhichShouldSucceed.txt +++ b/google-cloud-storage/src/test/resources/com/google/cloud/storage/conformance/retry/testNamesWhichShouldSucceed.txt @@ -142,6 +142,7 @@ TestRetryConformance/1-[return-reset-connection_return-reset-connection]-storage TestRetryConformance/1-[return-reset-connection_return-reset-connection]-storage.serviceaccount.get-59 TestRetryConformance/2-[return-503_return-503]-storage.buckets.patch-101 TestRetryConformance/2-[return-503_return-503]-storage.buckets.patch-122 +TestRetryConformance/2-[return-503_return-503]-storage.objects.compose-35 TestRetryConformance/2-[return-503_return-503]-storage.objects.delete-37 TestRetryConformance/2-[return-503_return-503]-storage.objects.delete-38 TestRetryConformance/2-[return-503_return-503]-storage.objects.delete-68 @@ -151,6 +152,7 @@ TestRetryConformance/2-[return-503_return-503]-storage.objects.patch-57 TestRetryConformance/2-[return-503_return-503]-storage.objects.patch-80 TestRetryConformance/2-[return-reset-connection_return-503]-storage.buckets.patch-101 TestRetryConformance/2-[return-reset-connection_return-503]-storage.buckets.patch-122 +TestRetryConformance/2-[return-reset-connection_return-503]-storage.objects.compose-35 TestRetryConformance/2-[return-reset-connection_return-503]-storage.objects.delete-37 TestRetryConformance/2-[return-reset-connection_return-503]-storage.objects.delete-38 TestRetryConformance/2-[return-reset-connection_return-503]-storage.objects.delete-68 @@ -160,6 +162,7 @@ TestRetryConformance/2-[return-reset-connection_return-503]-storage.objects.patc TestRetryConformance/2-[return-reset-connection_return-503]-storage.objects.patch-80 TestRetryConformance/2-[return-reset-connection_return-reset-connection]-storage.buckets.patch-101 TestRetryConformance/2-[return-reset-connection_return-reset-connection]-storage.buckets.patch-122 +TestRetryConformance/2-[return-reset-connection_return-reset-connection]-storage.objects.compose-35 TestRetryConformance/2-[return-reset-connection_return-reset-connection]-storage.objects.delete-37 TestRetryConformance/2-[return-reset-connection_return-reset-connection]-storage.objects.delete-38 TestRetryConformance/2-[return-reset-connection_return-reset-connection]-storage.objects.delete-68 @@ -170,7 +173,7 @@ TestRetryConformance/2-[return-reset-connection_return-reset-connection]-storage TestRetryConformance/3-[return-503]-storage.buckets.patch-17 TestRetryConformance/3-[return-503]-storage.buckets.setIamPolicy-18 TestRetryConformance/3-[return-503]-storage.hmacKey.update-29 -TestRetryConformance/3-[return-503]-storage.objects.compose-35 +TestRetryConformance/3-[return-503]-storage.objects.compose-241 TestRetryConformance/3-[return-503]-storage.objects.delete-36 TestRetryConformance/3-[return-503]-storage.objects.delete-67 TestRetryConformance/3-[return-503]-storage.objects.insert-108 @@ -195,7 +198,7 @@ TestRetryConformance/3-[return-503]-storage.objects.rewrite-86 TestRetryConformance/3-[return-reset-connection]-storage.buckets.patch-17 TestRetryConformance/3-[return-reset-connection]-storage.buckets.setIamPolicy-18 TestRetryConformance/3-[return-reset-connection]-storage.hmacKey.update-29 -TestRetryConformance/3-[return-reset-connection]-storage.objects.compose-35 +TestRetryConformance/3-[return-reset-connection]-storage.objects.compose-241 TestRetryConformance/3-[return-reset-connection]-storage.objects.insert-108 TestRetryConformance/3-[return-reset-connection]-storage.objects.insert-109 TestRetryConformance/3-[return-reset-connection]-storage.objects.insert-110 @@ -303,7 +306,7 @@ TestRetryConformance/5-[return-400]-storage.object_acl.list-33 TestRetryConformance/5-[return-400]-storage.object_acl.list-65 TestRetryConformance/5-[return-400]-storage.object_acl.patch-34 TestRetryConformance/5-[return-400]-storage.object_acl.patch-66 -TestRetryConformance/5-[return-400]-storage.objects.compose-35 +TestRetryConformance/5-[return-400]-storage.objects.compose-241 TestRetryConformance/5-[return-400]-storage.objects.delete-36 TestRetryConformance/5-[return-400]-storage.objects.delete-67 TestRetryConformance/5-[return-400]-storage.objects.get-107 @@ -396,7 +399,7 @@ TestRetryConformance/5-[return-401]-storage.object_acl.list-33 TestRetryConformance/5-[return-401]-storage.object_acl.list-65 TestRetryConformance/5-[return-401]-storage.object_acl.patch-34 TestRetryConformance/5-[return-401]-storage.object_acl.patch-66 -TestRetryConformance/5-[return-401]-storage.objects.compose-35 +TestRetryConformance/5-[return-401]-storage.objects.compose-241 TestRetryConformance/5-[return-401]-storage.objects.delete-36 TestRetryConformance/5-[return-401]-storage.objects.delete-67 TestRetryConformance/5-[return-401]-storage.objects.get-107 @@ -467,6 +470,7 @@ TestRetryConformance/6-[return-503_return-400]-storage.object_acl.get-31 TestRetryConformance/6-[return-503_return-400]-storage.object_acl.get-63 TestRetryConformance/6-[return-503_return-400]-storage.object_acl.list-33 TestRetryConformance/6-[return-503_return-400]-storage.object_acl.list-65 +TestRetryConformance/6-[return-503_return-400]-storage.objects.compose-35 TestRetryConformance/6-[return-503_return-400]-storage.objects.delete-37 TestRetryConformance/6-[return-503_return-400]-storage.objects.delete-38 TestRetryConformance/6-[return-503_return-400]-storage.objects.delete-68 @@ -523,6 +527,7 @@ TestRetryConformance/6-[return-reset-connection_return-401]-storage.object_acl.g TestRetryConformance/6-[return-reset-connection_return-401]-storage.object_acl.get-63 TestRetryConformance/6-[return-reset-connection_return-401]-storage.object_acl.list-33 TestRetryConformance/6-[return-reset-connection_return-401]-storage.object_acl.list-65 +TestRetryConformance/6-[return-reset-connection_return-401]-storage.objects.compose-35 TestRetryConformance/6-[return-reset-connection_return-401]-storage.objects.delete-37 TestRetryConformance/6-[return-reset-connection_return-401]-storage.objects.delete-38 TestRetryConformance/6-[return-reset-connection_return-401]-storage.objects.delete-68