Skip to content
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

bugfix: fix incorrect getAnnotation about class and method #2617

Merged
merged 22 commits into from
May 19, 2020

Conversation

funky-eyes
Copy link
Contributor

@funky-eyes funky-eyes commented Apr 26, 2020

Ⅰ. Describe what this PR did

bug fix for getAnnotation
1.类上有注解,方法无 可正常运行
2.方法有注解,类上无 可正常运行
3.类上方法上都有,优先读取方法上注解 可正常运行
4.基类方法调用时,排除基类调用 可正常运行
5.类重写基类方法,比如toString后,重写的方法属于本类方法,因此会开启全局事务.

Ⅱ. Does this pull request fix one issue?

fix #2562

Ⅲ. Why don't you add test cases (unit test/integration test)?

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@codecov-io
Copy link

codecov-io commented Apr 26, 2020

Codecov Report

Merging #2617 into develop will decrease coverage by 0.14%.
The diff coverage is 23.52%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #2617      +/-   ##
=============================================
- Coverage      50.92%   50.77%   -0.15%     
+ Complexity      2817     2816       -1     
=============================================
  Files            558      558              
  Lines          17941    17944       +3     
  Branches        2129     2101      -28     
=============================================
- Hits            9136     9111      -25     
- Misses          7938     7939       +1     
- Partials         867      894      +27     
Impacted Files Coverage Δ Complexity Δ
...ing/annotation/GlobalTransactionalInterceptor.java 10.40% <15.38%> (+1.59%) 4.00 <2.00> (+2.00)
...ta/spring/annotation/GlobalTransactionScanner.java 51.51% <50.00%> (-0.57%) 17.00 <0.00> (ø)
...obuf/convertor/BranchRegisterRequestConvertor.java 90.47% <0.00%> (-9.53%) 3.00% <0.00%> (ø%)
...otobuf/convertor/GlobalBeginResponseConvertor.java 92.59% <0.00%> (-7.41%) 3.00% <0.00%> (ø%)
...otobuf/convertor/GlobalCommitRequestConvertor.java 93.75% <0.00%> (-6.25%) 3.00% <0.00%> (ø%)
...otobuf/convertor/GlobalStatusRequestConvertor.java 93.75% <0.00%> (-6.25%) 3.00% <0.00%> (ø%)
...obuf/convertor/GlobalRollbackRequestConvertor.java 93.75% <0.00%> (-6.25%) 3.00% <0.00%> (ø%)
...protobuf/convertor/RegisterTMRequestConvertor.java 94.73% <0.00%> (-5.27%) 3.00% <0.00%> (ø%)
...protobuf/convertor/RegisterRMRequestConvertor.java 90.47% <0.00%> (-4.77%) 3.00% <0.00%> (ø%)
...otobuf/convertor/BranchReportRequestConvertor.java 95.45% <0.00%> (-4.55%) 3.00% <0.00%> (ø%)
... and 17 more

@zjinlei
Copy link
Contributor

zjinlei commented Apr 27, 2020

add test case

@funky-eyes
Copy link
Contributor Author

add test case

ok

Copy link
Contributor

@helloworlde helloworlde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@funky-eyes funky-eyes changed the title bugfix: for getAnnotation bugfix: for getAnnotation error Apr 27, 2020
Copy link
Contributor

@l81893521 l81893521 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@funky-eyes funky-eyes changed the title bugfix: for getAnnotation error bugfix: fix get annotation error Apr 27, 2020
@funky-eyes
Copy link
Contributor Author

@zjinlei PTAL

@@ -37,6 +36,8 @@
private static Class<?> targetClass = null;
private static GlobalTransactional transactional = null;

private static final GlobalTransactionalInterceptor globalTransactionalInterceptor=new GlobalTransactionalInterceptor(null);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The global variable declare in testGetAnnotation method?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The global variable declare in testGetAnnotation method?

PTAL @jsbxyyx

Copy link
Member

@jsbxyyx jsbxyyx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zjinlei zjinlei changed the title bugfix: fix get annotation error bugfix: fix incorrect scanning of annotations about classes and methods Apr 27, 2020
@zjinlei zjinlei changed the title bugfix: fix incorrect scanning of annotations about classes and methods bugfix: fix incorrect getAnnotation about classes and methods Apr 27, 2020
@zjinlei zjinlei changed the title bugfix: fix incorrect getAnnotation about classes and methods bugfix: fix incorrect getAnnotation about class and method Apr 27, 2020
@funky-eyes
Copy link
Contributor Author

@zjinlei @slievrly PTAL

@zjinlei zjinlei added the Do Not Merge Do not merge into develop label Apr 28, 2020
@slievrly
Copy link
Member

toString,hashcode these methods also take distributed transactions?

@yougecn
Copy link

yougecn commented Apr 28, 2020

测试了一下,注解加在类上、方法上、类及方法上混用、嵌套调用都没问题

@funky-eyes
Copy link
Contributor Author

funky-eyes commented Apr 28, 2020

toString,hashcode these methods also take distributed transactions?

PTAL @slievrly

@jsbxyyx
Copy link
Member

jsbxyyx commented Apr 29, 2020

add hashCode and equals ... test case?

@funky-eyes funky-eyes requested a review from zjinlei May 3, 2020 05:12
@funky-eyes funky-eyes closed this May 8, 2020
@funky-eyes funky-eyes reopened this May 8, 2020
@funky-eyes funky-eyes closed this May 8, 2020
@funky-eyes funky-eyes reopened this May 8, 2020
@codecov-commenter
Copy link

codecov-commenter commented May 19, 2020

Codecov Report

Merging #2617 into develop will increase coverage by 0.10%.
The diff coverage is 47.05%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #2617      +/-   ##
=============================================
+ Coverage      50.95%   51.05%   +0.10%     
- Complexity      2830     2836       +6     
=============================================
  Files            563      563              
  Lines          18043    18047       +4     
  Branches        2136     2138       +2     
=============================================
+ Hits            9193     9214      +21     
+ Misses          7980     7951      -29     
- Partials         870      882      +12     
Impacted Files Coverage Δ Complexity Δ
...ing/annotation/GlobalTransactionalInterceptor.java 24.60% <46.15%> (+15.80%) 7.00 <2.00> (+5.00)
...ta/spring/annotation/GlobalTransactionScanner.java 51.51% <50.00%> (-0.57%) 17.00 <0.00> (ø)

@funky-eyes funky-eyes closed this May 19, 2020
@funky-eyes funky-eyes reopened this May 19, 2020
Copy link
Member

@slievrly slievrly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@slievrly slievrly removed the Do Not Merge Do not merge into develop label May 19, 2020
@zjinlei zjinlei removed their request for review May 19, 2020 03:48
@slievrly slievrly merged commit a627e36 into apache:develop May 19, 2020
@slievrly slievrly added this to the 1.3.0 milestone Jun 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

期望@GlobalTransactional支持ElementType.TYPE
9 participants