Browse Source

[root mappers] fix conflict when updating mapping with _all disabled

_all reports a conflict since #7377. However, it was not checked if _all
was actually configured in the updated mapping. Therefore whenever _all
was disabled a mapping could not be updated unless _all was again added to the
updated mapping.
Also, add enabled setting to mapping always whenever enabled was set explicitely.

closes #8423
closes #8426
Britta Weber 11 năm trước cách đây
mục cha
commit
4b5592cc59

+ 12 - 12
src/main/java/org/elasticsearch/index/mapper/internal/AllFieldMapper.java

@@ -75,7 +75,7 @@ public class AllFieldMapper extends AbstractFieldMapper<String> implements Inter
     public static class Defaults extends AbstractFieldMapper.Defaults {
         public static final String NAME = AllFieldMapper.NAME;
         public static final String INDEX_NAME = AllFieldMapper.NAME;
-        public static final boolean ENABLED = true;
+        public static final EnabledAttributeMapper ENABLED = EnabledAttributeMapper.UNSET_ENABLED;
 
         public static final FieldType FIELD_TYPE = new FieldType();
 
@@ -88,7 +88,7 @@ public class AllFieldMapper extends AbstractFieldMapper<String> implements Inter
 
     public static class Builder extends AbstractFieldMapper.Builder<Builder, AllFieldMapper> {
 
-        private boolean enabled = Defaults.ENABLED;
+        private EnabledAttributeMapper enabled = Defaults.ENABLED;
 
         // an internal flag, automatically set if we encounter boosting
         boolean autoBoost = false;
@@ -99,7 +99,7 @@ public class AllFieldMapper extends AbstractFieldMapper<String> implements Inter
             indexName = Defaults.INDEX_NAME;
         }
 
-        public Builder enabled(boolean enabled) {
+        public Builder enabled(EnabledAttributeMapper enabled) {
             this.enabled = enabled;
             return this;
         }
@@ -127,7 +127,7 @@ public class AllFieldMapper extends AbstractFieldMapper<String> implements Inter
                 String fieldName = Strings.toUnderscoreCase(entry.getKey());
                 Object fieldNode = entry.getValue();
                 if (fieldName.equals("enabled")) {
-                    builder.enabled(nodeBooleanValue(fieldNode));
+                    builder.enabled(nodeBooleanValue(fieldNode) ? EnabledAttributeMapper.ENABLED : EnabledAttributeMapper.DISABLED);
                     iterator.remove();
                 } else if (fieldName.equals("auto_boost")) {
                     builder.autoBoost = nodeBooleanValue(fieldNode);
@@ -139,7 +139,7 @@ public class AllFieldMapper extends AbstractFieldMapper<String> implements Inter
     }
 
 
-    private boolean enabled;
+    private EnabledAttributeMapper enabledState;
     // The autoBoost flag is automatically set based on indexed docs on the mappings
     // if a doc is indexed with a specific boost value and part of _all, it is automatically
     // set to true. This allows to optimize (automatically, which we like) for the common case
@@ -152,7 +152,7 @@ public class AllFieldMapper extends AbstractFieldMapper<String> implements Inter
     }
 
     protected AllFieldMapper(String name, FieldType fieldType, NamedAnalyzer indexAnalyzer, NamedAnalyzer searchAnalyzer,
-                             boolean enabled, boolean autoBoost, PostingsFormatProvider postingsProvider,
+                             EnabledAttributeMapper enabled, boolean autoBoost, PostingsFormatProvider postingsProvider,
                              DocValuesFormatProvider docValuesProvider, SimilarityProvider similarity, Loading normsLoading,
                              @Nullable Settings fieldDataSettings, Settings indexSettings) {
         super(new Names(name, name, name, name), 1.0f, fieldType, null, indexAnalyzer, searchAnalyzer, postingsProvider, docValuesProvider,
@@ -160,13 +160,13 @@ public class AllFieldMapper extends AbstractFieldMapper<String> implements Inter
         if (hasDocValues()) {
             throw new MapperParsingException("Field [" + names.fullName() + "] is always tokenized and cannot have doc values");
         }
-        this.enabled = enabled;
+        this.enabledState = enabled;
         this.autoBoost = autoBoost;
 
     }
 
     public boolean enabled() {
-        return this.enabled;
+        return this.enabledState.enabled;
     }
 
     @Override
@@ -216,7 +216,7 @@ public class AllFieldMapper extends AbstractFieldMapper<String> implements Inter
 
     @Override
     protected void parseCreateField(ParseContext context, List<Field> fields) throws IOException {
-        if (!enabled) {
+        if (!enabledState.enabled) {
             return;
         }
         // reset the entries
@@ -283,8 +283,8 @@ public class AllFieldMapper extends AbstractFieldMapper<String> implements Inter
     }
 
     private void innerToXContent(XContentBuilder builder, boolean includeDefaults) throws IOException {
-        if (includeDefaults || enabled != Defaults.ENABLED) {
-            builder.field("enabled", enabled);
+        if (includeDefaults || enabledState != Defaults.ENABLED) {
+            builder.field("enabled", enabledState.enabled);
         }
         if (includeDefaults || autoBoost != false) {
             builder.field("auto_boost", autoBoost);
@@ -353,7 +353,7 @@ public class AllFieldMapper extends AbstractFieldMapper<String> implements Inter
 
     @Override
     public void merge(Mapper mergeWith, MergeContext mergeContext) throws MergeMappingException {
-        if (((AllFieldMapper)mergeWith).enabled() != this.enabled()) {
+        if (((AllFieldMapper)mergeWith).enabled() != this.enabled() && ((AllFieldMapper)mergeWith).enabledState != Defaults.ENABLED) {
             mergeContext.addConflict("mapper [" + names.fullName() + "] enabled is " + this.enabled() + " now encountering "+ ((AllFieldMapper)mergeWith).enabled());
         }
         super.merge(mergeWith, mergeContext);

+ 49 - 0
src/test/java/org/elasticsearch/index/mapper/update/UpdateMappingOnClusterTests.java

@@ -30,6 +30,7 @@ import org.elasticsearch.test.ElasticsearchIntegrationTest;
 import org.junit.Test;
 
 import java.io.IOException;
+import java.util.HashMap;
 import java.util.LinkedHashMap;
 
 import static org.elasticsearch.common.io.Streams.copyToStringFromClasspath;
@@ -70,6 +71,54 @@ public class UpdateMappingOnClusterTests extends ElasticsearchIntegrationTest {
         testConflict(mapping, mappingUpdate, errorMessage);
     }
 
+
+    @Test
+    public void test_all_with_default() throws Exception {
+        String defaultMapping = jsonBuilder().startObject().startObject("_default_")
+                .startObject("_all")
+                .field("enabled", false)
+                .endObject()
+                .endObject().endObject().string();
+        client().admin().indices().prepareCreate("index").addMapping("_default_", defaultMapping).get();
+        String docMapping = jsonBuilder().startObject()
+                .startObject("doc")
+                .endObject()
+                .endObject().string();
+        PutMappingResponse response = client().admin().indices().preparePutMapping("index").setType("doc").setSource(docMapping).get();
+        assertTrue(response.isAcknowledged());
+        String docMappingUpdate = jsonBuilder().startObject().startObject("doc")
+                .startObject("properties")
+                .startObject("text")
+                .field("type", "string")
+                .endObject()
+                .endObject()
+                .endObject()
+                .endObject().string();
+        response = client().admin().indices().preparePutMapping("index").setType("doc").setSource(docMappingUpdate).get();
+        assertTrue(response.isAcknowledged());
+        String docMappingAllExplicitEnabled = jsonBuilder().startObject()
+                .startObject("doc_all_enabled")
+                .startObject("_all")
+                .field("enabled", true)
+                .endObject()
+                .endObject()
+                .endObject().string();
+        response = client().admin().indices().preparePutMapping("index").setType("doc_all_enabled").setSource(docMappingAllExplicitEnabled).get();
+        assertTrue(response.isAcknowledged());
+
+        GetMappingsResponse mapping = client().admin().indices().prepareGetMappings("index").get();
+        HashMap props = (HashMap)mapping.getMappings().get("index").get("doc").getSourceAsMap().get("_all");
+        assertThat((Boolean)props.get("enabled"), equalTo(false));
+        props = (HashMap)mapping.getMappings().get("index").get("doc").getSourceAsMap().get("properties");
+        assertNotNull(props);
+        assertNotNull(props.get("text"));
+        props = (HashMap)mapping.getMappings().get("index").get("doc_all_enabled").getSourceAsMap().get("_all");
+        assertThat((Boolean)props.get("enabled"), equalTo(true));
+        props = (HashMap)mapping.getMappings().get("index").get("_default_").getSourceAsMap().get("_all");
+        assertThat((Boolean)props.get("enabled"), equalTo(false));
+
+    }
+
     @Test
     public void test_doc_valuesInvalidMapping() throws Exception {
         String mapping = jsonBuilder().startObject().startObject("mappings").startObject(TYPE).startObject("_all").startObject("fielddata").field("format", "doc_values").endObject().endObject().endObject().endObject().endObject().string();

+ 1 - 1
src/test/java/org/elasticsearch/index/mapper/update/UpdateMappingTests.java

@@ -64,7 +64,7 @@ public class UpdateMappingTests extends ElasticsearchSingleNodeTest {
     public void test_all_enabled_after_enabled() throws Exception {
         XContentBuilder mapping = XContentFactory.jsonBuilder().startObject().startObject("_all").field("enabled", true).endObject().endObject();
         XContentBuilder mappingUpdate = XContentFactory.jsonBuilder().startObject().startObject("_all").field("enabled", true).endObject().startObject("properties").startObject("text").field("type", "string").endObject().endObject().endObject();
-        XContentBuilder expectedMapping = XContentFactory.jsonBuilder().startObject().startObject("type").startObject("properties").startObject("text").field("type", "string").endObject().endObject().endObject().endObject();
+        XContentBuilder expectedMapping = XContentFactory.jsonBuilder().startObject().startObject("type").startObject("_all").field("enabled", true).endObject().startObject("properties").startObject("text").field("type", "string").endObject().endObject().endObject().endObject();
         testNoConflictWhileMergingAndMappingChanged(mapping, mappingUpdate, expectedMapping);
     }