From d6d3d364eb5c4729eb5ae2b2d252a57ffd9fc4ee Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Fri, 22 Dec 2017 15:12:44 +0100 Subject: [PATCH] Clean up of CacheEntryUpdater (CacheEntryUpdater renamed to CacheUpdateHandler) --- .../http/impl/cache/BasicHttpCache.java | 14 +- ...ryUpdater.java => CacheUpdateHandler.java} | 138 +++++++----------- ...dater.java => TestCacheUpdateHandler.java} | 6 +- 3 files changed, 59 insertions(+), 99 deletions(-) rename httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/{CacheEntryUpdater.java => CacheUpdateHandler.java} (57%) rename httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/{TestCacheEntryUpdater.java => TestCacheUpdateHandler.java} (98%) 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 61f6311a4..eb4f3d7db 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 @@ -49,7 +49,7 @@ class BasicHttpCache implements HttpCache { - private final CacheEntryUpdater cacheEntryUpdater; + private final CacheUpdateHandler cacheUpdateHandler; private final CacheKeyGenerator cacheKeyGenerator; private final HttpCacheInvalidator cacheInvalidator; private final HttpCacheStorage storage; @@ -61,7 +61,7 @@ public BasicHttpCache( final HttpCacheStorage storage, final CacheKeyGenerator cacheKeyGenerator, final HttpCacheInvalidator cacheInvalidator) { - this.cacheEntryUpdater = new CacheEntryUpdater(resourceFactory); + this.cacheUpdateHandler = new CacheUpdateHandler(resourceFactory); this.cacheKeyGenerator = cacheKeyGenerator; this.storage = storage; this.cacheInvalidator = cacheInvalidator; @@ -130,7 +130,7 @@ void storeVariantEntry( @Override public HttpCacheEntry execute(final HttpCacheEntry existing) throws ResourceIOException { - return cacheEntryUpdater.updateParentCacheEntry(req.getRequestUri(), existing, entry, variantKey, variantURI); + return cacheUpdateHandler.updateParentCacheEntry(req.getRequestUri(), existing, entry, variantKey, variantURI); } }); @@ -152,7 +152,7 @@ public void reuseVariantEntryFor( @Override public HttpCacheEntry execute(final HttpCacheEntry existing) throws ResourceIOException { - return cacheEntryUpdater.updateParentCacheEntry(req.getRequestUri(), existing, entry, variantKey, variantCacheKey); + return cacheUpdateHandler.updateParentCacheEntry(req.getRequestUri(), existing, entry, variantKey, variantCacheKey); } }); @@ -169,7 +169,7 @@ public HttpCacheEntry updateCacheEntry( final HttpResponse originResponse, final Date requestSent, final Date responseReceived) throws ResourceIOException { - final HttpCacheEntry updatedEntry = cacheEntryUpdater.updateCacheEntry( + final HttpCacheEntry updatedEntry = cacheUpdateHandler.updateCacheEntry( request.getRequestUri(), stale, requestSent, @@ -183,7 +183,7 @@ public HttpCacheEntry updateCacheEntry( public HttpCacheEntry updateVariantCacheEntry(final HttpHost target, final HttpRequest request, final HttpCacheEntry stale, final HttpResponse originResponse, final Date requestSent, final Date responseReceived, final String cacheKey) throws ResourceIOException { - final HttpCacheEntry updatedEntry = cacheEntryUpdater.updateCacheEntry( + final HttpCacheEntry updatedEntry = cacheUpdateHandler.updateCacheEntry( request.getRequestUri(), stale, requestSent, @@ -200,7 +200,7 @@ public HttpCacheEntry createCacheEntry( final ByteArrayBuffer content, final Date requestSent, final Date responseReceived) throws ResourceIOException { - final HttpCacheEntry entry = cacheEntryUpdater.createtCacheEntry(request, originResponse, content, requestSent, responseReceived); + final HttpCacheEntry entry = cacheUpdateHandler.createtCacheEntry(request, originResponse, content, requestSent, responseReceived); storeInCache(host, request, entry); return entry; } diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheEntryUpdater.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheUpdateHandler.java similarity index 57% rename from httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheEntryUpdater.java rename to httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheUpdateHandler.java index 899fa858b..5034e04d4 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheEntryUpdater.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheUpdateHandler.java @@ -26,12 +26,9 @@ */ package org.apache.hc.client5.http.impl.cache; -import java.util.ArrayList; -import java.util.Arrays; import java.util.Date; import java.util.HashMap; -import java.util.List; -import java.util.ListIterator; +import java.util.Iterator; import java.util.Map; import org.apache.hc.client5.http.cache.HeaderConstants; @@ -47,48 +44,46 @@ 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.MessageHeaders; +import org.apache.hc.core5.http.message.HeaderGroup; import org.apache.hc.core5.util.Args; import org.apache.hc.core5.util.ByteArrayBuffer; /** - * Update a {@link HttpCacheEntry} with new or updated information based on the latest - * 304 status response from the Server. Use the {@link HttpResponse} to perform - * the processChallenge. + * Creates new {@link HttpCacheEntry}s and updates existing ones with new or updated information + * based on the response from the origin server. * - * @since 4.1 + * @since 5.0 */ @Contract(threading = ThreadingBehavior.IMMUTABLE) -class CacheEntryUpdater { +class CacheUpdateHandler { private final ResourceFactory resourceFactory; - CacheEntryUpdater() { + CacheUpdateHandler() { this(new HeapResourceFactory()); } - CacheEntryUpdater(final ResourceFactory resourceFactory) { + CacheUpdateHandler(final ResourceFactory resourceFactory) { super(); this.resourceFactory = resourceFactory; } + /** + * Creates a cache entry for the given request, origin response message and response content. + */ public HttpCacheEntry createtCacheEntry( final HttpRequest request, final HttpResponse originResponse, final ByteArrayBuffer content, final Date requestSent, final Date responseReceived) throws ResourceIOException { - final Resource resource; - if (content != null) { - resource = resourceFactory.generate(request.getRequestUri(), content.array(), 0, content.length()); - } else { - resource = null; - } return new HttpCacheEntry( requestSent, responseReceived, originResponse.getCode(), originResponse.getAllHeaders(), - resource); + content != null ? resourceFactory.generate(request.getRequestUri(), content.array(), 0, content.length()) : null); } /** @@ -116,77 +111,6 @@ public HttpCacheEntry updateCacheEntry( resource); } - protected Header[] mergeHeaders(final HttpCacheEntry entry, final HttpResponse response) { - - if (entryAndResponseHaveDateHeader(entry, response) - && entryDateHeaderNewerThenResponse(entry, response)) { - // Don't merge headers, keep the entry's headers as they are newer. - return entry.getAllHeaders(); - } - - final List
cacheEntryHeaderList = new ArrayList<>(Arrays.asList(entry - .getAllHeaders())); - removeCacheHeadersThatMatchResponse(cacheEntryHeaderList, response); - removeCacheEntry1xxWarnings(cacheEntryHeaderList, entry); - cacheEntryHeaderList.addAll(Arrays.asList(response.getAllHeaders())); - - return cacheEntryHeaderList.toArray(new Header[cacheEntryHeaderList.size()]); - } - - private void removeCacheHeadersThatMatchResponse(final List
cacheEntryHeaderList, - final HttpResponse response) { - for (final Header responseHeader : response.getAllHeaders()) { - final ListIterator
cacheEntryHeaderListIter = cacheEntryHeaderList.listIterator(); - - while (cacheEntryHeaderListIter.hasNext()) { - final String cacheEntryHeaderName = cacheEntryHeaderListIter.next().getName(); - - if (cacheEntryHeaderName.equals(responseHeader.getName())) { - cacheEntryHeaderListIter.remove(); - } - } - } - } - - private void removeCacheEntry1xxWarnings(final List
cacheEntryHeaderList, final HttpCacheEntry entry) { - final ListIterator
cacheEntryHeaderListIter = cacheEntryHeaderList.listIterator(); - - while (cacheEntryHeaderListIter.hasNext()) { - final String cacheEntryHeaderName = cacheEntryHeaderListIter.next().getName(); - - if (HeaderConstants.WARNING.equals(cacheEntryHeaderName)) { - for (final Header cacheEntryWarning : entry.getHeaders(HeaderConstants.WARNING)) { - if (cacheEntryWarning.getValue().startsWith("1")) { - cacheEntryHeaderListIter.remove(); - } - } - } - } - } - - private boolean entryDateHeaderNewerThenResponse(final HttpCacheEntry entry, final HttpResponse response) { - final Date entryDate = DateUtils.parseDate(entry.getFirstHeader(HttpHeaders.DATE) - .getValue()); - final Date responseDate = DateUtils.parseDate(response.getFirstHeader(HttpHeaders.DATE) - .getValue()); - if (entryDate == null || responseDate == null) { - return false; - } - if (!entryDate.after(responseDate)) { - return false; - } - return true; - } - - private boolean entryAndResponseHaveDateHeader(final HttpCacheEntry entry, final HttpResponse response) { - if (entry.getFirstHeader(HttpHeaders.DATE) != null - && response.getFirstHeader(HttpHeaders.DATE) != null) { - return true; - } - - return false; - } - public HttpCacheEntry updateParentCacheEntry( final String requestId, final HttpCacheEntry existing, @@ -213,4 +137,40 @@ public HttpCacheEntry updateParentCacheEntry( variantMap); } + private static Date getDate(final MessageHeaders messageHeaders) { + final Header dateHeader = messageHeaders.getFirstHeader(HttpHeaders.DATE); + return dateHeader != null ? DateUtils.parseDate(dateHeader.getValue()): null; + } + + private Header[] mergeHeaders(final HttpCacheEntry entry, final HttpResponse response) { + final Date cacheDate = getDate(entry); + final Date responseDate = getDate(response); + if (cacheDate != null && responseDate != null && cacheDate.after(responseDate)) { + // Don't merge headers, keep the entry's headers as they are newer. + return entry.getAllHeaders(); + } + final HeaderGroup headerGroup = new HeaderGroup(); + headerGroup.setHeaders(entry.getAllHeaders()); + // Remove cache headers that match response + for (final Iterator
it = response.headerIterator(); it.hasNext(); ) { + final Header responseHeader = it.next(); + headerGroup.removeHeaders(responseHeader.getName()); + } + // remove cache entry 1xx warnings + for (final Iterator
it = headerGroup.headerIterator(); it.hasNext(); ) { + final Header cacheHeader = it.next(); + if (HeaderConstants.WARNING.equalsIgnoreCase(cacheHeader.getName())) { + final String warningValue = cacheHeader.getValue(); + if (warningValue != null && warningValue.startsWith("1")) { + it.remove(); + } + } + } + for (final Iterator
it = response.headerIterator(); it.hasNext(); ) { + final Header responseHeader = it.next(); + headerGroup.addHeader(responseHeader); + } + return headerGroup.getAllHeaders(); + } + } diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheEntryUpdater.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheUpdateHandler.java similarity index 98% rename from httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheEntryUpdater.java rename to httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheUpdateHandler.java index 1f533531d..817010547 100644 --- a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheEntryUpdater.java +++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheUpdateHandler.java @@ -46,12 +46,12 @@ import org.junit.Before; import org.junit.Test; -public class TestCacheEntryUpdater { +public class TestCacheUpdateHandler { private Date requestDate; private Date responseDate; - private CacheEntryUpdater impl; + private CacheUpdateHandler impl; private HttpCacheEntry entry; private Date now; private Date oneSecondAgo; @@ -73,7 +73,7 @@ public void setUp() throws Exception { response = new BasicHttpResponse(HttpStatus.SC_NOT_MODIFIED, "Not Modified"); - impl = new CacheEntryUpdater(); + impl = new CacheUpdateHandler(); } @Test