From d75c5c2be210fd5aed1167c25e958fbd591fc55a Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Mon, 2 Oct 2017 11:25:39 +0200 Subject: [PATCH] Removed dependency on classic (blocking) I/O APIs from ConditionalRequestBuilder --- .../client5/http/impl/cache/CachingExec.java | 7 +- .../impl/cache/ConditionalRequestBuilder.java | 26 ++++--- .../http/impl/cache/TestCachingExec.java | 2 +- .../http/impl/cache/TestCachingExecChain.java | 17 +++-- .../cache/TestConditionalRequestBuilder.java | 74 +++++++++---------- 5 files changed, 63 insertions(+), 63 deletions(-) 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 51c13d8d0..df951a79a 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 @@ -44,6 +44,7 @@ import org.apache.hc.client5.http.cache.HttpCacheStorage; import org.apache.hc.client5.http.cache.ResourceFactory; import org.apache.hc.client5.http.classic.ExecChain; import org.apache.hc.client5.http.classic.ExecChainHandler; +import org.apache.hc.client5.http.impl.classic.ClassicRequestCopier; import org.apache.hc.client5.http.protocol.HttpClientContext; import org.apache.hc.client5.http.utils.DateUtils; import org.apache.hc.core5.annotation.Contract; @@ -120,7 +121,7 @@ public class CachingExec implements ExecChainHandler { private final CachedHttpResponseGenerator responseGenerator; private final CacheableRequestPolicy cacheableRequestPolicy; private final CachedResponseSuitabilityChecker suitabilityChecker; - private final ConditionalRequestBuilder conditionalRequestBuilder; + private final ConditionalRequestBuilder conditionalRequestBuilder; private final ResponseProtocolCompliance responseCompliance; private final RequestProtocolCompliance requestCompliance; private final ResponseCachingPolicy responseCachingPolicy; @@ -147,7 +148,7 @@ public class CachingExec implements ExecChainHandler { this.responseGenerator = new CachedHttpResponseGenerator(this.validityPolicy); this.cacheableRequestPolicy = new CacheableRequestPolicy(); this.suitabilityChecker = new CachedResponseSuitabilityChecker(this.validityPolicy, this.cacheConfig); - this.conditionalRequestBuilder = new ConditionalRequestBuilder(); + this.conditionalRequestBuilder = new ConditionalRequestBuilder<>(ClassicRequestCopier.INSTANCE); this.responseCompliance = new ResponseProtocolCompliance(); this.requestCompliance = new RequestProtocolCompliance(this.cacheConfig.isWeakETagOnPutDeleteAllowed()); this.responseCachingPolicy = new ResponseCachingPolicy( @@ -174,7 +175,7 @@ public class CachingExec implements ExecChainHandler { final CachedHttpResponseGenerator responseGenerator, final CacheableRequestPolicy cacheableRequestPolicy, final CachedResponseSuitabilityChecker suitabilityChecker, - final ConditionalRequestBuilder conditionalRequestBuilder, + final ConditionalRequestBuilder conditionalRequestBuilder, final ResponseProtocolCompliance responseCompliance, final RequestProtocolCompliance requestCompliance, final CacheConfig config, diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ConditionalRequestBuilder.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ConditionalRequestBuilder.java index d298c93af..1efaa619a 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ConditionalRequestBuilder.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ConditionalRequestBuilder.java @@ -31,12 +31,12 @@ 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.impl.classic.ClassicRequestCopier; +import org.apache.hc.client5.http.impl.MessageCopier; 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.Header; import org.apache.hc.core5.http.HeaderElement; +import org.apache.hc.core5.http.HttpRequest; import org.apache.hc.core5.http.ProtocolException; import org.apache.hc.core5.http.message.MessageSupport; @@ -44,7 +44,13 @@ import org.apache.hc.core5.http.message.MessageSupport; * @since 4.1 */ @Contract(threading = ThreadingBehavior.IMMUTABLE) -class ConditionalRequestBuilder { +class ConditionalRequestBuilder { + + private final MessageCopier messageCopier; + + ConditionalRequestBuilder(final MessageCopier messageCopier) { + this.messageCopier = messageCopier; + } /** * When a {@link HttpCacheEntry} is stale but 'might' be used as a response @@ -57,9 +63,8 @@ class ConditionalRequestBuilder { * @return the wrapped request * @throws ProtocolException when I am unable to build a new origin request. */ - public ClassicHttpRequest buildConditionalRequest(final ClassicHttpRequest request, final HttpCacheEntry cacheEntry) - throws ProtocolException { - final ClassicHttpRequest newRequest = ClassicRequestCopier.INSTANCE.copy(request); + public T buildConditionalRequest(final T request, final HttpCacheEntry cacheEntry) throws ProtocolException { + final T newRequest = messageCopier.copy(request); newRequest.setHeaders(request.getAllHeaders()); final Header eTag = cacheEntry.getFirstHeader(HeaderConstants.ETAG); if (eTag != null) { @@ -97,9 +102,8 @@ class ConditionalRequestBuilder { * @param variants * @return the wrapped request */ - public ClassicHttpRequest buildConditionalRequestFromVariants(final ClassicHttpRequest request, - final Map variants) { - final ClassicHttpRequest newRequest = ClassicRequestCopier.INSTANCE.copy(request); + public T buildConditionalRequestFromVariants(final T request, final Map variants) { + final T newRequest = messageCopier.copy(request); newRequest.setHeaders(request.getAllHeaders()); // we do not support partial content so all etags are used @@ -127,8 +131,8 @@ class ConditionalRequestBuilder { * @param request client request we are trying to satisfy * @return an unconditional validation request */ - public ClassicHttpRequest buildUnconditionalRequest(final ClassicHttpRequest request) { - final ClassicHttpRequest newRequest = ClassicRequestCopier.INSTANCE.copy(request); + public T buildUnconditionalRequest(final T request) { + final T newRequest = messageCopier.copy(request); newRequest.addHeader(HeaderConstants.CACHE_CONTROL,HeaderConstants.CACHE_CONTROL_NO_CACHE); newRequest.addHeader(HeaderConstants.PRAGMA,HeaderConstants.CACHE_CONTROL_NO_CACHE); newRequest.removeHeaders(HeaderConstants.IF_RANGE); 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 daeb4688e..54e2e6bb0 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 @@ -105,7 +105,7 @@ public class TestCachingExec extends TestCachingExecChain { final CachedHttpResponseGenerator mockResponseGenerator, final CacheableRequestPolicy mockRequestPolicy, final CachedResponseSuitabilityChecker mockSuitabilityChecker, - final ConditionalRequestBuilder mockConditionalRequestBuilder, + final ConditionalRequestBuilder mockConditionalRequestBuilder, final ResponseProtocolCompliance mockResponseProtocolCompliance, final RequestProtocolCompliance mockRequestProtocolCompliance, final CacheConfig config, final AsynchronousValidator asyncValidator) { 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 8a3dfce2e..9669193e9 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 @@ -105,7 +105,7 @@ public abstract class TestCachingExecChain { protected CachedHttpResponseGenerator mockResponseGenerator; private HttpClientResponseHandler mockHandler; private ClassicHttpRequest mockUriRequest; - protected ConditionalRequestBuilder mockConditionalRequestBuilder; + protected ConditionalRequestBuilder mockConditionalRequestBuilder; private HttpRequest mockConditionalRequest; protected ResponseProtocolCompliance mockResponseProtocolCompliance; protected RequestProtocolCompliance mockRequestProtocolCompliance; @@ -151,13 +151,14 @@ public abstract class TestCachingExecChain { mockRequestProtocolCompliance, config, asyncValidator); } - public abstract CachingExec createCachingExecChain(HttpCache responseCache, CacheValidityPolicy validityPolicy, - ResponseCachingPolicy responseCachingPolicy, CachedHttpResponseGenerator responseGenerator, - CacheableRequestPolicy cacheableRequestPolicy, - CachedResponseSuitabilityChecker suitabilityChecker, - ConditionalRequestBuilder conditionalRequestBuilder, - ResponseProtocolCompliance responseCompliance, RequestProtocolCompliance requestCompliance, - CacheConfig config, AsynchronousValidator asynchRevalidator); + public abstract CachingExec createCachingExecChain( + HttpCache responseCache, CacheValidityPolicy validityPolicy, + ResponseCachingPolicy responseCachingPolicy, CachedHttpResponseGenerator responseGenerator, + CacheableRequestPolicy cacheableRequestPolicy, + CachedResponseSuitabilityChecker suitabilityChecker, + ConditionalRequestBuilder conditionalRequestBuilder, + ResponseProtocolCompliance responseCompliance, RequestProtocolCompliance requestCompliance, + CacheConfig config, AsynchronousValidator asynchRevalidator); public abstract CachingExec createCachingExecChain(HttpCache cache, CacheConfig config); diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestConditionalRequestBuilder.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestConditionalRequestBuilder.java index d5c596eb5..b664d8d5d 100644 --- a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestConditionalRequestBuilder.java +++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestConditionalRequestBuilder.java @@ -31,18 +31,16 @@ import java.util.HashMap; import java.util.Iterator; import java.util.Map; -import org.apache.hc.client5.http.HttpRoute; import org.apache.hc.client5.http.cache.HeaderConstants; import org.apache.hc.client5.http.cache.HttpCacheEntry; -import org.apache.hc.client5.http.impl.classic.ClassicRequestCopier; +import org.apache.hc.client5.http.impl.RequestCopier; import org.apache.hc.client5.http.utils.DateUtils; -import org.apache.hc.core5.http.ClassicHttpRequest; import org.apache.hc.core5.http.Header; import org.apache.hc.core5.http.HeaderElement; -import org.apache.hc.core5.http.HttpHost; +import org.apache.hc.core5.http.HttpRequest; import org.apache.hc.core5.http.ProtocolException; -import org.apache.hc.core5.http.message.BasicClassicHttpRequest; import org.apache.hc.core5.http.message.BasicHeader; +import org.apache.hc.core5.http.message.BasicHttpRequest; import org.apache.hc.core5.http.message.MessageSupport; import org.junit.Assert; import org.junit.Before; @@ -50,17 +48,13 @@ import org.junit.Test; public class TestConditionalRequestBuilder { - private ConditionalRequestBuilder impl; - private HttpHost host; - private HttpRoute route; - private ClassicHttpRequest request; + private ConditionalRequestBuilder impl; + private HttpRequest request; @Before public void setUp() throws Exception { - impl = new ConditionalRequestBuilder(); - host = new HttpHost("foo.example.com", 80); - route = new HttpRoute(host); - request = new BasicClassicHttpRequest("GET", "/"); + impl = new ConditionalRequestBuilder<>(RequestCopier.INSTANCE); + request = new BasicHttpRequest("GET", "/"); } @Test @@ -69,16 +63,16 @@ public class TestConditionalRequestBuilder { final String theUri = "/theuri"; final String lastModified = "this is my last modified date"; - final ClassicHttpRequest basicRequest = new BasicClassicHttpRequest(theMethod, theUri); + final HttpRequest basicRequest = new BasicHttpRequest(theMethod, theUri); basicRequest.addHeader("Accept-Encoding", "gzip"); - final ClassicHttpRequest requestWrapper = ClassicRequestCopier.INSTANCE.copy(basicRequest); + final HttpRequest requestWrapper = RequestCopier.INSTANCE.copy(basicRequest); final Header[] headers = new Header[] { new BasicHeader("Date", DateUtils.formatDate(new Date())), new BasicHeader("Last-Modified", lastModified) }; final HttpCacheEntry cacheEntry = HttpTestUtils.makeCacheEntry(headers); - final ClassicHttpRequest newRequest = impl.buildConditionalRequest(requestWrapper, cacheEntry); + final HttpRequest newRequest = impl.buildConditionalRequest(requestWrapper, cacheEntry); Assert.assertNotSame(basicRequest, newRequest); @@ -106,10 +100,10 @@ public class TestConditionalRequestBuilder { new BasicHeader("Last-Modified", lmDate), new BasicHeader("ETag", etag) }; - final ClassicHttpRequest basicRequest = new BasicClassicHttpRequest("GET", "/"); - final ClassicHttpRequest requestWrapper = ClassicRequestCopier.INSTANCE.copy(basicRequest); + final HttpRequest basicRequest = new BasicHttpRequest("GET", "/"); + final HttpRequest requestWrapper = RequestCopier.INSTANCE.copy(basicRequest); final HttpCacheEntry cacheEntry = HttpTestUtils.makeCacheEntry(headers); - final ClassicHttpRequest result = impl.buildConditionalRequest(requestWrapper, cacheEntry); + final HttpRequest result = impl.buildConditionalRequest(requestWrapper, cacheEntry); Assert.assertEquals(lmDate, result.getFirstHeader("If-Modified-Since").getValue()); Assert.assertEquals(etag, @@ -122,9 +116,9 @@ public class TestConditionalRequestBuilder { final String theUri = "/theuri"; final String theETag = "this is my eTag"; - final ClassicHttpRequest basicRequest = new BasicClassicHttpRequest(theMethod, theUri); + final HttpRequest basicRequest = new BasicHttpRequest(theMethod, theUri); basicRequest.addHeader("Accept-Encoding", "gzip"); - final ClassicHttpRequest requestWrapper = ClassicRequestCopier.INSTANCE.copy(basicRequest); + final HttpRequest requestWrapper = RequestCopier.INSTANCE.copy(basicRequest); final Header[] headers = new Header[] { new BasicHeader("Date", DateUtils.formatDate(new Date())), @@ -133,7 +127,7 @@ public class TestConditionalRequestBuilder { final HttpCacheEntry cacheEntry = HttpTestUtils.makeCacheEntry(headers); - final ClassicHttpRequest newRequest = impl.buildConditionalRequest(requestWrapper, cacheEntry); + final HttpRequest newRequest = impl.buildConditionalRequest(requestWrapper, cacheEntry); Assert.assertNotSame(basicRequest, newRequest); @@ -151,8 +145,8 @@ public class TestConditionalRequestBuilder { @Test public void testCacheEntryWithMustRevalidateDoesEndToEndRevalidation() throws Exception { - final ClassicHttpRequest basicRequest = new BasicClassicHttpRequest("GET","/"); - final ClassicHttpRequest requestWrapper = ClassicRequestCopier.INSTANCE.copy(basicRequest); + final HttpRequest basicRequest = new BasicHttpRequest("GET","/"); + final HttpRequest requestWrapper = RequestCopier.INSTANCE.copy(basicRequest); final Date now = new Date(); final Date elevenSecondsAgo = new Date(now.getTime() - 11 * 1000L); final Date tenSecondsAgo = new Date(now.getTime() - 10 * 1000L); @@ -164,7 +158,7 @@ public class TestConditionalRequestBuilder { new BasicHeader("Cache-Control","max-age=5, must-revalidate") }; final HttpCacheEntry cacheEntry = HttpTestUtils.makeCacheEntry(elevenSecondsAgo, nineSecondsAgo, cacheEntryHeaders); - final ClassicHttpRequest result = impl.buildConditionalRequest(requestWrapper, cacheEntry); + final HttpRequest result = impl.buildConditionalRequest(requestWrapper, cacheEntry); boolean foundMaxAge0 = false; @@ -180,8 +174,8 @@ public class TestConditionalRequestBuilder { @Test public void testCacheEntryWithProxyRevalidateDoesEndToEndRevalidation() throws Exception { - final ClassicHttpRequest basicRequest = new BasicClassicHttpRequest("GET", "/"); - final ClassicHttpRequest requestWrapper = ClassicRequestCopier.INSTANCE.copy(basicRequest); + final HttpRequest basicRequest = new BasicHttpRequest("GET", "/"); + final HttpRequest requestWrapper = RequestCopier.INSTANCE.copy(basicRequest); final Date now = new Date(); final Date elevenSecondsAgo = new Date(now.getTime() - 11 * 1000L); final Date tenSecondsAgo = new Date(now.getTime() - 10 * 1000L); @@ -193,7 +187,7 @@ public class TestConditionalRequestBuilder { new BasicHeader("Cache-Control","max-age=5, proxy-revalidate") }; final HttpCacheEntry cacheEntry = HttpTestUtils.makeCacheEntry(elevenSecondsAgo, nineSecondsAgo, cacheEntryHeaders); - final ClassicHttpRequest result = impl.buildConditionalRequest(requestWrapper, cacheEntry); + final HttpRequest result = impl.buildConditionalRequest(requestWrapper, cacheEntry); boolean foundMaxAge0 = false; final Iterator it = MessageSupport.iterate(result, HeaderConstants.CACHE_CONTROL); @@ -209,7 +203,7 @@ public class TestConditionalRequestBuilder { @Test public void testBuildUnconditionalRequestUsesGETMethod() throws Exception { - final ClassicHttpRequest result = impl.buildUnconditionalRequest(request); + final HttpRequest result = impl.buildUnconditionalRequest(request); Assert.assertEquals("GET", result.getMethod()); } @@ -217,15 +211,15 @@ public class TestConditionalRequestBuilder { public void testBuildUnconditionalRequestUsesRequestUri() throws Exception { final String uri = "/theURI"; - request = new BasicClassicHttpRequest("GET", uri); - final ClassicHttpRequest result = impl.buildUnconditionalRequest(request); + request = new BasicHttpRequest("GET", uri); + final HttpRequest result = impl.buildUnconditionalRequest(request); Assert.assertEquals(uri, result.getRequestUri()); } @Test public void testBuildUnconditionalRequestAddsCacheControlNoCache() throws Exception { - final ClassicHttpRequest result = impl.buildUnconditionalRequest(request); + final HttpRequest result = impl.buildUnconditionalRequest(request); boolean ccNoCacheFound = false; final Iterator it = MessageSupport.iterate(result, HeaderConstants.CACHE_CONTROL); while (it.hasNext()) { @@ -240,7 +234,7 @@ public class TestConditionalRequestBuilder { @Test public void testBuildUnconditionalRequestAddsPragmaNoCache() throws Exception { - final ClassicHttpRequest result = impl.buildUnconditionalRequest(request); + final HttpRequest result = impl.buildUnconditionalRequest(request); boolean ccNoCacheFound = false; final Iterator it = MessageSupport.iterate(result, HeaderConstants.PRAGMA); while (it.hasNext()) { @@ -256,7 +250,7 @@ public class TestConditionalRequestBuilder { public void testBuildUnconditionalRequestDoesNotUseIfRange() throws Exception { request.addHeader("If-Range","\"etag\""); - final ClassicHttpRequest result = impl.buildUnconditionalRequest(request); + final HttpRequest result = impl.buildUnconditionalRequest(request); Assert.assertNull(result.getFirstHeader("If-Range")); } @@ -264,7 +258,7 @@ public class TestConditionalRequestBuilder { public void testBuildUnconditionalRequestDoesNotUseIfMatch() throws Exception { request.addHeader("If-Match","\"etag\""); - final ClassicHttpRequest result = impl.buildUnconditionalRequest(request); + final HttpRequest result = impl.buildUnconditionalRequest(request); Assert.assertNull(result.getFirstHeader("If-Match")); } @@ -272,7 +266,7 @@ public class TestConditionalRequestBuilder { public void testBuildUnconditionalRequestDoesNotUseIfNoneMatch() throws Exception { request.addHeader("If-None-Match","\"etag\""); - final ClassicHttpRequest result = impl.buildUnconditionalRequest(request); + final HttpRequest result = impl.buildUnconditionalRequest(request); Assert.assertNull(result.getFirstHeader("If-None-Match")); } @@ -280,7 +274,7 @@ public class TestConditionalRequestBuilder { public void testBuildUnconditionalRequestDoesNotUseIfUnmodifiedSince() throws Exception { request.addHeader("If-Unmodified-Since", DateUtils.formatDate(new Date())); - final ClassicHttpRequest result = impl.buildUnconditionalRequest(request); + final HttpRequest result = impl.buildUnconditionalRequest(request); Assert.assertNull(result.getFirstHeader("If-Unmodified-Since")); } @@ -288,7 +282,7 @@ public class TestConditionalRequestBuilder { public void testBuildUnconditionalRequestDoesNotUseIfModifiedSince() throws Exception { request.addHeader("If-Modified-Since", DateUtils.formatDate(new Date())); - final ClassicHttpRequest result = impl.buildUnconditionalRequest(request); + final HttpRequest result = impl.buildUnconditionalRequest(request); Assert.assertNull(result.getFirstHeader("If-Modified-Since")); } @@ -296,7 +290,7 @@ public class TestConditionalRequestBuilder { public void testBuildUnconditionalRequestCarriesOtherRequestHeaders() throws Exception { request.addHeader("User-Agent","MyBrowser/1.0"); - final ClassicHttpRequest result = impl.buildUnconditionalRequest(request); + final HttpRequest result = impl.buildUnconditionalRequest(request); Assert.assertEquals("MyBrowser/1.0", result.getFirstHeader("User-Agent").getValue()); } @@ -312,7 +306,7 @@ public class TestConditionalRequestBuilder { variantEntries.put(etag2, new Variant("C","D",HttpTestUtils.makeCacheEntry(new Header[] { new BasicHeader("ETag", etag2) }))); variantEntries.put(etag3, new Variant("E","F",HttpTestUtils.makeCacheEntry(new Header[] { new BasicHeader("ETag", etag3) }))); - final ClassicHttpRequest conditional = impl.buildConditionalRequestFromVariants(request, variantEntries); + final HttpRequest conditional = impl.buildConditionalRequestFromVariants(request, variantEntries); // seems like a lot of work, but necessary, check for existence and exclusiveness String ifNoneMatch = conditional.getFirstHeader(HeaderConstants.IF_NONE_MATCH).getValue();