Skip to content

Commit f2b0258

Browse files
shanshinshanshin
authored
Implemented encoding null in key and value of a map in Protobuf (#2910)
Also improved `null` encoding error messages. Resolves #2760
1 parent b43a654 commit f2b0258

File tree

3 files changed

+65
-27
lines changed

3 files changed

+65
-27
lines changed

‎formats/protobuf/commonMain/src/kotlinx/serialization/protobuf/internal/ProtobufTaggedEncoder.kt

+18-11
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ internal abstract class ProtobufTaggedEncoder : ProtobufTaggedBase(), Encoder, C
1414
ACCEPTABLE,
1515
OPTIONAL,
1616
COLLECTION,
17+
LIST_ELEMENT,
1718
NOT_NULL
1819
}
1920
private var nullableMode: NullableMode = NullableMode.NOT_NULL
@@ -37,7 +38,8 @@ internal abstract class ProtobufTaggedEncoder : ProtobufTaggedBase(), Encoder, C
3738
if (nullableMode != NullableMode.ACCEPTABLE) {
3839
val message = when (nullableMode) {
3940
NullableMode.OPTIONAL -> "'null' is not supported for optional properties in ProtoBuf"
40-
NullableMode.COLLECTION -> "'null' is not supported for collection types in ProtoBuf"
41+
NullableMode.COLLECTION -> "'null' is not supported as the value of collection types in ProtoBuf"
42+
NullableMode.LIST_ELEMENT -> "'null' is not supported as the value of a list element in ProtoBuf"
4143
NullableMode.NOT_NULL -> "'null' is not allowed for not-null properties"
4244
else -> "'null' is not supported in ProtoBuf";
4345
}
@@ -137,12 +139,12 @@ internal abstract class ProtobufTaggedEncoder : ProtobufTaggedBase(), Encoder, C
137139
NullableMode.OPTIONAL
138140
else {
139141
val elementDescriptor = descriptor.getElementDescriptor(index)
140-
if (elementDescriptor.kind.isMapOrList())
141-
NullableMode.COLLECTION
142-
else if (!descriptor.kind.isMapOrList() && elementDescriptor.isNullable) // or: `serializer.descriptor`
143-
NullableMode.ACCEPTABLE
144-
else
145-
NullableMode.NOT_NULL
142+
when {
143+
!elementDescriptor.isNullable -> NullableMode.NOT_NULL
144+
elementDescriptor.kind.isMapOrList() -> NullableMode.COLLECTION
145+
descriptor.kind == StructureKind.LIST -> NullableMode.LIST_ELEMENT
146+
else -> NullableMode.ACCEPTABLE
147+
}
146148
}
147149

148150
pushTag(descriptor.getTag(index))
@@ -157,10 +159,15 @@ internal abstract class ProtobufTaggedEncoder : ProtobufTaggedBase(), Encoder, C
157159
) {
158160
nullableMode = if (descriptor.isElementOptional(index))
159161
NullableMode.OPTIONAL
160-
else if (descriptor.getElementDescriptor(index).kind.isMapOrList())
161-
NullableMode.COLLECTION
162-
else
163-
NullableMode.ACCEPTABLE
162+
else {
163+
// we don't check nullability of element because this function called always for nullable `value`
164+
val elementDescriptor = descriptor.getElementDescriptor(index)
165+
when {
166+
elementDescriptor.kind.isMapOrList() -> NullableMode.COLLECTION
167+
descriptor.kind == StructureKind.LIST -> NullableMode.LIST_ELEMENT
168+
else -> NullableMode.ACCEPTABLE
169+
}
170+
}
164171

165172
pushTag(descriptor.getTag(index))
166173
encodeNullableSerializableValue(serializer, value)

‎formats/protobuf/commonTest/src/kotlinx/serialization/protobuf/ProtobufCollectionsTest.kt

+46-15
Original file line numberDiff line numberDiff line change
@@ -16,23 +16,21 @@ class ProtobufCollectionsTest {
1616
@Serializable
1717
data class MapWithNullableNestedLists(val m: Map<List<Int>?, List<Int>?>)
1818

19+
@Serializable
20+
data class MapWithNullableNestedMaps(val m: Map<Map<String, Int>?, Map<String, Int>?>)
21+
1922
@Serializable
2023
data class NullableListElement(val l: List<Int?>)
2124

2225
@Serializable
2326
data class NullableMapKey(val m: Map<Int?, Int>)
2427

2528
@Serializable
26-
data class NullableMapValue(val m: Map<Int, Int?>)
29+
data class NullableMap(val m: Map<Int?, Int?>)
2730

2831
@Test
2932
fun testEncodeNullAsListElement() {
30-
assertFailsWith(SerializationException::class) { ProtoBuf.encodeToByteArray(NullableListElement(listOf(null))) }
31-
}
32-
33-
@Test
34-
fun testEncodeNullAsMapKey() {
35-
assertFailsWith(SerializationException::class) { ProtoBuf.encodeToByteArray(NullableMapKey(mapOf(null to 42))) }
33+
assertFailsWithMessage<SerializationException> ("'null' is not supported as the value of a list element in ProtoBuf") { ProtoBuf.encodeToByteArray(NullableListElement(listOf(null))) }
3634
}
3735

3836
@Test
@@ -54,15 +52,46 @@ class ProtobufCollectionsTest {
5452
}
5553

5654
@Test
57-
fun testEncodeNullAsMapValue() {
58-
assertFailsWith(SerializationException::class) { ProtoBuf.encodeToByteArray(NullableMapValue(mapOf(42 to null))) }
55+
fun testNullMap() {
56+
val keyNull = NullableMap(mapOf(null to 42))
57+
val valueNull = NullableMap(mapOf(42 to null))
58+
val bothNull = NullableMap(mapOf(null to null))
59+
60+
val encodedKeyNull = ProtoBuf.encodeToHexString(keyNull)
61+
val encodedValueNull = ProtoBuf.encodeToHexString(valueNull)
62+
val encodedBothNull = ProtoBuf.encodeToHexString(bothNull)
63+
assertEquals(encodedKeyNull, "0a02102a")
64+
assertEquals(encodedValueNull, "0a02082a")
65+
assertEquals(encodedBothNull, "0a00")
66+
67+
val decodedKeyNull = ProtoBuf.decodeFromHexString<NullableMap>(encodedKeyNull)
68+
val decodedValueNull = ProtoBuf.decodeFromHexString<NullableMap>(encodedValueNull)
69+
val decodedBothNull = ProtoBuf.decodeFromHexString<NullableMap>(encodedBothNull)
70+
assertEquals(decodedKeyNull, keyNull)
71+
assertEquals(decodedValueNull, valueNull)
72+
assertEquals(decodedBothNull, bothNull)
73+
}
74+
75+
@Test
76+
fun testRepeatNullKeyInMap() {
77+
// there are two entries in message: (null to 42) and (null to 43), the last one is used
78+
val decoded = ProtoBuf.decodeFromHexString<NullableMap>("0a04102a102b")
79+
assertEquals(NullableMap(mapOf(null to 43)), decoded)
80+
}
81+
82+
@Test
83+
fun testCollectionsInNullableMap() {
84+
assertFailsWithMessage<SerializationException> ("'null' is not supported as the value of collection types in ProtoBuf") { ProtoBuf.encodeToByteArray(MapWithNullableNestedLists(mapOf(null to listOf(42))) ) }
85+
assertFailsWithMessage<SerializationException> ("'null' is not supported as the value of collection types in ProtoBuf") { ProtoBuf.encodeToByteArray(MapWithNullableNestedLists(mapOf(listOf(42) to null)) ) }
86+
assertFailsWithMessage<SerializationException> ("'null' is not supported as the value of collection types in ProtoBuf") { ProtoBuf.encodeToByteArray(MapWithNullableNestedMaps(mapOf(null to mapOf("key" to 42))) ) }
87+
assertFailsWithMessage<SerializationException> ("'null' is not supported as the value of collection types in ProtoBuf") { ProtoBuf.encodeToByteArray(MapWithNullableNestedMaps(mapOf(mapOf("key" to 42) to null)) ) }
5988
}
6089

6190
@Test
6291
fun testEncodeMapWithNullableValue() {
63-
val map = NullableMapValue(mapOf(42 to 43))
92+
val map = NullableMap(mapOf(42 to 43))
6493
val bytes = ProtoBuf.encodeToByteArray(map)
65-
val decoded = ProtoBuf.decodeFromByteArray<NullableMapValue>(bytes)
94+
val decoded = ProtoBuf.decodeFromByteArray<NullableMap>(bytes)
6695
assertEquals(map, decoded)
6796
}
6897

@@ -76,7 +105,9 @@ class ProtobufCollectionsTest {
76105

77106
@Test
78107
fun testNestedListIsNull() {
79-
assertFailsWith(SerializationException::class) { ProtoBuf.encodeToByteArray(ListWithNestedList(listOf(null))) }
108+
assertFailsWithMessage<SerializationException>("'null' is not supported as the value of collection types in ProtoBuf") {
109+
ProtoBuf.encodeToByteArray(ListWithNestedList(listOf(null)))
110+
}
80111
}
81112

82113
@Test
@@ -97,8 +128,8 @@ class ProtobufCollectionsTest {
97128

98129
@Test
99130
fun testNestedListsAreNullInMap() {
100-
assertFailsWith(SerializationException::class) { ProtoBuf.encodeToByteArray(MapWithNullableNestedLists(mapOf(null to emptyList()))) }
101-
assertFailsWith(SerializationException::class) { ProtoBuf.encodeToByteArray(MapWithNullableNestedLists(mapOf(emptyList<Int>() to null))) }
102-
assertFailsWith(SerializationException::class) { ProtoBuf.encodeToByteArray(MapWithNullableNestedLists(mapOf(null to null))) }
131+
assertFailsWithMessage<SerializationException> ("'null' is not supported as the value of collection types in ProtoBuf") { ProtoBuf.encodeToByteArray(MapWithNullableNestedLists(mapOf(null to emptyList()))) }
132+
assertFailsWithMessage<SerializationException> ("'null' is not supported as the value of collection types in ProtoBuf") { ProtoBuf.encodeToByteArray(MapWithNullableNestedLists(mapOf(emptyList<Int>() to null))) }
133+
assertFailsWithMessage<SerializationException> ("'null' is not supported as the value of collection types in ProtoBuf") { ProtoBuf.encodeToByteArray(MapWithNullableNestedLists(mapOf(null to null))) }
103134
}
104135
}

‎formats/protobuf/commonTest/src/kotlinx/serialization/protobuf/ProtobufTypeParameterTest.kt

+1-1
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ class ProtobufTypeParameterTest {
7373
fail()
7474
} catch (e: SerializationException) {
7575
assertEquals(
76-
"'null' is not supported for collection types in ProtoBuf", e.message
76+
"'null' is not supported as the value of collection types in ProtoBuf", e.message
7777
)
7878
}
7979
}

0 commit comments

Comments
 (0)