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 HttpAutoConfiguration always instantiation in springboot env #3097

Merged
merged 8 commits into from
Sep 15, 2020

Conversation

PeineLiang
Copy link
Contributor

@PeineLiang PeineLiang commented Sep 9, 2020

Ⅰ. Describe what this PR did

fix HttpAutoConfiguration always instantiation in springboot env

Ⅱ. Does this pull request fix one issue?

fixes #3095

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

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@codecov-commenter
Copy link

codecov-commenter commented Sep 9, 2020

Codecov Report

Merging #3097 into develop will increase coverage by 0.03%.
The diff coverage is 0.00%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #3097      +/-   ##
=============================================
+ Coverage      50.56%   50.60%   +0.03%     
  Complexity      3101     3101              
=============================================
  Files            599      599              
  Lines          19514    19498      -16     
  Branches        2404     2404              
=============================================
- Hits            9867     9866       -1     
+ Misses          8656     8641      -15     
  Partials         991      991              
Impacted Files Coverage Δ Complexity Δ
...ring/boot/autoconfigure/HttpAutoConfiguration.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...torage/file/store/FileTransactionStoreManager.java 56.77% <0.00%> (-0.65%) 28.00% <0.00%> (-1.00%)
...in/java/io/seata/server/session/GlobalSession.java 82.88% <0.00%> (+0.45%) 71.00% <0.00%> (+1.00%)

Copy link
Contributor

@funky-eyes funky-eyes left a comment

Choose a reason for hiding this comment

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

LGTM please add test case for HttpCondition

@wangliang181230
Copy link
Contributor

wangliang181230 commented Sep 9, 2020

springboot有提供@ConditionalOnWebApplication的啊。
不用自己开发HttpCondition

@funky-eyes funky-eyes added the module/integration integration module label Sep 9, 2020
@funky-eyes funky-eyes added this to the 1.4.0 milestone Sep 9, 2020
@PeineLiang
Copy link
Contributor Author

PeineLiang commented Sep 13, 2020

修改了实现方式:
1.在seata-http中引入spring-boot-autoconfigure。
2.在HttpAutoConfiguration类上加注解@ConditionalOnWebApplication,web项目默认开启拦截,要关闭时需要用户排除。
3.HttpAutoConfiguration改为继承WebMvcConfigurerAdapter,删掉没有重写的方法,让代码更清晰。

@PeineLiang
Copy link
Contributor Author

@l81893521 @a364176773 @wangliang181230 麻烦review一下,或者有其他的改进建议吗

@PeineLiang PeineLiang changed the title bugfix: fix HttpAutoConfiguration always instantiation bugfix: fix HttpAutoConfiguration always instantiation in springboot env Sep 13, 2020
Copy link
Contributor

@wangliang181230 wangliang181230 left a comment

Choose a reason for hiding this comment

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

LGTM

integration/http/pom.xml Outdated Show resolved Hide resolved
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

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 for @wangliang181230

@l81893521 l81893521 merged commit 668623f into apache:develop Sep 15, 2020
l81893521 pushed a commit to l81893521/seata that referenced this pull request Oct 22, 2020
hicf pushed a commit to hicf/seata that referenced this pull request Nov 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module/integration integration module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

optimize: HttpAutoConfiguration always instantiation
6 participants