Mappings: Make sure that multi fields are serialized in alphabetic or…

…der to ensure that the source is always the same. Closes elastic#7215
mute · Aug 11, 2014 · 698496f · 698496f
1 parent 6d66af1
commit 698496f
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 6 deletions.
diff --git a/src/main/java/org/elasticsearch/index/mapper/core/AbstractFieldMapper.java b/src/main/java/org/elasticsearch/index/mapper/core/AbstractFieldMapper.java
@@ -59,10 +59,7 @@
 import org.elasticsearch.index.similarity.SimilarityProvider;
 
 import java.io.IOException;
-import java.util.ArrayList;
-import java.util.List;
-import java.util.Locale;
-import java.util.Map;
+import java.util.*;
 
 /**
  *
@@ -991,9 +988,17 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
                 builder.field("path", pathType.name().toLowerCase(Locale.ROOT));
             }
             if (!mappers.isEmpty()) {
+                // sort the mappers so we get consistent serialization format
+                Mapper[] sortedMappers = mappers.values().toArray(Mapper.class);
+                Arrays.sort(sortedMappers, new Comparator<Mapper>() {
+                    @Override
+                    public int compare(Mapper o1, Mapper o2) {
+                        return o1.name().compareTo(o2.name());
+                    }
+                });
                 builder.startObject("fields");
-                for (ObjectCursor<Mapper> cursor : mappers.values()) {
-                    cursor.value.toXContent(builder, params);
+                for (Mapper mapper : sortedMappers) {
+                    mapper.toXContent(builder, params);
                 }
                 builder.endObject();
             }

diff --git a/src/test/java/org/elasticsearch/index/mapper/multifield/MultiFieldTests.java b/src/test/java/org/elasticsearch/index/mapper/multifield/MultiFieldTests.java
@@ -22,6 +22,9 @@
 import org.apache.lucene.index.IndexableField;
 import org.elasticsearch.common.bytes.BytesArray;
 import org.elasticsearch.common.bytes.BytesReference;
+import org.elasticsearch.common.xcontent.XContentBuilder;
+import org.elasticsearch.common.xcontent.XContentHelper;
+import org.elasticsearch.common.xcontent.support.XContentMapValues;
 import org.elasticsearch.index.mapper.DocumentMapper;
 import org.elasticsearch.index.mapper.DocumentMapperParser;
 import org.elasticsearch.index.mapper.FieldMapper;
@@ -32,6 +35,9 @@
 import org.elasticsearch.test.ElasticsearchTestCase;
 import org.junit.Test;
 
+import java.util.Arrays;
+import java.util.Map;
+
 import static org.elasticsearch.common.io.Streams.copyToBytesFromClasspath;
 import static org.elasticsearch.common.io.Streams.copyToStringFromClasspath;
 import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
@@ -405,4 +411,35 @@ public void testConvertMultiFieldCompletion() throws Exception {
         assertThat(f.fieldType().stored(), equalTo(false));
         assertThat(f.fieldType().indexed(), equalTo(true));
     }
+
+    @Test
+    // The underlying order of the fields in multi fields in the mapping source should always be consistent, if not this
+    // can to unnecessary re-syncing of the mappings between the local instance and cluster state
+    public void testMultiFieldsInConsistentOrder() throws Exception {
+        String[] multiFieldNames = new String[randomIntBetween(2, 10)];
+        for (int i = 0; i < multiFieldNames.length; i++) {
+            multiFieldNames[i] = randomAsciiOfLength(4);
+        }
+
+        XContentBuilder builder = jsonBuilder().startObject().startObject("type").startObject("properties")
+                .startObject("my_field").field("type", "string").startObject("fields");
+        for (String multiFieldName : multiFieldNames) {
+            builder = builder.startObject(multiFieldName).field("type", "string").endObject();
+        }
+        builder = builder.endObject().endObject().endObject().endObject().endObject();
+        String mapping = builder.string();
+        DocumentMapper docMapper = MapperTestUtils.newParser().parse(mapping);
+        Arrays.sort(multiFieldNames);
+
+        Map<String, Object> sourceAsMap = XContentHelper.convertToMap(docMapper.mappingSource().compressed(), true).v2();
+        @SuppressWarnings("unchecked")
+        Map<String, Object> multiFields = (Map<String, Object>) XContentMapValues.extractValue("type.properties.my_field.fields", sourceAsMap);
+        assertThat(multiFields.size(), equalTo(multiFieldNames.length));
+
+        int i = 0;
+        // underlying map is LinkedHashMap, so this ok:
+        for (String field : multiFields.keySet()) {
+            assertThat(field, equalTo(multiFieldNames[i++]));
+        }
+    }
 }