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

responseContent() remove excess ByteBufFlux conversion #2792

Merged
merged 6 commits into from
May 5, 2023

Conversation

manzhizhen
Copy link
Contributor

@manzhizhen manzhizhen commented May 3, 2023

When I looked at the implementation of the class HttpClientFinalizer code, I found that the logic of the responseContent method was a bit strange. HttpClientFinalizer.contentReceiver represents ChannelOperations::receive, while ChannelOperations::receive internal implementation has already made a call to ByteBufFlux.fromInbound to return a ByteBufFlux, and a call and encapsulation of ByteBufFlux.fromInbound has also been made in the responseContent method, This is actually unnecessary, so you can directly refer to the internal implementation of ChannelOperations::receive in the responseContent method to generate ByteBufFlux, which will result in better performance and clearer logic. Similarly, there are similar issues with WebsocketFinalizer:responseContent, which is why this PR.

@manzhizhen manzhizhen closed this May 3, 2023
@manzhizhen manzhizhen reopened this May 3, 2023
@violetagg
Copy link
Member

@manzhizhen Please use ./gradlew spotlessApply in order to fix the copyright end date to 2023

@manzhizhen
Copy link
Contributor Author

@manzhizhen Please use ./gradlew spotlessApply in order to fix the copyright end date to 2023

ok, I do it.

@violetagg violetagg added the type/enhancement A general enhancement label May 3, 2023
@violetagg violetagg added this to the 1.0.32 milestone May 3, 2023
@violetagg
Copy link
Member

The failed test on CI for Windows OS is not relevant as it is in quic module, while this change is in http module.

@manzhizhen
Copy link
Contributor Author

The failed test on CI for Windows OS is not relevant as it is in quic module, while this change is in http module.

Okay, get it

Copy link
Contributor

@pderop pderop left a comment

Choose a reason for hiding this comment

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

looking good.

yizhenqiang added 2 commits May 4, 2023 22:15
…xcess ByteBufFlux conversion, preserve the contentReceiver constant
…xcess ByteBufFlux conversion, preserve the contentReceiver constant
@violetagg violetagg changed the base branch from main to 1.0.x May 5, 2023 06:21
@violetagg violetagg changed the base branch from 1.0.x to main May 5, 2023 06:22
@violetagg violetagg merged commit 162c11e into reactor:main May 5, 2023
@violetagg violetagg changed the title responseContent() remove excess ByteBufFlux conversion responseContent() remove excess ByteBufFlux conversion May 5, 2023
violetagg added a commit that referenced this pull request May 5, 2023
@violetagg
Copy link
Member

@manzhizhen Thanks for the PR.
I back ported the change to 1.0.x branch for 1.0.32 version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants