From ae471b32e27d7d906e3137044d02d0d18df78bef Mon Sep 17 00:00:00 2001 From: Luwei Ge Date: Sat, 1 Mar 2025 00:40:34 +0000 Subject: [PATCH 01/10] remove call credentials from call options if DirectPath --- .../google/api/gax/grpc/GrpcCallContext.java | 51 ++++++++++++++++++- 1 file changed, 49 insertions(+), 2 deletions(-) diff --git a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java index 137060595d..69c5296b09 100644 --- a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java +++ b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java @@ -96,6 +96,7 @@ public final class GrpcCallContext implements ApiCallContext { private final ImmutableMap> extraHeaders; private final ApiCallContextOptions options; private final EndpointContext endpointContext; + private final boolean isDirectPath; /** Returns an empty instance with a null channel and default {@link CallOptions}. */ public static GrpcCallContext createDefault() { @@ -144,6 +145,36 @@ private GrpcCallContext( @Nullable RetrySettings retrySettings, @Nullable Set retryableCodes, @Nullable EndpointContext endpointContext) { + this( + channel, + credentials, + callOptions, + timeout, + streamWaitTimeout, + streamIdleTimeout, + channelAffinity, + extraHeaders, + options, + retrySettings, + retryableCodes, + endpointContext, + false); + } + + private GrpcCallContext( + Channel channel, + @Nullable Credentials credentials, + CallOptions callOptions, + @Nullable java.time.Duration timeout, + @Nullable java.time.Duration streamWaitTimeout, + @Nullable java.time.Duration streamIdleTimeout, + @Nullable Integer channelAffinity, + ImmutableMap> extraHeaders, + ApiCallContextOptions options, + @Nullable RetrySettings retrySettings, + @Nullable Set retryableCodes, + @Nullable EndpointContext endpointContext, + boolean isDirectPath) { this.channel = channel; this.credentials = credentials; this.callOptions = Preconditions.checkNotNull(callOptions); @@ -159,6 +190,7 @@ private GrpcCallContext( // a valid EndpointContext with user configurations after the client has been initialized. this.endpointContext = endpointContext == null ? EndpointContext.getDefaultInstance() : endpointContext; + this.isDirectPath = isDirectPath; } /** @@ -210,7 +242,20 @@ public GrpcCallContext withTransportChannel(TransportChannel inputChannel) { "Expected GrpcTransportChannel, got " + inputChannel.getClass().getName()); } GrpcTransportChannel transportChannel = (GrpcTransportChannel) inputChannel; - return withChannel(transportChannel.getChannel()); + return new GrpcCallContext( + transportChannel.getChannel(), + credentials, + callOptions, + timeout, + streamWaitTimeout, + streamIdleTimeout, + channelAffinity, + extraHeaders, + options, + retrySettings, + retryableCodes, + endpointContext, + transportChannel.isDirectPath()); } @Override @@ -535,7 +580,9 @@ public Channel getChannel() { /** The {@link CallOptions} set on this context. */ public CallOptions getCallOptions() { - return callOptions; + if (!isDirectPath) return callOptions; + // Remove the CallCredentials attached to the callOptions if it's DirectPath. + return callOptions.withCallCredentials(null); } /** This method is obsolete. Use {@link #getStreamWaitTimeoutDuration()} instead. */ From 39489bc42ee5a36f87c7da18e3e82fdc6e3629d0 Mon Sep 17 00:00:00 2001 From: Luwei Ge Date: Sat, 1 Mar 2025 01:14:26 +0000 Subject: [PATCH 02/10] fix ctor issue --- .../google/api/gax/grpc/GrpcCallContext.java | 72 ++++++++----------- 1 file changed, 29 insertions(+), 43 deletions(-) diff --git a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java index 69c5296b09..236dfe1322 100644 --- a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java +++ b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java @@ -112,7 +112,8 @@ public static GrpcCallContext createDefault() { ApiCallContextOptions.getDefaultOptions(), null, null, - null); + null, + false); } /** Returns an instance with the given channel and {@link CallOptions}. */ @@ -129,35 +130,7 @@ public static GrpcCallContext of(Channel channel, CallOptions callOptions) { ApiCallContextOptions.getDefaultOptions(), null, null, - null); - } - - private GrpcCallContext( - Channel channel, - @Nullable Credentials credentials, - CallOptions callOptions, - @Nullable java.time.Duration timeout, - @Nullable java.time.Duration streamWaitTimeout, - @Nullable java.time.Duration streamIdleTimeout, - @Nullable Integer channelAffinity, - ImmutableMap> extraHeaders, - ApiCallContextOptions options, - @Nullable RetrySettings retrySettings, - @Nullable Set retryableCodes, - @Nullable EndpointContext endpointContext) { - this( - channel, - credentials, - callOptions, - timeout, - streamWaitTimeout, - streamIdleTimeout, - channelAffinity, - extraHeaders, - options, - retrySettings, - retryableCodes, - endpointContext, + null, false); } @@ -231,7 +204,8 @@ public GrpcCallContext withCredentials(Credentials newCredentials) { options, retrySettings, retryableCodes, - endpointContext); + endpointContext, + isDirectPath); } @Override @@ -273,7 +247,8 @@ public GrpcCallContext withEndpointContext(EndpointContext endpointContext) { options, retrySettings, retryableCodes, - endpointContext); + endpointContext, + isDirectPath); } /** This method is obsolete. Use {@link #withTimeoutDuration(java.time.Duration)} instead. */ @@ -307,7 +282,8 @@ public GrpcCallContext withTimeoutDuration(@Nullable java.time.Duration timeout) options, retrySettings, retryableCodes, - endpointContext); + endpointContext, + isDirectPath); } /** This method is obsolete. Use {@link #getTimeoutDuration()} instead. */ @@ -355,7 +331,8 @@ public GrpcCallContext withStreamWaitTimeoutDuration( options, retrySettings, retryableCodes, - endpointContext); + endpointContext, + isDirectPath); } /** @@ -389,7 +366,8 @@ public GrpcCallContext withStreamIdleTimeoutDuration( options, retrySettings, retryableCodes, - endpointContext); + endpointContext, + isDirectPath); } @BetaApi("The surface for channel affinity is not stable yet and may change in the future.") @@ -406,7 +384,8 @@ public GrpcCallContext withChannelAffinity(@Nullable Integer affinity) { options, retrySettings, retryableCodes, - endpointContext); + endpointContext, + isDirectPath); } @BetaApi("The surface for extra headers is not stable yet and may change in the future.") @@ -427,7 +406,8 @@ public GrpcCallContext withExtraHeaders(Map> extraHeaders) options, retrySettings, retryableCodes, - endpointContext); + endpointContext, + isDirectPath); } @Override @@ -449,7 +429,8 @@ public GrpcCallContext withRetrySettings(RetrySettings retrySettings) { options, retrySettings, retryableCodes, - endpointContext); + endpointContext, + isDirectPath); } @Override @@ -471,7 +452,8 @@ public GrpcCallContext withRetryableCodes(Set retryableCodes) { options, retrySettings, retryableCodes, - endpointContext); + endpointContext, + isDirectPath); } @Override @@ -570,7 +552,8 @@ public ApiCallContext merge(ApiCallContext inputCallContext) { newOptions, newRetrySettings, newRetryableCodes, - endpointContext); + endpointContext, + isDirectPath); } /** The {@link Channel} set on this context. */ @@ -649,7 +632,8 @@ public GrpcCallContext withChannel(Channel newChannel) { options, retrySettings, retryableCodes, - endpointContext); + endpointContext, + isDirectPath); } /** Returns a new instance with the call options set to the given call options. */ @@ -666,7 +650,8 @@ public GrpcCallContext withCallOptions(CallOptions newCallOptions) { options, retrySettings, retryableCodes, - endpointContext); + endpointContext, + isDirectPath); } public GrpcCallContext withRequestParamsDynamicHeaderOption(String requestParams) { @@ -710,7 +695,8 @@ public GrpcCallContext withOption(Key key, T value) { newOptions, retrySettings, retryableCodes, - endpointContext); + endpointContext, + isDirectPath); } /** {@inheritDoc} */ From f7244a8c00ffc67cfae185520f492034c4cb8f41 Mon Sep 17 00:00:00 2001 From: Luwei Ge Date: Mon, 3 Mar 2025 23:09:47 +0000 Subject: [PATCH 03/10] add tests --- .../com/google/api/gax/grpc/GrpcCallContext.java | 3 ++- .../google/api/gax/grpc/GrpcCallContextTest.java | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java index 236dfe1322..25b108be19 100644 --- a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java +++ b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java @@ -633,7 +633,8 @@ public GrpcCallContext withChannel(Channel newChannel) { retrySettings, retryableCodes, endpointContext, - isDirectPath); + // Defaults to false again since we cannot tell whether the channel is DirectPath. + false); } /** Returns a new instance with the call options set to the given call options. */ diff --git a/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/GrpcCallContextTest.java b/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/GrpcCallContextTest.java index 63de03a88a..32280671e4 100644 --- a/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/GrpcCallContextTest.java +++ b/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/GrpcCallContextTest.java @@ -100,6 +100,21 @@ void testWithTransportChannelWrongType() { } } + @Test + void testWithTransportChannelIsDirectcPath() { + ManagedChannel channel = Mockito.mock(ManagedChannel.class); + Credentials credentials = Mockito.mock(Credentials.class); + GrpcCallContext context = GrpcCallContext.createDefault().withCredentials(null); + assertNotNull(context.getCallOptions().getCredentials()); + context = + context.withTransportChannel( + GrpcTransportChannel.newBuilder() + .setDirectPath(true) + .setManagedChannel(channel) + .build()); + assertNull(context.getCallOptions().getCredentials()); + } + @Test void testMergeWrongType() { try { From 618c321e9882ac9e5e4ddd3add6dd0f851c6b81d Mon Sep 17 00:00:00 2001 From: Luwei Ge Date: Mon, 3 Mar 2025 23:15:01 +0000 Subject: [PATCH 04/10] fix tests --- .../test/java/com/google/api/gax/grpc/GrpcCallContextTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/GrpcCallContextTest.java b/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/GrpcCallContextTest.java index 32280671e4..a43b22ea4b 100644 --- a/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/GrpcCallContextTest.java +++ b/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/GrpcCallContextTest.java @@ -104,7 +104,7 @@ void testWithTransportChannelWrongType() { void testWithTransportChannelIsDirectcPath() { ManagedChannel channel = Mockito.mock(ManagedChannel.class); Credentials credentials = Mockito.mock(Credentials.class); - GrpcCallContext context = GrpcCallContext.createDefault().withCredentials(null); + GrpcCallContext context = GrpcCallContext.createDefault().withCredentials(credentials); assertNotNull(context.getCallOptions().getCredentials()); context = context.withTransportChannel( From 4c739eacfdf1067adf2080dfed811d8e46f31fab Mon Sep 17 00:00:00 2001 From: Luwei Ge Date: Mon, 3 Mar 2025 23:20:45 +0000 Subject: [PATCH 05/10] fix tests --- .../test/java/com/google/api/gax/grpc/GrpcCallContextTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/GrpcCallContextTest.java b/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/GrpcCallContextTest.java index a43b22ea4b..48af758ba5 100644 --- a/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/GrpcCallContextTest.java +++ b/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/GrpcCallContextTest.java @@ -42,6 +42,7 @@ import com.google.api.gax.rpc.testing.FakeTransportChannel; import com.google.api.gax.tracing.ApiTracer; import com.google.auth.Credentials; +import com.google.auth.oauth2.GoogleCredentials; import com.google.common.collect.ImmutableMap; import com.google.common.truth.Truth; import io.grpc.CallOptions; @@ -103,7 +104,7 @@ void testWithTransportChannelWrongType() { @Test void testWithTransportChannelIsDirectcPath() { ManagedChannel channel = Mockito.mock(ManagedChannel.class); - Credentials credentials = Mockito.mock(Credentials.class); + Credentials credentials = Mockito.mock(GoogleCredentials.class); GrpcCallContext context = GrpcCallContext.createDefault().withCredentials(credentials); assertNotNull(context.getCallOptions().getCredentials()); context = From 11ce53e933f27aacc2fb0ef9a705bf6bdab7c0ad Mon Sep 17 00:00:00 2001 From: Luwei Ge Date: Mon, 3 Mar 2025 23:26:47 +0000 Subject: [PATCH 06/10] fix typo and slightly more coverage --- .../java/com/google/api/gax/grpc/GrpcCallContextTest.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/GrpcCallContextTest.java b/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/GrpcCallContextTest.java index 48af758ba5..4a2a41aaae 100644 --- a/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/GrpcCallContextTest.java +++ b/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/GrpcCallContextTest.java @@ -102,7 +102,7 @@ void testWithTransportChannelWrongType() { } @Test - void testWithTransportChannelIsDirectcPath() { + void testWithTransportChannelIsDirectPath() { ManagedChannel channel = Mockito.mock(ManagedChannel.class); Credentials credentials = Mockito.mock(GoogleCredentials.class); GrpcCallContext context = GrpcCallContext.createDefault().withCredentials(credentials); @@ -114,6 +114,10 @@ void testWithTransportChannelIsDirectcPath() { .setManagedChannel(channel) .build()); assertNull(context.getCallOptions().getCredentials()); + + // This should revert isDirectPath to false. + context = context.withChannel(channel); + assertNotNull(context.getCallOptions().getCredentials()); } @Test From 19920fa1dd01882d81a49da3101089ec59e83bb2 Mon Sep 17 00:00:00 2001 From: Luwei Ge Date: Wed, 5 Mar 2025 18:08:50 +0000 Subject: [PATCH 07/10] make the getter dumb --- .../google/api/gax/grpc/GrpcCallContext.java | 20 ++++++++------ .../api/gax/grpc/GrpcCallContextTest.java | 26 +++++++++++++++++++ 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java index 25b108be19..94be3c840f 100644 --- a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java +++ b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java @@ -195,7 +195,7 @@ public GrpcCallContext withCredentials(Credentials newCredentials) { return new GrpcCallContext( channel, newCredentials, - callOptions.withCallCredentials(callCredentials), + isDirectPath ? callOptions : callOptions.withCallCredentials(callCredentials), timeout, streamWaitTimeout, streamIdleTimeout, @@ -483,8 +483,11 @@ public ApiCallContext merge(ApiCallContext inputCallContext) { newDeadline = callOptions.getDeadline(); } + boolean newIsDirectPath = grpcCallContext.isDirectPath; + CallCredentials newCallCredentials = grpcCallContext.callOptions.getCredentials(); - if (newCallCredentials == null) { + // If newIsDirectPath is true, then newChannel is guanrateed to be not null. + if (newCallCredentials == null && !newIsDirectPath) { newCallCredentials = callOptions.getCredentials(); } @@ -553,7 +556,7 @@ public ApiCallContext merge(ApiCallContext inputCallContext) { newRetrySettings, newRetryableCodes, endpointContext, - isDirectPath); + newIsDirectPath); } /** The {@link Channel} set on this context. */ @@ -563,9 +566,7 @@ public Channel getChannel() { /** The {@link CallOptions} set on this context. */ public CallOptions getCallOptions() { - if (!isDirectPath) return callOptions; - // Remove the CallCredentials attached to the callOptions if it's DirectPath. - return callOptions.withCallCredentials(null); + return callOptions; } /** This method is obsolete. Use {@link #getStreamWaitTimeoutDuration()} instead. */ @@ -623,7 +624,10 @@ public GrpcCallContext withChannel(Channel newChannel) { return new GrpcCallContext( newChannel, credentials, - callOptions, + // Attach back the credentials to callOptions since we default to non-DirectPath. + credentials != null + ? callOptions.withCallCredentials(MoreCallCredentials.from(credentials)) + : callOptions, timeout, streamWaitTimeout, streamIdleTimeout, @@ -642,7 +646,7 @@ public GrpcCallContext withCallOptions(CallOptions newCallOptions) { return new GrpcCallContext( channel, credentials, - newCallOptions, + isDirectPath ? newCallOptions.withCallCredentials(null) : newCallOptions, timeout, streamWaitTimeout, streamIdleTimeout, diff --git a/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/GrpcCallContextTest.java b/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/GrpcCallContextTest.java index 4a2a41aaae..73ae212a36 100644 --- a/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/GrpcCallContextTest.java +++ b/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/GrpcCallContextTest.java @@ -45,9 +45,11 @@ import com.google.auth.oauth2.GoogleCredentials; import com.google.common.collect.ImmutableMap; import com.google.common.truth.Truth; +import io.grpc.CallCredentials; import io.grpc.CallOptions; import io.grpc.ManagedChannel; import io.grpc.Metadata.Key; +import io.grpc.auth.MoreCallCredentials; import java.io.IOException; import java.util.ArrayList; import java.util.Collections; @@ -115,6 +117,11 @@ void testWithTransportChannelIsDirectPath() { .build()); assertNull(context.getCallOptions().getCredentials()); + // Call credentials from the call options will be stripped. + context.withCallOptions( + CallOptions.DEFAULT.withCallCredentials(MoreCallCredentials.from(credentials))); + assertNull(context.getCallOptions().getCredentials()); + // This should revert isDirectPath to false. context = context.withChannel(channel); assertNotNull(context.getCallOptions().getCredentials()); @@ -340,6 +347,25 @@ void testMergeWithCustomCallOptions() { .isEqualTo(ctx2.getCallOptions().getOption(key)); } + @Test + void testMergeWithIsDirectPath() { + ManagedChannel channel = Mockito.mock(ManagedChannel.class); + CallCredentials callCredentials = Mockito.mock(CallCredentials.class); + GrpcCallContext ctx1 = + GrpcCallContext.createDefault() + .withCallOptions(CallOptions.DEFAULT.withCallCredentials(callCredentials)); + GrpcCallContext ctx2 = + GrpcCallContext.createDefault() + .withTransportChannel( + GrpcTransportChannel.newBuilder() + .setDirectPath(true) + .setManagedChannel(channel) + .build()); + + GrpcCallContext merged = (GrpcCallContext) ctx1.merge(ctx2); + assertNull(merged.getCallOptions().getCredentials()); + } + @Test void testWithExtraHeaders() { Map> extraHeaders = From 255dceed18725107344d2353dfb7468018438867 Mon Sep 17 00:00:00 2001 From: Luwei Ge Date: Wed, 5 Mar 2025 18:15:41 +0000 Subject: [PATCH 08/10] fix tests --- .../src/main/java/com/google/api/gax/grpc/GrpcCallContext.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java index 94be3c840f..5821a028b5 100644 --- a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java +++ b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java @@ -219,7 +219,7 @@ public GrpcCallContext withTransportChannel(TransportChannel inputChannel) { return new GrpcCallContext( transportChannel.getChannel(), credentials, - callOptions, + transportChannel.isDirectPath() ? callOptions.withCallCredentials(null) : callOptions, timeout, streamWaitTimeout, streamIdleTimeout, From f8232caead014b8715bfffa5f074c89c79579991 Mon Sep 17 00:00:00 2001 From: Luwei Ge Date: Wed, 5 Mar 2025 20:34:31 +0000 Subject: [PATCH 09/10] address comment --- .../com/google/api/gax/grpc/GrpcCallContext.java | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java index 5821a028b5..dd641af569 100644 --- a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java +++ b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java @@ -150,7 +150,10 @@ private GrpcCallContext( boolean isDirectPath) { this.channel = channel; this.credentials = credentials; - this.callOptions = Preconditions.checkNotNull(callOptions); + Preconditions.checkNotNull(callOptions); + // CallCredentials is stripped from CallOptions because CallCredentials are attached + // to ChannelCredentials in DirectPath flows. Adding it again would duplicate the headers. + this.callOptions = isDirectPath ? callOptions.withCallCredentials(null) : callOptions; this.timeout = timeout; this.streamWaitTimeout = streamWaitTimeout; this.streamIdleTimeout = streamIdleTimeout; @@ -195,7 +198,7 @@ public GrpcCallContext withCredentials(Credentials newCredentials) { return new GrpcCallContext( channel, newCredentials, - isDirectPath ? callOptions : callOptions.withCallCredentials(callCredentials), + callOptions.withCallCredentials(callCredentials), timeout, streamWaitTimeout, streamIdleTimeout, @@ -219,7 +222,7 @@ public GrpcCallContext withTransportChannel(TransportChannel inputChannel) { return new GrpcCallContext( transportChannel.getChannel(), credentials, - transportChannel.isDirectPath() ? callOptions.withCallCredentials(null) : callOptions, + callOptions, timeout, streamWaitTimeout, streamIdleTimeout, @@ -624,7 +627,6 @@ public GrpcCallContext withChannel(Channel newChannel) { return new GrpcCallContext( newChannel, credentials, - // Attach back the credentials to callOptions since we default to non-DirectPath. credentials != null ? callOptions.withCallCredentials(MoreCallCredentials.from(credentials)) : callOptions, @@ -646,7 +648,7 @@ public GrpcCallContext withCallOptions(CallOptions newCallOptions) { return new GrpcCallContext( channel, credentials, - isDirectPath ? newCallOptions.withCallCredentials(null) : newCallOptions, + newCallOptions, timeout, streamWaitTimeout, streamIdleTimeout, From 9ac129191ba3306ac53a9b36336be6df59b8b428 Mon Sep 17 00:00:00 2001 From: Luwei Ge Date: Fri, 7 Mar 2025 20:23:31 +0000 Subject: [PATCH 10/10] address comment --- .../com/google/api/gax/grpc/GrpcCallContext.java | 16 ++++++++-------- .../google/api/gax/grpc/GrpcCallContextTest.java | 4 ---- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java index dd641af569..70f70c45c1 100644 --- a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java +++ b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java @@ -489,8 +489,7 @@ public ApiCallContext merge(ApiCallContext inputCallContext) { boolean newIsDirectPath = grpcCallContext.isDirectPath; CallCredentials newCallCredentials = grpcCallContext.callOptions.getCredentials(); - // If newIsDirectPath is true, then newChannel is guanrateed to be not null. - if (newCallCredentials == null && !newIsDirectPath) { + if (newCallCredentials == null) { newCallCredentials = callOptions.getCredentials(); } @@ -622,14 +621,16 @@ public Map> getExtraHeaders() { return extraHeaders; } - /** Returns a new instance with the channel set to the given channel. */ + /** + * This method is obsolete. Use {@link #withTransportChannel()} instead. Returns a new instance + * with the channel set to the given channel. + */ + @ObsoleteApi("Use withTransportChannel() instead") public GrpcCallContext withChannel(Channel newChannel) { return new GrpcCallContext( newChannel, credentials, - credentials != null - ? callOptions.withCallCredentials(MoreCallCredentials.from(credentials)) - : callOptions, + callOptions, timeout, streamWaitTimeout, streamIdleTimeout, @@ -639,8 +640,7 @@ public GrpcCallContext withChannel(Channel newChannel) { retrySettings, retryableCodes, endpointContext, - // Defaults to false again since we cannot tell whether the channel is DirectPath. - false); + isDirectPath); } /** Returns a new instance with the call options set to the given call options. */ diff --git a/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/GrpcCallContextTest.java b/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/GrpcCallContextTest.java index 73ae212a36..5f30ed58c9 100644 --- a/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/GrpcCallContextTest.java +++ b/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/GrpcCallContextTest.java @@ -121,10 +121,6 @@ void testWithTransportChannelIsDirectPath() { context.withCallOptions( CallOptions.DEFAULT.withCallCredentials(MoreCallCredentials.from(credentials))); assertNull(context.getCallOptions().getCredentials()); - - // This should revert isDirectPath to false. - context = context.withChannel(channel); - assertNotNull(context.getCallOptions().getCredentials()); } @Test