Consider alternative RuntimeHints API

[philwebb]

The RuntimeHints API now has quite a bit of use across the portfolio and a few common patterns are starting to emerge. We might be able to refactor the registration API to make a few of these common patterns easier.

Here are some of the common calls that we're seeing:

Introspect vs Invoke methods

Calling registerMethod allows introspection and invocation hints to be added. The following calls are quite common but it's not always obvious what they will do:

hints.reflection().registerMethod(method); // registers with invoke
hints.reflection().registerMethod(method, hint -> {}); // registers with introspect
hints.reflection().registerMethod(method, hint -> {hint.withMode(ExecutableMode.INTROSPECT)}); // registers with introspect
hints.reflection().registerMethod(method, hint -> {hint.withMode(ExecutableMode.INVOKE)}); // registers with invoke

We're quite often seeing the hint builder lambda being pulled into a static final to aid readability:

hints.reflection().registerMethod(method, INTROSPECT);
hints.reflection().registerMethod(method, INVOKE);

Additionally, the ExecutableHint class uses a List of ExecutableMode values, but since INVOKE also implies INTROSPECT this should not be necessary.

Read vs Write

Similar to above, the registerField method doesn't immediately indicate if reading or writing is allowed:

hints.reflection().registerField(field); // Allows write
hints.reflection().registerField(field, hint -> {}); // Allows read

TypeReference.of()

Often the API expects a TypeReference parameter but the user has either a Class<?> or a String. This results in a lot of calls to TypeReference.of in the caller code:

hints.reflection().registerType(TypeReference.of(type), hint -> {});

Applying the same hints to multiple types

It's appears that often the same hints need to be applied to multiple types. For example in MessagingAnnotationsRuntimeHints and JacksonRuntimeHints.

Often the caller uses a Stream to do this:

Arrays.stream(type1, type2, type3).forEach(type -> hints.reflection()....);

Reflective method lookups

Slightly less common, but still used now and again is calling ReflectionUtils to lookup a Method so that it can be passed to the API. A (now outdated) example can be found in CloudFoundryWebEndpointServletHandlerMapping.

Method linksMethod = ReflectionUtils.findMethod(CloudFoundryLinksHandler.class, "links",
					HttpServletRequest.class, HttpServletResponse.class);
			Assert.state(linksMethod != null, "Unable to find 'links' method");
			hints.reflection().registerMethod(linksMethod);

Similarly, withMethod calls often need to create a List for the parameter types. Sometimes Collections.emptyList() is used, sometimes a Stream is used with a mapping function to convert to a TypeReference.

See InstrumentedMethodTests for some examples.

hints.reflection().registerType(String.class, typeHint -> typeHint.withMethod("toString", Collections.emptyList(), methodHint -> methodHint.setModes(ExecutableMode.INTROSPECT)));

Naming consistency

As the API has been expanded some naming inconsistencies have crept in. For example, ProxyHints refers to "JDK proxies", where as SerializationHints refers to "Java serialization". The ReflectionHints class provides a typeHints() Stream, where as most of the other APIs don't end with hint (e.g. SerializationHints.javaSerialization().

Resource include/excludes

The ResourcePatternHints is a container for includes and excludes and multiple ResourcePatternHints can be registered. It's possible that callers may expect that these includes and excludes are related, however, when the JSON is written all excludes and all includes are included together. See ResourceHintsPredicates.AggregatedResourcePatternHints for an example of the flattening that needs to occur when using the pattern hints.

[snicoll]

Regarding "Introspect vs Invoke methods" and "Read vs Write" the empty customizer being different than the method that takes no customizer is an oversight and can easily be fixed.

This results in a lot of calls to TypeReference.of in the caller code:

Can you expand what you mean by "a lot"?

A (now outdated) example can be found in

It's outdated as we're not supposed to do method lookups manually. The intention of the API is to provide a way to specify a method handle that was retrieved as part of some processing.

Naming consistency

That's a good point. ProxyHints used to cover both classes and JDK proxies. I think it makes sense to keep the proxy bits as it may very well evolve in the future.

however, when the JSON is written all excludes and all includes are included together. See

That is a bug that I am aware of and kept forgetting to report.

[philwebb]

Can you expand what you mean by "a lot"?

