Moshi: PolymorphicJsonAdapterFactory, .failOnUnknown() and Kotlin's Sealed Classes

[mfjtrifork]

Currently having an issue with PolymorphicJsonAdapterFactory, .failOnUnknown() and Kotlins sealed classs.
My situation: I have a JSON string with a list of objects which are subtypes of MyClass with one shared property type and i want to convert this to a Kotlin List of these subclasses. I also want to enable .failOnUnknown on my adapter when debugging (Due to still being in development and i want my tests to fail if the parsing encounters unexpected labels).
The MyClass and it's subtypes

enum class MyType {
    ClassA,
    ClassB
}

sealed class MyClass(
        @Json(name = "type") val type: MyType
) {
    @JsonClass(generateAdapter = true)
    class A(
            @Json(name = "type") type: MyType,
            val value: Double
    ) : MyClass(type)

    @JsonClass(generateAdapter = true)
    class B(
            @Json(name = "type") type: MyType,
            val text: String
    ) : MyClass(type)
}

My adapter factory setup:

PolymorphicJsonAdapterFactory.of(MyClass::class.java, "type")
        .withSubtype(
                MyClass.A::class.java,
                MyType.ClassA.name
        )
        .withSubtype(
                MyClass.B::class.java,
                MyType.ClassB.name
        )

My JsonAdapter setup

Moshi
        .Builder()
        .add(adapterFactory)
        .add(KotlinJsonAdapterFactory())
        .build()
        .adapter<MyClass>(MyClass::class.java)
        .failOnUnknown()

Everything works well, if the first property of the JSON objects in the list is the type label [{ "type": "ClassA", "value": 0.1 }], but if the JSON is structured like this: [{ "value": 0.1 , "type": "ClassA" }], it fails due failOnUnknown being enabled and hitting the value label first, which doesn't exist in MyClass.

Any thoughts on this - is there a better approach? :)

[swankjesse]

It's a bug in our PolymorphicJsonAdapter implementation. It needs to turn off failOnUnknown when it seeks the type label.

[mfjtrifork]

@swankjesse What is the course of action now - do you guys need more from me or? :)
Edit: And thanks for the reply btw. 👍

[NightlyNexus]

This is fixed in master https://github.com/square/moshi/pull/792

[mfjtrifork]

@NightlyNexus Do you have any info on when this will be released? :)

[timmc]

I'm not so sure #792 actually fixes this. Using the latest 1.9.0-SNAPSHOT I still get a failing test with this reproduction case:

import com.squareup.moshi.Moshi
import com.squareup.moshi.adapters.PolymorphicJsonAdapterFactory
import com.squareup.moshi.kotlin.reflect.KotlinJsonAdapterFactory
import org.junit.Test

class UtilsTest {

    @Test
    fun polymorph() {
        val polyAdapter = PolymorphicJsonAdapterFactory.of(Parent::class.java, "which")
            .withSubtype(FocalChild::class.java, "focal")
            .withSubtype(OtherChild::class.java, "other")
        val moshi = Moshi.Builder()
            .add(polyAdapter)
            .add(KotlinJsonAdapterFactory())
            .build()

        val json = """
            {
              "which": "focal",
              "stuff": "nonsense"
            }
        """.trimIndent()
        println(moshi.adapter(Parent::class.java).failOnUnknown().fromJson(json))
    }
}

sealed class Parent(
    val which: String
)

data class FocalChild(
    val stuff: String
): Parent("focal")

data class OtherChild(
    val foo: String
): Parent("other")

Produces com.squareup.moshi.JsonDataException: Cannot skip unexpected NAME at $. even though the type key is first and even if I move the data classes into the body of the sealed class (as shown in the initial post).

[timmc]

(This also happens if I remove the inherited which field entirely, which puts it in better line with the documentation, or even if I make Parent into just a blank interface.)

[timmc]

The stack trace, for reference:

com.squareup.moshi.JsonDataException: Cannot skip unexpected NAME at $.
    at com.squareup.moshi.JsonUtf8Reader.skipName(JsonUtf8Reader.java:589)
    at com.squareup.moshi.kotlin.reflect.KotlinJsonAdapter.fromJson(KotlinJsonAdapter.kt:72)
    at com.squareup.moshi.internal.NullSafeJsonAdapter.fromJson(NullSafeJsonAdapter.java:40)
    at com.squareup.moshi.adapters.PolymorphicJsonAdapterFactory$PolymorphicJsonAdapter.fromJson(PolymorphicJsonAdapterFactory.java:248)
    at com.squareup.moshi.internal.NullSafeJsonAdapter.fromJson(NullSafeJsonAdapter.java:40)
    at com.squareup.moshi.JsonAdapter$4.fromJson(JsonAdapter.java:212)
    at com.squareup.moshi.JsonAdapter.fromJson(JsonAdapter.java:42)
    at org.timmc.cavern.spelunk.UtilsTest.polymorph(UtilsTest.kt:50)
        [...]

And on 1.9.0-SNAPSHOT I seem to be able to work around this problem by explicitly adding the label field to each subtype. I think that's the issue—the label field isn't getting properly skipped when invoked the subtype's adapter.

sealed class Parent(
)

data class FocalChild(
    val which: String = "focal",
    val stuff: String
): Parent()

data class OtherChild(
    val which: String = "other",
    val foo: String
): Parent()

ETA: This workaround works on 1.8.0 as well, without the commit from that PR.

[timmc]

...never mind, that workaround results in the label being written twice in the output, as in #874. And marking the label field @Transient doesn't help, because that just reverts to the unwanted behavior. So, no workaround.

[ZacSweers]

I think the solution for this is we need to be able to tell it to skip a known unknown when delegating. I think the test that was added for this is missing a variant of this subtyping, which is why it was missed before.

[ZacSweers]

I've put up a proposal implementation to resolve this in #925