core: make newNameResolver() change backward compatible for callers. …

…(v1.19.x backport) (#5573) We assumed that we were the only caller. Turns out there are forwarding NameResolver too. Implementing the old override will give them time to migrate to the new API. Resolves #5556 Backport of #5560
grpc · Apr 10, 2019 · 4c35781 · 4c35781
1 parent 38b5449
commit 4c35781
Show file tree

Hide file tree

Showing 2 changed files with 73 additions and 2 deletions.
diff --git a/core/src/main/java/io/grpc/NameResolver.java b/core/src/main/java/io/grpc/NameResolver.java
@@ -16,6 +16,8 @@
 
 package io.grpc;
 
+import static com.google.common.base.Preconditions.checkNotNull;
+
 import java.lang.annotation.Documented;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
@@ -131,8 +133,19 @@ public abstract static class Factory {
      */
     @Nullable
     @Deprecated
-    public NameResolver newNameResolver(URI targetUri, Attributes params) {
-      throw new UnsupportedOperationException("This method is going to be deleted");
+    public NameResolver newNameResolver(URI targetUri, final Attributes params) {
+      Helper helper = new Helper() {
+          @Override
+          public int getDefaultPort() {
+            return checkNotNull(params.get(PARAMS_DEFAULT_PORT), "default port not available");
+          }
+
+          @Override
+          public ProxyDetector getProxyDetector() {
+            return checkNotNull(params.get(PARAMS_PROXY_DETECTOR), "proxy detector not available");
+          }
+        };
+      return newNameResolver(targetUri, helper);
     }
 
     /**

diff --git a/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java b/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java
@@ -3325,6 +3325,64 @@ public String getDefaultScheme() {
     assertThat(attrs.get(NameResolver.Factory.PARAMS_PROXY_DETECTOR)).isSameAs(neverProxy);
   }
 
+  @Test
+  @Deprecated
+  public void nameResolverParams_forwardingResolverWithOldApi() {
+    final AtomicReference<NameResolver.Helper> capturedHelper = new AtomicReference<>();
+    final NameResolver noopResolver = new NameResolver() {
+        @Override
+        public String getServiceAuthority() {
+          return "fake-authority";
+        }
+
+        @Override
+        public void start(Listener listener) {
+        }
+
+        @Override
+        public void shutdown() {}
+      };
+    ProxyDetector neverProxy = new ProxyDetector() {
+        @Override
+        public ProxiedSocketAddress proxyFor(SocketAddress targetAddress) {
+          return null;
+        }
+      };
+    final NameResolver.Factory factory = new NameResolver.Factory() {
+        @Override
+        public NameResolver newNameResolver(URI targetUri, NameResolver.Helper helper) {
+          capturedHelper.set(helper);
+          return noopResolver;
+        }
+
+        @Override
+        public String getDefaultScheme() {
+          return "fakescheme";
+        }
+      };
+
+    // A forwarding factory still with the old API can forward to a delegate factory that has
+    // migrated to the new API.
+    NameResolver.Factory oldApiForwardingFactory = new NameResolver.Factory() {
+        @Override
+        public NameResolver newNameResolver(URI targetUri, Attributes params) {
+          return factory.newNameResolver(targetUri, params);
+        }
+
+        @Override
+        public String getDefaultScheme() {
+          return factory.getDefaultScheme();
+        }
+      };
+    channelBuilder.nameResolverFactory(oldApiForwardingFactory).proxyDetector(neverProxy);
+    createChannel();
+
+    NameResolver.Helper helper = capturedHelper.get();
+    assertThat(helper).isNotNull();
+    assertThat(helper.getDefaultPort()).isEqualTo(DEFAULT_PORT);
+    assertThat(helper.getProxyDetector()).isSameAs(neverProxy);
+  }
+
   @Test
   public void getAuthorityAfterShutdown() throws Exception {
     createChannel();