Teach ObjectParser a happy pattern (#50691) (#50710)

We *very* commonly have object with ctors like:
```
public Foo(String name)
```

And then declare a bunch of setters on the object. Every aggregation
works like this, for example. This change teaches `ObjectParser` how to
build these aggregations all on its own, without any help. This'll make
it much cleaner to parse aggs, and, probably, a bunch of other things.
It'll let us remove lots of wrapping. I've used this new power for the
`avg` aggregation just to prove that it works outside of a unit test.
This commit is contained in:
Nik Everett 2020-01-07 11:57:41 -05:00 committed by GitHub
parent 8009b07ccb
commit deb0991667
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 44 additions and 26 deletions

View File

@ -32,8 +32,10 @@ import java.util.Map;
import java.util.function.BiConsumer; import java.util.function.BiConsumer;
import java.util.function.BiFunction; import java.util.function.BiFunction;
import java.util.function.Consumer; import java.util.function.Consumer;
import java.util.function.Function;
import java.util.function.Supplier; import java.util.function.Supplier;
import static java.util.Objects.requireNonNull;
import static org.elasticsearch.common.xcontent.XContentParser.Token.START_ARRAY; import static org.elasticsearch.common.xcontent.XContentParser.Token.START_ARRAY;
import static org.elasticsearch.common.xcontent.XContentParser.Token.START_OBJECT; import static org.elasticsearch.common.xcontent.XContentParser.Token.START_OBJECT;
import static org.elasticsearch.common.xcontent.XContentParser.Token.VALUE_BOOLEAN; import static org.elasticsearch.common.xcontent.XContentParser.Token.VALUE_BOOLEAN;
@ -147,7 +149,7 @@ public final class ObjectParser<Value, Context> extends AbstractObjectParser<Val
private final Map<String, FieldParser> fieldParserMap = new HashMap<>(); private final Map<String, FieldParser> fieldParserMap = new HashMap<>();
private final String name; private final String name;
private final Supplier<Value> valueSupplier; private final Function<Context, Value> valueBuilder;
private final UnknownFieldParser<Value, Context> unknownFieldParser; private final UnknownFieldParser<Value, Context> unknownFieldParser;
/** /**
@ -155,16 +157,27 @@ public final class ObjectParser<Value, Context> extends AbstractObjectParser<Val
* @param name the parsers name, used to reference the parser in exceptions and messages. * @param name the parsers name, used to reference the parser in exceptions and messages.
*/ */
public ObjectParser(String name) { public ObjectParser(String name) {
this(name, null); this(name, errorOnUnknown(), null);
} }
/** /**
* Creates a new ObjectParser. * Creates a new ObjectParser.
* @param name the parsers name, used to reference the parser in exceptions and messages. * @param name the parsers name, used to reference the parser in exceptions and messages.
* @param valueSupplier a supplier that creates a new Value instance used when the parser is used as an inner object parser. * @param valueSupplier A supplier that creates a new Value instance. Used when the parser is used as an inner object parser.
*/ */
public ObjectParser(String name, @Nullable Supplier<Value> valueSupplier) { public ObjectParser(String name, @Nullable Supplier<Value> valueSupplier) {
this(name, errorOnUnknown(), valueSupplier); this(name, errorOnUnknown(), c -> valueSupplier.get());
}
/**
* Creates a new ObjectParser.
* @param name the parsers name, used to reference the parser in exceptions and messages.
* @param valueBuilder A function that creates a new Value from the parse Context. Used
* when the parser is used as an inner object parser.
*/
public static <Value, Context> ObjectParser<Value, Context> fromBuilder(String name, Function<Context, Value> valueBuilder) {
requireNonNull(valueBuilder, "Use the single argument ctor instead");
return new ObjectParser<Value, Context>(name, errorOnUnknown(), valueBuilder);
} }
/** /**
@ -175,7 +188,7 @@ public final class ObjectParser<Value, Context> extends AbstractObjectParser<Val
* @param valueSupplier a supplier that creates a new Value instance used when the parser is used as an inner object parser. * @param valueSupplier a supplier that creates a new Value instance used when the parser is used as an inner object parser.
*/ */
public ObjectParser(String name, boolean ignoreUnknownFields, @Nullable Supplier<Value> valueSupplier) { public ObjectParser(String name, boolean ignoreUnknownFields, @Nullable Supplier<Value> valueSupplier) {
this(name, ignoreUnknownFields ? ignoreUnknown() : errorOnUnknown(), valueSupplier); this(name, ignoreUnknownFields ? ignoreUnknown() : errorOnUnknown(), c -> valueSupplier.get());
} }
/** /**
@ -185,7 +198,7 @@ public final class ObjectParser<Value, Context> extends AbstractObjectParser<Val
* @param valueSupplier a supplier that creates a new Value instance used when the parser is used as an inner object parser. * @param valueSupplier a supplier that creates a new Value instance used when the parser is used as an inner object parser.
*/ */
public ObjectParser(String name, UnknownFieldConsumer<Value> unknownFieldConsumer, @Nullable Supplier<Value> valueSupplier) { public ObjectParser(String name, UnknownFieldConsumer<Value> unknownFieldConsumer, @Nullable Supplier<Value> valueSupplier) {
this(name, consumeUnknownField(unknownFieldConsumer), valueSupplier); this(name, consumeUnknownField(unknownFieldConsumer), c -> valueSupplier.get());
} }
/** /**
@ -202,19 +215,20 @@ public final class ObjectParser<Value, Context> extends AbstractObjectParser<Val
BiConsumer<Value, C> unknownFieldConsumer, BiConsumer<Value, C> unknownFieldConsumer,
@Nullable Supplier<Value> valueSupplier @Nullable Supplier<Value> valueSupplier
) { ) {
this(name, unknownIsNamedXContent(categoryClass, unknownFieldConsumer), valueSupplier); this(name, unknownIsNamedXContent(categoryClass, unknownFieldConsumer), c -> valueSupplier.get());
} }
/** /**
* Creates a new ObjectParser instance with a name. * Creates a new ObjectParser instance with a name.
* @param name the parsers name, used to reference the parser in exceptions and messages. * @param name the parsers name, used to reference the parser in exceptions and messages.
* @param unknownFieldParser how to parse unknown fields * @param unknownFieldParser how to parse unknown fields
* @param valueSupplier a supplier that creates a new Value instance used when the parser is used as an inner object parser. * @param valueBuilder builds the value from the context. Used when the ObjectParser is not passed a value.
*/ */
private ObjectParser(String name, UnknownFieldParser<Value, Context> unknownFieldParser, @Nullable Supplier<Value> valueSupplier) { private ObjectParser(String name, UnknownFieldParser<Value, Context> unknownFieldParser,
@Nullable Function<Context, Value> valueBuilder) {
this.name = name; this.name = name;
this.valueSupplier = valueSupplier;
this.unknownFieldParser = unknownFieldParser; this.unknownFieldParser = unknownFieldParser;
this.valueBuilder = valueBuilder;
} }
/** /**
@ -226,10 +240,10 @@ public final class ObjectParser<Value, Context> extends AbstractObjectParser<Val
*/ */
@Override @Override
public Value parse(XContentParser parser, Context context) throws IOException { public Value parse(XContentParser parser, Context context) throws IOException {
if (valueSupplier == null) { if (valueBuilder == null) {
throw new NullPointerException("valueSupplier is not set"); throw new NullPointerException("valueBuilder is not set");
} }
return parse(parser, valueSupplier.get(), context); return parse(parser, valueBuilder.apply(context), context);
} }
/** /**
@ -277,11 +291,8 @@ public final class ObjectParser<Value, Context> extends AbstractObjectParser<Val
@Override @Override
public Value apply(XContentParser parser, Context context) { public Value apply(XContentParser parser, Context context) {
if (valueSupplier == null) {
throw new NullPointerException("valueSupplier is not set");
}
try { try {
return parse(parser, valueSupplier.get(), context); return parse(parser, context);
} catch (IOException e) { } catch (IOException e) {
throw new XContentParseException(parser.getTokenLocation(), "[" + name + "] failed to parse object", e); throw new XContentParseException(parser.getTokenLocation(), "[" + name + "] failed to parse object", e);
} }

View File

@ -39,6 +39,7 @@ import java.util.Map;
import java.util.concurrent.atomic.AtomicReference; import java.util.concurrent.atomic.AtomicReference;
import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.instanceOf;
@ -806,4 +807,11 @@ public class ObjectParserTests extends ESTestCase {
assertEquals("[1:2] [test] unable to parse Object with name [not_supported_field]: parser not found", ex.getMessage()); assertEquals("[1:2] [test] unable to parse Object with name [not_supported_field]: parser not found", ex.getMessage());
} }
} }
public void testContextBuilder() throws IOException {
ObjectParser<AtomicReference<String>, String> parser = ObjectParser.fromBuilder("test", AtomicReference::new);
String context = randomAlphaOfLength(5);
AtomicReference<String> parsed = parser.parse(createParser(JsonXContent.jsonXContent, "{}"), context);
assertThat(parsed.get(), equalTo(context));
}
} }

View File

@ -264,7 +264,7 @@ public class AnalyzeAction extends ActionType<AnalyzeAction.Response> {
return request; return request;
} }
private static final ObjectParser<Request, Void> PARSER = new ObjectParser<>("analyze_request", null); private static final ObjectParser<Request, Void> PARSER = new ObjectParser<>("analyze_request");
static { static {
PARSER.declareStringArray(Request::text, new ParseField("text")); PARSER.declareStringArray(Request::text, new ParseField("text"));
PARSER.declareString(Request::analyzer, new ParseField("analyzer")); PARSER.declareString(Request::analyzer, new ParseField("analyzer"));

View File

@ -45,7 +45,7 @@ import static org.elasticsearch.action.ValidateActions.addValidationError;
*/ */
public class ResizeRequest extends AcknowledgedRequest<ResizeRequest> implements IndicesRequest, ToXContentObject { public class ResizeRequest extends AcknowledgedRequest<ResizeRequest> implements IndicesRequest, ToXContentObject {
public static final ObjectParser<ResizeRequest, Void> PARSER = new ObjectParser<>("resize_request", null); public static final ObjectParser<ResizeRequest, Void> PARSER = new ObjectParser<>("resize_request");
static { static {
PARSER.declareField((parser, request, context) -> request.getTargetIndexRequest().settings(parser.map()), PARSER.declareField((parser, request, context) -> request.getTargetIndexRequest().settings(parser.map()),
new ParseField("settings"), ObjectParser.ValueType.OBJECT); new ParseField("settings"), ObjectParser.ValueType.OBJECT);

View File

@ -42,14 +42,13 @@ import java.util.Map;
public class AvgAggregationBuilder extends ValuesSourceAggregationBuilder.LeafOnly<ValuesSource.Numeric, AvgAggregationBuilder> { public class AvgAggregationBuilder extends ValuesSourceAggregationBuilder.LeafOnly<ValuesSource.Numeric, AvgAggregationBuilder> {
public static final String NAME = "avg"; public static final String NAME = "avg";
private static final ObjectParser<AvgAggregationBuilder, Void> PARSER; private static final ObjectParser<AvgAggregationBuilder, String> PARSER = ObjectParser.fromBuilder(NAME, AvgAggregationBuilder::new);
static { static {
PARSER = new ObjectParser<>(AvgAggregationBuilder.NAME);
ValuesSourceParserHelper.declareNumericFields(PARSER, true, true, false); ValuesSourceParserHelper.declareNumericFields(PARSER, true, true, false);
} }
public static AggregationBuilder parse(String aggregationName, XContentParser parser) throws IOException { public static AggregationBuilder parse(String aggregationName, XContentParser parser) throws IOException {
return PARSER.parse(parser, new AvgAggregationBuilder(aggregationName), null); return PARSER.parse(parser, aggregationName);
} }
public AvgAggregationBuilder(String name) { public AvgAggregationBuilder(String name) {

View File

@ -45,7 +45,7 @@ public class QueryRescorerBuilder extends RescorerBuilder<QueryRescorerBuilder>
private static final ParseField RESCORE_QUERY_WEIGHT_FIELD = new ParseField("rescore_query_weight"); private static final ParseField RESCORE_QUERY_WEIGHT_FIELD = new ParseField("rescore_query_weight");
private static final ParseField SCORE_MODE_FIELD = new ParseField("score_mode"); private static final ParseField SCORE_MODE_FIELD = new ParseField("score_mode");
private static final ObjectParser<InnerBuilder, Void> QUERY_RESCORE_PARSER = new ObjectParser<>(NAME, null); private static final ObjectParser<InnerBuilder, Void> QUERY_RESCORE_PARSER = new ObjectParser<>(NAME);
static { static {
QUERY_RESCORE_PARSER.declareObject(InnerBuilder::setQueryBuilder, (p, c) -> { QUERY_RESCORE_PARSER.declareObject(InnerBuilder::setQueryBuilder, (p, c) -> {
try { try {

View File

@ -75,7 +75,7 @@ public class CompletionSuggestionBuilder extends SuggestionBuilder<CompletionSug
* "payload" : STRING_ARRAY * "payload" : STRING_ARRAY
* } * }
*/ */
private static final ObjectParser<CompletionSuggestionBuilder.InnerBuilder, Void> PARSER = new ObjectParser<>(SUGGESTION_NAME, null); private static final ObjectParser<CompletionSuggestionBuilder.InnerBuilder, Void> PARSER = new ObjectParser<>(SUGGESTION_NAME);
static { static {
PARSER.declareField((parser, completionSuggestionContext, context) -> { PARSER.declareField((parser, completionSuggestionContext, context) -> {
if (parser.currentToken() == XContentParser.Token.VALUE_BOOLEAN) { if (parser.currentToken() == XContentParser.Token.VALUE_BOOLEAN) {

View File

@ -95,7 +95,7 @@ public final class CategoryQueryContext implements ToXContentObject {
return result; return result;
} }
private static final ObjectParser<Builder, Void> CATEGORY_PARSER = new ObjectParser<>(NAME, null); private static final ObjectParser<Builder, Void> CATEGORY_PARSER = new ObjectParser<>(NAME);
static { static {
CATEGORY_PARSER.declareField(Builder::setCategory, XContentParser::text, new ParseField(CONTEXT_VALUE), CATEGORY_PARSER.declareField(Builder::setCategory, XContentParser::text, new ParseField(CONTEXT_VALUE),
ObjectParser.ValueType.VALUE); ObjectParser.ValueType.VALUE);

View File

@ -112,7 +112,7 @@ public final class GeoQueryContext implements ToXContentObject {
return new Builder(); return new Builder();
} }
private static final ObjectParser<GeoQueryContext.Builder, Void> GEO_CONTEXT_PARSER = new ObjectParser<>(NAME, null); private static final ObjectParser<GeoQueryContext.Builder, Void> GEO_CONTEXT_PARSER = new ObjectParser<>(NAME);
static { static {
GEO_CONTEXT_PARSER.declareField((parser, geoQueryContext, GEO_CONTEXT_PARSER.declareField((parser, geoQueryContext,
geoContextMapping) -> geoQueryContext.setGeoPoint(GeoUtils.parseGeoPoint(parser)), geoContextMapping) -> geoQueryContext.setGeoPoint(GeoUtils.parseGeoPoint(parser)),