-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Correct reading HTTP client response body without content type + ByteBuffer
converters
#10975
Conversation
ByteBuffer
converters
conversionService.addConverter( | ||
java.nio.ByteBuffer.class, | ||
ByteBuf.class, | ||
nioBuffer -> Unpooled.copiedBuffer(nioBuffer) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why copy and not wrap?
} | ||
}); | ||
if (res instanceof FullNettyClientHttpResponse response) { | ||
response.onComplete(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are you sure we don't need the onComplete here
@@ -1447,7 +1432,6 @@ public void onComplete() { | |||
try { | |||
FullHttpResponse fullHttpResponse = new DefaultFullHttpResponse(nettyResponse.protocolVersion(), nettyResponse.status(), buffer, nettyResponse.headers(), new DefaultHttpHeaders(true)); | |||
final FullNettyClientHttpResponse<Object> fullNettyClientHttpResponse = new FullNettyClientHttpResponse<>(fullHttpResponse, handlerRegistry, (Argument<Object>) errorType, true, conversionService); | |||
fullNettyClientHttpResponse.onComplete(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
|
||
@Override | ||
public void onComplete() { | ||
this.complete = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, complete was unused? 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the field was highlighted as not used
|
The buffer of the client response should be discarded