From 8269a236be41775bd2ad167d13230241d89b8613 Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Mon, 11 Sep 2023 14:01:09 +0200 Subject: [PATCH 1/2] HTTPCLIENT-2277, regression: corrected wrong variant entry keys in cache hits returned by `BasicHttpCache#getVariants` --- .../apache/hc/client5/http/impl/cache/BasicHttpCache.java | 5 +++-- .../hc/client5/http/impl/cache/TestBasicHttpCache.java | 6 ++++-- 2 files changed, 7 insertions(+), 4 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 d48ec2f9f..aa56600ae 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 @@ -179,9 +179,10 @@ public List getVariants(final CacheHit hit) { if (root != null && root.hasVariants()) { final List variants = new ArrayList<>(); for (final String variantKey : root.getVariants()) { - final HttpCacheEntry variant = getInternal(variantKey + rootKey); + final String variantCacheKey = variantKey + rootKey; + final HttpCacheEntry variant = getInternal(variantCacheKey); if (variant != null) { - variants.add(new CacheHit(hit.rootKey, variantKey, variant)); + variants.add(new CacheHit(rootKey, variantCacheKey, variant)); } } return variants; 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 67e6bef86..b1c86b3c0 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 @@ -261,8 +261,10 @@ public void testGetVariantCacheEntriesReturnsAllVariants() throws Exception { assertNotNull(variantMap); assertEquals(2, variantMap.size()); - MatcherAssert.assertThat(variantMap.get("{accept-encoding=gzip}"), HttpCacheEntryMatcher.equivalent(hit1.entry)); - MatcherAssert.assertThat(variantMap.get("{accept-encoding=identity}"), HttpCacheEntryMatcher.equivalent(hit2.entry)); + MatcherAssert.assertThat(variantMap.get("{accept-encoding=gzip}" + rootKey), + HttpCacheEntryMatcher.equivalent(hit1.entry)); + MatcherAssert.assertThat(variantMap.get("{accept-encoding=identity}" + rootKey), + HttpCacheEntryMatcher.equivalent(hit2.entry)); } @Test From 9f5161793ebd5c0253dcba7a81203c21c0cc3bf6 Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Wed, 13 Sep 2023 19:23:46 +0200 Subject: [PATCH 2/2] HTTPCLIENT-2277: revised cache update handling * improved and simplified internal cache API * corrected cache update logic after a variant negotiation * added more tests --- .../http/cache/HttpCacheEntryFactory.java | 36 +- .../http/impl/cache/AsyncCachingExec.java | 30 +- .../http/impl/cache/BasicHttpAsyncCache.java | 282 ++++---- .../http/impl/cache/BasicHttpCache.java | 102 +-- .../http/impl/cache/CacheKeyGenerator.java | 33 +- .../client5/http/impl/cache/CachingExec.java | 7 +- .../http/impl/cache/HttpAsyncCache.java | 16 +- .../hc/client5/http/impl/cache/HttpCache.java | 15 +- .../http/cache/TestHttpCacheEntryFactory.java | 10 +- .../http/impl/cache/HttpTestUtils.java | 10 +- .../impl/cache/TestBasicHttpAsyncCache.java | 638 +++++++++++++++--- .../http/impl/cache/TestBasicHttpCache.java | 184 ++++- .../impl/cache/TestCacheKeyGenerator.java | 21 +- 13 files changed, 968 insertions(+), 416 deletions(-) diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/cache/HttpCacheEntryFactory.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/cache/HttpCacheEntryFactory.java index b639840bc..8d44f594a 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/cache/HttpCacheEntryFactory.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/cache/HttpCacheEntryFactory.java @@ -173,38 +173,24 @@ static String normalizeRequestUri(final HttpHost host, final HttpRequest request /** * Creates a new root {@link HttpCacheEntry} (parent of multiple variants). * - * @param requestInstant Date/time when the request was made (Used for age calculations) - * @param responseInstant Date/time that the response came back (Used for age calculations) - * @param host Target host - * @param request Original client request (a deep copy of this object is made) + * @param latestVariant The most recently created variant entry * @param variants describing cache entries that are variants of this parent entry; this * maps a "variant key" (derived from the varying request headers) to a * "cache key" (where in the cache storage the particular variant is * located) */ - public HttpCacheEntry createRoot(final Instant requestInstant, - final Instant responseInstant, - final HttpHost host, - final HttpRequest request, - final HttpResponse response, + public HttpCacheEntry createRoot(final HttpCacheEntry latestVariant, final Collection variants) { - Args.notNull(requestInstant, "Request instant"); - Args.notNull(responseInstant, "Response instant"); - Args.notNull(host, "Host"); - Args.notNull(request, "Request"); - Args.notNull(response, "Origin response"); - final String requestUri = normalizeRequestUri(host, request); - final HeaderGroup requestHeaders = filterHopByHopHeaders(request); - final HeaderGroup responseHeaders = filterHopByHopHeaders(response); - ensureDate(responseHeaders, responseInstant); + Args.notNull(latestVariant, "Request"); + Args.notNull(variants, "Variants"); return new HttpCacheEntry( - requestInstant, - responseInstant, - request.getMethod(), - requestUri, - requestHeaders, - response.getCode(), - responseHeaders, + latestVariant.getRequestInstant(), + latestVariant.getResponseInstant(), + latestVariant.getRequestMethod(), + latestVariant.getRequestURI(), + headers(latestVariant.requestHeaderIterator()), + latestVariant.getStatus(), + headers(latestVariant.headerIterator()), null, variants); } diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/AsyncCachingExec.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/AsyncCachingExec.java index c9e9987ab..50aac133e 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/AsyncCachingExec.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/AsyncCachingExec.java @@ -938,8 +938,10 @@ void negotiateResponseFromVariants( void updateVariantCacheEntry(final HttpResponse backendResponse, final Instant responseDate, final CacheHit match) { recordCacheUpdate(scope.clientContext); - operation.setDependency(responseCache.update( + operation.setDependency(responseCache.storeFromNegotiated( match, + target, + request, backendResponse, requestDate, responseDate, @@ -953,31 +955,7 @@ public void completed(final CacheHit hit) { } else { try { final SimpleHttpResponse cacheResponse = responseGenerator.generateResponse(request, hit.entry); - operation.setDependency(responseCache.storeReusing( - hit, - target, - request, - backendResponse, - requestDate, - responseDate, - new FutureCallback() { - - @Override - public void completed(final CacheHit result) { - triggerResponse(cacheResponse, scope, asyncExecCallback); - } - - @Override - public void failed(final Exception ex) { - asyncExecCallback.failed(ex); - } - - @Override - public void cancelled() { - asyncExecCallback.failed(new InterruptedIOException()); - } - - })); + triggerResponse(cacheResponse, scope, asyncExecCallback); } catch (final ResourceIOException ex) { asyncExecCallback.failed(ex); } diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/BasicHttpAsyncCache.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/BasicHttpAsyncCache.java index d86ddbb49..5b204bf74 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/BasicHttpAsyncCache.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/BasicHttpAsyncCache.java @@ -38,6 +38,7 @@ import java.util.stream.Collectors; import org.apache.hc.client5.http.cache.HttpAsyncCacheStorage; +import org.apache.hc.client5.http.cache.HttpCacheCASOperation; import org.apache.hc.client5.http.cache.HttpCacheEntry; import org.apache.hc.client5.http.cache.HttpCacheEntryFactory; import org.apache.hc.client5.http.cache.HttpCacheUpdateException; @@ -45,6 +46,7 @@ import org.apache.hc.client5.http.cache.ResourceFactory; import org.apache.hc.client5.http.cache.ResourceIOException; import org.apache.hc.client5.http.impl.Operations; +import org.apache.hc.core5.concurrent.CallbackContribution; import org.apache.hc.core5.concurrent.Cancellable; import org.apache.hc.core5.concurrent.ComplexCancellable; import org.apache.hc.core5.concurrent.FutureCallback; @@ -142,9 +144,12 @@ public void cancelled() { })); return; + } else { + callback.completed(new CacheMatch(null, new CacheHit(rootKey, root))); } + } else { + callback.completed(new CacheMatch(new CacheHit(rootKey, root), null)); } - callback.completed(new CacheMatch(new CacheHit(rootKey, root), null)); } else { callback.completed(null); } @@ -220,135 +225,163 @@ public void cancelled() { return complexCancellable; } - Cancellable storeEntry( - final String rootKey, - final HttpCacheEntry entry, - final FutureCallback callback) { + Cancellable storeInternal(final String cacheKey, final HttpCacheEntry entry, final FutureCallback callback) { if (LOG.isDebugEnabled()) { - LOG.debug("Put entry in cache: {}", rootKey); + LOG.debug("Store entry in cache: {}", cacheKey); } - return storage.putEntry(rootKey, entry, new FutureCallback() { + return storage.putEntry(cacheKey, entry, new FutureCallback() { @Override public void completed(final Boolean result) { - callback.completed(new CacheHit(rootKey, entry)); + if (callback != null) { + callback.completed(result); + } } @Override public void failed(final Exception ex) { if (ex instanceof ResourceIOException) { if (LOG.isWarnEnabled()) { - LOG.warn("I/O error storing cache entry with key {}", rootKey); + LOG.warn("I/O error storing cache entry with key {}", cacheKey); + } + if (callback != null) { + callback.completed(false); } - callback.completed(new CacheHit(rootKey, entry)); } else { - callback.failed(ex); + if (callback != null) { + callback.failed(ex); + } } } @Override public void cancelled() { - callback.cancelled(); + if (callback != null) { + callback.cancelled(); + } } }); } - Cancellable storeVariant( - final HttpHost host, - final HttpRequest request, - final HttpResponse originResponse, - final Instant requestSent, - final Instant responseReceived, - final String rootKey, - final HttpCacheEntry entry, - final FutureCallback callback) { - final List variantNames = CacheKeyGenerator.variantNames(entry); - final String variantKey = cacheKeyGenerator.generateVariantKey(request, variantNames); - final String variantCacheKey = variantKey + rootKey; - - if (LOG.isDebugEnabled()) { - LOG.debug("Put variant entry in cache: {}", variantCacheKey); - } - - return storage.putEntry(variantCacheKey, entry, new FutureCallback() { + Cancellable updateInternal(final String cacheKey, final HttpCacheCASOperation casOperation, final FutureCallback callback) { + return storage.updateEntry(cacheKey, casOperation, new FutureCallback() { @Override public void completed(final Boolean result) { - if (LOG.isDebugEnabled()) { - LOG.debug("Update root entry: {}", rootKey); + if (callback != null) { + callback.completed(result); } + } - storage.updateEntry(rootKey, - existing -> { - final Set variantMap = existing != null ? new HashSet<>(existing.getVariants()) : new HashSet<>(); - variantMap.add(variantKey); - return cacheEntryFactory.createRoot(requestSent, responseReceived, host, request, originResponse, variantMap); - }, - new FutureCallback() { - - @Override - public void completed(final Boolean result) { - callback.completed(new CacheHit(rootKey, variantCacheKey, entry)); - } + @Override + public void failed(final Exception ex) { + if (ex instanceof HttpCacheUpdateException) { + if (LOG.isWarnEnabled()) { + LOG.warn("Cannot update cache entry with key {}", cacheKey); + } + if (callback != null) { + callback.completed(false); + } + } else if (ex instanceof ResourceIOException) { + if (LOG.isWarnEnabled()) { + LOG.warn("I/O error updating cache entry with key {}", cacheKey); + } + if (callback != null) { + callback.completed(false); + } + } else { + if (callback != null) { + callback.failed(ex); + } + } + } - @Override - public void failed(final Exception ex) { - if (ex instanceof HttpCacheUpdateException) { - if (LOG.isWarnEnabled()) { - LOG.warn("Cannot update cache entry with key {}", rootKey); - } - } else if (ex instanceof ResourceIOException) { - if (LOG.isWarnEnabled()) { - LOG.warn("I/O error updating cache entry with key {}", rootKey); - } - } else { - callback.failed(ex); - } - } + @Override + public void cancelled() { + if (callback != null) { + callback.cancelled(); + } + } - @Override - public void cancelled() { - callback.cancelled(); - } + }); + } + + private void removeInternal(final String cacheKey) { + storage.removeEntry(cacheKey, new FutureCallback() { - }); + @Override + public void completed(final Boolean result) { } @Override public void failed(final Exception ex) { - if (ex instanceof ResourceIOException) { - if (LOG.isWarnEnabled()) { - LOG.warn("I/O error updating cache entry with key {}", variantCacheKey); + if (LOG.isWarnEnabled()) { + if (ex instanceof ResourceIOException) { + LOG.warn("I/O error removing cache entry with key {}", cacheKey); + } else { + LOG.warn("Unexpected error removing cache entry with key {}", cacheKey, ex); } - callback.completed(new CacheHit(rootKey, variantCacheKey, entry)); - } else { - callback.failed(ex); } } @Override public void cancelled() { - callback.cancelled(); } }); } Cancellable store( - final HttpHost host, - final HttpRequest request, - final HttpResponse originResponse, - final Instant requestSent, - final Instant responseReceived, final String rootKey, + final String variantKey, final HttpCacheEntry entry, final FutureCallback callback) { - if (entry.containsHeader(HttpHeaders.VARY)) { - return storeVariant(host, request, originResponse, requestSent, responseReceived, rootKey, entry, callback); + if (variantKey == null) { + if (LOG.isDebugEnabled()) { + LOG.debug("Store entry in cache: {}", rootKey); + } + return storeInternal(rootKey, entry, new CallbackContribution(callback) { + + @Override + public void completed(final Boolean result) { + callback.completed(new CacheHit(rootKey, entry)); + } + + }); } else { - return storeEntry(rootKey, entry, callback); + final String variantCacheKey = variantKey + rootKey; + + if (LOG.isDebugEnabled()) { + LOG.debug("Store variant entry in cache: {}", variantCacheKey); + } + + return storeInternal(variantCacheKey, entry, new CallbackContribution(callback) { + + @Override + public void completed(final Boolean result) { + if (LOG.isDebugEnabled()) { + LOG.debug("Update root entry: {}", rootKey); + } + + updateInternal(rootKey, + existing -> { + final Set variantMap = existing != null ? new HashSet<>(existing.getVariants()) : new HashSet<>(); + variantMap.add(variantKey); + return cacheEntryFactory.createRoot(entry, variantMap); + }, + new CallbackContribution(callback) { + + @Override + public void completed(final Boolean result) { + callback.completed(new CacheHit(rootKey, variantCacheKey, entry)); + } + + }); + } + + }); } } @@ -383,15 +416,8 @@ public Cancellable store( return Operations.nonCancellable(); } final HttpCacheEntry entry = cacheEntryFactory.create(requestSent, responseReceived, host, request, originResponse, resource); - return store( - host, - request, - originResponse, - requestSent, - responseReceived, - rootKey, - entry, - callback); + final String variantKey = cacheKeyGenerator.generateVariantKey(request, entry); + return store(rootKey,variantKey, entry, callback); } @Override @@ -403,106 +429,56 @@ public Cancellable update( final Instant requestSent, final Instant responseReceived, final FutureCallback callback) { - final String entryKey = stale.getEntryKey(); if (LOG.isDebugEnabled()) { - LOG.debug("Update cache entry: {}", entryKey); + LOG.debug("Update cache entry: {}", stale.getEntryKey()); } final HttpCacheEntry updatedEntry = cacheEntryFactory.createUpdated( requestSent, responseReceived, originResponse, stale.entry); - - if (LOG.isDebugEnabled()) { - LOG.debug("Put entry in cache: {}", entryKey); - } - - return store( - host, - request, - originResponse, - requestSent, - responseReceived, - entryKey, - updatedEntry, - callback); + final String variantKey = cacheKeyGenerator.generateVariantKey(request, updatedEntry); + return store(stale.rootKey, variantKey, updatedEntry, callback); } @Override - public Cancellable update( - final CacheHit stale, + public Cancellable storeFromNegotiated( + final CacheHit negotiated, + final HttpHost host, + final HttpRequest request, final HttpResponse originResponse, final Instant requestSent, final Instant responseReceived, final FutureCallback callback) { - final String entryKey = stale.getEntryKey(); if (LOG.isDebugEnabled()) { - LOG.debug("Update cache entry (no root): {}", entryKey); + LOG.debug("Update negotiated cache entry: {}", negotiated.getEntryKey()); } final HttpCacheEntry updatedEntry = cacheEntryFactory.createUpdated( requestSent, responseReceived, originResponse, - stale.entry); - return storeEntry( - entryKey, - updatedEntry, - callback); - } - - @Override - public Cancellable storeReusing( - final CacheHit hit, - final HttpHost host, - final HttpRequest request, - final HttpResponse originResponse, - final Instant requestSent, - final Instant responseReceived, - final FutureCallback callback) { - final String rootKey = cacheKeyGenerator.generateKey(host, request); - if (LOG.isDebugEnabled()) { - LOG.debug("Store cache entry using existing entry: {} -> {}", rootKey, hit.rootKey); - } - return store(host, request, originResponse, requestSent, responseReceived, rootKey, hit.entry, callback); - } - - private void evictEntry(final String cacheKey) { - storage.removeEntry(cacheKey, new FutureCallback() { + negotiated.entry); - @Override - public void completed(final Boolean result) { - } + storeInternal(negotiated.getEntryKey(), updatedEntry, null); - @Override - public void failed(final Exception ex) { - if (LOG.isWarnEnabled()) { - if (ex instanceof ResourceIOException) { - LOG.warn("I/O error removing cache entry with key {}", cacheKey); - } else { - LOG.warn("Unexpected error removing cache entry with key {}", cacheKey, ex); - } - } - } - - @Override - public void cancelled() { - } - - }); + final String rootKey = cacheKeyGenerator.generateKey(host, request); + final HttpCacheEntry copy = cacheEntryFactory.copy(updatedEntry); + final String variantKey = cacheKeyGenerator.generateVariantKey(request, copy); + return store(rootKey, variantKey, copy, callback); } private void evictAll(final HttpCacheEntry root, final String rootKey) { if (LOG.isDebugEnabled()) { LOG.debug("Evicting root cache entry {}", rootKey); } - evictEntry(rootKey); + removeInternal(rootKey); if (root.hasVariants()) { for (final String variantKey : root.getVariants()) { final String variantEntryKey = variantKey + rootKey; if (LOG.isDebugEnabled()) { LOG.debug("Evicting variant cache entry {}", variantEntryKey); } - evictEntry(variantEntryKey); + removeInternal(variantEntryKey); } } } 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 aa56600ae..360a014ba 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 @@ -190,53 +190,33 @@ public List getVariants(final CacheHit hit) { return Collections.emptyList(); } - CacheHit store( - final HttpHost host, - final HttpRequest request, - final HttpResponse originResponse, - final Instant requestSent, - final Instant responseReceived, - final String rootKey, - final HttpCacheEntry entry) { - if (entry.containsHeader(HttpHeaders.VARY)) { - return storeVariant(host, request, originResponse, requestSent, responseReceived, rootKey, entry); - } else { + CacheHit store(final String rootKey, final String variantKey, final HttpCacheEntry entry) { + if (variantKey == null) { if (LOG.isDebugEnabled()) { - LOG.debug("Put entry in cache: {}", rootKey); + LOG.debug("Store entry in cache: {}", rootKey); } storeInternal(rootKey, entry); return new CacheHit(rootKey, entry); - } - } + } else { + final String variantCacheKey = variantKey + rootKey; - CacheHit storeVariant( - final HttpHost host, - final HttpRequest request, - final HttpResponse originResponse, - final Instant requestSent, - final Instant responseReceived, - final String rootKey, - final HttpCacheEntry entry) { - final List variantNames = CacheKeyGenerator.variantNames(entry); - final String variantKey = cacheKeyGenerator.generateVariantKey(request, variantNames); - final String variantCacheKey = variantKey + rootKey; + if (LOG.isDebugEnabled()) { + LOG.debug("Store variant entry in cache: {}", variantCacheKey); + } - if (LOG.isDebugEnabled()) { - LOG.debug("Put variant entry in cache: {}", variantCacheKey); - } + storeInternal(variantCacheKey, entry); - storeInternal(variantCacheKey, entry); + if (LOG.isDebugEnabled()) { + LOG.debug("Update root entry: {}", rootKey); + } - if (LOG.isDebugEnabled()) { - LOG.debug("Update root entry: {}", rootKey); + updateInternal(rootKey, existing -> { + final Set variants = existing != null ? new HashSet<>(existing.getVariants()) : new HashSet<>(); + variants.add(variantKey); + return cacheEntryFactory.createRoot(entry, variants); + }); + return new CacheHit(rootKey, variantCacheKey, entry); } - - updateInternal(rootKey, existing -> { - final Set variants = existing != null ? new HashSet<>(existing.getVariants()) : new HashSet<>(); - variants.add(variantKey); - return cacheEntryFactory.createRoot(requestSent, responseReceived, host, request, originResponse, variants); - }); - return new CacheHit(rootKey, variantCacheKey, entry); } @Override @@ -249,7 +229,7 @@ public CacheHit store( final Instant responseReceived) { final String rootKey = cacheKeyGenerator.generateKey(host, request); if (LOG.isDebugEnabled()) { - LOG.debug("Store cache entry: {}", rootKey); + LOG.debug("Create cache entry: {}", rootKey); } final Resource resource; try { @@ -268,7 +248,8 @@ public CacheHit store( return new CacheHit(rootKey, backup); } final HttpCacheEntry entry = cacheEntryFactory.create(requestSent, responseReceived, host, request, originResponse, resource); - return store(host, request, originResponse, requestSent, responseReceived, rootKey, entry); + final String variantKey = cacheKeyGenerator.generateVariantKey(request, entry); + return store(rootKey,variantKey, entry); } @Override @@ -279,55 +260,40 @@ public CacheHit update( final HttpResponse originResponse, final Instant requestSent, final Instant responseReceived) { - final String entryKey = stale.getEntryKey(); if (LOG.isDebugEnabled()) { - LOG.debug("Update cache entry: {}", entryKey); + LOG.debug("Update cache entry: {}", stale.getEntryKey()); } final HttpCacheEntry updatedEntry = cacheEntryFactory.createUpdated( requestSent, responseReceived, originResponse, stale.entry); - return store(host, request, originResponse, requestSent, responseReceived, entryKey, updatedEntry); + final String variantKey = cacheKeyGenerator.generateVariantKey(request, updatedEntry); + return store(stale.rootKey, variantKey, updatedEntry); } @Override - public CacheHit update( - final CacheHit stale, + public CacheHit storeFromNegotiated( + final CacheHit negotiated, + final HttpHost host, + final HttpRequest request, final HttpResponse originResponse, final Instant requestSent, final Instant responseReceived) { - final String entryKey = stale.getEntryKey(); if (LOG.isDebugEnabled()) { - LOG.debug("Update cache entry (no root)): {}", entryKey); + LOG.debug("Update negotiated cache entry: {}", negotiated.getEntryKey()); } final HttpCacheEntry updatedEntry = cacheEntryFactory.createUpdated( requestSent, responseReceived, originResponse, - stale.entry); + negotiated.entry); + storeInternal(negotiated.getEntryKey(), updatedEntry); - if (LOG.isDebugEnabled()) { - LOG.debug("Put entry in cache: {}", entryKey); - } - - storeInternal(entryKey, updatedEntry); - return new CacheHit(stale.rootKey, stale.variantKey, updatedEntry); - } - - @Override - public CacheHit storeReusing( - final CacheHit hit, - final HttpHost host, - final HttpRequest request, - final HttpResponse originResponse, - final Instant requestSent, - final Instant responseReceived) { final String rootKey = cacheKeyGenerator.generateKey(host, request); - if (LOG.isDebugEnabled()) { - LOG.debug("Store cache entry using existing entry: {} -> {}", rootKey, hit.rootKey); - } - return store(host, request, originResponse, requestSent, responseReceived, rootKey, hit.entry); + final HttpCacheEntry copy = cacheEntryFactory.copy(updatedEntry); + final String variantKey = cacheKeyGenerator.generateVariantKey(request, copy); + return store(rootKey, variantKey, copy); } private void evictAll(final HttpCacheEntry root, final String rootKey) { diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheKeyGenerator.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheKeyGenerator.java index ca7c3cb16..2eb773128 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheKeyGenerator.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheKeyGenerator.java @@ -156,6 +156,23 @@ public String generateVariantKey(final HttpRequest request, final Collection variantNames = variantNames(entry); + return generateVariantKey(request, variantNames); + } else { + return null; + } + } + /** * Computes a key for the given {@link HttpHost} and {@link HttpRequest} * that can be used as a unique identifier for cached resources. if the request has a @@ -179,20 +196,4 @@ public String generateKey(final HttpHost host, final HttpRequest request, final } } - /** - * Computes a "variant key" from the headers of a given request that are - * covered by the Vary header of a given cache entry. Any request whose - * varying headers match those of this request should have the same - * variant key. - * @param req originating request - * @param entry cache entry in question that has variants - * @return variant key - * - * @deprecated Use {@link #generateVariantKey(HttpRequest, Collection)}. - */ - @Deprecated - public String generateVariantKey(final HttpRequest req, final HttpCacheEntry entry) { - return generateVariantKey(req, variantNames(entry)); - } - } 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 12b013e94..2cfc51459 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 @@ -520,13 +520,12 @@ ClassicHttpResponse negotiateResponseFromVariants( recordCacheUpdate(scope.clientContext); - final CacheHit hit = responseCache.update(match, backendResponse, requestDate, responseDate); + final CacheHit hit = responseCache.storeFromNegotiated(match, target, request, backendResponse, requestDate, responseDate); if (shouldSendNotModifiedResponse(request, hit.entry)) { return convert(responseGenerator.generateNotModifiedResponse(hit.entry), scope); + } else { + return convert(responseGenerator.generateResponse(request, hit.entry), scope); } - final SimpleHttpResponse response = responseGenerator.generateResponse(request, hit.entry); - responseCache.storeReusing(hit, target, request, backendResponse, requestDate, responseDate); - return convert(response, scope); } catch (final IOException | RuntimeException ex) { backendResponse.close(); throw ex; diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/HttpAsyncCache.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/HttpAsyncCache.java index 55fb87929..6dd341cda 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/HttpAsyncCache.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/HttpAsyncCache.java @@ -76,22 +76,12 @@ Cancellable update( Instant responseReceived, FutureCallback callback); - /** - * Updates {@link HttpCacheEntry} using details from a 304 {@link HttpResponse}. - */ - Cancellable update( - CacheHit stale, - HttpResponse originResponse, - Instant requestSent, - Instant responseReceived, - FutureCallback callback); - /** * Stores {@link HttpRequest} / {@link HttpResponse} exchange details in the cache - * re-using the resource of the existing {@link HttpCacheEntry}. + * from the negotiated {@link HttpCacheEntry}. */ - Cancellable storeReusing( - CacheHit hit, + Cancellable storeFromNegotiated( + CacheHit negotiated, HttpHost host, HttpRequest request, HttpResponse originResponse, 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 1529d95e3..7160ccdb3 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 @@ -71,21 +71,12 @@ CacheHit update( Instant requestSent, Instant responseReceived); - /** - * Updates {@link HttpCacheEntry} using details from a 304 {@link HttpResponse}. - */ - CacheHit update( - CacheHit stale, - HttpResponse originResponse, - Instant requestSent, - Instant responseReceived); - /** * Stores {@link HttpRequest} / {@link HttpResponse} exchange details in the cache - * re-using the resource of the existing {@link HttpCacheEntry}. + * from the negotiated {@link HttpCacheEntry}. */ - CacheHit storeReusing( - CacheHit hit, + CacheHit storeFromNegotiated( + CacheHit negotiated, HttpHost host, HttpRequest request, HttpResponse originResponse, diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/cache/TestHttpCacheEntryFactory.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/cache/TestHttpCacheEntryFactory.java index d80871257..24adff6ea 100644 --- a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/cache/TestHttpCacheEntryFactory.java +++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/cache/TestHttpCacheEntryFactory.java @@ -233,14 +233,16 @@ public void testCreateRootVariantEntry() { new BasicHeader("X-custom", "my stuff") ); + final HttpCacheEntry newEntry = impl.create(tenSecondsAgo, oneSecondAgo, host, request, response, HttpTestUtils.makeRandomResource(1024)); + final Set variants = new HashSet<>(); variants.add("variant1"); variants.add("variant2"); variants.add("variant3"); - final HttpCacheEntry newEntry = impl.createRoot(tenSecondsAgo, oneSecondAgo, host, request, response, variants); + final HttpCacheEntry newRoot = impl.createRoot(newEntry, variants); - MatcherAssert.assertThat(newEntry, HttpCacheEntryMatcher.equivalent( + MatcherAssert.assertThat(newRoot, HttpCacheEntryMatcher.equivalent( HttpTestUtils.makeCacheEntry( tenSecondsAgo, oneSecondAgo, @@ -261,8 +263,8 @@ public void testCreateRootVariantEntry() { variants ))); - Assertions.assertTrue(newEntry.hasVariants()); - Assertions.assertNull(newEntry.getResource()); + Assertions.assertTrue(newRoot.hasVariants()); + Assertions.assertNull(newRoot.getResource()); } @Test 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 6ce227949..c587856ef 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 @@ -33,6 +33,7 @@ import java.util.Objects; import java.util.Random; import java.util.concurrent.CountDownLatch; +import java.util.function.Consumer; import org.apache.hc.client5.http.cache.HttpCacheEntry; import org.apache.hc.client5.http.cache.HttpCacheEntryFactory; @@ -372,11 +373,14 @@ public static ClassicHttpResponse make500Response() { return new BasicClassicHttpResponse(HttpStatus.SC_INTERNAL_SERVER_ERROR, "Internal Server Error"); } - public static FutureCallback countDown(final CountDownLatch latch) { + public static FutureCallback countDown(final CountDownLatch latch, final Consumer consumer) { return new FutureCallback() { @Override public void completed(final T result) { + if (consumer != null) { + consumer.accept(result); + } latch.countDown(); } @@ -396,4 +400,8 @@ public void cancelled() { } + public static FutureCallback countDown(final CountDownLatch latch) { + return countDown(latch, null); + } + } diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestBasicHttpAsyncCache.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestBasicHttpAsyncCache.java index e8fc30e4e..f8632e65d 100644 --- a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestBasicHttpAsyncCache.java +++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestBasicHttpAsyncCache.java @@ -26,22 +26,39 @@ */ package org.apache.hc.client5.http.impl.cache; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; import java.net.URI; import java.time.Instant; +import java.util.Collection; import java.util.HashSet; +import java.util.Map; import java.util.Set; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.atomic.AtomicReference; +import java.util.stream.Collectors; +import org.apache.hc.client5.http.HeadersMatcher; +import org.apache.hc.client5.http.cache.HttpCacheEntry; +import org.apache.hc.client5.http.classic.methods.HttpGet; import org.apache.hc.client5.http.utils.DateUtils; +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.message.BasicHeader; import org.apache.hc.core5.http.message.BasicHttpRequest; +import org.apache.hc.core5.http.message.BasicHttpResponse; import org.apache.hc.core5.net.URIBuilder; +import org.apache.hc.core5.util.ByteArrayBuffer; +import org.hamcrest.MatcherAssert; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -52,7 +69,7 @@ public class TestBasicHttpAsyncCache { private HttpHost host; private Instant now; private Instant tenSecondsAgo; - private SimpleHttpAsyncCacheStorage mockStorage; + private SimpleHttpAsyncCacheStorage backing; private BasicHttpAsyncCache impl; @BeforeEach @@ -60,11 +77,480 @@ public void setUp() { host = new HttpHost("foo.example.com"); now = Instant.now(); tenSecondsAgo = now.minusSeconds(10); - mockStorage = Mockito.spy(new SimpleHttpAsyncCacheStorage()); - impl = new BasicHttpAsyncCache(HeapResourceFactory.INSTANCE, mockStorage); + backing = Mockito.spy(new SimpleHttpAsyncCacheStorage()); + impl = new BasicHttpAsyncCache(HeapResourceFactory.INSTANCE, backing); + } + + @Test + public void testGetCacheEntryReturnsNullOnCacheMiss() throws Exception { + final HttpHost host = new HttpHost("foo.example.com"); + final HttpRequest request = new HttpGet("http://foo.example.com/bar"); + + final CountDownLatch latch1 = new CountDownLatch(1); + final AtomicReference resultRef = new AtomicReference<>(); + + impl.match(host, request, HttpTestUtils.countDown(latch1, resultRef::set)); + + latch1.await(); + + assertNull(resultRef.get()); + } + + @Test + public void testGetCacheEntryFetchesFromCacheOnCacheHitIfNoVariants() throws Exception { + final HttpCacheEntry entry = HttpTestUtils.makeCacheEntry(); + assertFalse(entry.hasVariants()); + final HttpHost host = new HttpHost("foo.example.com"); + final HttpRequest request = new HttpGet("http://foo.example.com/bar"); + + final String key = CacheKeyGenerator.INSTANCE.generateKey(host, request); + + backing.map.put(key,entry); + + final CountDownLatch latch1 = new CountDownLatch(1); + final AtomicReference resultRef = new AtomicReference<>(); + + impl.match(host, request, HttpTestUtils.countDown(latch1, resultRef::set)); + + latch1.await(); + final CacheMatch result = resultRef.get(); + + assertNotNull(result); + assertNotNull(result.hit); + assertSame(entry, result.hit.entry); + } + + @Test + public void testGetCacheEntryReturnsNullIfNoVariantInCache() throws Exception { + final HttpRequest origRequest = new HttpGet("http://foo.example.com/bar"); + origRequest.setHeader("Accept-Encoding","gzip"); + + final ByteArrayBuffer buf = HttpTestUtils.makeRandomBuffer(128); + final HttpResponse origResponse = new BasicHttpResponse(HttpStatus.SC_OK, "OK"); + origResponse.setHeader("Date", DateUtils.formatStandardDate(now)); + origResponse.setHeader("Cache-Control", "max-age=3600, public"); + origResponse.setHeader("ETag", "\"etag\""); + origResponse.setHeader("Vary", "Accept-Encoding"); + origResponse.setHeader("Content-Encoding","gzip"); + + final CountDownLatch latch1 = new CountDownLatch(1); + + impl.store(host, origRequest, origResponse, buf, now, now, HttpTestUtils.countDown(latch1)); + + latch1.await(); + + final HttpRequest request = new HttpGet("http://foo.example.com/bar"); + + final CountDownLatch latch2 = new CountDownLatch(1); + final AtomicReference resultRef = new AtomicReference<>(); + impl.match(host, request, HttpTestUtils.countDown(latch2, resultRef::set)); + + latch2.await(); + final CacheMatch result = resultRef.get(); + + assertNotNull(result); + assertNull(result.hit); + } + + @Test + public void testGetCacheEntryReturnsVariantIfPresentInCache() throws Exception { + final HttpRequest origRequest = new HttpGet("http://foo.example.com/bar"); + origRequest.setHeader("Accept-Encoding","gzip"); + + final ByteArrayBuffer buf = HttpTestUtils.makeRandomBuffer(128); + final HttpResponse origResponse = new BasicHttpResponse(HttpStatus.SC_OK, "OK"); + origResponse.setHeader("Date", DateUtils.formatStandardDate(now)); + origResponse.setHeader("Cache-Control", "max-age=3600, public"); + origResponse.setHeader("ETag", "\"etag\""); + origResponse.setHeader("Vary", "Accept-Encoding"); + origResponse.setHeader("Content-Encoding","gzip"); + + final CountDownLatch latch1 = new CountDownLatch(1); + + impl.store(host, origRequest, origResponse, buf, now, now, HttpTestUtils.countDown(latch1)); + + latch1.await(); + + final HttpRequest request = new HttpGet("http://foo.example.com/bar"); + request.setHeader("Accept-Encoding","gzip"); + + final CountDownLatch latch2 = new CountDownLatch(1); + final AtomicReference resultRef = new AtomicReference<>(); + impl.match(host, request, HttpTestUtils.countDown(latch2, resultRef::set)); + + latch2.await(); + final CacheMatch result = resultRef.get(); + + assertNotNull(result); + assertNotNull(result.hit); + } + + @Test + public void testGetCacheEntryReturnsVariantWithMostRecentDateHeader() throws Exception { + final HttpRequest origRequest = new HttpGet("http://foo.example.com/bar"); + origRequest.setHeader("Accept-Encoding", "gzip"); + + final ByteArrayBuffer buf = HttpTestUtils.makeRandomBuffer(128); + + // Create two response variants with different Date headers + final HttpResponse origResponse1 = new BasicHttpResponse(HttpStatus.SC_OK, "OK"); + origResponse1.setHeader(HttpHeaders.DATE, DateUtils.formatStandardDate(now.minusSeconds(3600))); + origResponse1.setHeader(HttpHeaders.CACHE_CONTROL, "max-age=3600, public"); + origResponse1.setHeader(HttpHeaders.ETAG, "\"etag1\""); + origResponse1.setHeader(HttpHeaders.VARY, "Accept-Encoding"); + + final HttpResponse origResponse2 = new BasicHttpResponse(HttpStatus.SC_OK, "OK"); + origResponse2.setHeader(HttpHeaders.DATE, DateUtils.formatStandardDate(now)); + origResponse2.setHeader(HttpHeaders.CACHE_CONTROL, "max-age=3600, public"); + origResponse2.setHeader(HttpHeaders.ETAG, "\"etag2\""); + origResponse2.setHeader(HttpHeaders.VARY, "Accept-Encoding"); + + // Store the two variants in cache + final CountDownLatch latch1 = new CountDownLatch(2); + + impl.store(host, origRequest, origResponse1, buf, now, now, HttpTestUtils.countDown(latch1)); + impl.store(host, origRequest, origResponse2, buf, now, now, HttpTestUtils.countDown(latch1)); + + latch1.await(); + + final HttpRequest request = new HttpGet("http://foo.example.com/bar"); + request.setHeader("Accept-Encoding", "gzip"); + + final CountDownLatch latch2 = new CountDownLatch(1); + final AtomicReference resultRef = new AtomicReference<>(); + impl.match(host, request, HttpTestUtils.countDown(latch2, resultRef::set)); + + latch2.await(); + final CacheMatch result = resultRef.get(); + + assertNotNull(result); + assertNotNull(result.hit); + final HttpCacheEntry entry = result.hit.entry; + assertNotNull(entry); + + // Retrieve the ETag header value from the original response and assert that + // the returned cache entry has the same ETag value + final String expectedEtag = origResponse2.getFirstHeader(HttpHeaders.ETAG).getValue(); + final String actualEtag = entry.getFirstHeader(HttpHeaders.ETAG).getValue(); + + assertEquals(expectedEtag, actualEtag); + } + + @Test + public void testGetVariantsRootNoVariants() throws Exception { + final HttpCacheEntry root = HttpTestUtils.makeCacheEntry(); + + final CountDownLatch latch1 = new CountDownLatch(1); + final AtomicReference> resultRef = new AtomicReference<>(); + impl.getVariants(new CacheHit("root-key", root), HttpTestUtils.countDown(latch1, resultRef::set)); + + latch1.await(); + final Collection variants = resultRef.get(); + + assertNotNull(variants); + assertEquals(0, variants.size()); + } + + @Test + public void testGetVariantsRootNonExistentVariants() throws Exception { + final Set varinats = new HashSet<>(); + varinats.add("variant1"); + varinats.add("variant2"); + final HttpCacheEntry root = HttpTestUtils.makeCacheEntry(varinats); + + final CountDownLatch latch1 = new CountDownLatch(1); + final AtomicReference> resultRef = new AtomicReference<>(); + impl.getVariants(new CacheHit("root-key", root), HttpTestUtils.countDown(latch1, resultRef::set)); + + latch1.await(); + final Collection variants = resultRef.get(); + + assertNotNull(variants); + assertEquals(0, variants.size()); + } + + @Test + public void testGetVariantCacheEntriesReturnsAllVariants() throws Exception { + final HttpHost host = new HttpHost("foo.example.com"); + final URI uri = new URI("http://foo.example.com/bar"); + final HttpRequest req1 = new HttpGet(uri); + req1.setHeader("Accept-Encoding", "gzip"); + + final String rootKey = CacheKeyGenerator.INSTANCE.generateKey(uri); + + final HttpResponse resp1 = HttpTestUtils.make200Response(); + resp1.setHeader("Date", DateUtils.formatStandardDate(now)); + resp1.setHeader("Cache-Control", "max-age=3600, public"); + resp1.setHeader("ETag", "\"etag1\""); + resp1.setHeader("Vary", "Accept-Encoding"); + resp1.setHeader("Content-Encoding","gzip"); + + final HttpRequest req2 = new HttpGet(uri); + req2.setHeader("Accept-Encoding", "identity"); + + final HttpResponse resp2 = HttpTestUtils.make200Response(); + resp2.setHeader("Date", DateUtils.formatStandardDate(now)); + resp2.setHeader("Cache-Control", "max-age=3600, public"); + resp2.setHeader("ETag", "\"etag2\""); + resp2.setHeader("Vary", "Accept-Encoding"); + resp2.setHeader("Content-Encoding","gzip"); + + final CountDownLatch latch1 = new CountDownLatch(2); + + final AtomicReference resultRef1 = new AtomicReference<>(); + final AtomicReference resultRef2 = new AtomicReference<>(); + + impl.store(host, req1, resp1, null, now, now, HttpTestUtils.countDown(latch1, resultRef1::set)); + impl.store(host, req2, resp2, null, now, now, HttpTestUtils.countDown(latch1, resultRef2::set)); + + latch1.await(); + + final CacheHit hit1 = resultRef1.get(); + final CacheHit hit2 = resultRef2.get(); + + final Set variants = new HashSet<>(); + variants.add("{accept-encoding=gzip}"); + variants.add("{accept-encoding=identity}"); + + final CountDownLatch latch2 = new CountDownLatch(1); + final AtomicReference> resultRef3 = new AtomicReference<>(); + + impl.getVariants(new CacheHit(hit1.rootKey, HttpTestUtils.makeCacheEntry(variants)), + HttpTestUtils.countDown(latch2, resultRef3::set)); + + latch2.await(); + + final Map variantMap = resultRef3.get().stream() + .collect(Collectors.toMap(CacheHit::getEntryKey, e -> e.entry)); + + assertNotNull(variantMap); + assertEquals(2, variantMap.size()); + MatcherAssert.assertThat(variantMap.get("{accept-encoding=gzip}" + rootKey), + HttpCacheEntryMatcher.equivalent(hit1.entry)); + MatcherAssert.assertThat(variantMap.get("{accept-encoding=identity}" + rootKey), + HttpCacheEntryMatcher.equivalent(hit2.entry)); + } + + @Test + public void testUpdateCacheEntry() throws Exception { + final HttpHost host = new HttpHost("foo.example.com"); + final URI uri = new URI("http://foo.example.com/bar"); + final HttpRequest req1 = new HttpGet(uri); + + final HttpResponse resp1 = HttpTestUtils.make200Response(); + resp1.setHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)); + resp1.setHeader("Cache-Control", "max-age=3600, public"); + resp1.setHeader("ETag", "\"etag1\""); + resp1.setHeader("Content-Encoding","gzip"); + + final HttpRequest revalidate = new HttpGet(uri); + revalidate.setHeader("If-None-Match","\"etag1\""); + + final HttpResponse resp2 = HttpTestUtils.make304Response(); + resp2.setHeader("Date", DateUtils.formatStandardDate(now)); + resp2.setHeader("Cache-Control", "max-age=3600, public"); + + final CountDownLatch latch1 = new CountDownLatch(1); + + final AtomicReference resultRef1 = new AtomicReference<>(); + impl.store(host, req1, resp1, null, now, now, HttpTestUtils.countDown(latch1, resultRef1::set)); + + latch1.await(); + final CacheHit hit1 = resultRef1.get(); + + Assertions.assertNotNull(hit1); + Assertions.assertEquals(1, backing.map.size()); + Assertions.assertSame(hit1.entry, backing.map.get(hit1.getEntryKey())); + + final CountDownLatch latch2 = new CountDownLatch(1); + + final AtomicReference resultRef2 = new AtomicReference<>(); + impl.update(hit1, host, req1, resp2, now, now, HttpTestUtils.countDown(latch2, resultRef2::set)); + + latch2.await(); + final CacheHit updated = resultRef2.get(); + + Assertions.assertNotNull(updated); + Assertions.assertEquals(1, backing.map.size()); + Assertions.assertSame(updated.entry, backing.map.get(hit1.getEntryKey())); + + MatcherAssert.assertThat( + updated.entry.getHeaders(), + HeadersMatcher.same( + new BasicHeader("Server", "MockOrigin/1.0"), + new BasicHeader("ETag", "\"etag1\""), + new BasicHeader("Content-Encoding","gzip"), + new BasicHeader("Date", DateUtils.formatStandardDate(now)), + new BasicHeader("Cache-Control", "max-age=3600, public") + )); + } + + @Test + public void testUpdateVariantCacheEntry() throws Exception { + final HttpHost host = new HttpHost("foo.example.com"); + final URI uri = new URI("http://foo.example.com/bar"); + final HttpRequest req1 = new HttpGet(uri); + req1.setHeader("User-Agent", "agent1"); + + final HttpResponse resp1 = HttpTestUtils.make200Response(); + resp1.setHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)); + resp1.setHeader("Cache-Control", "max-age=3600, public"); + resp1.setHeader("ETag", "\"etag1\""); + resp1.setHeader("Content-Encoding","gzip"); + resp1.setHeader("Vary", "User-Agent"); + + final HttpRequest revalidate = new HttpGet(uri); + revalidate.setHeader("If-None-Match","\"etag1\""); + + final HttpResponse resp2 = HttpTestUtils.make304Response(); + resp2.setHeader("Date", DateUtils.formatStandardDate(now)); + resp2.setHeader("Cache-Control", "max-age=3600, public"); + + final CountDownLatch latch1 = new CountDownLatch(1); + + final AtomicReference resultRef1 = new AtomicReference<>(); + impl.store(host, req1, resp1, null, now, now, HttpTestUtils.countDown(latch1, resultRef1::set)); + + latch1.await(); + final CacheHit hit1 = resultRef1.get(); + + Assertions.assertNotNull(hit1); + Assertions.assertEquals(2, backing.map.size()); + Assertions.assertSame(hit1.entry, backing.map.get(hit1.getEntryKey())); + + final CountDownLatch latch2 = new CountDownLatch(1); + + final AtomicReference resultRef2 = new AtomicReference<>(); + impl.update(hit1, host, req1, resp2, now, now, HttpTestUtils.countDown(latch2, resultRef2::set)); + + latch2.await(); + final CacheHit updated = resultRef2.get(); + + Assertions.assertNotNull(updated); + Assertions.assertEquals(2, backing.map.size()); + Assertions.assertSame(updated.entry, backing.map.get(hit1.getEntryKey())); + + MatcherAssert.assertThat( + updated.entry.getHeaders(), + HeadersMatcher.same( + new BasicHeader("Server", "MockOrigin/1.0"), + new BasicHeader("ETag", "\"etag1\""), + new BasicHeader("Content-Encoding","gzip"), + new BasicHeader("Vary","User-Agent"), + new BasicHeader("Date", DateUtils.formatStandardDate(now)), + new BasicHeader("Cache-Control", "max-age=3600, public") + )); + } + + @Test + public void testUpdateCacheEntryTurnsVariant() throws Exception { + final HttpHost host = new HttpHost("foo.example.com"); + final URI uri = new URI("http://foo.example.com/bar"); + final HttpRequest req1 = new HttpGet(uri); + req1.setHeader("User-Agent", "agent1"); + + final HttpResponse resp1 = HttpTestUtils.make200Response(); + resp1.setHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)); + resp1.setHeader("Cache-Control", "max-age=3600, public"); + resp1.setHeader("ETag", "\"etag1\""); + resp1.setHeader("Content-Encoding","gzip"); + + final HttpRequest revalidate = new HttpGet(uri); + revalidate.setHeader("If-None-Match","\"etag1\""); + + final HttpResponse resp2 = HttpTestUtils.make304Response(); + resp2.setHeader("Date", DateUtils.formatStandardDate(now)); + resp2.setHeader("Cache-Control", "max-age=3600, public"); + resp2.setHeader("Vary", "User-Agent"); + + final CountDownLatch latch1 = new CountDownLatch(1); + + final AtomicReference resultRef1 = new AtomicReference<>(); + impl.store(host, req1, resp1, null, now, now, HttpTestUtils.countDown(latch1, resultRef1::set)); + + latch1.await(); + final CacheHit hit1 = resultRef1.get(); + + Assertions.assertNotNull(hit1); + Assertions.assertEquals(1, backing.map.size()); + Assertions.assertSame(hit1.entry, backing.map.get(hit1.getEntryKey())); + + final CountDownLatch latch2 = new CountDownLatch(1); + + final AtomicReference resultRef2 = new AtomicReference<>(); + impl.update(hit1, host, req1, resp2, now, now, HttpTestUtils.countDown(latch2, resultRef2::set)); + + latch2.await(); + final CacheHit updated = resultRef2.get(); + + Assertions.assertNotNull(updated); + Assertions.assertEquals(2, backing.map.size()); + + MatcherAssert.assertThat( + updated.entry.getHeaders(), + HeadersMatcher.same( + new BasicHeader("Server", "MockOrigin/1.0"), + new BasicHeader("ETag", "\"etag1\""), + new BasicHeader("Content-Encoding","gzip"), + new BasicHeader("Date", DateUtils.formatStandardDate(now)), + new BasicHeader("Cache-Control", "max-age=3600, public"), + new BasicHeader("Vary","User-Agent"))); + } + + @Test + public void testStoreFromNegotiatedVariant() throws Exception { + final HttpHost host = new HttpHost("foo.example.com"); + final URI uri = new URI("http://foo.example.com/bar"); + final HttpRequest req1 = new HttpGet(uri); + req1.setHeader("User-Agent", "agent1"); + + final HttpResponse resp1 = HttpTestUtils.make200Response(); + resp1.setHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)); + resp1.setHeader("Cache-Control", "max-age=3600, public"); + resp1.setHeader("ETag", "\"etag1\""); + resp1.setHeader("Content-Encoding","gzip"); + resp1.setHeader("Vary", "User-Agent"); + + final CountDownLatch latch1 = new CountDownLatch(1); + + final AtomicReference resultRef1 = new AtomicReference<>(); + impl.store(host, req1, resp1, null, now, now, HttpTestUtils.countDown(latch1, resultRef1::set)); + + latch1.await(); + final CacheHit hit1 = resultRef1.get(); + + Assertions.assertNotNull(hit1); + Assertions.assertEquals(2, backing.map.size()); + Assertions.assertSame(hit1.entry, backing.map.get(hit1.getEntryKey())); + + final HttpRequest req2 = new HttpGet(uri); + req2.setHeader("User-Agent", "agent2"); + + final HttpResponse resp2 = HttpTestUtils.make304Response(); + resp2.setHeader("Date", DateUtils.formatStandardDate(now)); + resp2.setHeader("Cache-Control", "max-age=3600, public"); + + final CountDownLatch latch2 = new CountDownLatch(1); + + final AtomicReference resultRef2 = new AtomicReference<>(); + impl.storeFromNegotiated(hit1, host, req2, resp2, now, now, HttpTestUtils.countDown(latch2, resultRef2::set)); + + final CacheHit hit2 = resultRef2.get(); + + Assertions.assertNotNull(hit2); + Assertions.assertEquals(3, backing.map.size()); + + MatcherAssert.assertThat( + hit2.entry.getHeaders(), + HeadersMatcher.same( + new BasicHeader("Server", "MockOrigin/1.0"), + new BasicHeader("ETag", "\"etag1\""), + new BasicHeader("Content-Encoding","gzip"), + new BasicHeader("Vary","User-Agent"), + new BasicHeader("Date", DateUtils.formatStandardDate(now)), + new BasicHeader("Cache-Control", "max-age=3600, public"))); } - // Tests @Test public void testInvalidatesUnsafeRequests() throws Exception { final HttpRequest request = new BasicHttpRequest("POST", "/path"); @@ -72,16 +558,16 @@ public void testInvalidatesUnsafeRequests() throws Exception { final String key = CacheKeyGenerator.INSTANCE.generateKey(host, request); - mockStorage.putEntry(key, HttpTestUtils.makeCacheEntry()); + backing.putEntry(key, HttpTestUtils.makeCacheEntry()); final CountDownLatch latch = new CountDownLatch(1); impl.evictInvalidatedEntries(host, request, response, HttpTestUtils.countDown(latch)); latch.await(); - verify(mockStorage).getEntry(Mockito.eq(key), Mockito.any()); - verify(mockStorage).removeEntry(Mockito.eq(key), Mockito.any()); - Assertions.assertNull(mockStorage.getEntry(key)); + verify(backing).getEntry(Mockito.eq(key), Mockito.any()); + verify(backing).removeEntry(Mockito.eq(key), Mockito.any()); + Assertions.assertNull(backing.getEntry(key)); } @Test @@ -94,7 +580,7 @@ public void testDoesNotInvalidateSafeRequests() throws Exception { latch1.await(); - verifyNoMoreInteractions(mockStorage); + verifyNoMoreInteractions(backing); final HttpRequest request2 = new BasicHttpRequest("HEAD", "/"); final HttpResponse response2 = HttpTestUtils.make200Response(); @@ -104,7 +590,7 @@ public void testDoesNotInvalidateSafeRequests() throws Exception { latch2.await(); - verifyNoMoreInteractions(mockStorage); + verifyNoMoreInteractions(backing); } @Test @@ -119,23 +605,23 @@ public void testInvalidatesUnsafeRequestsWithVariants() throws Exception { final HttpResponse response = HttpTestUtils.make200Response(); - mockStorage.putEntry(rootKey, HttpTestUtils.makeCacheEntry(variants)); - mockStorage.putEntry(variantKey1, HttpTestUtils.makeCacheEntry()); - mockStorage.putEntry(variantKey2, HttpTestUtils.makeCacheEntry()); + backing.putEntry(rootKey, HttpTestUtils.makeCacheEntry(variants)); + backing.putEntry(variantKey1, HttpTestUtils.makeCacheEntry()); + backing.putEntry(variantKey2, HttpTestUtils.makeCacheEntry()); final CountDownLatch latch = new CountDownLatch(1); impl.evictInvalidatedEntries(host, request, response, HttpTestUtils.countDown(latch)); latch.await(); - verify(mockStorage).getEntry(Mockito.eq(rootKey), Mockito.any()); - verify(mockStorage).removeEntry(Mockito.eq(rootKey), Mockito.any()); - verify(mockStorage).removeEntry(Mockito.eq(variantKey1), Mockito.any()); - verify(mockStorage).removeEntry(Mockito.eq(variantKey2), Mockito.any()); + verify(backing).getEntry(Mockito.eq(rootKey), Mockito.any()); + verify(backing).removeEntry(Mockito.eq(rootKey), Mockito.any()); + verify(backing).removeEntry(Mockito.eq(variantKey1), Mockito.any()); + verify(backing).removeEntry(Mockito.eq(variantKey2), Mockito.any()); - Assertions.assertNull(mockStorage.getEntry(rootKey)); - Assertions.assertNull(mockStorage.getEntry(variantKey1)); - Assertions.assertNull(mockStorage.getEntry(variantKey2)); + Assertions.assertNull(backing.getEntry(rootKey)); + Assertions.assertNull(backing.getEntry(variantKey1)); + Assertions.assertNull(backing.getEntry(variantKey2)); } @Test @@ -153,8 +639,8 @@ public void testInvalidateUriSpecifiedByContentLocationAndFresher() throws Excep response.setHeader("Date", DateUtils.formatStandardDate(now)); response.setHeader("Content-Location", contentUri.toASCIIString()); - mockStorage.putEntry(rootKey, HttpTestUtils.makeCacheEntry()); - mockStorage.putEntry(contentKey, HttpTestUtils.makeCacheEntry( + backing.putEntry(rootKey, HttpTestUtils.makeCacheEntry()); + backing.putEntry(contentKey, HttpTestUtils.makeCacheEntry( new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)), new BasicHeader("ETag", "\"old-etag\"") )); @@ -164,10 +650,10 @@ public void testInvalidateUriSpecifiedByContentLocationAndFresher() throws Excep latch.await(); - verify(mockStorage).getEntry(Mockito.eq(rootKey), Mockito.any()); - verify(mockStorage).removeEntry(Mockito.eq(rootKey), Mockito.any()); - verify(mockStorage).getEntry(Mockito.eq(contentKey), Mockito.any()); - verify(mockStorage).removeEntry(Mockito.eq(contentKey), Mockito.any()); + verify(backing).getEntry(Mockito.eq(rootKey), Mockito.any()); + verify(backing).removeEntry(Mockito.eq(rootKey), Mockito.any()); + verify(backing).getEntry(Mockito.eq(contentKey), Mockito.any()); + verify(backing).removeEntry(Mockito.eq(contentKey), Mockito.any()); } @Test @@ -185,8 +671,8 @@ public void testInvalidateUriSpecifiedByLocationAndFresher() throws Exception { response.setHeader("Date", DateUtils.formatStandardDate(now)); response.setHeader("Location", contentUri.toASCIIString()); - mockStorage.putEntry(rootKey, HttpTestUtils.makeCacheEntry()); - mockStorage.putEntry(contentKey, HttpTestUtils.makeCacheEntry( + backing.putEntry(rootKey, HttpTestUtils.makeCacheEntry()); + backing.putEntry(contentKey, HttpTestUtils.makeCacheEntry( new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)), new BasicHeader("ETag", "\"old-etag\"") )); @@ -196,10 +682,10 @@ public void testInvalidateUriSpecifiedByLocationAndFresher() throws Exception { latch.await(); - verify(mockStorage).getEntry(Mockito.eq(rootKey), Mockito.any()); - verify(mockStorage).removeEntry(Mockito.eq(rootKey), Mockito.any()); - verify(mockStorage).getEntry(Mockito.eq(contentKey), Mockito.any()); - verify(mockStorage).removeEntry(Mockito.eq(contentKey), Mockito.any()); + verify(backing).getEntry(Mockito.eq(rootKey), Mockito.any()); + verify(backing).removeEntry(Mockito.eq(rootKey), Mockito.any()); + verify(backing).getEntry(Mockito.eq(contentKey), Mockito.any()); + verify(backing).removeEntry(Mockito.eq(contentKey), Mockito.any()); } @Test @@ -219,7 +705,7 @@ public void testDoesNotInvalidateForUnsuccessfulResponse() throws Exception { latch.await(); - verifyNoMoreInteractions(mockStorage); + verifyNoMoreInteractions(backing); } @Test @@ -238,9 +724,9 @@ public void testInvalidateUriSpecifiedByContentLocationNonCanonical() throws Exc response.setHeader("Content-Location", contentUri.toASCIIString()); - mockStorage.putEntry(rootKey, HttpTestUtils.makeCacheEntry()); + backing.putEntry(rootKey, HttpTestUtils.makeCacheEntry()); - mockStorage.putEntry(contentKey, HttpTestUtils.makeCacheEntry( + backing.putEntry(contentKey, HttpTestUtils.makeCacheEntry( new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)), new BasicHeader("ETag", "\"old-etag\""))); @@ -249,12 +735,12 @@ public void testInvalidateUriSpecifiedByContentLocationNonCanonical() throws Exc latch.await(); - verify(mockStorage).getEntry(Mockito.eq(rootKey), Mockito.any()); - verify(mockStorage).removeEntry(Mockito.eq(rootKey), Mockito.any()); - verify(mockStorage).getEntry(Mockito.eq(contentKey), Mockito.any()); - verify(mockStorage).removeEntry(Mockito.eq(contentKey), Mockito.any()); - Assertions.assertNull(mockStorage.getEntry(rootKey)); - Assertions.assertNull(mockStorage.getEntry(contentKey)); + verify(backing).getEntry(Mockito.eq(rootKey), Mockito.any()); + verify(backing).removeEntry(Mockito.eq(rootKey), Mockito.any()); + verify(backing).getEntry(Mockito.eq(contentKey), Mockito.any()); + verify(backing).removeEntry(Mockito.eq(contentKey), Mockito.any()); + Assertions.assertNull(backing.getEntry(rootKey)); + Assertions.assertNull(backing.getEntry(contentKey)); } @Test @@ -273,9 +759,9 @@ public void testInvalidateUriSpecifiedByContentLocationRelative() throws Excepti response.setHeader("Content-Location", "/bar"); - mockStorage.putEntry(rootKey, HttpTestUtils.makeCacheEntry()); + backing.putEntry(rootKey, HttpTestUtils.makeCacheEntry()); - mockStorage.putEntry(contentKey, HttpTestUtils.makeCacheEntry( + backing.putEntry(contentKey, HttpTestUtils.makeCacheEntry( new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)), new BasicHeader("ETag", "\"old-etag\""))); @@ -284,12 +770,12 @@ public void testInvalidateUriSpecifiedByContentLocationRelative() throws Excepti latch.await(); - verify(mockStorage).getEntry(Mockito.eq(rootKey), Mockito.any()); - verify(mockStorage).removeEntry(Mockito.eq(rootKey), Mockito.any()); - verify(mockStorage).getEntry(Mockito.eq(contentKey), Mockito.any()); - verify(mockStorage).removeEntry(Mockito.eq(contentKey), Mockito.any()); - Assertions.assertNull(mockStorage.getEntry(rootKey)); - Assertions.assertNull(mockStorage.getEntry(contentKey)); + verify(backing).getEntry(Mockito.eq(rootKey), Mockito.any()); + verify(backing).removeEntry(Mockito.eq(rootKey), Mockito.any()); + verify(backing).getEntry(Mockito.eq(contentKey), Mockito.any()); + verify(backing).removeEntry(Mockito.eq(contentKey), Mockito.any()); + Assertions.assertNull(backing.getEntry(rootKey)); + Assertions.assertNull(backing.getEntry(contentKey)); } @Test @@ -306,15 +792,15 @@ public void testDoesNotInvalidateUriSpecifiedByContentLocationOtherOrigin() thro response.setHeader("Date", DateUtils.formatStandardDate(now)); response.setHeader("Content-Location", contentUri.toASCIIString()); - mockStorage.putEntry(contentKey, HttpTestUtils.makeCacheEntry()); + backing.putEntry(contentKey, HttpTestUtils.makeCacheEntry()); final CountDownLatch latch = new CountDownLatch(1); impl.evictInvalidatedEntries(host, request, response, HttpTestUtils.countDown(latch)); latch.await(); - verify(mockStorage, Mockito.never()).getEntry(contentKey); - verify(mockStorage, Mockito.never()).removeEntry(Mockito.eq(contentKey), Mockito.any()); + verify(backing, Mockito.never()).getEntry(contentKey); + verify(backing, Mockito.never()).removeEntry(Mockito.eq(contentKey), Mockito.any()); } @Test @@ -331,7 +817,7 @@ public void testDoesNotInvalidateUriSpecifiedByContentLocationIfEtagsMatch() thr response.setHeader("Date", DateUtils.formatStandardDate(now)); response.setHeader("Content-Location", contentUri.toASCIIString()); - mockStorage.putEntry(contentKey, HttpTestUtils.makeCacheEntry( + backing.putEntry(contentKey, HttpTestUtils.makeCacheEntry( new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)), new BasicHeader("ETag", "\"same-etag\""))); @@ -340,8 +826,8 @@ public void testDoesNotInvalidateUriSpecifiedByContentLocationIfEtagsMatch() thr latch.await(); - verify(mockStorage).getEntry(Mockito.eq(contentKey), Mockito.any()); - verify(mockStorage, Mockito.never()).removeEntry(Mockito.eq(contentKey), Mockito.any()); + verify(backing).getEntry(Mockito.eq(contentKey), Mockito.any()); + verify(backing, Mockito.never()).removeEntry(Mockito.eq(contentKey), Mockito.any()); } @Test @@ -358,7 +844,7 @@ public void testDoesNotInvalidateUriSpecifiedByContentLocationIfOlder() throws E response.setHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)); response.setHeader("Content-Location", contentUri.toASCIIString()); - mockStorage.putEntry(contentKey, HttpTestUtils.makeCacheEntry( + backing.putEntry(contentKey, HttpTestUtils.makeCacheEntry( new BasicHeader("Date", DateUtils.formatStandardDate(now)), new BasicHeader("ETag", "\"old-etag\""))); @@ -367,8 +853,8 @@ public void testDoesNotInvalidateUriSpecifiedByContentLocationIfOlder() throws E latch.await(); - verify(mockStorage).getEntry(Mockito.eq(contentKey), Mockito.any()); - verify(mockStorage, Mockito.never()).removeEntry(Mockito.eq(contentKey), Mockito.any()); + verify(backing).getEntry(Mockito.eq(contentKey), Mockito.any()); + verify(backing, Mockito.never()).removeEntry(Mockito.eq(contentKey), Mockito.any()); } @Test @@ -385,7 +871,7 @@ public void testDoesNotInvalidateUriSpecifiedByContentLocationIfResponseHasNoEta response.setHeader("Date", DateUtils.formatStandardDate(now)); response.setHeader("Content-Location", contentUri.toASCIIString()); - mockStorage.putEntry(contentKey, HttpTestUtils.makeCacheEntry( + backing.putEntry(contentKey, HttpTestUtils.makeCacheEntry( new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)), new BasicHeader("ETag", "\"old-etag\""))); @@ -394,8 +880,8 @@ public void testDoesNotInvalidateUriSpecifiedByContentLocationIfResponseHasNoEta latch.await(); - verify(mockStorage).getEntry(Mockito.eq(contentKey), Mockito.any()); - verify(mockStorage, Mockito.never()).removeEntry(Mockito.eq(contentKey), Mockito.any()); + verify(backing).getEntry(Mockito.eq(contentKey), Mockito.any()); + verify(backing, Mockito.never()).removeEntry(Mockito.eq(contentKey), Mockito.any()); } @Test @@ -412,7 +898,7 @@ public void testDoesNotInvalidateUriSpecifiedByContentLocationIfEntryHasNoEtag() response.setHeader("Date", DateUtils.formatStandardDate(now)); response.setHeader("Content-Location", contentUri.toASCIIString()); - mockStorage.putEntry(contentKey, HttpTestUtils.makeCacheEntry( + backing.putEntry(contentKey, HttpTestUtils.makeCacheEntry( new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)))); final CountDownLatch latch = new CountDownLatch(1); @@ -420,8 +906,8 @@ public void testDoesNotInvalidateUriSpecifiedByContentLocationIfEntryHasNoEtag() latch.await(); - verify(mockStorage).getEntry(Mockito.eq(contentKey), Mockito.any()); - verify(mockStorage, Mockito.never()).removeEntry(Mockito.eq(contentKey), Mockito.any()); + verify(backing).getEntry(Mockito.eq(contentKey), Mockito.any()); + verify(backing, Mockito.never()).removeEntry(Mockito.eq(contentKey), Mockito.any()); } @Test @@ -438,7 +924,7 @@ public void testInvalidatesUriSpecifiedByContentLocationIfResponseHasNoDate() th response.removeHeaders("Date"); response.setHeader("Content-Location", contentUri.toASCIIString()); - mockStorage.putEntry(contentKey, HttpTestUtils.makeCacheEntry( + backing.putEntry(contentKey, HttpTestUtils.makeCacheEntry( new BasicHeader("ETag", "\"old-etag\""), new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)))); @@ -447,8 +933,8 @@ public void testInvalidatesUriSpecifiedByContentLocationIfResponseHasNoDate() th latch.await(); - verify(mockStorage).getEntry(Mockito.eq(contentKey), Mockito.any()); - verify(mockStorage).removeEntry(Mockito.eq(contentKey), Mockito.any()); + verify(backing).getEntry(Mockito.eq(contentKey), Mockito.any()); + verify(backing).removeEntry(Mockito.eq(contentKey), Mockito.any()); } @Test @@ -465,7 +951,7 @@ public void testInvalidatesUriSpecifiedByContentLocationIfEntryHasNoDate() throw response.setHeader("Date", DateUtils.formatStandardDate(now)); response.setHeader("Content-Location", contentUri.toASCIIString()); - mockStorage.putEntry(contentKey, HttpTestUtils.makeCacheEntry( + backing.putEntry(contentKey, HttpTestUtils.makeCacheEntry( new BasicHeader("ETag", "\"old-etag\""))); final CountDownLatch latch = new CountDownLatch(1); @@ -473,8 +959,8 @@ public void testInvalidatesUriSpecifiedByContentLocationIfEntryHasNoDate() throw latch.await(); - verify(mockStorage).getEntry(Mockito.eq(contentKey), Mockito.any()); - verify(mockStorage).removeEntry(Mockito.eq(contentKey), Mockito.any()); + verify(backing).getEntry(Mockito.eq(contentKey), Mockito.any()); + verify(backing).removeEntry(Mockito.eq(contentKey), Mockito.any()); } @Test @@ -491,7 +977,7 @@ public void testInvalidatesUriSpecifiedByContentLocationIfResponseHasMalformedDa response.setHeader("Date", "huh?"); response.setHeader("Content-Location", contentUri.toASCIIString()); - mockStorage.putEntry(contentKey, HttpTestUtils.makeCacheEntry( + backing.putEntry(contentKey, HttpTestUtils.makeCacheEntry( new BasicHeader("ETag", "\"old-etag\""), new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)))); @@ -500,8 +986,8 @@ public void testInvalidatesUriSpecifiedByContentLocationIfResponseHasMalformedDa latch.await(); - verify(mockStorage).getEntry(Mockito.eq(contentKey), Mockito.any()); - verify(mockStorage).removeEntry(Mockito.eq(contentKey), Mockito.any()); + verify(backing).getEntry(Mockito.eq(contentKey), Mockito.any()); + verify(backing).removeEntry(Mockito.eq(contentKey), Mockito.any()); } @Test @@ -518,7 +1004,7 @@ public void testInvalidatesUriSpecifiedByContentLocationIfEntryHasMalformedDate( response.setHeader("Date", DateUtils.formatStandardDate(now)); response.setHeader("Content-Location", contentUri.toASCIIString()); - mockStorage.putEntry(contentKey, HttpTestUtils.makeCacheEntry( + backing.putEntry(contentKey, HttpTestUtils.makeCacheEntry( new BasicHeader("ETag", "\"old-etag\""), new BasicHeader("Date", "huh?"))); @@ -527,8 +1013,8 @@ public void testInvalidatesUriSpecifiedByContentLocationIfEntryHasMalformedDate( latch.await(); - verify(mockStorage).getEntry(Mockito.eq(contentKey), Mockito.any()); - verify(mockStorage).removeEntry(Mockito.eq(contentKey), Mockito.any()); + verify(backing).getEntry(Mockito.eq(contentKey), Mockito.any()); + verify(backing).removeEntry(Mockito.eq(contentKey), Mockito.any()); } } 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 b1c86b3c0..359188273 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 @@ -43,6 +43,7 @@ import java.util.Set; import java.util.stream.Collectors; +import org.apache.hc.client5.http.HeadersMatcher; import org.apache.hc.client5.http.cache.HttpCacheEntry; import org.apache.hc.client5.http.classic.methods.HttpGet; import org.apache.hc.client5.http.utils.DateUtils; @@ -79,20 +80,6 @@ public void setUp() throws Exception { impl = new BasicHttpCache(new HeapResourceFactory(), backing); } - @Test - public void testStoreInCachePutsNonVariantEntryInPlace() throws Exception { - final HttpCacheEntry entry = HttpTestUtils.makeCacheEntry(); - assertFalse(entry.hasVariants()); - final HttpHost host = new HttpHost("foo.example.com"); - final HttpRequest req = new HttpGet("http://foo.example.com/bar"); - final HttpResponse resp = HttpTestUtils.make200Response(); - - final String key = CacheKeyGenerator.INSTANCE.generateKey(host, req); - - impl.store(host, req, resp, now, now, key, entry); - assertSame(entry, backing.map.get(key)); - } - @Test public void testGetCacheEntryReturnsNullOnCacheMiss() throws Exception { final HttpHost host = new HttpHost("foo.example.com"); @@ -256,8 +243,8 @@ public void testGetVariantCacheEntriesReturnsAllVariants() throws Exception { variants.add("{accept-encoding=identity}"); final Map variantMap = impl.getVariants(new CacheHit(hit1.rootKey, - HttpTestUtils.makeCacheEntry(variants))).stream() - .collect(Collectors.toMap(CacheHit::getEntryKey, e -> e.entry)); + HttpTestUtils.makeCacheEntry(variants))).stream() + .collect(Collectors.toMap(CacheHit::getEntryKey, e -> e.entry)); assertNotNull(variantMap); assertEquals(2, variantMap.size()); @@ -267,6 +254,171 @@ public void testGetVariantCacheEntriesReturnsAllVariants() throws Exception { HttpCacheEntryMatcher.equivalent(hit2.entry)); } + @Test + public void testUpdateCacheEntry() throws Exception { + final HttpHost host = new HttpHost("foo.example.com"); + final URI uri = new URI("http://foo.example.com/bar"); + final HttpRequest req1 = new HttpGet(uri); + + final HttpResponse resp1 = HttpTestUtils.make200Response(); + resp1.setHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)); + resp1.setHeader("Cache-Control", "max-age=3600, public"); + resp1.setHeader("ETag", "\"etag1\""); + resp1.setHeader("Content-Encoding","gzip"); + + final HttpRequest revalidate = new HttpGet(uri); + revalidate.setHeader("If-None-Match","\"etag1\""); + + final HttpResponse resp2 = HttpTestUtils.make304Response(); + resp2.setHeader("Date", DateUtils.formatStandardDate(now)); + resp2.setHeader("Cache-Control", "max-age=3600, public"); + + final CacheHit hit1 = impl.store(host, req1, resp1, null, now, now); + Assertions.assertNotNull(hit1); + Assertions.assertEquals(1, backing.map.size()); + Assertions.assertSame(hit1.entry, backing.map.get(hit1.getEntryKey())); + + final CacheHit updated = impl.update(hit1, host, req1, resp2, now, now); + Assertions.assertNotNull(updated); + Assertions.assertEquals(1, backing.map.size()); + Assertions.assertSame(updated.entry, backing.map.get(hit1.getEntryKey())); + + MatcherAssert.assertThat( + updated.entry.getHeaders(), + HeadersMatcher.same( + new BasicHeader("Server", "MockOrigin/1.0"), + new BasicHeader("ETag", "\"etag1\""), + new BasicHeader("Content-Encoding","gzip"), + new BasicHeader("Date", DateUtils.formatStandardDate(now)), + new BasicHeader("Cache-Control", "max-age=3600, public") + )); + } + + @Test + public void testUpdateVariantCacheEntry() throws Exception { + final HttpHost host = new HttpHost("foo.example.com"); + final URI uri = new URI("http://foo.example.com/bar"); + final HttpRequest req1 = new HttpGet(uri); + req1.setHeader("User-Agent", "agent1"); + + final HttpResponse resp1 = HttpTestUtils.make200Response(); + resp1.setHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)); + resp1.setHeader("Cache-Control", "max-age=3600, public"); + resp1.setHeader("ETag", "\"etag1\""); + resp1.setHeader("Content-Encoding","gzip"); + resp1.setHeader("Vary", "User-Agent"); + + final HttpRequest revalidate = new HttpGet(uri); + revalidate.setHeader("If-None-Match","\"etag1\""); + + final HttpResponse resp2 = HttpTestUtils.make304Response(); + resp2.setHeader("Date", DateUtils.formatStandardDate(now)); + resp2.setHeader("Cache-Control", "max-age=3600, public"); + + final CacheHit hit1 = impl.store(host, req1, resp1, null, now, now); + Assertions.assertNotNull(hit1); + Assertions.assertEquals(2, backing.map.size()); + Assertions.assertSame(hit1.entry, backing.map.get(hit1.getEntryKey())); + + final CacheHit updated = impl.update(hit1, host, req1, resp2, now, now); + Assertions.assertNotNull(updated); + Assertions.assertEquals(2, backing.map.size()); + Assertions.assertSame(updated.entry, backing.map.get(hit1.getEntryKey())); + + MatcherAssert.assertThat( + updated.entry.getHeaders(), + HeadersMatcher.same( + new BasicHeader("Server", "MockOrigin/1.0"), + new BasicHeader("ETag", "\"etag1\""), + new BasicHeader("Content-Encoding","gzip"), + new BasicHeader("Vary","User-Agent"), + new BasicHeader("Date", DateUtils.formatStandardDate(now)), + new BasicHeader("Cache-Control", "max-age=3600, public") + )); + } + + @Test + public void testUpdateCacheEntryTurnsVariant() throws Exception { + final HttpHost host = new HttpHost("foo.example.com"); + final URI uri = new URI("http://foo.example.com/bar"); + final HttpRequest req1 = new HttpGet(uri); + req1.setHeader("User-Agent", "agent1"); + + final HttpResponse resp1 = HttpTestUtils.make200Response(); + resp1.setHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)); + resp1.setHeader("Cache-Control", "max-age=3600, public"); + resp1.setHeader("ETag", "\"etag1\""); + resp1.setHeader("Content-Encoding","gzip"); + + final HttpRequest revalidate = new HttpGet(uri); + revalidate.setHeader("If-None-Match","\"etag1\""); + + final HttpResponse resp2 = HttpTestUtils.make304Response(); + resp2.setHeader("Date", DateUtils.formatStandardDate(now)); + resp2.setHeader("Cache-Control", "max-age=3600, public"); + resp2.setHeader("Vary", "User-Agent"); + + final CacheHit hit1 = impl.store(host, req1, resp1, null, now, now); + Assertions.assertNotNull(hit1); + Assertions.assertEquals(1, backing.map.size()); + Assertions.assertSame(hit1.entry, backing.map.get(hit1.getEntryKey())); + + final CacheHit updated = impl.update(hit1, host, req1, resp2, now, now); + Assertions.assertNotNull(updated); + Assertions.assertEquals(2, backing.map.size()); + + MatcherAssert.assertThat( + updated.entry.getHeaders(), + HeadersMatcher.same( + new BasicHeader("Server", "MockOrigin/1.0"), + new BasicHeader("ETag", "\"etag1\""), + new BasicHeader("Content-Encoding","gzip"), + new BasicHeader("Date", DateUtils.formatStandardDate(now)), + new BasicHeader("Cache-Control", "max-age=3600, public"), + new BasicHeader("Vary","User-Agent"))); + } + + @Test + public void testStoreFromNegotiatedVariant() throws Exception { + final HttpHost host = new HttpHost("foo.example.com"); + final URI uri = new URI("http://foo.example.com/bar"); + final HttpRequest req1 = new HttpGet(uri); + req1.setHeader("User-Agent", "agent1"); + + final HttpResponse resp1 = HttpTestUtils.make200Response(); + resp1.setHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)); + resp1.setHeader("Cache-Control", "max-age=3600, public"); + resp1.setHeader("ETag", "\"etag1\""); + resp1.setHeader("Content-Encoding","gzip"); + resp1.setHeader("Vary", "User-Agent"); + + final CacheHit hit1 = impl.store(host, req1, resp1, null, now, now); + Assertions.assertNotNull(hit1); + Assertions.assertEquals(2, backing.map.size()); + Assertions.assertSame(hit1.entry, backing.map.get(hit1.getEntryKey())); + + final HttpRequest req2 = new HttpGet(uri); + req2.setHeader("User-Agent", "agent2"); + + final HttpResponse resp2 = HttpTestUtils.make304Response(); + resp2.setHeader("Date", DateUtils.formatStandardDate(now)); + resp2.setHeader("Cache-Control", "max-age=3600, public"); + + final CacheHit hit2 = impl.storeFromNegotiated(hit1, host, req2, resp2, now, now); + Assertions.assertNotNull(hit2); + Assertions.assertEquals(3, backing.map.size()); + + MatcherAssert.assertThat( + hit2.entry.getHeaders(), + HeadersMatcher.same( + new BasicHeader("Server", "MockOrigin/1.0"), + new BasicHeader("ETag", "\"etag1\""), + new BasicHeader("Content-Encoding","gzip"), + new BasicHeader("Vary","User-Agent"), + new BasicHeader("Date", DateUtils.formatStandardDate(now)), + new BasicHeader("Cache-Control", "max-age=3600, public"))); + } + @Test public void testInvalidatesUnsafeRequests() throws Exception { final HttpRequest request = new BasicHttpRequest("POST","/path"); diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheKeyGenerator.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheKeyGenerator.java index 178b3e6a0..a9c808e3e 100644 --- a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheKeyGenerator.java +++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheKeyGenerator.java @@ -29,10 +29,12 @@ import java.util.Arrays; import java.util.Collections; +import org.apache.hc.client5.http.cache.HttpCacheEntry; import org.apache.hc.client5.http.classic.methods.HttpGet; 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.message.BasicHeader; import org.apache.hc.core5.http.message.BasicHttpRequest; import org.apache.hc.core5.http.support.BasicRequestBuilder; import org.junit.jupiter.api.Assertions; @@ -43,11 +45,9 @@ public class TestCacheKeyGenerator { private CacheKeyGenerator extractor; - private HttpHost defaultHost; @BeforeEach public void setUp() throws Exception { - defaultHost = new HttpHost("foo.example.com"); extractor = CacheKeyGenerator.INSTANCE; } @@ -311,4 +311,21 @@ public void testGetVariantKeyInputNoMatchingHeaders() { Assertions.assertEquals("{accept-encoding=&user-agent=}", extractor.generateVariantKey(request, Arrays.asList(HttpHeaders.ACCEPT_ENCODING, HttpHeaders.USER_AGENT))); } + + @Test + public void testGetVariantKeyFromCachedResponse() { + final HttpRequest request = BasicRequestBuilder.get("/blah") + .addHeader("User-Agent", "agent1") + .addHeader("Accept-Encoding", "text/plain") + .build(); + + final HttpCacheEntry entry1 = HttpTestUtils.makeCacheEntry(); + Assertions.assertNull(extractor.generateVariantKey(request, entry1)); + + final HttpCacheEntry entry2 = HttpTestUtils.makeCacheEntry( + new BasicHeader("Vary", "User-Agent, Accept-Encoding") + ); + Assertions.assertEquals("{accept-encoding=text%2Fplain&user-agent=agent1}", extractor.generateVariantKey(request, entry2)); + } + }