From c1aa7088de60b0e9e70a945087614946b1fd7ad8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Thu, 11 Feb 2021 11:08:43 +0100 Subject: [PATCH] Collect multiple paths from XContentMapValues (#68540) Currently, when a document source mixed json object and dotted syntax like e.g. { "foo" : { "bar" : 0 }, "foo.bar" : 1}, extracting the values from the source map via XContentMapValues#extractValue returns after the first value for a path has been found. Instead we should exhaust all possibilities and return a list of objects of we find more than one value when extending the lookup path. Closes #68215 --- .../xcontent/support/XContentMapValues.java | 20 ++++++--- .../support/XContentMapValuesTests.java | 12 ++++++ .../fetch/subphase/FieldFetcherTests.java | 43 +++++++++++++++++++ 3 files changed, 69 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/xcontent/support/XContentMapValues.java b/server/src/main/java/org/elasticsearch/common/xcontent/support/XContentMapValues.java index 8da02b9a7c111..57420c5c79db3 100644 --- a/server/src/main/java/org/elasticsearch/common/xcontent/support/XContentMapValues.java +++ b/server/src/main/java/org/elasticsearch/common/xcontent/support/XContentMapValues.java @@ -154,19 +154,27 @@ private static Object extractValue(String[] pathElements, Map map = (Map) currentValue; String key = pathElements[index]; int nextIndex = index + 1; + List extractedValues = new ArrayList<>(); while (true) { if (map.containsKey(key)) { Object mapValue = map.get(key); if (mapValue == null) { - return nullValue; - } - Object val = extractValue(pathElements, nextIndex, mapValue, nullValue); - if (val != null) { - return val; + extractedValues.add(nullValue); + } else { + Object val = extractValue(pathElements, nextIndex, mapValue, nullValue); + if (val != null) { + extractedValues.add(val); + } } } if (nextIndex == pathElements.length) { - return null; + if (extractedValues.size() == 0) { + return null; + } else if (extractedValues.size() == 1) { + return extractedValues.get(0); + } else { + return extractedValues; + } } key += "." + pathElements[nextIndex]; nextIndex++; diff --git a/server/src/test/java/org/elasticsearch/common/xcontent/support/XContentMapValuesTests.java b/server/src/test/java/org/elasticsearch/common/xcontent/support/XContentMapValuesTests.java index cd47e0cfd2929..c6da6b5c8c923 100644 --- a/server/src/test/java/org/elasticsearch/common/xcontent/support/XContentMapValuesTests.java +++ b/server/src/test/java/org/elasticsearch/common/xcontent/support/XContentMapValuesTests.java @@ -33,6 +33,7 @@ import static org.elasticsearch.common.xcontent.XContentHelper.convertToMap; import static org.elasticsearch.common.xcontent.XContentHelper.toXContent; import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.hasEntry; import static org.hamcrest.Matchers.hasKey; import static org.hamcrest.Matchers.hasSize; @@ -197,6 +198,17 @@ public void testExtractValueMixedObjects() throws IOException { } } + public void testExtractValueMixedDottedObjectNotation() throws IOException { + XContentBuilder builder = XContentFactory.jsonBuilder().startObject() + .startObject("foo").field("cat", "meow").endObject() + .field("foo.cat", "miau") + .endObject(); + try (XContentParser parser = createParser(JsonXContent.jsonXContent, Strings.toString(builder))) { + Map map = parser.map(); + assertThat((List) XContentMapValues.extractValue("foo.cat", map), containsInAnyOrder("meow", "miau")); + } + } + public void testExtractRawValue() throws Exception { XContentBuilder builder = XContentFactory.jsonBuilder().startObject() .field("test", "value") diff --git a/server/src/test/java/org/elasticsearch/search/fetch/subphase/FieldFetcherTests.java b/server/src/test/java/org/elasticsearch/search/fetch/subphase/FieldFetcherTests.java index b1cf59afd16a0..79d406dd7f8f5 100644 --- a/server/src/test/java/org/elasticsearch/search/fetch/subphase/FieldFetcherTests.java +++ b/server/src/test/java/org/elasticsearch/search/fetch/subphase/FieldFetcherTests.java @@ -98,6 +98,49 @@ public void testMixedObjectValues() throws IOException { DocumentField field = fields.get("foo.bar"); assertThat(field.getValues().size(), equalTo(1)); assertThat(field.getValue(), equalTo("baz")); + + source = XContentFactory.jsonBuilder().startObject() + .startObject("foo").field("cat", "meow").endObject() + .field("foo.cat", "miau") + .endObject(); + + doc = mapperService.documentMapper().parse(source(Strings.toString(source))); + + fields = fetchFields(mapperService, source, "foo.cat"); + assertThat(fields.size(), equalTo(1)); + + field = fields.get("foo.cat"); + assertThat(field.getValues().size(), equalTo(2)); + assertThat(field.getValues(), containsInAnyOrder("meow", "miau")); + + source = XContentFactory.jsonBuilder().startObject() + .startObject("foo").field("cat", "meow").endObject() + .array("foo.cat", "miau", "purr") + .endObject(); + + doc = mapperService.documentMapper().parse(source(Strings.toString(source))); + + fields = fetchFields(mapperService, source, "foo.cat"); + assertThat(fields.size(), equalTo(1)); + + field = fields.get("foo.cat"); + assertThat(field.getValues().size(), equalTo(3)); + assertThat(field.getValues(), containsInAnyOrder("meow", "miau", "purr")); + } + + public void testMixedDottedObjectSyntax() throws IOException { + MapperService mapperService = createMapperService(); + XContentBuilder source = XContentFactory.jsonBuilder().startObject() + .startObject("object").field("field", "value").endObject() + .field("object.field", "value2") + .endObject(); + + Map fields = fetchFields(mapperService, source, "*"); + assertThat(fields.size(), equalTo(1)); + + DocumentField field = fields.get("object.field"); + assertThat(field.getValues().size(), equalTo(2)); + assertThat(field.getValues(), containsInAnyOrder("value", "value2")); } public void testNonExistentField() throws IOException {