diff --git a/CHANGELOG.md b/CHANGELOG.md index a390056d46..e3bb6e423d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -78,6 +78,13 @@ to docs, or any other relevant information. specific to the Temporal CLI dev server implementation and may no longer be valid if that implementation changes. ### Fixed +- Fixed workflow task completion pagination, which threw `UnsupportedOperationException` before + sending the first page. A completion that needed pagination timed out on every attempt instead + of being sent. The same fix lets `RpcRetryOptions.newBuilder(options).addDoNotRetry(...)` extend + existing options, which also threw `UnsupportedOperationException`. +- `RpcRetryOptions` no longer changes when the builder that created it is reused. Previously, + `addDoNotRetry` calls made after `build()` or `validateBuildWithDefaults()` also appeared in + the options already built. - `ActivityClient` now sends the `temporal-namespace` gRPC header like `WorkflowClient` and `ScheduleClient` do, so standalone activity calls are no longer rejected with `PERMISSION_DENIED` by namespace-scoped auth (e.g. Temporal Cloud API keys). diff --git a/temporal-serviceclient/src/main/java/io/temporal/serviceclient/RpcRetryOptions.java b/temporal-serviceclient/src/main/java/io/temporal/serviceclient/RpcRetryOptions.java index bddcca5a14..61c874b82c 100644 --- a/temporal-serviceclient/src/main/java/io/temporal/serviceclient/RpcRetryOptions.java +++ b/temporal-serviceclient/src/main/java/io/temporal/serviceclient/RpcRetryOptions.java @@ -95,7 +95,7 @@ private Builder(RpcRetryOptions options) { this.congestionInitialInterval = options.getCongestionInitialInterval(); this.maximumInterval = options.getMaximumInterval(); this.maximumJitterCoefficient = options.getMaximumJitterCoefficient(); - this.doNotRetry = options.getDoNotRetry(); + this.doNotRetry = new ArrayList<>(options.getDoNotRetry()); } /** @@ -413,7 +413,7 @@ private RpcRetryOptions( this.maximumAttempts = maximumAttempts; this.maximumInterval = maximumInterval; this.maximumJitterCoefficient = maximumJitterCoefficient; - this.doNotRetry = Collections.unmodifiableList(doNotRetry); + this.doNotRetry = Collections.unmodifiableList(new ArrayList<>(doNotRetry)); } public Duration getInitialInterval() { diff --git a/temporal-serviceclient/src/test/java/io/temporal/serviceclient/RpcRetryOptionsTest.java b/temporal-serviceclient/src/test/java/io/temporal/serviceclient/RpcRetryOptionsTest.java index 8b66a2ded3..56c8e89aab 100644 --- a/temporal-serviceclient/src/test/java/io/temporal/serviceclient/RpcRetryOptionsTest.java +++ b/temporal-serviceclient/src/test/java/io/temporal/serviceclient/RpcRetryOptionsTest.java @@ -2,7 +2,12 @@ import static org.junit.Assert.assertEquals; +import io.grpc.Status; import java.time.Duration; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; import org.junit.Test; public class RpcRetryOptionsTest { @@ -27,4 +32,42 @@ public void setRetryOptionsMergesCongestionInitialInterval() { assertEquals(Duration.ofMillis(200), merged.getInitialInterval()); assertEquals(Duration.ofSeconds(7), merged.getCongestionInitialInterval()); } + + @Test + public void aBuilderCopiedFromOptionsCanAddDoNotRetryItems() { + RpcRetryOptions source = + RpcRetryOptions.newBuilder() + .addDoNotRetry(Status.Code.NOT_FOUND, null) + .validateBuildWithDefaults(); + + RpcRetryOptions copy = + RpcRetryOptions.newBuilder(source) + .addDoNotRetry(Status.Code.ABORTED, null) + .validateBuildWithDefaults(); + + assertEquals(Arrays.asList(Status.Code.NOT_FOUND, Status.Code.ABORTED), codes(copy)); + assertEquals(Collections.singletonList(Status.Code.NOT_FOUND), codes(source)); + } + + @Test + public void reusingABuilderDoesNotChangeOptionsItAlreadyBuilt() { + RpcRetryOptions.Builder builder = + RpcRetryOptions.newBuilder().addDoNotRetry(Status.Code.NOT_FOUND, null); + RpcRetryOptions validated = builder.validateBuildWithDefaults(); + RpcRetryOptions built = builder.build(); + + builder.addDoNotRetry(Status.Code.ABORTED, null); + + assertEquals(Collections.singletonList(Status.Code.NOT_FOUND), codes(validated)); + assertEquals(Collections.singletonList(Status.Code.NOT_FOUND), codes(built)); + assertEquals(Arrays.asList(Status.Code.NOT_FOUND, Status.Code.ABORTED), codes(builder.build())); + } + + private static List codes(RpcRetryOptions options) { + List codes = new ArrayList<>(); + for (RpcRetryOptions.DoNotRetryItem item : options.getDoNotRetry()) { + codes.add(item.getCode()); + } + return codes; + } } diff --git a/temporal-testing/src/main/java/io/temporal/testing/internal/devserver/SdkJavaTestServerProfile.java b/temporal-testing/src/main/java/io/temporal/testing/internal/devserver/SdkJavaTestServerProfile.java index 37073eb4bf..decae29151 100644 --- a/temporal-testing/src/main/java/io/temporal/testing/internal/devserver/SdkJavaTestServerProfile.java +++ b/temporal-testing/src/main/java/io/temporal/testing/internal/devserver/SdkJavaTestServerProfile.java @@ -150,7 +150,13 @@ private static List repositoryServerArguments() { "--dynamic-config-value", "frontend.workerCommandsEnabled=true", "--dynamic-config-value", - "system.enableCancelActivityWorkerCommand=true"); + "system.enableCancelActivityWorkerCommand=true", + "--dynamic-config-value", + "history.enableWorkflowTaskCompletionPagination=true", + // Pagination clears the gRPC request limit, but the recombined completion still persists as + // one transaction, so the persistence limit must be above it. + "--dynamic-config-value", + "system.transactionSizeLimit=33554432"); } private static File workingDirectory() {