PR comments

- PairSocketAddress
- use correct DnsNameResolver error handling
- let ProxyDetector be passed in as getNameResolverParams

Author: zpencer

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

@@ -25,8 +25,8 @@
 import io.grpc.EquivalentAddressGroup;
 import io.grpc.NameResolver;
 import io.grpc.Status;
-import io.grpc.internal.ProxyDetector.ProxiedInetSocketAddress;
 import io.grpc.internal.SharedResourceHolder.Resource;
+import java.io.IOException;
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.net.SocketAddress;
@@ -149,14 +149,20 @@ public void run() {
           ProxyParameters proxy;
           try {
             proxy = proxyDetector.proxyFor(destination);
-          } catch (Exception e) {
+          } catch (IOException e) {
             savedListener.onError(
                 Status.UNAVAILABLE.withDescription("Unable to resolve host " + host).withCause(e));
             return;
           }
           if (proxy != null) {
             EquivalentAddressGroup server =
-                new EquivalentAddressGroup(new ProxiedInetSocketAddress(destination, proxy));
+                new EquivalentAddressGroup(
+                    new PairSocketAddress(
+                        destination,
+                        Attributes
+                            .newBuilder()
+                            .set(ProxyDetector.PROXY_PARAMS_KEY, proxy)
+                            .build()));
             savedListener.onAddresses(Collections.singletonList(server), Attributes.EMPTY);
             return;
           }

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

@@ -37,6 +37,8 @@
  * </ul>
  */
 public final class DnsNameResolverProvider extends NameResolverProvider {
+  public static final Attributes.Key<ProxyDetector> PROXY_DETECTOR_KEY
+      = Attributes.Key.of("proxy-detector-key");
 
   private static final String SCHEME = "dns";
 
@@ -47,12 +49,17 @@ public DnsNameResolver newNameResolver(URI targetUri, Attributes params) {
       Preconditions.checkArgument(targetPath.startsWith("/"),
           "the path component (%s) of the target (%s) must start with '/'", targetPath, targetUri);
       String name = targetPath.substring(1);
+      // We want to eventually let the ProxyDetector be passed in from ManagedChannel
+      ProxyDetector proxyDetector = params.get(PROXY_DETECTOR_KEY);
+      if (proxyDetector == null) {
+        proxyDetector = GrpcUtil.getDefaultProxyDetector();
+      }
       return new DnsNameResolver(
           targetUri.getAuthority(),
           name,
           params,
           GrpcUtil.SHARED_CHANNEL_EXECUTOR,
-          GrpcUtil.getProxyDetector());
+          proxyDetector);
     } else {
       return null;
     }

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

@@ -249,7 +249,7 @@ public ProxyParameters proxyFor(SocketAddress targetServerAddress) {
   /**
    * Returns a proxy detector appropriate for the current environment.
    */
-  public static ProxyDetector getProxyDetector() {
+  public static ProxyDetector getDefaultProxyDetector() {
     if (IS_RESTRICTED_APPENGINE) {
       return NOOP_PROXY_DETECTOR;
     } else {

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

@@ -37,7 +37,6 @@
 import io.grpc.MethodDescriptor;
 import io.grpc.Status;
 import io.grpc.internal.Channelz.ChannelStats;
-import io.grpc.internal.ProxyDetector.ProxiedInetSocketAddress;
 import java.net.SocketAddress;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -212,9 +211,9 @@ private void startNewTransport() {
     SocketAddress address = addrs.get(addressIndex);
 
     ProxyParameters proxy = null;
-    if (address instanceof ProxiedInetSocketAddress) {
-      proxy = ((ProxiedInetSocketAddress) address).getProxyParameters();
-      address = ((ProxiedInetSocketAddress) address).getDestination();
+    if (address instanceof PairSocketAddress) {
+      proxy = ((PairSocketAddress) address).getAttributes().get(ProxyDetector.PROXY_PARAMS_KEY);
+      address = ((PairSocketAddress) address).getAddress();
     }
 
     ConnectionClientTransport transport =

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

@@ -0,0 +1,45 @@
+/*
+ * Copyright 2018, 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.internal;
+
+import com.google.common.annotations.VisibleForTesting;
+import io.grpc.Attributes;
+import java.net.SocketAddress;
+
+/**
+ * A data structure to associate a {@link SocketAddress} with {@link Attributes}.
+ */
+final class PairSocketAddress extends SocketAddress {
+  private static final long serialVersionUID = -6854992294603212793L;
+
+  private final SocketAddress address;
+  private final Attributes attributes;
+
+  @VisibleForTesting
+  PairSocketAddress(SocketAddress address, Attributes attributes) {
+    this.address = address;
+    this.attributes = attributes;
+  }
+
+  public Attributes getAttributes() {
+    return attributes;
+  }
+
+  public SocketAddress getAddress() {
+    return address;
+  }
+}

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

@@ -16,9 +16,8 @@
 
 package io.grpc.internal;
 
-import com.google.common.annotations.VisibleForTesting;
+import io.grpc.Attributes;
 import java.io.IOException;
-import java.net.InetSocketAddress;
 import java.net.SocketAddress;
 import javax.annotation.Nullable;
 
@@ -29,34 +28,12 @@
  * {@link io.grpc.NameResolver}.
  */
 public interface ProxyDetector {
+  Attributes.Key<ProxyParameters> PROXY_PARAMS_KEY = Attributes.Key.of("proxy-params-key");
   /**
    * Given a target address, returns which proxy address should be used. If no proxy should be
    * used, then return value will be null. The address of the {@link ProxyParameters} is always
    * resolved. This throws if the proxy address cannot be resolved.
    */
   @Nullable
   ProxyParameters proxyFor(SocketAddress targetServerAddress) throws IOException;
-
-  // This class does not extend from InetSocketAddress because there is no public constructor
-  // that allows subclass instances be unresolved.
-  final class ProxiedInetSocketAddress extends SocketAddress {
-    private static final long serialVersionUID = -6854992294603212793L;
-
-    private final InetSocketAddress destination;
-    private final ProxyParameters proxyParams;
-
-    @VisibleForTesting
-    public ProxiedInetSocketAddress(InetSocketAddress destination, ProxyParameters proxyParams) {
-      this.destination = destination;
-      this.proxyParams = proxyParams;
-    }
-
-    public ProxyParameters getProxyParameters() {
-      return proxyParams;
-    }
-
-    public InetSocketAddress getDestination() {
-      return destination;
-    }
-  }
 }

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

@@ -36,7 +36,6 @@
 import io.grpc.NameResolver;
 import io.grpc.internal.DnsNameResolver.DelegateResolver;
 import io.grpc.internal.DnsNameResolver.ResolutionResults;
-import io.grpc.internal.ProxyDetector.ProxiedInetSocketAddress;
 import io.grpc.internal.SharedResourceHolder.Resource;
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
@@ -266,9 +265,9 @@ public void doNotResolveWhenProxyDetected() throws Exception {
     EquivalentAddressGroup eag = result.get(0);
     assertThat(eag.getAddresses()).hasSize(1);
 
-    ProxiedInetSocketAddress socketAddress = (ProxiedInetSocketAddress) eag.getAddresses().get(0);
-    assertSame(proxyParameters, socketAddress.getProxyParameters());
-    assertTrue(socketAddress.getDestination().isUnresolved());
+    PairSocketAddress socketAddress = (PairSocketAddress) eag.getAddresses().get(0);
+    assertSame(proxyParameters, socketAddress.getAttributes().get(ProxyDetector.PROXY_PARAMS_KEY));
+    assertTrue(((InetSocketAddress) socketAddress.getAddress()).isUnresolved());
   }
 
   private void testInvalidUri(URI uri) {