throw exception if a copy_to is within a multi field

Copy to within multi field is ignored from 2.0 on, see elastic#10802.
Instead of just ignoring it, we should throw an exception if this
is found in the mapping when a mapping is added. For already
existing indices we should at least log a warning.

related to elastic#14946

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

@@ -19,7 +19,6 @@
 
 package org.elasticsearch.index.mapper;
 
-import com.google.common.collect.ImmutableMap;
 import org.elasticsearch.Version;
 import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.ParseFieldMatcher;
@@ -134,6 +133,24 @@ public Version indexVersionCreated() {
             public ParseFieldMatcher parseFieldMatcher() {
                 return parseFieldMatcher;
             }
+
+            public boolean isWithinMultiField() { return false; }
+
+            protected Map<String, TypeParser> typeParsers() { return typeParsers; }
+
+            public ParserContext createMultiFieldContext(ParserContext in) {
+                return new MultiFieldParserContext(in) {
+                    @Override
+                    public boolean isWithinMultiField() { return true; }
+                };
+            }
+
+            class MultiFieldParserContext extends ParserContext {
+                MultiFieldParserContext(ParserContext in) {
+                    super(in.type(), in.analysisService, in.similarityLookupService(), in.mapperService(), in.typeParsers(), in.indexVersionCreated(), in.parseFieldMatcher());
+                }
+            }
+
         }
 
         Mapper.Builder<?,?> parse(String name, Map<String, Object> node, ParserContext parserContext) throws MapperParsingException;

plugins/mapper-attachments/src/test/java/org/elasticsearch/mapper/attachments/MapperTestUtils.java → core/src/main/java/org/elasticsearch/index/mapper/core/MapperTestUtils.java

@@ -17,7 +17,7 @@
  * under the License.
  */
 
-package org.elasticsearch.mapper.attachments;
+package org.elasticsearch.index.mapper.core;
 
 import org.elasticsearch.Version;
 import org.elasticsearch.cluster.metadata.IndexMetaData;
@@ -31,6 +31,7 @@
 import org.elasticsearch.index.IndexNameModule;
 import org.elasticsearch.index.analysis.AnalysisModule;
 import org.elasticsearch.index.analysis.AnalysisService;
+import org.elasticsearch.index.mapper.Mapper;
 import org.elasticsearch.index.mapper.MapperService;
 import org.elasticsearch.index.settings.IndexSettingsModule;
 import org.elasticsearch.index.similarity.SimilarityLookupService;
@@ -44,37 +45,50 @@
 public class MapperTestUtils {
 
     public static MapperService newMapperService(Path tempDir, Settings indexSettings) {
-        return newMapperService(new Index("test"), Settings.builder()
-                .put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)
-                .put("path.home", tempDir)
-                .put(indexSettings)
-                .build());
+        return newMapperService(tempDir, indexSettings, Version.CURRENT);
+
+    }
+
+    public static MapperService newMapperService(Path tempDir, Settings indexSettings, Version version) {
+        return newMapperService(tempDir, indexSettings, null, null, version);
+
+    }
+
+    public static MapperService newMapperService(Path tempDir, Settings indexSettings, String contentType, Mapper.TypeParser typeParser) {
+        return newMapperService(tempDir, indexSettings, contentType, typeParser, Version.CURRENT);
     }
 
