-
Notifications
You must be signed in to change notification settings - Fork 624
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
allow encoding literal JSON values #2041
Conversation
I think more tests and specialist knowledge is needed, but the approach is ready for review. I'm really keen to provide a way of supporting BigDecimal, as it's been a blocker in a couple of my projects for a long time. |
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.
When do you guys foresee there will be progress on this? I have a use case for it in my project
Hi, thanks for your PR! I think I have time for it on this week, stay tuned |
formats/json/commonMain/src/kotlinx/serialization/json/internal/Composers.kt
Show resolved
Hide resolved
formats/json/commonMain/src/kotlinx/serialization/json/internal/Composers.kt
Outdated
Show resolved
Hide resolved
formats/json-tests/jvmTest/src/kotlinx/serialization/BigDecimalTest.kt
Outdated
Show resolved
Hide resolved
formats/json-tests/jvmTest/src/kotlinx/serialization/BigDecimalTest.kt
Outdated
Show resolved
Hide resolved
formats/json/commonMain/src/kotlinx/serialization/json/JsonElement.kt
Outdated
Show resolved
Hide resolved
formats/json/commonMain/src/kotlinx/serialization/json/internal/StreamingJsonEncoder.kt
Outdated
Show resolved
Hide resolved
formats/json/commonMain/src/kotlinx/serialization/json/JsonElement.kt
Outdated
Show resolved
Hide resolved
I think we should have additional design discussion on a general approach (using |
formats/json-tests/jvmTest/src/kotlinx/serialization/BigDecimalTest.kt
Outdated
Show resolved
Hide resolved
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.
After a discussion, the following points were established:
-
Function should accept arbitrary strings and do not validate them with regex. This will allow users to do various json-embeddings, support non-base10 numbers, different notations, etc and will also simplify an invariant for users and save us from potential problems with regex correctness and performance.
-
Function should accept a
String?
and returnJsonNull
if a null was passed to it, for consistency withJsonPrimitive
overloads. -
However, if string literal
"null"
is passed (i.e.value == "null"
), function should throwIllegalArgumentException
. This is motivated by the fact that when we serialize such a literal, if effective becomes indistinguishable fromJsonNull
and can't be deserialized back toJsonLiteral
. We cannot know for sure if this was a user intent to do so or bug in validation. Better to prohibit such cases. IAE message can sound like this: "It is impossible to create a literal unquoted value of 'null'. If you want to create json null literal, use JsonNull object, otherwise, use quoted literal."
Summarising this all up, I suggest to name the new function 'JsonUnquotedPrimitive'. What do you think?
...n-tests/commonTest/src/kotlinx/serialization/json/serializers/JsonPrimitiveSerializerTest.kt
Outdated
Show resolved
Hide resolved
formats/json-tests/jvmTest/src/kotlinx/serialization/BigDecimalTest.kt
Outdated
Show resolved
Hide resolved
formats/json-tests/jvmTest/src/kotlinx/serialization/BigDecimalTest.kt
Outdated
Show resolved
Hide resolved
formats/json/commonMain/src/kotlinx/serialization/json/JsonElement.kt
Outdated
Show resolved
Hide resolved
formats/json/commonMain/src/kotlinx/serialization/json/JsonElement.kt
Outdated
Show resolved
Hide resolved
formats/json-tests/commonTest/src/kotlinx/serialization/json/serializers/JsonRawElementTest.kt
Outdated
Show resolved
Hide resolved
Also, I've just noted that the PR is opened against |
I've updated the PR to point at |
That's a normal situation. We only merge |
2d1e859
to
ff8fd99
Compare
…ment.kt Co-authored-by: Leonid Startsev <[email protected]>
…ment.kt Co-authored-by: Leonid Startsev <[email protected]>
…ment.kt Co-authored-by: Leonid Startsev <[email protected]>
…ment.kt Co-authored-by: Leonid Startsev <[email protected]>
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.
Great job, thanks for implementing this important feature!
Thanks! And thanks to you and @qwwdfsad for reviewing. Do you know when this will be available? Does KxS have snapshot releases that I can use? |
No, we don't usually publish snapshots. The next release is going to be around Kotlin 1.8.0 release, in a month or two |
This PR provides a new function for encoding raw JSON content, without quoting it as a string. This allows for encoding JSON numbers of any size or precision, so BigDecimal and BigInteger can be supported. Fixes Kotlin#1051 Fixes Kotlin#1405 The implementation is similar to how unsigned numbers are handled. JsonUnquotedLiteral() is a new function that allows creating literal JSON content. Added val coerceToInlineType to JsonLiteral, so that JsonUnquotedLiteral could use encodeInline() Defined val jsonUnquotedLiteralDescriptor as a 'marker', for use with encodeInline() ComposerForUnquotedLiterals (based on ComposerForUnsignedNumbers) will 'override' the encoder when a JsonLiteral has the jsonUnquotedLiteralDescriptor marker, and will encode the content as a string without surrounding quotes.
This PR provides a new function for encoding raw JSON content, without quoting it as a string. This allows for encoding JSON numbers of any size or precision, so BigDecimal and BigInteger can be supported.
Link to view the updated documentation
fix: #1051
fix: #1405
related:
The implementation is similar to how unsigned numbers are handled.
JsonUnquotedLiteral()
is a new function that allows creating literal JSON content.val coerceToInlineType
toJsonLiteral
, so thatJsonUnquotedLiteral
could useencodeInline()
val jsonUnquotedLiteralDescriptor
as a 'marker', for use withencodeInline()
ComposerForUnquotedLiterals
(based onComposerForUnsignedNumbers
) will 'override' the encoder when aJsonLiteral
has thejsonUnquotedLiteralDescriptor
marker, and will encode the content as a string without surrounding quotes.I'm not sure how to proceed with decoding literal numbers.I just realised, there's no need for special implementation - primitives are all decoded the same.