Skip to content

Commit 0afa265

Browse files
droberts195droberts195
authored
[ML] Consistent pattern for strict/lenient parser names (#32399)
Previously we had two patterns for naming of strict and lenient parsers. Some classes had CONFIG_PARSER and METADATA_PARSER, and used an enum to pass the parser type to nested parsers. Other classes had STRICT_PARSER and LENIENT_PARSER and used ternary operators to pass the parser type to nested parsers. This change makes all ML classes use the second of the patterns described above.
1 parent 63a0436 commit 0afa265

36 files changed

+321
-418
lines changed

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/XPackClientPlugin.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -373,7 +373,7 @@ public List<NamedXContentRegistry.Entry> getNamedXContent() {
373373
return Arrays.asList(
374374
// ML - Custom metadata
375375
new NamedXContentRegistry.Entry(MetaData.Custom.class, new ParseField("ml"),
376-
parser -> MlMetadata.METADATA_PARSER.parse(parser, null).build()),
376+
parser -> MlMetadata.LENIENT_PARSER.parse(parser, null).build()),
377377
// ML - Persistent action requests
378378
new NamedXContentRegistry.Entry(PersistentTaskParams.class, new ParseField(StartDatafeedAction.TASK_NAME),
379379
StartDatafeedAction.DatafeedParams::fromXContent),

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/MlMetadata.java

+4-4
Original file line numberDiff line numberDiff line change
@@ -61,12 +61,12 @@ public class MlMetadata implements XPackPlugin.XPackMetaDataCustom {
6161

6262
public static final MlMetadata EMPTY_METADATA = new MlMetadata(Collections.emptySortedMap(), Collections.emptySortedMap());
6363
// This parser follows the pattern that metadata is parsed leniently (to allow for enhancements)
64-
public static final ObjectParser<Builder, Void> METADATA_PARSER = new ObjectParser<>("ml_metadata", true, Builder::new);
64+
public static final ObjectParser<Builder, Void> LENIENT_PARSER = new ObjectParser<>("ml_metadata", true, Builder::new);
6565

6666
static {
67-
METADATA_PARSER.declareObjectArray(Builder::putJobs, (p, c) -> Job.METADATA_PARSER.apply(p, c).build(), JOBS_FIELD);
68-
METADATA_PARSER.declareObjectArray(Builder::putDatafeeds,
69-
(p, c) -> DatafeedConfig.METADATA_PARSER.apply(p, c).build(), DATAFEEDS_FIELD);
67+
LENIENT_PARSER.declareObjectArray(Builder::putJobs, (p, c) -> Job.LENIENT_PARSER.apply(p, c).build(), JOBS_FIELD);
68+
LENIENT_PARSER.declareObjectArray(Builder::putDatafeeds,
69+
(p, c) -> DatafeedConfig.LENIENT_PARSER.apply(p, c).build(), DATAFEEDS_FIELD);
7070
}
7171

7272
private final SortedMap<String, Job> jobs;

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/MlParserType.java

-19
This file was deleted.

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/PutDatafeedAction.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public Response newResponse() {
3939
public static class Request extends AcknowledgedRequest<Request> implements ToXContentObject {
4040

4141
public static Request parseRequest(String datafeedId, XContentParser parser) {
42-
DatafeedConfig.Builder datafeed = DatafeedConfig.CONFIG_PARSER.apply(parser, null);
42+
DatafeedConfig.Builder datafeed = DatafeedConfig.STRICT_PARSER.apply(parser, null);
4343
datafeed.setId(datafeedId);
4444
return new Request(datafeed.build());
4545
}

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/PutJobAction.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ public Response newResponse() {
4242
public static class Request extends AcknowledgedRequest<Request> implements ToXContentObject {
4343

4444
public static Request parseRequest(String jobId, XContentParser parser) {
45-
Job.Builder jobBuilder = Job.CONFIG_PARSER.apply(parser, null);
45+
Job.Builder jobBuilder = Job.STRICT_PARSER.apply(parser, null);
4646
if (jobBuilder.getId() == null) {
4747
jobBuilder.setId(jobId);
4848
} else if (!Strings.isNullOrEmpty(jobId) && !jobId.equals(jobBuilder.getId())) {

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/ValidateDetectorAction.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public static class Request extends ActionRequest implements ToXContentObject {
4848
private Detector detector;
4949

5050
public static Request parseRequest(XContentParser parser) {
51-
Detector detector = Detector.CONFIG_PARSER.apply(parser, null).build();
51+
Detector detector = Detector.STRICT_PARSER.apply(parser, null).build();
5252
return new Request(detector);
5353
}
5454

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/ValidateJobConfigAction.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public static class Request extends ActionRequest {
4949
private Job job;
5050

5151
public static Request parseRequest(XContentParser parser) {
52-
Job.Builder job = Job.CONFIG_PARSER.apply(parser, null);
52+
Job.Builder job = Job.STRICT_PARSER.apply(parser, null);
5353
// When jobs are PUT their ID must be supplied in the URL - assume this will
5454
// be valid unless an invalid job ID is specified in the JSON to be validated
5555
job.setId(job.getId() != null ? job.getId() : "ok");

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/ChunkingConfig.java

+21-29
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,10 @@
1616
import org.elasticsearch.common.xcontent.ToXContentObject;
1717
import org.elasticsearch.common.xcontent.XContentBuilder;
1818
import org.elasticsearch.common.xcontent.XContentParser;
19-
import org.elasticsearch.xpack.core.ml.MlParserType;
2019
import org.elasticsearch.xpack.core.ml.utils.ExceptionsHelper;
2120

2221
import java.io.IOException;
23-
import java.util.EnumMap;
2422
import java.util.Locale;
25-
import java.util.Map;
2623
import java.util.Objects;
2724

2825
/**
@@ -34,32 +31,27 @@ public class ChunkingConfig implements ToXContentObject, Writeable {
3431
public static final ParseField TIME_SPAN_FIELD = new ParseField("time_span");
3532

3633
// These parsers follow the pattern that metadata is parsed leniently (to allow for enhancements), whilst config is parsed strictly
37-
public static final ConstructingObjectParser<ChunkingConfig, Void> METADATA_PARSER = new ConstructingObjectParser<>(
38-
"chunking_config", true, a -> new ChunkingConfig((Mode) a[0], (TimeValue) a[1]));
39-
public static final ConstructingObjectParser<ChunkingConfig, Void> CONFIG_PARSER = new ConstructingObjectParser<>(
40-
"chunking_config", false, a -> new ChunkingConfig((Mode) a[0], (TimeValue) a[1]));
41-
public static final Map<MlParserType, ConstructingObjectParser<ChunkingConfig, Void>> PARSERS =
42-
new EnumMap<>(MlParserType.class);
43-
44-
static {
45-
PARSERS.put(MlParserType.METADATA, METADATA_PARSER);
46-
PARSERS.put(MlParserType.CONFIG, CONFIG_PARSER);
47-
for (MlParserType parserType : MlParserType.values()) {
48-
ConstructingObjectParser<ChunkingConfig, Void> parser = PARSERS.get(parserType);
49-
assert parser != null;
50-
parser.declareField(ConstructingObjectParser.constructorArg(), p -> {
51-
if (p.currentToken() == XContentParser.Token.VALUE_STRING) {
52-
return Mode.fromString(p.text());
53-
}
54-
throw new IllegalArgumentException("Unsupported token [" + p.currentToken() + "]");
55-
}, MODE_FIELD, ValueType.STRING);
56-
parser.declareField(ConstructingObjectParser.optionalConstructorArg(), p -> {
57-
if (p.currentToken() == XContentParser.Token.VALUE_STRING) {
58-
return TimeValue.parseTimeValue(p.text(), TIME_SPAN_FIELD.getPreferredName());
59-
}
60-
throw new IllegalArgumentException("Unsupported token [" + p.currentToken() + "]");
61-
}, TIME_SPAN_FIELD, ValueType.STRING);
62-
}
34+
public static final ConstructingObjectParser<ChunkingConfig, Void> LENIENT_PARSER = createParser(true);
35+
public static final ConstructingObjectParser<ChunkingConfig, Void> STRICT_PARSER = createParser(false);
36+
37+
private static ConstructingObjectParser<ChunkingConfig, Void> createParser(boolean ignoreUnknownFields) {
38+
ConstructingObjectParser<ChunkingConfig, Void> parser = new ConstructingObjectParser<>(
39+
"chunking_config", ignoreUnknownFields, a -> new ChunkingConfig((Mode) a[0], (TimeValue) a[1]));
40+
41+
parser.declareField(ConstructingObjectParser.constructorArg(), p -> {
42+
if (p.currentToken() == XContentParser.Token.VALUE_STRING) {
43+
return Mode.fromString(p.text());
44+
}
45+
throw new IllegalArgumentException("Unsupported token [" + p.currentToken() + "]");
46+
}, MODE_FIELD, ValueType.STRING);
47+
parser.declareField(ConstructingObjectParser.optionalConstructorArg(), p -> {
48+
if (p.currentToken() == XContentParser.Token.VALUE_STRING) {
49+
return TimeValue.parseTimeValue(p.text(), TIME_SPAN_FIELD.getPreferredName());
50+
}
51+
throw new IllegalArgumentException("Unsupported token [" + p.currentToken() + "]");
52+
}, TIME_SPAN_FIELD, ValueType.STRING);
53+
54+
return parser;
6355
}
6456

6557
private final Mode mode;

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedConfig.java

+40-40
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import org.elasticsearch.search.aggregations.metrics.max.MaxAggregationBuilder;
2626
import org.elasticsearch.search.aggregations.support.ValuesSourceAggregationBuilder;
2727
import org.elasticsearch.search.builder.SearchSourceBuilder;
28-
import org.elasticsearch.xpack.core.ml.MlParserType;
2928
import org.elasticsearch.xpack.core.ml.datafeed.extractor.ExtractorUtils;
3029
import org.elasticsearch.xpack.core.ml.job.config.Job;
3130
import org.elasticsearch.xpack.core.ml.job.messages.Messages;
@@ -38,7 +37,6 @@
3837
import java.util.ArrayList;
3938
import java.util.Collections;
4039
import java.util.Comparator;
41-
import java.util.EnumMap;
4240
import java.util.List;
4341
import java.util.Map;
4442
import java.util.Objects;
@@ -87,44 +85,46 @@ public class DatafeedConfig extends AbstractDiffable<DatafeedConfig> implements
8785
public static final ParseField HEADERS = new ParseField("headers");
8886

8987
// These parsers follow the pattern that metadata is parsed leniently (to allow for enhancements), whilst config is parsed strictly
90-
public static final ObjectParser<Builder, Void> METADATA_PARSER = new ObjectParser<>("datafeed_config", true, Builder::new);
91-
public static final ObjectParser<Builder, Void> CONFIG_PARSER = new ObjectParser<>("datafeed_config", false, Builder::new);
92-
public static final Map<MlParserType, ObjectParser<Builder, Void>> PARSERS = new EnumMap<>(MlParserType.class);
93-
94-
static {
95-
PARSERS.put(MlParserType.METADATA, METADATA_PARSER);
96-
PARSERS.put(MlParserType.CONFIG, CONFIG_PARSER);
97-
for (MlParserType parserType : MlParserType.values()) {
98-
ObjectParser<Builder, Void> parser = PARSERS.get(parserType);
99-
assert parser != null;
100-
parser.declareString(Builder::setId, ID);
101-
parser.declareString(Builder::setJobId, Job.ID);
102-
parser.declareStringArray(Builder::setIndices, INDEXES);
103-
parser.declareStringArray(Builder::setIndices, INDICES);
104-
parser.declareStringArray(Builder::setTypes, TYPES);
105-
parser.declareString((builder, val) ->
106-
builder.setQueryDelay(TimeValue.parseTimeValue(val, QUERY_DELAY.getPreferredName())), QUERY_DELAY);
107-
parser.declareString((builder, val) ->
108-
builder.setFrequency(TimeValue.parseTimeValue(val, FREQUENCY.getPreferredName())), FREQUENCY);
109-
parser.declareObject(Builder::setQuery, (p, c) -> AbstractQueryBuilder.parseInnerQueryBuilder(p), QUERY);
110-
parser.declareObject(Builder::setAggregations, (p, c) -> AggregatorFactories.parseAggregators(p), AGGREGATIONS);
111-
parser.declareObject(Builder::setAggregations, (p, c) -> AggregatorFactories.parseAggregators(p), AGGS);
112-
parser.declareObject(Builder::setScriptFields, (p, c) -> {
113-
List<SearchSourceBuilder.ScriptField> parsedScriptFields = new ArrayList<>();
114-
while (p.nextToken() != XContentParser.Token.END_OBJECT) {
115-
parsedScriptFields.add(new SearchSourceBuilder.ScriptField(p));
116-
}
117-
parsedScriptFields.sort(Comparator.comparing(SearchSourceBuilder.ScriptField::fieldName));
118-
return parsedScriptFields;
119-
}, SCRIPT_FIELDS);
120-
parser.declareInt(Builder::setScrollSize, SCROLL_SIZE);
121-
// TODO this is to read former _source field. Remove in v7.0.0
122-
parser.declareBoolean((builder, value) -> {}, SOURCE);
123-
parser.declareObject(Builder::setChunkingConfig, ChunkingConfig.PARSERS.get(parserType), CHUNKING_CONFIG);
124-
}
125-
// Headers are only parsed by the metadata parser, so headers supplied in the _body_ of a REST request will be rejected.
126-
// (For config headers are explicitly transferred from the auth headers by code in the put/update datafeed actions.)
127-
METADATA_PARSER.declareObject(Builder::setHeaders, (p, c) -> p.mapStrings(), HEADERS);
88+
public static final ObjectParser<Builder, Void> LENIENT_PARSER = createParser(true);
89+
public static final ObjectParser<Builder, Void> STRICT_PARSER = createParser(false);
90+
91+
private static ObjectParser<Builder, Void> createParser(boolean ignoreUnknownFields) {
92+
ObjectParser<Builder, Void> parser = new ObjectParser<>("datafeed_config", ignoreUnknownFields, Builder::new);
93+
94+
parser.declareString(Builder::setId, ID);
95+
parser.declareString(Builder::setJobId, Job.ID);
96+
parser.declareStringArray(Builder::setIndices, INDEXES);
97+
parser.declareStringArray(Builder::setIndices, INDICES);
98+
parser.declareStringArray(Builder::setTypes, TYPES);
99+
parser.declareString((builder, val) ->
100+
builder.setQueryDelay(TimeValue.parseTimeValue(val, QUERY_DELAY.getPreferredName())), QUERY_DELAY);
101+
parser.declareString((builder, val) ->
102+
builder.setFrequency(TimeValue.parseTimeValue(val, FREQUENCY.getPreferredName())), FREQUENCY);
103+
parser.declareObject(Builder::setQuery, (p, c) -> AbstractQueryBuilder.parseInnerQueryBuilder(p), QUERY);
104+
parser.declareObject(Builder::setAggregations, (p, c) -> AggregatorFactories.parseAggregators(p), AGGREGATIONS);
105+
parser.declareObject(Builder::setAggregations, (p, c) -> AggregatorFactories.parseAggregators(p), AGGS);
106+
parser.declareObject(Builder::setScriptFields, (p, c) -> {
107+
List<SearchSourceBuilder.ScriptField> parsedScriptFields = new ArrayList<>();
108+
while (p.nextToken() != XContentParser.Token.END_OBJECT) {
109+
parsedScriptFields.add(new SearchSourceBuilder.ScriptField(p));
110+
}
111+
parsedScriptFields.sort(Comparator.comparing(SearchSourceBuilder.ScriptField::fieldName));
112+
return parsedScriptFields;
113+
}, SCRIPT_FIELDS);
114+
parser.declareInt(Builder::setScrollSize, SCROLL_SIZE);
115+
// TODO this is to read former _source field. Remove in v7.0.0
116+
parser.declareBoolean((builder, value) -> {
117+
}, SOURCE);
118+
parser.declareObject(Builder::setChunkingConfig, ignoreUnknownFields ? ChunkingConfig.LENIENT_PARSER : ChunkingConfig.STRICT_PARSER,
119+
CHUNKING_CONFIG);
120+
121+
if (ignoreUnknownFields) {
122+
// Headers are not parsed by the strict (config) parser, so headers supplied in the _body_ of a REST request will be rejected.
123+
// (For config, headers are explicitly transferred from the auth headers by code in the put/update datafeed actions.)
124+
parser.declareObject(Builder::setHeaders, (p, c) -> p.mapStrings(), HEADERS);
125+
}
126+
127+
return parser;
128128
}
129129

130130
private final String id;

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedUpdate.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ public class DatafeedUpdate implements Writeable, ToXContentObject {
6868
return parsedScriptFields;
6969
}, DatafeedConfig.SCRIPT_FIELDS);
7070
PARSER.declareInt(Builder::setScrollSize, DatafeedConfig.SCROLL_SIZE);
71-
PARSER.declareObject(Builder::setChunkingConfig, ChunkingConfig.CONFIG_PARSER, DatafeedConfig.CHUNKING_CONFIG);
71+
PARSER.declareObject(Builder::setChunkingConfig, ChunkingConfig.STRICT_PARSER, DatafeedConfig.CHUNKING_CONFIG);
7272
}
7373

7474
private final String id;

0 commit comments

Comments
 (0)