Merge pull request #383 from square/jwilson_1230_policy

Remove more stuff from Policy.

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

@@ -30,7 +30,7 @@ import static com.squareup.okhttp.internal.http.HttpURLConnectionImpl.HTTP_MOVED
 import static com.squareup.okhttp.internal.http.HttpURLConnectionImpl.HTTP_MULT_CHOICE;
 import static com.squareup.okhttp.internal.http.HttpURLConnectionImpl.HTTP_PROXY_AUTH;
 import static com.squareup.okhttp.internal.http.HttpURLConnectionImpl.HTTP_SEE_OTHER;
-import static com.squareup.okhttp.internal.http.HttpURLConnectionImpl.HTTP_TEMP_REDIRECT;
+import static com.squareup.okhttp.internal.http.StatusLine.HTTP_TEMP_REDIRECT;
 import static com.squareup.okhttp.internal.http.HttpURLConnectionImpl.HTTP_UNAUTHORIZED;
 
 final class Job implements Runnable, Policy {
@@ -57,18 +57,6 @@ final class Job implements Runnable, Policy {
     return request.body().contentLength();
   }
 
-  @Override public boolean getUseCaches() {
-    return true;
-  }
-
-  @Override public long getIfModifiedSince() {
-    return 0; // For HttpURLConnection only. We let the cache drive this.
-  }
-
-  @Override public boolean usingProxy() {
-    return false; // We let the connection decide this.
-  }
-
   @Override public void setSelectedProxy(Proxy proxy) {
     // Do nothing.
   }

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

@@ -37,7 +37,7 @@ import javax.net.ssl.HttpsURLConnection;
 import javax.net.ssl.SSLSocketFactory;
 
 /** Configures and creates HTTP connections. */
-public final class OkHttpClient implements URLStreamHandlerFactory {
+public final class OkHttpClient implements URLStreamHandlerFactory, Cloneable {
   private static final List<String> DEFAULT_TRANSPORTS
       = Util.immutableList(Arrays.asList("spdy/3", "http/1.1"));
 
@@ -61,11 +61,6 @@ public final class OkHttpClient implements URLStreamHandlerFactory {
     dispatcher = new Dispatcher();
   }
 
-  private OkHttpClient(OkHttpClient copyFrom) {
-    routeDatabase = copyFrom.routeDatabase;
-    dispatcher = copyFrom.dispatcher;
-  }
-
   /**
    * Sets the default connect timeout for new connections. A value of 0 means no timeout.
    *
@@ -370,34 +365,47 @@ public final class OkHttpClient implements URLStreamHandlerFactory {
   }
 
   /**
-   * Returns a shallow copy of this OkHttpClient that uses the system-wide default for
-   * each field that hasn't been explicitly configured.
+   * Returns a shallow copy of this OkHttpClient that uses the system-wide
+   * default for each field that hasn't been explicitly configured.
    */
   private OkHttpClient copyWithDefaults() {
-    OkHttpClient result = new OkHttpClient(this);
-    result.proxy = proxy;
-    result.proxySelector = proxySelector != null ? proxySelector : ProxySelector.getDefault();
-    result.cookieHandler = cookieHandler != null ? cookieHandler : CookieHandler.getDefault();
-    result.responseCache = responseCache != null
-        ? responseCache
-        : toOkResponseCacheOrNull(ResponseCache.getDefault());
-    result.sslSocketFactory = sslSocketFactory != null
-        ? sslSocketFactory
-        : HttpsURLConnection.getDefaultSSLSocketFactory();
-    result.hostnameVerifier = hostnameVerifier != null
-        ? hostnameVerifier
-        : OkHostnameVerifier.INSTANCE;
-    result.authenticator = authenticator != null
-        ? authenticator
-        : HttpAuthenticator.SYSTEM_DEFAULT;
-    result.connectionPool = connectionPool != null ? connectionPool : ConnectionPool.getDefault();
-    result.followProtocolRedirects = followProtocolRedirects;
-    result.transports = transports != null ? transports : DEFAULT_TRANSPORTS;
-    result.connectTimeout = connectTimeout;
-    result.readTimeout = readTimeout;
+    OkHttpClient result = clone();
+    if (result.proxySelector == null) {
+      result.proxySelector = ProxySelector.getDefault();
+    }
+    if (result.cookieHandler == null) {
+      result.cookieHandler = CookieHandler.getDefault();
+    }
+    if (result.responseCache == null) {
+      result.responseCache = toOkResponseCacheOrNull(ResponseCache.getDefault());
+    }
+    if (result.sslSocketFactory == null) {
+      result.sslSocketFactory = HttpsURLConnection.getDefaultSSLSocketFactory();
+    }
+    if (result.hostnameVerifier == null) {
+      result.hostnameVerifier = OkHostnameVerifier.INSTANCE;
+    }
+    if (result.authenticator == null) {
+      result.authenticator = HttpAuthenticator.SYSTEM_DEFAULT;
+    }
+    if (result.connectionPool == null) {
+      result.connectionPool = ConnectionPool.getDefault();
+    }
+    if (result.transports == null) {
+      result.transports = DEFAULT_TRANSPORTS;
+    }
     return result;
   }
 
+  /** Returns a shallow copy of this OkHttpClient. */
+  @Override public OkHttpClient clone() {
+    try {
+      return (OkHttpClient) super.clone();
+    } catch (CloneNotSupportedException e) {
+      throw new AssertionError();
+    }
+  }
+
   private OkResponseCache toOkResponseCacheOrNull(ResponseCache cache) {
     return cache instanceof OkResponseCache ? ((OkResponseCache) cache) : null;
   }

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

@@ -36,7 +36,6 @@ import java.net.CacheRequest;
 import java.net.CookieHandler;
 import java.net.URL;
 import java.net.UnknownHostException;
-import java.util.Date;
 import java.util.zip.GZIPInputStream;
 import javax.net.ssl.HostnameVerifier;
 import javax.net.ssl.SSLSocketFactory;
@@ -44,6 +43,9 @@ import javax.net.ssl.SSLSocketFactory;
 import static com.squareup.okhttp.internal.Util.EMPTY_INPUT_STREAM;
 import static com.squareup.okhttp.internal.Util.getDefaultPort;
 import static com.squareup.okhttp.internal.Util.getEffectivePort;
+import static com.squareup.okhttp.internal.http.StatusLine.HTTP_CONTINUE;
+import static java.net.HttpURLConnection.HTTP_NOT_MODIFIED;
+import static java.net.HttpURLConnection.HTTP_NO_CONTENT;
 
 /**
  * Handles a single HTTP request/response pair. Each HTTP engine follows this
@@ -67,10 +69,6 @@ import static com.squareup.okhttp.internal.Util.getEffectivePort;
  * recycled. By default, this socket connection is held when the last byte of
  * the response is consumed. To release the connection when it is no longer
  * required, use {@link #automaticallyReleaseConnectionToPool()}.
- *
- * <p>Since we permit redirects across protocols (HTTP to HTTPS or vice versa),
- * the implementation type of the connection doesn't necessarily match the
- * implementation type of its HttpEngine.
  */
 public class HttpEngine {
   private static final Response.Body EMPTY_BODY = new Response.Body() {
@@ -88,8 +86,6 @@ public class HttpEngine {
     }
   };
 
-  public static final int HTTP_CONTINUE = 100;
-
   final Policy policy;
   final OkHttpClient client;
 
@@ -202,7 +198,6 @@ public class HttpEngine {
    */
   private void initResponseSource() throws IOException {
     responseSource = ResponseSource.NETWORK;
-    if (!policy.getUseCaches()) return;
 
     OkResponseCache responseCache = client.getOkResponseCache();
     if (responseCache == null) return;
@@ -271,7 +266,7 @@ public class HttpEngine {
       }
       SSLSocketFactory sslSocketFactory = null;
       HostnameVerifier hostnameVerifier = null;
-      if (request.url().getProtocol().equalsIgnoreCase("https")) {
+      if (request.isHttps()) {
         sslSocketFactory = client.getSslSocketFactory();
         hostnameVerifier = client.getHostnameVerifier();
       }
@@ -288,14 +283,8 @@ public class HttpEngine {
     } else {
       connection.updateReadTimeout(client.getReadTimeout());
     }
-    connected(connection);
-  }
 
-  /**
-   * Called after a socket connection has been created or retrieved from the
-   * pool. Subclasses use this hook to get a reference to the TLS data.
-   */
-  private void connected(Connection connection) {
+    // Update the policy to tell 'em which proxy we ended up going with.
     policy.setSelectedProxy(connection.getRoute().getProxy());
   }
 
@@ -343,9 +332,7 @@ public class HttpEngine {
   /** Returns the engine's response. */
   // TODO: the returned body will always be null.
   public final Response getResponse() {
-    if (response == null) {
-      throw new IllegalStateException();
-    }
+    if (response == null) throw new IllegalStateException();
     return response;
   }
 
@@ -368,8 +355,6 @@ public class HttpEngine {
   }
 
   private void maybeCache() throws IOException {
-    // Are we caching at all?
-    if (!policy.getUseCaches()) return;
     OkResponseCache responseCache = client.getOkResponseCache();
     if (responseCache == null) return;
 
@@ -457,8 +442,8 @@ public class HttpEngine {
     }
 
     if ((responseCode < HTTP_CONTINUE || responseCode >= 200)
-        && responseCode != HttpURLConnectionImpl.HTTP_NO_CONTENT
-        && responseCode != HttpURLConnectionImpl.HTTP_NOT_MODIFIED) {
+        && responseCode != HTTP_NO_CONTENT
+        && responseCode != HTTP_NOT_MODIFIED) {
       return true;
     }
 
@@ -503,11 +488,6 @@ public class HttpEngine {
       result.setContentType("application/x-www-form-urlencoded");
     }
 
-    long ifModifiedSince = policy.getIfModifiedSince();
-    if (ifModifiedSince != 0) {
-      result.setIfModifiedSince(new Date(ifModifiedSince));
-    }
-
     CookieHandler cookieHandler = client.getCookieHandler();
     if (cookieHandler != null) {
       result.addCookies(cookieHandler.get(request.uri(), request.getHeaders().toMultimap(null)));
@@ -602,7 +582,7 @@ public class HttpEngine {
     }
 
     if (hasResponseBody()) {
-      maybeCache(); // reentrant. this calls into user code which may call back into this!
+      maybeCache();
     }
 
     initContentStream(transport.getTransferStream(cacheRequest));

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

@@ -30,6 +30,7 @@ import java.net.ProtocolException;
 import java.net.Socket;
 
 import static com.squareup.okhttp.internal.Util.checkOffsetAndCount;
+import static com.squareup.okhttp.internal.http.StatusLine.HTTP_CONTINUE;
 
 public final class HttpTransport implements Transport {
   /**
@@ -165,7 +166,7 @@ public final class HttpTransport implements Transport {
       headersBuilder.readHeaders(in);
       responseBuilder.rawHeaders(headersBuilder.build());
 
-      if (statusLine.code() != HttpEngine.HTTP_CONTINUE) return responseBuilder;
+      if (statusLine.code() != HTTP_CONTINUE) return responseBuilder;
     }
   }
 

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

@@ -38,12 +38,14 @@ import java.security.Permission;
 import java.security.cert.CertificateException;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.Date;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.TimeUnit;
 import javax.net.ssl.SSLHandshakeException;
 
 import static com.squareup.okhttp.internal.Util.getEffectivePort;
+import static com.squareup.okhttp.internal.http.StatusLine.HTTP_TEMP_REDIRECT;
 
 /**
  * This implementation uses HttpEngine to send requests and receive responses.
@@ -55,15 +57,10 @@ import static com.squareup.okhttp.internal.Util.getEffectivePort;
  * is <strong>not</strong> used to indicate not whether this URLConnection is
  * currently connected. Instead, it indicates whether a connection has ever been
  * attempted. Once a connection has been attempted, certain properties (request
- * header fields, request method, etc.) are immutable. Test the {@code
- * connection} field on this class for null/non-null to determine of an instance
- * is currently connected to a server.
+ * header fields, request method, etc.) are immutable.
  */
 public class HttpURLConnectionImpl extends HttpURLConnection implements Policy {
 
-  /** Numeric status code, 307: Temporary Redirect. */
-  public static final int HTTP_TEMP_REDIRECT = 307;
-
   /**
    * How many redirects should we follow? Chrome follows 21; Firefox, curl,
    * and wget follow 20; Safari follows 16; and HTTP/1.0 recommends 5.
@@ -278,7 +275,14 @@ public class HttpURLConnectionImpl extends HttpURLConnection implements Policy {
         .method(method, null) // No body: that's provided later!
         .rawHeaders(requestHeaders.build())
         .build();
-    return new HttpEngine(client, this, request, connection, requestBody);
+
+    // If we're currently not using caches, make sure the engine's client doesn't have one.
+    OkHttpClient engineClient = client;
+    if (engineClient.getOkResponseCache() != null && !getUseCaches()) {
+      engineClient = client.clone().setOkResponseCache(null);
+    }
+
+    return new HttpEngine(engineClient, this, request, connection, requestBody);
   }
 
   /**
@@ -524,6 +528,15 @@ public class HttpURLConnectionImpl extends HttpURLConnection implements Policy {
     }
   }
 
+  @Override public void setIfModifiedSince(long newValue) {
+    super.setIfModifiedSince(newValue);
+    if (ifModifiedSince != 0) {
+      requestHeaders.set("If-Modified-Since", HttpDate.format(new Date(ifModifiedSince)));
+    } else {
+      requestHeaders.removeAll("If-Modified-Since");
+    }
+  }
+
   @Override public final void addRequestProperty(String field, String value) {
     if (connected) {
       throw new IllegalStateException("Cannot add request property after connection is made");

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

@@ -18,15 +18,6 @@ package com.squareup.okhttp.internal.http;
 import java.net.Proxy;
 
 public interface Policy {
-  /** Returns true if HTTP response caches should be used. */
-  boolean getUseCaches();
-
-  /** Returns the If-Modified-Since timestamp, or 0 if none is set. */
-  long getIfModifiedSince();
-
-  /** Returns true if a non-direct proxy is specified. */
-  boolean usingProxy();
-
   /** @see java.net.HttpURLConnection#setChunkedStreamingMode(int) */
   int getChunkLength();
 

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

@@ -4,6 +4,10 @@ import java.io.IOException;
 import java.net.ProtocolException;
 
 public final class StatusLine {
+  /** Numeric status code, 307: Temporary Redirect. */
+  public static final int HTTP_TEMP_REDIRECT = 307;
+  public static final int HTTP_CONTINUE = 100;
+
   private final String statusLine;
   private final int httpMinorVersion;
   private final int responseCode;

okhttp/src/test/java/com/squareup/okhttp/internal/http/URLConnectionTest.java

@@ -75,6 +75,7 @@ import org.junit.Ignore;
 import org.junit.Test;
 
 import static com.squareup.okhttp.OkAuthenticator.Credential;
+import static com.squareup.okhttp.internal.http.StatusLine.HTTP_TEMP_REDIRECT;
 import static com.squareup.okhttp.mockwebserver.SocketPolicy.DISCONNECT_AT_END;
 import static com.squareup.okhttp.mockwebserver.SocketPolicy.DISCONNECT_AT_START;
 import static com.squareup.okhttp.mockwebserver.SocketPolicy.SHUTDOWN_INPUT_AT_END;
@@ -1778,7 +1779,7 @@ public final class URLConnectionTest {
 
   private void test307Redirect(String method) throws Exception {
     MockResponse response1 = new MockResponse()
-        .setResponseCode(HttpURLConnectionImpl.HTTP_TEMP_REDIRECT)
+        .setResponseCode(HTTP_TEMP_REDIRECT)
         .addHeader("Location: /page2");
     if (!method.equals("HEAD")) {
       response1.setBody("This page has moved!");