fix: properly handle nested and sibling structures

This closes #57
esensar · Oct 26, 2021 · d436bb1 · d436bb1
1 parent 036bb11
commit d436bb1
Show file tree

Hide file tree

Showing 5 changed files with 65 additions and 5 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,7 @@
 All notable changes to this project will be documented in this file. This change log follows the conventions of [keepachangelog.com](http://keepachangelog.com/).
 
 ## [Unreleased]
+- Fixed issues with nested structure deserialization ([#57][i57])
 
 ## [0.4.1] - 2021-10-24
 - Fixed issues with platform specific release artifacts ([#55][i55])
@@ -86,4 +87,5 @@ MsgPack.default.encodeToByteArray(...)
 [i19]: https://github.com/esensar/kotlinx-serialization-msgpack/issues/19
 [i20]: https://github.com/esensar/kotlinx-serialization-msgpack/issues/20
 [i55]: https://github.com/esensar/kotlinx-serialization-msgpack/issues/55
+[i57]: https://github.com/esensar/kotlinx-serialization-msgpack/issues/57
 [p40]: https://github.com/esensar/kotlinx-serialization-msgpack/pull/40
diff --git a/...monMain/kotlin/com.ensarsarajcic.kotlinx.serialization.msgpack/internal/MsgPackDecoder.kt b/...monMain/kotlin/com.ensarsarajcic.kotlinx.serialization.msgpack/internal/MsgPackDecoder.kt
@@ -27,10 +27,13 @@ internal class BasicMsgPackDecoder(
 
     override fun decodeElementIndex(descriptor: SerialDescriptor): Int {
         if (descriptor.kind in arrayOf(StructureKind.CLASS, StructureKind.OBJECT)) {
-            // TODO Improve structure end logic
-            //      This will probably fail in nested structures
-            val fieldName = kotlin.runCatching { decodeString() }.getOrNull() ?: return CompositeDecoder.DECODE_DONE
-            return descriptor.getElementIndex(fieldName)
+            val next = dataBuffer.peekSafely()
+            if (next != null && MsgPackType.String.isString(next)) {
+                val fieldName = kotlin.runCatching { decodeString() }.getOrNull() ?: return CompositeDecoder.DECODE_DONE
+                return descriptor.getElementIndex(fieldName)
+            } else {
+                return CompositeDecoder.DECODE_DONE
+            }
         }
         return 0
     }
@@ -190,6 +193,15 @@ internal class ClassMsgPackDecoder(
 ) : Decoder by basicMsgPackDecoder, CompositeDecoder by basicMsgPackDecoder, MsgPackTypeDecoder by basicMsgPackDecoder {
     override val serializersModule: SerializersModule = basicMsgPackDecoder.serializersModule
 
+    private var decodedElements = 0
+
+    override fun decodeElementIndex(descriptor: SerialDescriptor): Int {
+        if (decodedElements >= descriptor.elementsCount) return CompositeDecoder.DECODE_DONE
+        val result = basicMsgPackDecoder.decodeElementIndex(descriptor)
+        if (result != CompositeDecoder.DECODE_DONE) decodedElements++
+        return result
+    }
+
     override fun decodeSequentially(): Boolean = false
 }
 

diff --git a/...onMain/kotlin/com.ensarsarajcic.kotlinx.serialization.msgpack/stream/MsgPackDataBuffer.kt b/...onMain/kotlin/com.ensarsarajcic.kotlinx.serialization.msgpack/stream/MsgPackDataBuffer.kt
@@ -22,6 +22,7 @@ class MsgPackDataInputBuffer(private val byteArray: ByteArray) : MsgPackDataBuff
     }
 
     fun peek(): Byte = byteArray.getOrNull(index) ?: throw Exception("End of stream")
+    fun peekSafely(): Byte? = byteArray.getOrNull(index)
 
     // Increases index only if next byte is not null
     fun nextByteOrNull(): Byte? = byteArray.getOrNull(index)?.also { index++ }

diff --git a/...pack/src/commonTest/kotlin/com/ensarsarajcic/kotlinx/serialization/msgpack/MsgPackTest.kt b/...pack/src/commonTest/kotlin/com/ensarsarajcic/kotlinx/serialization/msgpack/MsgPackTest.kt
@@ -5,15 +5,19 @@ import kotlinx.serialization.KSerializer
 import kotlinx.serialization.builtins.ArraySerializer
 import kotlinx.serialization.builtins.ByteArraySerializer
 import kotlinx.serialization.builtins.MapSerializer
+import kotlinx.serialization.builtins.PairSerializer
+import kotlinx.serialization.builtins.TripleSerializer
 import kotlinx.serialization.builtins.nullable
 import kotlinx.serialization.builtins.serializer
+import kotlinx.serialization.decodeFromByteArray
 import kotlinx.serialization.encodeToByteArray
-import kotlinx.serialization.serializer
 import kotlin.test.Test
 import kotlin.test.assertEquals
 import kotlin.test.assertTrue
 import kotlin.test.fail
 
+typealias NestedMessage = Pair<List<Pair<String, String>>, String>
+
 internal class MsgPackTest {
     @Test
     fun testBooleanEncode() {
@@ -290,6 +294,41 @@ internal class MsgPackTest {
         testPairs(TestData.uIntTestPairs.map { it.first }, Int.serializer())
     }
 
+    @Test
+    fun testPairsEncode() {
+        testEncodePairs(PairSerializer(String.serializer(), String.serializer()), *TestData.pairsTestPairs)
+    }
+
+    @Test
+    fun testPairsDecode() {
+        testDecodePairs(PairSerializer(String.serializer(), String.serializer()), *TestData.pairsTestPairs)
+    }
+
+    @Test
+    fun testTriplesEncode() {
+        testEncodePairs(
+            TripleSerializer(String.serializer(), String.serializer(), String.serializer()),
+            *TestData.triplesTestPairs
+        )
+    }
+
+    @Test
+    fun testTriplesDecode() {
+        testDecodePairs(TripleSerializer(String.serializer(), String.serializer(), String.serializer()), *TestData.triplesTestPairs)
+    }
+
+    @Test
+    fun testNestedStructures() {
+        val sm1: NestedMessage = listOf("Alice" to "Bob", "Charley" to "Delta") to "Random message Body here"
+        val result = MsgPack.encodeToByteArray(sm1)
+        println(result.toHex())
+        println(result.toList())
+        println(result.size)
+        println(MsgPack.decodeFromByteArray<Pair<Int, Int>>(MsgPack.encodeToByteArray(1 to 2)))
+        val result2 = MsgPack.decodeFromByteArray<NestedMessage>(result)
+        assertEquals(sm1, result2)
+    }
+
     @Test
     fun testStrictWrites() {
         fun <T> testPairs(dataList: Array<Pair<String, T>>, serializer: KSerializer<T>) {

diff --git a/...msgpack/src/commonTest/kotlin/com/ensarsarajcic/kotlinx/serialization/msgpack/TestData.kt b/...msgpack/src/commonTest/kotlin/com/ensarsarajcic/kotlinx/serialization/msgpack/TestData.kt
@@ -149,6 +149,12 @@ object TestData {
     val sampleClassTestPairs = arrayOf(
         "83aa74657374537472696e67a3646566a774657374496e747bab74657374426f6f6c65616ec3" to SampleClass("def", 123, true)
     )
+    val pairsTestPairs: Array<Pair<String, Pair<String, String>>> = arrayOf(
+        "82a56669727374a5416c696365a67365636f6e64a3426f62" to Pair("Alice", "Bob")
+    )
+    val triplesTestPairs: Array<Pair<String, Triple<String, String, String>>> = arrayOf(
+        "83a56669727374a5416c696365a67365636f6e64a3426f62a57468697264a454657374" to Triple("Alice", "Bob", "Test")
+    )
 }
 
 @Serializable(with = CustomExtensionSerializer::class)