avro-kotlin / avro4k

Avro format support for Kotlin
Apache License 2.0
198 stars 37 forks source link

Stronger typing of data #265

Closed vongohren closed 1 month ago

vongohren commented 1 month ago

Describe the bug I want to have stronger type safty than this library offers. Every data creation in v2 has a return type of Any? In v1 one had toRecord

My code which does this jobb:

class KafkaProducer(
    val kafkaTemplate: KafkaTemplate<String, GenericRecord>
) {
    private val logger = LoggerFactory.getLogger(this::class.java)

    @OptIn(ExperimentalSerializationApi::class)
    fun sendAuditEvent(auditEvent: AuditEvent): Boolean {
        val topic = auditEvent.topic
        val className = auditEvent::class.simpleName
        try {
            if (topic.isEmpty()) {
                throw IllegalArgumentException("Topic for audit event must be set, preferably in the event class")
            }

            val genericData: Any? = Avro.encodeToGenericData(auditEvent)
            kafkaTemplate.send(
                topic,
                UUID.randomUUID().toString(),
                genericData as GenericRecord
            )
            // Returning true if the event was passed along successfully so we can use that in logs
            return true
        } catch (e: Exception) {
            logger.error("Failed to send Audit Kafka event: $className on topic $topic", e)
            // Returning false if the event was not passed along so we can use that in logs
            return false
        }
    }
} 

So for the send function which is a powerful method but springframework kafka, I want to be able to not having to cast to a random class.

So just wondering if there are any other method to follow.

To Reproduce Code to reproduce the behavior:

Use the kode in the explanation

In the explanation

Expected behavior Simple class passing without unsafe typing

Desktop (please complete the following information):

Additional context Add any other context about the problem here.

Chuckame commented 1 month ago

I'm sorry but I don't see any random class cast, as encodeToGenericData is not able to detect statically if the given content is a data class instance (which is encoded as a GenericRecord), a collection or an array (which is encoded as a GenericArray), a ByteArray (which is encoded as a ByteBuffer), a ByteArray wrapped in a value class having the @AvroFixed annotation (which is encoded as a GenericFixed), or a primitive value.

Your use case is also specific (but common I agree) where your event is a data class which always encode to a GenericRecord. But if you pass one of the other type, you will have something else than a GenericRecord.

Let's take the example of the kotlinx Json library: when you encode any data class using Json.encodeToElement, it always return a JsonElement, where it needs to be cast to JsonObject to allow iteration over fields by example. This is the same as ObjectMapper from jackson, or even the same from the standard java avro library.

Does it sound right for you ?

Chuckame commented 1 month ago

As a counter proposition, I would propose you to directly have a KafkaTemplate<String, Any?>, as I suppose you are using confluent schema registry, so everything returned by encodeToGenericData should be fully compatible with the serializer.

It would remove your random cast while still working the exact same as currently 😄

vongohren commented 1 month ago

@Chuckame thanks for the diligent feedback and I do agree. Just coming from earlier versions where it was a bit more stricter with the toRecord method. But I think your suggestion might be an valid option yes.

I will try it out :D

Closing for now, coming back if there are any further thoughts

Chuckame commented 1 month ago

With pleasure ! Yes, the v1 was only compatible with serializing data classes to records. Now v2 is able to serialize everything, that's why!

Don't hesitate to come back, thanks for your interest !