Since users might not have a concrete need to work with TypeReference, we should introduce ProxyHints.registerJdkProxy(String...) to simplify use of the API for registering a dynamic proxy based on fully qualified class names of the required interfaces.
Comment From: snicoll
I don't think we should do this. If we do, then every place that accepts a TypeReference must add a String variant, and I don't think that we should do that either as it would bloat the API quite a bit.
If you don't have a class handy, then a TypeReference is the way to go.
Comment From: sbrannen
I had already implemented the feature and committed it before I saw your comments, so I'm reopening this issue to discuss the topic within the team.
I don't think we should do this. If we do, then every place that accepts a TypeReference must add a String variant, and I don't think that we should do that either as it would bloat the API quite a bit.
If you don't have a class handy, then a TypeReference is the way to go.
I'd argue that TypeReference is more of an internal implementation detail that normal users need not be bothered with.
There may be some particular use cases where normal users need to work with TypeReference, but for the most common use cases users will be happier with supplying a Class or String. Furthermore, supporting String will remove lots of superfluous TypeReference.of(<string>) occurrences within user code.
Playing devil's advocate... we don't require users to supply a TypeReference for a Class, so why should we require them to supply a TypeReference for a String?
As a concrete example, this issue was a requirement for #28745 to improve the developer experience. The newly introduced AopProxyUtils.completeJdkProxyInterfaces(*) methods could of course be refactored to return TypeReference[] instead of Class<?>[] and String[], but that seemed unnatural and unnecessary.
Comment From: sbrannen
Team Decision: revert the changes made in commit b560c10d4cea3f9e149153078ec8c42e843d53dd and revisit this general topic (i.e., use of TypeReference in public-facing APIs) in the 6.0 M6 time frame.
Comment From: snicoll
I'd argue that TypeReference is more of an internal implementation detail that normal users need not be bothered with.
Why is that? It's a public class with public of convenient methods. Nothing there seems to indicate it is an internal implementation.
Furthermore, supporting String will remove lots of superfluous TypeReference.of(
) occurrences within user code.
If that is a problem, then let's address it separately. SimpleTypeReference is essentially a wrapper around the String value and I don't expect a lot of them to be created. Also, this should only be triggered at build time when AOT runs.
It doesn't feel right that we focus on one of the many methods that accept either a Class or a TypeReference in the hint package.
Playing devil's advocate... we don't require users to supply a TypeReference for a Class, so why should we require them to supply a TypeReference for a String?
Because using Class should be the vast majority of use cases and therefore it seemed a good idea to offer a shortcut for it. If you start offering a String-based approach, you're covering 99.9% of the use cases so why offer a variant for TypeReference then? You could then remove those and keep that indeed private (for real this time) but what happens if you want to create a hint for a class you've generated. Surely, going from ClassName to the name of the class shouldn't be that hard but it can be tricky if you have an inner class. That's why TypeReference was created in the first place.
As a concrete example, this issue was a requirement for https://github.com/spring-projects/spring-framework/issues/28745 to improve the developer experience. The newly introduced AopProxyUtils.completeJdkProxyInterfaces(*) methods could of course be refactored to return TypeReference[] instead of Class<?>[] and String[], but that seemed unnatural and unnecessary.
I don't think this method should have been designed this way. Rather, I think it should have returned a Customizer of the JdkProxyHint.Builder. This way you could register the proxy the usual way, passing the customizer of the builder to complete the set of interfaces that it needs. Doing it this way also means you don't have to care what input the caller use for user interface.
Comment From: sbrannen
I'd argue that TypeReference is more of an internal implementation detail that normal users need not be bothered with.
Why is that? It's a public class with public
ofconvenient methods. Nothing there seems to indicate it is an internal implementation.
We're not questioning whether TypeReference should be public, and we are not questioning its utility.
Rather, we are debating whether TypeReference should be so prevalent in the entry points into the AOT APIs.
Furthermore, supporting String will remove lots of superfluous TypeReference.of() occurrences within user code.
If that is a problem, then let's address it separately.
SimpleTypeReferenceis essentially a wrapper around the String value and I don't expect a lot of them to be created. Also, this should only be triggered at build time when AOT runs.
In this regard, we are not talking about performance but rather about the developer experience and the API presented to the user for the most common use cases.
It doesn't feel right that we focus on one of the many methods that accept either a
Classor aTypeReferencein the hint package.
Indeed, and that's why we reverted the change for M5 in order to discuss the broader topic for M6.
Playing devil's advocate... we don't require users to supply a TypeReference for a Class, so why should we require them to supply a TypeReference for a String?
Because using
Classshould be the vast majority of use cases and therefore it seemed a good idea to offer a shortcut for it.
That's a good point and explains the original rationale for the decision. In your absence, we realized we might not have the full picture. So input like this helps to shape the discussion.
If you start offering a
String-based approach, you're covering 99.9% of the use cases so why offer a variant forTypeReferencethen? You could then remove those and keep that indeed private (for real this time)
That is something the team wishes to explore in greater detail.
but what happens if you want to create a hint for a class you've generated. Surely, going from
ClassNameto the name of the class shouldn't be that hard but it can be tricky if you have an inner class. That's whyTypeReferencewas created in the first place.
We definitely do not plan to make existing use cases more difficult. Rather, our goal is to simplify common use cases and simplify the entry points into the AOT APIs. If there are existing use cases that effectively require the use of TypeReference outside of internal framework code, we would want to retain that functionality.
As a concrete example, this issue was a requirement for #28745 to improve the developer experience. The newly introduced AopProxyUtils.completeJdkProxyInterfaces(*) methods could of course be refactored to return TypeReference[] instead of Class<?>[] and String[], but that seemed unnatural and unnecessary.
I don't think this method should have been designed this way. Rather, I think it should have returned a
Customizerof theJdkProxyHint.Builder. This way you could register the proxy the usual way, passing the customizer of the builder to complete the set of interfaces that it needs. Doing it this way also means you don't have to care what input the caller use for user interface.
For that particular convenience feature, the team wanted to add new methods to AopProxyUtils while not making AopProxyUtils directly dependent on AOT-specific APIs. If we decide to follow your recommendations, we may want to move the code from AopProxyUtils to a new AOT-specific class in spring-aop.
In summary, there are many facets to discuss. In light of that, we should probably dedicate a portion of a team meeting to dive deeper into this topic.
Comment From: sbrannen
Superseded by:
-
28977
-
29011