Skip to content

Conversation

@papegaaij
Copy link
Contributor

This allows registering custom TypeAdapters to customize (un)marshalling of these value types. This fixes #2053.

I've decided not to pass methods like getFloatValue to the Gson instance. The gson.fromJson calls do impose some overhead and they all end up in the corresponding JsonElement.getAsXXX methods. I did make it easier to subclass JsonParseNode to create your own subclass if needed. That would allow a user to override one of the primitive parsing methods.

This allows registering custom TypeAdapters to customize (un)marshalling
of these value types.
@papegaaij papegaaij requested a review from a team as a code owner November 17, 2025 12:52
Copy link
Member

@baywet baywet left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!

In addition to the comments left inline, I think we should also have the "primitive" types use the adapter model for consistency with dotnet, and for consistency of behaviour.

https://github.com/microsoft/kiota-dotnet/blob/ae9a82ff2d1445ad2f1ef26a9d9e918a402b8a26/src/serialization/json/KiotaJsonSerializationContext.cs#L11

@github-project-automation github-project-automation bot moved this to In Progress 🚧 in Kiota Nov 17, 2025
@papegaaij
Copy link
Contributor Author

Now that JsonParseNode delegates parsing of all value types to Gson, the implementation is no longer symmetric. Maybe, the JsonSerializationWriter should also delegate writing for all value types to Gson? So rathter than calling writer.value(bool), it will be gson.getAdapter(Boolean.class).write(writer, value). What do you think?

Also, these 3 parsing methods still work on strings: getEnumValue, getEnumSetValue and getByteArrayValue. What do you think about these? The first 2 methods depend on a ValuedEnumParser. I don't think it will be possible to handle those via Gson. getByteArrayValue, on the other hand, should be possible to handle via a TypeAdapter, I think.

Copy link
Member

@baywet baywet left a comment

Choose a reason for hiding this comment

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

getting really close! I think there's still some work needed around the intersection and union types.

baywet
baywet previously approved these changes Nov 20, 2025
Copy link
Member

@baywet baywet left a comment

Choose a reason for hiding this comment

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

Thank you for making the changes!

@baywet
Copy link
Member

baywet commented Nov 20, 2025

@papegaaij because we're shipping to be compatible with android as well, there are some "new" java features we can't use.
One of them is the diamond <>, the type needs to be passed explicitly instead.
Would you mind making the update please?

baywet
baywet previously approved these changes Nov 20, 2025
Copy link
Member

@baywet baywet left a comment

Choose a reason for hiding this comment

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

Thank you for making the changes!

@baywet
Copy link
Member

baywet commented Nov 20, 2025

@papegaaij this test is now failing because it has a dependency on the JSON serialization infrastructure. Would you mind having a look please?

baywet
baywet previously approved these changes Nov 20, 2025
Copy link
Member

@baywet baywet left a comment

Choose a reason for hiding this comment

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

Thank you for making the changes!

@baywet baywet enabled auto-merge (squash) November 20, 2025 14:50
auto-merge was automatically disabled November 20, 2025 14:54

Head branch was pushed to by a user without write access

@baywet baywet enabled auto-merge (squash) November 20, 2025 14:58
@baywet baywet merged commit 41d7c77 into microsoft:main Nov 20, 2025
14 checks passed
@github-project-automation github-project-automation bot moved this from In Progress 🚧 to Done ✔️ in Kiota Nov 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Allow customization of type parsing in JSON responses

2 participants