Strict dynamic setting: Refuse to index a document with fields not present in the mapping definition, closes #643.

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");
+    }
+}

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<T extends Builder, Y extends ObjectMapper> extends XContentMapper.Builder<T, Y> {
 
         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<String, XContentMapper> mappers) {
+        protected ObjectMapper createMapper(String name, boolean enabled, Dynamic dynamic, ContentPath.Type pathType, Map<String, XContentMapper> 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<String, XContentMapper> mappers) {
+    ObjectMapper(String name, boolean enabled, Dynamic dynamic, ContentPath.Type pathType, Map<String, XContentMapper> 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,9 +586,17 @@ 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);
         }
+        // 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);
         }

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<String, XContentMapper> mappers) {
+        @Override protected ObjectMapper createMapper(String name, boolean enabled, Dynamic dynamic, ContentPath.Type pathType, Map<String, XContentMapper> 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<String, XContentMapper> mappers,
+    RootObjectMapper(String name, boolean enabled, Dynamic dynamic, ContentPath.Type pathType, Map<String, XContentMapper> mappers,
                      FormatDateTimeFormatter[] dateTimeFormatters, DynamicTemplate dynamicTemplates[]) {
         super(name, enabled, dynamic, pathType, mappers);
         this.dynamicTemplates = dynamicTemplates;

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
+        }
+    }
+}

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);
     }