Skip to content
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

Add concerte type for JsonPrimitives #1298

Open
NamekMaster opened this issue Jan 20, 2021 · 10 comments
Open

Add concerte type for JsonPrimitives #1298

NamekMaster opened this issue Jan 20, 2021 · 10 comments

Comments

@NamekMaster
Copy link

What is your use-case and why do you need this feature?

I found its very complex when I try to parse the json string when the type is unknown, for example, I cannot decode json to Map<String, Any?> except Map<String, JsonElement>, because I shouldn't expose JsonElement to caller, so I try to parse them manually and I cannot get real type of JsonPrimitive, the Api only provide some converter method and isString to determine whether it is a String, even if I can detect and parse the string to real type just like Int, Float, Boolean ... it's not very convenient.

Describe the solution you'd like

add a way to determine the real type of JsonPrimitive or just parse them as Any like other json parse library.

@sandwwraith
Copy link
Member

It's not that easy. See, in {"primitive": 234}, JsonPrimitive(234) can be used as Short, Int, Long, Float or Double. There's no unambiguous from JsonPrimitive content to Any and different libs use different strategies (e.g. treat all numbers as double, or 'if number does not have fraction part, translate to int, if it's too big, use long, and as a last resort, use double'). We do not want embed any of the strategies in the library, because different strategies are tailored for different needs. Hence, we provide you a JsonPrimitive that can be treated as any of these types when possible (see functions JsonPrimitive.int, JsonPrimitive.boolean etc). If you do not want to think too much, just use toString — strings will be quoted, and numbers/booleans unquoted.

@set321go
Copy link

set321go commented Feb 1, 2021

It would be great if you could put that into the Docs, while its annoying to have to deal with it myself its a reasonable argument as to why something i expected to be serializable (a class from std lib) is not.

@rgmz
Copy link

rgmz commented Oct 5, 2021

I asked a similar question in the Kotlin Slack.

@sandwwraith: While your explanation makes complete sense, I still wish there was an easier way that didn't involve guesswork (e.g., value classes for JsonString, JsonBoolean, and JsonNumber). Iterating through all possible types feels wasteful and imprecise.

I'll include my 'good enough' attempt at this, for posterity. Be warned that this is far from perfect, and doesn't handle all cases (Short / Long / Float).

private val INT_REGEX = Regex("""^-?\d+$""")
private val DOUBLE_REGEX = Regex("""^-?\d+\.\d+(?:E-?\d+)?$""")

val JsonPrimitive.valueOrNull: Any?
    get() = when {
        this is JsonNull -> null
        this.isString -> this.content
        else -> this.content.toBooleanStrictOrNull()
            ?: when {
                INT_REGEX.matches(this.content) -> this.int
                DOUBLE_REGEX.matches(this.content) -> this.double
                else -> throw IllegalArgumentException("Unknown type for JSON value: ${this.content}")
            }
    }
Basic unit tests (click to expand)

Written using Kotest runner & property testing.

class JsonTest : DescribeSpec({
    context("JsonPrimitive") {
        it("returns null") {
            val value = JsonPrimitive(null as String?).valueOrNull
            value.shouldBeNull()
        }

        it("returns String") {
            checkAll<String> { str ->
                val value = JsonPrimitive(str).valueOrNull
                value.shouldNotBeNull()
                value.shouldBeTypeOf<String>()
                value shouldBe str
            }
        }

        it("returns Boolean") {
            checkAll<Boolean> { bool ->
                val value = JsonPrimitive(bool).valueOrNull
                value.shouldNotBeNull()
                value.shouldBeTypeOf<Boolean>()
                value shouldBe bool
            }
        }

        it("returns Int") {
            checkAll<Int> { int ->
                val value = JsonPrimitive(int).valueOrNull
                value.shouldNotBeNull()
                value.shouldBeTypeOf<Int>()
                value shouldBe int
            }
        }

        it("returns Double") {
            checkAll<Double> { double ->
                if (double.isNaN() || double.isInfinite()) return@checkAll

                val value = JsonPrimitive(double).valueOrNull
                value.shouldNotBeNull()
                value.shouldBeTypeOf<Double>()
                value shouldBe double
            }
        }
})

@bluenote10
Copy link

bluenote10 commented Dec 20, 2021

It's not that easy. See, in {"primitive": 234}, JsonPrimitive(234) can be used as Short, Int, Long, Float or Double.

Kotlin has Number and the constructor of JsonPrimitive requires it to be a Number at some point anyway. Can't it be modeled explicitly using Number? Something like:

sealed class JsonData {}

object JsonNull: JsonData()
data class JsonNumber(val value: Number): JsonData()
data class JsonBoolean(val value: Boolean): JsonData()
data class JsonString(val value: String): JsonData()
data class JsonArray(val value: List<JsonData>): JsonData()
data class JsonObject(val value: Map<String, JsonData>): JsonData()

The way this is currently implemented is a bit strange: On the one hand the constructors of JsonPrimitive take the value explicitly as String?, Number?, and Boolean?. But then the constructor of JsonLiteral throws away all the valuable type information, and the valuable value itself 😉 and rather converts it to a string. The strategy of being agnostic with respect to the representation would only make sense if the original raw content value would get passed in directly, right? But if it already has been converted to String/Number/Boolean there is no point of stringifying it only for the purpose of memory storage.

Also note that this strategy has unnecessary overheads in terms of memory and runtime. Storing for instance a raw boolean as Boolean requires less memory then storing it as the string "true". Similarly all the value extraction functions below have to do unnecessary work on every value access:

public val JsonPrimitive.int: Int get() = content.toInt()
/**
* Returns content of the current element as int or `null` if current element is not a valid representation of number
*/
public val JsonPrimitive.intOrNull: Int? get() = content.toIntOrNull()
/**
* Returns content of current element as long
* @throws NumberFormatException if current element is not a valid representation of number
*/
public val JsonPrimitive.long: Long get() = content.toLong()
/**
* Returns content of current element as long or `null` if current element is not a valid representation of number
*/
public val JsonPrimitive.longOrNull: Long? get() = content.toLongOrNull()
/**
* Returns content of current element as double
* @throws NumberFormatException if current element is not a valid representation of number
*/
public val JsonPrimitive.double: Double get() = content.toDouble()
/**
* Returns content of current element as double or `null` if current element is not a valid representation of number
*/
public val JsonPrimitive.doubleOrNull: Double? get() = content.toDoubleOrNull()
/**
* Returns content of current element as float
* @throws NumberFormatException if current element is not a valid representation of number
*/
public val JsonPrimitive.float: Float get() = content.toFloat()
/**
* Returns content of current element as float or `null` if current element is not a valid representation of number
*/
public val JsonPrimitive.floatOrNull: Float? get() = content.toFloatOrNull()
/**
* Returns content of current element as boolean
* @throws IllegalStateException if current element doesn't represent boolean
*/
public val JsonPrimitive.boolean: Boolean get() = content.toBooleanStrictOrNull() ?: throw IllegalStateException("$this does not represent a Boolean")
/**
* Returns content of current element as boolean or `null` if current element is not a valid representation of boolean
*/
public val JsonPrimitive.booleanOrNull: Boolean? get() = content.toBooleanStrictOrNull()

@sandwwraith
Copy link
Member

sandwwraith commented Dec 20, 2021

The strategy of being agnostic with respect to the representation would only make sense if the original raw content value would get passed in directly, right?

You're completely right. JsonPrimitive constructors from Number, Boolean, etc are provided for user convenience when you create the Json tree manually. The parser (used in Json.parseToJsonElement) operates and stores only raw strings:

private fun readValue(isString: Boolean): JsonPrimitive {
val string = if (isLenient || !isString) {
lexer.consumeStringLenient()
} else {
lexer.consumeString()
}
if (!isString && string == NULL) return JsonNull
return JsonLiteral(string, isString)
}

@sandwwraith
Copy link
Member

However, it is still possible to handle booleans there separately, so users can easily differentiate whether it is String, Boolean, Null, or Number. While we still do not aim to provide a number-parsing strategy, differentiating of booleans may help in some use-cases

@bluenote10
Copy link

However, it is still possible to handle booleans there separately, so users can easily differentiate whether it is String, Boolean, Null, or Number.

That was exactly what I was thinking now as well. Basically, the technique of postponing the value extraction only makes sense for JsonNumber (allowing for use of e.g. BigInt libraries etc.). Modeling JsonString and JsonBoolean as explicit subclasses would still be beneficial. In particular, modelling JsonString explicitly would also avoid the necessity of storing the isString information as a field (having to differentiate between isString true and false cases when working with JsonPrimitive feels like a source of awkwardness).

@gnawf
Copy link

gnawf commented Apr 29, 2022

Just ran into this problem. I am parsing user input in the form of String to JsonElement to put as JWT claims (for a HTTP Client). I need to put Kotlin/Java primitives i.e. Int, String, Double, Map, List as arguments into the claims object.

This is a solved problem in Gson and Jackson but I was hoping to use a Kotlin 1st solution for sealed classes etc… Guess not.

fviernau added a commit to oss-review-toolkit/ort that referenced this issue Jul 12, 2022
The server does return vulnerabilities which do not have a severity
value in the dedicated property. The unspecified `databaseSpecific`
property often times holds a primitive `severity` property with values
such as `[HIGH, MEDIUM, LOW]`. Make use of these values as a fallback as
these to provide more indication than a `null` value.

Note: The data model of 'osv/client' currently uses subtypes of
JsonElement to expose a couple of unspecified JSON objects as
properties. Accessing these requires the client code to add
'kotlinx.serialization' as dependency which is not nice. A solution to
that would be to use "raw" string values containing the JSON, which is
unfortunately not yet possible but may become so in the future, see
[1][2][3].

So, for now add 'kotlinx.serialization' as dependency to the advisor in
order to access the property and leave a FIXME comment as reminder.

[1] Kotlin/kotlinx.serialization#1298
[2] Kotlin/kotlinx.serialization#1405
[3] Kotlin/kotlinx.serialization#1058

Signed-off-by: Frank Viernau <frank_viernau@epam.com>
fviernau added a commit to oss-review-toolkit/ort that referenced this issue Jul 13, 2022
The server does return vulnerabilities which do not have a severity
value in the dedicated property. The unspecified `databaseSpecific`
property often times holds a primitive `severity` property with values
such as `[HIGH, MEDIUM, LOW]`. Make use of these values as a fallback as
these to provide more indication than a `null` value.

Note: The data model of 'osv/client' currently uses subtypes of
JsonElement to expose a couple of unspecified JSON objects as
properties. Accessing these requires the client code to add
'kotlinx.serialization' as dependency which is not nice. A solution to
that would be to use "raw" string values containing the JSON, which is
unfortunately not yet possible but may become so in the future, see
[1][2][3].

So, for now add 'kotlinx.serialization' as dependency to the advisor in
order to access the property and leave a FIXME comment as reminder.

[1] Kotlin/kotlinx.serialization#1298
[2] Kotlin/kotlinx.serialization#1405
[3] Kotlin/kotlinx.serialization#1058

Signed-off-by: Frank Viernau <frank_viernau@epam.com>
fviernau added a commit to oss-review-toolkit/ort that referenced this issue Jul 13, 2022
The server does return vulnerabilities which do not have a severity
value in the dedicated property. The unspecified `databaseSpecific`
property often times holds a primitive `severity` property with values
such as `[HIGH, MEDIUM, LOW]`. Make use of these values as a fallback as
these to provide more indication than a `null` value.

Note: The data model of 'osv/client' currently uses subtypes of
JsonElement to expose a couple of unspecified JSON objects as
properties. Accessing these requires the client code to add
'kotlinx.serialization' as dependency which is not nice. A solution to
that would be to use "raw" string values containing the JSON, which is
unfortunately not yet possible but may become so in the future, see
[1][2][3].

So, for now add 'kotlinx.serialization' as dependency to the advisor in
order to access the property and leave a FIXME comment as reminder.

[1] Kotlin/kotlinx.serialization#1298
[2] Kotlin/kotlinx.serialization#1405
[3] Kotlin/kotlinx.serialization#1058

Signed-off-by: Frank Viernau <frank_viernau@epam.com>
fviernau added a commit to oss-review-toolkit/ort that referenced this issue Jul 13, 2022
The server does return vulnerabilities which do not have a severity
value in the dedicated property. The unspecified `databaseSpecific`
property often times holds a primitive `severity` property with values
such as `[HIGH, MEDIUM, LOW]`. Make use of these values as a fallback as
these to provide more indication than a `null` value.

Note: The data model of 'osv/client' currently uses subtypes of
JsonElement to expose a couple of unspecified JSON objects as
properties. Accessing these requires the client code to add
'kotlinx.serialization' as dependency which is not nice. A solution to
that would be to use "raw" string values containing the JSON, which is
unfortunately not yet possible but may become so in the future, see
[1][2][3].

So, for now add 'kotlinx.serialization' as dependency to the advisor in
order to access the property and leave a FIXME comment as reminder.

[1] Kotlin/kotlinx.serialization#1298
[2] Kotlin/kotlinx.serialization#1405
[3] Kotlin/kotlinx.serialization#1058

Signed-off-by: Frank Viernau <frank_viernau@epam.com>
fviernau added a commit to oss-review-toolkit/ort that referenced this issue Jul 13, 2022
The server does return vulnerabilities which do not have a severity
value in the dedicated property. The unspecified `databaseSpecific`
property often times holds a primitive `severity` property with values
such as `[HIGH, MEDIUM, LOW]`. Make use of these values as a fallback as
these to provide more indication than a `null` value.

Note: The data model of 'osv/client' currently uses subtypes of
JsonElement to expose a couple of unspecified JSON objects as
properties. Accessing these requires the client code to add
'kotlinx.serialization' as dependency which is not nice. A solution to
that would be to use "raw" string values containing the JSON, which is
unfortunately not yet possible but may become so in the future, see
[1][2][3].

So, for now add 'kotlinx.serialization' as dependency to the advisor in
order to access the property and leave a FIXME comment as reminder.

[1] Kotlin/kotlinx.serialization#1298
[2] Kotlin/kotlinx.serialization#1405
[3] Kotlin/kotlinx.serialization#1058

Signed-off-by: Frank Viernau <frank_viernau@epam.com>
fviernau added a commit to oss-review-toolkit/ort that referenced this issue Jul 13, 2022
The server does return vulnerabilities which do not have a severity
value in the dedicated property. The unspecified `databaseSpecific`
property often times holds a primitive `severity` property with values
such as `[HIGH, MEDIUM, LOW]`. Make use of these values as a fallback as
these to provide more indication than a `null` value.

Note: The data model of 'osv/client' currently uses subtypes of
JsonElement to expose a couple of unspecified JSON objects as
properties. Accessing these requires the client code to add
'kotlinx.serialization' as dependency which is not nice. A solution to
that would be to use "raw" string values containing the JSON, which is
unfortunately not yet possible but may become so in the future, see
[1][2][3].

So, for now add 'kotlinx.serialization' as dependency to the advisor in
order to access the property and leave a FIXME comment as reminder.

[1] Kotlin/kotlinx.serialization#1298
[2] Kotlin/kotlinx.serialization#1405
[3] Kotlin/kotlinx.serialization#1058

Signed-off-by: Frank Viernau <frank_viernau@epam.com>
@rescribet
Copy link

To add another use case, I'm converting between JsonObject and a map of AWS SDK AttributeValue. That also only has a number type (fromN), but contains other types like a string set.

Notice the hacky primitive.booleanOrNull

fun JsonObject.toAWSStructure(): Map<String, AttributeValue> =
    entries.associate { (k, v) -> k to v.toAWSStructure() }

fun JsonElement.toAWSStructure(): AttributeValue = when (this) {
    is JsonArray -> AttributeValue.fromL(this.jsonArray.map { it.toAWSStructure() })
    is JsonObject -> AttributeValue.fromM(this.toAWSStructure())
    is JsonNull -> AttributeValue.fromNul(true)
    is JsonPrimitive -> {
        val primitive = this.jsonPrimitive
        when {
            primitive.isString -> AttributeValue.fromS(this.content)
            primitive.booleanOrNull != null -> AttributeValue.fromBool(primitive.boolean)
            else -> AttributeValue.fromN(primitive.content)
        }
    }
}

Completing the type system would make it more concise and safe:

fun JsonElement.toAWSStructure(): AttributeValue = when (this) {
    is JsonObject -> AttributeValue.fromM(this.toAWSStructure())
    is JsonArray -> AttributeValue.fromL(this.jsonArray.map { it.toAWSStructure() })
    is JsonNull -> AttributeValue.fromNul(true)
    is JsonString -> AttributeValue.fromS(this.content)
    is JsonBool -> AttributeValue.fromBool(primitive.boolean)
    is JsonNumber -> AttributeValue.fromBool(primitive.content)
}

@chirag4semandex
Copy link

Found myself in the same boat, here is an alternate way to go around it

    when (val value = this[key]) {
        is JsonArray -> {  // handle arrays  }

        is JsonObject ->  {  //handle objects }
        is JsonNull -> null
        is JsonPrimitive -> {
            if( value.isString ) {
                value.content
            } else {
                value.booleanOrNull ?:
                value.intOrNull ?:
                value.longOrNull ?:
                value.doubleOrNull ?:
                value.content
            }
        }
        else -> value.toString()
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants