You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hc.apache.org by ol...@apache.org on 2023/10/07 08:58:49 UTC
[httpcomponents-client] 01/02: HTTPCLIENT-2293: Via header improvements * optimized parting of Via response header values in order to generate less intermediate garbage * optimized Via message header generation * moved Via request header generation to a cache request interceptor * moved Via response header generation to a cache response interceptor
This is an automated email from the ASF dual-hosted git repository.
olegk pushed a commit to branch 5.4.x
in repository https://gitbox.apache.org/repos/asf/httpcomponents-client.git
commit 29f5ff88fd9f290212c827a259dbcf6c85187ab8
Author: Oleg Kalnichevski <ol...@apache.org>
AuthorDate: Tue Oct 3 14:23:28 2023 +0200
HTTPCLIENT-2293: Via header improvements
* optimized parting of Via response header values in order to generate less intermediate garbage
* optimized Via message header generation
* moved Via request header generation to a cache request interceptor
* moved Via response header generation to a cache response interceptor
---
.../client5/http/impl/cache/AsyncCachingExec.java | 9 --
.../hc/client5/http/impl/cache/CachingExec.java | 8 --
.../client5/http/impl/cache/CachingExecBase.java | 37 ------
.../impl/cache/CachingH2AsyncClientBuilder.java | 2 +
.../impl/cache/CachingHttpAsyncClientBuilder.java | 2 +
.../http/impl/cache/CachingHttpClientBuilder.java | 2 +
.../client5/http/impl/cache/RequestViaCache.java | 58 ++++++++
.../http/impl/cache/ResponseCachingPolicy.java | 12 +-
.../client5/http/impl/cache/ResponseViaCache.java | 58 ++++++++
.../client5/http/impl/cache/ViaCacheGenerator.java | 62 +++++++++
.../http/impl/cache/TestCachingExecChain.java | 146 ---------------------
.../http/impl/cache/TestProtocolRequirements.java | 128 ------------------
.../http/impl/cache/TestViaCacheGenerator.java | 75 +++++++++++
13 files changed, 263 insertions(+), 336 deletions(-)
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 50aac133e..e343597c9 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
@@ -217,7 +217,6 @@ class AsyncCachingExec extends CachingExecBase implements AsyncExecChainHandler
final URIAuthority authority = request.getAuthority();
final String scheme = request.getScheme();
final HttpHost target = authority != null ? new HttpHost(scheme, authority) : route.getTargetHost();
- final String via = generateViaHeader(request);
// default response context
setResponseStatus(context, CacheResponseStatus.CACHE_MISS);
@@ -228,8 +227,6 @@ class AsyncCachingExec extends CachingExecBase implements AsyncExecChainHandler
return;
}
- request.addHeader(HttpHeaders.VIA,via);
-
final RequestCacheControl requestCacheControl = CacheControlHeaderParser.INSTANCE.parse(request);
if (cacheableRequestPolicy.isServableFromCache(requestCacheControl, request)) {
@@ -296,8 +293,6 @@ class AsyncCachingExec extends CachingExecBase implements AsyncExecChainHandler
final HttpResponse backendResponse,
final EntityDetails entityDetails) throws HttpException, IOException {
final Instant responseDate = getCurrentDate();
- backendResponse.addHeader(HttpHeaders.VIA, generateViaHeader(backendResponse));
-
final AsyncExecCallback callback = new BackendResponseHandler(target, request, requestDate, responseDate, scope, asyncExecCallback);
callbackRef.set(callback);
return callback.handleResponse(backendResponse, entityDetails);
@@ -749,8 +744,6 @@ class AsyncCachingExec extends CachingExecBase implements AsyncExecChainHandler
}
AsyncExecCallback evaluateResponse(final HttpResponse backendResponse, final Instant responseDate) {
- backendResponse.addHeader(HttpHeaders.VIA, generateViaHeader(backendResponse));
-
final int statusCode = backendResponse.getCode();
if (statusCode == HttpStatus.SC_NOT_MODIFIED || statusCode == HttpStatus.SC_OK) {
recordCacheUpdate(scope.clientContext);
@@ -980,8 +973,6 @@ class AsyncCachingExec extends CachingExecBase implements AsyncExecChainHandler
final HttpResponse backendResponse,
final EntityDetails entityDetails) throws HttpException, IOException {
final Instant responseDate = getCurrentDate();
- backendResponse.addHeader(HttpHeaders.VIA, generateViaHeader(backendResponse));
-
final AsyncExecCallback callback;
// Handle 304 Not Modified responses
if (backendResponse.getCode() == HttpStatus.SC_NOT_MODIFIED) {
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 2cfc51459..2754996a3 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
@@ -154,7 +154,6 @@ class CachingExec extends CachingExecBase implements ExecChainHandler {
final URIAuthority authority = request.getAuthority();
final String scheme = request.getScheme();
final HttpHost target = authority != null ? new HttpHost(scheme, authority) : route.getTargetHost();
- final String via = generateViaHeader(request);
// default response context
setResponseStatus(context, CacheResponseStatus.CACHE_MISS);
@@ -167,8 +166,6 @@ class CachingExec extends CachingExecBase implements ExecChainHandler {
final CacheHit hit = result != null ? result.hit : null;
final CacheHit root = result != null ? result.root : null;
- request.addHeader(HttpHeaders.VIA, via);
-
final RequestCacheControl requestCacheControl = CacheControlHeaderParser.INSTANCE.parse(request);
if (!cacheableRequestPolicy.isServableFromCache(requestCacheControl, request)) {
LOG.debug("Request is not servable from cache");
@@ -220,7 +217,6 @@ class CachingExec extends CachingExecBase implements ExecChainHandler {
LOG.debug("Calling the backend");
final ClassicHttpResponse backendResponse = chain.proceed(request, scope);
try {
- backendResponse.addHeader(HttpHeaders.VIA, generateViaHeader(backendResponse));
return handleBackendResponse(target, request, scope, requestDate, getCurrentDate(), backendResponse);
} catch (final IOException | RuntimeException ex) {
backendResponse.close();
@@ -331,8 +327,6 @@ class CachingExec extends CachingExecBase implements ExecChainHandler {
responseDate = getCurrentDate();
}
- backendResponse.addHeader(HttpHeaders.VIA, generateViaHeader(backendResponse));
-
final int statusCode = backendResponse.getCode();
if (statusCode == HttpStatus.SC_NOT_MODIFIED || statusCode == HttpStatus.SC_OK) {
recordCacheUpdate(scope.clientContext);
@@ -490,8 +484,6 @@ class CachingExec extends CachingExecBase implements ExecChainHandler {
try {
final Instant responseDate = getCurrentDate();
- backendResponse.addHeader(HttpHeaders.VIA, generateViaHeader(backendResponse));
-
if (backendResponse.getCode() != HttpStatus.SC_NOT_MODIFIED) {
return handleBackendResponse(target, request, scope, requestDate, responseDate, backendResponse);
} else {
diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingExecBase.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingExecBase.java
index 273514aa4..00959860d 100644
--- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingExecBase.java
+++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingExecBase.java
@@ -28,8 +28,6 @@ package org.apache.hc.client5.http.impl.cache;
import java.io.IOException;
import java.time.Instant;
-import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.atomic.AtomicLong;
import org.apache.hc.client5.http.async.methods.SimpleHttpResponse;
@@ -40,17 +38,12 @@ import org.apache.hc.client5.http.cache.ResourceIOException;
import org.apache.hc.core5.http.Header;
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;
import org.apache.hc.core5.http.HttpStatus;
-import org.apache.hc.core5.http.HttpVersion;
import org.apache.hc.core5.http.Method;
-import org.apache.hc.core5.http.ProtocolVersion;
-import org.apache.hc.core5.http.URIScheme;
import org.apache.hc.core5.http.protocol.HttpContext;
import org.apache.hc.core5.util.TimeValue;
-import org.apache.hc.core5.util.VersionInfo;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -60,7 +53,6 @@ public class CachingExecBase {
final AtomicLong cacheMisses = new AtomicLong();
final AtomicLong cacheUpdates = new AtomicLong();
- final Map<ProtocolVersion, String> viaHeaders = new ConcurrentHashMap<>(4);
final ResponseCachingPolicy responseCachingPolicy;
final CacheValidityPolicy validityPolicy;
final CachedHttpResponseGenerator responseGenerator;
@@ -230,35 +222,6 @@ public class CachingExecBase {
return false;
}
- String generateViaHeader(final HttpMessage msg) {
-
- if (msg.getVersion() == null) {
- msg.setVersion(HttpVersion.DEFAULT);
- }
- final ProtocolVersion pv = msg.getVersion();
- final String existingEntry = viaHeaders.get(msg.getVersion());
- if (existingEntry != null) {
- return existingEntry;
- }
-
- final VersionInfo vi = VersionInfo.loadVersionInfo("org.apache.hc.client5", getClass().getClassLoader());
- final String release = (vi != null) ? vi.getRelease() : VersionInfo.UNAVAILABLE;
-
- final String value;
- final int major = pv.getMajor();
- final int minor = pv.getMinor();
- if (URIScheme.HTTP.same(pv.getProtocol())) {
- value = String.format("%d.%d localhost (Apache-HttpClient/%s (cache))", major, minor,
- release);
- } else {
- value = String.format("%s/%d.%d localhost (Apache-HttpClient/%s (cache))", pv.getProtocol(), major,
- minor, release);
- }
- viaHeaders.put(pv, value);
-
- return value;
- }
-
void setResponseStatus(final HttpContext context, final CacheResponseStatus value) {
if (context != null) {
context.setAttribute(HttpCacheContext.CACHE_RESPONSE_STATUS, value);
diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingH2AsyncClientBuilder.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingH2AsyncClientBuilder.java
index 108beb9f3..aff56365f 100644
--- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingH2AsyncClientBuilder.java
+++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingH2AsyncClientBuilder.java
@@ -66,6 +66,8 @@ public class CachingH2AsyncClientBuilder extends H2AsyncClientBuilder {
protected CachingH2AsyncClientBuilder() {
super();
addResponseInterceptorFirst(ResponseProtocolCompliance.INSTANCE);
+ addResponseInterceptorLast(ResponseViaCache.INSTANCE);
+ addRequestInterceptorLast(RequestViaCache.INSTANCE);
this.deleteCache = true;
}
diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingHttpAsyncClientBuilder.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingHttpAsyncClientBuilder.java
index 50656dff3..2f31f030f 100644
--- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingHttpAsyncClientBuilder.java
+++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingHttpAsyncClientBuilder.java
@@ -66,6 +66,8 @@ public class CachingHttpAsyncClientBuilder extends HttpAsyncClientBuilder {
protected CachingHttpAsyncClientBuilder() {
super();
addResponseInterceptorFirst(ResponseProtocolCompliance.INSTANCE);
+ addResponseInterceptorLast(ResponseViaCache.INSTANCE);
+ addRequestInterceptorLast(RequestViaCache.INSTANCE);
this.deleteCache = true;
}
diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingHttpClientBuilder.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingHttpClientBuilder.java
index ebbd19dd9..56634d4f0 100644
--- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingHttpClientBuilder.java
+++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingHttpClientBuilder.java
@@ -62,6 +62,8 @@ public class CachingHttpClientBuilder extends HttpClientBuilder {
protected CachingHttpClientBuilder() {
super();
addResponseInterceptorFirst(ResponseProtocolCompliance.INSTANCE);
+ addResponseInterceptorLast(ResponseViaCache.INSTANCE);
+ addRequestInterceptorLast(RequestViaCache.INSTANCE);
this.deleteCache = true;
}
diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/RequestViaCache.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/RequestViaCache.java
new file mode 100644
index 000000000..3379c0449
--- /dev/null
+++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/RequestViaCache.java
@@ -0,0 +1,58 @@
+/*
+ * ====================================================================
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ * ====================================================================
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals on behalf of the Apache Software Foundation. For more
+ * information on the Apache Software Foundation, please see
+ * <http://www.apache.org/>.
+ *
+ */
+package org.apache.hc.client5.http.impl.cache;
+
+import java.io.IOException;
+
+import org.apache.hc.core5.annotation.Contract;
+import org.apache.hc.core5.annotation.ThreadingBehavior;
+import org.apache.hc.core5.http.EntityDetails;
+import org.apache.hc.core5.http.HttpException;
+import org.apache.hc.core5.http.HttpHeaders;
+import org.apache.hc.core5.http.HttpRequest;
+import org.apache.hc.core5.http.HttpRequestInterceptor;
+import org.apache.hc.core5.http.ProtocolVersion;
+import org.apache.hc.core5.http.protocol.HttpContext;
+
+/**
+ * This response interceptor adds {@literal VIA} header to all outgoing
+ * request messages dispatched through the caching layer.
+ */
+@Contract(threading = ThreadingBehavior.IMMUTABLE)
+class RequestViaCache implements HttpRequestInterceptor {
+
+ public static final RequestViaCache INSTANCE = new RequestViaCache();
+
+ @Override
+ public void process(final HttpRequest request,
+ final EntityDetails entity,
+ final HttpContext context) throws HttpException, IOException {
+ final ProtocolVersion protocolVersion = context.getProtocolVersion();
+ request.addHeader(HttpHeaders.VIA, ViaCacheGenerator.INSTANCE.lookup(protocolVersion));
+ }
+
+}
diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ResponseCachingPolicy.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ResponseCachingPolicy.java
index 74239cf00..83f000415 100644
--- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ResponseCachingPolicy.java
+++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ResponseCachingPolicy.java
@@ -43,6 +43,7 @@ import org.apache.hc.core5.http.HttpStatus;
import org.apache.hc.core5.http.HttpVersion;
import org.apache.hc.core5.http.Method;
import org.apache.hc.core5.http.ProtocolVersion;
+import org.apache.hc.core5.http.message.BasicTokenIterator;
import org.apache.hc.core5.http.message.MessageSupport;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -336,15 +337,10 @@ class ResponseCachingPolicy {
}
private boolean from1_0Origin(final HttpResponse response) {
- final Iterator<HeaderElement> it = MessageSupport.iterate(response, HttpHeaders.VIA);
+ final Iterator<String> it = new BasicTokenIterator(response.headerIterator(HttpHeaders.VIA));
if (it.hasNext()) {
- final HeaderElement elt = it.next();
- final String proto = elt.toString().split("\\s")[0];
- if (proto.contains("/")) {
- return proto.equals("HTTP/1.0");
- } else {
- return proto.equals("1.0");
- }
+ final String token = it.next();
+ return token.startsWith("1.0 ") || token.startsWith("HTTP/1.0 ");
}
final ProtocolVersion version = response.getVersion() != null ? response.getVersion() : HttpVersion.DEFAULT;
return HttpVersion.HTTP_1_0.equals(version);
diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ResponseViaCache.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ResponseViaCache.java
new file mode 100644
index 000000000..205933b16
--- /dev/null
+++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ResponseViaCache.java
@@ -0,0 +1,58 @@
+/*
+ * ====================================================================
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ * ====================================================================
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals on behalf of the Apache Software Foundation. For more
+ * information on the Apache Software Foundation, please see
+ * <http://www.apache.org/>.
+ *
+ */
+package org.apache.hc.client5.http.impl.cache;
+
+import java.io.IOException;
+
+import org.apache.hc.core5.annotation.Contract;
+import org.apache.hc.core5.annotation.ThreadingBehavior;
+import org.apache.hc.core5.http.EntityDetails;
+import org.apache.hc.core5.http.HttpException;
+import org.apache.hc.core5.http.HttpHeaders;
+import org.apache.hc.core5.http.HttpResponse;
+import org.apache.hc.core5.http.HttpResponseInterceptor;
+import org.apache.hc.core5.http.ProtocolVersion;
+import org.apache.hc.core5.http.protocol.HttpContext;
+
+/**
+ * This response interceptor adds {@literal VIA} header to all incoming
+ * response messages dispatched through the caching layer.
+ */
+@Contract(threading = ThreadingBehavior.IMMUTABLE)
+class ResponseViaCache implements HttpResponseInterceptor {
+
+ public static final ResponseViaCache INSTANCE = new ResponseViaCache();
+
+ @Override
+ public void process(final HttpResponse response,
+ final EntityDetails entity,
+ final HttpContext context) throws HttpException, IOException {
+ final ProtocolVersion protocolVersion = context.getProtocolVersion();
+ response.addHeader(HttpHeaders.VIA, ViaCacheGenerator.INSTANCE.lookup(protocolVersion));
+ }
+
+}
diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ViaCacheGenerator.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ViaCacheGenerator.java
new file mode 100644
index 000000000..22260b86d
--- /dev/null
+++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ViaCacheGenerator.java
@@ -0,0 +1,62 @@
+/*
+ * ====================================================================
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ * ====================================================================
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals on behalf of the Apache Software Foundation. For more
+ * information on the Apache Software Foundation, please see
+ * <http://www.apache.org/>.
+ *
+ */
+package org.apache.hc.client5.http.impl.cache;
+
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+
+import org.apache.hc.core5.http.ProtocolVersion;
+import org.apache.hc.core5.http.URIScheme;
+import org.apache.hc.core5.util.VersionInfo;
+
+final class ViaCacheGenerator {
+
+ final static ViaCacheGenerator INSTANCE = new ViaCacheGenerator();
+
+ final ConcurrentMap<ProtocolVersion, String> internalCache = new ConcurrentHashMap<>(4);
+
+ String generateViaHeader(final VersionInfo vi, final ProtocolVersion pv) {
+ final StringBuilder buf = new StringBuilder();
+ if (!URIScheme.HTTP.same(pv.getProtocol())) {
+ buf.append(pv.getProtocol()).append('/');
+ }
+ buf.append(pv.getMajor()).append('.').append(pv.getMinor());
+ buf.append(' ').append("localhost").append(' ');
+ buf.append("(Apache-HttpClient/");
+ final String release = (vi != null) ? vi.getRelease() : VersionInfo.UNAVAILABLE;
+ buf.append(release).append(" (cache))");
+ return buf.toString();
+ }
+
+ String lookup(final ProtocolVersion pv) {
+ return internalCache.computeIfAbsent(pv, (v) -> {
+ final VersionInfo vi = VersionInfo.loadVersionInfo("org.apache.hc.client5", getClass().getClassLoader());
+ return generateViaHeader(vi, v);
+ });
+ }
+
+}
diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachingExecChain.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachingExecChain.java
index c54f34c65..1a31989d4 100644
--- a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachingExecChain.java
+++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachingExecChain.java
@@ -45,7 +45,6 @@ import org.apache.hc.client5.http.auth.StandardAuthScheme;
import org.apache.hc.client5.http.cache.CacheResponseStatus;
import org.apache.hc.client5.http.cache.HttpCacheContext;
import org.apache.hc.client5.http.cache.HttpCacheEntry;
-import org.apache.hc.client5.http.cache.HttpCacheEntryFactory;
import org.apache.hc.client5.http.cache.HttpCacheStorage;
import org.apache.hc.client5.http.classic.ExecChain;
import org.apache.hc.client5.http.classic.ExecRuntime;
@@ -58,9 +57,7 @@ import org.apache.hc.core5.http.ClassicHttpResponse;
import org.apache.hc.core5.http.Header;
import org.apache.hc.core5.http.HttpException;
import org.apache.hc.core5.http.HttpHost;
-import org.apache.hc.core5.http.HttpRequest;
import org.apache.hc.core5.http.HttpStatus;
-import org.apache.hc.core5.http.HttpVersion;
import org.apache.hc.core5.http.io.entity.EntityUtils;
import org.apache.hc.core5.http.io.entity.InputStreamEntity;
import org.apache.hc.core5.http.io.support.ClassicRequestBuilder;
@@ -71,7 +68,6 @@ import org.apache.hc.core5.net.URIAuthority;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
-import org.mockito.ArgumentCaptor;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.MockitoAnnotations;
@@ -86,24 +82,6 @@ public class TestCachingExecChain {
HttpCacheStorage mockStorage;
@Mock
DefaultCacheRevalidator cacheRevalidator;
- @Mock
- CachedHttpResponseGenerator responseGenerator;
- @Mock
- HttpCacheEntryFactory cacheEntryFactory;
- @Mock
- CacheValidityPolicy validityPolicy;
- @Mock
- ResponseCachingPolicy responseCachingPolicy;
- @Mock
- CacheableRequestPolicy cacheableRequestPolicy;
- @Mock
- CachedResponseSuitabilityChecker suitabilityChecker;
- @Mock
- ResponseProtocolCompliance responseCompliance;
- @Mock
- ConditionalRequestBuilder<ClassicHttpRequest> conditionalRequestBuilder;
- @Mock
- HttpCache responseCache;
HttpRoute route;
HttpHost host;
@@ -245,27 +223,6 @@ public class TestCachingExecChain {
Assertions.assertEquals(CacheResponseStatus.CACHE_MODULE_RESPONSE, context.getCacheResponseStatus());
}
- @Test
- public void testRecordsClientProtocolInViaHeaderIfRequestNotServableFromCache() throws Exception {
- final ClassicHttpRequest originalRequest = new BasicClassicHttpRequest("GET", "/");
- originalRequest.setVersion(HttpVersion.HTTP_1_0);
- final ClassicHttpRequest req = originalRequest;
- req.setHeader("Cache-Control", "no-cache");
- final ClassicHttpResponse resp = new BasicClassicHttpResponse(HttpStatus.SC_NO_CONTENT, "No Content");
-
- Mockito.when(mockExecChain.proceed(Mockito.any(), Mockito.any())).thenReturn(resp);
-
- execute(req);
-
- final ArgumentCaptor<ClassicHttpRequest> reqCapture = ArgumentCaptor.forClass(ClassicHttpRequest.class);
- Mockito.verify(mockExecChain).proceed(reqCapture.capture(), Mockito.any());
-
- final HttpRequest captured = reqCapture.getValue();
- final String via = captured.getFirstHeader("Via").getValue();
- final String proto = via.split("\\s+")[0];
- Assertions.assertTrue("http/1.0".equalsIgnoreCase(proto) || "1.0".equalsIgnoreCase(proto));
- }
-
@Test
public void testSetsCacheMissContextIfRequestNotServableFromCache() throws Exception {
final ClassicHttpRequest req = new HttpGet("http://foo.example.com/");
@@ -278,34 +235,6 @@ public class TestCachingExecChain {
Assertions.assertEquals(CacheResponseStatus.CACHE_MISS, context.getCacheResponseStatus());
}
- @Test
- public void testSetsViaHeaderOnResponseIfRequestNotServableFromCache() throws Exception {
- final ClassicHttpRequest req = new HttpGet("http://foo.example.com/");
- req.setHeader("Cache-Control", "no-cache");
- final ClassicHttpResponse resp = new BasicClassicHttpResponse(HttpStatus.SC_NO_CONTENT, "No Content");
-
- Mockito.when(mockExecChain.proceed(Mockito.any(), Mockito.any())).thenReturn(resp);
-
- final ClassicHttpResponse result = execute(req);
- Assertions.assertNotNull(result.getFirstHeader("Via"));
- }
-
- @Test
- public void testSetsViaHeaderOnResponseForCacheMiss() throws Exception {
- final ClassicHttpRequest req1 = new HttpGet("http://foo.example.com/");
- final ClassicHttpResponse resp1 = new BasicClassicHttpResponse(HttpStatus.SC_OK, "OK");
- resp1.setEntity(HttpTestUtils.makeBody(128));
- resp1.setHeader("Content-Length", "128");
- resp1.setHeader("ETag", "\"etag\"");
- resp1.setHeader("Date", DateUtils.formatStandardDate(Instant.now()));
- resp1.setHeader("Cache-Control", "public, max-age=3600");
-
- Mockito.when(mockExecChain.proceed(Mockito.any(), Mockito.any())).thenReturn(resp1);
-
- final ClassicHttpResponse result = execute(req1);
- Assertions.assertNotNull(result.getFirstHeader("Via"));
- }
-
@Test
public void testSetsCacheHitContextIfRequestServedFromCache() throws Exception {
final ClassicHttpRequest req1 = new HttpGet("http://foo.example.com/");
@@ -324,24 +253,6 @@ public class TestCachingExecChain {
Assertions.assertEquals(CacheResponseStatus.CACHE_HIT, context.getCacheResponseStatus());
}
- @Test
- public void testSetsViaHeaderOnResponseIfRequestServedFromCache() throws Exception {
- final ClassicHttpRequest req1 = new HttpGet("http://foo.example.com/");
- final ClassicHttpRequest req2 = new HttpGet("http://foo.example.com/");
- final ClassicHttpResponse resp1 = new BasicClassicHttpResponse(HttpStatus.SC_OK, "OK");
- resp1.setEntity(HttpTestUtils.makeBody(128));
- resp1.setHeader("Content-Length", "128");
- resp1.setHeader("ETag", "\"etag\"");
- resp1.setHeader("Date", DateUtils.formatStandardDate(Instant.now()));
- resp1.setHeader("Cache-Control", "public, max-age=3600");
-
- Mockito.when(mockExecChain.proceed(Mockito.any(), Mockito.any())).thenReturn(resp1);
-
- execute(req1);
- final ClassicHttpResponse result = execute(req2);
- Assertions.assertNotNull(result.getFirstHeader("Via"));
- }
-
@Test
public void testReturns304ForIfModifiedSinceHeaderIfRequestServedFromCache() throws Exception {
final Instant now = Instant.now();
@@ -630,38 +541,6 @@ public class TestCachingExecChain {
Assertions.assertEquals(CacheResponseStatus.VALIDATED, context.getCacheResponseStatus());
}
- @Test
- public void testSetsViaHeaderIfRequestWasSuccessfullyValidated() throws Exception {
- final Instant now = Instant.now();
- final Instant tenSecondsAgo = now.minusSeconds(10);
-
- final ClassicHttpRequest req1 = new HttpGet("http://foo.example.com/");
- final ClassicHttpRequest req2 = new HttpGet("http://foo.example.com/");
-
- final ClassicHttpResponse resp1 = new BasicClassicHttpResponse(HttpStatus.SC_OK, "OK");
- resp1.setEntity(HttpTestUtils.makeBody(128));
- resp1.setHeader("Content-Length", "128");
- resp1.setHeader("ETag", "\"etag\"");
- resp1.setHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo));
- resp1.setHeader("Cache-Control", "public, max-age=5");
-
- final ClassicHttpResponse resp2 = new BasicClassicHttpResponse(HttpStatus.SC_OK, "OK");
- resp2.setEntity(HttpTestUtils.makeBody(128));
- resp2.setHeader("Content-Length", "128");
- resp2.setHeader("ETag", "\"etag\"");
- resp2.setHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo));
- resp2.setHeader("Cache-Control", "public, max-age=5");
-
- Mockito.when(mockExecChain.proceed(Mockito.any(), Mockito.any())).thenReturn(resp1);
-
- execute(req1);
-
- Mockito.when(mockExecChain.proceed(Mockito.any(), Mockito.any())).thenReturn(resp2);
-
- final ClassicHttpResponse result = execute(req2);
- Assertions.assertNotNull(result.getFirstHeader("Via"));
- }
-
@Test
public void testSetsModuleResponseContextIfValidationRequiredButFailed() throws Exception {
final Instant now = Instant.now();
@@ -713,31 +592,6 @@ public class TestCachingExecChain {
Assertions.assertEquals(CacheResponseStatus.CACHE_HIT, context.getCacheResponseStatus());
}
- @Test
- public void testSetViaHeaderIfValidationFailsButNotRequired() throws Exception {
- final Instant now = Instant.now();
- final Instant tenSecondsAgo = now.minusSeconds(10);
-
- final ClassicHttpRequest req1 = new HttpGet("http://foo.example.com/");
- final ClassicHttpRequest req2 = new HttpGet("http://foo.example.com/");
-
- final ClassicHttpResponse resp1 = new BasicClassicHttpResponse(HttpStatus.SC_OK, "OK");
- resp1.setEntity(HttpTestUtils.makeBody(128));
- resp1.setHeader("Content-Length", "128");
- resp1.setHeader("ETag", "\"etag\"");
- resp1.setHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo));
- resp1.setHeader("Cache-Control", "public, max-age=5");
-
- Mockito.when(mockExecChain.proceed(Mockito.any(), Mockito.any())).thenReturn(resp1);
-
- execute(req1);
-
- Mockito.when(mockExecChain.proceed(Mockito.any(), Mockito.any())).thenThrow(new IOException());
-
- final ClassicHttpResponse result = execute(req2);
- Assertions.assertNotNull(result.getFirstHeader("Via"));
- }
-
@Test
public void testReturns304ForIfNoneMatchPassesIfRequestServedFromOrigin() throws Exception {
diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestProtocolRequirements.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestProtocolRequirements.java
index 73c02017d..404a99b72 100644
--- a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestProtocolRequirements.java
+++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestProtocolRequirements.java
@@ -36,8 +36,6 @@ import java.time.temporal.ChronoUnit;
import java.util.Iterator;
import java.util.List;
import java.util.Random;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
import org.apache.hc.client5.http.HttpRoute;
import org.apache.hc.client5.http.auth.StandardAuthScheme;
@@ -55,7 +53,6 @@ import org.apache.hc.core5.http.HttpException;
import org.apache.hc.core5.http.HttpHeaders;
import org.apache.hc.core5.http.HttpHost;
import org.apache.hc.core5.http.HttpStatus;
-import org.apache.hc.core5.http.HttpVersion;
import org.apache.hc.core5.http.io.support.ClassicRequestBuilder;
import org.apache.hc.core5.http.message.BasicClassicHttpRequest;
import org.apache.hc.core5.http.message.BasicClassicHttpResponse;
@@ -2261,129 +2258,4 @@ public class TestProtocolRequirements {
Assertions.assertEquals(server, result.getFirstHeader("Server").getValue());
}
- @Test
- public void testProperlyFormattedViaHeaderIsAddedToRequests() throws Exception {
- request.removeHeaders(HttpHeaders.VIA);
- Mockito.when(mockExecChain.proceed(Mockito.any(), Mockito.any())).thenReturn(originResponse);
-
- execute(request);
-
- final ArgumentCaptor<ClassicHttpRequest> reqCapture = ArgumentCaptor.forClass(ClassicHttpRequest.class);
- Mockito.verify(mockExecChain).proceed(reqCapture.capture(), Mockito.any());
-
- final ClassicHttpRequest captured = reqCapture.getValue();
- final String via = captured.getFirstHeader(HttpHeaders.VIA).getValue();
- assertValidViaHeader(via);
- }
-
- @Test
- public void testProperlyFormattedViaHeaderIsAddedToResponses() throws Exception {
- originResponse.removeHeaders(HttpHeaders.VIA);
- Mockito.when(mockExecChain.proceed(Mockito.any(), Mockito.any())).thenReturn(originResponse);
- final ClassicHttpResponse result = execute(request);
- assertValidViaHeader(result.getFirstHeader(HttpHeaders.VIA).getValue());
- }
-
- private void assertValidViaHeader(final String via) {
- // Via = HttpHeaders.VIA ":" 1#( received-protocol received-by [ comment ] )
- // received-protocol = [ protocol-name "/" ] protocol-version
- // protocol-name = token
- // protocol-version = token
- // received-by = ( host [ ":" port ] ) | pseudonym
- // pseudonym = token
-
- final String[] parts = via.split("\\s+");
- Assertions.assertTrue(parts.length >= 2);
-
- // received protocol
- final String receivedProtocol = parts[0];
- final String[] protocolParts = receivedProtocol.split("/");
- Assertions.assertTrue(protocolParts.length >= 1);
- Assertions.assertTrue(protocolParts.length <= 2);
-
- final String tokenRegexp = "[^\\p{Cntrl}()<>@,;:\\\\\"/\\[\\]?={} \\t]+";
- for(final String protocolPart : protocolParts) {
- Assertions.assertTrue(Pattern.matches(tokenRegexp, protocolPart));
- }
-
- // received-by
- if (!Pattern.matches(tokenRegexp, parts[1])) {
- // host : port
- new HttpHost(parts[1]); // TODO - unused - is this a test bug? else use Assertions.assertNotNull
- }
-
- // comment
- if (parts.length > 2) {
- final StringBuilder buf = new StringBuilder(parts[2]);
- for(int i=3; i<parts.length; i++) {
- buf.append(" "); buf.append(parts[i]);
- }
- Assertions.assertTrue(isValidComment(buf.toString()));
- }
- }
-
- private boolean isValidComment(final String s) {
- final String leafComment = "^\\(([^\\p{Cntrl}()]|\\\\\\p{ASCII})*\\)$";
- final String nestedPrefix = "^\\(([^\\p{Cntrl}()]|\\\\\\p{ASCII})*\\(";
- final String nestedSuffix = "\\)([^\\p{Cntrl}()]|\\\\\\p{ASCII})*\\)$";
-
- if (Pattern.matches(leafComment,s)) {
- return true;
- }
- final Matcher pref = Pattern.compile(nestedPrefix).matcher(s);
- final Matcher suff = Pattern.compile(nestedSuffix).matcher(s);
- if (!pref.find()) {
- return false;
- }
- if (!suff.find()) {
- return false;
- }
- return isValidComment(s.substring(pref.end() - 1, suff.start() + 1));
- }
-
- @Test
- public void testViaHeaderOnRequestProperlyRecordsClientProtocol() throws Exception {
- final ClassicHttpRequest originalRequest = new BasicClassicHttpRequest("GET", "/");
- originalRequest.setVersion(HttpVersion.HTTP_1_0);
- request = originalRequest;
- request.removeHeaders(HttpHeaders.VIA);
-
- Mockito.when(mockExecChain.proceed(Mockito.any(), Mockito.any())).thenReturn(originResponse);
-
- execute(request);
-
- final ArgumentCaptor<ClassicHttpRequest> reqCapture = ArgumentCaptor.forClass(ClassicHttpRequest.class);
- Mockito.verify(mockExecChain).proceed(reqCapture.capture(), Mockito.any());
-
- final ClassicHttpRequest captured = reqCapture.getValue();
- final String via = captured.getFirstHeader(HttpHeaders.VIA).getValue();
- final String protocol = via.split("\\s+")[0];
- final String[] protoParts = protocol.split("/");
- if (protoParts.length > 1) {
- Assertions.assertTrue("http".equalsIgnoreCase(protoParts[0]));
- }
- Assertions.assertEquals("1.0",protoParts[protoParts.length-1]);
- }
-
- @Test
- public void testViaHeaderOnResponseProperlyRecordsOriginProtocol() throws Exception {
-
- originResponse = new BasicClassicHttpResponse(HttpStatus.SC_NO_CONTENT, "No Content");
- originResponse.setVersion(HttpVersion.HTTP_1_0);
-
- Mockito.when(mockExecChain.proceed(Mockito.any(), Mockito.any())).thenReturn(originResponse);
-
- final ClassicHttpResponse result = execute(request);
-
- final String via = result.getFirstHeader(HttpHeaders.VIA).getValue();
- final String protocol = via.split("\\s+")[0];
- final String[] protoParts = protocol.split("/");
- Assertions.assertTrue(protoParts.length >= 1);
- Assertions.assertTrue(protoParts.length <= 2);
- if (protoParts.length > 1) {
- Assertions.assertTrue("http".equalsIgnoreCase(protoParts[0]));
- }
- Assertions.assertEquals("1.0", protoParts[protoParts.length - 1]);
- }
-
}
diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestViaCacheGenerator.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestViaCacheGenerator.java
new file mode 100644
index 000000000..83fff1a65
--- /dev/null
+++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestViaCacheGenerator.java
@@ -0,0 +1,75 @@
+/*
+ * ====================================================================
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ * ====================================================================
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals on behalf of the Apache Software Foundation. For more
+ * information on the Apache Software Foundation, please see
+ * <http://www.apache.org/>.
+ *
+ */
+package org.apache.hc.client5.http.impl.cache;
+
+
+import org.apache.hc.core5.http.HttpVersion;
+import org.hamcrest.MatcherAssert;
+import org.hamcrest.Matchers;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+public class TestViaCacheGenerator {
+
+ private ViaCacheGenerator impl;
+
+ @BeforeEach
+ public void setUp() {
+ impl = new ViaCacheGenerator();
+ }
+
+ @Test
+ public void testViaValueGeneration() {
+ Assertions.assertEquals("1.1 localhost (Apache-HttpClient/UNAVAILABLE (cache))",
+ impl.generateViaHeader(null, HttpVersion.DEFAULT));
+ Assertions.assertEquals("2.0 localhost (Apache-HttpClient/UNAVAILABLE (cache))",
+ impl.generateViaHeader(null, HttpVersion.HTTP_2));
+ }
+
+ @Test
+ public void testViaValueLookup() {
+ MatcherAssert.assertThat(impl.lookup(HttpVersion.DEFAULT),
+ Matchers.startsWith("1.1 localhost (Apache-HttpClient/"));
+ MatcherAssert.assertThat(impl.lookup(HttpVersion.HTTP_1_0),
+ Matchers.startsWith("1.0 localhost (Apache-HttpClient/"));
+ MatcherAssert.assertThat(impl.lookup(HttpVersion.HTTP_1_1),
+ Matchers.startsWith("1.1 localhost (Apache-HttpClient/"));
+ MatcherAssert.assertThat(impl.lookup(HttpVersion.HTTP_2),
+ Matchers.startsWith("2.0 localhost (Apache-HttpClient/"));
+ MatcherAssert.assertThat(impl.lookup(HttpVersion.HTTP_2_0),
+ Matchers.startsWith("2.0 localhost (Apache-HttpClient/"));
+ MatcherAssert.assertThat(impl.lookup(HttpVersion.HTTP_1_0),
+ Matchers.startsWith("1.0 localhost (Apache-HttpClient/"));
+ MatcherAssert.assertThat(impl.lookup(HttpVersion.HTTP_1_1),
+ Matchers.startsWith("1.1 localhost (Apache-HttpClient/"));
+ MatcherAssert.assertThat(impl.lookup(HttpVersion.HTTP_2_0),
+ Matchers.startsWith("2.0 localhost (Apache-HttpClient/"));
+ Assertions.assertEquals(3, impl.internalCache.size());
+ }
+
+}