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 wrong proxy of datasource bean #2323

Merged
merged 30 commits into from
Mar 25, 2020
Merged

bugfix:fix wrong proxy of datasource bean #2323

merged 30 commits into from
Mar 25, 2020

Conversation

xingfudeshi
Copy link
Member

@xingfudeshi xingfudeshi commented Feb 28, 2020

Ⅰ. Describe what this PR did

fix wrong proxy of datasource bean

Ⅱ. Does this pull request fix one issue?

fixes #2322 #2311 #2321

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

I've done integration testing.

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@codecov-io
Copy link

codecov-io commented Feb 28, 2020

Codecov Report

Merging #2323 into develop will decrease coverage by 0.02%.
The diff coverage is 2.94%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #2323      +/-   ##
=============================================
- Coverage      51.74%   51.71%   -0.03%     
+ Complexity      2665     2664       -1     
=============================================
  Files            518      519       +1     
  Lines          16805    16810       +5     
  Branches        2031     2032       +1     
=============================================
- Hits            8695     8694       -1     
- Misses          7293     7298       +5     
- Partials         817      818       +1     
Impacted Files Coverage Δ Complexity Δ
...ing/boot/autoconfigure/SeataAutoConfiguration.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...ation/datasource/AutoDataSourceProxyRegistrar.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...ion/datasource/SeataAutoDataSourceProxyAdvice.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...on/datasource/SeataAutoDataSourceProxyCreator.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...boot/autoconfigure/properties/SeataProperties.java 44.44% <25.00%> (-3.39%) 6.00 <0.00> (ø)
...torage/file/store/FileTransactionStoreManager.java 56.50% <0.00%> (-0.64%) 29.00% <0.00%> (-1.00%)

@ggndnn
Copy link
Contributor

ggndnn commented Mar 11, 2020

@xingfudeshi
I think this pr is not a final solution of the issues mentioned above, e.g., the cause of #2321 is that seata used cglib to create a proxy instance whose type was a subclass of DynamicDataSource, but it's field resolvedDataSources was not initialized, and more importantly, the proxied bean was not repeatedly proxied before.

So here are some advices:

  1. To create proxy in spring way when auto proxying running in spring context, some frameworks assume that the proxy class is created in spring way, e.g., mybatis-plus.
  2. To provide include or exclude kind attributes to @EnableAutoDataSourceProxy helping users to control auto proxy target.
    e.g., @EnableAutoDataSourceProxy(include=HikariDataSource.class, exclude= AbstractRoutingDataSource.class), something like this.
  3. To recursive check duplication auto proxying, though I am not so sure whether this point is necessary.

@zjinlei zjinlei added this to the 1.2.0 milestone Mar 12, 2020
@xingfudeshi xingfudeshi changed the title bugfix:fix duplicate proxy of datasource bean (WIP)bugfix:fix duplicate proxy of datasource bean Mar 13, 2020
@xingfudeshi xingfudeshi changed the title (WIP)bugfix:fix duplicate proxy of datasource bean bugfix:fix duplicate proxy of datasource bean Mar 13, 2020
@xingfudeshi xingfudeshi changed the title bugfix:fix duplicate proxy of datasource bean (WIP)bugfix:fix duplicate proxy of datasource bean Mar 15, 2020
@xingfudeshi xingfudeshi changed the title (WIP)bugfix:fix duplicate proxy of datasource bean bugfix:fix duplicate proxy of datasource bean Mar 15, 2020
@xingfudeshi
Copy link
Member Author

@xingfudeshi
I think this pr is not a final solution of the issues mentioned above, e.g., the cause of #2321 is that seata used cglib to create a proxy instance whose type was a subclass of DynamicDataSource, but it's field resolvedDataSources was not initialized, and more importantly, the proxied bean was not repeatedly proxied before.

So here are some advices:

  1. To create proxy in spring way when auto proxying running in spring context, some frameworks assume that the proxy class is created in spring way, e.g., mybatis-plus.
  2. To provide include or exclude kind attributes to @EnableAutoDataSourceProxy helping users to control auto proxy target.
    e.g., @EnableAutoDataSourceProxy(include=HikariDataSource.class, exclude= AbstractRoutingDataSource.class), something like this.
  3. To recursive check duplication auto proxying, though I am not so sure whether this point is necessary.

Please review.

@xingfudeshi xingfudeshi changed the title bugfix:fix duplicate proxy of datasource bean optimize:refactor of auto proxy of datasource Mar 23, 2020
@xingfudeshi xingfudeshi changed the title optimize:refactor of auto proxy of datasource bugfix:fix wrong proxy of datasource bean Mar 23, 2020
Copy link
Contributor

@ggndnn ggndnn 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
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 merged commit 1d468c4 into apache:develop Mar 25, 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.

当使用eureka或者nacos的时候,自动代理无法使用
5 participants