diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/StrictDynamicMappingException.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/StrictDynamicMappingException.java new file mode 100644 index 00000000000..4817e4affce --- /dev/null +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/StrictDynamicMappingException.java @@ -0,0 +1,30 @@ +/* + * Licensed to Elastic Search and Shay Banon under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. Elastic Search 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.index.mapper; + +/** + * @author kimchy (shay.banon) + */ +public class StrictDynamicMappingException extends MapperException { + + public StrictDynamicMappingException(String fieldName) { + super("mapping set to strict, dynamic introduction of [" + fieldName + "] is not allowed"); + } +} \ No newline at end of file diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/xcontent/ObjectMapper.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/xcontent/ObjectMapper.java index ce5ee49c431..1a6bb034c93 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/xcontent/ObjectMapper.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/xcontent/ObjectMapper.java @@ -53,15 +53,21 @@ public class ObjectMapper implements XContentMapper, IncludeInAllMapper { public static class Defaults { public static final boolean ENABLED = true; - public static final boolean DYNAMIC = true; + public static final Dynamic DYNAMIC = null; // not set, inherited from father public static final ContentPath.Type PATH_TYPE = ContentPath.Type.FULL; } + public static enum Dynamic { + TRUE, + FALSE, + STRICT + } + public static class Builder extends XContentMapper.Builder { protected boolean enabled = Defaults.ENABLED; - protected boolean dynamic = Defaults.DYNAMIC; + protected Dynamic dynamic = Defaults.DYNAMIC; protected ContentPath.Type pathType = Defaults.PATH_TYPE; @@ -79,7 +85,7 @@ public class ObjectMapper implements XContentMapper, IncludeInAllMapper { return builder; } - public T dynamic(boolean dynamic) { + public T dynamic(Dynamic dynamic) { this.dynamic = dynamic; return builder; } @@ -119,7 +125,7 @@ public class ObjectMapper implements XContentMapper, IncludeInAllMapper { return (Y) objectMapper; } - protected ObjectMapper createMapper(String name, boolean enabled, boolean dynamic, ContentPath.Type pathType, Map mappers) { + protected ObjectMapper createMapper(String name, boolean enabled, Dynamic dynamic, ContentPath.Type pathType, Map mappers) { return new ObjectMapper(name, enabled, dynamic, pathType, mappers); } } @@ -134,7 +140,12 @@ public class ObjectMapper implements XContentMapper, IncludeInAllMapper { Object fieldNode = entry.getValue(); if (fieldName.equals("dynamic")) { - builder.dynamic(nodeBooleanValue(fieldNode)); + String value = fieldNode.toString(); + if (value.equals("strict")) { + builder.dynamic(Dynamic.STRICT); + } else { + builder.dynamic(nodeBooleanValue(fieldNode) ? Dynamic.TRUE : Dynamic.FALSE); + } } else if (fieldName.equals("type")) { String type = fieldNode.toString(); if (!type.equals("object")) { @@ -196,7 +207,7 @@ public class ObjectMapper implements XContentMapper, IncludeInAllMapper { private final boolean enabled; - private final boolean dynamic; + private final Dynamic dynamic; private final ContentPath.Type pathType; @@ -211,11 +222,11 @@ public class ObjectMapper implements XContentMapper, IncludeInAllMapper { } - protected ObjectMapper(String name, boolean enabled, boolean dynamic, ContentPath.Type pathType) { + protected ObjectMapper(String name, boolean enabled, Dynamic dynamic, ContentPath.Type pathType) { this(name, enabled, dynamic, pathType, null); } - ObjectMapper(String name, boolean enabled, boolean dynamic, ContentPath.Type pathType, Map mappers) { + ObjectMapper(String name, boolean enabled, Dynamic dynamic, ContentPath.Type pathType, Map mappers) { this.name = name; this.enabled = enabled; this.dynamic = dynamic; @@ -258,6 +269,10 @@ public class ObjectMapper implements XContentMapper, IncludeInAllMapper { } } + public final Dynamic dynamic() { + return this.dynamic; + } + public void parse(ParseContext context) throws IOException { if (!enabled) { context.parser().skipChildren(); @@ -315,7 +330,13 @@ public class ObjectMapper implements XContentMapper, IncludeInAllMapper { if (objectMapper != null) { objectMapper.parse(context); } else { - if (dynamic) { + Dynamic dynamic = this.dynamic; + if (dynamic == null) { + dynamic = context.root().dynamic(); + } + if (dynamic == Dynamic.STRICT) { + throw new StrictDynamicMappingException(currentFieldName); + } else if (dynamic == Dynamic.TRUE) { // we sync here just so we won't add it twice. Its not the end of the world // to sync here since next operations will get it before synchronized (mutex) { @@ -377,7 +398,14 @@ public class ObjectMapper implements XContentMapper, IncludeInAllMapper { mapper.parse(context); return; } - if (!dynamic) { + Dynamic dynamic = this.dynamic; + if (dynamic == null) { + dynamic = context.root().dynamic(); + } + if (dynamic == Dynamic.STRICT) { + throw new StrictDynamicMappingException(currentFieldName); + } + if (dynamic == Dynamic.FALSE) { return; } // we sync here since we don't want to add this field twice to the document mapper @@ -558,8 +586,16 @@ public class ObjectMapper implements XContentMapper, IncludeInAllMapper { if (mappers.isEmpty()) { // only write the object content type if there are no properties, otherwise, it is automatically detected builder.field("type", CONTENT_TYPE); } - if (dynamic != Defaults.DYNAMIC) { - builder.field("dynamic", dynamic); + // grr, ugly! on root, dynamic defaults to TRUE, on childs, it defaults to null to + // inherit the root behavior + if (this instanceof RootObjectMapper) { + if (dynamic != Dynamic.TRUE) { + builder.field("dynamic", dynamic); + } + } else { + if (dynamic != Defaults.DYNAMIC) { + builder.field("dynamic", dynamic); + } } if (enabled != Defaults.ENABLED) { builder.field("enabled", enabled); diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/xcontent/RootObjectMapper.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/xcontent/RootObjectMapper.java index 51e7c890e50..5918713cabc 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/xcontent/RootObjectMapper.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/xcontent/RootObjectMapper.java @@ -92,7 +92,7 @@ public class RootObjectMapper extends ObjectMapper { } - @Override protected ObjectMapper createMapper(String name, boolean enabled, boolean dynamic, ContentPath.Type pathType, Map mappers) { + @Override protected ObjectMapper createMapper(String name, boolean enabled, Dynamic dynamic, ContentPath.Type pathType, Map mappers) { FormatDateTimeFormatter[] dates = null; if (dateTimeFormatters == null) { dates = new FormatDateTimeFormatter[0]; @@ -102,6 +102,10 @@ public class RootObjectMapper extends ObjectMapper { } else { dates = dateTimeFormatters.toArray(new FormatDateTimeFormatter[dateTimeFormatters.size()]); } + // root dynamic must not be null, since its the default + if (dynamic == null) { + dynamic = Dynamic.TRUE; + } return new RootObjectMapper(name, enabled, dynamic, pathType, mappers, dates, dynamicTemplates.toArray(new DynamicTemplate[dynamicTemplates.size()])); @@ -158,7 +162,7 @@ public class RootObjectMapper extends ObjectMapper { private volatile DynamicTemplate dynamicTemplates[]; - RootObjectMapper(String name, boolean enabled, boolean dynamic, ContentPath.Type pathType, Map mappers, + RootObjectMapper(String name, boolean enabled, Dynamic dynamic, ContentPath.Type pathType, Map mappers, FormatDateTimeFormatter[] dateTimeFormatters, DynamicTemplate dynamicTemplates[]) { super(name, enabled, dynamic, pathType, mappers); this.dynamicTemplates = dynamicTemplates; diff --git a/modules/elasticsearch/src/test/java/org/elasticsearch/index/mapper/xcontent/dynamic/DynamicMappingTests.java b/modules/elasticsearch/src/test/java/org/elasticsearch/index/mapper/xcontent/dynamic/DynamicMappingTests.java new file mode 100644 index 00000000000..17279a03c91 --- /dev/null +++ b/modules/elasticsearch/src/test/java/org/elasticsearch/index/mapper/xcontent/dynamic/DynamicMappingTests.java @@ -0,0 +1,147 @@ +/* + * Licensed to Elastic Search and Shay Banon under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. Elastic Search 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.index.mapper.xcontent.dynamic; + +import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.index.mapper.ParsedDocument; +import org.elasticsearch.index.mapper.StrictDynamicMappingException; +import org.elasticsearch.index.mapper.xcontent.MapperTests; +import org.elasticsearch.index.mapper.xcontent.XContentDocumentMapper; +import org.testng.annotations.Test; + +import java.io.IOException; + +import static org.hamcrest.MatcherAssert.*; +import static org.hamcrest.Matchers.*; + +@Test +public class DynamicMappingTests { + + @Test public void testDynamicTrue() throws IOException { + String mapping = XContentFactory.jsonBuilder().startObject().startObject("type") + .field("dynamic", "true") + .startObject("properties") + .startObject("field1").field("type", "string").endObject() + .endObject() + .endObject().endObject().string(); + + XContentDocumentMapper defaultMapper = MapperTests.newParser().parse(mapping); + + ParsedDocument doc = defaultMapper.parse("type", "1", XContentFactory.jsonBuilder() + .startObject() + .field("field1", "value1") + .field("field2", "value2") + .copiedBytes()); + + assertThat(doc.doc().get("field1"), equalTo("value1")); + assertThat(doc.doc().get("field2"), equalTo("value2")); + } + + @Test public void testDynamicFalse() throws IOException { + String mapping = XContentFactory.jsonBuilder().startObject().startObject("type") + .field("dynamic", "false") + .startObject("properties") + .startObject("field1").field("type", "string").endObject() + .endObject() + .endObject().endObject().string(); + + XContentDocumentMapper defaultMapper = MapperTests.newParser().parse(mapping); + + ParsedDocument doc = defaultMapper.parse("type", "1", XContentFactory.jsonBuilder() + .startObject() + .field("field1", "value1") + .field("field2", "value2") + .copiedBytes()); + + assertThat(doc.doc().get("field1"), equalTo("value1")); + assertThat(doc.doc().get("field2"), nullValue()); + } + + + @Test public void testDynamicStrict() throws IOException { + String mapping = XContentFactory.jsonBuilder().startObject().startObject("type") + .field("dynamic", "strict") + .startObject("properties") + .startObject("field1").field("type", "string").endObject() + .endObject() + .endObject().endObject().string(); + + XContentDocumentMapper defaultMapper = MapperTests.newParser().parse(mapping); + + try { + defaultMapper.parse("type", "1", XContentFactory.jsonBuilder() + .startObject() + .field("field1", "value1") + .field("field2", "value2") + .copiedBytes()); + assert false; + } catch (StrictDynamicMappingException e) { + // all is well + } + } + + @Test public void testDynamicFalseWithInnerObjectButDynamicSetOnRoot() throws IOException { + String mapping = XContentFactory.jsonBuilder().startObject().startObject("type") + .field("dynamic", "false") + .startObject("properties") + .startObject("obj1").startObject("properties") + .startObject("field1").field("type", "string").endObject() + .endObject().endObject() + .endObject() + .endObject().endObject().string(); + + XContentDocumentMapper defaultMapper = MapperTests.newParser().parse(mapping); + + ParsedDocument doc = defaultMapper.parse("type", "1", XContentFactory.jsonBuilder() + .startObject().startObject("obj1") + .field("field1", "value1") + .field("field2", "value2") + .endObject() + .copiedBytes()); + + assertThat(doc.doc().get("obj1.field1"), equalTo("value1")); + assertThat(doc.doc().get("obj1.field2"), nullValue()); + } + + @Test public void testDynamicStrictWithInnerObjectButDynamicSetOnRoot() throws IOException { + String mapping = XContentFactory.jsonBuilder().startObject().startObject("type") + .field("dynamic", "strict") + .startObject("properties") + .startObject("obj1").startObject("properties") + .startObject("field1").field("type", "string").endObject() + .endObject().endObject() + .endObject() + .endObject().endObject().string(); + + XContentDocumentMapper defaultMapper = MapperTests.newParser().parse(mapping); + + try { + defaultMapper.parse("type", "1", XContentFactory.jsonBuilder() + .startObject().startObject("obj1") + .field("field1", "value1") + .field("field2", "value2") + .endObject() + .copiedBytes()); + assert false; + } catch (StrictDynamicMappingException e) { + // all is well + } + } +} \ No newline at end of file diff --git a/modules/test/integration/src/test/java/org/elasticsearch/test/integration/search/compress/SearchSourceCompressTests.java b/modules/test/integration/src/test/java/org/elasticsearch/test/integration/search/compress/SearchSourceCompressTests.java index 9c2cc4eebea..dbbbca32884 100644 --- a/modules/test/integration/src/test/java/org/elasticsearch/test/integration/search/compress/SearchSourceCompressTests.java +++ b/modules/test/integration/src/test/java/org/elasticsearch/test/integration/search/compress/SearchSourceCompressTests.java @@ -61,7 +61,7 @@ public class SearchSourceCompressTests extends AbstractNodesTests { verifySource(true); } - @Test public void testSourceFieldPlainExplciit() throws IOException { + @Test public void testSourceFieldPlainExplicit() throws IOException { verifySource(false); }