From e63dbd24c58b10c9962d9f28bf92df2899539b0d Mon Sep 17 00:00:00 2001
From: Kun Zhang <zhangkun@google.com>
Date: Mon, 8 Apr 2019 11:38:58 -0700
Subject: [PATCH] core: make newNameResolver() change backward compatible for
 callers.

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
---
 core/src/main/java/io/grpc/NameResolver.java  | 25 +++++++-
 .../grpc/internal/ManagedChannelImplTest.java | 59 +++++++++++++++++++
 2 files changed, 82 insertions(+), 2 deletions(-)

diff --git a/core/src/main/java/io/grpc/NameResolver.java b/core/src/main/java/io/grpc/NameResolver.java
index 006287ac0b1..70df4f056d5 100644
--- a/core/src/main/java/io/grpc/NameResolver.java
+++ b/core/src/main/java/io/grpc/NameResolver.java
@@ -142,6 +142,10 @@ public abstract static class Factory {
     public static final Attributes.Key<ProxyDetector> PARAMS_PROXY_DETECTOR =
         Attributes.Key.create("params-proxy-detector");
 
+    @Deprecated
+    private static final Attributes.Key<SynchronizationContext> PARAMS_SYNC_CONTEXT =
+        Attributes.Key.create("params-sync-context");
+
     /**
      * Creates a {@link NameResolver} for the given target URI, or {@code null} if the given URI
      * cannot be resolved by this factory. The decision should be solely based on the scheme of the
@@ -158,8 +162,24 @@ 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");
+          }
+
+          @Override
+          public SynchronizationContext getSynchronizationContext() {
+            return checkNotNull(params.get(PARAMS_SYNC_CONTEXT), "sync context not available");
+          }
+        };
+      return newNameResolver(targetUri, helper);
     }
 
     /**
@@ -180,6 +200,7 @@ public NameResolver newNameResolver(URI targetUri, Helper helper) {
           Attributes.newBuilder()
               .set(PARAMS_DEFAULT_PORT, helper.getDefaultPort())
               .set(PARAMS_PROXY_DETECTOR, helper.getProxyDetector())
+              .set(PARAMS_SYNC_CONTEXT, helper.getSynchronizationContext())
               .build());
     }
 
diff --git a/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java b/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java
index 5414e85499e..9504ce09899 100644
--- a/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java
+++ b/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java
@@ -3413,6 +3413,65 @@ 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);
+    assertThat(helper.getSynchronizationContext()).isSameAs(channel.syncContext);
+  }
+
   @Test
   public void getAuthorityAfterShutdown() throws Exception {
     createChannel();