core: OverrideAuthorityNameResolverFactory should forward refresh() (#3062)

The current implementation has a bug where certain methods are not forwarded to the delegate.

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

@@ -453,21 +453,11 @@ public abstract class AbstractManagedChannelImplBuilder
       if (resolver == null) {
         return null;
       }
-      return new NameResolver() {
+      return new ForwardingNameResolver(resolver) {
         @Override
         public String getServiceAuthority() {
           return authorityOverride;
         }
-
-        @Override
-        public void start(Listener listener) {
-          resolver.start(listener);
-        }
-
-        @Override
-        public void shutdown() {
-          resolver.shutdown();
-        }
       };
     }
 

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

@@ -0,0 +1,65 @@
+/*
+ * Copyright 2017, Google Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ *    * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *    * Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following disclaimer
+ * in the documentation and/or other materials provided with the
+ * distribution.
+ *
+ *    * Neither the name of Google Inc. nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package io.grpc.internal;
+
+import io.grpc.NameResolver;
+
+/**
+* A forwarding class to ensure non overridden methods are forwarded to the delegate.
+ */
+public class ForwardingNameResolver extends NameResolver {
+  private final NameResolver delegate;
+
+  public ForwardingNameResolver(NameResolver delegate) {
+    this.delegate = delegate;
+  }
+
+  @Override
+  public String getServiceAuthority() {
+    return delegate.getServiceAuthority();
+  }
+
+  @Override
+  public void start(Listener listener) {
+    delegate.start(listener);
+  }
+
+  @Override
+  public void shutdown() {
+    delegate.shutdown();
+  }
+
+  @Override
+  public void refresh() {
+    delegate.refresh();
+  }
+}

core/src/test/java/io/grpc/internal/AbstractManagedChannelImplBuilderTest.java

@@ -35,7 +35,10 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.fail;
 import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 import io.grpc.Attributes;
@@ -106,7 +109,7 @@ public class AbstractManagedChannelImplBuilderTest {
   }
 
   @Test
-  public void overrideAuthorityNameResolverWrapsDelegateTest() {
+  public void overrideAuthorityNameResolver_overridesAuthority() {
     NameResolver nameResolverMock = mock(NameResolver.class);
     NameResolver.Factory wrappedFactory = mock(NameResolver.Factory.class);
     when(wrappedFactory.newNameResolver(any(URI.class), any(Attributes.class)))
@@ -122,7 +125,7 @@ public class AbstractManagedChannelImplBuilderTest {
   }
 
   @Test
-  public void overrideAuthorityNameResolverWontWrapNullTest() {
+  public void overrideAuthorityNameResolver_wontWrapNull() {
     NameResolver.Factory wrappedFactory = mock(NameResolver.Factory.class);
     when(wrappedFactory.newNameResolver(any(URI.class), any(Attributes.class))).thenReturn(null);
     NameResolver.Factory factory =
@@ -131,4 +134,27 @@ public class AbstractManagedChannelImplBuilderTest {
     assertEquals(null,
         factory.newNameResolver(URI.create("dns:///localhost:443"), Attributes.EMPTY));
   }
+
+  @Test
+  public void overrideAuthorityNameResolver_forwardsNonOverridenCalls() {
+    NameResolver.Factory wrappedFactory = mock(NameResolver.Factory.class);
+    NameResolver mockResolver = mock(NameResolver.class);
+    when(wrappedFactory.newNameResolver(any(URI.class), any(Attributes.class)))
+        .thenReturn(mockResolver);
+    NameResolver.Factory factory =
+        new AbstractManagedChannelImplBuilder.OverrideAuthorityNameResolverFactory(wrappedFactory,
+            "override:5678");
+    NameResolver overrideResolver =
+        factory.newNameResolver(URI.create("dns:///localhost:443"), Attributes.EMPTY);
+
+    NameResolver.Listener listener = mock(NameResolver.Listener.class);
+    overrideResolver.start(listener);
+    verify(mockResolver).start(eq(listener));
+
+    overrideResolver.shutdown();
+    verify(mockResolver, times(1)).shutdown();
+
+    overrideResolver.refresh();
+    verify(mockResolver, times(1)).refresh();
+  }
 }