Skip to content
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.

Sign up for GitHub

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

Enable InstantiatingObjectParser to pass context as a first argument #79206

Merged
merged 3 commits into from
Oct 18, 2021
Merged

Enable InstantiatingObjectParser to pass context as a first argument #79206

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4497d9a
Enable InstantiatingObjectParser to pass context as a first argument
imotov Oct 14, 2021
88ec32c
Improve the error message for annotated constructor
imotov Oct 14, 2021
18a10ca
Merge branch 'master' into improve-instanciating-parser
elasticmachine Oct 18, 2021
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
66 changes: 52 additions & 14 deletions libs/x-content/src/main/java/org/elasticsearch/xcontent/InstantiatingObjectParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,12 @@
* <p>
* 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
* declared fields. If there are more than 2 constructors with the same number of arguments, one of them needs to be marked with
* {@linkplain ParserConstructor} annotation.
*
* It is also possible for the constructor to accept Context as the first parameter, in this case as in the case with multiple constructors
* it is required for the constructor to be marked with {@linkplain ParserConstructor} annotation.
*
* <pre>{@code
* public static class Thing{
* public Thing(String animal, String vegetable, int mineral) {
Expand All @@ -37,14 +41,35 @@
*
* }
*
* private static final InstantiatingObjectParser<Thing, SomeContext> PARSER = new InstantiatingObjectParser<>("thing", Thing.class);
* private static final InstantiatingObjectParser<Thing, SomeContext> PARSER;
* static {
* InstantiatingObjectParser.Builder<Thing, SomeContext> parser =
* InstantiatingObjectParser,builder<>("thing", true, Thing.class);
* 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 = parser.build()
* }
* }</pre>
* <pre>{@code
*
* public static class AnotherThing {
* @ParserConstructor
* public AnotherThing(SomeContext continent, String animal, String vegetable, int mineral) {
* ....
* }
* }
*
* private static final InstantiatingObjectParser<AnotherThing, SomeContext> PARSER;
* 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()
* InstantiatingObjectParser.Builder<AnotherThing, SomeContext> parser =
* InstantiatingObjectParser,builder<>("thing", true, AnotherThing.class);
* parser.declareString(constructorArg(), new ParseField("animal"));
* parser.declareString(constructorArg(), new ParseField("vegetable"));
* parser.declareInt(optionalConstructorArg(), new ParseField("mineral"));
* PARSER = parser.build()
* }
* }</pre>
*/
Expand Down Expand Up @@ -72,7 +97,7 @@ public Builder(String name, Class<Value> valueClass) {
}

public Builder(String name, boolean ignoreUnknownFields, Class<Value> valueClass) {
this.constructingObjectParser = new ConstructingObjectParser<>(name, ignoreUnknownFields, this::build);
this.constructingObjectParser = new ConstructingObjectParser<>(name, ignoreUnknownFields, this::buildInstance);
this.valueClass = valueClass;
}

Expand All @@ -87,9 +112,15 @@ public InstantiatingObjectParser<Value, Context> build() {
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());
if (c.getParameterCount() < neededArguments || c.getParameterCount() > neededArguments + 1) {
throw new IllegalArgumentException(
"Annotated constructor doesn't have "
+ neededArguments
+ " or "
+ (neededArguments + 1)
+ " arguments in the class "
+ valueClass.getName()
);
}
constructor = c;
}
Expand Down Expand Up @@ -154,13 +185,20 @@ public void declareExclusiveFieldSet(String... exclusiveSet) {
constructingObjectParser.declareExclusiveFieldSet(exclusiveSet);
}

