-
Notifications
You must be signed in to change notification settings - Fork 31
Delegate parsing and writing of value types to Gson #2054
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
Conversation
This allows registering custom TypeAdapters to customize (un)marshalling of these value types.
baywet
left a comment
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.
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.
...onents/serialization/json/src/main/java/com/microsoft/kiota/serialization/JsonParseNode.java
Show resolved
Hide resolved
...serialization/json/src/main/java/com/microsoft/kiota/serialization/JsonParseNodeFactory.java
Show resolved
Hide resolved
...ialization/json/src/main/java/com/microsoft/kiota/serialization/JsonSerializationWriter.java
Outdated
Show resolved
Hide resolved
...ialization/json/src/main/java/com/microsoft/kiota/serialization/JsonSerializationWriter.java
Outdated
Show resolved
Hide resolved
...onents/serialization/json/src/main/java/com/microsoft/kiota/serialization/JsonParseNode.java
Outdated
Show resolved
Hide resolved
...onents/serialization/json/src/main/java/com/microsoft/kiota/serialization/JsonParseNode.java
Show resolved
Hide resolved
...onents/serialization/json/src/main/java/com/microsoft/kiota/serialization/JsonParseNode.java
Outdated
Show resolved
Hide resolved
...ion/json/src/main/java/com/microsoft/kiota/serialization/JsonSerializationWriterFactory.java
Show resolved
Hide resolved
|
Now that Also, these 3 parsing methods still work on strings: |
...onents/serialization/json/src/main/java/com/microsoft/kiota/serialization/JsonParseNode.java
Outdated
Show resolved
Hide resolved
...onents/serialization/json/src/main/java/com/microsoft/kiota/serialization/JsonParseNode.java
Outdated
Show resolved
Hide resolved
...onents/serialization/json/src/main/java/com/microsoft/kiota/serialization/JsonParseNode.java
Outdated
Show resolved
Hide resolved
...ion/json/src/main/java/com/microsoft/kiota/serialization/JsonSerializationWriterFactory.java
Outdated
Show resolved
Hide resolved
...ialization/json/src/main/java/com/microsoft/kiota/serialization/JsonSerializationWriter.java
Show resolved
Hide resolved
...onents/serialization/json/src/main/java/com/microsoft/kiota/serialization/JsonParseNode.java
Outdated
Show resolved
Hide resolved
...onents/serialization/json/src/main/java/com/microsoft/kiota/serialization/JsonParseNode.java
Outdated
Show resolved
Hide resolved
...ization/json/src/test/java/com/microsoft/kiota/serialization/mocks/IntersectionTypeMock.java
Outdated
Show resolved
Hide resolved
baywet
left a comment
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.
getting really close! I think there's still some work needed around the intersection and union types.
baywet
left a comment
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.
Thank you for making the changes!
|
@papegaaij because we're shipping to be compatible with android as well, there are some "new" java features we can't use. |
baywet
left a comment
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.
Thank you for making the changes!
...onents/serialization/json/src/main/java/com/microsoft/kiota/serialization/JsonParseNode.java
Outdated
Show resolved
Hide resolved
|
@papegaaij this test is now failing because it has a dependency on the JSON serialization infrastructure. Would you mind having a look please? Line 64 in 9decb26
|
baywet
left a comment
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.
Thank you for making the changes!
Head branch was pushed to by a user without write access
This allows registering custom TypeAdapters to customize (un)marshalling of these value types. This fixes #2053.
I've decided not to pass methods like
getFloatValueto the Gson instance. Thegson.fromJsoncalls do impose some overhead and they all end up in the correspondingJsonElement.getAsXXXmethods. I did make it easier to subclassJsonParseNodeto create your own subclass if needed. That would allow a user to override one of the primitive parsing methods.