DO NOT MERGE attempt to support sealed interface multiple inheritance

[williamboxhall]

@thake another weird edgecase that I encountered in our real-world application on top of Avro4k 1.5.0.

We happen to need to use multiple/diamond sealed interface inheritance in our application which confuses Avro4k:

Duplicate in union:com.github.avrokotlin.avro4k.schema.NegateExpr
org.apache.avro.AvroRuntimeException: Duplicate in union:com.github.avrokotlin.avro4k.schema.NegateExpr
    at org.apache.avro.Schema$UnionSchema.<init>(Schema.java:1197)
    at org.apache.avro.Schema.createUnion(Schema.java:247)
    at com.github.avrokotlin.avro4k.schema.UnionSchemaFor.schema(UnionSchemaFor.kt:20)
    at com.github.avrokotlin.avro4k.schema.ClassSchemaFor.buildField(ClassSchemaFor.kt:84)
    at com.github.avrokotlin.avro4k.schema.ClassSchemaFor.dataClassSchema(ClassSchemaFor.kt:63)
    at com.github.avrokotlin.avro4k.schema.ClassSchemaFor.schema(ClassSchemaFor.kt:43)
    at com.github.avrokotlin.avro4k.Avro.schema(Avro.kt:264)
    at com.github.avrokotlin.avro4k.Avro.schema(Avro.kt:269)

This is because the subtype scan finds NegateExpr twice, once through each branch of the diamond inheritance. As you can see in the second commit, I've tried just filtering out duplicates, but then I ran into a much more obscure error that I don't completely understand and probably need your help with:

Class 'NegateExpr' is not registered for polymorphic serialization in the scope of 'Expr'.
kotlinx.serialization.SerializationException: Class 'NegateExpr' is not registered for polymorphic serialization in the scope of 'Expr'.
Mark the base class as 'sealed' or register the serializer explicitly.
    at kotlinx.serialization.internal.AbstractPolymorphicSerializerKt.throwSubtypeNotRegistered(AbstractPolymorphicSerializer.kt:102)
    at kotlinx.serialization.internal.AbstractPolymorphicSerializerKt.throwSubtypeNotRegistered(AbstractPolymorphicSerializer.kt:113)
    at kotlinx.serialization.PolymorphicSerializerKt.findPolymorphicSerializer(PolymorphicSerializer.kt:96)
    at kotlinx.serialization.internal.AbstractPolymorphicSerializer.serialize(AbstractPolymorphicSerializer.kt:32)
    at kotlinx.serialization.encoding.Encoder$DefaultImpls.encodeSerializableValue(Encoding.kt:282)
    at kotlinx.serialization.encoding.AbstractEncoder.encodeSerializableValue(AbstractEncoder.kt:18)
    at kotlinx.serialization.encoding.AbstractEncoder.encodeSerializableElement(AbstractEncoder.kt:80)
    at com.github.avrokotlin.avro4k.schema.ReferencingMixedPolymorphic.write$Self(MixedPolymorphicSchemaTest.kt:40)
    at com.github.avrokotlin.avro4k.schema.ReferencingMixedPolymorphic$$serializer.serialize(MixedPolymorphicSchemaTest.kt:40)
    at com.github.avrokotlin.avro4k.schema.ReferencingMixedPolymorphic$$serializer.serialize(MixedPolymorphicSchemaTest.kt:40)
    at kotlinx.serialization.encoding.Encoder$DefaultImpls.encodeSerializableValue(Encoding.kt:282)
    at kotlinx.serialization.encoding.AbstractEncoder.encodeSerializableValue(AbstractEncoder.kt:18)
    at com.github.avrokotlin.avro4k.Avro.toRecord(Avro.kt:240)
    at com.github.avrokotlin.avro4k.Avro$openOutputStream$builder$1$1.invoke(Avro.kt:217)
    at com.github.avrokotlin.avro4k.Avro$openOutputStream$builder$1$1.invoke(Avro.kt:217)

