From 20f4290d01e55536e93653f1f18cdf347a9aa45c Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Mon, 2 Oct 2017 11:13:42 +0200 Subject: [PATCH] Removed dependency on classic (blocking) I/O APIs from HttpCache --- .../http/impl/cache/BasicHttpCache.java | 91 +----- .../client5/http/impl/cache/CachingExec.java | 151 +++++++--- .../impl/cache/CachingHttpClientBuilder.java | 2 +- .../hc/client5/http/impl/cache/HttpCache.java | 15 +- .../http/impl/cache/HttpTestUtils.java | 8 + .../http/impl/cache/TestBasicHttpCache.java | 263 ++---------------- .../http/impl/cache/TestCachingExec.java | 9 +- .../http/impl/cache/TestCachingExecChain.java | 176 +++++++++--- .../cache/TestHttpCacheJiraNumber1147.java | 2 +- .../impl/cache/TestProtocolRequirements.java | 8 +- .../async/methods/SimpleHttpResponse.java | 32 +++ 11 files changed, 338 insertions(+), 419 deletions(-) diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/BasicHttpCache.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/BasicHttpCache.java index d692fa45b..d58b30ef0 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/BasicHttpCache.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/BasicHttpCache.java @@ -27,7 +27,6 @@ package org.apache.hc.client5.http.impl.cache; import java.io.IOException; -import java.io.InputStream; import java.util.Arrays; import java.util.Date; import java.util.HashMap; @@ -43,16 +42,10 @@ import org.apache.hc.client5.http.cache.HttpCacheUpdateCallback; import org.apache.hc.client5.http.cache.HttpCacheUpdateException; import org.apache.hc.client5.http.cache.Resource; import org.apache.hc.client5.http.cache.ResourceFactory; -import org.apache.hc.core5.http.ClassicHttpResponse; import org.apache.hc.core5.http.Header; -import org.apache.hc.core5.http.HttpEntity; -import org.apache.hc.core5.http.HttpHeaders; import org.apache.hc.core5.http.HttpHost; import org.apache.hc.core5.http.HttpRequest; import org.apache.hc.core5.http.HttpResponse; -import org.apache.hc.core5.http.HttpStatus; -import org.apache.hc.core5.http.io.entity.ByteArrayEntity; -import org.apache.hc.core5.http.message.BasicClassicHttpResponse; import org.apache.hc.core5.util.ByteArrayBuffer; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -65,9 +58,7 @@ class BasicHttpCache implements HttpCache { private final CacheKeyGenerator uriExtractor; private final ResourceFactory resourceFactory; - private final long maxObjectSizeBytes; private final CacheEntryUpdater cacheEntryUpdater; - private final CachedHttpResponseGenerator responseGenerator; private final HttpCacheInvalidator cacheInvalidator; private final HttpCacheStorage storage; @@ -76,14 +67,11 @@ class BasicHttpCache implements HttpCache { public BasicHttpCache( final ResourceFactory resourceFactory, final HttpCacheStorage storage, - final CacheConfig config, final CacheKeyGenerator uriExtractor, final HttpCacheInvalidator cacheInvalidator) { this.resourceFactory = resourceFactory; this.uriExtractor = uriExtractor; this.cacheEntryUpdater = new CacheEntryUpdater(resourceFactory); - this.maxObjectSizeBytes = config.getMaxObjectSize(); - this.responseGenerator = new CachedHttpResponseGenerator(); this.storage = storage; this.cacheInvalidator = cacheInvalidator; } @@ -91,21 +79,16 @@ class BasicHttpCache implements HttpCache { public BasicHttpCache( final ResourceFactory resourceFactory, final HttpCacheStorage storage, - final CacheConfig config, final CacheKeyGenerator uriExtractor) { - this( resourceFactory, storage, config, uriExtractor, - new CacheInvalidator(uriExtractor, storage)); + this(resourceFactory, storage, uriExtractor, new CacheInvalidator(uriExtractor, storage)); } - public BasicHttpCache( - final ResourceFactory resourceFactory, - final HttpCacheStorage storage, - final CacheConfig config) { - this( resourceFactory, storage, config, new CacheKeyGenerator()); + public BasicHttpCache(final ResourceFactory resourceFactory, final HttpCacheStorage storage) { + this( resourceFactory, storage, new CacheKeyGenerator()); } public BasicHttpCache(final CacheConfig config) { - this(new HeapResourceFactory(), new BasicHttpCacheStorage(config), config); + this(new HeapResourceFactory(), new BasicHttpCacheStorage(config)); } public BasicHttpCache() { @@ -194,42 +177,6 @@ class BasicHttpCache implements HttpCache { } } - boolean isIncompleteResponse(final HttpResponse resp, final Resource resource) { - final int status = resp.getCode(); - if (status != HttpStatus.SC_OK - && status != HttpStatus.SC_PARTIAL_CONTENT) { - return false; - } - final Header hdr = resp.getFirstHeader(HttpHeaders.CONTENT_LENGTH); - if (hdr == null) { - return false; - } - final int contentLength; - try { - contentLength = Integer.parseInt(hdr.getValue()); - } catch (final NumberFormatException nfe) { - return false; - } - if (resource == null) { - return false; - } - return (resource.length() < contentLength); - } - - ClassicHttpResponse generateIncompleteResponseError( - final HttpResponse response, final Resource resource) { - final Integer contentLength = Integer.valueOf(response.getFirstHeader(HttpHeaders.CONTENT_LENGTH).getValue()); - final ClassicHttpResponse error = new BasicClassicHttpResponse(HttpStatus.SC_BAD_GATEWAY, "Bad Gateway"); - error.setHeader("Content-Type","text/plain;charset=UTF-8"); - final String msg = String.format("Received incomplete response " + - "with Content-Length %d but actual body length %d", - contentLength, resource.length()); - final byte[] msgBytes = msg.getBytes(); - error.setHeader("Content-Length", Integer.toString(msgBytes.length)); - error.setEntity(new ByteArrayEntity(msgBytes)); - return error; - } - HttpCacheEntry doGetUpdatedParentEntry( final String requestId, final HttpCacheEntry existing, @@ -284,37 +231,19 @@ class BasicHttpCache implements HttpCache { return updatedEntry; } - @Override - public ClassicHttpResponse cacheAndReturnResponse( + public HttpCacheEntry createCacheEntry( final HttpHost host, final HttpRequest request, - final ClassicHttpResponse originResponse, + final HttpResponse originResponse, + final ByteArrayBuffer content, final Date requestSent, final Date responseReceived) throws IOException { final Resource resource; - final HttpEntity entity = originResponse.getEntity(); - if (entity != null) { - final ByteArrayBuffer buf = new ByteArrayBuffer(1024); - final InputStream instream = entity.getContent(); - final byte[] tmp = new byte[2048]; - long total = 0; - int l; - while ((l = instream.read(tmp)) != -1) { - buf.append(tmp, 0, l); - total += l; - if (total > maxObjectSizeBytes) { - originResponse.setEntity(new CombinedEntity(entity, buf)); - return originResponse; - } - } - resource = resourceFactory.generate(request.getRequestUri(), buf.array(), 0, buf.length()); + if (content != null) { + resource = resourceFactory.generate(request.getRequestUri(), content.array(), 0, content.length()); } else { resource = null; } - originResponse.close(); - if (isIncompleteResponse(originResponse, resource)) { - return generateIncompleteResponseError(originResponse, resource); - } final HttpCacheEntry entry = new HttpCacheEntry( requestSent, responseReceived, @@ -322,7 +251,7 @@ class BasicHttpCache implements HttpCache { originResponse.getAllHeaders(), resource); storeInCache(host, request, entry); - return responseGenerator.generateResponse(request, entry); + return entry; } @Override diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingExec.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingExec.java index d1670b114..51c13d8d0 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingExec.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingExec.java @@ -27,6 +27,7 @@ package org.apache.hc.client5.http.impl.cache; import java.io.IOException; +import java.io.InputStream; import java.util.Date; import java.util.HashMap; import java.util.Iterator; @@ -49,8 +50,10 @@ import org.apache.hc.core5.annotation.Contract; import org.apache.hc.core5.annotation.ThreadingBehavior; import org.apache.hc.core5.http.ClassicHttpRequest; import org.apache.hc.core5.http.ClassicHttpResponse; +import org.apache.hc.core5.http.ContentType; import org.apache.hc.core5.http.Header; import org.apache.hc.core5.http.HeaderElement; +import org.apache.hc.core5.http.HttpEntity; import org.apache.hc.core5.http.HttpException; import org.apache.hc.core5.http.HttpHeaders; import org.apache.hc.core5.http.HttpHost; @@ -60,12 +63,14 @@ import org.apache.hc.core5.http.HttpResponse; import org.apache.hc.core5.http.HttpStatus; import org.apache.hc.core5.http.HttpVersion; import org.apache.hc.core5.http.ProtocolVersion; +import org.apache.hc.core5.http.io.entity.StringEntity; import org.apache.hc.core5.http.message.BasicClassicHttpResponse; import org.apache.hc.core5.http.message.MessageSupport; import org.apache.hc.core5.http.protocol.HttpContext; import org.apache.hc.core5.http.protocol.HttpCoreContext; import org.apache.hc.core5.net.URIAuthority; import org.apache.hc.core5.util.Args; +import org.apache.hc.core5.util.ByteArrayBuffer; import org.apache.hc.core5.util.VersionInfo; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -155,7 +160,7 @@ public class CachingExec implements ExecChainHandler { final ResourceFactory resourceFactory, final HttpCacheStorage storage, final CacheConfig config) { - this(new BasicHttpCache(resourceFactory, storage, config), config); + this(new BasicHttpCache(resourceFactory, storage), config); } public CachingExec() { @@ -423,7 +428,7 @@ public class CachingExec implements ExecChainHandler { final ClassicHttpRequest request, final HttpContext context, final HttpCacheEntry entry, - final Date now) { + final Date now) throws IOException { if (staleResponseNotAllowed(request, entry, now)) { return generateGatewayTimeout(context); } else { @@ -723,48 +728,52 @@ public class CachingExec implements ExecChainHandler { Date requestDate = getCurrentDate(); ClassicHttpResponse backendResponse = chain.proceed(conditionalRequest, scope); - Date responseDate = getCurrentDate(); + try { + Date responseDate = getCurrentDate(); - if (revalidationResponseIsTooOld(backendResponse, cacheEntry)) { - backendResponse.close(); - final ClassicHttpRequest unconditional = conditionalRequestBuilder.buildUnconditionalRequest( - scope.originalRequest); - requestDate = getCurrentDate(); - backendResponse = chain.proceed(unconditional, scope); - responseDate = getCurrentDate(); - } - - backendResponse.addHeader(HeaderConstants.VIA, generateViaHeader(backendResponse)); - - final int statusCode = backendResponse.getCode(); - if (statusCode == HttpStatus.SC_NOT_MODIFIED || statusCode == HttpStatus.SC_OK) { - recordCacheUpdate(scope.clientContext); - } - - if (statusCode == HttpStatus.SC_NOT_MODIFIED) { - final HttpCacheEntry updatedEntry = responseCache.updateCacheEntry( - target, request, cacheEntry, - backendResponse, requestDate, responseDate); - if (suitabilityChecker.isConditional(request) - && suitabilityChecker.allConditionalsMatch(request, updatedEntry, new Date())) { - return responseGenerator - .generateNotModifiedResponse(updatedEntry); - } - return responseGenerator.generateResponse(request, updatedEntry); - } - - if (staleIfErrorAppliesTo(statusCode) - && !staleResponseNotAllowed(request, cacheEntry, getCurrentDate()) - && validityPolicy.mayReturnStaleIfError(request, cacheEntry, responseDate)) { - try { - final ClassicHttpResponse cachedResponse = responseGenerator.generateResponse(request, cacheEntry); - cachedResponse.addHeader(HeaderConstants.WARNING, "110 localhost \"Response is stale\""); - return cachedResponse; - } finally { + if (revalidationResponseIsTooOld(backendResponse, cacheEntry)) { backendResponse.close(); + final ClassicHttpRequest unconditional = conditionalRequestBuilder.buildUnconditionalRequest( + scope.originalRequest); + requestDate = getCurrentDate(); + backendResponse = chain.proceed(unconditional, scope); + responseDate = getCurrentDate(); } + + backendResponse.addHeader(HeaderConstants.VIA, generateViaHeader(backendResponse)); + + final int statusCode = backendResponse.getCode(); + if (statusCode == HttpStatus.SC_NOT_MODIFIED || statusCode == HttpStatus.SC_OK) { + recordCacheUpdate(scope.clientContext); + } + + if (statusCode == HttpStatus.SC_NOT_MODIFIED) { + final HttpCacheEntry updatedEntry = responseCache.updateCacheEntry( + target, request, cacheEntry, + backendResponse, requestDate, responseDate); + if (suitabilityChecker.isConditional(request) + && suitabilityChecker.allConditionalsMatch(request, updatedEntry, new Date())) { + return responseGenerator.generateNotModifiedResponse(updatedEntry); + } + return responseGenerator.generateResponse(request, updatedEntry); + } + + if (staleIfErrorAppliesTo(statusCode) + && !staleResponseNotAllowed(request, cacheEntry, getCurrentDate()) + && validityPolicy.mayReturnStaleIfError(request, cacheEntry, responseDate)) { + try { + final ClassicHttpResponse cachedResponse = responseGenerator.generateResponse(request, cacheEntry); + cachedResponse.addHeader(HeaderConstants.WARNING, "110 localhost \"Response is stale\""); + return cachedResponse; + } finally { + backendResponse.close(); + } + } + return handleBackendResponse(target, conditionalRequest, scope, requestDate, responseDate, backendResponse); + } catch (final IOException | RuntimeException ex) { + backendResponse.close(); + throw ex; } - return handleBackendResponse(target, conditionalRequest, scope, requestDate, responseDate, backendResponse); } private boolean staleIfErrorAppliesTo(final int statusCode) { @@ -774,6 +783,66 @@ public class CachingExec implements ExecChainHandler { || statusCode == HttpStatus.SC_GATEWAY_TIMEOUT; } + boolean isIncompleteResponse(final HttpResponse resp, final ByteArrayBuffer buffer) { + if (buffer == null) { + return false; + } + final int status = resp.getCode(); + if (status != HttpStatus.SC_OK && status != HttpStatus.SC_PARTIAL_CONTENT) { + return false; + } + final Header hdr = resp.getFirstHeader(HttpHeaders.CONTENT_LENGTH); + if (hdr == null) { + return false; + } + final int contentLength; + try { + contentLength = Integer.parseInt(hdr.getValue()); + } catch (final NumberFormatException nfe) { + return false; + } + return buffer.length() < contentLength; + } + + public ClassicHttpResponse cacheAndReturnResponse( + final HttpHost target, + final HttpRequest request, + final ClassicHttpResponse backendResponse, + final Date requestSent, + final Date responseReceived) throws IOException { final ByteArrayBuffer buf; + final HttpEntity entity = backendResponse.getEntity(); + if (entity != null) { + buf = new ByteArrayBuffer(1024); + final InputStream instream = entity.getContent(); + final byte[] tmp = new byte[2048]; + long total = 0; + int l; + while ((l = instream.read(tmp)) != -1) { + buf.append(tmp, 0, l); + total += l; + if (total > cacheConfig.getMaxObjectSize()) { + backendResponse.setEntity(new CombinedEntity(entity, buf)); + return backendResponse; + } + } + } else { + buf = null; + } + if (buf != null && isIncompleteResponse(backendResponse, buf)) { + final Header h = backendResponse.getFirstHeader(HttpHeaders.CONTENT_LENGTH); + final ClassicHttpResponse error = new BasicClassicHttpResponse(HttpStatus.SC_BAD_GATEWAY, "Bad Gateway"); + final String msg = String.format("Received incomplete response " + + "with Content-Length %s but actual body length %d", + h != null ? h.getValue() : null, buf.length()); + error.setEntity(new StringEntity(msg, ContentType.TEXT_PLAIN)); + backendResponse.close(); + return error; + } + backendResponse.close(); + final HttpCacheEntry entry = responseCache.createCacheEntry(target, request, backendResponse, buf, requestSent, responseReceived); + return responseGenerator.generateResponse(request, entry); + } + ClassicHttpResponse handleBackendResponse( final HttpHost target, final ClassicHttpRequest request, @@ -789,7 +858,7 @@ public class CachingExec implements ExecChainHandler { responseCache.flushInvalidatedCacheEntriesFor(target, request, backendResponse); if (cacheable && !alreadyHaveNewerCacheEntry(target, request, backendResponse)) { storeRequestIfModifiedSinceFor304Response(request, backendResponse); - return responseCache.cacheAndReturnResponse(target, request, backendResponse, requestDate, responseDate); + return cacheAndReturnResponse(target, request, backendResponse, requestDate, responseDate); } if (!cacheable) { try { diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingHttpClientBuilder.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingHttpClientBuilder.java index cf0252e21..8f5babf4c 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingHttpClientBuilder.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingHttpClientBuilder.java @@ -147,7 +147,7 @@ public class CachingHttpClientBuilder extends HttpClientBuilder { final CacheKeyGenerator uriExtractor = new CacheKeyGenerator(); final HttpCache httpCache = new BasicHttpCache( resourceFactoryCopy, - storageCopy, config, + storageCopy, uriExtractor, this.httpCacheInvalidator != null ? this.httpCacheInvalidator : new CacheInvalidator(uriExtractor, storageCopy)); diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/HttpCache.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/HttpCache.java index d2cf58e63..3f03604ef 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/HttpCache.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/HttpCache.java @@ -31,10 +31,10 @@ import java.util.Date; import java.util.Map; import org.apache.hc.client5.http.cache.HttpCacheEntry; -import org.apache.hc.core5.http.ClassicHttpResponse; import org.apache.hc.core5.http.HttpHost; import org.apache.hc.core5.http.HttpRequest; import org.apache.hc.core5.http.HttpResponse; +import org.apache.hc.core5.util.ByteArrayBuffer; /** * @since 4.1 @@ -94,14 +94,19 @@ interface HttpCache { * @param host * @param request * @param originResponse + * @param content * @param requestSent * @param responseReceived - * @return the {@link HttpResponse} + * @return new {@link HttpCacheEntry} * @throws IOException */ - ClassicHttpResponse cacheAndReturnResponse(HttpHost host, - HttpRequest request, ClassicHttpResponse originResponse, - Date requestSent, Date responseReceived) + HttpCacheEntry createCacheEntry( + HttpHost host, + HttpRequest request, + HttpResponse originResponse, + ByteArrayBuffer content, + Date requestSent, + Date responseReceived) throws IOException; /** diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/HttpTestUtils.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/HttpTestUtils.java index fe2505033..a6fce24d1 100644 --- a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/HttpTestUtils.java +++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/HttpTestUtils.java @@ -52,6 +52,7 @@ import org.apache.hc.core5.http.message.BasicClassicHttpRequest; import org.apache.hc.core5.http.message.BasicClassicHttpResponse; import org.apache.hc.core5.http.message.BasicHeader; import org.apache.hc.core5.http.message.MessageSupport; +import org.apache.hc.core5.util.ByteArrayBuffer; import org.apache.hc.core5.util.LangUtils; import org.junit.Assert; @@ -243,6 +244,13 @@ public class HttpTestUtils { return bytes; } + public static ByteArrayBuffer getRandomBuffer(final int nbytes) { + final ByteArrayBuffer buf = new ByteArrayBuffer(nbytes); + buf.setLength(nbytes); + new Random().nextBytes(buf.array()); + return buf; + } + /** Generates a response body with random content. * @param nbytes length of the desired response body * @return an {@link HttpEntity} diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestBasicHttpCache.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestBasicHttpCache.java index 6b4d47346..e5b9b5798 100644 --- a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestBasicHttpCache.java +++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestBasicHttpCache.java @@ -31,18 +31,13 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; -import static org.junit.Assert.assertTrue; -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; -import java.io.InputStream; import java.util.Date; import java.util.HashMap; import java.util.Map; import org.apache.hc.client5.http.cache.HeaderConstants; import org.apache.hc.client5.http.cache.HttpCacheEntry; -import org.apache.hc.client5.http.cache.Resource; import org.apache.hc.client5.http.classic.methods.HttpDelete; import org.apache.hc.client5.http.classic.methods.HttpGet; import org.apache.hc.client5.http.classic.methods.HttpHead; @@ -50,16 +45,14 @@ import org.apache.hc.client5.http.classic.methods.HttpOptions; import org.apache.hc.client5.http.classic.methods.HttpPost; import org.apache.hc.client5.http.classic.methods.HttpTrace; import org.apache.hc.client5.http.utils.DateUtils; -import org.apache.hc.core5.http.ClassicHttpResponse; import org.apache.hc.core5.http.Header; -import org.apache.hc.core5.http.HttpEntity; import org.apache.hc.core5.http.HttpHost; import org.apache.hc.core5.http.HttpRequest; +import org.apache.hc.core5.http.HttpResponse; import org.apache.hc.core5.http.HttpStatus; -import org.apache.hc.core5.http.io.entity.BasicHttpEntity; -import org.apache.hc.core5.http.io.entity.ByteArrayEntity; -import org.apache.hc.core5.http.message.BasicClassicHttpResponse; import org.apache.hc.core5.http.message.BasicHeader; +import org.apache.hc.core5.http.message.BasicHttpResponse; +import org.apache.hc.core5.util.ByteArrayBuffer; import org.junit.Assert; import org.junit.Before; import org.junit.Test; @@ -72,7 +65,7 @@ public class TestBasicHttpCache { @Before public void setUp() throws Exception { backing = new SimpleHttpCacheStorage(); - impl = new BasicHttpCache(new HeapResourceFactory(), backing, CacheConfig.DEFAULT); + impl = new BasicHttpCache(new HeapResourceFactory(), backing); } @Test @@ -136,7 +129,7 @@ public class TestBasicHttpCache { throws Exception { final HttpHost host = new HttpHost("foo.example.com"); final HttpRequest req = new HttpPost("/foo"); - final ClassicHttpResponse resp = HttpTestUtils.make200Response(); + final HttpResponse resp = HttpTestUtils.make200Response(); resp.setHeader("Content-Location", "/bar"); resp.setHeader(HeaderConstants.ETAG, "\"etag\""); final String key = (new CacheKeyGenerator()).generateKey(host, new HttpGet("/bar")); @@ -158,7 +151,7 @@ public class TestBasicHttpCache { throws Exception { final HttpHost host = new HttpHost("foo.example.com"); final HttpRequest req = new HttpGet("/foo"); - final ClassicHttpResponse resp = HttpTestUtils.make200Response(); + final HttpResponse resp = HttpTestUtils.make200Response(); resp.setHeader("Content-Location", "/bar"); final String key = (new CacheKeyGenerator()).generateKey(host, new HttpGet("/bar")); @@ -188,127 +181,6 @@ public class TestBasicHttpCache { assertNull(backing.map.get(key)); } - @Test - public void testRecognizesComplete200Response() - throws Exception { - final ClassicHttpResponse resp = new BasicClassicHttpResponse(HttpStatus.SC_OK, "OK"); - final byte[] bytes = HttpTestUtils.getRandomBytes(128); - resp.setEntity(new ByteArrayEntity(bytes)); - resp.setHeader("Content-Length","128"); - final Resource resource = new HeapResource(bytes); - - assertFalse(impl.isIncompleteResponse(resp, resource)); - } - - @Test - public void testRecognizesComplete206Response() - throws Exception { - final ClassicHttpResponse resp = new BasicClassicHttpResponse(HttpStatus.SC_PARTIAL_CONTENT, "Partial Content"); - final byte[] bytes = HttpTestUtils.getRandomBytes(128); - final Resource resource = new HeapResource(bytes); - resp.setEntity(new ByteArrayEntity(bytes)); - resp.setHeader("Content-Length","128"); - resp.setHeader("Content-Range","bytes 0-127/255"); - - assertFalse(impl.isIncompleteResponse(resp, resource)); - } - - @Test - public void testRecognizesIncomplete200Response() - throws Exception { - final ClassicHttpResponse resp = new BasicClassicHttpResponse(HttpStatus.SC_OK, "OK"); - final byte[] bytes = HttpTestUtils.getRandomBytes(128); - final Resource resource = new HeapResource(bytes); - resp.setEntity(new ByteArrayEntity(bytes)); - resp.setHeader("Content-Length","256"); - - assertTrue(impl.isIncompleteResponse(resp, resource)); - } - - @Test - public void testIgnoresIncompleteNon200Or206Responses() - throws Exception { - final ClassicHttpResponse resp = new BasicClassicHttpResponse(HttpStatus.SC_FORBIDDEN, "Forbidden"); - final byte[] bytes = HttpTestUtils.getRandomBytes(128); - final Resource resource = new HeapResource(bytes); - resp.setEntity(new ByteArrayEntity(bytes)); - resp.setHeader("Content-Length","256"); - - assertFalse(impl.isIncompleteResponse(resp, resource)); - } - - @Test - public void testResponsesWithoutExplicitContentLengthAreComplete() - throws Exception { - final ClassicHttpResponse resp = new BasicClassicHttpResponse(HttpStatus.SC_OK, "OK"); - final byte[] bytes = HttpTestUtils.getRandomBytes(128); - final Resource resource = new HeapResource(bytes); - resp.setEntity(new ByteArrayEntity(bytes)); - - assertFalse(impl.isIncompleteResponse(resp, resource)); - } - - @Test - public void testResponsesWithUnparseableContentLengthHeaderAreComplete() - throws Exception { - final ClassicHttpResponse resp = new BasicClassicHttpResponse(HttpStatus.SC_OK, "OK"); - final byte[] bytes = HttpTestUtils.getRandomBytes(128); - final Resource resource = new HeapResource(bytes); - resp.setHeader("Content-Length","foo"); - resp.setEntity(new ByteArrayEntity(bytes)); - - assertFalse(impl.isIncompleteResponse(resp, resource)); - } - - @Test - public void testNullResourcesAreComplete() - throws Exception { - final ClassicHttpResponse resp = new BasicClassicHttpResponse(HttpStatus.SC_OK, "OK"); - resp.setHeader("Content-Length","256"); - - assertFalse(impl.isIncompleteResponse(resp, null)); - } - - @Test - public void testIncompleteResponseErrorProvidesPlainTextErrorMessage() - throws Exception { - final ClassicHttpResponse resp = new BasicClassicHttpResponse(HttpStatus.SC_OK, "OK"); - final byte[] bytes = HttpTestUtils.getRandomBytes(128); - final Resource resource = new HeapResource(bytes); - resp.setEntity(new ByteArrayEntity(bytes)); - resp.setHeader("Content-Length","256"); - - final ClassicHttpResponse result = impl.generateIncompleteResponseError(resp, resource); - final Header ctype = result.getFirstHeader("Content-Type"); - assertEquals("text/plain;charset=UTF-8", ctype.getValue()); - } - - @Test - public void testIncompleteResponseErrorProvidesNonEmptyErrorMessage() - throws Exception { - final ClassicHttpResponse resp = new BasicClassicHttpResponse(HttpStatus.SC_OK, "OK"); - final byte[] bytes = HttpTestUtils.getRandomBytes(128); - final Resource resource = new HeapResource(bytes); - resp.setEntity(new ByteArrayEntity(bytes)); - resp.setHeader("Content-Length","256"); - - final ClassicHttpResponse result = impl.generateIncompleteResponseError(resp, resource); - final int clen = Integer.parseInt(result.getFirstHeader("Content-Length").getValue()); - assertTrue(clen > 0); - final HttpEntity body = result.getEntity(); - if (body.getContentLength() < 0) { - final InputStream is = body.getContent(); - int bytes_read = 0; - while((is.read()) != -1) { - bytes_read++; - } - is.close(); - assertEquals(clen, bytes_read); - } else { - assertTrue(body.getContentLength() == clen); - } - } - @Test public void testCacheUpdateAddsVariantURIToParentEntry() throws Exception { final String parentCacheKey = "parentCacheKey"; @@ -340,50 +212,6 @@ public class TestBasicHttpCache { assertSame(entry, backing.map.get(key)); } - @Test - public void testTooLargeResponsesAreNotCached() throws Exception { - final HttpHost host = new HttpHost("foo.example.com"); - final HttpRequest request = new HttpGet("http://foo.example.com/bar"); - - final Date now = new Date(); - final Date requestSent = new Date(now.getTime() - 3 * 1000L); - final Date responseGenerated = new Date(now.getTime() - 2 * 1000L); - final Date responseReceived = new Date(now.getTime() - 1 * 1000L); - - final ClassicHttpResponse originResponse = new BasicClassicHttpResponse(HttpStatus.SC_OK, "OK"); - originResponse.setEntity(HttpTestUtils.makeBody(CacheConfig.DEFAULT_MAX_OBJECT_SIZE_BYTES + 1)); - originResponse.setHeader("Cache-Control","public, max-age=3600"); - originResponse.setHeader("Date", DateUtils.formatDate(responseGenerated)); - originResponse.setHeader("ETag", "\"etag\""); - - final ClassicHttpResponse result = impl.cacheAndReturnResponse(host, request, originResponse, requestSent, responseReceived); - assertEquals(0, backing.map.size()); - assertSame(originResponse, result); - } - - - @Test - public void testSmallEnoughResponsesAreCached() throws Exception { - final HttpHost host = new HttpHost("foo.example.com"); - final HttpRequest request = new HttpGet("http://foo.example.com/bar"); - - final Date now = new Date(); - final Date requestSent = new Date(now.getTime() - 3 * 1000L); - final Date responseGenerated = new Date(now.getTime() - 2 * 1000L); - final Date responseReceived = new Date(now.getTime() - 1 * 1000L); - - final ClassicHttpResponse originResponse = new BasicClassicHttpResponse(HttpStatus.SC_OK, "OK"); - originResponse.setEntity(HttpTestUtils.makeBody(CacheConfig.DEFAULT_MAX_OBJECT_SIZE_BYTES - 1)); - originResponse.setHeader("Cache-Control","public, max-age=3600"); - originResponse.setHeader("Date", DateUtils.formatDate(responseGenerated)); - originResponse.setHeader("ETag", "\"etag\""); - - final ClassicHttpResponse result = impl.cacheAndReturnResponse(host, request, originResponse, requestSent, responseReceived); - assertEquals(1, backing.map.size()); - assertTrue(backing.map.containsKey((new CacheKeyGenerator()).generateKey(host, request))); - assertTrue(HttpTestUtils.semanticallyTransparent(originResponse, result)); - } - @Test public void testGetCacheEntryReturnsNullOnCacheMiss() throws Exception { final HttpHost host = new HttpHost("foo.example.com"); @@ -410,20 +238,21 @@ public class TestBasicHttpCache { @Test public void testGetCacheEntryReturnsNullIfNoVariantInCache() throws Exception { final HttpHost host = new HttpHost("foo.example.com"); - final HttpRequest request = new HttpGet("http://foo.example.com/bar"); final HttpRequest origRequest = new HttpGet("http://foo.example.com/bar"); origRequest.setHeader("Accept-Encoding","gzip"); - final ClassicHttpResponse origResponse = new BasicClassicHttpResponse(HttpStatus.SC_OK, "OK"); - origResponse.setEntity(HttpTestUtils.makeBody(128)); + final ByteArrayBuffer buf = HttpTestUtils.getRandomBuffer(128); + final HttpResponse origResponse = new BasicHttpResponse(HttpStatus.SC_OK, "OK"); origResponse.setHeader("Date", DateUtils.formatDate(new Date())); origResponse.setHeader("Cache-Control", "max-age=3600, public"); origResponse.setHeader("ETag", "\"etag\""); origResponse.setHeader("Vary", "Accept-Encoding"); origResponse.setHeader("Content-Encoding","gzip"); - impl.cacheAndReturnResponse(host, origRequest, origResponse, new Date(), new Date()); + impl.createCacheEntry(host, origRequest, origResponse, buf, new Date(), new Date()); + + final HttpRequest request = new HttpGet("http://foo.example.com/bar"); final HttpCacheEntry result = impl.getCacheEntry(host, request); assertNull(result); } @@ -431,21 +260,22 @@ public class TestBasicHttpCache { @Test public void testGetCacheEntryReturnsVariantIfPresentInCache() throws Exception { final HttpHost host = new HttpHost("foo.example.com"); - final HttpRequest request = new HttpGet("http://foo.example.com/bar"); - request.setHeader("Accept-Encoding","gzip"); final HttpRequest origRequest = new HttpGet("http://foo.example.com/bar"); origRequest.setHeader("Accept-Encoding","gzip"); - final ClassicHttpResponse origResponse = new BasicClassicHttpResponse(HttpStatus.SC_OK, "OK"); - origResponse.setEntity(HttpTestUtils.makeBody(128)); + final ByteArrayBuffer buf = HttpTestUtils.getRandomBuffer(128); + final HttpResponse origResponse = new BasicHttpResponse(HttpStatus.SC_OK, "OK"); origResponse.setHeader("Date", DateUtils.formatDate(new Date())); origResponse.setHeader("Cache-Control", "max-age=3600, public"); origResponse.setHeader("ETag", "\"etag\""); origResponse.setHeader("Vary", "Accept-Encoding"); origResponse.setHeader("Content-Encoding","gzip"); - impl.cacheAndReturnResponse(host, origRequest, origResponse, new Date(), new Date()); + impl.createCacheEntry(host, origRequest, origResponse, buf, new Date(), new Date()); + + final HttpRequest request = new HttpGet("http://foo.example.com/bar"); + request.setHeader("Accept-Encoding","gzip"); final HttpCacheEntry result = impl.getCacheEntry(host, request); assertNotNull(result); } @@ -467,7 +297,7 @@ public class TestBasicHttpCache { final HttpRequest req1 = new HttpGet("http://foo.example.com/bar"); req1.setHeader("Accept-Encoding", "gzip"); - final ClassicHttpResponse resp1 = HttpTestUtils.make200Response(); + final HttpResponse resp1 = HttpTestUtils.make200Response(); resp1.setHeader("Date", DateUtils.formatDate(new Date())); resp1.setHeader("Cache-Control", "max-age=3600, public"); resp1.setHeader("ETag", "\"etag1\""); @@ -478,7 +308,7 @@ public class TestBasicHttpCache { final HttpRequest req2 = new HttpGet("http://foo.example.com/bar"); req2.setHeader("Accept-Encoding", "identity"); - final ClassicHttpResponse resp2 = HttpTestUtils.make200Response(); + final HttpResponse resp2 = HttpTestUtils.make200Response(); resp2.setHeader("Date", DateUtils.formatDate(new Date())); resp2.setHeader("Cache-Control", "max-age=3600, public"); resp2.setHeader("ETag", "\"etag2\""); @@ -486,8 +316,8 @@ public class TestBasicHttpCache { resp2.setHeader("Content-Encoding","gzip"); resp2.setHeader("Vary", "Accept-Encoding"); - impl.cacheAndReturnResponse(host, req1, resp1, new Date(), new Date()); - impl.cacheAndReturnResponse(host, req2, resp2, new Date(), new Date()); + impl.createCacheEntry(host, req1, resp1, null, new Date(), new Date()); + impl.createCacheEntry(host, req2, resp2, null, new Date(), new Date()); final Map variants = impl.getVariantCacheEntriesWithEtags(host, req1); @@ -496,55 +326,4 @@ public class TestBasicHttpCache { } - @Test - public void testOriginalResponseWithNoContentSizeHeaderIsReleased() throws Exception { - final HttpHost host = new HttpHost("foo.example.com"); - final HttpRequest request = new HttpGet("http://foo.example.com/bar"); - - final Date now = new Date(); - final Date requestSent = new Date(now.getTime() - 3 * 1000L); - final Date responseGenerated = new Date(now.getTime() - 2 * 1000L); - final Date responseReceived = new Date(now.getTime() - 1 * 1000L); - - final ClassicHttpResponse originResponse = new BasicClassicHttpResponse(HttpStatus.SC_OK, "OK"); - final BasicHttpEntity entity = new BasicHttpEntity(); - final ConsumableInputStream inputStream = new ConsumableInputStream(new ByteArrayInputStream(HttpTestUtils.getRandomBytes(CacheConfig.DEFAULT_MAX_OBJECT_SIZE_BYTES - 1))); - entity.setContent(inputStream); - originResponse.setEntity(entity); - originResponse.setHeader("Cache-Control","public, max-age=3600"); - originResponse.setHeader("Date", DateUtils.formatDate(responseGenerated)); - originResponse.setHeader("ETag", "\"etag\""); - - final ClassicHttpResponse result = impl.cacheAndReturnResponse(host, request, originResponse, requestSent, responseReceived); - IOUtils.consume(result.getEntity()); - assertTrue(inputStream.wasClosed()); - } - - @Test - public void testEntryUpdate() throws Exception { - - final HeapResourceFactory rf = new HeapResourceFactory(); - - impl = new BasicHttpCache(rf, backing, CacheConfig.DEFAULT); - - final HttpHost host = new HttpHost("foo.example.com"); - - final HttpRequest origRequest = new HttpGet("http://foo.example.com/bar"); - origRequest.setHeader("Accept-Encoding","gzip"); - - final ClassicHttpResponse origResponse = new BasicClassicHttpResponse(HttpStatus.SC_OK, "OK"); - origResponse.setEntity(HttpTestUtils.makeBody(128)); - origResponse.setHeader("Date", DateUtils.formatDate(new Date())); - origResponse.setHeader("Cache-Control", "max-age=3600, public"); - origResponse.setHeader("ETag", "\"etag\""); - origResponse.setHeader("Vary", "Accept-Encoding"); - origResponse.setHeader("Content-Encoding","gzip"); - - final ClassicHttpResponse response = impl.cacheAndReturnResponse( - host, origRequest, origResponse, new Date(), new Date()); - final HttpEntity entity = response.getEntity(); - Assert.assertNotNull(entity); - IOUtils.copyAndClose(entity.getContent(), new ByteArrayOutputStream()); - } - } diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachingExec.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachingExec.java index 366dea09f..daeb4688e 100644 --- a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachingExec.java +++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachingExec.java @@ -290,7 +290,7 @@ public class TestCachingExec extends TestCachingExecChain { eq(responseDate))) .andReturn(updatedEntry); expect(mockSuitabilityChecker.isConditional(request)).andReturn(false); - responseIsGeneratedFromCache(); + responseIsGeneratedFromCache(HttpTestUtils.make200Response()); replayMocks(); impl.revalidateCacheEntry(host, request, scope, mockExecChain, entry); @@ -396,14 +396,11 @@ public class TestCachingExec extends TestCachingExecChain { cacheEntrySuitable(true); cacheEntryValidatable(true); - expect(mockResponseGenerator.generateResponse(isA(HttpRequest.class), isA(HttpCacheEntry.class))) - .andReturn(mockBackendResponse); + responseIsGeneratedFromCache(HttpTestUtils.make200Response()); replayMocks(); - final HttpResponse result = impl.execute(request, scope, mockExecChain); + impl.execute(request, scope, mockExecChain); verifyMocks(); - - Assert.assertSame(mockBackendResponse, result); } private IExpectationSetters implExpectsAnyRequestAndReturn( diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachingExecChain.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachingExecChain.java index bd9f0a83f..8a3dfce2e 100644 --- a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachingExecChain.java +++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachingExecChain.java @@ -31,10 +31,12 @@ import static org.easymock.EasyMock.eq; import static org.easymock.EasyMock.expect; import static org.easymock.EasyMock.expectLastCall; import static org.easymock.EasyMock.isA; +import static org.easymock.EasyMock.same; import static org.easymock.classextension.EasyMock.createNiceMock; import static org.easymock.classextension.EasyMock.replay; import static org.easymock.classextension.EasyMock.verify; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; @@ -54,7 +56,6 @@ import org.apache.hc.client5.http.cache.HttpCacheContext; import org.apache.hc.client5.http.cache.HttpCacheEntry; import org.apache.hc.client5.http.cache.HttpCacheStorage; import org.apache.hc.client5.http.classic.ExecChain; -import org.apache.hc.client5.http.classic.ExecChainHandler; import org.apache.hc.client5.http.classic.ExecRuntime; import org.apache.hc.client5.http.classic.methods.HttpGet; import org.apache.hc.client5.http.classic.methods.HttpOptions; @@ -77,6 +78,7 @@ import org.apache.hc.core5.http.message.BasicClassicHttpRequest; import org.apache.hc.core5.http.message.BasicClassicHttpResponse; import org.apache.hc.core5.http.message.BasicHeader; import org.apache.hc.core5.net.URIAuthority; +import org.apache.hc.core5.util.ByteArrayBuffer; import org.easymock.Capture; import org.easymock.EasyMock; import org.easymock.IExpectationSetters; @@ -89,7 +91,7 @@ import junit.framework.AssertionFailedError; @SuppressWarnings("boxing") // test code public abstract class TestCachingExecChain { - private ExecChainHandler impl; + private CachingExec impl; protected CacheValidityPolicy mockValidityPolicy; protected CacheableRequestPolicy mockRequestPolicy; @@ -103,7 +105,6 @@ public abstract class TestCachingExecChain { protected CachedHttpResponseGenerator mockResponseGenerator; private HttpClientResponseHandler mockHandler; private ClassicHttpRequest mockUriRequest; - private ClassicHttpResponse mockCachedResponse; protected ConditionalRequestBuilder mockConditionalRequestBuilder; private HttpRequest mockConditionalRequest; protected ResponseProtocolCompliance mockResponseProtocolCompliance; @@ -131,7 +132,6 @@ public abstract class TestCachingExecChain { mockUriRequest = createNiceMock(ClassicHttpRequest.class); mockCacheEntry = createNiceMock(HttpCacheEntry.class); mockResponseGenerator = createNiceMock(CachedHttpResponseGenerator.class); - mockCachedResponse = createNiceMock(ClassicHttpResponse.class); mockConditionalRequestBuilder = createNiceMock(ConditionalRequestBuilder.class); mockConditionalRequest = createNiceMock(HttpRequest.class); mockResponseProtocolCompliance = createNiceMock(ResponseProtocolCompliance.class); @@ -151,7 +151,7 @@ public abstract class TestCachingExecChain { mockRequestProtocolCompliance, config, asyncValidator); } - public abstract ExecChainHandler createCachingExecChain(HttpCache responseCache, CacheValidityPolicy validityPolicy, + public abstract CachingExec createCachingExecChain(HttpCache responseCache, CacheValidityPolicy validityPolicy, ResponseCachingPolicy responseCachingPolicy, CachedHttpResponseGenerator responseGenerator, CacheableRequestPolicy cacheableRequestPolicy, CachedResponseSuitabilityChecker suitabilityChecker, @@ -159,7 +159,7 @@ public abstract class TestCachingExecChain { ResponseProtocolCompliance responseCompliance, RequestProtocolCompliance requestCompliance, CacheConfig config, AsynchronousValidator asynchRevalidator); - public abstract ExecChainHandler createCachingExecChain(HttpCache cache, CacheConfig config); + public abstract CachingExec createCachingExecChain(HttpCache cache, CacheConfig config); protected ClassicHttpResponse execute(final ClassicHttpRequest request) throws IOException, HttpException { return impl.execute(ClassicRequestCopier.INSTANCE.copy(request), new ExecChain.Scope( @@ -187,7 +187,6 @@ public abstract class TestCachingExecChain { replay(mockCache); replay(mockHandler); replay(mockUriRequest); - replay(mockCachedResponse); replay(mockConditionalRequestBuilder); replay(mockConditionalRequest); replay(mockResponseProtocolCompliance); @@ -206,7 +205,6 @@ public abstract class TestCachingExecChain { verify(mockCache); verify(mockHandler); verify(mockUriRequest); - verify(mockCachedResponse); verify(mockConditionalRequestBuilder); verify(mockConditionalRequest); verify(mockResponseProtocolCompliance); @@ -314,22 +312,20 @@ public abstract class TestCachingExecChain { requestPolicyAllowsCaching(true); getCacheEntryReturns(mockCacheEntry); cacheEntrySuitable(true); - responseIsGeneratedFromCache(); + responseIsGeneratedFromCache(HttpTestUtils.make200Response()); requestIsFatallyNonCompliant(null); entryHasStaleness(0L); replayMocks(); final ClassicHttpResponse result = execute(request); verifyMocks(); - - Assert.assertSame(mockCachedResponse, result); } @Test public void testNonCacheableResponseIsNotCachedAndIsReturnedAsIs() throws Exception { final CacheConfig configDefault = CacheConfig.DEFAULT; impl = createCachingExecChain(new BasicHttpCache(new HeapResourceFactory(), - mockStorage, configDefault), configDefault); + mockStorage), configDefault); final ClassicHttpRequest req1 = HttpTestUtils.makeDefaultRequest(); final ClassicHttpResponse resp1 = HttpTestUtils.make200Response(); @@ -354,7 +350,7 @@ public abstract class TestCachingExecChain { requestPolicyAllowsCaching(true); cacheEntrySuitable(true); getCacheEntryReturns(mockCacheEntry); - responseIsGeneratedFromCache(); + responseIsGeneratedFromCache(HttpTestUtils.make200Response()); entryHasStaleness(0L); replayMocks(); @@ -1265,31 +1261,134 @@ public abstract class TestCachingExecChain { } @Test - public void testTreatsCacheIOExceptionsAsCacheMiss() throws Exception { + public void testRecognizesComplete200Response() + throws Exception { + final ClassicHttpResponse resp = new BasicClassicHttpResponse(HttpStatus.SC_OK, "OK"); + final ByteArrayBuffer buf = HttpTestUtils.getRandomBuffer(128); + resp.setHeader("Content-Length","128"); + assertFalse(impl.isIncompleteResponse(resp, buf)); + } - impl = createCachingExecChain(mockCache, CacheConfig.DEFAULT); - final ClassicHttpResponse resp = HttpTestUtils.make200Response(); + @Test + public void testRecognizesComplete206Response() + throws Exception { + final ClassicHttpResponse resp = new BasicClassicHttpResponse(HttpStatus.SC_PARTIAL_CONTENT, "Partial Content"); + final ByteArrayBuffer buf = HttpTestUtils.getRandomBuffer(128); + resp.setHeader("Content-Length","128"); + resp.setHeader("Content-Range","bytes 0-127/255"); + assertFalse(impl.isIncompleteResponse(resp, buf)); + } - mockCache.flushInvalidatedCacheEntriesFor(host, request); - expectLastCall().andThrow(new IOException()).anyTimes(); - mockCache.flushInvalidatedCacheEntriesFor(isA(HttpHost.class), isA(HttpRequest.class), - isA(HttpResponse.class)); - expectLastCall().anyTimes(); - expect(mockCache.getCacheEntry(eq(host), isA(HttpRequest.class))).andThrow( - new IOException()).anyTimes(); - expect(mockCache.getVariantCacheEntriesWithEtags(eq(host), isA(HttpRequest.class))) - .andThrow(new IOException()).anyTimes(); - expect( - mockCache.cacheAndReturnResponse(eq(host), isA(HttpRequest.class), - isA(ClassicHttpResponse.class), isA(Date.class), isA(Date.class))) - .andReturn(resp).anyTimes(); - expect( - mockExecChain.proceed(isA(ClassicHttpRequest.class), isA(ExecChain.Scope.class))).andReturn(resp); + @Test + public void testRecognizesIncomplete200Response() + throws Exception { + final ClassicHttpResponse resp = new BasicClassicHttpResponse(HttpStatus.SC_OK, "OK"); + final ByteArrayBuffer buf = HttpTestUtils.getRandomBuffer(128); + resp.setHeader("Content-Length","256"); + + assertTrue(impl.isIncompleteResponse(resp, buf)); + } + + @Test + public void testIgnoresIncompleteNon200Or206Responses() + throws Exception { + final ClassicHttpResponse resp = new BasicClassicHttpResponse(HttpStatus.SC_FORBIDDEN, "Forbidden"); + final ByteArrayBuffer buf = HttpTestUtils.getRandomBuffer(128); + resp.setHeader("Content-Length","256"); + + assertFalse(impl.isIncompleteResponse(resp, buf)); + } + + @Test + public void testResponsesWithoutExplicitContentLengthAreComplete() + throws Exception { + final ClassicHttpResponse resp = new BasicClassicHttpResponse(HttpStatus.SC_OK, "OK"); + final ByteArrayBuffer buf = HttpTestUtils.getRandomBuffer(128); + + assertFalse(impl.isIncompleteResponse(resp, buf)); + } + + @Test + public void testResponsesWithUnparseableContentLengthHeaderAreComplete() + throws Exception { + final ClassicHttpResponse resp = new BasicClassicHttpResponse(HttpStatus.SC_OK, "OK"); + final ByteArrayBuffer buf = HttpTestUtils.getRandomBuffer(128); + resp.setHeader("Content-Length","foo"); + assertFalse(impl.isIncompleteResponse(resp, buf)); + } + + @Test + public void testNullResourcesAreComplete() + throws Exception { + final ClassicHttpResponse resp = new BasicClassicHttpResponse(HttpStatus.SC_OK, "OK"); + resp.setHeader("Content-Length","256"); + + assertFalse(impl.isIncompleteResponse(resp, null)); + } + + @Test + public void testTooLargeResponsesAreNotCached() throws Exception { + mockCache = EasyMock.createStrictMock(HttpCache.class); + impl = createCachingExecChain(mockCache, mockValidityPolicy, + mockResponsePolicy, mockResponseGenerator, mockRequestPolicy, mockSuitabilityChecker, + mockConditionalRequestBuilder, mockResponseProtocolCompliance, + mockRequestProtocolCompliance, config, asyncValidator); + + final HttpHost host = new HttpHost("foo.example.com"); + final HttpRequest request = new HttpGet("http://foo.example.com/bar"); + + final Date now = new Date(); + final Date requestSent = new Date(now.getTime() - 3 * 1000L); + final Date responseGenerated = new Date(now.getTime() - 2 * 1000L); + final Date responseReceived = new Date(now.getTime() - 1 * 1000L); + + final ClassicHttpResponse originResponse = new BasicClassicHttpResponse(HttpStatus.SC_OK, "OK"); + originResponse.setEntity(HttpTestUtils.makeBody(CacheConfig.DEFAULT_MAX_OBJECT_SIZE_BYTES + 1)); + originResponse.setHeader("Cache-Control","public, max-age=3600"); + originResponse.setHeader("Date", DateUtils.formatDate(responseGenerated)); + originResponse.setHeader("ETag", "\"etag\""); replayMocks(); - final ClassicHttpResponse result = execute(request); + + impl.cacheAndReturnResponse(host, request, originResponse, requestSent, responseReceived); + + verifyMocks(); + } + + @Test + public void testSmallEnoughResponsesAreCached() throws Exception { + final HttpHost host = new HttpHost("foo.example.com"); + final HttpRequest request = new HttpGet("http://foo.example.com/bar"); + + final Date now = new Date(); + final Date requestSent = new Date(now.getTime() - 3 * 1000L); + final Date responseGenerated = new Date(now.getTime() - 2 * 1000L); + final Date responseReceived = new Date(now.getTime() - 1 * 1000L); + + final ClassicHttpResponse originResponse = new BasicClassicHttpResponse(HttpStatus.SC_OK, "OK"); + originResponse.setEntity(HttpTestUtils.makeBody(CacheConfig.DEFAULT_MAX_OBJECT_SIZE_BYTES - 1)); + originResponse.setHeader("Cache-Control","public, max-age=3600"); + originResponse.setHeader("Date", DateUtils.formatDate(responseGenerated)); + originResponse.setHeader("ETag", "\"etag\""); + + final HttpCacheEntry httpCacheEntry = HttpTestUtils.makeCacheEntry(); + final ClassicHttpResponse response = HttpTestUtils.make200Response(); + + EasyMock.expect(mockCache.createCacheEntry( + eq(host), + same(request), + same(originResponse), + isA(ByteArrayBuffer.class), + eq(requestSent), + eq(responseReceived))).andReturn(httpCacheEntry).once(); + EasyMock.expect(mockResponseGenerator.generateResponse( + same(request), + same(httpCacheEntry))).andReturn(response).once(); + replayMocks(); + + impl.cacheAndReturnResponse(host, request, originResponse, requestSent, responseReceived); + verifyMocks(); - Assert.assertSame(resp, result); } @Test @@ -1332,14 +1431,12 @@ public abstract class TestCachingExecChain { requestPolicyAllowsCaching(true); getCacheEntryReturns(entry); cacheEntrySuitable(true); - responseIsGeneratedFromCache(); + responseIsGeneratedFromCache(HttpTestUtils.make200Response()); entryHasStaleness(0); replayMocks(); final ClassicHttpResponse resp = execute(request); verifyMocks(); - - Assert.assertSame(mockCachedResponse, resp); } @Test @@ -1669,10 +1766,11 @@ public abstract class TestCachingExecChain { .andReturn(staleness); } - protected void responseIsGeneratedFromCache() { + protected void responseIsGeneratedFromCache(final ClassicHttpResponse cachedResponse) throws IOException { expect( - mockResponseGenerator.generateResponse((ClassicHttpRequest) anyObject(), (HttpCacheEntry) anyObject())) - .andReturn(mockCachedResponse); + mockResponseGenerator.generateResponse( + (ClassicHttpRequest) anyObject(), + (HttpCacheEntry) anyObject())).andReturn(cachedResponse); } protected void doesNotFlushCache() throws IOException { diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestHttpCacheJiraNumber1147.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestHttpCacheJiraNumber1147.java index 72aa987d3..8754afe16 100644 --- a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestHttpCacheJiraNumber1147.java +++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestHttpCacheJiraNumber1147.java @@ -113,7 +113,7 @@ public class TestHttpCacheJiraNumber1147 { isA(ClassicHttpRequest.class), isA(ExecChain.Scope.class))).thenReturn(response); - final BasicHttpCache cache = new BasicHttpCache(resourceFactory, httpCacheStorage, cacheConfig); + final BasicHttpCache cache = new BasicHttpCache(resourceFactory, httpCacheStorage); final ExecChainHandler t = createCachingExecChain(cache, cacheConfig); final ExecChain.Scope scope = new ExecChain.Scope("teset", route, get, mockEndpoint, context); diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestProtocolRequirements.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestProtocolRequirements.java index 5729a8163..6eb3436c7 100644 --- a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestProtocolRequirements.java +++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestProtocolRequirements.java @@ -56,6 +56,7 @@ import org.apache.hc.core5.http.message.BasicClassicHttpRequest; import org.apache.hc.core5.http.message.BasicClassicHttpResponse; import org.apache.hc.core5.http.message.BasicHeader; import org.apache.hc.core5.http.message.MessageSupport; +import org.apache.hc.core5.util.ByteArrayBuffer; import org.easymock.Capture; import org.easymock.EasyMock; import org.junit.Assert; @@ -2617,7 +2618,7 @@ public class TestProtocolRequirements extends AbstractProtocolTest { validated.setHeader("Content-Length", "128"); validated.setEntity(new ByteArrayEntity(bytes)); - final ClassicHttpResponse reconstructed = HttpTestUtils.make200Response(); + final HttpCacheEntry cacheEntry = HttpTestUtils.makeCacheEntry(); final Capture cap = new Capture<>(); @@ -2633,12 +2634,13 @@ public class TestProtocolRequirements extends AbstractProtocolTest { EasyMock.expect(mockCache.getCacheEntry( EasyMock.isA(HttpHost.class), EasyMock.isA(ClassicHttpRequest.class))).andReturn(entry).times(0, 1); - EasyMock.expect(mockCache.cacheAndReturnResponse( + EasyMock.expect(mockCache.createCacheEntry( EasyMock.isA(HttpHost.class), EasyMock.isA(ClassicHttpRequest.class), eqCloseableResponse(validated), + EasyMock.isA(ByteArrayBuffer.class), EasyMock.isA(Date.class), - EasyMock.isA(Date.class))).andReturn(reconstructed).times(0, 1); + EasyMock.isA(Date.class))).andReturn(cacheEntry).times(0, 1); replayMocks(); final ClassicHttpResponse result = execute(request); diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/async/methods/SimpleHttpResponse.java b/httpclient5/src/main/java/org/apache/hc/client5/http/async/methods/SimpleHttpResponse.java index 7d1686d33..47a98b13c 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/async/methods/SimpleHttpResponse.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/async/methods/SimpleHttpResponse.java @@ -43,6 +43,10 @@ public final class SimpleHttpResponse extends BasicHttpResponse { super(code); } + public SimpleHttpResponse(final int code, final String reasonPhrase) { + super(code, reasonPhrase); + } + public static SimpleHttpResponse copy(final HttpResponse original) { Args.notNull(original, "HTTP response"); final SimpleHttpResponse copy = new SimpleHttpResponse(original.getCode()); @@ -53,6 +57,34 @@ public final class SimpleHttpResponse extends BasicHttpResponse { return copy; } + public static SimpleHttpResponse create(final int code) { + return new SimpleHttpResponse(code); + } + + public static SimpleHttpResponse create(final int code, final String content, final ContentType contentType) { + final SimpleHttpResponse response = new SimpleHttpResponse(code); + if (content != null) { + response.setBodyText(content, contentType); + } + return response; + } + + public static SimpleHttpResponse create(final int code, final String content) { + return create(code, content, ContentType.TEXT_PLAIN); + } + + public static SimpleHttpResponse create(final int code, final byte[] content, final ContentType contentType) { + final SimpleHttpResponse response = new SimpleHttpResponse(code); + if (content != null) { + response.setBodyBytes(content, contentType); + } + return response; + } + + public static SimpleHttpResponse create(final int code, final byte[] content) { + return create(code, content, ContentType.TEXT_PLAIN); + } + public void setBody(final SimpleBody body) { this.body = body; }