Mappings: Fix doc values representation to always serliaze if explicitly set

When doc values are explicitly set to the default value serialization
is skipped. This means the alternate way of specifying doc values,
through `fielddata.format: doc_values`, will take precedense if
present.

This change fixes doc values to always be serialized when an explicit value
was passed, so that it continues to take precedence over
`fielddata.format`.

closes #10297
closes #10302

src/main/java/org/elasticsearch/index/mapper/core/AbstractFieldMapper.java

@@ -294,7 +294,7 @@ public abstract class AbstractFieldMapper<T> implements FieldMapper<T> {
     protected final Names names;
     protected float boost;
     protected FieldType fieldType;
-    protected final boolean docValues;
+    protected final Boolean docValues;
     protected final NamedAnalyzer indexAnalyzer;
     protected NamedAnalyzer searchAnalyzer;
     protected final SimilarityProvider similarity;
@@ -349,13 +349,9 @@ public abstract class AbstractFieldMapper<T> implements FieldMapper<T> {
         } else if (fieldDataType != null && FieldDataType.DOC_VALUES_FORMAT_VALUE.equals(fieldDataType.getFormat(indexSettings))) {
             // convoluted way to enable doc values, should be removed in the future
             this.docValues = true;
-        } else if (Version.indexCreated(indexSettings).onOrAfter(Version.V_2_0_0)) {
-            // 2.0+ index, default to true when appropriate
-            this.docValues = defaultDocValues();
         } else {
-            // old default, disable
-            this.docValues = false;
-        } 
+            this.docValues = null; // use the default
+        }
         this.multiFields = multiFields;
         this.copyTo = copyTo;
     }
@@ -380,7 +376,7 @@ public abstract class AbstractFieldMapper<T> implements FieldMapper<T> {
 
     @Override
     public final boolean hasDocValues() {
-        return docValues;
+        return docValues == null ? defaultDocValues() : docValues;
     }
 
     @Override
@@ -723,9 +719,7 @@ public abstract class AbstractFieldMapper<T> implements FieldMapper<T> {
         if (includeDefaults || fieldType.stored() != defaultFieldType.stored()) {
             builder.field("store", fieldType.stored());
         }
-        if (includeDefaults || hasDocValues() != defaultDocValues()) {
-            builder.field(TypeParsers.DOC_VALUES, docValues);
-        }
+        doXContentDocValues(builder, includeDefaults);
         if (includeDefaults || fieldType.storeTermVectors() != defaultFieldType.storeTermVectors()) {
             builder.field("term_vector", termVectorOptionsToString(fieldType));
         }
@@ -778,6 +772,12 @@ public abstract class AbstractFieldMapper<T> implements FieldMapper<T> {
             }
         }
     }
+    
+    protected void doXContentDocValues(XContentBuilder builder, boolean includeDefaults) throws IOException {
+        if (includeDefaults || docValues != null) {
+            builder.field(TypeParsers.DOC_VALUES, hasDocValues());
+        }
+    }
 
     protected static String indexOptionToString(IndexOptions indexOption) {
         switch (indexOption) {

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

@@ -42,7 +42,6 @@ import org.elasticsearch.index.mapper.RootMapper;
 import org.elasticsearch.index.mapper.core.DateFieldMapper;
 import org.elasticsearch.index.mapper.core.LongFieldMapper;
 import org.elasticsearch.index.mapper.core.NumberFieldMapper;
-import org.elasticsearch.index.mapper.core.TypeParsers;
 
 import java.io.IOException;
 import java.util.Iterator;
@@ -328,9 +327,7 @@ public class TimestampFieldMapper extends DateFieldMapper implements InternalMap
         if (includeDefaults || fieldType.stored() != Defaults.FIELD_TYPE.stored()) {
             builder.field("store", fieldType.stored());
         }
-        if (includeDefaults || hasDocValues() != defaultDocValues()) {
-            builder.field(TypeParsers.DOC_VALUES, docValues);
-        }
+        doXContentDocValues(builder, includeDefaults);
         if (includeDefaults || path != Defaults.PATH) {
             builder.field("path", path);
         }

src/test/java/org/elasticsearch/index/mapper/timestamp/TimestampMappingTests.java

@@ -664,10 +664,10 @@ public class TimestampMappingTests extends ElasticsearchSingleNodeTest {
         mapping2.endObject()
                 .endObject().endObject();
 
-        testConflict(mapping1.string(), mapping2.string(), parser, (path1 == path2 ? null : "Cannot update path in _timestamp value"));
+        assertConflict(mapping1.string(), mapping2.string(), parser, (path1 == path2 ? null : "Cannot update path in _timestamp value"));
     }
 
-    void testConflict(String mapping1, String mapping2, DocumentMapperParser parser, String conflict) throws IOException {
+    void assertConflict(String mapping1, String mapping2, DocumentMapperParser parser, String conflict) throws IOException {
         DocumentMapper docMapper = parser.parse(mapping1);
         docMapper.refreshSource();
         docMapper = parser.parse(docMapper.mappingSource().string());
@@ -677,4 +677,59 @@ public class TimestampMappingTests extends ElasticsearchSingleNodeTest {
             assertThat(mergeResult.conflicts()[0], containsString(conflict));
         }
     }
+    
+    public void testDocValuesSerialization() throws Exception {
+        // default
+        String mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
+            .startObject("_timestamp")
+            .endObject().endObject().endObject().string();
+        assertDocValuesSerialization(mapping);
+
+        // just format specified
+        mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
+            .startObject("_timestamp")
+            .startObject("fielddata").field("format", "doc_values").endObject()
+            .endObject().endObject().endObject().string();
+        assertDocValuesSerialization(mapping);
+
+        // explicitly enabled
+        mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
+            .startObject("_timestamp")
+            .field("doc_values", true)
+            .endObject().endObject().endObject().string();
+        assertDocValuesSerialization(mapping);
+
+        // explicitly disabled
+        mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
+            .startObject("_timestamp")
+            .field("doc_values", false)
+            .endObject().endObject().endObject().string();
+        assertDocValuesSerialization(mapping);
+
+        // explicitly enabled, with format
+        mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
+            .startObject("_timestamp")
+            .field("doc_values", true)
+            .startObject("fielddata").field("format", "doc_values").endObject()
+            .endObject().endObject().endObject().string();
+        assertDocValuesSerialization(mapping);
+
+        // explicitly disabled, with format
+        mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
+            .startObject("_timestamp")
+            .field("doc_values", false)
+            .startObject("fielddata").field("format", "doc_values").endObject()
+            .endObject().endObject().endObject().string();
+        assertDocValuesSerialization(mapping);
+    }
+    
+    void assertDocValuesSerialization(String mapping) throws Exception {
+        DocumentMapperParser parser = createIndex("test_doc_values").mapperService().documentMapperParser();
+        DocumentMapper docMapper = parser.parse(mapping);
+        boolean docValues= docMapper.timestampFieldMapper().hasDocValues();
+        docMapper.refreshSource();
+        docMapper = parser.parse(docMapper.mappingSource().string());
+        assertThat(docMapper.timestampFieldMapper().hasDocValues(), equalTo(docValues));
+        assertAcked(client().admin().indices().prepareDelete("test_doc_values"));
+    }
 }