Merge pull request #11171 from rjernst/fix/11116

Mappings: Add back support for enabled/includes/excludes in _source

src/main/java/org/elasticsearch/index/mapper/internal/SourceFieldMapper.java

@@ -54,6 +54,7 @@ import org.elasticsearch.index.mapper.RootMapper;
 import org.elasticsearch.index.mapper.core.AbstractFieldMapper;
 
 import java.io.IOException;
+import java.util.Arrays;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
@@ -150,7 +151,7 @@ public class SourceFieldMapper extends AbstractFieldMapper<byte[]> implements Ro
                 Map.Entry<String, Object> entry = iterator.next();
                 String fieldName = Strings.toUnderscoreCase(entry.getKey());
                 Object fieldNode = entry.getValue();
-                if (fieldName.equals("enabled") && parserContext.indexVersionCreated().before(Version.V_2_0_0)) {
+                if (fieldName.equals("enabled")) {
                     builder.enabled(nodeBooleanValue(fieldNode));
                     iterator.remove();
                 } else if (fieldName.equals("compress") && parserContext.indexVersionCreated().before(Version.V_2_0_0)) {
@@ -172,7 +173,7 @@ public class SourceFieldMapper extends AbstractFieldMapper<byte[]> implements Ro
                 } else if ("format".equals(fieldName)) {
                     builder.format(nodeStringValue(fieldNode, null));
                     iterator.remove();
-                } else if (fieldName.equals("includes") && parserContext.indexVersionCreated().before(Version.V_2_0_0)) {
+                } else if (fieldName.equals("includes")) {
                     List<Object> values = (List<Object>) fieldNode;
                     String[] includes = new String[values.size()];
                     for (int i = 0; i < includes.length; i++) {
@@ -180,7 +181,7 @@ public class SourceFieldMapper extends AbstractFieldMapper<byte[]> implements Ro
                     }
                     builder.includes(includes);
                     iterator.remove();
-                } else if (fieldName.equals("excludes") && parserContext.indexVersionCreated().before(Version.V_2_0_0)) {
+                } else if (fieldName.equals("excludes")) {
                     List<Object> values = (List<Object>) fieldNode;
                     String[] excludes = new String[values.size()];
                     for (int i = 0; i < excludes.length; i++) {
@@ -197,11 +198,14 @@ public class SourceFieldMapper extends AbstractFieldMapper<byte[]> implements Ro
 
     private final boolean enabled;
 
+    /** indicates whether the source will always exist and be complete, for use by features like the update API */
+    private final boolean complete;
+
     private Boolean compress;
     private long compressThreshold;
 
-    private String[] includes;
+    private final String[] includes;
-    private String[] excludes;
+    private final String[] excludes;
 
     private String format;
 
@@ -218,23 +222,28 @@ public class SourceFieldMapper extends AbstractFieldMapper<byte[]> implements Ro
         this.enabled = enabled;
         this.compress = compress;
         this.compressThreshold = compressThreshold;
-        this.includes = includes;
+        this.includes = includes == null ? Strings.EMPTY_ARRAY : includes;
-        this.excludes = excludes;
+        this.excludes = excludes == null ? Strings.EMPTY_ARRAY : excludes;
         this.format = format;
         this.formatContentType = format == null ? null : XContentType.fromRestContentType(format);
+        this.complete = enabled && includes == null && excludes == null;
     }
 
     public boolean enabled() {
-        return this.enabled;
+        return enabled;
     }
 
     public String[] excludes() {
-        return this.excludes != null ? this.excludes : Strings.EMPTY_ARRAY;
+        return excludes;
 
     }
 
     public String[] includes() {
-        return this.includes != null ? this.includes : Strings.EMPTY_ARRAY;
+        return includes;
+    }
+
+    public boolean isComplete() {
+        return complete;
     }
 
     @Override
@@ -420,19 +429,23 @@ public class SourceFieldMapper extends AbstractFieldMapper<byte[]> implements Ro
     @Override
     public void merge(Mapper mergeWith, MergeResult mergeResult) throws MergeMappingException {
         SourceFieldMapper sourceMergeWith = (SourceFieldMapper) mergeWith;
-        if (!mergeResult.simulate()) {
+        if (mergeResult.simulate()) {
+            if (this.enabled != sourceMergeWith.enabled) {
+                mergeResult.addConflict("Cannot update enabled setting for [_source]");
+            }
+            if (Arrays.equals(includes, sourceMergeWith.includes) == false) {
+                mergeResult.addConflict("Cannot update includes setting for [_source]");
+            }
+            if (Arrays.equals(excludes, sourceMergeWith.excludes) == false) {
+                mergeResult.addConflict("Cannot update excludes setting for [_source]");
+            }
+        } else {
             if (sourceMergeWith.compress != null) {
                 this.compress = sourceMergeWith.compress;
             }
             if (sourceMergeWith.compressThreshold != -1) {
                 this.compressThreshold = sourceMergeWith.compressThreshold;
             }
-            if (sourceMergeWith.includes != null) {
-                this.includes = sourceMergeWith.includes;
-            }
-            if (sourceMergeWith.excludes != null) {
-                this.excludes = sourceMergeWith.excludes;
-            }
         }
     }
 }

src/main/java/org/elasticsearch/search/highlight/HighlightPhase.java

@@ -86,8 +86,8 @@ public class HighlightPhase extends AbstractComponent implements FetchSubPhase {
 
             if (context.highlight().forceSource(field)) {
                 SourceFieldMapper sourceFieldMapper = context.mapperService().documentMapper(hitContext.hit().type()).sourceMapper();
-                if (!sourceFieldMapper.enabled()) {
+                if (!sourceFieldMapper.isComplete()) {
-                    throw new IllegalArgumentException("source is forced for fields " +  fieldNamesToHighlight + " but type [" + hitContext.hit().type() + "] has disabled _source");
+                    throw new IllegalArgumentException("source is forced for fields " +  fieldNamesToHighlight + " but type [" + hitContext.hit().type() + "] has incomplete _source");
                 }
             }
 

src/test/java/org/elasticsearch/index/mapper/source/DefaultSourceMappingTests.java

@@ -35,6 +35,8 @@ import org.elasticsearch.index.IndexService;
 import org.elasticsearch.test.ElasticsearchSingleNodeTest;
 import org.junit.Test;
 
+import java.io.IOException;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
@@ -42,7 +44,6 @@ import java.util.Map;
 import static org.hamcrest.Matchers.*;
 
 public class DefaultSourceMappingTests extends ElasticsearchSingleNodeTest {
-    Settings backcompatSettings = ImmutableSettings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.V_1_4_2.id).build();
 
     public void testNoFormat() throws Exception {
         String mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
@@ -80,8 +81,8 @@ public class DefaultSourceMappingTests extends ElasticsearchSingleNodeTest {
 
         documentMapper = parser.parse(mapping);
         doc = documentMapper.parse("type", "1", XContentFactory.smileBuilder().startObject()
-                .field("field", "value")
+            .field("field", "value")
-                .endObject().bytes());
+            .endObject().bytes());
 
         assertThat(XContentFactory.xContentType(doc.source()), equalTo(XContentType.JSON));
     }
@@ -91,6 +92,7 @@ public class DefaultSourceMappingTests extends ElasticsearchSingleNodeTest {
                 .startObject("_source").field("format", "json").field("compress", true).endObject()
                 .endObject().endObject().string();
 
+        Settings backcompatSettings = ImmutableSettings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.V_1_4_2.id).build();
         DocumentMapperParser parser = createIndex("test", backcompatSettings).mapperService().documentMapperParser();
         DocumentMapper documentMapper = parser.parse(mapping);
         ParsedDocument doc = documentMapper.parse("type", "1", XContentFactory.jsonBuilder().startObject()
@@ -111,19 +113,12 @@ public class DefaultSourceMappingTests extends ElasticsearchSingleNodeTest {
         assertThat(XContentFactory.xContentType(uncompressed), equalTo(XContentType.JSON));
     }
 
-    public void testIncludesBackcompat() throws Exception {
+    public void testIncludes() throws Exception {
         String mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
             .startObject("_source").field("includes", new String[]{"path1*"}).endObject()
             .endObject().endObject().string();
 
-        try {
+        DocumentMapper documentMapper = createIndex("test").mapperService().documentMapperParser().parse(mapping);
-            createIndex("testbad").mapperService().documentMapperParser().parse(mapping);
-            fail("includes should not be allowed");
-        } catch (MapperParsingException e) {
-            assertTrue(e.getMessage().contains("unsupported parameters"));
-        }
-
-        DocumentMapper documentMapper = createIndex("test", backcompatSettings).mapperService().documentMapperParser().parse(mapping);
 
         ParsedDocument doc = documentMapper.parse("type", "1", XContentFactory.jsonBuilder().startObject()
             .startObject("path1").field("field1", "value1").endObject()
@@ -136,19 +131,12 @@ public class DefaultSourceMappingTests extends ElasticsearchSingleNodeTest {
         assertThat(sourceAsMap.containsKey("path2"), equalTo(false));
     }
 
-    public void testExcludesBackcompat() throws Exception {
+    public void testExcludes() throws Exception {
         String mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
             .startObject("_source").field("excludes", new String[]{"path1*"}).endObject()
             .endObject().endObject().string();
 
-        try {
+        DocumentMapper documentMapper = createIndex("test").mapperService().documentMapperParser().parse(mapping);
-            createIndex("testbad").mapperService().documentMapperParser().parse(mapping);
-            fail("excludes should not be allowed");
-        } catch (MapperParsingException e) {
-            assertTrue(e.getMessage().contains("unsupported parameters"));
-        }
-
-        DocumentMapper documentMapper = createIndex("test", backcompatSettings).mapperService().documentMapperParser().parse(mapping);
 
         ParsedDocument doc = documentMapper.parse("type", "1", XContentFactory.jsonBuilder().startObject()
             .startObject("path1").field("field1", "value1").endObject()
@@ -161,12 +149,12 @@ public class DefaultSourceMappingTests extends ElasticsearchSingleNodeTest {
         assertThat(sourceAsMap.containsKey("path2"), equalTo(true));
     }
 
-    public void testDefaultMappingAndNoMappingBackcompat() throws Exception {
+    public void testDefaultMappingAndNoMapping() throws Exception {
         String defaultMapping = XContentFactory.jsonBuilder().startObject().startObject(MapperService.DEFAULT_MAPPING)
                 .startObject("_source").field("enabled", false).endObject()
                 .endObject().endObject().string();
 
-        DocumentMapperParser parser = createIndex("test", backcompatSettings).mapperService().documentMapperParser();
+        DocumentMapperParser parser = createIndex("test").mapperService().documentMapperParser();
         DocumentMapper mapper = parser.parse("my_type", null, defaultMapping);
         assertThat(mapper.type(), equalTo("my_type"));
         assertThat(mapper.sourceMapper().enabled(), equalTo(false));
@@ -189,7 +177,7 @@ public class DefaultSourceMappingTests extends ElasticsearchSingleNodeTest {
         }
     }
 
-    public void testDefaultMappingAndWithMappingOverrideBackcompat() throws Exception {
+    public void testDefaultMappingAndWithMappingOverride() throws Exception {
         String defaultMapping = XContentFactory.jsonBuilder().startObject().startObject(MapperService.DEFAULT_MAPPING)
                 .startObject("_source").field("enabled", false).endObject()
                 .endObject().endObject().string();
@@ -198,17 +186,17 @@ public class DefaultSourceMappingTests extends ElasticsearchSingleNodeTest {
                 .startObject("_source").field("enabled", true).endObject()
                 .endObject().endObject().string();
 
-        DocumentMapper mapper = createIndex("test", backcompatSettings).mapperService().documentMapperParser().parse("my_type", mapping, defaultMapping);
+        DocumentMapper mapper = createIndex("test").mapperService().documentMapperParser().parse("my_type", mapping, defaultMapping);
         assertThat(mapper.type(), equalTo("my_type"));
         assertThat(mapper.sourceMapper().enabled(), equalTo(true));
     }
 
-    public void testDefaultMappingAndNoMappingWithMapperServiceBackcompat() throws Exception {
+    public void testDefaultMappingAndNoMappingWithMapperService() throws Exception {
         String defaultMapping = XContentFactory.jsonBuilder().startObject().startObject(MapperService.DEFAULT_MAPPING)
                 .startObject("_source").field("enabled", false).endObject()
                 .endObject().endObject().string();
 
-        MapperService mapperService = createIndex("test", backcompatSettings).mapperService();
+        MapperService mapperService = createIndex("test").mapperService();
         mapperService.merge(MapperService.DEFAULT_MAPPING, new CompressedString(defaultMapping), true);
 
         DocumentMapper mapper = mapperService.documentMapperWithAutoCreate("my_type").v1();
@@ -216,12 +204,12 @@ public class DefaultSourceMappingTests extends ElasticsearchSingleNodeTest {
         assertThat(mapper.sourceMapper().enabled(), equalTo(false));
     }
 
-    public void testDefaultMappingAndWithMappingOverrideWithMapperServiceBackcompat() throws Exception {
+    public void testDefaultMappingAndWithMappingOverrideWithMapperService() throws Exception {
         String defaultMapping = XContentFactory.jsonBuilder().startObject().startObject(MapperService.DEFAULT_MAPPING)
                 .startObject("_source").field("enabled", false).endObject()
                 .endObject().endObject().string();
 
-        MapperService mapperService = createIndex("test", backcompatSettings).mapperService();
+        MapperService mapperService = createIndex("test").mapperService();
         mapperService.merge(MapperService.DEFAULT_MAPPING, new CompressedString(defaultMapping), true);
 
         String mapping = XContentFactory.jsonBuilder().startObject().startObject("my_type")
@@ -233,4 +221,90 @@ public class DefaultSourceMappingTests extends ElasticsearchSingleNodeTest {
         assertThat(mapper.type(), equalTo("my_type"));
         assertThat(mapper.sourceMapper().enabled(), equalTo(true));
     }
+
+    void assertConflicts(String mapping1, String mapping2, DocumentMapperParser parser, String... conflicts) throws IOException {
+        DocumentMapper docMapper = parser.parse(mapping1);
+        docMapper.refreshSource();
+        docMapper = parser.parse(docMapper.mappingSource().string());
+        MergeResult mergeResult = docMapper.merge(parser.parse(mapping2).mapping(), true);
+
+        List<String> expectedConflicts = new ArrayList<>(Arrays.asList(conflicts));
+        for (String conflict : mergeResult.buildConflicts()) {
+            assertTrue("found unexpected conflict [" + conflict + "]", expectedConflicts.remove(conflict));
+        }
+        assertTrue("missing conflicts: " + Arrays.toString(expectedConflicts.toArray()), expectedConflicts.isEmpty());
+    }
+
+    public void testEnabledNotUpdateable() throws Exception {
+        DocumentMapperParser parser = createIndex("test").mapperService().documentMapperParser();
+        // using default of true
+        String mapping1 = XContentFactory.jsonBuilder().startObject().startObject("type").endObject().endObject().string();
+        String mapping2 = XContentFactory.jsonBuilder().startObject().startObject("type")
+            .startObject("_source").field("enabled", false).endObject()
+            .endObject().endObject().string();
+        assertConflicts(mapping1, mapping2, parser, "Cannot update enabled setting for [_source]");
+
+        // not changing is ok
+        String mapping3 = XContentFactory.jsonBuilder().startObject().startObject("type")
+            .startObject("_source").field("enabled", true).endObject()
+            .endObject().endObject().string();
+        assertConflicts(mapping1, mapping3, parser);
+    }
+
+    public void testIncludesNotUpdateable() throws Exception {
+        DocumentMapperParser parser = createIndex("test").mapperService().documentMapperParser();
+        String defaultMapping = XContentFactory.jsonBuilder().startObject().startObject("type").endObject().endObject().string();
+        String mapping1 = XContentFactory.jsonBuilder().startObject().startObject("type")
+            .startObject("_source").array("includes", "foo.*").endObject()
+            .endObject().endObject().string();
+        assertConflicts(defaultMapping, mapping1, parser, "Cannot update includes setting for [_source]");
+        assertConflicts(mapping1, defaultMapping, parser, "Cannot update includes setting for [_source]");
+
+        String mapping2 = XContentFactory.jsonBuilder().startObject().startObject("type")
+            .startObject("_source").array("includes", "foo.*", "bar.*").endObject()
+            .endObject().endObject().string();
+        assertConflicts(mapping1, mapping2, parser, "Cannot update includes setting for [_source]");
+
+        // not changing is ok
+        assertConflicts(mapping1, mapping1, parser);
+    }
+
+    public void testExcludesNotUpdateable() throws Exception {
+        DocumentMapperParser parser = createIndex("test").mapperService().documentMapperParser();
+        String defaultMapping = XContentFactory.jsonBuilder().startObject().startObject("type").endObject().endObject().string();
+        String mapping1 = XContentFactory.jsonBuilder().startObject().startObject("type")
+            .startObject("_source").array("excludes", "foo.*").endObject()
+            .endObject().endObject().string();
+        assertConflicts(defaultMapping, mapping1, parser, "Cannot update excludes setting for [_source]");
+        assertConflicts(mapping1, defaultMapping, parser, "Cannot update excludes setting for [_source]");
+
+        String mapping2 = XContentFactory.jsonBuilder().startObject().startObject("type")
+            .startObject("_source").array("excludes", "foo.*", "bar.*").endObject()
+            .endObject().endObject().string();
+        assertConflicts(mapping1, mapping2, parser, "Cannot update excludes setting for [_source]");
+
+        // not changing is ok
+        assertConflicts(mapping1, mapping1, parser);
+    }
+
+    public void testComplete() throws Exception {
+        DocumentMapperParser parser = createIndex("test").mapperService().documentMapperParser();
+        String mapping = XContentFactory.jsonBuilder().startObject().startObject("type").endObject().endObject().string();
+        assertTrue(parser.parse(mapping).sourceMapper().isComplete());
+
+        mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
+            .startObject("_source").field("enabled", false).endObject()
+            .endObject().endObject().string();
+        assertFalse(parser.parse(mapping).sourceMapper().isComplete());
+
+        mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
+            .startObject("_source").array("includes", "foo.*").endObject()
+            .endObject().endObject().string();
+        assertFalse(parser.parse(mapping).sourceMapper().isComplete());
+
+        mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
+            .startObject("_source").array("excludes", "foo.*").endObject()
+            .endObject().endObject().string();
+        assertFalse(parser.parse(mapping).sourceMapper().isComplete());
+    }
 }

src/test/java/org/elasticsearch/indices/mapping/UpdateMappingIntegrationTests.java

@@ -212,102 +212,6 @@ public class UpdateMappingIntegrationTests extends ElasticsearchIntegrationTest
         assertThat(putMappingResponse.isAcknowledged(), equalTo(true));
     }
 
-
-    @SuppressWarnings("unchecked")
-    @Test
-    public void updateIncludeExcludeBackcompat() throws Exception {
-        assertAcked(prepareCreate("test").setSettings(IndexMetaData.SETTING_VERSION_CREATED, Version.V_1_4_2.id)
-            .addMapping("type", jsonBuilder().startObject().startObject("type").startObject("properties")
-                    .startObject("normal").field("type", "long").endObject()
-                    .startObject("exclude").field("type", "long").endObject()
-                    .startObject("include").field("type", "long").endObject()
-                    .endObject().endObject().endObject()));
-        ensureGreen(); // make sure that replicas are initialized so the refresh command will work them too
-
-        logger.info("Index doc");
-        index("test", "type", "1", JsonXContent.contentBuilder().startObject()
-                        .field("normal", 1).field("exclude", 1).field("include", 1)
-                        .endObject()
-        );
-        refresh(); // commit it for later testing.
-
-        logger.info("Adding exclude settings");
-        PutMappingResponse putResponse = client().admin().indices().preparePutMapping("test").setType("type").setSource(
-                JsonXContent.contentBuilder().startObject().startObject("type")
-                        .startObject("_source")
-                        .startArray("excludes").value("exclude").endArray()
-                        .endObject().endObject()
-        ).get();
-
-        assertTrue(putResponse.isAcknowledged());
-
-        // changed mapping doesn't affect indexed documents (checking backward compatibility)
-        GetResponse getResponse = client().prepareGet("test", "type", "1").setRealtime(false).get();
-        assertThat(getResponse.getSource(), hasKey("normal"));
-        assertThat(getResponse.getSource(), hasKey("exclude"));
-        assertThat(getResponse.getSource(), hasKey("include"));
-
-
-        logger.info("Index doc again");
-        index("test", "type", "1", JsonXContent.contentBuilder().startObject()
-                        .field("normal", 2).field("exclude", 1).field("include", 2)
-                        .endObject()
-        );
-
-        // but do affect newly indexed docs
-        getResponse = get("test", "type", "1");
-        assertThat(getResponse.getSource(), hasKey("normal"));
-        assertThat(getResponse.getSource(), not(hasKey("exclude")));
-        assertThat(getResponse.getSource(), hasKey("include"));
-
-        logger.info("Changing mapping to includes");
-        putResponse = client().admin().indices().preparePutMapping("test").setType("type").setSource(
-                JsonXContent.contentBuilder().startObject().startObject("type")
-                        .startObject("_source")
-                        .startArray("excludes").endArray()
-                        .startArray("includes").value("include").endArray()
-                        .endObject().endObject()
-        ).get();
-        assertTrue(putResponse.isAcknowledged());
-
-        GetMappingsResponse getMappingsResponse = client().admin().indices().prepareGetMappings("test").get();
-        MappingMetaData typeMapping = getMappingsResponse.getMappings().get("test").get("type");
-        assertThat((Map<String, Object>) typeMapping.getSourceAsMap().get("_source"), hasKey("includes"));
-        ArrayList<String> includes = (ArrayList<String>) ((Map<String, Object>) typeMapping.getSourceAsMap().get("_source")).get("includes");
-        assertThat(includes, contains("include"));
-        assertThat((Map<String, Object>) typeMapping.getSourceAsMap().get("_source"), hasKey("excludes"));
-        assertThat((ArrayList<String>) ((Map<String, Object>) typeMapping.getSourceAsMap().get("_source")).get("excludes"), emptyIterable());
-
-        logger.info("Indexing doc yet again");
-        index("test", "type", "1", JsonXContent.contentBuilder().startObject()
-                        .field("normal", 3).field("exclude", 3).field("include", 3)
-                        .endObject()
-        );
-
-        getResponse = get("test", "type", "1");
-        assertThat(getResponse.getSource(), not(hasKey("normal")));
-        assertThat(getResponse.getSource(), not(hasKey("exclude")));
-        assertThat(getResponse.getSource(), hasKey("include"));
-
-        logger.info("Adding excludes, but keep includes");
-        putResponse = client().admin().indices().preparePutMapping("test").setType("type").setSource(
-                JsonXContent.contentBuilder().startObject().startObject("type")
-                        .startObject("_source")
-                        .startArray("excludes").value("*.excludes").endArray()
-                        .endObject().endObject()
-        ).get();
-        assertTrue(putResponse.isAcknowledged());
-
-        getMappingsResponse = client().admin().indices().prepareGetMappings("test").get();
-        typeMapping = getMappingsResponse.getMappings().get("test").get("type");
-        assertThat((Map<String, Object>) typeMapping.getSourceAsMap().get("_source"), hasKey("includes"));
-        includes = (ArrayList<String>) ((Map<String, Object>) typeMapping.getSourceAsMap().get("_source")).get("includes");
-        assertThat(includes, contains("include"));
-        assertThat((Map<String, Object>) typeMapping.getSourceAsMap().get("_source"), hasKey("excludes"));
-        ArrayList<String> excludes = (ArrayList<String>) ((Map<String, Object>) typeMapping.getSourceAsMap().get("_source")).get("excludes");
-        assertThat(excludes, contains("*.excludes"));
-    }
-
     @SuppressWarnings("unchecked")
     @Test
     public void updateDefaultMappingSettings() throws Exception {