-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
HHH-19229 Faster checks in ReflectHelper #9835
base: main
Are you sure you want to change the base?
HHH-19229 Faster checks in ReflectHelper #9835
Conversation
Thanks for your pull request! This pull request appears to follow the contribution rules. › This message was automatically generated. |
8185714
to
247843c
Compare
The oracle job failure is an occasional, transient problem - probably not anything wrong with your PR. The code analysis though you will need to resolve. |
247843c
to
4168f91
Compare
4168f91
to
789d3c9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a couple comments. I found it weird that tests were passing since the checks you implemented were not correct, and I found some duplicated logic in org.hibernate.property.access.internal.AccessStrategyHelper#checkIsMethodVariant
.
Could you please delete this method and ensure we use ReflectHelper#verifyNoIsVariantExists
instead?
&& declaredMethod.getName().startsWith("is") | ||
&& declaredMethod.getName().regionMatches(0, stemName, 0, stemName.length() ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code style and correct region-matching.
&& declaredMethod.getName().startsWith("is") | |
&& declaredMethod.getName().regionMatches(0, stemName, 0, stemName.length() ) | |
&& declaredMethod.getName().startsWith( "is" ) | |
&& declaredMethod.getName().regionMatches( 2, stemName, 0, stemName.length() ) |
&& declaredMethod.getName().startsWith("is") | ||
&& declaredMethod.getName().regionMatches(0, stemName, 0, stemName.length() ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
&& declaredMethod.getName().startsWith("is") | |
&& declaredMethod.getName().regionMatches(0, stemName, 0, stemName.length() ) | |
&& declaredMethod.getName().startsWith( "get" ) | |
&& declaredMethod.getName().regionMatches( 3, stemName, 0, stemName.length() ) |
Make the checks in ReflectHelper faster.
See detailed description in Jira
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license
and can be relicensed under the terms of the LGPL v2.1 license in the future at the maintainers' discretion.
For more information on licensing, please check here.