private Value build(Object[] args) {
private Value buildInstance(Object[] args, Context context) {
if (constructor == null) {
throw new IllegalArgumentException("InstantiatingObjectParser for type " + valueClass.getName() + " has to be finalized " +
"before the first use");
}
try {
return constructor.newInstance(args);
if (constructor.getParameterCount() != args.length) {
Object[] newArgs = new Object[args.length + 1];
System.arraycopy(args, 0, newArgs, 1, args.length);
newArgs[0] = context;
return constructor.newInstance(newArgs);
} else {
return constructor.newInstance(args);
}
} catch (Exception ex) {
throw new IllegalArgumentException("Cannot instantiate an object of " + valueClass.getName(), ex);
}
Expand Down
85 changes: 80 additions & 5 deletions libs/x-content/src/test/java/org/elasticsearch/xcontent/InstantiatingObjectParserTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,8 @@

package org.elasticsearch.xcontent;

import org.elasticsearch.xcontent.InstantiatingObjectParser;
import org.elasticsearch.xcontent.ParseField;
import org.elasticsearch.xcontent.ParserConstructor;
import org.elasticsearch.xcontent.json.JsonXContent;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xcontent.json.JsonXContent;

import java.io.IOException;
import java.util.Objects;
Expand Down Expand Up @@ -217,8 +214,10 @@ public void testAnnotationWrongArgumentNumber() {
InstantiatingObjectParser.Builder<Annotations, Void> builder = InstantiatingObjectParser.builder("foo", Annotations.class);
builder.declareInt(constructorArg(), new ParseField("a"));
builder.declareString(constructorArg(), new ParseField("b"));
builder.declareInt(constructorArg(), new ParseField("c"));
builder.declareString(constructorArg(), new ParseField("d"));
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, builder::build);
assertThat(e.getMessage(), containsString("Annotated constructor doesn't have 2 arguments in the class"));
assertThat(e.getMessage(), containsString("Annotated constructor doesn't have 4 or 5 arguments in the class"));
}

public void testDoubleDeclarationThrowsException() throws IOException {
Expand All @@ -240,4 +239,80 @@ class DoubleFieldDeclaration {
assertThat(exception, instanceOf(IllegalArgumentException.class));
assertThat(exception.getMessage(), startsWith("Parser already registered for name=[name]"));
}

public static class ContextArgument {
final String context;
final int a;
final String b;
final long c;

public ContextArgument() {
this(1, "2", 3);
}

public ContextArgument(int a, String b) {
this(a, b, -1);
}


public ContextArgument(int a, String b, long c) {
this(null, a, b, c);
}

public ContextArgument(String context, int a, String b, long c) {
this.context = context;
this.a = a;
this.b = b;
this.c = c;
}

@ParserConstructor
public ContextArgument(String context, int a, String b, String c) {
this.context = context;
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;
ContextArgument that = (ContextArgument) 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 testContextAsArgument() throws IOException {
InstantiatingObjectParser.Builder<ContextArgument, String> builder = InstantiatingObjectParser.builder(
"foo",
ContextArgument.class
);
builder.declareInt(constructorArg(), new ParseField("a"));
builder.declareString(constructorArg(), new ParseField("b"));
builder.declareString(constructorArg(), new ParseField("c"));
InstantiatingObjectParser<ContextArgument, String> parser = builder.build();
try (XContentParser contentParser = createParser(JsonXContent.jsonXContent, "{\"a\": 5, \"b\":\"6\", \"c\": \"7\"}")) {
assertThat(parser.parse(contentParser, "context"), equalTo(new ContextArgument("context", 5, "6", 7)));
}
}

public void testContextAsArgumentWrongArgumentNumber() {
InstantiatingObjectParser.Builder<ContextArgument, String> builder = InstantiatingObjectParser.builder(
"foo",
ContextArgument.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 or 3 arguments in the class"));
}

}
109 changes: 76 additions & 33 deletions server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilities.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,14 @@

import org.elasticsearch.Version;
import org.elasticsearch.index.mapper.TimeSeriesParams;
import org.elasticsearch.xcontent.InstantiatingObjectParser;
import org.elasticsearch.xcontent.ParseField;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.io.stream.Writeable;
import org.elasticsearch.xcontent.ConstructingObjectParser;
import org.elasticsearch.xcontent.ParserConstructor;
import org.elasticsearch.xcontent.ToXContentObject;
import org.elasticsearch.xcontent.XContentBuilder;
import org.elasticsearch.xcontent.XContentParser;
Expand Down Expand Up @@ -159,6 +161,59 @@ public FieldCapabilities(String name, String type,

}

/**
* Constructor for a set of indices used by parser
* @param name The name of the field
* @param type The type associated with the field.
* @param isMetadataField Whether this field is a metadata field.
* @param isSearchable Whether this field is indexed for search.
* @param isAggregatable Whether this field can be aggregated on.
* @param isDimension Whether this field can be used as dimension
* @param metricType If this field is a metric field, returns the metric's type or null for non-metrics fields
* @param indices The list of indices where this field name is defined as {@code type},
* or null if all indices have the same {@code type} for the field.
* @param nonSearchableIndices The list of indices where this field is not searchable,
* or null if the field is searchable in all indices.
* @param nonAggregatableIndices The list of indices where this field is not aggregatable,
* or null if the field is aggregatable in all indices.
* @param nonDimensionIndices The list of indices where this field is not a dimension
* @param metricConflictsIndices The list of indices where this field is has different metric types or not mark as a metric
* @param meta Merged metadata across indices.
*/
@SuppressWarnings("unused")
@ParserConstructor
public FieldCapabilities(
String name,
String type,
Boolean isMetadataField,
boolean isSearchable,
boolean isAggregatable,
Boolean isDimension,
String metricType,
List<String> indices,
List<String> nonSearchableIndices,
List<String> nonAggregatableIndices,
List<String> nonDimensionIndices,
List<String> metricConflictsIndices,
Map<String, Set<String>> meta
) {
this(
name,
type,
isMetadataField == null ? false : isMetadataField,
isSearchable,
isAggregatable,
isDimension == null ? false : isDimension,
metricType != null ? Enum.valueOf(TimeSeriesParams.MetricType.class, metricType) : null,
indices != null ? indices.toArray(new String[0]) : null,
nonSearchableIndices != null ? nonSearchableIndices.toArray(new String[0]) : null,
nonAggregatableIndices != null ? nonAggregatableIndices.toArray(new String[0]) : null,
nonDimensionIndices != null ? nonDimensionIndices.toArray(new String[0]) : null,
metricConflictsIndices != null ? metricConflictsIndices.toArray(new String[0]) : null,
meta != null ? meta : Collections.emptyMap()
);
}

FieldCapabilities(StreamInput in) throws IOException {
this.name = in.readString();
this.type = in.readString();
Expand Down Expand Up @@ -254,43 +309,31 @@ public static FieldCapabilities fromXContent(String name, XContentParser parser)
}

@SuppressWarnings("unchecked")
private static final ConstructingObjectParser<FieldCapabilities, String> PARSER = new ConstructingObjectParser<>(
"field_capabilities",
true,
(a, name) -> new FieldCapabilities(
name,
(String) a[0],
a[3] == null ? false : (boolean) a[3],
(boolean) a[1],
(boolean) a[2],
a[4] == null ? false : (boolean) a[4],
a[5] != null ? Enum.valueOf(TimeSeriesParams.MetricType.class, (String) a[5]) : null,
a[6] != null ? ((List<String>) a[6]).toArray(new String[0]) : null,
a[7] != null ? ((List<String>) a[7]).toArray(new String[0]) : null,
a[8] != null ? ((List<String>) a[8]).toArray(new String[0]) : null,
a[9] != null ? ((List<String>) a[9]).toArray(new String[0]) : null,
a[10] != null ? ((List<String>) a[10]).toArray(new String[0]) : null,
a[11] != null ? ((Map<String, Set<String>>) a[11]) : Collections.emptyMap()
)
);
private static final InstantiatingObjectParser<FieldCapabilities, String> PARSER;

static {
PARSER.declareString(ConstructingObjectParser.constructorArg(), TYPE_FIELD); // 0
PARSER.declareBoolean(ConstructingObjectParser.constructorArg(), SEARCHABLE_FIELD); // 1
PARSER.declareBoolean(ConstructingObjectParser.constructorArg(), AGGREGATABLE_FIELD); // 2
PARSER.declareBoolean(ConstructingObjectParser.optionalConstructorArg(), IS_METADATA_FIELD); // 3
PARSER.declareBoolean(ConstructingObjectParser.optionalConstructorArg(), TIME_SERIES_DIMENSION_FIELD); // 4
PARSER.declareString(ConstructingObjectParser.optionalConstructorArg(), TIME_SERIES_METRIC_FIELD); // 5
PARSER.declareStringArray(ConstructingObjectParser.optionalConstructorArg(), INDICES_FIELD); // 6
PARSER.declareStringArray(ConstructingObjectParser.optionalConstructorArg(), NON_SEARCHABLE_INDICES_FIELD); // 7
PARSER.declareStringArray(ConstructingObjectParser.optionalConstructorArg(), NON_AGGREGATABLE_INDICES_FIELD); // 8
PARSER.declareStringArray(ConstructingObjectParser.optionalConstructorArg(), NON_DIMENSION_INDICES_FIELD); // 9
PARSER.declareStringArray(ConstructingObjectParser.optionalConstructorArg(), METRIC_CONFLICTS_INDICES_FIELD); // 10
PARSER.declareObject(
InstantiatingObjectParser.Builder<FieldCapabilities, String> parser = InstantiatingObjectParser.builder(
"field_capabilities",
true,
FieldCapabilities.class
);
parser.declareString(ConstructingObjectParser.constructorArg(), TYPE_FIELD);
parser.declareBoolean(ConstructingObjectParser.optionalConstructorArg(), IS_METADATA_FIELD);
parser.declareBoolean(ConstructingObjectParser.constructorArg(), SEARCHABLE_FIELD);
parser.declareBoolean(ConstructingObjectParser.constructorArg(), AGGREGATABLE_FIELD);
parser.declareBoolean(ConstructingObjectParser.optionalConstructorArg(), TIME_SERIES_DIMENSION_FIELD);
parser.declareString(ConstructingObjectParser.optionalConstructorArg(), TIME_SERIES_METRIC_FIELD);
parser.declareStringArray(ConstructingObjectParser.optionalConstructorArg(), INDICES_FIELD);
parser.declareStringArray(ConstructingObjectParser.optionalConstructorArg(), NON_SEARCHABLE_INDICES_FIELD);
parser.declareStringArray(ConstructingObjectParser.optionalConstructorArg(), NON_AGGREGATABLE_INDICES_FIELD);
parser.declareStringArray(ConstructingObjectParser.optionalConstructorArg(), NON_DIMENSION_INDICES_FIELD);
parser.declareStringArray(ConstructingObjectParser.optionalConstructorArg(), METRIC_CONFLICTS_INDICES_FIELD);
parser.declareObject(
ConstructingObjectParser.optionalConstructorArg(),
(parser, context) -> parser.map(HashMap::new, p -> Set.copyOf(p.list())),
(p, context) -> p.map(HashMap::new, v -> Set.copyOf(v.list())),
META_FIELD
); // 11
);
PARSER = parser.build();
}

/**
Expand Down