api: change ConfigSelector.Result to use callback instead of interceptor

We found that the interceptor approach for `ConfigSelector` would be adding a layer of indirection for no gain: The API Result selectConfig(LoadBalancer.PickSubchannelArgs args) consumes headers among other inputs, because route matching might need to match the headers; and the API produces ClientInterceptor among other outputs. But the headers is not available until clientCall.start(listner, headers), whereas the interceptor need be applied to the call before clientCall.start(). So the input is not available until the output is applied. That means we will need to delay calling the downstream newCall() (which will either be RealChannel or the interceptor) until start() is called.

So we want to change to the other approach similar to what c-core is taking: Have `Result(Object config, CallOptions, Runnable committedCallback)`, where CallOption is the selector modified CallOption, and committedCallback is used to monitor the call lifecycle.

api/src/main/java/io/grpc/InternalConfigSelector.java

@@ -38,12 +38,14 @@ public abstract class InternalConfigSelector {
 
   public static final class Result {
     private final Object config;
+    private final CallOptions callOptions;
     @Nullable
-    private final ClientInterceptor interceptor;
+    private final Runnable committedCallback;
 
-    private Result(Object config, @Nullable ClientInterceptor interceptor) {
+    private Result(Object config, CallOptions callOptions, @Nullable Runnable committedCallback) {
       this.config = checkNotNull(config, "config");
-      this.interceptor = interceptor;
+      this.callOptions = checkNotNull(callOptions, "callOptions");
+      this.committedCallback = committedCallback;
     }
 
     /**
@@ -55,12 +57,18 @@ public abstract class InternalConfigSelector {
     }
 
     /**
-     * Returns an interceptor that would be used to modify CallOptions, in addition to monitoring
-     * call lifecycle.
+     * Returns a config-selector-modified CallOptions for the RPC.
+     */
+    public CallOptions getCallOptions() {
+      return callOptions;
+    }
+
+    /**
+     * Returns a callback to be invoked when the RPC no longer needs a picker.
      */
     @Nullable
-    public ClientInterceptor getInterceptor() {
-      return interceptor;
+    public Runnable getCommittedCallback() {
+      return committedCallback;
     }
 
     public static Builder newBuilder() {
@@ -69,7 +77,8 @@ public abstract class InternalConfigSelector {
 
     public static final class Builder {
       private Object config;
-      private ClientInterceptor interceptor;
+      private CallOptions callOptions;
+      private Runnable committedCallback;
 
       private Builder() {}
 
@@ -83,18 +92,28 @@ public abstract class InternalConfigSelector {
         return this;
       }
 
+      /**
+       * Sets the CallOptions.
+       *
+       * @return this
+       */
+      public Builder setCallOptions(CallOptions callOptions) {
+        this.callOptions = checkNotNull(callOptions, "callOptions");
+        return this;
+      }
+
       /**
        * Sets the interceptor.
        *
        * @return this
        */
-      public Builder setInterceptor(@Nullable ClientInterceptor interceptor) {
-        this.interceptor = interceptor;
+      public Builder setCommittedCallback(@Nullable Runnable committedCallback) {
+        this.committedCallback = committedCallback;
         return this;
       }
 
       public Result build() {
-        return new Result(config, interceptor);
+        return new Result(config, callOptions, committedCallback);
       }
     }
   }

api/src/test/java/io/grpc/InternalConfigSelectorTest.java

@@ -17,7 +17,6 @@
 package io.grpc;
 
 import static com.google.common.truth.Truth.assertThat;
-import static org.mockito.Mockito.mock;
 
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -29,15 +28,26 @@ public class InternalConfigSelectorTest {
   @Test
   public void resultBuilder() {
     Object config = "fake_config";
-    ClientInterceptor interceptor = mock(ClientInterceptor.class);
+    CallOptions callOptions = CallOptions.DEFAULT.withAuthority("fake authority");
+    Runnable committedCallback = new Runnable() {
+      @Override
+      public void run() {}
+    };
     InternalConfigSelector.Result.Builder builder = InternalConfigSelector.Result.newBuilder();
 
-    InternalConfigSelector.Result result = builder.setConfig(config).build();
+    InternalConfigSelector.Result result =
+        builder.setConfig(config).setCallOptions(callOptions).build();
     assertThat(result.getConfig()).isEqualTo(config);
-    assertThat(result.getInterceptor()).isNull();
+    assertThat(result.getCallOptions()).isEqualTo(callOptions);
+    assertThat(result.getCommittedCallback()).isNull();
 
-    result = builder.setConfig(config).setInterceptor(interceptor).build();
+    result = builder
+        .setConfig(config)
+        .setCallOptions(callOptions)
+        .setCommittedCallback(committedCallback)
+        .build();
     assertThat(result.getConfig()).isEqualTo(config);
-    assertThat(result.getInterceptor()).isSameInstanceAs(interceptor);
+    assertThat(result.getCallOptions()).isEqualTo(callOptions);
+    assertThat(result.getCommittedCallback()).isSameInstanceAs(committedCallback);
   }
 }