From 429be454a3c31a937064c4342d2cb7b0928056e0 Mon Sep 17 00:00:00 2001 From: Michael Bausor Date: Mon, 25 Apr 2016 13:21:19 -0700 Subject: [PATCH 1/9] Make ServiceApiSettings provider interfaces public Make ChannelProvider, ExecutorProvider and CredentialsProvider interfaces public. This allows the ApiSettings object to be constructed without instantiating the channel, executor or credentials until they are required by the Api object. Pre-push hook installed. Change-Id: I2b88beb112c24e8e35d5d58b4883bd4be0d15706 --- .../api/gax/core/ConnectionSettings.java | 30 +-- .../api/gax/grpc/ApiCallSettingsTyped.java | 4 +- .../api/gax/grpc/ServiceApiSettings.java | 178 ++++++++++-------- 3 files changed, 124 insertions(+), 88 deletions(-) diff --git a/src/main/java/com/google/api/gax/core/ConnectionSettings.java b/src/main/java/com/google/api/gax/core/ConnectionSettings.java index dfe1e9fa7..ebc66012e 100644 --- a/src/main/java/com/google/api/gax/core/ConnectionSettings.java +++ b/src/main/java/com/google/api/gax/core/ConnectionSettings.java @@ -56,25 +56,31 @@ @AutoValue public abstract class ConnectionSettings { - /* - * package-private so that the AutoValue derived class can access it + /** + * Provides an interface to hold and acquire the credentials that will be used to call the + * service. */ - interface CredentialsProvider { + public interface CredentialsProvider { + /** + * Gets the credentials which will be used to call the service. If the credentials have not been + * acquired yet, then they will be acquired when this function is called. + */ Credentials getCredentials() throws IOException; } /** - * Gets the credentials which will be used to call the service. If the credentials - * have not been acquired yet, then they will be acquired when this function is called. + * Gets the credentials which will be used to call the service. If the credentials have not been + * acquired yet, then they will be acquired when this function is called. */ - public Credentials getCredentials() throws IOException { + public Credentials getOrBuildCredentials() throws IOException { return getCredentialsProvider().getCredentials(); } - /* - * package-private so that the AutoValue derived class can access it + /** + * The credentials to use in order to call the service. Credentials will not be acquired until + * they are required. */ - abstract CredentialsProvider getCredentialsProvider(); + public abstract CredentialsProvider getCredentialsProvider(); /** * The address used to reach the service. @@ -97,10 +103,10 @@ public Builder toBuilder() { @AutoValue.Builder public abstract static class Builder { - /* - * package-private so that the AutoValue derived class can access it + /** + * Set the credentials to use in order to call the service. */ - abstract Builder setCredentialsProvider(CredentialsProvider provider); + public abstract Builder setCredentialsProvider(CredentialsProvider provider); /** * Sets the credentials to use in order to call the service. diff --git a/src/main/java/com/google/api/gax/grpc/ApiCallSettingsTyped.java b/src/main/java/com/google/api/gax/grpc/ApiCallSettingsTyped.java index 16c0135c2..bbc54569c 100644 --- a/src/main/java/com/google/api/gax/grpc/ApiCallSettingsTyped.java +++ b/src/main/java/com/google/api/gax/grpc/ApiCallSettingsTyped.java @@ -44,8 +44,8 @@ protected ApiCallable createBaseCallable( new DescriptorClientCallFactory<>(methodDescriptor); ApiCallable callable = new ApiCallable<>(new DirectCallable<>(clientCallFactory), this); - ManagedChannel channel = serviceSettings.getChannel(); - ScheduledExecutorService executor = serviceSettings.getExecutor(); + ManagedChannel channel = serviceSettings.getOrBuildChannel(); + ScheduledExecutorService executor = serviceSettings.getOrBuildExecutor(); if (getRetryableCodes() != null) { callable = callable.retryableOn(ImmutableSet.copyOf(getRetryableCodes())); } diff --git a/src/main/java/com/google/api/gax/grpc/ServiceApiSettings.java b/src/main/java/com/google/api/gax/grpc/ServiceApiSettings.java index ec6335b35..abc99a72a 100644 --- a/src/main/java/com/google/api/gax/grpc/ServiceApiSettings.java +++ b/src/main/java/com/google/api/gax/grpc/ServiceApiSettings.java @@ -51,12 +51,44 @@ */ public abstract class ServiceApiSettings { - private final ManagedChannel channel; - private final boolean shouldAutoCloseChannel; - private final ScheduledExecutorService executor; + /** + * Provides an interface to hold and build the channel that will be used. If the channel does not + * already exist, it will not be constructed until getChannel is called. + */ + public interface ChannelProvider { + /** + * Connection settings used to build the channel. If a channel is provided directly this will be + * set to null. + */ + @Nullable + ConnectionSettings connectionSettings(); + + /** + * Indicates whether the channel should be closed by the containing API class. + */ + boolean shouldAutoClose(); + + /** + * Get the channel to be used to connect to the service. The first time this is called, if the + * channel does not already exist, it will be created. + */ + ManagedChannel getChannel(Executor executor) throws IOException; + } - @Nullable - private final ConnectionSettings connectionSettings; + /** + * Provides an interface to hold and create the Executor to be used. If the executor does not + * already exist, it will not be constructed until getExecutor is called. + */ + public interface ExecutorProvider { + /** + * Get the executor to be used to connect to the service. The first time this is called, if the + * executor does not already exist, it will be created. + */ + ScheduledExecutorService getExecutor(); + } + + private final ChannelProvider channelProvider; + private final ExecutorProvider executorProvider; private final String generatorName; private final String generatorVersion; @@ -66,34 +98,51 @@ public abstract class ServiceApiSettings { /** * Constructs an instance of ServiceApiSettings. */ - protected ServiceApiSettings(ManagedChannel channel, - boolean shouldAutoCloseChannel, - ScheduledExecutorService executor, - ConnectionSettings connectionSettings, - String generatorName, - String generatorVersion, - String clientLibName, - String clientLibVersion) { - this.channel = channel; - this.executor = executor; - this.connectionSettings = connectionSettings; - this.shouldAutoCloseChannel = shouldAutoCloseChannel; + protected ServiceApiSettings( + ChannelProvider channelProvider, + ExecutorProvider executorProvider, + String generatorName, + String generatorVersion, + String clientLibName, + String clientLibVersion) { + this.channelProvider = channelProvider; + this.executorProvider = executorProvider; this.clientLibName = clientLibName; this.clientLibVersion = clientLibVersion; this.generatorName = generatorName; this.generatorVersion = generatorVersion; } - public final ManagedChannel getChannel() { - return channel; + /** + * Return the channel to be used to connect to the service, retrieved using the channelProvider. + * If no channel was set, a default channel will be instantiated. + */ + public final ManagedChannel getOrBuildChannel() throws IOException { + return getChannelProvider().getChannel(getOrBuildExecutor()); } - public final ScheduledExecutorService getExecutor() { - return executor; + /** + * Return the channel provider. If no channel provider was set, the default channel provider will + * be returned. + */ + public final ChannelProvider getChannelProvider() { + return channelProvider; } - public final boolean shouldAutoCloseChannel() { - return shouldAutoCloseChannel; + /** + * The Executor used for channels, retries, and bundling, retrieved using the executorProvider. If + * no executor was set, a default executor will be instantiated. + */ + public final ScheduledExecutorService getOrBuildExecutor() { + return getExecutorProvider().getExecutor(); + } + + /** + * Return the executor provider. It no executor provider was set, the default executor provider + * will be returned. + */ + public final ExecutorProvider getExecutorProvider() { + return executorProvider; } public abstract static class Builder { @@ -114,16 +163,6 @@ public abstract static class Builder { private ChannelProvider channelProvider; private ExecutorProvider executorProvider; - private interface ChannelProvider { - ConnectionSettings connectionSettings(); - boolean shouldAutoClose(); - ManagedChannel getChannel(Executor executor) throws IOException; - } - - private interface ExecutorProvider { - ScheduledExecutorService getExecutor(); - } - protected Builder(ConnectionSettings connectionSettings) { this(); channelProvider = createChannelProvider(connectionSettings); @@ -134,11 +173,7 @@ protected Builder(ConnectionSettings connectionSettings) { */ protected Builder(ServiceApiSettings settings) { this(); - if (settings.connectionSettings != null) { - channelProvider = createChannelProvider(settings.connectionSettings); - } else { - channelProvider = createChannelProvider(settings.channel, settings.shouldAutoCloseChannel); - } + this.channelProvider = settings.channelProvider; this.clientLibName = settings.clientLibName; this.clientLibVersion = settings.clientLibVersion; this.serviceGeneratorName = settings.generatorName; @@ -165,13 +200,20 @@ public ScheduledExecutorService getExecutor() { }; } + /** + * Set the executor provider to be used. + */ + public Builder setExecutorProvider(ExecutorProvider executorProvider) { + this.executorProvider = executorProvider; + return this; + } /** * Sets the executor to use for channels, retries, and bundling. * * It is up to the user to terminate the {@code Executor} when it is no longer needed. */ - public Builder setExecutor(final ScheduledExecutorService executor) { + public Builder provideExecutorWith(final ScheduledExecutorService executor) { executorProvider = new ExecutorProvider() { @Override public ScheduledExecutorService getExecutor() { @@ -182,44 +224,31 @@ public ScheduledExecutorService getExecutor() { } /** - * Sets a channel for this ServiceApiSettings to use. This prevents a channel - * from being created. - * - * See class documentation for more details on channels. - */ - public Builder provideChannelWith( - final ManagedChannel channel, final boolean shouldAutoClose) { - channelProvider = createChannelProvider(channel, shouldAutoClose); - return this; - } - - /** - * Provides the connection settings necessary to create a channel. + * Set the channel provider to be used. */ - public Builder provideChannelWith( - final ConnectionSettings settings) { - channelProvider = createChannelProvider(settings); + public Builder setChannelProvider(ChannelProvider channelProvider) { + this.channelProvider = channelProvider; return this; } /** - * The channel used to send requests to the service. - * - * If no channel was set, a default channel will be instantiated, using - * the connection settings provided. + * Sets a channel for this ServiceApiSettings to use. This prevents a channel from being + * created. * * See class documentation for more details on channels. */ - public ManagedChannel getOrBuildChannel() throws IOException { - return channelProvider.getChannel(this.getOrBuildExecutor()); + public Builder provideChannelWith(final ManagedChannel channel, final boolean shouldAutoClose) { + setChannelProvider(createChannelProvider(channel, shouldAutoClose)); + return this; } /** - * The Executor used for channels, retries, and bundling.. - * If no executor was set, a default executor will be instantiated. + * Provides the connection settings necessary to create a channel. */ - public ScheduledExecutorService getOrBuildExecutor() { - return executorProvider.getExecutor(); + public Builder provideChannelWith( + final ConnectionSettings settings) { + setChannelProvider(createChannelProvider(settings)); + return this; } /** @@ -240,6 +269,14 @@ public Builder setClientLibHeader(String name, String version) { return this; } + public ChannelProvider getChannelProvider() { + return channelProvider; + } + + public ExecutorProvider getExecutorProvider() { + return executorProvider; + } + public String getClientLibName() { return clientLibName; } @@ -256,14 +293,6 @@ public String getGeneratorVersion() { return serviceGeneratorVersion; } - public ConnectionSettings getConnectionSettings() { - return channelProvider.connectionSettings(); - } - - public boolean shouldAutoCloseChannel() { - return channelProvider.shouldAutoClose(); - } - /** * Performs a merge, using only non-null fields */ @@ -289,6 +318,7 @@ protected Builder applyToAllApiMethods( private ChannelProvider createChannelProvider(final ConnectionSettings settings) { return new ChannelProvider() { private ManagedChannel channel = null; + @Override public ManagedChannel getChannel(Executor executor) throws IOException { if (channel != null) { @@ -296,7 +326,7 @@ public ManagedChannel getChannel(Executor executor) throws IOException { } List interceptors = Lists.newArrayList(); - interceptors.add(new ClientAuthInterceptor(settings.getCredentials(), executor)); + interceptors.add(new ClientAuthInterceptor(settings.getOrBuildCredentials(), executor)); interceptors.add(new HeaderInterceptor(serviceHeader())); channel = NettyChannelBuilder.forAddress(settings.getServiceAddress(), settings.getPort()) From 94ba6a1f4a1b35bdbe9e3092900dc1e4593a51ea Mon Sep 17 00:00:00 2001 From: Michael Bausor Date: Mon, 25 Apr 2016 14:22:48 -0700 Subject: [PATCH 2/9] Use executor for channel Pre-push hook installed. Change-Id: I6742fae6a8b1052ba1d5a9fc6bcebb90fa87cd7e --- .../com/google/api/gax/grpc/ServiceApiSettings.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/google/api/gax/grpc/ServiceApiSettings.java b/src/main/java/com/google/api/gax/grpc/ServiceApiSettings.java index abc99a72a..7d4534684 100644 --- a/src/main/java/com/google/api/gax/grpc/ServiceApiSettings.java +++ b/src/main/java/com/google/api/gax/grpc/ServiceApiSettings.java @@ -329,10 +329,12 @@ public ManagedChannel getChannel(Executor executor) throws IOException { interceptors.add(new ClientAuthInterceptor(settings.getOrBuildCredentials(), executor)); interceptors.add(new HeaderInterceptor(serviceHeader())); - channel = NettyChannelBuilder.forAddress(settings.getServiceAddress(), settings.getPort()) - .negotiationType(NegotiationType.TLS) - .intercept(interceptors) - .build(); + channel = + NettyChannelBuilder.forAddress(settings.getServiceAddress(), settings.getPort()) + .negotiationType(NegotiationType.TLS) + .intercept(interceptors) + .executor(executor) + .build(); return channel; } From 566052faea45d0e4f4cbfdaf68db0900dffb3cf5 Mon Sep 17 00:00:00 2001 From: Michael Bausor Date: Tue, 26 Apr 2016 13:25:53 -0700 Subject: [PATCH 3/9] Restructed Provider interfaces for ServiceApiSettings Moved interfaces to separate files Added shouldAutoClose parameter to ExecutorProvider Added OperationNotSupportedException to ExecutorProvider and ChannelProvider when a fixed executor/channel is accessed multiple times. Updated documentation Pre-push hook installed. Change-Id: Icb29408ad0854908932675723a2cc831d0111949 --- .../api/gax/core/ConnectionSettings.java | 12 -- .../api/gax/core/CredentialsProvider.java | 16 ++ .../api/gax/grpc/ApiCallSettingsTyped.java | 4 +- .../com/google/api/gax/grpc/ApiCallable.java | 17 +- .../com/google/api/gax/grpc/ApiException.java | 10 +- .../api/gax/grpc/BundlingCallSettings.java | 8 +- .../google/api/gax/grpc/ChannelProvider.java | 49 +++++ .../google/api/gax/grpc/ExecutorProvider.java | 37 ++++ .../gax/grpc/PageStreamingCallSettings.java | 12 +- .../api/gax/grpc/ServiceApiSettings.java | 174 ++++++++---------- .../api/gax/grpc/SimpleCallSettings.java | 8 +- 11 files changed, 215 insertions(+), 132 deletions(-) create mode 100644 src/main/java/com/google/api/gax/core/CredentialsProvider.java create mode 100644 src/main/java/com/google/api/gax/grpc/ChannelProvider.java create mode 100644 src/main/java/com/google/api/gax/grpc/ExecutorProvider.java diff --git a/src/main/java/com/google/api/gax/core/ConnectionSettings.java b/src/main/java/com/google/api/gax/core/ConnectionSettings.java index ebc66012e..3000cc519 100644 --- a/src/main/java/com/google/api/gax/core/ConnectionSettings.java +++ b/src/main/java/com/google/api/gax/core/ConnectionSettings.java @@ -56,18 +56,6 @@ @AutoValue public abstract class ConnectionSettings { - /** - * Provides an interface to hold and acquire the credentials that will be used to call the - * service. - */ - public interface CredentialsProvider { - /** - * Gets the credentials which will be used to call the service. If the credentials have not been - * acquired yet, then they will be acquired when this function is called. - */ - Credentials getCredentials() throws IOException; - } - /** * Gets the credentials which will be used to call the service. If the credentials have not been * acquired yet, then they will be acquired when this function is called. diff --git a/src/main/java/com/google/api/gax/core/CredentialsProvider.java b/src/main/java/com/google/api/gax/core/CredentialsProvider.java new file mode 100644 index 000000000..0e19f9a72 --- /dev/null +++ b/src/main/java/com/google/api/gax/core/CredentialsProvider.java @@ -0,0 +1,16 @@ +package com.google.api.gax.core; + +import com.google.auth.Credentials; + +import java.io.IOException; + +/** + * Provides an interface to hold and acquire the credentials that will be used to call the service. + */ +public interface CredentialsProvider { + /** + * Gets the credentials which will be used to call the service. If the credentials have not been + * acquired yet, then they will be acquired when this function is called. + */ + Credentials getCredentials() throws IOException; +} diff --git a/src/main/java/com/google/api/gax/grpc/ApiCallSettingsTyped.java b/src/main/java/com/google/api/gax/grpc/ApiCallSettingsTyped.java index bbc54569c..60c56795b 100644 --- a/src/main/java/com/google/api/gax/grpc/ApiCallSettingsTyped.java +++ b/src/main/java/com/google/api/gax/grpc/ApiCallSettingsTyped.java @@ -39,13 +39,11 @@ protected ApiCallSettingsTyped(ImmutableSet retryableCodes, } protected ApiCallable createBaseCallable( - ServiceApiSettings serviceSettings) throws IOException { + ManagedChannel channel, ScheduledExecutorService executor) throws IOException { ClientCallFactory clientCallFactory = new DescriptorClientCallFactory<>(methodDescriptor); ApiCallable callable = new ApiCallable<>(new DirectCallable<>(clientCallFactory), this); - ManagedChannel channel = serviceSettings.getOrBuildChannel(); - ScheduledExecutorService executor = serviceSettings.getOrBuildExecutor(); if (getRetryableCodes() != null) { callable = callable.retryableOn(ImmutableSet.copyOf(getRetryableCodes())); } diff --git a/src/main/java/com/google/api/gax/grpc/ApiCallable.java b/src/main/java/com/google/api/gax/grpc/ApiCallable.java index 5cdab666b..3de596904 100644 --- a/src/main/java/com/google/api/gax/grpc/ApiCallable.java +++ b/src/main/java/com/google/api/gax/grpc/ApiCallable.java @@ -42,6 +42,7 @@ import com.google.common.util.concurrent.UncheckedExecutionException; import io.grpc.Channel; +import io.grpc.ManagedChannel; import io.grpc.Status; import io.grpc.StatusRuntimeException; @@ -114,8 +115,8 @@ public final class ApiCallable { */ public static ApiCallable create( SimpleCallSettings simpleCallSettings, - ServiceApiSettings serviceSettings) throws IOException { - return simpleCallSettings.create(serviceSettings); + ManagedChannel channel, ScheduledExecutorService executor) throws IOException { + return simpleCallSettings.create(channel, executor); } /** @@ -131,8 +132,8 @@ public static ApiCallable create( public static ApiCallable> createPagedVariant( PageStreamingCallSettings pageStreamingCallSettings, - ServiceApiSettings serviceSettings) throws IOException { - return pageStreamingCallSettings.createPagedVariant(serviceSettings); + ManagedChannel channel, ScheduledExecutorService executor) throws IOException { + return pageStreamingCallSettings.createPagedVariant(channel, executor); } /** @@ -148,8 +149,8 @@ ApiCallable> createPagedVariant( public static ApiCallable create( PageStreamingCallSettings pageStreamingCallSettings, - ServiceApiSettings serviceSettings) throws IOException { - return pageStreamingCallSettings.create(serviceSettings); + ManagedChannel channel, ScheduledExecutorService executor) throws IOException { + return pageStreamingCallSettings.create(channel, executor); } /** @@ -164,8 +165,8 @@ ApiCallable create( */ public static ApiCallable create( BundlingCallSettings bundlingCallSettings, - ServiceApiSettings serviceSettings) throws IOException { - return bundlingCallSettings.create(serviceSettings); + ManagedChannel channel, ScheduledExecutorService executor) throws IOException { + return bundlingCallSettings.create(channel, executor); } /** diff --git a/src/main/java/com/google/api/gax/grpc/ApiException.java b/src/main/java/com/google/api/gax/grpc/ApiException.java index 1c586b9ac..2d7b4ff0b 100644 --- a/src/main/java/com/google/api/gax/grpc/ApiException.java +++ b/src/main/java/com/google/api/gax/grpc/ApiException.java @@ -38,7 +38,9 @@ /** * Represents an exception thrown during an RPC call. * - *

It stores information useful for functionalities in {@link ApiCallable}. + *

+ * It stores information useful for functionalities in {@link ApiCallable}. For more information + * about the status codes returned by the underlying grpc exception see {@link Status}. */ public class ApiException extends RuntimeException { private final Status.Code statusCode; @@ -58,9 +60,9 @@ public boolean isRetryable() { } /** - * Returns the status code of the underlying grpc exception. In cases - * where the underlying exception is not of type StatusException or - * StatusRuntimeException, the status code will be Status.Code.UNKNOWN + * Returns the status code of the underlying grpc exception. In cases where the underlying + * exception is not of type StatusException or StatusRuntimeException, the status code will be + * Status.Code.UNKNOWN. For more information about status codes see {@link Status}. */ public Status.Code getStatusCode() { return statusCode; diff --git a/src/main/java/com/google/api/gax/grpc/BundlingCallSettings.java b/src/main/java/com/google/api/gax/grpc/BundlingCallSettings.java index 2765cdfd7..3f22f5a7e 100644 --- a/src/main/java/com/google/api/gax/grpc/BundlingCallSettings.java +++ b/src/main/java/com/google/api/gax/grpc/BundlingCallSettings.java @@ -3,11 +3,13 @@ import com.google.api.gax.core.RetrySettings; import com.google.common.collect.ImmutableSet; +import io.grpc.ManagedChannel; import io.grpc.MethodDescriptor; import io.grpc.Status; import java.io.IOException; import java.util.Set; +import java.util.concurrent.ScheduledExecutorService; /** * A settings class to configure an ApiCallable for calls to an API method that supports @@ -22,9 +24,9 @@ public final class BundlingCallSettings /** * Package-private, for use by ApiCallable. */ - ApiCallable create( - ServiceApiSettings serviceSettings) throws IOException { - ApiCallable baseCallable = createBaseCallable(serviceSettings); + ApiCallable create(ManagedChannel channel, ScheduledExecutorService executor) + throws IOException { + ApiCallable baseCallable = createBaseCallable(channel, executor); bundlerFactory = new BundlerFactory<>(bundlingDescriptor, bundlingSettings); return baseCallable.bundling(bundlingDescriptor, bundlerFactory); } diff --git a/src/main/java/com/google/api/gax/grpc/ChannelProvider.java b/src/main/java/com/google/api/gax/grpc/ChannelProvider.java new file mode 100644 index 000000000..7a2524a3e --- /dev/null +++ b/src/main/java/com/google/api/gax/grpc/ChannelProvider.java @@ -0,0 +1,49 @@ +package com.google.api.gax.grpc; + +import com.google.api.gax.core.ConnectionSettings; + +import io.grpc.ManagedChannel; + +import java.io.IOException; +import java.util.concurrent.Executor; + +import javax.annotation.Nullable; +import javax.naming.OperationNotSupportedException; + +/** + * Provides an interface to hold and build the channel that will be used. If the channel does not + * already exist, it will be constructed when {@link #getChannel} is called. + * + * Implementations of {@link ChannelProvider} may choose to create a new {@link ManagedChannel} for + * each call to {@link #getChannel}, or may return a fixed {@link ManagedChannel} instance. In cases + * where the same {@link ManagedChannel} instance is returned, for example by a + * {@link ChannelProvider} created using the + * {@link ServiceApiSettings#provideChannelWith(ManagedChannel, boolean)} method, and + * shouldAutoClose returns true, the {@link #getChannel} method will throw an + * {@link OperationNotSupportedException} if it is called more than once. This is to prevent the + * same {@link ManagedChannel} being closed prematurely when it is used by multiple client objects. + */ +public interface ChannelProvider { + /** + * Connection settings used to build the channel. If a channel is provided directly this will be + * set to null. + */ + @Nullable + ConnectionSettings connectionSettings(); + + /** + * Indicates whether the channel should be closed by the containing API class. + */ + boolean shouldAutoClose(); + + /** + * Get the channel to be used to connect to the service. The first time this is called, if the + * channel does not already exist, it will be created. + * + * If the {@link ChannelProvider} is configured to return a fixed {@link ManagedChannel} object + * and to return shouldAutoClose as true, then after the first call to {@link #getChannel}, + * subsequent calls should throw an {@link OperationNotSupportedException}. See interface level + * docs for {@link ChannelProvider} for more details. + */ + ManagedChannel getChannel(Executor executor) throws IOException, OperationNotSupportedException; +} diff --git a/src/main/java/com/google/api/gax/grpc/ExecutorProvider.java b/src/main/java/com/google/api/gax/grpc/ExecutorProvider.java new file mode 100644 index 000000000..eff1487c0 --- /dev/null +++ b/src/main/java/com/google/api/gax/grpc/ExecutorProvider.java @@ -0,0 +1,37 @@ +package com.google.api.gax.grpc; + +import java.util.concurrent.ScheduledExecutorService; + +import javax.naming.OperationNotSupportedException; + +/** + * Provides an interface to hold and create the Executor to be used. If the executor does not + * already exist, it will be constructed when {@link #getExecutor} is called. + * + * Implementations of ExecutorProvider may choose to create a new {@link ScheduledExecutorService} + * for each call to {@link #getExecutor}, or may return a fixed {@link ScheduledExecutorService} + * instance. In cases where the same {@link ScheduledExecutorService} instance is returned, for + * example by an {@link ExecutorProvider} created using the + * {@link ServiceApiSettings#provideExecutorWith(ScheduledExecutorService, boolean)} method, and + * shouldAutoClose returns true, the {@link #getExecutor} method will throw an + * {@link OperationNotSupportedException} if it is called more than once. This is to prevent the + * same {@link ScheduledExecutorService} being closed prematurely when it is used by multiple client + * objects. + */ +public interface ExecutorProvider { + /** + * Indicates whether the channel should be closed by the containing API class. + */ + boolean shouldAutoClose(); + + /** + * Get the executor to be used to connect to the service. The first time this is called, if the + * executor does not already exist, it will be created. + * + * If the {@link ExecutorProvider} is configured to return a fixed + * {@link ScheduledExecutorService} object and to return shouldAutoClose as true, then after the + * first call to {@link #getExecutor}, subsequent calls should throw an {@link ExecutorProvider}. + * See interface level docs for {@link ExecutorProvider} for more details. + */ + ScheduledExecutorService getExecutor() throws OperationNotSupportedException; +} diff --git a/src/main/java/com/google/api/gax/grpc/PageStreamingCallSettings.java b/src/main/java/com/google/api/gax/grpc/PageStreamingCallSettings.java index 3cb1db69e..499903b77 100644 --- a/src/main/java/com/google/api/gax/grpc/PageStreamingCallSettings.java +++ b/src/main/java/com/google/api/gax/grpc/PageStreamingCallSettings.java @@ -4,11 +4,13 @@ import com.google.api.gax.core.RetrySettings; import com.google.common.collect.ImmutableSet; +import io.grpc.ManagedChannel; import io.grpc.MethodDescriptor; import io.grpc.Status; import java.io.IOException; import java.util.Set; +import java.util.concurrent.ScheduledExecutorService; /** @@ -22,17 +24,17 @@ public final class PageStreamingCallSettings /** * Package-private, for use by ApiCallable. */ - ApiCallable create( - ServiceApiSettings serviceSettings) throws IOException { - return createBaseCallable(serviceSettings); + ApiCallable create(ManagedChannel channel, ScheduledExecutorService executor) + throws IOException { + return createBaseCallable(channel, executor); } /** * Package-private, for use by ApiCallable. */ ApiCallable> createPagedVariant( - ServiceApiSettings serviceSettings) throws IOException { - ApiCallable baseCallable = createBaseCallable(serviceSettings); + ManagedChannel channel, ScheduledExecutorService executor) throws IOException { + ApiCallable baseCallable = createBaseCallable(channel, executor); return baseCallable.pageStreaming(pageDescriptor); } diff --git a/src/main/java/com/google/api/gax/grpc/ServiceApiSettings.java b/src/main/java/com/google/api/gax/grpc/ServiceApiSettings.java index 7d4534684..5c200ea24 100644 --- a/src/main/java/com/google/api/gax/grpc/ServiceApiSettings.java +++ b/src/main/java/com/google/api/gax/grpc/ServiceApiSettings.java @@ -19,7 +19,7 @@ import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledThreadPoolExecutor; -import javax.annotation.Nullable; +import javax.naming.OperationNotSupportedException; /** * A base settings class to configure a service API class. @@ -51,42 +51,6 @@ */ public abstract class ServiceApiSettings { - /** - * Provides an interface to hold and build the channel that will be used. If the channel does not - * already exist, it will not be constructed until getChannel is called. - */ - public interface ChannelProvider { - /** - * Connection settings used to build the channel. If a channel is provided directly this will be - * set to null. - */ - @Nullable - ConnectionSettings connectionSettings(); - - /** - * Indicates whether the channel should be closed by the containing API class. - */ - boolean shouldAutoClose(); - - /** - * Get the channel to be used to connect to the service. The first time this is called, if the - * channel does not already exist, it will be created. - */ - ManagedChannel getChannel(Executor executor) throws IOException; - } - - /** - * Provides an interface to hold and create the Executor to be used. If the executor does not - * already exist, it will not be constructed until getExecutor is called. - */ - public interface ExecutorProvider { - /** - * Get the executor to be used to connect to the service. The first time this is called, if the - * executor does not already exist, it will be created. - */ - ScheduledExecutorService getExecutor(); - } - private final ChannelProvider channelProvider; private final ExecutorProvider executorProvider; @@ -117,7 +81,8 @@ protected ServiceApiSettings( * Return the channel to be used to connect to the service, retrieved using the channelProvider. * If no channel was set, a default channel will be instantiated. */ - public final ManagedChannel getOrBuildChannel() throws IOException { + public final ManagedChannel getOrBuildChannel() + throws IOException, OperationNotSupportedException { return getChannelProvider().getChannel(getOrBuildExecutor()); } @@ -133,7 +98,7 @@ public final ChannelProvider getChannelProvider() { * The Executor used for channels, retries, and bundling, retrieved using the executorProvider. If * no executor was set, a default executor will be instantiated. */ - public final ScheduledExecutorService getOrBuildExecutor() { + public final ScheduledExecutorService getOrBuildExecutor() throws OperationNotSupportedException { return getExecutorProvider().getExecutor(); } @@ -186,48 +151,54 @@ private Builder() { serviceGeneratorName = DEFAULT_GENERATOR_NAME; serviceGeneratorVersion = DEFAULT_VERSION; - executorProvider = new ExecutorProvider() { - private ScheduledExecutorService executor = null; - @Override - public ScheduledExecutorService getExecutor() { - if (executor != null) { - return executor; - } - executor = MoreExecutors.getExitingScheduledExecutorService( - new ScheduledThreadPoolExecutor(DEFAULT_EXECUTOR_THREADS)); - return executor; - } - }; - } - - /** - * Set the executor provider to be used. - */ - public Builder setExecutorProvider(ExecutorProvider executorProvider) { - this.executorProvider = executorProvider; - return this; + executorProvider = + new ExecutorProvider() { + @Override + public ScheduledExecutorService getExecutor() { + return MoreExecutors.getExitingScheduledExecutorService( + new ScheduledThreadPoolExecutor(DEFAULT_EXECUTOR_THREADS)); + } + + @Override + public boolean shouldAutoClose() { + return true; + } + }; } /** * Sets the executor to use for channels, retries, and bundling. * - * It is up to the user to terminate the {@code Executor} when it is no longer needed. - */ - public Builder provideExecutorWith(final ScheduledExecutorService executor) { - executorProvider = new ExecutorProvider() { - @Override - public ScheduledExecutorService getExecutor() { - return executor; - } - }; - return this; - } - - /** - * Set the channel provider to be used. + * If multiple Api objects will use this executor, shouldAutoClose must be set to false to + * prevent the ExecutorProvider from throwing an OperationNotSupportedException. See + * {@link ExecutorProvider} for more details. */ - public Builder setChannelProvider(ChannelProvider channelProvider) { - this.channelProvider = channelProvider; + public Builder provideExecutorWith( + final ScheduledExecutorService executor, final boolean shouldAutoClose) { + executorProvider = + new ExecutorProvider() { + private boolean executorProvided = false; + + @Override + public ScheduledExecutorService getExecutor() throws OperationNotSupportedException { + if (executorProvided) { + if (shouldAutoClose) { + throw new OperationNotSupportedException( + "A fixed executor cannot be re-used when shouldAutoClose is set to true. " + + "Try calling provideExecutorWith with shouldAutoClose set to false, or " + + "using a channel created from a ConnectionSettings object."); + } + } else { + executorProvided = true; + } + return executor; + } + + @Override + public boolean shouldAutoClose() { + return shouldAutoClose; + } + }; return this; } @@ -236,9 +207,13 @@ public Builder setChannelProvider(ChannelProvider channelProvider) { * created. * * See class documentation for more details on channels. + * + * If multiple Api objects will use this channel, shouldAutoClose must be set to false to + * prevent the ChannelProvider from throwing an OperationNotSupportedException. See + * {@link ChannelProvider} for more details. */ public Builder provideChannelWith(final ManagedChannel channel, final boolean shouldAutoClose) { - setChannelProvider(createChannelProvider(channel, shouldAutoClose)); + channelProvider = createChannelProvider(channel, shouldAutoClose); return this; } @@ -247,7 +222,7 @@ public Builder provideChannelWith(final ManagedChannel channel, final boolean sh */ public Builder provideChannelWith( final ConnectionSettings settings) { - setChannelProvider(createChannelProvider(settings)); + channelProvider = createChannelProvider(settings); return this; } @@ -317,25 +292,17 @@ protected Builder applyToAllApiMethods( private ChannelProvider createChannelProvider(final ConnectionSettings settings) { return new ChannelProvider() { - private ManagedChannel channel = null; - @Override public ManagedChannel getChannel(Executor executor) throws IOException { - if (channel != null) { - return channel; - } - List interceptors = Lists.newArrayList(); interceptors.add(new ClientAuthInterceptor(settings.getOrBuildCredentials(), executor)); interceptors.add(new HeaderInterceptor(serviceHeader())); - channel = - NettyChannelBuilder.forAddress(settings.getServiceAddress(), settings.getPort()) - .negotiationType(NegotiationType.TLS) - .intercept(interceptors) - .executor(executor) - .build(); - return channel; + return NettyChannelBuilder.forAddress(settings.getServiceAddress(), settings.getPort()) + .negotiationType(NegotiationType.TLS) + .intercept(interceptors) + .executor(executor) + .build(); } @Override @@ -355,9 +322,14 @@ private String serviceHeader() { gaxVersion = DEFAULT_VERSION; } String javaVersion = Runtime.class.getPackage().getImplementationVersion(); - return String.format("%s/%s;%s/%s;gax/%s;java/%s", - clientLibName, clientLibVersion, serviceGeneratorName, serviceGeneratorVersion, - gaxVersion, javaVersion); + return String.format( + "%s/%s;%s/%s;gax/%s;java/%s", + clientLibName, + clientLibVersion, + serviceGeneratorName, + serviceGeneratorVersion, + gaxVersion, + javaVersion); } }; } @@ -365,14 +337,28 @@ private String serviceHeader() { private ChannelProvider createChannelProvider(final ManagedChannel channel, final boolean shouldAutoClose) { return new ChannelProvider() { + private boolean channelProvided = false; + @Override - public ManagedChannel getChannel(Executor executor) { + public ManagedChannel getChannel(Executor executor) throws OperationNotSupportedException { + if (channelProvided) { + if (shouldAutoClose) { + throw new OperationNotSupportedException( + "A fixed channel cannot be re-used when shouldAutoClose is set to true. " + + "Try calling provideChannelWith with shouldAutoClose set to false, or " + + "using a channel created from a ConnectionSettings object."); + } + } else { + channelProvided = true; + } return channel; } + @Override public boolean shouldAutoClose() { return shouldAutoClose; } + @Override public ConnectionSettings connectionSettings() { return null; diff --git a/src/main/java/com/google/api/gax/grpc/SimpleCallSettings.java b/src/main/java/com/google/api/gax/grpc/SimpleCallSettings.java index d26e0dd18..e6486f19a 100644 --- a/src/main/java/com/google/api/gax/grpc/SimpleCallSettings.java +++ b/src/main/java/com/google/api/gax/grpc/SimpleCallSettings.java @@ -3,11 +3,13 @@ import com.google.api.gax.core.RetrySettings; import com.google.common.collect.ImmutableSet; +import io.grpc.ManagedChannel; import io.grpc.MethodDescriptor; import io.grpc.Status; import java.io.IOException; import java.util.Set; +import java.util.concurrent.ScheduledExecutorService; /** * A settings class to configure an ApiCallable for calls to a simple API method (i.e. that @@ -19,9 +21,9 @@ public final class SimpleCallSettings /** * Package-private, for use by ApiCallable. */ - ApiCallable create( - ServiceApiSettings serviceSettings) throws IOException { - return createBaseCallable(serviceSettings); + ApiCallable create(ManagedChannel channel, ScheduledExecutorService executor) + throws IOException { + return createBaseCallable(channel, executor); } private SimpleCallSettings(ImmutableSet retryableCodes, From 82ab4a2aa6067860f097dd8ab241dfc927c46553 Mon Sep 17 00:00:00 2001 From: Michael Bausor Date: Tue, 26 Apr 2016 13:37:31 -0700 Subject: [PATCH 4/9] Updated annotations Removed exceptions not thrown Updated params in javadocs Fixed broken doc links Pre-push hook installed. Change-Id: I2a259ea65dd9a9094a67fdf8d78ad820abf3d7e2 --- .../api/gax/grpc/ApiCallSettingsTyped.java | 3 +- .../com/google/api/gax/grpc/ApiCallable.java | 58 ++++++++++--------- .../api/gax/grpc/BundlingCallSettings.java | 5 +- .../google/api/gax/grpc/ChannelProvider.java | 10 ++-- .../google/api/gax/grpc/ExecutorProvider.java | 11 ++-- .../gax/grpc/PageStreamingCallSettings.java | 7 +-- .../api/gax/grpc/SimpleCallSettings.java | 5 +- 7 files changed, 48 insertions(+), 51 deletions(-) diff --git a/src/main/java/com/google/api/gax/grpc/ApiCallSettingsTyped.java b/src/main/java/com/google/api/gax/grpc/ApiCallSettingsTyped.java index 60c56795b..d889076f9 100644 --- a/src/main/java/com/google/api/gax/grpc/ApiCallSettingsTyped.java +++ b/src/main/java/com/google/api/gax/grpc/ApiCallSettingsTyped.java @@ -7,7 +7,6 @@ import io.grpc.MethodDescriptor; import io.grpc.Status; -import java.io.IOException; import java.util.concurrent.ScheduledExecutorService; /** @@ -39,7 +38,7 @@ protected ApiCallSettingsTyped(ImmutableSet retryableCodes, } protected ApiCallable createBaseCallable( - ManagedChannel channel, ScheduledExecutorService executor) throws IOException { + ManagedChannel channel, ScheduledExecutorService executor) { ClientCallFactory clientCallFactory = new DescriptorClientCallFactory<>(methodDescriptor); ApiCallable callable = diff --git a/src/main/java/com/google/api/gax/grpc/ApiCallable.java b/src/main/java/com/google/api/gax/grpc/ApiCallable.java index 3de596904..b1069073d 100644 --- a/src/main/java/com/google/api/gax/grpc/ApiCallable.java +++ b/src/main/java/com/google/api/gax/grpc/ApiCallable.java @@ -46,7 +46,6 @@ import io.grpc.Status; import io.grpc.StatusRuntimeException; -import java.io.IOException; import java.util.concurrent.ScheduledExecutorService; import javax.annotation.Nullable; @@ -104,68 +103,71 @@ public final class ApiCallable { private final ApiCallSettings settings; /** - * Create a callable object that represents a simple API method. - * Public only for technical reasons - for advanced usage + * Create a callable object that represents a simple API method. Public only for technical reasons + * - for advanced usage * - * @param simpleCallSettings {@link com.google.api.gax.grpc.SimpleCallSettings} to configure - * the method-level settings with. - * @param serviceSettings{@link com.google.api.gax.grpc.ServiceApiSettings} - * to configure the service-level settings with. + * @param simpleCallSettings {@link com.google.api.gax.grpc.SimpleCallSettings} to configure the + * method-level settings with. + * @param channel {@link ManagedChannel} to use to connect to the service. + * @param executor {@link ScheduledExecutorService} to use to when connecting to the service. * @return {@link com.google.api.gax.grpc.ApiCallable} callable object. */ public static ApiCallable create( SimpleCallSettings simpleCallSettings, - ManagedChannel channel, ScheduledExecutorService executor) throws IOException { + ManagedChannel channel, + ScheduledExecutorService executor) { return simpleCallSettings.create(channel, executor); } /** - * Create a paged callable object that represents a page-streaming API method. - * Public only for technical reasons - for advanced usage + * Create a paged callable object that represents a page-streaming API method. Public only for + * technical reasons - for advanced usage * * @param pageStreamingCallSettings {@link com.google.api.gax.grpc.PageStreamingCallSettings} to * configure the page-streaming related settings with. - * @param serviceSettings{@link com.google.api.gax.grpc.ServiceApiSettings} - * to configure the service-level settings with. + * @param channel {@link ManagedChannel} to use to connect to the service. + * @param executor {@link ScheduledExecutorService} to use to when connecting to the service. * @return {@link com.google.api.gax.grpc.ApiCallable} callable object. */ public static ApiCallable> createPagedVariant( PageStreamingCallSettings pageStreamingCallSettings, - ManagedChannel channel, ScheduledExecutorService executor) throws IOException { + ManagedChannel channel, + ScheduledExecutorService executor) { return pageStreamingCallSettings.createPagedVariant(channel, executor); } /** - * Create a base callable object that represents a page-streaming API method. - * Public only for technical reasons - for advanced usage + * Create a base callable object that represents a page-streaming API method. Public only for + * technical reasons - for advanced usage * * @param pageStreamingCallSettings {@link com.google.api.gax.grpc.PageStreamingCallSettings} to * configure the page-streaming related settings with. - * @param serviceSettings{@link com.google.api.gax.grpc.ServiceApiSettings} - * to configure the service-level settings with. + * @param channel {@link ManagedChannel} to use to connect to the service. + * @param executor {@link ScheduledExecutorService} to use to when connecting to the service. * @return {@link com.google.api.gax.grpc.ApiCallable} callable object. */ - public static - ApiCallable create( - PageStreamingCallSettings pageStreamingCallSettings, - ManagedChannel channel, ScheduledExecutorService executor) throws IOException { + public static ApiCallable create( + PageStreamingCallSettings pageStreamingCallSettings, + ManagedChannel channel, + ScheduledExecutorService executor) { return pageStreamingCallSettings.create(channel, executor); } /** - * Create a callable object that represents a bundling API method. - * Public only for technical reasons - for advanced usage + * Create a callable object that represents a bundling API method. Public only for technical + * reasons - for advanced usage * - * @param bundlingCallSettings {@link com.google.api.gax.grpc.BundlingSettings} to configure - * the bundling related settings with. - * @param serviceSettings{@link com.google.api.gax.grpc.ServiceApiSettings} - * to configure the service-level settings with. + * @param bundlingCallSettings {@link com.google.api.gax.grpc.BundlingSettings} to configure the + * bundling related settings with. + * @param channel {@link ManagedChannel} to use to connect to the service. + * @param executor {@link ScheduledExecutorService} to use to when connecting to the service. * @return {@link com.google.api.gax.grpc.ApiCallable} callable object. */ public static ApiCallable create( BundlingCallSettings bundlingCallSettings, - ManagedChannel channel, ScheduledExecutorService executor) throws IOException { + ManagedChannel channel, + ScheduledExecutorService executor) { return bundlingCallSettings.create(channel, executor); } diff --git a/src/main/java/com/google/api/gax/grpc/BundlingCallSettings.java b/src/main/java/com/google/api/gax/grpc/BundlingCallSettings.java index 3f22f5a7e..c83df6b76 100644 --- a/src/main/java/com/google/api/gax/grpc/BundlingCallSettings.java +++ b/src/main/java/com/google/api/gax/grpc/BundlingCallSettings.java @@ -7,7 +7,6 @@ import io.grpc.MethodDescriptor; import io.grpc.Status; -import java.io.IOException; import java.util.Set; import java.util.concurrent.ScheduledExecutorService; @@ -24,8 +23,8 @@ public final class BundlingCallSettings /** * Package-private, for use by ApiCallable. */ - ApiCallable create(ManagedChannel channel, ScheduledExecutorService executor) - throws IOException { + ApiCallable create( + ManagedChannel channel, ScheduledExecutorService executor) { ApiCallable baseCallable = createBaseCallable(channel, executor); bundlerFactory = new BundlerFactory<>(bundlingDescriptor, bundlingSettings); return baseCallable.bundling(bundlingDescriptor, bundlerFactory); diff --git a/src/main/java/com/google/api/gax/grpc/ChannelProvider.java b/src/main/java/com/google/api/gax/grpc/ChannelProvider.java index 7a2524a3e..23daa349e 100644 --- a/src/main/java/com/google/api/gax/grpc/ChannelProvider.java +++ b/src/main/java/com/google/api/gax/grpc/ChannelProvider.java @@ -17,11 +17,11 @@ * Implementations of {@link ChannelProvider} may choose to create a new {@link ManagedChannel} for * each call to {@link #getChannel}, or may return a fixed {@link ManagedChannel} instance. In cases * where the same {@link ManagedChannel} instance is returned, for example by a - * {@link ChannelProvider} created using the - * {@link ServiceApiSettings#provideChannelWith(ManagedChannel, boolean)} method, and - * shouldAutoClose returns true, the {@link #getChannel} method will throw an - * {@link OperationNotSupportedException} if it is called more than once. This is to prevent the - * same {@link ManagedChannel} being closed prematurely when it is used by multiple client objects. + * {@link ChannelProvider} created using the {@link ServiceApiSettings} + * provideChannelWith(ManagedChannel, boolean) method, and shouldAutoClose returns true, the + * {@link #getChannel} method will throw an {@link OperationNotSupportedException} if it is called + * more than once. This is to prevent the same {@link ManagedChannel} being closed prematurely when + * it is used by multiple client objects. */ public interface ChannelProvider { /** diff --git a/src/main/java/com/google/api/gax/grpc/ExecutorProvider.java b/src/main/java/com/google/api/gax/grpc/ExecutorProvider.java index eff1487c0..5a26a5133 100644 --- a/src/main/java/com/google/api/gax/grpc/ExecutorProvider.java +++ b/src/main/java/com/google/api/gax/grpc/ExecutorProvider.java @@ -11,12 +11,11 @@ * Implementations of ExecutorProvider may choose to create a new {@link ScheduledExecutorService} * for each call to {@link #getExecutor}, or may return a fixed {@link ScheduledExecutorService} * instance. In cases where the same {@link ScheduledExecutorService} instance is returned, for - * example by an {@link ExecutorProvider} created using the - * {@link ServiceApiSettings#provideExecutorWith(ScheduledExecutorService, boolean)} method, and - * shouldAutoClose returns true, the {@link #getExecutor} method will throw an - * {@link OperationNotSupportedException} if it is called more than once. This is to prevent the - * same {@link ScheduledExecutorService} being closed prematurely when it is used by multiple client - * objects. + * example by an {@link ExecutorProvider} created using the {@link ServiceApiSettings} + * provideExecutorWith(ScheduledExecutorService, boolean) method, and shouldAutoClose returns true, + * the {@link #getExecutor} method will throw an {@link OperationNotSupportedException} if it is + * called more than once. This is to prevent the same {@link ScheduledExecutorService} being closed + * prematurely when it is used by multiple client objects. */ public interface ExecutorProvider { /** diff --git a/src/main/java/com/google/api/gax/grpc/PageStreamingCallSettings.java b/src/main/java/com/google/api/gax/grpc/PageStreamingCallSettings.java index 499903b77..336cbf52d 100644 --- a/src/main/java/com/google/api/gax/grpc/PageStreamingCallSettings.java +++ b/src/main/java/com/google/api/gax/grpc/PageStreamingCallSettings.java @@ -8,7 +8,6 @@ import io.grpc.MethodDescriptor; import io.grpc.Status; -import java.io.IOException; import java.util.Set; import java.util.concurrent.ScheduledExecutorService; @@ -24,8 +23,8 @@ public final class PageStreamingCallSettings /** * Package-private, for use by ApiCallable. */ - ApiCallable create(ManagedChannel channel, ScheduledExecutorService executor) - throws IOException { + ApiCallable create( + ManagedChannel channel, ScheduledExecutorService executor) { return createBaseCallable(channel, executor); } @@ -33,7 +32,7 @@ ApiCallable create(ManagedChannel channel, ScheduledExecuto * Package-private, for use by ApiCallable. */ ApiCallable> createPagedVariant( - ManagedChannel channel, ScheduledExecutorService executor) throws IOException { + ManagedChannel channel, ScheduledExecutorService executor) { ApiCallable baseCallable = createBaseCallable(channel, executor); return baseCallable.pageStreaming(pageDescriptor); } diff --git a/src/main/java/com/google/api/gax/grpc/SimpleCallSettings.java b/src/main/java/com/google/api/gax/grpc/SimpleCallSettings.java index e6486f19a..55de84bae 100644 --- a/src/main/java/com/google/api/gax/grpc/SimpleCallSettings.java +++ b/src/main/java/com/google/api/gax/grpc/SimpleCallSettings.java @@ -7,7 +7,6 @@ import io.grpc.MethodDescriptor; import io.grpc.Status; -import java.io.IOException; import java.util.Set; import java.util.concurrent.ScheduledExecutorService; @@ -21,8 +20,8 @@ public final class SimpleCallSettings /** * Package-private, for use by ApiCallable. */ - ApiCallable create(ManagedChannel channel, ScheduledExecutorService executor) - throws IOException { + ApiCallable create( + ManagedChannel channel, ScheduledExecutorService executor) { return createBaseCallable(channel, executor); } From 0d24ef5a79192149c35d5e8ca1c3f6fd32968a1e Mon Sep 17 00:00:00 2001 From: Michael Bausor Date: Tue, 26 Apr 2016 16:13:17 -0700 Subject: [PATCH 5/9] Changed exception type Changed OperationNotSupportedException to IllegalStateException Fixed doc errors Pre-push hook installed. Change-Id: I33529e918b1e22ce643c03dd46f207a16adfe8ad --- .../com/google/api/gax/grpc/ApiCallable.java | 2 +- .../google/api/gax/grpc/ChannelProvider.java | 13 +++++----- .../google/api/gax/grpc/ExecutorProvider.java | 15 ++++++----- .../api/gax/grpc/ServiceApiSettings.java | 25 ++++++++----------- 4 files changed, 25 insertions(+), 30 deletions(-) diff --git a/src/main/java/com/google/api/gax/grpc/ApiCallable.java b/src/main/java/com/google/api/gax/grpc/ApiCallable.java index b1069073d..ccbf65ab4 100644 --- a/src/main/java/com/google/api/gax/grpc/ApiCallable.java +++ b/src/main/java/com/google/api/gax/grpc/ApiCallable.java @@ -109,7 +109,7 @@ public final class ApiCallable { * @param simpleCallSettings {@link com.google.api.gax.grpc.SimpleCallSettings} to configure the * method-level settings with. * @param channel {@link ManagedChannel} to use to connect to the service. - * @param executor {@link ScheduledExecutorService} to use to when connecting to the service. + * @param executor {@link ScheduledExecutorService} to use when connecting to the service. * @return {@link com.google.api.gax.grpc.ApiCallable} callable object. */ public static ApiCallable create( diff --git a/src/main/java/com/google/api/gax/grpc/ChannelProvider.java b/src/main/java/com/google/api/gax/grpc/ChannelProvider.java index 23daa349e..3b0020e47 100644 --- a/src/main/java/com/google/api/gax/grpc/ChannelProvider.java +++ b/src/main/java/com/google/api/gax/grpc/ChannelProvider.java @@ -8,7 +8,6 @@ import java.util.concurrent.Executor; import javax.annotation.Nullable; -import javax.naming.OperationNotSupportedException; /** * Provides an interface to hold and build the channel that will be used. If the channel does not @@ -19,9 +18,9 @@ * where the same {@link ManagedChannel} instance is returned, for example by a * {@link ChannelProvider} created using the {@link ServiceApiSettings} * provideChannelWith(ManagedChannel, boolean) method, and shouldAutoClose returns true, the - * {@link #getChannel} method will throw an {@link OperationNotSupportedException} if it is called - * more than once. This is to prevent the same {@link ManagedChannel} being closed prematurely when - * it is used by multiple client objects. + * {@link #getChannel} method will throw an {@link IllegalStateException} if it is called more than + * once. This is to prevent the same {@link ManagedChannel} being closed prematurely when it is used + * by multiple client objects. */ public interface ChannelProvider { /** @@ -42,8 +41,8 @@ public interface ChannelProvider { * * If the {@link ChannelProvider} is configured to return a fixed {@link ManagedChannel} object * and to return shouldAutoClose as true, then after the first call to {@link #getChannel}, - * subsequent calls should throw an {@link OperationNotSupportedException}. See interface level - * docs for {@link ChannelProvider} for more details. + * subsequent calls should throw an {@link IllegalStateException}. See interface level docs for + * {@link ChannelProvider} for more details. */ - ManagedChannel getChannel(Executor executor) throws IOException, OperationNotSupportedException; + ManagedChannel getChannel(Executor executor) throws IOException, IllegalStateException; } diff --git a/src/main/java/com/google/api/gax/grpc/ExecutorProvider.java b/src/main/java/com/google/api/gax/grpc/ExecutorProvider.java index 5a26a5133..03d4ce054 100644 --- a/src/main/java/com/google/api/gax/grpc/ExecutorProvider.java +++ b/src/main/java/com/google/api/gax/grpc/ExecutorProvider.java @@ -2,8 +2,6 @@ import java.util.concurrent.ScheduledExecutorService; -import javax.naming.OperationNotSupportedException; - /** * Provides an interface to hold and create the Executor to be used. If the executor does not * already exist, it will be constructed when {@link #getExecutor} is called. @@ -13,9 +11,9 @@ * instance. In cases where the same {@link ScheduledExecutorService} instance is returned, for * example by an {@link ExecutorProvider} created using the {@link ServiceApiSettings} * provideExecutorWith(ScheduledExecutorService, boolean) method, and shouldAutoClose returns true, - * the {@link #getExecutor} method will throw an {@link OperationNotSupportedException} if it is - * called more than once. This is to prevent the same {@link ScheduledExecutorService} being closed - * prematurely when it is used by multiple client objects. + * the {@link #getExecutor} method will throw an {@link IllegalStateException} if it is called more + * than once. This is to prevent the same {@link ScheduledExecutorService} being closed prematurely + * when it is used by multiple client objects. */ public interface ExecutorProvider { /** @@ -29,8 +27,9 @@ public interface ExecutorProvider { * * If the {@link ExecutorProvider} is configured to return a fixed * {@link ScheduledExecutorService} object and to return shouldAutoClose as true, then after the - * first call to {@link #getExecutor}, subsequent calls should throw an {@link ExecutorProvider}. - * See interface level docs for {@link ExecutorProvider} for more details. + * first call to {@link #getExecutor}, subsequent calls should throw an + * {@link IllegalStateException}. See interface level docs for {@link ExecutorProvider} for more + * details. */ - ScheduledExecutorService getExecutor() throws OperationNotSupportedException; + ScheduledExecutorService getExecutor() throws IllegalStateException; } diff --git a/src/main/java/com/google/api/gax/grpc/ServiceApiSettings.java b/src/main/java/com/google/api/gax/grpc/ServiceApiSettings.java index 5c200ea24..a13a19ce1 100644 --- a/src/main/java/com/google/api/gax/grpc/ServiceApiSettings.java +++ b/src/main/java/com/google/api/gax/grpc/ServiceApiSettings.java @@ -19,8 +19,6 @@ import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledThreadPoolExecutor; -import javax.naming.OperationNotSupportedException; - /** * A base settings class to configure a service API class. * @@ -81,8 +79,7 @@ protected ServiceApiSettings( * Return the channel to be used to connect to the service, retrieved using the channelProvider. * If no channel was set, a default channel will be instantiated. */ - public final ManagedChannel getOrBuildChannel() - throws IOException, OperationNotSupportedException { + public final ManagedChannel getOrBuildChannel() throws IOException, IllegalStateException { return getChannelProvider().getChannel(getOrBuildExecutor()); } @@ -98,7 +95,7 @@ public final ChannelProvider getChannelProvider() { * The Executor used for channels, retries, and bundling, retrieved using the executorProvider. If * no executor was set, a default executor will be instantiated. */ - public final ScheduledExecutorService getOrBuildExecutor() throws OperationNotSupportedException { + public final ScheduledExecutorService getOrBuildExecutor() throws IllegalStateException { return getExecutorProvider().getExecutor(); } @@ -170,23 +167,23 @@ public boolean shouldAutoClose() { * Sets the executor to use for channels, retries, and bundling. * * If multiple Api objects will use this executor, shouldAutoClose must be set to false to - * prevent the ExecutorProvider from throwing an OperationNotSupportedException. See + * prevent the {@link ExecutorProvider} from throwing an {@link IllegalStateException}. See * {@link ExecutorProvider} for more details. */ public Builder provideExecutorWith( final ScheduledExecutorService executor, final boolean shouldAutoClose) { executorProvider = new ExecutorProvider() { - private boolean executorProvided = false; + private volatile boolean executorProvided = false; @Override - public ScheduledExecutorService getExecutor() throws OperationNotSupportedException { + public ScheduledExecutorService getExecutor() throws IllegalStateException { if (executorProvided) { if (shouldAutoClose) { - throw new OperationNotSupportedException( + throw new IllegalStateException( "A fixed executor cannot be re-used when shouldAutoClose is set to true. " - + "Try calling provideExecutorWith with shouldAutoClose set to false, or " - + "using a channel created from a ConnectionSettings object."); + + "Try calling provideExecutorWith with shouldAutoClose set to false " + + "or using the default executor."); } } else { executorProvided = true; @@ -209,7 +206,7 @@ public boolean shouldAutoClose() { * See class documentation for more details on channels. * * If multiple Api objects will use this channel, shouldAutoClose must be set to false to - * prevent the ChannelProvider from throwing an OperationNotSupportedException. See + * prevent the {@link ChannelProvider} from throwing an {@link IllegalStateException}. See * {@link ChannelProvider} for more details. */ public Builder provideChannelWith(final ManagedChannel channel, final boolean shouldAutoClose) { @@ -340,10 +337,10 @@ private ChannelProvider createChannelProvider(final ManagedChannel channel, private boolean channelProvided = false; @Override - public ManagedChannel getChannel(Executor executor) throws OperationNotSupportedException { + public ManagedChannel getChannel(Executor executor) throws IllegalStateException { if (channelProvided) { if (shouldAutoClose) { - throw new OperationNotSupportedException( + throw new IllegalStateException( "A fixed channel cannot be re-used when shouldAutoClose is set to true. " + "Try calling provideChannelWith with shouldAutoClose set to false, or " + "using a channel created from a ConnectionSettings object."); From 3169d16e8103da57275316054cdda2eb710aa32f Mon Sep 17 00:00:00 2001 From: Michael Bausor Date: Wed, 27 Apr 2016 11:51:31 -0700 Subject: [PATCH 6/9] Fixes and improvements Added unit tests Removed getOrBuild methods Set executorProvider in ServiceApiSettings constructor Pre-push hook installed. Change-Id: I193be2e6a6ad2e35b5f1def5182e19d9dc14ab29 --- .../api/gax/grpc/ServiceApiSettings.java | 17 +- .../api/gax/grpc/ServiceApiSettingsTest.java | 153 ++++++++++++++++++ 2 files changed, 154 insertions(+), 16 deletions(-) create mode 100644 src/test/java/com/google/api/gax/grpc/ServiceApiSettingsTest.java diff --git a/src/main/java/com/google/api/gax/grpc/ServiceApiSettings.java b/src/main/java/com/google/api/gax/grpc/ServiceApiSettings.java index a13a19ce1..58e2e8b9c 100644 --- a/src/main/java/com/google/api/gax/grpc/ServiceApiSettings.java +++ b/src/main/java/com/google/api/gax/grpc/ServiceApiSettings.java @@ -75,14 +75,6 @@ protected ServiceApiSettings( this.generatorVersion = generatorVersion; } - /** - * Return the channel to be used to connect to the service, retrieved using the channelProvider. - * If no channel was set, a default channel will be instantiated. - */ - public final ManagedChannel getOrBuildChannel() throws IOException, IllegalStateException { - return getChannelProvider().getChannel(getOrBuildExecutor()); - } - /** * Return the channel provider. If no channel provider was set, the default channel provider will * be returned. @@ -91,14 +83,6 @@ public final ChannelProvider getChannelProvider() { return channelProvider; } - /** - * The Executor used for channels, retries, and bundling, retrieved using the executorProvider. If - * no executor was set, a default executor will be instantiated. - */ - public final ScheduledExecutorService getOrBuildExecutor() throws IllegalStateException { - return getExecutorProvider().getExecutor(); - } - /** * Return the executor provider. It no executor provider was set, the default executor provider * will be returned. @@ -136,6 +120,7 @@ protected Builder(ConnectionSettings connectionSettings) { protected Builder(ServiceApiSettings settings) { this(); this.channelProvider = settings.channelProvider; + this.executorProvider = settings.executorProvider; this.clientLibName = settings.clientLibName; this.clientLibVersion = settings.clientLibVersion; this.serviceGeneratorName = settings.generatorName; diff --git a/src/test/java/com/google/api/gax/grpc/ServiceApiSettingsTest.java b/src/test/java/com/google/api/gax/grpc/ServiceApiSettingsTest.java new file mode 100644 index 000000000..13b383259 --- /dev/null +++ b/src/test/java/com/google/api/gax/grpc/ServiceApiSettingsTest.java @@ -0,0 +1,153 @@ +package com.google.api.gax.grpc; + +import com.google.api.gax.core.ConnectionSettings; +import com.google.common.collect.ImmutableList; +import com.google.common.truth.Truth; + +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; +import org.mockito.Mockito; + +import io.grpc.ManagedChannel; + +import java.io.IOException; +import java.util.concurrent.ScheduledExecutorService; + +/** + * Tests for {@link ServiceApiSettings}. + */ +@RunWith(JUnit4.class) +public class ServiceApiSettingsTest { + + @Rule public ExpectedException thrown = ExpectedException.none(); + + private static class FakeSettings extends ServiceApiSettings { + + public static final String DEFAULT_SERVICE_ADDRESS = "pubsub-experimental.googleapis.com"; + public static final int DEFAULT_SERVICE_PORT = 443; + public static final ImmutableList DEFAULT_SERVICE_SCOPES = + ImmutableList.builder() + .add("https://www.googleapis.com/auth/pubsub") + .add("https://www.googleapis.com/auth/cloud-platform") + .build(); + public static final ConnectionSettings DEFAULT_CONNECTION_SETTINGS = + ConnectionSettings.newBuilder() + .setServiceAddress(DEFAULT_SERVICE_ADDRESS) + .setPort(DEFAULT_SERVICE_PORT) + .provideCredentialsWith(DEFAULT_SERVICE_SCOPES) + .build(); + + public static Builder createBuilder(ConnectionSettings connectionSettings) { + return new Builder(connectionSettings); + } + + private FakeSettings(Builder settingsBuilder) throws IOException { + super( + settingsBuilder.getChannelProvider(), + settingsBuilder.getExecutorProvider(), + settingsBuilder.getGeneratorName(), + settingsBuilder.getGeneratorVersion(), + settingsBuilder.getClientLibName(), + settingsBuilder.getClientLibVersion()); + } + + private static class Builder extends ServiceApiSettings.Builder { + + private Builder(ConnectionSettings connectionSettings) { + super(connectionSettings); + } + + @Override + public FakeSettings build() throws IOException { + return new FakeSettings(this); + } + + @Override + public Builder provideExecutorWith( + final ScheduledExecutorService executor, boolean shouldAutoClose) { + super.provideExecutorWith(executor, shouldAutoClose); + return this; + } + + @Override + public Builder provideChannelWith(ManagedChannel channel, boolean shouldAutoClose) { + super.provideChannelWith(channel, shouldAutoClose); + return this; + } + + @Override + public Builder provideChannelWith(ConnectionSettings settings) { + super.provideChannelWith(settings); + return this; + } + } + } + + @Test + public void fixedChannelAutoClose() throws IOException { + thrown.expect(IllegalStateException.class); + ManagedChannel channel = Mockito.mock(ManagedChannel.class); + FakeSettings settings = + FakeSettings.createBuilder(FakeSettings.DEFAULT_CONNECTION_SETTINGS) + .provideChannelWith(channel, true) + .build(); + ChannelProvider channelProvider = settings.getChannelProvider(); + ScheduledExecutorService executor = settings.getExecutorProvider().getExecutor(); + ManagedChannel channelA = channelProvider.getChannel(executor); + ManagedChannel channelB = channelProvider.getChannel(executor); + } + + @Test + public void fixedChannelNoAutoClose() throws IOException { + ManagedChannel channel = Mockito.mock(ManagedChannel.class); + FakeSettings settings = + FakeSettings.createBuilder(FakeSettings.DEFAULT_CONNECTION_SETTINGS) + .provideChannelWith(channel, false) + .build(); + ChannelProvider channelProvider = settings.getChannelProvider(); + ScheduledExecutorService executor = settings.getExecutorProvider().getExecutor(); + ManagedChannel channelA = channelProvider.getChannel(executor); + ManagedChannel channelB = channelProvider.getChannel(executor); + Truth.assertThat(channelA).isEqualTo(channelB); + } + + @Test + public void defaultExecutor() throws IOException { + FakeSettings settings = + FakeSettings.createBuilder(FakeSettings.DEFAULT_CONNECTION_SETTINGS).build(); + ExecutorProvider executorProvider = settings.getExecutorProvider(); + ScheduledExecutorService executorA = executorProvider.getExecutor(); + ScheduledExecutorService executorB = executorProvider.getExecutor(); + Truth.assertThat(executorA).isNotEqualTo(executorB); + } + + @Test + public void fixedExecutorAutoClose() throws IOException { + thrown.expect(IllegalStateException.class); + ScheduledExecutorService executor = Mockito.mock(ScheduledExecutorService.class); + FakeSettings settings = + FakeSettings.createBuilder(FakeSettings.DEFAULT_CONNECTION_SETTINGS) + .provideExecutorWith(executor, true) + .build(); + ExecutorProvider executorProvider = settings.getExecutorProvider(); + ScheduledExecutorService executorA = executorProvider.getExecutor(); + ScheduledExecutorService executorB = executorProvider.getExecutor(); + } + + @Test + public void fixedExecutorNoAutoClose() throws IOException { + ScheduledExecutorService executor = Mockito.mock(ScheduledExecutorService.class); + FakeSettings settings = + FakeSettings.createBuilder(FakeSettings.DEFAULT_CONNECTION_SETTINGS) + .provideExecutorWith(executor, false) + .build(); + ExecutorProvider executorProvider = settings.getExecutorProvider(); + ScheduledExecutorService executorA = executorProvider.getExecutor(); + ScheduledExecutorService executorB = executorProvider.getExecutor(); + Truth.assertThat(executorA).isEqualTo(executorB); + } +} + From f0ea68c2470a9bbce28d3438d9a845e0dfa312d9 Mon Sep 17 00:00:00 2001 From: Michael Bausor Date: Thu, 28 Apr 2016 12:57:11 -0700 Subject: [PATCH 7/9] Renamed getChannel/getExecutor methods for providers Pre-push hook installed. Change-Id: Icd5d157d0d164f8eb2536778fbafbea7c5c546a2 --- .../api/gax/core/ConnectionSettings.java | 2 +- .../google/api/gax/grpc/ChannelProvider.java | 14 +++++------ .../google/api/gax/grpc/ExecutorProvider.java | 21 ++++++++-------- .../api/gax/grpc/ServiceApiSettings.java | 10 ++++---- .../api/gax/grpc/ServiceApiSettingsTest.java | 24 +++++++++---------- 5 files changed, 36 insertions(+), 35 deletions(-) diff --git a/src/main/java/com/google/api/gax/core/ConnectionSettings.java b/src/main/java/com/google/api/gax/core/ConnectionSettings.java index 3000cc519..1098ffb17 100644 --- a/src/main/java/com/google/api/gax/core/ConnectionSettings.java +++ b/src/main/java/com/google/api/gax/core/ConnectionSettings.java @@ -60,7 +60,7 @@ public abstract class ConnectionSettings { * Gets the credentials which will be used to call the service. If the credentials have not been * acquired yet, then they will be acquired when this function is called. */ - public Credentials getOrBuildCredentials() throws IOException { + public Credentials getCredentials() throws IOException { return getCredentialsProvider().getCredentials(); } diff --git a/src/main/java/com/google/api/gax/grpc/ChannelProvider.java b/src/main/java/com/google/api/gax/grpc/ChannelProvider.java index 3b0020e47..957d7de4f 100644 --- a/src/main/java/com/google/api/gax/grpc/ChannelProvider.java +++ b/src/main/java/com/google/api/gax/grpc/ChannelProvider.java @@ -11,16 +11,16 @@ /** * Provides an interface to hold and build the channel that will be used. If the channel does not - * already exist, it will be constructed when {@link #getChannel} is called. + * already exist, it will be constructed when {@link #getOrBuildChannel} is called. * * Implementations of {@link ChannelProvider} may choose to create a new {@link ManagedChannel} for - * each call to {@link #getChannel}, or may return a fixed {@link ManagedChannel} instance. In cases - * where the same {@link ManagedChannel} instance is returned, for example by a + * each call to {@link #getOrBuildChannel}, or may return a fixed {@link ManagedChannel} instance. + * In cases where the same {@link ManagedChannel} instance is returned, for example by a * {@link ChannelProvider} created using the {@link ServiceApiSettings} * provideChannelWith(ManagedChannel, boolean) method, and shouldAutoClose returns true, the - * {@link #getChannel} method will throw an {@link IllegalStateException} if it is called more than - * once. This is to prevent the same {@link ManagedChannel} being closed prematurely when it is used - * by multiple client objects. + * {@link #getOrBuildChannel} method will throw an {@link IllegalStateException} if it is called + * more than once. This is to prevent the same {@link ManagedChannel} being closed prematurely when + * it is used by multiple client objects. */ public interface ChannelProvider { /** @@ -44,5 +44,5 @@ public interface ChannelProvider { * subsequent calls should throw an {@link IllegalStateException}. See interface level docs for * {@link ChannelProvider} for more details. */ - ManagedChannel getChannel(Executor executor) throws IOException, IllegalStateException; + ManagedChannel getOrBuildChannel(Executor executor) throws IOException, IllegalStateException; } diff --git a/src/main/java/com/google/api/gax/grpc/ExecutorProvider.java b/src/main/java/com/google/api/gax/grpc/ExecutorProvider.java index 03d4ce054..e9f610d41 100644 --- a/src/main/java/com/google/api/gax/grpc/ExecutorProvider.java +++ b/src/main/java/com/google/api/gax/grpc/ExecutorProvider.java @@ -4,16 +4,17 @@ /** * Provides an interface to hold and create the Executor to be used. If the executor does not - * already exist, it will be constructed when {@link #getExecutor} is called. + * already exist, it will be constructed when {@link #getOrBuildExecutor} is called. * * Implementations of ExecutorProvider may choose to create a new {@link ScheduledExecutorService} - * for each call to {@link #getExecutor}, or may return a fixed {@link ScheduledExecutorService} - * instance. In cases where the same {@link ScheduledExecutorService} instance is returned, for - * example by an {@link ExecutorProvider} created using the {@link ServiceApiSettings} - * provideExecutorWith(ScheduledExecutorService, boolean) method, and shouldAutoClose returns true, - * the {@link #getExecutor} method will throw an {@link IllegalStateException} if it is called more - * than once. This is to prevent the same {@link ScheduledExecutorService} being closed prematurely - * when it is used by multiple client objects. + * for each call to {@link #getOrBuildExecutor}, or may return a fixed + * {@link ScheduledExecutorService} instance. In cases where the same + * {@link ScheduledExecutorService} instance is returned, for example by an {@link ExecutorProvider} + * created using the {@link ServiceApiSettings} provideExecutorWith(ScheduledExecutorService, + * boolean) method, and shouldAutoClose returns true, the {@link #getOrBuildExecutor} method will + * throw an {@link IllegalStateException} if it is called more than once. This is to prevent the + * same {@link ScheduledExecutorService} being closed prematurely when it is used by multiple client + * objects. */ public interface ExecutorProvider { /** @@ -27,9 +28,9 @@ public interface ExecutorProvider { * * If the {@link ExecutorProvider} is configured to return a fixed * {@link ScheduledExecutorService} object and to return shouldAutoClose as true, then after the - * first call to {@link #getExecutor}, subsequent calls should throw an + * first call to {@link #getOrBuildExecutor}, subsequent calls should throw an * {@link IllegalStateException}. See interface level docs for {@link ExecutorProvider} for more * details. */ - ScheduledExecutorService getExecutor() throws IllegalStateException; + ScheduledExecutorService getOrBuildExecutor() throws IllegalStateException; } diff --git a/src/main/java/com/google/api/gax/grpc/ServiceApiSettings.java b/src/main/java/com/google/api/gax/grpc/ServiceApiSettings.java index 58e2e8b9c..3d68f58b0 100644 --- a/src/main/java/com/google/api/gax/grpc/ServiceApiSettings.java +++ b/src/main/java/com/google/api/gax/grpc/ServiceApiSettings.java @@ -136,7 +136,7 @@ private Builder() { executorProvider = new ExecutorProvider() { @Override - public ScheduledExecutorService getExecutor() { + public ScheduledExecutorService getOrBuildExecutor() { return MoreExecutors.getExitingScheduledExecutorService( new ScheduledThreadPoolExecutor(DEFAULT_EXECUTOR_THREADS)); } @@ -162,7 +162,7 @@ public Builder provideExecutorWith( private volatile boolean executorProvided = false; @Override - public ScheduledExecutorService getExecutor() throws IllegalStateException { + public ScheduledExecutorService getOrBuildExecutor() throws IllegalStateException { if (executorProvided) { if (shouldAutoClose) { throw new IllegalStateException( @@ -275,9 +275,9 @@ protected Builder applyToAllApiMethods( private ChannelProvider createChannelProvider(final ConnectionSettings settings) { return new ChannelProvider() { @Override - public ManagedChannel getChannel(Executor executor) throws IOException { + public ManagedChannel getOrBuildChannel(Executor executor) throws IOException { List interceptors = Lists.newArrayList(); - interceptors.add(new ClientAuthInterceptor(settings.getOrBuildCredentials(), executor)); + interceptors.add(new ClientAuthInterceptor(settings.getCredentials(), executor)); interceptors.add(new HeaderInterceptor(serviceHeader())); return NettyChannelBuilder.forAddress(settings.getServiceAddress(), settings.getPort()) @@ -322,7 +322,7 @@ private ChannelProvider createChannelProvider(final ManagedChannel channel, private boolean channelProvided = false; @Override - public ManagedChannel getChannel(Executor executor) throws IllegalStateException { + public ManagedChannel getOrBuildChannel(Executor executor) throws IllegalStateException { if (channelProvided) { if (shouldAutoClose) { throw new IllegalStateException( diff --git a/src/test/java/com/google/api/gax/grpc/ServiceApiSettingsTest.java b/src/test/java/com/google/api/gax/grpc/ServiceApiSettingsTest.java index 13b383259..73ff623a9 100644 --- a/src/test/java/com/google/api/gax/grpc/ServiceApiSettingsTest.java +++ b/src/test/java/com/google/api/gax/grpc/ServiceApiSettingsTest.java @@ -95,9 +95,9 @@ public void fixedChannelAutoClose() throws IOException { .provideChannelWith(channel, true) .build(); ChannelProvider channelProvider = settings.getChannelProvider(); - ScheduledExecutorService executor = settings.getExecutorProvider().getExecutor(); - ManagedChannel channelA = channelProvider.getChannel(executor); - ManagedChannel channelB = channelProvider.getChannel(executor); + ScheduledExecutorService executor = settings.getExecutorProvider().getOrBuildExecutor(); + ManagedChannel channelA = channelProvider.getOrBuildChannel(executor); + ManagedChannel channelB = channelProvider.getOrBuildChannel(executor); } @Test @@ -108,9 +108,9 @@ public void fixedChannelNoAutoClose() throws IOException { .provideChannelWith(channel, false) .build(); ChannelProvider channelProvider = settings.getChannelProvider(); - ScheduledExecutorService executor = settings.getExecutorProvider().getExecutor(); - ManagedChannel channelA = channelProvider.getChannel(executor); - ManagedChannel channelB = channelProvider.getChannel(executor); + ScheduledExecutorService executor = settings.getExecutorProvider().getOrBuildExecutor(); + ManagedChannel channelA = channelProvider.getOrBuildChannel(executor); + ManagedChannel channelB = channelProvider.getOrBuildChannel(executor); Truth.assertThat(channelA).isEqualTo(channelB); } @@ -119,8 +119,8 @@ public void defaultExecutor() throws IOException { FakeSettings settings = FakeSettings.createBuilder(FakeSettings.DEFAULT_CONNECTION_SETTINGS).build(); ExecutorProvider executorProvider = settings.getExecutorProvider(); - ScheduledExecutorService executorA = executorProvider.getExecutor(); - ScheduledExecutorService executorB = executorProvider.getExecutor(); + ScheduledExecutorService executorA = executorProvider.getOrBuildExecutor(); + ScheduledExecutorService executorB = executorProvider.getOrBuildExecutor(); Truth.assertThat(executorA).isNotEqualTo(executorB); } @@ -133,8 +133,8 @@ public void fixedExecutorAutoClose() throws IOException { .provideExecutorWith(executor, true) .build(); ExecutorProvider executorProvider = settings.getExecutorProvider(); - ScheduledExecutorService executorA = executorProvider.getExecutor(); - ScheduledExecutorService executorB = executorProvider.getExecutor(); + ScheduledExecutorService executorA = executorProvider.getOrBuildExecutor(); + ScheduledExecutorService executorB = executorProvider.getOrBuildExecutor(); } @Test @@ -145,8 +145,8 @@ public void fixedExecutorNoAutoClose() throws IOException { .provideExecutorWith(executor, false) .build(); ExecutorProvider executorProvider = settings.getExecutorProvider(); - ScheduledExecutorService executorA = executorProvider.getExecutor(); - ScheduledExecutorService executorB = executorProvider.getExecutor(); + ScheduledExecutorService executorA = executorProvider.getOrBuildExecutor(); + ScheduledExecutorService executorB = executorProvider.getOrBuildExecutor(); Truth.assertThat(executorA).isEqualTo(executorB); } } From 1992d364a07605e82e8bee1974afde54b244e832 Mon Sep 17 00:00:00 2001 From: Michael Bausor Date: Thu, 28 Apr 2016 13:19:14 -0700 Subject: [PATCH 8/9] Renamed getX methods in interfaces, removed throws IllegalState Pre-push hook installed. Change-Id: I10039ae000dae902b30a9df43fdd5c1316191281 --- src/main/java/com/google/api/gax/grpc/ChannelProvider.java | 4 ++-- src/main/java/com/google/api/gax/grpc/ExecutorProvider.java | 2 +- src/main/java/com/google/api/gax/grpc/ServiceApiSettings.java | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/google/api/gax/grpc/ChannelProvider.java b/src/main/java/com/google/api/gax/grpc/ChannelProvider.java index 957d7de4f..8beed2f03 100644 --- a/src/main/java/com/google/api/gax/grpc/ChannelProvider.java +++ b/src/main/java/com/google/api/gax/grpc/ChannelProvider.java @@ -40,9 +40,9 @@ public interface ChannelProvider { * channel does not already exist, it will be created. * * If the {@link ChannelProvider} is configured to return a fixed {@link ManagedChannel} object - * and to return shouldAutoClose as true, then after the first call to {@link #getChannel}, + * and to return shouldAutoClose as true, then after the first call to {@link #getOrBuildChannel}, * subsequent calls should throw an {@link IllegalStateException}. See interface level docs for * {@link ChannelProvider} for more details. */ - ManagedChannel getOrBuildChannel(Executor executor) throws IOException, IllegalStateException; + ManagedChannel getOrBuildChannel(Executor executor) throws IOException; } diff --git a/src/main/java/com/google/api/gax/grpc/ExecutorProvider.java b/src/main/java/com/google/api/gax/grpc/ExecutorProvider.java index e9f610d41..61204d260 100644 --- a/src/main/java/com/google/api/gax/grpc/ExecutorProvider.java +++ b/src/main/java/com/google/api/gax/grpc/ExecutorProvider.java @@ -32,5 +32,5 @@ public interface ExecutorProvider { * {@link IllegalStateException}. See interface level docs for {@link ExecutorProvider} for more * details. */ - ScheduledExecutorService getOrBuildExecutor() throws IllegalStateException; + ScheduledExecutorService getOrBuildExecutor(); } diff --git a/src/main/java/com/google/api/gax/grpc/ServiceApiSettings.java b/src/main/java/com/google/api/gax/grpc/ServiceApiSettings.java index 3d68f58b0..bebcf7250 100644 --- a/src/main/java/com/google/api/gax/grpc/ServiceApiSettings.java +++ b/src/main/java/com/google/api/gax/grpc/ServiceApiSettings.java @@ -162,7 +162,7 @@ public Builder provideExecutorWith( private volatile boolean executorProvided = false; @Override - public ScheduledExecutorService getOrBuildExecutor() throws IllegalStateException { + public ScheduledExecutorService getOrBuildExecutor() { if (executorProvided) { if (shouldAutoClose) { throw new IllegalStateException( @@ -322,7 +322,7 @@ private ChannelProvider createChannelProvider(final ManagedChannel channel, private boolean channelProvided = false; @Override - public ManagedChannel getOrBuildChannel(Executor executor) throws IllegalStateException { + public ManagedChannel getOrBuildChannel(Executor executor) { if (channelProvided) { if (shouldAutoClose) { throw new IllegalStateException( From 8f34a6ec38bd72f7fc74819484ca390a90e1e911 Mon Sep 17 00:00:00 2001 From: Michael Bausor Date: Thu, 28 Apr 2016 14:37:07 -0700 Subject: [PATCH 9/9] Updated comments Pre-push hook installed. Change-Id: I4ca6d7d72ad1941846bb9afcb68ee7d518317e3a --- src/main/java/com/google/api/gax/grpc/ChannelProvider.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/google/api/gax/grpc/ChannelProvider.java b/src/main/java/com/google/api/gax/grpc/ChannelProvider.java index 8beed2f03..6fec85763 100644 --- a/src/main/java/com/google/api/gax/grpc/ChannelProvider.java +++ b/src/main/java/com/google/api/gax/grpc/ChannelProvider.java @@ -37,7 +37,9 @@ public interface ChannelProvider { /** * Get the channel to be used to connect to the service. The first time this is called, if the - * channel does not already exist, it will be created. + * channel does not already exist, it will be created. The {@link Executor} will only be used when + * the channel is created. For implementations returning a fixed {@link ManagedChannel} object, + * the executor is unused. * * If the {@link ChannelProvider} is configured to return a fixed {@link ManagedChannel} object * and to return shouldAutoClose as true, then after the first call to {@link #getOrBuildChannel},