-    private static MapperService newMapperService(Index index, Settings indexSettings) {
+    private static MapperService newMapperService(Path tempDir, Settings indexSettings, String contentType, Mapper.TypeParser typeParser, Version version) {
         IndicesModule indicesModule = new IndicesModule();
-        indicesModule.registerMapper(AttachmentMapper.CONTENT_TYPE, new AttachmentMapper.TypeParser());
+        if (contentType != null && typeParser != null) {
+            indicesModule.registerMapper(contentType, typeParser);
+        }
+        Settings settings = Settings.builder()
+            .put(IndexMetaData.SETTING_VERSION_CREATED, version)
+            .put("path.home", tempDir)
+            .put(indexSettings)
+            .build();
         MapperRegistry mapperRegistry = indicesModule.getMapperRegistry();
-        return new MapperService(index,
-                                 indexSettings, 
-                                 newAnalysisService(indexSettings),
-                                 newSimilarityLookupService(indexSettings), 
-                                 null,
-                                 mapperRegistry);
+        return new MapperService(new Index("test"),
+            settings,
+            newAnalysisService(settings),
+            newSimilarityLookupService(settings),
+            null,
+            mapperRegistry);
     }
 
-    private  static AnalysisService newAnalysisService(Settings indexSettings) {
+    private static AnalysisService newAnalysisService(Settings indexSettings) {
         Injector parentInjector = new ModulesBuilder().add(new SettingsModule(indexSettings), new EnvironmentModule(new Environment(indexSettings))).createInjector();
         Index index = new Index("test");
         Injector injector = new ModulesBuilder().add(
-                new IndexSettingsModule(index, indexSettings),
-                new IndexNameModule(index),
-                new AnalysisModule(indexSettings, parentInjector.getInstance(IndicesAnalysisService.class))).createChildInjector(parentInjector);
+            new IndexSettingsModule(index, indexSettings),
+            new IndexNameModule(index),
+            new AnalysisModule(indexSettings, parentInjector.getInstance(IndicesAnalysisService.class))).createChildInjector(parentInjector);
 
         return injector.getInstance(AnalysisService.class);
     }
 
-    private  static SimilarityLookupService newSimilarityLookupService(Settings indexSettings) {
+    private static SimilarityLookupService newSimilarityLookupService(Settings indexSettings) {
         return new SimilarityLookupService(new Index("test"), indexSettings);
     }
 }

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

@@ -25,6 +25,7 @@
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.joda.FormatDateTimeFormatter;
 import org.elasticsearch.common.joda.Joda;
+import org.elasticsearch.common.logging.ESLoggerFactory;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.settings.loader.SettingsLoader;
 import org.elasticsearch.index.analysis.NamedAnalyzer;
@@ -184,11 +185,12 @@ public static void parseNumberField(NumberFieldMapper.Builder builder, String na
     public static void parseField(FieldMapper.Builder builder, String name, Map<String, Object> fieldNode, Mapper.TypeParser.ParserContext parserContext) {
         NamedAnalyzer indexAnalyzer = builder.fieldType().indexAnalyzer();
         NamedAnalyzer searchAnalyzer = builder.fieldType().searchAnalyzer();
+        Version indexVersionCreated = parserContext.indexVersionCreated();
         for (Iterator<Map.Entry<String, Object>> iterator = fieldNode.entrySet().iterator(); iterator.hasNext();) {
             Map.Entry<String, Object> entry = iterator.next();
             final String propName = Strings.toUnderscoreCase(entry.getKey());
             final Object propNode = entry.getValue();
-            if (propName.equals("index_name") && parserContext.indexVersionCreated().before(Version.V_2_0_0_beta1)) {
+            if (propName.equals("index_name") && indexVersionCreated.before(Version.V_2_0_0_beta1)) {
                 builder.indexName(propNode.toString());
                 iterator.remove();
             } else if (propName.equals("store")) {
@@ -242,7 +244,7 @@ public static void parseField(FieldMapper.Builder builder, String name, Map<Stri
                 iterator.remove();
             } else if (propName.equals("omit_term_freq_and_positions")) {
                 final IndexOptions op = nodeBooleanValue(propNode) ? IndexOptions.DOCS : IndexOptions.DOCS_AND_FREQS_AND_POSITIONS;
-                if (parserContext.indexVersionCreated().onOrAfter(Version.V_1_0_0_RC2)) {
+                if (indexVersionCreated.onOrAfter(Version.V_1_0_0_RC2)) {
                     throw new ElasticsearchParseException("'omit_term_freq_and_positions' is not supported anymore - use ['index_options' : 'docs']  instead");
                 }
                 // deprecated option for BW compat
@@ -252,8 +254,8 @@ public static void parseField(FieldMapper.Builder builder, String name, Map<Stri
                 builder.indexOptions(nodeIndexOptionValue(propNode));
                 iterator.remove();
             } else if (propName.equals("analyzer") || // for backcompat, reading old indexes, remove for v3.0
-                       propName.equals("index_analyzer") && parserContext.indexVersionCreated().before(Version.V_2_0_0_beta1)) {
-                
+                       propName.equals("index_analyzer") && indexVersionCreated.before(Version.V_2_0_0_beta1)) {
+
                 NamedAnalyzer analyzer = parserContext.analysisService().analyzer(propNode.toString());
                 if (analyzer == null) {
                     throw new MapperParsingException("analyzer [" + propNode.toString() + "] not found for field [" + name + "]");
@@ -270,10 +272,10 @@ public static void parseField(FieldMapper.Builder builder, String name, Map<Stri
             } else if (propName.equals("include_in_all")) {
                 builder.includeInAll(nodeBooleanValue(propNode));
                 iterator.remove();
-            } else if (propName.equals("postings_format") && parserContext.indexVersionCreated().before(Version.V_2_0_0_beta1)) {
+            } else if (propName.equals("postings_format") && indexVersionCreated.before(Version.V_2_0_0_beta1)) {
                 // ignore for old indexes
                 iterator.remove();
-            } else if (propName.equals("doc_values_format") && parserContext.indexVersionCreated().before(Version.V_2_0_0_beta1)) {
+            } else if (propName.equals("doc_values_format") && indexVersionCreated.before(Version.V_2_0_0_beta1)) {
                 // ignore for old indexes
                 iterator.remove();
             } else if (propName.equals("similarity")) {
@@ -284,14 +286,23 @@ public static void parseField(FieldMapper.Builder builder, String name, Map<Stri
                 builder.fieldDataSettings(settings);
                 iterator.remove();
             } else if (propName.equals("copy_to")) {
+                if (parserContext.isWithinMultiField()) {
+                    if (indexVersionCreated.after(Version.V_2_1_0) ||
+                        (indexVersionCreated.after(Version.V_2_0_1) && indexVersionCreated.before(Version.V_2_1_0))) {
+                        throw new MapperParsingException("copy_to in multi fields is not allowed. Found the copy_to in field [" + name + "] which is within a multi field.");
+                    } else {
+                        ESLoggerFactory.getLogger("mapping [" + parserContext.type() + "]").warn("Found a copy_to in field [" + name + "] which is within a multi field. This feature has been removed and the copy_to will be ignored.");
+                        // we still parse this, otherwise the message will only appear once and the copy_to removed. After that it will appear again. Better to have it always.
+                    }
+                }
                 parseCopyFields(propNode, builder);
                 iterator.remove();
             }
         }
 
         if (indexAnalyzer == null) {
             if (searchAnalyzer != null) {
-                // If the index was created before 2.0 then we are trying to upgrade the mappings so use the default indexAnalyzer 
+                // If the index was created before 2.0 then we are trying to upgrade the mappings so use the default indexAnalyzer
                 // instead of throwing an exception so the user is able to upgrade
                 if (parserContext.indexVersionCreated().before(Version.V_2_0_0_beta1)) {
                     indexAnalyzer = parserContext.analysisService().defaultIndexAnalyzer();
@@ -307,6 +318,7 @@ public static void parseField(FieldMapper.Builder builder, String name, Map<Stri
     }
 
     public static boolean parseMultiField(FieldMapper.Builder builder, String name, Mapper.TypeParser.ParserContext parserContext, String propName, Object propNode) {
+        parserContext = parserContext.createMultiFieldContext(parserContext);
         if (propName.equals("path") && parserContext.indexVersionCreated().before(Version.V_2_0_0_beta1)) {
             builder.multiFieldPathType(parsePathType(name, propNode.toString()));
             return true;

core/src/test/java/org/elasticsearch/index/mapper/core/MultiFieldCopyToMapperTests.java

@@ -0,0 +1,74 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+
+package org.elasticsearch.index.mapper.core;
+
+import org.elasticsearch.Version;
+import org.elasticsearch.common.compress.CompressedXContent;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.xcontent.XContentBuilder;
+import org.elasticsearch.index.mapper.MapperParsingException;
+import org.elasticsearch.index.mapper.MapperService;
+import org.elasticsearch.test.ESTestCase;
+
+import java.io.IOException;
+
+import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
+import static org.elasticsearch.test.ESTokenStreamTestCase.randomVersion;
+import static org.hamcrest.core.IsEqual.equalTo;
+
+public class MultiFieldCopyToMapperTests extends ESTestCase {
+
+    public void testCopyToWithinMultiFieldWithRandomVersion() throws IOException {
+        XContentBuilder mapping = jsonBuilder();
+        mapping.startObject()
+            .startObject("type")
+            .startObject("properties")
+            .startObject("a")
+            .field("type", "string")
+            .endObject()
+            .startObject("b")
+            .field("type", "string")
+            .startObject("fields")
+            .startObject("c")
+            .field("type", "string")
+            .field("copy_to", "a")
+            .endObject()
+            .endObject()
+            .endObject()
+            .endObject()
+            .endObject()
+            .endObject();
+        Version indexVersion = randomVersion();
+        MapperService mapperService = MapperTestUtils.newMapperService(createTempDir(), Settings.EMPTY, indexVersion);
+        if (indexVersion.after(Version.V_2_1_0) ||
+            (indexVersion.after(Version.V_2_0_1) && indexVersion.before(Version.V_2_1_0))) {
+            try {
+                mapperService.parse("type", new CompressedXContent(mapping.string()), true);
+                fail("Parsing should throw an exception becasue the mapping contains a copy_to in a multi field");
+            } catch (MapperParsingException e) {
+                assertThat(e.getMessage(), equalTo("copy_to in multi fields is not allowed. Found the copy_to in field [c] which is within a multi field."));
+            }
+        } else {
+            // this should just work
+            mapperService.parse("type", new CompressedXContent(mapping.string()), true);
+        }
+    }
+}

docs/reference/migration/migrate_2_0/mapping.asciidoc

@@ -429,3 +429,11 @@ to use the old default of 0. This was done to prevent phrase queries from
 matching across different values of the same term unexpectedly. Specifically,
 100 was chosen to cause phrase queries with slops up to 99 to match only within
 a single value of a field.
+
+==== copy_to and multi fields
+
+A <<copy-to,copy_to>> within a <<multi-fields,multi field>> is ignored from version 2.0 on. With any version after
+2.1 or 2.0.1 creating a mapping that has a copy_to within a multi field will result 
+in an exception.
+
+

plugins/mapper-attachments/src/test/java/org/elasticsearch/mapper/attachments/DateAttachmentMapperTests.java

@@ -22,6 +22,7 @@
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.index.mapper.DocumentMapper;
 import org.elasticsearch.index.mapper.DocumentMapperParser;
+import org.elasticsearch.index.mapper.core.MapperTestUtils;
 import org.elasticsearch.index.mapper.core.StringFieldMapper;
 import org.junit.Before;
 
@@ -37,7 +38,7 @@ public class DateAttachmentMapperTests extends AttachmentUnitTestCase {
 
     @Before
     public void setupMapperParser() throws Exception {
-        mapperParser = MapperTestUtils.newMapperService(createTempDir(), Settings.EMPTY).documentMapperParser();
+        mapperParser = MapperTestUtils.newMapperService(createTempDir(), Settings.EMPTY, AttachmentMapper.CONTENT_TYPE, new AttachmentMapper.TypeParser()).documentMapperParser();
     }
 
     public void testSimpleMappings() throws Exception {

plugins/mapper-attachments/src/test/java/org/elasticsearch/mapper/attachments/EncryptedDocMapperTests.java

@@ -25,7 +25,7 @@
 import org.elasticsearch.index.mapper.DocumentMapperParser;
 import org.elasticsearch.index.mapper.MapperParsingException;
 import org.elasticsearch.index.mapper.ParseContext;
-import org.elasticsearch.mapper.attachments.AttachmentMapper;
+import org.elasticsearch.index.mapper.core.MapperTestUtils;
 
 import java.io.IOException;
 
@@ -42,7 +42,7 @@
 public class EncryptedDocMapperTests extends AttachmentUnitTestCase {
 
     public void testMultipleDocsEncryptedLast() throws IOException {
-        DocumentMapperParser mapperParser = MapperTestUtils.newMapperService(createTempDir(), Settings.EMPTY).documentMapperParser();
+        DocumentMapperParser mapperParser = MapperTestUtils.newMapperService(createTempDir(), Settings.EMPTY, AttachmentMapper.CONTENT_TYPE, new AttachmentMapper.TypeParser()).documentMapperParser();
 
         String mapping = copyToStringFromClasspath("/org/elasticsearch/index/mapper/attachment/test/unit/encrypted/test-mapping.json");
         DocumentMapper docMapper = mapperParser.parse(mapping);
@@ -72,7 +72,7 @@ public void testMultipleDocsEncryptedLast() throws IOException {
     }
 
     public void testMultipleDocsEncryptedFirst() throws IOException {
-        DocumentMapperParser mapperParser = MapperTestUtils.newMapperService(createTempDir(), Settings.EMPTY).documentMapperParser();
+        DocumentMapperParser mapperParser = MapperTestUtils.newMapperService(createTempDir(), Settings.EMPTY, AttachmentMapper.CONTENT_TYPE, new AttachmentMapper.TypeParser()).documentMapperParser();
         String mapping = copyToStringFromClasspath("/org/elasticsearch/index/mapper/attachment/test/unit/encrypted/test-mapping.json");
         DocumentMapper docMapper = mapperParser.parse(mapping);
         byte[] html = copyToBytesFromClasspath("/org/elasticsearch/index/mapper/attachment/test/sample-files/htmlWithValidDateMeta.html");
@@ -105,7 +105,7 @@ public void testMultipleDocsEncryptedNotIgnoringErrors() throws IOException {
             DocumentMapperParser mapperParser = MapperTestUtils.newMapperService(createTempDir(),
                 Settings.builder()
                     .put("index.mapping.attachment.ignore_errors", false)
-                    .build()).documentMapperParser();
+                    .build(), AttachmentMapper.CONTENT_TYPE, new AttachmentMapper.TypeParser()).documentMapperParser();
 
             String mapping = copyToStringFromClasspath("/org/elasticsearch/index/mapper/attachment/test/unit/encrypted/test-mapping.json");
             DocumentMapper docMapper = mapperParser.parse(mapping);

plugins/mapper-attachments/src/test/java/org/elasticsearch/mapper/attachments/LanguageDetectionAttachmentMapperTests.java

@@ -24,8 +24,8 @@
 import org.elasticsearch.index.mapper.DocumentMapper;
 import org.elasticsearch.index.mapper.DocumentMapperParser;
 import org.elasticsearch.index.mapper.ParseContext;
+import org.elasticsearch.index.mapper.core.MapperTestUtils;
 import org.elasticsearch.index.mapper.core.StringFieldMapper;
-import org.elasticsearch.mapper.attachments.AttachmentMapper;
 import org.junit.Before;
 
 import java.io.IOException;
@@ -52,7 +52,7 @@ public void setupMapperParser(boolean langDetect) throws IOException {
         DocumentMapperParser mapperParser = MapperTestUtils.newMapperService(createTempDir(),
             Settings.settingsBuilder()
                 .put("index.mapping.attachment.detect_language", langDetect)
-            .build()).documentMapperParser();
+                .build(), AttachmentMapper.CONTENT_TYPE, new AttachmentMapper.TypeParser()).documentMapperParser();
         String mapping = copyToStringFromClasspath("/org/elasticsearch/index/mapper/attachment/test/unit/language/language-mapping.json");
         docMapper = mapperParser.parse(mapping);