Perhaps the wrong phrase. More than none. (edit: Here's a good example)

It's outdated as we're not supposed to do method lookups manually. The intention of the API is to provide a way to specify a method handle that was retrieved as part of some processing

True, but I found method lookup also used in quite a few tests.

[philwebb]

I've pushed an experimental alternative API to https://github.com/philwebb/spring-framework/tree/gh-28977 for review.

The main difference is that there's now a fluent style API that can be used rather than the builder. There are two commits on that branch, the first changes the API and the second adapts the existing code to use it.

[snicoll]

Perhaps the wrong phrase. More than none. (edit: Here's a good example)

It's in the same ballpark as the rest of the feedback IMO. The current API is missing a registerTypes with a list of Classand that's also an oversight.

True, but I found method lookup also used in quite a few tests.

OK. I don't understand what the problem is with that.

[philwebb]

It's in the same ballpark as the rest of the feedback IMO. The current API is missing a registerTypes with a list of Classand that's also an oversight.

I was trying to list interesting things I'd found in case we didn't want to take on a new API and instead wanted to make a few tweaks to the existing one.

OK. I don't understand what the problem is with that.

There's not a problem, but it it's easy to support method lookup in the API then it might be worth doing for the tests, even if it's not used much in src/main.

FileNativeConfigurationWriterTests is a good example.

Another one that isn't in test code is SchedulerFactoryBeanRuntimeHints

[sbrannen]

Often the API expects a TypeReference parameter but the user has either a Class<?> or a String. This results in a lot of calls to TypeReference.of in the caller code:

This is directly related to #28781. See discussions in comment section for details.

[philwebb]

Thanks for link @sbrannen, I'd forgotten about that discussion. For the experimental API I consistently added Class<?>, String and TypeReference methods for all calls. Although it adds a few more lines to the code (6 more if you exclude javadoc), I don't personally think it adds much cognitive load since all the methods are named the same.

Here's an example where you can call forPublicFieldsIn(...) with whatever type you have.

I was a little worried about the String variants because there are some rules that the javadoc on TypeReference spells out. We could repeat that warning in the hint API javadoc, or provide a {@link ...} to TypeReference.of(String).

[sdeleuze]

I think there is a lot of added value in the various examples listed in the description, and this is very useful inputs for improving the API.

For example, I agree there are some places where we should probably provide some Class or String based variants for popular usage patterns, I have already identified some like onReachableType(Class<?>) which is missing, but I am not sure we should add x3 TypeReference / String / Class variants everywhere. I think Stéphane has already improved some other areas based on the inputs.

I guess it is a matter of taste, but I think I am in favor of doing a few improvements here and there on current API rather than changing most of it for the fluent one of https://github.com/philwebb/spring-framework/tree/gh-28977. Also keep in mind that the underlying GraalVM capabilities will evolve outside of our control, and we will likely need to adapt our APIs to follow. In that context, I have the feeling that the builder one is more future proof.

[mhalbritter]

To get another data point: I really like the API from Phil - especially the consistent register, when and for pattern, it reads really nice. However I can't predict how easy it will be to evolve in the future if the underlying model evolves. I'm happy to help refactoring the existing hints to the new API if we decide to use it :)

[philwebb]

and we will likely need to adapt our APIs to follow. In that context, I have the feeling that the builder one is more future proof.

Do we have any example of changes that have been made in the past? I'd be curious to know they types of changes we might face and in which ways the builder API is more resilient to them.

[sdeleuze]

They added conditions with reachable types (more conditions could come) and method querying in addition to invocation. More recently lambda serialization, see spring-attic/spring-native#1670.

[jhoeller]

After a lot of consideration, Stephane and I have taken several suggestions from here into #29011 - within the current API design style. It turns out that a few overloaded methods, e.g. taking the execution mode enum directly, bring a lot of value and make usage arguably as convenient as with a fluent variant - without breaking backwards compatibility, and with an API style that remains closer to the typical style in other core Spring mechanisms. With quite minimal changes, this takes us to a scenario where most use cases are covered by direct method calls without a builder, turning the builder into an advanced-only facility.

While there is certainly still room for further improvement, e.g. explicitly named methods for stronger semantic guidance, there are also benefits with the current style. Not least of it all, the typical/common need is first-class with direct registration methods at the hints API level, whereas advanced options - that most people don't need to care about - are separated into the builder.

Having spent quite a bit of time with the API design today, I rather strongly prefer the direct registration methods in terms of naming and argument ordering. With so few arguments commonly needed, a fluent API style cannot really show its strengths much, at the expense of nested calls and readability that is sometimes a bit backwards (e.g. registerRead().forField(field)), and with a lack of "register this with the common default" guidance.

With respect to the "JDK proxy" versus "Java serialization" naming, that was actually quite intentional since those are the common terms that we use for those mechanisms in other places. A "JDK dynamic proxy" as opposed to a CGLIB proxy is what we use for interface-based proxying (provided by the JDK itself as opposed to Java classes generated by us), and "Java serialization" is the Java language mechanism for serialization. From that perspective, I'd rather keep those distinct terms.

In any case, the list of inconsistencies and inconveniences above has turned out to be very valuable. I'll keep this issue open for a few remaining points, in particular the resource pattern processing. However, from where we stand right now, we are likely to incorporate remaining suggestions into the current API design, with as little backwards compatibility impact as possible.

As for the further evolution of this facility, I find that hard to predict. Whether a particular style of API is better suited for long-term evolution along with GraalVM, that's hard to say. Since the builder style is at least not worse than a fluent API in that respect, I don't see this as a reason to switch to a different API style from where we are. Also, this API level is primarily for integrators, so pragmatically I see it from a "good enough" perspective in terms of usage guidance and maintainability.

[snicoll]

I've created issues for the two remaining items so I am going to close this now.