From 01ccbfc5d72bab14bc53fe5fd24a5aa3684ebdd0 Mon Sep 17 00:00:00 2001 From: Chuckame Date: Wed, 10 Jul 2024 22:41:44 +0200 Subject: [PATCH] fix: No more automatic padding for fixed type --- .../direct/AbstractAvroDirectEncoder.kt | 57 +++++++++---------- .../generic/AbstractAvroGenericEncoder.kt | 45 ++++++++------- .../encoder/generic/FixedGenericEncoder.kt | 4 +- .../avrokotlin/avro4k/internal/helpers.kt | 25 ++------ .../avro4k/encoding/AvroFixedEncodingTest.kt | 35 ++++++++---- 5 files changed, 80 insertions(+), 86 deletions(-) diff --git a/src/main/kotlin/com/github/avrokotlin/avro4k/internal/encoder/direct/AbstractAvroDirectEncoder.kt b/src/main/kotlin/com/github/avrokotlin/avro4k/internal/encoder/direct/AbstractAvroDirectEncoder.kt index d1829ca..999711c 100644 --- a/src/main/kotlin/com/github/avrokotlin/avro4k/internal/encoder/direct/AbstractAvroDirectEncoder.kt +++ b/src/main/kotlin/com/github/avrokotlin/avro4k/internal/encoder/direct/AbstractAvroDirectEncoder.kt @@ -7,7 +7,6 @@ import com.github.avrokotlin.avro4k.encodeResolving import com.github.avrokotlin.avro4k.internal.BadEncodedValueError import com.github.avrokotlin.avro4k.internal.SerializerLocatorMiddleware import com.github.avrokotlin.avro4k.internal.isFullNameOrAliasMatch -import com.github.avrokotlin.avro4k.internal.zeroPadded import kotlinx.serialization.SerializationException import kotlinx.serialization.SerializationStrategy import kotlinx.serialization.descriptors.PolymorphicKind @@ -136,7 +135,7 @@ internal sealed class AbstractAvroDirectEncoder( override fun encodeBytes(value: ByteBuffer) { encodeResolving( - { BadEncodedValueError(value, currentWriterSchema, Schema.Type.BYTES, Schema.Type.STRING) } + { BadEncodedValueError(value, currentWriterSchema, Schema.Type.BYTES, Schema.Type.STRING, Schema.Type.FIXED) } ) { when (it.type) { Schema.Type.STRING, @@ -145,12 +144,13 @@ internal sealed class AbstractAvroDirectEncoder( { binaryEncoder.writeBytes(value) } } - Schema.Type.FIXED -> - if (value.remaining() <= it.fixedSize) { - { binaryEncoder.writeFixed(value.array().zeroPadded(it, endPadded = false)) } + Schema.Type.FIXED -> { + if (value.remaining() == it.fixedSize) { + { binaryEncoder.writeFixed(value.array()) } } else { null } + } else -> null } @@ -159,7 +159,7 @@ internal sealed class AbstractAvroDirectEncoder( override fun encodeBytes(value: ByteArray) { encodeResolving( - { BadEncodedValueError(value, currentWriterSchema, Schema.Type.BYTES, Schema.Type.STRING) } + { BadEncodedValueError(value, currentWriterSchema, Schema.Type.BYTES, Schema.Type.STRING, Schema.Type.FIXED) } ) { when (it.type) { Schema.Type.STRING, @@ -168,12 +168,13 @@ internal sealed class AbstractAvroDirectEncoder( { binaryEncoder.writeBytes(value) } } - Schema.Type.FIXED -> - if (value.size <= it.fixedSize) { - { binaryEncoder.writeFixed(value.zeroPadded(it, endPadded = false)) } + Schema.Type.FIXED -> { + if (value.size == it.fixedSize) { + { binaryEncoder.writeFixed(value) } } else { null } + } else -> null } @@ -182,22 +183,16 @@ internal sealed class AbstractAvroDirectEncoder( override fun encodeFixed(value: GenericFixed) { encodeResolving( - { BadEncodedValueError(value, currentWriterSchema, Schema.Type.FIXED) } + { BadEncodedValueError(value, currentWriterSchema, Schema.Type.FIXED, Schema.Type.STRING, Schema.Type.BYTES) } ) { when (it.type) { - Schema.Type.FIXED -> - when (it.fullName) { - value.schema.fullName -> - when (it.fixedSize) { - value.bytes().size -> { - { binaryEncoder.writeFixed(value.bytes()) } - } - - else -> null - } - - else -> null + Schema.Type.FIXED -> { + if (it.fullName == value.schema.fullName && it.fixedSize == value.bytes().size) { + { binaryEncoder.writeFixed(value.bytes()) } + } else { + null } + } Schema.Type.STRING, Schema.Type.BYTES, @@ -212,16 +207,14 @@ internal sealed class AbstractAvroDirectEncoder( override fun encodeFixed(value: ByteArray) { encodeResolving( - { BadEncodedValueError(value, currentWriterSchema, Schema.Type.FIXED) } + { BadEncodedValueError(value, currentWriterSchema, Schema.Type.FIXED, Schema.Type.STRING, Schema.Type.BYTES) } ) { when (it.type) { Schema.Type.FIXED -> - when (it.fixedSize) { - value.size -> { - { binaryEncoder.writeFixed(value) } - } - - else -> null + if (it.fixedSize == value.size) { + { binaryEncoder.writeFixed(value) } + } else { + null } Schema.Type.STRING, @@ -414,7 +407,11 @@ internal sealed class AbstractAvroDirectEncoder( } Schema.Type.FIXED -> { - { binaryEncoder.writeFixed(value.encodeToByteArray().zeroPadded(it, endPadded = true)) } + if (value.length == it.fixedSize) { + { binaryEncoder.writeFixed(value.encodeToByteArray()) } + } else { + null + } } Schema.Type.ENUM -> { diff --git a/src/main/kotlin/com/github/avrokotlin/avro4k/internal/encoder/generic/AbstractAvroGenericEncoder.kt b/src/main/kotlin/com/github/avrokotlin/avro4k/internal/encoder/generic/AbstractAvroGenericEncoder.kt index 75cf280..a064891 100644 --- a/src/main/kotlin/com/github/avrokotlin/avro4k/internal/encoder/generic/AbstractAvroGenericEncoder.kt +++ b/src/main/kotlin/com/github/avrokotlin/avro4k/internal/encoder/generic/AbstractAvroGenericEncoder.kt @@ -8,7 +8,6 @@ import com.github.avrokotlin.avro4k.internal.BadEncodedValueError import com.github.avrokotlin.avro4k.internal.SerializerLocatorMiddleware import com.github.avrokotlin.avro4k.internal.isFullNameOrAliasMatch import com.github.avrokotlin.avro4k.internal.toIntExact -import com.github.avrokotlin.avro4k.internal.zeroPadded import kotlinx.serialization.SerializationException import kotlinx.serialization.SerializationStrategy import kotlinx.serialization.descriptors.PolymorphicKind @@ -141,7 +140,11 @@ internal abstract class AbstractAvroGenericEncoder : AbstractEncoder(), AvroEnco } Schema.Type.FIXED -> { - { encodeValue(value.array().toPaddedGenericFixed(schema, endPadded = false)) } + if (value.remaining() == schema.fixedSize) { + { encodeValue(value.array()) } + } else { + null + } } Schema.Type.STRING -> { @@ -163,7 +166,11 @@ internal abstract class AbstractAvroGenericEncoder : AbstractEncoder(), AvroEnco } Schema.Type.FIXED -> { - { encodeValue(value.toPaddedGenericFixed(schema, endPadded = false)) } + if (value.size == schema.fixedSize) { + { encodeValue(value) } + } else { + null + } } Schema.Type.STRING -> { @@ -181,12 +188,10 @@ internal abstract class AbstractAvroGenericEncoder : AbstractEncoder(), AvroEnco ) { schema -> when (schema.type) { Schema.Type.FIXED -> - when (schema.fullName) { - value.schema.fullName -> { - { encodeValue(value) } - } - - else -> null + if (schema.fullName == value.schema.fullName && schema.fixedSize == value.bytes().size) { + { encodeValue(value) } + } else { + null } Schema.Type.BYTES -> { @@ -208,7 +213,11 @@ internal abstract class AbstractAvroGenericEncoder : AbstractEncoder(), AvroEnco ) { schema -> when (schema.type) { Schema.Type.FIXED -> { - { encodeValue(value.toPaddedGenericFixed(schema, endPadded = false)) } + if (value.size == schema.fixedSize) { + { encodeValue(value) } + } else { + null + } } Schema.Type.BYTES -> { @@ -382,7 +391,11 @@ internal abstract class AbstractAvroGenericEncoder : AbstractEncoder(), AvroEnco } Schema.Type.FIXED -> { - { encodeValue(value.encodeToByteArray().toPaddedGenericFixed(schema, endPadded = true)) } + if (value.length == schema.fixedSize) { + { encodeValue(value.encodeToByteArray()) } + } else { + null + } } Schema.Type.ENUM -> { @@ -424,14 +437,4 @@ internal abstract class AbstractAvroGenericEncoder : AbstractEncoder(), AvroEnco } } } -} - -private fun ByteArray.toPaddedGenericFixed( - schema: Schema, - endPadded: Boolean, -): GenericFixed { - return GenericData.Fixed( - schema, - zeroPadded(schema, endPadded = endPadded) - ) } \ No newline at end of file diff --git a/src/main/kotlin/com/github/avrokotlin/avro4k/internal/encoder/generic/FixedGenericEncoder.kt b/src/main/kotlin/com/github/avrokotlin/avro4k/internal/encoder/generic/FixedGenericEncoder.kt index f212c6d..e6f02b1 100644 --- a/src/main/kotlin/com/github/avrokotlin/avro4k/internal/encoder/generic/FixedGenericEncoder.kt +++ b/src/main/kotlin/com/github/avrokotlin/avro4k/internal/encoder/generic/FixedGenericEncoder.kt @@ -16,10 +16,10 @@ internal class FixedGenericEncoder( private val onEncoded: (GenericFixed) -> Unit, ) : AbstractEncoder() { private val buffer = ByteArray(schema.fixedSize) - private var pos = schema.fixedSize - arraySize + private var pos = 0 init { - if (arraySize > schema.fixedSize) { + if (arraySize != schema.fixedSize) { throw SerializationException("Actual collection size $arraySize is greater than schema fixed size $schema") } } diff --git a/src/main/kotlin/com/github/avrokotlin/avro4k/internal/helpers.kt b/src/main/kotlin/com/github/avrokotlin/avro4k/internal/helpers.kt index 3812169..16f9319 100644 --- a/src/main/kotlin/com/github/avrokotlin/avro4k/internal/helpers.kt +++ b/src/main/kotlin/com/github/avrokotlin/avro4k/internal/helpers.kt @@ -7,7 +7,6 @@ import com.github.avrokotlin.avro4k.AvroAlias import com.github.avrokotlin.avro4k.AvroProp import kotlinx.serialization.ExperimentalSerializationApi import kotlinx.serialization.InternalSerializationApi -import kotlinx.serialization.SerializationException import kotlinx.serialization.descriptors.PolymorphicKind import kotlinx.serialization.descriptors.SerialDescriptor import kotlinx.serialization.descriptors.SerialKind @@ -48,7 +47,7 @@ internal fun Schema.isNamedSchema(): Boolean { } internal fun Schema.isFullNameOrAliasMatch(descriptor: SerialDescriptor): Boolean { - return isFullNameMatch(descriptor.nonNullSerialName) || descriptor.findAnnotation()?.value?.any { isFullNameMatch(it) } == true + return isFullNameMatch(descriptor.nonNullSerialName) || descriptor.aliases.any { isFullNameMatch(it) } } internal fun Schema.isFullNameMatch(fullNameToMatch: String): Boolean { @@ -57,6 +56,9 @@ internal fun Schema.isFullNameMatch(fullNameToMatch: String): Boolean { aliases.any { it == fullNameToMatch } } +internal val SerialDescriptor.aliases: Set get() = + findAnnotation()?.value?.toSet() ?: emptySet() + private val SCHEMA_PLACEHOLDER = Schema.create(Schema.Type.NULL) internal fun Schema.copy( @@ -172,23 +174,4 @@ internal val AvroProp.jsonNode: JsonNode internal fun SerialDescriptor.getElementIndexNullable(name: String): Int? { return getElementIndex(name).takeIf { it >= 0 } -} - -internal fun ByteArray.zeroPadded( - schema: Schema, - endPadded: Boolean, -): ByteArray { - if (size > schema.fixedSize) { - throw SerializationException("Actual byte array size $size is greater than schema fixed size $schema") - } - val padSize = schema.fixedSize - size - return if (padSize > 0) { - if (endPadded) { - this + ByteArray(padSize) - } else { - ByteArray(padSize) + this - } - } else { - this - } } \ No newline at end of file diff --git a/src/test/kotlin/com/github/avrokotlin/avro4k/encoding/AvroFixedEncodingTest.kt b/src/test/kotlin/com/github/avrokotlin/avro4k/encoding/AvroFixedEncodingTest.kt index 3a90468..30b05a6 100644 --- a/src/test/kotlin/com/github/avrokotlin/avro4k/encoding/AvroFixedEncodingTest.kt +++ b/src/test/kotlin/com/github/avrokotlin/avro4k/encoding/AvroFixedEncodingTest.kt @@ -12,6 +12,7 @@ import kotlinx.serialization.Serializable import kotlinx.serialization.SerializationException import kotlinx.serialization.encodeToByteArray import org.apache.avro.generic.GenericData +import org.junit.jupiter.api.assertThrows import kotlin.io.path.Path internal class AvroFixedEncodingTest : StringSpec({ @@ -57,26 +58,36 @@ internal class AvroFixedEncodingTest : StringSpec({ } } - "encode/decode ByteArray as FIXED when schema is Type.Fixed" { - AvroAssertions.assertThat(ByteArrayFixedTest(byteArrayOf(1, 4, 9))) - .isEncodedAs( - record(byteArrayOf(0, 0, 0, 0, 0, 1, 4, 9)), - expectedDecodedValue = ByteArrayFixedTest(byteArrayOf(0, 0, 0, 0, 0, 1, 4, 9)) - ) - } + "Should fail when the fixed size is not respected" { + assertThrows { + Avro.encodeToByteArray(ByteArrayFixedTest(byteArrayOf(1, 4, 9))) + } - "encode/decode strings as GenericFixed and pad bytes when schema is Type.FIXED" { @Serializable @SerialName("Foo") data class StringFoo( @AvroFixed(7) val a: String?, ) + assertThrows { + Avro.encodeToByteArray(StringFoo("hello")) + } + } + + "encode/decode ByteArray as FIXED" { + AvroAssertions.assertThat(ByteArrayFixedTest(byteArrayOf(0, 0, 0, 0, 0, 1, 4, 9))) + .isEncodedAs(record(byteArrayOf(0, 0, 0, 0, 0, 1, 4, 9))) + } + + "encode/decode strings as GenericFixed when schema is Type.FIXED" { + @Serializable + @SerialName("Foo") + data class StringFoo( + @AvroFixed(5) val a: String?, + ) + AvroAssertions.assertThat(StringFoo("hello")) - .isEncodedAs( - record(byteArrayOf(104, 101, 108, 108, 111, 0, 0)), - StringFoo(String("hello".toByteArray() + byteArrayOf(0, 0))) - ) + .isEncodedAs(record(byteArrayOf(104, 101, 108, 108, 111))) } // "Handle FIXED in unions with the good and bad fullNames and aliases" {