From c7ca64cc080ba9426e82e9223f71d0e9177a7868 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Tue, 26 May 2015 16:14:54 +0200 Subject: [PATCH] Fix typed parameters in IndexRequestBuilder and CreateIndexRequestBuilder IndexRequestBuilder#setSource as well as CreateIndexRequestBuilder#setSettings and CreateIndexRequestBuilder#setSouce() will not work with Map argument although the API looks like it should. This PR fixes the problem introducing correct wildcard parameters and adds tests. Closes #10825 --- .../indices/create/CreateIndexRequest.java | 4 +- .../create/CreateIndexRequestBuilder.java | 4 +- .../action/index/IndexRequestBuilder.java | 4 +- .../create/CreateIndexRequestBuilderTest.java | 112 ++++++++++++++++++ .../action/index/IndexRequestBuilderTest.java | 87 ++++++++++++++ .../HeadersAndContextCopyClientTests.java | 52 +++----- .../org/elasticsearch/rest/NoOpClient.java | 54 +++++++++ 7 files changed, 276 insertions(+), 41 deletions(-) create mode 100644 src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexRequestBuilderTest.java create mode 100644 src/test/java/org/elasticsearch/action/index/IndexRequestBuilderTest.java create mode 100644 src/test/java/org/elasticsearch/rest/NoOpClient.java diff --git a/src/main/java/org/elasticsearch/action/admin/indices/create/CreateIndexRequest.java b/src/main/java/org/elasticsearch/action/admin/indices/create/CreateIndexRequest.java index 873e6e9e8ca..3a174484ef9 100644 --- a/src/main/java/org/elasticsearch/action/admin/indices/create/CreateIndexRequest.java +++ b/src/main/java/org/elasticsearch/action/admin/indices/create/CreateIndexRequest.java @@ -377,9 +377,9 @@ public class CreateIndexRequest extends AcknowledgedRequest * Sets the settings and mappings as a single source. */ @SuppressWarnings("unchecked") - public CreateIndexRequest source(Map source) { + public CreateIndexRequest source(Map source) { boolean found = false; - for (Map.Entry entry : source.entrySet()) { + for (Map.Entry entry : source.entrySet()) { String name = entry.getKey(); if (name.equals("settings")) { found = true; diff --git a/src/main/java/org/elasticsearch/action/admin/indices/create/CreateIndexRequestBuilder.java b/src/main/java/org/elasticsearch/action/admin/indices/create/CreateIndexRequestBuilder.java index 12648db563a..637c6d7ba08 100644 --- a/src/main/java/org/elasticsearch/action/admin/indices/create/CreateIndexRequestBuilder.java +++ b/src/main/java/org/elasticsearch/action/admin/indices/create/CreateIndexRequestBuilder.java @@ -93,7 +93,7 @@ public class CreateIndexRequestBuilder extends AcknowledgedRequestBuilder source) { + public CreateIndexRequestBuilder setSettings(Map source) { request.settings(source); return this; } @@ -223,7 +223,7 @@ public class CreateIndexRequestBuilder extends AcknowledgedRequestBuilder source) { + public CreateIndexRequestBuilder setSource(Map source) { request.source(source); return this; } diff --git a/src/main/java/org/elasticsearch/action/index/IndexRequestBuilder.java b/src/main/java/org/elasticsearch/action/index/IndexRequestBuilder.java index cf494358628..5b6674e38a1 100644 --- a/src/main/java/org/elasticsearch/action/index/IndexRequestBuilder.java +++ b/src/main/java/org/elasticsearch/action/index/IndexRequestBuilder.java @@ -90,7 +90,7 @@ public class IndexRequestBuilder extends ReplicationRequestBuilder source) { + public IndexRequestBuilder setSource(Map source) { request.source(source); return this; } @@ -100,7 +100,7 @@ public class IndexRequestBuilder extends ReplicationRequestBuilder source, XContentType contentType) { + public IndexRequestBuilder setSource(Map source, XContentType contentType) { request.source(source, contentType); return this; } diff --git a/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexRequestBuilderTest.java b/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexRequestBuilderTest.java new file mode 100644 index 00000000000..31576c38d06 --- /dev/null +++ b/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexRequestBuilderTest.java @@ -0,0 +1,112 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch 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.action.admin.indices.create; + +import org.elasticsearch.action.index.IndexRequestBuilderTest; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.rest.NoOpClient; +import org.elasticsearch.test.ElasticsearchTestCase; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.util.HashMap; +import java.util.Map; + +public class CreateIndexRequestBuilderTest extends ElasticsearchTestCase { + + private static final String KEY = "my.settings.key"; + private static final String VALUE = "my.settings.value"; + private NoOpClient testClient; + + @Override + @Before + public void setUp() throws Exception { + super.setUp(); + this.testClient = new NoOpClient(getTestName()); + } + + @Override + @After + public void tearDown() throws Exception { + this.testClient.close(); + super.tearDown(); + } + + /** + * test setting the source with available setters + */ + @Test + public void testSetSource() throws IOException { + CreateIndexRequestBuilder builder = new CreateIndexRequestBuilder(this.testClient, CreateIndexAction.INSTANCE); + builder.setSource("{\""+KEY+"\" : \""+VALUE+"\"}"); + assertEquals(VALUE, builder.request().settings().get(KEY)); + + XContentBuilder xContent = XContentFactory.jsonBuilder().startObject().field(KEY, VALUE).endObject(); + xContent.close(); + builder.setSource(xContent); + assertEquals(VALUE, builder.request().settings().get(KEY)); + + ByteArrayOutputStream docOut = new ByteArrayOutputStream(); + XContentBuilder doc = XContentFactory.jsonBuilder(docOut).startObject().field(KEY, VALUE).endObject(); + doc.close(); + builder.setSource(docOut.toByteArray()); + assertEquals(VALUE, builder.request().settings().get(KEY)); + + Map settingsMap = new HashMap<>(); + settingsMap.put(KEY, VALUE); + builder.setSettings(settingsMap); + assertEquals(VALUE, builder.request().settings().get(KEY)); + } + + /** + * test setting the settings with available setters + */ + @Test + public void testSetSettings() throws IOException { + CreateIndexRequestBuilder builder = new CreateIndexRequestBuilder(this.testClient, CreateIndexAction.INSTANCE); + builder.setSettings(KEY, VALUE); + assertEquals(VALUE, builder.request().settings().get(KEY)); + + builder.setSettings("{\""+KEY+"\" : \""+VALUE+"\"}"); + assertEquals(VALUE, builder.request().settings().get(KEY)); + + builder.setSettings(Settings.builder().put(KEY, VALUE)); + assertEquals(VALUE, builder.request().settings().get(KEY)); + + builder.setSettings(Settings.builder().put(KEY, VALUE).build()); + assertEquals(VALUE, builder.request().settings().get(KEY)); + + Map settingsMap = new HashMap<>(); + settingsMap.put(KEY, VALUE); + builder.setSettings(settingsMap); + assertEquals(VALUE, builder.request().settings().get(KEY)); + + XContentBuilder xContent = XContentFactory.jsonBuilder().startObject().field(KEY, VALUE).endObject(); + xContent.close(); + builder.setSettings(xContent); + assertEquals(VALUE, builder.request().settings().get(KEY)); + } + +} diff --git a/src/test/java/org/elasticsearch/action/index/IndexRequestBuilderTest.java b/src/test/java/org/elasticsearch/action/index/IndexRequestBuilderTest.java new file mode 100644 index 00000000000..478e12051d6 --- /dev/null +++ b/src/test/java/org/elasticsearch/action/index/IndexRequestBuilderTest.java @@ -0,0 +1,87 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch 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.action.index; + +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.common.xcontent.XContentHelper; +import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.rest.NoOpClient; +import org.elasticsearch.test.ElasticsearchTestCase; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import java.io.ByteArrayOutputStream; +import java.util.HashMap; +import java.util.Map; + +public class IndexRequestBuilderTest extends ElasticsearchTestCase { + + private static final String EXPECTED_SOURCE = "{\"SomeKey\":\"SomeValue\"}"; + private NoOpClient testClient; + + @Override + @Before + public void setUp() throws Exception { + super.setUp(); + this.testClient = new NoOpClient(getTestName()); + } + + @Override + @After + public void tearDown() throws Exception { + this.testClient.close(); + super.tearDown(); + } + + /** + * test setting the source for the request with different available setters + */ + @Test + public void testSetSource() throws Exception { + IndexRequestBuilder indexRequestBuilder = new IndexRequestBuilder(this.testClient, IndexAction.INSTANCE); + Map source = new HashMap<>(); + source.put("SomeKey", "SomeValue"); + indexRequestBuilder.setSource(source); + assertEquals(EXPECTED_SOURCE, XContentHelper.convertToJson(indexRequestBuilder.request().source(), true)); + + indexRequestBuilder.setSource(source, XContentType.JSON); + assertEquals(EXPECTED_SOURCE, XContentHelper.convertToJson(indexRequestBuilder.request().source(), true)); + + indexRequestBuilder.setSource("SomeKey", "SomeValue"); + assertEquals(EXPECTED_SOURCE, XContentHelper.convertToJson(indexRequestBuilder.request().source(), true)); + + // force the Object... setter + indexRequestBuilder.setSource((Object) "SomeKey", "SomeValue"); + assertEquals(EXPECTED_SOURCE, XContentHelper.convertToJson(indexRequestBuilder.request().source(), true)); + + ByteArrayOutputStream docOut = new ByteArrayOutputStream(); + XContentBuilder doc = XContentFactory.jsonBuilder(docOut).startObject().field("SomeKey", "SomeValue").endObject(); + doc.close(); + indexRequestBuilder.setSource(docOut.toByteArray()); + assertEquals(EXPECTED_SOURCE, XContentHelper.convertToJson(indexRequestBuilder.request().source(), true)); + + doc = XContentFactory.jsonBuilder().startObject().field("SomeKey", "SomeValue").endObject(); + doc.close(); + indexRequestBuilder.setSource(doc); + assertEquals(EXPECTED_SOURCE, XContentHelper.convertToJson(indexRequestBuilder.request().source(), true)); + } +} diff --git a/src/test/java/org/elasticsearch/rest/HeadersAndContextCopyClientTests.java b/src/test/java/org/elasticsearch/rest/HeadersAndContextCopyClientTests.java index 9d87de9a354..6a110cd5da4 100644 --- a/src/test/java/org/elasticsearch/rest/HeadersAndContextCopyClientTests.java +++ b/src/test/java/org/elasticsearch/rest/HeadersAndContextCopyClientTests.java @@ -20,8 +20,9 @@ package org.elasticsearch.rest; import com.google.common.collect.Maps; -import org.elasticsearch.ElasticsearchException; -import org.elasticsearch.action.*; + +import org.elasticsearch.action.ActionRequest; +import org.elasticsearch.action.ActionRequestBuilder; import org.elasticsearch.action.admin.cluster.health.ClusterHealthRequest; import org.elasticsearch.action.admin.cluster.state.ClusterStateRequest; import org.elasticsearch.action.admin.cluster.stats.ClusterStatsRequest; @@ -31,24 +32,26 @@ import org.elasticsearch.action.admin.indices.flush.FlushRequest; import org.elasticsearch.action.get.GetRequest; import org.elasticsearch.action.index.IndexRequest; import org.elasticsearch.action.search.SearchRequest; -import org.elasticsearch.client.*; -import org.elasticsearch.client.support.AbstractClient; -import org.elasticsearch.client.support.Headers; +import org.elasticsearch.client.Client; +import org.elasticsearch.client.Requests; import org.elasticsearch.common.collect.ImmutableOpenMap; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.test.ElasticsearchTestCase; import org.elasticsearch.test.rest.FakeRestRequest; -import org.elasticsearch.threadpool.ThreadPool; import org.junit.Test; -import java.util.*; +import java.util.Arrays; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.notNullValue; -import static org.hamcrest.Matchers.*; +import static org.hamcrest.Matchers.is; public class HeadersAndContextCopyClientTests extends ElasticsearchTestCase { @@ -108,7 +111,7 @@ public class HeadersAndContextCopyClientTests extends ElasticsearchTestCase { expectedContext.putAll(transportContext); expectedContext.putAll(restContext); - try (Client client = client(new NoOpClient(), new FakeRestRequest(restHeaders, restContext), usefulRestHeaders)) { + try (Client client = client(new NoOpClient(getTestName()), new FakeRestRequest(restHeaders, restContext), usefulRestHeaders)) { SearchRequest searchRequest = Requests.searchRequest(); putHeaders(searchRequest, transportHeaders); @@ -154,7 +157,7 @@ public class HeadersAndContextCopyClientTests extends ElasticsearchTestCase { expectedContext.putAll(transportContext); expectedContext.putAll(restContext); - try (Client client = client(new NoOpClient(), new FakeRestRequest(restHeaders, expectedContext), usefulRestHeaders)) { + try (Client client = client(new NoOpClient(getTestName()), new FakeRestRequest(restHeaders, expectedContext), usefulRestHeaders)) { ClusterHealthRequest clusterHealthRequest = Requests.clusterHealthRequest(); putHeaders(clusterHealthRequest, transportHeaders); @@ -200,7 +203,7 @@ public class HeadersAndContextCopyClientTests extends ElasticsearchTestCase { expectedContext.putAll(transportContext); expectedContext.putAll(restContext); - try (Client client = client(new NoOpClient(), new FakeRestRequest(restHeaders, restContext), usefulRestHeaders)) { + try (Client client = client(new NoOpClient(getTestName()), new FakeRestRequest(restHeaders, restContext), usefulRestHeaders)) { CreateIndexRequest createIndexRequest = Requests.createIndexRequest("test"); putHeaders(createIndexRequest, transportHeaders); @@ -246,7 +249,7 @@ public class HeadersAndContextCopyClientTests extends ElasticsearchTestCase { expectedContext.putAll(transportContext); expectedContext.putAll(restContext); - try (Client client = client(new NoOpClient(), new FakeRestRequest(restHeaders, restContext), usefulRestHeaders)) { + try (Client client = client(new NoOpClient(getTestName()), new FakeRestRequest(restHeaders, restContext), usefulRestHeaders)) { ActionRequestBuilder requestBuilders[] = new ActionRequestBuilder[]{ client.prepareIndex("index", "type"), @@ -287,7 +290,7 @@ public class HeadersAndContextCopyClientTests extends ElasticsearchTestCase { expectedContext.putAll(transportContext); expectedContext.putAll(restContext); - try (Client client = client(new NoOpClient(), new FakeRestRequest(restHeaders, restContext), usefulRestHeaders)) { + try (Client client = client(new NoOpClient(getTestName()), new FakeRestRequest(restHeaders, restContext), usefulRestHeaders)) { ActionRequestBuilder requestBuilders[] = new ActionRequestBuilder[]{ client.admin().cluster().prepareNodesInfo(), @@ -327,7 +330,7 @@ public class HeadersAndContextCopyClientTests extends ElasticsearchTestCase { expectedContext.putAll(transportContext); expectedContext.putAll(restContext); - try (Client client = client(new NoOpClient(), new FakeRestRequest(restHeaders, restContext), usefulRestHeaders)) { + try (Client client = client(new NoOpClient(getTestName()), new FakeRestRequest(restHeaders, restContext), usefulRestHeaders)) { ActionRequestBuilder requestBuilders[] = new ActionRequestBuilder[]{ client.admin().indices().prepareValidateQuery(), @@ -420,25 +423,4 @@ public class HeadersAndContextCopyClientTests extends ElasticsearchTestCase { } } } - - private class NoOpClient extends AbstractClient { - - public NoOpClient() { - super(Settings.EMPTY, new ThreadPool(getTestName()), Headers.EMPTY); - } - - @Override - protected > void doExecute(Action action, Request request, ActionListener listener) { - listener.onResponse(null); - } - - @Override - public void close() { - try { - terminate(threadPool()); - } catch (Throwable t) { - throw new ElasticsearchException(t.getMessage(), t); - } - } - } } diff --git a/src/test/java/org/elasticsearch/rest/NoOpClient.java b/src/test/java/org/elasticsearch/rest/NoOpClient.java new file mode 100644 index 00000000000..245bdb96a33 --- /dev/null +++ b/src/test/java/org/elasticsearch/rest/NoOpClient.java @@ -0,0 +1,54 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch 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.rest; + +import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.action.Action; +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.ActionRequest; +import org.elasticsearch.action.ActionRequestBuilder; +import org.elasticsearch.action.ActionResponse; +import org.elasticsearch.client.support.AbstractClient; +import org.elasticsearch.client.support.Headers; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.threadpool.ThreadPool; + +import java.util.concurrent.TimeUnit; + +public class NoOpClient extends AbstractClient { + + public NoOpClient(String testName) { + super(Settings.EMPTY, new ThreadPool(testName), Headers.EMPTY); + } + + @Override + protected > void doExecute(Action action, Request request, ActionListener listener) { + listener.onResponse(null); + } + + @Override + public void close() { + try { + ThreadPool.terminate(threadPool(), 10, TimeUnit.SECONDS); + } catch (Throwable t) { + throw new ElasticsearchException(t.getMessage(), t); + } + } +} \ No newline at end of file