diff --git a/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java b/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java index 776f2569f..fe6ccb40e 100644 --- a/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java +++ b/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java @@ -135,6 +135,11 @@ private InstantiatingGrpcChannelProvider(Builder builder) { : builder.directPathServiceConfig; } + /** + * @deprecated If executor is not set, this channel provider will create channels with default + * grpc executor. + */ + @Deprecated @Override public boolean needsExecutor() { return executor == null; @@ -212,9 +217,7 @@ public TransportChannelProvider withCredentials(Credentials credentials) { @Override public TransportChannel getTransportChannel() throws IOException { - if (needsExecutor()) { - throw new IllegalStateException("getTransportChannel() called when needsExecutor() is true"); - } else if (needsHeaders()) { + if (needsHeaders()) { throw new IllegalStateException("getTransportChannel() called when needsHeaders() is true"); } else if (needsEndpoint()) { throw new IllegalStateException("getTransportChannel() called when needsEndpoint() is true"); diff --git a/gax-grpc/src/test/java/com/google/api/gax/grpc/testing/LocalChannelProvider.java b/gax-grpc/src/test/java/com/google/api/gax/grpc/testing/LocalChannelProvider.java index 00ba7a5f1..5e538a06c 100644 --- a/gax-grpc/src/test/java/com/google/api/gax/grpc/testing/LocalChannelProvider.java +++ b/gax-grpc/src/test/java/com/google/api/gax/grpc/testing/LocalChannelProvider.java @@ -74,6 +74,7 @@ public boolean shouldAutoClose() { return true; } + @Deprecated @Override public boolean needsExecutor() { return false; diff --git a/gax-httpjson/src/main/java/com/google/api/gax/httpjson/InstantiatingHttpJsonChannelProvider.java b/gax-httpjson/src/main/java/com/google/api/gax/httpjson/InstantiatingHttpJsonChannelProvider.java index b6da11a39..2e4ff935b 100644 --- a/gax-httpjson/src/main/java/com/google/api/gax/httpjson/InstantiatingHttpJsonChannelProvider.java +++ b/gax-httpjson/src/main/java/com/google/api/gax/httpjson/InstantiatingHttpJsonChannelProvider.java @@ -93,6 +93,11 @@ private InstantiatingHttpJsonChannelProvider( this.mtlsProvider = mtlsProvider; } + /** + * @deprecated If executor is not set, this channel provider will create channels with default + * executor defined in {@link ManagedHttpJsonChannel}. + */ + @Deprecated @Override public boolean needsExecutor() { return executor == null; @@ -149,9 +154,7 @@ public String getTransportName() { @Override public TransportChannel getTransportChannel() throws IOException { - if (needsExecutor()) { - throw new IllegalStateException("getTransportChannel() called when needsExecutor() is true"); - } else if (needsHeaders()) { + if (needsHeaders()) { throw new IllegalStateException("getTransportChannel() called when needsHeaders() is true"); } else { try { diff --git a/gax-httpjson/src/main/java/com/google/api/gax/httpjson/ManagedHttpJsonChannel.java b/gax-httpjson/src/main/java/com/google/api/gax/httpjson/ManagedHttpJsonChannel.java index df8afd391..af3e9bbd7 100644 --- a/gax-httpjson/src/main/java/com/google/api/gax/httpjson/ManagedHttpJsonChannel.java +++ b/gax-httpjson/src/main/java/com/google/api/gax/httpjson/ManagedHttpJsonChannel.java @@ -37,12 +37,14 @@ import com.google.api.core.BetaApi; import com.google.api.core.SettableApiFuture; import com.google.api.gax.core.BackgroundResource; +import com.google.api.gax.core.InstantiatingExecutorProvider; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import java.io.IOException; import java.util.LinkedList; import java.util.List; import java.util.concurrent.Executor; +import java.util.concurrent.ExecutorService; import java.util.concurrent.TimeUnit; import javax.annotation.Nullable; @@ -50,6 +52,8 @@ @BetaApi public class ManagedHttpJsonChannel implements HttpJsonChannel, BackgroundResource { private static final JsonFactory JSON_FACTORY = GsonFactory.getDefaultInstance(); + private static final ExecutorService DEFAULT_EXECUTOR = + InstantiatingExecutorProvider.newBuilder().build().getExecutor(); private final Executor executor; private final String endpoint; @@ -134,7 +138,9 @@ public boolean awaitTermination(long duration, TimeUnit unit) throws Interrupted public void close() {} public static Builder newBuilder() { - return new Builder().setHeaderEnhancers(new LinkedList()); + return new Builder() + .setHeaderEnhancers(new LinkedList()) + .setExecutor(DEFAULT_EXECUTOR); } public static class Builder { @@ -147,7 +153,7 @@ public static class Builder { private Builder() {} public Builder setExecutor(Executor executor) { - this.executor = executor; + this.executor = Preconditions.checkNotNull(executor); return this; } @@ -167,7 +173,6 @@ public Builder setHttpTransport(HttpTransport httpTransport) { } public ManagedHttpJsonChannel build() { - Preconditions.checkNotNull(executor); Preconditions.checkNotNull(endpoint); return new ManagedHttpJsonChannel( executor, endpoint, jsonFactory, headerEnhancers, httpTransport); diff --git a/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java b/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java index cd50f2fb8..ff18e77d3 100644 --- a/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java +++ b/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java @@ -50,7 +50,6 @@ import java.util.List; import java.util.Map; import java.util.Set; -import java.util.concurrent.Executor; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import javax.annotation.Nonnull; @@ -73,6 +72,10 @@ public abstract class ClientContext { */ public abstract List getBackgroundResources(); + /** + * Gets the executor to use for running scheduled API call logic (such as retries and long-running + * operations). + */ public abstract ScheduledExecutorService getExecutor(); @Nullable @@ -163,8 +166,8 @@ static String getEndpoint( public static ClientContext create(StubSettings settings) throws IOException { ApiClock clock = settings.getClock(); - ExecutorProvider executorProvider = settings.getExecutorProvider(); - final ScheduledExecutorService executor = executorProvider.getExecutor(); + ExecutorProvider backgroundExecutorProvider = settings.getBackgroundExecutorProvider(); + final ScheduledExecutorService backgroundExecutor = backgroundExecutorProvider.getExecutor(); Credentials credentials = settings.getCredentialsProvider().getCredentials(); @@ -177,8 +180,11 @@ public static ClientContext create(StubSettings settings) throws IOException { } TransportChannelProvider transportChannelProvider = settings.getTransportChannelProvider(); - if (transportChannelProvider.needsExecutor()) { - transportChannelProvider = transportChannelProvider.withExecutor((Executor) executor); + // After needsExecutor and StubSettings#setExecutorProvider are deprecated, transport channel + // executor can only be set from TransportChannelProvider#withExecutor directly, and a provider + // will have a default executor if it needs one. + if (transportChannelProvider.needsExecutor() && settings.getExecutorProvider() != null) { + transportChannelProvider = transportChannelProvider.withExecutor(backgroundExecutor); } Map headers = getHeadersFromSettings(settings); if (transportChannelProvider.needsHeaders()) { @@ -216,7 +222,7 @@ public static ClientContext create(StubSettings settings) throws IOException { watchdogProvider = watchdogProvider.withClock(clock); } if (watchdogProvider.needsExecutor()) { - watchdogProvider = watchdogProvider.withExecutor(executor); + watchdogProvider = watchdogProvider.withExecutor(backgroundExecutor); } watchdog = watchdogProvider.getWatchdog(); } @@ -226,8 +232,8 @@ public static ClientContext create(StubSettings settings) throws IOException { if (transportChannelProvider.shouldAutoClose()) { backgroundResources.add(transportChannel); } - if (executorProvider.shouldAutoClose()) { - backgroundResources.add(new ExecutorAsBackgroundResource(executor)); + if (backgroundExecutorProvider.shouldAutoClose()) { + backgroundResources.add(new ExecutorAsBackgroundResource(backgroundExecutor)); } if (watchdogProvider != null && watchdogProvider.shouldAutoClose()) { backgroundResources.add(watchdog); @@ -235,7 +241,7 @@ public static ClientContext create(StubSettings settings) throws IOException { return newBuilder() .setBackgroundResources(backgroundResources.build()) - .setExecutor(executor) + .setExecutor(backgroundExecutor) .setCredentials(credentials) .setTransportChannel(transportChannel) .setHeaders(ImmutableMap.copyOf(settings.getHeaderProvider().getHeaders())) @@ -289,6 +295,10 @@ public abstract static class Builder { public abstract Builder setBackgroundResources(List backgroundResources); + /** + * Sets the executor to use for running scheduled API call logic (such as retries and + * long-running operations). + */ public abstract Builder setExecutor(ScheduledExecutorService value); public abstract Builder setCredentials(Credentials value); diff --git a/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java b/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java index 7575fea2b..bd4891ccc 100644 --- a/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java +++ b/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java @@ -36,6 +36,7 @@ import com.google.api.gax.core.ExecutorProvider; import com.google.common.base.MoreObjects; import java.io.IOException; +import java.util.concurrent.Executor; import javax.annotation.Nonnull; import javax.annotation.Nullable; import org.threeten.bp.Duration; @@ -63,10 +64,16 @@ public final StubSettings getStubSettings() { return stubSettings; } + /** @deprecated Please use {@link #getBackgroundExecutorProvider()} */ + @Deprecated public final ExecutorProvider getExecutorProvider() { return stubSettings.getExecutorProvider(); } + public final ExecutorProvider getBackgroundExecutorProvider() { + return stubSettings.getBackgroundExecutorProvider(); + } + public final TransportChannelProvider getTransportChannelProvider() { return stubSettings.getTransportChannelProvider(); } @@ -112,6 +119,7 @@ public final Duration getWatchdogCheckInterval() { public String toString() { return MoreObjects.toStringHelper(this) .add("executorProvider", getExecutorProvider()) + .add("backgroundExecutorProvider", getBackgroundExecutorProvider()) .add("transportChannelProvider", getTransportChannelProvider()) .add("credentialsProvider", getCredentialsProvider()) .add("headerProvider", getHeaderProvider()) @@ -159,9 +167,27 @@ protected StubSettings.Builder getStubSettings() { * call logic (such as retries and long-running operations), and also to pass to the transport * settings if an executor is needed for the transport and it doesn't have its own executor * provider. + * + * @deprecated Please use {@link #setBackgroundExecutorProvider(ExecutorProvider)} for setting + * executor to use for running scheduled API call logic. To set executor for {@link + * TransportChannelProvider}, please use {@link + * TransportChannelProvider#withExecutor(Executor)} instead. */ + @Deprecated public B setExecutorProvider(ExecutorProvider executorProvider) { stubSettings.setExecutorProvider(executorProvider); + stubSettings.setBackgroundExecutorProvider(executorProvider); + return self(); + } + + /** + * Sets the ExecutorProvider to use for getting the executor to use for running scheduled API + * call logic (such as retries and long-running operations). This will not set the executor in + * {@link TransportChannelProvider}. To set executor for {@link TransportChannelProvider}, + * please use {@link TransportChannelProvider#withExecutor(Executor)}. + */ + public B setBackgroundExecutorProvider(ExecutorProvider executorProvider) { + stubSettings.setBackgroundExecutorProvider(executorProvider); return self(); } @@ -238,11 +264,29 @@ public B setWatchdogCheckInterval(@Nullable Duration checkInterval) { return self(); } - /** Gets the ExecutorProvider that was previously set on this Builder. */ + /** + * 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), + * and also to pass to the transport settings if an executor is needed for the transport and it + * doesn't have its own executor provider. + * + * @deprecated Please use {@link #getBackgroundExecutorProvider()} for getting the executor + * provider that's used for running scheduled API call logic. + */ + @Deprecated public ExecutorProvider getExecutorProvider() { return stubSettings.getExecutorProvider(); } + /** + * 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). + * This ExecutorProvider is not used to set the executor in {@link TransportChannelProvider}. + */ + public ExecutorProvider getBackgroundExecutorProvider() { + return stubSettings.getBackgroundExecutorProvider(); + } + /** Gets the TransportProvider that was previously set on this Builder. */ public TransportChannelProvider getTransportChannelProvider() { return stubSettings.getTransportChannelProvider(); @@ -303,6 +347,7 @@ protected static void applyToAllUnaryMethods( public String toString() { return MoreObjects.toStringHelper(this) .add("executorProvider", getExecutorProvider()) + .add("backgroundExecutorProvider", getBackgroundExecutorProvider()) .add("transportChannelProvider", getTransportChannelProvider()) .add("credentialsProvider", getCredentialsProvider()) .add("headerProvider", getHeaderProvider()) diff --git a/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java b/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java index 0dc88c36c..825aa9d7a 100644 --- a/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java +++ b/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java @@ -45,6 +45,7 @@ import com.google.common.base.MoreObjects; import com.google.common.base.Preconditions; import java.io.IOException; +import java.util.concurrent.Executor; import javax.annotation.Nonnull; import javax.annotation.Nullable; import org.threeten.bp.Duration; @@ -63,7 +64,7 @@ public abstract class StubSettings> { static final String QUOTA_PROJECT_ID_HEADER_KEY = "x-goog-user-project"; - private final ExecutorProvider executorProvider; + private final ExecutorProvider backgroundExecutorProvider; private final CredentialsProvider credentialsProvider; private final HeaderProvider headerProvider; private final HeaderProvider internalHeaderProvider; @@ -75,6 +76,8 @@ public abstract class StubSettings> { @Nullable private final WatchdogProvider streamWatchdogProvider; @Nonnull private final Duration streamWatchdogCheckInterval; @Nonnull private final ApiTracerFactory tracerFactory; + // Track if deprecated setExecutorProvider is called + private boolean deprecatedExecutorProviderSet; /** * Indicate when creating transport whether it is allowed to use mTLS endpoint instead of the @@ -86,7 +89,7 @@ public abstract class StubSettings> { /** Constructs an instance of StubSettings. */ protected StubSettings(Builder builder) { - this.executorProvider = builder.executorProvider; + this.backgroundExecutorProvider = builder.backgroundExecutorProvider; this.transportChannelProvider = builder.transportChannelProvider; this.credentialsProvider = builder.credentialsProvider; this.headerProvider = builder.headerProvider; @@ -99,10 +102,17 @@ protected StubSettings(Builder builder) { this.streamWatchdogProvider = builder.streamWatchdogProvider; this.streamWatchdogCheckInterval = builder.streamWatchdogCheckInterval; this.tracerFactory = builder.tracerFactory; + this.deprecatedExecutorProviderSet = builder.deprecatedExecutorProviderSet; } + /** @deprecated Please use {@link #getBackgroundExecutorProvider()}. */ + @Deprecated public final ExecutorProvider getExecutorProvider() { - return executorProvider; + return deprecatedExecutorProviderSet ? backgroundExecutorProvider : null; + } + + public final ExecutorProvider getBackgroundExecutorProvider() { + return backgroundExecutorProvider; } public final TransportChannelProvider getTransportChannelProvider() { @@ -168,7 +178,7 @@ public ApiTracerFactory getTracerFactory() { public String toString() { return MoreObjects.toStringHelper(this) - .add("executorProvider", executorProvider) + .add("backgroundExecutorProvider", backgroundExecutorProvider) .add("transportChannelProvider", transportChannelProvider) .add("credentialsProvider", credentialsProvider) .add("headerProvider", headerProvider) @@ -189,7 +199,7 @@ public String toString() { public abstract static class Builder< SettingsT extends StubSettings, B extends Builder> { - private ExecutorProvider executorProvider; + private ExecutorProvider backgroundExecutorProvider; private CredentialsProvider credentialsProvider; private HeaderProvider headerProvider; private HeaderProvider internalHeaderProvider; @@ -201,6 +211,7 @@ public abstract static class Builder< @Nullable private WatchdogProvider streamWatchdogProvider; @Nonnull private Duration streamWatchdogCheckInterval; @Nonnull private ApiTracerFactory tracerFactory; + private boolean deprecatedExecutorProviderSet; /** * Indicate when creating transport whether it is allowed to use mTLS endpoint instead of the @@ -212,7 +223,7 @@ public abstract static class Builder< /** Create a builder from a StubSettings object. */ protected Builder(StubSettings settings) { - this.executorProvider = settings.executorProvider; + this.backgroundExecutorProvider = settings.backgroundExecutorProvider; this.transportChannelProvider = settings.transportChannelProvider; this.credentialsProvider = settings.credentialsProvider; this.headerProvider = settings.headerProvider; @@ -225,6 +236,7 @@ protected Builder(StubSettings settings) { this.streamWatchdogProvider = settings.streamWatchdogProvider; this.streamWatchdogCheckInterval = settings.streamWatchdogCheckInterval; this.tracerFactory = settings.tracerFactory; + this.deprecatedExecutorProviderSet = settings.deprecatedExecutorProviderSet; } /** Get Quota Project ID from Client Context * */ @@ -246,7 +258,7 @@ private static String getQuotaProjectIdFromClientContext(ClientContext clientCon protected Builder(ClientContext clientContext) { if (clientContext == null) { - this.executorProvider = InstantiatingExecutorProvider.newBuilder().build(); + this.backgroundExecutorProvider = InstantiatingExecutorProvider.newBuilder().build(); this.transportChannelProvider = null; this.credentialsProvider = NoCredentialsProvider.create(); this.headerProvider = new NoHeaderProvider(); @@ -258,8 +270,12 @@ protected Builder(ClientContext clientContext) { this.streamWatchdogProvider = InstantiatingWatchdogProvider.create(); this.streamWatchdogCheckInterval = Duration.ofSeconds(10); this.tracerFactory = BaseApiTracerFactory.getInstance(); + this.deprecatedExecutorProviderSet = false; } else { - this.executorProvider = FixedExecutorProvider.create(clientContext.getExecutor()); + ExecutorProvider fixedExecutorProvider = + FixedExecutorProvider.create(clientContext.getExecutor()); + this.deprecatedExecutorProviderSet = true; + this.backgroundExecutorProvider = fixedExecutorProvider; this.transportChannelProvider = FixedTransportChannelProvider.create(clientContext.getTransportChannel()); this.credentialsProvider = FixedCredentialsProvider.create(clientContext.getCredentials()); @@ -293,9 +309,31 @@ protected B self() { * call logic (such as retries and long-running operations), and also to pass to the transport * settings if an executor is needed for the transport and it doesn't have its own executor * provider. + * + * @deprecated Please use {@link #setBackgroundExecutorProvider(ExecutorProvider)} for setting + * executor to use for running scheduled API call logic. To set executor for {@link + * TransportChannelProvider}, please use {@link + * TransportChannelProvider#withExecutor(Executor)} instead. */ + @Deprecated public B setExecutorProvider(ExecutorProvider executorProvider) { - this.executorProvider = executorProvider; + // For backward compatibility, this will set backgroundExecutorProvider and mark + // deprecatedExecutorProviderSet to true. In ClientContext#create(), if + // TransportChannelProvider doesn't have an executor, and deprecatedExecutorProviderSet is + // true, backgroundExecutorProvider will be used as TransportChannelProvider's executor. + // After this method is deprecated, TransportChannelProvider's executor can only be set with + // TransportChannelProvider#withExecutor. + this.deprecatedExecutorProviderSet = true; + this.backgroundExecutorProvider = executorProvider; + return self(); + } + + /** + * Sets the executor to use for running scheduled API call logic (such as retries and + * long-running operations). + */ + public B setBackgroundExecutorProvider(ExecutorProvider backgroundExecutorProvider) { + this.backgroundExecutorProvider = backgroundExecutorProvider; return self(); } @@ -416,9 +454,15 @@ public B setTracerFactory(@Nonnull ApiTracerFactory tracerFactory) { return self(); } - /** Gets the ExecutorProvider that was previously set on this Builder. */ + /** @deprecated Please use {@link #getBackgroundExecutorProvider()}. */ + @Deprecated public ExecutorProvider getExecutorProvider() { - return executorProvider; + return deprecatedExecutorProviderSet ? backgroundExecutorProvider : null; + } + + /** Gets the ExecutorProvider that was previously set on this Builder. */ + public ExecutorProvider getBackgroundExecutorProvider() { + return backgroundExecutorProvider; } /** Gets the TransportProvider that was previously set on this Builder. */ @@ -493,7 +537,7 @@ protected static void applyToAllUnaryMethods( public String toString() { return MoreObjects.toStringHelper(this) - .add("executorProvider", executorProvider) + .add("backgroundExecutorProvider", backgroundExecutorProvider) .add("transportChannelProvider", transportChannelProvider) .add("credentialsProvider", credentialsProvider) .add("headerProvider", headerProvider) diff --git a/gax/src/main/java/com/google/api/gax/rpc/TransportChannelProvider.java b/gax/src/main/java/com/google/api/gax/rpc/TransportChannelProvider.java index 3cfc9349e..160adf02f 100644 --- a/gax/src/main/java/com/google/api/gax/rpc/TransportChannelProvider.java +++ b/gax/src/main/java/com/google/api/gax/rpc/TransportChannelProvider.java @@ -49,12 +49,11 @@ * * 

  * TransportChannelProvider transportChannelProvider = ...;
- * if (transportChannelProvider.needsExecutor()) {
- *   transportChannelProvider = transportChannelProvider.withExecutor(executor);
- * }
  * if (transportChannelProvider.needsHeaders()) {
  *   transportChannelProvider = transportChannelProvider.withHeaders(headers);
  * }
+ * // optional: set executor for TransportChannel
+ * transportChannelProvider.withExecutor(executor);
  * TransportChannel transportChannel = transportChannelProvider.getTransportChannel();
  *
*/ @@ -63,14 +62,15 @@ public interface TransportChannelProvider { /** Indicates whether the TransportChannel should be closed by the containing client class. */ boolean shouldAutoClose(); - /** True if the TransportProvider needs an executor. */ - boolean needsExecutor(); - /** - * Sets the executor to use when constructing a new {@link TransportChannel}.. + * True if the TransportProvider needs an executor. * - *

This method should only be called if {@link #needsExecutor()} returns true. + * @deprecated Channel providers will have default executors if they need one. */ + @Deprecated + boolean needsExecutor(); + + /** Sets the executor to use when constructing a new {@link TransportChannel}. */ TransportChannelProvider withExecutor(Executor executor); /** @deprecated Please use {@link #withExecutor(Executor)}. */ @@ -82,7 +82,7 @@ public interface TransportChannelProvider { boolean needsHeaders(); /** - * Sets the headers to use when constructing a new {@link TransportChannel}.. + * Sets the headers to use when constructing a new {@link TransportChannel}. * *

This method should only be called if {@link #needsHeaders()} returns true. */ diff --git a/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java b/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java index 9bacd72c0..7fe418dd6 100644 --- a/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java +++ b/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java @@ -176,12 +176,10 @@ public TransportChannelProvider withPoolSize(int size) { @Override public TransportChannel getTransportChannel() throws IOException { - if (needsExecutor()) { - throw new IllegalStateException("Needs Executor"); - } if (needsCredentials()) { throw new IllegalStateException("Needs Credentials"); } + transport.setExecutor(executor); return transport; } @@ -719,4 +717,56 @@ public void testSwitchToMtlsEndpointAllowed() throws IOException { assertFalse(settings.getSwitchToMtlsEndpointAllowed()); assertEquals(endpoint, settings.getEndpoint()); } + + @Test + public void testExecutorSettings() throws Exception { + TransportChannelProvider transportChannelProvider = + new FakeTransportProvider( + FakeTransportChannel.create(new FakeChannel()), null, true, null, null); + + ClientSettings.Builder builder = + new FakeClientSettings.Builder() + .setTransportChannelProvider(transportChannelProvider) + .setCredentialsProvider( + FixedCredentialsProvider.create(Mockito.mock(GoogleCredentials.class))); + + // By default, if executor is not set, channel provider should not have an executor set + ClientContext context = ClientContext.create(builder.build()); + FakeTransportChannel transportChannel = (FakeTransportChannel) context.getTransportChannel(); + assertThat(transportChannel.getExecutor()).isNull(); + + ExecutorProvider channelExecutorProvider = + FixedExecutorProvider.create(Mockito.mock(ScheduledExecutorService.class)); + builder.setTransportChannelProvider( + transportChannelProvider.withExecutor((Executor) channelExecutorProvider.getExecutor())); + context = ClientContext.create(builder.build()); + transportChannel = (FakeTransportChannel) context.getTransportChannel(); + assertThat(transportChannel.getExecutor()) + .isSameInstanceAs(channelExecutorProvider.getExecutor()); + + ExecutorProvider executorProvider = + FixedExecutorProvider.create(Mockito.mock(ScheduledExecutorService.class)); + assertThat(channelExecutorProvider.getExecutor()) + .isNotSameInstanceAs(executorProvider.getExecutor()); + + // For backward compatibility, if executor is set from stubSettings.setExecutor, and transport + // channel already has an executor, the ExecutorProvider set in stubSettings won't override + // transport channel's executor + builder.setExecutorProvider(executorProvider); + context = ClientContext.create(builder.build()); + transportChannel = (FakeTransportChannel) context.getTransportChannel(); + assertThat(transportChannel.getExecutor()) + .isSameInstanceAs(channelExecutorProvider.getExecutor()); + + // For backward compatibility, if executor is set from stubSettings.setExecutor, and transport + // channel doesn't have an executor, transport channel will get the executor from + // stubSettings.setExecutor + builder.setExecutorProvider(executorProvider); + builder.setTransportChannelProvider( + new FakeTransportProvider( + FakeTransportChannel.create(new FakeChannel()), null, true, null, null)); + context = ClientContext.create(builder.build()); + transportChannel = (FakeTransportChannel) context.getTransportChannel(); + assertThat(transportChannel.getExecutor()).isSameInstanceAs(executorProvider.getExecutor()); + } } diff --git a/gax/src/test/java/com/google/api/gax/rpc/ClientSettingsTest.java b/gax/src/test/java/com/google/api/gax/rpc/ClientSettingsTest.java index d777f506a..b39d59cbd 100644 --- a/gax/src/test/java/com/google/api/gax/rpc/ClientSettingsTest.java +++ b/gax/src/test/java/com/google/api/gax/rpc/ClientSettingsTest.java @@ -109,7 +109,8 @@ public class ClientSettingsTest { @Test public void testEmptyBuilder() throws Exception { FakeClientSettings.Builder builder = new FakeClientSettings.Builder(); - Truth.assertThat(builder.getExecutorProvider()) + Truth.assertThat(builder.getExecutorProvider()).isNull(); + Truth.assertThat(builder.getBackgroundExecutorProvider()) .isInstanceOf(InstantiatingExecutorProvider.class); Truth.assertThat(builder.getTransportChannelProvider()).isNull(); Truth.assertThat(builder.getCredentialsProvider()).isInstanceOf(NoCredentialsProvider.class); @@ -124,6 +125,8 @@ public void testEmptyBuilder() throws Exception { FakeClientSettings settings = builder.build(); Truth.assertThat(settings.getExecutorProvider()) .isSameInstanceAs(builder.getExecutorProvider()); + Truth.assertThat(settings.getBackgroundExecutorProvider()) + .isSameInstanceAs(builder.getBackgroundExecutorProvider()); Truth.assertThat(settings.getTransportChannelProvider()) .isSameInstanceAs(builder.getTransportChannelProvider()); Truth.assertThat(settings.getCredentialsProvider()) @@ -139,6 +142,7 @@ public void testEmptyBuilder() throws Exception { String settingsString = settings.toString(); Truth.assertThat(settingsString).contains("executorProvider"); + Truth.assertThat(settingsString).contains("backgroundExecutorProvider"); Truth.assertThat(settingsString).contains("transportChannelProvider"); Truth.assertThat(settingsString).contains("credentialsProvider"); Truth.assertThat(settingsString).contains("clock"); @@ -172,7 +176,9 @@ public void testBuilder() throws Exception { builder.setWatchdogCheckInterval(watchdogCheckInterval); builder.setQuotaProjectId(quotaProjectId); + // For backward compatibility, backgroundExecutorProvider is set to executorProvider Truth.assertThat(builder.getExecutorProvider()).isSameInstanceAs(executorProvider); + Truth.assertThat(builder.getBackgroundExecutorProvider()).isSameInstanceAs(executorProvider); Truth.assertThat(builder.getTransportChannelProvider()).isSameInstanceAs(transportProvider); Truth.assertThat(builder.getCredentialsProvider()).isSameInstanceAs(credentialsProvider); Truth.assertThat(builder.getClock()).isSameInstanceAs(clock); @@ -184,6 +190,7 @@ public void testBuilder() throws Exception { String builderString = builder.toString(); Truth.assertThat(builderString).contains("executorProvider"); + Truth.assertThat(builderString).contains("backgroundExecutorProvider"); Truth.assertThat(builderString).contains("transportChannelProvider"); Truth.assertThat(builderString).contains("credentialsProvider"); Truth.assertThat(builderString).contains("clock"); @@ -223,6 +230,10 @@ public void testBuilderFromClientContext() throws Exception { FakeClientSettings.Builder builder = new FakeClientSettings.Builder(clientContext); Truth.assertThat(builder.getExecutorProvider()).isInstanceOf(FixedExecutorProvider.class); + Truth.assertThat(builder.getBackgroundExecutorProvider()) + .isInstanceOf(FixedExecutorProvider.class); + Truth.assertThat(builder.getExecutorProvider()) + .isSameInstanceAs(builder.getBackgroundExecutorProvider()); Truth.assertThat(builder.getTransportChannelProvider()) .isInstanceOf(FixedTransportChannelProvider.class); Truth.assertThat(builder.getCredentialsProvider()).isInstanceOf(FixedCredentialsProvider.class); @@ -263,6 +274,7 @@ public void testBuilderFromSettings() throws Exception { FakeClientSettings.Builder newBuilder = new FakeClientSettings.Builder(settings); Truth.assertThat(newBuilder.getExecutorProvider()).isSameInstanceAs(executorProvider); + Truth.assertThat(newBuilder.getBackgroundExecutorProvider()).isSameInstanceAs(executorProvider); Truth.assertThat(newBuilder.getTransportChannelProvider()).isSameInstanceAs(transportProvider); Truth.assertThat(newBuilder.getCredentialsProvider()).isSameInstanceAs(credentialsProvider); Truth.assertThat(newBuilder.getClock()).isSameInstanceAs(clock); diff --git a/gax/src/test/java/com/google/api/gax/rpc/testing/FakeTransportChannel.java b/gax/src/test/java/com/google/api/gax/rpc/testing/FakeTransportChannel.java index b6c56a701..0d4abac8f 100644 --- a/gax/src/test/java/com/google/api/gax/rpc/testing/FakeTransportChannel.java +++ b/gax/src/test/java/com/google/api/gax/rpc/testing/FakeTransportChannel.java @@ -32,6 +32,7 @@ import com.google.api.core.InternalApi; import com.google.api.gax.rpc.TransportChannel; import java.util.Map; +import java.util.concurrent.Executor; import java.util.concurrent.TimeUnit; @InternalApi("for testing") @@ -39,6 +40,7 @@ public class FakeTransportChannel implements TransportChannel { private final FakeChannel channel; private volatile boolean isShutdown = false; private volatile Map headers; + private volatile Executor executor; private FakeTransportChannel(FakeChannel channel) { this.channel = channel; @@ -102,4 +104,12 @@ public void setHeaders(Map headers) { public Map getHeaders() { return this.headers; } + + public void setExecutor(Executor executor) { + this.executor = executor; + } + + public Executor getExecutor() { + return executor; + } }