From 98cc672bb7b30bbf96b65f80ac4690e8325b569b Mon Sep 17 00:00:00 2001 From: Prabhash Date: Sat, 27 Jun 2026 14:40:34 +0200 Subject: [PATCH] Add HTTP QUERY method support (RFC 10008) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RFC 10008 defines QUERY as a safe, idempotent, cacheable HTTP method that carries a request body describing query criteria โ€” a body-carrying counterpart to GET. - `Request.HttpMethod.QUERY(true)` registered as a first-class method - `RequestLine` Javadoc updated to list QUERY as a valid method name - `feign.mock.HttpMethod` gains a `QUERY` constant; without it, `RequestKey.create(Request)` threw `IllegalArgumentException` on any QUERY request routed through `MockClient` - `MockClientTest` adds `queryMock()` to verify end-to-end routing - `AbstractClientTest` adds a `query()` test verifying that QUERY transmits a body with the correct `Content-Type` and `Content-Length` - `DefaultClientTest` overrides `query()` to assert `RetryableException` wrapping `ProtocolException` โ€” `HttpURLConnection.setRequestMethod` does not accept QUERY (mirrors the existing PATCH pattern) - `GoogleHttpClientTest` and `AbstractJAXRSClientTest` override `query()` for the same reason (both delegate to `HttpURLConnection` under the hood) - `HttpCacheInterceptor` includes QUERY in its default cacheable set - Cache key incorporates `Arrays.hashCode(body)` when a request body is present to reduce cross-body collisions - `HttpCacheInterceptorTest` verifies that distinct bodies produce separate cache entries and that same-body repeats trigger conditional revalidation **Out of scope:** `Accept-Query` response header (RFC 10008 ยง6) is not implemented; servers indicate QUERY support via `Allow` already. --- CHANGELOG.md | 6 +++ core/src/main/java/feign/Request.java | 3 +- core/src/main/java/feign/RequestLine.java | 7 +-- .../test/java/feign/DefaultContractTest.java | 5 ++ .../java/feign/client/AbstractClientTest.java | 26 ++++++++++ .../java/feign/client/DefaultClientTest.java | 12 +++++ .../GoogleHttpClientTest.java | 4 ++ .../feign/cache/HttpCacheInterceptor.java | 13 +++-- .../feign/cache/HttpCacheInterceptorTest.java | 52 +++++++++++++++++++ .../feign/jaxrs2/AbstractJAXRSClientTest.java | 9 ++++ mock/src/main/java/feign/mock/HttpMethod.java | 3 +- .../test/java/feign/mock/MockClientTest.java | 21 ++++++++ 12 files changed, 153 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 42d4bb75fe..51fad7326f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +### Version 13.14 + +* Add support for the HTTP QUERY method (RFC 10008) โ€” safe, idempotent, and cacheable with a + request body. `HttpCacheInterceptor` includes QUERY in its default cacheable set and + incorporates a body hash into the cache key to reduce cross-body collisions. + ### Version 13.12 * `UrlencodedFormContentProcessor` now honors `CollectionFormat` from `@RequestLine`/`RequestTemplate` for array and diff --git a/core/src/main/java/feign/Request.java b/core/src/main/java/feign/Request.java index a3627a164d..f9645774fb 100644 --- a/core/src/main/java/feign/Request.java +++ b/core/src/main/java/feign/Request.java @@ -44,7 +44,8 @@ public enum HttpMethod { CONNECT, OPTIONS, TRACE, - PATCH(true); + PATCH(true), + QUERY(true); private final boolean withBody; diff --git a/core/src/main/java/feign/RequestLine.java b/core/src/main/java/feign/RequestLine.java index 626c618757..4fc9178372 100644 --- a/core/src/main/java/feign/RequestLine.java +++ b/core/src/main/java/feign/RequestLine.java @@ -35,9 +35,10 @@ * *

