Skip to content

Commit 5678ad4

Browse files
macastelazmacastelaz
committed
fix(gax): address PR 13995 AI review findings, Javadoc doclint errors, and upstream test filter typo
1 parent 49772f0 commit 5678ad4

9 files changed

Lines changed: 67 additions & 12 deletions

File tree

‎sdk-platform-java/gax-java/gax-grpc/pom.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@
162162
<artifactId>maven-surefire-plugin</artifactId>
163163
<configuration>
164164
<!-- These tests require an Env Var to be set. Use -PenvVarTest to ONLY run these tests -->
165-
<test>!InstantiatingGrpcChannelProviderTest#testLogDirectPathMisconfig_AttemptDirectPathNotSetAndAttemptDirectPathXdsSetViaEnv_warns,!InstantiatingGrpcChannelProviderTest#canUseDirectPath_directPathEnvVarNotSet_attemptDirectPathIsTrue,InstantiatingGrpcChannelProviderTest#testLogDirectPathMisconfigWrongCredential</test>
165+
<test>!InstantiatingGrpcChannelProviderTest#testLogDirectPathMisconfig_AttemptDirectPathNotSetAndAttemptDirectPathXdsSetViaEnv_warns,!InstantiatingGrpcChannelProviderTest#canUseDirectPath_directPathEnvVarNotSet_attemptDirectPathIsTrue,!InstantiatingGrpcChannelProviderTest#testLogDirectPathMisconfigWrongCredential</test>
166166
<!-- <test>!InstantiatingGrpcChannelProviderTest#testLogDirectPathMisconfig_AttemptDirectPathNotSetAndAttemptDirectPathXdsSetViaEnv_warns</test> -->
167167
</configuration>
168168
</plugin>

