Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: No more automatic padding for fixed type #235

Merged
merged 1 commit into from
Jul 10, 2024
Merged

fix: No more automatic padding for fixed type #235

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 27 additions & 30 deletions .../kotlin/com/github/avrokotlin/avro4k/internal/encoder/direct/AbstractAvroDirectEncoder.kt
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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