From 19555128176dea80a5f625262eda40b9dfe7e9b2 Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Thu, 23 Sep 2021 15:33:52 -0600 Subject: [PATCH 01/13] Add support for rest compatibility headers to the LLRC This adds support for the headers necessary for REST version compatibility to the Low Level Rest Client (LLRC). Compatibility mode can be turned on either with the `.setAPICompatibilityMode(true)` when creating the client, or by setting the `ELASTIC_CLIENT_APIVERSIONING` to `true` similar to our other Elasticsearch clients. Resolves #77859 --- client/rest/build.gradle | 1 + .../rest/qa/compatibility-used/build.gradle | 20 ++ .../client/EnvCompatibilityTests.java | 179 ++++++++++++++++++ .../org/elasticsearch/client/RestClient.java | 118 +++++++++++- .../client/RestClientBuilder.java | 11 +- .../client/RestClientAPICompatTests.java | 159 ++++++++++++++++ settings.gradle | 1 + 7 files changed, 484 insertions(+), 5 deletions(-) create mode 100644 client/rest/qa/compatibility-used/build.gradle create mode 100644 client/rest/qa/compatibility-used/src/test/java/org/elasticsearch/client/EnvCompatibilityTests.java create mode 100644 client/rest/src/test/java/org/elasticsearch/client/RestClientAPICompatTests.java diff --git a/client/rest/build.gradle b/client/rest/build.gradle index fc0069440f139..64e0301af163f 100644 --- a/client/rest/build.gradle +++ b/client/rest/build.gradle @@ -21,6 +21,7 @@ import org.elasticsearch.gradle.internal.conventions.precommit.LicenseHeadersTas apply plugin: 'elasticsearch.build' apply plugin: 'elasticsearch.publish' +apply plugin: 'elasticsearch.internal-test-artifact' targetCompatibility = JavaVersion.VERSION_1_8 sourceCompatibility = JavaVersion.VERSION_1_8 diff --git a/client/rest/qa/compatibility-used/build.gradle b/client/rest/qa/compatibility-used/build.gradle new file mode 100644 index 0000000000000..f5d6f263ec7b5 --- /dev/null +++ b/client/rest/qa/compatibility-used/build.gradle @@ -0,0 +1,20 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +apply plugin: 'elasticsearch.java' +apply plugin: 'elasticsearch.internal-test-artifact' + +dependencies { + testImplementation project(':client:rest') + testImplementation testArtifact(project(':client:rest')) +} + +// Set the API versioning flag for unit tests +tasks.named('test').configure { + environment 'ELASTIC_CLIENT_APIVERSIONING', 'true' +} diff --git a/client/rest/qa/compatibility-used/src/test/java/org/elasticsearch/client/EnvCompatibilityTests.java b/client/rest/qa/compatibility-used/src/test/java/org/elasticsearch/client/EnvCompatibilityTests.java new file mode 100644 index 0000000000000..7cae1f6f66e28 --- /dev/null +++ b/client/rest/qa/compatibility-used/src/test/java/org/elasticsearch/client/EnvCompatibilityTests.java @@ -0,0 +1,179 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.client; + +import com.sun.net.httpserver.HttpExchange; +import com.sun.net.httpserver.HttpHandler; +import com.sun.net.httpserver.HttpServer; + +import org.apache.http.HttpEntity; +import org.apache.http.HttpHost; +import org.apache.http.entity.ContentType; +import org.apache.http.entity.StringEntity; +import org.elasticsearch.mocksocket.MockHttpServer; +import org.junit.AfterClass; +import org.junit.Assert; +import org.junit.BeforeClass; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.nio.charset.StandardCharsets; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; + +public class EnvCompatibilityTests extends RestClientTestCase { + + private static HttpServer httpServer; + + @BeforeClass + public static void startHttpServer() throws Exception { + httpServer = MockHttpServer.createHttp(new InetSocketAddress(InetAddress.getLoopbackAddress(), 0), 0); + httpServer.createContext("/", new APIHeaderHandler()); + httpServer.start(); + } + + @AfterClass + public static void stopHttpServers() { + httpServer.stop(0); + httpServer = null; + } + + private static class APIHeaderHandler implements HttpHandler { + @Override + public void handle(HttpExchange exchange) throws IOException { + + // Decode body (if any) + String contentType = exchange.getRequestHeaders().getFirst("Content-Type"); + String accept = exchange.getRequestHeaders().getFirst("Accept"); + + ByteArrayOutputStream bao = new ByteArrayOutputStream(); + + // Outputs # + bao.write(String.valueOf(contentType).getBytes(StandardCharsets.UTF_8)); + bao.write('#'); + bao.write(String.valueOf(accept).getBytes(StandardCharsets.UTF_8)); + bao.close(); + + byte[] bytes = bao.toByteArray(); + + exchange.sendResponseHeaders(200, bytes.length); + + exchange.getResponseBody().write(bytes); + exchange.close(); + } + } + + /** Read all bytes of an input stream and close it. */ + private static byte[] readAll(InputStream in) throws IOException { + byte[] buffer = new byte[1024]; + ByteArrayOutputStream bos = new ByteArrayOutputStream(); + int len = 0; + while ((len = in.read(buffer)) > 0) { + bos.write(buffer, 0, len); + } + in.close(); + return bos.toByteArray(); + } + + private RestClient createClient(Boolean apiCompat) { + InetSocketAddress address = httpServer.getAddress(); + RestClientBuilder builder = RestClient.builder(new HttpHost(address.getHostString(), address.getPort(), "http")); + if (apiCompat != null) { + builder.setAPICompatibilityMode(apiCompat); + } + return builder.build(); + } + + public void testAPICompatOff() throws Exception { + RestClient restClient = createClient(false); + + Request request = new Request("GET", "/"); + request.setEntity(new StringEntity("{}", ContentType.APPLICATION_JSON)); + + Response response = restClient.performRequest(request); + + Assert.assertTrue(response.getEntity().getContentLength() > 0); + checkResponse("application/json; charset=UTF-8#null", response); + + request = new Request("GET", "/"); + request.setEntity(new StringEntity("aoeu", ContentType.TEXT_PLAIN)); + + response = restClient.performRequest(request); + + Assert.assertTrue(response.getEntity().getContentLength() > 0); + checkResponse("text/plain; charset=ISO-8859-1#null", response); + + restClient.close(); + } + + public void testAPICompatOn() throws Exception { + RestClient restClient = createClient(true); + + // Send non-compressed request, expect non-compressed response + Request request = new Request("POST", "/"); + request.setEntity(new StringEntity("{}", ContentType.APPLICATION_JSON)); + + Response response = restClient.performRequest(request); + + Assert.assertTrue(response.getEntity().getContentLength() > 0); + checkResponse("application/vnd.elasticsearch+json; compatible-with=7; charset=UTF-8" + + "#application/vnd.elasticsearch+json; compatible-with=7; charset=UTF-8", + response); + + // Test with no entity, the default header should still be added + request = new Request("GET", "/"); + response = restClient.performRequest(request); + Assert.assertTrue(response.getEntity().getContentLength() > 0); + checkResponse("application/vnd.elasticsearch+json; compatible-with=7" + + "#application/vnd.elasticsearch+json; compatible-with=7", + response); + + restClient.close(); + } + + public void testAPICompatOnThroughEnvVariable() throws Exception { + assertThat("expected ENV variable to be set but it was not, Gradle should set this environment variable automatically", + System.getenv("ELASTIC_CLIENT_APIVERSIONING"), equalTo("true")); + RestClient restClient = createClient(null); + + // Send non-compressed request, expect non-compressed response + Request request = new Request("POST", "/"); + request.setEntity(new StringEntity("{}", ContentType.APPLICATION_JSON)); + + Response response = restClient.performRequest(request); + + Assert.assertTrue(response.getEntity().getContentLength() > 0); + checkResponse("application/vnd.elasticsearch+json; compatible-with=7; charset=UTF-8" + + "#application/vnd.elasticsearch+json; compatible-with=7; charset=UTF-8", + response); + + // Test with no entity, the default header should still be added + request = new Request("GET", "/"); + response = restClient.performRequest(request); + Assert.assertTrue(response.getEntity().getContentLength() > 0); + checkResponse("application/vnd.elasticsearch+json; compatible-with=7" + + "#application/vnd.elasticsearch+json; compatible-with=7", + response); + + restClient.close(); + } + + private static void checkResponse(String expected, Response response) throws Exception { + HttpEntity entity = response.getEntity(); + Assert.assertNotNull(entity); + + String content = new String(readAll(entity.getContent()), StandardCharsets.UTF_8); + assertThat(expected, containsString(content)); + } +} diff --git a/client/rest/src/main/java/org/elasticsearch/client/RestClient.java b/client/rest/src/main/java/org/elasticsearch/client/RestClient.java index b684682c412b1..b38a8183d3722 100644 --- a/client/rest/src/main/java/org/elasticsearch/client/RestClient.java +++ b/client/rest/src/main/java/org/elasticsearch/client/RestClient.java @@ -28,9 +28,9 @@ import org.apache.http.HttpResponse; import org.apache.http.client.AuthCache; import org.apache.http.client.ClientProtocolException; +import org.apache.http.client.config.RequestConfig; import org.apache.http.client.entity.GzipCompressingEntity; import org.apache.http.client.entity.GzipDecompressingEntity; -import org.apache.http.client.config.RequestConfig; import org.apache.http.client.methods.HttpEntityEnclosingRequestBase; import org.apache.http.client.methods.HttpHead; import org.apache.http.client.methods.HttpOptions; @@ -51,7 +51,6 @@ import org.apache.http.nio.protocol.HttpAsyncResponseConsumer; import org.apache.http.protocol.HTTP; -import javax.net.ssl.SSLHandshakeException; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.Closeable; @@ -81,6 +80,7 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Collectors; import java.util.zip.GZIPOutputStream; +import javax.net.ssl.SSLHandshakeException; import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.Collections.singletonList; @@ -103,6 +103,10 @@ * Requests can be traced by enabling trace logging for "tracer". The trace logger outputs requests and responses in curl format. */ public class RestClient implements Closeable { + /** + * Environment variable determining whether to send the 7.x compatibility header + */ + public static final String API_VERSIONING_ENV_VARIABLE = "ELASTIC_CLIENT_APIVERSIONING"; private static final Log logger = LogFactory.getLog(RestClient.class); @@ -116,6 +120,7 @@ public class RestClient implements Closeable { private final FailureListener failureListener; private final NodeSelector nodeSelector; private volatile NodeTuple> nodeTuple; + private volatile boolean useCompatibility = false; private final WarningsHandler warningsHandler; private final boolean compressionEnabled; @@ -129,6 +134,9 @@ public class RestClient implements Closeable { this.nodeSelector = nodeSelector; this.warningsHandler = strictDeprecationMode ? WarningsHandler.STRICT : WarningsHandler.PERMISSIVE; this.compressionEnabled = compressionEnabled; + if ("true".equals(System.getenv(API_VERSIONING_ENV_VARIABLE))) { + this.useCompatibility = true; + } setNodes(nodes); } @@ -224,6 +232,19 @@ public synchronized void setNodes(Collection nodes) { this.blacklist.clear(); } + /** + * Configure whether to automatically add version compatibility headers to requests. + * Defaults to off (false) unless the "ELASTIC_CLIENT_APIVERSIONING" + * environment variable has been set to "true". + */ + public void setApiCompatibilityMode(boolean enabled) { + this.useCompatibility = enabled; + } + + public boolean getApiCompatibilityMode() { + return this.useCompatibility; + } + /** * Get the list of nodes that the client knows about. The list is * unmodifiable. @@ -733,6 +754,49 @@ public void remove() { } } + private enum EntityType { + JSON() { + @Override + public String header() { + return "json"; + } + }, + NDJSON() { + @Override + public String header() { + return "x-ndjson"; + } + }, + YAML() { + @Override + public String header() { + return "yaml"; + } + }, + SMILE() { + @Override + public String header() { + return "smile"; + } + }, + CBOR() { + @Override + public String header() { + return "cbor"; + } + }; + + public static final String APPLICATION_PREFIX = "application/"; + public static final String APPLICATION_VND_PREFIX = "application/vnd.elasticsearch+"; + + public abstract String header(); + + @Override + public String toString() { + return header(); + } + } + private class InternalRequest { private final Request request; private final Set ignoreErrorCodes; @@ -750,13 +814,14 @@ private class InternalRequest { URI uri = buildUri(pathPrefix, request.getEndpoint(), params); this.httpRequest = createHttpRequest(request.getMethod(), uri, request.getEntity(), compressionEnabled); this.cancellable = Cancellable.fromRequest(httpRequest); - setHeaders(httpRequest, request.getOptions().getHeaders()); + HttpEntity requestEntity = request.getEntity(); + setHeaders(httpRequest, requestEntity == null ? null : requestEntity.getContentType(), request.getOptions().getHeaders()); setRequestConfig(httpRequest, request.getOptions().getRequestConfig()); this.warningsHandler = request.getOptions().getWarningsHandler() == null ? RestClient.this.warningsHandler : request.getOptions().getWarningsHandler(); } - private void setHeaders(HttpRequest req, Collection
requestHeaders) { + private void setHeaders(HttpRequest req, Header entityHeader, Collection
requestHeaders) { // request headers override default headers, so we don't add default headers if they exist as request headers final Set requestNames = new HashSet<>(requestHeaders.size()); for (Header requestHeader : requestHeaders) { @@ -768,11 +833,56 @@ private void setHeaders(HttpRequest req, Collection
requestHeaders) { req.addHeader(defaultHeader); } } + // Add compatibility request headers if compatibility mode has been enabled + if (useCompatibility) { + addCompatibilityFor(req, entityHeader, "Content-Type"); + addCompatibilityFor(req, entityHeader, "Accept"); + } if (compressionEnabled) { req.addHeader("Accept-Encoding", "gzip"); } } + /** + * Go through all the request's existing headers, looking for {@code headerName} headers and if they exist, + * changing them to use version compatibility. If no request headers are changed, modify the entity type header if appropriate + */ + private void addCompatibilityFor(HttpRequest req, Header entityHeader, String headerName) { + // Modify any existing "Content-Type" headers on the request to use the version compatibility, if available + boolean contentTypeModified = false; + for (Header header : req.getHeaders(headerName)) { + contentTypeModified = contentTypeModified || modifyHeader(req, header, headerName); + } + + // If there were no request-specific headers, modify the request entity's header to be compatible + if (entityHeader != null && contentTypeModified == false) { + contentTypeModified = modifyHeader(req, entityHeader, headerName); + } + + // If there were no changed headers at all, add the default compatibility header + if (contentTypeModified == false) { + req.addHeader(headerName, EntityType.APPLICATION_VND_PREFIX + EntityType.JSON.header() + "; compatible-with=7"); + } + } + + /** + * Modify the given header to be version compatible, if necessary. + * Returns true if a modification was made, false otherwise. + */ + private boolean modifyHeader(HttpRequest req, Header header, String headerName) { + for (EntityType type : EntityType.values()) { + final String headerValue = header.getValue(); + if (headerValue.contains(EntityType.APPLICATION_PREFIX + type.header())) { + String newHeaderValue = headerValue.replace(EntityType.APPLICATION_PREFIX + type.header(), + EntityType.APPLICATION_VND_PREFIX + type + "; compatible-with=7"); + req.removeHeader(header); + req.addHeader(headerName, newHeaderValue); + return true; + } + } + return false; + } + private void setRequestConfig(HttpRequestBase requestBase, RequestConfig requestConfig) { if (requestConfig != null) { requestBase.setConfig(requestConfig); diff --git a/client/rest/src/main/java/org/elasticsearch/client/RestClientBuilder.java b/client/rest/src/main/java/org/elasticsearch/client/RestClientBuilder.java index ed3e7f392a773..67aede52e993a 100644 --- a/client/rest/src/main/java/org/elasticsearch/client/RestClientBuilder.java +++ b/client/rest/src/main/java/org/elasticsearch/client/RestClientBuilder.java @@ -30,7 +30,6 @@ import org.apache.http.protocol.HttpContext; import org.apache.http.util.VersionInfo; -import javax.net.ssl.SSLContext; import java.io.IOException; import java.io.InputStream; import java.security.AccessController; @@ -40,6 +39,7 @@ import java.util.Locale; import java.util.Objects; import java.util.Properties; +import javax.net.ssl.SSLContext; /** * Helps creating a new {@link RestClient}. Allows to set the most common http client configuration options when internally @@ -69,6 +69,7 @@ public final class RestClientBuilder { private boolean strictDeprecationMode = false; private boolean compressionEnabled = false; private boolean metaHeaderEnabled = true; + private Boolean useAPICompatibility = null; static { @@ -245,6 +246,11 @@ public RestClientBuilder setCompressionEnabled(boolean compressionEnabled) { return this; } + public RestClientBuilder setAPICompatibilityMode(Boolean enabled) { + this.useAPICompatibility = enabled; + return this; + } + /** * Whether to send a {@code X-Elastic-Client-Meta} header that describes the runtime environment. It contains * information that is similar to what could be found in {@code User-Agent}. Using a separate header allows @@ -267,6 +273,9 @@ public RestClient build() { (PrivilegedAction) this::createHttpClient); RestClient restClient = new RestClient(httpClient, defaultHeaders, nodes, pathPrefix, failureListener, nodeSelector, strictDeprecationMode, compressionEnabled); + if (this.useAPICompatibility != null) { + restClient.setApiCompatibilityMode(this.useAPICompatibility); + } httpClient.start(); return restClient; } diff --git a/client/rest/src/test/java/org/elasticsearch/client/RestClientAPICompatTests.java b/client/rest/src/test/java/org/elasticsearch/client/RestClientAPICompatTests.java new file mode 100644 index 0000000000000..7201c23b89f08 --- /dev/null +++ b/client/rest/src/test/java/org/elasticsearch/client/RestClientAPICompatTests.java @@ -0,0 +1,159 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. 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. + */ + +package org.elasticsearch.client; + +import com.sun.net.httpserver.HttpExchange; +import com.sun.net.httpserver.HttpHandler; +import com.sun.net.httpserver.HttpServer; + +import org.apache.http.HttpEntity; +import org.apache.http.HttpHost; +import org.apache.http.entity.ContentType; +import org.apache.http.entity.StringEntity; +import org.elasticsearch.mocksocket.MockHttpServer; +import org.junit.AfterClass; +import org.junit.Assert; +import org.junit.BeforeClass; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.nio.charset.StandardCharsets; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; + +public class RestClientAPICompatTests extends RestClientTestCase { + + private static HttpServer httpServer; + + @BeforeClass + public static void startHttpServer() throws Exception { + httpServer = MockHttpServer.createHttp(new InetSocketAddress(InetAddress.getLoopbackAddress(), 0), 0); + httpServer.createContext("/", new APIHeaderHandler()); + httpServer.start(); + } + + @AfterClass + public static void stopHttpServers() { + httpServer.stop(0); + httpServer = null; + } + + private static class APIHeaderHandler implements HttpHandler { + @Override + public void handle(HttpExchange exchange) throws IOException { + + // Decode body (if any) + String contentType = exchange.getRequestHeaders().getFirst("Content-Type"); + String accept = exchange.getRequestHeaders().getFirst("Accept"); + + ByteArrayOutputStream bao = new ByteArrayOutputStream(); + + // Outputs # + bao.write(String.valueOf(contentType).getBytes(StandardCharsets.UTF_8)); + bao.write('#'); + bao.write(String.valueOf(accept).getBytes(StandardCharsets.UTF_8)); + bao.close(); + + byte[] bytes = bao.toByteArray(); + + exchange.sendResponseHeaders(200, bytes.length); + + exchange.getResponseBody().write(bytes); + exchange.close(); + } + } + + /** Read all bytes of an input stream and close it. */ + private static byte[] readAll(InputStream in) throws IOException { + byte[] buffer = new byte[1024]; + ByteArrayOutputStream bos = new ByteArrayOutputStream(); + int len = 0; + while ((len = in.read(buffer)) > 0) { + bos.write(buffer, 0, len); + } + in.close(); + return bos.toByteArray(); + } + + private RestClient createClient(boolean apiCompat) { + InetSocketAddress address = httpServer.getAddress(); + return RestClient.builder(new HttpHost(address.getHostString(), address.getPort(), "http")) + .setAPICompatibilityMode(apiCompat) + .build(); + } + + public void testAPICompatOff() throws Exception { + RestClient restClient = createClient(false); + + Request request = new Request("GET", "/"); + request.setEntity(new StringEntity("{}", ContentType.APPLICATION_JSON)); + + Response response = restClient.performRequest(request); + + Assert.assertTrue(response.getEntity().getContentLength() > 0); + checkResponse("application/json; charset=UTF-8#null", response); + + request = new Request("GET", "/"); + request.setEntity(new StringEntity("aoeu", ContentType.TEXT_PLAIN)); + + response = restClient.performRequest(request); + + Assert.assertTrue(response.getEntity().getContentLength() > 0); + checkResponse("text/plain; charset=ISO-8859-1#null", response); + + restClient.close(); + } + + public void testAPICompatOn() throws Exception { + RestClient restClient = createClient(true); + + Request request = new Request("POST", "/"); + request.setEntity(new StringEntity("{}", ContentType.APPLICATION_JSON)); + + Response response = restClient.performRequest(request); + + Assert.assertTrue(response.getEntity().getContentLength() > 0); + checkResponse("application/vnd.elasticsearch+json; compatible-with=7; charset=UTF-8" + + "#application/vnd.elasticsearch+json; compatible-with=7; charset=UTF-8", + response); + + // Test with no entity, the default header should still be added + request = new Request("GET", "/"); + response = restClient.performRequest(request); + Assert.assertTrue(response.getEntity().getContentLength() > 0); + checkResponse("application/vnd.elasticsearch+json; compatible-with=7" + + "#application/vnd.elasticsearch+json; compatible-with=7", + response); + + restClient.close(); + } + + private static void checkResponse(String expected, Response response) throws Exception { + HttpEntity entity = response.getEntity(); + Assert.assertNotNull(entity); + + String content = new String(readAll(entity.getContent()), StandardCharsets.UTF_8); + assertThat(content, containsString(expected)); + } +} diff --git a/settings.gradle b/settings.gradle index e3c74074eba1b..73a95881f39a9 100644 --- a/settings.gradle +++ b/settings.gradle @@ -19,6 +19,7 @@ List projects = [ 'rest-api-spec', 'docs', 'client:rest', + 'client:rest:qa:compatibility-used', 'client:rest-high-level', 'client:rest-high-level:qa:ssl-enabled', 'client:sniffer', From 23cdd5900514b3f57750480aea5a178f9cf42c3c Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Thu, 30 Sep 2021 08:51:40 -0600 Subject: [PATCH 02/13] Spotless --- .../client/EnvCompatibilityTests.java | 39 ++++++++++++------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/client/rest/qa/compatibility-used/src/test/java/org/elasticsearch/client/EnvCompatibilityTests.java b/client/rest/qa/compatibility-used/src/test/java/org/elasticsearch/client/EnvCompatibilityTests.java index 7cae1f6f66e28..88cf1d1277e62 100644 --- a/client/rest/qa/compatibility-used/src/test/java/org/elasticsearch/client/EnvCompatibilityTests.java +++ b/client/rest/qa/compatibility-used/src/test/java/org/elasticsearch/client/EnvCompatibilityTests.java @@ -88,7 +88,7 @@ private static byte[] readAll(InputStream in) throws IOException { private RestClient createClient(Boolean apiCompat) { InetSocketAddress address = httpServer.getAddress(); - RestClientBuilder builder = RestClient.builder(new HttpHost(address.getHostString(), address.getPort(), "http")); + RestClientBuilder builder = RestClient.builder(new HttpHost(address.getHostString(), address.getPort(), "http")); if (apiCompat != null) { builder.setAPICompatibilityMode(apiCompat); } @@ -127,24 +127,30 @@ public void testAPICompatOn() throws Exception { Response response = restClient.performRequest(request); Assert.assertTrue(response.getEntity().getContentLength() > 0); - checkResponse("application/vnd.elasticsearch+json; compatible-with=7; charset=UTF-8" + - "#application/vnd.elasticsearch+json; compatible-with=7; charset=UTF-8", - response); + checkResponse( + "application/vnd.elasticsearch+json; compatible-with=7; charset=UTF-8" + + "#application/vnd.elasticsearch+json; compatible-with=7; charset=UTF-8", + response + ); // Test with no entity, the default header should still be added request = new Request("GET", "/"); response = restClient.performRequest(request); Assert.assertTrue(response.getEntity().getContentLength() > 0); - checkResponse("application/vnd.elasticsearch+json; compatible-with=7" + - "#application/vnd.elasticsearch+json; compatible-with=7", - response); + checkResponse( + "application/vnd.elasticsearch+json; compatible-with=7" + "#application/vnd.elasticsearch+json; compatible-with=7", + response + ); restClient.close(); } public void testAPICompatOnThroughEnvVariable() throws Exception { - assertThat("expected ENV variable to be set but it was not, Gradle should set this environment variable automatically", - System.getenv("ELASTIC_CLIENT_APIVERSIONING"), equalTo("true")); + assertThat( + "expected ENV variable to be set but it was not, Gradle should set this environment variable automatically", + System.getenv("ELASTIC_CLIENT_APIVERSIONING"), + equalTo("true") + ); RestClient restClient = createClient(null); // Send non-compressed request, expect non-compressed response @@ -154,17 +160,20 @@ public void testAPICompatOnThroughEnvVariable() throws Exception { Response response = restClient.performRequest(request); Assert.assertTrue(response.getEntity().getContentLength() > 0); - checkResponse("application/vnd.elasticsearch+json; compatible-with=7; charset=UTF-8" + - "#application/vnd.elasticsearch+json; compatible-with=7; charset=UTF-8", - response); + checkResponse( + "application/vnd.elasticsearch+json; compatible-with=7; charset=UTF-8" + + "#application/vnd.elasticsearch+json; compatible-with=7; charset=UTF-8", + response + ); // Test with no entity, the default header should still be added request = new Request("GET", "/"); response = restClient.performRequest(request); Assert.assertTrue(response.getEntity().getContentLength() > 0); - checkResponse("application/vnd.elasticsearch+json; compatible-with=7" + - "#application/vnd.elasticsearch+json; compatible-with=7", - response); + checkResponse( + "application/vnd.elasticsearch+json; compatible-with=7" + "#application/vnd.elasticsearch+json; compatible-with=7", + response + ); restClient.close(); } From 66a26026efe39acddbfc71781eb17a83518db9a6 Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Thu, 30 Sep 2021 12:11:03 -0600 Subject: [PATCH 03/13] Fixup RestClient --- .../org/elasticsearch/client/RestClient.java | 64 ++++++++++++++----- .../client/RestClientAPICompatTests.java | 5 +- 2 files changed, 49 insertions(+), 20 deletions(-) diff --git a/client/rest/src/main/java/org/elasticsearch/client/RestClient.java b/client/rest/src/main/java/org/elasticsearch/client/RestClient.java index b38a8183d3722..870f23d43225a 100644 --- a/client/rest/src/main/java/org/elasticsearch/client/RestClient.java +++ b/client/rest/src/main/java/org/elasticsearch/client/RestClient.java @@ -758,38 +758,66 @@ private enum EntityType { JSON() { @Override public String header() { - return "json"; + return "application/json"; + } + @Override + public String compatibleHeader() { + return "application/vnd.elasticsearch+json; compatible-with=7"; } }, NDJSON() { @Override public String header() { - return "x-ndjson"; + return "application/x-ndjson"; + } + @Override + public String compatibleHeader() { + return "application/vnd.elasticsearch+x-ndjson; compatible-with=7"; + } + }, + STAR() { + @Override + public String header() { + return "application/*"; + } + @Override + public String compatibleHeader() { + return "application/vnd.elasticsearch+json; compatible-with=7"; } }, YAML() { @Override public String header() { - return "yaml"; + return "application/yaml"; + } + @Override + public String compatibleHeader() { + return "application/vnd.elasticsearch+yaml; compatible-with=7"; } }, SMILE() { @Override public String header() { - return "smile"; + return "application/smile"; + } + @Override + public String compatibleHeader() { + return "application/vnd.elasticsearch+smile; compatible-with=7"; } }, CBOR() { @Override public String header() { - return "cbor"; + return "application/cbor"; + } + @Override + public String compatibleHeader() { + return "application/vnd.elasticsearch+cbor; compatible-with=7"; } }; - public static final String APPLICATION_PREFIX = "application/"; - public static final String APPLICATION_VND_PREFIX = "application/vnd.elasticsearch+"; - public abstract String header(); + public abstract String compatibleHeader(); @Override public String toString() { @@ -836,7 +864,13 @@ private void setHeaders(HttpRequest req, Header entityHeader, Collection
// Add compatibility request headers if compatibility mode has been enabled if (useCompatibility) { addCompatibilityFor(req, entityHeader, "Content-Type"); - addCompatibilityFor(req, entityHeader, "Accept"); + if (req.containsHeader("Accept")) { + addCompatibilityFor(req, entityHeader, "Accept"); + } else { + // There is no entity, and no existing accept header, but we still need one + // with compatibility, so use the compatible JSON (default output) format + req.addHeader("Accept", EntityType.JSON.compatibleHeader()); + } } if (compressionEnabled) { req.addHeader("Accept-Encoding", "gzip"); @@ -847,7 +881,7 @@ private void setHeaders(HttpRequest req, Header entityHeader, Collection
* Go through all the request's existing headers, looking for {@code headerName} headers and if they exist, * changing them to use version compatibility. If no request headers are changed, modify the entity type header if appropriate */ - private void addCompatibilityFor(HttpRequest req, Header entityHeader, String headerName) { + private boolean addCompatibilityFor(HttpRequest req, Header entityHeader, String headerName) { // Modify any existing "Content-Type" headers on the request to use the version compatibility, if available boolean contentTypeModified = false; for (Header header : req.getHeaders(headerName)) { @@ -859,10 +893,7 @@ private void addCompatibilityFor(HttpRequest req, Header entityHeader, String he contentTypeModified = modifyHeader(req, entityHeader, headerName); } - // If there were no changed headers at all, add the default compatibility header - if (contentTypeModified == false) { - req.addHeader(headerName, EntityType.APPLICATION_VND_PREFIX + EntityType.JSON.header() + "; compatible-with=7"); - } + return contentTypeModified; } /** @@ -872,9 +903,8 @@ private void addCompatibilityFor(HttpRequest req, Header entityHeader, String he private boolean modifyHeader(HttpRequest req, Header header, String headerName) { for (EntityType type : EntityType.values()) { final String headerValue = header.getValue(); - if (headerValue.contains(EntityType.APPLICATION_PREFIX + type.header())) { - String newHeaderValue = headerValue.replace(EntityType.APPLICATION_PREFIX + type.header(), - EntityType.APPLICATION_VND_PREFIX + type + "; compatible-with=7"); + if (headerValue.startsWith(type.header())) { + String newHeaderValue = headerValue.replace(type.header(), type.compatibleHeader()); req.removeHeader(header); req.addHeader(headerName, newHeaderValue); return true; diff --git a/client/rest/src/test/java/org/elasticsearch/client/RestClientAPICompatTests.java b/client/rest/src/test/java/org/elasticsearch/client/RestClientAPICompatTests.java index 7201c23b89f08..5d2b7de5a59cf 100644 --- a/client/rest/src/test/java/org/elasticsearch/client/RestClientAPICompatTests.java +++ b/client/rest/src/test/java/org/elasticsearch/client/RestClientAPICompatTests.java @@ -135,15 +135,14 @@ public void testAPICompatOn() throws Exception { Assert.assertTrue(response.getEntity().getContentLength() > 0); checkResponse("application/vnd.elasticsearch+json; compatible-with=7; charset=UTF-8" + - "#application/vnd.elasticsearch+json; compatible-with=7; charset=UTF-8", + "#application/vnd.elasticsearch+json; compatible-with=7", response); // Test with no entity, the default header should still be added request = new Request("GET", "/"); response = restClient.performRequest(request); Assert.assertTrue(response.getEntity().getContentLength() > 0); - checkResponse("application/vnd.elasticsearch+json; compatible-with=7" + - "#application/vnd.elasticsearch+json; compatible-with=7", + checkResponse("null#application/vnd.elasticsearch+json; compatible-with=7", response); restClient.close(); From 96651a3709422e5ff4489c9ec4a91172db179014 Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Thu, 30 Sep 2021 12:11:10 -0600 Subject: [PATCH 04/13] Add necessary gradle dependency --- client/rest/qa/compatibility-used/build.gradle | 1 + 1 file changed, 1 insertion(+) diff --git a/client/rest/qa/compatibility-used/build.gradle b/client/rest/qa/compatibility-used/build.gradle index f5d6f263ec7b5..0c03d18f61f82 100644 --- a/client/rest/qa/compatibility-used/build.gradle +++ b/client/rest/qa/compatibility-used/build.gradle @@ -12,6 +12,7 @@ apply plugin: 'elasticsearch.internal-test-artifact' dependencies { testImplementation project(':client:rest') testImplementation testArtifact(project(':client:rest')) + testImplementation project(':test:framework') } // Set the API versioning flag for unit tests From d62beac5015a08e85483db7030431b2f39d1be3a Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Thu, 30 Sep 2021 12:23:10 -0600 Subject: [PATCH 05/13] Fix tests --- client/rest/qa/compatibility-used/build.gradle | 1 + .../elasticsearch/client/EnvCompatibilityTests.java | 10 +++++----- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/client/rest/qa/compatibility-used/build.gradle b/client/rest/qa/compatibility-used/build.gradle index 0c03d18f61f82..3dfd7fc22a6cf 100644 --- a/client/rest/qa/compatibility-used/build.gradle +++ b/client/rest/qa/compatibility-used/build.gradle @@ -13,6 +13,7 @@ dependencies { testImplementation project(':client:rest') testImplementation testArtifact(project(':client:rest')) testImplementation project(':test:framework') + testImplementation project(':client:test') } // Set the API versioning flag for unit tests diff --git a/client/rest/qa/compatibility-used/src/test/java/org/elasticsearch/client/EnvCompatibilityTests.java b/client/rest/qa/compatibility-used/src/test/java/org/elasticsearch/client/EnvCompatibilityTests.java index 88cf1d1277e62..a0e62e5374916 100644 --- a/client/rest/qa/compatibility-used/src/test/java/org/elasticsearch/client/EnvCompatibilityTests.java +++ b/client/rest/qa/compatibility-used/src/test/java/org/elasticsearch/client/EnvCompatibilityTests.java @@ -129,7 +129,7 @@ public void testAPICompatOn() throws Exception { Assert.assertTrue(response.getEntity().getContentLength() > 0); checkResponse( "application/vnd.elasticsearch+json; compatible-with=7; charset=UTF-8" - + "#application/vnd.elasticsearch+json; compatible-with=7; charset=UTF-8", + + "#application/vnd.elasticsearch+json; compatible-with=7", response ); @@ -138,7 +138,7 @@ public void testAPICompatOn() throws Exception { response = restClient.performRequest(request); Assert.assertTrue(response.getEntity().getContentLength() > 0); checkResponse( - "application/vnd.elasticsearch+json; compatible-with=7" + "#application/vnd.elasticsearch+json; compatible-with=7", + "null#application/vnd.elasticsearch+json; compatible-with=7", response ); @@ -162,7 +162,7 @@ public void testAPICompatOnThroughEnvVariable() throws Exception { Assert.assertTrue(response.getEntity().getContentLength() > 0); checkResponse( "application/vnd.elasticsearch+json; compatible-with=7; charset=UTF-8" - + "#application/vnd.elasticsearch+json; compatible-with=7; charset=UTF-8", + + "#application/vnd.elasticsearch+json; compatible-with=7", response ); @@ -171,7 +171,7 @@ public void testAPICompatOnThroughEnvVariable() throws Exception { response = restClient.performRequest(request); Assert.assertTrue(response.getEntity().getContentLength() > 0); checkResponse( - "application/vnd.elasticsearch+json; compatible-with=7" + "#application/vnd.elasticsearch+json; compatible-with=7", + "null#application/vnd.elasticsearch+json; compatible-with=7", response ); @@ -183,6 +183,6 @@ private static void checkResponse(String expected, Response response) throws Exc Assert.assertNotNull(entity); String content = new String(readAll(entity.getContent()), StandardCharsets.UTF_8); - assertThat(expected, containsString(content)); + assertThat(content, containsString(expected)); } } From 35cc9f4faac0b331717d391aa9b543a13794d9df Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Thu, 30 Sep 2021 14:33:54 -0600 Subject: [PATCH 06/13] Spotleeeessssss --- .../elasticsearch/client/EnvCompatibilityTests.java | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/client/rest/qa/compatibility-used/src/test/java/org/elasticsearch/client/EnvCompatibilityTests.java b/client/rest/qa/compatibility-used/src/test/java/org/elasticsearch/client/EnvCompatibilityTests.java index a0e62e5374916..1774f94220414 100644 --- a/client/rest/qa/compatibility-used/src/test/java/org/elasticsearch/client/EnvCompatibilityTests.java +++ b/client/rest/qa/compatibility-used/src/test/java/org/elasticsearch/client/EnvCompatibilityTests.java @@ -137,10 +137,7 @@ public void testAPICompatOn() throws Exception { request = new Request("GET", "/"); response = restClient.performRequest(request); Assert.assertTrue(response.getEntity().getContentLength() > 0); - checkResponse( - "null#application/vnd.elasticsearch+json; compatible-with=7", - response - ); + checkResponse("null#application/vnd.elasticsearch+json; compatible-with=7", response); restClient.close(); } @@ -170,10 +167,7 @@ public void testAPICompatOnThroughEnvVariable() throws Exception { request = new Request("GET", "/"); response = restClient.performRequest(request); Assert.assertTrue(response.getEntity().getContentLength() > 0); - checkResponse( - "null#application/vnd.elasticsearch+json; compatible-with=7", - response - ); + checkResponse("null#application/vnd.elasticsearch+json; compatible-with=7", response); restClient.close(); } From 98548b2fd81dc5fa4312d186148c744be6340739 Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Mon, 4 Oct 2021 14:39:56 -0600 Subject: [PATCH 07/13] Remove from low-level client --- .../org/elasticsearch/client/RestClient.java | 145 +----------------- .../client/RestClientBuilder.java | 9 -- 2 files changed, 2 insertions(+), 152 deletions(-) diff --git a/client/rest/src/main/java/org/elasticsearch/client/RestClient.java b/client/rest/src/main/java/org/elasticsearch/client/RestClient.java index 870f23d43225a..c6a4236e9f841 100644 --- a/client/rest/src/main/java/org/elasticsearch/client/RestClient.java +++ b/client/rest/src/main/java/org/elasticsearch/client/RestClient.java @@ -103,11 +103,6 @@ * Requests can be traced by enabling trace logging for "tracer". The trace logger outputs requests and responses in curl format. */ public class RestClient implements Closeable { - /** - * Environment variable determining whether to send the 7.x compatibility header - */ - public static final String API_VERSIONING_ENV_VARIABLE = "ELASTIC_CLIENT_APIVERSIONING"; - private static final Log logger = LogFactory.getLog(RestClient.class); private final CloseableHttpAsyncClient client; @@ -120,7 +115,6 @@ public class RestClient implements Closeable { private final FailureListener failureListener; private final NodeSelector nodeSelector; private volatile NodeTuple> nodeTuple; - private volatile boolean useCompatibility = false; private final WarningsHandler warningsHandler; private final boolean compressionEnabled; @@ -134,9 +128,6 @@ public class RestClient implements Closeable { this.nodeSelector = nodeSelector; this.warningsHandler = strictDeprecationMode ? WarningsHandler.STRICT : WarningsHandler.PERMISSIVE; this.compressionEnabled = compressionEnabled; - if ("true".equals(System.getenv(API_VERSIONING_ENV_VARIABLE))) { - this.useCompatibility = true; - } setNodes(nodes); } @@ -232,19 +223,6 @@ public synchronized void setNodes(Collection nodes) { this.blacklist.clear(); } - /** - * Configure whether to automatically add version compatibility headers to requests. - * Defaults to off (false) unless the "ELASTIC_CLIENT_APIVERSIONING" - * environment variable has been set to "true". - */ - public void setApiCompatibilityMode(boolean enabled) { - this.useCompatibility = enabled; - } - - public boolean getApiCompatibilityMode() { - return this.useCompatibility; - } - /** * Get the list of nodes that the client knows about. The list is * unmodifiable. @@ -754,77 +732,6 @@ public void remove() { } } - private enum EntityType { - JSON() { - @Override - public String header() { - return "application/json"; - } - @Override - public String compatibleHeader() { - return "application/vnd.elasticsearch+json; compatible-with=7"; - } - }, - NDJSON() { - @Override - public String header() { - return "application/x-ndjson"; - } - @Override - public String compatibleHeader() { - return "application/vnd.elasticsearch+x-ndjson; compatible-with=7"; - } - }, - STAR() { - @Override - public String header() { - return "application/*"; - } - @Override - public String compatibleHeader() { - return "application/vnd.elasticsearch+json; compatible-with=7"; - } - }, - YAML() { - @Override - public String header() { - return "application/yaml"; - } - @Override - public String compatibleHeader() { - return "application/vnd.elasticsearch+yaml; compatible-with=7"; - } - }, - SMILE() { - @Override - public String header() { - return "application/smile"; - } - @Override - public String compatibleHeader() { - return "application/vnd.elasticsearch+smile; compatible-with=7"; - } - }, - CBOR() { - @Override - public String header() { - return "application/cbor"; - } - @Override - public String compatibleHeader() { - return "application/vnd.elasticsearch+cbor; compatible-with=7"; - } - }; - - public abstract String header(); - public abstract String compatibleHeader(); - - @Override - public String toString() { - return header(); - } - } - private class InternalRequest { private final Request request; private final Set ignoreErrorCodes; @@ -842,14 +749,13 @@ private class InternalRequest { URI uri = buildUri(pathPrefix, request.getEndpoint(), params); this.httpRequest = createHttpRequest(request.getMethod(), uri, request.getEntity(), compressionEnabled); this.cancellable = Cancellable.fromRequest(httpRequest); - HttpEntity requestEntity = request.getEntity(); - setHeaders(httpRequest, requestEntity == null ? null : requestEntity.getContentType(), request.getOptions().getHeaders()); + setHeaders(httpRequest, request.getOptions().getHeaders()); setRequestConfig(httpRequest, request.getOptions().getRequestConfig()); this.warningsHandler = request.getOptions().getWarningsHandler() == null ? RestClient.this.warningsHandler : request.getOptions().getWarningsHandler(); } - private void setHeaders(HttpRequest req, Header entityHeader, Collection
requestHeaders) { + private void setHeaders(HttpRequest req, Collection
requestHeaders) { // request headers override default headers, so we don't add default headers if they exist as request headers final Set requestNames = new HashSet<>(requestHeaders.size()); for (Header requestHeader : requestHeaders) { @@ -861,58 +767,11 @@ private void setHeaders(HttpRequest req, Header entityHeader, Collection
req.addHeader(defaultHeader); } } - // Add compatibility request headers if compatibility mode has been enabled - if (useCompatibility) { - addCompatibilityFor(req, entityHeader, "Content-Type"); - if (req.containsHeader("Accept")) { - addCompatibilityFor(req, entityHeader, "Accept"); - } else { - // There is no entity, and no existing accept header, but we still need one - // with compatibility, so use the compatible JSON (default output) format - req.addHeader("Accept", EntityType.JSON.compatibleHeader()); - } - } if (compressionEnabled) { req.addHeader("Accept-Encoding", "gzip"); } } - /** - * Go through all the request's existing headers, looking for {@code headerName} headers and if they exist, - * changing them to use version compatibility. If no request headers are changed, modify the entity type header if appropriate - */ - private boolean addCompatibilityFor(HttpRequest req, Header entityHeader, String headerName) { - // Modify any existing "Content-Type" headers on the request to use the version compatibility, if available - boolean contentTypeModified = false; - for (Header header : req.getHeaders(headerName)) { - contentTypeModified = contentTypeModified || modifyHeader(req, header, headerName); - } - - // If there were no request-specific headers, modify the request entity's header to be compatible - if (entityHeader != null && contentTypeModified == false) { - contentTypeModified = modifyHeader(req, entityHeader, headerName); - } - - return contentTypeModified; - } - - /** - * Modify the given header to be version compatible, if necessary. - * Returns true if a modification was made, false otherwise. - */ - private boolean modifyHeader(HttpRequest req, Header header, String headerName) { - for (EntityType type : EntityType.values()) { - final String headerValue = header.getValue(); - if (headerValue.startsWith(type.header())) { - String newHeaderValue = headerValue.replace(type.header(), type.compatibleHeader()); - req.removeHeader(header); - req.addHeader(headerName, newHeaderValue); - return true; - } - } - return false; - } - private void setRequestConfig(HttpRequestBase requestBase, RequestConfig requestConfig) { if (requestConfig != null) { requestBase.setConfig(requestConfig); diff --git a/client/rest/src/main/java/org/elasticsearch/client/RestClientBuilder.java b/client/rest/src/main/java/org/elasticsearch/client/RestClientBuilder.java index 67aede52e993a..27550663b0e3e 100644 --- a/client/rest/src/main/java/org/elasticsearch/client/RestClientBuilder.java +++ b/client/rest/src/main/java/org/elasticsearch/client/RestClientBuilder.java @@ -69,7 +69,6 @@ public final class RestClientBuilder { private boolean strictDeprecationMode = false; private boolean compressionEnabled = false; private boolean metaHeaderEnabled = true; - private Boolean useAPICompatibility = null; static { @@ -246,11 +245,6 @@ public RestClientBuilder setCompressionEnabled(boolean compressionEnabled) { return this; } - public RestClientBuilder setAPICompatibilityMode(Boolean enabled) { - this.useAPICompatibility = enabled; - return this; - } - /** * Whether to send a {@code X-Elastic-Client-Meta} header that describes the runtime environment. It contains * information that is similar to what could be found in {@code User-Agent}. Using a separate header allows @@ -273,9 +267,6 @@ public RestClient build() { (PrivilegedAction) this::createHttpClient); RestClient restClient = new RestClient(httpClient, defaultHeaders, nodes, pathPrefix, failureListener, nodeSelector, strictDeprecationMode, compressionEnabled); - if (this.useAPICompatibility != null) { - restClient.setApiCompatibilityMode(this.useAPICompatibility); - } httpClient.start(); return restClient; } From c55608a589b506cc572a944523a1abeace62dbe8 Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Mon, 4 Oct 2021 15:35:10 -0600 Subject: [PATCH 08/13] WIP move compatibility handler into HLRC instead of LLRC --- .../client/RestHighLevelClient.java | 167 +++++++++++++++++- .../client/RestHighLevelClientBuilder.java | 54 ++++++ .../client/RestHighLevelClientTests.java | 89 ++++++++++ .../elasticsearch/client/RequestOptions.java | 23 +++ .../client/RestClientAPICompatTests.java | 1 - 5 files changed, 331 insertions(+), 3 deletions(-) create mode 100644 client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClientBuilder.java diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java index a742fcb0f1a0f..12ab84f82a0d5 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java @@ -8,7 +8,9 @@ package org.elasticsearch.client; +import org.apache.http.Header; import org.apache.http.HttpEntity; +import org.apache.http.HttpRequest; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.elasticsearch.Build; @@ -253,11 +255,16 @@ public class RestHighLevelClient implements Closeable { private static final Logger logger = LogManager.getLogger(RestHighLevelClient.class); + /** + * Environment variable determining whether to send the 7.x compatibility header + */ + public static final String API_VERSIONING_ENV_VARIABLE = "ELASTIC_CLIENT_APIVERSIONING"; // To be called using performClientRequest and performClientRequestAsync to ensure version compatibility check private final RestClient client; private final NamedXContentRegistry registry; private final CheckedConsumer doClose; + private final boolean useAPICompatibility; /** Do not access directly but through getVersionValidationFuture() */ private volatile ListenableFuture> versionValidationFuture; @@ -310,11 +317,28 @@ protected RestHighLevelClient(RestClientBuilder restClientBuilder, List doClose, List namedXContentEntries) { + this(restClient, doClose, namedXContentEntries, null); + } + + /** + * Creates a {@link RestHighLevelClient} given the low level {@link RestClient} that it should use to perform requests and + * a list of entries that allow to parse custom response sections added to Elasticsearch through plugins. + * This constructor can be called by subclasses in case an externally created low-level REST client needs to be provided. + * The consumer argument allows to control what needs to be done when the {@link #close()} method is called. + * Also subclasses can provide parsers for custom response sections added to Elasticsearch through plugins. + */ + protected RestHighLevelClient(RestClient restClient, CheckedConsumer doClose, + List namedXContentEntries, Boolean useAPICompatibility) { this.client = Objects.requireNonNull(restClient, "restClient must not be null"); this.doClose = Objects.requireNonNull(doClose, "doClose consumer must not be null"); this.registry = new NamedXContentRegistry( - Stream.of(getDefaultNamedXContents().stream(), getProvidedNamedXContents().stream(), namedXContentEntries.stream()) - .flatMap(Function.identity()).collect(toList())); + Stream.of(getDefaultNamedXContents().stream(), getProvidedNamedXContents().stream(), namedXContentEntries.stream()) + .flatMap(Function.identity()).collect(toList())); + if (useAPICompatibility == null && "true".equals(System.getenv(API_VERSIONING_ENV_VARIABLE))) { + this.useAPICompatibility = true; + } else { + this.useAPICompatibility = Boolean.TRUE.equals(useAPICompatibility); + } } /** @@ -2016,7 +2040,82 @@ protected static boolean convertExistsResponse(Response response) { return response.getStatusLine().getStatusCode() == 200; } + private enum EntityType { + JSON() { + @Override + public String header() { + return "application/json"; + } + @Override + public String compatibleHeader() { + return "application/vnd.elasticsearch+json; compatible-with=7"; + } + }, + NDJSON() { + @Override + public String header() { + return "application/x-ndjson"; + } + @Override + public String compatibleHeader() { + return "application/vnd.elasticsearch+x-ndjson; compatible-with=7"; + } + }, + STAR() { + @Override + public String header() { + return "application/*"; + } + @Override + public String compatibleHeader() { + return "application/vnd.elasticsearch+json; compatible-with=7"; + } + }, + YAML() { + @Override + public String header() { + return "application/yaml"; + } + @Override + public String compatibleHeader() { + return "application/vnd.elasticsearch+yaml; compatible-with=7"; + } + }, + SMILE() { + @Override + public String header() { + return "application/smile"; + } + @Override + public String compatibleHeader() { + return "application/vnd.elasticsearch+smile; compatible-with=7"; + } + }, + CBOR() { + @Override + public String header() { + return "application/cbor"; + } + @Override + public String compatibleHeader() { + return "application/vnd.elasticsearch+cbor; compatible-with=7"; + } + }; + + public abstract String header(); + public abstract String compatibleHeader(); + + @Override + public String toString() { + return header(); + } + } + private Cancellable performClientRequestAsync(Request request, ResponseListener listener) { + // Add compatibility request headers if compatibility mode has been enabled + if (this.useAPICompatibility) { + modifyRequestForCompatibility(request); + } ListenableFuture> versionCheck = getVersionValidationFuture(); @@ -2068,7 +2167,71 @@ public void onFailure(Exception e) { return result; }; + + /** + * Go through all the request's existing headers, looking for {@code headerName} headers and if they exist, + * changing them to use version compatibility. If no request headers are changed, modify the entity type header if appropriate + */ + boolean addCompatibilityFor(RequestOptions.Builder newOptions, Header entityHeader, String headerName) { + // Modify any existing "Content-Type" headers on the request to use the version compatibility, if available + boolean contentTypeModified = false; + for (Header header : newOptions.getHeaders()) { + if (headerName.equals(header.getName()) == false) { + continue; + } + contentTypeModified = contentTypeModified || modifyHeader(newOptions, header, headerName); + } + + // If there were no request-specific headers, modify the request entity's header to be compatible + if (entityHeader != null && contentTypeModified == false) { + contentTypeModified = modifyHeader(newOptions, entityHeader, headerName); + } + + return contentTypeModified; + } + + /** + * Modify the given header to be version compatible, if necessary. + * Returns true if a modification was made, false otherwise. + */ + boolean modifyHeader(RequestOptions.Builder newOptions, Header header, String headerName) { + for (EntityType type : EntityType.values()) { + final String headerValue = header.getValue(); + if (headerValue.startsWith(type.header())) { + String newHeaderValue = headerValue.replace(type.header(), type.compatibleHeader()); + newOptions.removeHeader(header.getName()); + newOptions.addHeader(headerName, newHeaderValue); + return true; + } + } + return false; + } + + /** + * Make all necessary changes to support API compatibility for the given request. This includes + * modifying the "Content-Type" and "Accept" headers if present, or modifying the header based + * on the request's entity type. + */ + void modifyRequestForCompatibility(Request request) { + final Header entityHeader = request.getEntity() == null ? null : request.getEntity().getContentType(); + final RequestOptions.Builder newOptions = request.getOptions().toBuilder(); + + addCompatibilityFor(newOptions, entityHeader, "Content-Type"); + if (request.getOptions().containsHeader("Accept")) { + addCompatibilityFor(newOptions, entityHeader, "Accept"); + } else { + // There is no entity, and no existing accept header, but we still need one + // with compatibility, so use the compatible JSON (default output) format + newOptions.addHeader("Accept", EntityType.JSON.compatibleHeader()); + } + request.setOptions(newOptions); + } + private Response performClientRequest(Request request) throws IOException { + // Add compatibility request headers if compatibility mode has been enabled + if (this.useAPICompatibility) { + modifyRequestForCompatibility(request); + } Optional versionValidation; try { diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClientBuilder.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClientBuilder.java new file mode 100644 index 0000000000000..c79d4eaba9ee4 --- /dev/null +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClientBuilder.java @@ -0,0 +1,54 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.client; + +import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.search.SearchRequest; +import org.elasticsearch.common.xcontent.NamedXContentRegistry; +import org.elasticsearch.core.CheckedConsumer; + +import java.io.IOException; +import java.util.Collections; +import java.util.List; + +/** + * Helper to build a {@link RestHighLevelClient}, allowing setting the low-level client that + * should be used as well as whether API compatibility should be used. + */ + +public class RestHighLevelClientBuilder { + private final RestClient restClient; + private CheckedConsumer closeHandler = RestClient::close; + private List namedXContentEntries = Collections.emptyList(); + private Boolean apiCompatibilityMode = null; + + public RestHighLevelClientBuilder(RestClient restClient) { + this.restClient = restClient; + } + + public RestHighLevelClientBuilder closeHandler(CheckedConsumer closeHandler) { + this.closeHandler = closeHandler; + return this; + } + + public RestHighLevelClientBuilder namedXContentEntries(List namedXContentEntries) { + this.namedXContentEntries = namedXContentEntries; + return this; + } + + public RestHighLevelClientBuilder setApiCompatibilityMode(Boolean enabled) { + this.apiCompatibilityMode = enabled; + return this; + } + + public RestHighLevelClient build() { + return new RestHighLevelClient(this.restClient, this.closeHandler, this.namedXContentEntries, this.apiCompatibilityMode); + } +} diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/RestHighLevelClientTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/RestHighLevelClientTests.java index 45384216bd0d5..b83a85adc2e13 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/RestHighLevelClientTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/RestHighLevelClientTests.java @@ -10,6 +10,9 @@ import com.fasterxml.jackson.core.JsonParseException; +import joptsimple.internal.Strings; + +import org.apache.http.Header; import org.apache.http.HttpEntity; import org.apache.http.HttpHost; import org.apache.http.HttpResponse; @@ -19,6 +22,8 @@ import org.apache.http.StatusLine; import org.apache.http.client.methods.HttpGet; import org.apache.http.entity.ContentType; +import org.apache.http.entity.StringEntity; +import org.apache.http.message.BasicHeader; import org.apache.http.message.BasicHttpResponse; import org.apache.http.message.BasicRequestLine; import org.apache.http.message.BasicStatusLine; @@ -139,6 +144,7 @@ import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -147,6 +153,7 @@ import static org.hamcrest.CoreMatchers.endsWith; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.instanceOf; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.hasItems; import static org.mockito.Matchers.any; import static org.mockito.Matchers.argThat; @@ -1226,6 +1233,88 @@ public void testCancellationForwarding() throws Exception { verify(cancellable, times(1)).cancel(); } + public void testModifyHeader() { + RequestOptions.Builder builder = RequestOptions.DEFAULT.toBuilder(); + assertTrue(restHighLevelClient.modifyHeader(builder, + new BasicHeader("Content-Type", "application/json; Charset=UTF-16"), "Content-Type")); + + assertThat(builder.getHeaders().stream().map(h -> h.getName() + "=>" + h.getValue()).collect(Collectors.joining(",")), + containsString("Content-Type=>application/vnd.elasticsearch+json; compatible-with=7; Charset=UTF-16")); + + builder = RequestOptions.DEFAULT.toBuilder(); + assertFalse(restHighLevelClient.modifyHeader(builder, new BasicHeader("Content-Type", "other"), "Content-Type")); + + assertThat(builder.getHeaders().stream().map(h -> h.getName() + "=>" + h.getValue()).collect(Collectors.joining(",")), + equalTo("")); + } + + public void testAddCompatibilityFor() { + RequestOptions.Builder builder = RequestOptions.DEFAULT.toBuilder(); + Header entityHeader = new BasicHeader("Content-Type", "application/json"); + String headerName = "Content-Type"; + + // No request headers, use entity header + assertTrue(restHighLevelClient.addCompatibilityFor(builder, entityHeader, headerName)); + assertThat(builder.getHeaders().stream().map(h -> h.getName() + "=>" + h.getValue()).collect(Collectors.joining(",")), + containsString("Content-Type=>application/vnd.elasticsearch+json; compatible-with=7")); + + // Request has a header, ignore entity header + builder = RequestOptions.DEFAULT.toBuilder().addHeader("Content-Type", "application/yaml Charset=UTF-32"); + assertTrue(restHighLevelClient.addCompatibilityFor(builder, entityHeader, headerName)); + assertThat(builder.getHeaders().stream().map(h -> h.getName() + "=>" + h.getValue()).collect(Collectors.joining(",")), + containsString("Content-Type=>application/vnd.elasticsearch+yaml; compatible-with=7 Charset=UTF-32")); + + // Request has no headers, and no entity, no changes + builder = RequestOptions.DEFAULT.toBuilder(); + assertFalse(restHighLevelClient.addCompatibilityFor(builder, null, headerName)); + assertThat(builder.getHeaders().stream().map(h -> h.getName() + "=>" + h.getValue()).collect(Collectors.joining(",")), + equalTo("")); + } + + public void testModifyForCompatibility() { + final Function allHeaders = r -> + r.getOptions().getHeaders().stream().map(h -> h.getName() + "=>" + h.getValue()).collect(Collectors.joining(",")); + + Request req = new Request("POST", "/"); + + restHighLevelClient.modifyRequestForCompatibility(req); + + assertThat(allHeaders.apply(req), containsString("")); + + // With an entity + req = new Request("POST", "/"); + req.setEntity(new StringEntity("{}", ContentType.APPLICATION_JSON)); + restHighLevelClient.modifyRequestForCompatibility(req); + + assertThat(allHeaders.apply(req), + containsString("Content-Type=>application/vnd.elasticsearch+json; compatible-with=7; charset=UTF-8," + + "Accept=>application/vnd.elasticsearch+json; compatible-with=7")); + + // With "Content-Type" headers already set + req = new Request("POST", "/"); + req.setEntity(new StringEntity("{}", ContentType.TEXT_PLAIN)); + req.setOptions(RequestOptions.DEFAULT.toBuilder().addHeader("Content-Type", "application/json; Charset=UTF-16")); + restHighLevelClient.modifyRequestForCompatibility(req); + + assertThat(allHeaders.apply(req), + containsString("Content-Type=>application/vnd.elasticsearch+json; compatible-with=7; Charset=UTF-16," + + "Accept=>application/vnd.elasticsearch+json; compatible-with=7")); + + // With "Content-Type" and "Accept" headers already set + req = new Request("POST", "/"); + req.setEntity(new StringEntity("{}", ContentType.TEXT_PLAIN)); + req.setOptions(RequestOptions.DEFAULT.toBuilder() + .addHeader("Content-Type", "application/json; Charset=UTF-16") + .addHeader("Accept", "application/yaml; Charset=UTF-32")); + // TODO: Fixme, ConcurrentModificationException here + restHighLevelClient.modifyRequestForCompatibility(req); + + assertThat(allHeaders.apply(req), + containsString("Content-Type=>application/vnd.elasticsearch+json; compatible-with=7; Charset=UTF-16," + + "Accept=>application/vnd.elasticsearch+yaml; compatible-with=7; Charset=UTF-32")); + + } + private static void assertSyncMethod(Method method, String apiName, List booleanReturnMethods) { //A few methods return a boolean rather than a response object if (apiName.equals("ping") || apiName.contains("exist") || booleanReturnMethods.contains(apiName)) { diff --git a/client/rest/src/main/java/org/elasticsearch/client/RequestOptions.java b/client/rest/src/main/java/org/elasticsearch/client/RequestOptions.java index c5303cb18e055..3bf0386e7cc15 100644 --- a/client/rest/src/main/java/org/elasticsearch/client/RequestOptions.java +++ b/client/rest/src/main/java/org/elasticsearch/client/RequestOptions.java @@ -71,6 +71,13 @@ public List
getHeaders() { return headers; } + /** + * Return true if the options contain the given header + */ + public boolean containsHeader(String name) { + return headers.stream().anyMatch(h -> name.equals(h.getName())); + } + public Map getParameters() { return parameters; } @@ -202,6 +209,22 @@ public Builder addHeader(String name, String value) { return this; } + /** + * Remove all headers with the given name. + */ + public Builder removeHeader(String name) { + Objects.requireNonNull(name, "header name cannot be null"); + this.headers.removeIf(h -> name.equals(h.getName())); + return this; + } + + /** + * Return all headers for the request + */ + public List
getHeaders() { + return this.headers; + } + /** * Add the provided parameter to the request. */ diff --git a/client/rest/src/test/java/org/elasticsearch/client/RestClientAPICompatTests.java b/client/rest/src/test/java/org/elasticsearch/client/RestClientAPICompatTests.java index 5d2b7de5a59cf..eba1ed124c82a 100644 --- a/client/rest/src/test/java/org/elasticsearch/client/RestClientAPICompatTests.java +++ b/client/rest/src/test/java/org/elasticsearch/client/RestClientAPICompatTests.java @@ -99,7 +99,6 @@ private static byte[] readAll(InputStream in) throws IOException { private RestClient createClient(boolean apiCompat) { InetSocketAddress address = httpServer.getAddress(); return RestClient.builder(new HttpHost(address.getHostString(), address.getPort(), "http")) - .setAPICompatibilityMode(apiCompat) .build(); } From 3f89e934d6a54c63d790e7075a6ee0f3d4fb58fa Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Tue, 5 Oct 2021 16:44:57 -0600 Subject: [PATCH 09/13] Use equalsIgnoreCase for header comparison --- .../client/RestHighLevelClient.java | 21 +++++++++---------- .../elasticsearch/client/RequestOptions.java | 4 ++-- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java index 12ab84f82a0d5..9791b95010bcd 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java @@ -10,7 +10,6 @@ import org.apache.http.Header; import org.apache.http.HttpEntity; -import org.apache.http.HttpRequest; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.elasticsearch.Build; @@ -69,17 +68,17 @@ import org.elasticsearch.client.core.TermVectorsRequest; import org.elasticsearch.client.core.TermVectorsResponse; import org.elasticsearch.client.tasks.TaskSubmissionResponse; -import org.elasticsearch.common.util.concurrent.FutureUtils; -import org.elasticsearch.core.CheckedConsumer; -import org.elasticsearch.core.CheckedFunction; -import org.elasticsearch.common.xcontent.ParseField; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.util.concurrent.FutureUtils; import org.elasticsearch.common.util.concurrent.ListenableFuture; import org.elasticsearch.common.xcontent.ContextParser; import org.elasticsearch.common.xcontent.DeprecationHandler; import org.elasticsearch.common.xcontent.NamedXContentRegistry; +import org.elasticsearch.common.xcontent.ParseField; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.core.CheckedConsumer; +import org.elasticsearch.core.CheckedFunction; import org.elasticsearch.index.rankeval.RankEvalRequest; import org.elasticsearch.index.rankeval.RankEvalResponse; import org.elasticsearch.index.reindex.BulkByScrollResponse; @@ -132,18 +131,18 @@ import org.elasticsearch.search.aggregations.bucket.range.RangeAggregationBuilder; import org.elasticsearch.search.aggregations.bucket.sampler.InternalSampler; import org.elasticsearch.search.aggregations.bucket.sampler.ParsedSampler; +import org.elasticsearch.search.aggregations.bucket.terms.DoubleTerms; import org.elasticsearch.search.aggregations.bucket.terms.LongRareTerms; +import org.elasticsearch.search.aggregations.bucket.terms.LongTerms; +import org.elasticsearch.search.aggregations.bucket.terms.ParsedDoubleTerms; import org.elasticsearch.search.aggregations.bucket.terms.ParsedLongRareTerms; +import org.elasticsearch.search.aggregations.bucket.terms.ParsedLongTerms; import org.elasticsearch.search.aggregations.bucket.terms.ParsedSignificantLongTerms; import org.elasticsearch.search.aggregations.bucket.terms.ParsedSignificantStringTerms; import org.elasticsearch.search.aggregations.bucket.terms.ParsedStringRareTerms; +import org.elasticsearch.search.aggregations.bucket.terms.ParsedStringTerms; import org.elasticsearch.search.aggregations.bucket.terms.SignificantLongTerms; import org.elasticsearch.search.aggregations.bucket.terms.SignificantStringTerms; -import org.elasticsearch.search.aggregations.bucket.terms.DoubleTerms; -import org.elasticsearch.search.aggregations.bucket.terms.LongTerms; -import org.elasticsearch.search.aggregations.bucket.terms.ParsedDoubleTerms; -import org.elasticsearch.search.aggregations.bucket.terms.ParsedLongTerms; -import org.elasticsearch.search.aggregations.bucket.terms.ParsedStringTerms; import org.elasticsearch.search.aggregations.bucket.terms.StringRareTerms; import org.elasticsearch.search.aggregations.bucket.terms.StringTerms; import org.elasticsearch.search.aggregations.metrics.AvgAggregationBuilder; @@ -2176,7 +2175,7 @@ boolean addCompatibilityFor(RequestOptions.Builder newOptions, Header entityHea // Modify any existing "Content-Type" headers on the request to use the version compatibility, if available boolean contentTypeModified = false; for (Header header : newOptions.getHeaders()) { - if (headerName.equals(header.getName()) == false) { + if (headerName.equalsIgnoreCase(header.getName()) == false) { continue; } contentTypeModified = contentTypeModified || modifyHeader(newOptions, header, headerName); diff --git a/client/rest/src/main/java/org/elasticsearch/client/RequestOptions.java b/client/rest/src/main/java/org/elasticsearch/client/RequestOptions.java index 3bf0386e7cc15..6ddd3fa557966 100644 --- a/client/rest/src/main/java/org/elasticsearch/client/RequestOptions.java +++ b/client/rest/src/main/java/org/elasticsearch/client/RequestOptions.java @@ -75,7 +75,7 @@ public List
getHeaders() { * Return true if the options contain the given header */ public boolean containsHeader(String name) { - return headers.stream().anyMatch(h -> name.equals(h.getName())); + return headers.stream().anyMatch(h -> name.equalsIgnoreCase(h.getName())); } public Map getParameters() { @@ -214,7 +214,7 @@ public Builder addHeader(String name, String value) { */ public Builder removeHeader(String name) { Objects.requireNonNull(name, "header name cannot be null"); - this.headers.removeIf(h -> name.equals(h.getName())); + this.headers.removeIf(h -> name.equalsIgnoreCase(h.getName())); return this; } From ef743bac109b8246d30ea9c7c84c1c56eee75911 Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Wed, 6 Oct 2021 10:09:38 -0600 Subject: [PATCH 10/13] Copy headers before modifying them --- .../main/java/org/elasticsearch/client/RestHighLevelClient.java | 2 +- .../java/org/elasticsearch/client/RestHighLevelClientTests.java | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java index 9791b95010bcd..d511aab047bfe 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java @@ -2174,7 +2174,7 @@ public void onFailure(Exception e) { boolean addCompatibilityFor(RequestOptions.Builder newOptions, Header entityHeader, String headerName) { // Modify any existing "Content-Type" headers on the request to use the version compatibility, if available boolean contentTypeModified = false; - for (Header header : newOptions.getHeaders()) { + for (Header header : new ArrayList<>(newOptions.getHeaders())) { if (headerName.equalsIgnoreCase(header.getName()) == false) { continue; } diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/RestHighLevelClientTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/RestHighLevelClientTests.java index b83a85adc2e13..b85b8b8897c10 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/RestHighLevelClientTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/RestHighLevelClientTests.java @@ -1306,7 +1306,6 @@ public void testModifyForCompatibility() { req.setOptions(RequestOptions.DEFAULT.toBuilder() .addHeader("Content-Type", "application/json; Charset=UTF-16") .addHeader("Accept", "application/yaml; Charset=UTF-32")); - // TODO: Fixme, ConcurrentModificationException here restHighLevelClient.modifyRequestForCompatibility(req); assertThat(allHeaders.apply(req), From 2604955e0de14079db90766581ae12881f02c930 Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Wed, 6 Oct 2021 14:33:38 -0600 Subject: [PATCH 11/13] Remove LLRC tests --- client/rest/build.gradle | 1 - .../rest/qa/compatibility-used/build.gradle | 22 --- .../client/EnvCompatibilityTests.java | 182 ------------------ .../client/RestClientAPICompatTests.java | 157 --------------- settings.gradle | 1 - 5 files changed, 363 deletions(-) delete mode 100644 client/rest/qa/compatibility-used/build.gradle delete mode 100644 client/rest/qa/compatibility-used/src/test/java/org/elasticsearch/client/EnvCompatibilityTests.java delete mode 100644 client/rest/src/test/java/org/elasticsearch/client/RestClientAPICompatTests.java diff --git a/client/rest/build.gradle b/client/rest/build.gradle index 64e0301af163f..fc0069440f139 100644 --- a/client/rest/build.gradle +++ b/client/rest/build.gradle @@ -21,7 +21,6 @@ import org.elasticsearch.gradle.internal.conventions.precommit.LicenseHeadersTas apply plugin: 'elasticsearch.build' apply plugin: 'elasticsearch.publish' -apply plugin: 'elasticsearch.internal-test-artifact' targetCompatibility = JavaVersion.VERSION_1_8 sourceCompatibility = JavaVersion.VERSION_1_8 diff --git a/client/rest/qa/compatibility-used/build.gradle b/client/rest/qa/compatibility-used/build.gradle deleted file mode 100644 index 3dfd7fc22a6cf..0000000000000 --- a/client/rest/qa/compatibility-used/build.gradle +++ /dev/null @@ -1,22 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0 and the Server Side Public License, v 1; you may not use this file except - * in compliance with, at your election, the Elastic License 2.0 or the Server - * Side Public License, v 1. - */ - -apply plugin: 'elasticsearch.java' -apply plugin: 'elasticsearch.internal-test-artifact' - -dependencies { - testImplementation project(':client:rest') - testImplementation testArtifact(project(':client:rest')) - testImplementation project(':test:framework') - testImplementation project(':client:test') -} - -// Set the API versioning flag for unit tests -tasks.named('test').configure { - environment 'ELASTIC_CLIENT_APIVERSIONING', 'true' -} diff --git a/client/rest/qa/compatibility-used/src/test/java/org/elasticsearch/client/EnvCompatibilityTests.java b/client/rest/qa/compatibility-used/src/test/java/org/elasticsearch/client/EnvCompatibilityTests.java deleted file mode 100644 index 1774f94220414..0000000000000 --- a/client/rest/qa/compatibility-used/src/test/java/org/elasticsearch/client/EnvCompatibilityTests.java +++ /dev/null @@ -1,182 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0 and the Server Side Public License, v 1; you may not use this file except - * in compliance with, at your election, the Elastic License 2.0 or the Server - * Side Public License, v 1. - */ - -package org.elasticsearch.client; - -import com.sun.net.httpserver.HttpExchange; -import com.sun.net.httpserver.HttpHandler; -import com.sun.net.httpserver.HttpServer; - -import org.apache.http.HttpEntity; -import org.apache.http.HttpHost; -import org.apache.http.entity.ContentType; -import org.apache.http.entity.StringEntity; -import org.elasticsearch.mocksocket.MockHttpServer; -import org.junit.AfterClass; -import org.junit.Assert; -import org.junit.BeforeClass; - -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.io.InputStream; -import java.net.InetAddress; -import java.net.InetSocketAddress; -import java.nio.charset.StandardCharsets; - -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.equalTo; - -public class EnvCompatibilityTests extends RestClientTestCase { - - private static HttpServer httpServer; - - @BeforeClass - public static void startHttpServer() throws Exception { - httpServer = MockHttpServer.createHttp(new InetSocketAddress(InetAddress.getLoopbackAddress(), 0), 0); - httpServer.createContext("/", new APIHeaderHandler()); - httpServer.start(); - } - - @AfterClass - public static void stopHttpServers() { - httpServer.stop(0); - httpServer = null; - } - - private static class APIHeaderHandler implements HttpHandler { - @Override - public void handle(HttpExchange exchange) throws IOException { - - // Decode body (if any) - String contentType = exchange.getRequestHeaders().getFirst("Content-Type"); - String accept = exchange.getRequestHeaders().getFirst("Accept"); - - ByteArrayOutputStream bao = new ByteArrayOutputStream(); - - // Outputs # - bao.write(String.valueOf(contentType).getBytes(StandardCharsets.UTF_8)); - bao.write('#'); - bao.write(String.valueOf(accept).getBytes(StandardCharsets.UTF_8)); - bao.close(); - - byte[] bytes = bao.toByteArray(); - - exchange.sendResponseHeaders(200, bytes.length); - - exchange.getResponseBody().write(bytes); - exchange.close(); - } - } - - /** Read all bytes of an input stream and close it. */ - private static byte[] readAll(InputStream in) throws IOException { - byte[] buffer = new byte[1024]; - ByteArrayOutputStream bos = new ByteArrayOutputStream(); - int len = 0; - while ((len = in.read(buffer)) > 0) { - bos.write(buffer, 0, len); - } - in.close(); - return bos.toByteArray(); - } - - private RestClient createClient(Boolean apiCompat) { - InetSocketAddress address = httpServer.getAddress(); - RestClientBuilder builder = RestClient.builder(new HttpHost(address.getHostString(), address.getPort(), "http")); - if (apiCompat != null) { - builder.setAPICompatibilityMode(apiCompat); - } - return builder.build(); - } - - public void testAPICompatOff() throws Exception { - RestClient restClient = createClient(false); - - Request request = new Request("GET", "/"); - request.setEntity(new StringEntity("{}", ContentType.APPLICATION_JSON)); - - Response response = restClient.performRequest(request); - - Assert.assertTrue(response.getEntity().getContentLength() > 0); - checkResponse("application/json; charset=UTF-8#null", response); - - request = new Request("GET", "/"); - request.setEntity(new StringEntity("aoeu", ContentType.TEXT_PLAIN)); - - response = restClient.performRequest(request); - - Assert.assertTrue(response.getEntity().getContentLength() > 0); - checkResponse("text/plain; charset=ISO-8859-1#null", response); - - restClient.close(); - } - - public void testAPICompatOn() throws Exception { - RestClient restClient = createClient(true); - - // Send non-compressed request, expect non-compressed response - Request request = new Request("POST", "/"); - request.setEntity(new StringEntity("{}", ContentType.APPLICATION_JSON)); - - Response response = restClient.performRequest(request); - - Assert.assertTrue(response.getEntity().getContentLength() > 0); - checkResponse( - "application/vnd.elasticsearch+json; compatible-with=7; charset=UTF-8" - + "#application/vnd.elasticsearch+json; compatible-with=7", - response - ); - - // Test with no entity, the default header should still be added - request = new Request("GET", "/"); - response = restClient.performRequest(request); - Assert.assertTrue(response.getEntity().getContentLength() > 0); - checkResponse("null#application/vnd.elasticsearch+json; compatible-with=7", response); - - restClient.close(); - } - - public void testAPICompatOnThroughEnvVariable() throws Exception { - assertThat( - "expected ENV variable to be set but it was not, Gradle should set this environment variable automatically", - System.getenv("ELASTIC_CLIENT_APIVERSIONING"), - equalTo("true") - ); - RestClient restClient = createClient(null); - - // Send non-compressed request, expect non-compressed response - Request request = new Request("POST", "/"); - request.setEntity(new StringEntity("{}", ContentType.APPLICATION_JSON)); - - Response response = restClient.performRequest(request); - - Assert.assertTrue(response.getEntity().getContentLength() > 0); - checkResponse( - "application/vnd.elasticsearch+json; compatible-with=7; charset=UTF-8" - + "#application/vnd.elasticsearch+json; compatible-with=7", - response - ); - - // Test with no entity, the default header should still be added - request = new Request("GET", "/"); - response = restClient.performRequest(request); - Assert.assertTrue(response.getEntity().getContentLength() > 0); - checkResponse("null#application/vnd.elasticsearch+json; compatible-with=7", response); - - restClient.close(); - } - - private static void checkResponse(String expected, Response response) throws Exception { - HttpEntity entity = response.getEntity(); - Assert.assertNotNull(entity); - - String content = new String(readAll(entity.getContent()), StandardCharsets.UTF_8); - assertThat(content, containsString(expected)); - } -} diff --git a/client/rest/src/test/java/org/elasticsearch/client/RestClientAPICompatTests.java b/client/rest/src/test/java/org/elasticsearch/client/RestClientAPICompatTests.java deleted file mode 100644 index eba1ed124c82a..0000000000000 --- a/client/rest/src/test/java/org/elasticsearch/client/RestClientAPICompatTests.java +++ /dev/null @@ -1,157 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. 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. - */ - -package org.elasticsearch.client; - -import com.sun.net.httpserver.HttpExchange; -import com.sun.net.httpserver.HttpHandler; -import com.sun.net.httpserver.HttpServer; - -import org.apache.http.HttpEntity; -import org.apache.http.HttpHost; -import org.apache.http.entity.ContentType; -import org.apache.http.entity.StringEntity; -import org.elasticsearch.mocksocket.MockHttpServer; -import org.junit.AfterClass; -import org.junit.Assert; -import org.junit.BeforeClass; - -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.io.InputStream; -import java.net.InetAddress; -import java.net.InetSocketAddress; -import java.nio.charset.StandardCharsets; - -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.containsString; - -public class RestClientAPICompatTests extends RestClientTestCase { - - private static HttpServer httpServer; - - @BeforeClass - public static void startHttpServer() throws Exception { - httpServer = MockHttpServer.createHttp(new InetSocketAddress(InetAddress.getLoopbackAddress(), 0), 0); - httpServer.createContext("/", new APIHeaderHandler()); - httpServer.start(); - } - - @AfterClass - public static void stopHttpServers() { - httpServer.stop(0); - httpServer = null; - } - - private static class APIHeaderHandler implements HttpHandler { - @Override - public void handle(HttpExchange exchange) throws IOException { - - // Decode body (if any) - String contentType = exchange.getRequestHeaders().getFirst("Content-Type"); - String accept = exchange.getRequestHeaders().getFirst("Accept"); - - ByteArrayOutputStream bao = new ByteArrayOutputStream(); - - // Outputs # - bao.write(String.valueOf(contentType).getBytes(StandardCharsets.UTF_8)); - bao.write('#'); - bao.write(String.valueOf(accept).getBytes(StandardCharsets.UTF_8)); - bao.close(); - - byte[] bytes = bao.toByteArray(); - - exchange.sendResponseHeaders(200, bytes.length); - - exchange.getResponseBody().write(bytes); - exchange.close(); - } - } - - /** Read all bytes of an input stream and close it. */ - private static byte[] readAll(InputStream in) throws IOException { - byte[] buffer = new byte[1024]; - ByteArrayOutputStream bos = new ByteArrayOutputStream(); - int len = 0; - while ((len = in.read(buffer)) > 0) { - bos.write(buffer, 0, len); - } - in.close(); - return bos.toByteArray(); - } - - private RestClient createClient(boolean apiCompat) { - InetSocketAddress address = httpServer.getAddress(); - return RestClient.builder(new HttpHost(address.getHostString(), address.getPort(), "http")) - .build(); - } - - public void testAPICompatOff() throws Exception { - RestClient restClient = createClient(false); - - Request request = new Request("GET", "/"); - request.setEntity(new StringEntity("{}", ContentType.APPLICATION_JSON)); - - Response response = restClient.performRequest(request); - - Assert.assertTrue(response.getEntity().getContentLength() > 0); - checkResponse("application/json; charset=UTF-8#null", response); - - request = new Request("GET", "/"); - request.setEntity(new StringEntity("aoeu", ContentType.TEXT_PLAIN)); - - response = restClient.performRequest(request); - - Assert.assertTrue(response.getEntity().getContentLength() > 0); - checkResponse("text/plain; charset=ISO-8859-1#null", response); - - restClient.close(); - } - - public void testAPICompatOn() throws Exception { - RestClient restClient = createClient(true); - - Request request = new Request("POST", "/"); - request.setEntity(new StringEntity("{}", ContentType.APPLICATION_JSON)); - - Response response = restClient.performRequest(request); - - Assert.assertTrue(response.getEntity().getContentLength() > 0); - checkResponse("application/vnd.elasticsearch+json; compatible-with=7; charset=UTF-8" + - "#application/vnd.elasticsearch+json; compatible-with=7", - response); - - // Test with no entity, the default header should still be added - request = new Request("GET", "/"); - response = restClient.performRequest(request); - Assert.assertTrue(response.getEntity().getContentLength() > 0); - checkResponse("null#application/vnd.elasticsearch+json; compatible-with=7", - response); - - restClient.close(); - } - - private static void checkResponse(String expected, Response response) throws Exception { - HttpEntity entity = response.getEntity(); - Assert.assertNotNull(entity); - - String content = new String(readAll(entity.getContent()), StandardCharsets.UTF_8); - assertThat(content, containsString(expected)); - } -} diff --git a/settings.gradle b/settings.gradle index 73a95881f39a9..e3c74074eba1b 100644 --- a/settings.gradle +++ b/settings.gradle @@ -19,7 +19,6 @@ List projects = [ 'rest-api-spec', 'docs', 'client:rest', - 'client:rest:qa:compatibility-used', 'client:rest-high-level', 'client:rest-high-level:qa:ssl-enabled', 'client:sniffer', From 1cbbc3c8b7ab2865a6860ca47ad7acd687559dd7 Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Wed, 6 Oct 2021 14:35:50 -0600 Subject: [PATCH 12/13] Fix precommit --- .../org/elasticsearch/client/RestHighLevelClientBuilder.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClientBuilder.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClientBuilder.java index c79d4eaba9ee4..df2724f1f1982 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClientBuilder.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClientBuilder.java @@ -8,9 +8,6 @@ package org.elasticsearch.client; -import org.elasticsearch.ElasticsearchException; -import org.elasticsearch.action.ActionListener; -import org.elasticsearch.action.search.SearchRequest; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.core.CheckedConsumer; From fc47e1b9c97bd19f69016747ea1595d87a50f2b0 Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Wed, 6 Oct 2021 14:37:21 -0600 Subject: [PATCH 13/13] Remove unused import --- .../org/elasticsearch/client/RestHighLevelClientTests.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/RestHighLevelClientTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/RestHighLevelClientTests.java index b85b8b8897c10..4f7511684f493 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/RestHighLevelClientTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/RestHighLevelClientTests.java @@ -10,8 +10,6 @@ import com.fasterxml.jackson.core.JsonParseException; -import joptsimple.internal.Strings; - import org.apache.http.Header; import org.apache.http.HttpEntity; import org.apache.http.HttpHost; @@ -93,9 +91,7 @@ import org.elasticsearch.client.transform.transforms.TimeRetentionPolicyConfig; import org.elasticsearch.client.transform.transforms.TimeSyncConfig; import org.elasticsearch.cluster.ClusterName; -import org.elasticsearch.core.CheckedFunction; import org.elasticsearch.common.bytes.BytesReference; -import org.elasticsearch.core.Tuple; import org.elasticsearch.common.util.set.Sets; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.ToXContent; @@ -105,6 +101,8 @@ import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.common.xcontent.cbor.CborXContent; import org.elasticsearch.common.xcontent.smile.SmileXContent; +import org.elasticsearch.core.CheckedFunction; +import org.elasticsearch.core.Tuple; import org.elasticsearch.index.rankeval.DiscountedCumulativeGain; import org.elasticsearch.index.rankeval.EvaluationMetric; import org.elasticsearch.index.rankeval.ExpectedReciprocalRank;