Skip to content

Commit

Permalink
HTTPCLIENT-2284: Cache entry representation improvements:
Browse files Browse the repository at this point in the history 
* request URI stored in cache is now normalized and is presently expected to be the same as the root key
* removed references to variant cache keys from the cache entry
* Variants in root entries are now represented as a set, not as a map
  • Loading branch information
@ok2c
ok2c committed Aug 22, 2023
1 parent 6bdd389 commit 05078cf
Show file tree
Hide file tree
Showing 19 changed files with 225 additions and 206 deletions.
35 changes: 18 additions & 17 deletions httpclient5-cache/src/main/java/org/apache/hc/client5/http/cache/HttpCacheEntry.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,15 @@

import java.io.Serializable;
import java.time.Instant;
import java.util.Collection;
import java.util.Collections;
import java.util.Date;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;

import org.apache.hc.client5.http.utils.DateUtils;
import org.apache.hc.core5.annotation.Contract;
Expand Down Expand Up @@ -69,7 +73,7 @@ public class HttpCacheEntry implements MessageHeaders, Serializable {
private final int status;
private final HeaderGroup responseHeaders;
private final Resource resource;
private final Map<String, String> variantMap;
private final Set<String> variants;

/**
* Internal constructor that makes no validation of the input parameters and makes
Expand All @@ -85,7 +89,7 @@ public HttpCacheEntry(
final int status,
final HeaderGroup responseHeaders,
final Resource resource,
final Map<String, String> variantMap) {
final Collection<String> variants) {
super();
this.requestDate = requestDate;
this.responseDate = responseDate;
Expand All @@ -95,7 +99,7 @@ public HttpCacheEntry(
this.status = status;
this.responseHeaders = responseHeaders;
this.resource = resource;
this.variantMap = variantMap != null ? new HashMap<>(variantMap) : null;
this.variants = variants != null ? Collections.unmodifiableSet(new HashSet<>(variants)) : null;
}

/**
Expand Down Expand Up @@ -164,7 +168,7 @@ public HttpCacheEntry(
this.responseHeaders = new HeaderGroup();
this.responseHeaders.setHeaders(responseHeaders);
this.resource = resource;
this.variantMap = variantMap != null ? new HashMap<>(variantMap) : null;
this.variants = variantMap != null ? Collections.unmodifiableSet(new HashSet<>(variantMap.keySet())) : null;
}

/**
Expand Down Expand Up @@ -353,30 +357,27 @@ public Resource getResource() {
* Indicates whether the origin response indicated the associated
* resource had variants (i.e. that the Vary header was set on the
* origin response).
* @return {@code true} if this cached response was a variant
*/
public boolean hasVariants() {
return containsHeader(HttpHeaders.VARY);
return variants != null;
}

/**
* Returns all known variants.
*
* @since 5.3
*/
public boolean isVariantRoot() {
return variantMap != null;
public Set<String> getVariants() {
return variants != null ? variants : Collections.emptySet();
}

/**
* Returns an index about where in the cache different variants for
* a given resource are stored. This maps "variant keys" to "cache keys",
* where the variant key is derived from the varying request headers,
* and the cache key is the location in the
* {@link HttpCacheStorage} where that
* particular variant is stored. The first variant returned is used as
* the "parent" entry to hold this index of the other variants.
* @deprecated No longer applicable. Use {@link #getVariants()} instead.
*/
@Deprecated
public Map<String, String> getVariantMap() {
return variantMap != null ? Collections.unmodifiableMap(variantMap) : Collections.emptyMap();
return variants != null ? variants.stream()
.collect(Collectors.toMap(e -> e, e -> e + requestURI)) : Collections.emptyMap();
}

/**
Expand Down Expand Up @@ -418,7 +419,7 @@ public String toString() {
", status=" + status +
", responseHeaders=" + responseHeaders +
", resource=" + resource +
", variantMap=" + variantMap +
", variants=" + variants +
'}';
}
}
33 changes: 25 additions & 8 deletions httpclient5-cache/src/main/java/org/apache/hc/client5/http/cache/HttpCacheEntryFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,17 @@
*/
package org.apache.hc.client5.http.cache;

import java.net.URI;
import java.time.Instant;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
import java.util.TreeSet;

import org.apache.hc.client5.http.impl.cache.CacheSupport;
import org.apache.hc.client5.http.impl.cache.DateSupport;
import org.apache.hc.client5.http.utils.DateUtils;
import org.apache.hc.core5.annotation.Contract;
Expand All @@ -43,6 +45,7 @@
import org.apache.hc.core5.http.Header;
import org.apache.hc.core5.http.HeaderElements;
import org.apache.hc.core5.http.HttpHeaders;
import org.apache.hc.core5.http.HttpHost;
import org.apache.hc.core5.http.HttpMessage;
import org.apache.hc.core5.http.HttpRequest;
import org.apache.hc.core5.http.HttpResponse;
Expand Down Expand Up @@ -161,67 +164,81 @@ static void ensureDate(final HeaderGroup headers, final Instant instant) {
}
}

static String normalizeRequestUri(final HttpHost host, final HttpRequest request) {
final String s = CacheSupport.getRequestUri(request, host);
final URI normalizeRequestUri = CacheSupport.normalize(s);
return normalizeRequestUri.toASCIIString();
}

/**
* 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 variantMap describing cache entries that are variants of this parent entry; this
* @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,
final Map<String, String> variantMap) {
final Collection<String> 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);
return new HttpCacheEntry(
requestInstant,
responseInstant,
request.getMethod(),
request.getRequestUri(),
requestUri,
requestHeaders,
response.getCode(),
responseHeaders,
null,
variantMap);
variants);
}

/**
* Create a new {@link HttpCacheEntry} with the given {@link Resource}.
*
* @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 response Origin response (a deep copy of this object is made)
* @param resource Resource representing origin response body
*/
public HttpCacheEntry create(final Instant requestInstant,
final Instant responseInstant,
final HttpHost host,
final HttpRequest request,
final HttpResponse response,
final Resource resource) {
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);
return new HttpCacheEntry(
requestInstant,
responseInstant,
request.getMethod(),
request.getRequestUri(),
requestUri,
requestHeaders,
response.getCode(),
responseHeaders,
Expand Down Expand Up @@ -282,7 +299,7 @@ public HttpCacheEntry copy(final HttpCacheEntry entry) {
entry.getStatus(),
headers(entry.headerIterator()),
entry.getResource(),
entry.isVariantRoot() ? new HashMap<>(entry.getVariantMap()) : null);
entry.hasVariants() ? new HashSet<>(entry.getVariants()) : null);
}

}
4 changes: 3 additions & 1 deletion httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/AsyncCachingExec.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -703,6 +703,7 @@ void triggerUpdatedCacheEntryResponse(final HttpResponse backendResponse, final
recordCacheUpdate(scope.clientContext);
operation.setDependency(responseCache.update(
hit,
target,
request,
backendResponse,
requestDate,
Expand Down Expand Up @@ -880,7 +881,7 @@ private void handleCacheMiss(
triggerResponse(cacheResponse, scope, asyncExecCallback);
}

if (partialMatch != null && partialMatch.entry.isVariantRoot()) {
if (partialMatch != null && partialMatch.entry.hasVariants()) {
operation.setDependency(responseCache.getVariants(
partialMatch,
new FutureCallback<Collection<CacheHit>>() {
Expand Down Expand Up @@ -1016,6 +1017,7 @@ public void completed(final CacheMatch result) {
}
responseCache.update(
hit,
target,
request,
backendResponse,
requestDate,
Expand Down
50 changes: 30 additions & 20 deletions ...lient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/BasicHttpAsyncCache.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
import java.time.Instant;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
Expand Down Expand Up @@ -104,14 +104,14 @@ public Cancellable match(final HttpHost host, final HttpRequest request, final F
@Override
public void completed(final HttpCacheEntry root) {
if (root != null) {
if (root.isVariantRoot()) {
if (root.hasVariants()) {
final List<String> variantNames = CacheKeyGenerator.variantNames(root);
final String variantKey = cacheKeyGenerator.generateVariantKey(request, variantNames);
final String cacheKey = root.getVariantMap().get(variantKey);
if (LOG.isDebugEnabled()) {
LOG.debug("Get cache variant entry: {}", cacheKey);
}
if (cacheKey != null) {
if (root.getVariants().contains(variantKey)) {
final String cacheKey = variantKey + rootKey;
if (LOG.isDebugEnabled()) {
LOG.debug("Get cache variant entry: {}", cacheKey);
}
complexCancellable.setDependency(storage.getEntry(
cacheKey,
new FutureCallback<HttpCacheEntry>() {
Expand Down Expand Up @@ -179,8 +179,11 @@ public Cancellable getVariants(
}
final ComplexCancellable complexCancellable = new ComplexCancellable();
final HttpCacheEntry root = hit.entry;
if (root != null && root.isVariantRoot()) {
final Set<String> variantCacheKeys = root.getVariantMap().keySet();
final String rootKey = hit.rootKey;
if (root != null && root.hasVariants()) {
final List<String> variantCacheKeys = root.getVariants().stream()
.map(e -> e + rootKey)
.collect(Collectors.toList());
complexCancellable.setDependency(storage.getEntries(
variantCacheKeys,
new FutureCallback<Map<String, HttpCacheEntry>>() {
Expand Down Expand Up @@ -253,6 +256,7 @@ public void cancelled() {
}

Cancellable storeVariant(
final HttpHost host,
final HttpRequest request,
final HttpResponse originResponse,
final Instant requestSent,
Expand All @@ -278,9 +282,9 @@ public void completed(final Boolean result) {

storage.updateEntry(rootKey,
existing -> {
final Map<String,String> variantMap = existing != null ? new HashMap<>(existing.getVariantMap()) : new HashMap<>();
variantMap.put(variantKey, variantCacheKey);
return cacheEntryFactory.createRoot(requestSent, responseReceived, request, originResponse, variantMap);
final Set<String> variantMap = existing != null ? new HashSet<>(existing.getVariants()) : new HashSet<>();
variantMap.add(variantKey);
return cacheEntryFactory.createRoot(requestSent, responseReceived, host, request, originResponse, variantMap);
},
new FutureCallback<Boolean>() {

Expand Down Expand Up @@ -333,15 +337,16 @@ public void cancelled() {
}

Cancellable store(
final HttpHost host,
final HttpRequest request,
final HttpResponse originResponse,
final Instant requestSent,
final Instant responseReceived,
final String rootKey,
final HttpCacheEntry entry,
final FutureCallback<CacheHit> callback) {
if (entry.hasVariants()) {
return storeVariant(request, originResponse, requestSent, responseReceived, rootKey, entry, callback);
if (entry.containsHeader(HttpHeaders.VARY)) {
return storeVariant(host, request, originResponse, requestSent, responseReceived, rootKey, entry, callback);
} else {
return storeEntry(rootKey, entry, callback);
}
Expand Down Expand Up @@ -370,14 +375,16 @@ public Cancellable store(
final HttpCacheEntry backup = cacheEntryFactory.create(
requestSent,
responseReceived,
host,
request,
originResponse,
content != null ? HeapResourceFactory.INSTANCE.generate(null, content.array(), 0, content.length()) : null);
callback.completed(new CacheHit(rootKey, backup));
return Operations.nonCancellable();
}
final HttpCacheEntry entry = cacheEntryFactory.create(requestSent, responseReceived, request, originResponse, resource);
final HttpCacheEntry entry = cacheEntryFactory.create(requestSent, responseReceived, host, request, originResponse, resource);
return store(
host,
request,
originResponse,
requestSent,
Expand All @@ -390,6 +397,7 @@ public Cancellable store(
@Override
public Cancellable update(
final CacheHit stale,
final HttpHost host,
final HttpRequest request,
final HttpResponse originResponse,
final Instant requestSent,
Expand All @@ -410,6 +418,7 @@ public Cancellable update(
}

return store(
host,
request,
originResponse,
requestSent,
Expand Down Expand Up @@ -454,7 +463,7 @@ public Cancellable storeReusing(
if (LOG.isDebugEnabled()) {
LOG.debug("Store cache entry using existing entry: {} -> {}", rootKey, hit.rootKey);
}
return store(request, originResponse, requestSent, responseReceived, rootKey, hit.entry, callback);
return store(host, request, originResponse, requestSent, responseReceived, rootKey, hit.entry, callback);
}

private void evictEntry(final String cacheKey) {
Expand Down Expand Up @@ -487,12 +496,13 @@ private void evictAll(final HttpCacheEntry root, final String rootKey) {
LOG.debug("Evicting root cache entry {}", rootKey);
}
evictEntry(rootKey);
if (root.isVariantRoot()) {
for (final String variantKey : root.getVariantMap().values()) {
if (root.hasVariants()) {
for (final String variantKey : root.getVariants()) {
final String variantEntryKey = variantKey + rootKey;
if (LOG.isDebugEnabled()) {
LOG.debug("Evicting variant cache entry {}", variantKey);
LOG.debug("Evicting variant cache entry {}", variantEntryKey);
}
evictEntry(variantKey);
evictEntry(variantEntryKey);
}
}
}
Expand Down
Loading

0 comments on commit 05078cf

Please sign in to comment.