Drop "funny" functions building parsers (#50715) (#50814)

Replaces the "funny"
`Function<String, ConstructingObjectParser<T, Void>>` with a much
simpler `ConstructingObjectParser<T, String>`. This makes pretty much
all of our object parsers static.
This commit is contained in:
Nik Everett 2020-01-09 15:53:03 -05:00 committed by GitHub
parent f70e8f6ab5
commit ae40e22452
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 87 additions and 122 deletions

View File

@ -46,7 +46,7 @@ public class GetRollupCapsResponse {
if (token.equals(XContentParser.Token.FIELD_NAME)) {
String pattern = parser.currentName();
RollableIndexCaps cap = RollableIndexCaps.PARSER.apply(pattern).apply(parser, null);
RollableIndexCaps cap = RollableIndexCaps.PARSER.parse(parser, pattern);
jobs.put(pattern, cap);
}
}

View File

@ -46,7 +46,7 @@ public class GetRollupIndexCapsResponse {
if (token.equals(XContentParser.Token.FIELD_NAME)) {
String pattern = parser.currentName();
RollableIndexCaps cap = RollableIndexCaps.PARSER.apply(pattern).apply(parser, null);
RollableIndexCaps cap = RollableIndexCaps.PARSER.apply(parser, pattern);
jobs.put(pattern, cap);
}
}

View File

@ -28,9 +28,10 @@ import java.util.Collections;
import java.util.Comparator;
import java.util.List;
import java.util.Objects;
import java.util.function.Function;
import java.util.stream.Collectors;
import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg;
/**
* Represents the rollup capabilities of a non-rollup index. E.g. what values/aggregations
* were rolled up for this index, in what rollup jobs that data is stored and where those
@ -41,16 +42,15 @@ import java.util.stream.Collectors;
public class RollableIndexCaps implements ToXContentFragment {
private static final ParseField ROLLUP_JOBS = new ParseField("rollup_jobs");
public static final Function<String, ConstructingObjectParser<RollableIndexCaps, Void>> PARSER = indexName -> {
@SuppressWarnings("unchecked")
ConstructingObjectParser<RollableIndexCaps, Void> p
= new ConstructingObjectParser<>(indexName, true,
a -> new RollableIndexCaps(indexName, (List<RollupJobCaps>) a[0]));
p.declareObjectArray(ConstructingObjectParser.constructorArg(), RollupJobCaps.PARSER::apply,
ROLLUP_JOBS);
return p;
};
public static final ConstructingObjectParser<RollableIndexCaps, String> PARSER = new ConstructingObjectParser<>(
ROLLUP_JOBS.getPreferredName(), true, (Object[] args, String indexName) -> {
@SuppressWarnings("unchecked")
List<RollupJobCaps> caps = (List<RollupJobCaps>) args[0];
return new RollableIndexCaps(indexName, caps);
});
static {
PARSER.declareObjectArray(constructorArg(), (p, name) -> RollupJobCaps.PARSER.parse(p, null), ROLLUP_JOBS);
}
private final String indexName;
private final List<RollupJobCaps> jobCaps;

View File

@ -454,7 +454,7 @@ public class SearchModule {
registerAggregation(new AggregationSpec(ScriptedMetricAggregationBuilder.NAME, ScriptedMetricAggregationBuilder::new,
ScriptedMetricAggregationBuilder::parse).addResultReader(InternalScriptedMetric::new));
registerAggregation((new AggregationSpec(CompositeAggregationBuilder.NAME, CompositeAggregationBuilder::new,
CompositeAggregationBuilder::parse).addResultReader(InternalComposite::new)));
(name, p) -> CompositeAggregationBuilder.PARSER.parse(p, name)).addResultReader(InternalComposite::new)));
registerFromPlugin(plugins, SearchPlugin::getAggregations, this::registerAggregation);
}
@ -540,7 +540,7 @@ public class SearchModule {
BucketScriptPipelineAggregationBuilder.NAME,
BucketScriptPipelineAggregationBuilder::new,
BucketScriptPipelineAggregator::new,
BucketScriptPipelineAggregationBuilder::parse));
(name, p) -> BucketScriptPipelineAggregationBuilder.PARSER.parse(p, name)));
registerPipelineAggregation(new PipelineAggregationSpec(
BucketSelectorPipelineAggregationBuilder.NAME,
BucketSelectorPipelineAggregationBuilder::new,
@ -557,10 +557,10 @@ public class SearchModule {
SerialDiffPipelineAggregator::new,
SerialDiffPipelineAggregationBuilder::parse));
registerPipelineAggregation(new PipelineAggregationSpec(
MovFnPipelineAggregationBuilder.NAME,
MovFnPipelineAggregationBuilder::new,
MovFnPipelineAggregator::new,
MovFnPipelineAggregationBuilder::parse));
MovFnPipelineAggregationBuilder.NAME,
MovFnPipelineAggregationBuilder::new,
MovFnPipelineAggregator::new,
(name, p) -> MovFnPipelineAggregationBuilder.PARSER.parse(p, name)));
registerFromPlugin(plugins, SearchPlugin::getPipelineAggregations, this::registerPipelineAggregation);
}

View File

@ -24,7 +24,6 @@ import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.xcontent.ConstructingObjectParser;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.index.query.QueryShardContext;
import org.elasticsearch.search.aggregations.AbstractAggregationBuilder;
import org.elasticsearch.search.aggregations.AggregationBuilder;
@ -39,7 +38,8 @@ import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.function.Function;
import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg;
public class CompositeAggregationBuilder extends AbstractAggregationBuilder<CompositeAggregationBuilder> {
public static final String NAME = "composite";
@ -48,27 +48,17 @@ public class CompositeAggregationBuilder extends AbstractAggregationBuilder<Comp
public static final ParseField SIZE_FIELD_NAME = new ParseField("size");
public static final ParseField SOURCES_FIELD_NAME = new ParseField("sources");
private static final Function<String, ConstructingObjectParser<CompositeAggregationBuilder, Void>> PARSER = name -> {
@SuppressWarnings("unchecked")
ConstructingObjectParser<CompositeAggregationBuilder, Void> parser = new ConstructingObjectParser<>(NAME, a -> {
CompositeAggregationBuilder builder = new CompositeAggregationBuilder(name, (List<CompositeValuesSourceBuilder<?>>)a[0]);
if (a[1] != null) {
builder.size((Integer)a[1]);
}
if (a[2] != null) {
builder.aggregateAfter((Map<String, Object>)a[2]);
}
return builder;
});
parser.declareObjectArray(ConstructingObjectParser.constructorArg(),
public static final ConstructingObjectParser<CompositeAggregationBuilder, String> PARSER = new ConstructingObjectParser<>(
NAME, false, (args, name) -> {
@SuppressWarnings("unchecked")
List<CompositeValuesSourceBuilder<?>> sources = (List<CompositeValuesSourceBuilder<?>>) args[0];
return new CompositeAggregationBuilder(name, sources);
});
static {
PARSER.declareObjectArray(constructorArg(),
(p, c) -> CompositeValuesSourceParserHelper.fromXContent(p), SOURCES_FIELD_NAME);
parser.declareInt(ConstructingObjectParser.optionalConstructorArg(), SIZE_FIELD_NAME);
parser.declareObject(ConstructingObjectParser.optionalConstructorArg(), (p, context) -> p.map(), AFTER_FIELD_NAME);
return parser;
};
public static CompositeAggregationBuilder parse(String aggregationName, XContentParser parser) throws IOException {
return PARSER.apply(aggregationName).parse(parser, null);
PARSER.declareInt(CompositeAggregationBuilder::size, SIZE_FIELD_NAME);
PARSER.declareObject(CompositeAggregationBuilder::aggregateAfter, (p, context) -> p.map(), AFTER_FIELD_NAME);
}
private List<CompositeValuesSourceBuilder<?>> sources;

View File

@ -37,8 +37,8 @@ import java.util.Map;
import java.util.Map.Entry;
import java.util.Objects;
import java.util.TreeMap;
import java.util.function.Function;
import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg;
import static org.elasticsearch.search.aggregations.pipeline.PipelineAggregator.Parser.BUCKETS_PATH;
import static org.elasticsearch.search.aggregations.pipeline.PipelineAggregator.Parser.FORMAT;
import static org.elasticsearch.search.aggregations.pipeline.PipelineAggregator.Parser.GAP_POLICY;
@ -51,31 +51,24 @@ public class BucketScriptPipelineAggregationBuilder extends AbstractPipelineAggr
private String format = null;
private GapPolicy gapPolicy = GapPolicy.SKIP;
private static final Function<String, ConstructingObjectParser<BucketScriptPipelineAggregationBuilder, Void>> PARSER
= name -> {
public static final ConstructingObjectParser<BucketScriptPipelineAggregationBuilder, String> PARSER = new ConstructingObjectParser<>(
NAME, false, (args, name) -> {
@SuppressWarnings("unchecked")
Map<String, String> bucketsPathsMap = (Map<String, String>) args[0];
return new BucketScriptPipelineAggregationBuilder(name, bucketsPathsMap, (Script) args[1]);
});
static {
PARSER.declareField(constructorArg(), BucketScriptPipelineAggregationBuilder::extractBucketPath,
BUCKETS_PATH_FIELD, ObjectParser.ValueType.OBJECT_ARRAY_OR_STRING);
Script.declareScript(PARSER, constructorArg());
@SuppressWarnings("unchecked")
ConstructingObjectParser<BucketScriptPipelineAggregationBuilder, Void> parser = new ConstructingObjectParser<>(
BucketScriptPipelineAggregationBuilder.NAME,
false,
o -> new BucketScriptPipelineAggregationBuilder(name, (Map<String, String>) o[0], (Script) o[1]));
parser.declareField(ConstructingObjectParser.constructorArg()
, BucketScriptPipelineAggregationBuilder::extractBucketPath
, BUCKETS_PATH_FIELD
, ObjectParser.ValueType.OBJECT_ARRAY_OR_STRING);
parser.declareField(ConstructingObjectParser.constructorArg(),
(p, c) -> Script.parse(p), Script.SCRIPT_PARSE_FIELD, ObjectParser.ValueType.OBJECT_OR_STRING);
parser.declareString(BucketScriptPipelineAggregationBuilder::format, FORMAT);
parser.declareField(BucketScriptPipelineAggregationBuilder::gapPolicy, p -> {
PARSER.declareString(BucketScriptPipelineAggregationBuilder::format, FORMAT);
PARSER.declareField(BucketScriptPipelineAggregationBuilder::gapPolicy, p -> {
if (p.currentToken() == XContentParser.Token.VALUE_STRING) {
return GapPolicy.parse(p.text().toLowerCase(Locale.ROOT), p.getTokenLocation());
}
throw new IllegalArgumentException("Unsupported token [" + p.currentToken() + "]");
}, GAP_POLICY, ObjectParser.ValueType.STRING);
return parser;
};
@ -205,10 +198,6 @@ public class BucketScriptPipelineAggregationBuilder extends AbstractPipelineAggr
return builder;
}
public static BucketScriptPipelineAggregationBuilder parse(String aggName, XContentParser parser) {
return PARSER.apply(aggName).apply(parser, null);
}
@Override
protected boolean overrideBucketsPath() {
return true;

View File

@ -40,8 +40,8 @@ import java.util.Collection;
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
import java.util.function.Function;
import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg;
import static org.elasticsearch.search.aggregations.pipeline.PipelineAggregator.Parser.BUCKETS_PATH;
import static org.elasticsearch.search.aggregations.pipeline.PipelineAggregator.Parser.FORMAT;
import static org.elasticsearch.search.aggregations.pipeline.PipelineAggregator.Parser.GAP_POLICY;
@ -58,29 +58,23 @@ public class MovFnPipelineAggregationBuilder extends AbstractPipelineAggregation
private int window;
private int shift;
private static final Function<String, ConstructingObjectParser<MovFnPipelineAggregationBuilder, Void>> PARSER
= name -> {
ConstructingObjectParser<MovFnPipelineAggregationBuilder, Void> parser = new ConstructingObjectParser<>(
MovFnPipelineAggregationBuilder.NAME,
false,
o -> new MovFnPipelineAggregationBuilder(name, (String) o[0], (Script) o[1], (int)o[2]));
parser.declareString(ConstructingObjectParser.constructorArg(), BUCKETS_PATH_FIELD);
parser.declareField(ConstructingObjectParser.constructorArg(),
public static final ConstructingObjectParser<MovFnPipelineAggregationBuilder, String> PARSER = new ConstructingObjectParser<>(
NAME, false,
(args, name) -> new MovFnPipelineAggregationBuilder(name, (String) args[0], (Script) args[1], (int)args[2]));
static {
PARSER.declareString(constructorArg(), BUCKETS_PATH_FIELD);
PARSER.declareField(constructorArg(),
(p, c) -> Script.parse(p), Script.SCRIPT_PARSE_FIELD, ObjectParser.ValueType.OBJECT_OR_STRING);
parser.declareInt(ConstructingObjectParser.constructorArg(), WINDOW);
PARSER.declareInt(constructorArg(), WINDOW);
parser.declareInt(MovFnPipelineAggregationBuilder::setShift, SHIFT);
parser.declareString(MovFnPipelineAggregationBuilder::format, FORMAT);
parser.declareField(MovFnPipelineAggregationBuilder::gapPolicy, p -> {
PARSER.declareInt(MovFnPipelineAggregationBuilder::setShift, SHIFT);
PARSER.declareString(MovFnPipelineAggregationBuilder::format, FORMAT);
PARSER.declareField(MovFnPipelineAggregationBuilder::gapPolicy, p -> {
if (p.currentToken() == XContentParser.Token.VALUE_STRING) {
return GapPolicy.parse(p.text().toLowerCase(Locale.ROOT), p.getTokenLocation());
}
throw new IllegalArgumentException("Unsupported token [" + p.currentToken() + "]");
}, GAP_POLICY, ObjectParser.ValueType.STRING);
return parser;
};
@ -212,10 +206,6 @@ public class MovFnPipelineAggregationBuilder extends AbstractPipelineAggregation
return builder;
}
public static MovFnPipelineAggregationBuilder parse(String aggName, XContentParser parser) {
return PARSER.apply(aggName).apply(parser, null);
}
/**
* Used for serialization testing, since pipeline aggs serialize themselves as a named object but are parsed
* as a regular object with the name passed in.
@ -228,7 +218,7 @@ public class MovFnPipelineAggregationBuilder extends AbstractPipelineAggregation
String aggName = parser.currentName();
parser.nextToken(); // "moving_fn"
parser.nextToken(); // start_object
return PARSER.apply(aggName).apply(parser, null);
return PARSER.apply(parser, aggName);
}
}

View File

@ -645,7 +645,7 @@ public class BucketScriptIT extends ESIntegTestCase {
.endObject()
.endObject();
BucketScriptPipelineAggregationBuilder bucketScriptAgg =
BucketScriptPipelineAggregationBuilder.parse("seriesArithmetic", createParser(content));
BucketScriptPipelineAggregationBuilder.PARSER.parse(createParser(content), "seriesArithmetic");
SearchResponse response = client()
.prepareSearch("idx", "idx_unmapped")
@ -690,7 +690,7 @@ public class BucketScriptIT extends ESIntegTestCase {
.endObject()
.endObject();
BucketScriptPipelineAggregationBuilder bucketScriptAgg =
BucketScriptPipelineAggregationBuilder.parse("seriesArithmetic", createParser(content));
BucketScriptPipelineAggregationBuilder.PARSER.parse(createParser(content), "seriesArithmetic");
SearchResponse response = client()
.prepareSearch("idx", "idx_unmapped")
@ -747,7 +747,7 @@ public class BucketScriptIT extends ESIntegTestCase {
.endObject()
.endObject();
BucketScriptPipelineAggregationBuilder bucketScriptAgg =
BucketScriptPipelineAggregationBuilder.parse("seriesArithmetic", createParser(content));
BucketScriptPipelineAggregationBuilder.PARSER.parse(createParser(content), "seriesArithmetic");
SearchResponse response = client()
.prepareSearch("idx", "idx_unmapped")

View File

@ -71,7 +71,8 @@ public class BucketScriptTests extends BasePipelineAggregationTestCase<BucketScr
.field("lang", "expression")
.endObject()
.endObject();
BucketScriptPipelineAggregationBuilder builder1 = BucketScriptPipelineAggregationBuilder.parse("count", createParser(content));
BucketScriptPipelineAggregationBuilder builder1 = BucketScriptPipelineAggregationBuilder.PARSER.parse(
createParser(content), "count");
assertEquals(builder1.getBucketsPaths().length , 1);
assertEquals(builder1.getBucketsPaths()[0], "_count");
@ -86,7 +87,8 @@ public class BucketScriptTests extends BasePipelineAggregationTestCase<BucketScr
.field("lang", "expression")
.endObject()
.endObject();
BucketScriptPipelineAggregationBuilder builder2 = BucketScriptPipelineAggregationBuilder.parse("count", createParser(content));
BucketScriptPipelineAggregationBuilder builder2 = BucketScriptPipelineAggregationBuilder.PARSER.parse(
createParser(content), "count");
assertEquals(builder2.getBucketsPaths().length , 2);
assertEquals(builder2.getBucketsPaths()[0], "_count1");
assertEquals(builder2.getBucketsPaths()[1], "_count2");
@ -99,7 +101,8 @@ public class BucketScriptTests extends BasePipelineAggregationTestCase<BucketScr
.field("lang", "expression")
.endObject()
.endObject();
BucketScriptPipelineAggregationBuilder builder3 = BucketScriptPipelineAggregationBuilder.parse("count", createParser(content));
BucketScriptPipelineAggregationBuilder builder3 = BucketScriptPipelineAggregationBuilder.PARSER.parse(
createParser(content), "count");
assertEquals(builder3.getBucketsPaths().length , 2);
assertEquals(builder3.getBucketsPaths()[0], "_count1");
assertEquals(builder3.getBucketsPaths()[1], "_count2");

View File

@ -52,7 +52,7 @@ public class AnalyticsPlugin extends Plugin implements SearchPlugin, ActionPlugi
CumulativeCardinalityPipelineAggregationBuilder.NAME,
CumulativeCardinalityPipelineAggregationBuilder::new,
CumulativeCardinalityPipelineAggregator::new,
CumulativeCardinalityPipelineAggregationBuilder::parse)
(name, p) -> CumulativeCardinalityPipelineAggregationBuilder.PARSER.parse(p, name))
);
}

View File

@ -9,7 +9,6 @@ import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.xcontent.ConstructingObjectParser;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.license.LicenseUtils;
import org.elasticsearch.search.DocValueFormat;
import org.elasticsearch.search.aggregations.AggregationBuilder;
@ -18,36 +17,41 @@ import org.elasticsearch.search.aggregations.PipelineAggregationBuilder;
import org.elasticsearch.search.aggregations.pipeline.AbstractPipelineAggregationBuilder;
import org.elasticsearch.search.aggregations.pipeline.BucketMetricsParser;
import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator;
import org.elasticsearch.xpack.core.XPackField;
import org.elasticsearch.xpack.analytics.AnalyticsPlugin;
import org.elasticsearch.xpack.core.XPackField;
import java.io.IOException;
import java.util.Collection;
import java.util.Map;
import java.util.Objects;
import java.util.function.Function;
import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg;
import static org.elasticsearch.search.aggregations.pipeline.PipelineAggregator.Parser.BUCKETS_PATH;
import static org.elasticsearch.search.aggregations.pipeline.PipelineAggregator.Parser.FORMAT;
public class CumulativeCardinalityPipelineAggregationBuilder
extends AbstractPipelineAggregationBuilder<CumulativeCardinalityPipelineAggregationBuilder> {
extends AbstractPipelineAggregationBuilder<CumulativeCardinalityPipelineAggregationBuilder> {
public static final String NAME = "cumulative_cardinality";
public static final ConstructingObjectParser<CumulativeCardinalityPipelineAggregationBuilder, String> PARSER =
new ConstructingObjectParser<>(NAME, false, (args, name) -> {
if (AnalyticsPlugin.getLicenseState().isAnalyticsAllowed() == false) {
throw LicenseUtils.newComplianceException(XPackField.ANALYTICS);
}
// Increment usage here since it is a good boundary between internal and external, and should correlate 1:1 with
// usage and not internal instantiations
AnalyticsPlugin.cumulativeCardUsage.incrementAndGet();
return new CumulativeCardinalityPipelineAggregationBuilder(name, (String) args[0]);
});
static {
PARSER.declareString(constructorArg(), BUCKETS_PATH_FIELD);
PARSER.declareString(CumulativeCardinalityPipelineAggregationBuilder::format, FORMAT);
}
private String format;
private static final Function<String, ConstructingObjectParser<CumulativeCardinalityPipelineAggregationBuilder, Void>> PARSER
= name -> {
ConstructingObjectParser<CumulativeCardinalityPipelineAggregationBuilder, Void> parser = new ConstructingObjectParser<>(
CumulativeCardinalityPipelineAggregationBuilder.NAME,
false,
o -> new CumulativeCardinalityPipelineAggregationBuilder(name, (String) o[0]));
parser.declareString(ConstructingObjectParser.constructorArg(), BUCKETS_PATH_FIELD);
parser.declareString(CumulativeCardinalityPipelineAggregationBuilder::format, FORMAT);
return parser;
};
public CumulativeCardinalityPipelineAggregationBuilder(String name, String bucketsPath) {
super(name, NAME, new String[] { bucketsPath });
}
@ -115,17 +119,6 @@ public class CumulativeCardinalityPipelineAggregationBuilder
return builder;
}
public static CumulativeCardinalityPipelineAggregationBuilder parse(String aggName, XContentParser parser) {
if (AnalyticsPlugin.getLicenseState().isAnalyticsAllowed() == false) {
throw LicenseUtils.newComplianceException(XPackField.ANALYTICS);
}
// Increment usage here since it is a good boundary between internal and external, and should correlate 1:1 with
// usage and not internal instantiations
AnalyticsPlugin.cumulativeCardUsage.incrementAndGet();
return PARSER.apply(aggName).apply(parser, null);
}
@Override
public int hashCode() {
return Objects.hash(super.hashCode(), format);

View File

@ -237,7 +237,7 @@ public class Pivot {
config.toCompositeAggXContent(builder, forChangeDetection);
XContentParser parser = builder.generator().contentType().xContent().createParser(NamedXContentRegistry.EMPTY,
LoggingDeprecationHandler.INSTANCE, BytesReference.bytes(builder).streamInput());
compositeAggregation = CompositeAggregationBuilder.parse(COMPOSITE_AGGREGATION_NAME, parser);
compositeAggregation = CompositeAggregationBuilder.PARSER.parse(parser, COMPOSITE_AGGREGATION_NAME);
} catch (IOException e) {
throw new RuntimeException(TransformMessages.TRANSFORM_PIVOT_FAILED_TO_CREATE_COMPOSITE_AGGREGATION, e);
}