Better request method check (no functional changes)

This commit is contained in:
Oleg Kalnichevski 2023-06-27 14:08:26 +02:00
parent abbfd8202a
commit e469cbb78d
13 changed files with 71 additions and 40 deletions

View File

@ -33,12 +33,40 @@ package org.apache.hc.client5.http.cache;
*/
public class HeaderConstants {
/**
* @deprecated Use {@link org.apache.hc.core5.http.Method}
*/
@Deprecated
public static final String GET_METHOD = "GET";
/**
* @deprecated Use {@link org.apache.hc.core5.http.Method}
*/
@Deprecated
public static final String HEAD_METHOD = "HEAD";
/**
* @deprecated Use {@link org.apache.hc.core5.http.Method}
*/
@Deprecated
public static final String OPTIONS_METHOD = "OPTIONS";
/**
* @deprecated Use {@link org.apache.hc.core5.http.Method}
*/
@Deprecated
public static final String PUT_METHOD = "PUT";
/**
* @deprecated Use {@link org.apache.hc.core5.http.Method}
*/
@Deprecated
public static final String DELETE_METHOD = "DELETE";
/**
* @deprecated Use {@link org.apache.hc.core5.http.Method}
*/
@Deprecated
public static final String TRACE_METHOD = "TRACE";
/**
* @deprecated Use {@link org.apache.hc.core5.http.Method}
*/
@Deprecated
public static final String POST_METHOD = "POST";
/**

View File

@ -28,13 +28,13 @@ package org.apache.hc.client5.http.impl.cache;
import java.net.URI;
import org.apache.hc.client5.http.cache.HeaderConstants;
import org.apache.hc.client5.http.cache.HttpCacheEntry;
import org.apache.hc.client5.http.utils.URIUtils;
import org.apache.hc.core5.http.Header;
import org.apache.hc.core5.http.HttpHeaders;
import org.apache.hc.core5.http.HttpRequest;
import org.apache.hc.core5.http.HttpResponse;
import org.apache.hc.core5.http.Method;
class CacheInvalidatorBase {
@ -43,11 +43,11 @@ class CacheInvalidatorBase {
}
static boolean requestIsGet(final HttpRequest req) {
return req.getMethod().equals((HeaderConstants.GET_METHOD));
return Method.GET.isSame(req.getMethod());
}
static boolean isAHeadCacheEntry(final HttpCacheEntry parentCacheEntry) {
return parentCacheEntry != null && parentCacheEntry.getRequestMethod().equals(HeaderConstants.HEAD_METHOD);
return parentCacheEntry != null && Method.HEAD.isSame(parentCacheEntry.getRequestMethod());
}
static boolean isSameHost(final URI requestURI, final URI targetURI) {
@ -60,7 +60,7 @@ class CacheInvalidatorBase {
}
static boolean notGetOrHeadRequest(final String method) {
return !(HeaderConstants.GET_METHOD.equals(method) || HeaderConstants.HEAD_METHOD.equals(method));
return !(Method.GET.isSame(method) || Method.HEAD.isSame(method));
}
private static URI getLocationURI(final URI requestUri, final HttpResponse response, final String headerName) {

View File

@ -26,10 +26,10 @@
*/
package org.apache.hc.client5.http.impl.cache;
import org.apache.hc.client5.http.cache.HeaderConstants;
import org.apache.hc.core5.http.HttpHeaders;
import org.apache.hc.core5.http.HttpRequest;
import org.apache.hc.core5.http.HttpVersion;
import org.apache.hc.core5.http.Method;
import org.apache.hc.core5.http.ProtocolVersion;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@ -57,7 +57,7 @@ class CacheableRequestPolicy {
return false;
}
if (!method.equals(HeaderConstants.GET_METHOD) && !method.equals(HeaderConstants.HEAD_METHOD)) {
if (!Method.GET.isSame(method) && !Method.HEAD.isSame(method)) {
if (LOG.isDebugEnabled()) {
LOG.debug("{} request is not serveable from cache", method);
}

View File

@ -29,7 +29,6 @@ package org.apache.hc.client5.http.impl.cache;
import java.time.Instant;
import org.apache.hc.client5.http.async.methods.SimpleHttpResponse;
import org.apache.hc.client5.http.cache.HeaderConstants;
import org.apache.hc.client5.http.cache.HttpCacheEntry;
import org.apache.hc.client5.http.cache.Resource;
import org.apache.hc.client5.http.cache.ResourceIOException;
@ -41,6 +40,7 @@ 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.message.BasicHeader;
import org.apache.hc.core5.util.TimeValue;
@ -160,7 +160,7 @@ class CachedHttpResponseGenerator {
}
private boolean responseShouldContainEntity(final HttpRequest request, final HttpCacheEntry cacheEntry) {
return request.getMethod().equals(HeaderConstants.GET_METHOD) && cacheEntry.getResource() != null;
return Method.GET.isSame(request.getMethod()) && cacheEntry.getResource() != null;
}
/**

View File

@ -29,7 +29,6 @@ package org.apache.hc.client5.http.impl.cache;
import java.time.Instant;
import java.util.Iterator;
import org.apache.hc.client5.http.cache.HeaderConstants;
import org.apache.hc.client5.http.cache.HttpCacheEntry;
import org.apache.hc.client5.http.utils.DateUtils;
import org.apache.hc.core5.http.Header;
@ -37,6 +36,7 @@ import org.apache.hc.core5.http.HeaderElement;
import org.apache.hc.core5.http.HttpHeaders;
import org.apache.hc.core5.http.HttpRequest;
import org.apache.hc.core5.http.HttpStatus;
import org.apache.hc.core5.http.Method;
import org.apache.hc.core5.http.message.MessageSupport;
import org.apache.hc.core5.util.TimeValue;
import org.slf4j.Logger;
@ -176,7 +176,11 @@ class CachedResponseSuitabilityChecker {
}
private boolean isGet(final HttpRequest request) {
return request.getMethod().equals(HeaderConstants.GET_METHOD);
return Method.GET.isSame(request.getMethod());
}
private boolean isHead(final HttpRequest request) {
return Method.HEAD.isSame(request.getMethod());
}
private boolean entryIsNotA204Response(final HttpCacheEntry entry) {
@ -201,17 +205,18 @@ class CachedResponseSuitabilityChecker {
}
/**
* Determines whether the given request is a {@link HeaderConstants#GET_METHOD} request and the associated cache entry was created by a
* {@link HeaderConstants#HEAD_METHOD} request.
* Determines whether the given request is a {@link org.apache.hc.core5.http.Method#GET} request and the
* associated cache entry was created by a {@link org.apache.hc.core5.http.Method#HEAD} request.
*
* @param request The {@link HttpRequest} to check if it is a {@link HeaderConstants#GET_METHOD} request.
* @param entry The {@link HttpCacheEntry} to check if it was created by a {@link HeaderConstants#HEAD_METHOD} request.
* @return true if the request is a {@link HeaderConstants#GET_METHOD} request and the cache entry was created by a
* {@link HeaderConstants#HEAD_METHOD} request, otherwise {@code false}.
* @param request The {@link HttpRequest} to check if it is a {@link org.apache.hc.core5.http.Method#GET} request.
* @param entry The {@link HttpCacheEntry} to check if it was created by
* a {@link org.apache.hc.core5.http.Method#HEAD} request.
* @return true if the request is a {@link org.apache.hc.core5.http.Method#GET} request and the cache entry was
* created by a {@link org.apache.hc.core5.http.Method#HEAD} request, otherwise {@code false}.
* @since 5.3
*/
public boolean isGetRequestWithHeadCacheEntry(final HttpRequest request, final HttpCacheEntry entry) {
return isGet(request) && HeaderConstants.HEAD_METHOD.equalsIgnoreCase(entry.getRequestMethod());
return isGet(request) && Method.HEAD.isSame(entry.getRequestMethod());
}

View File

@ -35,7 +35,6 @@ import java.util.concurrent.atomic.AtomicLong;
import org.apache.hc.client5.http.async.methods.SimpleHttpResponse;
import org.apache.hc.client5.http.cache.CacheResponseStatus;
import org.apache.hc.client5.http.cache.HeaderConstants;
import org.apache.hc.client5.http.cache.HttpCacheContext;
import org.apache.hc.client5.http.cache.HttpCacheEntry;
import org.apache.hc.client5.http.cache.ResourceIOException;
@ -48,6 +47,7 @@ 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;
@ -332,7 +332,7 @@ public class CachingExecBase {
}
boolean clientRequestsOurOptions(final HttpRequest request) {
if (!HeaderConstants.OPTIONS_METHOD.equals(request.getMethod())) {
if (!Method.OPTIONS.isSame(request.getMethod())) {
return false;
}

View File

@ -33,11 +33,11 @@ import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
import org.apache.hc.client5.http.cache.HeaderConstants;
import org.apache.hc.core5.http.Header;
import org.apache.hc.core5.http.HttpHeaders;
import org.apache.hc.core5.http.HttpRequest;
import org.apache.hc.core5.http.HttpVersion;
import org.apache.hc.core5.http.Method;
import org.apache.hc.core5.http.ProtocolVersion;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@ -97,7 +97,7 @@ class RequestProtocolCompliance {
}
private void decrementOPTIONSMaxForwardsIfGreaterThen0(final HttpRequest request) {
if (!HeaderConstants.OPTIONS_METHOD.equals(request.getMethod())) {
if (!Method.OPTIONS.isSame(request.getMethod())) {
return;
}
@ -131,7 +131,7 @@ class RequestProtocolCompliance {
private RequestProtocolError requestHasWeakETagAndRange(final HttpRequest request) {
final String method = request.getMethod();
if (!(HeaderConstants.GET_METHOD.equals(method) || HeaderConstants.HEAD_METHOD.equals(method))) {
if (!(Method.GET.isSame(method) || Method.HEAD.isSame(method))) {
return null;
}
@ -165,8 +165,7 @@ class RequestProtocolCompliance {
private RequestProtocolError requestHasWeekETagForPUTOrDELETEIfMatch(final HttpRequest request, final boolean resourceExists) {
final String method = request.getMethod();
if (!(HeaderConstants.PUT_METHOD.equals(method) || HeaderConstants.DELETE_METHOD.equals(method)
|| HeaderConstants.POST_METHOD.equals(method))
if (!(Method.PUT.isSame(method) || Method.DELETE.isSame(method) || Method.POST.isSame(method))
) {
return null;
}

View File

@ -35,7 +35,6 @@ import java.util.HashSet;
import java.util.Iterator;
import java.util.Set;
import org.apache.hc.client5.http.cache.HeaderConstants;
import org.apache.hc.client5.http.cache.HttpCacheEntry;
import org.apache.hc.client5.http.utils.DateUtils;
import org.apache.hc.core5.http.Header;
@ -45,6 +44,7 @@ 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.message.MessageSupport;
import org.slf4j.Logger;
@ -164,8 +164,7 @@ class ResponseCachingPolicy {
public boolean isResponseCacheable(final ResponseCacheControl cacheControl, final String httpMethod, final HttpResponse response) {
boolean cacheable = false;
if (!HeaderConstants.GET_METHOD.equals(httpMethod) && !HeaderConstants.HEAD_METHOD.equals(httpMethod)
&& !HeaderConstants.POST_METHOD.equals(httpMethod)) {
if (!Method.GET.isSame(httpMethod) && !Method.HEAD.isSame(httpMethod) && !Method.POST.isSame((httpMethod))) {
if (LOG.isDebugEnabled()) {
LOG.debug("{} method response is not cacheable", httpMethod);
}

View File

@ -32,7 +32,6 @@ import java.util.ArrayList;
import java.util.List;
import org.apache.hc.client5.http.ClientProtocolException;
import org.apache.hc.client5.http.cache.HeaderConstants;
import org.apache.hc.client5.http.utils.DateUtils;
import org.apache.hc.core5.http.Header;
import org.apache.hc.core5.http.HeaderElement;
@ -42,6 +41,7 @@ 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.message.BasicHeader;
import org.apache.hc.core5.http.message.MessageSupport;
@ -169,7 +169,7 @@ class ResponseProtocolCompliance {
private void ensure200ForOPTIONSRequestWithNoBodyHasContentLengthZero(final HttpRequest request,
final HttpResponse response) {
if (!request.getMethod().equalsIgnoreCase(HeaderConstants.OPTIONS_METHOD)) {
if (!Method.OPTIONS.isSame(request.getMethod())) {
return;
}

View File

@ -45,6 +45,7 @@ import org.apache.hc.client5.http.impl.cache.HttpTestUtils;
import org.apache.hc.client5.http.utils.DateUtils;
import org.apache.hc.core5.http.Header;
import org.apache.hc.core5.http.HttpStatus;
import org.apache.hc.core5.http.Method;
import org.apache.hc.core5.http.message.BasicHeader;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
@ -151,7 +152,7 @@ public class TestHttpCacheEntry {
new BasicHeader("bar", "barValue2")
};
entry = makeEntry(headers);
assertEquals(HeaderConstants.GET_METHOD, entry.getRequestMethod());
assertEquals(Method.GET.name(), entry.getRequestMethod());
}
@Test

View File

@ -140,7 +140,7 @@ public class TestCacheableRequestPolicy {
Assertions.assertFalse(policy.isServableFromCache(cacheControl, request));
final BasicHttpRequest request2 = new BasicHttpRequest("get", "someUri");
final BasicHttpRequest request2 = new BasicHttpRequest("huh", "someUri");
Assertions.assertFalse(policy.isServableFromCache(cacheControl, request2));

View File

@ -33,7 +33,6 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
import java.util.Collections;
import java.util.List;
import org.apache.hc.client5.http.cache.HeaderConstants;
import org.apache.hc.core5.http.HttpHeaders;
import org.apache.hc.core5.http.HttpRequest;
import org.apache.hc.core5.http.HttpVersion;
@ -103,7 +102,7 @@ public class TestRequestProtocolCompliance {
@Test
public void testRequestWithMultipleIfMatchHeaders() {
final HttpRequest req = new BasicHttpRequest(HeaderConstants.PUT_METHOD, "http://example.com/");
final HttpRequest req = new BasicHttpRequest("PUT", "http://example.com/");
req.addHeader(HttpHeaders.IF_MATCH, "W/\"weak1\"");
req.addHeader(HttpHeaders.IF_MATCH, "W/\"weak2\"");
assertEquals(1, impl.requestIsFatallyNonCompliant(req, false).size());
@ -111,7 +110,7 @@ public class TestRequestProtocolCompliance {
@Test
public void testRequestWithMultipleIfNoneMatchHeaders() {
final HttpRequest req = new BasicHttpRequest(HeaderConstants.PUT_METHOD, "http://example.com/");
final HttpRequest req = new BasicHttpRequest("PUT", "http://example.com/");
req.addHeader(HttpHeaders.IF_NONE_MATCH, "W/\"weak1\"");
req.addHeader(HttpHeaders.IF_NONE_MATCH, "W/\"weak2\"");
assertEquals(1, impl.requestIsFatallyNonCompliant(req, false).size());
@ -119,7 +118,7 @@ public class TestRequestProtocolCompliance {
@Test
public void testRequestWithPreconditionFailed() {
final HttpRequest req = new BasicHttpRequest(HeaderConstants.GET_METHOD, "http://example.com/");
final HttpRequest req = new BasicHttpRequest("GET", "http://example.com/");
req.addHeader(HttpHeaders.IF_MATCH, "W/\"weak1\"");
req.addHeader(HttpHeaders.RANGE, "1");
req.addHeader(HttpHeaders.IF_RANGE, "W/\"weak2\""); // ETag doesn't match with If-Match ETag
@ -130,7 +129,7 @@ public class TestRequestProtocolCompliance {
@Test
public void testRequestWithValidIfRangeDate() {
final HttpRequest req = new BasicHttpRequest(HeaderConstants.GET_METHOD, "http://example.com/");
final HttpRequest req = new BasicHttpRequest("GET", "http://example.com/");
req.addHeader(HttpHeaders.RANGE, "bytes=0-499");
req.addHeader(HttpHeaders.LAST_MODIFIED, "Wed, 21 Oct 2023 07:28:00 GMT");
req.addHeader(HttpHeaders.IF_RANGE, "Wed, 21 Oct 2023 07:28:00 GMT");
@ -139,7 +138,7 @@ public class TestRequestProtocolCompliance {
@Test
public void testRequestWithInvalidDateFormat() {
final HttpRequest req = new BasicHttpRequest(HeaderConstants.GET_METHOD, "http://example.com/");
final HttpRequest req = new BasicHttpRequest("GET", "http://example.com/");
req.addHeader(HttpHeaders.RANGE, "bytes=0-499");
req.addHeader(HttpHeaders.LAST_MODIFIED, "Wed, 21 Oct 2023 07:28:00 GMT");
req.addHeader(HttpHeaders.IF_RANGE, "20/10/2023");
@ -148,7 +147,7 @@ public class TestRequestProtocolCompliance {
@Test
public void testRequestWithMissingIfRangeDate() {
final HttpRequest req = new BasicHttpRequest(HeaderConstants.GET_METHOD, "http://example.com/");
final HttpRequest req = new BasicHttpRequest("GET", "http://example.com/");
req.addHeader(HttpHeaders.RANGE, "bytes=0-499");
req.addHeader(HttpHeaders.LAST_MODIFIED, "Wed, 21 Oct 2023 07:28:00 GMT");
assertTrue(impl.requestIsFatallyNonCompliant(req, false).isEmpty());
@ -158,7 +157,7 @@ public class TestRequestProtocolCompliance {
public void testRequestWithWeakETagAndRangeAndDAte() {
// Setup request with GET method, Range header, If-Range header starting with "W/",
// and a Last-Modified date that doesn't match the If-Range date
final HttpRequest req = new BasicHttpRequest(HeaderConstants.GET_METHOD, "http://example.com/");
final HttpRequest req = new BasicHttpRequest("GET", "http://example.com/");
req.addHeader(HttpHeaders.RANGE, "bytes=0-499");
req.addHeader(HttpHeaders.LAST_MODIFIED, "Fri, 20 Oct 2023 07:28:00 GMT");
req.addHeader(HttpHeaders.IF_RANGE, "Wed, 18 Oct 2023 07:28:00 GMT");

View File

@ -512,7 +512,7 @@ public class TestResponseCachingPolicy {
Assertions.assertFalse(policy.isResponseCacheable(responseCacheControl, "PUT", response));
Assertions.assertFalse(policy.isResponseCacheable(responseCacheControl, "get", response));
Assertions.assertFalse(policy.isResponseCacheable(responseCacheControl, "huh", response));
}
@Test