-
Notifications
You must be signed in to change notification settings - Fork 8.9k
support @GlobalTransactional and @GlobalLock in parent class and interface #7794
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
base: 2.x
Are you sure you want to change the base?
Conversation
…er to the last,so it can override the io.seata.saga.rm.SagaResourceManager
…s and interface
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 2.x #7794 +/- ##
============================================
+ Coverage 69.96% 70.93% +0.97%
- Complexity 992 993 +1
============================================
Files 1322 1322
Lines 50177 50237 +60
Branches 5932 5947 +15
============================================
+ Hits 35104 35634 +530
+ Misses 12143 11666 -477
- Partials 2930 2937 +7
🚀 New features to boost your workflow:
|
| * @return the annotation | ||
| * @param <A> | ||
| */ | ||
| public static <A extends Annotation> A findAnnotationInHierarchy( |
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.
I believe method-level annotation data should take precedence. Only if a method does not have that annotation should we check the corresponding method in the superclass. This ensures that if the same annotation appears with different configured values, the value on the method is the one that ultimately applies.
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.
you are right! the invoker will check the actual class first just as you wish
code is here:
private <A extends Annotation> A findAnnotation(Method method, Class<?> targetClass, Class<A> annotationClass) {
A anno = getAnnotation(method, targetClass, annotationClass);
if (anno == null) {
anno = ReflectionUtil.findAnnotationInHierarchy(
targetClass, method.getName(), method.getParameterTypes(), annotationClass);
}
return anno;
}| Class<?> superClass = targetClass.getSuperclass(); | ||
| while (superClass != null && superClass != Object.class) { | ||
| try { | ||
| Method superMethod = superClass.getDeclaredMethod(methodName, paramTypes); |
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.
Should we attempt to read the annotation from the superclass instead of only checking the method itself?
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.
update, pls have a check
…parent class and all interace
Ⅰ. Describe what this PR did
support
@GlobalTransactionaland@GlobalLockin parent class and interfaceⅡ. Does this pull request fix one issue?
fixes #7793
Ⅲ. Why don't you add test cases (unit test/integration test)?
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews