Move SubchannelStateListener to Subchannel.start().

Also stop passing Subchannel to SubchannelStateListener.  This is part
of the bigger change to remove Subchannel from all arguments on the
LoadBalancer API, so that wrapping of Subchannel won't cause identity
problems.  Details in #5676

Author: zhangkun83

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

@@ -663,22 +663,19 @@ public boolean equals(Object other) {
   /**
    * Arguments for {@link Helper#createSubchannel(CreateSubchannelArgs)}.
    *
-   * @since 1.21.0
+   * @since 1.22.0
    */
   @ExperimentalApi("https://github.com/grpc/grpc-java/issues/1771")
   public static final class CreateSubchannelArgs {
     private final List<EquivalentAddressGroup> addrs;
     private final Attributes attrs;
-    private final SubchannelStateListener stateListener;
     private final Object[][] customOptions;
 
     private CreateSubchannelArgs(
-        List<EquivalentAddressGroup> addrs, Attributes attrs, Object[][] customOptions,
-        SubchannelStateListener stateListener) {
+        List<EquivalentAddressGroup> addrs, Attributes attrs, Object[][] customOptions) {
       this.addrs = checkNotNull(addrs, "addresses are not set");
       this.attrs = checkNotNull(attrs, "attrs");
       this.customOptions = checkNotNull(customOptions, "customOptions");
-      this.stateListener = checkNotNull(stateListener, "SubchannelStateListener is not set");
     }
 
     /**
@@ -711,19 +708,11 @@ public <T> T getOption(Key<T> key) {
       return key.defaultValue;
     }
 
-    /**
-     * Returns the state listener.
-     */
-    public SubchannelStateListener getStateListener() {
-      return stateListener;
-    }
-
     /**
      * Returns a builder with the same initial values as this object.
      */
     public Builder toBuilder() {
-      return newBuilder().setAddresses(addrs).setAttributes(attrs).setStateListener(stateListener)
-          .copyCustomOptions(customOptions);
+      return newBuilder().setAddresses(addrs).setAttributes(attrs).copyCustomOptions(customOptions);
     }
 
     /**
@@ -738,7 +727,6 @@ public String toString() {
       return MoreObjects.toStringHelper(this)
           .add("addrs", addrs)
           .add("attrs", attrs)
-          .add("listener", stateListener)
           .add("customOptions", Arrays.deepToString(customOptions))
           .toString();
     }
@@ -748,7 +736,6 @@ public static final class Builder {
 
       private List<EquivalentAddressGroup> addrs;
       private Attributes attrs = Attributes.EMPTY;
-      private SubchannelStateListener stateListener;
       private Object[][] customOptions = new Object[0][2];
 
       Builder() {
@@ -823,23 +810,11 @@ public Builder setAttributes(Attributes attrs) {
         return this;
       }
 
-      /**
-       * Receives state changes of the created Subchannel.  The listener is called from
-       * the {@link Helper#getSynchronizationContext Synchronization Context}.  It's safe to share
-       * the listener among multiple Subchannels.
-       *
-       * <p>This is a <strong>required</strong> property.
-       */
-      public Builder setStateListener(SubchannelStateListener listener) {
-        this.stateListener = checkNotNull(listener, "listener");
-        return this;
-      }
-
       /**
        * Creates a new args object.
        */
       public CreateSubchannelArgs build() {
-        return new CreateSubchannelArgs(addrs, attrs, customOptions, stateListener);
+        return new CreateSubchannelArgs(addrs, attrs, customOptions);
       }
     }
 
@@ -950,15 +925,10 @@ public Subchannel createSubchannel(List<EquivalentAddressGroup> addrs, Attribute
      * Subchannel, and can be accessed later through {@link Subchannel#getAttributes
      * Subchannel.getAttributes()}.
      *
-     * <p>This method <strong>must be called from the {@link #getSynchronizationContext
-     * Synchronization Context}</strong>, otherwise it may throw. This is to avoid the race between
-     * the caller and {@link SubchannelStateListener#onSubchannelState}.  See <a
-     * href="https://github.com/grpc/grpc-java/issues/5015">#5015</a> for more discussions.
-     *
      * <p>The LoadBalancer is responsible for closing unused Subchannels, and closing all
      * Subchannels within {@link #shutdown}.
      *
-     * @since 1.21.0
+     * @since 1.22.0
      */
     public Subchannel createSubchannel(CreateSubchannelArgs args) {
       throw new UnsupportedOperationException();
@@ -1154,6 +1124,20 @@ public ChannelLogger getChannelLogger() {
   @ThreadSafe
   @ExperimentalApi("https://github.com/grpc/grpc-java/issues/1771")
   public abstract static class Subchannel {
+    /**
+     * Starts the Subchannel.  Can only be called once.
+     *
+     * <p>This method <strong>must be called from the {@link #getSynchronizationContext
+     * Synchronization Context}</strong>, otherwise it may throw. This is to avoid the race between
+     * the caller and {@link SubchannelStateListener#onSubchannelState}.  See <a
+     * href="https://github.com/grpc/grpc-java/issues/5015">#5015</a> for more discussions.
+     *
+     * @param listener receives state updates for this Subchannel.
+     */
+    public void start(SubchannelStateListener listener) {
+      throw new UnsupportedOperationException("Not implemented");
+    }
+
     /**
      * Shuts down the Subchannel.  After this method is called, this Subchannel should no longer
      * be returned by the latest {@link SubchannelPicker picker}, and can be safely discarded.
@@ -1164,6 +1148,7 @@ public abstract static class Subchannel {
 
     /**
      * Asks the Subchannel to create a connection (aka transport), if there isn't an active one.
+     * Has no effect if {@link #start} has not been called or {@link #shutdown} has been called.
      *
      * @since 1.2.0
      */
@@ -1242,13 +1227,12 @@ public ChannelLogger getChannelLogger() {
   }
 
   /**
-   * Receives state changes for one or more {@link Subchannel}s. All methods are run under {@link
+   * Receives state changes for one {@link Subchannel}. All methods are run under {@link
    * Helper#getSynchronizationContext}.
    *
-   * @since 1.21.0
+   * @since 1.22.0
    */
   public interface SubchannelStateListener {
-    
     /**
      * Handles a state change on a Subchannel.
      *
@@ -1266,12 +1250,11 @@ public interface SubchannelStateListener {
      * already terminated, thus there won't be further requests to LoadBalancer.  Therefore,
      * SHUTDOWN can be safely ignored.
      *
-     * @param subchannel the involved Subchannel
      * @param newState the new state
      *
-     * @since 1.21.0
+     * @since 1.22.0
      */
-    void onSubchannelState(Subchannel subchannel, ConnectivityStateInfo newState);
+    void onSubchannelState(ConnectivityStateInfo newState);
   }
 
   /**

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

@@ -97,7 +97,7 @@ public static <T> void testMethodsForwarded(
       try {
         method.invoke(verify(mockDelegate), args);
       } catch (InvocationTargetException e) {
-        throw new AssertionError(String.format("Method was not forwarded: %s", method));
+        throw new RuntimeException(String.format("Method was not forwarded: %s", method), e);
       }
     }
 
@@ -126,7 +126,7 @@ public interface ArgumentProvider {
      * method once.  It is recommended that each invocation returns a distinctive object for the
      * same type, in order to verify that arguments are passed by the tested class correctly.
      *
-     * @return a value to be passed as an argument.  If {@code null}, {@link Default#defaultValue}
+     * @return a value to be passed as an argument.  If {@code null}, {@link Defaults#defaultValue}
      *         will be used.
      */
     @Nullable Object get(Method method, int argPos, Class<?> clazz);

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

@@ -163,7 +163,6 @@ public void helper_createSubchannelList_throws() {
       new NoopHelper().createSubchannel(CreateSubchannelArgs.newBuilder()
           .setAddresses(eag)
           .setAttributes(attrs)
-          .setStateListener(subchannelStateListener)
           .build());
       fail("Should throw");
     } catch (UnsupportedOperationException e) {
@@ -234,7 +233,6 @@ public void createSubchannelArgs_option_keyOps() {
     CreateSubchannelArgs args = CreateSubchannelArgs.newBuilder()
         .setAddresses(eag)
         .setAttributes(attrs)
-        .setStateListener(subchannelStateListener)
         .build();
     assertThat(args.getOption(testKey)).isNull();
     assertThat(args.getOption(testWithDefaultKey)).isSameInstanceAs(testValue);
@@ -247,7 +245,6 @@ public void createSubchannelArgs_option_addGet() {
     CreateSubchannelArgs args = CreateSubchannelArgs.newBuilder()
         .setAddresses(eag)
         .setAttributes(attrs)
-        .setStateListener(subchannelStateListener)
         .addOption(testKey, testValue)
         .build();
     assertThat(args.getOption(testKey)).isEqualTo(testValue);
@@ -261,7 +258,6 @@ public void createSubchannelArgs_option_lastOneWins() {
     CreateSubchannelArgs args = CreateSubchannelArgs.newBuilder()
         .setAddresses(eag)
         .setAttributes(attrs)
-        .setStateListener(subchannelStateListener)
         .addOption(testKey, testValue1)
         .addOption(testKey, testValue2)
         .build();
@@ -275,13 +271,11 @@ public void createSubchannelArgs_build() {
     CreateSubchannelArgs args = CreateSubchannelArgs.newBuilder()
         .setAddresses(eag)
         .setAttributes(attrs)
-        .setStateListener(subchannelStateListener)
         .addOption(testKey, testValue)
         .build();
     CreateSubchannelArgs rebuildedArgs = args.toBuilder().build();
     assertThat(rebuildedArgs.getAddresses()).containsExactly(eag);
     assertThat(rebuildedArgs.getAttributes()).isSameInstanceAs(attrs);
-    assertThat(rebuildedArgs.getStateListener()).isSameInstanceAs(subchannelStateListener);
     assertThat(rebuildedArgs.getOption(testKey)).isSameInstanceAs(testValue);
   }
 
@@ -291,13 +285,11 @@ public void createSubchannelArgs_toString() {
     CreateSubchannelArgs args = CreateSubchannelArgs.newBuilder()
         .setAddresses(eag)
         .setAttributes(attrs)
-        .setStateListener(subchannelStateListener)
         .addOption(testKey, "test-value")
         .build();
     String str = args.toString();
     assertThat(str).contains("addrs=");
     assertThat(str).contains("attrs=");
-    assertThat(str).contains("listener=");
     assertThat(str).contains("customOptions=");
   }