Skip to content

Fix direct serialization of primitive types in DEFAULT_JSONP_MAPPER #1524

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Xtansia
Copy link
Collaborator

@Xtansia Xtansia commented Apr 14, 2025

Description

Fix direct serialization of primitive types in DEFAULT_JSONP_MAPPER as used by toJsonString().

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.

@@ -78,11 +80,31 @@ public JsonProvider jsonProvider() {
public <T> void serialize(T value, JsonGenerator generator) {
if (value instanceof JsonpSerializable) {
((JsonpSerializable) value).serialize(generator, this);
return;
} else if (value instanceof JsonValue) {
Copy link
Collaborator

@reta reta Apr 16, 2025

Choose a reason for hiding this comment

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

@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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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.

@bdelbosc
Copy link

Hi, any feedback on this PR? SearchRequest#toJsonString is not usable without this fix.

Xtansia added 3 commits May 22, 2025 09:55
Signed-off-by: Thomas Farr <tsfarr@amazon.com>
Signed-off-by: Thomas Farr <tsfarr@amazon.com>
Signed-off-by: Thomas Farr <tsfarr@amazon.com>
@Xtansia Xtansia force-pushed the fix/toJsonString-primitive-types branch from 68718dd to 6d43be5 Compare May 21, 2025 23:00
Signed-off-by: Thomas Farr <tsfarr@amazon.com>
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.

[BUG] Minimal IndexRequest fails to serialize to JSON
3 participants