The string must begin with a valid {@linkplain feign.Request.HttpMethod HTTP method name} * (e.g. {@linkplain feign.Request.HttpMethod#GET GET}, {@linkplain feign.Request.HttpMethod#POST - * POST}, {@linkplain feign.Request.HttpMethod#PUT PUT}), followed by a space and a URI template. - * If only the HTTP method is specified (e.g. {@code "DELETE"}), the request will use the base URL - * defined for the client. + * POST}, {@linkplain feign.Request.HttpMethod#PUT PUT}, {@linkplain + * feign.Request.HttpMethod#QUERY QUERY}), followed by a space and a URI template. If only the + * HTTP method is specified (e.g. {@code "DELETE"}), the request will use the base URL defined for + * the client. * *

Example: * diff --git a/core/src/test/java/feign/DefaultContractTest.java b/core/src/test/java/feign/DefaultContractTest.java index 6b26d1ed0e..4cbe306920 100644 --- a/core/src/test/java/feign/DefaultContractTest.java +++ b/core/src/test/java/feign/DefaultContractTest.java @@ -59,6 +59,8 @@ void httpMethods() throws Exception { assertThat(parseAndValidateMetadata(Methods.class, "get").template()).hasMethod("GET"); assertThat(parseAndValidateMetadata(Methods.class, "delete").template()).hasMethod("DELETE"); + + assertThat(parseAndValidateMetadata(Methods.class, "query").template()).hasMethod("QUERY"); } @Test @@ -422,6 +424,9 @@ interface Methods { @RequestLine("DELETE /") void delete(); + + @RequestLine("QUERY /") + void query(); } interface BodyParams { diff --git a/core/src/test/java/feign/client/AbstractClientTest.java b/core/src/test/java/feign/client/AbstractClientTest.java index bb01525c99..b37fd7d7ff 100644 --- a/core/src/test/java/feign/client/AbstractClientTest.java +++ b/core/src/test/java/feign/client/AbstractClientTest.java @@ -243,6 +243,28 @@ public void noResponseBodyForPatch() { api.noPatchBody(); } + /** + * Some client implementation tests should override this test if the QUERY operation is + * unsupported. + */ + @Test + public void query() throws Exception { + server.enqueue(new MockResponse().setBody("foo")); + server.enqueue(new MockResponse()); + + TestInterface api = + newBuilder().target(TestInterface.class, "http://localhost:" + server.getPort()); + + assertThat(api.query("body")).isEqualTo("foo"); + + MockWebServerAssertions.assertThat(server.takeRequest()) + .hasHeaders( + entry("Accept", Collections.singletonList("text/plain")), + entry("Content-Type", Collections.singletonList("application/json")), + entry("Content-Length", Collections.singletonList("4"))) + .hasMethod("QUERY"); + } + @Test public void parsesResponseMissingLength() throws IOException { server.enqueue(new MockResponse().setChunkedBody("foo", 1)); @@ -583,6 +605,10 @@ public interface TestInterface { @Headers("Accept: text/plain") String patch(String body); + @RequestLine("QUERY /") + @Headers({"Accept: text/plain", "Content-Type: application/json"}) + String query(String body); + @RequestLine("POST") String noPostBody(); diff --git a/core/src/test/java/feign/client/DefaultClientTest.java b/core/src/test/java/feign/client/DefaultClientTest.java index c61b1b0a4d..c7933bdb09 100644 --- a/core/src/test/java/feign/client/DefaultClientTest.java +++ b/core/src/test/java/feign/client/DefaultClientTest.java @@ -128,6 +128,18 @@ public void noResponseBodyForPatch() { assertThat(exception).hasCauseInstanceOf(ProtocolException.class); } + /** + * {@link java.net.HttpURLConnection} does not support the QUERY method. For now, prefer okhttp. + * + * @see java.net.HttpURLConnection#setRequestMethod + */ + @Test + @Override + public void query() throws Exception { + RetryableException exception = assertThrows(RetryableException.class, super::query); + assertThat(exception).hasCauseInstanceOf(ProtocolException.class); + } + @Test void canOverrideHostnameVerifier() throws IOException, InterruptedException { server.useHttps(TrustingSSLSocketFactory.get("bad.example.com"), false); diff --git a/googlehttpclient/src/test/java/feign/googlehttpclient/GoogleHttpClientTest.java b/googlehttpclient/src/test/java/feign/googlehttpclient/GoogleHttpClientTest.java index 4f0702f2ad..6ee3d4c25f 100644 --- a/googlehttpclient/src/test/java/feign/googlehttpclient/GoogleHttpClientTest.java +++ b/googlehttpclient/src/test/java/feign/googlehttpclient/GoogleHttpClientTest.java @@ -44,6 +44,10 @@ public void noResponseBodyForPatch() {} @Override public void patch() {} + // NetHttpTransport is backed by HttpURLConnection, which does not support QUERY. + @Override + public void query() throws Exception {} + @Override public void parsesUnauthorizedResponseBody() {} diff --git a/http-cache/src/main/java/feign/cache/HttpCacheInterceptor.java b/http-cache/src/main/java/feign/cache/HttpCacheInterceptor.java index f95d66d95b..be9e2cfb9b 100644 --- a/http-cache/src/main/java/feign/cache/HttpCacheInterceptor.java +++ b/http-cache/src/main/java/feign/cache/HttpCacheInterceptor.java @@ -23,6 +23,7 @@ import feign.interceptor.Invocation; import feign.interceptor.MethodInterceptor; import java.time.Instant; +import java.util.Arrays; import java.util.Collection; import java.util.Map; import java.util.function.Function; @@ -36,7 +37,8 @@ *

Successful responses (2xx) carrying an {@code ETag} or {@code Last-Modified} header are * stored. Responses with {@code Cache-Control: no-store} are skipped. * - *

Default scope is HTTP {@code GET} and {@code HEAD}; override via {@link #cacheable(Function)}. + *

Default scope is HTTP {@code GET}, {@code HEAD}, and {@code QUERY}; override via {@link + * #cacheable(Function)}. * *

Important: 304 detection relies on the configured {@link feign.codec.ErrorDecoder} * raising a {@link FeignException} for non-2xx responses (the default behaviour). If a custom error @@ -129,12 +131,17 @@ private void maybeStore(String key, Object result, Response response) { private static String defaultKey(Invocation invocation) { RequestTemplate template = invocation.requestTemplate(); - return invocation.methodMetadata().configKey() + "|" + template.method() + " " + template.url(); + String base = + invocation.methodMetadata().configKey() + "|" + template.method() + " " + template.url(); + byte[] body = template.body(); + return body != null ? base + "|" + Arrays.hashCode(body) : base; } private static Boolean defaultCacheable(RequestTemplate template) { String method = template.method(); - return "GET".equalsIgnoreCase(method) || "HEAD".equalsIgnoreCase(method); + return "GET".equalsIgnoreCase(method) + || "HEAD".equalsIgnoreCase(method) + || "QUERY".equalsIgnoreCase(method); } private static boolean containsNoStore(Map> headers) { diff --git a/http-cache/src/test/java/feign/cache/HttpCacheInterceptorTest.java b/http-cache/src/test/java/feign/cache/HttpCacheInterceptorTest.java index 677505bfcd..3607f6bd5e 100644 --- a/http-cache/src/test/java/feign/cache/HttpCacheInterceptorTest.java +++ b/http-cache/src/test/java/feign/cache/HttpCacheInterceptorTest.java @@ -18,10 +18,18 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import feign.Client; import feign.Feign; import feign.FeignException; import feign.Param; import feign.RequestLine; +import feign.Response; +import feign.Util; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; import okhttp3.mockwebserver.MockResponse; import okhttp3.mockwebserver.MockWebServer; import okhttp3.mockwebserver.RecordedRequest; @@ -44,6 +52,9 @@ interface Api { @RequestLine("POST /things") String create(String body); + + @RequestLine("QUERY /things") + String query(String body); } private Api api() { @@ -120,6 +131,47 @@ void postRequestsBypassCache() throws Exception { assertThat(recorded.getHeader("If-None-Match")).isNull(); } + @Test + void queryRequestsParticipateInCache() { + AtomicInteger calls = new AtomicInteger(); + Client client = + (request, options) -> { + calls.incrementAndGet(); + if (request.headers().containsKey("If-None-Match")) { + return Response.builder() + .status(304) + .headers(Collections.emptyMap()) + .request(request) + .build(); + } + String body = request.body() != null ? new String(request.body(), Util.UTF_8) : ""; + Map> headers = new HashMap<>(); + headers.put("ETag", Collections.singletonList("\"" + body + "\"")); + return Response.builder() + .status(200) + .headers(headers) + .body("result-" + body, Util.UTF_8) + .request(request) + .build(); + }; + + Api api = + Feign.builder() + .client(client) + .methodInterceptor(new HttpCacheInterceptor(store)) + .target(Api.class, "http://localhost:0"); + + String first = api.query("q1"); + String second = api.query("q2"); + String third = api.query("q1"); + + assertThat(first).isEqualTo("result-q1"); + assertThat(second).isEqualTo("result-q2"); + assertThat(third).isEqualTo("result-q1"); + assertThat(calls.get()).isEqualTo(3); + assertThat(store.size()).isEqualTo(2); + } + @Test void noStoreCacheControlPreventsStorage() throws Exception { server.enqueue( diff --git a/jaxrs2/src/test/java/feign/jaxrs2/AbstractJAXRSClientTest.java b/jaxrs2/src/test/java/feign/jaxrs2/AbstractJAXRSClientTest.java index 21936f0e09..b4dc676fd1 100644 --- a/jaxrs2/src/test/java/feign/jaxrs2/AbstractJAXRSClientTest.java +++ b/jaxrs2/src/test/java/feign/jaxrs2/AbstractJAXRSClientTest.java @@ -44,6 +44,15 @@ public void patch() throws Exception { } } + @Override + public void query() throws Exception { + try { + super.query(); + } catch (final RuntimeException _) { + Assumptions.assumeFalse(false, "JaxRS client do not support QUERY requests"); + } + } + @Override public void noResponseBodyForPut() throws Exception { try { diff --git a/mock/src/main/java/feign/mock/HttpMethod.java b/mock/src/main/java/feign/mock/HttpMethod.java index fa220e88e9..1840feb5f2 100644 --- a/mock/src/main/java/feign/mock/HttpMethod.java +++ b/mock/src/main/java/feign/mock/HttpMethod.java @@ -24,5 +24,6 @@ public enum HttpMethod { TRACE, OPTIONS, CONNECT, - PATCH + PATCH, + QUERY } diff --git a/mock/src/test/java/feign/mock/MockClientTest.java b/mock/src/test/java/feign/mock/MockClientTest.java index 77916f4194..0489a5164a 100644 --- a/mock/src/test/java/feign/mock/MockClientTest.java +++ b/mock/src/test/java/feign/mock/MockClientTest.java @@ -56,6 +56,10 @@ List contributors( @RequestLine("PATCH /repos/{owner}/{repo}/contributors") List patchContributors(@Param("owner") String owner, @Param("repo") String repo); + @RequestLine("QUERY /repos/{owner}/{repo}/contributors") + @Headers("Content-Type: application/json") + List queryContributors(@Param("owner") String owner, @Param("repo") String repo); + @RequestLine("POST /repos/{owner}/{repo}/contributors") @Headers({"Content-Type: application/json"}) @Body("%7B\"login\":\"{login}\",\"type\":\"{type}\"%7D") @@ -171,6 +175,23 @@ void missHttpMethod() { } } + @Test + void queryMock() throws IOException { + try (InputStream input = getClass().getResourceAsStream("/fixtures/contributors.json")) { + byte[] data = toByteArray(input); + MockClient client = + new MockClient().ok(HttpMethod.QUERY, "/repos/netflix/feign/contributors", data); + GitHub api = + Feign.builder() + .decoder(new AssertionDecoder(new GsonDecoder())) + .client(client) + .target(new MockTarget<>(GitHub.class)); + List contributors = api.queryContributors("netflix", "feign"); + assertThat(contributors).hasSize(30); + client.verifyStatus(); + } + } + @Test void paramsEncoding() { List contributors = github.contributors("7 7", "netflix", "feign");