Use a builder to eliminate BinderServer's long list of ctor params (#11235)
diff --git a/binder/src/androidTest/java/io/grpc/binder/internal/BinderTransportTest.java b/binder/src/androidTest/java/io/grpc/binder/internal/BinderTransportTest.java index 91e66c5..a78eb28 100644 --- a/binder/src/androidTest/java/io/grpc/binder/internal/BinderTransportTest.java +++ b/binder/src/androidTest/java/io/grpc/binder/internal/BinderTransportTest.java
@@ -24,7 +24,6 @@ import io.grpc.binder.AndroidComponentAddress; import io.grpc.binder.BindServiceFlags; import io.grpc.binder.BinderChannelCredentials; -import io.grpc.binder.BinderInternal; import io.grpc.binder.HostServices; import io.grpc.binder.InboundParcelablePolicy; import io.grpc.binder.SecurityPolicies; @@ -70,12 +69,11 @@ protected InternalServer newServer(List<ServerStreamTracer.Factory> streamTracerFactories) { AndroidComponentAddress addr = HostServices.allocateService(appContext); - BinderServer binderServer = new BinderServer(addr, - executorServicePool, - streamTracerFactories, - BinderInternal.createPolicyChecker(SecurityPolicies.serverInternalOnly()), - InboundParcelablePolicy.DEFAULT, - /* transportSecurityShutdownListener=*/ () -> {}); + BinderServer binderServer = new BinderServer.Builder() + .setListenAddress(addr) + .setExecutorServicePool(executorServicePool) + .setStreamTracerFactories(streamTracerFactories) + .build(); HostServices.configureService(addr, HostServices.serviceParamsBuilder()
diff --git a/binder/src/main/java/io/grpc/binder/BinderServerBuilder.java b/binder/src/main/java/io/grpc/binder/BinderServerBuilder.java index 158f794..af5f9ee 100644 --- a/binder/src/main/java/io/grpc/binder/BinderServerBuilder.java +++ b/binder/src/main/java/io/grpc/binder/BinderServerBuilder.java
@@ -29,18 +29,13 @@ import io.grpc.binder.internal.BinderServer; import io.grpc.binder.internal.BinderTransportSecurity; import io.grpc.internal.FixedObjectPool; -import io.grpc.internal.GrpcUtil; import io.grpc.internal.ServerImplBuilder; import io.grpc.internal.ObjectPool; -import io.grpc.internal.SharedResourcePool; -import java.io.Closeable; import java.io.File; import java.util.concurrent.Executor; import java.util.concurrent.ScheduledExecutorService; -import javax.annotation.Nullable; - /** * Builder for a server that services requests from an Android Service. */ @@ -72,28 +67,17 @@ } private final ServerImplBuilder serverImplBuilder; - private ObjectPool<ScheduledExecutorService> schedulerPool = - SharedResourcePool.forResource(GrpcUtil.TIMER_SERVICE); - private ServerSecurityPolicy securityPolicy; - private InboundParcelablePolicy inboundParcelablePolicy; + private final BinderServer.Builder internalBuilder = new BinderServer.Builder(); private boolean isBuilt; - @Nullable private BinderTransportSecurity.ShutdownListener shutdownListener = null; private BinderServerBuilder( AndroidComponentAddress listenAddress, IBinderReceiver binderReceiver) { - securityPolicy = SecurityPolicies.serverInternalOnly(); - inboundParcelablePolicy = InboundParcelablePolicy.DEFAULT; + internalBuilder.setListenAddress(listenAddress); serverImplBuilder = new ServerImplBuilder(streamTracerFactories -> { - BinderServer server = new BinderServer( - listenAddress, - schedulerPool, - streamTracerFactories, - BinderInternal.createPolicyChecker(securityPolicy), - inboundParcelablePolicy, - // 'shutdownListener' should have been set by build() - checkNotNull(shutdownListener)); + internalBuilder.setStreamTracerFactories(streamTracerFactories); + BinderServer server = internalBuilder.build(); BinderInternal.setIBinder(binderReceiver, server.getHostBinder()); return server; }); @@ -132,8 +116,8 @@ */ public BinderServerBuilder scheduledExecutorService( ScheduledExecutorService scheduledExecutorService) { - schedulerPool = - new FixedObjectPool<>(checkNotNull(scheduledExecutorService, "scheduledExecutorService")); + internalBuilder.setExecutorServicePool( + new FixedObjectPool<>(checkNotNull(scheduledExecutorService, "scheduledExecutorService"))); return this; } @@ -146,7 +130,7 @@ * @return this */ public BinderServerBuilder securityPolicy(ServerSecurityPolicy securityPolicy) { - this.securityPolicy = checkNotNull(securityPolicy, "securityPolicy"); + internalBuilder.setServerSecurityPolicy(securityPolicy); return this; } @@ -154,7 +138,7 @@ @ExperimentalApi("https://github.com/grpc/grpc-java/issues/8022") public BinderServerBuilder inboundParcelablePolicy( InboundParcelablePolicy inboundParcelablePolicy) { - this.inboundParcelablePolicy = checkNotNull(inboundParcelablePolicy, "inboundParcelablePolicy"); + internalBuilder.setInboundParcelablePolicy(inboundParcelablePolicy); return this; } @@ -173,7 +157,7 @@ * * @return the new Server */ - @Override // For javadoc refinement only. + @Override public Server build() { // Since we install a final interceptor here, we need to ensure we're only built once. checkState(!isBuilt, "BinderServerBuilder can only be used to build one server instance."); @@ -182,7 +166,7 @@ ObjectPool<? extends Executor> executorPool = serverImplBuilder.getExecutorPool(); Executor executor = executorPool.getObject(); BinderTransportSecurity.installAuthInterceptor(this, executor); - shutdownListener = () -> executorPool.returnObject(executor); + internalBuilder.setShutdownListener(() -> executorPool.returnObject(executor)); return super.build(); } }
diff --git a/binder/src/main/java/io/grpc/binder/internal/BinderServer.java b/binder/src/main/java/io/grpc/binder/internal/BinderServer.java index 72faa33..03af19c 100644 --- a/binder/src/main/java/io/grpc/binder/internal/BinderServer.java +++ b/binder/src/main/java/io/grpc/binder/internal/BinderServer.java
@@ -28,10 +28,15 @@ import io.grpc.InternalInstrumented; import io.grpc.ServerStreamTracer; import io.grpc.binder.AndroidComponentAddress; +import io.grpc.binder.BinderInternal; import io.grpc.binder.InboundParcelablePolicy; +import io.grpc.binder.SecurityPolicies; +import io.grpc.binder.ServerSecurityPolicy; +import io.grpc.internal.GrpcUtil; import io.grpc.internal.InternalServer; import io.grpc.internal.ObjectPool; import io.grpc.internal.ServerListener; +import io.grpc.internal.SharedResourcePool; import java.io.IOException; import java.net.SocketAddress; import java.util.List; @@ -68,24 +73,14 @@ @GuardedBy("this") private boolean shutdown; - /** - * @param transportSecurityShutdownListener represents resources that should be cleaned up once - * the server shuts down. - */ - public BinderServer( - AndroidComponentAddress listenAddress, - ObjectPool<ScheduledExecutorService> executorServicePool, - List<? extends ServerStreamTracer.Factory> streamTracerFactories, - BinderTransportSecurity.ServerPolicyChecker serverPolicyChecker, - InboundParcelablePolicy inboundParcelablePolicy, - BinderTransportSecurity.ShutdownListener transportSecurityShutdownListener) { - this.listenAddress = listenAddress; - this.executorServicePool = executorServicePool; + private BinderServer(Builder builder) { + this.listenAddress = checkNotNull(builder.listenAddress); + this.executorServicePool = builder.executorServicePool; this.streamTracerFactories = - ImmutableList.copyOf(checkNotNull(streamTracerFactories, "streamTracerFactories")); - this.serverPolicyChecker = checkNotNull(serverPolicyChecker, "serverPolicyChecker"); - this.inboundParcelablePolicy = inboundParcelablePolicy; - this.transportSecurityShutdownListener = transportSecurityShutdownListener; + ImmutableList.copyOf(checkNotNull(builder.streamTracerFactories, "streamTracerFactories")); + this.serverPolicyChecker = BinderInternal.createPolicyChecker(builder.serverSecurityPolicy); + this.inboundParcelablePolicy = builder.inboundParcelablePolicy; + this.transportSecurityShutdownListener = builder.shutdownListener; hostServiceBinder = new LeakSafeOneWayBinder(this); } @@ -169,4 +164,84 @@ } return false; } + + /** Fluent builder of {@link BinderServer} instances. */ + public static class Builder { + @Nullable AndroidComponentAddress listenAddress; + @Nullable List<? extends ServerStreamTracer.Factory> streamTracerFactories; + + ObjectPool<ScheduledExecutorService> executorServicePool = + SharedResourcePool.forResource(GrpcUtil.TIMER_SERVICE); + ServerSecurityPolicy serverSecurityPolicy = SecurityPolicies.serverInternalOnly(); + InboundParcelablePolicy inboundParcelablePolicy = InboundParcelablePolicy.DEFAULT; + BinderTransportSecurity.ShutdownListener shutdownListener = () -> {}; + + public BinderServer build() { + return new BinderServer(this); + } + + /** + * Sets the "listen" address for this server. + * + * <p>This is somewhat of a grpc-java formality. Binder servers don't really listen, rather, + * Android creates and destroys them according to client needs. + * + * <p>Required. + */ + public Builder setListenAddress(AndroidComponentAddress listenAddress) { + this.listenAddress = listenAddress; + return this; + } + + /** + * Sets the source for {@link ServerStreamTracer}s that will be installed on all new streams. + * + * <p>Required. + */ + public Builder setStreamTracerFactories(List<? extends ServerStreamTracer.Factory> streamTracerFactories) { + this.streamTracerFactories = streamTracerFactories; + return this; + } + + /** + * Sets the executor to be used for scheduling channel timers. + * + * <p>Optional. A process-wide default executor will be used if unset. + */ + public Builder setExecutorServicePool( + ObjectPool<ScheduledExecutorService> executorServicePool) { + this.executorServicePool = checkNotNull(executorServicePool, "executorServicePool"); + return this; + } + + /** + * Sets the {@link ServerSecurityPolicy} to be used for built servers. + * + * Optional, {@link SecurityPolicies#serverInternalOnly()} is the default. + */ + public Builder setServerSecurityPolicy(ServerSecurityPolicy serverSecurityPolicy) { + this.serverSecurityPolicy = checkNotNull(serverSecurityPolicy, "serverSecurityPolicy"); + return this; + } + + /** + * Sets the {@link InboundParcelablePolicy} to be used for built servers. + * + * Optional, {@link InboundParcelablePolicy#DEFAULT} is the default. + */ + public Builder setInboundParcelablePolicy(InboundParcelablePolicy inboundParcelablePolicy) { + this.inboundParcelablePolicy = checkNotNull(inboundParcelablePolicy, "inboundParcelablePolicy"); + return this; + } + + /** + * Installs a callback that will be invoked when this server is {@link #shutdown()} + * + * <p>Optional. + */ + public Builder setShutdownListener(BinderTransportSecurity.ShutdownListener shutdownListener) { + this.shutdownListener = checkNotNull(shutdownListener, "shutdownListener"); + return this; + } + } }