Mappings: Enforce non-null settings.

No that we are using the index created version to make index-time decisions,
assuming that the version is the current version when settings are null is
very error-prone. Instead we should ensure that settings are always non-null
and contain the version when the index was created.

Close #7032

src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java

@@ -148,7 +148,6 @@ public class DocumentMapper implements ToXContent {
 
         private final String index;
 
-        @Nullable
         private final Settings indexSettings;
 
         private final RootObjectMapper rootObjectMapper;
@@ -157,7 +156,7 @@ public class DocumentMapper implements ToXContent {
 
         private final Mapper.BuilderContext builderContext;
 
-        public Builder(String index, @Nullable Settings indexSettings, RootObjectMapper.Builder builder) {
+        public Builder(String index, Settings indexSettings, RootObjectMapper.Builder builder) {
             this.index = index;
             this.indexSettings = indexSettings;
             this.builderContext = new Mapper.BuilderContext(indexSettings, new ContentPath(1));

src/main/java/org/elasticsearch/index/mapper/Mapper.java

@@ -45,7 +45,7 @@ public interface Mapper extends ToXContent {
         private final Settings indexSettings;
         private final ContentPath contentPath;
 
-        public BuilderContext(@Nullable Settings indexSettings, ContentPath contentPath) {
+        public BuilderContext(Settings indexSettings, ContentPath contentPath) {
             this.contentPath = contentPath;
             this.indexSettings = indexSettings;
         }

src/main/java/org/elasticsearch/index/mapper/MapperBuilders.java

@@ -19,7 +19,6 @@
 
 package org.elasticsearch.index.mapper;
 
-import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.index.mapper.core.*;
 import org.elasticsearch.index.mapper.geo.GeoPointFieldMapper;
@@ -38,11 +37,7 @@ public final class MapperBuilders {
 
     }
 
-    public static DocumentMapper.Builder doc(String index, RootObjectMapper.Builder objectBuilder) {
-        return new DocumentMapper.Builder(index, null, objectBuilder);
-    }
-
-    public static DocumentMapper.Builder doc(String index, @Nullable Settings settings, RootObjectMapper.Builder objectBuilder) {
+    public static DocumentMapper.Builder doc(String index, Settings settings, RootObjectMapper.Builder objectBuilder) {
         return new DocumentMapper.Builder(index, settings, objectBuilder);
     }
 

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

@@ -199,7 +199,7 @@ public abstract class NumberFieldMapper<T extends Number> extends AbstractFieldM
         }
         this.ignoreMalformed = ignoreMalformed;
         this.coerce = coerce;
-        Version v = indexSettings == null ? Version.CURRENT : Version.indexCreated(indexSettings);
+        Version v = Version.indexCreated(indexSettings);
         this.useSortedNumericDocValues = v.onOrAfter(Version.V_1_4_0);
     }
 

src/test/java/org/elasticsearch/index/fielddata/AbstractFieldDataTests.java

@@ -59,7 +59,7 @@ public abstract class AbstractFieldDataTests extends ElasticsearchSingleNodeTest
 
     public <IFD extends IndexFieldData<?>> IFD getForField(FieldDataType type, String fieldName) {
         final FieldMapper<?> mapper;
-        final BuilderContext context = new BuilderContext(null, new ContentPath(1));
+        final BuilderContext context = new BuilderContext(indexService.settingsService().getSettings(), new ContentPath(1));
         if (type.getType().equals("string")) {
             mapper = MapperBuilders.stringField(fieldName).tokenized(false).fieldDataSettings(type.getSettings()).build(context);
         } else if (type.getType().equals("float")) {

src/test/java/org/elasticsearch/index/fielddata/IndexFieldDataServiceTests.java

@@ -33,6 +33,7 @@ import org.elasticsearch.index.mapper.FieldMapper;
 import org.elasticsearch.index.mapper.Mapper.BuilderContext;
 import org.elasticsearch.index.mapper.MapperBuilders;
 import org.elasticsearch.index.mapper.core.*;
+import org.elasticsearch.index.service.IndexService;
 import org.elasticsearch.test.ElasticsearchSingleNodeTest;
 
 import java.util.Arrays;
@@ -47,9 +48,10 @@ public class IndexFieldDataServiceTests extends ElasticsearchSingleNodeTest {
     private static Settings DOC_VALUES_SETTINGS = ImmutableSettings.builder().put(FieldDataType.FORMAT_KEY, FieldDataType.DOC_VALUES_FORMAT_VALUE).build();
 
     public void testGetForFieldDefaults() {
-        final IndexFieldDataService ifdService = createIndex("test").fieldData();
+        final IndexService indexService = createIndex("test");
+        final IndexFieldDataService ifdService = indexService.fieldData();
         for (boolean docValues : Arrays.asList(true, false)) {
-            final BuilderContext ctx = new BuilderContext(null, new ContentPath(1));
+            final BuilderContext ctx = new BuilderContext(indexService.settingsService().getSettings(), new ContentPath(1));
             final StringFieldMapper stringMapper = new StringFieldMapper.Builder("string").tokenized(false).fieldDataSettings(docValues ? DOC_VALUES_SETTINGS : ImmutableSettings.EMPTY).build(ctx);
             ifdService.clear();
             IndexFieldData<?> fd = ifdService.getForField(stringMapper);
@@ -96,8 +98,9 @@ public class IndexFieldDataServiceTests extends ElasticsearchSingleNodeTest {
 
     @SuppressWarnings("unchecked")
     public void testByPassDocValues() {
-        final IndexFieldDataService ifdService = createIndex("test").fieldData();
-        final BuilderContext ctx = new BuilderContext(null, new ContentPath(1));
+        final IndexService indexService = createIndex("test");
+        final IndexFieldDataService ifdService = indexService.fieldData();
+        final BuilderContext ctx = new BuilderContext(indexService.settingsService().getSettings(), new ContentPath(1));
         final StringFieldMapper stringMapper = MapperBuilders.stringField("string").tokenized(false).fieldDataSettings(DOC_VALUES_SETTINGS).fieldDataSettings(ImmutableSettings.builder().put("format", "fst").build()).build(ctx);
         ifdService.clear();
         IndexFieldData<?> fd = ifdService.getForField(stringMapper);
@@ -127,8 +130,9 @@ public class IndexFieldDataServiceTests extends ElasticsearchSingleNodeTest {
     }
 
     public void testChangeFieldDataFormat() throws Exception {
-        final IndexFieldDataService ifdService = createIndex("test").fieldData();
-        final BuilderContext ctx = new BuilderContext(null, new ContentPath(1));
+        final IndexService indexService = createIndex("test");
+        final IndexFieldDataService ifdService = indexService.fieldData();
+        final BuilderContext ctx = new BuilderContext(indexService.settingsService().getSettings(), new ContentPath(1));
         final StringFieldMapper mapper1 = MapperBuilders.stringField("s").tokenized(false).fieldDataSettings(ImmutableSettings.builder().put(FieldDataType.FORMAT_KEY, "paged_bytes").build()).build(ctx);
         final IndexWriter writer = new IndexWriter(new RAMDirectory(), new IndexWriterConfig(TEST_VERSION_CURRENT, new KeywordAnalyzer()));
         Document doc = new Document();

src/test/java/org/elasticsearch/index/mapper/multifield/MultiFieldTests.java

@@ -22,12 +22,14 @@ package org.elasticsearch.index.mapper.multifield;
 import org.apache.lucene.index.IndexableField;
 import org.elasticsearch.common.bytes.BytesArray;
 import org.elasticsearch.common.bytes.BytesReference;
+import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.index.mapper.DocumentMapper;
 import org.elasticsearch.index.mapper.DocumentMapperParser;
 import org.elasticsearch.index.mapper.FieldMapper;
 import org.elasticsearch.index.mapper.ParseContext.Document;
 import org.elasticsearch.index.mapper.core.*;
 import org.elasticsearch.index.mapper.geo.GeoPointFieldMapper;
+import org.elasticsearch.index.service.IndexService;
 import org.elasticsearch.test.ElasticsearchSingleNodeTest;
 import org.junit.Test;
 
@@ -127,9 +129,11 @@ public class MultiFieldTests extends ElasticsearchSingleNodeTest {
 
     @Test
     public void testBuildThenParse() throws Exception {
-        DocumentMapperParser mapperParser = createIndex("test").mapperService().documentMapperParser();
+        IndexService indexService = createIndex("test");
+        Settings settings = indexService.settingsService().getSettings();
+        DocumentMapperParser mapperParser = indexService.mapperService().documentMapperParser();
 
-        DocumentMapper builderDocMapper = doc("test", rootObject("person").add(
+        DocumentMapper builderDocMapper = doc("test", settings, rootObject("person").add(
                 stringField("name").store(true)
                         .addMultiField(stringField("indexed").index(true).tokenized(true))
                         .addMultiField(stringField("not_indexed").index(false).store(true))

src/test/java/org/elasticsearch/index/mapper/simple/SimpleMapperTests.java

@@ -26,6 +26,7 @@ import org.elasticsearch.common.settings.ImmutableSettings;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.index.mapper.*;
 import org.elasticsearch.index.mapper.ParseContext.Document;
+import org.elasticsearch.index.service.IndexService;
 import org.elasticsearch.test.ElasticsearchSingleNodeTest;
 import org.junit.Test;
 
@@ -41,8 +42,10 @@ public class SimpleMapperTests extends ElasticsearchSingleNodeTest {
 
     @Test
     public void testSimpleMapper() throws Exception {
-        DocumentMapperParser mapperParser = createIndex("test").mapperService().documentMapperParser();
-        DocumentMapper docMapper = doc("test",
+        IndexService indexService = createIndex("test");
+        Settings settings = indexService.settingsService().getSettings();
+        DocumentMapperParser mapperParser = indexService.mapperService().documentMapperParser();
+        DocumentMapper docMapper = doc("test", settings,
                 rootObject("person")
                         .add(object("name").add(stringField("first").store(true).index(false)))
         ).build(mapperParser);
@@ -118,8 +121,10 @@ public class SimpleMapperTests extends ElasticsearchSingleNodeTest {
 
     @Test
     public void testNoDocumentSent() throws Exception {
-        DocumentMapperParser mapperParser = createIndex("test").mapperService().documentMapperParser();
-        DocumentMapper docMapper = doc("test",
+        IndexService indexService = createIndex("test");
+        Settings settings = indexService.settingsService().getSettings();
+        DocumentMapperParser mapperParser = indexService.mapperService().documentMapperParser();
+        DocumentMapper docMapper = doc("test", settings,
                 rootObject("person")
                         .add(object("name").add(stringField("first").store(true).index(false)))
         ).build(mapperParser);

src/test/java/org/elasticsearch/index/mapper/string/SimpleStringMappingTests.java

@@ -35,6 +35,7 @@ import org.elasticsearch.index.mapper.DocumentMapper.MergeResult;
 import org.elasticsearch.index.mapper.Mapper.BuilderContext;
 import org.elasticsearch.index.mapper.ParseContext.Document;
 import org.elasticsearch.index.mapper.core.StringFieldMapper;
+import org.elasticsearch.index.service.IndexService;
 import org.elasticsearch.test.ElasticsearchSingleNodeTest;
 import org.junit.Before;
 import org.junit.Test;
@@ -49,11 +50,13 @@ public class SimpleStringMappingTests extends ElasticsearchSingleNodeTest {
 
     private static Settings DOC_VALUES_SETTINGS = ImmutableSettings.builder().put(FieldDataType.FORMAT_KEY, FieldDataType.DOC_VALUES_FORMAT_VALUE).build();
 
+    IndexService indexService;
     DocumentMapperParser parser;
 
     @Before
     public void before() {
-        parser = createIndex("test").mapperService().documentMapperParser();
+        indexService = createIndex("test");
+        parser = indexService.mapperService().documentMapperParser();
     }
 
     @Test
@@ -279,7 +282,7 @@ public class SimpleStringMappingTests extends ElasticsearchSingleNodeTest {
 
     public void testDocValues() throws Exception {
         // doc values only work on non-analyzed content
-        final BuilderContext ctx = new BuilderContext(null, new ContentPath(1));
+        final BuilderContext ctx = new BuilderContext(indexService.settingsService().getSettings(), new ContentPath(1));
         try {
             new StringFieldMapper.Builder("anything").fieldDataSettings(DOC_VALUES_SETTINGS).build(ctx);
             fail();

src/test/java/org/elasticsearch/index/search/FieldDataTermsFilterTests.java

@@ -72,25 +72,25 @@ public class FieldDataTermsFilterTests extends ElasticsearchSingleNodeTest {
     public void setup() throws Exception {
         super.setUp();
 
-        // setup field mappers
-        strMapper = new StringFieldMapper.Builder("str_value")
-                .build(new Mapper.BuilderContext(null, new ContentPath(1)));
-
-        lngMapper = new LongFieldMapper.Builder("lng_value")
-                .build(new Mapper.BuilderContext(null, new ContentPath(1)));
-
-        dblMapper = new DoubleFieldMapper.Builder("dbl_value")
-                .build(new Mapper.BuilderContext(null, new ContentPath(1)));
-
         // create index and fielddata service
-        Settings settings = ImmutableSettings.builder().put("index.fielddata.cache", "none").build();
-        IndexService indexService = createIndex("test", settings);
+        IndexService indexService = createIndex("test", ImmutableSettings.builder().put("index.fielddata.cache", "none").build());
+        Settings settings = indexService.settingsService().getSettings();
         ifdService = indexService.injector().getInstance(IndexFieldDataService.class);
         IndexQueryParserService parserService = indexService.queryParserService();
         parseContext = new QueryParseContext(indexService.index(), parserService);
         writer = new IndexWriter(new RAMDirectory(),
                 new IndexWriterConfig(Lucene.VERSION, new StandardAnalyzer(Lucene.VERSION)));
 
+        // setup field mappers
+        strMapper = new StringFieldMapper.Builder("str_value")
+                .build(new Mapper.BuilderContext(settings, new ContentPath(1)));
+
+        lngMapper = new LongFieldMapper.Builder("lng_value")
+                .build(new Mapper.BuilderContext(settings, new ContentPath(1)));
+
+        dblMapper = new DoubleFieldMapper.Builder("dbl_value")
+                .build(new Mapper.BuilderContext(settings, new ContentPath(1)));
+
         int numDocs = 10;
         for (int i = 0; i < numDocs; i++) {
             Document d = new Document();