As you can see I'm able to work around this by adding an explicit mapping from the top-level base class to the diamond inheriting subclass, but this is obviously not a good solution:

      val module = SerializersModule {
         polymorphic(OtherUnaryExpr::class) {
            subclass(ConstExpr::class)
         }
         polymorphic(OtherBinaryExpr::class) {
            subclass(MultiplicationExpr::class)
         }
         // this shouldn't be necessary
         polymorphic(Expr::class) {
            subclass(NegateExpr::class)
         }
      }

Keen to see if you can find a better solution to this problem as it's beyond the limits of my kotlinx.serialization, avro and avro4k knowledge.

[williamboxhall]

Hey @thake, would you like me to translate this into a Github issue?

[thake]

@williamboxhall no need. I am working on a solution to this on my spare time.

[williamboxhall]

Wonderful to hear @thake! We've been running into the second stacktrace in our application - it just seems like a general sealed interface issue. I noticed there isn't a SealedInterfaceIOTest which I think would expose this problem, let me know if you'd like me to contribute one on your branch :)

[williamboxhall]

@thake I just had a closer look and I believe the issue is that kotlinx-serialization expects the explicit serializers for each concrete type to be registered manually - and there is no process of traversing the type hierarchy and register each subtype explicitly. Do you think your implementation will resolve this? This isn't a problem for schema generation, only for serialization

[williamboxhall]

I came up with a pretty dirty hack to allow us to work around this in our application:

    polymorphic(SkillsCoachPublishedEventV1::class) {
        val subtypes = listOf(
            AccountSettingsPublishedEventV1::class,
            NotificationPreferencesPublishedEventV1::class,
            ProgramSubscriptionPublishedEventV1::class,
            InvitationPublishedEventV1::class,
            CohortPublishedEventV1::class
        )

        // explicitly register each concrete class
        subtypes.forEach {
            it.asNestedSealedConcreteClasses().forEach {
                subclass(it as KClass<SkillsCoachPublishedEventV1>, it.serializer())
            }
        }
    }
...
fun <T: Any> KClass<T>.asNestedSealedConcreteClasses(): List<KClass<out T>> {
    return when (this.isFinal) {
        true -> listOf(this)
        false -> this.sealedSubclasses.flatMap { it.asNestedSealedConcreteClasses() }
    }
}

this works fine and unblocks us

[thake]

A nice workaround! The workaround also seems to be compatible with the default json serialization of kotlinx.serialization.

I'm still working on this issue, but I can't give you any specific schedule when it will be ready. I'm currently trying out various options how to implement it correctly.

William Boxhall @.***> schrieb am Mo., 27. Sept. 2021, 04:05:

I came up with a pretty dirty hack to allow us to work around this in our application:

polymorphic(SkillsCoachPublishedEventV1::class) {
    val subtypes = listOf(
        AccountSettingsPublishedEventV1::class,
        NotificationPreferencesPublishedEventV1::class,
        ProgramSubscriptionPublishedEventV1::class,
        InvitationPublishedEventV1::class,
        CohortPublishedEventV1::class
    )

    subtypes.forEach {
        it.asNestedSealedConcreteClasses().forEach {
            subclass(it as KClass<SkillsCoachPublishedEventV1>, it.serializer())
        }
    }
}...fun <T: Any> KClass<T>.asNestedSealedConcreteClasses(): List<KClass<out T>> {
return when (this.isFinal) {
    true -> listOf(this)
    false -> this.sealedSubclasses.flatMap { it.asNestedSealedConcreteClasses() }
}

}

this works fine and unblocks us

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/avro-kotlin/avro4k/pull/113#issuecomment-927448270, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC7XPENZWJR52YHVVCWD57TUD7GOFANCNFSM5DWBHBLA .

[thake]

Closing this PR as the fix PR #119 includes the provided test code.