Skip to content

Internal: make sure ParseField is always used in combination with parse flags #11859

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ protected ShardValidateQueryResponse shardOperation(ShardValidateQueryRequest re
DefaultSearchContext searchContext = new DefaultSearchContext(0,
new ShardSearchLocalRequest(request.types(), request.nowInMillis(), request.filteringAliases()),
null, searcher, indexService, indexShard,
scriptService, pageCacheRecycler, bigArrays, threadPool.estimatedTimeInMillisCounter()
scriptService, pageCacheRecycler, bigArrays, threadPool.estimatedTimeInMillisCounter(), parseFieldMatcher
);
SearchContext.setCurrent(searchContext);
try {
Expand All @@ -187,10 +187,7 @@ protected ShardValidateQueryResponse shardOperation(ShardValidateQueryRequest re
} catch (QueryParsingException e) {
valid = false;
error = e.getDetailedMessage();
} catch (AssertionError e) {
valid = false;
error = e.getMessage();
} catch (IOException e) {
} catch (AssertionError|IOException e) {
valid = false;
error = e.getMessage();
} finally {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ protected ShardExistsResponse shardOperation(ShardExistsRequest request) {
SearchContext context = new DefaultSearchContext(0,
new ShardSearchLocalRequest(request.types(), request.nowInMillis(), request.filteringAliases()),
shardTarget, indexShard.acquireSearcher("exists"), indexService, indexShard,
scriptService, pageCacheRecycler, bigArrays, threadPool.estimatedTimeInMillisCounter());
scriptService, pageCacheRecycler, bigArrays, threadPool.estimatedTimeInMillisCounter(), parseFieldMatcher);
SearchContext.setCurrent(context);

try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ protected ExplainResponse shardOperation(ExplainRequest request, ShardId shardId
0, new ShardSearchLocalRequest(new String[]{request.type()}, request.nowInMillis, request.filteringAlias()),
null, result.searcher(), indexService, indexShard,
scriptService, pageCacheRecycler,
bigArrays, threadPool.estimatedTimeInMillisCounter()
bigArrays, threadPool.estimatedTimeInMillisCounter(), parseFieldMatcher
);
SearchContext.setCurrent(context);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.elasticsearch.action.support.IndicesOptions;
import org.elasticsearch.client.Requests;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.ParseFieldMatcher;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.bytes.BytesReference;
Expand Down Expand Up @@ -239,7 +240,7 @@ public SearchRequest searchType(SearchType searchType) {
* "query_then_fetch"/"queryThenFetch", and "query_and_fetch"/"queryAndFetch".
*/
public SearchRequest searchType(String searchType) {
return searchType(SearchType.fromString(searchType));
return searchType(SearchType.fromString(searchType, ParseFieldMatcher.EMPTY));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package org.elasticsearch.action.search;

import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.ParseFieldMatcher;

/**
* Search type represent the manner at which the search operation is executed.
Expand Down Expand Up @@ -108,7 +109,7 @@ public static SearchType fromId(byte id) {
* one of "dfs_query_then_fetch"/"dfsQueryThenFetch", "dfs_query_and_fetch"/"dfsQueryAndFetch",
* "query_then_fetch"/"queryThenFetch", "query_and_fetch"/"queryAndFetch", and "scan".
*/
public static SearchType fromString(String searchType) {
public static SearchType fromString(String searchType, ParseFieldMatcher parseFieldMatcher) {
if (searchType == null) {
return SearchType.DEFAULT;
}
Expand All @@ -122,7 +123,7 @@ public static SearchType fromString(String searchType) {
return SearchType.QUERY_AND_FETCH;
} else if ("scan".equals(searchType)) {
return SearchType.SCAN;
} else if (COUNT_VALUE.match(searchType)) {
} else if (parseFieldMatcher.match(searchType, COUNT_VALUE)) {
return SearchType.COUNT;
} else {
throw new IllegalArgumentException("No search type for [" + searchType + "]");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package org.elasticsearch.action.support;

import org.elasticsearch.action.*;
import org.elasticsearch.common.ParseFieldMatcher;
import org.elasticsearch.common.component.AbstractComponent;
import org.elasticsearch.common.logging.ESLogger;
import org.elasticsearch.common.settings.Settings;
Expand All @@ -37,9 +38,11 @@ public abstract class TransportAction<Request extends ActionRequest, Response ex
protected final ThreadPool threadPool;
protected final String actionName;
private final ActionFilter[] filters;
protected final ParseFieldMatcher parseFieldMatcher;

protected TransportAction(Settings settings, String actionName, ThreadPool threadPool, ActionFilters actionFilters) {
super(settings);
this.parseFieldMatcher = new ParseFieldMatcher(settings);
this.actionName = actionName;
this.filters = actionFilters.filters();
this.threadPool = threadPool;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.elasticsearch.action.index.IndexRequest;
import org.elasticsearch.action.support.single.instance.InstanceShardOperationRequest;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.ParseFieldMatcher;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.io.stream.StreamInput;
Expand Down Expand Up @@ -291,13 +292,13 @@ public String scriptLang() {
public UpdateRequest addScriptParam(String name, Object value) {
Script script = script();
if (script == null) {
HashMap<String, Object> scriptParams = new HashMap<String, Object>();
HashMap<String, Object> scriptParams = new HashMap<>();
scriptParams.put(name, value);
updateOrCreateScript(null, null, null, scriptParams);
} else {
Map<String, Object> scriptParams = script.getParams();
if (scriptParams == null) {
scriptParams = new HashMap<String, Object>();
scriptParams = new HashMap<>();
scriptParams.put(name, value);
updateOrCreateScript(null, null, null, scriptParams);
} else {
Expand Down Expand Up @@ -648,7 +649,8 @@ public UpdateRequest source(BytesReference source) throws Exception {
if (token == XContentParser.Token.FIELD_NAME) {
currentFieldName = parser.currentName();
} else if ("script".equals(currentFieldName) && token == XContentParser.Token.START_OBJECT) {
script = Script.parse(parser);
//here we don't have settings available, unable to throw strict deprecation exceptions
script = Script.parse(parser, ParseFieldMatcher.EMPTY);
} else if ("params".equals(currentFieldName)) {
scriptParams = parser.map();
} else if ("scripted_upsert".equals(currentFieldName)) {
Expand All @@ -666,7 +668,8 @@ public UpdateRequest source(BytesReference source) throws Exception {
} else if ("detect_noop".equals(currentFieldName)) {
detectNoop(parser.booleanValue());
} else {
scriptParameterParser.token(currentFieldName, token, parser);
//here we don't have settings available, unable to throw deprecation exceptions
scriptParameterParser.token(currentFieldName, token, parser, ParseFieldMatcher.EMPTY);
}
}
// Don't have a script using the new API so see if it is specified with the old API
Expand Down
14 changes: 6 additions & 8 deletions core/src/main/java/org/elasticsearch/common/ParseField.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,18 @@
import java.util.HashSet;

/**
* Holds a field that can be found in a request while parsing and its different variants, which may be deprecated.
*/
public class ParseField {
private final String camelCaseName;
private final String underscoreName;
private final String[] deprecatedNames;
private String allReplacedWith = null;

public static final EnumSet<Flag> EMPTY_FLAGS = EnumSet.noneOf(Flag.class);
static final EnumSet<Flag> EMPTY_FLAGS = EnumSet.noneOf(Flag.class);
static final EnumSet<Flag> STRICT_FLAGS = EnumSet.of(Flag.STRICT);

public static enum Flag {
enum Flag {
STRICT
}

Expand All @@ -47,7 +49,7 @@ public ParseField(String value, String... deprecatedNames) {
set.add(Strings.toCamelCase(depName));
set.add(Strings.toUnderscoreCase(depName));
}
this.deprecatedNames = set.toArray(new String[0]);
this.deprecatedNames = set.toArray(new String[set.size()]);
}
}

Expand Down Expand Up @@ -78,11 +80,7 @@ public ParseField withAllDeprecated(String allReplacedWith) {
return parseField;
}

public boolean match(String currentFieldName) {
return match(currentFieldName, EMPTY_FLAGS);
}

public boolean match(String currentFieldName, EnumSet<Flag> flags) {
boolean match(String currentFieldName, EnumSet<Flag> flags) {
if (allReplacedWith == null && (currentFieldName.equals(camelCaseName) || currentFieldName.equals(underscoreName))) {
return true;
}
Expand Down
61 changes: 61 additions & 0 deletions core/src/main/java/org/elasticsearch/common/ParseFieldMatcher.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/*
* 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.common;

import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.index.query.IndexQueryParserService;

import java.util.EnumSet;

/**
* Matcher to use in combination with {@link ParseField} while parsing requests. Matches a {@link ParseField}
* against a field name and throw deprecation exception depending on the current value of the {@link IndexQueryParserService#PARSE_STRICT} setting.
*/
public class ParseFieldMatcher {

public static final ParseFieldMatcher EMPTY = new ParseFieldMatcher(ParseField.EMPTY_FLAGS);
public static final ParseFieldMatcher STRICT = new ParseFieldMatcher(ParseField.STRICT_FLAGS);

private final EnumSet<ParseField.Flag> parseFlags;

public ParseFieldMatcher(Settings settings) {
if (settings.getAsBoolean(IndexQueryParserService.PARSE_STRICT, false)) {
this.parseFlags = EnumSet.of(ParseField.Flag.STRICT);
} else {
this.parseFlags = ParseField.EMPTY_FLAGS;
}
}

public ParseFieldMatcher(EnumSet<ParseField.Flag> parseFlags) {
this.parseFlags = parseFlags;
}

/**
* Matches a {@link ParseField} against a field name, and throws deprecation exception depending on the current
* value of the {@link IndexQueryParserService#PARSE_STRICT} setting.
* @param fieldName the field name found in the request while parsing
* @param parseField the parse field that we are looking for
* @throws IllegalArgumentException whenever we are in strict mode and the request contained a deprecated field
* @return true whenever the parse field that we are looking for was found, false otherwise
*/
public boolean match(String fieldName, ParseField parseField) {
return parseField.match(fieldName, parseFlags);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@

import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Maps;

import org.elasticsearch.Version;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.ParseFieldMatcher;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.collect.MapBuilder;
import org.elasticsearch.common.collect.Tuple;
Expand All @@ -38,35 +38,10 @@
import org.elasticsearch.index.AbstractIndexComponent;
import org.elasticsearch.index.Index;
import org.elasticsearch.index.analysis.AnalysisService;
import org.elasticsearch.index.mapper.core.BinaryFieldMapper;
import org.elasticsearch.index.mapper.core.BooleanFieldMapper;
import org.elasticsearch.index.mapper.core.ByteFieldMapper;
import org.elasticsearch.index.mapper.core.CompletionFieldMapper;
import org.elasticsearch.index.mapper.core.DateFieldMapper;
import org.elasticsearch.index.mapper.core.DoubleFieldMapper;
import org.elasticsearch.index.mapper.core.FloatFieldMapper;
import org.elasticsearch.index.mapper.core.IntegerFieldMapper;
import org.elasticsearch.index.mapper.core.LongFieldMapper;
import org.elasticsearch.index.mapper.core.Murmur3FieldMapper;
import org.elasticsearch.index.mapper.core.ShortFieldMapper;
import org.elasticsearch.index.mapper.core.StringFieldMapper;
import org.elasticsearch.index.mapper.core.TokenCountFieldMapper;
import org.elasticsearch.index.mapper.core.TypeParsers;
import org.elasticsearch.index.mapper.core.*;
import org.elasticsearch.index.mapper.geo.GeoPointFieldMapper;
import org.elasticsearch.index.mapper.geo.GeoShapeFieldMapper;
import org.elasticsearch.index.mapper.internal.AllFieldMapper;
import org.elasticsearch.index.mapper.internal.FieldNamesFieldMapper;
import org.elasticsearch.index.mapper.internal.IdFieldMapper;
import org.elasticsearch.index.mapper.internal.IndexFieldMapper;
import org.elasticsearch.index.mapper.internal.ParentFieldMapper;
import org.elasticsearch.index.mapper.internal.RoutingFieldMapper;
import org.elasticsearch.index.mapper.internal.SizeFieldMapper;
import org.elasticsearch.index.mapper.internal.SourceFieldMapper;
import org.elasticsearch.index.mapper.internal.TTLFieldMapper;
import org.elasticsearch.index.mapper.internal.TimestampFieldMapper;
import org.elasticsearch.index.mapper.internal.TypeFieldMapper;
import org.elasticsearch.index.mapper.internal.UidFieldMapper;
import org.elasticsearch.index.mapper.internal.VersionFieldMapper;
import org.elasticsearch.index.mapper.internal.*;
import org.elasticsearch.index.mapper.ip.IpFieldMapper;
import org.elasticsearch.index.mapper.object.ObjectMapper;
import org.elasticsearch.index.mapper.object.RootObjectMapper;
Expand Down Expand Up @@ -96,13 +71,15 @@ public class DocumentMapperParser extends AbstractIndexComponent {

private final Object typeParsersMutex = new Object();
private final Version indexVersionCreated;
private final ParseFieldMatcher parseFieldMatcher;

private volatile ImmutableMap<String, Mapper.TypeParser> typeParsers;
private volatile ImmutableMap<String, Mapper.TypeParser> rootTypeParsers;

public DocumentMapperParser(Index index, @IndexSettings Settings indexSettings, MapperService mapperService, AnalysisService analysisService,
SimilarityLookupService similarityLookupService, ScriptService scriptService) {
super(index, indexSettings);
this.parseFieldMatcher = new ParseFieldMatcher(indexSettings);
this.mapperService = mapperService;
this.analysisService = analysisService;
this.similarityLookupService = similarityLookupService;
Expand Down Expand Up @@ -168,7 +145,7 @@ public void putRootTypeParser(String type, Mapper.TypeParser typeParser) {
}

public Mapper.TypeParser.ParserContext parserContext() {
return new Mapper.TypeParser.ParserContext(analysisService, similarityLookupService, mapperService, typeParsers, indexVersionCreated);
return new Mapper.TypeParser.ParserContext(analysisService, similarityLookupService, mapperService, typeParsers, indexVersionCreated, parseFieldMatcher);
}

public DocumentMapper parse(String source) throws MapperParsingException {
Expand Down Expand Up @@ -296,7 +273,7 @@ private static String getRemainingFields(Map<String, ?> map) {
}

private void parseTransform(DocumentMapper.Builder docBuilder, Map<String, Object> transformConfig, Version indexVersionCreated) {
Script script = Script.parse(transformConfig, true);
Script script = Script.parse(transformConfig, true, parseFieldMatcher);
if (script != null) {
docBuilder.transform(scriptService, script);
}
Expand Down
12 changes: 10 additions & 2 deletions core/src/main/java/org/elasticsearch/index/mapper/Mapper.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import com.google.common.collect.ImmutableMap;
import org.elasticsearch.Version;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.ParseFieldMatcher;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.ToXContent;
Expand Down Expand Up @@ -92,14 +93,17 @@ class ParserContext {

private final Version indexVersionCreated;

private final ParseFieldMatcher parseFieldMatcher;

public ParserContext(AnalysisService analysisService, SimilarityLookupService similarityLookupService,
MapperService mapperService,
ImmutableMap<String, TypeParser> typeParsers, Version indexVersionCreated) {
MapperService mapperService, ImmutableMap<String, TypeParser> typeParsers,
Version indexVersionCreated, ParseFieldMatcher parseFieldMatcher) {
this.analysisService = analysisService;
this.similarityLookupService = similarityLookupService;
this.mapperService = mapperService;
this.typeParsers = typeParsers;
this.indexVersionCreated = indexVersionCreated;
this.parseFieldMatcher = parseFieldMatcher;
}

public AnalysisService analysisService() {
Expand All @@ -121,6 +125,10 @@ public TypeParser typeParser(String type) {
public Version indexVersionCreated() {
return indexVersionCreated;
}

public ParseFieldMatcher parseFieldMatcher() {
return parseFieldMatcher;
}
}

Mapper.Builder<?,?> parse(String name, Map<String, Object> node, ParserContext parserContext) throws MapperParsingException;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@

import com.carrotsearch.hppc.ObjectArrayList;
import org.apache.lucene.document.Field;
import org.apache.lucene.document.FieldType;
import org.apache.lucene.index.DocValuesType;
import org.apache.lucene.index.IndexOptions;
import org.apache.lucene.store.ByteArrayDataOutput;
import org.apache.lucene.util.BytesRef;
Expand All @@ -35,7 +33,6 @@
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.compress.CompressorFactory;
import org.elasticsearch.common.compress.NotXContentException;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.CollectionUtils;
import org.elasticsearch.common.xcontent.XContentParser;
Expand Down Expand Up @@ -98,7 +95,7 @@ public Mapper.Builder parse(String name, Map<String, Object> node, ParserContext
Map.Entry<String, Object> entry = iterator.next();
String fieldName = entry.getKey();
if (parserContext.indexVersionCreated().before(Version.V_2_0_0) &&
(COMPRESS.match(fieldName) || COMPRESS_THRESHOLD.match(fieldName))) {
(parserContext.parseFieldMatcher().match(fieldName, COMPRESS) || parserContext.parseFieldMatcher().match(fieldName, COMPRESS_THRESHOLD))) {
iterator.remove();
}
}
Expand Down
Loading