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: xid header lowercase #1789

Merged
merged 15 commits into from
Nov 12, 2019
Merged

bugfix: xid header lowercase #1789

merged 15 commits into from
Nov 12, 2019

Conversation

jsbxyyx
Copy link
Member

@jsbxyyx jsbxyyx commented Oct 18, 2019

Ⅰ. Describe what this PR did

Ⅱ. Does this pull request fix one issue?

fixes #1728

Ⅲ. 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 Oct 18, 2019

Codecov Report

Merging #1789 into develop will increase coverage by <.01%.
The diff coverage is 66.66%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #1789      +/-   ##
=============================================
+ Coverage      55.27%   55.28%   +<.01%     
- Complexity      2383     2386       +3     
=============================================
  Files            422      422              
  Lines          14282    14299      +17     
  Branches        1698     1702       +4     
=============================================
+ Hits            7895     7905      +10     
- Misses          5673     5678       +5     
- Partials         714      716       +2
Impacted Files Coverage Δ Complexity Δ
...ta/integration/grpc/interceptor/GrpcHeaderKey.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...terceptor/server/ServerTransactionInterceptor.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...eata/integration/motan/MotanTransactionFilter.java 67.85% <100%> (+5.35%) 7 <2> (+2) ⬆️
...ion/sofa/rpc/TransactionContextProviderFilter.java 60% <100%> (+6.15%) 7 <2> (+2) ⬆️
...ion/sofa/rpc/TransactionContextConsumerFilter.java 40% <80%> (+5.38%) 5 <1> (+1) ⬆️
...in/java/io/seata/server/session/GlobalSession.java 84.54% <0%> (-0.49%) 67% <0%> (-1%)
...o/seata/server/coordinator/DefaultCoordinator.java 48.01% <0%> (-0.4%) 27% <0%> (-1%)
...server/store/file/FileTransactionStoreManager.java 56.96% <0%> (+0.31%) 29% <0%> (ø) ⬇️

Copy link
Member

@xingfudeshi xingfudeshi 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 self-requested a review October 21, 2019 02:50
@slievrly
Copy link
Member

@jsbxyyx Are there any other better options? If the consumer and provider do not use the same version after modification, the compatibility issue will be triggered.

@jsbxyyx
Copy link
Member Author

jsbxyyx commented Oct 22, 2019

@slievrly For the old version, it seems that there is no good solution.

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.

I researched the dubbo community and confirmed whether it is a bug. I think I need to be compatible with the previous version. I think I can make two judgments in the filter.

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.

Are other rpc frameworks unaffected?

@jsbxyyx
Copy link
Member Author

jsbxyyx commented Oct 29, 2019

fix done

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
Copy link
Member

@jsbxyyx jsbxyyx merged commit b7bda8a into apache:develop Nov 12, 2019
@jsbxyyx jsbxyyx changed the title optimize: xid header lowercase bugfix: xid header lowercase Nov 13, 2019
booogu pushed a commit to booogu/seata that referenced this pull request Nov 14, 2019
@slievrly slievrly added this to the 1.0 milestone Dec 2, 2019
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.

RpcContext.getContext().getAttachment(RootContext.KEY_XID) is null
5 participants