Merge pull request #1247 from square/jwilson_1227_proxyselector

Fix pooling with proxy selectors.

okhttp-tests/src/test/java/com/squareup/okhttp/AddressTest.java

@@ -0,0 +1,51 @@
+/*
+ * Copyright (C) 2014 Square, Inc.
+ *
+ * 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 com.squareup.okhttp;
+
+import com.squareup.okhttp.internal.Util;
+import com.squareup.okhttp.internal.http.AuthenticatorAdapter;
+import com.squareup.okhttp.internal.http.RecordingProxySelector;
+import java.util.List;
+import javax.net.SocketFactory;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+
+public final class AddressTest {
+  private SocketFactory socketFactory = SocketFactory.getDefault();
+  private Authenticator authenticator = AuthenticatorAdapter.INSTANCE;
+  private List<Protocol> protocols = Util.immutableList(Protocol.HTTP_1_1);
+  private List<ConnectionSpec> connectionSpecs = Util.immutableList(ConnectionSpec.MODERN_TLS);
+  private RecordingProxySelector proxySelector = new RecordingProxySelector();
+
+  @Test public void equalsAndHashcode() throws Exception {
+    Address a = new Address("square.com", 80, socketFactory, null, null, null,
+        authenticator, null, protocols, connectionSpecs, proxySelector);
+    Address b = new Address("square.com", 80, socketFactory, null, null, null,
+        authenticator, null, protocols, connectionSpecs, proxySelector);
+    assertEquals(a, b);
+    assertEquals(a.hashCode(), b.hashCode());
+  }
+
+  @Test public void differentProxySelectorsAreDifferent() throws Exception {
+    Address a = new Address("square.com", 80, socketFactory, null, null, null,
+        authenticator, null, protocols, connectionSpecs, new RecordingProxySelector());
+    Address b = new Address("square.com", 80, socketFactory, null, null, null,
+        authenticator, null, protocols, connectionSpecs, new RecordingProxySelector());
+    assertFalse(a.equals(b));
+  }
+}

okhttp-tests/src/test/java/com/squareup/okhttp/ConnectionPoolTest.java

@@ -19,6 +19,7 @@
 import com.squareup.okhttp.internal.SslContextBuilder;
 import com.squareup.okhttp.internal.Util;
 import com.squareup.okhttp.internal.http.AuthenticatorAdapter;
+import com.squareup.okhttp.internal.http.RecordingProxySelector;
 import com.squareup.okhttp.mockwebserver.MockWebServer;
 import java.io.IOException;
 import java.net.InetAddress;
@@ -70,6 +71,7 @@ public final class ConnectionPoolTest {
 
   private void setUp(int poolSize) throws Exception {
     SocketFactory socketFactory = SocketFactory.getDefault();
+    RecordingProxySelector proxySelector = new RecordingProxySelector();
 
     spdyServer = new MockWebServer();
     httpServer = new MockWebServer();
@@ -81,15 +83,15 @@ private void setUp(int poolSize) throws Exception {
     httpServer.play();
     httpAddress = new Address(httpServer.getHostName(), httpServer.getPort(), socketFactory, null,
         null, null, AuthenticatorAdapter.INSTANCE, null,
-        Util.immutableList(Protocol.SPDY_3, Protocol.HTTP_1_1), connectionSpecs);
+        Util.immutableList(Protocol.SPDY_3, Protocol.HTTP_1_1), connectionSpecs, proxySelector);
     httpSocketAddress = new InetSocketAddress(InetAddress.getByName(httpServer.getHostName()),
         httpServer.getPort());
 
     spdyServer.play();
     spdyAddress = new Address(spdyServer.getHostName(), spdyServer.getPort(), socketFactory,
         sslContext.getSocketFactory(), new RecordingHostnameVerifier(), CertificatePinner.DEFAULT,
-        AuthenticatorAdapter.INSTANCE, null,
-        Util.immutableList(Protocol.SPDY_3, Protocol.HTTP_1_1), connectionSpecs);
+        AuthenticatorAdapter.INSTANCE, null, Util.immutableList(Protocol.SPDY_3, Protocol.HTTP_1_1),
+        connectionSpecs, proxySelector);
     spdySocketAddress = new InetSocketAddress(InetAddress.getByName(spdyServer.getHostName()),
         spdyServer.getPort());
 

okhttp-tests/src/test/java/com/squareup/okhttp/internal/http/RouteSelectorTest.java

@@ -147,7 +147,7 @@ public final class RouteSelectorTest {
 
   @Test public void explicitProxyTriesThatProxysAddressesOnly() throws Exception {
     Address address = new Address(uriHost, uriPort, socketFactory, null, null, null, authenticator,
-        proxyA, protocols, connectionSpecs);
+        proxyA, protocols, connectionSpecs, proxySelector);
     client.setProxy(proxyA);
     RouteSelector routeSelector = RouteSelector.get(httpRequest, client);
 
@@ -165,7 +165,7 @@ public final class RouteSelectorTest {
 
   @Test public void explicitDirectProxy() throws Exception {
     Address address = new Address(uriHost, uriPort, socketFactory, null, null, null, authenticator,
-        NO_PROXY, protocols, connectionSpecs);
+        NO_PROXY, protocols, connectionSpecs, proxySelector);
     client.setProxy(NO_PROXY);
     RouteSelector routeSelector = RouteSelector.get(httpRequest, client);
 
@@ -328,7 +328,7 @@ public final class RouteSelectorTest {
 
   @Test public void multipleProxiesMultipleInetAddressesMultipleConfigurations() throws Exception {
     Address address = new Address(uriHost, uriPort, socketFactory, sslSocketFactory,
-        hostnameVerifier, null, authenticator, null, protocols, connectionSpecs);
+        hostnameVerifier, null, authenticator, null, protocols, connectionSpecs, proxySelector);
     proxySelector.proxies.add(proxyA);
     proxySelector.proxies.add(proxyB);
     RouteSelector routeSelector = RouteSelector.get(httpsRequest, client);
@@ -431,7 +431,7 @@ private void assertConnection(Connection connection, Address address, Proxy prox
   /** Returns an address that's without an SSL socket factory or hostname verifier. */
   private Address httpAddress() {
     return new Address(uriHost, uriPort, socketFactory, null, null, null, authenticator, null,
-        protocols, connectionSpecs);
+        protocols, connectionSpecs, proxySelector);
   }
 
   private static InetAddress[] makeFakeAddresses(int prefix, int count) {

okhttp/src/main/java/com/squareup/okhttp/Address.java

@@ -17,6 +17,7 @@
 
 import com.squareup.okhttp.internal.Util;
 import java.net.Proxy;
+import java.net.ProxySelector;
 import java.util.List;
 import javax.net.SocketFactory;
 import javax.net.ssl.HostnameVerifier;
@@ -45,15 +46,17 @@ public final class Address {
   final Authenticator authenticator;
   final List<Protocol> protocols;
   final List<ConnectionSpec> connectionSpecs;
+  final ProxySelector proxySelector;
 
   public Address(String uriHost, int uriPort, SocketFactory socketFactory,
       SSLSocketFactory sslSocketFactory, HostnameVerifier hostnameVerifier,
       CertificatePinner certificatePinner, Authenticator authenticator, Proxy proxy,
-      List<Protocol> protocols, List<ConnectionSpec> connectionSpecs) {
+      List<Protocol> protocols, List<ConnectionSpec> connectionSpecs, ProxySelector proxySelector) {
     if (uriHost == null) throw new NullPointerException("uriHost == null");
     if (uriPort <= 0) throw new IllegalArgumentException("uriPort <= 0: " + uriPort);
     if (authenticator == null) throw new IllegalArgumentException("authenticator == null");
     if (protocols == null) throw new IllegalArgumentException("protocols == null");
+    if (proxySelector == null) throw new IllegalArgumentException("proxySelector == null");
     this.proxy = proxy;
     this.uriHost = uriHost;
     this.uriPort = uriPort;
@@ -64,6 +67,7 @@ public Address(String uriHost, int uriPort, SocketFactory socketFactory,
     this.authenticator = authenticator;
     this.protocols = Util.immutableList(protocols);
     this.connectionSpecs = Util.immutableList(connectionSpecs);
+    this.proxySelector = proxySelector;
   }
 
   /** Returns the hostname of the origin server. */
@@ -121,12 +125,20 @@ public List<ConnectionSpec> getConnectionSpecs() {
 
   /**
    * Returns this address's explicitly-specified HTTP proxy, or null to
-   * delegate to the HTTP client's proxy selector.
+   * delegate to the {@linkplain #getProxySelector proxy selector}.
    */
   public Proxy getProxy() {
     return proxy;
   }
 
+  /**
+   * Returns this address's proxy selector. Only used if the proxy is null. If none of this
+   * selector's proxies are reachable, a direct connection will be attempted.
+   */
+  public ProxySelector getProxySelector() {
+    return proxySelector;
+  }
+
   @Override public boolean equals(Object other) {
     if (other instanceof Address) {
       Address that = (Address) other;
@@ -137,21 +149,25 @@ && equal(this.sslSocketFactory, that.sslSocketFactory)
           && equal(this.hostnameVerifier, that.hostnameVerifier)
           && equal(this.certificatePinner, that.certificatePinner)
           && equal(this.authenticator, that.authenticator)
-          && equal(this.protocols, that.protocols);
+          && equal(this.protocols, that.protocols)
+          && equal(this.connectionSpecs, that.connectionSpecs)
+          && equal(this.proxySelector, that.proxySelector);
     }
     return false;
   }
 
   @Override public int hashCode() {
     int result = 17;
+    result = 31 * result + (proxy != null ? proxy.hashCode() : 0);
     result = 31 * result + uriHost.hashCode();
     result = 31 * result + uriPort;
     result = 31 * result + (sslSocketFactory != null ? sslSocketFactory.hashCode() : 0);
     result = 31 * result + (hostnameVerifier != null ? hostnameVerifier.hashCode() : 0);
     result = 31 * result + (certificatePinner != null ? certificatePinner.hashCode() : 0);
     result = 31 * result + authenticator.hashCode();
-    result = 31 * result + (proxy != null ? proxy.hashCode() : 0);
     result = 31 * result + protocols.hashCode();
+    result = 31 * result + connectionSpecs.hashCode();
+    result = 31 * result + proxySelector.hashCode();
     return result;
   }
 }

okhttp/src/main/java/com/squareup/okhttp/internal/http/RouteSelector.java

@@ -18,8 +18,8 @@
 import com.squareup.okhttp.Address;
 import com.squareup.okhttp.CertificatePinner;
 import com.squareup.okhttp.Connection;
-import com.squareup.okhttp.ConnectionSpec;
 import com.squareup.okhttp.ConnectionPool;
+import com.squareup.okhttp.ConnectionSpec;
 import com.squareup.okhttp.OkHttpClient;
 import com.squareup.okhttp.Request;
 import com.squareup.okhttp.Route;
@@ -30,7 +30,6 @@
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.net.Proxy;
-import java.net.ProxySelector;
 import java.net.SocketAddress;
 import java.net.SocketException;
 import java.net.URI;
@@ -56,7 +55,6 @@ public final class RouteSelector {
   private final URI uri;
   private final Network network;
   private final OkHttpClient client;
-  private final ProxySelector proxySelector;
   private final ConnectionPool pool;
   private final RouteDatabase routeDatabase;
   private final Request request;
@@ -85,7 +83,6 @@ private RouteSelector(Address address, URI uri, OkHttpClient client, Request req
     this.address = address;
     this.uri = uri;
     this.client = client;
-    this.proxySelector = client.getProxySelector();
     this.pool = client.getConnectionPool();
     this.routeDatabase = Internal.instance.routeDatabase(client);
     this.network = Internal.instance.network(client);
@@ -112,7 +109,7 @@ public static RouteSelector get(Request request, OkHttpClient client) throws IOE
     Address address = new Address(uriHost, getEffectivePort(request.url()),
         client.getSocketFactory(), sslSocketFactory, hostnameVerifier, certificatePinner,
         client.getAuthenticator(), client.getProxy(), client.getProtocols(),
-        client.getConnectionSpecs());
+        client.getConnectionSpecs(), client.getProxySelector());
 
     return new RouteSelector(address, request.uri(), client, request);
   }
@@ -188,9 +185,9 @@ public void connectFailed(Connection connection, IOException failure) {
     if (Internal.instance.recycleCount(connection) > 0) return;
 
     Route failedRoute = connection.getRoute();
-    if (failedRoute.getProxy().type() != Proxy.Type.DIRECT && proxySelector != null) {
+    if (failedRoute.getProxy().type() != Proxy.Type.DIRECT && address.getProxySelector() != null) {
       // Tell the proxy selector when we fail to connect on a fresh connection.
-      proxySelector.connectFailed(uri, failedRoute.getProxy().address(), failure);
+      address.getProxySelector().connectFailed(uri, failedRoute.getProxy().address(), failure);
     }
 
     routeDatabase.failed(failedRoute);
@@ -219,7 +216,7 @@ private void resetNextProxy(URI uri, Proxy proxy) {
       // Try each of the ProxySelector choices until one connection succeeds. If none succeed
       // then we'll try a direct connection below.
       proxies = new ArrayList<>();
-      List<Proxy> selectedProxies = proxySelector.select(uri);
+      List<Proxy> selectedProxies = client.getProxySelector().select(uri);
       if (selectedProxies != null) proxies.addAll(selectedProxies);
       // Finally try a direct connection. We only try it once!
       proxies.removeAll(Collections.singleton(Proxy.NO_PROXY));