From 558af8eff27273fa7129917503ea1133e23fa91d Mon Sep 17 00:00:00 2001 From: detmerl Date: Tue, 27 Aug 2024 13:08:39 -0400 Subject: [PATCH 01/38] added setApiKey() method to client settings --- gapic-generator-java-pom-parent/pom.xml | 2 +- .../com/google/api/gax/rpc/ClientContext.java | 5 ++++ .../google/api/gax/rpc/ClientSettings.java | 14 +++++++++++ .../com/google/api/gax/rpc/StubSettings.java | 23 +++++++++++++++++++ 4 files changed, 43 insertions(+), 1 deletion(-) diff --git a/gapic-generator-java-pom-parent/pom.xml b/gapic-generator-java-pom-parent/pom.xml index c5cbdccaec..1f242ed128 100644 --- a/gapic-generator-java-pom-parent/pom.xml +++ b/gapic-generator-java-pom-parent/pom.xml @@ -27,7 +27,7 @@ consistent across modules in this repository --> 1.3.2 1.66.0 - 1.24.1 + 1.24.1-SNAPSHOT 1.44.2 2.11.0 33.2.1-jre diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java index 09c05b7838..0c6013db43 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java @@ -43,6 +43,7 @@ import com.google.api.gax.rpc.internal.QuotaProjectIdHidingCredentials; import com.google.api.gax.tracing.ApiTracerFactory; import com.google.api.gax.tracing.BaseApiTracerFactory; +import com.google.auth.ApiKeyCredentials; import com.google.auth.Credentials; import com.google.auth.oauth2.GdchCredentials; import com.google.auto.value.AutoValue; @@ -240,6 +241,10 @@ public static ClientContext create(StubSettings settings) throws IOException { if (credentials != null) { defaultCallContext = defaultCallContext.withCredentials(credentials); } + //TODO: if we decided to add setApiKey method need to check and throw exception if apikey and credentials are provided + if (settings.getApiKey() != null) { + defaultCallContext = defaultCallContext.withCredentials(ApiKeyCredentials.create(settings.getApiKey())); + } defaultCallContext = defaultCallContext.withEndpointContext(endpointContext); WatchdogProvider watchdogProvider = settings.getStreamWatchdogProvider(); diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java index b5e54484dd..ad06cd113d 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java @@ -107,6 +107,8 @@ public final String getQuotaProjectId() { return stubSettings.getQuotaProjectId(); } + public final String getApiKey() {return stubSettings.getApiKey();} + @Nullable public final WatchdogProvider getWatchdogProvider() { return stubSettings.getStreamWatchdogProvider(); @@ -302,6 +304,15 @@ public B setGdchApiAudience(@Nullable String gdchApiAudience) { return self(); } + /** + * Sets the API key. The API key will be passed to API call request via the x-goog-api-key + * header to authenticate the API call. + */ + public B setApiKey(String apiKey) { + stubSettings.setApiKey(apiKey); + return self(); + } + /** * Gets the ExecutorProvider that was previously set on this Builder. This ExecutorProvider is * to use for running asynchronous API call logic (such as retries and long-running operations), @@ -364,6 +375,8 @@ public WatchdogProvider getWatchdogProvider() { return stubSettings.getStreamWatchdogProvider(); } + public String getApiKey() {return stubSettings.getApiKey(); } + /** This method is obsolete. Use {@link #getWatchdogCheckIntervalDuration()} instead */ @Nullable @ObsoleteApi("Use getWatchdogCheckIntervalDuration() instead") @@ -405,6 +418,7 @@ public String toString() { .add("watchdogProvider", getWatchdogProvider()) .add("watchdogCheckInterval", getWatchdogCheckIntervalDuration()) .add("gdchApiAudience", getGdchApiAudience()) + .add("apiKey", getApiKey()) .toString(); } } diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java index daeda4c445..f78daf0eef 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java @@ -82,6 +82,7 @@ public abstract class StubSettings> { // Track if deprecated setExecutorProvider is called private boolean deprecatedExecutorProviderSet; @Nonnull private final EndpointContext endpointContext; + private final String apiKey; /** * Indicate when creating transport whether it is allowed to use mTLS endpoint instead of the @@ -107,6 +108,7 @@ protected StubSettings(Builder builder) { this.deprecatedExecutorProviderSet = builder.deprecatedExecutorProviderSet; this.gdchApiAudience = builder.gdchApiAudience; this.endpointContext = buildEndpointContext(builder); + this.apiKey = builder.apiKey; } /** @@ -234,6 +236,10 @@ public final String getGdchApiAudience() { return gdchApiAudience; } + public final String getApiKey() { + return apiKey; + } + @Override public String toString() { return MoreObjects.toStringHelper(this) @@ -252,6 +258,7 @@ public String toString() { .add("streamWatchdogCheckInterval", streamWatchdogCheckInterval) .add("tracerFactory", tracerFactory) .add("gdchApiAudience", gdchApiAudience) + .add("apiKey", apiKey) .toString(); } @@ -277,6 +284,7 @@ public abstract static class Builder< private boolean deprecatedExecutorProviderSet; private String universeDomain; private final EndpointContext endpointContext; + private String apiKey; /** * Indicate when creating transport whether it is allowed to use mTLS endpoint instead of the @@ -301,6 +309,7 @@ protected Builder(StubSettings settings) { this.tracerFactory = settings.tracerFactory; this.deprecatedExecutorProviderSet = settings.deprecatedExecutorProviderSet; this.gdchApiAudience = settings.gdchApiAudience; + this.apiKey = settings.apiKey; // The follow settings will be set to the original user configurations as the // EndpointContext will be rebuilt in the constructor. @@ -353,6 +362,7 @@ protected Builder(ClientContext clientContext) { this.mtlsEndpoint = null; this.switchToMtlsEndpointAllowed = false; this.universeDomain = null; + this.apiKey = null; // Attempt to create an empty, non-functioning EndpointContext by default. The client will // have // a valid EndpointContext with user configurations after the client has been initialized. @@ -574,6 +584,15 @@ public B setTracerFactory(@Nonnull ApiTracerFactory tracerFactory) { return self(); } + /** + * Sets the API key. The API key will be passed to API call request via the x-goog-api-key + * header to authenticate the API call. + */ + public B setApiKey(String apiKey) { + this.apiKey = apiKey; + return self(); + } + /** @deprecated Please use {@link #getBackgroundExecutorProvider()}. */ @Deprecated public ExecutorProvider getExecutorProvider() { @@ -616,6 +635,10 @@ public ApiClock getClock() { return clock; } + public String getApiKey() { + return apiKey; + } + /** * @return the resolved endpoint when the Builder was created. If invoked after * `StubSettings.newBuilder()` is called, it will return the clientSettingsEndpoint value. From dad48d4f2e1f508474b640b0f59307eb04682067 Mon Sep 17 00:00:00 2001 From: detmerl Date: Wed, 28 Aug 2024 10:29:16 -0400 Subject: [PATCH 02/38] cleaned up and added logic for throwing error if both api key and credentials are provided --- .../com/google/api/gax/rpc/ClientContext.java | 4 +- .../google/api/gax/rpc/ClientSettings.java | 16 ++++--- .../com/google/api/gax/rpc/StubSettings.java | 11 ++--- .../google/api/gax/rpc/ClientContextTest.java | 31 +++++++++++-- .../api/gax/rpc/ClientSettingsTest.java | 45 +++++++++++++++++++ 5 files changed, 92 insertions(+), 15 deletions(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java index 0c6013db43..d66406c0ad 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java @@ -238,12 +238,12 @@ public static ClientContext create(StubSettings settings) throws IOException { ApiCallContext defaultCallContext = transportChannel.getEmptyCallContext().withTransportChannel(transportChannel); + if (credentials != null) { defaultCallContext = defaultCallContext.withCredentials(credentials); } - //TODO: if we decided to add setApiKey method need to check and throw exception if apikey and credentials are provided if (settings.getApiKey() != null) { - defaultCallContext = defaultCallContext.withCredentials(ApiKeyCredentials.create(settings.getApiKey())); + defaultCallContext = defaultCallContext.withCredentials(ApiKeyCredentials.create(settings.getApiKey())); } defaultCallContext = defaultCallContext.withEndpointContext(endpointContext); diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java index ad06cd113d..6e06cb0727 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java @@ -38,6 +38,7 @@ import com.google.api.gax.core.ExecutorProvider; import com.google.common.base.MoreObjects; import java.io.IOException; +import java.lang.IllegalArgumentException; import java.util.concurrent.Executor; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -58,6 +59,9 @@ public abstract class ClientSettings /** Constructs an instance of ClientSettings. */ protected ClientSettings(Builder builder) throws IOException { + if (builder.stubSettings.getApiKey() != null && builder.stubSettings.getCredentialsProvider().getCredentials() != null) { + throw new IllegalArgumentException("You can not provide both ApiKey and Credentials for a client."); + } this.stubSettings = builder.stubSettings.build(); } @@ -107,13 +111,13 @@ public final String getQuotaProjectId() { return stubSettings.getQuotaProjectId(); } - public final String getApiKey() {return stubSettings.getApiKey();} - @Nullable public final WatchdogProvider getWatchdogProvider() { return stubSettings.getStreamWatchdogProvider(); } + public final String getApiKey() { return stubSettings.getApiKey(); } + /** This method is obsolete. Use {@link #getWatchdogCheckIntervalDuration()} instead. */ @Nonnull @ObsoleteApi("Use getWatchdogCheckIntervalDuration() instead") @@ -131,6 +135,7 @@ public final String getGdchApiAudience() { return stubSettings.getGdchApiAudience(); } + public String toString() { return MoreObjects.toStringHelper(this) .add("executorProvider", getExecutorProvider()) @@ -146,6 +151,7 @@ public String toString() { .add("watchdogProvider", getWatchdogProvider()) .add("watchdogCheckInterval", getWatchdogCheckInterval()) .add("gdchApiAudience", getGdchApiAudience()) + .add("apiKey", getApiKey()) .toString(); } @@ -305,8 +311,7 @@ public B setGdchApiAudience(@Nullable String gdchApiAudience) { } /** - * Sets the API key. The API key will be passed to API call request via the x-goog-api-key - * header to authenticate the API call. + * Sets the API key. The API key will get translated to an [ApiKeyCredentials] and stored in [CallContext]. */ public B setApiKey(String apiKey) { stubSettings.setApiKey(apiKey); @@ -375,7 +380,8 @@ public WatchdogProvider getWatchdogProvider() { return stubSettings.getStreamWatchdogProvider(); } - public String getApiKey() {return stubSettings.getApiKey(); } + /** Gets the ApiKey that was previously set on this Builder. */ + public String getApiKey() { return stubSettings.getApiKey(); } /** This method is obsolete. Use {@link #getWatchdogCheckIntervalDuration()} instead */ @Nullable diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java index f78daf0eef..40b581868a 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java @@ -236,8 +236,9 @@ public final String getGdchApiAudience() { return gdchApiAudience; } + /** Gets the ApiKey that should be used for authentication. If an empty string was provided it will return null */ public final String getApiKey() { - return apiKey; + return (apiKey == null || apiKey.isEmpty()) ? null : apiKey; } @Override @@ -585,8 +586,7 @@ public B setTracerFactory(@Nonnull ApiTracerFactory tracerFactory) { } /** - * Sets the API key. The API key will be passed to API call request via the x-goog-api-key - * header to authenticate the API call. + * Sets the API key. The API key will get translated to an [ApiKeyCredentials] and stored in [CallContext]. */ public B setApiKey(String apiKey) { this.apiKey = apiKey; @@ -635,8 +635,9 @@ public ApiClock getClock() { return clock; } - public String getApiKey() { - return apiKey; + /** Gets the ApiKey that was previously set on this Builder. If an empty string was provided it will return null */ + public final String getApiKey() { + return (apiKey == null || apiKey.isEmpty()) ? null : apiKey; } /** diff --git a/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java b/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java index 5efce3fbe9..39f716a879 100644 --- a/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java +++ b/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java @@ -52,6 +52,7 @@ import com.google.api.gax.rpc.testing.FakeClientSettings; import com.google.api.gax.rpc.testing.FakeStubSettings; import com.google.api.gax.rpc.testing.FakeTransportChannel; +import com.google.auth.ApiKeyCredentials; import com.google.auth.Credentials; import com.google.auth.oauth2.ComputeEngineCredentials; import com.google.auth.oauth2.GdchCredentials; @@ -205,9 +206,6 @@ public TransportChannelProvider withPoolSize(int size) { @Override public TransportChannel getTransportChannel() throws IOException { - if (needsCredentials()) { - throw new IllegalStateException("Needs Credentials"); - } transport.setExecutor(executor); return transport; } @@ -1074,4 +1072,31 @@ public void testStreamWatchdogInterval_backportMethodsBehaveCorrectly() { ct -> ct.getStreamWatchdogCheckIntervalDuration(), ct -> ct.getStreamWatchdogCheckInterval()); } + + @Test + public void testSetApiKey_createsApiCredentials() throws IOException { + String apiKey = "key"; + FakeStubSettings.Builder builder = new FakeStubSettings.Builder(); + InterceptingExecutor executor = new InterceptingExecutor(1); + FakeTransportChannel transportChannel = FakeTransportChannel.create(new FakeChannel()); + FakeTransportProvider transportProvider = + new FakeTransportProvider( + transportChannel, + executor, + true, + ImmutableMap.of(), + null, + DEFAULT_ENDPOINT); + builder.setTransportChannelProvider(transportProvider); + + HeaderProvider headerProvider = Mockito.mock(HeaderProvider.class); + Mockito.when(headerProvider.getHeaders()).thenReturn(ImmutableMap.of()); + builder.setHeaderProvider(headerProvider); + builder.setApiKey(apiKey); + + ClientContext context = ClientContext.create(builder.build()); + + FakeCallContext fakeCallContext = (FakeCallContext) context.getDefaultCallContext(); + assertThat(fakeCallContext.getCredentials()).isInstanceOf(ApiKeyCredentials.class); + } } diff --git a/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientSettingsTest.java b/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientSettingsTest.java index 3e99f7471c..95409da115 100644 --- a/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientSettingsTest.java +++ b/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientSettingsTest.java @@ -31,6 +31,7 @@ import static com.google.api.gax.util.TimeConversionTestUtils.testDurationMethod; import static org.junit.jupiter.api.Assertions.fail; +import static org.mockito.Mockito.when; import com.google.api.core.ApiClock; import com.google.api.core.NanoClock; @@ -56,6 +57,9 @@ import java.util.concurrent.ScheduledExecutorService; import java.util.function.Function; import java.util.function.Supplier; + +import org.junit.Assert; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.mockito.Mockito; @@ -120,6 +124,7 @@ void testEmptyBuilder() throws Exception { Truth.assertThat(builder.getWatchdogCheckIntervalDuration()) .isGreaterThan(java.time.Duration.ZERO); Truth.assertThat(builder.getQuotaProjectId()).isNull(); + Truth.assertThat(builder.getApiKey()).isNull(); FakeClientSettings settings = builder.build(); Truth.assertThat(settings.getExecutorProvider()) @@ -150,6 +155,7 @@ void testEmptyBuilder() throws Exception { Truth.assertThat(settingsString).contains("watchdogProvider"); Truth.assertThat(settingsString).contains("watchdogCheckInterval"); Truth.assertThat(settingsString).contains(("quotaProjectId")); + Truth.assertThat(settingsString).contains(("apiKey")); } @Test @@ -165,6 +171,7 @@ void testBuilder() throws Exception { WatchdogProvider watchdogProvider = Mockito.mock(WatchdogProvider.class); java.time.Duration watchdogCheckInterval = java.time.Duration.ofSeconds(13); String quotaProjectId = "test_quota_project_id"; + String apiKey = "api_key"; builder.setExecutorProvider(executorProvider); builder.setTransportChannelProvider(transportProvider); @@ -175,6 +182,7 @@ void testBuilder() throws Exception { builder.setWatchdogProvider(watchdogProvider); builder.setWatchdogCheckIntervalDuration(watchdogCheckInterval); builder.setQuotaProjectId(quotaProjectId); + builder.setApiKey(apiKey); // For backward compatibility, backgroundExecutorProvider is set to executorProvider Truth.assertThat(builder.getExecutorProvider()).isSameInstanceAs(executorProvider); @@ -188,6 +196,7 @@ void testBuilder() throws Exception { Truth.assertThat(builder.getWatchdogCheckIntervalDuration()) .isSameInstanceAs(watchdogCheckInterval); Truth.assertThat(builder.getQuotaProjectId()).isEqualTo(quotaProjectId); + Truth.assertThat(builder.getApiKey()).isEqualTo(apiKey); String builderString = builder.toString(); Truth.assertThat(builderString).contains("executorProvider"); @@ -200,6 +209,7 @@ void testBuilder() throws Exception { Truth.assertThat(builderString).contains("watchdogProvider"); Truth.assertThat(builderString).contains("watchdogCheckInterval"); Truth.assertThat(builderString).contains("quotaProjectId"); + Truth.assertThat(builderString).contains("apiKey"); } @Test @@ -262,6 +272,7 @@ void testBuilderFromSettings() throws Exception { WatchdogProvider watchdogProvider = Mockito.mock(WatchdogProvider.class); java.time.Duration watchdogCheckInterval = java.time.Duration.ofSeconds(14); String quotaProjectId = "test_builder_from_settings_quotaProjectId"; + String apiKey = "api_key"; builder.setExecutorProvider(executorProvider); builder.setTransportChannelProvider(transportProvider); @@ -272,6 +283,7 @@ void testBuilderFromSettings() throws Exception { builder.setWatchdogProvider(watchdogProvider); builder.setWatchdogCheckIntervalDuration(watchdogCheckInterval); builder.setQuotaProjectId(quotaProjectId); + builder.setApiKey(apiKey); FakeClientSettings settings = builder.build(); FakeClientSettings.Builder newBuilder = new FakeClientSettings.Builder(settings); @@ -288,6 +300,7 @@ void testBuilderFromSettings() throws Exception { Truth.assertThat(newBuilder.getWatchdogCheckIntervalDuration()) .isEqualTo(watchdogCheckInterval); Truth.assertThat(newBuilder.getQuotaProjectId()).isEqualTo(quotaProjectId); + Truth.assertThat(newBuilder.getApiKey()).isEqualTo(apiKey); } @Test @@ -566,4 +579,36 @@ public void testWatchdogCheckInterval_backportMethodsBehaveCorrectly() { cs -> cs.getWatchdogCheckIntervalDuration(), cs -> cs.getWatchdogCheckInterval()); } + + @Test + void testClientSettingsBuilder_throwsErrorIfApiKeyAndCredentialsAreProvided() throws Exception { + FakeClientSettings.Builder builder = new FakeClientSettings.Builder(); + CredentialsProvider credentialsProvider = Mockito.mock(CredentialsProvider.class); + when(credentialsProvider.getCredentials()).thenReturn( Mockito.mock(Credentials.class)); + builder.setCredentialsProvider(credentialsProvider); + builder.setApiKey("api_key"); + + try { + builder.build(); + fail("No exception raised"); + } catch (IllegalArgumentException e) { + Assertions.assertTrue(e.getMessage().contains("You can not provide both ApiKey and Credentials for a client.")); + } + } + + @Test + void testEmptyApiKeyClientSettingsBuild_isTreatedAsNull() throws Exception { + FakeClientSettings.Builder builder = new FakeClientSettings.Builder(); + CredentialsProvider credentialsProvider = Mockito.mock(CredentialsProvider.class); + Credentials credentials = Mockito.mock(Credentials.class); + when(credentialsProvider.getCredentials()).thenReturn(credentials); + builder.setCredentialsProvider(credentialsProvider); + builder.setApiKey(""); + + + FakeClientSettings fakeClientSettings = builder.build(); + + Assertions.assertEquals(fakeClientSettings.getCredentialsProvider().getCredentials(), credentials); + Assertions.assertNull(fakeClientSettings.getApiKey()); + } } From f6afbef19b331af2a6a4c3d679d484073b0800ef Mon Sep 17 00:00:00 2001 From: detmerl Date: Wed, 28 Aug 2024 10:40:58 -0400 Subject: [PATCH 03/38] fixed formatting --- .../com/google/api/gax/rpc/ClientContext.java | 3 +- .../google/api/gax/rpc/ClientSettings.java | 1 - .../com/google/api/gax/rpc/StubSettings.java | 33 ++++++++++++++----- .../google/api/gax/rpc/ClientContextTest.java | 9 ++--- .../api/gax/rpc/ClientSettingsTest.java | 2 -- 5 files changed, 29 insertions(+), 19 deletions(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java index d66406c0ad..c410a31cb2 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java @@ -243,7 +243,8 @@ public static ClientContext create(StubSettings settings) throws IOException { defaultCallContext = defaultCallContext.withCredentials(credentials); } if (settings.getApiKey() != null) { - defaultCallContext = defaultCallContext.withCredentials(ApiKeyCredentials.create(settings.getApiKey())); + defaultCallContext = + defaultCallContext.withCredentials(ApiKeyCredentials.create(settings.getApiKey())); } defaultCallContext = defaultCallContext.withEndpointContext(endpointContext); diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java index 6e06cb0727..ee08b66c8d 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java @@ -38,7 +38,6 @@ import com.google.api.gax.core.ExecutorProvider; import com.google.common.base.MoreObjects; import java.io.IOException; -import java.lang.IllegalArgumentException; import java.util.concurrent.Executor; import javax.annotation.Nonnull; import javax.annotation.Nullable; diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java index 40b581868a..1b88cf552a 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java @@ -132,7 +132,9 @@ private EndpointContext buildEndpointContext(Builder builder) { } } - /** @deprecated Please use {@link #getBackgroundExecutorProvider()}. */ + /** + * @deprecated Please use {@link #getBackgroundExecutorProvider()}. + */ @Deprecated public final ExecutorProvider getExecutorProvider() { return deprecatedExecutorProviderSet ? backgroundExecutorProvider : null; @@ -174,17 +176,23 @@ protected String getServiceName() { return ""; } - /** @return the fully resolved universe domain used by the client */ + /** + * @return the fully resolved universe domain used by the client + */ public final String getUniverseDomain() { return endpointContext.resolvedUniverseDomain(); } - /** @return the fully resolved endpoint used by the client */ + /** + * @return the fully resolved endpoint used by the client + */ public String getEndpoint() { return endpointContext.resolvedEndpoint(); } - /** @return the newly created EndpointContext */ + /** + * @return the newly created EndpointContext + */ @InternalApi final EndpointContext getEndpointContext() { return endpointContext; @@ -236,7 +244,10 @@ public final String getGdchApiAudience() { return gdchApiAudience; } - /** Gets the ApiKey that should be used for authentication. If an empty string was provided it will return null */ + /** + * Gets the ApiKey that should be used for authentication. If an empty string was provided it will + * return null + */ public final String getApiKey() { return (apiKey == null || apiKey.isEmpty()) ? null : apiKey; } @@ -586,14 +597,17 @@ public B setTracerFactory(@Nonnull ApiTracerFactory tracerFactory) { } /** - * Sets the API key. The API key will get translated to an [ApiKeyCredentials] and stored in [CallContext]. + * Sets the API key. The API key will get translated to an [ApiKeyCredentials] and stored in + * [CallContext]. */ public B setApiKey(String apiKey) { this.apiKey = apiKey; return self(); } - /** @deprecated Please use {@link #getBackgroundExecutorProvider()}. */ + /** + * @deprecated Please use {@link #getBackgroundExecutorProvider()}. + */ @Deprecated public ExecutorProvider getExecutorProvider() { return deprecatedExecutorProviderSet ? backgroundExecutorProvider : null; @@ -635,7 +649,10 @@ public ApiClock getClock() { return clock; } - /** Gets the ApiKey that was previously set on this Builder. If an empty string was provided it will return null */ + /** + * Gets the ApiKey that was previously set on this Builder. If an empty string was provided it + * will return null + */ public final String getApiKey() { return (apiKey == null || apiKey.isEmpty()) ? null : apiKey; } diff --git a/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java b/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java index 39f716a879..2f7f59bd21 100644 --- a/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java +++ b/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java @@ -1080,13 +1080,8 @@ public void testSetApiKey_createsApiCredentials() throws IOException { InterceptingExecutor executor = new InterceptingExecutor(1); FakeTransportChannel transportChannel = FakeTransportChannel.create(new FakeChannel()); FakeTransportProvider transportProvider = - new FakeTransportProvider( - transportChannel, - executor, - true, - ImmutableMap.of(), - null, - DEFAULT_ENDPOINT); + new FakeTransportProvider( + transportChannel, executor, true, ImmutableMap.of(), null, DEFAULT_ENDPOINT); builder.setTransportChannelProvider(transportProvider); HeaderProvider headerProvider = Mockito.mock(HeaderProvider.class); diff --git a/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientSettingsTest.java b/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientSettingsTest.java index 95409da115..f7098c15f2 100644 --- a/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientSettingsTest.java +++ b/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientSettingsTest.java @@ -57,8 +57,6 @@ import java.util.concurrent.ScheduledExecutorService; import java.util.function.Function; import java.util.function.Supplier; - -import org.junit.Assert; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.mockito.Mockito; From a516d950d39a64ab3510072ba8a43e37faaa6de6 Mon Sep 17 00:00:00 2001 From: detmerl Date: Wed, 28 Aug 2024 10:42:29 -0400 Subject: [PATCH 04/38] fixed formatting --- .../google/api/gax/rpc/ClientSettings.java | 22 +++++++++++++------ .../api/gax/rpc/ClientSettingsTest.java | 9 ++++---- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java index ee08b66c8d..28f1e1e552 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java @@ -58,8 +58,10 @@ public abstract class ClientSettings /** Constructs an instance of ClientSettings. */ protected ClientSettings(Builder builder) throws IOException { - if (builder.stubSettings.getApiKey() != null && builder.stubSettings.getCredentialsProvider().getCredentials() != null) { - throw new IllegalArgumentException("You can not provide both ApiKey and Credentials for a client."); + if (builder.stubSettings.getApiKey() != null + && builder.stubSettings.getCredentialsProvider().getCredentials() != null) { + throw new IllegalArgumentException( + "You can not provide both ApiKey and Credentials for a client."); } this.stubSettings = builder.stubSettings.build(); } @@ -68,7 +70,9 @@ public final StubSettings getStubSettings() { return stubSettings; } - /** @deprecated Please use {@link #getBackgroundExecutorProvider()} */ + /** + * @deprecated Please use {@link #getBackgroundExecutorProvider()} + */ @Deprecated public final ExecutorProvider getExecutorProvider() { return stubSettings.getExecutorProvider(); @@ -115,7 +119,9 @@ public final WatchdogProvider getWatchdogProvider() { return stubSettings.getStreamWatchdogProvider(); } - public final String getApiKey() { return stubSettings.getApiKey(); } + public final String getApiKey() { + return stubSettings.getApiKey(); + } /** This method is obsolete. Use {@link #getWatchdogCheckIntervalDuration()} instead. */ @Nonnull @@ -134,7 +140,6 @@ public final String getGdchApiAudience() { return stubSettings.getGdchApiAudience(); } - public String toString() { return MoreObjects.toStringHelper(this) .add("executorProvider", getExecutorProvider()) @@ -310,7 +315,8 @@ public B setGdchApiAudience(@Nullable String gdchApiAudience) { } /** - * Sets the API key. The API key will get translated to an [ApiKeyCredentials] and stored in [CallContext]. + * Sets the API key. The API key will get translated to an [ApiKeyCredentials] and stored in + * [CallContext]. */ public B setApiKey(String apiKey) { stubSettings.setApiKey(apiKey); @@ -380,7 +386,9 @@ public WatchdogProvider getWatchdogProvider() { } /** Gets the ApiKey that was previously set on this Builder. */ - public String getApiKey() { return stubSettings.getApiKey(); } + public String getApiKey() { + return stubSettings.getApiKey(); + } /** This method is obsolete. Use {@link #getWatchdogCheckIntervalDuration()} instead */ @Nullable diff --git a/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientSettingsTest.java b/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientSettingsTest.java index f7098c15f2..7d208a9e29 100644 --- a/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientSettingsTest.java +++ b/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientSettingsTest.java @@ -582,7 +582,7 @@ public void testWatchdogCheckInterval_backportMethodsBehaveCorrectly() { void testClientSettingsBuilder_throwsErrorIfApiKeyAndCredentialsAreProvided() throws Exception { FakeClientSettings.Builder builder = new FakeClientSettings.Builder(); CredentialsProvider credentialsProvider = Mockito.mock(CredentialsProvider.class); - when(credentialsProvider.getCredentials()).thenReturn( Mockito.mock(Credentials.class)); + when(credentialsProvider.getCredentials()).thenReturn(Mockito.mock(Credentials.class)); builder.setCredentialsProvider(credentialsProvider); builder.setApiKey("api_key"); @@ -590,7 +590,8 @@ void testClientSettingsBuilder_throwsErrorIfApiKeyAndCredentialsAreProvided() th builder.build(); fail("No exception raised"); } catch (IllegalArgumentException e) { - Assertions.assertTrue(e.getMessage().contains("You can not provide both ApiKey and Credentials for a client.")); + Assertions.assertTrue( + e.getMessage().contains("You can not provide both ApiKey and Credentials for a client.")); } } @@ -603,10 +604,10 @@ void testEmptyApiKeyClientSettingsBuild_isTreatedAsNull() throws Exception { builder.setCredentialsProvider(credentialsProvider); builder.setApiKey(""); - FakeClientSettings fakeClientSettings = builder.build(); - Assertions.assertEquals(fakeClientSettings.getCredentialsProvider().getCredentials(), credentials); + Assertions.assertEquals( + fakeClientSettings.getCredentialsProvider().getCredentials(), credentials); Assertions.assertNull(fakeClientSettings.getApiKey()); } } From f7ec0faf8481c6066732bffb680ad745b3a2af85 Mon Sep 17 00:00:00 2001 From: detmerl Date: Thu, 12 Sep 2024 16:12:01 -0400 Subject: [PATCH 05/38] wip --- .../com/google/api/gax/rpc/ClientContext.java | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java index c410a31cb2..96caf9e745 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java @@ -177,8 +177,16 @@ public static ClientContext create(StubSettings settings) throws IOException { EndpointContext endpointContext = settings.getEndpointContext(); String endpoint = endpointContext.resolvedEndpoint(); - String settingsGdchApiAudience = settings.getGdchApiAudience(); + String apiKey = settings.getApiKey(); + Credentials credentials = settings.getCredentialsProvider().getCredentials(); + + if (apiKey != null && credentials != null) { + throw new Exception(); + } + + String settingsGdchApiAudience = settings.getGdchApiAudience(); + boolean usingGDCH = credentials instanceof GdchCredentials; if (usingGDCH) { // Can only determine if the GDC-H is being used via the Credentials. The Credentials object @@ -217,6 +225,10 @@ public static ClientContext create(StubSettings settings) throws IOException { credentials = new QuotaProjectIdHidingCredentials(credentials); } + if (apiKey != null) { + credentials = ApiKeyCredentials.create(settings.getApiKey()); + } + TransportChannelProvider transportChannelProvider = settings.getTransportChannelProvider(); // After needsExecutor and StubSettings#setExecutorProvider are deprecated, transport channel // executor can only be set from TransportChannelProvider#withExecutor directly, and a provider @@ -242,10 +254,6 @@ public static ClientContext create(StubSettings settings) throws IOException { if (credentials != null) { defaultCallContext = defaultCallContext.withCredentials(credentials); } - if (settings.getApiKey() != null) { - defaultCallContext = - defaultCallContext.withCredentials(ApiKeyCredentials.create(settings.getApiKey())); - } defaultCallContext = defaultCallContext.withEndpointContext(endpointContext); WatchdogProvider watchdogProvider = settings.getStreamWatchdogProvider(); From db1674b560545fb1ac5e3633df305407e107ac0f Mon Sep 17 00:00:00 2001 From: detmerl Date: Mon, 16 Sep 2024 12:58:10 -0400 Subject: [PATCH 06/38] wip --- .../com/google/api/gax/rpc/ClientContext.java | 77 +++++++++---------- .../google/api/gax/rpc/ClientSettings.java | 5 -- .../google/api/gax/rpc/ClientContextTest.java | 35 +++++++-- .../api/gax/rpc/ClientSettingsTest.java | 17 ---- 4 files changed, 67 insertions(+), 67 deletions(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java index 96caf9e745..738ab47d7a 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java @@ -176,59 +176,58 @@ public static ClientContext create(StubSettings settings) throws IOException { // A valid EndpointContext should have been created in the StubSettings EndpointContext endpointContext = settings.getEndpointContext(); String endpoint = endpointContext.resolvedEndpoint(); - String apiKey = settings.getApiKey(); - Credentials credentials = settings.getCredentialsProvider().getCredentials(); - if (apiKey != null && credentials != null) { - throw new Exception(); + throw new IllegalArgumentException( + "You can not provide both ApiKey and Credentials for a client."); } - - String settingsGdchApiAudience = settings.getGdchApiAudience(); - - boolean usingGDCH = credentials instanceof GdchCredentials; - if (usingGDCH) { - // Can only determine if the GDC-H is being used via the Credentials. The Credentials object - // is resolved in the ClientContext and must be passed to the EndpointContext. Rebuild the - // endpointContext only on GDC-H flows. - endpointContext = endpointContext.withGDCH(); - // Resolve the new endpoint with the GDC-H flow - endpoint = endpointContext.resolvedEndpoint(); - // We recompute the GdchCredentials with the audience - String audienceString; - if (!Strings.isNullOrEmpty(settingsGdchApiAudience)) { - audienceString = settingsGdchApiAudience; - } else if (!Strings.isNullOrEmpty(endpoint)) { - audienceString = endpoint; - } else { - throw new IllegalArgumentException("Could not infer GDCH api audience from settings"); - } - - URI gdchAudienceUri; - try { - gdchAudienceUri = URI.create(audienceString); - } catch (IllegalArgumentException ex) { // thrown when passing a malformed uri string - throw new IllegalArgumentException("The GDC-H API audience string is not a valid URI", ex); + if (apiKey != null) { + credentials = ApiKeyCredentials.create(settings.getApiKey()); + } else { + // check if need to adjust credentials for Gdch audience + String settingsGdchApiAudience = settings.getGdchApiAudience(); + boolean usingGDCH = credentials instanceof GdchCredentials; + if (usingGDCH) { + // Can only determine if the GDC-H is being used via the Credentials. The Credentials object + // is resolved in the ClientContext and must be passed to the EndpointContext. Rebuild the + // endpointContext only on GDC-H flows. + endpointContext = endpointContext.withGDCH(); + // Resolve the new endpoint with the GDC-H flow + endpoint = endpointContext.resolvedEndpoint(); + // We recompute the GdchCredentials with the audience + String audienceString; + if (!Strings.isNullOrEmpty(settingsGdchApiAudience)) { + audienceString = settingsGdchApiAudience; + } else if (!Strings.isNullOrEmpty(endpoint)) { + audienceString = endpoint; + } else { + throw new IllegalArgumentException("Could not infer GDCH api audience from settings"); + } + + URI gdchAudienceUri; + try { + gdchAudienceUri = URI.create(audienceString); + } catch (IllegalArgumentException ex) { // thrown when passing a malformed uri string + throw new IllegalArgumentException( + "The GDC-H API audience string is not a valid URI", ex); + } + credentials = ((GdchCredentials) credentials).createWithGdchAudience(gdchAudienceUri); + } else if (!Strings.isNullOrEmpty(settingsGdchApiAudience)) { + throw new IllegalArgumentException( + "GDC-H API audience can only be set when using GdchCredentials"); } - credentials = ((GdchCredentials) credentials).createWithGdchAudience(gdchAudienceUri); - } else if (!Strings.isNullOrEmpty(settingsGdchApiAudience)) { - throw new IllegalArgumentException( - "GDC-H API audience can only be set when using GdchCredentials"); } if (settings.getQuotaProjectId() != null && credentials != null) { // If the quotaProjectId is set, wrap original credentials with correct quotaProjectId as // QuotaProjectIdHidingCredentials. - // Ensure that a custom set quota project id takes priority over one detected by credentials. + // Ensure that a custom set quota project id takes priority over one detected by + // credentials. // Avoid the backend receiving possibly conflict values of quotaProjectId credentials = new QuotaProjectIdHidingCredentials(credentials); } - if (apiKey != null) { - credentials = ApiKeyCredentials.create(settings.getApiKey()); - } - TransportChannelProvider transportChannelProvider = settings.getTransportChannelProvider(); // After needsExecutor and StubSettings#setExecutorProvider are deprecated, transport channel // executor can only be set from TransportChannelProvider#withExecutor directly, and a provider diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java index 28f1e1e552..69437b830f 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java @@ -58,11 +58,6 @@ public abstract class ClientSettings /** Constructs an instance of ClientSettings. */ protected ClientSettings(Builder builder) throws IOException { - if (builder.stubSettings.getApiKey() != null - && builder.stubSettings.getCredentialsProvider().getCredentials() != null) { - throw new IllegalArgumentException( - "You can not provide both ApiKey and Credentials for a client."); - } this.stubSettings = builder.stubSettings.build(); } diff --git a/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java b/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java index 2f7f59bd21..294f0a6432 100644 --- a/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java +++ b/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java @@ -31,12 +31,7 @@ import static com.google.api.gax.util.TimeConversionTestUtils.testDurationMethod; import static com.google.common.truth.Truth.assertThat; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertNotSame; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.*; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -67,6 +62,8 @@ import java.util.concurrent.Executor; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledThreadPoolExecutor; + +import org.apache.http.impl.client.SystemDefaultCredentialsProvider; import org.junit.jupiter.api.Test; import org.mockito.Mockito; @@ -1094,4 +1091,30 @@ public void testSetApiKey_createsApiCredentials() throws IOException { FakeCallContext fakeCallContext = (FakeCallContext) context.getDefaultCallContext(); assertThat(fakeCallContext.getCredentials()).isInstanceOf(ApiKeyCredentials.class); } + + @Test + void testCreateClient_throwsErrorIfApiKeyAndCredentialsAreProvided() throws Exception { + String apiKey = "key"; + FakeStubSettings.Builder builder = new FakeStubSettings.Builder(); + InterceptingExecutor executor = new InterceptingExecutor(1); + FakeTransportChannel transportChannel = FakeTransportChannel.create(new FakeChannel()); + FakeTransportProvider transportProvider = + new FakeTransportProvider( + transportChannel, executor, true, ImmutableMap.of(), null, DEFAULT_ENDPOINT); + builder.setTransportChannelProvider(transportProvider); + + HeaderProvider headerProvider = Mockito.mock(HeaderProvider.class); + Mockito.when(headerProvider.getHeaders()).thenReturn(ImmutableMap.of()); + builder.setHeaderProvider(headerProvider); + builder.setApiKey(apiKey); + builder.setCredentialsProvider(Mockito.mock(CredentialsProvider.class)); + + try { + ClientContext context = ClientContext.create(builder.build()); + fail("No exception raised"); + } catch (IllegalArgumentException e) { + assert( + e.getMessage().contains("You can not provide both ApiKey and Credentials for a client.")); + } + } } diff --git a/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientSettingsTest.java b/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientSettingsTest.java index 7d208a9e29..79a9818fe9 100644 --- a/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientSettingsTest.java +++ b/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientSettingsTest.java @@ -578,23 +578,6 @@ public void testWatchdogCheckInterval_backportMethodsBehaveCorrectly() { cs -> cs.getWatchdogCheckInterval()); } - @Test - void testClientSettingsBuilder_throwsErrorIfApiKeyAndCredentialsAreProvided() throws Exception { - FakeClientSettings.Builder builder = new FakeClientSettings.Builder(); - CredentialsProvider credentialsProvider = Mockito.mock(CredentialsProvider.class); - when(credentialsProvider.getCredentials()).thenReturn(Mockito.mock(Credentials.class)); - builder.setCredentialsProvider(credentialsProvider); - builder.setApiKey("api_key"); - - try { - builder.build(); - fail("No exception raised"); - } catch (IllegalArgumentException e) { - Assertions.assertTrue( - e.getMessage().contains("You can not provide both ApiKey and Credentials for a client.")); - } - } - @Test void testEmptyApiKeyClientSettingsBuild_isTreatedAsNull() throws Exception { FakeClientSettings.Builder builder = new FakeClientSettings.Builder(); From 9a139519b2ff398d4fccfca5fa5deb954e3249d3 Mon Sep 17 00:00:00 2001 From: detmerl Date: Mon, 16 Sep 2024 15:59:21 -0400 Subject: [PATCH 07/38] wip --- .../com/google/api/gax/rpc/ClientContext.java | 72 ++++++++++--------- 1 file changed, 39 insertions(+), 33 deletions(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java index 738ab47d7a..7602e34173 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java @@ -183,40 +183,25 @@ public static ClientContext create(StubSettings settings) throws IOException { "You can not provide both ApiKey and Credentials for a client."); } if (apiKey != null) { + // if API key exists it becomes the default credential credentials = ApiKeyCredentials.create(settings.getApiKey()); - } else { - // check if need to adjust credentials for Gdch audience - String settingsGdchApiAudience = settings.getGdchApiAudience(); - boolean usingGDCH = credentials instanceof GdchCredentials; - if (usingGDCH) { - // Can only determine if the GDC-H is being used via the Credentials. The Credentials object - // is resolved in the ClientContext and must be passed to the EndpointContext. Rebuild the - // endpointContext only on GDC-H flows. - endpointContext = endpointContext.withGDCH(); - // Resolve the new endpoint with the GDC-H flow - endpoint = endpointContext.resolvedEndpoint(); - // We recompute the GdchCredentials with the audience - String audienceString; - if (!Strings.isNullOrEmpty(settingsGdchApiAudience)) { - audienceString = settingsGdchApiAudience; - } else if (!Strings.isNullOrEmpty(endpoint)) { - audienceString = endpoint; - } else { - throw new IllegalArgumentException("Could not infer GDCH api audience from settings"); - } - - URI gdchAudienceUri; - try { - gdchAudienceUri = URI.create(audienceString); - } catch (IllegalArgumentException ex) { // thrown when passing a malformed uri string - throw new IllegalArgumentException( - "The GDC-H API audience string is not a valid URI", ex); - } - credentials = ((GdchCredentials) credentials).createWithGdchAudience(gdchAudienceUri); - } else if (!Strings.isNullOrEmpty(settingsGdchApiAudience)) { - throw new IllegalArgumentException( - "GDC-H API audience can only be set when using GdchCredentials"); - } + } + + // check if need to adjust credentials/endpoint/endpointContext for GDC-H + String settingsGdchApiAudience = settings.getGdchApiAudience(); + boolean usingGDCH = credentials instanceof GdchCredentials; + if (usingGDCH) { + // Can only determine if the GDC-H is being used via the Credentials. The Credentials object + // is resolved in the ClientContext and must be passed to the EndpointContext. Rebuild the + // endpointContext only on GDC-H flows. + endpointContext = endpointContext.withGDCH(); + // Resolve the new endpoint with the GDC-H flow + endpoint = endpointContext.resolvedEndpoint(); + // We recompute the GdchCredentials with the audience + credentials = getGdchCredentials(settingsGdchApiAudience, endpoint, credentials); + } else if (!Strings.isNullOrEmpty(settingsGdchApiAudience)) { + throw new IllegalArgumentException( + "GDC-H API audience can only be set when using GdchCredentials"); } if (settings.getQuotaProjectId() != null && credentials != null) { @@ -304,6 +289,27 @@ public static ClientContext create(StubSettings settings) throws IOException { .build(); } + private static Credentials getGdchCredentials(String settingsGdchApiAudience, String endpoint, Credentials credentials) throws IOException { + String audienceString; + if (!Strings.isNullOrEmpty(settingsGdchApiAudience)) { + audienceString = settingsGdchApiAudience; + } else if (!Strings.isNullOrEmpty(endpoint)) { + audienceString = endpoint; + } else { + throw new IllegalArgumentException("Could not infer GDCH api audience from settings"); + } + + URI gdchAudienceUri; + try { + gdchAudienceUri = URI.create(audienceString); + } catch (IllegalArgumentException ex) { // thrown when passing a malformed uri string + throw new IllegalArgumentException( + "The GDC-H API audience string is not a valid URI", ex); + } + credentials = ((GdchCredentials) credentials).createWithGdchAudience(gdchAudienceUri); + return credentials; + } + /** * Getting a header map from HeaderProvider and InternalHeaderProvider from settings with Quota * Project Id. From 62a39562891bdd0be14b340a933bdd2685b940d4 Mon Sep 17 00:00:00 2001 From: detmerl Date: Tue, 17 Sep 2024 10:57:04 -0400 Subject: [PATCH 08/38] clean up --- gapic-generator-java-pom-parent/pom.xml | 2 +- .../java/com/google/api/gax/rpc/ClientContext.java | 7 +++++-- .../com/google/api/gax/rpc/ClientSettings.java | 6 +++--- .../com/google/api/gax/rpc/ClientContextTest.java | 4 +--- gax-java/pom.xml | 14 +++++++------- 5 files changed, 17 insertions(+), 16 deletions(-) diff --git a/gapic-generator-java-pom-parent/pom.xml b/gapic-generator-java-pom-parent/pom.xml index 1f242ed128..f41632fe49 100644 --- a/gapic-generator-java-pom-parent/pom.xml +++ b/gapic-generator-java-pom-parent/pom.xml @@ -27,7 +27,7 @@ consistent across modules in this repository --> 1.3.2 1.66.0 - 1.24.1-SNAPSHOT + 1.25.1-SNAPSHOT 1.44.2 2.11.0 33.2.1-jre diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java index 7602e34173..40b84bbe33 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java @@ -234,7 +234,6 @@ public static ClientContext create(StubSettings settings) throws IOException { ApiCallContext defaultCallContext = transportChannel.getEmptyCallContext().withTransportChannel(transportChannel); - if (credentials != null) { defaultCallContext = defaultCallContext.withCredentials(credentials); } @@ -289,7 +288,11 @@ public static ClientContext create(StubSettings settings) throws IOException { .build(); } - private static Credentials getGdchCredentials(String settingsGdchApiAudience, String endpoint, Credentials credentials) throws IOException { + /** + * Constructs a new {@link Credentials} object based on credentials provided with a GDC-H audience + */ + private static Credentials getGdchCredentials( + String settingsGdchApiAudience, String endpoint, Credentials credentials) throws IOException { String audienceString; if (!Strings.isNullOrEmpty(settingsGdchApiAudience)) { audienceString = settingsGdchApiAudience; diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java index 69437b830f..13aa1229b7 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java @@ -65,9 +65,7 @@ public final StubSettings getStubSettings() { return stubSettings; } - /** - * @deprecated Please use {@link #getBackgroundExecutorProvider()} - */ + /** @deprecated Please use {@link #getBackgroundExecutorProvider()} */ @Deprecated public final ExecutorProvider getExecutorProvider() { return stubSettings.getExecutorProvider(); @@ -312,6 +310,8 @@ public B setGdchApiAudience(@Nullable String gdchApiAudience) { /** * Sets the API key. The API key will get translated to an [ApiKeyCredentials] and stored in * [CallContext]. + * + * Note: you can not set an API key and credentials object in the same Settings. It will fail when creating the client. */ public B setApiKey(String apiKey) { stubSettings.setApiKey(apiKey); diff --git a/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java b/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java index 294f0a6432..e3312a0dff 100644 --- a/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java +++ b/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java @@ -1080,7 +1080,6 @@ public void testSetApiKey_createsApiCredentials() throws IOException { new FakeTransportProvider( transportChannel, executor, true, ImmutableMap.of(), null, DEFAULT_ENDPOINT); builder.setTransportChannelProvider(transportProvider); - HeaderProvider headerProvider = Mockito.mock(HeaderProvider.class); Mockito.when(headerProvider.getHeaders()).thenReturn(ImmutableMap.of()); builder.setHeaderProvider(headerProvider); @@ -1102,7 +1101,6 @@ void testCreateClient_throwsErrorIfApiKeyAndCredentialsAreProvided() throws Exce new FakeTransportProvider( transportChannel, executor, true, ImmutableMap.of(), null, DEFAULT_ENDPOINT); builder.setTransportChannelProvider(transportProvider); - HeaderProvider headerProvider = Mockito.mock(HeaderProvider.class); Mockito.when(headerProvider.getHeaders()).thenReturn(ImmutableMap.of()); builder.setHeaderProvider(headerProvider); @@ -1110,7 +1108,7 @@ void testCreateClient_throwsErrorIfApiKeyAndCredentialsAreProvided() throws Exce builder.setCredentialsProvider(Mockito.mock(CredentialsProvider.class)); try { - ClientContext context = ClientContext.create(builder.build()); + ClientContext.create(builder.build()); fail("No exception raised"); } catch (IllegalArgumentException e) { assert( diff --git a/gax-java/pom.xml b/gax-java/pom.xml index 99025fc608..e8014aa2fd 100644 --- a/gax-java/pom.xml +++ b/gax-java/pom.xml @@ -4,14 +4,14 @@ com.google.api gax-parent pom - 2.52.1-SNAPSHOT + 2.53.1-SNAPSHOT GAX (Google Api eXtensions) for Java (Parent) Google Api eXtensions for Java (Parent) com.google.api gapic-generator-java-pom-parent - 2.44.1-SNAPSHOT + 2.45.1-SNAPSHOT ../gapic-generator-java-pom-parent @@ -50,7 +50,7 @@ com.google.api api-common - 2.35.1-SNAPSHOT + 2.36.1-SNAPSHOT com.google.auth @@ -98,24 +98,24 @@ com.google.api gax - 2.52.1-SNAPSHOT + 2.53.1-SNAPSHOT com.google.api gax - 2.52.1-SNAPSHOT + 2.53.1-SNAPSHOT test-jar testlib com.google.api.grpc proto-google-common-protos - 2.43.1-SNAPSHOT + 2.44.1-SNAPSHOT com.google.api.grpc grpc-google-common-protos - 2.43.1-SNAPSHOT + 2.44.1-SNAPSHOT io.grpc From fa251cf0e38947edf5e7f9aafa4a3d33552adb6c Mon Sep 17 00:00:00 2001 From: detmerl Date: Tue, 17 Sep 2024 11:16:29 -0400 Subject: [PATCH 09/38] clean up --- .../com/google/api/gax/rpc/ClientContext.java | 8 +++----- .../google/api/gax/rpc/ClientSettings.java | 3 ++- .../com/google/api/gax/rpc/StubSettings.java | 20 +++++-------------- .../google/api/gax/rpc/ClientContextTest.java | 18 ++++++++++------- 4 files changed, 21 insertions(+), 28 deletions(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java index 40b84bbe33..974b99d0c1 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java @@ -180,7 +180,7 @@ public static ClientContext create(StubSettings settings) throws IOException { Credentials credentials = settings.getCredentialsProvider().getCredentials(); if (apiKey != null && credentials != null) { throw new IllegalArgumentException( - "You can not provide both ApiKey and Credentials for a client."); + "You can not provide both ApiKey and Credentials for a client."); } if (apiKey != null) { // if API key exists it becomes the default credential @@ -207,8 +207,7 @@ public static ClientContext create(StubSettings settings) throws IOException { if (settings.getQuotaProjectId() != null && credentials != null) { // If the quotaProjectId is set, wrap original credentials with correct quotaProjectId as // QuotaProjectIdHidingCredentials. - // Ensure that a custom set quota project id takes priority over one detected by - // credentials. + // Ensure that a custom set quota project id takes priority over one detected by credentials. // Avoid the backend receiving possibly conflict values of quotaProjectId credentials = new QuotaProjectIdHidingCredentials(credentials); } @@ -306,8 +305,7 @@ private static Credentials getGdchCredentials( try { gdchAudienceUri = URI.create(audienceString); } catch (IllegalArgumentException ex) { // thrown when passing a malformed uri string - throw new IllegalArgumentException( - "The GDC-H API audience string is not a valid URI", ex); + throw new IllegalArgumentException("The GDC-H API audience string is not a valid URI", ex); } credentials = ((GdchCredentials) credentials).createWithGdchAudience(gdchAudienceUri); return credentials; diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java index 13aa1229b7..01c8c3748c 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java @@ -311,7 +311,8 @@ public B setGdchApiAudience(@Nullable String gdchApiAudience) { * Sets the API key. The API key will get translated to an [ApiKeyCredentials] and stored in * [CallContext]. * - * Note: you can not set an API key and credentials object in the same Settings. It will fail when creating the client. + *

Note: you can not set an API key and credentials object in the same Settings. It will fail + * when creating the client. */ public B setApiKey(String apiKey) { stubSettings.setApiKey(apiKey); diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java index 1b88cf552a..0876f35346 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java @@ -132,9 +132,7 @@ private EndpointContext buildEndpointContext(Builder builder) { } } - /** - * @deprecated Please use {@link #getBackgroundExecutorProvider()}. - */ + /** @deprecated Please use {@link #getBackgroundExecutorProvider()}. */ @Deprecated public final ExecutorProvider getExecutorProvider() { return deprecatedExecutorProviderSet ? backgroundExecutorProvider : null; @@ -176,23 +174,17 @@ protected String getServiceName() { return ""; } - /** - * @return the fully resolved universe domain used by the client - */ + /** @return the fully resolved universe domain used by the client */ public final String getUniverseDomain() { return endpointContext.resolvedUniverseDomain(); } - /** - * @return the fully resolved endpoint used by the client - */ + /** @return the fully resolved endpoint used by the client */ public String getEndpoint() { return endpointContext.resolvedEndpoint(); } - /** - * @return the newly created EndpointContext - */ + /** @return the newly created EndpointContext */ @InternalApi final EndpointContext getEndpointContext() { return endpointContext; @@ -605,9 +597,7 @@ public B setApiKey(String apiKey) { return self(); } - /** - * @deprecated Please use {@link #getBackgroundExecutorProvider()}. - */ + /** @deprecated Please use {@link #getBackgroundExecutorProvider()}. */ @Deprecated public ExecutorProvider getExecutorProvider() { return deprecatedExecutorProviderSet ? backgroundExecutorProvider : null; diff --git a/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java b/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java index e3312a0dff..771a73b061 100644 --- a/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java +++ b/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java @@ -31,7 +31,13 @@ import static com.google.api.gax.util.TimeConversionTestUtils.testDurationMethod; import static com.google.common.truth.Truth.assertThat; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNotSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -62,8 +68,6 @@ import java.util.concurrent.Executor; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledThreadPoolExecutor; - -import org.apache.http.impl.client.SystemDefaultCredentialsProvider; import org.junit.jupiter.api.Test; import org.mockito.Mockito; @@ -1098,8 +1102,8 @@ void testCreateClient_throwsErrorIfApiKeyAndCredentialsAreProvided() throws Exce InterceptingExecutor executor = new InterceptingExecutor(1); FakeTransportChannel transportChannel = FakeTransportChannel.create(new FakeChannel()); FakeTransportProvider transportProvider = - new FakeTransportProvider( - transportChannel, executor, true, ImmutableMap.of(), null, DEFAULT_ENDPOINT); + new FakeTransportProvider( + transportChannel, executor, true, ImmutableMap.of(), null, DEFAULT_ENDPOINT); builder.setTransportChannelProvider(transportProvider); HeaderProvider headerProvider = Mockito.mock(HeaderProvider.class); Mockito.when(headerProvider.getHeaders()).thenReturn(ImmutableMap.of()); @@ -1111,8 +1115,8 @@ void testCreateClient_throwsErrorIfApiKeyAndCredentialsAreProvided() throws Exce ClientContext.create(builder.build()); fail("No exception raised"); } catch (IllegalArgumentException e) { - assert( - e.getMessage().contains("You can not provide both ApiKey and Credentials for a client.")); + assert (e.getMessage() + .contains("You can not provide both ApiKey and Credentials for a client.")); } } } From 334a4e81a65bfe9a08a876078ceb0460331b92fc Mon Sep 17 00:00:00 2001 From: detmerl Date: Thu, 19 Sep 2024 11:24:07 -0400 Subject: [PATCH 10/38] cleaned up tests/logic --- .../com/google/api/gax/rpc/ClientContext.java | 34 +++++++++---------- .../com/google/api/gax/rpc/StubSettings.java | 4 +-- .../google/api/gax/rpc/ClientContextTest.java | 4 ++- 3 files changed, 22 insertions(+), 20 deletions(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java index 974b99d0c1..eb3485258f 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java @@ -185,23 +185,23 @@ public static ClientContext create(StubSettings settings) throws IOException { if (apiKey != null) { // if API key exists it becomes the default credential credentials = ApiKeyCredentials.create(settings.getApiKey()); - } - - // check if need to adjust credentials/endpoint/endpointContext for GDC-H - String settingsGdchApiAudience = settings.getGdchApiAudience(); - boolean usingGDCH = credentials instanceof GdchCredentials; - if (usingGDCH) { - // Can only determine if the GDC-H is being used via the Credentials. The Credentials object - // is resolved in the ClientContext and must be passed to the EndpointContext. Rebuild the - // endpointContext only on GDC-H flows. - endpointContext = endpointContext.withGDCH(); - // Resolve the new endpoint with the GDC-H flow - endpoint = endpointContext.resolvedEndpoint(); - // We recompute the GdchCredentials with the audience - credentials = getGdchCredentials(settingsGdchApiAudience, endpoint, credentials); - } else if (!Strings.isNullOrEmpty(settingsGdchApiAudience)) { - throw new IllegalArgumentException( - "GDC-H API audience can only be set when using GdchCredentials"); + } else { + // check if need to adjust credentials/endpoint/endpointContext for GDC-H + String settingsGdchApiAudience = settings.getGdchApiAudience(); + boolean usingGDCH = credentials instanceof GdchCredentials; + if (usingGDCH) { + // Can only determine if the GDC-H is being used via the Credentials. The Credentials object + // is resolved in the ClientContext and must be passed to the EndpointContext. Rebuild the + // endpointContext only on GDC-H flows. + endpointContext = endpointContext.withGDCH(); + // Resolve the new endpoint with the GDC-H flow + endpoint = endpointContext.resolvedEndpoint(); + // We recompute the GdchCredentials with the audience + credentials = getGdchCredentials(settingsGdchApiAudience, endpoint, credentials); + } else if (!Strings.isNullOrEmpty(settingsGdchApiAudience)) { + throw new IllegalArgumentException( + "GDC-H API audience can only be set when using GdchCredentials"); + } } if (settings.getQuotaProjectId() != null && credentials != null) { diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java index 0876f35346..800dbb9c1a 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java @@ -589,8 +589,8 @@ public B setTracerFactory(@Nonnull ApiTracerFactory tracerFactory) { } /** - * Sets the API key. The API key will get translated to an [ApiKeyCredentials] and stored in - * [CallContext]. + * Sets the API key. The API key will get translated to an {@link + * com.google.auth.ApiKeyCredentials} and stored in {@link ClientContext}. */ public B setApiKey(String apiKey) { this.apiKey = apiKey; diff --git a/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java b/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java index 771a73b061..7f6a3e86f5 100644 --- a/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java +++ b/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java @@ -1109,7 +1109,9 @@ void testCreateClient_throwsErrorIfApiKeyAndCredentialsAreProvided() throws Exce Mockito.when(headerProvider.getHeaders()).thenReturn(ImmutableMap.of()); builder.setHeaderProvider(headerProvider); builder.setApiKey(apiKey); - builder.setCredentialsProvider(Mockito.mock(CredentialsProvider.class)); + CredentialsProvider credentialsProvider = Mockito.mock(CredentialsProvider.class); + Mockito.when(credentialsProvider.getCredentials()).thenReturn(Mockito.mock(Credentials.class)); + builder.setCredentialsProvider(credentialsProvider); try { ClientContext.create(builder.build()); From edb658feb50a48413f00b3e260e02a6a110c2c8f Mon Sep 17 00:00:00 2001 From: detmerl Date: Thu, 19 Sep 2024 11:26:36 -0400 Subject: [PATCH 11/38] cleaned up formatting --- .../src/main/java/com/google/api/gax/rpc/ClientContext.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java index eb3485258f..e3fa86efcc 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java @@ -288,7 +288,8 @@ public static ClientContext create(StubSettings settings) throws IOException { } /** - * Constructs a new {@link Credentials} object based on credentials provided with a GDC-H audience + * Constructs a new {@link com.google.auth.Credentials} object based on credentials provided with + * a GDC-H audience */ private static Credentials getGdchCredentials( String settingsGdchApiAudience, String endpoint, Credentials credentials) throws IOException { From 33f64a101812d8173ae2271805a995b005b21b8a Mon Sep 17 00:00:00 2001 From: detmerl Date: Thu, 19 Sep 2024 11:44:18 -0400 Subject: [PATCH 12/38] updated to use assertThrows --- .../com/google/api/gax/rpc/ClientContextTest.java | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java b/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java index 7f6a3e86f5..b41556d580 100644 --- a/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java +++ b/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java @@ -37,7 +37,6 @@ import static org.junit.jupiter.api.Assertions.assertNotSame; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.junit.jupiter.api.Assertions.fail; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -68,6 +67,7 @@ import java.util.concurrent.Executor; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledThreadPoolExecutor; +import org.junit.*; import org.junit.jupiter.api.Test; import org.mockito.Mockito; @@ -1113,12 +1113,7 @@ void testCreateClient_throwsErrorIfApiKeyAndCredentialsAreProvided() throws Exce Mockito.when(credentialsProvider.getCredentials()).thenReturn(Mockito.mock(Credentials.class)); builder.setCredentialsProvider(credentialsProvider); - try { - ClientContext.create(builder.build()); - fail("No exception raised"); - } catch (IllegalArgumentException e) { - assert (e.getMessage() - .contains("You can not provide both ApiKey and Credentials for a client.")); - } + Assert.assertThrows( + IllegalArgumentException.class, () -> ClientContext.create(builder.build())); } } From ae0f28103f58fa838605b75cf6b960a22f8a3d4a Mon Sep 17 00:00:00 2001 From: detmerl Date: Mon, 23 Sep 2024 10:01:46 -0400 Subject: [PATCH 13/38] fixed imports --- .../test/java/com/google/api/gax/rpc/ClientContextTest.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java b/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java index b41556d580..1c11709bd3 100644 --- a/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java +++ b/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java @@ -67,7 +67,6 @@ import java.util.concurrent.Executor; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledThreadPoolExecutor; -import org.junit.*; import org.junit.jupiter.api.Test; import org.mockito.Mockito; @@ -1113,7 +1112,6 @@ void testCreateClient_throwsErrorIfApiKeyAndCredentialsAreProvided() throws Exce Mockito.when(credentialsProvider.getCredentials()).thenReturn(Mockito.mock(Credentials.class)); builder.setCredentialsProvider(credentialsProvider); - Assert.assertThrows( - IllegalArgumentException.class, () -> ClientContext.create(builder.build())); + assertThrows(IllegalArgumentException.class, () -> ClientContext.create(builder.build())); } } From 5e346e6a763aec46317809fe1f6aabf19c02f100 Mon Sep 17 00:00:00 2001 From: detmerl Date: Mon, 23 Sep 2024 10:09:01 -0400 Subject: [PATCH 14/38] fixed imports --- .../java/com/google/api/gax/rpc/ClientSettingsTest.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientSettingsTest.java b/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientSettingsTest.java index 79a9818fe9..c2d4627d0f 100644 --- a/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientSettingsTest.java +++ b/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientSettingsTest.java @@ -30,6 +30,8 @@ package com.google.api.gax.rpc; import static com.google.api.gax.util.TimeConversionTestUtils.testDurationMethod; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.fail; import static org.mockito.Mockito.when; @@ -57,7 +59,6 @@ import java.util.concurrent.ScheduledExecutorService; import java.util.function.Function; import java.util.function.Supplier; -import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.mockito.Mockito; @@ -588,9 +589,7 @@ void testEmptyApiKeyClientSettingsBuild_isTreatedAsNull() throws Exception { builder.setApiKey(""); FakeClientSettings fakeClientSettings = builder.build(); - - Assertions.assertEquals( - fakeClientSettings.getCredentialsProvider().getCredentials(), credentials); - Assertions.assertNull(fakeClientSettings.getApiKey()); + assertEquals(fakeClientSettings.getCredentialsProvider().getCredentials(), credentials); + assertNull(fakeClientSettings.getApiKey()); } } From 66cc0a7bf1b99e422014d93593de95175dade183 Mon Sep 17 00:00:00 2001 From: detmerl Date: Mon, 23 Sep 2024 19:07:12 -0400 Subject: [PATCH 15/38] updated logic to validate if multiple credentials are passed in via an end user + added showcase tests --- .../com/google/api/gax/rpc/ClientContext.java | 7 +- .../google/api/gax/rpc/ClientSettings.java | 13 +- .../google/api/gax/rpc/ClientContextTest.java | 12 +- .../api/gax/rpc/ClientSettingsTest.java | 17 +- .../api/gax/rpc/testing/FakeStubSettings.java | 23 +++ .../showcase/v1beta1/it/ITApiCredentials.java | 146 ++++++++++++++++++ .../v1beta1/it/ITApiVersionHeaders.java | 98 +----------- .../util/GrpcCapturingClientInterceptor.java | 102 ++++++++++++ .../HttpJsonCapturingClientInterceptor.java | 87 +++++++++++ 9 files changed, 390 insertions(+), 115 deletions(-) create mode 100644 showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITApiCredentials.java create mode 100644 showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/util/GrpcCapturingClientInterceptor.java create mode 100644 showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/util/HttpJsonCapturingClientInterceptor.java diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java index e3fa86efcc..c410a51415 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java @@ -177,15 +177,12 @@ public static ClientContext create(StubSettings settings) throws IOException { EndpointContext endpointContext = settings.getEndpointContext(); String endpoint = endpointContext.resolvedEndpoint(); String apiKey = settings.getApiKey(); - Credentials credentials = settings.getCredentialsProvider().getCredentials(); - if (apiKey != null && credentials != null) { - throw new IllegalArgumentException( - "You can not provide both ApiKey and Credentials for a client."); - } + Credentials credentials; if (apiKey != null) { // if API key exists it becomes the default credential credentials = ApiKeyCredentials.create(settings.getApiKey()); } else { + credentials = settings.getCredentialsProvider().getCredentials(); // check if need to adjust credentials/endpoint/endpointContext for GDC-H String settingsGdchApiAudience = settings.getGdchApiAudience(); boolean usingGDCH = credentials instanceof GdchCredentials; diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java index 01c8c3748c..383ebf9272 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java @@ -58,6 +58,10 @@ public abstract class ClientSettings /** Constructs an instance of ClientSettings. */ protected ClientSettings(Builder builder) throws IOException { + if (builder.stubSettings.getApiKey() != null && builder.explicitCredentialsProvided) { + throw new IllegalArgumentException( + "You can not provide both ApiKey and Credentials for a client."); + } this.stubSettings = builder.stubSettings.build(); } @@ -159,6 +163,8 @@ public abstract static class Builder< private StubSettings.Builder stubSettings; + private boolean explicitCredentialsProvided = false; + /** Create a builder from a ClientSettings object. */ protected Builder(ClientSettings settings) { this.stubSettings = settings.stubSettings.toBuilder(); @@ -213,6 +219,7 @@ public B setBackgroundExecutorProvider(ExecutorProvider executorProvider) { /** Sets the CredentialsProvider to use for getting the credentials to make calls with. */ public B setCredentialsProvider(CredentialsProvider credentialsProvider) { + explicitCredentialsProvided = true; stubSettings.setCredentialsProvider(credentialsProvider); return self(); } @@ -298,9 +305,9 @@ public B setWatchdogCheckIntervalDuration(@Nullable java.time.Duration checkInte /** * Sets the GDC-H api audience. This is intended only to be used with {@link * com.google.auth.oauth2.GdchCredentials} If this field is set and other type of {@link - * com.google.auth.Credentials} is used then an {@link IllegalArgumentException} will be thrown. - * If the provided credentials already have an api audience, then it will be overriden by this - * audience + * com.google.auth.Credentials} is used then an {@link java.lang.IllegalArgumentException} will + * be thrown. If the provided credentials already have an api audience, then it will be + * overriden by this audience */ public B setGdchApiAudience(@Nullable String gdchApiAudience) { stubSettings.setGdchApiAudience(gdchApiAudience); diff --git a/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java b/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java index 1c11709bd3..95956e5963 100644 --- a/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java +++ b/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java @@ -1095,9 +1095,9 @@ public void testSetApiKey_createsApiCredentials() throws IOException { } @Test - void testCreateClient_throwsErrorIfApiKeyAndCredentialsAreProvided() throws Exception { + public void testSetApiKey_withDefaultCredentials_overridesCredentials() throws IOException { String apiKey = "key"; - FakeStubSettings.Builder builder = new FakeStubSettings.Builder(); + FakeStubSettings.Builder builder = FakeStubSettings.newBuilder(); InterceptingExecutor executor = new InterceptingExecutor(1); FakeTransportChannel transportChannel = FakeTransportChannel.create(new FakeChannel()); FakeTransportProvider transportProvider = @@ -1108,10 +1108,10 @@ void testCreateClient_throwsErrorIfApiKeyAndCredentialsAreProvided() throws Exce Mockito.when(headerProvider.getHeaders()).thenReturn(ImmutableMap.of()); builder.setHeaderProvider(headerProvider); builder.setApiKey(apiKey); - CredentialsProvider credentialsProvider = Mockito.mock(CredentialsProvider.class); - Mockito.when(credentialsProvider.getCredentials()).thenReturn(Mockito.mock(Credentials.class)); - builder.setCredentialsProvider(credentialsProvider); - assertThrows(IllegalArgumentException.class, () -> ClientContext.create(builder.build())); + ClientContext context = ClientContext.create(builder.build()); + + FakeCallContext fakeCallContext = (FakeCallContext) context.getDefaultCallContext(); + assertThat(fakeCallContext.getCredentials()).isInstanceOf(ApiKeyCredentials.class); } } diff --git a/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientSettingsTest.java b/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientSettingsTest.java index c2d4627d0f..930e7a906b 100644 --- a/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientSettingsTest.java +++ b/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientSettingsTest.java @@ -30,9 +30,7 @@ package com.google.api.gax.rpc; import static com.google.api.gax.util.TimeConversionTestUtils.testDurationMethod; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.fail; +import static org.junit.jupiter.api.Assertions.*; import static org.mockito.Mockito.when; import com.google.api.core.ApiClock; @@ -282,7 +280,6 @@ void testBuilderFromSettings() throws Exception { builder.setWatchdogProvider(watchdogProvider); builder.setWatchdogCheckIntervalDuration(watchdogCheckInterval); builder.setQuotaProjectId(quotaProjectId); - builder.setApiKey(apiKey); FakeClientSettings settings = builder.build(); FakeClientSettings.Builder newBuilder = new FakeClientSettings.Builder(settings); @@ -299,7 +296,6 @@ void testBuilderFromSettings() throws Exception { Truth.assertThat(newBuilder.getWatchdogCheckIntervalDuration()) .isEqualTo(watchdogCheckInterval); Truth.assertThat(newBuilder.getQuotaProjectId()).isEqualTo(quotaProjectId); - Truth.assertThat(newBuilder.getApiKey()).isEqualTo(apiKey); } @Test @@ -579,6 +575,17 @@ public void testWatchdogCheckInterval_backportMethodsBehaveCorrectly() { cs -> cs.getWatchdogCheckInterval()); } + @Test + void testClientSettingsBuilder_throwsErrorIfApiKeyAndCredentialsAreProvided() throws Exception { + FakeClientSettings.Builder builder = new FakeClientSettings.Builder(); + CredentialsProvider credentialsProvider = Mockito.mock(CredentialsProvider.class); + when(credentialsProvider.getCredentials()).thenReturn(Mockito.mock(Credentials.class)); + builder.setCredentialsProvider(credentialsProvider); + builder.setApiKey("api_key"); + + assertThrows(IllegalArgumentException.class, builder::build); + } + @Test void testEmptyApiKeyClientSettingsBuild_isTreatedAsNull() throws Exception { FakeClientSettings.Builder builder = new FakeClientSettings.Builder(); diff --git a/gax-java/gax/src/test/java/com/google/api/gax/rpc/testing/FakeStubSettings.java b/gax-java/gax/src/test/java/com/google/api/gax/rpc/testing/FakeStubSettings.java index c5f0cc81f4..d42b71784a 100644 --- a/gax-java/gax/src/test/java/com/google/api/gax/rpc/testing/FakeStubSettings.java +++ b/gax-java/gax/src/test/java/com/google/api/gax/rpc/testing/FakeStubSettings.java @@ -30,17 +30,26 @@ package com.google.api.gax.rpc.testing; import com.google.api.core.InternalApi; +import com.google.api.gax.core.GoogleCredentialsProvider; import com.google.api.gax.rpc.ClientContext; import com.google.api.gax.rpc.StubSettings; +import com.google.common.collect.ImmutableList; import java.io.IOException; @InternalApi("for testing") public class FakeStubSettings extends StubSettings { + private static final ImmutableList DEFAULT_SERVICE_SCOPES = + ImmutableList.builder().build(); + private FakeStubSettings(Builder builder) throws IOException { super(builder); } + public static Builder newBuilder() { + return Builder.createDefault(); + } + @Override public String getServiceName() { return "test"; @@ -69,5 +78,19 @@ public Builder() { public com.google.api.gax.rpc.testing.FakeStubSettings build() throws IOException { return new com.google.api.gax.rpc.testing.FakeStubSettings(this); } + + /** Returns default settings values. */ + public static Builder createDefault() { + Builder builder = new Builder(((ClientContext) null)); + builder.setCredentialsProvider(defaultCredentialsProviderBuilder()); + return builder; + } + + /** Returns default credentials provider. */ + public static GoogleCredentialsProvider defaultCredentialsProviderBuilder() { + return GoogleCredentialsProvider.newBuilder() + .setScopesToApply(DEFAULT_SERVICE_SCOPES) + .setUseJwtAccessWithScope(true).build(); + } } } diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITApiCredentials.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITApiCredentials.java new file mode 100644 index 0000000000..147ec16b99 --- /dev/null +++ b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITApiCredentials.java @@ -0,0 +1,146 @@ +/* + * Copyright 2024 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.showcase.v1beta1.it; + +import static com.google.common.truth.Truth.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import com.google.api.client.http.javanet.NetHttpTransport; +import com.google.api.gax.core.FixedCredentialsProvider; +import com.google.auth.ApiKeyCredentials; +import com.google.common.collect.ImmutableList; +import com.google.showcase.v1beta1.EchoClient; +import com.google.showcase.v1beta1.EchoRequest; +import com.google.showcase.v1beta1.EchoSettings; +import com.google.showcase.v1beta1.it.util.GrpcCapturingClientInterceptor; +import com.google.showcase.v1beta1.it.util.HttpJsonCapturingClientInterceptor; +import com.google.showcase.v1beta1.it.util.TestClientInitializer; +import io.grpc.ManagedChannelBuilder; +import io.grpc.Metadata; +import java.io.IOException; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.concurrent.TimeUnit; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import com.google.auth.http.AuthHttpConstants; + +/** Test suite to confirm a client can be instantiated with API key credentials and sent to back end*/ +class ITApiCredentials { + + private static final String API_KEY = "fake_api_key"; + private static final String API_KEY_AUTH_HEADER = "x-goog-api-key"; + private static final String HTTP_RESPONSE_HEADER_STRING = + "x-showcase-request-" + API_KEY_AUTH_HEADER; + private static final String GRPC_ENDPOINT = "localhost:7469"; + private static final String HTTP_ENDPOINT = "http://localhost:7469"; + + private static HttpJsonCapturingClientInterceptor httpJsonInterceptor; + private static EchoClient grpcClient; + private static EchoClient httpJsonClient; + private static GrpcCapturingClientInterceptor grpcInterceptor; + + @BeforeEach + void setup() throws IOException { + grpcInterceptor = new GrpcCapturingClientInterceptor(); + httpJsonInterceptor = new HttpJsonCapturingClientInterceptor(); + } + + @AfterEach + void tearDown() throws InterruptedException { + if (httpJsonClient != null) { + httpJsonClient.close(); + httpJsonClient.awaitTermination( + TestClientInitializer.AWAIT_TERMINATION_SECONDS, TimeUnit.SECONDS); + } + if (grpcClient != null) { + grpcClient.close(); + grpcClient.awaitTermination( + TestClientInitializer.AWAIT_TERMINATION_SECONDS, TimeUnit.SECONDS); + } + } + + @Test + void testCreateGrpcClient_withApiKey_sendsApiHeaderToServer() throws IOException { + Metadata.Key apiKeyAuthHeader = + Metadata.Key.of(API_KEY_AUTH_HEADER, Metadata.ASCII_STRING_MARSHALLER); + Metadata.Key defaultAuthorizationHeader = + Metadata.Key.of(AuthHttpConstants.AUTHORIZATION, Metadata.ASCII_STRING_MARSHALLER); + EchoSettings settings = + EchoSettings.newBuilder() + .setApiKey(API_KEY) + .setEndpoint(GRPC_ENDPOINT) + .setTransportChannelProvider( + EchoSettings.defaultGrpcTransportProviderBuilder() + .setChannelConfigurator(ManagedChannelBuilder::usePlaintext) + .setInterceptorProvider(() -> ImmutableList.of(grpcInterceptor)) + .build()) + .build(); + grpcClient = EchoClient.create(settings); + + grpcClient.echo(EchoRequest.newBuilder().build()); + + String headerValue = grpcInterceptor.metadata.get(apiKeyAuthHeader); + assertThat(headerValue).isEqualTo(API_KEY); + String defaultAuthorizationHeaderValue = + grpcInterceptor.metadata.get(defaultAuthorizationHeader); + assertThat(defaultAuthorizationHeaderValue).isNull(); + } + + @Test + void testCreateGrpcClient_withApiKeyAndExplicitCredentials_throwsException() throws IOException { + assertThrows( + IllegalArgumentException.class, + () -> + EchoSettings.newBuilder() + .setApiKey(API_KEY) + .setEndpoint(GRPC_ENDPOINT) + .setCredentialsProvider( + FixedCredentialsProvider.create(ApiKeyCredentials.create("invalid"))) + .setTransportChannelProvider( + EchoSettings.defaultGrpcTransportProviderBuilder() + .setChannelConfigurator(ManagedChannelBuilder::usePlaintext) + .setInterceptorProvider(() -> ImmutableList.of(grpcInterceptor)) + .build()) + .build()); + } + + @Test + void testCreateHttpClient_withApiKey_sendsApiHeaderToServer() throws Exception { + EchoSettings httpJsonEchoSettings = + EchoSettings.newHttpJsonBuilder() + .setApiKey(API_KEY) + .setTransportChannelProvider( + EchoSettings.defaultHttpJsonTransportProviderBuilder() + .setHttpTransport( + new NetHttpTransport.Builder().doNotValidateCertificate().build()) + .setEndpoint(HTTP_ENDPOINT) + .setInterceptorProvider(() -> ImmutableList.of(httpJsonInterceptor)) + .build()) + .build(); + httpJsonClient = EchoClient.create(httpJsonEchoSettings); + + httpJsonClient.echo(EchoRequest.newBuilder().build()); + + ArrayList headerValues = + (ArrayList) + httpJsonInterceptor.metadata.getHeaders().get(HTTP_RESPONSE_HEADER_STRING); + String headerValue = headerValues.get(0); + assertThat(headerValue).isEqualTo(API_KEY); + } +} diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITApiVersionHeaders.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITApiVersionHeaders.java index 5f892c5886..cb823db498 100644 --- a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITApiVersionHeaders.java +++ b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITApiVersionHeaders.java @@ -24,6 +24,8 @@ import com.google.api.gax.rpc.StubSettings; import com.google.common.collect.ImmutableList; import com.google.showcase.v1beta1.*; +import com.google.showcase.v1beta1.it.util.GrpcCapturingClientInterceptor; +import com.google.showcase.v1beta1.it.util.HttpJsonCapturingClientInterceptor; import com.google.showcase.v1beta1.it.util.TestClientInitializer; import com.google.showcase.v1beta1.stub.ComplianceStubSettings; import com.google.showcase.v1beta1.stub.EchoStubSettings; @@ -52,102 +54,6 @@ class ITApiVersionHeaders { "Header provider can't override the header: " + ApiClientHeaderProvider.API_VERSION_HEADER_KEY; - // Implement a client interceptor to retrieve the trailing metadata from response. - private static class GrpcCapturingClientInterceptor implements ClientInterceptor { - private Metadata metadata; - - @Override - public ClientCall interceptCall( - MethodDescriptor method, final CallOptions callOptions, Channel next) { - ClientCall call = next.newCall(method, callOptions); - return new ForwardingClientCall.SimpleForwardingClientCall(call) { - @Override - public void start(Listener responseListener, Metadata headers) { - Listener wrappedListener = - new SimpleForwardingClientCallListener(responseListener) { - @Override - public void onClose(Status status, Metadata trailers) { - if (status.isOk()) { - metadata = trailers; - } - super.onClose(status, trailers); - } - }; - - super.start(wrappedListener, headers); - } - }; - } - } - - private static class SimpleForwardingClientCallListener - extends ClientCall.Listener { - private final ClientCall.Listener delegate; - - SimpleForwardingClientCallListener(ClientCall.Listener delegate) { - this.delegate = delegate; - } - - @Override - public void onHeaders(Metadata headers) { - delegate.onHeaders(headers); - } - - @Override - public void onMessage(RespT message) { - delegate.onMessage(message); - } - - @Override - public void onClose(Status status, Metadata trailers) { - delegate.onClose(status, trailers); - } - - @Override - public void onReady() { - delegate.onReady(); - } - } - // Implement a client interceptor to retrieve the response headers - private static class HttpJsonCapturingClientInterceptor implements HttpJsonClientInterceptor { - private HttpJsonMetadata metadata; - - @Override - public HttpJsonClientCall interceptCall( - ApiMethodDescriptor method, - HttpJsonCallOptions callOptions, - HttpJsonChannel next) { - HttpJsonClientCall call = next.newCall(method, callOptions); - return new ForwardingHttpJsonClientCall.SimpleForwardingHttpJsonClientCall< - RequestT, ResponseT>(call) { - @Override - public void start(Listener responseListener, HttpJsonMetadata requestHeaders) { - Listener forwardingResponseListener = - new ForwardingHttpJsonClientCallListener.SimpleForwardingHttpJsonClientCallListener< - ResponseT>(responseListener) { - @Override - public void onHeaders(HttpJsonMetadata responseHeaders) { - metadata = responseHeaders; - super.onHeaders(responseHeaders); - } - - @Override - public void onMessage(ResponseT message) { - super.onMessage(message); - } - - @Override - public void onClose(int statusCode, HttpJsonMetadata trailers) { - super.onClose(statusCode, trailers); - } - }; - - super.start(forwardingResponseListener, requestHeaders); - } - }; - } - } - private static HttpJsonCapturingClientInterceptor httpJsonInterceptor; private static GrpcCapturingClientInterceptor grpcInterceptor; private static HttpJsonCapturingClientInterceptor httpJsonComplianceInterceptor; diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/util/GrpcCapturingClientInterceptor.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/util/GrpcCapturingClientInterceptor.java new file mode 100644 index 0000000000..bdd569e419 --- /dev/null +++ b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/util/GrpcCapturingClientInterceptor.java @@ -0,0 +1,102 @@ +/* + * Copyright 2024 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.showcase.v1beta1.it.util; + +import static com.google.common.truth.Truth.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import com.google.api.gax.rpc.ApiClientHeaderProvider; +import com.google.api.gax.rpc.FixedHeaderProvider; +import com.google.api.gax.rpc.StubSettings; +import com.google.common.collect.ImmutableList; +import com.google.showcase.v1beta1.it.util.TestClientInitializer; +import com.google.showcase.v1beta1.stub.ComplianceStubSettings; +import com.google.showcase.v1beta1.stub.EchoStubSettings; +import io.grpc.CallOptions; +import io.grpc.ClientInterceptor; +import io.grpc.Channel; +import io.grpc.ClientCall; + +import io.grpc.Metadata; +import io.grpc.MethodDescriptor; +import java.io.IOException; +import java.util.ArrayList; +import java.util.concurrent.TimeUnit; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import com.google.api.gax.httpjson.ApiMethodDescriptor; +import com.google.api.gax.httpjson.HttpJsonMetadata; +import io.grpc.ForwardingClientCall; +import io.grpc.Status; + + +/** Implements a client interceptor to retrieve the metadata from a GRPC client request. */ +public class GrpcCapturingClientInterceptor implements ClientInterceptor { + public Metadata metadata; + + @Override + public ClientCall interceptCall( + MethodDescriptor method, final CallOptions callOptions, Channel next) { + ClientCall call = next.newCall(method, callOptions); + return new ForwardingClientCall.SimpleForwardingClientCall(call) { + @Override + public void start(Listener responseListener, Metadata headers) { + Listener wrappedListener = + new SimpleForwardingClientCallListener(responseListener) { + @Override + public void onClose(Status status, Metadata trailers) { + if (status.isOk()) { + metadata = trailers; + } + super.onClose(status, trailers); + } + }; + + super.start(wrappedListener, headers); + } + }; + } + + private static class SimpleForwardingClientCallListener + extends ClientCall.Listener { + private final ClientCall.Listener delegate; + + SimpleForwardingClientCallListener(ClientCall.Listener delegate) { + this.delegate = delegate; + } + + @Override + public void onHeaders(Metadata headers) { + delegate.onHeaders(headers); + } + + @Override + public void onMessage(RespT message) { + delegate.onMessage(message); + } + + @Override + public void onClose(Status status, Metadata trailers) { + delegate.onClose(status, trailers); + } + + @Override + public void onReady() { + delegate.onReady(); + } + } +} diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/util/HttpJsonCapturingClientInterceptor.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/util/HttpJsonCapturingClientInterceptor.java new file mode 100644 index 0000000000..fdc3e4ab58 --- /dev/null +++ b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/util/HttpJsonCapturingClientInterceptor.java @@ -0,0 +1,87 @@ +/* + * Copyright 2024 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.showcase.v1beta1.it.util; + +import static com.google.common.truth.Truth.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; + + +import com.google.api.gax.rpc.ApiClientHeaderProvider; +import com.google.api.gax.httpjson.HttpJsonClientInterceptor; +import com.google.api.gax.rpc.FixedHeaderProvider; +import com.google.api.gax.rpc.StubSettings; +import com.google.common.collect.ImmutableList; +import com.google.showcase.v1beta1.it.util.GrpcCapturingClientInterceptor; +import com.google.showcase.v1beta1.it.util.HttpJsonCapturingClientInterceptor; +import com.google.showcase.v1beta1.it.util.TestClientInitializer; +import com.google.showcase.v1beta1.stub.ComplianceStubSettings; +import com.google.showcase.v1beta1.stub.EchoStubSettings; +import com.google.api.gax.httpjson.HttpJsonMetadata; +import java.io.IOException; +import com.google.common.collect.ImmutableList; +import java.util.ArrayList; +import java.util.concurrent.TimeUnit; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import com.google.api.gax.httpjson.HttpJsonCallOptions; +import com.google.api.gax.httpjson.HttpJsonChannel; +import com.google.api.gax.httpjson.HttpJsonClientCall; +import com.google.api.gax.httpjson.ApiMethodDescriptor; +import com.google.api.gax.httpjson.ForwardingHttpJsonClientCall; +import com.google.api.gax.httpjson.ForwardingHttpJsonClientCallListener; +import io.grpc.Status; + +/** Implements a client interceptor to retrieve the response headers from a HTTP client request. */ +public class HttpJsonCapturingClientInterceptor implements HttpJsonClientInterceptor { + public HttpJsonMetadata metadata; + + @Override + public HttpJsonClientCall interceptCall( + ApiMethodDescriptor method, + HttpJsonCallOptions callOptions, + HttpJsonChannel next) { + HttpJsonClientCall call = next.newCall(method, callOptions); + return new ForwardingHttpJsonClientCall.SimpleForwardingHttpJsonClientCall< + RequestT, ResponseT>(call) { + @Override + public void start(Listener responseListener, HttpJsonMetadata requestHeaders) { + Listener forwardingResponseListener = + new ForwardingHttpJsonClientCallListener.SimpleForwardingHttpJsonClientCallListener< + ResponseT>(responseListener) { + @Override + public void onHeaders(HttpJsonMetadata responseHeaders) { + metadata = responseHeaders; + super.onHeaders(responseHeaders); + } + + @Override + public void onMessage(ResponseT message) { + super.onMessage(message); + } + + @Override + public void onClose(int statusCode, HttpJsonMetadata trailers) { + super.onClose(statusCode, trailers); + } + }; + + super.start(forwardingResponseListener, requestHeaders); + } + }; + } +} \ No newline at end of file From 69c57e95fc1ea08ebd7e4bfcaefd29ee6a189e41 Mon Sep 17 00:00:00 2001 From: detmerl Date: Mon, 23 Sep 2024 19:07:42 -0400 Subject: [PATCH 16/38] formatting --- .../com/google/showcase/v1beta1/it/ITApiCredentials.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITApiCredentials.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITApiCredentials.java index 147ec16b99..b9774ee988 100644 --- a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITApiCredentials.java +++ b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITApiCredentials.java @@ -22,6 +22,7 @@ import com.google.api.client.http.javanet.NetHttpTransport; import com.google.api.gax.core.FixedCredentialsProvider; import com.google.auth.ApiKeyCredentials; +import com.google.auth.http.AuthHttpConstants; import com.google.common.collect.ImmutableList; import com.google.showcase.v1beta1.EchoClient; import com.google.showcase.v1beta1.EchoRequest; @@ -32,15 +33,15 @@ import io.grpc.ManagedChannelBuilder; import io.grpc.Metadata; import java.io.IOException; -import java.nio.file.Path; import java.util.ArrayList; import java.util.concurrent.TimeUnit; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import com.google.auth.http.AuthHttpConstants; -/** Test suite to confirm a client can be instantiated with API key credentials and sent to back end*/ +/** + * Test suite to confirm a client can be instantiated with API key credentials and sent to back end + */ class ITApiCredentials { private static final String API_KEY = "fake_api_key"; From 023a4e42b39f3137795766533a4ec457ed77d9ff Mon Sep 17 00:00:00 2001 From: detmerl Date: Mon, 23 Sep 2024 19:12:39 -0400 Subject: [PATCH 17/38] cleanup --- .../src/main/java/com/google/api/gax/rpc/ClientSettings.java | 2 +- .../java/com/google/showcase/v1beta1/it/ITApiCredentials.java | 3 ++- .../v1beta1/it/util/GrpcCapturingClientInterceptor.java | 2 +- .../v1beta1/it/util/HttpJsonCapturingClientInterceptor.java | 1 - 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java index 383ebf9272..4dfb6e2201 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java @@ -319,7 +319,7 @@ public B setGdchApiAudience(@Nullable String gdchApiAudience) { * [CallContext]. * *

Note: you can not set an API key and credentials object in the same Settings. It will fail - * when creating the client. + * when building the settings. */ public B setApiKey(String apiKey) { stubSettings.setApiKey(apiKey); diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITApiCredentials.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITApiCredentials.java index b9774ee988..b9f539cc8e 100644 --- a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITApiCredentials.java +++ b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITApiCredentials.java @@ -40,7 +40,8 @@ import org.junit.jupiter.api.Test; /** - * Test suite to confirm a client can be instantiated with API key credentials and sent to back end + * Test suite to confirm a client can be instantiated with API key credentials and correct + * authorization sent via headers to back end */ class ITApiCredentials { diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/util/GrpcCapturingClientInterceptor.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/util/GrpcCapturingClientInterceptor.java index bdd569e419..156ee16e47 100644 --- a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/util/GrpcCapturingClientInterceptor.java +++ b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/util/GrpcCapturingClientInterceptor.java @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + package com.google.showcase.v1beta1.it.util; import static com.google.common.truth.Truth.assertThat; @@ -43,7 +44,6 @@ import io.grpc.ForwardingClientCall; import io.grpc.Status; - /** Implements a client interceptor to retrieve the metadata from a GRPC client request. */ public class GrpcCapturingClientInterceptor implements ClientInterceptor { public Metadata metadata; diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/util/HttpJsonCapturingClientInterceptor.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/util/HttpJsonCapturingClientInterceptor.java index fdc3e4ab58..777398cc96 100644 --- a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/util/HttpJsonCapturingClientInterceptor.java +++ b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/util/HttpJsonCapturingClientInterceptor.java @@ -19,7 +19,6 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.jupiter.api.Assertions.assertThrows; - import com.google.api.gax.rpc.ApiClientHeaderProvider; import com.google.api.gax.httpjson.HttpJsonClientInterceptor; import com.google.api.gax.rpc.FixedHeaderProvider; From d7c7a72e06de7f1583662acd800f7755f95ce815 Mon Sep 17 00:00:00 2001 From: detmerl Date: Tue, 24 Sep 2024 07:46:29 -0400 Subject: [PATCH 18/38] cleanup --- .../com/google/api/gax/rpc/testing/FakeStubSettings.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/gax-java/gax/src/test/java/com/google/api/gax/rpc/testing/FakeStubSettings.java b/gax-java/gax/src/test/java/com/google/api/gax/rpc/testing/FakeStubSettings.java index d42b71784a..4004a24404 100644 --- a/gax-java/gax/src/test/java/com/google/api/gax/rpc/testing/FakeStubSettings.java +++ b/gax-java/gax/src/test/java/com/google/api/gax/rpc/testing/FakeStubSettings.java @@ -40,7 +40,7 @@ public class FakeStubSettings extends StubSettings { private static final ImmutableList DEFAULT_SERVICE_SCOPES = - ImmutableList.builder().build(); + ImmutableList.builder().build(); private FakeStubSettings(Builder builder) throws IOException { super(builder); @@ -89,8 +89,9 @@ public static Builder createDefault() { /** Returns default credentials provider. */ public static GoogleCredentialsProvider defaultCredentialsProviderBuilder() { return GoogleCredentialsProvider.newBuilder() - .setScopesToApply(DEFAULT_SERVICE_SCOPES) - .setUseJwtAccessWithScope(true).build(); + .setScopesToApply(DEFAULT_SERVICE_SCOPES) + .setUseJwtAccessWithScope(true) + .build(); } } } From 0d48f413b6d84115e9c5148a40f4005b4e6ce880 Mon Sep 17 00:00:00 2001 From: detmerl Date: Tue, 24 Sep 2024 08:25:45 -0400 Subject: [PATCH 19/38] cleanup --- .../java/com/google/api/gax/rpc/ClientSettings.java | 10 +++++----- .../com/google/api/gax/rpc/ClientSettingsTest.java | 5 ++++- .../google/showcase/v1beta1/it/ITApiCredentials.java | 1 + 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java index 4dfb6e2201..7cf0ef737d 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java @@ -305,9 +305,9 @@ public B setWatchdogCheckIntervalDuration(@Nullable java.time.Duration checkInte /** * Sets the GDC-H api audience. This is intended only to be used with {@link * com.google.auth.oauth2.GdchCredentials} If this field is set and other type of {@link - * com.google.auth.Credentials} is used then an {@link java.lang.IllegalArgumentException} will - * be thrown. If the provided credentials already have an api audience, then it will be - * overriden by this audience + * com.google.auth.Credentials} is used then an {@link IllegalArgumentException} will be thrown. + * If the provided credentials already have an api audience, then it will be overriden by this + * audience */ public B setGdchApiAudience(@Nullable String gdchApiAudience) { stubSettings.setGdchApiAudience(gdchApiAudience); @@ -315,8 +315,8 @@ public B setGdchApiAudience(@Nullable String gdchApiAudience) { } /** - * Sets the API key. The API key will get translated to an [ApiKeyCredentials] and stored in - * [CallContext]. + * Sets the API key. The API key will get translated to an {@link + * com.google.auth.ApiKeyCredentials} and stored in {@link ClientContext}. * *

Note: you can not set an API key and credentials object in the same Settings. It will fail * when building the settings. diff --git a/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientSettingsTest.java b/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientSettingsTest.java index 930e7a906b..050fb5cdb7 100644 --- a/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientSettingsTest.java +++ b/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientSettingsTest.java @@ -30,7 +30,10 @@ package com.google.api.gax.rpc; import static com.google.api.gax.util.TimeConversionTestUtils.testDurationMethod; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.fail; import static org.mockito.Mockito.when; import com.google.api.core.ApiClock; diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITApiCredentials.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITApiCredentials.java index b9f539cc8e..0fa6952b73 100644 --- a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITApiCredentials.java +++ b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITApiCredentials.java @@ -97,6 +97,7 @@ void testCreateGrpcClient_withApiKey_sendsApiHeaderToServer() throws IOException grpcClient.echo(EchoRequest.newBuilder().build()); + assertThat(settings.getApiKey()).isEqualTo(API_KEY); String headerValue = grpcInterceptor.metadata.get(apiKeyAuthHeader); assertThat(headerValue).isEqualTo(API_KEY); String defaultAuthorizationHeaderValue = From bce5abb4f45fd8b1f678e0e18aa4cf0e8a27f0df Mon Sep 17 00:00:00 2001 From: detmerl Date: Thu, 26 Sep 2024 14:40:16 -0400 Subject: [PATCH 20/38] added handling of deduping credential headers for GRPC calls + additional showcase tests for this scenario --- .../InstantiatingGrpcChannelProvider.java | 19 +- .../InstantiatingGrpcChannelProviderTest.java | 46 ++++ .../com/google/api/gax/rpc/ClientContext.java | 53 ++-- .../google/api/gax/rpc/ClientSettings.java | 11 +- .../com/google/api/gax/rpc/StubSettings.java | 7 +- .../api/gax/rpc/ClientSettingsTest.java | 29 -- .../showcase/v1beta1/it/ITApiCredentials.java | 149 ---------- .../v1beta1/it/ITApiKeyCredentials.java | 257 ++++++++++++++++++ 8 files changed, 355 insertions(+), 216 deletions(-) delete mode 100644 showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITApiCredentials.java create mode 100644 showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITApiKeyCredentials.java diff --git a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java index 5d601e8fa6..1adc79e40a 100644 --- a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java +++ b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java @@ -63,6 +63,7 @@ import java.nio.charset.StandardCharsets; import java.security.GeneralSecurityException; import java.security.KeyStore; +import java.util.HashMap; import java.util.Map; import java.util.concurrent.Executor; import java.util.concurrent.ScheduledExecutorService; @@ -407,8 +408,10 @@ ChannelCredentials createMtlsChannelCredentials() throws IOException, GeneralSec } private ManagedChannel createSingleChannel() throws IOException { + Map mergedHeaders = mergeHeadersWithCredentialHeaders(); GrpcHeaderInterceptor headerInterceptor = - new GrpcHeaderInterceptor(headerProvider.getHeaders()); + new GrpcHeaderInterceptor(ImmutableMap.copyOf(mergedHeaders)); + GrpcMetadataHandlerInterceptor metadataHandlerInterceptor = new GrpcMetadataHandlerInterceptor(); @@ -496,6 +499,20 @@ private ManagedChannel createSingleChannel() throws IOException { return managedChannel; } + // dedup any headers explicitly set with headers provided by credential, with preference to + // credential headers + private Map mergeHeadersWithCredentialHeaders() { + Map userHeaders = new HashMap<>(headerProvider.getHeaders()); + if (credentials != null) { + try { + userHeaders.keySet().removeAll(credentials.getRequestMetadata().keySet()); + } catch (Exception e) { + // no-op, if we can't retrieve credentials metadata we will leave headers intact + } + } + return userHeaders; + } + /** * Marked as Internal Api and intended for internal use. DirectPath must be enabled via the * settings and a few other configurations/settings must also be valid for the request to go diff --git a/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java b/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java index a4bcb65a1a..ead85f0629 100644 --- a/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java +++ b/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java @@ -34,10 +34,12 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.truth.Truth.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; import com.google.api.core.ApiFunction; import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider.Builder; +import com.google.api.gax.rpc.FixedHeaderProvider; import com.google.api.gax.rpc.HeaderProvider; import com.google.api.gax.rpc.TransportChannel; import com.google.api.gax.rpc.TransportChannelProvider; @@ -877,6 +879,50 @@ public void canUseDirectPath_nonGDUUniverseDomain() { Truth.assertThat(provider.canUseDirectPath()).isFalse(); } + @Test + public void createChannel_handlesMatchCredentialAndExplicitHeaders() throws IOException { + String apiHeaderKey = "fake_api_key_2"; + Map header = + new HashMap() { + { + put("x-goog-api-key", apiHeaderKey); + } + }; + FixedHeaderProvider headerProvider = FixedHeaderProvider.create(header); + + InstantiatingGrpcChannelProvider.Builder builder = + InstantiatingGrpcChannelProvider.newBuilder() + .setCredentials(computeEngineCredentials) + .setHeaderProvider(headerProvider) + .setEndpoint("test.random.com:443"); + InstantiatingGrpcChannelProvider provider = builder.build(); + + // calls createChannel + TransportChannel transportChannel = provider.getTransportChannel(); + + assertNotNull(transportChannel); + transportChannel.shutdownNow(); + } + + @Test + public void createChannel_handlesNullCredentials() throws IOException { + Map header = new HashMap(); + FixedHeaderProvider headerProvider = FixedHeaderProvider.create(header); + + InstantiatingGrpcChannelProvider.Builder builder = + InstantiatingGrpcChannelProvider.newBuilder() + .setHeaderProvider(headerProvider) + .setEndpoint("test.random.com:443"); + + InstantiatingGrpcChannelProvider provider = builder.build(); + + // calls createChannel + TransportChannel transportChannel = provider.getTransportChannel(); + + assertNotNull(transportChannel); + transportChannel.shutdownNow(); + } + private static class FakeLogHandler extends Handler { List records = new ArrayList<>(); diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java index c410a51415..71ff9ad4cf 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java @@ -176,29 +176,24 @@ public static ClientContext create(StubSettings settings) throws IOException { // A valid EndpointContext should have been created in the StubSettings EndpointContext endpointContext = settings.getEndpointContext(); String endpoint = endpointContext.resolvedEndpoint(); - String apiKey = settings.getApiKey(); - Credentials credentials; - if (apiKey != null) { - // if API key exists it becomes the default credential - credentials = ApiKeyCredentials.create(settings.getApiKey()); - } else { - credentials = settings.getCredentialsProvider().getCredentials(); - // check if need to adjust credentials/endpoint/endpointContext for GDC-H - String settingsGdchApiAudience = settings.getGdchApiAudience(); - boolean usingGDCH = credentials instanceof GdchCredentials; - if (usingGDCH) { - // Can only determine if the GDC-H is being used via the Credentials. The Credentials object - // is resolved in the ClientContext and must be passed to the EndpointContext. Rebuild the - // endpointContext only on GDC-H flows. - endpointContext = endpointContext.withGDCH(); - // Resolve the new endpoint with the GDC-H flow - endpoint = endpointContext.resolvedEndpoint(); - // We recompute the GdchCredentials with the audience - credentials = getGdchCredentials(settingsGdchApiAudience, endpoint, credentials); - } else if (!Strings.isNullOrEmpty(settingsGdchApiAudience)) { - throw new IllegalArgumentException( - "GDC-H API audience can only be set when using GdchCredentials"); - } + Credentials credentials = getCredentials(settings); + // check if need to adjust credentials/endpoint/endpointContext for GDC-H + String settingsGdchApiAudience = settings.getGdchApiAudience(); + boolean usingGDCH = credentials instanceof GdchCredentials; + if (usingGDCH) { + // Can only determine if the GDC-H is being used via the Credentials. The Credentials object + // is resolved in the ClientContext and must be passed to the EndpointContext. Rebuild the + // endpointContext only on GDC-H flows. + endpointContext = endpointContext.withGDCH(); + // Resolve the new endpoint with the GDC-H flow + endpoint = endpointContext.resolvedEndpoint(); + // We recompute the GdchCredentials with the audience + credentials = + getGdchCredentials( + settingsGdchApiAudience, endpointContext.resolvedEndpoint(), credentials); + } else if (!Strings.isNullOrEmpty(settingsGdchApiAudience)) { + throw new IllegalArgumentException( + "GDC-H API audience can only be set when using GdchCredentials"); } if (settings.getQuotaProjectId() != null && credentials != null) { @@ -284,6 +279,18 @@ public static ClientContext create(StubSettings settings) throws IOException { .build(); } + /** Determines which credentials to use. Order */ + private static Credentials getCredentials(StubSettings settings) throws IOException { + Credentials credentials; + if (settings.getApiKey() != null) { + // if API key exists it becomes the default credential + credentials = ApiKeyCredentials.create(settings.getApiKey()); + } else { + credentials = settings.getCredentialsProvider().getCredentials(); + } + return credentials; + } + /** * Constructs a new {@link com.google.auth.Credentials} object based on credentials provided with * a GDC-H audience diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java index 7cf0ef737d..2ed659cdf1 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java @@ -58,10 +58,6 @@ public abstract class ClientSettings /** Constructs an instance of ClientSettings. */ protected ClientSettings(Builder builder) throws IOException { - if (builder.stubSettings.getApiKey() != null && builder.explicitCredentialsProvided) { - throw new IllegalArgumentException( - "You can not provide both ApiKey and Credentials for a client."); - } this.stubSettings = builder.stubSettings.build(); } @@ -163,8 +159,6 @@ public abstract static class Builder< private StubSettings.Builder stubSettings; - private boolean explicitCredentialsProvided = false; - /** Create a builder from a ClientSettings object. */ protected Builder(ClientSettings settings) { this.stubSettings = settings.stubSettings.toBuilder(); @@ -219,7 +213,6 @@ public B setBackgroundExecutorProvider(ExecutorProvider executorProvider) { /** Sets the CredentialsProvider to use for getting the credentials to make calls with. */ public B setCredentialsProvider(CredentialsProvider credentialsProvider) { - explicitCredentialsProvided = true; stubSettings.setCredentialsProvider(credentialsProvider); return self(); } @@ -318,8 +311,8 @@ public B setGdchApiAudience(@Nullable String gdchApiAudience) { * Sets the API key. The API key will get translated to an {@link * com.google.auth.ApiKeyCredentials} and stored in {@link ClientContext}. * - *

Note: you can not set an API key and credentials object in the same Settings. It will fail - * when building the settings. + *

Note: If you set an API key and {@link CredentialsProvider} in the same Settings. The api + * key will override any credentials provided. */ public B setApiKey(String apiKey) { stubSettings.setApiKey(apiKey); diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java index 800dbb9c1a..49a521aaa9 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java @@ -236,12 +236,9 @@ public final String getGdchApiAudience() { return gdchApiAudience; } - /** - * Gets the ApiKey that should be used for authentication. If an empty string was provided it will - * return null - */ + /** Gets the ApiKey that should be used for authentication. */ public final String getApiKey() { - return (apiKey == null || apiKey.isEmpty()) ? null : apiKey; + return apiKey; } @Override diff --git a/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientSettingsTest.java b/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientSettingsTest.java index 050fb5cdb7..a5e96f9196 100644 --- a/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientSettingsTest.java +++ b/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientSettingsTest.java @@ -30,11 +30,7 @@ package com.google.api.gax.rpc; import static com.google.api.gax.util.TimeConversionTestUtils.testDurationMethod; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.fail; -import static org.mockito.Mockito.when; import com.google.api.core.ApiClock; import com.google.api.core.NanoClock; @@ -577,29 +573,4 @@ public void testWatchdogCheckInterval_backportMethodsBehaveCorrectly() { cs -> cs.getWatchdogCheckIntervalDuration(), cs -> cs.getWatchdogCheckInterval()); } - - @Test - void testClientSettingsBuilder_throwsErrorIfApiKeyAndCredentialsAreProvided() throws Exception { - FakeClientSettings.Builder builder = new FakeClientSettings.Builder(); - CredentialsProvider credentialsProvider = Mockito.mock(CredentialsProvider.class); - when(credentialsProvider.getCredentials()).thenReturn(Mockito.mock(Credentials.class)); - builder.setCredentialsProvider(credentialsProvider); - builder.setApiKey("api_key"); - - assertThrows(IllegalArgumentException.class, builder::build); - } - - @Test - void testEmptyApiKeyClientSettingsBuild_isTreatedAsNull() throws Exception { - FakeClientSettings.Builder builder = new FakeClientSettings.Builder(); - CredentialsProvider credentialsProvider = Mockito.mock(CredentialsProvider.class); - Credentials credentials = Mockito.mock(Credentials.class); - when(credentialsProvider.getCredentials()).thenReturn(credentials); - builder.setCredentialsProvider(credentialsProvider); - builder.setApiKey(""); - - FakeClientSettings fakeClientSettings = builder.build(); - assertEquals(fakeClientSettings.getCredentialsProvider().getCredentials(), credentials); - assertNull(fakeClientSettings.getApiKey()); - } } diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITApiCredentials.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITApiCredentials.java deleted file mode 100644 index 0fa6952b73..0000000000 --- a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITApiCredentials.java +++ /dev/null @@ -1,149 +0,0 @@ -/* - * Copyright 2024 Google LLC - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.google.showcase.v1beta1.it; - -import static com.google.common.truth.Truth.assertThat; -import static org.junit.jupiter.api.Assertions.assertThrows; - -import com.google.api.client.http.javanet.NetHttpTransport; -import com.google.api.gax.core.FixedCredentialsProvider; -import com.google.auth.ApiKeyCredentials; -import com.google.auth.http.AuthHttpConstants; -import com.google.common.collect.ImmutableList; -import com.google.showcase.v1beta1.EchoClient; -import com.google.showcase.v1beta1.EchoRequest; -import com.google.showcase.v1beta1.EchoSettings; -import com.google.showcase.v1beta1.it.util.GrpcCapturingClientInterceptor; -import com.google.showcase.v1beta1.it.util.HttpJsonCapturingClientInterceptor; -import com.google.showcase.v1beta1.it.util.TestClientInitializer; -import io.grpc.ManagedChannelBuilder; -import io.grpc.Metadata; -import java.io.IOException; -import java.util.ArrayList; -import java.util.concurrent.TimeUnit; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; - -/** - * Test suite to confirm a client can be instantiated with API key credentials and correct - * authorization sent via headers to back end - */ -class ITApiCredentials { - - private static final String API_KEY = "fake_api_key"; - private static final String API_KEY_AUTH_HEADER = "x-goog-api-key"; - private static final String HTTP_RESPONSE_HEADER_STRING = - "x-showcase-request-" + API_KEY_AUTH_HEADER; - private static final String GRPC_ENDPOINT = "localhost:7469"; - private static final String HTTP_ENDPOINT = "http://localhost:7469"; - - private static HttpJsonCapturingClientInterceptor httpJsonInterceptor; - private static EchoClient grpcClient; - private static EchoClient httpJsonClient; - private static GrpcCapturingClientInterceptor grpcInterceptor; - - @BeforeEach - void setup() throws IOException { - grpcInterceptor = new GrpcCapturingClientInterceptor(); - httpJsonInterceptor = new HttpJsonCapturingClientInterceptor(); - } - - @AfterEach - void tearDown() throws InterruptedException { - if (httpJsonClient != null) { - httpJsonClient.close(); - httpJsonClient.awaitTermination( - TestClientInitializer.AWAIT_TERMINATION_SECONDS, TimeUnit.SECONDS); - } - if (grpcClient != null) { - grpcClient.close(); - grpcClient.awaitTermination( - TestClientInitializer.AWAIT_TERMINATION_SECONDS, TimeUnit.SECONDS); - } - } - - @Test - void testCreateGrpcClient_withApiKey_sendsApiHeaderToServer() throws IOException { - Metadata.Key apiKeyAuthHeader = - Metadata.Key.of(API_KEY_AUTH_HEADER, Metadata.ASCII_STRING_MARSHALLER); - Metadata.Key defaultAuthorizationHeader = - Metadata.Key.of(AuthHttpConstants.AUTHORIZATION, Metadata.ASCII_STRING_MARSHALLER); - EchoSettings settings = - EchoSettings.newBuilder() - .setApiKey(API_KEY) - .setEndpoint(GRPC_ENDPOINT) - .setTransportChannelProvider( - EchoSettings.defaultGrpcTransportProviderBuilder() - .setChannelConfigurator(ManagedChannelBuilder::usePlaintext) - .setInterceptorProvider(() -> ImmutableList.of(grpcInterceptor)) - .build()) - .build(); - grpcClient = EchoClient.create(settings); - - grpcClient.echo(EchoRequest.newBuilder().build()); - - assertThat(settings.getApiKey()).isEqualTo(API_KEY); - String headerValue = grpcInterceptor.metadata.get(apiKeyAuthHeader); - assertThat(headerValue).isEqualTo(API_KEY); - String defaultAuthorizationHeaderValue = - grpcInterceptor.metadata.get(defaultAuthorizationHeader); - assertThat(defaultAuthorizationHeaderValue).isNull(); - } - - @Test - void testCreateGrpcClient_withApiKeyAndExplicitCredentials_throwsException() throws IOException { - assertThrows( - IllegalArgumentException.class, - () -> - EchoSettings.newBuilder() - .setApiKey(API_KEY) - .setEndpoint(GRPC_ENDPOINT) - .setCredentialsProvider( - FixedCredentialsProvider.create(ApiKeyCredentials.create("invalid"))) - .setTransportChannelProvider( - EchoSettings.defaultGrpcTransportProviderBuilder() - .setChannelConfigurator(ManagedChannelBuilder::usePlaintext) - .setInterceptorProvider(() -> ImmutableList.of(grpcInterceptor)) - .build()) - .build()); - } - - @Test - void testCreateHttpClient_withApiKey_sendsApiHeaderToServer() throws Exception { - EchoSettings httpJsonEchoSettings = - EchoSettings.newHttpJsonBuilder() - .setApiKey(API_KEY) - .setTransportChannelProvider( - EchoSettings.defaultHttpJsonTransportProviderBuilder() - .setHttpTransport( - new NetHttpTransport.Builder().doNotValidateCertificate().build()) - .setEndpoint(HTTP_ENDPOINT) - .setInterceptorProvider(() -> ImmutableList.of(httpJsonInterceptor)) - .build()) - .build(); - httpJsonClient = EchoClient.create(httpJsonEchoSettings); - - httpJsonClient.echo(EchoRequest.newBuilder().build()); - - ArrayList headerValues = - (ArrayList) - httpJsonInterceptor.metadata.getHeaders().get(HTTP_RESPONSE_HEADER_STRING); - String headerValue = headerValues.get(0); - assertThat(headerValue).isEqualTo(API_KEY); - } -} diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITApiKeyCredentials.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITApiKeyCredentials.java new file mode 100644 index 0000000000..467c9cab30 --- /dev/null +++ b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITApiKeyCredentials.java @@ -0,0 +1,257 @@ +/* + * Copyright 2024 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.showcase.v1beta1.it; + +import static com.google.common.truth.Truth.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import com.google.api.client.http.javanet.NetHttpTransport; +import com.google.api.gax.core.FixedCredentialsProvider; +import com.google.auth.ApiKeyCredentials; +import com.google.auth.http.AuthHttpConstants; +import com.google.common.collect.ImmutableList; +import com.google.showcase.v1beta1.EchoClient; +import com.google.showcase.v1beta1.EchoRequest; +import com.google.showcase.v1beta1.EchoSettings; +import com.google.showcase.v1beta1.it.util.GrpcCapturingClientInterceptor; +import com.google.showcase.v1beta1.it.util.HttpJsonCapturingClientInterceptor; +import com.google.showcase.v1beta1.it.util.TestClientInitializer; +import io.grpc.ManagedChannelBuilder; +import io.grpc.Metadata; +import java.io.IOException; +import java.util.ArrayList; +import java.util.concurrent.TimeUnit; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import java.util.HashMap; +import java.util.Map; +import com.google.api.gax.rpc.TransportChannelProvider; +import com.google.api.gax.rpc.FixedHeaderProvider; +import java.util.List; +import java.util.stream.Collectors; +import java.util.stream.StreamSupport; + +/** + * Test suite to confirm a client can be instantiated with API key credentials and correct + * authorization sent via headers to back end + */ +class ITApiKeyCredentials { + + private static final String API_KEY = "fake_api_key"; + private static final String API_KEY_AUTH_HEADER = "x-goog-api-key"; + private static final String HTTP_RESPONSE_HEADER_STRING = + "x-showcase-request-" + API_KEY_AUTH_HEADER; + private static final String GRPC_ENDPOINT = "localhost:7469"; + private static final String HTTP_ENDPOINT = "http://localhost:7469"; + + private static HttpJsonCapturingClientInterceptor httpJsonInterceptor; + private static EchoClient grpcClient; + private static EchoClient httpJsonClient; + private static GrpcCapturingClientInterceptor grpcInterceptor; + + @BeforeEach + void setup() throws IOException { + grpcInterceptor = new GrpcCapturingClientInterceptor(); + httpJsonInterceptor = new HttpJsonCapturingClientInterceptor(); + } + + @AfterEach + void tearDown() throws InterruptedException { + if (httpJsonClient != null) { + httpJsonClient.close(); + httpJsonClient.awaitTermination( + TestClientInitializer.AWAIT_TERMINATION_SECONDS, TimeUnit.SECONDS); + } + if (grpcClient != null) { + grpcClient.close(); + grpcClient.awaitTermination( + TestClientInitializer.AWAIT_TERMINATION_SECONDS, TimeUnit.SECONDS); + } + } + + @Test + void testCreateGrpcClient_withApiKey_sendsApiHeaderToServer() throws IOException { + Metadata.Key apiKeyAuthHeader = + Metadata.Key.of(API_KEY_AUTH_HEADER, Metadata.ASCII_STRING_MARSHALLER); + Metadata.Key defaultAuthorizationHeader = + Metadata.Key.of(AuthHttpConstants.AUTHORIZATION, Metadata.ASCII_STRING_MARSHALLER); + EchoSettings settings = + EchoSettings.newBuilder() + .setApiKey(API_KEY) + .setEndpoint(GRPC_ENDPOINT) + .setTransportChannelProvider( + EchoSettings.defaultGrpcTransportProviderBuilder() + .setChannelConfigurator(ManagedChannelBuilder::usePlaintext) + .setInterceptorProvider(() -> ImmutableList.of(grpcInterceptor)) + .build()) + .build(); + grpcClient = EchoClient.create(settings); + + grpcClient.echo(EchoRequest.newBuilder().build()); + + assertThat(settings.getApiKey()).isEqualTo(API_KEY); + String headerValue = grpcInterceptor.metadata.get(apiKeyAuthHeader); + assertThat(headerValue).isEqualTo(API_KEY); + String defaultAuthorizationHeaderValue = + grpcInterceptor.metadata.get(defaultAuthorizationHeader); + assertThat(defaultAuthorizationHeaderValue).isNull(); + } + + @Test + void testCreateGrpcClient_withApiKeySetViaSetterAndHeader_dedupsHeader() throws IOException { + String apiHeaderKey = "fake_api_key_2"; + Metadata.Key apiKeyAuthHeader = + Metadata.Key.of(API_KEY_AUTH_HEADER, Metadata.ASCII_STRING_MARSHALLER); + Metadata.Key defaultAuthorizationHeader = + Metadata.Key.of(AuthHttpConstants.AUTHORIZATION, Metadata.ASCII_STRING_MARSHALLER); + Map header = new HashMap() { {put(API_KEY_AUTH_HEADER, apiHeaderKey);}}; + FixedHeaderProvider headerProvider = FixedHeaderProvider.create(header); + + TransportChannelProvider transportChannelProvider = EchoSettings.defaultGrpcTransportProviderBuilder().setHeaderProvider(headerProvider) + .setChannelConfigurator(ManagedChannelBuilder::usePlaintext) + .setInterceptorProvider(() -> ImmutableList.of(grpcInterceptor)) + .build(); + EchoSettings settings = + EchoSettings.newBuilder() + .setApiKey(API_KEY) + .setEndpoint(GRPC_ENDPOINT) + .setTransportChannelProvider(transportChannelProvider) + .build(); + grpcClient = EchoClient.create(settings); + + grpcClient.echo(EchoRequest.newBuilder().build()); + + Iterable matchingHeaders = grpcInterceptor.metadata.getAll(apiKeyAuthHeader); + List matchingHeadersList = StreamSupport.stream(matchingHeaders.spliterator(), false) + .collect(Collectors.toList()); + + String headerValue = matchingHeadersList.get(0); + assertThat(headerValue).isEqualTo(API_KEY); + assertThat(matchingHeadersList.size()).isEqualTo(1); + } + + @Test + void testCreateGrpcClient_withApiKeyAndExplicitCredentials_overridesCredentials() throws IOException { + Metadata.Key apiKeyAuthHeader = + Metadata.Key.of(API_KEY_AUTH_HEADER, Metadata.ASCII_STRING_MARSHALLER); + EchoSettings settings = + EchoSettings.newBuilder() + .setApiKey(API_KEY) + .setEndpoint(GRPC_ENDPOINT) + .setCredentialsProvider( + FixedCredentialsProvider.create(ApiKeyCredentials.create("invalid"))) + .setTransportChannelProvider( + EchoSettings.defaultGrpcTransportProviderBuilder() + .setChannelConfigurator(ManagedChannelBuilder::usePlaintext) + .setInterceptorProvider(() -> ImmutableList.of(grpcInterceptor)) + .build()) + .build(); + grpcClient = EchoClient.create(settings); + + grpcClient.echo(EchoRequest.newBuilder().build()); + + String headerValue = grpcInterceptor.metadata.get(apiKeyAuthHeader); + assertThat(headerValue).isEqualTo(API_KEY); + } + + @Test + void testCreateHttpClient_withApiKey_sendsApiHeaderToServer() throws Exception { + EchoSettings httpJsonEchoSettings = + EchoSettings.newHttpJsonBuilder() + .setApiKey(API_KEY) + .setTransportChannelProvider( + EchoSettings.defaultHttpJsonTransportProviderBuilder() + .setHttpTransport( + new NetHttpTransport.Builder().doNotValidateCertificate().build()) + .setEndpoint(HTTP_ENDPOINT) + .setInterceptorProvider(() -> ImmutableList.of(httpJsonInterceptor)) + .build()) + .build(); + httpJsonClient = EchoClient.create(httpJsonEchoSettings); + + httpJsonClient.echo(EchoRequest.newBuilder().build()); + + ArrayList headerValues = + (ArrayList) + httpJsonInterceptor.metadata.getHeaders().get(HTTP_RESPONSE_HEADER_STRING); + String headerValue = headerValues.get(0); + assertThat(headerValue).isEqualTo(API_KEY); + } + + @Test + void testCreateHttpClient_withApiKeySetViaSetterAndHeader_dedupsHeader() throws Exception { + String apiHeaderKey = "fake_api_key_2"; + Metadata.Key apiKeyAuthHeader = + Metadata.Key.of(API_KEY_AUTH_HEADER, Metadata.ASCII_STRING_MARSHALLER); + Metadata.Key defaultAuthorizationHeader = + Metadata.Key.of(AuthHttpConstants.AUTHORIZATION, Metadata.ASCII_STRING_MARSHALLER); + Map header = new HashMap() { {put(API_KEY_AUTH_HEADER, apiHeaderKey);}}; + FixedHeaderProvider headerProvider = FixedHeaderProvider.create(header); + + TransportChannelProvider transportChannelProvider = EchoSettings.defaultHttpJsonTransportProviderBuilder().setHeaderProvider(headerProvider) + .setInterceptorProvider(() -> ImmutableList.of(httpJsonInterceptor)) + .build(); + EchoSettings httpJsonEchoSettings = + EchoSettings.newHttpJsonBuilder() + .setApiKey(API_KEY) + .setTransportChannelProvider( + EchoSettings.defaultHttpJsonTransportProviderBuilder() + .setHttpTransport( + new NetHttpTransport.Builder().doNotValidateCertificate().build()) + .setEndpoint(HTTP_ENDPOINT) + .setInterceptorProvider(() -> ImmutableList.of(httpJsonInterceptor)) + .build()) + .build(); + httpJsonClient = EchoClient.create(httpJsonEchoSettings); + + httpJsonClient.echo(EchoRequest.newBuilder().build()); + + ArrayList headerValues = + (ArrayList) + httpJsonInterceptor.metadata.getHeaders().get(HTTP_RESPONSE_HEADER_STRING); + String headerValue = headerValues.get(0); + assertThat(headerValue).isEqualTo(API_KEY); + assertThat(headerValues.size()).isEqualTo(1); + } + + @Test + void testCreateHttpClient_withApiKeyAndExplicitCredentials_overridesCredentials() throws Exception { + EchoSettings httpJsonEchoSettings = + EchoSettings.newHttpJsonBuilder() + .setApiKey(API_KEY) + .setCredentialsProvider( + FixedCredentialsProvider.create(ApiKeyCredentials.create("invalid"))) + .setTransportChannelProvider( + EchoSettings.defaultHttpJsonTransportProviderBuilder() + .setHttpTransport( + new NetHttpTransport.Builder().doNotValidateCertificate().build()) + .setEndpoint(HTTP_ENDPOINT) + .setInterceptorProvider(() -> ImmutableList.of(httpJsonInterceptor)) + .build()) + .build(); + httpJsonClient = EchoClient.create(httpJsonEchoSettings); + + httpJsonClient.echo(EchoRequest.newBuilder().build()); + + ArrayList headerValues = + (ArrayList) + httpJsonInterceptor.metadata.getHeaders().get(HTTP_RESPONSE_HEADER_STRING); + String headerValue = headerValues.get(0); + assertThat(headerValue).isEqualTo(API_KEY); + } +} From d4670c5a28347dbfaaabd15da0a89f66bee8b97e Mon Sep 17 00:00:00 2001 From: detmerl Date: Thu, 26 Sep 2024 14:54:54 -0400 Subject: [PATCH 21/38] lint fixes --- .../InstantiatingGrpcChannelProviderTest.java | 11 +- .../com/google/api/gax/rpc/ClientContext.java | 2 +- .../google/api/gax/rpc/ClientSettings.java | 2 +- .../api/gax/rpc/ClientSettingsTest.java | 1 - .../v1beta1/it/ITApiKeyCredentials.java | 135 ++++++++++-------- 5 files changed, 79 insertions(+), 72 deletions(-) diff --git a/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java b/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java index ead85f0629..4d71db080e 100644 --- a/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java +++ b/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java @@ -882,14 +882,9 @@ public void canUseDirectPath_nonGDUUniverseDomain() { @Test public void createChannel_handlesMatchCredentialAndExplicitHeaders() throws IOException { String apiHeaderKey = "fake_api_key_2"; - Map header = - new HashMap() { - { - put("x-goog-api-key", apiHeaderKey); - } - }; + Map header = new HashMap<>(); + header.put("x-goog-api-key", apiHeaderKey); FixedHeaderProvider headerProvider = FixedHeaderProvider.create(header); - InstantiatingGrpcChannelProvider.Builder builder = InstantiatingGrpcChannelProvider.newBuilder() .setCredentials(computeEngineCredentials) @@ -908,12 +903,10 @@ public void createChannel_handlesMatchCredentialAndExplicitHeaders() throws IOEx public void createChannel_handlesNullCredentials() throws IOException { Map header = new HashMap(); FixedHeaderProvider headerProvider = FixedHeaderProvider.create(header); - InstantiatingGrpcChannelProvider.Builder builder = InstantiatingGrpcChannelProvider.newBuilder() .setHeaderProvider(headerProvider) .setEndpoint("test.random.com:443"); - InstantiatingGrpcChannelProvider provider = builder.build(); // calls createChannel diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java index 71ff9ad4cf..924f58462d 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java @@ -279,7 +279,7 @@ public static ClientContext create(StubSettings settings) throws IOException { .build(); } - /** Determines which credentials to use. Order */ + /** Determines which credentials to use. API key overrides credentials provided by provider. */ private static Credentials getCredentials(StubSettings settings) throws IOException { Credentials credentials; if (settings.getApiKey() != null) { diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java index 2ed659cdf1..511ecc91c5 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java @@ -311,7 +311,7 @@ public B setGdchApiAudience(@Nullable String gdchApiAudience) { * Sets the API key. The API key will get translated to an {@link * com.google.auth.ApiKeyCredentials} and stored in {@link ClientContext}. * - *

Note: If you set an API key and {@link CredentialsProvider} in the same Settings. The api + *

Note: If you set an API key and {@link CredentialsProvider} in the same ClientSettings the API * key will override any credentials provided. */ public B setApiKey(String apiKey) { diff --git a/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientSettingsTest.java b/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientSettingsTest.java index a5e96f9196..e48df7af54 100644 --- a/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientSettingsTest.java +++ b/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientSettingsTest.java @@ -268,7 +268,6 @@ void testBuilderFromSettings() throws Exception { WatchdogProvider watchdogProvider = Mockito.mock(WatchdogProvider.class); java.time.Duration watchdogCheckInterval = java.time.Duration.ofSeconds(14); String quotaProjectId = "test_builder_from_settings_quotaProjectId"; - String apiKey = "api_key"; builder.setExecutorProvider(executorProvider); builder.setTransportChannelProvider(transportProvider); diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITApiKeyCredentials.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITApiKeyCredentials.java index 467c9cab30..7c98f59403 100644 --- a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITApiKeyCredentials.java +++ b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITApiKeyCredentials.java @@ -17,10 +17,11 @@ package com.google.showcase.v1beta1.it; import static com.google.common.truth.Truth.assertThat; -import static org.junit.jupiter.api.Assertions.assertThrows; import com.google.api.client.http.javanet.NetHttpTransport; import com.google.api.gax.core.FixedCredentialsProvider; +import com.google.api.gax.rpc.FixedHeaderProvider; +import com.google.api.gax.rpc.TransportChannelProvider; import com.google.auth.ApiKeyCredentials; import com.google.auth.http.AuthHttpConstants; import com.google.common.collect.ImmutableList; @@ -34,17 +35,15 @@ import io.grpc.Metadata; import java.io.IOException; import java.util.ArrayList; -import java.util.concurrent.TimeUnit; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; import java.util.HashMap; -import java.util.Map; -import com.google.api.gax.rpc.TransportChannelProvider; -import com.google.api.gax.rpc.FixedHeaderProvider; import java.util.List; +import java.util.Map; +import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; import java.util.stream.StreamSupport; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; /** * Test suite to confirm a client can be instantiated with API key credentials and correct @@ -116,29 +115,36 @@ void testCreateGrpcClient_withApiKey_sendsApiHeaderToServer() throws IOException void testCreateGrpcClient_withApiKeySetViaSetterAndHeader_dedupsHeader() throws IOException { String apiHeaderKey = "fake_api_key_2"; Metadata.Key apiKeyAuthHeader = - Metadata.Key.of(API_KEY_AUTH_HEADER, Metadata.ASCII_STRING_MARSHALLER); + Metadata.Key.of(API_KEY_AUTH_HEADER, Metadata.ASCII_STRING_MARSHALLER); Metadata.Key defaultAuthorizationHeader = - Metadata.Key.of(AuthHttpConstants.AUTHORIZATION, Metadata.ASCII_STRING_MARSHALLER); - Map header = new HashMap() { {put(API_KEY_AUTH_HEADER, apiHeaderKey);}}; + Metadata.Key.of(AuthHttpConstants.AUTHORIZATION, Metadata.ASCII_STRING_MARSHALLER); + Map header = + new HashMap() { + { + put(API_KEY_AUTH_HEADER, apiHeaderKey); + } + }; FixedHeaderProvider headerProvider = FixedHeaderProvider.create(header); - TransportChannelProvider transportChannelProvider = EchoSettings.defaultGrpcTransportProviderBuilder().setHeaderProvider(headerProvider) + TransportChannelProvider transportChannelProvider = + EchoSettings.defaultGrpcTransportProviderBuilder() + .setHeaderProvider(headerProvider) .setChannelConfigurator(ManagedChannelBuilder::usePlaintext) .setInterceptorProvider(() -> ImmutableList.of(grpcInterceptor)) - .build(); + .build(); EchoSettings settings = - EchoSettings.newBuilder() - .setApiKey(API_KEY) - .setEndpoint(GRPC_ENDPOINT) - .setTransportChannelProvider(transportChannelProvider) - .build(); + EchoSettings.newBuilder() + .setApiKey(API_KEY) + .setEndpoint(GRPC_ENDPOINT) + .setTransportChannelProvider(transportChannelProvider) + .build(); grpcClient = EchoClient.create(settings); grpcClient.echo(EchoRequest.newBuilder().build()); - Iterable matchingHeaders = grpcInterceptor.metadata.getAll(apiKeyAuthHeader); - List matchingHeadersList = StreamSupport.stream(matchingHeaders.spliterator(), false) - .collect(Collectors.toList()); + Iterable matchingHeaders = grpcInterceptor.metadata.getAll(apiKeyAuthHeader); + List matchingHeadersList = + StreamSupport.stream(matchingHeaders.spliterator(), false).collect(Collectors.toList()); String headerValue = matchingHeadersList.get(0); assertThat(headerValue).isEqualTo(API_KEY); @@ -146,20 +152,21 @@ void testCreateGrpcClient_withApiKeySetViaSetterAndHeader_dedupsHeader() throws } @Test - void testCreateGrpcClient_withApiKeyAndExplicitCredentials_overridesCredentials() throws IOException { + void testCreateGrpcClient_withApiKeyAndExplicitCredentials_overridesCredentials() + throws IOException { Metadata.Key apiKeyAuthHeader = - Metadata.Key.of(API_KEY_AUTH_HEADER, Metadata.ASCII_STRING_MARSHALLER); + Metadata.Key.of(API_KEY_AUTH_HEADER, Metadata.ASCII_STRING_MARSHALLER); EchoSettings settings = - EchoSettings.newBuilder() + EchoSettings.newBuilder() .setApiKey(API_KEY) .setEndpoint(GRPC_ENDPOINT) .setCredentialsProvider( - FixedCredentialsProvider.create(ApiKeyCredentials.create("invalid"))) + FixedCredentialsProvider.create(ApiKeyCredentials.create("invalid"))) .setTransportChannelProvider( - EchoSettings.defaultGrpcTransportProviderBuilder() - .setChannelConfigurator(ManagedChannelBuilder::usePlaintext) - .setInterceptorProvider(() -> ImmutableList.of(grpcInterceptor)) - .build()) + EchoSettings.defaultGrpcTransportProviderBuilder() + .setChannelConfigurator(ManagedChannelBuilder::usePlaintext) + .setInterceptorProvider(() -> ImmutableList.of(grpcInterceptor)) + .build()) .build(); grpcClient = EchoClient.create(settings); @@ -197,60 +204,68 @@ void testCreateHttpClient_withApiKey_sendsApiHeaderToServer() throws Exception { void testCreateHttpClient_withApiKeySetViaSetterAndHeader_dedupsHeader() throws Exception { String apiHeaderKey = "fake_api_key_2"; Metadata.Key apiKeyAuthHeader = - Metadata.Key.of(API_KEY_AUTH_HEADER, Metadata.ASCII_STRING_MARSHALLER); + Metadata.Key.of(API_KEY_AUTH_HEADER, Metadata.ASCII_STRING_MARSHALLER); Metadata.Key defaultAuthorizationHeader = - Metadata.Key.of(AuthHttpConstants.AUTHORIZATION, Metadata.ASCII_STRING_MARSHALLER); - Map header = new HashMap() { {put(API_KEY_AUTH_HEADER, apiHeaderKey);}}; + Metadata.Key.of(AuthHttpConstants.AUTHORIZATION, Metadata.ASCII_STRING_MARSHALLER); + Map header = + new HashMap() { + { + put(API_KEY_AUTH_HEADER, apiHeaderKey); + } + }; FixedHeaderProvider headerProvider = FixedHeaderProvider.create(header); - TransportChannelProvider transportChannelProvider = EchoSettings.defaultHttpJsonTransportProviderBuilder().setHeaderProvider(headerProvider) + TransportChannelProvider transportChannelProvider = + EchoSettings.defaultHttpJsonTransportProviderBuilder() + .setHeaderProvider(headerProvider) .setInterceptorProvider(() -> ImmutableList.of(httpJsonInterceptor)) .build(); EchoSettings httpJsonEchoSettings = - EchoSettings.newHttpJsonBuilder() - .setApiKey(API_KEY) - .setTransportChannelProvider( - EchoSettings.defaultHttpJsonTransportProviderBuilder() - .setHttpTransport( - new NetHttpTransport.Builder().doNotValidateCertificate().build()) - .setEndpoint(HTTP_ENDPOINT) - .setInterceptorProvider(() -> ImmutableList.of(httpJsonInterceptor)) - .build()) - .build(); + EchoSettings.newHttpJsonBuilder() + .setApiKey(API_KEY) + .setTransportChannelProvider( + EchoSettings.defaultHttpJsonTransportProviderBuilder() + .setHttpTransport( + new NetHttpTransport.Builder().doNotValidateCertificate().build()) + .setEndpoint(HTTP_ENDPOINT) + .setInterceptorProvider(() -> ImmutableList.of(httpJsonInterceptor)) + .build()) + .build(); httpJsonClient = EchoClient.create(httpJsonEchoSettings); httpJsonClient.echo(EchoRequest.newBuilder().build()); ArrayList headerValues = - (ArrayList) - httpJsonInterceptor.metadata.getHeaders().get(HTTP_RESPONSE_HEADER_STRING); + (ArrayList) + httpJsonInterceptor.metadata.getHeaders().get(HTTP_RESPONSE_HEADER_STRING); String headerValue = headerValues.get(0); assertThat(headerValue).isEqualTo(API_KEY); assertThat(headerValues.size()).isEqualTo(1); } @Test - void testCreateHttpClient_withApiKeyAndExplicitCredentials_overridesCredentials() throws Exception { + void testCreateHttpClient_withApiKeyAndExplicitCredentials_overridesCredentials() + throws Exception { EchoSettings httpJsonEchoSettings = - EchoSettings.newHttpJsonBuilder() - .setApiKey(API_KEY) - .setCredentialsProvider( - FixedCredentialsProvider.create(ApiKeyCredentials.create("invalid"))) - .setTransportChannelProvider( - EchoSettings.defaultHttpJsonTransportProviderBuilder() - .setHttpTransport( - new NetHttpTransport.Builder().doNotValidateCertificate().build()) - .setEndpoint(HTTP_ENDPOINT) - .setInterceptorProvider(() -> ImmutableList.of(httpJsonInterceptor)) - .build()) - .build(); + EchoSettings.newHttpJsonBuilder() + .setApiKey(API_KEY) + .setCredentialsProvider( + FixedCredentialsProvider.create(ApiKeyCredentials.create("invalid"))) + .setTransportChannelProvider( + EchoSettings.defaultHttpJsonTransportProviderBuilder() + .setHttpTransport( + new NetHttpTransport.Builder().doNotValidateCertificate().build()) + .setEndpoint(HTTP_ENDPOINT) + .setInterceptorProvider(() -> ImmutableList.of(httpJsonInterceptor)) + .build()) + .build(); httpJsonClient = EchoClient.create(httpJsonEchoSettings); httpJsonClient.echo(EchoRequest.newBuilder().build()); ArrayList headerValues = - (ArrayList) - httpJsonInterceptor.metadata.getHeaders().get(HTTP_RESPONSE_HEADER_STRING); + (ArrayList) + httpJsonInterceptor.metadata.getHeaders().get(HTTP_RESPONSE_HEADER_STRING); String headerValue = headerValues.get(0); assertThat(headerValue).isEqualTo(API_KEY); } From 1eda03f4e0fc18fd2817c37c1bb74fa6c45605b4 Mon Sep 17 00:00:00 2001 From: detmerl Date: Thu, 26 Sep 2024 15:49:25 -0400 Subject: [PATCH 22/38] lint fixes + additional showcase coverage --- .../main/java/com/google/api/gax/rpc/ClientSettings.java | 4 ++-- .../google/showcase/v1beta1/it/ITApiKeyCredentials.java | 7 ++++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java index 511ecc91c5..65f2e92d40 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java @@ -311,8 +311,8 @@ public B setGdchApiAudience(@Nullable String gdchApiAudience) { * Sets the API key. The API key will get translated to an {@link * com.google.auth.ApiKeyCredentials} and stored in {@link ClientContext}. * - *

Note: If you set an API key and {@link CredentialsProvider} in the same ClientSettings the API - * key will override any credentials provided. + *

Note: If you set an API key and {@link CredentialsProvider} in the same ClientSettings the + * API key will override any credentials provided. */ public B setApiKey(String apiKey) { stubSettings.setApiKey(apiKey); diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITApiKeyCredentials.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITApiKeyCredentials.java index 7c98f59403..6228882963 100644 --- a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITApiKeyCredentials.java +++ b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITApiKeyCredentials.java @@ -89,7 +89,7 @@ void testCreateGrpcClient_withApiKey_sendsApiHeaderToServer() throws IOException Metadata.Key.of(API_KEY_AUTH_HEADER, Metadata.ASCII_STRING_MARSHALLER); Metadata.Key defaultAuthorizationHeader = Metadata.Key.of(AuthHttpConstants.AUTHORIZATION, Metadata.ASCII_STRING_MARSHALLER); - EchoSettings settings = + EchoSettings.Builder echoBuilder = EchoSettings.newBuilder() .setApiKey(API_KEY) .setEndpoint(GRPC_ENDPOINT) @@ -97,12 +97,13 @@ void testCreateGrpcClient_withApiKey_sendsApiHeaderToServer() throws IOException EchoSettings.defaultGrpcTransportProviderBuilder() .setChannelConfigurator(ManagedChannelBuilder::usePlaintext) .setInterceptorProvider(() -> ImmutableList.of(grpcInterceptor)) - .build()) - .build(); + .build()); + EchoSettings settings = echoBuilder.build(); grpcClient = EchoClient.create(settings); grpcClient.echo(EchoRequest.newBuilder().build()); + assertThat(echoBuilder.getApiKey()).isEqualTo(API_KEY); assertThat(settings.getApiKey()).isEqualTo(API_KEY); String headerValue = grpcInterceptor.metadata.get(apiKeyAuthHeader); assertThat(headerValue).isEqualTo(API_KEY); From 50cbea1143a902af83ce560da9d13106d5bab113 Mon Sep 17 00:00:00 2001 From: detmerl Date: Mon, 30 Sep 2024 10:54:31 -0400 Subject: [PATCH 23/38] cleaned up error checking in dedup + updated tests and java doc --- .../InstantiatingGrpcChannelProvider.java | 8 ++- .../InstantiatingGrpcChannelProviderTest.java | 70 +++++++++++++++---- .../google/api/gax/rpc/ClientSettings.java | 4 +- .../com/google/api/gax/rpc/StubSettings.java | 5 ++ .../v1beta1/it/ITApiKeyCredentials.java | 52 -------------- 5 files changed, 69 insertions(+), 70 deletions(-) diff --git a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java index 1adc79e40a..9ce3910fc8 100644 --- a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java +++ b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java @@ -64,6 +64,7 @@ import java.security.GeneralSecurityException; import java.security.KeyStore; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.concurrent.Executor; import java.util.concurrent.ScheduledExecutorService; @@ -505,8 +506,11 @@ private Map mergeHeadersWithCredentialHeaders() { Map userHeaders = new HashMap<>(headerProvider.getHeaders()); if (credentials != null) { try { - userHeaders.keySet().removeAll(credentials.getRequestMetadata().keySet()); - } catch (Exception e) { + Map> credentialRequestMetatData = credentials.getRequestMetadata(); + if (credentialRequestMetatData != null) { + userHeaders.keySet().removeAll(credentialRequestMetatData.keySet()); + } + } catch (IOException e) { // no-op, if we can't retrieve credentials metadata we will leave headers intact } } diff --git a/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java b/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java index 4d71db080e..3d8dfe6930 100644 --- a/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java +++ b/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java @@ -36,6 +36,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.when; import com.google.api.core.ApiFunction; import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider.Builder; @@ -46,6 +47,7 @@ import com.google.api.gax.rpc.internal.EnvironmentProvider; import com.google.api.gax.rpc.mtls.AbstractMtlsTransportChannelTest; import com.google.api.gax.rpc.mtls.MtlsProvider; +import com.google.auth.ApiKeyCredentials; import com.google.auth.Credentials; import com.google.auth.oauth2.CloudShellCredentials; import com.google.auth.oauth2.ComputeEngineCredentials; @@ -289,9 +291,9 @@ void testChannelConfigurator() throws IOException { ManagedChannelBuilder swappedBuilder = Mockito.mock(ManagedChannelBuilder.class); ManagedChannel fakeChannel = Mockito.mock(ManagedChannel.class); - Mockito.when(swappedBuilder.build()).thenReturn(fakeChannel); + when(swappedBuilder.build()).thenReturn(fakeChannel); - Mockito.when(channelConfigurator.apply(channelBuilderCaptor.capture())) + when(channelConfigurator.apply(channelBuilderCaptor.capture())) .thenReturn(swappedBuilder); // Invoke the provider @@ -711,7 +713,7 @@ void testLogDirectPathMisconfigNotOnGCE() throws Exception { public void canUseDirectPath_happyPath() throws IOException { System.setProperty("os.name", "Linux"); EnvironmentProvider envProvider = Mockito.mock(EnvironmentProvider.class); - Mockito.when( + when( envProvider.getenv( InstantiatingGrpcChannelProvider.DIRECT_PATH_ENV_DISABLE_DIRECT_PATH)) .thenReturn("false"); @@ -736,7 +738,7 @@ public void canUseDirectPath_happyPath() throws IOException { public void canUseDirectPath_directPathEnvVarDisabled() throws IOException { System.setProperty("os.name", "Linux"); EnvironmentProvider envProvider = Mockito.mock(EnvironmentProvider.class); - Mockito.when( + when( envProvider.getenv( InstantiatingGrpcChannelProvider.DIRECT_PATH_ENV_DISABLE_DIRECT_PATH)) .thenReturn("true"); @@ -788,7 +790,7 @@ public void canUseDirectPath_nonComputeCredentials() { System.setProperty("os.name", "Linux"); Credentials credentials = Mockito.mock(Credentials.class); EnvironmentProvider envProvider = Mockito.mock(EnvironmentProvider.class); - Mockito.when( + when( envProvider.getenv( InstantiatingGrpcChannelProvider.DIRECT_PATH_ENV_DISABLE_DIRECT_PATH)) .thenReturn("false"); @@ -807,7 +809,7 @@ public void canUseDirectPath_nonComputeCredentials() { public void canUseDirectPath_isNotOnComputeEngine_invalidOsNameSystemProperty() { System.setProperty("os.name", "Not Linux"); EnvironmentProvider envProvider = Mockito.mock(EnvironmentProvider.class); - Mockito.when( + when( envProvider.getenv( InstantiatingGrpcChannelProvider.DIRECT_PATH_ENV_DISABLE_DIRECT_PATH)) .thenReturn("false"); @@ -826,7 +828,7 @@ public void canUseDirectPath_isNotOnComputeEngine_invalidOsNameSystemProperty() public void canUseDirectPath_isNotOnComputeEngine_invalidSystemProductName() { System.setProperty("os.name", "Linux"); EnvironmentProvider envProvider = Mockito.mock(EnvironmentProvider.class); - Mockito.when( + when( envProvider.getenv( InstantiatingGrpcChannelProvider.DIRECT_PATH_ENV_DISABLE_DIRECT_PATH)) .thenReturn("false"); @@ -845,7 +847,7 @@ public void canUseDirectPath_isNotOnComputeEngine_invalidSystemProductName() { public void canUseDirectPath_isNotOnComputeEngine_unableToGetSystemProductName() { System.setProperty("os.name", "Linux"); EnvironmentProvider envProvider = Mockito.mock(EnvironmentProvider.class); - Mockito.when( + when( envProvider.getenv( InstantiatingGrpcChannelProvider.DIRECT_PATH_ENV_DISABLE_DIRECT_PATH)) .thenReturn("false"); @@ -863,7 +865,7 @@ public void canUseDirectPath_isNotOnComputeEngine_unableToGetSystemProductName() public void canUseDirectPath_nonGDUUniverseDomain() { System.setProperty("os.name", "Linux"); EnvironmentProvider envProvider = Mockito.mock(EnvironmentProvider.class); - Mockito.when( + when( envProvider.getenv( InstantiatingGrpcChannelProvider.DIRECT_PATH_ENV_DISABLE_DIRECT_PATH)) .thenReturn("false"); @@ -881,14 +883,11 @@ public void canUseDirectPath_nonGDUUniverseDomain() { @Test public void createChannel_handlesMatchCredentialAndExplicitHeaders() throws IOException { - String apiHeaderKey = "fake_api_key_2"; - Map header = new HashMap<>(); - header.put("x-goog-api-key", apiHeaderKey); - FixedHeaderProvider headerProvider = FixedHeaderProvider.create(header); + ApiKeyCredentials apiKeyCredentials = ApiKeyCredentials.create("fake_api_key"); InstantiatingGrpcChannelProvider.Builder builder = InstantiatingGrpcChannelProvider.newBuilder() - .setCredentials(computeEngineCredentials) - .setHeaderProvider(headerProvider) + .setCredentials(apiKeyCredentials) + .setHeaderProvider(getHeaderProviderWithApiKeyHeader()) .setEndpoint("test.random.com:443"); InstantiatingGrpcChannelProvider provider = builder.build(); @@ -916,6 +915,47 @@ public void createChannel_handlesNullCredentials() throws IOException { transportChannel.shutdownNow(); } + @Test + public void createChannel_handlesNullCredentialsMetadataRequest() throws IOException { + Credentials credentials = Mockito.mock(Credentials.class); + when(credentials.getRequestMetadata()).thenReturn(null); + InstantiatingGrpcChannelProvider.Builder builder = + InstantiatingGrpcChannelProvider.newBuilder() + .setHeaderProvider(getHeaderProviderWithApiKeyHeader()) + .setEndpoint("test.random.com:443"); + InstantiatingGrpcChannelProvider provider = builder.build(); + + // calls createChannel + TransportChannel transportChannel = provider.getTransportChannel(); + + assertNotNull(transportChannel); + transportChannel.shutdownNow(); + } + + @Test + public void createChannel_handlesErrorRetrievingCredentialsMetadataRequest() throws IOException { + Credentials credentials = Mockito.mock(Credentials.class); + when(credentials.getRequestMetadata()).thenThrow(new IOException("Error getting request metadata")); + InstantiatingGrpcChannelProvider.Builder builder = + InstantiatingGrpcChannelProvider.newBuilder() + .setHeaderProvider(getHeaderProviderWithApiKeyHeader()) + .setEndpoint("test.random.com:443"); + InstantiatingGrpcChannelProvider provider = builder.build(); + + // calls createChannel + TransportChannel transportChannel = provider.getTransportChannel(); + + assertNotNull(transportChannel); + transportChannel.shutdownNow(); + } + + private FixedHeaderProvider getHeaderProviderWithApiKeyHeader() { + String apiHeaderKey = "fake_api_key_2"; + Map header = new HashMap<>(); + header.put("x-goog-api-key", apiHeaderKey); + return FixedHeaderProvider.create(header); + } + private static class FakeLogHandler extends Handler { List records = new ArrayList<>(); diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java index 65f2e92d40..1263252bcb 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java @@ -311,7 +311,9 @@ public B setGdchApiAudience(@Nullable String gdchApiAudience) { * Sets the API key. The API key will get translated to an {@link * com.google.auth.ApiKeyCredentials} and stored in {@link ClientContext}. * - *

Note: If you set an API key and {@link CredentialsProvider} in the same ClientSettings the + * API Key authorization is not supported for every product. Please check the documentation for each product to confirm if it is supported. + * + * If you set an API key and {@link CredentialsProvider} in the same ClientSettings the * API key will override any credentials provided. */ public B setApiKey(String apiKey) { diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java index 49a521aaa9..ae9310ad9e 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java @@ -588,6 +588,11 @@ public B setTracerFactory(@Nonnull ApiTracerFactory tracerFactory) { /** * Sets the API key. The API key will get translated to an {@link * com.google.auth.ApiKeyCredentials} and stored in {@link ClientContext}. + * + * API Key authorization is not supported for every product. Please check the documentation for each product to confirm if it is supported. + * + * If you set an API key and {@link CredentialsProvider} in the same ClientSettings the + * API key will override any credentials provided. */ public B setApiKey(String apiKey) { this.apiKey = apiKey; diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITApiKeyCredentials.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITApiKeyCredentials.java index 6228882963..19ee177f3f 100644 --- a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITApiKeyCredentials.java +++ b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITApiKeyCredentials.java @@ -152,31 +152,6 @@ void testCreateGrpcClient_withApiKeySetViaSetterAndHeader_dedupsHeader() throws assertThat(matchingHeadersList.size()).isEqualTo(1); } - @Test - void testCreateGrpcClient_withApiKeyAndExplicitCredentials_overridesCredentials() - throws IOException { - Metadata.Key apiKeyAuthHeader = - Metadata.Key.of(API_KEY_AUTH_HEADER, Metadata.ASCII_STRING_MARSHALLER); - EchoSettings settings = - EchoSettings.newBuilder() - .setApiKey(API_KEY) - .setEndpoint(GRPC_ENDPOINT) - .setCredentialsProvider( - FixedCredentialsProvider.create(ApiKeyCredentials.create("invalid"))) - .setTransportChannelProvider( - EchoSettings.defaultGrpcTransportProviderBuilder() - .setChannelConfigurator(ManagedChannelBuilder::usePlaintext) - .setInterceptorProvider(() -> ImmutableList.of(grpcInterceptor)) - .build()) - .build(); - grpcClient = EchoClient.create(settings); - - grpcClient.echo(EchoRequest.newBuilder().build()); - - String headerValue = grpcInterceptor.metadata.get(apiKeyAuthHeader); - assertThat(headerValue).isEqualTo(API_KEY); - } - @Test void testCreateHttpClient_withApiKey_sendsApiHeaderToServer() throws Exception { EchoSettings httpJsonEchoSettings = @@ -243,31 +218,4 @@ void testCreateHttpClient_withApiKeySetViaSetterAndHeader_dedupsHeader() throws assertThat(headerValue).isEqualTo(API_KEY); assertThat(headerValues.size()).isEqualTo(1); } - - @Test - void testCreateHttpClient_withApiKeyAndExplicitCredentials_overridesCredentials() - throws Exception { - EchoSettings httpJsonEchoSettings = - EchoSettings.newHttpJsonBuilder() - .setApiKey(API_KEY) - .setCredentialsProvider( - FixedCredentialsProvider.create(ApiKeyCredentials.create("invalid"))) - .setTransportChannelProvider( - EchoSettings.defaultHttpJsonTransportProviderBuilder() - .setHttpTransport( - new NetHttpTransport.Builder().doNotValidateCertificate().build()) - .setEndpoint(HTTP_ENDPOINT) - .setInterceptorProvider(() -> ImmutableList.of(httpJsonInterceptor)) - .build()) - .build(); - httpJsonClient = EchoClient.create(httpJsonEchoSettings); - - httpJsonClient.echo(EchoRequest.newBuilder().build()); - - ArrayList headerValues = - (ArrayList) - httpJsonInterceptor.metadata.getHeaders().get(HTTP_RESPONSE_HEADER_STRING); - String headerValue = headerValues.get(0); - assertThat(headerValue).isEqualTo(API_KEY); - } } From 351389c5b0304c4b0f941c5253d7706fe8e06ead Mon Sep 17 00:00:00 2001 From: detmerl Date: Mon, 30 Sep 2024 10:57:04 -0400 Subject: [PATCH 24/38] lint fix --- .../InstantiatingGrpcChannelProviderTest.java | 46 +++++++------------ .../google/api/gax/rpc/ClientSettings.java | 5 +- .../com/google/api/gax/rpc/StubSettings.java | 5 +- 3 files changed, 22 insertions(+), 34 deletions(-) diff --git a/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java b/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java index 3d8dfe6930..54028f1b1d 100644 --- a/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java +++ b/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java @@ -293,8 +293,7 @@ void testChannelConfigurator() throws IOException { ManagedChannel fakeChannel = Mockito.mock(ManagedChannel.class); when(swappedBuilder.build()).thenReturn(fakeChannel); - when(channelConfigurator.apply(channelBuilderCaptor.capture())) - .thenReturn(swappedBuilder); + when(channelConfigurator.apply(channelBuilderCaptor.capture())).thenReturn(swappedBuilder); // Invoke the provider InstantiatingGrpcChannelProvider.newBuilder() @@ -713,9 +712,7 @@ void testLogDirectPathMisconfigNotOnGCE() throws Exception { public void canUseDirectPath_happyPath() throws IOException { System.setProperty("os.name", "Linux"); EnvironmentProvider envProvider = Mockito.mock(EnvironmentProvider.class); - when( - envProvider.getenv( - InstantiatingGrpcChannelProvider.DIRECT_PATH_ENV_DISABLE_DIRECT_PATH)) + when(envProvider.getenv(InstantiatingGrpcChannelProvider.DIRECT_PATH_ENV_DISABLE_DIRECT_PATH)) .thenReturn("false"); InstantiatingGrpcChannelProvider.Builder builder = InstantiatingGrpcChannelProvider.newBuilder() @@ -738,9 +735,7 @@ public void canUseDirectPath_happyPath() throws IOException { public void canUseDirectPath_directPathEnvVarDisabled() throws IOException { System.setProperty("os.name", "Linux"); EnvironmentProvider envProvider = Mockito.mock(EnvironmentProvider.class); - when( - envProvider.getenv( - InstantiatingGrpcChannelProvider.DIRECT_PATH_ENV_DISABLE_DIRECT_PATH)) + when(envProvider.getenv(InstantiatingGrpcChannelProvider.DIRECT_PATH_ENV_DISABLE_DIRECT_PATH)) .thenReturn("true"); InstantiatingGrpcChannelProvider.Builder builder = InstantiatingGrpcChannelProvider.newBuilder() @@ -790,9 +785,7 @@ public void canUseDirectPath_nonComputeCredentials() { System.setProperty("os.name", "Linux"); Credentials credentials = Mockito.mock(Credentials.class); EnvironmentProvider envProvider = Mockito.mock(EnvironmentProvider.class); - when( - envProvider.getenv( - InstantiatingGrpcChannelProvider.DIRECT_PATH_ENV_DISABLE_DIRECT_PATH)) + when(envProvider.getenv(InstantiatingGrpcChannelProvider.DIRECT_PATH_ENV_DISABLE_DIRECT_PATH)) .thenReturn("false"); InstantiatingGrpcChannelProvider.Builder builder = InstantiatingGrpcChannelProvider.newBuilder() @@ -809,9 +802,7 @@ public void canUseDirectPath_nonComputeCredentials() { public void canUseDirectPath_isNotOnComputeEngine_invalidOsNameSystemProperty() { System.setProperty("os.name", "Not Linux"); EnvironmentProvider envProvider = Mockito.mock(EnvironmentProvider.class); - when( - envProvider.getenv( - InstantiatingGrpcChannelProvider.DIRECT_PATH_ENV_DISABLE_DIRECT_PATH)) + when(envProvider.getenv(InstantiatingGrpcChannelProvider.DIRECT_PATH_ENV_DISABLE_DIRECT_PATH)) .thenReturn("false"); InstantiatingGrpcChannelProvider.Builder builder = InstantiatingGrpcChannelProvider.newBuilder() @@ -828,9 +819,7 @@ public void canUseDirectPath_isNotOnComputeEngine_invalidOsNameSystemProperty() public void canUseDirectPath_isNotOnComputeEngine_invalidSystemProductName() { System.setProperty("os.name", "Linux"); EnvironmentProvider envProvider = Mockito.mock(EnvironmentProvider.class); - when( - envProvider.getenv( - InstantiatingGrpcChannelProvider.DIRECT_PATH_ENV_DISABLE_DIRECT_PATH)) + when(envProvider.getenv(InstantiatingGrpcChannelProvider.DIRECT_PATH_ENV_DISABLE_DIRECT_PATH)) .thenReturn("false"); InstantiatingGrpcChannelProvider.Builder builder = InstantiatingGrpcChannelProvider.newBuilder() @@ -847,9 +836,7 @@ public void canUseDirectPath_isNotOnComputeEngine_invalidSystemProductName() { public void canUseDirectPath_isNotOnComputeEngine_unableToGetSystemProductName() { System.setProperty("os.name", "Linux"); EnvironmentProvider envProvider = Mockito.mock(EnvironmentProvider.class); - when( - envProvider.getenv( - InstantiatingGrpcChannelProvider.DIRECT_PATH_ENV_DISABLE_DIRECT_PATH)) + when(envProvider.getenv(InstantiatingGrpcChannelProvider.DIRECT_PATH_ENV_DISABLE_DIRECT_PATH)) .thenReturn("false"); InstantiatingGrpcChannelProvider.Builder builder = InstantiatingGrpcChannelProvider.newBuilder() @@ -865,9 +852,7 @@ public void canUseDirectPath_isNotOnComputeEngine_unableToGetSystemProductName() public void canUseDirectPath_nonGDUUniverseDomain() { System.setProperty("os.name", "Linux"); EnvironmentProvider envProvider = Mockito.mock(EnvironmentProvider.class); - when( - envProvider.getenv( - InstantiatingGrpcChannelProvider.DIRECT_PATH_ENV_DISABLE_DIRECT_PATH)) + when(envProvider.getenv(InstantiatingGrpcChannelProvider.DIRECT_PATH_ENV_DISABLE_DIRECT_PATH)) .thenReturn("false"); String nonGDUEndpoint = "test.random.com:443"; InstantiatingGrpcChannelProvider.Builder builder = @@ -920,9 +905,9 @@ public void createChannel_handlesNullCredentialsMetadataRequest() throws IOExcep Credentials credentials = Mockito.mock(Credentials.class); when(credentials.getRequestMetadata()).thenReturn(null); InstantiatingGrpcChannelProvider.Builder builder = - InstantiatingGrpcChannelProvider.newBuilder() - .setHeaderProvider(getHeaderProviderWithApiKeyHeader()) - .setEndpoint("test.random.com:443"); + InstantiatingGrpcChannelProvider.newBuilder() + .setHeaderProvider(getHeaderProviderWithApiKeyHeader()) + .setEndpoint("test.random.com:443"); InstantiatingGrpcChannelProvider provider = builder.build(); // calls createChannel @@ -935,11 +920,12 @@ public void createChannel_handlesNullCredentialsMetadataRequest() throws IOExcep @Test public void createChannel_handlesErrorRetrievingCredentialsMetadataRequest() throws IOException { Credentials credentials = Mockito.mock(Credentials.class); - when(credentials.getRequestMetadata()).thenThrow(new IOException("Error getting request metadata")); + when(credentials.getRequestMetadata()) + .thenThrow(new IOException("Error getting request metadata")); InstantiatingGrpcChannelProvider.Builder builder = - InstantiatingGrpcChannelProvider.newBuilder() - .setHeaderProvider(getHeaderProviderWithApiKeyHeader()) - .setEndpoint("test.random.com:443"); + InstantiatingGrpcChannelProvider.newBuilder() + .setHeaderProvider(getHeaderProviderWithApiKeyHeader()) + .setEndpoint("test.random.com:443"); InstantiatingGrpcChannelProvider provider = builder.build(); // calls createChannel diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java index 1263252bcb..2d5cba7f24 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java @@ -311,9 +311,10 @@ public B setGdchApiAudience(@Nullable String gdchApiAudience) { * Sets the API key. The API key will get translated to an {@link * com.google.auth.ApiKeyCredentials} and stored in {@link ClientContext}. * - * API Key authorization is not supported for every product. Please check the documentation for each product to confirm if it is supported. + *

API Key authorization is not supported for every product. Please check the documentation + * for each product to confirm if it is supported. * - * If you set an API key and {@link CredentialsProvider} in the same ClientSettings the + *

Note: If you set an API key and {@link CredentialsProvider} in the same ClientSettings the * API key will override any credentials provided. */ public B setApiKey(String apiKey) { diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java index ae9310ad9e..6de67844b3 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java @@ -589,9 +589,10 @@ public B setTracerFactory(@Nonnull ApiTracerFactory tracerFactory) { * Sets the API key. The API key will get translated to an {@link * com.google.auth.ApiKeyCredentials} and stored in {@link ClientContext}. * - * API Key authorization is not supported for every product. Please check the documentation for each product to confirm if it is supported. + *

API Key authorization is not supported for every product. Please check the documentation + * for each product to confirm if it is supported. * - * If you set an API key and {@link CredentialsProvider} in the same ClientSettings the + *

Note: If you set an API key and {@link CredentialsProvider} in the same ClientSettings the * API key will override any credentials provided. */ public B setApiKey(String apiKey) { From ca193044d7ca44a917107bce3d79946e380d596c Mon Sep 17 00:00:00 2001 From: detmerl Date: Mon, 30 Sep 2024 11:02:08 -0400 Subject: [PATCH 25/38] lint fix --- .../InstantiatingGrpcChannelProviderTest.java | 38 +++++++++++++------ 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java b/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java index 54028f1b1d..684806c607 100644 --- a/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java +++ b/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java @@ -36,7 +36,6 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.mockito.Mockito.when; import com.google.api.core.ApiFunction; import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider.Builder; @@ -291,9 +290,10 @@ void testChannelConfigurator() throws IOException { ManagedChannelBuilder swappedBuilder = Mockito.mock(ManagedChannelBuilder.class); ManagedChannel fakeChannel = Mockito.mock(ManagedChannel.class); - when(swappedBuilder.build()).thenReturn(fakeChannel); + Mockito.when(swappedBuilder.build()).thenReturn(fakeChannel); - when(channelConfigurator.apply(channelBuilderCaptor.capture())).thenReturn(swappedBuilder); + Mockito.when(channelConfigurator.apply(channelBuilderCaptor.capture())) + .thenReturn(swappedBuilder); // Invoke the provider InstantiatingGrpcChannelProvider.newBuilder() @@ -712,7 +712,9 @@ void testLogDirectPathMisconfigNotOnGCE() throws Exception { public void canUseDirectPath_happyPath() throws IOException { System.setProperty("os.name", "Linux"); EnvironmentProvider envProvider = Mockito.mock(EnvironmentProvider.class); - when(envProvider.getenv(InstantiatingGrpcChannelProvider.DIRECT_PATH_ENV_DISABLE_DIRECT_PATH)) + Mockito.when( + envProvider.getenv( + InstantiatingGrpcChannelProvider.DIRECT_PATH_ENV_DISABLE_DIRECT_PATH)) .thenReturn("false"); InstantiatingGrpcChannelProvider.Builder builder = InstantiatingGrpcChannelProvider.newBuilder() @@ -735,7 +737,9 @@ public void canUseDirectPath_happyPath() throws IOException { public void canUseDirectPath_directPathEnvVarDisabled() throws IOException { System.setProperty("os.name", "Linux"); EnvironmentProvider envProvider = Mockito.mock(EnvironmentProvider.class); - when(envProvider.getenv(InstantiatingGrpcChannelProvider.DIRECT_PATH_ENV_DISABLE_DIRECT_PATH)) + Mockito.when( + envProvider.getenv( + InstantiatingGrpcChannelProvider.DIRECT_PATH_ENV_DISABLE_DIRECT_PATH)) .thenReturn("true"); InstantiatingGrpcChannelProvider.Builder builder = InstantiatingGrpcChannelProvider.newBuilder() @@ -785,7 +789,9 @@ public void canUseDirectPath_nonComputeCredentials() { System.setProperty("os.name", "Linux"); Credentials credentials = Mockito.mock(Credentials.class); EnvironmentProvider envProvider = Mockito.mock(EnvironmentProvider.class); - when(envProvider.getenv(InstantiatingGrpcChannelProvider.DIRECT_PATH_ENV_DISABLE_DIRECT_PATH)) + Mockito.when( + envProvider.getenv( + InstantiatingGrpcChannelProvider.DIRECT_PATH_ENV_DISABLE_DIRECT_PATH)) .thenReturn("false"); InstantiatingGrpcChannelProvider.Builder builder = InstantiatingGrpcChannelProvider.newBuilder() @@ -802,7 +808,9 @@ public void canUseDirectPath_nonComputeCredentials() { public void canUseDirectPath_isNotOnComputeEngine_invalidOsNameSystemProperty() { System.setProperty("os.name", "Not Linux"); EnvironmentProvider envProvider = Mockito.mock(EnvironmentProvider.class); - when(envProvider.getenv(InstantiatingGrpcChannelProvider.DIRECT_PATH_ENV_DISABLE_DIRECT_PATH)) + Mockito.when( + envProvider.getenv( + InstantiatingGrpcChannelProvider.DIRECT_PATH_ENV_DISABLE_DIRECT_PATH)) .thenReturn("false"); InstantiatingGrpcChannelProvider.Builder builder = InstantiatingGrpcChannelProvider.newBuilder() @@ -819,7 +827,9 @@ public void canUseDirectPath_isNotOnComputeEngine_invalidOsNameSystemProperty() public void canUseDirectPath_isNotOnComputeEngine_invalidSystemProductName() { System.setProperty("os.name", "Linux"); EnvironmentProvider envProvider = Mockito.mock(EnvironmentProvider.class); - when(envProvider.getenv(InstantiatingGrpcChannelProvider.DIRECT_PATH_ENV_DISABLE_DIRECT_PATH)) + Mockito.when( + envProvider.getenv( + InstantiatingGrpcChannelProvider.DIRECT_PATH_ENV_DISABLE_DIRECT_PATH)) .thenReturn("false"); InstantiatingGrpcChannelProvider.Builder builder = InstantiatingGrpcChannelProvider.newBuilder() @@ -836,7 +846,9 @@ public void canUseDirectPath_isNotOnComputeEngine_invalidSystemProductName() { public void canUseDirectPath_isNotOnComputeEngine_unableToGetSystemProductName() { System.setProperty("os.name", "Linux"); EnvironmentProvider envProvider = Mockito.mock(EnvironmentProvider.class); - when(envProvider.getenv(InstantiatingGrpcChannelProvider.DIRECT_PATH_ENV_DISABLE_DIRECT_PATH)) + Mockito.when( + envProvider.getenv( + InstantiatingGrpcChannelProvider.DIRECT_PATH_ENV_DISABLE_DIRECT_PATH)) .thenReturn("false"); InstantiatingGrpcChannelProvider.Builder builder = InstantiatingGrpcChannelProvider.newBuilder() @@ -852,7 +864,9 @@ public void canUseDirectPath_isNotOnComputeEngine_unableToGetSystemProductName() public void canUseDirectPath_nonGDUUniverseDomain() { System.setProperty("os.name", "Linux"); EnvironmentProvider envProvider = Mockito.mock(EnvironmentProvider.class); - when(envProvider.getenv(InstantiatingGrpcChannelProvider.DIRECT_PATH_ENV_DISABLE_DIRECT_PATH)) + Mockito.when( + envProvider.getenv( + InstantiatingGrpcChannelProvider.DIRECT_PATH_ENV_DISABLE_DIRECT_PATH)) .thenReturn("false"); String nonGDUEndpoint = "test.random.com:443"; InstantiatingGrpcChannelProvider.Builder builder = @@ -903,7 +917,7 @@ public void createChannel_handlesNullCredentials() throws IOException { @Test public void createChannel_handlesNullCredentialsMetadataRequest() throws IOException { Credentials credentials = Mockito.mock(Credentials.class); - when(credentials.getRequestMetadata()).thenReturn(null); + Mockito.when(credentials.getRequestMetadata()).thenReturn(null); InstantiatingGrpcChannelProvider.Builder builder = InstantiatingGrpcChannelProvider.newBuilder() .setHeaderProvider(getHeaderProviderWithApiKeyHeader()) @@ -920,7 +934,7 @@ public void createChannel_handlesNullCredentialsMetadataRequest() throws IOExcep @Test public void createChannel_handlesErrorRetrievingCredentialsMetadataRequest() throws IOException { Credentials credentials = Mockito.mock(Credentials.class); - when(credentials.getRequestMetadata()) + Mockito.when(credentials.getRequestMetadata()) .thenThrow(new IOException("Error getting request metadata")); InstantiatingGrpcChannelProvider.Builder builder = InstantiatingGrpcChannelProvider.newBuilder() From aa6a0065fcbb29b75fff28d4b13a3cd0ea8e4057 Mon Sep 17 00:00:00 2001 From: detmerl Date: Mon, 30 Sep 2024 11:06:26 -0400 Subject: [PATCH 26/38] cleaned up java docs so stub settings and client settings are matching --- .../main/java/com/google/api/gax/rpc/ClientSettings.java | 3 ++- .../main/java/com/google/api/gax/rpc/StubSettings.java | 9 +++------ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java index 2d5cba7f24..234fd4d19d 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java @@ -112,6 +112,7 @@ public final WatchdogProvider getWatchdogProvider() { return stubSettings.getStreamWatchdogProvider(); } + /** Gets the API Key that should be used for authentication. */ public final String getApiKey() { return stubSettings.getApiKey(); } @@ -384,7 +385,7 @@ public WatchdogProvider getWatchdogProvider() { return stubSettings.getStreamWatchdogProvider(); } - /** Gets the ApiKey that was previously set on this Builder. */ + /** Gets the API Key that was previously set on this Builder. */ public String getApiKey() { return stubSettings.getApiKey(); } diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java index 6de67844b3..03c9519e38 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java @@ -236,7 +236,7 @@ public final String getGdchApiAudience() { return gdchApiAudience; } - /** Gets the ApiKey that should be used for authentication. */ + /** Gets the API Key that should be used for authentication. */ public final String getApiKey() { return apiKey; } @@ -642,12 +642,9 @@ public ApiClock getClock() { return clock; } - /** - * Gets the ApiKey that was previously set on this Builder. If an empty string was provided it - * will return null - */ + /** Gets the API Key that was previously set on this Builder. */ public final String getApiKey() { - return (apiKey == null || apiKey.isEmpty()) ? null : apiKey; + return apiKey; } /** From 0938405837d17570e6bfa5b367dd896ec54b4b59 Mon Sep 17 00:00:00 2001 From: detmerl Date: Mon, 30 Sep 2024 11:13:08 -0400 Subject: [PATCH 27/38] lint fix --- .../com/google/showcase/v1beta1/it/ITApiKeyCredentials.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITApiKeyCredentials.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITApiKeyCredentials.java index 19ee177f3f..5867bcfea9 100644 --- a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITApiKeyCredentials.java +++ b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITApiKeyCredentials.java @@ -19,10 +19,8 @@ import static com.google.common.truth.Truth.assertThat; import com.google.api.client.http.javanet.NetHttpTransport; -import com.google.api.gax.core.FixedCredentialsProvider; import com.google.api.gax.rpc.FixedHeaderProvider; import com.google.api.gax.rpc.TransportChannelProvider; -import com.google.auth.ApiKeyCredentials; import com.google.auth.http.AuthHttpConstants; import com.google.common.collect.ImmutableList; import com.google.showcase.v1beta1.EchoClient; From a39bba0bb6004b81a8fd46c3bc4e490252871219 Mon Sep 17 00:00:00 2001 From: detmerl Date: Mon, 30 Sep 2024 13:11:43 -0400 Subject: [PATCH 28/38] fixed gdch IT tests --- .../com/google/api/gax/rpc/ClientContext.java | 11 +++++------ .../com/google/showcase/v1beta1/it/ITGdch.java | 18 ++++++++++++++---- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java index 924f58462d..ee0bc36b93 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java @@ -47,6 +47,7 @@ import com.google.auth.Credentials; import com.google.auth.oauth2.GdchCredentials; import com.google.auto.value.AutoValue; +import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Sets; @@ -188,9 +189,7 @@ public static ClientContext create(StubSettings settings) throws IOException { // Resolve the new endpoint with the GDC-H flow endpoint = endpointContext.resolvedEndpoint(); // We recompute the GdchCredentials with the audience - credentials = - getGdchCredentials( - settingsGdchApiAudience, endpointContext.resolvedEndpoint(), credentials); + credentials = getGdchCredentials(settingsGdchApiAudience, endpoint, credentials); } else if (!Strings.isNullOrEmpty(settingsGdchApiAudience)) { throw new IllegalArgumentException( "GDC-H API audience can only be set when using GdchCredentials"); @@ -295,7 +294,8 @@ private static Credentials getCredentials(StubSettings settings) throws IOExcept * Constructs a new {@link com.google.auth.Credentials} object based on credentials provided with * a GDC-H audience */ - private static Credentials getGdchCredentials( + @VisibleForTesting + public static GdchCredentials getGdchCredentials( String settingsGdchApiAudience, String endpoint, Credentials credentials) throws IOException { String audienceString; if (!Strings.isNullOrEmpty(settingsGdchApiAudience)) { @@ -312,8 +312,7 @@ private static Credentials getGdchCredentials( } catch (IllegalArgumentException ex) { // thrown when passing a malformed uri string throw new IllegalArgumentException("The GDC-H API audience string is not a valid URI", ex); } - credentials = ((GdchCredentials) credentials).createWithGdchAudience(gdchAudienceUri); - return credentials; + return ((GdchCredentials) credentials).createWithGdchAudience(gdchAudienceUri); } /** diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITGdch.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITGdch.java index f043d45cfc..77703a681b 100644 --- a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITGdch.java +++ b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITGdch.java @@ -136,6 +136,12 @@ void testCreateClient_withGdchCredentialAndNoAudience_defaultsToEndpointBasedAud // we create the client as usual - no audience passed String testEndpoint = "custom-endpoint:123"; settings = settings.toBuilder().setEndpoint(testEndpoint).build(); + // need to register the new credentials with updated endpoint before creating + // TransportChannelProvider, as they must be valid to retrieve credential headers (used for + // de-duping) + registerCredential( + ClientContext.getGdchCredentials( + null, testEndpoint, settings.getCredentialsProvider().getCredentials())); context = ClientContext.create(settings); stubSettings = EchoStubSettings.newBuilder(context).build(); client = EchoClient.create(stubSettings.createStub()); @@ -161,7 +167,6 @@ void testCreateClient_withGdchCredentialAndNoAudience_defaultsToEndpointBasedAud // However, the credentials prepared in ClientContext should be able to refresh since the // audience would be // internally defaulted the endpoint of the StubSettings - registerCredential(fromContext); ((GdchCredentials) fromContext).refreshAccessToken(); String usedAudience = transportFactory.transport.getLastAudienceSent(); assertEquals(testEndpoint, usedAudience); @@ -182,8 +187,14 @@ void testCreateClient_withGdchCredentialWithValidAudience_usesCredentialWithPass // audience. It should // be created without issues String testAudience = "valid-audience"; - settings = - settings.toBuilder().setGdchApiAudience(testAudience).setEndpoint("localhost:7469").build(); + String endpoint = "localhost:7469"; + settings = settings.toBuilder().setGdchApiAudience(testAudience).setEndpoint(endpoint).build(); + // need to register the new credentials with updated audience before creating + // TransportChannelProvider, as they must be valid to retrieve credential headers (used for + // de-duping) + registerCredential( + ClientContext.getGdchCredentials( + testAudience, endpoint, settings.getCredentialsProvider().getCredentials())); context = ClientContext.create(settings); stubSettings = EchoStubSettings.newBuilder(context).build(); client = EchoClient.create(stubSettings.createStub()); @@ -206,7 +217,6 @@ void testCreateClient_withGdchCredentialWithValidAudience_usesCredentialWithPass // But the credentials prepared in ClientContext should be able to refresh since the audience // would be internally // set to the one passed in stub settings - registerCredential(fromContext); ((GdchCredentials) fromContext).refreshAccessToken(); String usedAudience = transportFactory.transport.getLastAudienceSent(); assertEquals(testAudience, usedAudience); From f64279e3fc83e598424fcd7f58988e9d1ca02f20 Mon Sep 17 00:00:00 2001 From: detmerl Date: Mon, 30 Sep 2024 15:49:18 -0400 Subject: [PATCH 29/38] updated so credential deduping happens during the object build process --- .../InstantiatingGrpcChannelProvider.java | 30 +++++-- .../InstantiatingGrpcChannelProviderTest.java | 89 +++++++++---------- 2 files changed, 62 insertions(+), 57 deletions(-) diff --git a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java index 9ce3910fc8..e0dc095d2c 100644 --- a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java +++ b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java @@ -125,6 +125,7 @@ public final class InstantiatingGrpcChannelProvider implements TransportChannelP @Nullable private final Boolean allowNonDefaultServiceAccount; @VisibleForTesting final ImmutableMap directPathServiceConfig; @Nullable private final MtlsProvider mtlsProvider; + private final Map providedHeadersWithDuplicatesRemoved = new HashMap<>(); @Nullable private final ApiFunction channelConfigurator; @@ -409,9 +410,8 @@ ChannelCredentials createMtlsChannelCredentials() throws IOException, GeneralSec } private ManagedChannel createSingleChannel() throws IOException { - Map mergedHeaders = mergeHeadersWithCredentialHeaders(); GrpcHeaderInterceptor headerInterceptor = - new GrpcHeaderInterceptor(ImmutableMap.copyOf(mergedHeaders)); + new GrpcHeaderInterceptor(providedHeadersWithDuplicatesRemoved); GrpcMetadataHandlerInterceptor metadataHandlerInterceptor = new GrpcMetadataHandlerInterceptor(); @@ -500,21 +500,25 @@ private ManagedChannel createSingleChannel() throws IOException { return managedChannel; } - // dedup any headers explicitly set with headers provided by credential, with preference to - // credential headers - private Map mergeHeadersWithCredentialHeaders() { - Map userHeaders = new HashMap<>(headerProvider.getHeaders()); + /* Remove any provided headers that will also get set by credentials. They will be added as part of the grpc call when performing auth + * {@link io.grpc.auth.GoogleAuthLibraryCallCredentials#applyRequestMetadata}. GRPC does not dedup headers {@link https://github.com/grpc/grpc-java/blob/a140e1bb0cfa662bcdb7823d73320eb8d49046f1/api/src/main/java/io/grpc/Metadata.java#L504} so we must before initiating call. + */ + private void removeCredentialDuplicateHeaders() { + if (headerProvider != null) { + providedHeadersWithDuplicatesRemoved.putAll(headerProvider.getHeaders()); + } if (credentials != null) { try { Map> credentialRequestMetatData = credentials.getRequestMetadata(); if (credentialRequestMetatData != null) { - userHeaders.keySet().removeAll(credentialRequestMetatData.keySet()); + providedHeadersWithDuplicatesRemoved + .keySet() + .removeAll(credentialRequestMetatData.keySet()); } } catch (IOException e) { // no-op, if we can't retrieve credentials metadata we will leave headers intact } } - return userHeaders; } /** @@ -585,6 +589,11 @@ public boolean shouldAutoClose() { return true; } + /** @return final headers that will be sent with GRPC call with any duplications removed */ + public Map getProvidedHeadersWithDuplicatesRemoved() { + return providedHeadersWithDuplicatesRemoved; + } + public Builder toBuilder() { return new Builder(this); } @@ -904,7 +913,10 @@ public Builder setDirectPathServiceConfig(Map serviceConfig) { } public InstantiatingGrpcChannelProvider build() { - return new InstantiatingGrpcChannelProvider(this); + InstantiatingGrpcChannelProvider instantiatingGrpcChannelProvider = + new InstantiatingGrpcChannelProvider(this); + instantiatingGrpcChannelProvider.removeCredentialDuplicateHeaders(); + return instantiatingGrpcChannelProvider; } /** diff --git a/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java b/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java index 684806c607..651a87b590 100644 --- a/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java +++ b/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java @@ -34,7 +34,7 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.truth.Truth.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import com.google.api.core.ApiFunction; @@ -82,6 +82,8 @@ class InstantiatingGrpcChannelProviderTest extends AbstractMtlsTransportChannelTest { private static final String DEFAULT_ENDPOINT = "test.googleapis.com:443"; + private static final String API_KEY_HEADER_VALUE = "fake_api_key_2"; + private static final String API_KEY_AUTH_HEADER = "x-goog-api-key"; private static String originalOSName; private ComputeEngineCredentials computeEngineCredentials; @@ -709,7 +711,7 @@ void testLogDirectPathMisconfigNotOnGCE() throws Exception { } @Test - public void canUseDirectPath_happyPath() throws IOException { + public void canUseDirectPath_happyPath() { System.setProperty("os.name", "Linux"); EnvironmentProvider envProvider = Mockito.mock(EnvironmentProvider.class); Mockito.when( @@ -721,20 +723,14 @@ public void canUseDirectPath_happyPath() throws IOException { .setAttemptDirectPath(true) .setCredentials(computeEngineCredentials) .setEndpoint(DEFAULT_ENDPOINT) - .setEnvProvider(envProvider) - .setHeaderProvider(Mockito.mock(HeaderProvider.class)); + .setEnvProvider(envProvider); InstantiatingGrpcChannelProvider provider = new InstantiatingGrpcChannelProvider(builder, GCE_PRODUCTION_NAME_AFTER_2016); Truth.assertThat(provider.canUseDirectPath()).isTrue(); - - // verify this info is passed correctly to transport channel - TransportChannel transportChannel = provider.getTransportChannel(); - Truth.assertThat(((GrpcTransportChannel) transportChannel).isDirectPath()).isTrue(); - transportChannel.shutdownNow(); } @Test - public void canUseDirectPath_directPathEnvVarDisabled() throws IOException { + public void canUseDirectPath_directPathEnvVarDisabled() { System.setProperty("os.name", "Linux"); EnvironmentProvider envProvider = Mockito.mock(EnvironmentProvider.class); Mockito.when( @@ -746,16 +742,10 @@ public void canUseDirectPath_directPathEnvVarDisabled() throws IOException { .setAttemptDirectPath(true) .setCredentials(computeEngineCredentials) .setEndpoint(DEFAULT_ENDPOINT) - .setEnvProvider(envProvider) - .setHeaderProvider(Mockito.mock(HeaderProvider.class)); + .setEnvProvider(envProvider); InstantiatingGrpcChannelProvider provider = new InstantiatingGrpcChannelProvider(builder, GCE_PRODUCTION_NAME_AFTER_2016); Truth.assertThat(provider.canUseDirectPath()).isFalse(); - - // verify this info is passed correctly to transport channel - TransportChannel transportChannel = provider.getTransportChannel(); - Truth.assertThat(((GrpcTransportChannel) transportChannel).isDirectPath()).isFalse(); - transportChannel.shutdownNow(); } @Test @@ -881,58 +871,63 @@ public void canUseDirectPath_nonGDUUniverseDomain() { } @Test - public void createChannel_handlesMatchCredentialAndExplicitHeaders() throws IOException { - ApiKeyCredentials apiKeyCredentials = ApiKeyCredentials.create("fake_api_key"); + public void providerInitializedWithNonConflictingHeaders_retainsHeaders() { InstantiatingGrpcChannelProvider.Builder builder = InstantiatingGrpcChannelProvider.newBuilder() - .setCredentials(apiKeyCredentials) .setHeaderProvider(getHeaderProviderWithApiKeyHeader()) .setEndpoint("test.random.com:443"); InstantiatingGrpcChannelProvider provider = builder.build(); + assertEquals(1, provider.getProvidedHeadersWithDuplicatesRemoved().size()); + assertEquals( + API_KEY_HEADER_VALUE, + provider.getProvidedHeadersWithDuplicatesRemoved().get(API_KEY_AUTH_HEADER)); + } - // calls createChannel - TransportChannel transportChannel = provider.getTransportChannel(); - - assertNotNull(transportChannel); - transportChannel.shutdownNow(); + @Test + public void providersInitializedWithConflictingHeaders_removesDuplicates() throws IOException { + String correctApiKey = "fake_api_key"; + ApiKeyCredentials apiKeyCredentials = ApiKeyCredentials.create(correctApiKey); + InstantiatingGrpcChannelProvider.Builder builder = + InstantiatingGrpcChannelProvider.newBuilder() + .setCredentials(apiKeyCredentials) + .setHeaderProvider(getHeaderProviderWithApiKeyHeader()) + .setEndpoint("test.random.com:443"); + InstantiatingGrpcChannelProvider provider = builder.build(); + assertEquals(0, provider.getProvidedHeadersWithDuplicatesRemoved().size()); + assertNull(provider.getProvidedHeadersWithDuplicatesRemoved().get(API_KEY_AUTH_HEADER)); } @Test - public void createChannel_handlesNullCredentials() throws IOException { + public void buildProvider_handlesNullHeaderProvider() { Map header = new HashMap(); - FixedHeaderProvider headerProvider = FixedHeaderProvider.create(header); + // FixedHeaderProvider headerProvider = FixedHeaderProvider.create(header); InstantiatingGrpcChannelProvider.Builder builder = InstantiatingGrpcChannelProvider.newBuilder() - .setHeaderProvider(headerProvider) + // .setHeaderProvider(headerProvider) .setEndpoint("test.random.com:443"); InstantiatingGrpcChannelProvider provider = builder.build(); - - // calls createChannel - TransportChannel transportChannel = provider.getTransportChannel(); - - assertNotNull(transportChannel); - transportChannel.shutdownNow(); + assertEquals(0, provider.getProvidedHeadersWithDuplicatesRemoved().size()); } @Test - public void createChannel_handlesNullCredentialsMetadataRequest() throws IOException { + public void buildProvider_handlesNullCredentialsMetadataRequest() throws IOException { Credentials credentials = Mockito.mock(Credentials.class); Mockito.when(credentials.getRequestMetadata()).thenReturn(null); InstantiatingGrpcChannelProvider.Builder builder = InstantiatingGrpcChannelProvider.newBuilder() .setHeaderProvider(getHeaderProviderWithApiKeyHeader()) .setEndpoint("test.random.com:443"); - InstantiatingGrpcChannelProvider provider = builder.build(); - // calls createChannel - TransportChannel transportChannel = provider.getTransportChannel(); + InstantiatingGrpcChannelProvider provider = builder.build(); - assertNotNull(transportChannel); - transportChannel.shutdownNow(); + assertEquals(1, provider.getProvidedHeadersWithDuplicatesRemoved().size()); + assertEquals( + API_KEY_HEADER_VALUE, + provider.getProvidedHeadersWithDuplicatesRemoved().get(API_KEY_AUTH_HEADER)); } @Test - public void createChannel_handlesErrorRetrievingCredentialsMetadataRequest() throws IOException { + public void buildProvider_handlesErrorRetrievingCredentialsMetadataRequest() throws IOException { Credentials credentials = Mockito.mock(Credentials.class); Mockito.when(credentials.getRequestMetadata()) .thenThrow(new IOException("Error getting request metadata")); @@ -942,17 +937,15 @@ public void createChannel_handlesErrorRetrievingCredentialsMetadataRequest() thr .setEndpoint("test.random.com:443"); InstantiatingGrpcChannelProvider provider = builder.build(); - // calls createChannel - TransportChannel transportChannel = provider.getTransportChannel(); - - assertNotNull(transportChannel); - transportChannel.shutdownNow(); + assertEquals(1, provider.getProvidedHeadersWithDuplicatesRemoved().size()); + assertEquals( + API_KEY_HEADER_VALUE, + provider.getProvidedHeadersWithDuplicatesRemoved().get(API_KEY_AUTH_HEADER)); } private FixedHeaderProvider getHeaderProviderWithApiKeyHeader() { - String apiHeaderKey = "fake_api_key_2"; Map header = new HashMap<>(); - header.put("x-goog-api-key", apiHeaderKey); + header.put("x-goog-api-key", API_KEY_HEADER_VALUE); return FixedHeaderProvider.create(header); } From 703139ba9d43becd3eedae5eff331bb957e18e5d Mon Sep 17 00:00:00 2001 From: detmerl Date: Mon, 30 Sep 2024 16:01:40 -0400 Subject: [PATCH 30/38] additional cleanup --- .../InstantiatingGrpcChannelProvider.java | 16 +++++++-------- .../InstantiatingGrpcChannelProviderTest.java | 20 +++++++++---------- .../google/showcase/v1beta1/it/ITGdch.java | 16 +++++++-------- 3 files changed, 26 insertions(+), 26 deletions(-) diff --git a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java index e0dc095d2c..d0e7b83866 100644 --- a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java +++ b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java @@ -125,7 +125,7 @@ public final class InstantiatingGrpcChannelProvider implements TransportChannelP @Nullable private final Boolean allowNonDefaultServiceAccount; @VisibleForTesting final ImmutableMap directPathServiceConfig; @Nullable private final MtlsProvider mtlsProvider; - private final Map providedHeadersWithDuplicatesRemoved = new HashMap<>(); + private final Map headersWithDuplicatesRemoved = new HashMap<>(); @Nullable private final ApiFunction channelConfigurator; @@ -411,7 +411,7 @@ ChannelCredentials createMtlsChannelCredentials() throws IOException, GeneralSec private ManagedChannel createSingleChannel() throws IOException { GrpcHeaderInterceptor headerInterceptor = - new GrpcHeaderInterceptor(providedHeadersWithDuplicatesRemoved); + new GrpcHeaderInterceptor(headersWithDuplicatesRemoved); GrpcMetadataHandlerInterceptor metadataHandlerInterceptor = new GrpcMetadataHandlerInterceptor(); @@ -501,17 +501,17 @@ private ManagedChannel createSingleChannel() throws IOException { } /* Remove any provided headers that will also get set by credentials. They will be added as part of the grpc call when performing auth - * {@link io.grpc.auth.GoogleAuthLibraryCallCredentials#applyRequestMetadata}. GRPC does not dedup headers {@link https://github.com/grpc/grpc-java/blob/a140e1bb0cfa662bcdb7823d73320eb8d49046f1/api/src/main/java/io/grpc/Metadata.java#L504} so we must before initiating call. + * {@link io.grpc.auth.GoogleAuthLibraryCallCredentials#applyRequestMetadata}. GRPC does not dedup headers {@link https://github.com/grpc/grpc-java/blob/a140e1bb0cfa662bcdb7823d73320eb8d49046f1/api/src/main/java/io/grpc/Metadata.java#L504} so we must before initiating the call. */ private void removeCredentialDuplicateHeaders() { if (headerProvider != null) { - providedHeadersWithDuplicatesRemoved.putAll(headerProvider.getHeaders()); + headersWithDuplicatesRemoved.putAll(headerProvider.getHeaders()); } if (credentials != null) { try { Map> credentialRequestMetatData = credentials.getRequestMetadata(); if (credentialRequestMetatData != null) { - providedHeadersWithDuplicatesRemoved + headersWithDuplicatesRemoved .keySet() .removeAll(credentialRequestMetatData.keySet()); } @@ -589,9 +589,9 @@ public boolean shouldAutoClose() { return true; } - /** @return final headers that will be sent with GRPC call with any duplications removed */ - public Map getProvidedHeadersWithDuplicatesRemoved() { - return providedHeadersWithDuplicatesRemoved; + /** @return list of provided headers that will be sent with GRPC call with any duplicates removed see {@link #removeCredentialDuplicateHeaders()} */ + public Map getHeadersWithDuplicatesRemoved() { + return headersWithDuplicatesRemoved; } public Builder toBuilder() { diff --git a/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java b/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java index 651a87b590..1d9f7c4ea5 100644 --- a/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java +++ b/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java @@ -83,7 +83,7 @@ class InstantiatingGrpcChannelProviderTest extends AbstractMtlsTransportChannelTest { private static final String DEFAULT_ENDPOINT = "test.googleapis.com:443"; private static final String API_KEY_HEADER_VALUE = "fake_api_key_2"; - private static final String API_KEY_AUTH_HEADER = "x-goog-api-key"; + private static final String API_KEY_AUTH_HEADER_KEY = "x-goog-api-key"; private static String originalOSName; private ComputeEngineCredentials computeEngineCredentials; @@ -877,10 +877,10 @@ public void providerInitializedWithNonConflictingHeaders_retainsHeaders() { .setHeaderProvider(getHeaderProviderWithApiKeyHeader()) .setEndpoint("test.random.com:443"); InstantiatingGrpcChannelProvider provider = builder.build(); - assertEquals(1, provider.getProvidedHeadersWithDuplicatesRemoved().size()); + assertEquals(1, provider.getHeadersWithDuplicatesRemoved().size()); assertEquals( API_KEY_HEADER_VALUE, - provider.getProvidedHeadersWithDuplicatesRemoved().get(API_KEY_AUTH_HEADER)); + provider.getHeadersWithDuplicatesRemoved().get(API_KEY_AUTH_HEADER_KEY)); } @Test @@ -893,8 +893,8 @@ public void providersInitializedWithConflictingHeaders_removesDuplicates() throw .setHeaderProvider(getHeaderProviderWithApiKeyHeader()) .setEndpoint("test.random.com:443"); InstantiatingGrpcChannelProvider provider = builder.build(); - assertEquals(0, provider.getProvidedHeadersWithDuplicatesRemoved().size()); - assertNull(provider.getProvidedHeadersWithDuplicatesRemoved().get(API_KEY_AUTH_HEADER)); + assertEquals(0, provider.getHeadersWithDuplicatesRemoved().size()); + assertNull(provider.getHeadersWithDuplicatesRemoved().get(API_KEY_AUTH_HEADER_KEY)); } @Test @@ -906,7 +906,7 @@ public void buildProvider_handlesNullHeaderProvider() { // .setHeaderProvider(headerProvider) .setEndpoint("test.random.com:443"); InstantiatingGrpcChannelProvider provider = builder.build(); - assertEquals(0, provider.getProvidedHeadersWithDuplicatesRemoved().size()); + assertEquals(0, provider.getHeadersWithDuplicatesRemoved().size()); } @Test @@ -920,10 +920,10 @@ public void buildProvider_handlesNullCredentialsMetadataRequest() throws IOExcep InstantiatingGrpcChannelProvider provider = builder.build(); - assertEquals(1, provider.getProvidedHeadersWithDuplicatesRemoved().size()); + assertEquals(1, provider.getHeadersWithDuplicatesRemoved().size()); assertEquals( API_KEY_HEADER_VALUE, - provider.getProvidedHeadersWithDuplicatesRemoved().get(API_KEY_AUTH_HEADER)); + provider.getHeadersWithDuplicatesRemoved().get(API_KEY_AUTH_HEADER_KEY)); } @Test @@ -937,10 +937,10 @@ public void buildProvider_handlesErrorRetrievingCredentialsMetadataRequest() thr .setEndpoint("test.random.com:443"); InstantiatingGrpcChannelProvider provider = builder.build(); - assertEquals(1, provider.getProvidedHeadersWithDuplicatesRemoved().size()); + assertEquals(1, provider.getHeadersWithDuplicatesRemoved().size()); assertEquals( API_KEY_HEADER_VALUE, - provider.getProvidedHeadersWithDuplicatesRemoved().get(API_KEY_AUTH_HEADER)); + provider.getHeadersWithDuplicatesRemoved().get(API_KEY_AUTH_HEADER_KEY)); } private FixedHeaderProvider getHeaderProviderWithApiKeyHeader() { diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITGdch.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITGdch.java index 77703a681b..350f3d02d6 100644 --- a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITGdch.java +++ b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITGdch.java @@ -137,11 +137,11 @@ void testCreateClient_withGdchCredentialAndNoAudience_defaultsToEndpointBasedAud String testEndpoint = "custom-endpoint:123"; settings = settings.toBuilder().setEndpoint(testEndpoint).build(); // need to register the new credentials with updated endpoint before creating - // TransportChannelProvider, as they must be valid to retrieve credential headers (used for + // TransportChannelProvider, as we need to retrieve credential headers (used for // de-duping) - registerCredential( - ClientContext.getGdchCredentials( - null, testEndpoint, settings.getCredentialsProvider().getCredentials())); + Credentials updatedCredentials = ClientContext.getGdchCredentials( + null, testEndpoint, settings.getCredentialsProvider().getCredentials()); + registerCredential(updatedCredentials); context = ClientContext.create(settings); stubSettings = EchoStubSettings.newBuilder(context).build(); client = EchoClient.create(stubSettings.createStub()); @@ -190,11 +190,11 @@ void testCreateClient_withGdchCredentialWithValidAudience_usesCredentialWithPass String endpoint = "localhost:7469"; settings = settings.toBuilder().setGdchApiAudience(testAudience).setEndpoint(endpoint).build(); // need to register the new credentials with updated audience before creating - // TransportChannelProvider, as they must be valid to retrieve credential headers (used for + // TransportChannelProvider, as we need to retrieve credential headers (used for // de-duping) - registerCredential( - ClientContext.getGdchCredentials( - testAudience, endpoint, settings.getCredentialsProvider().getCredentials())); + Credentials updatedCredentials = ClientContext.getGdchCredentials( + testAudience, endpoint, settings.getCredentialsProvider().getCredentials()); + registerCredential(updatedCredentials); context = ClientContext.create(settings); stubSettings = EchoStubSettings.newBuilder(context).build(); client = EchoClient.create(stubSettings.createStub()); From 245f8e28ffadff16eca030ada49b7f9c3ea686dc Mon Sep 17 00:00:00 2001 From: detmerl Date: Mon, 30 Sep 2024 16:04:16 -0400 Subject: [PATCH 31/38] lint --- .../test/java/com/google/showcase/v1beta1/it/ITGdch.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITGdch.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITGdch.java index 350f3d02d6..b67028237e 100644 --- a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITGdch.java +++ b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITGdch.java @@ -139,7 +139,8 @@ void testCreateClient_withGdchCredentialAndNoAudience_defaultsToEndpointBasedAud // need to register the new credentials with updated endpoint before creating // TransportChannelProvider, as we need to retrieve credential headers (used for // de-duping) - Credentials updatedCredentials = ClientContext.getGdchCredentials( + Credentials updatedCredentials = + ClientContext.getGdchCredentials( null, testEndpoint, settings.getCredentialsProvider().getCredentials()); registerCredential(updatedCredentials); context = ClientContext.create(settings); @@ -192,7 +193,8 @@ void testCreateClient_withGdchCredentialWithValidAudience_usesCredentialWithPass // need to register the new credentials with updated audience before creating // TransportChannelProvider, as we need to retrieve credential headers (used for // de-duping) - Credentials updatedCredentials = ClientContext.getGdchCredentials( + Credentials updatedCredentials = + ClientContext.getGdchCredentials( testAudience, endpoint, settings.getCredentialsProvider().getCredentials()); registerCredential(updatedCredentials); context = ClientContext.create(settings); From 0dc642ed477c652dbd090b4736b28f3f3330624d Mon Sep 17 00:00:00 2001 From: detmerl Date: Mon, 30 Sep 2024 16:07:50 -0400 Subject: [PATCH 32/38] lint --- .../api/gax/grpc/InstantiatingGrpcChannelProvider.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java index d0e7b83866..304fb9947b 100644 --- a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java +++ b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java @@ -511,9 +511,7 @@ private void removeCredentialDuplicateHeaders() { try { Map> credentialRequestMetatData = credentials.getRequestMetadata(); if (credentialRequestMetatData != null) { - headersWithDuplicatesRemoved - .keySet() - .removeAll(credentialRequestMetatData.keySet()); + headersWithDuplicatesRemoved.keySet().removeAll(credentialRequestMetatData.keySet()); } } catch (IOException e) { // no-op, if we can't retrieve credentials metadata we will leave headers intact @@ -589,7 +587,10 @@ public boolean shouldAutoClose() { return true; } - /** @return list of provided headers that will be sent with GRPC call with any duplicates removed see {@link #removeCredentialDuplicateHeaders()} */ + /** + * @return list of provided headers that will be sent with GRPC call with any duplicates removed + * see {@link #removeCredentialDuplicateHeaders()} + */ public Map getHeadersWithDuplicatesRemoved() { return headersWithDuplicatesRemoved; } From 60fbed4dfa5e235826dc7fc8eb01b4f9264fce4a Mon Sep 17 00:00:00 2001 From: detmerl Date: Tue, 1 Oct 2024 14:05:38 -0400 Subject: [PATCH 33/38] updated to only dedup API key credential headers --- .../InstantiatingGrpcChannelProvider.java | 21 +++---- .../InstantiatingGrpcChannelProviderTest.java | 56 +++++++++++++------ 2 files changed, 47 insertions(+), 30 deletions(-) diff --git a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java index 304fb9947b..e5d71db6a1 100644 --- a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java +++ b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java @@ -43,6 +43,7 @@ import com.google.api.gax.rpc.TransportChannelProvider; import com.google.api.gax.rpc.internal.EnvironmentProvider; import com.google.api.gax.rpc.mtls.MtlsProvider; +import com.google.auth.ApiKeyCredentials; import com.google.auth.Credentials; import com.google.auth.oauth2.ComputeEngineCredentials; import com.google.common.annotations.VisibleForTesting; @@ -125,7 +126,7 @@ public final class InstantiatingGrpcChannelProvider implements TransportChannelP @Nullable private final Boolean allowNonDefaultServiceAccount; @VisibleForTesting final ImmutableMap directPathServiceConfig; @Nullable private final MtlsProvider mtlsProvider; - private final Map headersWithDuplicatesRemoved = new HashMap<>(); + @VisibleForTesting final Map headersWithDuplicatesRemoved = new HashMap<>(); @Nullable private final ApiFunction channelConfigurator; @@ -500,14 +501,16 @@ private ManagedChannel createSingleChannel() throws IOException { return managedChannel; } - /* Remove any provided headers that will also get set by credentials. They will be added as part of the grpc call when performing auth + /* Remove provided headers that will also get set by {@link com.google.auth.ApiKeyCredentials}. They will be added as part of the grpc call when performing auth * {@link io.grpc.auth.GoogleAuthLibraryCallCredentials#applyRequestMetadata}. GRPC does not dedup headers {@link https://github.com/grpc/grpc-java/blob/a140e1bb0cfa662bcdb7823d73320eb8d49046f1/api/src/main/java/io/grpc/Metadata.java#L504} so we must before initiating the call. + * + * Note: This is specific for ApiKeyCredentials as duplicate API key headers causes a failure on the back end. At this time we are not sure of the behavior for other credentials. */ - private void removeCredentialDuplicateHeaders() { + private void removeApiKeyCredentialDuplicateHeaders() { if (headerProvider != null) { headersWithDuplicatesRemoved.putAll(headerProvider.getHeaders()); } - if (credentials != null) { + if (credentials != null && credentials instanceof ApiKeyCredentials) { try { Map> credentialRequestMetatData = credentials.getRequestMetadata(); if (credentialRequestMetatData != null) { @@ -587,14 +590,6 @@ public boolean shouldAutoClose() { return true; } - /** - * @return list of provided headers that will be sent with GRPC call with any duplicates removed - * see {@link #removeCredentialDuplicateHeaders()} - */ - public Map getHeadersWithDuplicatesRemoved() { - return headersWithDuplicatesRemoved; - } - public Builder toBuilder() { return new Builder(this); } @@ -916,7 +911,7 @@ public Builder setDirectPathServiceConfig(Map serviceConfig) { public InstantiatingGrpcChannelProvider build() { InstantiatingGrpcChannelProvider instantiatingGrpcChannelProvider = new InstantiatingGrpcChannelProvider(this); - instantiatingGrpcChannelProvider.removeCredentialDuplicateHeaders(); + instantiatingGrpcChannelProvider.removeApiKeyCredentialDuplicateHeaders(); return instantiatingGrpcChannelProvider; } diff --git a/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java b/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java index 1d9f7c4ea5..fed99f15c6 100644 --- a/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java +++ b/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java @@ -48,6 +48,7 @@ import com.google.api.gax.rpc.mtls.MtlsProvider; import com.google.auth.ApiKeyCredentials; import com.google.auth.Credentials; +import com.google.auth.http.*; import com.google.auth.oauth2.CloudShellCredentials; import com.google.auth.oauth2.ComputeEngineCredentials; import com.google.common.collect.ImmutableList; @@ -876,15 +877,17 @@ public void providerInitializedWithNonConflictingHeaders_retainsHeaders() { InstantiatingGrpcChannelProvider.newBuilder() .setHeaderProvider(getHeaderProviderWithApiKeyHeader()) .setEndpoint("test.random.com:443"); + InstantiatingGrpcChannelProvider provider = builder.build(); - assertEquals(1, provider.getHeadersWithDuplicatesRemoved().size()); + + assertEquals(1, provider.headersWithDuplicatesRemoved.size()); assertEquals( - API_KEY_HEADER_VALUE, - provider.getHeadersWithDuplicatesRemoved().get(API_KEY_AUTH_HEADER_KEY)); + API_KEY_HEADER_VALUE, provider.headersWithDuplicatesRemoved.get(API_KEY_AUTH_HEADER_KEY)); } @Test - public void providersInitializedWithConflictingHeaders_removesDuplicates() throws IOException { + public void providersInitializedWithConflictingApiKeyCredentialHeaders_removesDuplicates() + throws IOException { String correctApiKey = "fake_api_key"; ApiKeyCredentials apiKeyCredentials = ApiKeyCredentials.create(correctApiKey); InstantiatingGrpcChannelProvider.Builder builder = @@ -892,21 +895,42 @@ public void providersInitializedWithConflictingHeaders_removesDuplicates() throw .setCredentials(apiKeyCredentials) .setHeaderProvider(getHeaderProviderWithApiKeyHeader()) .setEndpoint("test.random.com:443"); + InstantiatingGrpcChannelProvider provider = builder.build(); - assertEquals(0, provider.getHeadersWithDuplicatesRemoved().size()); - assertNull(provider.getHeadersWithDuplicatesRemoved().get(API_KEY_AUTH_HEADER_KEY)); + + assertEquals(0, provider.headersWithDuplicatesRemoved.size()); + assertNull(provider.headersWithDuplicatesRemoved.get(API_KEY_AUTH_HEADER_KEY)); } @Test - public void buildProvider_handlesNullHeaderProvider() { - Map header = new HashMap(); - // FixedHeaderProvider headerProvider = FixedHeaderProvider.create(header); + public void + providersInitializedWithConflictingNonApiKeyCredentialHeaders_doesNotRemoveDuplicates() + throws IOException { + String authProvidedHeader = "Bearer token"; + Map header = new HashMap<>(); + header.put(AuthHttpConstants.AUTHORIZATION, authProvidedHeader); InstantiatingGrpcChannelProvider.Builder builder = InstantiatingGrpcChannelProvider.newBuilder() - // .setHeaderProvider(headerProvider) + .setCredentials(computeEngineCredentials) + .setHeaderProvider(FixedHeaderProvider.create(header)) .setEndpoint("test.random.com:443"); + + InstantiatingGrpcChannelProvider provider = builder.build(); + + assertEquals(1, provider.headersWithDuplicatesRemoved.size()); + assertEquals( + authProvidedHeader, + provider.headersWithDuplicatesRemoved.get(AuthHttpConstants.AUTHORIZATION)); + } + + @Test + public void buildProvider_handlesNullHeaderProvider() { + InstantiatingGrpcChannelProvider.Builder builder = + InstantiatingGrpcChannelProvider.newBuilder().setEndpoint("test.random.com:443"); + InstantiatingGrpcChannelProvider provider = builder.build(); - assertEquals(0, provider.getHeadersWithDuplicatesRemoved().size()); + + assertEquals(0, provider.headersWithDuplicatesRemoved.size()); } @Test @@ -920,10 +944,9 @@ public void buildProvider_handlesNullCredentialsMetadataRequest() throws IOExcep InstantiatingGrpcChannelProvider provider = builder.build(); - assertEquals(1, provider.getHeadersWithDuplicatesRemoved().size()); + assertEquals(1, provider.headersWithDuplicatesRemoved.size()); assertEquals( - API_KEY_HEADER_VALUE, - provider.getHeadersWithDuplicatesRemoved().get(API_KEY_AUTH_HEADER_KEY)); + API_KEY_HEADER_VALUE, provider.headersWithDuplicatesRemoved.get(API_KEY_AUTH_HEADER_KEY)); } @Test @@ -937,10 +960,9 @@ public void buildProvider_handlesErrorRetrievingCredentialsMetadataRequest() thr .setEndpoint("test.random.com:443"); InstantiatingGrpcChannelProvider provider = builder.build(); - assertEquals(1, provider.getHeadersWithDuplicatesRemoved().size()); + assertEquals(1, provider.headersWithDuplicatesRemoved.size()); assertEquals( - API_KEY_HEADER_VALUE, - provider.getHeadersWithDuplicatesRemoved().get(API_KEY_AUTH_HEADER_KEY)); + API_KEY_HEADER_VALUE, provider.headersWithDuplicatesRemoved.get(API_KEY_AUTH_HEADER_KEY)); } private FixedHeaderProvider getHeaderProviderWithApiKeyHeader() { From 68f38aee3b3e66e19cb24ce2ef2ed8f969c791c7 Mon Sep 17 00:00:00 2001 From: detmerl Date: Tue, 1 Oct 2024 14:15:04 -0400 Subject: [PATCH 34/38] language fixes --- .../google/api/gax/grpc/InstantiatingGrpcChannelProvider.java | 2 +- .../api/gax/grpc/InstantiatingGrpcChannelProviderTest.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java index e5d71db6a1..3276346c6c 100644 --- a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java +++ b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java @@ -517,7 +517,7 @@ private void removeApiKeyCredentialDuplicateHeaders() { headersWithDuplicatesRemoved.keySet().removeAll(credentialRequestMetatData.keySet()); } } catch (IOException e) { - // no-op, if we can't retrieve credentials metadata we will leave headers intact + // unreachable, there is no scenario that getRequestMetatData for ApiKeyCredentials will throw an IOException } } } diff --git a/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java b/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java index fed99f15c6..c634aa0f11 100644 --- a/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java +++ b/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java @@ -48,7 +48,7 @@ import com.google.api.gax.rpc.mtls.MtlsProvider; import com.google.auth.ApiKeyCredentials; import com.google.auth.Credentials; -import com.google.auth.http.*; +import com.google.auth.http.AuthHttpConstants; import com.google.auth.oauth2.CloudShellCredentials; import com.google.auth.oauth2.ComputeEngineCredentials; import com.google.common.collect.ImmutableList; @@ -967,7 +967,7 @@ public void buildProvider_handlesErrorRetrievingCredentialsMetadataRequest() thr private FixedHeaderProvider getHeaderProviderWithApiKeyHeader() { Map header = new HashMap<>(); - header.put("x-goog-api-key", API_KEY_HEADER_VALUE); + header.put(API_KEY_AUTH_HEADER_KEY, API_KEY_HEADER_VALUE); return FixedHeaderProvider.create(header); } From d3492b3c896ec8a17462d1db261c682cd9d6f182 Mon Sep 17 00:00:00 2001 From: detmerl Date: Tue, 1 Oct 2024 14:15:24 -0400 Subject: [PATCH 35/38] lint --- .../google/api/gax/grpc/InstantiatingGrpcChannelProvider.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java index 3276346c6c..ae4d7f9e51 100644 --- a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java +++ b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java @@ -517,7 +517,8 @@ private void removeApiKeyCredentialDuplicateHeaders() { headersWithDuplicatesRemoved.keySet().removeAll(credentialRequestMetatData.keySet()); } } catch (IOException e) { - // unreachable, there is no scenario that getRequestMetatData for ApiKeyCredentials will throw an IOException + // unreachable, there is no scenario that getRequestMetatData for ApiKeyCredentials will + // throw an IOException } } } From 1330841a876f7616cb50a4f9842f909983fe5c1c Mon Sep 17 00:00:00 2001 From: detmerl Date: Tue, 1 Oct 2024 14:20:38 -0400 Subject: [PATCH 36/38] fixed changes to existing tests --- .../InstantiatingGrpcChannelProviderTest.java | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java b/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java index c634aa0f11..75ed58f729 100644 --- a/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java +++ b/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java @@ -712,7 +712,7 @@ void testLogDirectPathMisconfigNotOnGCE() throws Exception { } @Test - public void canUseDirectPath_happyPath() { + public void canUseDirectPath_happyPath() throws IOException { System.setProperty("os.name", "Linux"); EnvironmentProvider envProvider = Mockito.mock(EnvironmentProvider.class); Mockito.when( @@ -724,14 +724,20 @@ public void canUseDirectPath_happyPath() { .setAttemptDirectPath(true) .setCredentials(computeEngineCredentials) .setEndpoint(DEFAULT_ENDPOINT) - .setEnvProvider(envProvider); + .setEnvProvider(envProvider) + .setHeaderProvider(Mockito.mock(HeaderProvider.class)); InstantiatingGrpcChannelProvider provider = new InstantiatingGrpcChannelProvider(builder, GCE_PRODUCTION_NAME_AFTER_2016); Truth.assertThat(provider.canUseDirectPath()).isTrue(); + + // verify this info is passed correctly to transport channel + TransportChannel transportChannel = provider.getTransportChannel(); + Truth.assertThat(((GrpcTransportChannel) transportChannel).isDirectPath()).isTrue(); + transportChannel.shutdownNow(); } @Test - public void canUseDirectPath_directPathEnvVarDisabled() { + public void canUseDirectPath_directPathEnvVarDisabled() throws IOException { System.setProperty("os.name", "Linux"); EnvironmentProvider envProvider = Mockito.mock(EnvironmentProvider.class); Mockito.when( @@ -743,10 +749,16 @@ public void canUseDirectPath_directPathEnvVarDisabled() { .setAttemptDirectPath(true) .setCredentials(computeEngineCredentials) .setEndpoint(DEFAULT_ENDPOINT) - .setEnvProvider(envProvider); + .setEnvProvider(envProvider) + .setHeaderProvider(Mockito.mock(HeaderProvider.class)); InstantiatingGrpcChannelProvider provider = new InstantiatingGrpcChannelProvider(builder, GCE_PRODUCTION_NAME_AFTER_2016); Truth.assertThat(provider.canUseDirectPath()).isFalse(); + + // verify this info is passed correctly to transport channel + TransportChannel transportChannel = provider.getTransportChannel(); + Truth.assertThat(((GrpcTransportChannel) transportChannel).isDirectPath()).isFalse(); + transportChannel.shutdownNow(); } @Test From 768140daab23a071371362790da676e5ebd2e659 Mon Sep 17 00:00:00 2001 From: detmerl Date: Wed, 2 Oct 2024 09:11:20 -0400 Subject: [PATCH 37/38] fixed test modifiers --- .../InstantiatingGrpcChannelProviderTest.java | 15 ++++++--------- .../com/google/api/gax/rpc/ClientContext.java | 2 +- .../com/google/api/gax/rpc/ClientContextTest.java | 4 ++-- 3 files changed, 9 insertions(+), 12 deletions(-) diff --git a/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java b/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java index 75ed58f729..a58f9b8173 100644 --- a/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java +++ b/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java @@ -884,7 +884,7 @@ public void canUseDirectPath_nonGDUUniverseDomain() { } @Test - public void providerInitializedWithNonConflictingHeaders_retainsHeaders() { + void providerInitializedWithNonConflictingHeaders_retainsHeaders() { InstantiatingGrpcChannelProvider.Builder builder = InstantiatingGrpcChannelProvider.newBuilder() .setHeaderProvider(getHeaderProviderWithApiKeyHeader()) @@ -898,8 +898,7 @@ public void providerInitializedWithNonConflictingHeaders_retainsHeaders() { } @Test - public void providersInitializedWithConflictingApiKeyCredentialHeaders_removesDuplicates() - throws IOException { + void providersInitializedWithConflictingApiKeyCredentialHeaders_removesDuplicates() { String correctApiKey = "fake_api_key"; ApiKeyCredentials apiKeyCredentials = ApiKeyCredentials.create(correctApiKey); InstantiatingGrpcChannelProvider.Builder builder = @@ -915,9 +914,7 @@ public void providersInitializedWithConflictingApiKeyCredentialHeaders_removesDu } @Test - public void - providersInitializedWithConflictingNonApiKeyCredentialHeaders_doesNotRemoveDuplicates() - throws IOException { + void providersInitializedWithConflictingNonApiKeyCredentialHeaders_doesNotRemoveDuplicates() { String authProvidedHeader = "Bearer token"; Map header = new HashMap<>(); header.put(AuthHttpConstants.AUTHORIZATION, authProvidedHeader); @@ -936,7 +933,7 @@ public void providersInitializedWithConflictingApiKeyCredentialHeaders_removesDu } @Test - public void buildProvider_handlesNullHeaderProvider() { + void buildProvider_handlesNullHeaderProvider() { InstantiatingGrpcChannelProvider.Builder builder = InstantiatingGrpcChannelProvider.newBuilder().setEndpoint("test.random.com:443"); @@ -946,7 +943,7 @@ public void buildProvider_handlesNullHeaderProvider() { } @Test - public void buildProvider_handlesNullCredentialsMetadataRequest() throws IOException { + void buildProvider_handlesNullCredentialsMetadataRequest() throws IOException { Credentials credentials = Mockito.mock(Credentials.class); Mockito.when(credentials.getRequestMetadata()).thenReturn(null); InstantiatingGrpcChannelProvider.Builder builder = @@ -962,7 +959,7 @@ public void buildProvider_handlesNullCredentialsMetadataRequest() throws IOExcep } @Test - public void buildProvider_handlesErrorRetrievingCredentialsMetadataRequest() throws IOException { + void buildProvider_handlesErrorRetrievingCredentialsMetadataRequest() throws IOException { Credentials credentials = Mockito.mock(Credentials.class); Mockito.when(credentials.getRequestMetadata()) .thenThrow(new IOException("Error getting request metadata")); diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java index ee0bc36b93..0e73bb38ef 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java @@ -295,7 +295,7 @@ private static Credentials getCredentials(StubSettings settings) throws IOExcept * a GDC-H audience */ @VisibleForTesting - public static GdchCredentials getGdchCredentials( + static GdchCredentials getGdchCredentials( String settingsGdchApiAudience, String endpoint, Credentials credentials) throws IOException { String audienceString; if (!Strings.isNullOrEmpty(settingsGdchApiAudience)) { diff --git a/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java b/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java index 95956e5963..c4d2027632 100644 --- a/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java +++ b/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java @@ -1074,7 +1074,7 @@ public void testStreamWatchdogInterval_backportMethodsBehaveCorrectly() { } @Test - public void testSetApiKey_createsApiCredentials() throws IOException { + void testSetApiKey_createsApiCredentials() throws IOException { String apiKey = "key"; FakeStubSettings.Builder builder = new FakeStubSettings.Builder(); InterceptingExecutor executor = new InterceptingExecutor(1); @@ -1095,7 +1095,7 @@ public void testSetApiKey_createsApiCredentials() throws IOException { } @Test - public void testSetApiKey_withDefaultCredentials_overridesCredentials() throws IOException { + void testSetApiKey_withDefaultCredentials_overridesCredentials() throws IOException { String apiKey = "key"; FakeStubSettings.Builder builder = FakeStubSettings.newBuilder(); InterceptingExecutor executor = new InterceptingExecutor(1); From e20771d4ef51bb48d5b7ba75c4422027f1cd5bff Mon Sep 17 00:00:00 2001 From: detmerl Date: Wed, 2 Oct 2024 09:20:55 -0400 Subject: [PATCH 38/38] no longer need to pre-load gdch creds as we're not deduping headers for them --- .../google/showcase/v1beta1/it/ITGdch.java | 20 ++++--------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITGdch.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITGdch.java index b67028237e..f043d45cfc 100644 --- a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITGdch.java +++ b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITGdch.java @@ -136,13 +136,6 @@ void testCreateClient_withGdchCredentialAndNoAudience_defaultsToEndpointBasedAud // we create the client as usual - no audience passed String testEndpoint = "custom-endpoint:123"; settings = settings.toBuilder().setEndpoint(testEndpoint).build(); - // need to register the new credentials with updated endpoint before creating - // TransportChannelProvider, as we need to retrieve credential headers (used for - // de-duping) - Credentials updatedCredentials = - ClientContext.getGdchCredentials( - null, testEndpoint, settings.getCredentialsProvider().getCredentials()); - registerCredential(updatedCredentials); context = ClientContext.create(settings); stubSettings = EchoStubSettings.newBuilder(context).build(); client = EchoClient.create(stubSettings.createStub()); @@ -168,6 +161,7 @@ void testCreateClient_withGdchCredentialAndNoAudience_defaultsToEndpointBasedAud // However, the credentials prepared in ClientContext should be able to refresh since the // audience would be // internally defaulted the endpoint of the StubSettings + registerCredential(fromContext); ((GdchCredentials) fromContext).refreshAccessToken(); String usedAudience = transportFactory.transport.getLastAudienceSent(); assertEquals(testEndpoint, usedAudience); @@ -188,15 +182,8 @@ void testCreateClient_withGdchCredentialWithValidAudience_usesCredentialWithPass // audience. It should // be created without issues String testAudience = "valid-audience"; - String endpoint = "localhost:7469"; - settings = settings.toBuilder().setGdchApiAudience(testAudience).setEndpoint(endpoint).build(); - // need to register the new credentials with updated audience before creating - // TransportChannelProvider, as we need to retrieve credential headers (used for - // de-duping) - Credentials updatedCredentials = - ClientContext.getGdchCredentials( - testAudience, endpoint, settings.getCredentialsProvider().getCredentials()); - registerCredential(updatedCredentials); + settings = + settings.toBuilder().setGdchApiAudience(testAudience).setEndpoint("localhost:7469").build(); context = ClientContext.create(settings); stubSettings = EchoStubSettings.newBuilder(context).build(); client = EchoClient.create(stubSettings.createStub()); @@ -219,6 +206,7 @@ void testCreateClient_withGdchCredentialWithValidAudience_usesCredentialWithPass // But the credentials prepared in ClientContext should be able to refresh since the audience // would be internally // set to the one passed in stub settings + registerCredential(fromContext); ((GdchCredentials) fromContext).refreshAccessToken(); String usedAudience = transportFactory.transport.getLastAudienceSent(); assertEquals(testAudience, usedAudience);