-
Notifications
You must be signed in to change notification settings - Fork 25.4k
Helper for ConstructingObjectParser for record classes #132106
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,11 +10,17 @@ | |
package org.elasticsearch.xcontent; | ||
|
||
import org.elasticsearch.core.RestApiVersion; | ||
import org.elasticsearch.core.SuppressForbidden; | ||
import org.elasticsearch.xcontent.ObjectParser.NamedObjectParser; | ||
import org.elasticsearch.xcontent.ObjectParser.ValueType; | ||
|
||
import java.io.IOException; | ||
import java.lang.invoke.MethodHandle; | ||
import java.lang.invoke.MethodHandles; | ||
import java.lang.invoke.MethodType; | ||
import java.lang.reflect.RecordComponent; | ||
import java.util.ArrayList; | ||
import java.util.Arrays; | ||
import java.util.Collections; | ||
import java.util.EnumMap; | ||
import java.util.List; | ||
|
@@ -130,6 +136,50 @@ public ConstructingObjectParser(String name, boolean ignoreUnknownFields, Functi | |
this(name, ignoreUnknownFields, (args, context) -> builder.apply(args)); | ||
} | ||
|
||
/** | ||
* Build a parser for the given {@code recordClass}. | ||
* | ||
* @param name The name given to the delegate ObjectParser for error identification. Use what you'd use if the object worked with | ||
* ObjectParser. | ||
* @param ignoreUnknownFields Should this parser ignore unknown fields? This should generally be set to true only when parsing responses | ||
* from external systems, never when parsing requests from users. | ||
* @param recordClass the {@link Class} of the {@link Record} type to build | ||
* @param lookup a {@link java.lang.invoke.MethodHandles.Lookup Lookup} object that can access the record's canonical constructor; | ||
* typically just {@code MethodHandles.lookup()} called by some code that can access the constructor. | ||
* @return a function suitable to use as the {@code builder} argument for one of the constructors of this class. | ||
*/ | ||
public static <R extends Record, Context> ConstructingObjectParser<R, Context> forRecord( | ||
String name, | ||
boolean ignoreUnknownFields, | ||
Class<R> recordClass, | ||
MethodHandles.Lookup lookup | ||
) { | ||
Function<Object[], R> builder = recordBuilder(recordClass, lookup); | ||
return new ConstructingObjectParser<>(name, ignoreUnknownFields, (args, context) -> builder.apply(args)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (For the future... if this were able to consume the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, looking into this some more, my own CPS requirements actually need this functionality, so that means this PR is insufficient for my own needs in its current form. |
||
} | ||
|
||
private static <R extends Record> Function<Object[], R> recordBuilder(Class<R> recordClass, MethodHandles.Lookup lookup) { | ||
Class<?>[] ctorArgs = Arrays.stream(recordClass.getRecordComponents()).map(RecordComponent::getType).toArray(Class<?>[]::new); | ||
MethodHandle ctor; | ||
try { | ||
ctor = lookup.findConstructor(recordClass, MethodType.methodType(void.class, ctorArgs)); | ||
} catch (NoSuchMethodException | IllegalAccessException e) { | ||
throw new IllegalStateException("Cannot access record constructor", e); | ||
} | ||
return (args) -> recordClass.cast(constructRecord(args, ctor)); | ||
} | ||
|
||
@SuppressForbidden(reason = "We can't use invokeExact because we don't know the argument types statically") | ||
private static Object constructRecord(Object[] args, MethodHandle ctor) { | ||
Object result; | ||
try { | ||
result = ctor.invokeWithArguments(args); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Typically this area is rather performance sensitive, not sure how much we have to worry here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wondering how There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The javadocs say
I could perhaps do the |
||
} catch (Throwable e) { | ||
throw new IllegalStateException("Unable to call record constructor", e); | ||
} | ||
return result; | ||
} | ||
|
||
/** | ||
* Build the parser. | ||
* | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about restructuring this a bit for readability and have
recordBuilder
return the actual builderBiFunction
as expected byConstructingObjectParser
using<R extends Record> MethodHandle recordConstructor(Class<R> recordClass, MethodHandles.Lookup lookup)
and<R extends Record, Context> BiFunction<Object[], Context, R> recordBuilder(MethodHandle ctor)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I thought the same thing actually. I should do that.