From 4847553468baa8820dc7f070fb59501bccbeaf7e Mon Sep 17 00:00:00 2001 From: bellengao Date: Sun, 29 Mar 2020 22:02:05 +0800 Subject: [PATCH 1/4] Fix updating include_in_parent/include_in_root of nested field throws no error --- .../index/mapper/ObjectMapper.java | 10 ++++++ .../index/mapper/NestedObjectMapperTests.java | 33 +++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java index 157988dab3709..e42ec8107af0a 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java @@ -490,6 +490,16 @@ private static void checkEnabledFieldChange(ObjectMapper mergeWith, Mapper merge final String path = mergeWith.fullPath() + "." + mergeWithObjectMapper.simpleName() + ".enabled"; throw new MapperException("Can't update attribute for type [" + path + "] in index mapping"); } + + if (mergeIntoObjectMapper.nested().isIncludeInParent() != mergeWithObjectMapper.nested().isIncludeInParent()) { + final String path = mergeWith.fullPath() + "." + mergeWithObjectMapper.simpleName() + ".include_in_parent"; + throw new MapperException("Can't update attribute for type [" + path + "] in index mapping"); + } + + if (mergeIntoObjectMapper.nested().isIncludeInRoot() != mergeWithObjectMapper.nested().isIncludeInRoot()) { + final String path = mergeWith.fullPath() + "." + mergeWithObjectMapper.simpleName() + ".include_in_root"; + throw new MapperException("Can't update attribute for type [" + path + "] in index mapping"); + } } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/NestedObjectMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/NestedObjectMapperTests.java index 93b235f5e0bfe..1ee4103ba4306 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/NestedObjectMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/NestedObjectMapperTests.java @@ -43,6 +43,7 @@ import java.util.HashSet; import java.util.function.Function; +import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_VERSION_CREATED; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; @@ -742,4 +743,36 @@ public void testReorderParent() throws IOException { assertThat(doc.docs().get(1).get("nested1.field2"), equalTo("4")); assertThat(doc.docs().get(2).get("field"), equalTo("value")); } + + public void testMergeNestedMappings() { + final Settings indexSettings = Settings.builder().put(SETTING_VERSION_CREATED, Version.CURRENT).build(); + final Mapper.BuilderContext context = new Mapper.BuilderContext(indexSettings, new ContentPath()); + + ObjectMapper.Nested nested = ObjectMapper.Nested.newNested(false,false); + final ObjectMapper nestedMapper = new ObjectMapper.Builder("nested1").nested(nested).build(context); + final TextFieldMapper.TextFieldType fieldType = new TextFieldMapper.TextFieldType(); + final TextFieldMapper fieldMapper = new TextFieldMapper("field1", fieldType, fieldType, -1, null, + indexSettings, FieldMapper.MultiFields.empty(), FieldMapper.CopyTo.empty()); + nestedMapper.putMapper(fieldMapper); + final RootObjectMapper rootObjectMapper = new RootObjectMapper.Builder("type1").build(context); + rootObjectMapper.putMapper(nestedMapper); + + // cannot update `include_in_parent` dynamically + ObjectMapper.Nested newNested1 = ObjectMapper.Nested.newNested(true,false); + final ObjectMapper newNestedMapper1 = new ObjectMapper.Builder("nested1").nested(newNested1).build(context); + newNestedMapper1.putMapper(fieldMapper); + final RootObjectMapper mergeWith1 = new RootObjectMapper.Builder("type1").build(context); + mergeWith1.putMapper(newNestedMapper1); + MapperException e1 = expectThrows(MapperException.class, () -> rootObjectMapper.merge(mergeWith1)); + assertEquals("Can't update attribute for type [type1.nested1.include_in_parent] in index mapping", e1.getMessage()); + + // cannot update `include_in_root` dynamically + ObjectMapper.Nested newNested2 = ObjectMapper.Nested.newNested(false,true); + final ObjectMapper newNestedMapper2 = new ObjectMapper.Builder("nested1").nested(newNested2).build(context); + newNestedMapper2.putMapper(fieldMapper); + final RootObjectMapper mergeWith2 = new RootObjectMapper.Builder("type1").build(context); + mergeWith2.putMapper(newNestedMapper2); + MapperException e2 = expectThrows(MapperException.class, () -> rootObjectMapper.merge(mergeWith2)); + assertEquals("Can't update attribute for type [type1.nested1.include_in_root] in index mapping", e2.getMessage()); + } } From 8c7f1746f4298831be1edef26dc1cb3501a9e173 Mon Sep 17 00:00:00 2001 From: bellengao Date: Sat, 4 Apr 2020 16:34:15 +0800 Subject: [PATCH 2/4] modify the test method --- .../index/mapper/ObjectMapper.java | 20 ++++++- .../index/mapper/NestedObjectMapperTests.java | 55 ++++++++++--------- 2 files changed, 46 insertions(+), 29 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java index b1ec7d0361cb4..33ef59bba800f 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java @@ -465,9 +465,7 @@ protected void doMerge(final ObjectMapper mergeWith) { this.dynamic = mergeWith.dynamic; } - if (isEnabled() != mergeWith.isEnabled()) { - throw new MapperException("The [enabled] parameter can't be updated for the object mapping [" + name() + "]."); - } + checkObjectMapperParameters(mergeWith); for (Mapper mergeWithMapper : mergeWith) { Mapper mergeIntoMapper = mappers.get(mergeWithMapper.simpleName()); @@ -484,6 +482,22 @@ protected void doMerge(final ObjectMapper mergeWith) { } } + private void checkObjectMapperParameters(final ObjectMapper mergeWith) { + if (isEnabled() != mergeWith.isEnabled()) { + throw new MapperException("The [enabled] parameter can't be updated for the object mapping [" + name() + "]."); + } + + if (nested().isIncludeInParent() != mergeWith.nested().isIncludeInParent()) { + throw new MapperException("The [include_in_parent] parameter can't be updated for the nested object mapping [" + + name() + "]."); + } + + if (nested().isIncludeInRoot() != mergeWith.nested().isIncludeInRoot()) { + throw new MapperException("The [include_in_root] parameter can't be updated for the nested object mapping [" + + name() + "]."); + } + } + @Override public ObjectMapper updateFieldType(Map fullNameToFieldType) { List updatedMappers = null; diff --git a/server/src/test/java/org/elasticsearch/index/mapper/NestedObjectMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/NestedObjectMapperTests.java index cf229e3658eb9..9942247cbb2cb 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/NestedObjectMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/NestedObjectMapperTests.java @@ -43,7 +43,6 @@ import java.util.HashSet; import java.util.function.Function; -import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_VERSION_CREATED; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; @@ -744,35 +743,39 @@ public void testReorderParent() throws IOException { assertThat(doc.docs().get(2).get("field"), equalTo("value")); } - public void testMergeNestedMappings() { - final Settings indexSettings = Settings.builder().put(SETTING_VERSION_CREATED, Version.CURRENT).build(); - final Mapper.BuilderContext context = new Mapper.BuilderContext(indexSettings, new ContentPath()); + public void testMergeNestedMappings() throws IOException { + MapperService mapperService = createIndex("index1", Settings.EMPTY, jsonBuilder().startObject() + .startObject("properties") + .startObject("nested1") + .field("type", "nested") + .endObject() + .endObject().endObject()).mapperService(); + ObjectMapper objectMapper1 = mapperService.getObjectMapper("nested1"); - ObjectMapper.Nested nested = ObjectMapper.Nested.newNested(false,false); - final ObjectMapper nestedMapper = new ObjectMapper.Builder("nested1").nested(nested).build(context); - final TextFieldMapper.TextFieldType fieldType = new TextFieldMapper.TextFieldType(); - final TextFieldMapper fieldMapper = new TextFieldMapper("field1", fieldType, fieldType, -1, null, - indexSettings, FieldMapper.MultiFields.empty(), FieldMapper.CopyTo.empty()); - nestedMapper.putMapper(fieldMapper); - final RootObjectMapper rootObjectMapper = new RootObjectMapper.Builder("type1").build(context); - rootObjectMapper.putMapper(nestedMapper); + mapperService = createIndex("index2", Settings.EMPTY, jsonBuilder().startObject() + .startObject("properties") + .startObject("nested1") + .field("type", "nested") + .field("include_in_parent", true) + .endObject() + .endObject().endObject()).mapperService(); + ObjectMapper objectMapper2 = mapperService.getObjectMapper("nested1"); // cannot update `include_in_parent` dynamically - ObjectMapper.Nested newNested1 = ObjectMapper.Nested.newNested(true,false); - final ObjectMapper newNestedMapper1 = new ObjectMapper.Builder("nested1").nested(newNested1).build(context); - newNestedMapper1.putMapper(fieldMapper); - final RootObjectMapper mergeWith1 = new RootObjectMapper.Builder("type1").build(context); - mergeWith1.putMapper(newNestedMapper1); - MapperException e1 = expectThrows(MapperException.class, () -> rootObjectMapper.merge(mergeWith1)); - assertEquals("Can't update attribute for type [type1.nested1.include_in_parent] in index mapping", e1.getMessage()); + MapperException e1 = expectThrows(MapperException.class, () -> objectMapper1.merge(objectMapper2)); + assertEquals("The [include_in_parent] parameter can't be updated for the nested object mapping [nested1].", e1.getMessage()); + + mapperService = createIndex("index3", Settings.EMPTY, jsonBuilder().startObject() + .startObject("properties") + .startObject("nested1") + .field("type", "nested") + .field("include_in_root", true) + .endObject() + .endObject().endObject()).mapperService(); + ObjectMapper objectMapper3 = mapperService.getObjectMapper("nested1"); // cannot update `include_in_root` dynamically - ObjectMapper.Nested newNested2 = ObjectMapper.Nested.newNested(false,true); - final ObjectMapper newNestedMapper2 = new ObjectMapper.Builder("nested1").nested(newNested2).build(context); - newNestedMapper2.putMapper(fieldMapper); - final RootObjectMapper mergeWith2 = new RootObjectMapper.Builder("type1").build(context); - mergeWith2.putMapper(newNestedMapper2); - MapperException e2 = expectThrows(MapperException.class, () -> rootObjectMapper.merge(mergeWith2)); - assertEquals("Can't update attribute for type [type1.nested1.include_in_root] in index mapping", e2.getMessage()); + MapperException e2 = expectThrows(MapperException.class, () -> objectMapper1.merge(objectMapper3)); + assertEquals("The [include_in_root] parameter can't be updated for the nested object mapping [nested1].", e2.getMessage()); } } From 009da6a702329d28e1721e0a53542e519c962167 Mon Sep 17 00:00:00 2001 From: bellengao Date: Fri, 10 Apr 2020 21:14:31 +0800 Subject: [PATCH 3/4] Modify test code for the change --- .../index/mapper/NestedObjectMapperTests.java | 43 ++++++++++--------- 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/index/mapper/NestedObjectMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/NestedObjectMapperTests.java index 9942247cbb2cb..3b929090a621a 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/NestedObjectMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/NestedObjectMapperTests.java @@ -743,39 +743,42 @@ public void testReorderParent() throws IOException { assertThat(doc.docs().get(2).get("field"), equalTo("value")); } - public void testMergeNestedMappings() throws IOException { + public void testMergeNestedMappings() throws IOException { MapperService mapperService = createIndex("index1", Settings.EMPTY, jsonBuilder().startObject() .startObject("properties") .startObject("nested1") .field("type", "nested") .endObject() .endObject().endObject()).mapperService(); - ObjectMapper objectMapper1 = mapperService.getObjectMapper("nested1"); - mapperService = createIndex("index2", Settings.EMPTY, jsonBuilder().startObject() - .startObject("properties") - .startObject("nested1") - .field("type", "nested") - .field("include_in_parent", true) - .endObject() - .endObject().endObject()).mapperService(); - ObjectMapper objectMapper2 = mapperService.getObjectMapper("nested1"); + Function mapping1 = type -> { + try { + return Strings.toString(XContentFactory.jsonBuilder().startObject().startObject(type).startObject("properties") + .startObject("nested1").field("type", "nested").field("include_in_parent", true) + .endObject().endObject().endObject().endObject()); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + }; // cannot update `include_in_parent` dynamically - MapperException e1 = expectThrows(MapperException.class, () -> objectMapper1.merge(objectMapper2)); + MapperException e1 = expectThrows(MapperException.class, () -> mapperService.merge("type", + new CompressedXContent(mapping1.apply("type")), MergeReason.MAPPING_UPDATE)); assertEquals("The [include_in_parent] parameter can't be updated for the nested object mapping [nested1].", e1.getMessage()); - mapperService = createIndex("index3", Settings.EMPTY, jsonBuilder().startObject() - .startObject("properties") - .startObject("nested1") - .field("type", "nested") - .field("include_in_root", true) - .endObject() - .endObject().endObject()).mapperService(); - ObjectMapper objectMapper3 = mapperService.getObjectMapper("nested1"); + Function mapping2 = type -> { + try { + return Strings.toString(XContentFactory.jsonBuilder().startObject().startObject(type).startObject("properties") + .startObject("nested1").field("type", "nested").field("include_in_root", true) + .endObject().endObject().endObject().endObject()); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + }; // cannot update `include_in_root` dynamically - MapperException e2 = expectThrows(MapperException.class, () -> objectMapper1.merge(objectMapper3)); + MapperException e2 = expectThrows(MapperException.class, () -> mapperService.merge("type", + new CompressedXContent(mapping2.apply("type")), MergeReason.MAPPING_UPDATE)); assertEquals("The [include_in_root] parameter can't be updated for the nested object mapping [nested1].", e2.getMessage()); } } From addca5de628cebbfb583f0414f64bddce3262ee3 Mon Sep 17 00:00:00 2001 From: bellengao Date: Thu, 16 Apr 2020 20:14:57 +0800 Subject: [PATCH 4/4] Modify the test code --- .../index/mapper/NestedObjectMapperTests.java | 28 ++++++------------- 1 file changed, 8 insertions(+), 20 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/index/mapper/NestedObjectMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/NestedObjectMapperTests.java index 3b929090a621a..661306aea4f50 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/NestedObjectMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/NestedObjectMapperTests.java @@ -751,34 +751,22 @@ public void testMergeNestedMappings() throws IOException { .endObject() .endObject().endObject()).mapperService(); - Function mapping1 = type -> { - try { - return Strings.toString(XContentFactory.jsonBuilder().startObject().startObject(type).startObject("properties") - .startObject("nested1").field("type", "nested").field("include_in_parent", true) - .endObject().endObject().endObject().endObject()); - } catch (IOException e) { - throw new UncheckedIOException(e); - } - }; + String mapping1 = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type").startObject("properties") + .startObject("nested1").field("type", "nested").field("include_in_parent", true) + .endObject().endObject().endObject().endObject()); // cannot update `include_in_parent` dynamically MapperException e1 = expectThrows(MapperException.class, () -> mapperService.merge("type", - new CompressedXContent(mapping1.apply("type")), MergeReason.MAPPING_UPDATE)); + new CompressedXContent(mapping1), MergeReason.MAPPING_UPDATE)); assertEquals("The [include_in_parent] parameter can't be updated for the nested object mapping [nested1].", e1.getMessage()); - Function mapping2 = type -> { - try { - return Strings.toString(XContentFactory.jsonBuilder().startObject().startObject(type).startObject("properties") - .startObject("nested1").field("type", "nested").field("include_in_root", true) - .endObject().endObject().endObject().endObject()); - } catch (IOException e) { - throw new UncheckedIOException(e); - } - }; + String mapping2 = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type").startObject("properties") + .startObject("nested1").field("type", "nested").field("include_in_root", true) + .endObject().endObject().endObject().endObject()); // cannot update `include_in_root` dynamically MapperException e2 = expectThrows(MapperException.class, () -> mapperService.merge("type", - new CompressedXContent(mapping2.apply("type")), MergeReason.MAPPING_UPDATE)); + new CompressedXContent(mapping2), MergeReason.MAPPING_UPDATE)); assertEquals("The [include_in_root] parameter can't be updated for the nested object mapping [nested1].", e2.getMessage()); } }