Enforce that field aliases can only be specified on indexes with a si…

…ngle type.
elastic · Jul 24, 2018 · 0fe8493 · 0fe8493
1 parent 457d204
commit 0fe8493
Show file tree

Hide file tree

Showing 12 changed files with 131 additions and 42 deletions.
diff --git a/docs/reference/mapping/types/alias.asciidoc b/docs/reference/mapping/types/alias.asciidoc
@@ -1,6 +1,10 @@
 [[alias]]
 === Alias datatype
 
+NOTE: Field aliases can only be specified on indexes with a single mapping type. To add a field
+alias, the index must therefore have been created in 6.0 or later, or be an older index with
+the setting `index.mapping.single_type: true`. Please see <<removal-of-types>> for more information.
+
 An `alias` mapping defines an alternate name for a field in the  index.
 The alias can be used in place of the target field in <<search, search>> requests,
 and selected other APIs like <<search-field-caps, field capabilities>>.

diff --git a/...les/percolator/src/test/java/org/elasticsearch/percolator/PercolateQueryBuilderTests.java b/...les/percolator/src/test/java/org/elasticsearch/percolator/PercolateQueryBuilderTests.java
@@ -97,12 +97,20 @@ protected Collection<Class<? extends Plugin>> getPlugins() {
     @Override
     protected void initializeAdditionalMappings(MapperService mapperService) throws IOException {
         queryField = randomAlphaOfLength(4);
-        aliasField = randomAlphaOfLength(4);
 
         String docType = "_doc";
         mapperService.merge(docType, new CompressedXContent(Strings.toString(PutMappingRequest.buildFromSimplifiedDef(docType,
-                queryField, "type=percolator", aliasField, "type=alias,path=" + queryField
+                queryField, "type=percolator"
         ))), MapperService.MergeReason.MAPPING_UPDATE, false);
+
+        // Field aliases are only supported on indexes with a single type.
+        if (mapperService.getIndexSettings().isSingleType()) {
+            aliasField = randomAlphaOfLength(4);
+            mapperService.merge(docType, new CompressedXContent(Strings.toString(PutMappingRequest.buildFromSimplifiedDef(docType,
+                 aliasField, "type=alias,path=" + queryField
+            ))), MapperService.MergeReason.MAPPING_UPDATE, false);
+        }
+
         mapperService.merge(docType, new CompressedXContent(Strings.toString(PutMappingRequest.buildFromSimplifiedDef(docType,
                 STRING_FIELD_NAME, "type=text"
         ))), MapperService.MergeReason.MAPPING_UPDATE, false);
@@ -369,6 +377,8 @@ public void testSerializationFailsUnlessFetched() throws IOException {
     }
 
     public void testFieldAlias() throws IOException {
+        assumeTrue("Test only runs when there is a single mapping type.", isSingleType());
+
         QueryShardContext shardContext = createShardContext();
 
         PercolateQueryBuilder builder = doCreateTestQueryBuilder(false);

diff --git a/server/src/main/java/org/elasticsearch/index/mapper/FieldAliasMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/FieldAliasMapper.java
@@ -21,6 +21,7 @@
 
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.support.XContentMapValues;
+import org.elasticsearch.index.IndexSettings;
 
 import java.io.IOException;
 import java.util.Collections;
@@ -74,6 +75,10 @@ public Mapper merge(Mapper mergeWith, boolean updateAllTypes) {
         return mergeWith;
     }
 
+    /**
+     * Note: this method is a no-op because field aliases cannot be specified on indexes
+     * with more than one type.
+     */
     @Override
     public Mapper updateFieldType(Map<String, MappedFieldType> fullNameToFieldType) {
         return this;
@@ -96,6 +101,8 @@ public static class TypeParser implements Mapper.TypeParser {
         @Override
         public Mapper.Builder parse(String name, Map<String, Object> node, ParserContext parserContext)
             throws MapperParsingException {
+            checkIndexCompatibility(parserContext.mapperService().getIndexSettings(), name);
+
             FieldAliasMapper.Builder builder = new FieldAliasMapper.Builder(name);
             Object pathField = node.remove(Names.PATH);
             String path = XContentMapValues.nodeStringValue(pathField, null);
@@ -104,6 +111,13 @@ public Mapper.Builder parse(String name, Map<String, Object> node, ParserContext
             }
             return builder.path(path);
         }
+
+        private static void checkIndexCompatibility(IndexSettings settings, String name) {
+            if (!settings.isSingleType()) {
+                throw new IllegalStateException("Cannot create a field alias [" + name + "] " +
+                    "on index [" + settings.getIndex().getName() + "], as it has multiple types.");
+            }
+        }
     }
 
     public static class Builder extends Mapper.Builder<FieldAliasMapper.Builder, FieldAliasMapper> {

diff --git a/server/src/test/java/org/elasticsearch/index/mapper/FieldAliasMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/FieldAliasMapperTests.java
@@ -19,20 +19,32 @@
 
 package org.elasticsearch.index.mapper;
 
+import org.elasticsearch.Version;
+import org.elasticsearch.cluster.metadata.IndexMetaData;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.compress.CompressedXContent;
+import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.index.IndexService;
 import org.elasticsearch.index.mapper.MapperService.MergeReason;
+import org.elasticsearch.plugins.Plugin;
 import org.elasticsearch.test.ESSingleNodeTestCase;
+import org.elasticsearch.test.InternalSettingsPlugin;
+import org.elasticsearch.test.VersionUtils;
 import org.junit.Before;
 
 import java.io.IOException;
+import java.util.Collection;
 
 public class FieldAliasMapperTests extends ESSingleNodeTestCase {
     private MapperService mapperService;
     private DocumentMapperParser parser;
 
+    @Override
+    protected Collection<Class<? extends Plugin>> getPlugins() {
+        return pluginList(InternalSettingsPlugin.class);
+    }
+
     @Before
     public void setup() {
         IndexService indexService = createIndex("test");
@@ -164,4 +176,31 @@ public void testMergeFailure() throws IOException {
         assertEquals("Cannot merge a field alias mapping [alias-field] with a mapping that is not for a field alias.",
             exception.getMessage());
     }
+
+    public void testFieldAliasDisallowedWithMultipleTypes() throws IOException {
+        Version version = VersionUtils.randomVersionBetween(random(), null, Version.V_5_6_0);
+        Settings settings = Settings.builder()
+            .put(IndexMetaData.SETTING_VERSION_CREATED, version)
+            .build();
+        IndexService indexService = createIndex("alias-test", settings);
+        DocumentMapperParser parser = indexService.mapperService().documentMapperParser();
+
+        String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject()
+            .startObject("type")
+                .startObject("properties")
+                    .startObject("alias-field")
+                        .field("type", "alias")
+                        .field("path", "concrete-field")
+                    .endObject()
+                    .startObject("concrete-field")
+                        .field("type", "keyword")
+                    .endObject()
+                .endObject()
+            .endObject()
+        .endObject());
+        IllegalStateException e = expectThrows(IllegalStateException.class,
+            () -> parser.parse("type", new CompressedXContent(mapping)));
+        assertEquals("Cannot create a field alias [alias-field] on index [alias-test],"
+            + " as it has multiple types.", e.getMessage());
+    }
 }
diff --git a/server/src/test/java/org/elasticsearch/index/query/MultiMatchQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/MultiMatchQueryBuilderTests.java
@@ -235,9 +235,12 @@ public void testToQueryFieldsWildcard() throws Exception {
         DisjunctionMaxQuery dQuery = (DisjunctionMaxQuery) query;
         assertThat(dQuery.getTieBreakerMultiplier(), equalTo(1.0f));
         assertThat(dQuery.getDisjuncts().size(), equalTo(3));
-        assertThat(assertDisjunctionSubQuery(query, TermQuery.class, 0).getTerm(), equalTo(new Term(STRING_FIELD_NAME, "test")));
-        assertThat(assertDisjunctionSubQuery(query, TermQuery.class, 1).getTerm(), equalTo(new Term(STRING_FIELD_NAME_2, "test")));
-        assertThat(assertDisjunctionSubQuery(query, TermQuery.class, 2).getTerm(), equalTo(new Term(STRING_FIELD_NAME, "test")));
+        assertThat(assertDisjunctionSubQuery(query, TermQuery.class, 0).getTerm(),
+            equalTo(new Term(expectedFieldName(STRING_ALIAS_FIELD_NAME), "test")));
+        assertThat(assertDisjunctionSubQuery(query, TermQuery.class, 1).getTerm(),
+            equalTo(new Term(STRING_FIELD_NAME_2, "test")));
+        assertThat(assertDisjunctionSubQuery(query, TermQuery.class, 2).getTerm(),
+            equalTo(new Term(STRING_FIELD_NAME, "test")));
     }
 
     public void testToQueryFieldMissing() throws Exception {

diff --git a/server/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java
@@ -507,7 +507,7 @@ public void testToQueryFieldsWildcard() throws Exception {
         DisjunctionMaxQuery dQuery = (DisjunctionMaxQuery) query;
         assertThat(dQuery.getDisjuncts().size(), equalTo(3));
         assertThat(assertDisjunctionSubQuery(query, TermQuery.class, 0).getTerm(),
-            equalTo(new Term(STRING_FIELD_NAME, "test")));
+            equalTo(new Term(expectedFieldName(STRING_ALIAS_FIELD_NAME), "test")));
         assertThat(assertDisjunctionSubQuery(query, TermQuery.class, 1).getTerm(),
             equalTo(new Term(STRING_FIELD_NAME_2, "test")));
         assertThat(assertDisjunctionSubQuery(query, TermQuery.class, 2).getTerm(),

diff --git a/server/src/test/java/org/elasticsearch/index/query/RangeQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/RangeQueryBuilderTests.java
@@ -150,7 +150,9 @@ protected void doAssertLuceneQuery(RangeQueryBuilder queryBuilder, Query query,
 
         } else if (getCurrentTypes().length == 0 ||
             (expectedFieldName.equals(DATE_FIELD_NAME) == false
+                && expectedFieldName.equals(DATE_ALIAS_FIELD_NAME) == false
                 && expectedFieldName.equals(INT_FIELD_NAME) == false
+                && expectedFieldName.equals(INT_ALIAS_FIELD_NAME) == false
                 && expectedFieldName.equals(DATE_RANGE_FIELD_NAME) == false
                 && expectedFieldName.equals(INT_RANGE_FIELD_NAME) == false)) {
             assertThat(query, instanceOf(TermRangeQuery.class));

diff --git a/server/src/test/java/org/elasticsearch/index/query/SpanMultiTermQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/SpanMultiTermQueryBuilderTests.java
@@ -62,13 +62,16 @@ protected void initializeAdditionalMappings(MapperService mapperService) throws
             .startObject("prefix_field")
                 .field("type", "text")
                 .startObject("index_prefixes").endObject()
-            .endObject()
-            .startObject("prefix_field_alias")
-                .field("type", "alias")
-                .field("path", "prefix_field")
-            .endObject()
-        .endObject().endObject().endObject();
-
+            .endObject();
+
+        // Field aliases are only supported on indexes with a single type.
+        if (mapperService.getIndexSettings().isSingleType()) {
+            mapping.startObject("prefix_field_alias")
+                    .field("type", "alias")
+                    .field("path", "prefix_field")
+                .endObject();
+        }
+        mapping.endObject().endObject().endObject();
         mapperService.merge("_doc",
             new CompressedXContent(Strings.toString(mapping)), MapperService.MergeReason.MAPPING_UPDATE, true);
     }
@@ -178,7 +181,9 @@ public void testToQueryInnerSpanMultiTerm() throws IOException {
     }
 
     public void testToQueryInnerTermQuery() throws IOException {
-        String fieldName = randomFrom("prefix_field", "prefix_field_alias");
+        String fieldName = isSingleType()
+            ? randomFrom("prefix_field", "prefix_field_alias")
+            : "prefix_field";
         final QueryShardContext context = createShardContext();
         if (context.getIndexSettings().getIndexVersionCreated().onOrAfter(Version.V_6_4_0)) {
             Query query = new SpanMultiTermQueryBuilder(new PrefixQueryBuilder(fieldName, "foo"))

diff --git a/server/src/test/java/org/elasticsearch/index/query/TermsSetQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/TermsSetQueryBuilderTests.java
@@ -263,6 +263,8 @@ public void testDoToQuery_msmScriptField() throws Exception {
     }
 
     public void testFieldAlias() {
+        assumeTrue("Test runs only when there is a single mapping type.", isSingleType());
+
         List<String> randomTerms = Arrays.asList(generateRandomStringArray(5, 10, false, false));
         TermsSetQueryBuilder queryBuilder = new TermsSetQueryBuilder(STRING_ALIAS_FIELD_NAME, randomTerms)
             .setMinimumShouldMatchField("m_s_m");

diff --git a/server/src/test/java/org/elasticsearch/indices/mapping/SimpleGetFieldMappingsIT.java b/server/src/test/java/org/elasticsearch/indices/mapping/SimpleGetFieldMappingsIT.java
@@ -77,10 +77,6 @@ private XContentBuilder getMappingForType(String type) throws IOException {
                     .startObject("field1")
                         .field("type", "text")
                     .endObject()
-                   .startObject("alias")
-                        .field("type", "alias")
-                        .field("path", "field1")
-                    .endObject()
                     .startObject("obj")
                         .startObject("properties")
                             .startObject("subfield")
@@ -232,7 +228,18 @@ public void testSimpleGetFieldMappingsWithDefaults() throws Exception {
 
     @SuppressWarnings("unchecked")
     public void testGetFieldMappingsWithFieldAlias() throws Exception {
-        assertAcked(prepareCreate("test").addMapping("type", getMappingForType("type")));
+        XContentBuilder mapping = XContentFactory.jsonBuilder().startObject()
+            .startObject("properties")
+                .startObject("field1")
+                    .field("type", "text")
+                .endObject()
+               .startObject("alias")
+                    .field("type", "alias")
+                    .field("path", "field1")
+                .endObject()
+            .endObject()
+        .endObject();
+        assertAcked(prepareCreate("test").addMapping("type", mapping));
 
         GetFieldMappingsResponse response = client().admin().indices().prepareGetFieldMappings()
             .setFields("alias", "field1").get();

diff --git a/server/src/test/java/org/elasticsearch/search/geo/GeoPolygonIT.java b/server/src/test/java/org/elasticsearch/search/geo/GeoPolygonIT.java
@@ -19,20 +19,12 @@
 
 package org.elasticsearch.search.geo;
 
-import org.elasticsearch.Version;
 import org.elasticsearch.action.search.SearchResponse;
-import org.elasticsearch.cluster.metadata.IndexMetaData;
 import org.elasticsearch.common.geo.GeoPoint;
-import org.elasticsearch.common.settings.Settings;
-import org.elasticsearch.plugins.Plugin;
 import org.elasticsearch.search.SearchHit;
 import org.elasticsearch.test.ESIntegTestCase;
-import org.elasticsearch.test.InternalSettingsPlugin;
-import org.elasticsearch.test.VersionUtils;
 
 import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collection;
 import java.util.List;
 
 import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
@@ -46,18 +38,9 @@
 @ESIntegTestCase.SuiteScopeTestCase
 public class GeoPolygonIT extends ESIntegTestCase {
 
-    @Override
-    protected Collection<Class<? extends Plugin>> nodePlugins() {
-        return Arrays.asList(InternalSettingsPlugin.class); // uses index.version.created
-    }
-
     @Override
     protected void setupSuiteScopeCluster() throws Exception {
-        Version version = VersionUtils.randomVersionBetween(random(), Version.V_5_0_0,
-                Version.CURRENT);
-        Settings settings = Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, version).build();
-
-        assertAcked(prepareCreate("test").setSettings(settings).addMapping("type1", "location",
+        assertAcked(prepareCreate("test").addMapping("type1", "location",
             "type=geo_point", "alias",
             "type=alias,path=location"));
         ensureGreen();

diff --git a/test/framework/src/main/java/org/elasticsearch/test/AbstractBuilderTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/AbstractBuilderTestCase.java
@@ -143,6 +143,10 @@ protected static String[] getCurrentTypes() {
         return currentTypes;
     }
 
+    protected static boolean isSingleType() {
+        return serviceHolder.idxSettings.isSingleType();
+    }
+
     protected Collection<Class<? extends Plugin>> getPlugins() {
         return Collections.emptyList();
     }
@@ -217,7 +221,7 @@ protected Settings indexSettings() {
     }
 
     protected static String expectedFieldName(String builderFieldName) {
-        if (currentTypes.length == 0) {
+        if (currentTypes.length == 0 || !isSingleType()) {
             return builderFieldName;
         }
         return ALIAS_TO_CONCRETE_FIELD_NAME.getOrDefault(builderFieldName, builderFieldName);
@@ -384,20 +388,36 @@ public void onRemoval(ShardId shardId, Accountable accountable) {
                 mapperService.merge(type, new CompressedXContent(Strings.toString(PutMappingRequest.buildFromSimplifiedDef(type,
                     STRING_FIELD_NAME, "type=text",
                     STRING_FIELD_NAME_2, "type=keyword",
-                    STRING_ALIAS_FIELD_NAME, "type=alias,path=" + STRING_FIELD_NAME,
                     INT_FIELD_NAME, "type=integer",
-                    INT_ALIAS_FIELD_NAME, "type=alias,path=" + INT_FIELD_NAME,
                     INT_RANGE_FIELD_NAME, "type=integer_range",
                     DOUBLE_FIELD_NAME, "type=double",
                     BOOLEAN_FIELD_NAME, "type=boolean",
                     DATE_FIELD_NAME, "type=date",
-                    DATE_ALIAS_FIELD_NAME, "type=alias,path=" + DATE_FIELD_NAME,
                     DATE_RANGE_FIELD_NAME, "type=date_range",
                     OBJECT_FIELD_NAME, "type=object",
                     GEO_POINT_FIELD_NAME, "type=geo_point",
-                    GEO_POINT_ALIAS_FIELD_NAME, "type=alias,path=" + GEO_POINT_FIELD_NAME,
                     GEO_SHAPE_FIELD_NAME, "type=geo_shape"
                 ))), MapperService.MergeReason.MAPPING_UPDATE, false);
+
+                // Field aliases are only supported on indexes with a single type. If the index has multiple types, we
+                // still create fields with the same names as the alias fields, but with a concrete definition. This
+                // avoids the need for various test classes to check whether the index contains a single type.
+                if (idxSettings.isSingleType()) {
+                    mapperService.merge(type, new CompressedXContent(Strings.toString(PutMappingRequest.buildFromSimplifiedDef(type,
+                        STRING_ALIAS_FIELD_NAME, "type=alias,path=" + STRING_FIELD_NAME,
+                        INT_ALIAS_FIELD_NAME, "type=alias,path=" + INT_FIELD_NAME,
+                        DATE_ALIAS_FIELD_NAME, "type=alias,path=" + DATE_FIELD_NAME,
+                        GEO_POINT_ALIAS_FIELD_NAME, "type=alias,path=" + GEO_POINT_FIELD_NAME
+                    ))), MapperService.MergeReason.MAPPING_UPDATE, false);
+                } else {
+                    mapperService.merge(type, new CompressedXContent(Strings.toString(PutMappingRequest.buildFromSimplifiedDef(type,
+                        STRING_ALIAS_FIELD_NAME, "type=text",
+                        INT_ALIAS_FIELD_NAME, "type=integer",
+                        DATE_ALIAS_FIELD_NAME, "type=date",
+                        GEO_POINT_ALIAS_FIELD_NAME, "type=geo_point"
+                    ))), MapperService.MergeReason.MAPPING_UPDATE, false);
+                }
+
                 // also add mappings for two inner field in the object field
                 mapperService.merge(type, new CompressedXContent("{\"properties\":{\"" + OBJECT_FIELD_NAME + "\":{\"type\":\"object\","
                                 + "\"properties\":{\"" + DATE_FIELD_NAME + "\":{\"type\":\"date\"},\"" +