-
Notifications
You must be signed in to change notification settings - Fork 2.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix the non-default constructor mechanism of bytecode recording [2.13] #31212
Conversation
Bytecode recorders attempt to match constructor parameters to properties and skip setting properties that were initialized by the constructor. The assumption is that if the constructor has a parameter with the same name as the property, the property is set by the constructor. Otherwise it needs to be set using a setter. This matching was not implemented for the non-default constructor mechanism. This commit fixes that.
Fix the test case for quarkusio#31153
- Change creation of a large collection in `BytecodeRecorderTestCase.java` from using a loop to using `Collections.nCopies`
Bytecode recorders attempt to match properties to constructor parameters by the property/parameter name. However, parameters do not always have to have their names available. This commit fixes the bytecode recorder matching mechanism to avoid parameters that do not have an available name.
Thanks for your pull request! The title of your pull request does not follow our editorial rules. Could you have a look?
|
@rsvoboda what's your take on this one for 2.13? It's a bug and could happen for people writing their own extensions. |
Why was this dedicated PR created when #31183 has 2.13 backport label? |
@rsvoboda because it doesn't apply cleanly |
So the backport label should be removed from #31183 |
I don't have comments regarding the code changes. On the other hand it's kinda expected that people write extensions against the latest community. jobrunr is using 2.15.3.Final - https://github.com/jobrunr/jobrunr/blob/master/gradle.properties#L7 So I'm not super convinced that we need to merge this for 2.13. |
(but still open to the discussion ;) ) |
I'm okay in not backporting this |
I'll close it for now |
Bytecode recorders attempt to match constructor parameters to properties
and skip setting properties that were initialized by the constructor.
The assumption is that if the constructor has a parameter with the same
name as the property, the property is set by the constructor. Otherwise
it needs to be set using a setter.
This matching was not implemented for the non-default constructor mechanism.
This commit fixes that.