Skip to content

Commit

Permalink
fix: No more automatic padding for fixed type
Browse files Browse the repository at this point in the history
  • Loading branch information
@Chuckame
Chuckame committed Jul 10, 2024
1 parent 489b45a commit 01ccbfc
Show file tree
Hide file tree
Showing 5 changed files with 80 additions and 86 deletions.
57 changes: 27 additions & 30 deletions .../kotlin/com/github/avrokotlin/avro4k/internal/encoder/direct/AbstractAvroDirectEncoder.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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
}
Expand All @@ -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,
Expand All @@ -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
}
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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 -> {
Expand Down
45 changes: 24 additions & 21 deletions ...otlin/com/github/avrokotlin/avro4k/internal/encoder/generic/AbstractAvroGenericEncoder.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 -> {
Expand All @@ -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 -> {
Expand All @@ -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 -> {
Expand All @@ -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 -> {
Expand Down Expand Up @@ -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 -> {
Expand Down Expand Up @@ -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)
)
}
4 changes: 2 additions & 2 deletions src/main/kotlin/com/github/avrokotlin/avro4k/internal/encoder/generic/FixedGenericEncoder.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
}
Expand Down
25 changes: 4 additions & 21 deletions src/main/kotlin/com/github/avrokotlin/avro4k/internal/helpers.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -48,7 +47,7 @@ internal fun Schema.isNamedSchema(): Boolean {
}

internal fun Schema.isFullNameOrAliasMatch(descriptor: SerialDescriptor): Boolean {
return isFullNameMatch(descriptor.nonNullSerialName) || descriptor.findAnnotation<AvroAlias>()?.value?.any { isFullNameMatch(it) } == true
return isFullNameMatch(descriptor.nonNullSerialName) || descriptor.aliases.any { isFullNameMatch(it) }
}

internal fun Schema.isFullNameMatch(fullNameToMatch: String): Boolean {
Expand All @@ -57,6 +56,9 @@ internal fun Schema.isFullNameMatch(fullNameToMatch: String): Boolean {
aliases.any { it == fullNameToMatch }
}

internal val SerialDescriptor.aliases: Set<String> get() =
findAnnotation<AvroAlias>()?.value?.toSet() ?: emptySet()

private val SCHEMA_PLACEHOLDER = Schema.create(Schema.Type.NULL)

internal fun Schema.copy(
Expand Down Expand Up @@ -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
}
}
35 changes: 23 additions & 12 deletions src/test/kotlin/com/github/avrokotlin/avro4k/encoding/AvroFixedEncodingTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -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<SerializationException> {
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<SerializationException> {
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" {
Expand Down

0 comments on commit 01ccbfc

Please sign in to comment.