From 335c020cd77a18da46e59e73e08eea8a53303dd3 Mon Sep 17 00:00:00 2001 From: Chris Earle Date: Sat, 27 Aug 2016 01:12:55 -0400 Subject: [PATCH 01/10] Add support for a RestClient base path This enables simple support for proxies (beyond proxy host and proxy port, which is done via the RequestConfig)) to provide a base path in front of all requests performed by the RestClient. --- .../org/elasticsearch/client/RestClient.java | 21 ++++-- .../client/RestClientBuilder.java | 56 +++++++++++++++- .../client/RestClientBuilderTests.java | 59 +++++++++++++---- .../client/RestClientIntegTests.java | 64 +++++++++++++++++++ .../client/RestClientMultipleHostsTests.java | 2 +- .../client/RestClientSingleHostTests.java | 2 +- 6 files changed, 184 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 a6c3d82dd8f..26af479f668 100644 --- a/client/rest/src/main/java/org/elasticsearch/client/RestClient.java +++ b/client/rest/src/main/java/org/elasticsearch/client/RestClient.java @@ -89,17 +89,19 @@ public class RestClient implements Closeable { //we don't rely on default headers supported by HttpAsyncClient as those cannot be replaced private final Header[] defaultHeaders; private final long maxRetryTimeoutMillis; + private final String pathPrefix; private final AtomicInteger lastHostIndex = new AtomicInteger(0); private volatile Set hosts; private final ConcurrentMap blacklist = new ConcurrentHashMap<>(); private final FailureListener failureListener; RestClient(CloseableHttpAsyncClient client, long maxRetryTimeoutMillis, Header[] defaultHeaders, - HttpHost[] hosts, FailureListener failureListener) { + HttpHost[] hosts, String pathPrefix, FailureListener failureListener) { this.client = client; this.maxRetryTimeoutMillis = maxRetryTimeoutMillis; this.defaultHeaders = defaultHeaders; this.failureListener = failureListener; + this.pathPrefix = pathPrefix; setHosts(hosts); } @@ -280,7 +282,7 @@ public class RestClient implements Closeable { public void performRequestAsync(String method, String endpoint, Map params, HttpEntity entity, HttpAsyncResponseConsumer responseConsumer, ResponseListener responseListener, Header... headers) { - URI uri = buildUri(endpoint, params); + URI uri = buildUri(pathPrefix, endpoint, params); HttpRequestBase request = createHttpRequest(method, uri, entity); setHeaders(request, headers); FailureTrackingResponseListener failureTrackingResponseListener = new FailureTrackingResponseListener(responseListener); @@ -501,10 +503,21 @@ public class RestClient implements Closeable { return httpRequest; } - private static URI buildUri(String path, Map params) { + private static URI buildUri(String pathPrefix, String path, Map params) { Objects.requireNonNull(params, "params must not be null"); try { - URIBuilder uriBuilder = new URIBuilder(path); + String fullPath; + if (pathPrefix != null) { + if (path.startsWith("/")) { + fullPath = pathPrefix + path; + } else { + fullPath = pathPrefix + "/" + path; + } + } else { + fullPath = path; + } + + URIBuilder uriBuilder = new URIBuilder(fullPath); for (Map.Entry param : params.entrySet()) { uriBuilder.addParameter(param.getKey(), param.getValue()); } 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 4d5b72eba49..4f9f379d08e 100644 --- a/client/rest/src/main/java/org/elasticsearch/client/RestClientBuilder.java +++ b/client/rest/src/main/java/org/elasticsearch/client/RestClientBuilder.java @@ -51,12 +51,17 @@ public final class RestClientBuilder { private RestClient.FailureListener failureListener; private HttpClientConfigCallback httpClientConfigCallback; private RequestConfigCallback requestConfigCallback; + private String pathPrefix; /** * Creates a new builder instance and sets the hosts that the client will send requests to. + * + * @throws NullPointerException if {@code hosts} or any host is {@code null}. + * @throws IllegalArgumentException if {@code hosts} is empty. */ RestClientBuilder(HttpHost... hosts) { - if (hosts == null || hosts.length == 0) { + Objects.requireNonNull(hosts, "hosts must not be null"); + if (hosts.length == 0) { throw new IllegalArgumentException("no hosts provided"); } for (HttpHost host : hosts) { @@ -67,6 +72,8 @@ public final class RestClientBuilder { /** * Sets the default request headers, which will be sent along with each request + * + * @throws NullPointerException if {@code defaultHeaders} or any header is {@code null}. */ public RestClientBuilder setDefaultHeaders(Header[] defaultHeaders) { Objects.requireNonNull(defaultHeaders, "defaultHeaders must not be null"); @@ -79,6 +86,8 @@ public final class RestClientBuilder { /** * Sets the {@link RestClient.FailureListener} to be notified for each request failure + * + * @throws NullPointerException if {@code failureListener} is {@code null}. */ public RestClientBuilder setFailureListener(RestClient.FailureListener failureListener) { Objects.requireNonNull(failureListener, "failureListener must not be null"); @@ -90,7 +99,7 @@ public final class RestClientBuilder { * Sets the maximum timeout (in milliseconds) to honour in case of multiple retries of the same request. * {@link #DEFAULT_MAX_RETRY_TIMEOUT_MILLIS} if not specified. * - * @throws IllegalArgumentException if maxRetryTimeoutMillis is not greater than 0 + * @throws IllegalArgumentException if {@code maxRetryTimeoutMillis} is not greater than 0 */ public RestClientBuilder setMaxRetryTimeoutMillis(int maxRetryTimeoutMillis) { if (maxRetryTimeoutMillis <= 0) { @@ -102,6 +111,8 @@ public final class RestClientBuilder { /** * Sets the {@link HttpClientConfigCallback} to be used to customize http client configuration + * + * @throws NullPointerException if {@code httpClientConfigCallback} is {@code null}. */ public RestClientBuilder setHttpClientConfigCallback(HttpClientConfigCallback httpClientConfigCallback) { Objects.requireNonNull(httpClientConfigCallback, "httpClientConfigCallback must not be null"); @@ -111,6 +122,8 @@ public final class RestClientBuilder { /** * Sets the {@link RequestConfigCallback} to be used to customize http client configuration + * + * @throws NullPointerException if {@code requestConfigCallback} is {@code null}. */ public RestClientBuilder setRequestConfigCallback(RequestConfigCallback requestConfigCallback) { Objects.requireNonNull(requestConfigCallback, "requestConfigCallback must not be null"); @@ -118,6 +131,43 @@ public final class RestClientBuilder { return this; } + /** + * Sets the path's prefix for every request used by the http client. + *

+ * For example, if this is set to "/my/path", then any client request will become "/my/path/" + endpoint. + *

+ * In essence, every request's {@code endpoint} is prefixed by this {@code pathPrefix}. The path prefix is useful for when + * Elasticsearch is behind a proxy that provides a base path; it is not intended for other purposes and it should not be supplied in + * other scenarios. + * + * @throws NullPointerException if {@code pathPrefix} is {@code null}. + * @throws IllegalArgumentException if {@code pathPrefix} is empty, only '/', or ends with more than one '/'. + */ + public RestClientBuilder setPathPrefix(String pathPrefix) { + Objects.requireNonNull(pathPrefix, "pathPrefix must not be null"); + String cleanPathPrefix = pathPrefix; + + if (cleanPathPrefix.startsWith("/") == false) { + cleanPathPrefix = "/" + cleanPathPrefix; + } + + // best effort to ensure that it looks like "/base/path" rather than "/base/path/" + if (cleanPathPrefix.endsWith("/")) { + cleanPathPrefix = cleanPathPrefix.substring(0, cleanPathPrefix.length() - 1); + + if (cleanPathPrefix.endsWith("/")) { + throw new IllegalArgumentException("pathPrefix is malformed. too many trailing slashes: [" + pathPrefix + "]"); + } + } + + if (cleanPathPrefix.isEmpty() || "/".equals(cleanPathPrefix)) { + throw new IllegalArgumentException("pathPrefix must not be empty or '/': [" + pathPrefix + "]"); + } + + this.pathPrefix = cleanPathPrefix; + return this; + } + /** * Creates a new {@link RestClient} based on the provided configuration. */ @@ -126,7 +176,7 @@ public final class RestClientBuilder { failureListener = new RestClient.FailureListener(); } CloseableHttpAsyncClient httpClient = createHttpClient(); - RestClient restClient = new RestClient(httpClient, maxRetryTimeout, defaultHeaders, hosts, failureListener); + RestClient restClient = new RestClient(httpClient, maxRetryTimeout, defaultHeaders, hosts, pathPrefix, failureListener); httpClient.start(); return restClient; } diff --git a/client/rest/src/test/java/org/elasticsearch/client/RestClientBuilderTests.java b/client/rest/src/test/java/org/elasticsearch/client/RestClientBuilderTests.java index ca671862124..c9243d3aaf6 100644 --- a/client/rest/src/test/java/org/elasticsearch/client/RestClientBuilderTests.java +++ b/client/rest/src/test/java/org/elasticsearch/client/RestClientBuilderTests.java @@ -19,7 +19,6 @@ package org.elasticsearch.client; -import com.carrotsearch.randomizedtesting.generators.RandomInts; import org.apache.http.Header; import org.apache.http.HttpHost; import org.apache.http.client.config.RequestConfig; @@ -28,8 +27,10 @@ import org.apache.http.message.BasicHeader; import java.io.IOException; +import static org.hamcrest.Matchers.containsString; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThat; import static org.junit.Assert.fail; public class RestClientBuilderTests extends RestClientTestCase { @@ -38,8 +39,8 @@ public class RestClientBuilderTests extends RestClientTestCase { try { RestClient.builder((HttpHost[])null); fail("should have failed"); - } catch(IllegalArgumentException e) { - assertEquals("no hosts provided", e.getMessage()); + } catch(NullPointerException e) { + assertEquals("hosts must not be null", e.getMessage()); } try { @@ -62,7 +63,7 @@ public class RestClientBuilderTests extends RestClientTestCase { try { RestClient.builder(new HttpHost("localhost", 9200)) - .setMaxRetryTimeoutMillis(RandomInts.randomIntBetween(getRandom(), Integer.MIN_VALUE, 0)); + .setMaxRetryTimeoutMillis(randomIntBetween(Integer.MIN_VALUE, 0)); fail("should have failed"); } catch(IllegalArgumentException e) { assertEquals("maxRetryTimeoutMillis must be greater than 0", e.getMessage()); @@ -103,13 +104,13 @@ public class RestClientBuilderTests extends RestClientTestCase { assertEquals("requestConfigCallback must not be null", e.getMessage()); } - int numNodes = RandomInts.randomIntBetween(getRandom(), 1, 5); + int numNodes = randomIntBetween(1, 5); HttpHost[] hosts = new HttpHost[numNodes]; for (int i = 0; i < numNodes; i++) { hosts[i] = new HttpHost("localhost", 9200 + i); } RestClientBuilder builder = RestClient.builder(hosts); - if (getRandom().nextBoolean()) { + if (randomBoolean()) { builder.setHttpClientConfigCallback(new RestClientBuilder.HttpClientConfigCallback() { @Override public HttpAsyncClientBuilder customizeHttpClient(HttpAsyncClientBuilder httpClientBuilder) { @@ -117,7 +118,7 @@ public class RestClientBuilderTests extends RestClientTestCase { } }); } - if (getRandom().nextBoolean()) { + if (randomBoolean()) { builder.setRequestConfigCallback(new RestClientBuilder.RequestConfigCallback() { @Override public RequestConfig.Builder customizeRequestConfig(RequestConfig.Builder requestConfigBuilder) { @@ -125,19 +126,55 @@ public class RestClientBuilderTests extends RestClientTestCase { } }); } - if (getRandom().nextBoolean()) { - int numHeaders = RandomInts.randomIntBetween(getRandom(), 1, 5); + if (randomBoolean()) { + int numHeaders = randomIntBetween(1, 5); Header[] headers = new Header[numHeaders]; for (int i = 0; i < numHeaders; i++) { headers[i] = new BasicHeader("header" + i, "value"); } builder.setDefaultHeaders(headers); } - if (getRandom().nextBoolean()) { - builder.setMaxRetryTimeoutMillis(RandomInts.randomIntBetween(getRandom(), 1, Integer.MAX_VALUE)); + if (randomBoolean()) { + builder.setMaxRetryTimeoutMillis(randomIntBetween(1, Integer.MAX_VALUE)); + } + if (randomBoolean()) { + String pathPrefix = (randomBoolean() ? "/" : "") + randomAsciiOfLengthBetween(2, 5); + while (pathPrefix.length() < 20 && randomBoolean()) { + pathPrefix += "/" + randomAsciiOfLengthBetween(3, 6); + } + builder.setPathPrefix(pathPrefix + (randomBoolean() ? "/" : "")); } try (RestClient restClient = builder.build()) { assertNotNull(restClient); } } + + public void testSetPathPrefixNull() { + try { + RestClient.builder(new HttpHost("localhost", 9200)).setPathPrefix(null); + fail("pathPrefix set to null should fail!"); + } catch (final NullPointerException e) { + assertEquals("pathPrefix must not be null", e.getMessage()); + } + } + + public void testSetPathPrefixEmpty() { + assertSetPathPrefixThrows("/"); + assertSetPathPrefixThrows(""); + } + + public void testSetPathPrefixMalformed() { + assertSetPathPrefixThrows("//"); + assertSetPathPrefixThrows("base/path//"); + } + + private static void assertSetPathPrefixThrows(final String pathPrefix) { + try { + RestClient.builder(new HttpHost("localhost", 9200)).setPathPrefix(pathPrefix); + fail("path prefix [" + pathPrefix + "] should have failed"); + } catch (final IllegalArgumentException e) { + assertThat(e.getMessage(), containsString(pathPrefix)); + } + } + } diff --git a/client/rest/src/test/java/org/elasticsearch/client/RestClientIntegTests.java b/client/rest/src/test/java/org/elasticsearch/client/RestClientIntegTests.java index 79da041f379..a6f6829c60f 100644 --- a/client/rest/src/test/java/org/elasticsearch/client/RestClientIntegTests.java +++ b/client/rest/src/test/java/org/elasticsearch/client/RestClientIntegTests.java @@ -22,6 +22,7 @@ package org.elasticsearch.client; import com.carrotsearch.randomizedtesting.generators.RandomInts; import com.carrotsearch.randomizedtesting.generators.RandomStrings; import com.sun.net.httpserver.Headers; +import com.sun.net.httpserver.HttpContext; import com.sun.net.httpserver.HttpExchange; import com.sun.net.httpserver.HttpHandler; import com.sun.net.httpserver.HttpServer; @@ -60,6 +61,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; /** * Integration test to check interaction between {@link RestClient} and {@link org.apache.http.client.HttpClient}. @@ -205,6 +207,68 @@ public class RestClientIntegTests extends RestClientTestCase { bodyTest("GET"); } + /** + * Ensure that pathPrefix works as expected even when the path does not exist. + */ + public void testPathPrefixUnknownPath() throws IOException { + // guarantee no other test setup collides with this one and lets it sneak through + final String uniqueContextSuffix = "/testPathPrefixUnknownPath"; + final String pathPrefix = "dne/" + randomAsciiOfLengthBetween(1, 5) + "/"; + final int statusCode = randomStatusCode(getRandom()); + + try (final RestClient client = + RestClient.builder(new HttpHost(httpServer.getAddress().getHostString(), httpServer.getAddress().getPort())) + .setPathPrefix((randomBoolean() ? "/" : "") + pathPrefix).build()) { + + for (final String method : getHttpMethods()) { + Response esResponse; + try { + esResponse = client.performRequest(method, "/" + statusCode + uniqueContextSuffix); + if ("HEAD".equals(method) == false) { + fail("only HEAD requests should not throw an exception; 404 is expected"); + } + } catch(ResponseException e) { + esResponse = e.getResponse(); + } + + assertThat(esResponse.getRequestLine().getUri(), equalTo("/" + pathPrefix + statusCode + uniqueContextSuffix)); + assertThat(esResponse.getStatusLine().getStatusCode(), equalTo(404)); + } + } + } + + /** + * Ensure that pathPrefix works as expected. + */ + public void testPathPrefix() throws IOException { + // guarantee no other test setup collides with this one and lets it sneak through + final String uniqueContextSuffix = "/testPathPrefix"; + final String pathPrefix = "base/" + randomAsciiOfLengthBetween(1, 5) + "/"; + final int statusCode = randomStatusCode(getRandom()); + + final HttpContext context = + httpServer.createContext("/" + pathPrefix + statusCode + uniqueContextSuffix, new ResponseHandler(statusCode)); + + try (final RestClient client = + RestClient.builder(new HttpHost(httpServer.getAddress().getHostString(), httpServer.getAddress().getPort())) + .setPathPrefix((randomBoolean() ? "/" : "") + pathPrefix).build()) { + + for (final String method : getHttpMethods()) { + Response esResponse; + try { + esResponse = client.performRequest(method, "/" + statusCode + uniqueContextSuffix); + } catch(ResponseException e) { + esResponse = e.getResponse(); + } + + assertThat(esResponse.getRequestLine().getUri(), equalTo("/" + pathPrefix + statusCode + uniqueContextSuffix)); + assertThat(esResponse.getStatusLine().getStatusCode(), equalTo(statusCode)); + } + } finally { + httpServer.removeContext(context); + } + } + private void bodyTest(String method) throws IOException { String requestBody = "{ \"field\": \"value\" }"; StringEntity entity = new StringEntity(requestBody); diff --git a/client/rest/src/test/java/org/elasticsearch/client/RestClientMultipleHostsTests.java b/client/rest/src/test/java/org/elasticsearch/client/RestClientMultipleHostsTests.java index 2a2d279689a..049a216936f 100644 --- a/client/rest/src/test/java/org/elasticsearch/client/RestClientMultipleHostsTests.java +++ b/client/rest/src/test/java/org/elasticsearch/client/RestClientMultipleHostsTests.java @@ -101,7 +101,7 @@ public class RestClientMultipleHostsTests extends RestClientTestCase { httpHosts[i] = new HttpHost("localhost", 9200 + i); } failureListener = new HostsTrackingFailureListener(); - restClient = new RestClient(httpClient, 10000, new Header[0], httpHosts, failureListener); + restClient = new RestClient(httpClient, 10000, new Header[0], httpHosts, null, failureListener); } public void testRoundRobinOkStatusCodes() throws IOException { diff --git a/client/rest/src/test/java/org/elasticsearch/client/RestClientSingleHostTests.java b/client/rest/src/test/java/org/elasticsearch/client/RestClientSingleHostTests.java index e347dfecc12..a6ae30b01e8 100644 --- a/client/rest/src/test/java/org/elasticsearch/client/RestClientSingleHostTests.java +++ b/client/rest/src/test/java/org/elasticsearch/client/RestClientSingleHostTests.java @@ -141,7 +141,7 @@ public class RestClientSingleHostTests extends RestClientTestCase { } httpHost = new HttpHost("localhost", 9200); failureListener = new HostsTrackingFailureListener(); - restClient = new RestClient(httpClient, 10000, defaultHeaders, new HttpHost[]{httpHost}, failureListener); + restClient = new RestClient(httpClient, 10000, defaultHeaders, new HttpHost[]{httpHost}, null, failureListener); } /** From e19f2b63482f6a8586218e802e953002df41b357 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 30 Aug 2016 13:55:44 -0700 Subject: [PATCH 02/10] Tests: Improve rest suite names and generated test names for docs tests Rest test suites are currently only the directory above the yaml test file. That is confusing when there are more than one directory level which contain yaml tests, as there are in generated docs tests. This change makes rest tests use the full relative path to the rest test root as the suite name, and also makes the test names for docs tests a little clearer (that they are testing an example from a specific line number, instead of just the line number as an opaque test name). --- .../elasticsearch/gradle/doc/RestTestsFromSnippetsTask.groovy | 2 +- .../main/java/org/elasticsearch/test/rest/yaml/FileUtils.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTask.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTask.groovy index 61a07f4fbd4..fc7604ad1fd 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTask.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTask.groovy @@ -117,7 +117,7 @@ public class RestTestsFromSnippetsTask extends SnippetsTask { if (false == test.continued) { current.println('---') - current.println("\"$test.start\":") + current.println("\"line_$test.start\":") } if (test.skipTest) { current.println(" - skip:") diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/FileUtils.java b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/FileUtils.java index caaa8b2ec83..4519953819a 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/FileUtils.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/FileUtils.java @@ -155,7 +155,7 @@ public final class FileUtils { @Override public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException { if (file.toString().endsWith(fileSuffix)) { - String groupName = file.toAbsolutePath().getParent().getFileName().toString(); + String groupName = dir.relativize(file.getParent()).toString(); Set filesSet = files.get(groupName); if (filesSet == null) { filesSet = new HashSet<>(); From c05d5f925789154e62cb0bd94611cd4aedc49755 Mon Sep 17 00:00:00 2001 From: Chris Earle Date: Tue, 30 Aug 2016 17:18:01 -0400 Subject: [PATCH 03/10] Remove unknown HttpContext-based test as it fails unpredictably on different JVMs --- .../client/RestClientIntegTests.java | 30 ------------------- 1 file changed, 30 deletions(-) diff --git a/client/rest/src/test/java/org/elasticsearch/client/RestClientIntegTests.java b/client/rest/src/test/java/org/elasticsearch/client/RestClientIntegTests.java index a6f6829c60f..e7d7852de04 100644 --- a/client/rest/src/test/java/org/elasticsearch/client/RestClientIntegTests.java +++ b/client/rest/src/test/java/org/elasticsearch/client/RestClientIntegTests.java @@ -207,36 +207,6 @@ public class RestClientIntegTests extends RestClientTestCase { bodyTest("GET"); } - /** - * Ensure that pathPrefix works as expected even when the path does not exist. - */ - public void testPathPrefixUnknownPath() throws IOException { - // guarantee no other test setup collides with this one and lets it sneak through - final String uniqueContextSuffix = "/testPathPrefixUnknownPath"; - final String pathPrefix = "dne/" + randomAsciiOfLengthBetween(1, 5) + "/"; - final int statusCode = randomStatusCode(getRandom()); - - try (final RestClient client = - RestClient.builder(new HttpHost(httpServer.getAddress().getHostString(), httpServer.getAddress().getPort())) - .setPathPrefix((randomBoolean() ? "/" : "") + pathPrefix).build()) { - - for (final String method : getHttpMethods()) { - Response esResponse; - try { - esResponse = client.performRequest(method, "/" + statusCode + uniqueContextSuffix); - if ("HEAD".equals(method) == false) { - fail("only HEAD requests should not throw an exception; 404 is expected"); - } - } catch(ResponseException e) { - esResponse = e.getResponse(); - } - - assertThat(esResponse.getRequestLine().getUri(), equalTo("/" + pathPrefix + statusCode + uniqueContextSuffix)); - assertThat(esResponse.getStatusLine().getStatusCode(), equalTo(404)); - } - } - } - /** * Ensure that pathPrefix works as expected. */ From 2a7a187bf872ebe440f2916250e1f82d1b3259e1 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 30 Aug 2016 14:58:44 -0700 Subject: [PATCH 04/10] Silence rest util tests until the bogusness can be simplified --- .../java/org/elasticsearch/test/rest/yaml/FileUtilsTests.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/FileUtilsTests.java b/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/FileUtilsTests.java index 4387bf164fa..457152381ba 100644 --- a/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/FileUtilsTests.java +++ b/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/FileUtilsTests.java @@ -31,6 +31,7 @@ import static org.hamcrest.CoreMatchers.notNullValue; import static org.hamcrest.Matchers.greaterThan; public class FileUtilsTests extends ESTestCase { + @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/20240") public void testLoadSingleYamlSuite() throws Exception { Map> yamlSuites = FileUtils.findYamlSuites(null, "/rest-api-spec/test", "/rest-api-spec/test/suite1/10_basic"); assertSingleFile(yamlSuites, "suite1", "10_basic.yaml"); @@ -44,6 +45,7 @@ public class FileUtilsTests extends ESTestCase { assertSingleFile(yamlSuites, "suite1", "10_basic.yaml"); } + @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/20240") public void testLoadMultipleYamlSuites() throws Exception { //single directory Map> yamlSuites = FileUtils.findYamlSuites(null, "/rest-api-spec/test", "suite1"); From b8f4c92d41411e17ec45f3c83dc81d1f12d39751 Mon Sep 17 00:00:00 2001 From: Chris Earle Date: Wed, 24 Aug 2016 19:28:32 -0400 Subject: [PATCH 05/10] Allow RestClient to send array-based headers This enables the RestClient to send array-based (multi-valued) header values, rather than only sending whatever happened to be the _last_ value of the header. --- .../org/elasticsearch/client/RestClient.java | 13 ++- .../client/RestClientBuilder.java | 4 +- .../client/RestClientIntegTests.java | 62 ++++++--------- .../client/RestClientSingleHostTests.java | 79 +++++++++---------- client/test/build.gradle | 1 + .../client/RestClientTestCase.java | 76 ++++++++++++++++++ 6 files changed, 153 insertions(+), 82 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 26af479f668..d2301e1e8e7 100644 --- a/client/rest/src/main/java/org/elasticsearch/client/RestClient.java +++ b/client/rest/src/main/java/org/elasticsearch/client/RestClient.java @@ -362,12 +362,17 @@ public class RestClient implements Closeable { private void setHeaders(HttpRequest httpRequest, Header[] requestHeaders) { Objects.requireNonNull(requestHeaders, "request headers must not be null"); - for (Header defaultHeader : defaultHeaders) { - httpRequest.setHeader(defaultHeader); - } + // request headers override default headers, so we don't add default headers if they exist as request headers + final Set requestNames = new HashSet<>(requestHeaders.length); for (Header requestHeader : requestHeaders) { Objects.requireNonNull(requestHeader, "request header must not be null"); - httpRequest.setHeader(requestHeader); + httpRequest.addHeader(requestHeader); + requestNames.add(requestHeader.getName()); + } + for (Header defaultHeader : defaultHeaders) { + if (requestNames.contains(defaultHeader.getName()) == false) { + httpRequest.addHeader(defaultHeader); + } } } 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 4f9f379d08e..d342d59ade4 100644 --- a/client/rest/src/main/java/org/elasticsearch/client/RestClientBuilder.java +++ b/client/rest/src/main/java/org/elasticsearch/client/RestClientBuilder.java @@ -71,7 +71,9 @@ public final class RestClientBuilder { } /** - * Sets the default request headers, which will be sent along with each request + * Sets the default request headers, which will be sent along with each request. + *

+ * Request-time headers will always overwrite any default headers. * * @throws NullPointerException if {@code defaultHeaders} or any header is {@code null}. */ diff --git a/client/rest/src/test/java/org/elasticsearch/client/RestClientIntegTests.java b/client/rest/src/test/java/org/elasticsearch/client/RestClientIntegTests.java index e7d7852de04..9c5c50946d8 100644 --- a/client/rest/src/test/java/org/elasticsearch/client/RestClientIntegTests.java +++ b/client/rest/src/test/java/org/elasticsearch/client/RestClientIntegTests.java @@ -19,8 +19,6 @@ package org.elasticsearch.client; -import com.carrotsearch.randomizedtesting.generators.RandomInts; -import com.carrotsearch.randomizedtesting.generators.RandomStrings; import com.sun.net.httpserver.Headers; import com.sun.net.httpserver.HttpContext; import com.sun.net.httpserver.HttpExchange; @@ -28,10 +26,8 @@ import com.sun.net.httpserver.HttpHandler; import com.sun.net.httpserver.HttpServer; import org.apache.http.Consts; import org.apache.http.Header; -import org.apache.http.HttpEntity; import org.apache.http.HttpHost; import org.apache.http.entity.StringEntity; -import org.apache.http.message.BasicHeader; import org.apache.http.util.EntityUtils; import org.codehaus.mojo.animal_sniffer.IgnoreJRERequirement; import org.junit.AfterClass; @@ -83,13 +79,8 @@ public class RestClientIntegTests extends RestClientTestCase { for (int statusCode : getAllStatusCodes()) { createStatusCodeContext(httpServer, statusCode); } - int numHeaders = RandomInts.randomIntBetween(getRandom(), 0, 3); - defaultHeaders = new Header[numHeaders]; - for (int i = 0; i < numHeaders; i++) { - String headerName = "Header-default" + (getRandom().nextBoolean() ? i : ""); - String headerValue = RandomStrings.randomAsciiOfLengthBetween(getRandom(), 3, 10); - defaultHeaders[i] = new BasicHeader(headerName, headerValue); - } + int numHeaders = randomIntBetween(0, 5); + defaultHeaders = generateHeaders("Header-default", "Header-array", numHeaders); restClient = RestClient.builder(new HttpHost(httpServer.getAddress().getHostString(), httpServer.getAddress().getPort())) .setDefaultHeaders(defaultHeaders).build(); } @@ -148,44 +139,43 @@ public class RestClientIntegTests extends RestClientTestCase { */ public void testHeaders() throws IOException { for (String method : getHttpMethods()) { - Set standardHeaders = new HashSet<>( - Arrays.asList("Connection", "Host", "User-agent", "Date")); + final Set standardHeaders = new HashSet<>(Arrays.asList("Connection", "Host", "User-agent", "Date")); if (method.equals("HEAD") == false) { standardHeaders.add("Content-length"); } - int numHeaders = RandomInts.randomIntBetween(getRandom(), 1, 5); - Map expectedHeaders = new HashMap<>(); - for (Header defaultHeader : defaultHeaders) { - expectedHeaders.put(defaultHeader.getName(), defaultHeader.getValue()); - } - Header[] headers = new Header[numHeaders]; - for (int i = 0; i < numHeaders; i++) { - String headerName = "Header" + (getRandom().nextBoolean() ? i : ""); - String headerValue = RandomStrings.randomAsciiOfLengthBetween(getRandom(), 3, 10); - headers[i] = new BasicHeader(headerName, headerValue); - expectedHeaders.put(headerName, headerValue); - } - int statusCode = randomStatusCode(getRandom()); + final int numHeaders = randomIntBetween(1, 5); + final Header[] headers = generateHeaders("Header", "Header-array", numHeaders); + final Map> expectedHeaders = new HashMap<>(); + + addHeaders(expectedHeaders, defaultHeaders, headers); + + final int statusCode = randomStatusCode(getRandom()); Response esResponse; try { - esResponse = restClient.performRequest(method, "/" + statusCode, Collections.emptyMap(), - (HttpEntity)null, headers); + esResponse = restClient.performRequest(method, "/" + statusCode, Collections.emptyMap(), headers); } catch(ResponseException e) { esResponse = e.getResponse(); } assertThat(esResponse.getStatusLine().getStatusCode(), equalTo(statusCode)); - for (Header responseHeader : esResponse.getHeaders()) { - if (responseHeader.getName().startsWith("Header")) { - String headerValue = expectedHeaders.remove(responseHeader.getName()); - assertNotNull("found response header [" + responseHeader.getName() + "] that wasn't originally sent", headerValue); + for (final Header responseHeader : esResponse.getHeaders()) { + final String name = responseHeader.getName(); + final String value = responseHeader.getValue(); + if (name.startsWith("Header")) { + final List values = expectedHeaders.get(name); + assertNotNull("found response header [" + name + "] that wasn't originally sent: " + value, values); + assertTrue("found incorrect response header [" + name + "]: " + value, values.remove(value)); + + // we've collected them all + if (values.isEmpty()) { + expectedHeaders.remove(name); + } } else { - assertTrue("unknown header was returned " + responseHeader.getName(), - standardHeaders.remove(responseHeader.getName())); + assertTrue("unknown header was returned " + name, standardHeaders.remove(name)); } } - assertEquals("some headers that were sent weren't returned: " + expectedHeaders, 0, expectedHeaders.size()); - assertEquals("some expected standard headers weren't returned: " + standardHeaders, 0, standardHeaders.size()); + assertTrue("some headers that were sent weren't returned: " + expectedHeaders, expectedHeaders.isEmpty()); + assertTrue("some expected standard headers weren't returned: " + standardHeaders, standardHeaders.isEmpty()); } } diff --git a/client/rest/src/test/java/org/elasticsearch/client/RestClientSingleHostTests.java b/client/rest/src/test/java/org/elasticsearch/client/RestClientSingleHostTests.java index a6ae30b01e8..92e2b0da971 100644 --- a/client/rest/src/test/java/org/elasticsearch/client/RestClientSingleHostTests.java +++ b/client/rest/src/test/java/org/elasticsearch/client/RestClientSingleHostTests.java @@ -19,8 +19,6 @@ package org.elasticsearch.client; -import com.carrotsearch.randomizedtesting.generators.RandomInts; -import com.carrotsearch.randomizedtesting.generators.RandomStrings; import org.apache.http.Header; import org.apache.http.HttpEntity; import org.apache.http.HttpEntityEnclosingRequest; @@ -41,7 +39,6 @@ import org.apache.http.concurrent.FutureCallback; import org.apache.http.conn.ConnectTimeoutException; import org.apache.http.entity.StringEntity; import org.apache.http.impl.nio.client.CloseableHttpAsyncClient; -import org.apache.http.message.BasicHeader; import org.apache.http.message.BasicHttpResponse; import org.apache.http.message.BasicStatusLine; import org.apache.http.nio.protocol.HttpAsyncRequestProducer; @@ -58,7 +55,10 @@ import java.net.URI; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; +import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.Future; import static org.elasticsearch.client.RestClientTestUtil.getAllErrorStatusCodes; @@ -132,13 +132,8 @@ public class RestClientSingleHostTests extends RestClientTestCase { }); - int numHeaders = RandomInts.randomIntBetween(getRandom(), 0, 3); - defaultHeaders = new Header[numHeaders]; - for (int i = 0; i < numHeaders; i++) { - String headerName = "Header-default" + (getRandom().nextBoolean() ? i : ""); - String headerValue = RandomStrings.randomAsciiOfLengthBetween(getRandom(), 3, 10); - defaultHeaders[i] = new BasicHeader(headerName, headerValue); - } + int numHeaders = randomIntBetween(0, 3); + defaultHeaders = generateHeaders("Header-default", "Header-array", numHeaders); httpHost = new HttpHost("localhost", 9200); failureListener = new HostsTrackingFailureListener(); restClient = new RestClient(httpClient, 10000, defaultHeaders, new HttpHost[]{httpHost}, null, failureListener); @@ -333,20 +328,13 @@ public class RestClientSingleHostTests extends RestClientTestCase { */ public void testHeaders() throws IOException { for (String method : getHttpMethods()) { - Map expectedHeaders = new HashMap<>(); - for (Header defaultHeader : defaultHeaders) { - expectedHeaders.put(defaultHeader.getName(), defaultHeader.getValue()); - } - int numHeaders = RandomInts.randomIntBetween(getRandom(), 1, 5); - Header[] headers = new Header[numHeaders]; - for (int i = 0; i < numHeaders; i++) { - String headerName = "Header" + (getRandom().nextBoolean() ? i : ""); - String headerValue = RandomStrings.randomAsciiOfLengthBetween(getRandom(), 3, 10); - headers[i] = new BasicHeader(headerName, headerValue); - expectedHeaders.put(headerName, headerValue); - } + final int numHeaders = randomIntBetween(1, 5); + final Header[] headers = generateHeaders("Header", null, numHeaders); + final Map> expectedHeaders = new HashMap<>(); - int statusCode = randomStatusCode(getRandom()); + addHeaders(expectedHeaders, defaultHeaders, headers); + + final int statusCode = randomStatusCode(getRandom()); Response esResponse; try { esResponse = restClient.performRequest(method, "/" + statusCode, headers); @@ -355,10 +343,18 @@ public class RestClientSingleHostTests extends RestClientTestCase { } assertThat(esResponse.getStatusLine().getStatusCode(), equalTo(statusCode)); for (Header responseHeader : esResponse.getHeaders()) { - String headerValue = expectedHeaders.remove(responseHeader.getName()); - assertNotNull("found response header [" + responseHeader.getName() + "] that wasn't originally sent", headerValue); + final String name = responseHeader.getName(); + final String value = responseHeader.getValue(); + final List values = expectedHeaders.get(name); + assertNotNull("found response header [" + name + "] that wasn't originally sent: " + value, values); + assertTrue("found incorrect response header [" + name + "]: " + value, values.remove(value)); + + // we've collected them all + if (values.isEmpty()) { + expectedHeaders.remove(name); + } } - assertEquals("some headers that were sent weren't returned " + expectedHeaders, 0, expectedHeaders.size()); + assertTrue("some headers that were sent weren't returned " + expectedHeaders, expectedHeaders.isEmpty()); } } @@ -368,11 +364,11 @@ public class RestClientSingleHostTests extends RestClientTestCase { Map params = Collections.emptyMap(); boolean hasParams = randomBoolean(); if (hasParams) { - int numParams = RandomInts.randomIntBetween(getRandom(), 1, 3); + int numParams = randomIntBetween(1, 3); params = new HashMap<>(numParams); for (int i = 0; i < numParams; i++) { String paramKey = "param-" + i; - String paramValue = RandomStrings.randomAsciiOfLengthBetween(getRandom(), 3, 10); + String paramValue = randomAsciiOfLengthBetween(3, 10); params.put(paramKey, paramValue); uriBuilder.addParameter(paramKey, paramValue); } @@ -412,24 +408,24 @@ public class RestClientSingleHostTests extends RestClientTestCase { HttpEntity entity = null; boolean hasBody = request instanceof HttpEntityEnclosingRequest && getRandom().nextBoolean(); if (hasBody) { - entity = new StringEntity(RandomStrings.randomAsciiOfLengthBetween(getRandom(), 10, 100)); + entity = new StringEntity(randomAsciiOfLengthBetween(10, 100)); ((HttpEntityEnclosingRequest) request).setEntity(entity); } Header[] headers = new Header[0]; - for (Header defaultHeader : defaultHeaders) { - //default headers are expected but not sent for each request - request.setHeader(defaultHeader); + final int numHeaders = randomIntBetween(1, 5); + final Set uniqueNames = new HashSet<>(numHeaders); + if (randomBoolean()) { + headers = generateHeaders("Header", "Header-array", numHeaders); + for (Header header : headers) { + request.addHeader(header); + uniqueNames.add(header.getName()); + } } - if (getRandom().nextBoolean()) { - int numHeaders = RandomInts.randomIntBetween(getRandom(), 1, 5); - headers = new Header[numHeaders]; - for (int i = 0; i < numHeaders; i++) { - String headerName = "Header" + (getRandom().nextBoolean() ? i : ""); - String headerValue = RandomStrings.randomAsciiOfLengthBetween(getRandom(), 3, 10); - BasicHeader basicHeader = new BasicHeader(headerName, headerValue); - headers[i] = basicHeader; - request.setHeader(basicHeader); + for (Header defaultHeader : defaultHeaders) { + // request level headers override default headers + if (uniqueNames.contains(defaultHeader.getName()) == false) { + request.addHeader(defaultHeader); } } @@ -459,4 +455,5 @@ public class RestClientSingleHostTests extends RestClientTestCase { throw new UnsupportedOperationException(); } } + } diff --git a/client/test/build.gradle b/client/test/build.gradle index 05d044504ec..a7ffe79ac5c 100644 --- a/client/test/build.gradle +++ b/client/test/build.gradle @@ -30,6 +30,7 @@ install.enabled = false uploadArchives.enabled = false dependencies { + compile "org.apache.httpcomponents:httpcore:${versions.httpcore}" compile "com.carrotsearch.randomizedtesting:randomizedtesting-runner:${versions.randomizedrunner}" compile "junit:junit:${versions.junit}" compile "org.hamcrest:hamcrest-all:${versions.hamcrest}" diff --git a/client/test/src/main/java/org/elasticsearch/client/RestClientTestCase.java b/client/test/src/main/java/org/elasticsearch/client/RestClientTestCase.java index 8c506beb5ac..4296932a002 100644 --- a/client/test/src/main/java/org/elasticsearch/client/RestClientTestCase.java +++ b/client/test/src/main/java/org/elasticsearch/client/RestClientTestCase.java @@ -31,6 +31,15 @@ import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; import com.carrotsearch.randomizedtesting.annotations.ThreadLeakZombies; import com.carrotsearch.randomizedtesting.annotations.TimeoutSuite; +import org.apache.http.Header; +import org.apache.http.message.BasicHeader; + +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + @TestMethodProviders({ JUnit3MethodProvider.class }) @@ -43,4 +52,71 @@ import com.carrotsearch.randomizedtesting.annotations.TimeoutSuite; @TimeoutSuite(millis = 2 * 60 * 60 * 1000) public abstract class RestClientTestCase extends RandomizedTest { + /** + * Create the specified number of {@link Header}s. + *

+ * Generated header names will be the {@code baseName} plus its index or, rarely, the {@code arrayName} if it's supplied. + * + * @param baseName The base name to use for all headers. + * @param arrayName The optional ({@code null}able) array name to use randomly. + * @param headers The number of headers to create. + * @return Never {@code null}. + */ + protected static Header[] generateHeaders(final String baseName, final String arrayName, final int headers) { + final Header[] generated = new Header[headers]; + for (int i = 0; i < headers; i++) { + String headerName = baseName + i; + if (arrayName != null && rarely()) { + headerName = arrayName; + } + + generated[i] = new BasicHeader(headerName, randomAsciiOfLengthBetween(3, 10)); + } + return generated; + } + + /** + * Create a new {@link List} within the {@code map} if none exists for {@code name} or append to the existing list. + * + * @param map The map to manipulate. + * @param name The name to create/append the list for. + * @param value The value to add. + */ + private static void createOrAppendList(final Map> map, final String name, final String value) { + List values = map.get(name); + + if (values == null) { + values = new ArrayList<>(); + map.put(name, values); + } + + values.add(value); + } + + /** + * Add the {@code headers} to the {@code map} so that related tests can more easily assert that they exist. + *

+ * If both the {@code defaultHeaders} and {@code headers} contain the same {@link Header}, based on its + * {@linkplain Header#getName() name}, then this will only use the {@code Header}(s) from {@code headers}. + * + * @param map The map to build with name/value(s) pairs. + * @param defaultHeaders The headers to add to the map representing default headers. + * @param headers The headers to add to the map representing request-level headers. + * @see #createOrAppendList(Map, String, String) + */ + protected static void addHeaders(final Map> map, final Header[] defaultHeaders, final Header[] headers) { + final Set uniqueHeaders = new HashSet<>(); + for (final Header header : headers) { + final String name = header.getName(); + createOrAppendList(map, name, header.getValue()); + uniqueHeaders.add(name); + } + for (final Header defaultHeader : defaultHeaders) { + final String name = defaultHeader.getName(); + if (uniqueHeaders.contains(name) == false) { + createOrAppendList(map, name, defaultHeader.getValue()); + } + } + } + } From a68083f5cba18c9c109c99aafd758ebccbfdcbb8 Mon Sep 17 00:00:00 2001 From: Igor Motov Date: Tue, 30 Aug 2016 09:34:06 -0400 Subject: [PATCH 06/10] Make it possible for Ingest Processors to access AnalysisRegistry The analysis registry will be used in PMML plugin ingest processor. --- .../java/org/elasticsearch/ingest/IngestService.java | 7 +++++-- .../main/java/org/elasticsearch/ingest/Processor.java | 9 ++++++++- core/src/main/java/org/elasticsearch/node/Node.java | 2 +- .../org/elasticsearch/ingest/IngestServiceTests.java | 4 ++-- 4 files changed, 16 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/ingest/IngestService.java b/core/src/main/java/org/elasticsearch/ingest/IngestService.java index 07e2aa1fe51..5249ed7a7dc 100644 --- a/core/src/main/java/org/elasticsearch/ingest/IngestService.java +++ b/core/src/main/java/org/elasticsearch/ingest/IngestService.java @@ -27,6 +27,7 @@ import java.util.Map; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; +import org.elasticsearch.index.analysis.AnalysisRegistry; import org.elasticsearch.plugins.IngestPlugin; import org.elasticsearch.script.ScriptService; import org.elasticsearch.threadpool.ThreadPool; @@ -40,10 +41,12 @@ public class IngestService { private final PipelineExecutionService pipelineExecutionService; public IngestService(Settings settings, ThreadPool threadPool, - Environment env, ScriptService scriptService, List ingestPlugins) { + Environment env, ScriptService scriptService, AnalysisRegistry analysisRegistry, + List ingestPlugins) { + final TemplateService templateService = new InternalTemplateService(scriptService); Processor.Parameters parameters = new Processor.Parameters(env, scriptService, templateService, - threadPool.getThreadContext()); + analysisRegistry, threadPool.getThreadContext()); Map processorFactories = new HashMap<>(); for (IngestPlugin ingestPlugin : ingestPlugins) { Map newProcessors = ingestPlugin.getProcessors(parameters); diff --git a/core/src/main/java/org/elasticsearch/ingest/Processor.java b/core/src/main/java/org/elasticsearch/ingest/Processor.java index ef1cd882d22..af4ea954dd5 100644 --- a/core/src/main/java/org/elasticsearch/ingest/Processor.java +++ b/core/src/main/java/org/elasticsearch/ingest/Processor.java @@ -21,6 +21,7 @@ package org.elasticsearch.ingest; import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.env.Environment; +import org.elasticsearch.index.analysis.AnalysisRegistry; import org.elasticsearch.script.ScriptService; import java.util.Map; @@ -86,6 +87,11 @@ public interface Processor { */ public final TemplateService templateService; + /** + * Provide analyzer support + */ + public final AnalysisRegistry analysisRegistry; + /** * Allows processors to read headers set by {@link org.elasticsearch.action.support.ActionFilter} * instances that have run prior to in ingest. @@ -93,11 +99,12 @@ public interface Processor { public final ThreadContext threadContext; public Parameters(Environment env, ScriptService scriptService, TemplateService templateService, - ThreadContext threadContext) { + AnalysisRegistry analysisRegistry, ThreadContext threadContext) { this.env = env; this.scriptService = scriptService; this.templateService = templateService; this.threadContext = threadContext; + this.analysisRegistry = analysisRegistry; } } diff --git a/core/src/main/java/org/elasticsearch/node/Node.java b/core/src/main/java/org/elasticsearch/node/Node.java index 523e6faefb1..e5c61947ae7 100644 --- a/core/src/main/java/org/elasticsearch/node/Node.java +++ b/core/src/main/java/org/elasticsearch/node/Node.java @@ -311,7 +311,7 @@ public class Node implements Closeable { final TribeService tribeService = new TribeService(settings, clusterService, nodeEnvironment.nodeId()); resourcesToClose.add(tribeService); final IngestService ingestService = new IngestService(settings, threadPool, this.environment, - scriptModule.getScriptService(), pluginsService.filterPlugins(IngestPlugin.class)); + scriptModule.getScriptService(), analysisModule.getAnalysisRegistry(), pluginsService.filterPlugins(IngestPlugin.class)); ModulesBuilder modules = new ModulesBuilder(); // plugin modules must be added here, before others or we can get crazy injection errors... diff --git a/core/src/test/java/org/elasticsearch/ingest/IngestServiceTests.java b/core/src/test/java/org/elasticsearch/ingest/IngestServiceTests.java index 08cde7e04d8..3a842a4690a 100644 --- a/core/src/test/java/org/elasticsearch/ingest/IngestServiceTests.java +++ b/core/src/test/java/org/elasticsearch/ingest/IngestServiceTests.java @@ -39,7 +39,7 @@ public class IngestServiceTests extends ESTestCase { public void testIngestPlugin() { ThreadPool tp = Mockito.mock(ThreadPool.class); - IngestService ingestService = new IngestService(Settings.EMPTY, tp, null, null, Collections.singletonList(DUMMY_PLUGIN)); + IngestService ingestService = new IngestService(Settings.EMPTY, tp, null, null, null, Collections.singletonList(DUMMY_PLUGIN)); Map factories = ingestService.getPipelineStore().getProcessorFactories(); assertTrue(factories.containsKey("foo")); assertEquals(1, factories.size()); @@ -48,7 +48,7 @@ public class IngestServiceTests extends ESTestCase { public void testIngestPluginDuplicate() { ThreadPool tp = Mockito.mock(ThreadPool.class); IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> - new IngestService(Settings.EMPTY, tp, null, null, Arrays.asList(DUMMY_PLUGIN, DUMMY_PLUGIN)) + new IngestService(Settings.EMPTY, tp, null, null, null, Arrays.asList(DUMMY_PLUGIN, DUMMY_PLUGIN)) ); assertTrue(e.getMessage(), e.getMessage().contains("already registered")); } From 3fcb95b8140e576b604fb9f4fae5a1733016a7c5 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Tue, 30 Aug 2016 11:29:55 +0200 Subject: [PATCH 07/10] percolator: Fail indexing percolator queries containing either a has_child or has_parent query. Closes #2960 --- .../percolator/PercolatorFieldMapper.java | 29 ++++++++++------ .../PercolatorFieldMapperTests.java | 33 ++++++++++++------- .../percolator/PercolatorIT.java | 11 +++---- 3 files changed, 46 insertions(+), 27 deletions(-) diff --git a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorFieldMapper.java b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorFieldMapper.java index 32633775167..82e1d42ba23 100644 --- a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorFieldMapper.java +++ b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorFieldMapper.java @@ -53,6 +53,8 @@ import org.elasticsearch.index.mapper.ParseContext; import org.elasticsearch.index.query.BoolQueryBuilder; import org.elasticsearch.index.query.BoostingQueryBuilder; import org.elasticsearch.index.query.ConstantScoreQueryBuilder; +import org.elasticsearch.index.query.HasChildQueryBuilder; +import org.elasticsearch.index.query.HasParentQueryBuilder; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryParseContext; import org.elasticsearch.index.query.QueryShardContext; @@ -269,7 +271,7 @@ public class PercolatorFieldMapper extends FieldMapper { XContentParser parser = context.parser(); QueryBuilder queryBuilder = parseQueryBuilder(queryShardContext.newParseContext(parser), parser.getTokenLocation()); - verifyRangeQueries(queryBuilder); + verifyQuery(queryBuilder); // Fetching of terms, shapes and indexed scripts happen during this rewrite: queryBuilder = queryBuilder.rewrite(queryShardContext); @@ -356,19 +358,26 @@ public class PercolatorFieldMapper extends FieldMapper { } /** - * Fails if a range query with a date range is found based on current time + * Fails if a percolator contains an unsupported query. The following queries are not supported: + * 1) a range query with a date range based on current time + * 2) a has_child query + * 3) a has_parent query */ - static void verifyRangeQueries(QueryBuilder queryBuilder) { + static void verifyQuery(QueryBuilder queryBuilder) { if (queryBuilder instanceof RangeQueryBuilder) { RangeQueryBuilder rangeQueryBuilder = (RangeQueryBuilder) queryBuilder; if (rangeQueryBuilder.from() instanceof String) { String from = (String) rangeQueryBuilder.from(); String to = (String) rangeQueryBuilder.to(); if (from.contains("now") || to.contains("now")) { - throw new IllegalArgumentException("Percolator queries containing time range queries based on the " + - "current time are forbidden"); + throw new IllegalArgumentException("percolator queries containing time range queries based on the " + + "current time is unsupported"); } } + } else if (queryBuilder instanceof HasChildQueryBuilder) { + throw new IllegalArgumentException("the [has_child] query is unsupported inside a percolator query"); + } else if (queryBuilder instanceof HasParentQueryBuilder) { + throw new IllegalArgumentException("the [has_parent] query is unsupported inside a percolator query"); } else if (queryBuilder instanceof BoolQueryBuilder) { BoolQueryBuilder boolQueryBuilder = (BoolQueryBuilder) queryBuilder; List clauses = new ArrayList<>(); @@ -377,15 +386,15 @@ public class PercolatorFieldMapper extends FieldMapper { clauses.addAll(boolQueryBuilder.mustNot()); clauses.addAll(boolQueryBuilder.should()); for (QueryBuilder clause : clauses) { - verifyRangeQueries(clause); + verifyQuery(clause); } } else if (queryBuilder instanceof ConstantScoreQueryBuilder) { - verifyRangeQueries(((ConstantScoreQueryBuilder) queryBuilder).innerQuery()); + verifyQuery(((ConstantScoreQueryBuilder) queryBuilder).innerQuery()); } else if (queryBuilder instanceof FunctionScoreQueryBuilder) { - verifyRangeQueries(((FunctionScoreQueryBuilder) queryBuilder).query()); + verifyQuery(((FunctionScoreQueryBuilder) queryBuilder).query()); } else if (queryBuilder instanceof BoostingQueryBuilder) { - verifyRangeQueries(((BoostingQueryBuilder) queryBuilder).negativeQuery()); - verifyRangeQueries(((BoostingQueryBuilder) queryBuilder).positiveQuery()); + verifyQuery(((BoostingQueryBuilder) queryBuilder).negativeQuery()); + verifyQuery(((BoostingQueryBuilder) queryBuilder).positiveQuery()); } } diff --git a/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorFieldMapperTests.java b/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorFieldMapperTests.java index 364b90ef70a..df1e6ea6f8c 100644 --- a/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorFieldMapperTests.java +++ b/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorFieldMapperTests.java @@ -32,6 +32,7 @@ import org.apache.lucene.search.BooleanQuery; import org.apache.lucene.search.PhraseQuery; import org.apache.lucene.search.TermQuery; import org.apache.lucene.search.TermRangeQuery; +import org.apache.lucene.search.join.ScoreMode; import org.apache.lucene.util.BytesRef; import org.elasticsearch.Version; import org.elasticsearch.cluster.metadata.IndexMetaData; @@ -49,6 +50,8 @@ import org.elasticsearch.index.mapper.ParsedDocument; import org.elasticsearch.index.query.BoolQueryBuilder; import org.elasticsearch.index.query.BoostingQueryBuilder; import org.elasticsearch.index.query.ConstantScoreQueryBuilder; +import org.elasticsearch.index.query.HasChildQueryBuilder; +import org.elasticsearch.index.query.HasParentQueryBuilder; import org.elasticsearch.index.query.MatchAllQueryBuilder; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryParseContext; @@ -435,23 +438,31 @@ public class PercolatorFieldMapperTests extends ESSingleNodeTestCase { assertThat(e.getCause(), instanceOf(IllegalArgumentException.class)); } - public void testVerifyRangeQueries() { + public void testUnsupportedQueries() { RangeQueryBuilder rangeQuery1 = new RangeQueryBuilder("field").from("2016-01-01||/D").to("2017-01-01||/D"); RangeQueryBuilder rangeQuery2 = new RangeQueryBuilder("field").from("2016-01-01||/D").to("now"); - PercolatorFieldMapper.verifyRangeQueries(rangeQuery1); - expectThrows(IllegalArgumentException.class, () -> PercolatorFieldMapper.verifyRangeQueries(rangeQuery2)); - PercolatorFieldMapper.verifyRangeQueries(new BoolQueryBuilder().must(rangeQuery1)); + PercolatorFieldMapper.verifyQuery(rangeQuery1); + expectThrows(IllegalArgumentException.class, () -> PercolatorFieldMapper.verifyQuery(rangeQuery2)); + PercolatorFieldMapper.verifyQuery(new BoolQueryBuilder().must(rangeQuery1)); expectThrows(IllegalArgumentException.class, () -> - PercolatorFieldMapper.verifyRangeQueries(new BoolQueryBuilder().must(rangeQuery2))); - PercolatorFieldMapper.verifyRangeQueries(new ConstantScoreQueryBuilder((rangeQuery1))); + PercolatorFieldMapper.verifyQuery(new BoolQueryBuilder().must(rangeQuery2))); + PercolatorFieldMapper.verifyQuery(new ConstantScoreQueryBuilder((rangeQuery1))); expectThrows(IllegalArgumentException.class, () -> - PercolatorFieldMapper.verifyRangeQueries(new ConstantScoreQueryBuilder(rangeQuery2))); - PercolatorFieldMapper.verifyRangeQueries(new BoostingQueryBuilder(rangeQuery1, new MatchAllQueryBuilder())); + PercolatorFieldMapper.verifyQuery(new ConstantScoreQueryBuilder(rangeQuery2))); + PercolatorFieldMapper.verifyQuery(new BoostingQueryBuilder(rangeQuery1, new MatchAllQueryBuilder())); expectThrows(IllegalArgumentException.class, () -> - PercolatorFieldMapper.verifyRangeQueries(new BoostingQueryBuilder(rangeQuery2, new MatchAllQueryBuilder()))); - PercolatorFieldMapper.verifyRangeQueries(new FunctionScoreQueryBuilder(rangeQuery1, new RandomScoreFunctionBuilder())); + PercolatorFieldMapper.verifyQuery(new BoostingQueryBuilder(rangeQuery2, new MatchAllQueryBuilder()))); + PercolatorFieldMapper.verifyQuery(new FunctionScoreQueryBuilder(rangeQuery1, new RandomScoreFunctionBuilder())); expectThrows(IllegalArgumentException.class, () -> - PercolatorFieldMapper.verifyRangeQueries(new FunctionScoreQueryBuilder(rangeQuery2, new RandomScoreFunctionBuilder()))); + PercolatorFieldMapper.verifyQuery(new FunctionScoreQueryBuilder(rangeQuery2, new RandomScoreFunctionBuilder()))); + + HasChildQueryBuilder hasChildQuery = new HasChildQueryBuilder("_type", new MatchAllQueryBuilder(), ScoreMode.None); + expectThrows(IllegalArgumentException.class, () -> PercolatorFieldMapper.verifyQuery(hasChildQuery)); + expectThrows(IllegalArgumentException.class, () -> PercolatorFieldMapper.verifyQuery(new BoolQueryBuilder().must(hasChildQuery))); + + HasParentQueryBuilder hasParentQuery = new HasParentQueryBuilder("_type", new MatchAllQueryBuilder(), false); + expectThrows(IllegalArgumentException.class, () -> PercolatorFieldMapper.verifyQuery(hasParentQuery)); + expectThrows(IllegalArgumentException.class, () -> PercolatorFieldMapper.verifyQuery(new BoolQueryBuilder().must(hasParentQuery))); } private void assertQueryBuilder(BytesRef actual, QueryBuilder expected) throws IOException { diff --git a/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorIT.java b/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorIT.java index e4a10ce04a0..7d10b831bc8 100644 --- a/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorIT.java +++ b/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorIT.java @@ -1777,16 +1777,15 @@ public class PercolatorIT extends ESIntegTestCase { assertThat(response1.getMatches()[0].getId().string(), equalTo("1")); } - public void testParentChild() throws Exception { - // We don't fail p/c queries, but those queries are unusable because only a single document can be provided in - // the percolate api - + public void testFailParentChild() throws Exception { assertAcked(prepareCreate(INDEX_NAME) .addMapping(TYPE_NAME, "query", "type=percolator") .addMapping("child", "_parent", "type=parent").addMapping("parent")); - client().prepareIndex(INDEX_NAME, TYPE_NAME, "1") + Exception e = expectThrows(MapperParsingException.class, () -> client().prepareIndex(INDEX_NAME, TYPE_NAME, "1") .setSource(jsonBuilder().startObject().field("query", hasChildQuery("child", matchAllQuery(), ScoreMode.None)).endObject()) - .execute().actionGet(); + .get()); + assertThat(e.getCause(), instanceOf(IllegalArgumentException.class)); + assertThat(e.getCause().getMessage(), equalTo("the [has_child] query is unsupported inside a percolator query")); } public void testPercolateDocumentWithParentField() throws Exception { From 0c027acbf99e2082e552037a54b2d6b810af6afb Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Wed, 31 Aug 2016 08:11:03 -0400 Subject: [PATCH 08/10] Explicitly disable Netty key set replacement Netty replaces the backing set for the selector implementation. The value of doing this is questionable, and doing this requires permissions that we are not going to grant. This commit explicitly disables this optimization rather than relying on it failing due to lack of permissions. Relates #20249 --- distribution/src/main/resources/config/jvm.options | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/distribution/src/main/resources/config/jvm.options b/distribution/src/main/resources/config/jvm.options index 2feba025509..63eba4789fc 100644 --- a/distribution/src/main/resources/config/jvm.options +++ b/distribution/src/main/resources/config/jvm.options @@ -59,8 +59,9 @@ # use our provided JNA always versus the system one -Djna.nosys=true -# flag to explicitly tell Netty to not use unsafe +# flags to keep Netty from being unsafe -Dio.netty.noUnsafe=true +-Dio.netty.noKeySetOptimization=true ## heap dumps From 6f8a047942495c4b0298a3d3971c454d9fb71f4e Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Wed, 31 Aug 2016 09:46:27 -0400 Subject: [PATCH 09/10] Skip transport client plugin installed on JDK 9 This commit adds an assumption to PreBuiltTransportClientTests#testPluginInstalled on JDK 9. The underlying issue is that Netty attempts to access sun.nio.ch but this package is not exported from java.base on JDK 9. This throws an uncaught InaccessibleObjectException causing the test to fail. This assumption can be removed when Netty 4.1.6 is released as it will include a fix for this scenario. Relates #20251 --- .../transport/client/PreBuiltTransportClientTests.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/client/transport/src/test/java/org/elasticsearch/transport/client/PreBuiltTransportClientTests.java b/client/transport/src/test/java/org/elasticsearch/transport/client/PreBuiltTransportClientTests.java index 0b7c3380b94..a1d95b68af7 100644 --- a/client/transport/src/test/java/org/elasticsearch/transport/client/PreBuiltTransportClientTests.java +++ b/client/transport/src/test/java/org/elasticsearch/transport/client/PreBuiltTransportClientTests.java @@ -20,6 +20,7 @@ package org.elasticsearch.transport.client; import com.carrotsearch.randomizedtesting.RandomizedTest; +import org.apache.lucene.util.Constants; import org.elasticsearch.client.transport.TransportClient; import org.elasticsearch.common.network.NetworkModule; import org.elasticsearch.common.settings.Settings; @@ -40,6 +41,8 @@ public class PreBuiltTransportClientTests extends RandomizedTest { @Test public void testPluginInstalled() { + // TODO: remove when Netty 4.1.5 is upgraded to Netty 4.1.6 including https://github.com/netty/netty/pull/5778 + assumeFalse(Constants.JRE_IS_MINIMUM_JAVA9); try (TransportClient client = new PreBuiltTransportClient(Settings.EMPTY)) { Settings settings = client.settings(); assertEquals(Netty4Plugin.NETTY_TRANSPORT_NAME, NetworkModule.HTTP_DEFAULT_TYPE_SETTING.get(settings)); @@ -49,9 +52,7 @@ public class PreBuiltTransportClientTests extends RandomizedTest { @Test public void testInstallPluginTwice() { - - for (Class plugin : Arrays.asList(ReindexPlugin.class, PercolatorPlugin.class, - MustachePlugin.class)) { + for (Class plugin : Arrays.asList(ReindexPlugin.class, PercolatorPlugin.class, MustachePlugin.class)) { try { new PreBuiltTransportClient(Settings.EMPTY, plugin); fail("exception expected"); From 1a805bb675e71353db2eab8a976d058739e4594c Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Wed, 31 Aug 2016 10:51:17 -0400 Subject: [PATCH 10/10] Increase visibility of deprecation logger The deprecation logger is an important way to make visible features of Elasticsearch that are deprecated. Yet, the default logging makes the log messages for the deprecation logger invisible. We want these log messages to be visible, so the default logging for the deprecation logger should enable these log messages. This commit changes the log level of deprecation log message to warn, and configures the deprecation logger so that these log messages are visible out of the box. Relates #20254 --- .../elasticsearch/common/logging/DeprecationLogger.java | 4 ++-- .../org/elasticsearch/common/logging/ESLoggerTests.java | 2 +- distribution/src/main/resources/config/logging.yml | 4 ++-- docs/reference/setup/configuration.asciidoc | 9 ++++++--- 4 files changed, 11 insertions(+), 8 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/common/logging/DeprecationLogger.java b/core/src/main/java/org/elasticsearch/common/logging/DeprecationLogger.java index 5970f917322..afcef98fef7 100644 --- a/core/src/main/java/org/elasticsearch/common/logging/DeprecationLogger.java +++ b/core/src/main/java/org/elasticsearch/common/logging/DeprecationLogger.java @@ -131,9 +131,9 @@ public class DeprecationLogger { } } - logger.debug(formattedMsg); + logger.warn(formattedMsg); } else { - logger.debug(msg, params); + logger.warn(msg, params); } } diff --git a/core/src/test/java/org/elasticsearch/common/logging/ESLoggerTests.java b/core/src/test/java/org/elasticsearch/common/logging/ESLoggerTests.java index 67a6c0555c5..8826f456b63 100644 --- a/core/src/test/java/org/elasticsearch/common/logging/ESLoggerTests.java +++ b/core/src/test/java/org/elasticsearch/common/logging/ESLoggerTests.java @@ -138,7 +138,7 @@ public class ESLoggerTests extends ESTestCase { List deprecationEvents = deprecationAppender.getEvents(); LoggingEvent event = deprecationEvents.get(0); assertThat(event, notNullValue()); - assertThat(event.getLevel(), equalTo(Level.DEBUG)); + assertThat(event.getLevel(), equalTo(Level.WARN)); assertThat(event.getRenderedMessage(), equalTo("This is a deprecation message")); } diff --git a/distribution/src/main/resources/config/logging.yml b/distribution/src/main/resources/config/logging.yml index 11cd181ebd0..12cac3bd14e 100644 --- a/distribution/src/main/resources/config/logging.yml +++ b/distribution/src/main/resources/config/logging.yml @@ -6,8 +6,8 @@ logger: # log action execution errors for easier debugging action: DEBUG - # deprecation logging, turn to DEBUG to see them - deprecation: INFO, deprecation_log_file + # deprecation logging, turn to INFO to disable them + deprecation: WARN, deprecation_log_file # reduce the logging for aws, too much is logged under the default INFO com.amazonaws: WARN diff --git a/docs/reference/setup/configuration.asciidoc b/docs/reference/setup/configuration.asciidoc index 68f73fc96b8..88aaee8580d 100644 --- a/docs/reference/setup/configuration.asciidoc +++ b/docs/reference/setup/configuration.asciidoc @@ -136,14 +136,17 @@ out of the box. In addition to regular logging, Elasticsearch allows you to enable logging of deprecated actions. For example this allows you to determine early, if you need to migrate certain functionality in the future. By default, -deprecation logging is disabled. You can enable it in the `config/logging.yml` -file by setting the deprecation log level to `DEBUG`. +deprecation logging is enabled at the WARN level, the level at which all +deprecation log messages will be emitted. [source,yaml] -------------------------------------------------- -deprecation: DEBUG, deprecation_log_file +deprecation: WARN, deprecation_log_file -------------------------------------------------- This will create a daily rolling deprecation log file in your log directory. Check this file regularly, especially when you intend to upgrade to a new major version. + +You can disable it in the `config/logging.yml` file by setting the deprecation +log level to `INFO`.