Skip to content

Commit

Permalink
Deprecate support for chained multi-fields. (elastic#41926)
Browse files Browse the repository at this point in the history 
We now issue a deprecation warning if a multi-field definition contains a
`[fields]` entry. This PR also simplifies the definition of
`MultiFieldParserContext`.

Addresses elastic#41267.
  • Loading branch information
@jtibshirani
jtibshirani authored and Gurkan Kaymak committed May 27, 2019
1 parent 9dc386b commit 2b76317
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 6 deletions.
8 changes: 4 additions & 4 deletions server/src/main/java/org/elasticsearch/index/mapper/Mapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,17 +136,17 @@ public Supplier<QueryShardContext> queryShardContextSupplier() {
protected Function<String, SimilarityProvider> similarityLookupService() { return similarityLookupService; }

public ParserContext createMultiFieldContext(ParserContext in) {
return new MultiFieldParserContext(in) {
@Override
public boolean isWithinMultiField() { return true; }
};
return new MultiFieldParserContext(in);
}

static class MultiFieldParserContext extends ParserContext {
MultiFieldParserContext(ParserContext in) {
super(in.type(), in.similarityLookupService(), in.mapperService(), in.typeParsers(),
in.indexVersionCreated(), in.queryShardContextSupplier());
}

@Override
public boolean isWithinMultiField() { return true; }
}

}
Expand Down
14 changes: 12 additions & 2 deletions server/src/main/java/org/elasticsearch/index/mapper/TypeParsers.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@

package org.elasticsearch.index.mapper;

import org.apache.logging.log4j.LogManager;
import org.apache.lucene.index.IndexOptions;
import org.elasticsearch.ElasticsearchParseException;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.common.time.DateFormatter;
import org.elasticsearch.common.xcontent.support.XContentMapValues;
import org.elasticsearch.index.analysis.AnalysisMode;
Expand All @@ -37,6 +39,7 @@
import static org.elasticsearch.common.xcontent.support.XContentMapValues.nodeStringValue;

public class TypeParsers {
private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(TypeParsers.class));

public static final String DOC_VALUES = "doc_values";
public static final String INDEX_OPTIONS_DOCS = "docs";
Expand Down Expand Up @@ -214,11 +217,18 @@ 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("fields")) {
if (parserContext.isWithinMultiField()) {
deprecationLogger.deprecatedAndMaybeLog("multifield_within_multifield", "At least one multi-field, [" + name + "], was " +
"encountered that itself contains a multi-field. Defining multi-fields within a multi-field is deprecated and will " +
"no longer be supported in 8.0. To resolve the issue, all instances of [fields] that occur within a [fields] block " +
"should be removed from the mappings, either by flattening the chained [fields] blocks into a single level, or " +
"switching to [copy_to] if appropriate.");
}

final Map<String, Object> multiFieldsPropNodes;
parserContext = parserContext.createMultiFieldContext(parserContext);

final Map<String, Object> multiFieldsPropNodes;
if (propNode instanceof List && ((List<?>) propNode).isEmpty()) {
multiFieldsPropNodes = Collections.emptyMap();
} else if (propNode instanceof Map) {
Expand Down
12 changes: 12 additions & 0 deletions server/src/test/java/org/elasticsearch/index/mapper/ExternalFieldMapperTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,12 @@ public void testExternalValuesWithMultifield() throws Exception {

assertThat(raw, notNullValue());
assertThat(raw.binaryValue(), is(new BytesRef("foo")));

assertWarnings("At least one multi-field, [field], was " +
"encountered that itself contains a multi-field. Defining multi-fields within a multi-field is deprecated and will " +
"no longer be supported in 8.0. To resolve the issue, all instances of [fields] that occur within a [fields] block " +
"should be removed from the mappings, either by flattening the chained [fields] blocks into a single level, or " +
"switching to [copy_to] if appropriate.");
}

public void testExternalValuesWithMultifieldTwoLevels() throws Exception {
Expand Down Expand Up @@ -234,5 +240,11 @@ public void testExternalValuesWithMultifieldTwoLevels() throws Exception {

assertThat(doc.rootDoc().getField("field.raw"), notNullValue());
assertThat(doc.rootDoc().getField("field.raw").stringValue(), is("foo"));

assertWarnings("At least one multi-field, [field], was " +
"encountered that itself contains a multi-field. Defining multi-fields within a multi-field is deprecated and will " +
"no longer be supported in 8.0. To resolve the issue, all instances of [fields] that occur within a [fields] block " +
"should be removed from the mappings, either by flattening the chained [fields] blocks into a single level, or " +
"switching to [copy_to] if appropriate.");
}
}
37 changes: 37 additions & 0 deletions server/src/test/java/org/elasticsearch/index/mapper/TypeParsersTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,11 @@
import org.apache.lucene.analysis.standard.StandardAnalyzer;
import org.elasticsearch.Version;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentHelper;
import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.index.analysis.AbstractTokenFilterFactory;
import org.elasticsearch.index.analysis.AnalysisMode;
Expand All @@ -36,6 +40,7 @@
import org.elasticsearch.index.analysis.TokenFilterFactory;
import org.elasticsearch.test.ESTestCase;

import java.io.IOException;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
Expand Down Expand Up @@ -157,6 +162,38 @@ public void testParseTextFieldCheckAnalyzerWithSearchAnalyzerAnalysisMode() {
TypeParsers.parseTextField(builder, "name", new HashMap<>(fieldNode), parserContext);
}

public void testMultiFieldWithinMultiField() throws IOException {
TextFieldMapper.Builder builder = new TextFieldMapper.Builder("textField");

XContentBuilder mapping = XContentFactory.jsonBuilder().startObject()
.field("type", "keyword")
.startObject("fields")
.startObject("sub-field")
.field("type", "keyword")
.startObject("fields")
.startObject("sub-sub-field")
.field("type", "keyword")
.endObject()
.endObject()
.endObject()
.endObject()
.endObject();

Map<String, Object> fieldNode = XContentHelper.convertToMap(
BytesReference.bytes(mapping), true, mapping.contentType()).v2();

Mapper.TypeParser typeParser = new KeywordFieldMapper.TypeParser();
Mapper.TypeParser.ParserContext parserContext = new Mapper.TypeParser.ParserContext("type",
null, null, type -> typeParser, Version.CURRENT, null);

TypeParsers.parseField(builder, "some-field", fieldNode, parserContext);
assertWarnings("At least one multi-field, [sub-field], was " +
"encountered that itself contains a multi-field. Defining multi-fields within a multi-field is deprecated and will " +
"no longer be supported in 8.0. To resolve the issue, all instances of [fields] that occur within a [fields] block " +
"should be removed from the mappings, either by flattening the chained [fields] blocks into a single level, or " +
"switching to [copy_to] if appropriate.");
}

private Analyzer createAnalyzerWithMode(String name, AnalysisMode mode) {
TokenFilterFactory tokenFilter = new AbstractTokenFilterFactory(indexSettings, name, Settings.EMPTY) {
@Override
Expand Down

0 comments on commit 2b76317

Please sign in to comment.