Take entity class transformations into account during Hibernate proxy pre-generation #24059
Conversation
quarkus-bot
bot
added
area/arc
yrodiere
force-pushed
the
proxies-after-field-access-enhancement-i8932
branch
2 times, most recently
from
8574862 to
5632b54
Compare
yrodiere
force-pushed
the
proxies-after-field-access-enhancement-i8932
branch
from
5632b54 to
371d7bb
Compare
|
#24362 was merged, upgrading to ORM 5.6.7.Final, so tests added in this PR should now pass. Marking as ready for review, let's see what CI thinks of that... |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
yrodiere
force-pushed
the
proxies-after-field-access-enhancement-i8932
branch
2 times, most recently
from
b37aaae to
e93978f
Compare
This comment has been minimized.
This comment has been minimized.
|
I'm investigating the test failures. |
yrodiere
force-pushed
the
proxies-after-field-access-enhancement-i8932
branch
from
e93978f to
2d1d648
Compare
|
|
|
One build step of resteasy reactive seems to be depending on EDIT: Also, I'll try to only push once I manage to run the full build locally. Not sure how long the build will take locally, but it seems these changes have far-reaching implications so I'll have to go through that... |
yrodiere
force-pushed
the
proxies-after-field-access-enhancement-i8932
branch
from
2d1d648 to
4408a5b
Compare
yrodiere
force-pushed
the
proxies-after-field-access-enhancement-i8932
branch
2 times, most recently
from
d976db6 to
de6c767
Compare
|
Alright, I think I finally managed to solve the circular dependency problem in a clean way. I had to add a feature to bytecode recorders, though: the ability to define constants. I updated the PR description to explain the full reasoning behind all changes in this PR. @gsmet This ended up touching areas I'm not very familiar with... would you mind having a look? Or should I ask Stuart? As I understand he's a bit busy these days (well, so are you, but... :D) |
…es between build steps Compared to simply passing the value to a recorder proxy, defining a constant allows a build step A to push information to code recorded by another build step B, without introducing a dependency from build step B to build step A. This can be useful in complex dependency graphs.
This is a hack to avoid introducing circular dependencies between build steps. If we just passed the proxy definitions to #build as a normal build item, after the changes in the next commits, we would end up with the following dependencies: #pregenProxies => ProxyDefinitionsBuildItem => #build => BeanContainerListenerBuildItem => Arc container init => BeanContainerBuildItem => some RestEasy Reactive Method => BytecodeTransformerBuildItem => build step that transforms bytecode => TransformedClassesBuildItem => #pregenProxies Since the dependency from #preGenProxies to #build is only a static init thing (#build needs to pass the proxy definitions to the recorder), we get rid of the circular dependency by defining a constant to pass the proxy definitions to the recorder. That way, the dependency is only between #pregenProxies and the build step that generates the bytecode of bytecode recorders. See https://quarkusio.zulipchat.com/#narrow/stream/187038-dev/topic/Recorder.20introducing.20cyclic.20dependency.20between.20build.20steps
…ation In particular, don't forget that class transformation can add getters/setters to entities, and those must be proxied as well.
…ymorphic toOne association
yrodiere
force-pushed
the
proxies-after-field-access-enhancement-i8932
branch
from
de6c767 to
4b7b897
Compare
gsmet
added
the
triage/waiting-for-ci
|
Thanks @gsmet! |
quarkus-bot
bot
added
kind/bugfix
and removed
triage/waiting-for-ci
|
awesome :) thank you both |
Fixes #8932
Creating as draft, because this fix will only work after hibernate/hibernate-orm#4845 is merged, released, and we upgrade to the corresponding Hibernate ORM version.=> DoneThe core of this PR is simple enough to understand: the Hibernate ORM bytecode enhancer works on class definitions, and will only enhance those methods it can see in those class definitions. So any methods added by Panache (such as getters/setters) must be added before the Hibernate ORM bytecode enhancer starts to work, and thus we need to pass already-transformed classes to the Hibernate ORM bytecode enhancer, not the original bytecode.
This requires a bit of reorganization of build steps to pass the transformed bytecode to the build step that performs Hibernate ORM proxy generation. This also requires tuning the input to the proxy generation (again, to pass transformed bytecode).
One challenge was that the reorganization of build steps led to a circular dependency in some edge cases involving resteasy reactive. That circular dependency was artificial, however: one of the dependencies between build steps was only there to pass information that would only be passed to bytecode recorders and used during static init... so nothing that prevents build steps from running out of order.
To solve the circular dependency, I had to add a feature to bytecode recorders: the ability to define constants in recorders to break circular dependencies between build steps. See the second commit "Ability to define constants in recorders to break circular dependencies between build steps".
Compared to simply passing the value to a recorder proxy, defining a constant allows a build step A to push information to code recorded by another build step B, without introducing a dependency from build step B to build step A.
I know this new bytecode recorder feature would deserve a separate PR, but I thought it would be better to see a practical use of that feature in the same PR. And you can see a practical use in the third commit: "Pass pre-generated proxies to HibernateOrmRecorder as a constant".