From 3504755f445bce2be3eb4776dad1e791fefc5a69 Mon Sep 17 00:00:00 2001 From: Igor Motov Date: Wed, 22 Apr 2020 12:28:52 -0400 Subject: [PATCH] Add InstantiatingObjectParser (#55483) (#55604) Introduces InstantiatingObjectParser which is similar to the ConstructingObjectParser, but instantiates the object using its constructor instead of a builder function. Closes #52499 --- .../common/xcontent/AbstractObjectParser.java | 21 +- .../xcontent/ConstructingObjectParser.java | 10 +- .../xcontent/InstantiatingObjectParser.java | 199 +++++++++++++++ .../common/xcontent/ObjectParser.java | 23 +- .../common/xcontent/ParserConstructor.java | 35 +++ .../InstantiatingObjectParserTests.java | 231 ++++++++++++++++++ .../org/elasticsearch/tasks/TaskResult.java | 26 +- 7 files changed, 509 insertions(+), 36 deletions(-) create mode 100644 libs/x-content/src/main/java/org/elasticsearch/common/xcontent/InstantiatingObjectParser.java create mode 100644 libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ParserConstructor.java create mode 100644 libs/x-content/src/test/java/org/elasticsearch/common/xcontent/InstantiatingObjectParserTests.java diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/AbstractObjectParser.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/AbstractObjectParser.java index 8303cef383f..79bfcde77ab 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/AbstractObjectParser.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/AbstractObjectParser.java @@ -28,17 +28,12 @@ import java.io.IOException; import java.util.ArrayList; import java.util.List; import java.util.function.BiConsumer; -import java.util.function.BiFunction; import java.util.function.Consumer; /** * Superclass for {@link ObjectParser} and {@link ConstructingObjectParser}. Defines most of the "declare" methods so they can be shared. */ -public abstract class AbstractObjectParser - implements BiFunction, ContextParser { - - final List requiredFieldSets = new ArrayList<>(); - final List exclusiveFieldSets = new ArrayList<>(); +public abstract class AbstractObjectParser { /** * Declare some field. Usually it is easier to use {@link #declareString(BiConsumer, ParseField)} or @@ -313,12 +308,7 @@ public abstract class AbstractObjectParser * @param requiredSet * A set of required fields, where at least one of the fields in the array _must_ be present */ - public void declareRequiredFieldSet(String... requiredSet) { - if (requiredSet.length == 0) { - return; - } - this.requiredFieldSets.add(requiredSet); - } + public abstract void declareRequiredFieldSet(String... requiredSet); /** * Declares a set of fields of which at most one must appear for parsing to succeed @@ -332,12 +322,7 @@ public abstract class AbstractObjectParser * * @param exclusiveSet a set of field names, at most one of which must appear */ - public void declareExclusiveFieldSet(String... exclusiveSet) { - if (exclusiveSet.length == 0) { - return; - } - this.exclusiveFieldSets.add(exclusiveSet); - } + public abstract void declareExclusiveFieldSet(String... exclusiveSet); private interface IOSupplier { T get() throws IOException; diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ConstructingObjectParser.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ConstructingObjectParser.java index 52bfcc65160..c04851b9173 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ConstructingObjectParser.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ConstructingObjectParser.java @@ -73,7 +73,9 @@ import java.util.function.Function; * Note: if optional constructor arguments aren't specified then the number of allocations is always the worst case. *

*/ -public final class ConstructingObjectParser extends AbstractObjectParser { +public final class ConstructingObjectParser extends AbstractObjectParser implements + BiFunction, ContextParser{ + /** * Consumer that marks a field as a required constructor argument instead of a real object field. */ @@ -315,6 +317,10 @@ public final class ConstructingObjectParser extends AbstractObje } } + int getNumberOfFields() { + return this.constructorArgInfos.size(); + } + /** * Constructor arguments are detected by this "marker" consumer. It * keeps the API looking clean even if it is a bit sleezy. @@ -335,7 +341,7 @@ public final class ConstructingObjectParser extends AbstractObje constructorArgInfos.add(new ConstructorArgInfo(parseField, required)); return position; } - + @Override public String getName() { return objectParser.getName(); diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/InstantiatingObjectParser.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/InstantiatingObjectParser.java new file mode 100644 index 00000000000..3cb1804ef20 --- /dev/null +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/InstantiatingObjectParser.java @@ -0,0 +1,199 @@ +/* + * 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.xcontent; + +import org.elasticsearch.common.ParseField; + +import java.io.IOException; +import java.lang.reflect.Constructor; +import java.util.List; +import java.util.function.BiConsumer; +import java.util.function.BiFunction; +import java.util.function.Consumer; + +/** + * Like {@link ConstructingObjectParser} but works with objects which have a constructor that matches declared fields. + *

+ * Declaring a {@linkplain InstantiatingObjectParser} is intentionally quite similar to declaring an {@linkplain ConstructingObjectParser} + * with two important differences. + *

+ * The main differences being that it is using Builder to construct the parser and takes a class of the target object instead of the object + * builder. The target object must have exactly one constructor with the number and order of arguments matching the number of order of + * declared fields. If there are more then 2 constructors with the same number of arguments, one of them needs to be marked with + * {@linkplain ParserConstructor} annotation. + *

{@code
+ *   public static class Thing{
+ *       public Thing(String animal, String vegetable, int mineral) {
+ *           ....
+ *       }
+ *
+ *       public void setFruit(int fruit) { ... }
+ *
+ *       public void setBug(int bug) { ... }
+ *
+ *   }
+ *
+ *   private static final InstantiatingObjectParser PARSER = new InstantiatingObjectParser<>("thing", Thing.class);
+ *   static {
+ *       PARSER.declareString(constructorArg(), new ParseField("animal"));
+ *       PARSER.declareString(constructorArg(), new ParseField("vegetable"));
+ *       PARSER.declareInt(optionalConstructorArg(), new ParseField("mineral"));
+ *       PARSER.declareInt(Thing::setFruit, new ParseField("fruit"));
+ *       PARSER.declareInt(Thing::setBug, new ParseField("bug"));
+ *       PARSER.finalizeFields()
+ *   }
+ * }
+ */ +public class InstantiatingObjectParser + implements BiFunction, ContextParser { + + public static Builder builder(String name, boolean ignoreUnknownFields, Class valueClass) { + return new Builder<>(name, ignoreUnknownFields, valueClass); + } + + public static Builder builder(String name, Class valueClass) { + return new Builder<>(name, valueClass); + } + + public static class Builder extends AbstractObjectParser { + + private final ConstructingObjectParser constructingObjectParser; + + private final Class valueClass; + + private Constructor constructor; + + public Builder(String name, Class valueClass) { + this(name, false, valueClass); + } + + public Builder(String name, boolean ignoreUnknownFields, Class valueClass) { + this.constructingObjectParser = new ConstructingObjectParser<>(name, ignoreUnknownFields, this::build); + this.valueClass = valueClass; + } + + @SuppressWarnings("unchecked") + public InstantiatingObjectParser build() { + Constructor constructor = null; + int neededArguments = constructingObjectParser.getNumberOfFields(); + // Try to find an annotated constructor + for (Constructor c : valueClass.getConstructors()) { + if (c.getAnnotation(ParserConstructor.class) != null) { + if (constructor != null) { + throw new IllegalArgumentException("More then one public constructor with @ParserConstructor annotation exist in " + + "the class " + valueClass.getName()); + } + if (c.getParameterCount() != neededArguments) { + throw new IllegalArgumentException("Annotated constructor doesn't have " + neededArguments + + " arguments in the class " + valueClass.getName()); + } + constructor = c; + } + } + if (constructor == null) { + // fallback to a constructor with required number of arguments + for (Constructor c : valueClass.getConstructors()) { + if (c.getParameterCount() == neededArguments) { + if (constructor != null) { + throw new IllegalArgumentException("More then one public constructor with " + neededArguments + + " arguments found. The use of @ParserConstructor annotation is required for class " + valueClass.getName()); + } + constructor = c; + } + } + } + if (constructor == null) { + throw new IllegalArgumentException("No public constructors with " + neededArguments + " parameters exist in the class " + + valueClass.getName()); + } + this.constructor = (Constructor) constructor; + return new InstantiatingObjectParser<>(constructingObjectParser); + } + + @Override + public void declareField(BiConsumer consumer, ContextParser parser, ParseField parseField, + ObjectParser.ValueType type) { + constructingObjectParser.declareField(consumer, parser, parseField, type); + } + + @Override + public void declareNamedObject(BiConsumer consumer, ObjectParser.NamedObjectParser namedObjectParser, + ParseField parseField) { + constructingObjectParser.declareNamedObject(consumer, namedObjectParser, parseField); + } + + @Override + public void declareNamedObjects(BiConsumer> consumer, + ObjectParser.NamedObjectParser namedObjectParser, ParseField parseField) { + constructingObjectParser.declareNamedObjects(consumer, namedObjectParser, parseField); + } + + @Override + public void declareNamedObjects(BiConsumer> consumer, + ObjectParser.NamedObjectParser namedObjectParser, + Consumer orderedModeCallback, ParseField parseField) { + constructingObjectParser.declareNamedObjects(consumer, namedObjectParser, orderedModeCallback, parseField); + } + + @Override + public String getName() { + return constructingObjectParser.getName(); + } + + @Override + public void declareRequiredFieldSet(String... requiredSet) { + constructingObjectParser.declareRequiredFieldSet(requiredSet); + } + + @Override + public void declareExclusiveFieldSet(String... exclusiveSet) { + constructingObjectParser.declareExclusiveFieldSet(exclusiveSet); + } + + private Value build(Object[] args) { + if (constructor == null) { + throw new IllegalArgumentException("InstantiatingObjectParser for type " + valueClass.getName() + " has to be finalized " + + "before the first use"); + } + try { + return constructor.newInstance(args); + } catch (Exception ex) { + throw new IllegalArgumentException("Cannot instantiate an object of " + valueClass.getName(), ex); + } + } + } + + + private final ConstructingObjectParser constructingObjectParser; + + private InstantiatingObjectParser(ConstructingObjectParser constructingObjectParser) { + this.constructingObjectParser = constructingObjectParser; + } + + @Override + public Value parse(XContentParser parser, Context context) throws IOException { + return constructingObjectParser.parse(parser, context); + } + + @Override + public Value apply(XContentParser xContentParser, Context context) { + return constructingObjectParser.apply(xContentParser, context); + } +} diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ObjectParser.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ObjectParser.java index 73bbf812f59..1ed6627e515 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ObjectParser.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ObjectParser.java @@ -70,7 +70,12 @@ import static org.elasticsearch.common.xcontent.XContentParser.Token.VALUE_STRIN * It's highly recommended to use the high level declare methods like {@link #declareString(BiConsumer, ParseField)} instead of * {@link #declareField} which can be used to implement exceptional parsing operations not covered by the high level methods. */ -public final class ObjectParser extends AbstractObjectParser { +public final class ObjectParser extends AbstractObjectParser + implements BiFunction, ContextParser{ + + private final List requiredFieldSets = new ArrayList<>(); + private final List exclusiveFieldSets = new ArrayList<>(); + /** * Adapts an array (or varags) setter into a list setter. */ @@ -496,6 +501,22 @@ public final class ObjectParser extends AbstractObjectParser builder = InstantiatingObjectParser.builder("foo", NoAnnotations.class); + builder.declareInt(constructorArg(), new ParseField("a")); + builder.declareString(constructorArg(), new ParseField("b")); + builder.declareLong(constructorArg(), new ParseField("c")); + InstantiatingObjectParser parser = builder.build(); + try (XContentParser contentParser = createParser(JsonXContent.jsonXContent, "{\"a\": 5, \"b\":\"6\", \"c\": 7 }")) { + assertThat(parser.parse(contentParser, null), equalTo(new NoAnnotations(5, "6", 7))); + } + } + + public void testNoAnnotationWrongArgumentNumber() { + InstantiatingObjectParser.Builder builder = InstantiatingObjectParser.builder("foo", NoAnnotations.class); + builder.declareInt(constructorArg(), new ParseField("a")); + builder.declareString(constructorArg(), new ParseField("b")); + builder.declareLong(constructorArg(), new ParseField("c")); + builder.declareLong(constructorArg(), new ParseField("d")); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, builder::build); + assertThat(e.getMessage(), containsString("No public constructors with 4 parameters exist in the class")); + } + + public void testAmbiguousConstructor() { + InstantiatingObjectParser.Builder builder = InstantiatingObjectParser.builder("foo", NoAnnotations.class); + builder.declareInt(constructorArg(), new ParseField("a")); + builder.declareString(constructorArg(), new ParseField("b")); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, builder::build); + assertThat(e.getMessage(), containsString( + "More then one public constructor with 2 arguments found. The use of @ParserConstructor annotation is required" + )); + } + + public void testPrivateConstructor() { + InstantiatingObjectParser.Builder builder = InstantiatingObjectParser.builder("foo", NoAnnotations.class); + builder.declareInt(constructorArg(), new ParseField("a")); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, builder::build); + assertThat(e.getMessage(), containsString("No public constructors with 1 parameters exist in the class ")); + } + + public static class LonelyArgument { + public final int a; + + private String b; + + public LonelyArgument(int a) { + this.a = a; + this.b = "Not set"; + } + + public void setB(String b) { + this.b = b; + } + + public String getB() { + return b; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + LonelyArgument that = (LonelyArgument) o; + return a == that.a && + Objects.equals(b, that.b); + } + + @Override + public int hashCode() { + return Objects.hash(a, b); + } + } + + public void testOneArgConstructor() throws IOException { + InstantiatingObjectParser.Builder builder = InstantiatingObjectParser.builder("foo", LonelyArgument.class); + builder.declareInt(constructorArg(), new ParseField("a")); + InstantiatingObjectParser parser = builder.build(); + try (XContentParser contentParser = createParser(JsonXContent.jsonXContent, "{\"a\": 5 }")) { + assertThat(parser.parse(contentParser, null), equalTo(new LonelyArgument(5))); + } + } + + public void testSetNonConstructor() throws IOException { + InstantiatingObjectParser.Builder builder = InstantiatingObjectParser.builder("foo", LonelyArgument.class); + builder.declareInt(constructorArg(), new ParseField("a")); + builder.declareString(LonelyArgument::setB, new ParseField("b")); + InstantiatingObjectParser parser = builder.build(); + try (XContentParser contentParser = createParser(JsonXContent.jsonXContent, "{\"a\": 5, \"b\": \"set\" }")) { + LonelyArgument expected = parser.parse(contentParser, null); + assertThat(expected.a, equalTo(5)); + assertThat(expected.b, equalTo("set")); + } + } + + public static class Annotations { + final int a; + final String b; + final long c; + + public Annotations() { + this(1, "2", 3); + } + + public Annotations(int a, String b) { + this(a, b, -1); + } + + public Annotations(int a, String b, long c) { + this.a = a; + this.b = b; + this.c = c; + } + + @ParserConstructor + public Annotations(int a, String b, String c) { + this.a = a; + this.b = b; + this.c = Long.parseLong(c); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + Annotations that = (Annotations) o; + return a == that.a && + c == that.c && + Objects.equals(b, that.b); + } + + @Override + public int hashCode() { + return Objects.hash(a, b, c); + } + } + + public void testAnnotation() throws IOException { + InstantiatingObjectParser.Builder builder = InstantiatingObjectParser.builder("foo", Annotations.class); + builder.declareInt(constructorArg(), new ParseField("a")); + builder.declareString(constructorArg(), new ParseField("b")); + builder.declareString(constructorArg(), new ParseField("c")); + InstantiatingObjectParser parser = builder.build(); + try (XContentParser contentParser = createParser(JsonXContent.jsonXContent, "{\"a\": 5, \"b\":\"6\", \"c\": \"7\"}")) { + assertThat(parser.parse(contentParser, null), equalTo(new Annotations(5, "6", 7))); + } + } + + public void testAnnotationWrongArgumentNumber() { + InstantiatingObjectParser.Builder builder = InstantiatingObjectParser.builder("foo", Annotations.class); + builder.declareInt(constructorArg(), new ParseField("a")); + builder.declareString(constructorArg(), new ParseField("b")); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, builder::build); + assertThat(e.getMessage(), containsString("Annotated constructor doesn't have 2 arguments in the class")); + } + +} diff --git a/server/src/main/java/org/elasticsearch/tasks/TaskResult.java b/server/src/main/java/org/elasticsearch/tasks/TaskResult.java index 46b68ce1602..b2cf9c224b6 100644 --- a/server/src/main/java/org/elasticsearch/tasks/TaskResult.java +++ b/server/src/main/java/org/elasticsearch/tasks/TaskResult.java @@ -27,7 +27,7 @@ import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; -import org.elasticsearch.common.xcontent.ConstructingObjectParser; +import org.elasticsearch.common.xcontent.InstantiatingObjectParser; import org.elasticsearch.common.xcontent.ObjectParserHelper; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.ToXContentObject; @@ -79,7 +79,7 @@ public final class TaskResult implements Writeable, ToXContentObject { this(true, task, null, XContentHelper.toXContent(response, Requests.INDEX_CONTENT_TYPE, true)); } - private TaskResult(boolean completed, TaskInfo task, @Nullable BytesReference error, @Nullable BytesReference result) { + public TaskResult(boolean completed, TaskInfo task, @Nullable BytesReference error, @Nullable BytesReference result) { this.completed = completed; this.task = requireNonNull(task, "task is required"); this.error = error; @@ -174,21 +174,17 @@ public final class TaskResult implements Writeable, ToXContentObject { return builder; } - public static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( - "stored_task_result", a -> { - int i = 0; - boolean completed = (boolean) a[i++]; - TaskInfo task = (TaskInfo) a[i++]; - BytesReference error = (BytesReference) a[i++]; - BytesReference response = (BytesReference) a[i++]; - return new TaskResult(completed, task, error, response); - }); + public static final InstantiatingObjectParser PARSER; + static { - PARSER.declareBoolean(constructorArg(), new ParseField("completed")); - PARSER.declareObject(constructorArg(), TaskInfo.PARSER, new ParseField("task")); + InstantiatingObjectParser.Builder parser = InstantiatingObjectParser.builder( + "stored_task_result", true, TaskResult.class); + parser.declareBoolean(constructorArg(), new ParseField("completed")); + parser.declareObject(constructorArg(), TaskInfo.PARSER, new ParseField("task")); ObjectParserHelper parserHelper = new ObjectParserHelper<>(); - parserHelper.declareRawObject(PARSER, optionalConstructorArg(), new ParseField("error")); - parserHelper.declareRawObject(PARSER, optionalConstructorArg(), new ParseField("response")); + parserHelper.declareRawObject(parser, optionalConstructorArg(), new ParseField("error")); + parserHelper.declareRawObject(parser, optionalConstructorArg(), new ParseField("response")); + PARSER = parser.build(); } @Override