‎sdk-platform-java/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/ChannelPool.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -513,6 +513,7 @@ void refresh() {
513513
// replaces the list)
514514
synchronized (entryWriteLock) {
515515
if (workloadCertPath == null) {
516+
refreshAll();
516517
return;
517518
}
518519
String currentDiskFingerprint = getOrUpdateDiskFingerprint(workloadCertPath);

‎sdk-platform-java/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/ChannelPoolTest.java‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -529,6 +529,37 @@ void channelRefreshShouldSwapChannels() throws IOException {
529529
.newCall(Mockito.<MethodDescriptor<String, Integer>>any(), Mockito.any(CallOptions.class));
530530
}
531531

532+
@Test
533+
void testRefreshWithNullWorkloadCertPathSwapsChannel() throws IOException {
534+
ScheduledExecutorService executor =
535+
Mockito.mock(ScheduledExecutorService.class, Mockito.withSettings().withoutAnnotations());
536+
FixedExecutorProvider provider = FixedExecutorProvider.create(executor);
537+
ManagedChannel underlyingChannel1 = Mockito.mock(ManagedChannel.class);
538+
ManagedChannel underlyingChannel2 = Mockito.mock(ManagedChannel.class);
539+
FakeChannelFactory channelFactory =
540+
new FakeChannelFactory(ImmutableList.of(underlyingChannel1, underlyingChannel2));
541+
pool =
542+
new ChannelPool(
543+
ChannelPoolSettings.staticallySized(1).toBuilder()
544+
.setPreemptiveRefreshEnabled(true)
545+
.build(),
546+
channelFactory,
547+
provider,
548+
null);
549+
Mockito.reset(underlyingChannel1);
550+
551+
pool.newCall(FakeMethodDescriptor.<String, Integer>create(), CallOptions.DEFAULT);
552+
Mockito.verify(underlyingChannel1, Mockito.only())
553+
.newCall(Mockito.<MethodDescriptor<String, Integer>>any(), Mockito.any(CallOptions.class));
554+
555+
// Calling refresh() when workloadCertPath is null should fall back to refreshAll()
556+
pool.refresh();
557+
558+
pool.newCall(FakeMethodDescriptor.<String, Integer>create(), CallOptions.DEFAULT);
559+
Mockito.verify(underlyingChannel2, Mockito.only())
560+
.newCall(Mockito.<MethodDescriptor<String, Integer>>any(), Mockito.any(CallOptions.class));
561+
}
562+
532563
@Test
533564
void channelCountShouldNotChangeWhenOutstandingRpcsAreWithinLimits() throws Exception {
534565
ScheduledExecutorService executor =

‎sdk-platform-java/gax-java/gax-httpjson/src/main/java/com/google/api/gax/httpjson/InstantiatingHttpJsonChannelProvider.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,10 @@ public TransportChannelProvider withCredentials(Credentials credentials) {
199199
if (certificateBasedAccess.useMtlsClientCertificate()) {
200200
KeyStore mtlsKeyStore = mtlsProvider.getKeyStore();
201201
if (mtlsKeyStore != null) {
202-
return new NetHttpTransport.Builder().trustCertificates(null, mtlsKeyStore, "").build();
202+
NetHttpTransport.Builder builder = new NetHttpTransport.Builder();
203+
builder.trustCertificates(null, mtlsKeyStore, "");
204+
HttpJsonConscryptUtils.configureConscryptSecurityProvider(builder);
205+
return builder.build();
203206
}
204207
}
205208
return null;

‎sdk-platform-java/gax-java/gax-httpjson/src/main/java/com/google/api/gax/httpjson/ManagedHttpJsonChannel.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,12 @@ private ManagedHttpJsonChannel(
7878
this.executor = executor;
7979
this.usingDefaultExecutor = usingDefaultExecutor;
8080
this.endpoint = endpoint;
81-
this.httpTransport = httpTransport == null ? new NetHttpTransport() : httpTransport;
81+
this.httpTransport =
82+
httpTransport == null
83+
? HttpJsonConscryptUtils.configureConscryptSecurityProvider(
84+
new NetHttpTransport.Builder())
85+
.build()
86+
: httpTransport;
8287
this.usingDefaultTransport = usingDefaultTransport || httpTransport == null;
8388
this.deadlineScheduledExecutorService = Executors.newSingleThreadScheduledExecutor();
8489
}

‎sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/retrying/RetrySettings.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ public final org.threeten.bp.Duration getInitialRpcTimeout() {
189189
* connection has been terminated).
190190
*
191191
* <p>{@link #getTotalTimeout()} caps how long the logic should keep trying the RPC until it gives
192-
* up completely. If {@link #getTotalTimeout()} is set, initialRpcTimeout should be <=
192+
* up completely. If {@link #getTotalTimeout()} is set, initialRpcTimeout should be &lt;=
193193
* totalTimeout.
194194
*
195195
* <p>If there are no configurations, Retries have the default initial RPC timeout value of {@code
@@ -356,7 +356,7 @@ public final Builder setInitialRpcTimeout(org.threeten.bp.Duration initialTimeou
356356
* the connection has been terminated).
357357
*
358358
* <p>{@link #getTotalTimeout()} caps how long the logic should keep trying the RPC until it
359-
* gives up completely. If {@link #getTotalTimeout()} is set, initialRpcTimeout should be <=
359+
* gives up completely. If {@link #getTotalTimeout()} is set, initialRpcTimeout should be &lt;=
360360
* totalTimeout.
361361
*
362362
* <p>If there are no configurations, Retries have the default initial RPC timeout value of
@@ -491,7 +491,7 @@ public final org.threeten.bp.Duration getInitialRpcTimeout() {
491491
* the connection has been terminated).
492492
*
493493
* <p>{@link #getTotalTimeout()} caps how long the logic should keep trying the RPC until it
494-
* gives up completely. If {@link #getTotalTimeout()} is set, initialRpcTimeout should be <=
494+
* gives up completely. If {@link #getTotalTimeout()} is set, initialRpcTimeout should be &lt;=
495495
* totalTimeout.
496496
*
497497
* <p>If there are no configurations, Retries have the default initial RPC timeout value of

‎sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/rpc/BidiStreamingCallable.java‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -264,13 +264,18 @@ public void onError(Throwable t) {
264264
if (transportChannel != null && transportChannel.shouldRefresh()) {
265265
transportChannel.refresh();
266266
UnauthenticatedException causeEx = (UnauthenticatedException) t;
267-
t =
267+
UnauthenticatedException newEx =
268268
new UnauthenticatedException(
269269
causeEx.getMessage(),
270-
causeEx.getCause(),
270+
causeEx,
271271
causeEx.getStatusCode(),
272272
true, // isRetryable = true
273273
causeEx.getErrorDetails());
274+
newEx.setStackTrace(causeEx.getStackTrace());
275+
for (Throwable suppressed : causeEx.getSuppressed()) {
276+
newEx.addSuppressed(suppressed);
277+
}
278+
t = newEx;
274279
}
275280
}
276281
responseObserver.onError(t);

‎sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientStreamingCallable.java‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,13 +93,18 @@ public void onError(Throwable t) {
9393
if (transportChannel != null && transportChannel.shouldRefresh()) {
9494
transportChannel.refresh();
9595
UnauthenticatedException causeEx = (UnauthenticatedException) t;
96-
t =
96+
UnauthenticatedException newEx =
9797
new UnauthenticatedException(
9898
causeEx.getMessage(),
99-
causeEx.getCause(),
99+
causeEx,
100100
causeEx.getStatusCode(),
101101
true, // isRetryable = true
102102
causeEx.getErrorDetails());
103+
newEx.setStackTrace(causeEx.getStackTrace());
104+
for (Throwable suppressed : causeEx.getSuppressed()) {
105+
newEx.addSuppressed(suppressed);
106+
}
107+
t = newEx;
103108
}
104109
}
105110
responseObserver.onError(t);

‎sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/rpc/ServerStreamingAttemptCallable.java‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -246,13 +246,18 @@ public void onErrorImpl(Throwable t) {
246246
if (transportChannel != null && transportChannel.shouldRefresh()) {
247247
transportChannel.refresh();
248248
UnauthenticatedException causeEx = (UnauthenticatedException) cause;
249-
cause =
249+
UnauthenticatedException newEx =
250250
new UnauthenticatedException(
251251
causeEx.getMessage(),
252-
causeEx.getCause(),
252+
causeEx,
253253
causeEx.getStatusCode(),
254254
true, // isRetryable = true
255255
causeEx.getErrorDetails());
256+
newEx.setStackTrace(causeEx.getStackTrace());
257+
for (Throwable suppressed : causeEx.getSuppressed()) {
258+
newEx.addSuppressed(suppressed);
259+
}
260+
cause = newEx;
256261

257262
t = cause;
258263
}

0 commit comments

Comments
 (0)