ClassUtils.getUserClass should support ByteBuddy-generated proxies (e.g. from Hibernate 5.3) [SPR-16569] #21111
Comments
|
Oliver Drotbohm commented That should be of interest for Juergen Hoeller as well, as it would be cool if that was fixed in Spring's |
|
Juergen Hoeller commented Indeed. I suppose Hibernate 5.3 is using some other proxy generator then? We're really only handling CGLIB proxies there... Anyway, we got Hibernate 5.3 on our immediate roadmap for Spring Framework 5.1, so it's definitely going to be a topic. |
|
Oliver Drotbohm commented Looks like it's their switch to ByteBuddy that causes other classnames to be created now. We could of course add a third |
|
Oliver Drotbohm commented I've pinged Rafael Winterhalter on Twitter for some input on how to best detect those types reliably. If Jürgen thinks he'd like to fix this in Spring Framework, shall we move the ticket over to it then? |
|
Jens Schauder commented Hibernate uses Byte Buddy by now. It used Javaassist before which seemed to use sufficient similar class names. For Spring Data I have a fix that removes the detection of the proxy and replaces it by simply checking for assignability. For detecting Hibernate Proxies probably the correct way would be to check for the |
|
Jens Schauder commented Rafael seems to recommend introducing a marker interface https://stackoverflow.com/a/48336469/66686 which I guess is |
|
Oliver Drotbohm commented
That's quite a significant change in semantics that would make make type hierarchies now match when they didn't before, completely unrelated to the proxy issue. I'd actually like to make Framework's |
|
Oliver Drotbohm commented Yeah, that marker interface is not particularly helpful as it means you have to know what you're looking for. |
|
Juergen Hoeller commented What specifically are you using the In any case, what do the ByteBuddy-generated class names look like? They'll have to differ from the user-declared superclass in some form, so maybe we can identify some simple rules to identify whether we need to go with the superclass instead? |
|
Oliver Drotbohm commented It's basically everywhere we need an exact match to some user type so that we need to compare using From what I've seen Hibernate uses a nested |
|
Juergen Hoeller commented We can reliably detect (and ignore) inner classes through Could you please create an SPR ticket for this? |
|
Oliver Drotbohm commented Would we deliberately accept not being able to detect proxy types of inner classes? The name inspection might do the trick, although ByteBuddy also exposes a prefixing naming strategy so the proxy class could also start with some random extension and end with the super class' name. I've just moved the ticket over to the framework as is. |
|
Juergen Hoeller commented Well, an inner class as user class would still get accepted: We'd just assume that the given class is a generated class if it contains a '$' and doesn't have an enclosing class. Since a generated proxy class cannot be an inner class itself as far as I see (it just possibly extends an inner class that we'd return then), we should be safe there. As for a name check with the super class, we could also accept a match at the beginning or at the end. I'm not sure a name comparison is even necessary though. |
|
Rafael Winterhalter commented Just from the side-line: Byte Buddy aims to not leave any traces in a class to make things work out with class loaders and this is even more important with the JPMS where the boundaries can even be set within a class loader such that a proxy class's module does not read the Byte Buddy module. Also, the naming strategy is pluggable, as it is mentioned. I would recommend against relying on $$ as an indicator as many Scala types contain those for their function implementations. I would argue that you cannot reliably create some universal proxy detection as those classes might even be detected at runtime. Also, many people have started using Byte Buddy to generate non-proxy classes to avoid boilderplate. Even if you could detect Byte Buddy classes, this would not necessarily give you the desired results. The most reliable way would be to check for an implementing interface such as the Hibernate proxy interface but this requires more work by the means of knowing all frameworks that you want to detect proxies for such as Hibernate. |
|
Sanne Grinovero commented Hi all, The initial replacement was Javassist, which served us well for some time but we're switching to Byte Buddy for various reasons - among these a more proactive support for new JDK versions but also a wish to have the instrumented classes "clean": Javassist would introduce a runtime dependency on itself on all classes it would enhance. Javassist is still an option - and sadly is still a hard dependency even if you choose to use Byte Buddy for entity enhancement because of unrelated work in progress - but let's please keep polishing the Byte Buddy integration and work out issues such as this one as we plan to finalize removement of Javassist in Hibernate 6, making Bute Buddy the default and only option. More importantly, we don't call this "proxying" anymore as we're moving away from that, rather preferring "enhancing" of the class definition itself. Introducing a dyncamically generated sub-class has alwasy created significant complexity, not least the very reason as Oliver mentioned to need to have a I trust you already know that Hibernate ORM also releases Maven, Gradle and other tools to let users enhance their entities at compile time. Some containers like WildFly actually do apply the class enhancement phase on the classes of the domain model automatically before loading the deployment. In both cases the class definitions are enhanced before the class definitions are loaded so you don't end up having weirdly named classes extending the user domain, you just have the user domain which happens to have the right bytecode embedded. Understandably we want to support also a reasonable fall-back strategy for people who are not running in such smart containers and also decided to not apply build time enhancement so I agree on introducing something in our enhanced classes to "mark" them if that helps, but keep the above in mind if you prefer a strategy based on class names. I guess you could use the classname strategy to see if you actually need to use HTH |
|
Juergen Hoeller commented We'll have to revisit the conventions for this, so let's schedule it for Spring Framework 5.1 (along with other Hibernate 5.3 support topics). It is important to note that we don't need to generally detect proxies or generated classes there. We just want to identify the original user class for matching purposes (e.g. equality comparisons or hash keys), and only really for the common cases of runtime-generated subclasses. If in doubt, |
|
Juergen Hoeller commented Sanne Grinovero, is there a particular reason for the current naming strategy for proxy classes in Hibernate 5.3? Would it be an option to reintroduce the "$$" convention, or to formally document your new naming strategy (rather than leaving it as an implementation detail) so that we can support it by convention, without having to check for a Hibernate-specific marker interface? |
|
Sanne Grinovero commented Hi ~juergen.hoeller we're certainly open to options, but I wouldn't be able to commit on having "$$" in the name for the long run. Detailed reasons are off topic here, I can point to some more research if you're interested but for the purpose of this JIRA suffice it to say it would make memory consumption of the metadata in the JVM higher, and it's an area in which we're trying hard to "slim down"; working with our performance team and the OpenJDK team on such things. This wasn't the reason to have changed the name, just thinking we can't promise to keep such a naming strategy for the long term. Opened https://hibernate.atlassian.net/browse/HHH-12384 we can certainly agree to keep it stable for at least another minor (if we manage to fix it, haven't looked at feasibility yet). Would you possibly prefer that Hibernate ORM provided its own |
|
Sanne Grinovero commented I've been championing your case within the Hibernate team, yet since we all agreed that you shouldn't rely on the classname - especially not in the future - I wasn't allowed to simply revert to the old naming strategy. I resolved https://hibernate.atlassian.net/browse/HHH-12384 by introducing a global configuration flag. The ideal solution is for you to not need this at all, but until you have a better solution and for the sake of existing code you will need to enable it explicitly. For details: I wonder if that's going to be useful at all, but I guess it's better than nothing. |
|
Oliver Drotbohm commented We've introduced a dedicated, extensible proxy detection mechanism which Spring Data JPA now uses to check for |
|
Juergen Hoeller commented Closing this ticket for the time being... defining |
|
alienisty commented This is also affecting spring-data-rest-core. PersistentEntitiesResourceMappings uses ClassUtils.getUserClass() to detect proxies and therefore it fails identifying the RepositoryAwareResourceMetadata for Hibernate proxied classes in lazy loaded relationship. I think that if the proxy detection logic were implemented as a pluggable strategy, rather than using a static utility, the problem would be easily fixed. Spring Boot could also autoconfigure the correct strategy according to what is being used. |
|
alienisty commented I've been testing a patched implemetation of getUserClass as such:
public static Class<?> getUserClass(Class<?> clazz) {
Class<?> superclass = clazz.getSuperclass();
if (superclass != null && superclass != Object.class && superclass != clazz.getEnclosingClass()) {
String className = clazz.getName();
String superclassName = superclass.getName();
if (className.startsWith(superclassName)
&& className.charAt(superclassName.length()) == INNER_CLASS_SEPARATOR) {
return superclass;
}
}
return clazz;
}So far is picking correctly both Spring CGLIB-generated proxyies and Hibernate generated proxies |
spring-projects-issues
added
status: declined
Jens Schauder opened SPR-16569 and commented
Regarding the test failure for the latest Hibernate Snapshots (5.3.0-SNAPSHOT):
Hibernate seems to have changed the way class names get generated for their proxies. This causes
ClassUtils.getUserClassto fail to detect it as a proxy because the class name does not contain a$$. This breaks theAbstractPersistable.equalsmethod, triggering a test failure forAbstractPersistableIntegrationTests.Issue Links:
1 votes, 10 watchers
The text was updated successfully, but these errors were encountered: