diff --git a/okhttp-android-support/src/main/java/okhttp3/internal/huc/JavaApiConverter.java b/okhttp-android-support/src/main/java/okhttp3/internal/huc/JavaApiConverter.java index 0c7a75a87..2465372a5 100644 --- a/okhttp-android-support/src/main/java/okhttp3/internal/huc/JavaApiConverter.java +++ b/okhttp-android-support/src/main/java/okhttp3/internal/huc/JavaApiConverter.java @@ -106,7 +106,7 @@ public final class JavaApiConverter { okResponseBuilder.networkResponse(networkResponse); // Response headers - Headers okHeaders = extractOkResponseHeaders(httpUrlConnection); + Headers okHeaders = extractOkResponseHeaders(httpUrlConnection, okResponseBuilder); okResponseBuilder.headers(okHeaders); // Response body @@ -230,7 +230,7 @@ public final class JavaApiConverter { okResponseBuilder.message(statusLine.message); // Response headers - Headers okHeaders = extractOkHeaders(javaResponse); + Headers okHeaders = extractOkHeaders(javaResponse, okResponseBuilder); okResponseBuilder.headers(okHeaders); // Response body @@ -281,7 +281,7 @@ public final class JavaApiConverter { .method(requestMethod, placeholderBody); if (requestHeaders != null) { - Headers headers = extractOkHeaders(requestHeaders); + Headers headers = extractOkHeaders(requestHeaders, null); builder.headers(headers); } return builder.build(); @@ -292,7 +292,7 @@ public final class JavaApiConverter { * from the supplied {@link Response}. */ public static CacheResponse createJavaCacheResponse(final Response response) { - final Headers headers = response.headers(); + final Headers headers = withSyntheticHeaders(response); final ResponseBody body = response.body(); if (response.request().isHttps()) { final Handshake handshake = response.handshake(); @@ -382,6 +382,10 @@ public final class JavaApiConverter { * {@link Response}. */ static HttpURLConnection createJavaUrlConnectionForCachePut(Response okResponse) { + okResponse = okResponse.newBuilder() + .body(null) + .headers(withSyntheticHeaders(okResponse)) + .build(); Request request = okResponse.request(); // Create an object of the correct class in case the ResponseCache uses instanceof. if (request.isHttps()) { @@ -391,6 +395,13 @@ public final class JavaApiConverter { } } + private static Headers withSyntheticHeaders(Response okResponse) { + return okResponse.headers().newBuilder() + .add(OkHeaders.SENT_MILLIS, Long.toString(okResponse.sentRequestAtMillis())) + .add(OkHeaders.RECEIVED_MILLIS, Long.toString(okResponse.receivedResponseAtMillis())) + .build(); + } + /** * Extracts an immutable request header map from the supplied {@link Headers}. */ @@ -402,26 +413,30 @@ public final class JavaApiConverter { * Extracts OkHttp headers from the supplied {@link java.net.CacheResponse}. Only real headers are * extracted. See {@link #extractStatusLine(java.net.CacheResponse)}. */ - private static Headers extractOkHeaders(CacheResponse javaResponse) throws IOException { + private static Headers extractOkHeaders( + CacheResponse javaResponse, Response.Builder okResponseBuilder) throws IOException { Map> javaResponseHeaders = javaResponse.getHeaders(); - return extractOkHeaders(javaResponseHeaders); + return extractOkHeaders(javaResponseHeaders, okResponseBuilder); } /** * Extracts OkHttp headers from the supplied {@link java.net.HttpURLConnection}. Only real headers * are extracted. See {@link #extractStatusLine(java.net.HttpURLConnection)}. */ - private static Headers extractOkResponseHeaders(HttpURLConnection httpUrlConnection) { + private static Headers extractOkResponseHeaders( + HttpURLConnection httpUrlConnection, Response.Builder okResponseBuilder) { Map> javaResponseHeaders = httpUrlConnection.getHeaderFields(); - return extractOkHeaders(javaResponseHeaders); + return extractOkHeaders(javaResponseHeaders, okResponseBuilder); } /** * Extracts OkHttp headers from the supplied {@link Map}. Only real headers are extracted. Any - * entry (one with a {@code null} key) is discarded. + * entry (one with a {@code null} key) is discarded. Special internal headers used to track cache + * metadata are omitted from the result and added to {@code okResponseBuilder} instead. */ // @VisibleForTesting - static Headers extractOkHeaders(Map> javaHeaders) { + static Headers extractOkHeaders( + Map> javaHeaders, Response.Builder okResponseBuilder) { Headers.Builder okHeadersBuilder = new Headers.Builder(); for (Map.Entry> javaHeader : javaHeaders.entrySet()) { String name = javaHeader.getKey(); @@ -432,6 +447,16 @@ public final class JavaApiConverter { // explicitly ignored because Headers.Builder does not support null keys. continue; } + if (okResponseBuilder != null && javaHeader.getValue().size() == 1) { + if (name.equals(OkHeaders.SENT_MILLIS)) { + okResponseBuilder.sentRequestAtMillis(Long.valueOf(javaHeader.getValue().get(0))); + continue; + } + if (name.equals(OkHeaders.RECEIVED_MILLIS)) { + okResponseBuilder.receivedResponseAtMillis(Long.valueOf(javaHeader.getValue().get(0))); + continue; + } + } for (String value : javaHeader.getValue()) { Internal.instance.addLenient(okHeadersBuilder, name, value); } diff --git a/okhttp-android-support/src/test/java/okhttp3/internal/huc/JavaApiConverterTest.java b/okhttp-android-support/src/test/java/okhttp3/internal/huc/JavaApiConverterTest.java index 13f938249..f4c9b95da 100644 --- a/okhttp-android-support/src/test/java/okhttp3/internal/huc/JavaApiConverterTest.java +++ b/okhttp-android-support/src/test/java/okhttp3/internal/huc/JavaApiConverterTest.java @@ -597,7 +597,7 @@ public class JavaApiConverterTest { javaResponseHeaders.put("key1", Arrays.asList("value1_1", "value1_2")); javaResponseHeaders.put("key2", Arrays.asList("value2")); - Headers okHeaders = JavaApiConverter.extractOkHeaders(javaResponseHeaders); + Headers okHeaders = JavaApiConverter.extractOkHeaders(javaResponseHeaders, null); assertEquals(3, okHeaders.size()); // null entry should be stripped out assertEquals(Arrays.asList("value1_1", "value1_2"), okHeaders.values("key1")); assertEquals(Arrays.asList("value2"), okHeaders.values("key2")); diff --git a/okhttp-logging-interceptor/src/test/java/okhttp3/logging/HttpLoggingInterceptorTest.java b/okhttp-logging-interceptor/src/test/java/okhttp3/logging/HttpLoggingInterceptorTest.java index 3159ad2f7..429aa1212 100644 --- a/okhttp-logging-interceptor/src/test/java/okhttp3/logging/HttpLoggingInterceptorTest.java +++ b/okhttp-logging-interceptor/src/test/java/okhttp3/logging/HttpLoggingInterceptorTest.java @@ -193,8 +193,6 @@ public final class HttpLoggingInterceptorTest { .assertLogEqual("--> END GET") .assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms\\)") .assertLogEqual("Content-Length: 0") - .assertLogMatch("OkHttp-Sent-Millis: \\d+") - .assertLogMatch("OkHttp-Received-Millis: \\d+") .assertLogEqual("<-- END HTTP") .assertNoMoreLogs(); @@ -207,8 +205,6 @@ public final class HttpLoggingInterceptorTest { .assertLogEqual("--> END GET") .assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms\\)") .assertLogEqual("Content-Length: 0") - .assertLogMatch("OkHttp-Sent-Millis: \\d+") - .assertLogMatch("OkHttp-Received-Millis: \\d+") .assertLogEqual("<-- END HTTP") .assertNoMoreLogs(); } @@ -228,8 +224,6 @@ public final class HttpLoggingInterceptorTest { .assertLogEqual("--> END POST") .assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms\\)") .assertLogEqual("Content-Length: 0") - .assertLogMatch("OkHttp-Sent-Millis: \\d+") - .assertLogMatch("OkHttp-Received-Millis: \\d+") .assertLogEqual("<-- END HTTP") .assertNoMoreLogs(); @@ -244,8 +238,6 @@ public final class HttpLoggingInterceptorTest { .assertLogEqual("--> END POST") .assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms\\)") .assertLogEqual("Content-Length: 0") - .assertLogMatch("OkHttp-Sent-Millis: \\d+") - .assertLogMatch("OkHttp-Received-Millis: \\d+") .assertLogEqual("<-- END HTTP") .assertNoMoreLogs(); } @@ -264,8 +256,6 @@ public final class HttpLoggingInterceptorTest { .assertLogEqual("--> END POST") .assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms\\)") .assertLogEqual("Content-Length: 0") - .assertLogMatch("OkHttp-Sent-Millis: \\d+") - .assertLogMatch("OkHttp-Received-Millis: \\d+") .assertLogEqual("<-- END HTTP") .assertNoMoreLogs(); @@ -279,8 +269,6 @@ public final class HttpLoggingInterceptorTest { .assertLogEqual("--> END POST") .assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms\\)") .assertLogEqual("Content-Length: 0") - .assertLogMatch("OkHttp-Sent-Millis: \\d+") - .assertLogMatch("OkHttp-Received-Millis: \\d+") .assertLogEqual("<-- END HTTP") .assertNoMoreLogs(); } @@ -307,8 +295,6 @@ public final class HttpLoggingInterceptorTest { .assertLogEqual("--> END POST") .assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms\\)") .assertLogEqual("Content-Length: 0") - .assertLogMatch("OkHttp-Sent-Millis: \\d+") - .assertLogMatch("OkHttp-Received-Millis: \\d+") .assertLogEqual("<-- END HTTP") .assertNoMoreLogs(); @@ -323,8 +309,6 @@ public final class HttpLoggingInterceptorTest { .assertLogEqual("--> END POST") .assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms\\)") .assertLogEqual("Content-Length: 0") - .assertLogMatch("OkHttp-Sent-Millis: \\d+") - .assertLogMatch("OkHttp-Received-Millis: \\d+") .assertLogEqual("<-- END HTTP") .assertNoMoreLogs(); } @@ -344,8 +328,6 @@ public final class HttpLoggingInterceptorTest { .assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms\\)") .assertLogEqual("Content-Length: 6") .assertLogEqual("Content-Type: text/plain; charset=utf-8") - .assertLogMatch("OkHttp-Sent-Millis: \\d+") - .assertLogMatch("OkHttp-Received-Millis: \\d+") .assertLogEqual("<-- END HTTP") .assertNoMoreLogs(); @@ -359,8 +341,6 @@ public final class HttpLoggingInterceptorTest { .assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms\\)") .assertLogEqual("Content-Length: 6") .assertLogEqual("Content-Type: text/plain; charset=utf-8") - .assertLogMatch("OkHttp-Sent-Millis: \\d+") - .assertLogMatch("OkHttp-Received-Millis: \\d+") .assertLogEqual("<-- END HTTP") .assertNoMoreLogs(); } @@ -377,8 +357,6 @@ public final class HttpLoggingInterceptorTest { .assertLogEqual("--> END GET") .assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms\\)") .assertLogEqual("Content-Length: 0") - .assertLogMatch("OkHttp-Sent-Millis: \\d+") - .assertLogMatch("OkHttp-Received-Millis: \\d+") .assertLogEqual("<-- END HTTP (0-byte body)") .assertNoMoreLogs(); @@ -391,8 +369,6 @@ public final class HttpLoggingInterceptorTest { .assertLogEqual("--> END GET") .assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms\\)") .assertLogEqual("Content-Length: 0") - .assertLogMatch("OkHttp-Sent-Millis: \\d+") - .assertLogMatch("OkHttp-Received-Millis: \\d+") .assertLogEqual("<-- END HTTP (0-byte body)") .assertNoMoreLogs(); } @@ -418,8 +394,6 @@ public final class HttpLoggingInterceptorTest { .assertLogEqual("--> END GET") .assertLogMatch("<-- " + code + " No Content " + url + " \\(\\d+ms\\)") .assertLogEqual("Content-Length: 0") - .assertLogMatch("OkHttp-Sent-Millis: \\d+") - .assertLogMatch("OkHttp-Received-Millis: \\d+") .assertLogEqual("<-- END HTTP (0-byte body)") .assertNoMoreLogs(); @@ -432,8 +406,6 @@ public final class HttpLoggingInterceptorTest { .assertLogEqual("--> END GET") .assertLogMatch("<-- " + code + " No Content " + url + " \\(\\d+ms\\)") .assertLogEqual("Content-Length: 0") - .assertLogMatch("OkHttp-Sent-Millis: \\d+") - .assertLogMatch("OkHttp-Received-Millis: \\d+") .assertLogEqual("<-- END HTTP (0-byte body)") .assertNoMoreLogs(); } @@ -455,8 +427,6 @@ public final class HttpLoggingInterceptorTest { .assertLogEqual("--> END POST (3-byte body)") .assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms\\)") .assertLogEqual("Content-Length: 0") - .assertLogMatch("OkHttp-Sent-Millis: \\d+") - .assertLogMatch("OkHttp-Received-Millis: \\d+") .assertLogEqual("<-- END HTTP (0-byte body)") .assertNoMoreLogs(); @@ -473,8 +443,6 @@ public final class HttpLoggingInterceptorTest { .assertLogEqual("--> END POST (3-byte body)") .assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms\\)") .assertLogEqual("Content-Length: 0") - .assertLogMatch("OkHttp-Sent-Millis: \\d+") - .assertLogMatch("OkHttp-Received-Millis: \\d+") .assertLogEqual("<-- END HTTP (0-byte body)") .assertNoMoreLogs(); } @@ -494,8 +462,6 @@ public final class HttpLoggingInterceptorTest { .assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms\\)") .assertLogEqual("Content-Length: 6") .assertLogEqual("Content-Type: text/plain; charset=utf-8") - .assertLogMatch("OkHttp-Sent-Millis: \\d+") - .assertLogMatch("OkHttp-Received-Millis: \\d+") .assertLogEqual("") .assertLogEqual("Hello!") .assertLogEqual("<-- END HTTP (6-byte body)") @@ -511,8 +477,6 @@ public final class HttpLoggingInterceptorTest { .assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms\\)") .assertLogEqual("Content-Length: 6") .assertLogEqual("Content-Type: text/plain; charset=utf-8") - .assertLogMatch("OkHttp-Sent-Millis: \\d+") - .assertLogMatch("OkHttp-Received-Millis: \\d+") .assertLogEqual("") .assertLogEqual("Hello!") .assertLogEqual("<-- END HTTP (6-byte body)") @@ -534,8 +498,6 @@ public final class HttpLoggingInterceptorTest { .assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms\\)") .assertLogEqual("Transfer-encoding: chunked") .assertLogEqual("Content-Type: text/plain; charset=utf-8") - .assertLogMatch("OkHttp-Sent-Millis: \\d+") - .assertLogMatch("OkHttp-Received-Millis: \\d+") .assertLogEqual("") .assertLogEqual("Hello!") .assertLogEqual("<-- END HTTP (6-byte body)") @@ -551,8 +513,6 @@ public final class HttpLoggingInterceptorTest { .assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms\\)") .assertLogEqual("Transfer-encoding: chunked") .assertLogEqual("Content-Type: text/plain; charset=utf-8") - .assertLogMatch("OkHttp-Sent-Millis: \\d+") - .assertLogMatch("OkHttp-Received-Millis: \\d+") .assertLogEqual("") .assertLogEqual("Hello!") .assertLogEqual("<-- END HTTP (6-byte body)") @@ -581,8 +541,6 @@ public final class HttpLoggingInterceptorTest { .assertLogEqual("Content-Encoding: gzip") .assertLogEqual("Content-Type: text/plain; charset=utf-8") .assertLogMatch("Content-Length: \\d+") - .assertLogMatch("OkHttp-Sent-Millis: \\d+") - .assertLogMatch("OkHttp-Received-Millis: \\d+") .assertLogEqual("<-- END HTTP (encoded body omitted)") .assertNoMoreLogs(); @@ -591,8 +549,6 @@ public final class HttpLoggingInterceptorTest { .assertLogEqual("--> END GET") .assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms\\)") .assertLogEqual("Content-Type: text/plain; charset=utf-8") - .assertLogMatch("OkHttp-Sent-Millis: \\d+") - .assertLogMatch("OkHttp-Received-Millis: \\d+") .assertLogEqual("") .assertLogEqual("Hello, Hello, Hello") .assertLogEqual("<-- END HTTP (19-byte body)") @@ -618,8 +574,6 @@ public final class HttpLoggingInterceptorTest { .assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms\\)") .assertLogEqual("Content-Type: text/html; charset=0") .assertLogMatch("Content-Length: \\d+") - .assertLogMatch("OkHttp-Sent-Millis: \\d+") - .assertLogMatch("OkHttp-Received-Millis: \\d+") .assertLogMatch("") .assertLogEqual("Couldn't decode the response body; charset is likely malformed.") .assertLogEqual("<-- END HTTP") @@ -631,8 +585,6 @@ public final class HttpLoggingInterceptorTest { .assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms\\)") .assertLogEqual("Content-Type: text/html; charset=0") .assertLogMatch("Content-Length: \\d+") - .assertLogMatch("OkHttp-Sent-Millis: \\d+") - .assertLogMatch("OkHttp-Received-Millis: \\d+") .assertLogEqual("") .assertLogEqual("Couldn't decode the response body; charset is likely malformed.") .assertLogEqual("<-- END HTTP") @@ -670,8 +622,6 @@ public final class HttpLoggingInterceptorTest { .assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms\\)") .assertLogEqual("Content-Length: 9") .assertLogEqual("Content-Type: image/png; charset=utf-8") - .assertLogMatch("OkHttp-Sent-Millis: \\d+") - .assertLogMatch("OkHttp-Received-Millis: \\d+") .assertLogEqual("") .assertLogEqual("<-- END HTTP (binary 9-byte body omitted)") .assertNoMoreLogs(); @@ -686,8 +636,6 @@ public final class HttpLoggingInterceptorTest { .assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms\\)") .assertLogEqual("Content-Length: 9") .assertLogEqual("Content-Type: image/png; charset=utf-8") - .assertLogMatch("OkHttp-Sent-Millis: \\d+") - .assertLogMatch("OkHttp-Received-Millis: \\d+") .assertLogEqual("") .assertLogEqual("<-- END HTTP (binary 9-byte body omitted)") .assertNoMoreLogs(); diff --git a/okhttp-tests/src/test/java/okhttp3/CallTest.java b/okhttp-tests/src/test/java/okhttp3/CallTest.java index a847af99e..18ede8338 100644 --- a/okhttp-tests/src/test/java/okhttp3/CallTest.java +++ b/okhttp-tests/src/test/java/okhttp3/CallTest.java @@ -113,13 +113,25 @@ public final class CallTest { } @Test public void get() throws Exception { - server.enqueue(new MockResponse().setBody("abc").addHeader("Content-Type: text/plain")); + server.enqueue(new MockResponse() + .setBody("abc") + .clearHeaders() + .addHeader("content-type: text/plain") + .addHeader("content-length", "3")); - executeSynchronously("/", "User-Agent", "SyncApiTest") - .assertCode(200) + long sentAt = System.currentTimeMillis(); + RecordedResponse recordedResponse = executeSynchronously("/", "User-Agent", "SyncApiTest"); + long receivedAt = System.currentTimeMillis(); + + recordedResponse.assertCode(200) .assertSuccessful() - .assertHeader("Content-Type", "text/plain") - .assertBody("abc"); + .assertHeaders(new Headers.Builder() + .add("content-type", "text/plain") + .add("content-length", "3") + .build()) + .assertBody("abc") + .assertSentRequestAtMillis(sentAt, receivedAt) + .assertReceivedResponseAtMillis(sentAt, receivedAt); RecordedRequest recordedRequest = server.takeRequest(); assertEquals("GET", recordedRequest.getMethod()); @@ -1178,31 +1190,48 @@ public final class CallTest { // Store a response in the cache. HttpUrl url = server.url("/"); + long request1SentAt = System.currentTimeMillis(); executeSynchronously("/", "Accept-Language", "fr-CA", "Accept-Charset", "UTF-8") .assertCode(200) .assertBody("A"); + long request1ReceivedAt = System.currentTimeMillis(); assertNull(server.takeRequest().getHeader("If-None-Match")); // Hit that stored response. It's different, but Vary says it doesn't matter. + Thread.sleep(10); // Make sure the timestamps are unique. RecordedResponse cacheHit = executeSynchronously( "/", "Accept-Language", "en-US", "Accept-Charset", "UTF-8"); // Check the merged response. The request is the application's original request. cacheHit.assertCode(200) .assertBody("A") - .assertHeader("ETag", "v1") + .assertHeaders(new Headers.Builder() + .add("ETag", "v1") + .add("Cache-Control", "max-age=60") + .add("Vary", "Accept-Charset") + .add("Content-Length", "1") + .build()) .assertRequestUrl(url) .assertRequestHeader("Accept-Language", "en-US") - .assertRequestHeader("Accept-Charset", "UTF-8"); + .assertRequestHeader("Accept-Charset", "UTF-8") + .assertSentRequestAtMillis(request1SentAt, request1ReceivedAt) + .assertReceivedResponseAtMillis(request1SentAt, request1ReceivedAt); // Check the cached response. Its request contains only the saved Vary headers. cacheHit.cacheResponse() .assertCode(200) - .assertHeader("ETag", "v1") + .assertHeaders(new Headers.Builder() + .add("ETag", "v1") + .add("Cache-Control", "max-age=60") + .add("Vary", "Accept-Charset") + .add("Content-Length", "1") + .build()) .assertRequestMethod("GET") .assertRequestUrl(url) .assertRequestHeader("Accept-Language") - .assertRequestHeader("Accept-Charset", "UTF-8"); + .assertRequestHeader("Accept-Charset", "UTF-8") + .assertSentRequestAtMillis(request1SentAt, request1ReceivedAt) + .assertReceivedResponseAtMillis(request1SentAt, request1ReceivedAt); cacheHit.assertNoNetworkResponse(); } @@ -1222,15 +1251,20 @@ public final class CallTest { .build(); // Store a response in the cache. + long request1At = System.currentTimeMillis(); executeSynchronously("/", "Accept-Language", "fr-CA", "Accept-Charset", "UTF-8") .assertCode(200) .assertHeader("Donut", "a") .assertBody("A"); + long request1ReceivedAt = System.currentTimeMillis(); assertNull(server.takeRequest().getHeader("If-None-Match")); // Hit that stored response. It's different, but Vary says it doesn't matter. + Thread.sleep(10); // Make sure the timestamps are unique. + long request2SentAt = System.currentTimeMillis(); RecordedResponse cacheHit = executeSynchronously( "/", "Accept-Language", "en-US", "Accept-Charset", "UTF-8"); + long request2ReceivedAt = System.currentTimeMillis(); assertEquals("v1", server.takeRequest().getHeader("If-None-Match")); // Check the merged response. The request is the application's original request. @@ -1240,7 +1274,9 @@ public final class CallTest { .assertRequestUrl(server.url("/")) .assertRequestHeader("Accept-Language", "en-US") .assertRequestHeader("Accept-Charset", "UTF-8") - .assertRequestHeader("If-None-Match"); // No If-None-Match on the user's request. + .assertRequestHeader("If-None-Match") // No If-None-Match on the user's request. + .assertSentRequestAtMillis(request1At, request1ReceivedAt) + .assertReceivedResponseAtMillis(request1At, request1ReceivedAt); // Check the cached response. Its request contains only the saved Vary headers. cacheHit.cacheResponse() @@ -1250,7 +1286,9 @@ public final class CallTest { .assertRequestUrl(server.url("/")) .assertRequestHeader("Accept-Language") // No Vary on Accept-Language. .assertRequestHeader("Accept-Charset", "UTF-8") // Because of Vary on Accept-Charset. - .assertRequestHeader("If-None-Match"); // This wasn't present in the original request. + .assertRequestHeader("If-None-Match") // This wasn't present in the original request. + .assertSentRequestAtMillis(request1At, request1ReceivedAt) + .assertReceivedResponseAtMillis(request1At, request1ReceivedAt); // Check the network response. It has the caller's request, plus some caching headers. cacheHit.networkResponse() @@ -1258,7 +1296,9 @@ public final class CallTest { .assertHeader("Donut", "b") .assertRequestHeader("Accept-Language", "en-US") .assertRequestHeader("Accept-Charset", "UTF-8") - .assertRequestHeader("If-None-Match", "v1"); // If-None-Match in the validation request. + .assertRequestHeader("If-None-Match", "v1") // If-None-Match in the validation request. + .assertSentRequestAtMillis(request2SentAt, request2ReceivedAt) + .assertReceivedResponseAtMillis(request2SentAt, request2ReceivedAt); } @Test public void conditionalCacheHit_Async() throws Exception { @@ -1300,35 +1340,46 @@ public final class CallTest { .cache(cache) .build(); + long request1SentAt = System.currentTimeMillis(); executeSynchronously("/", "Accept-Language", "fr-CA", "Accept-Charset", "UTF-8") .assertCode(200) .assertBody("A"); + long request1ReceivedAt = System.currentTimeMillis(); assertNull(server.takeRequest().getHeader("If-None-Match")); // Different request, but Vary says it doesn't matter. - RecordedResponse cacheHit = executeSynchronously( + Thread.sleep(10); // Make sure the timestamps are unique. + long request2SentAt = System.currentTimeMillis(); + RecordedResponse cacheMiss = executeSynchronously( "/", "Accept-Language", "en-US", "Accept-Charset", "UTF-8"); + long request2ReceivedAt = System.currentTimeMillis(); assertEquals("v1", server.takeRequest().getHeader("If-None-Match")); // Check the user response. It has the application's original request. - cacheHit.assertCode(200) + cacheMiss.assertCode(200) .assertBody("B") .assertHeader("Donut", "b") - .assertRequestUrl(server.url("/")); + .assertRequestUrl(server.url("/")) + .assertSentRequestAtMillis(request2SentAt, request2ReceivedAt) + .assertReceivedResponseAtMillis(request2SentAt, request2ReceivedAt); // Check the cache response. Even though it's a miss, we used the cache. - cacheHit.cacheResponse() + cacheMiss.cacheResponse() .assertCode(200) .assertHeader("Donut", "a") .assertHeader("ETag", "v1") - .assertRequestUrl(server.url("/")); + .assertRequestUrl(server.url("/")) + .assertSentRequestAtMillis(request1SentAt, request1ReceivedAt) + .assertReceivedResponseAtMillis(request1SentAt, request1ReceivedAt); // Check the network response. It has the network request, plus caching headers. - cacheHit.networkResponse() + cacheMiss.networkResponse() .assertCode(200) .assertHeader("Donut", "b") .assertRequestHeader("If-None-Match", "v1") // If-None-Match in the validation request. - .assertRequestUrl(server.url("/")); + .assertRequestUrl(server.url("/")) + .assertSentRequestAtMillis(request2SentAt, request2ReceivedAt) + .assertReceivedResponseAtMillis(request2SentAt, request2ReceivedAt); } @Test public void conditionalCacheMiss_Async() throws Exception { diff --git a/okhttp-tests/src/test/java/okhttp3/RecordedResponse.java b/okhttp-tests/src/test/java/okhttp3/RecordedResponse.java index 6e1f0728b..ae7afa98d 100644 --- a/okhttp-tests/src/test/java/okhttp3/RecordedResponse.java +++ b/okhttp-tests/src/test/java/okhttp3/RecordedResponse.java @@ -16,7 +16,9 @@ package okhttp3; import java.io.IOException; +import java.text.SimpleDateFormat; import java.util.Arrays; +import java.util.Date; import okhttp3.ws.WebSocket; import static org.junit.Assert.assertEquals; @@ -79,6 +81,11 @@ public final class RecordedResponse { return this; } + public RecordedResponse assertHeaders(Headers headers) { + assertEquals(headers, response.headers()); + return this; + } + public RecordedResponse assertBody(String expectedBody) { assertEquals(expectedBody, body); return this; @@ -147,4 +154,23 @@ public final class RecordedResponse { assertTrue(failure.getMessage(), Arrays.asList(messages).contains(failure.getMessage())); return this; } + + public RecordedResponse assertSentRequestAtMillis(long minimum, long maximum) { + assertDateInRange(minimum, response.sentRequestAtMillis(), maximum); + return this; + } + + public RecordedResponse assertReceivedResponseAtMillis(long minimum, long maximum) { + assertDateInRange(minimum, response.receivedResponseAtMillis(), maximum); + return this; + } + + private void assertDateInRange(long minimum, long actual, long maximum) { + assertTrue("actual " + format(actual) + " < minimum " + format(maximum), actual >= minimum); + assertTrue("actual " + format(actual) + " > maximum " + format(minimum), actual <= maximum); + } + + private String format(long time) { + return new SimpleDateFormat("HH:mm:ss.SSS").format(new Date(time)); + } } diff --git a/okhttp-tests/src/test/java/okhttp3/internal/http/HeadersTest.java b/okhttp-tests/src/test/java/okhttp3/internal/http/HeadersTest.java index 4e649dc88..84f37d13a 100644 --- a/okhttp-tests/src/test/java/okhttp3/internal/http/HeadersTest.java +++ b/okhttp-tests/src/test/java/okhttp3/internal/http/HeadersTest.java @@ -31,7 +31,9 @@ import org.junit.Test; import static okhttp3.TestUtil.headerEntries; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; public final class HeadersTest { @@ -303,6 +305,16 @@ public final class HeadersTest { assertEquals(1, headerMap.get("user-agent").size()); } + @Test public void toMultimapUsesCanonicalCase() { + Headers headers = Headers.of( + "cache-control", "no-store", + "Cache-Control", "no-cache", + "User-Agent", "OkHttp"); + Map> headerMap = headers.toMultimap(); + assertEquals(2, headerMap.get("cache-control").size()); + assertEquals(1, headerMap.get("user-agent").size()); + } + @Test public void nameIndexesAreStrict() { Headers headers = Headers.of("a", "b", "c", "d"); try { @@ -354,4 +366,30 @@ public final class HeadersTest { expected.getMessage()); } } + + @Test public void headersEquals() { + Headers headers1 = new Headers.Builder() + .add("Connection", "close") + .add("Transfer-Encoding", "chunked") + .build(); + Headers headers2 = new Headers.Builder() + .add("Connection", "close") + .add("Transfer-Encoding", "chunked") + .build(); + assertTrue(headers1.equals(headers2)); + assertEquals(headers1.hashCode(), headers2.hashCode()); + } + + @Test public void headersNotEquals() { + Headers headers1 = new Headers.Builder() + .add("Connection", "close") + .add("Transfer-Encoding", "chunked") + .build(); + Headers headers2 = new Headers.Builder() + .add("Connection", "keep-alive") + .add("Transfer-Encoding", "chunked") + .build(); + assertFalse(headers1.equals(headers2)); + assertFalse(headers1.hashCode() == headers2.hashCode()); + } } diff --git a/okhttp-urlconnection/src/test/java/okhttp3/internal/huc/URLEncodingTest.java b/okhttp-urlconnection/src/test/java/okhttp3/internal/huc/URLEncodingTest.java index b61f266f4..1cc63d8f3 100644 --- a/okhttp-urlconnection/src/test/java/okhttp3/internal/huc/URLEncodingTest.java +++ b/okhttp-urlconnection/src/test/java/okhttp3/internal/huc/URLEncodingTest.java @@ -125,35 +125,25 @@ public final class URLEncodingTest { OkHttpClient.Builder builder = new OkHttpClient.Builder(); Internal.instance.setCache(builder, new InternalCache() { - @Override - public Response get(Request request) throws IOException { + @Override public Response get(Request request) throws IOException { uriReference.set(request.url().uri()); throw new UnsupportedOperationException(); } - @Override - public CacheRequest put(Response response) throws IOException { + @Override public CacheRequest put(Response response) throws IOException { return null; } - @Override - public void remove(Request request) throws IOException { - + @Override public void remove(Request request) throws IOException { } - @Override - public void update(Response cached, Response network) throws IOException { - + @Override public void update(Response cached, Response network) throws IOException { } - @Override - public void trackConditionalCacheHit() { - + @Override public void trackConditionalCacheHit() { } - @Override - public void trackResponse(CacheStrategy cacheStrategy) { - + @Override public void trackResponse(CacheStrategy cacheStrategy) { } }); diff --git a/okhttp/src/main/java/okhttp3/Cache.java b/okhttp/src/main/java/okhttp3/Cache.java index 7e0b2431a..7473edef7 100644 --- a/okhttp/src/main/java/okhttp3/Cache.java +++ b/okhttp/src/main/java/okhttp3/Cache.java @@ -478,6 +478,8 @@ public final class Cache implements Closeable, Flushable { private final String message; private final Headers responseHeaders; private final Handshake handshake; + private final long sentRequestMillis; + private final long receivedResponseMillis; /** * Reads an entry from an input stream. A typical entry looks like this: @@ -548,6 +550,16 @@ public final class Cache implements Closeable, Flushable { for (int i = 0; i < responseHeaderLineCount; i++) { responseHeadersBuilder.addLenient(source.readUtf8LineStrict()); } + String sendRequestMillisString = responseHeadersBuilder.get(OkHeaders.SENT_MILLIS); + String receivedResponseMillisString = responseHeadersBuilder.get(OkHeaders.RECEIVED_MILLIS); + responseHeadersBuilder.removeAll(OkHeaders.SENT_MILLIS); + responseHeadersBuilder.removeAll(OkHeaders.RECEIVED_MILLIS); + sentRequestMillis = sendRequestMillisString != null + ? Long.parseLong(sendRequestMillisString) + : 0L; + receivedResponseMillis = receivedResponseMillisString != null + ? Long.parseLong(receivedResponseMillisString) + : 0L; responseHeaders = responseHeadersBuilder.build(); if (isHttps()) { @@ -580,45 +592,55 @@ public final class Cache implements Closeable, Flushable { this.message = response.message(); this.responseHeaders = response.headers(); this.handshake = response.handshake(); + this.sentRequestMillis = response.sentRequestAtMillis(); + this.receivedResponseMillis = response.receivedResponseAtMillis(); } public void writeTo(DiskLruCache.Editor editor) throws IOException { BufferedSink sink = Okio.buffer(editor.newSink(ENTRY_METADATA)); - sink.writeUtf8(url); - sink.writeByte('\n'); - sink.writeUtf8(requestMethod); - sink.writeByte('\n'); - sink.writeDecimalLong(varyHeaders.size()); - sink.writeByte('\n'); + sink.writeUtf8(url) + .writeByte('\n'); + sink.writeUtf8(requestMethod) + .writeByte('\n'); + sink.writeDecimalLong(varyHeaders.size()) + .writeByte('\n'); for (int i = 0, size = varyHeaders.size(); i < size; i++) { - sink.writeUtf8(varyHeaders.name(i)); - sink.writeUtf8(": "); - sink.writeUtf8(varyHeaders.value(i)); - sink.writeByte('\n'); + sink.writeUtf8(varyHeaders.name(i)) + .writeUtf8(": ") + .writeUtf8(varyHeaders.value(i)) + .writeByte('\n'); } - sink.writeUtf8(new StatusLine(protocol, code, message).toString()); - sink.writeByte('\n'); - sink.writeDecimalLong(responseHeaders.size()); - sink.writeByte('\n'); + sink.writeUtf8(new StatusLine(protocol, code, message).toString()) + .writeByte('\n'); + sink.writeDecimalLong(responseHeaders.size() + 2) + .writeByte('\n'); for (int i = 0, size = responseHeaders.size(); i < size; i++) { - sink.writeUtf8(responseHeaders.name(i)); - sink.writeUtf8(": "); - sink.writeUtf8(responseHeaders.value(i)); - sink.writeByte('\n'); + sink.writeUtf8(responseHeaders.name(i)) + .writeUtf8(": ") + .writeUtf8(responseHeaders.value(i)) + .writeByte('\n'); } + sink.writeUtf8(OkHeaders.SENT_MILLIS) + .writeUtf8(": ") + .writeDecimalLong(sentRequestMillis) + .writeByte('\n'); + sink.writeUtf8(OkHeaders.RECEIVED_MILLIS) + .writeUtf8(": ") + .writeDecimalLong(receivedResponseMillis) + .writeByte('\n'); if (isHttps()) { sink.writeByte('\n'); - sink.writeUtf8(handshake.cipherSuite().javaName()); - sink.writeByte('\n'); + sink.writeUtf8(handshake.cipherSuite().javaName()) + .writeByte('\n'); writeCertList(sink, handshake.peerCertificates()); writeCertList(sink, handshake.localCertificates()); // The handshake’s TLS version is null on HttpsURLConnection and on older cached responses. if (handshake.tlsVersion() != null) { - sink.writeUtf8(handshake.tlsVersion().javaName()); - sink.writeByte('\n'); + sink.writeUtf8(handshake.tlsVersion().javaName()) + .writeByte('\n'); } } sink.close(); @@ -650,13 +672,13 @@ public final class Cache implements Closeable, Flushable { private void writeCertList(BufferedSink sink, List certificates) throws IOException { try { - sink.writeDecimalLong(certificates.size()); - sink.writeByte('\n'); + sink.writeDecimalLong(certificates.size()) + .writeByte('\n'); for (int i = 0, size = certificates.size(); i < size; i++) { byte[] bytes = certificates.get(i).getEncoded(); String line = ByteString.of(bytes).base64(); - sink.writeUtf8(line); - sink.writeByte('\n'); + sink.writeUtf8(line) + .writeByte('\n'); } } catch (CertificateEncodingException e) { throw new IOException(e.getMessage()); @@ -685,6 +707,8 @@ public final class Cache implements Closeable, Flushable { .headers(responseHeaders) .body(new CacheResponseBody(snapshot, contentType, contentLength)) .handshake(handshake) + .sentRequestAtMillis(sentRequestMillis) + .receivedResponseAtMillis(receivedResponseMillis) .build(); } } diff --git a/okhttp/src/main/java/okhttp3/Headers.java b/okhttp/src/main/java/okhttp3/Headers.java index 840a272d3..5080883af 100644 --- a/okhttp/src/main/java/okhttp3/Headers.java +++ b/okhttp/src/main/java/okhttp3/Headers.java @@ -18,10 +18,12 @@ package okhttp3; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.Date; import java.util.LinkedHashMap; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Set; import java.util.TreeSet; @@ -113,6 +115,41 @@ public final class Headers { return result; } + /** + * Returns true if {@code other} is a {@code Headers} object with the same headers, with the same + * casing, in the same order. Note that two headers instances may be semantically equal + * but not equal according to this method. In particular, none of the following sets of headers + * are equal according to this method: 
   {@code
+   *
+   *   1. Original
+   *   Content-Type: text/html
+   *   Content-Length: 50
+   *
+   *   2. Different order
+   *   Content-Length: 50
+   *   Content-Type: text/html
+   *
+   *   3. Different case
+   *   content-type: text/html
+   *   content-length: 50
+   *
+   *   4. Different values
+   *   Content-Type: text/html
+   *   Content-Length: 050
+   * }
+ * + * Applications that require semantically equal headers should convert them into a canonical form + * before comparing them for equality. + */ + @Override public boolean equals(Object other) { + return other instanceof Headers + && Arrays.equals(((Headers) other).namesAndValues, namesAndValues); + } + + @Override public int hashCode() { + return Arrays.hashCode(namesAndValues); + } + @Override public String toString() { StringBuilder result = new StringBuilder(); for (int i = 0, size = size(); i < size; i++) { @@ -122,9 +159,9 @@ public final class Headers { } public Map> toMultimap() { - Map> result = new LinkedHashMap>(); + Map> result = new LinkedHashMap<>(); for (int i = 0, size = size(); i < size; i++) { - String name = name(i); + String name = name(i).toLowerCase(Locale.US); List values = result.get(name); if (values == null) { values = new ArrayList<>(2); diff --git a/okhttp/src/main/java/okhttp3/Response.java b/okhttp/src/main/java/okhttp3/Response.java index 7a3d80264..eb0cf4a7f 100644 --- a/okhttp/src/main/java/okhttp3/Response.java +++ b/okhttp/src/main/java/okhttp3/Response.java @@ -46,6 +46,8 @@ public final class Response { private Response networkResponse; private Response cacheResponse; private final Response priorResponse; + private final long sentRequestAtMillis; + private final long receivedResponseAtMillis; private volatile CacheControl cacheControl; // Lazily initialized. @@ -60,6 +62,8 @@ public final class Response { this.networkResponse = builder.networkResponse; this.cacheResponse = builder.cacheResponse; this.priorResponse = builder.priorResponse; + this.sentRequestAtMillis = builder.sentRequestAtMillis; + this.receivedResponseAtMillis = builder.receivedResponseAtMillis; } /** @@ -238,6 +242,24 @@ public final class Response { return result != null ? result : (cacheControl = CacheControl.parse(headers)); } + /** + * Returns a {@linkplain System#currentTimeMillis() timestamp} taken immediately before OkHttp + * transmitted the initiating request over the network. If this response is being served from the + * cache then this is the timestamp of the original request. + */ + public long sentRequestAtMillis() { + return sentRequestAtMillis; + } + + /** + * Returns a {@linkplain System#currentTimeMillis() timestamp} taken immediately after OkHttp + * received this response's headers from the network. If this response is being served from the + * cache then this is the timestamp of the original response. + */ + public long receivedResponseAtMillis() { + return receivedResponseAtMillis; + } + @Override public String toString() { return "Response{protocol=" + protocol @@ -261,6 +283,8 @@ public final class Response { private Response networkResponse; private Response cacheResponse; private Response priorResponse; + private long sentRequestAtMillis; + private long receivedResponseAtMillis; public Builder() { headers = new Headers.Builder(); @@ -277,6 +301,8 @@ public final class Response { this.networkResponse = response.networkResponse; this.cacheResponse = response.cacheResponse; this.priorResponse = response.priorResponse; + this.sentRequestAtMillis = response.sentRequestAtMillis; + this.receivedResponseAtMillis = response.receivedResponseAtMillis; } public Builder request(Request request) { @@ -374,6 +400,16 @@ public final class Response { } } + public Builder sentRequestAtMillis(long sentRequestAtMillis) { + this.sentRequestAtMillis = sentRequestAtMillis; + return this; + } + + public Builder receivedResponseAtMillis(long receivedResponseAtMillis) { + this.receivedResponseAtMillis = receivedResponseAtMillis; + return this; + } + public Response build() { if (request == null) throw new IllegalStateException("request == null"); if (protocol == null) throw new IllegalStateException("protocol == null"); diff --git a/okhttp/src/main/java/okhttp3/internal/http/CacheStrategy.java b/okhttp/src/main/java/okhttp3/internal/http/CacheStrategy.java index 940bd68b7..7aa8af1a4 100644 --- a/okhttp/src/main/java/okhttp3/internal/http/CacheStrategy.java +++ b/okhttp/src/main/java/okhttp3/internal/http/CacheStrategy.java @@ -138,6 +138,8 @@ public final class CacheStrategy { this.cacheResponse = cacheResponse; if (cacheResponse != null) { + this.sentRequestMillis = cacheResponse.sentRequestAtMillis(); + this.receivedResponseMillis = cacheResponse.receivedResponseAtMillis(); Headers headers = cacheResponse.headers(); for (int i = 0, size = headers.size(); i < size; i++) { String fieldName = headers.name(i); @@ -154,10 +156,6 @@ public final class CacheStrategy { etag = value; } else if ("Age".equalsIgnoreCase(fieldName)) { ageSeconds = HeaderParser.parseSeconds(value, -1); - } else if (OkHeaders.SENT_MILLIS.equalsIgnoreCase(fieldName)) { - sentRequestMillis = Long.parseLong(value); - } else if (OkHeaders.RECEIVED_MILLIS.equalsIgnoreCase(fieldName)) { - receivedResponseMillis = Long.parseLong(value); } } } diff --git a/okhttp/src/main/java/okhttp3/internal/http/HttpEngine.java b/okhttp/src/main/java/okhttp3/internal/http/HttpEngine.java index 9643cbc59..17ee87a4a 100644 --- a/okhttp/src/main/java/okhttp3/internal/http/HttpEngine.java +++ b/okhttp/src/main/java/okhttp3/internal/http/HttpEngine.java @@ -220,6 +220,8 @@ public final class HttpEngine { .code(504) .message("Unsatisfiable Request (only-if-cached)") .body(EMPTY_BODY) + .sentRequestAtMillis(sentRequestMillis) + .receivedResponseAtMillis(System.currentTimeMillis()) .build(); return; } @@ -386,7 +388,7 @@ public final class HttpEngine { } // Offer this request to the cache. - storeRequest = responseCache.put(stripBody(userResponse)); + storeRequest = responseCache.put(userResponse); } /** @@ -615,7 +617,7 @@ public final class HttpEngine { // Content-Encoding header (as performed by initContentStream()). InternalCache responseCache = Internal.instance.internalCache(client); responseCache.trackConditionalCacheHit(); - responseCache.update(cacheResponse, stripBody(userResponse)); + responseCache.update(cacheResponse, userResponse); userResponse = unzip(userResponse); return; } else { @@ -726,8 +728,8 @@ public final class HttpEngine { Response networkResponse = httpStream.readResponseHeaders() .request(networkRequest) .handshake(streamAllocation.connection().handshake()) - .header(OkHeaders.SENT_MILLIS, Long.toString(sentRequestMillis)) - .header(OkHeaders.RECEIVED_MILLIS, Long.toString(System.currentTimeMillis())) + .sentRequestAtMillis(sentRequestMillis) + .receivedResponseAtMillis(System.currentTimeMillis()) .build(); if (!forWebSocket) {