Use a builder to eliminate BinderServer's long list of ctor params (#11235)

binder/src/androidTest/java/io/grpc/binder/internal/BinderTransportTest.java

@@ -24,7 +24,6 @@ import io.grpc.ServerStreamTracer;
 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 @@ public final class BinderTransportTest extends AbstractTransportTest {
   protected InternalServer newServer(List<ServerStreamTracer.Factory> streamTracerFactories) {
     AndroidComponentAddress addr = HostServices.allocateService(appContext);
 
-    BinderServer binderServer = new BinderServer(addr,
+    BinderServer binderServer = new BinderServer.Builder()
-        executorServicePool,
+        .setListenAddress(addr)
-        streamTracerFactories,
+        .setExecutorServicePool(executorServicePool)
-        BinderInternal.createPolicyChecker(SecurityPolicies.serverInternalOnly()),
+        .setStreamTracerFactories(streamTracerFactories)
-        InboundParcelablePolicy.DEFAULT,
+        .build();
-        /* transportSecurityShutdownListener=*/ () -> {});
 
     HostServices.configureService(addr,
         HostServices.serviceParamsBuilder()

binder/src/main/java/io/grpc/binder/BinderServerBuilder.java

@@ -29,18 +29,13 @@ import io.grpc.ServerBuilder;
 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 @@ public final class BinderServerBuilder
   }
 
   private final ServerImplBuilder serverImplBuilder;
-  private ObjectPool<ScheduledExecutorService> schedulerPool =
+  private final BinderServer.Builder internalBuilder = new BinderServer.Builder();
-      SharedResourcePool.forResource(GrpcUtil.TIMER_SERVICE);
-  private ServerSecurityPolicy securityPolicy;
-  private InboundParcelablePolicy inboundParcelablePolicy;
   private boolean isBuilt;
-  @Nullable private BinderTransportSecurity.ShutdownListener shutdownListener = null;
 
   private BinderServerBuilder(
       AndroidComponentAddress listenAddress,
       IBinderReceiver binderReceiver) {
-    securityPolicy = SecurityPolicies.serverInternalOnly();
+    internalBuilder.setListenAddress(listenAddress);
-    inboundParcelablePolicy = InboundParcelablePolicy.DEFAULT;
 
     serverImplBuilder = new ServerImplBuilder(streamTracerFactories -> {
-      BinderServer server = new BinderServer(
+      internalBuilder.setStreamTracerFactories(streamTracerFactories);
-          listenAddress,
+      BinderServer server = internalBuilder.build();
-          schedulerPool,
-          streamTracerFactories,
-          BinderInternal.createPolicyChecker(securityPolicy),
-          inboundParcelablePolicy,
-          // 'shutdownListener' should have been set by build()
-          checkNotNull(shutdownListener));
       BinderInternal.setIBinder(binderReceiver, server.getHostBinder());
       return server;
     });
@@ -132,8 +116,8 @@ public final class BinderServerBuilder
    */
   public BinderServerBuilder scheduledExecutorService(
       ScheduledExecutorService scheduledExecutorService) {
-     schedulerPool =
+     internalBuilder.setExecutorServicePool(
-          new FixedObjectPool<>(checkNotNull(scheduledExecutorService, "scheduledExecutorService"));
+          new FixedObjectPool<>(checkNotNull(scheduledExecutorService, "scheduledExecutorService")));
     return this;
   }
 
@@ -146,7 +130,7 @@ public final class BinderServerBuilder
    * @return this
    */
   public BinderServerBuilder securityPolicy(ServerSecurityPolicy securityPolicy) {
-    this.securityPolicy = checkNotNull(securityPolicy, "securityPolicy");
+    internalBuilder.setServerSecurityPolicy(securityPolicy);
     return this;
   }
 
@@ -154,7 +138,7 @@ public final class BinderServerBuilder
   @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 @@ public final class BinderServerBuilder
    *
    * @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 @@ public final class BinderServerBuilder
     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();
   }
 }

binder/src/main/java/io/grpc/binder/internal/BinderServer.java

@@ -28,10 +28,15 @@ import io.grpc.InternalChannelz.SocketStats;
 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 @@ public final class BinderServer implements InternalServer, LeakSafeOneWayBinder.
   @GuardedBy("this")
   private boolean shutdown;
 
-  /**
+  private BinderServer(Builder builder) {
-   * @param transportSecurityShutdownListener represents resources that should be cleaned up once
+    this.listenAddress = checkNotNull(builder.listenAddress);
-   *                                          the server shuts down.
+    this.executorServicePool = builder.executorServicePool;
-   */
-  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;
     this.streamTracerFactories =
-        ImmutableList.copyOf(checkNotNull(streamTracerFactories, "streamTracerFactories"));
+        ImmutableList.copyOf(checkNotNull(builder.streamTracerFactories, "streamTracerFactories"));
-    this.serverPolicyChecker = checkNotNull(serverPolicyChecker, "serverPolicyChecker");
+    this.serverPolicyChecker = BinderInternal.createPolicyChecker(builder.serverSecurityPolicy);
-    this.inboundParcelablePolicy = inboundParcelablePolicy;
+    this.inboundParcelablePolicy = builder.inboundParcelablePolicy;
-    this.transportSecurityShutdownListener = transportSecurityShutdownListener;
+    this.transportSecurityShutdownListener = builder.shutdownListener;
     hostServiceBinder = new LeakSafeOneWayBinder(this);
   }
 
@@ -169,4 +164,84 @@ public final class BinderServer implements InternalServer, LeakSafeOneWayBinder.
     }
     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;
+    }
+  }
 }