1
0
mirror of https://github.com/square/okhttp.git synced 2026-01-25 16:01:38 +03:00

Clean up around HttpEngine.sendRequest().

Move gateway timeout failures to CacheStrategy, and inline
methods nearby for a strict top-to-bottom flow in this method.

It becomes more obvious that the end of sendRequest has two
cases: we need a connection (opening if necessary) or we don't
need a connection (closing if necessary). Previously this was
true but not as explicit.
This commit is contained in:
jwilson
2013-12-31 20:52:49 -05:00
parent 3ed1e2a744
commit 957537774b
10 changed files with 178 additions and 166 deletions

View File

@@ -311,7 +311,7 @@ public final class Connection implements Closeable {
String requestLine = tunnelRequest.requestLine();
while (true) {
HttpTransport.writeRequest(out, request.headers(), requestLine);
Response response = HttpTransport.readResponse(request, in).build();
Response response = HttpTransport.readResponse(in).request(request).build();
switch (response.code()) {
case HTTP_OK:

View File

@@ -527,7 +527,8 @@ public final class HttpResponseCache extends ResponseCache implements OkResponse
public Response response(Request request, DiskLruCache.Snapshot snapshot) {
String contentType = responseHeaders.get("Content-Type");
String contentLength = responseHeaders.get("Content-Length");
return new Response.Builder(request)
return new Response.Builder()
.request(request)
.statusLine(statusLine)
.headers(responseHeaders)
.body(new CacheResponseBody(snapshot, contentType, contentLength))

View File

@@ -19,8 +19,8 @@ import com.squareup.okhttp.internal.Util;
import com.squareup.okhttp.internal.http.HeaderParser;
import com.squareup.okhttp.internal.http.Headers;
import com.squareup.okhttp.internal.http.HttpDate;
import com.squareup.okhttp.internal.http.SyntheticHeaders;
import com.squareup.okhttp.internal.http.StatusLine;
import com.squareup.okhttp.internal.http.SyntheticHeaders;
import java.io.ByteArrayOutputStream;
import java.io.Closeable;
import java.io.IOException;
@@ -144,12 +144,7 @@ public final class Response {
}
public Builder newBuilder() {
return new Builder(request)
.statusLine(statusLine)
.handshake(handshake)
.headers(headers)
.body(body)
.redirectedBy(redirectedBy);
return new Builder(this);
}
/**
@@ -597,16 +592,29 @@ public final class Response {
}
public static class Builder {
private final Request request;
private Request request;
private StatusLine statusLine;
private Handshake handshake;
private Headers.Builder headers = new Headers.Builder();
private Headers.Builder headers;
private Body body;
private Response redirectedBy;
public Builder(Request request) {
if (request == null) throw new IllegalArgumentException("request == null");
public Builder() {
headers = new Headers.Builder();
}
private Builder(Response response) {
this.request = response.request;
this.statusLine = response.statusLine;
this.handshake = response.handshake;
this.headers = response.headers.newBuilder();
this.body = response.body;
this.redirectedBy = response.redirectedBy;
}
public Builder request(Request request) {
this.request = request;
return this;
}
public Builder statusLine(StatusLine statusLine) {

View File

@@ -41,4 +41,8 @@ public enum ResponseSource {
public boolean requiresConnection() {
return this == CONDITIONAL_CACHE || this == NETWORK;
}
public boolean usesCache() {
return this == CACHE || this == CONDITIONAL_CACHE;
}
}

View File

@@ -1,11 +1,16 @@
package com.squareup.okhttp.internal.http;
import com.squareup.okhttp.MediaType;
import com.squareup.okhttp.Request;
import com.squareup.okhttp.Response;
import com.squareup.okhttp.ResponseSource;
import java.io.IOException;
import java.io.InputStream;
import java.net.HttpURLConnection;
import java.util.concurrent.TimeUnit;
import static com.squareup.okhttp.internal.Util.EMPTY_INPUT_STREAM;
/**
* Given a request and cached response, this figures out whether to use the
* network, the cache, or both.
@@ -15,6 +20,30 @@ import java.util.concurrent.TimeUnit;
* response may gain a warning if it is potentially stale.
*/
public final class CacheStrategy {
private static final Response.Body EMPTY_BODY = new Response.Body() {
@Override public boolean ready() throws IOException {
return true;
}
@Override public MediaType contentType() {
return null;
}
@Override public long contentLength() {
return 0;
}
@Override public InputStream byteStream() {
return EMPTY_INPUT_STREAM;
}
};
private static final StatusLine GATEWAY_TIMEOUT_STATUS_LINE;
static {
try {
GATEWAY_TIMEOUT_STATUS_LINE = new StatusLine("HTTP/1.1 504 Gateway Timeout");
} catch (IOException e) {
throw new AssertionError();
}
}
public final Request request;
public final Response response;
public final ResponseSource source;
@@ -114,8 +143,35 @@ public final class CacheStrategy {
* Returns a strategy to satisfy {@code request} using the a cached response
* {@code response}.
*/
public static CacheStrategy get(
long nowMillis, Response response, Request request) {
public static CacheStrategy get(long nowMillis, Response response, Request request) {
CacheStrategy candidate = getCandidate(nowMillis, response, request);
if (candidate.source != ResponseSource.CACHE && request.isOnlyIfCached()) {
// We're forbidden from using the network, but the cache is insufficient.
Response noneResponse = new Response.Builder()
.request(candidate.request)
.statusLine(GATEWAY_TIMEOUT_STATUS_LINE)
.setResponseSource(ResponseSource.NONE)
.body(EMPTY_BODY)
.build();
return new CacheStrategy(candidate.request, noneResponse, ResponseSource.NONE);
}
return candidate;
}
/** Returns a strategy to use assuming the request can use the network. */
private static CacheStrategy getCandidate(long nowMillis, Response response, Request request) {
// No cached response.
if (response == null) {
return new CacheStrategy(request, response, ResponseSource.NETWORK);
}
// Drop the cached response if it's missing a required handshake.
if (request.isHttps() && response.handshake() == null) {
return new CacheStrategy(request, response, ResponseSource.NETWORK);
}
// If this response shouldn't have been stored, it should never be used
// as a response source. This check should be redundant as long as the
// persistence store is well-behaved and the rules are constant.

View File

@@ -38,7 +38,6 @@ import java.util.zip.GZIPInputStream;
import javax.net.ssl.HostnameVerifier;
import javax.net.ssl.SSLSocketFactory;
import static com.squareup.okhttp.internal.Util.EMPTY_INPUT_STREAM;
import static com.squareup.okhttp.internal.Util.closeQuietly;
import static com.squareup.okhttp.internal.Util.getDefaultPort;
import static com.squareup.okhttp.internal.Util.getEffectivePort;
@@ -70,21 +69,6 @@ import static java.net.HttpURLConnection.HTTP_NO_CONTENT;
* required, use {@link #automaticallyReleaseConnectionToPool()}.
*/
public class HttpEngine {
private static final Response.Body EMPTY_BODY = new Response.Body() {
@Override public boolean ready() throws IOException {
return true;
}
@Override public MediaType contentType() {
return null;
}
@Override public long contentLength() {
return 0;
}
@Override public InputStream byteStream() {
return EMPTY_INPUT_STREAM;
}
};
final Policy policy;
final OkHttpClient client;
@@ -155,69 +139,59 @@ public class HttpEngine {
* writing the request body if it exists.
*/
public final void sendRequest() throws IOException {
if (responseSource != null) return;
if (responseSource != null) return; // Already sent.
if (transport != null) throw new IllegalStateException();
prepareRawRequestHeaders();
responseSource = chooseResponseSource();
OkResponseCache responseCache = client.getOkResponseCache();
Response cacheResponse = responseCache != null
? responseCache.get(request)
: null;
long now = System.currentTimeMillis();
CacheStrategy cacheStrategy = CacheStrategy.get(now, cacheResponse, request);
responseSource = cacheStrategy.source;
request = cacheStrategy.request;
if (responseCache != null) {
responseCache.trackResponse(responseSource);
}
if (responseSource != ResponseSource.NETWORK) {
validatingResponse = cacheStrategy.response;
}
if (cacheResponse != null && !responseSource.usesCache()) {
closeQuietly(cacheResponse.body()); // We don't need this cached response. Close it.
}
if (responseSource.requiresConnection()) {
sendSocketRequest();
} else if (connection != null) {
client.getConnectionPool().recycle(connection);
connection = null;
}
}
// Open a connection unless we inherited one from a redirect.
if (connection == null) {
connect();
}
/** Returns a source that can satisfy the request. */
private ResponseSource chooseResponseSource() throws IOException {
OkResponseCache responseCache = client.getOkResponseCache();
if (responseCache == null) return ResponseSource.NETWORK; // No cache? Easy decision.
transport = (Transport) connection.newTransport(this);
request = transport.prepareRequest(request);
Response candidate = responseCache.get(request);
ResponseSource result;
if (candidate == null) {
result = ResponseSource.NETWORK;
} else if (request.isHttps() && candidate.handshake() == null) {
// Drop the cached response if it's missing a required handshake.
result = ResponseSource.NETWORK;
// Create a request body if we don't have one already. We'll already have
// one if we're retrying a failed POST.
if (hasRequestBody() && requestBodyOut == null) {
requestBodyOut = transport.createRequestBody(request);
}
} else {
// We've got a lead on a cached response. Ask response strategy to analyze it.
long now = System.currentTimeMillis();
CacheStrategy cacheStrategy = CacheStrategy.get(now, candidate, request);
result = cacheStrategy.source;
this.request = cacheStrategy.request;
if (result == ResponseSource.CACHE || result == ResponseSource.CONDITIONAL_CACHE) {
this.validatingResponse = cacheStrategy.response;
}
}
if (candidate != null && result == ResponseSource.NETWORK) {
closeQuietly(candidate.body()); // We aren't using the cached response. Close it.
}
if (result == ResponseSource.CACHE) {
promoteValidatingResponse();
} else if (request.isOnlyIfCached()) {
// We're forbidden from using the network, but the cache is insufficient.
if (result == ResponseSource.CONDITIONAL_CACHE) {
closeQuietly(validatingResponse.body());
// We're using a cached response. Close the connection we may have inherited from a redirect.
if (connection != null) {
disconnect();
}
result = ResponseSource.NONE;
this.validatingResponse = new Response.Builder(request)
.statusLine(new StatusLine("HTTP/1.1 504 Gateway Timeout"))
.setResponseSource(result)
.body(EMPTY_BODY)
.build();
promoteValidatingResponse();
// No need for the network! Promote the cached response immediately.
this.response = validatingResponse;
if (validatingResponse.body() != null) {
initContentStream(validatingResponse.body().byteStream());
}
}
responseCache.trackResponse(result);
return result;
}
private Response cacheableResponse() {
@@ -229,30 +203,10 @@ public class HttpEngine {
.build();
}
private void sendSocketRequest() throws IOException {
if (connection == null) {
connect();
}
if (transport != null) {
throw new IllegalStateException();
}
transport = (Transport) connection.newTransport(this);
request = transport.prepareRequest(request);
if (hasRequestBody() && requestBodyOut == null) {
// Create a request body if we don't have one already. We'll already
// have one if we're retrying a failed POST.
requestBodyOut = transport.createRequestBody();
}
}
/** Connect to the origin server either directly or via a proxy. */
protected final void connect() throws IOException {
if (connection != null) {
return;
}
private void connect() throws IOException {
if (connection != null) throw new IllegalStateException();
if (routeSelector == null) {
String uriHost = request.url().getHost();
if (uriHost == null || uriHost.length() == 0) {
@@ -269,7 +223,9 @@ public class HttpEngine {
routeSelector = new RouteSelector(address, request.uri(), client.getProxySelector(),
client.getConnectionPool(), Dns.DEFAULT, client.getRoutesDatabase());
}
connection = routeSelector.next(request.method());
if (!connection.isConnected()) {
connection.connect(client.getConnectTimeout(), client.getReadTimeout(), getTunnelConfig());
client.getConnectionPool().maybeShare(connection);
@@ -282,26 +238,24 @@ public class HttpEngine {
policy.setSelectedProxy(connection.getRoute().getProxy());
}
/**
* Recycle the connection to the origin server. It is an error to call this
* with a request in flight.
*/
private void disconnect() {
client.getConnectionPool().recycle(connection);
connection = null;
}
/**
* Called immediately before the transport transmits HTTP request headers.
* This is used to observe the sent time should the request be cached.
*/
public void writingRequestHeaders() {
if (sentRequestMillis != -1) {
throw new IllegalStateException();
}
if (sentRequestMillis != -1) throw new IllegalStateException();
sentRequestMillis = System.currentTimeMillis();
}
private void promoteValidatingResponse() throws IOException {
if (this.responseBodyIn != null) throw new IllegalStateException();
this.response = validatingResponse;
if (validatingResponse.body() != null) {
initContentStream(validatingResponse.body().byteStream());
}
}
boolean hasRequestBody() {
String method = request.method();
return method.equals("POST") || method.equals("PUT") || method.equals("PATCH");
@@ -360,8 +314,7 @@ public class HttpEngine {
public final void automaticallyReleaseConnectionToPool() {
automaticallyReleaseConnectionToPool = true;
if (connection != null && connectionReleased) {
client.getConnectionPool().recycle(connection);
connection = null;
disconnect();
}
}
@@ -378,7 +331,7 @@ public class HttpEngine {
closeQuietly(responseBodyIn);
}
if (!connectionReleased && connection != null) {
if (connection != null && !connectionReleased) {
connectionReleased = true;
if (transport == null
@@ -386,8 +339,7 @@ public class HttpEngine {
closeQuietly(connection);
connection = null;
} else if (automaticallyReleaseConnectionToPool) {
client.getConnectionPool().recycle(connection);
connection = null;
disconnect();
}
}
}
@@ -505,7 +457,7 @@ public class HttpEngine {
int contentLength = ((RetryableOutputStream) requestBodyOut).contentLength();
request = request.newBuilder().setContentLength(contentLength).build();
}
transport.writeRequestHeaders();
transport.writeRequestHeaders(request);
}
if (requestBodyOut != null) {
@@ -518,10 +470,13 @@ public class HttpEngine {
transport.flushRequest();
response = transport.readResponseHeaders()
.newBuilder()
.request(request)
.handshake(connection.getHandshake())
.setLocalTimestamps(sentRequestMillis, System.currentTimeMillis())
.setResponseSource(responseSource)
.build();
connection.setHttpMinorVersion(response.httpMinorVersion());
receiveHeaders(response.headers());
if (responseSource == ResponseSource.CONDITIONAL_CACHE) {
if (validatingResponse.validate(response)) {

View File

@@ -78,23 +78,23 @@ public final class HttpTransport implements Transport {
return request;
}
@Override public OutputStream createRequestBody() throws IOException {
@Override public OutputStream createRequestBody(Request request) throws IOException {
// Stream a request body of unknown length.
if (httpEngine.getRequest().isChunked()) {
if (request.isChunked()) {
int chunkLength = httpEngine.policy.getChunkLength();
if (chunkLength == -1) chunkLength = DEFAULT_CHUNK_LENGTH;
writeRequestHeaders();
writeRequestHeaders(request);
return new ChunkedOutputStream(requestOut, chunkLength);
}
// Stream a request body of a known length.
long fixedContentLength = httpEngine.policy.getFixedContentLength();
if (fixedContentLength != -1) {
writeRequestHeaders();
writeRequestHeaders(request);
return new FixedLengthOutputStream(requestOut, fixedContentLength);
}
long contentLength = httpEngine.getRequest().getContentLength();
long contentLength = request.getContentLength();
if (contentLength > Integer.MAX_VALUE) {
throw new IllegalArgumentException("Use setFixedLengthStreamingMode() or "
+ "setChunkedStreamingMode() for requests larger than 2 GiB.");
@@ -102,7 +102,7 @@ public final class HttpTransport implements Transport {
// Buffer a request body of a known length.
if (contentLength != -1) {
writeRequestHeaders();
writeRequestHeaders(request);
return new RetryableOutputStream((int) contentLength);
}
@@ -133,22 +133,16 @@ public final class HttpTransport implements Transport {
* This ensures that the {@code Content-Length} header field receives the
* proper value.
*/
public void writeRequestHeaders() throws IOException {
public void writeRequestHeaders(Request request) throws IOException {
httpEngine.writingRequestHeaders();
Headers headersToSend = httpEngine.getRequest().getHeaders();
String requestLine = RequestLine.get(httpEngine.getRequest(),
String requestLine = RequestLine.get(request,
httpEngine.connection.getRoute().getProxy().type(),
httpEngine.connection.getHttpMinorVersion());
writeRequest(requestOut, headersToSend, requestLine);
writeRequest(requestOut, request.getHeaders(), requestLine);
}
@Override public Response readResponseHeaders() throws IOException {
Response response = readResponse(httpEngine.getRequest(), socketIn)
.handshake(httpEngine.connection.getHandshake())
.build();
httpEngine.connection.setHttpMinorVersion(response.httpMinorVersion());
httpEngine.receiveHeaders(response.headers());
return response;
@Override public Response.Builder readResponseHeaders() throws IOException {
return readResponse(socketIn);
}
/** Returns bytes of a request header for sending on an HTTP transport. */
@@ -167,14 +161,14 @@ public final class HttpTransport implements Transport {
}
/** Parses bytes of a response header from an HTTP transport. */
public static Response.Builder readResponse(Request request, InputStream in) throws IOException {
public static Response.Builder readResponse(InputStream in) throws IOException {
while (true) {
String statusLineString = Util.readAsciiLine(in);
StatusLine statusLine = new StatusLine(statusLineString);
Response.Builder responseBuilder = new Response.Builder(request);
responseBuilder.statusLine(statusLine);
responseBuilder.header(SyntheticHeaders.SELECTED_TRANSPORT, "http/1.1");
Response.Builder responseBuilder = new Response.Builder()
.statusLine(statusLine)
.header(SyntheticHeaders.SELECTED_TRANSPORT, "http/1.1");
Headers.Builder headersBuilder = new Headers.Builder();
headersBuilder.readHeaders(in);

View File

@@ -45,7 +45,7 @@ public final class SpdyTransport implements Transport {
@Override public Request prepareRequest(Request request) {
Request.Builder builder = request.newBuilder()
.header(":method", request.method())
.header(":scheme", httpEngine.getRequest().url().getProtocol())
.header(":scheme", request.url().getProtocol())
.header(":path", RequestLine.requestPath(request.url()))
.header(":version", RequestLine.version(httpEngine.connection.getHttpMinorVersion()))
.header(":host", HttpEngine.hostHeader(request.url()));
@@ -60,20 +60,20 @@ public final class SpdyTransport implements Transport {
return builder.build();
}
@Override public OutputStream createRequestBody() throws IOException {
@Override public OutputStream createRequestBody(Request request) throws IOException {
// TODO: if we aren't streaming up to the server, we should buffer the whole request
writeRequestHeaders();
writeRequestHeaders(request);
return stream.getOutputStream();
}
@Override public void writeRequestHeaders() throws IOException {
@Override public void writeRequestHeaders(Request request) throws IOException {
if (stream != null) return;
httpEngine.writingRequestHeaders();
boolean hasRequestBody = httpEngine.hasRequestBody();
boolean hasResponseBody = true;
stream = spdyConnection.newStream(writeNameValueBlock(httpEngine.getRequest().getHeaders()),
hasRequestBody, hasResponseBody);
stream = spdyConnection.newStream(
writeNameValueBlock(request.getHeaders()), hasRequestBody, hasResponseBody);
stream.setReadTimeout(httpEngine.client.getReadTimeout());
}
@@ -85,14 +85,8 @@ public final class SpdyTransport implements Transport {
stream.getOutputStream().close();
}
@Override public Response readResponseHeaders() throws IOException {
List<String> nameValueBlock = stream.getResponseHeaders();
Response response = readNameValueBlock(httpEngine.getRequest(), nameValueBlock)
.handshake(httpEngine.connection.getHandshake())
.build();
httpEngine.connection.setHttpMinorVersion(response.httpMinorVersion());
httpEngine.receiveHeaders(response.headers());
return response;
@Override public Response.Builder readResponseHeaders() throws IOException {
return readNameValueBlock(stream.getResponseHeaders());
}
/**
@@ -135,7 +129,7 @@ public final class SpdyTransport implements Transport {
}
/** Returns headers for a name value block containing a SPDY response. */
public static Response.Builder readNameValueBlock(Request request, List<String> nameValueBlock)
public static Response.Builder readNameValueBlock(List<String> nameValueBlock)
throws IOException {
if (nameValueBlock.size() % 2 != 0) {
throw new IllegalArgumentException("Unexpected name value block: " + nameValueBlock);
@@ -167,7 +161,7 @@ public final class SpdyTransport implements Transport {
if (status == null) throw new ProtocolException("Expected ':status' header not present");
if (version == null) throw new ProtocolException("Expected ':version' header not present");
return new Response.Builder(request)
return new Response.Builder()
.statusLine(new StatusLine(version + " " + status))
.headers(headersBuilder.build());
}

View File

@@ -47,10 +47,10 @@ interface Transport {
*/
// TODO: don't bother retransmitting the request body? It's quite a corner
// case and there's uncertainty whether Firefox or Chrome do this
OutputStream createRequestBody() throws IOException;
OutputStream createRequestBody(Request request) throws IOException;
/** This should update the HTTP engine's sentRequestMillis field. */
void writeRequestHeaders() throws IOException;
void writeRequestHeaders(Request request) throws IOException;
/**
* Sends the request body returned by {@link #createRequestBody} to the
@@ -62,7 +62,7 @@ interface Transport {
void flushRequest() throws IOException;
/** Read response headers and update the cookie manager. */
Response readResponseHeaders() throws IOException;
Response.Builder readResponseHeaders() throws IOException;
// TODO: make this the content stream?
InputStream getTransferStream(CacheRequest cacheRequest) throws IOException;

View File

@@ -33,7 +33,7 @@ public final class HeadersTest {
":status", "200 OK",
":version", "HTTP/1.1");
Request request = new Request.Builder().url("http://square.com/").build();
Response response = SpdyTransport.readNameValueBlock(request, nameValueBlock).build();
Response response = SpdyTransport.readNameValueBlock(nameValueBlock).request(request).build();
Headers headers = response.headers();
assertEquals(4, headers.length());
assertEquals("HTTP/1.1 200 OK", response.statusLine());