From 6a5539049e13ded0968a6af6b3e7e8f087a944bb Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Thu, 30 Aug 2018 10:18:51 +0200 Subject: [PATCH 1/4] Fix serialization of empty field capabilities response When no response are required (no indices match the requested patterns) the empty response throws an NPE in the transport serialization (writeTo). --- .../elasticsearch/action/fieldcaps/FieldCapabilities.java | 4 ++-- .../action/fieldcaps/FieldCapabilitiesRequest.java | 1 - .../action/fieldcaps/FieldCapabilitiesResponse.java | 1 + server/src/main/java/org/elasticsearch/client/Client.java | 2 +- .../org/elasticsearch/client/support/AbstractClient.java | 4 ++-- .../search/fieldcaps/FieldCapabilitiesIT.java | 8 ++++++++ 6 files changed, 14 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilities.java b/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilities.java index 21bb452430e7a..5cfdba9294634 100644 --- a/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilities.java +++ b/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilities.java @@ -166,14 +166,14 @@ public String getName() { } /** - * Whether this field is indexed for search on all indices. + * Whether this field can be aggregated on all indices. */ public boolean isAggregatable() { return isAggregatable; } /** - * Whether this field can be aggregated on all indices. + * Whether this field is indexed for search on all indices. */ public boolean isSearchable() { return isSearchable; diff --git a/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesRequest.java b/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesRequest.java index 22d231d3711be..e9e77df5f9030 100644 --- a/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesRequest.java +++ b/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesRequest.java @@ -111,7 +111,6 @@ public String[] fields() { } /** - * * The list of indices to lookup */ public FieldCapabilitiesRequest indices(String... indices) { diff --git a/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesResponse.java b/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesResponse.java index 178639bd4348f..65a0808a3e075 100644 --- a/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesResponse.java +++ b/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesResponse.java @@ -65,6 +65,7 @@ private FieldCapabilitiesResponse(Map> re */ FieldCapabilitiesResponse() { this.responseMap = Collections.emptyMap(); + this.indexResponses = Collections.emptyList(); } /** diff --git a/server/src/main/java/org/elasticsearch/client/Client.java b/server/src/main/java/org/elasticsearch/client/Client.java index adb2f509b999e..f97f618347af5 100644 --- a/server/src/main/java/org/elasticsearch/client/Client.java +++ b/server/src/main/java/org/elasticsearch/client/Client.java @@ -455,7 +455,7 @@ public interface Client extends ElasticsearchClient, Releasable { /** * Builder for the field capabilities request. */ - FieldCapabilitiesRequestBuilder prepareFieldCaps(); + FieldCapabilitiesRequestBuilder prepareFieldCaps(String... indices); /** * An action that returns the field capabilities from the provided request diff --git a/server/src/main/java/org/elasticsearch/client/support/AbstractClient.java b/server/src/main/java/org/elasticsearch/client/support/AbstractClient.java index 86d9d2c445f3f..553c92e6de86e 100644 --- a/server/src/main/java/org/elasticsearch/client/support/AbstractClient.java +++ b/server/src/main/java/org/elasticsearch/client/support/AbstractClient.java @@ -651,8 +651,8 @@ public ActionFuture fieldCaps(FieldCapabilitiesReques } @Override - public FieldCapabilitiesRequestBuilder prepareFieldCaps() { - return new FieldCapabilitiesRequestBuilder(this, FieldCapabilitiesAction.INSTANCE); + public FieldCapabilitiesRequestBuilder prepareFieldCaps(String... indices) { + return new FieldCapabilitiesRequestBuilder(this, FieldCapabilitiesAction.INSTANCE, indices); } static class Admin implements AdminClient { diff --git a/server/src/test/java/org/elasticsearch/search/fieldcaps/FieldCapabilitiesIT.java b/server/src/test/java/org/elasticsearch/search/fieldcaps/FieldCapabilitiesIT.java index 8440357758ea8..8b610cc92da05 100644 --- a/server/src/test/java/org/elasticsearch/search/fieldcaps/FieldCapabilitiesIT.java +++ b/server/src/test/java/org/elasticsearch/search/fieldcaps/FieldCapabilitiesIT.java @@ -21,6 +21,7 @@ import org.elasticsearch.action.fieldcaps.FieldCapabilities; import org.elasticsearch.action.fieldcaps.FieldCapabilitiesResponse; +import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.plugins.MapperPlugin; @@ -148,4 +149,11 @@ public void testFieldAliasFilteringWithWildcard() { assertEquals(1, response.get().size()); assertTrue(response.get().containsKey("distance")); } + + public void testEmptyIndexPattern() { + FieldCapabilitiesResponse response = client().prepareFieldCaps("empty_index_pattern*") + .setFields("*") + .execute().actionGet(); + assertEquals(0, response.get().size()); + } } From 749bba759a86db69f72a78f41fef6940b8a82577 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Thu, 30 Aug 2018 10:56:37 +0200 Subject: [PATCH 2/4] unused import --- .../org/elasticsearch/search/fieldcaps/FieldCapabilitiesIT.java | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/search/fieldcaps/FieldCapabilitiesIT.java b/server/src/test/java/org/elasticsearch/search/fieldcaps/FieldCapabilitiesIT.java index 8b610cc92da05..a117f0df3cce9 100644 --- a/server/src/test/java/org/elasticsearch/search/fieldcaps/FieldCapabilitiesIT.java +++ b/server/src/test/java/org/elasticsearch/search/fieldcaps/FieldCapabilitiesIT.java @@ -21,7 +21,6 @@ import org.elasticsearch.action.fieldcaps.FieldCapabilities; import org.elasticsearch.action.fieldcaps.FieldCapabilitiesResponse; -import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.plugins.MapperPlugin; From a605160944a4956e1b68dec8b01311342aefdfc2 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Thu, 30 Aug 2018 14:20:07 +0200 Subject: [PATCH 3/4] apply review comments --- .../fieldcaps/FieldCapabilitiesAction.java | 4 +- .../fieldcaps/FieldCapabilitiesResponse.java | 9 ++-- .../FieldCapabilitiesResponseTests.java | 53 +++++++++++++++---- .../search/fieldcaps/FieldCapabilitiesIT.java | 7 --- 4 files changed, 51 insertions(+), 22 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesAction.java b/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesAction.java index 39c6ecce308e0..f249527b0f8b9 100644 --- a/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesAction.java +++ b/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesAction.java @@ -21,6 +21,8 @@ import org.elasticsearch.action.Action; +import java.util.Collections; + public class FieldCapabilitiesAction extends Action { public static final FieldCapabilitiesAction INSTANCE = new FieldCapabilitiesAction(); @@ -32,6 +34,6 @@ private FieldCapabilitiesAction() { @Override public FieldCapabilitiesResponse newResponse() { - return new FieldCapabilitiesResponse(); + return new FieldCapabilitiesResponse(Collections.emptyMap()); } } diff --git a/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesResponse.java b/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesResponse.java index 65a0808a3e075..f908ec7b1b289 100644 --- a/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesResponse.java +++ b/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesResponse.java @@ -35,6 +35,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.stream.Collectors; /** @@ -56,16 +57,15 @@ public class FieldCapabilitiesResponse extends ActionResponse implements ToXCont private FieldCapabilitiesResponse(Map> responseMap, List indexResponses) { - this.responseMap = responseMap; - this.indexResponses = indexResponses; + this.responseMap = Objects.requireNonNull(responseMap); + this.indexResponses = Objects.requireNonNull(indexResponses); } /** * Used for serialization */ FieldCapabilitiesResponse() { - this.responseMap = Collections.emptyMap(); - this.indexResponses = Collections.emptyList(); + this(Collections.emptyMap(), Collections.emptyList()); } /** @@ -82,6 +82,7 @@ public Map> get() { List getIndexResponses() { return indexResponses; } + /** * * Get the field capabilities per type for the provided {@code field}. diff --git a/server/src/test/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesResponseTests.java b/server/src/test/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesResponseTests.java index b38240632421a..90b730660ddd9 100644 --- a/server/src/test/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesResponseTests.java +++ b/server/src/test/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesResponseTests.java @@ -28,11 +28,15 @@ import org.elasticsearch.test.AbstractStreamableXContentTestCase; import java.io.IOException; +import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.function.Predicate; +import static com.carrotsearch.randomizedtesting.RandomizedTest.randomAsciiLettersOfLength; + public class FieldCapabilitiesResponseTests extends AbstractStreamableXContentTestCase { @@ -48,22 +52,46 @@ protected FieldCapabilitiesResponse createBlankInstance() { @Override protected FieldCapabilitiesResponse createTestInstance() { - Map> responses = new HashMap<>(); + if (randomBoolean()) { + // merged responses + Map> responses = new HashMap<>(); + + String[] fields = generateRandomStringArray(5, 10, false, true); + assertNotNull(fields); + + for (String field : fields) { + Map typesToCapabilities = new HashMap<>(); + String[] types = generateRandomStringArray(5, 10, false, false); + assertNotNull(types); + + for (String type : types) { + typesToCapabilities.put(type, FieldCapabilitiesTests.randomFieldCaps(field)); + } + responses.put(field, typesToCapabilities); + } + return new FieldCapabilitiesResponse(responses); + } else { + // non-merged responses + List responses = new ArrayList<>(); + int numResponse = randomIntBetween(0, 10); + for (int i = 0; i < numResponse; i++) { + responses.add(createRandomIndexResponse()); + } + return new FieldCapabilitiesResponse(responses); + } + } + + + private FieldCapabilitiesIndexResponse createRandomIndexResponse() { + Map responses = new HashMap<>(); String[] fields = generateRandomStringArray(5, 10, false, true); assertNotNull(fields); for (String field : fields) { - Map typesToCapabilities = new HashMap<>(); - String[] types = generateRandomStringArray(5, 10, false, false); - assertNotNull(types); - - for (String type : types) { - typesToCapabilities.put(type, FieldCapabilitiesTests.randomFieldCaps(field)); - } - responses.put(field, typesToCapabilities); + responses.put(field, FieldCapabilitiesTests.randomFieldCaps(field)); } - return new FieldCapabilitiesResponse(responses); + return new FieldCapabilitiesIndexResponse(randomAsciiLettersOfLength(10), responses); } @Override @@ -138,6 +166,11 @@ public void testToXContent() throws IOException { "}").replaceAll("\\s+", ""), generatedResponse); } + public void testEmptyResponse() throws IOException { + FieldCapabilitiesResponse testInstance = new FieldCapabilitiesResponse(); + assertSerialization(testInstance); + } + private static FieldCapabilitiesResponse createSimpleResponse() { Map titleCapabilities = new HashMap<>(); titleCapabilities.put("text", new FieldCapabilities("title", "text", true, false)); diff --git a/server/src/test/java/org/elasticsearch/search/fieldcaps/FieldCapabilitiesIT.java b/server/src/test/java/org/elasticsearch/search/fieldcaps/FieldCapabilitiesIT.java index a117f0df3cce9..8440357758ea8 100644 --- a/server/src/test/java/org/elasticsearch/search/fieldcaps/FieldCapabilitiesIT.java +++ b/server/src/test/java/org/elasticsearch/search/fieldcaps/FieldCapabilitiesIT.java @@ -148,11 +148,4 @@ public void testFieldAliasFilteringWithWildcard() { assertEquals(1, response.get().size()); assertTrue(response.get().containsKey("distance")); } - - public void testEmptyIndexPattern() { - FieldCapabilitiesResponse response = client().prepareFieldCaps("empty_index_pattern*") - .setFields("*") - .execute().actionGet(); - assertEquals(0, response.get().size()); - } } From de6229f7d8bb5ff685d811c3e735651d81a7dab2 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Thu, 30 Aug 2018 14:42:38 +0200 Subject: [PATCH 4/4] change ctr --- .../action/fieldcaps/FieldCapabilitiesAction.java | 4 +--- .../action/fieldcaps/TransportFieldCapabilitiesAction.java | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesAction.java b/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesAction.java index f249527b0f8b9..39c6ecce308e0 100644 --- a/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesAction.java +++ b/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesAction.java @@ -21,8 +21,6 @@ import org.elasticsearch.action.Action; -import java.util.Collections; - public class FieldCapabilitiesAction extends Action { public static final FieldCapabilitiesAction INSTANCE = new FieldCapabilitiesAction(); @@ -34,6 +32,6 @@ private FieldCapabilitiesAction() { @Override public FieldCapabilitiesResponse newResponse() { - return new FieldCapabilitiesResponse(Collections.emptyMap()); + return new FieldCapabilitiesResponse(); } } diff --git a/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesAction.java b/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesAction.java index ef0d19a265583..b8d1f477ac108 100644 --- a/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesAction.java +++ b/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesAction.java @@ -90,7 +90,7 @@ protected void doExecute(Task task, FieldCapabilitiesRequest request, final Acti } }; if (totalNumRequest == 0) { - listener.onResponse(new FieldCapabilitiesResponse()); + listener.onResponse(new FieldCapabilitiesResponse(Collections.emptyMap())); } else { ActionListener innerListener = new ActionListener() { @Override