Reject updates to the _default_ mapping. (#29165)

jpountz · web-flow · commit 8f9d2ee4e271 · 2018-03-21T10:44:11.000+01:00
This will reject mapping updates to the `_default_` mapping with 7.x indices and still emit a deprecation warning with 6.x indices. Relates #15613 Supersedes #28248
diff --git a/docs/reference/indices/put-mapping.asciidoc b/docs/reference/indices/put-mapping.asciidoc
@@ -50,11 +50,6 @@ PUT /twitter-1,twitter-2/_mapping/_doc <1>
 <1> Note that the indices specified (`twitter-1,twitter-2`) follows <<multi-index,multiple index names>> and wildcard format.
 
 
-NOTE: When updating the `_default_` mapping with the
-<<indices-put-mapping,PUT mapping>> API, the new mapping is not merged with
-the existing mapping.  Instead, the new `_default_` mapping replaces the
-existing one.
-
 [[updating-field-mappings]]
 [float]
 === Updating field mappings
diff --git a/docs/reference/mapping/dynamic-mapping.asciidoc b/docs/reference/mapping/dynamic-mapping.asciidoc
@@ -36,5 +36,3 @@ include::dynamic/field-mapping.asciidoc[]
 
 include::dynamic/templates.asciidoc[]
 
-include::dynamic/default-mapping.asciidoc[]
-
diff --git a/docs/reference/mapping/dynamic/default-mapping.asciidoc b/docs/reference/mapping/dynamic/default-mapping.asciidoc
diff --git a/docs/reference/migration/migrate_7_0/mappings.asciidoc b/docs/reference/migration/migrate_7_0/mappings.asciidoc
@@ -5,6 +5,12 @@
 
 The `_all` field deprecated in 6 have now been removed.
 
+==== The `_default_` mapping is no longer allowed
+
+The `_default_` mapping has been deprecated in 6.0 and is now no longer allowed
+in 7.0. Trying to configure a `_default_` mapping on 7.x indices will result in
+an error.
+
 ==== `index_options` for numeric fields has been removed
 
 The `index_options` field for numeric  fields has been deprecated in 6 and has now been removed.
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java b/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java
@@ -373,8 +373,10 @@ private synchronized Map<String, DocumentMapper> internalMerge(@Nullable Documen
         Map<String, DocumentMapper> results = new LinkedHashMap<>(documentMappers.size() + 1);
 
         if (defaultMapper != null) {
-            if (indexSettings.getIndexVersionCreated().onOrAfter(Version.V_6_0_0_beta1)
-                    && reason == MergeReason.MAPPING_UPDATE) { // only log in case of explicit mapping updates
+            if (indexSettings.getIndexVersionCreated().onOrAfter(Version.V_7_0_0_alpha1)) {
+                throw new IllegalArgumentException("The [default] mapping cannot be updated on index [" + index().getName() +
+                        "]: defaults mappings are not useful anymore now that indices can have at most one type.");
+            } else if (reason == MergeReason.MAPPING_UPDATE) { // only log in case of explicit mapping updates
                 DEPRECATION_LOGGER.deprecated("[_default_] mapping is deprecated since it is not useful anymore now that indexes " +
                         "cannot have more than one type");
             }
diff --git a/server/src/main/resources/org/elasticsearch/index/mapper/default-mapping.json b/server/src/main/resources/org/elasticsearch/index/mapper/default-mapping.json
diff --git a/server/src/main/resources/org/elasticsearch/index/mapper/script-mapping.json b/server/src/main/resources/org/elasticsearch/index/mapper/script-mapping.json
diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/get/GetIndexIT.java b/server/src/test/java/org/elasticsearch/action/admin/indices/get/GetIndexIT.java
@@ -235,11 +235,7 @@ private void assertMappings(GetIndexResponse response, String indexName) {
         assertThat(mappings.size(), equalTo(1));
         ImmutableOpenMap<String, MappingMetaData> indexMappings = mappings.get(indexName);
         assertThat(indexMappings, notNullValue());
-        assertThat(indexMappings.size(), anyOf(equalTo(1), equalTo(2)));
-        if (indexMappings.size() == 2) {
-            MappingMetaData mapping = indexMappings.get("_default_");
-            assertThat(mapping, notNullValue());
-        }
+        assertThat(indexMappings.size(), equalTo(1));
         MappingMetaData mapping = indexMappings.get("type1");
         assertThat(mapping, notNullValue());
         assertThat(mapping.type(), equalTo("type1"));
@@ -251,11 +247,7 @@ private void assertEmptyOrOnlyDefaultMappings(GetIndexResponse response, String
         assertThat(mappings.size(), equalTo(1));
         ImmutableOpenMap<String, MappingMetaData> indexMappings = mappings.get(indexName);
         assertThat(indexMappings, notNullValue());
-        assertThat(indexMappings.size(), anyOf(equalTo(0), equalTo(1)));
-        if (indexMappings.size() == 1) {
-            MappingMetaData mapping = indexMappings.get("_default_");
-            assertThat(mapping, notNullValue());
-        }
+        assertThat(indexMappings.size(), equalTo(0));
     }
 
     private void assertAliases(GetIndexResponse response, String indexName) {
diff --git a/server/src/test/java/org/elasticsearch/cluster/SpecificMasterNodesIT.java b/server/src/test/java/org/elasticsearch/cluster/SpecificMasterNodesIT.java
@@ -107,32 +107,6 @@ public void testElectOnlyBetweenMasterNodes() throws IOException {
         assertThat(internalCluster().masterClient().admin().cluster().prepareState().execute().actionGet().getState().nodes().getMasterNode().getName(), equalTo(nextMasterEligableNodeName));
     }
 
-    /**
-     * Tests that putting custom default mapping and then putting a type mapping will have the default mapping merged
-     * to the type mapping.
-     */
-    public void testCustomDefaultMapping() throws Exception {
-        logger.info("--> start master node / non data");
-        internalCluster().startNode(Settings.builder().put(Node.NODE_DATA_SETTING.getKey(), false).put(Node.NODE_MASTER_SETTING.getKey(), true));
-
-        logger.info("--> start data node / non master node");
-        internalCluster().startNode(Settings.builder().put(Node.NODE_DATA_SETTING.getKey(), true).put(Node.NODE_MASTER_SETTING.getKey(), false));
-
-        createIndex("test");
-        assertAcked(client().admin().indices().preparePutMapping("test").setType("_default_").setSource("timestamp", "type=date"));
-
-        MappingMetaData defaultMapping = client().admin().cluster().prepareState().get().getState().getMetaData().getIndices().get("test").getMappings().get("_default_");
-        Map<?,?> properties = (Map<?, ?>) defaultMapping.getSourceAsMap().get("properties");
-        assertThat(properties.get("timestamp"), notNullValue());
-
-        assertAcked(client().admin().indices().preparePutMapping("test").setType("_default_").setSource("timestamp", "type=date"));
-
-        assertAcked(client().admin().indices().preparePutMapping("test").setType("type1").setSource("foo", "enabled=true"));
-        MappingMetaData type1Mapping = client().admin().cluster().prepareState().get().getState().getMetaData().getIndices().get("test").getMappings().get("type1");
-        properties = (Map<?, ?>) type1Mapping.getSourceAsMap().get("properties");
-        assertThat(properties.get("timestamp"), notNullValue());
-    }
-
     public void testAliasFilterValidation() throws Exception {
         logger.info("--> start master node / non data");
         internalCluster().startNode(Settings.builder().put(Node.NODE_DATA_SETTING.getKey(), false).put(Node.NODE_MASTER_SETTING.getKey(), true));
diff --git a/server/src/test/java/org/elasticsearch/index/mapper/DynamicMappingTests.java b/server/src/test/java/org/elasticsearch/index/mapper/DynamicMappingTests.java
@@ -192,7 +192,10 @@ public void testTypeNotCreatedOnIndexFailure() throws IOException, InterruptedEx
         XContentBuilder mapping = jsonBuilder().startObject().startObject("_default_")
                 .field("dynamic", "strict")
                 .endObject().endObject();
-        createIndex("test", Settings.EMPTY, "_default_", mapping);
+        Settings settings = Settings.builder()
+                .put(IndexMetaData.SETTING_VERSION_CREATED, Version.V_6_3_0)
+                .build();
+        createIndex("test", settings, "_default_", mapping);
         try {
             client().prepareIndex().setIndex("test").setType("type").setSource(jsonBuilder().startObject().field("test", "test").endObject()).get();
             fail();
diff --git a/server/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java b/server/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java
@@ -21,6 +21,7 @@
 
 import org.elasticsearch.ExceptionsHelper;
 import org.elasticsearch.Version;
+import org.elasticsearch.cluster.metadata.IndexMetaData;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.compress.CompressedXContent;
@@ -85,25 +86,6 @@ public void testTypeNameTooLong() {
         assertTrue(e.getMessage(), e.getMessage().contains("mapping type name [" + type + "] is too long; limit is length 255 but was [256]"));
     }
 
-    public void testTypes() throws Exception {
-        IndexService indexService1 = createIndex("index1", Settings.builder().put("index.version.created", Version.V_5_6_0) // multi types
-            .build());
-        MapperService mapperService = indexService1.mapperService();
-        assertEquals(Collections.emptySet(), mapperService.types());
-
-        mapperService.merge("type1", new CompressedXContent("{\"type1\":{}}"), MapperService.MergeReason.MAPPING_UPDATE);
-        assertNull(mapperService.documentMapper(MapperService.DEFAULT_MAPPING));
-        assertEquals(Collections.singleton("type1"), mapperService.types());
-
-        mapperService.merge(MapperService.DEFAULT_MAPPING, new CompressedXContent("{\"_default_\":{}}"), MapperService.MergeReason.MAPPING_UPDATE);
-        assertNotNull(mapperService.documentMapper(MapperService.DEFAULT_MAPPING));
-        assertEquals(Collections.singleton("type1"), mapperService.types());
-
-        mapperService.merge("type2", new CompressedXContent("{\"type2\":{}}"), MapperService.MergeReason.MAPPING_UPDATE);
-        assertNotNull(mapperService.documentMapper(MapperService.DEFAULT_MAPPING));
-        assertEquals(new HashSet<>(Arrays.asList("type1", "type2")), mapperService.types());
-    }
-
     public void testTypeValidation() {
         InvalidTypeNameException e = expectThrows(InvalidTypeNameException.class, () -> MapperService.validateTypeName("_type"));
         assertEquals("mapping type name [_type] can't start with '_' unless it is called [_doc]", e.getMessage());
@@ -325,9 +307,19 @@ public void testForbidMultipleTypes() throws IOException {
         assertThat(e.getMessage(), Matchers.startsWith("Rejecting mapping update to [test] as the final mapping would have more than 1 type: "));
     }
 
-    public void testDefaultMappingIsDeprecated() throws IOException {
+    public void testDefaultMappingIsRejectedOn7() throws IOException {
         String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("_default_").endObject().endObject());
         MapperService mapperService = createIndex("test").mapperService();
+        IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
+                () -> mapperService.merge("_default_", new CompressedXContent(mapping), MergeReason.MAPPING_UPDATE));
+        assertEquals("The [default] mapping cannot be updated on index [test]: defaults mappings are not useful anymore now that indices " +
+                "can have at most one type.", e.getMessage());
+    }
+
+    public void testDefaultMappingIsDeprecatedOn6() throws IOException {
+        Settings settings = Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.V_6_3_0).build();
+        String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("_default_").endObject().endObject());
+        MapperService mapperService = createIndex("test", settings).mapperService();
         mapperService.merge("_default_", new CompressedXContent(mapping), MergeReason.MAPPING_UPDATE);
         assertWarnings("[_default_] mapping is deprecated since it is not useful anymore now that indexes " +
                 "cannot have more than one type");
diff --git a/server/src/test/java/org/elasticsearch/index/mapper/SourceFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/SourceFieldMapperTests.java
@@ -117,83 +117,7 @@ public void testExcludes() throws Exception {
         assertThat(sourceAsMap.containsKey("path2"), equalTo(true));
     }
 
-    public void testDefaultMappingAndNoMapping() throws Exception {
-        String defaultMapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject(MapperService.DEFAULT_MAPPING)
-                .startObject("_source").field("enabled", false).endObject()
-                .endObject().endObject());
-
-        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));
-        try {
-            mapper = parser.parse(null, null, defaultMapping);
-            assertThat(mapper.type(), equalTo("my_type"));
-            assertThat(mapper.sourceMapper().enabled(), equalTo(false));
-            fail();
-        } catch (MapperParsingException e) {
-            // all is well
-        }
-        try {
-            mapper = parser.parse(null, new CompressedXContent("{}"), defaultMapping);
-            assertThat(mapper.type(), equalTo("my_type"));
-            assertThat(mapper.sourceMapper().enabled(), equalTo(false));
-            fail();
-        } catch (MapperParsingException e) {
-            assertThat(e.getMessage(), equalTo("malformed mapping no root object found"));
-            // all is well
-        }
-    }
-
-    public void testDefaultMappingAndWithMappingOverride() throws Exception {
-        String defaultMapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject(MapperService.DEFAULT_MAPPING)
-                .startObject("_source").field("enabled", false).endObject()
-                .endObject().endObject());
-
-        String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("my_type")
-                .startObject("_source").field("enabled", true).endObject()
-                .endObject().endObject());
-
-        DocumentMapper mapper = createIndex("test").mapperService().documentMapperParser()
-            .parse("my_type", new CompressedXContent(mapping), defaultMapping);
-        assertThat(mapper.type(), equalTo("my_type"));
-        assertThat(mapper.sourceMapper().enabled(), equalTo(true));
-    }
-
-    public void testDefaultMappingAndNoMappingWithMapperService() throws Exception {
-        String defaultMapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject(MapperService.DEFAULT_MAPPING)
-                .startObject("_source").field("enabled", false).endObject()
-                .endObject().endObject());
-
-        Settings settings = Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.V_5_6_0).build();
-        MapperService mapperService = createIndex("test", settings).mapperService();
-        mapperService.merge(MapperService.DEFAULT_MAPPING, new CompressedXContent(defaultMapping), MapperService.MergeReason.MAPPING_UPDATE);
-
-        DocumentMapper mapper = mapperService.documentMapperWithAutoCreate("my_type").getDocumentMapper();
-        assertThat(mapper.type(), equalTo("my_type"));
-        assertThat(mapper.sourceMapper().enabled(), equalTo(false));
-    }
-
-    public void testDefaultMappingAndWithMappingOverrideWithMapperService() throws Exception {
-        String defaultMapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject(MapperService.DEFAULT_MAPPING)
-                .startObject("_source").field("enabled", false).endObject()
-                .endObject().endObject());
-
-        Settings settings = Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.V_5_6_0).build();
-        MapperService mapperService = createIndex("test", settings).mapperService();
-        mapperService.merge(MapperService.DEFAULT_MAPPING, new CompressedXContent(defaultMapping), MapperService.MergeReason.MAPPING_UPDATE);
-
-        String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("my_type")
-                .startObject("_source").field("enabled", true).endObject()
-                .endObject().endObject());
-        mapperService.merge("my_type", new CompressedXContent(mapping), MapperService.MergeReason.MAPPING_UPDATE);
-
-        DocumentMapper mapper = mapperService.documentMapper("my_type");
-        assertThat(mapper.type(), equalTo("my_type"));
-        assertThat(mapper.sourceMapper().enabled(), equalTo(true));
-    }
-
-    void assertConflicts(String mapping1, String mapping2, DocumentMapperParser parser, String... conflicts) throws IOException {
+    private void assertConflicts(String mapping1, String mapping2, DocumentMapperParser parser, String... conflicts) throws IOException {
         DocumentMapper docMapper = parser.parse("type", new CompressedXContent(mapping1));
         docMapper = parser.parse("type", docMapper.mappingSource());
         if (conflicts.length == 0) {
diff --git a/server/src/test/java/org/elasticsearch/indices/mapping/UpdateMappingIntegrationIT.java b/server/src/test/java/org/elasticsearch/indices/mapping/UpdateMappingIntegrationIT.java
@@ -26,6 +26,7 @@
 import org.elasticsearch.action.index.IndexRequestBuilder;
 import org.elasticsearch.action.search.SearchResponse;
 import org.elasticsearch.client.Client;
+import org.elasticsearch.cluster.metadata.IndexMetaData;
 import org.elasticsearch.cluster.metadata.MappingMetaData;
 import org.elasticsearch.common.Priority;
 import org.elasticsearch.common.collect.ImmutableOpenMap;
@@ -203,11 +204,13 @@ public void testUpdateMappingNoChanges() {
     @SuppressWarnings("unchecked")
     public void testUpdateDefaultMappingSettings() throws Exception {
         logger.info("Creating index with _default_ mappings");
-        client().admin().indices().prepareCreate("test").addMapping(MapperService.DEFAULT_MAPPING,
-                JsonXContent.contentBuilder().startObject().startObject(MapperService.DEFAULT_MAPPING)
+        client().admin().indices().prepareCreate("test")
+                .setSettings(Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.V_6_3_0).build())
+                .addMapping(MapperService.DEFAULT_MAPPING,
+                        JsonXContent.contentBuilder().startObject().startObject(MapperService.DEFAULT_MAPPING)
                         .field("date_detection", false)
                         .endObject().endObject()
-        ).get();
+                        ).get();
 
         GetMappingsResponse getResponse = client().admin().indices().prepareGetMappings("test").addTypes(MapperService.DEFAULT_MAPPING).get();
         Map<String, Object> defaultMapping = getResponse.getMappings().get("test").get(MapperService.DEFAULT_MAPPING).sourceAsMap();
diff --git a/server/src/test/java/org/elasticsearch/indices/template/SimpleIndexTemplateIT.java b/server/src/test/java/org/elasticsearch/indices/template/SimpleIndexTemplateIT.java

Original file line number	Diff line number	Diff line change
`@@ -36,5 +36,3 @@ include::dynamic/field-mapping.asciidoc[]`
`36`	`36`
`37`	`37`	`include::dynamic/templates.asciidoc[]`
`38`	`38`
`39`		`-include::dynamic/default-mapping.asciidoc[]`
`40`		`-`