SPR-8190: Added support for Byte Buddy AOP proxies and bean subclassing instantiation strategy.

[raphw]

The suggested proxy makers are working but this is still work in progress. I require input for any further steps:

There is still no mechanism to specify a specific another byte code provider. Also, there is potential of sharing code for the cglib and Byte Buddy proxy classes unit tests but for this PR, i tried to not change any existing code. There are no explicit tests for the cglib bean proxy maker which could serve as a specimen which is why this call is untested.

[pivotal-issuemaster]

@raphw Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

[pivotal-issuemaster]

@raphw Thank you for signing the Contributor License Agreement!

[danieljipa]

Is this PR going to be merged any time soon ? Hibernate now uses ByteBuddy and Spring data jpa project has difficulties with this: https://stackoverflow.com/questions/62538768/error-deleting-entitiy-with-spring-data-jpa

[raphw]

I'd love this obviously, but someone would need to have a look. I just updated the branch to have it in sync with the current master to reduce the barrier for this.

[jhoeller]

@raphw Thanks for updating the PR, this indeed brings it to our attention again :-)

A couple of comments from a first glance:

• Is the removal of "-Werror" intentional? Please rather use @SuppressWarnings wherever necessary.
• ClassUtils does not seem to be able to do anything general with ByteBuddy proxies, so the ByteBuddy constant seems out of place there. Let's put this within the ByteBuddy AOP implementation which actually uses that specific class naming pattern.
• As far as I can see, the PR doesn't cover configuration class processing at all. While we could keep using CGLIB for that purpose, it seems strange not to apply ByteBuddy there as well.

@danieljipa To the best of my understanding, this PR is not directly related to the Spring Data JPA issue. The problem there is detecting Hibernate's specific naming pattern for ByteBuddy-generated classes, a problem appearing now because our general CGLIB-capable utility methods (e.g. ClassUtils.getUserClass) were able to handle CGLIB-generated Hibernate entity classes as well since they happened to use the same common CGLIB naming pattern... whereas ByteBuddy leaves class naming completely up to the user, so we'd have to hard-code Hibernate's specific naming pattern or use some Hibernate-specific API to detect it.

[raphw]

Hello Juergen, thanks for your promt reaction! I will look into the configuration class processing. It must have slipped my eye when I originally implemented this PR. I added the infix to the ClassUtil to correlate it with cglib. For the final PR, I'd suggest to create some form of abstract base abstraction used by both cglib and Byte Buddy anyways since I had to duplicate a bunch of code. I'd be happy to do it but wanted to avoid touching a lot of existing code and I was not sure if you preferred the duplication anyways.

As for the Hibernate proxies: this is correct. I'd recommend you to use the internal utilities. Depending on using the build-plugin, classes might be subclassed or instrumented where in the latter case, no names are changed, of course. When detecting cglib names, you might also discover any class that is created by cglib but that is not meant to be a proxy. Also, the $$ use in names has increased also for use of other synthetic classes. Hibernate uses a marker interface what seems more reliable. You could just check if this interface is implemented which it is not if it is not even found on the class loader.

[raphw]

@jhoeller I chased the last bugs and now the entire project builds with all tests if using Byte-Buddy instead of cglib. I hope the size of the change set is not too daunting. The PR offers a Byte-Buddy-based proxy engine for all seven generation sites within spring-framework in a way that would even allow to remove cglib altogether. This also required some light refactorization of several unit tests that tested for the presence of a cglib proxy. I changed these tests to rather check for the presence of a class-based proxy independent of the proxy engine's implementation. As a consequence, no unit test within Spring framework assumes a particular proxy engine being used. This also renders a nice prove upon Spring not depending on a particular implementation since you now can build the entire project without using cglib.

The amount of additional code can easily be reduced by refactoring the related Cglib... and ByteBuddy... implementations of proxy generators to share a common base that implements the now duplicated code. I have kept the two implementation separate so far to keep the change set simpler to follow. Also, I was not sure if you'd prefer this approach.

Yesterday I still had a single test failure which I figured out today and I suspect a potential bug in spring-context as the cause. The failing test was testValueInjectionWithProviderFields in AutowiredConfigurationTests. Byte Buddy follows the JLS for it's subclasses; since the proxied ValueConfigWithProviderFields is package-private and is extended by a public class, it is required to override the unproxied setName2 method with a visibility bridge. cglib does not generate bridge methods which is why the setName2 method is overridden by Byte Buddy but not cglib. As a consequence, Spring gets confused about its own resolution and throws an exception. To avoid this failure, I disabled bridge class generation in ByteBuddyConfigurationClassEnhancer to make the test pass. Ideally, the proxy should be transparent, however, but I avoided a fix attempt to avoid further increasing this change set.

Please let me know if you see a chance to move this change forward and if you'd like me to refactor the Cglib... and ByteBuddy... implementations to share more common code in an abstract base parent. I'm happy to invest the time if the PR has a chance of getting merged.

[danieljipa]

Any news on this PR ? Thanks

[raphw]

I just rebased this onto master to resolve the merge conflicts. Maybe, with the Java 16 restrictions coming up, this would be a good way to revisit this PR?

[raphw]

It's soon gone two years. I understand if this is not considered, I am of course personally biased in wanting to include it, I just wonder if there's a point in keeping this PR open and rebase it, even improve it, or if I should simply close it. Thanks for any feedback.

[andrei-ivanov]

the silence on this PR from the team members is very weird 😕

[jhoeller]

@andrei-ivanov sorry for the silence, it's been a while indeed...

We are in the process of evaluating our options for Spring Framework 6 here, taking AOT generation of proxy classes into account (for native executables but also for regular JVM scenarios), and researching custom special-purpose class generation versus building on ByteBuddy's general facilities. All in all, we are planning towards a major overhaul of our legacy CGLIB usage, but only one such overhaul as a central theme in 6, and we can't quite commit to a specific arrangement yet. It's not even clear whether we'll keep CGLIB as a backwards compatibility option at all; we might replace it with a single new mechanism in 6.

@raphw I know it's painful to keep updating the PR for the time being. Feel free to let it sit for a while, it's fine inspiration for us in any case, and the detailed analysis you've been providing is very useful already. We do appreciate your efforts there but we're certainly not expecting you to spend extra cycles on the PR until we sorted out our overall class definition strategy for 6.

[raphw]

Thanks for the overview, I think that's a good issue to adress. I'm looking into a generic way of preprocessing proxy classes for Byte Buddy (which has a build plugin which could also be used for this very same purpose by you), maybe to give another incentive. Keep up the good work and let me know if you wonder about something related to code generation.

[jhoeller]

Since we eventually went with a full CGLIB fork for Spring Framework 6.0, including AOT support for capturing generated classes etc, I do not see us proceeding with ByteBuddy any time soon, not least of it all in order to keep our maintenance efforts at a manageable level. From that perspective, it seems appropriate to close this PR. Thanks @raphw for all your efforts there so far!