klaxon icon indicating copy to clipboard operation
klaxon copied to clipboard

Inconsistent treatment of delegated properties between serializing and deserializing

Open hughartk opened this issue 7 years ago • 5 comments

Related to https://github.com/cbeust/klaxon/issues/114, and is tested with 2.1.8.

Given:

interface Foo{ val x: Int }
data class FooImpl(override val x: Int): Foo
data class BarImpl(val y: Int, val foo: FooImpl): Foo by foo

Run the following code:

val originalJson= """{"foo" : {"x" : 1}, "y" : 1}"""
val instance = Klaxon().parse<BarImpl>(originalJson)!!
val newJson = Klaxon().toJsonString(instance)
println(newJson) //prints {"foo" : {"x" : 1}, "x" : 1, "y" : 1}
Klaxon().parse<BarImpl>(newJson)

The delegated property is expected to appear only in its originating class when we deserialize, but when we serialize, we duplicate it in the the outer object. Consequently, we error on the last line when we try to deserialize our own serialization with the following stacktrace:

Exception in thread "main" java.lang.NoSuchFieldException: x
	at java.lang.Class.getDeclaredField(Class.java:2070)
	at com.beust.klaxon.Klaxon$findConverterFromClass$1.invoke(Klaxon.kt:208)
	at com.beust.klaxon.Klaxon.findConverterFromClass(Klaxon.kt:217)
	at com.beust.klaxon.JsonObjectConverter.retrieveKeyValues(JsonObjectConverter.kt:80)
	at com.beust.klaxon.JsonObjectConverter.fromJson(JsonObjectConverter.kt:28)
	at com.beust.klaxon.DefaultConverter.convertValue(DefaultConverter.kt:165)
	at com.beust.klaxon.DefaultConverter.fromJson(DefaultConverter.kt:13)
	at com.beust.klaxon.JsonObjectConverter.retrieveKeyValues(JsonObjectConverter.kt:81)
	at com.beust.klaxon.JsonObjectConverter.fromJson(JsonObjectConverter.kt:28)
	at com.beust.klaxon.DefaultConverter.convertValue(DefaultConverter.kt:165)
	at com.beust.klaxon.DefaultConverter.fromJson(DefaultConverter.kt:13)
	at com.beust.klaxon.Klaxon.fromJsonObject(Klaxon.kt:272)

Happens regardless of whether we declare BarImpl's Foo parameter as Foo or FooImpl.

Problem goes away if BarImpl is given the body:

{
	@Json(ignored = true)
	override val x = foo.x
}

hughartk avatar Feb 24 '18 17:02 hughartk

When you are parsing this JSON with the type parameter as BarImpl, you are telling Klaxon that the expected shape of JSON is a JSON object with fields:

  • x of type Int
  • foo of type FooImpl

I think this is working as expected: Klaxon infers the shape of the JSON object from the constructor parameters, nothing else. Delegation is not relevant here.

cbeust avatar Feb 25 '18 03:02 cbeust

I agree that the way Klaxon is parsing the original JSON is fine. That is not the problem. The problem is that it doesn't serialize back out to JSON it the same way.

Note that when I deserialized and re-serialized a BarImpl instance, not only did I not reproduce the original JSON, I produced JSON that Klaxon can no longer parse back to the originating object type. I believe this is a pretty unambiguous bug. I should be able to serialize an object to JSON and then deserialize to get my object back.

hughartk avatar Feb 25 '18 05:02 hughartk

Fair enough.

cbeust avatar Feb 25 '18 05:02 cbeust

I cannot get this code to pass at all, can you re-paste a full working snippet?

cbeust avatar Feb 26 '18 05:02 cbeust

Here is a full .kt file that should print a stack trace when run, but should not crash. Comments in source describe steps and behavior.

import com.beust.klaxon.Klaxon

interface Foo{ val x: Int }
data class FooImpl(override val x: Int): Foo
data class BarImpl(val y: Int, val foo: FooImpl): Foo by foo

object Main {
    @JvmStatic
    fun main(argv: Array<String>) {
        val originalJson= """{"foo" : {"x" : 1}, "y" : 1}"""
        val instance = Klaxon().parse<BarImpl>(originalJson)!! //Going from JSON to BarImpl
        val newJson = Klaxon().toJsonString(instance) //Going back from BarImpl to JSON
        println(newJson) //prints {"foo" : {"x" : 1}, "x" : 1, "y" : 1} instead of the original JSON
        try {
            /* Attempting to go back to BarImpl again, from our newly generated JSON.
             * The following line would succeed if newJson was equal to originalJson, but since they differ,
             * it fails with a NoSuchFieldException */
            Klaxon().parse<BarImpl>(newJson)!!
        }
        catch (e: Exception)
        {
            e.printStackTrace()
        }
    }
}

hughartk avatar Feb 27 '18 01:02 hughartk