ClassUtils.getUserClass should support ByteBuddy-generated proxies (e.g. from Hibernate 5.3) [SPR-16569]

[spring-projects-issues]

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.getUserClass to fail to detect it as a proxy because the class name does not contain a $$. This breaks the AbstractPersistable.equals method, triggering a test failure for AbstractPersistableIntegrationTests.

---

Issue Links:

• Support for Hibernate ORM 5.3 [SPR-16303] #20850 Support for Hibernate ORM 5.3
• DATAJPA-1347 Improve proxy detection to properly detect Hibernate 5.3 proxies

1 votes, 10 watchers

[spring-projects-issues]

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 ClassUtils then, wouldn't it?

[spring-projects-issues]

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.

[spring-projects-issues]

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 ClassUtils instance (besides the one in Spring Framework and the one we have in ….data.repository.util) working around the issue but feels like other ecosystem projects might run into a ByteBuddy proxy sooner or later so that a more fundamental fix is preferred, I guess.

[spring-projects-issues]

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?

[spring-projects-issues]

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 HibernateProxy interface, which of course would be weird in Spring Framework Core.

[spring-projects-issues]

Jens Schauder commented

Rafael seems to recommend introducing a marker interface https://stackoverflow.com/a/48336469/66686 which I guess is HibernateProxy: https://stackoverflow.com/a/11228887/66686

[spring-projects-issues]

Oliver Drotbohm commented

For Spring Data I have a fix that removes the detection of the proxy and replaces it by simply checking for assignability.

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 ClassUtils to work with the new proxy types as well so that we can keep our code untouched, mostly because otherwise we'd literally have to check and fix all our usages of ClassUtils.getUserClass(…), not only for JPA and Hibernate but literally every place we deal with user types. Quite fundamentally our very own ClassTypeInformation in Spring Data Commons for example.

[spring-projects-issues]

Oliver Drotbohm commented

Yeah, that marker interface is not particularly helpful as it means you have to know what you're looking for.

[spring-projects-issues]

Juergen Hoeller commented

What specifically are you using the ClassUtils.getUserClass result for? For quite a few annotation lookup and even generics resolution cases, the proxy class could work as well as long as it retains all that information on the generated subclass (which ByteBuddy seems to do)...

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?

[spring-projects-issues]

Oliver Drotbohm commented

It's basically everywhere we need an exact match to some user type so that we need to compare using equals(…). That involves cache lookups as much as cases when we need to e.g. write user type information into a store to eventually be able to read the – in this case – MongoDB document back in again.

From what I've seen Hibernate uses a nested …MyEntityClass$HibernateProxy$yguZekHB (the latter just being randomly created characters). So it still involves 2 $ but they cannot be distinguished from multiply nested classes unfortunately :(. That aside, AFAIHS the naming strategy is completely pluggable in ByteBuddy.

[spring-projects-issues]

Juergen Hoeller commented

We can reliably detect (and ignore) inner classes through Class.getEnclosingClass() though... so could also check for a single '$' sign and consider it a generated class if there is no enclosing class. We could go even further and compare whether the super class name is the beginning of the generated class name, quite clearly knowing that we're dealing with a generated subclass then.

Could you please create an SPR ticket for this?

[spring-projects-issues]

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.

[spring-projects-issues]

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.

[spring-projects-issues]

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.

[spring-projects-issues]

Sanne Grinovero commented

Hi all,
I'd like to clarify some things from the Hibernate perspective and our plans. First off, we don't use CGLIB anymore since many years, it has been deprecated for long end then removed; it's no longer available as an option, nor seen among our dependencies since a long time.

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.
Raphael contributed the Byte Buddy integration (thanks again, that was awesome!) and with Hibernate ORM 5.2 it became an option, users being given a new configuration option to choose between Javassist and Byte Buddy but having still Javassist as the default. With Hibernate ORM 5.3 it became the default - so that's what made you notice now.

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 ClassUtils.getUserClass(…) and ensure to make consistent use of it across all framework layers. I'm sure you've seen users getting confused though, not least this approach is concerning from a JVM efficiency point of view.

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 ClassUtils for equality method, but be aware that even if the name is "clean" and matches the user domain class exactly (as it might be the one) this doesn't necessarily mean that the class was not enhanced.

HTH