Fix direct serialization of primitive types in DEFAULT_JSONP_MAPPER#1524
Fix direct serialization of primitive types in DEFAULT_JSONP_MAPPER#1524Xtansia wants to merge 4 commits intoopensearch-project:mainfrom
DEFAULT_JSONP_MAPPER#1524Conversation
9bbb2ec to
68718dd
Compare
| if (value instanceof JsonpSerializable) { | ||
| ((JsonpSerializable) value).serialize(generator, this); | ||
| return; | ||
| } else if (value instanceof JsonValue) { |
There was a problem hiding this comment.
@Xtansia the DEFAULT_JSONP_MAPPER was not designed to be generic JSON mapper, it shouldn't be used with primitive types. Back to the issue: what is expectation of sending the index request (which is just a string) to OpenSearch? I would expect the document to a class instance of JSON type.
There was a problem hiding this comment.
Have switched to a more sensible example. ExtendedBounds is generic and uses JsonpUtils.serialize, Which directly passes through to mapper.serialize if no explicit serializer is passed or type isn't JsonpSerializable. Which is the same mechanism that IndexRequest uses, just ExtendedBounds<Double> is an actively in-use type within the API types.
I don't think it's unreasonable to directly handle the primitives that map directly to JsonGenerator calls. The alternative is moving this logic into JsonpUtils.serialize.
|
Hi, any feedback on this PR? |
Signed-off-by: Thomas Farr <tsfarr@amazon.com>
Signed-off-by: Thomas Farr <tsfarr@amazon.com>
Signed-off-by: Thomas Farr <tsfarr@amazon.com>
68718dd to
6d43be5
Compare
Signed-off-by: Thomas Farr <tsfarr@amazon.com>
Description
Fix direct serialization of primitive types in
DEFAULT_JSONP_MAPPERas used bytoJsonString().Issues Resolved
Closes #1503
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.