inprocess,core: add ManagedChannelBuilder and ServerBuilder factory hiders

* inprocess,core: add ManagedChannelBuilder and ServerBuilder factory hiders

Because the factory for Channels and Servers resides on the builder
itself, it is easy for subclasses to accidentally inherit the
factory.  This causes confusion, because calling a static method on
a specific class may result in a different class.

This change adds hiding static factories to each builder, and a test
to enforce that each subclass hides the factory.  The test lives in
the interop tests, because it has a classpath dependency on all the
existing transports.

Minor note: the test scans the classpath using a Beta Guava API.
The test can be disabled if the API goes away.

core/src/main/java/io/grpc/ForwardingChannelBuilder.java

@@ -36,6 +36,20 @@ public abstract class ForwardingChannelBuilder<T extends ForwardingChannelBuilde
    */
   protected ForwardingChannelBuilder() {}
 
+  /**
+   * This method serves to force sub classes to "hide" this static factory.
+   */
+  public static ManagedChannelBuilder<?> forAddress(String name, int port) {
+    throw new UnsupportedOperationException("Subclass failed to hide static factory");
+  }
+
+  /**
+   * This method serves to force sub classes to "hide" this static factory.
+   */
+  public static ManagedChannelBuilder<?> forTarget(String target) {
+    throw new UnsupportedOperationException("Subclass failed to hide static factory");
+  }
+
   /**
    * Returns the delegated {@code ManagedChannelBuilder}.
    */

core/src/main/java/io/grpc/inprocess/InProcessChannelBuilder.java

@@ -48,6 +48,20 @@ public final class InProcessChannelBuilder extends
     return new InProcessChannelBuilder(name);
   }
 
+  /**
+   * Always fails.  Call {@link #forName} instead.
+   */
+  public static InProcessChannelBuilder forTarget(String target) {
+    throw new UnsupportedOperationException("call forName() instead");
+  }
+
+  /**
+   * Always fails.  Call {@link #forName} instead.
+   */
+  public static InProcessChannelBuilder forAddress(String name, int port) {
+    throw new UnsupportedOperationException("call forName() instead");
+  }
+
   private final String name;
 
   private InProcessChannelBuilder(String name) {

core/src/main/java/io/grpc/inprocess/InProcessServerBuilder.java

@@ -68,6 +68,13 @@ public final class InProcessServerBuilder
     return new InProcessServerBuilder(name);
   }
 
+  /**
+   * Always fails.  Call {@link #forName} instead.
+   */
+  public static InProcessServerBuilder forPort(int port) {
+    throw new UnsupportedOperationException("call forName() instead");
+  }
+
   private final String name;
 
   private InProcessServerBuilder(String name) {

core/src/main/java/io/grpc/internal/AbstractManagedChannelImplBuilder.java

@@ -56,6 +56,14 @@ public abstract class AbstractManagedChannelImplBuilder
         <T extends AbstractManagedChannelImplBuilder<T>> extends ManagedChannelBuilder<T> {
   private static final String DIRECT_ADDRESS_SCHEME = "directaddress";
 
+  public static ManagedChannelBuilder<?> forAddress(String name, int port) {
+    throw new UnsupportedOperationException("Subclass failed to hide static factory");
+  }
+
+  public static ManagedChannelBuilder<?> forTarget(String target) {
+    throw new UnsupportedOperationException("Subclass failed to hide static factory");
+  }
+
   /**
    * An idle timeout larger than this would disable idle mode.
    */

core/src/main/java/io/grpc/internal/AbstractServerImplBuilder.java

@@ -51,6 +51,10 @@ import javax.annotation.Nullable;
 public abstract class AbstractServerImplBuilder<T extends AbstractServerImplBuilder<T>>
         extends ServerBuilder<T> {
 
+  public static ServerBuilder<?> forPort(int port) {
+    throw new UnsupportedOperationException("Subclass failed to hide static factory");
+  }
+
   private static final ObjectPool<? extends Executor> DEFAULT_EXECUTOR_POOL =
       SharedResourcePool.forResource(GrpcUtil.SHARED_CHANNEL_EXECUTOR);
   private static final HandlerRegistry DEFAULT_FALLBACK_REGISTRY = new HandlerRegistry() {

interop-testing/src/test/java/io/grpc/ChannelAndServerBuilderTest.java

@@ -0,0 +1,95 @@
+/*
+ * Copyright 2017, gRPC Authors All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.grpc;
+
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertTrue;
+
+import com.google.common.reflect.ClassPath;
+import com.google.common.reflect.ClassPath.ClassInfo;
+import com.google.common.truth.Truth;
+import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import org.junit.Assume;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
+
+/**
+ * Tests that Channel and Server builders properly hide the static constructors.
+ */
+@RunWith(Parameterized.class)
+public class ChannelAndServerBuilderTest {
+
+  @Parameter
+  public Class<?> builderClass;
+
+  /**
+   * Javadoc.
+   */
+  @Parameters(name = "class={0}")
+  public static Collection<Object[]> params() throws Exception {
+    ClassLoader loader = ChannelAndServerBuilderTest.class.getClassLoader();
+    List<Object[]> classes = new ArrayList<Object[]>();
+    for (ClassInfo classInfo : ClassPath.from(loader).getTopLevelClassesRecursive("io.grpc")) {
+      Class<?> clazz = Class.forName(classInfo.getName(), false /*initialize*/, loader);
+      if (ServerBuilder.class.isAssignableFrom(clazz) && clazz != ServerBuilder.class) {
+        classes.add(new Object[]{clazz});
+      } else if (ManagedChannelBuilder.class.isAssignableFrom(clazz)
+          && clazz != ManagedChannelBuilder.class ) {
+        classes.add(new Object[]{clazz});
+      }
+    }
+    Truth.assertWithMessage("Unable to find any builder classes").that(classes).isNotEmpty();
+    return classes;
+  }
+
+  @Test
+  public void serverBuilderHidesMethod_forPort() throws Exception {
+    Assume.assumeTrue(ServerBuilder.class.isAssignableFrom(builderClass));
+    Method method = builderClass.getMethod("forPort", int.class);
+
+    assertTrue(Modifier.isStatic(method.getModifiers()));
+    assertTrue(ServerBuilder.class.isAssignableFrom(method.getReturnType()));
+    assertSame(builderClass, method.getDeclaringClass());
+  }
+
+  @Test
+  public void channelBuilderHidesMethod_forAddress() throws Exception {
+    Assume.assumeTrue(ManagedChannelBuilder.class.isAssignableFrom(builderClass));
+    Method method = builderClass.getMethod("forAddress", String.class, int.class);
+
+    assertTrue(Modifier.isStatic(method.getModifiers()));
+    assertTrue(ManagedChannelBuilder.class.isAssignableFrom(method.getReturnType()));
+    assertSame(builderClass, method.getDeclaringClass());
+  }
+
+  @Test
+  public void channelBuilderHidesMethod_forTarget() throws Exception {
+    Assume.assumeTrue(ManagedChannelBuilder.class.isAssignableFrom(builderClass));
+    Method method = builderClass.getMethod("forTarget", String.class);
+
+    assertTrue(Modifier.isStatic(method.getModifiers()));
+    assertTrue(ManagedChannelBuilder.class.isAssignableFrom(method.getReturnType()));
+    assertSame(builderClass, method.getDeclaringClass());
+  }
+}