diff --git a/temporal-serviceclient/src/main/java/io/temporal/serviceclient/CloudServiceStubsOptions.java b/temporal-serviceclient/src/main/java/io/temporal/serviceclient/CloudServiceStubsOptions.java index 2377db05f1..31b85e72ed 100644 --- a/temporal-serviceclient/src/main/java/io/temporal/serviceclient/CloudServiceStubsOptions.java +++ b/temporal-serviceclient/src/main/java/io/temporal/serviceclient/CloudServiceStubsOptions.java @@ -58,6 +58,11 @@ public int hashCode() { return Objects.hash(super.hashCode(), version); } + @Override + public String toString() { + return "CloudServiceStubsOptions{" + toStringFields() + ", version='" + version + '\'' + '}'; + } + /** Builder is the builder for ClientOptions. */ public static class Builder extends ServiceStubsOptions.Builder { private String version; diff --git a/temporal-serviceclient/src/main/java/io/temporal/serviceclient/OperatorServiceStubsOptions.java b/temporal-serviceclient/src/main/java/io/temporal/serviceclient/OperatorServiceStubsOptions.java index 92cc42a751..5551aab0f5 100644 --- a/temporal-serviceclient/src/main/java/io/temporal/serviceclient/OperatorServiceStubsOptions.java +++ b/temporal-serviceclient/src/main/java/io/temporal/serviceclient/OperatorServiceStubsOptions.java @@ -20,6 +20,11 @@ private OperatorServiceStubsOptions(ServiceStubsOptions serviceStubsOptions) { super(serviceStubsOptions); } + @Override + public String toString() { + return "OperatorServiceStubsOptions{" + toStringFields() + '}'; + } + /** Builder is the builder for ClientOptions. */ public static class Builder extends ServiceStubsOptions.Builder { 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..801bb2b166 100644 --- a/temporal-serviceclient/src/main/java/io/temporal/serviceclient/RpcRetryOptions.java +++ b/temporal-serviceclient/src/main/java/io/temporal/serviceclient/RpcRetryOptions.java @@ -489,7 +489,7 @@ public String toString() { return "RetryOptions{" + "initialInterval=" + initialInterval - + "congestionInitialInterval=" + + ", congestionInitialInterval=" + congestionInitialInterval + ", backoffCoefficient=" + backoffCoefficient diff --git a/temporal-serviceclient/src/main/java/io/temporal/serviceclient/ServiceStubsOptions.java b/temporal-serviceclient/src/main/java/io/temporal/serviceclient/ServiceStubsOptions.java index 4161dca7f3..ebc1d58474 100644 --- a/temporal-serviceclient/src/main/java/io/temporal/serviceclient/ServiceStubsOptions.java +++ b/temporal-serviceclient/src/main/java/io/temporal/serviceclient/ServiceStubsOptions.java @@ -413,8 +413,16 @@ public int hashCode() { @Override public String toString() { - return "ServiceStubsOptions{" - + "channel=" + return "ServiceStubsOptions{" + toStringFields() + '}'; + } + + /** + * Renders the fields declared on this class as a comma separated list, without the enclosing type + * name or braces. Subclasses use this to include inherited fields in their own {@link + * #toString()} instead of dropping them. + */ + String toStringFields() { + return "channel=" + channel + ", target='" + target @@ -423,6 +431,8 @@ public String toString() { + channelInitializer + ", enableHttps=" + enableHttps + + ", apiKeyProvided=" + + apiKeyProvided + ", sslContext=" + sslContext + ", healthCheckAttemptTimeout=" @@ -445,8 +455,10 @@ public String toString() { + connectionBackoffResetFrequency + ", grpcReconnectFrequency=" + grpcReconnectFrequency - + ", headers=" - + headers + // Only the header names are rendered. Values are omitted because they routinely carry + // credentials, for example an Authorization header set through setHeaders. + + ", headerNames=" + + (headers == null ? null : headers.keys()) + ", grpcMetadataProviders=" + grpcMetadataProviders + ", grpcClientInterceptors=" @@ -454,8 +466,7 @@ public String toString() { + ", metricsScope=" + metricsScope + ", grpcCompression=" - + grpcCompression - + '}'; + + grpcCompression; } public static class Builder> { diff --git a/temporal-serviceclient/src/main/java/io/temporal/serviceclient/WorkflowServiceStubsOptions.java b/temporal-serviceclient/src/main/java/io/temporal/serviceclient/WorkflowServiceStubsOptions.java index f22a2ac67e..2f89349dfc 100644 --- a/temporal-serviceclient/src/main/java/io/temporal/serviceclient/WorkflowServiceStubsOptions.java +++ b/temporal-serviceclient/src/main/java/io/temporal/serviceclient/WorkflowServiceStubsOptions.java @@ -137,7 +137,8 @@ public int hashCode() { @Override public String toString() { return "WorkflowServiceStubsOptions{" - + "disableHealthCheck=" + + toStringFields() + + ", disableHealthCheck=" + disableHealthCheck + ", rpcLongPollTimeout=" + rpcLongPollTimeout 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..0887c00a29 100644 --- a/temporal-serviceclient/src/test/java/io/temporal/serviceclient/RpcRetryOptionsTest.java +++ b/temporal-serviceclient/src/test/java/io/temporal/serviceclient/RpcRetryOptionsTest.java @@ -1,6 +1,7 @@ package io.temporal.serviceclient; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import java.time.Duration; import org.junit.Test; @@ -27,4 +28,20 @@ public void setRetryOptionsMergesCongestionInitialInterval() { assertEquals(Duration.ofMillis(200), merged.getInitialInterval()); assertEquals(Duration.ofSeconds(7), merged.getCongestionInitialInterval()); } + + /** + * toString omitted the separator before congestionInitialInterval, so the two durations ran + * together as a single unreadable token. + */ + @Test + public void toStringSeparatesInitialAndCongestionInterval() { + String rendered = + RpcRetryOptions.newBuilder() + .setInitialInterval(Duration.ofMillis(100)) + .setCongestionInitialInterval(Duration.ofSeconds(1)) + .validateBuildWithDefaults() + .toString(); + + assertTrue(rendered, rendered.contains(", congestionInitialInterval=")); + } } diff --git a/temporal-serviceclient/src/test/java/io/temporal/serviceclient/ServiceStubsOptionsTest.java b/temporal-serviceclient/src/test/java/io/temporal/serviceclient/ServiceStubsOptionsTest.java index 0c0e76d745..7a42cf0a28 100644 --- a/temporal-serviceclient/src/test/java/io/temporal/serviceclient/ServiceStubsOptionsTest.java +++ b/temporal-serviceclient/src/test/java/io/temporal/serviceclient/ServiceStubsOptionsTest.java @@ -2,6 +2,7 @@ import static org.junit.Assert.*; +import io.grpc.Metadata; import org.junit.Test; public class ServiceStubsOptionsTest { @@ -177,4 +178,94 @@ public void testGrpcCompressionNonePassesThroughBuilderCopy() { assertEquals(GrpcCompression.NONE, copied.getGrpcCompression()); } + + @Test + public void testWorkflowServiceStubsOptionsToStringIncludesInheritedFields() { + WorkflowServiceStubsOptions options = + WorkflowServiceStubsOptions.newBuilder() + .setTarget("localhost:7233") + .validateAndBuildWithDefaults(); + + String rendered = options.toString(); + + assertTrue(rendered.startsWith("WorkflowServiceStubsOptions{")); + // Inherited fields used to be dropped entirely. + assertTrue(rendered, rendered.contains("target='localhost:7233'")); + assertTrue(rendered, rendered.contains("enableHttps=")); + assertTrue(rendered, rendered.contains("rpcTimeout=")); + assertTrue(rendered, rendered.contains("grpcCompression=")); + // Fields declared on the subclass are still present. + assertTrue(rendered, rendered.contains("disableHealthCheck=")); + assertTrue(rendered, rendered.contains("rpcLongPollTimeout=")); + // Inherited fields are inlined, not nested inside a second wrapper. + assertFalse(rendered, rendered.contains("{ServiceStubsOptions{")); + } + + @Test + public void testOperatorServiceStubsOptionsToString() { + OperatorServiceStubsOptions options = + OperatorServiceStubsOptions.newBuilder() + .setTarget("localhost:7233") + .validateAndBuildWithDefaults(); + + String rendered = options.toString(); + + assertTrue(rendered.startsWith("OperatorServiceStubsOptions{")); + assertTrue(rendered, rendered.contains("target='localhost:7233'")); + assertTrue(rendered, rendered.contains("rpcTimeout=")); + } + + @Test + public void testCloudServiceStubsOptionsToStringIncludesVersion() { + CloudServiceStubsOptions options = + CloudServiceStubsOptions.newBuilder() + .setTarget("localhost:7233") + .setVersion("v1") + .validateAndBuildWithDefaults(); + + String rendered = options.toString(); + + assertTrue(rendered.startsWith("CloudServiceStubsOptions{")); + assertTrue(rendered, rendered.contains("target='localhost:7233'")); + assertTrue(rendered, rendered.contains("version='v1'")); + } + + @Test + public void testToStringDoesNotLeakApiKey() { + WorkflowServiceStubsOptions options = + WorkflowServiceStubsOptions.newBuilder() + .setTarget("localhost:7233") + .addApiKey(() -> "super-secret-api-key") + .validateAndBuildWithDefaults(); + + String rendered = options.toString(); + + assertFalse(rendered, rendered.contains("super-secret-api-key")); + // The fact that an API key was configured is still useful when debugging. + assertTrue(rendered, rendered.contains("apiKeyProvided=true")); + } + + @Test + public void testToStringRendersHeaderNamesWithoutValues() { + Metadata headers = new Metadata(); + headers.put( + Metadata.Key.of("authorization", Metadata.ASCII_STRING_MARSHALLER), + "Bearer super-secret-token"); + headers.put(Metadata.Key.of("x-custom", Metadata.ASCII_STRING_MARSHALLER), "plain-value"); + + WorkflowServiceStubsOptions options = + WorkflowServiceStubsOptions.newBuilder() + .setTarget("localhost:7233") + .setHeaders(headers) + .validateAndBuildWithDefaults(); + + String rendered = options.toString(); + + // Metadata.toString renders values in the clear, so it must not be embedded directly. + assertFalse(rendered, rendered.contains("super-secret-token")); + assertFalse(rendered, rendered.contains("plain-value")); + // Header names are still reported, which is what makes the output useful for debugging. + assertTrue(rendered, rendered.contains("authorization")); + assertTrue(rendered, rendered.contains("x-custom")); + } }