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
Fix serialization bug in painless execute api request #36075
Fix serialization bug in painless execute api request #36075
Conversation
The `xContentType` was incorrectly serialized: `xContentType.mediaTypeWithoutParameters()` was used to serialize, but `xContentType.mediaType()` was used to de-serialize. Also the serialization test class did not test `ContextSetup` well, this was due to limitation in serialization test base class. Changed the test class to manually test xcontent serialization, so that for both binary and xcontent serialization tests, the `ContextSetup` is properly tested. Closes elastic#36050
Pinging @elastic/es-core-infra |
} | ||
|
||
@Override | ||
public void writeTo(StreamOutput out) throws IOException { | ||
out.writeOptionalString(index); | ||
out.writeOptionalBytesReference(document); | ||
out.writeOptionalString(xContentType != null ? xContentType.mediaType(): null); | ||
out.writeOptionalString(xContentType != null ? xContentType.mediaTypeWithoutParameters(): null); |
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.
This line fixes the bug reported in #36050. The rest of this change is to improve testing for the painless execute api request serialization.
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.
LGTM
The `xContentType` was incorrectly serialized: `xContentType.mediaTypeWithoutParameters()` was used to serialize, but `xContentType.mediaType()` was used to de-serialize. Also the serialization test class did not test `ContextSetup` well, this was due to limitation in serialization test base class. Changed the test class to manually test xcontent serialization, so that for both binary and xcontent serialization tests, the `ContextSetup` is properly tested. Closes #36050
The `xContentType` was incorrectly serialized: `xContentType.mediaTypeWithoutParameters()` was used to serialize, but `xContentType.mediaType()` was used to de-serialize. Also the serialization test class did not test `ContextSetup` well, this was due to limitation in serialization test base class. Changed the test class to manually test xcontent serialization, so that for both binary and xcontent serialization tests, the `ContextSetup` is properly tested. Closes #36050
The
xContentType
was incorrectly serialized:xContentType.mediaTypeWithoutParameters()
was used to serialize, butxContentType.mediaType()
was used to de-serialize.Also the serialization test class did not test
ContextSetup
well,this was due to limitation in serialization test base class. Changed the
test class to manually test xcontent serialization, so that for both binary
and xcontent serialization tests, the
ContextSetup
is properly tested.Closes #36050