Skip to content

Commit

Permalink
Review fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
shanshin committed Apr 23, 2021
1 parent 3c6cabe commit 7fd492a
Show file tree
Hide file tree
Showing 15 changed files with 361 additions and 310 deletions.
7 changes: 4 additions & 3 deletions formats/protobuf/api/kotlinx-serialization-protobuf.api
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,9 @@ public final class kotlinx/serialization/protobuf/ProtoType$Impl : kotlinx/seria
public final synthetic fun type ()Lkotlinx/serialization/protobuf/ProtoIntegerType;
}

public final class kotlinx/serialization/protobuf/schema/GenerationKt {
public static final fun generateProto2Schema (Ljava/util/List;Ljava/lang/String;Ljava/util/Map;)Ljava/lang/String;
public static synthetic fun generateProto2Schema$default (Ljava/util/List;Ljava/lang/String;Ljava/util/Map;ILjava/lang/Object;)Ljava/lang/String;
public final class kotlinx/serialization/protobuf/schema/ProtoBufSchemaGenerator {
public static final field INSTANCE Lkotlinx/serialization/protobuf/schema/ProtoBufSchemaGenerator;
public final fun generateSchemaText (Ljava/util/List;Ljava/lang/String;Ljava/util/Map;)Ljava/lang/String;
public static synthetic fun generateSchemaText$default (Lkotlinx/serialization/protobuf/schema/ProtoBufSchemaGenerator;Ljava/util/List;Ljava/lang/String;Ljava/util/Map;ILjava/lang/Object;)Ljava/lang/String;
}

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -42,63 +42,63 @@ class SchemaValidationsTest {
@Test
fun testInvalidEnumElementSerialName() {
val descriptors = listOf(InvalidEnumElementName.serializer().descriptor)
assertFailsWith(IllegalArgumentException::class) { generateProto2Schema(descriptors) }
assertFailsWith(IllegalArgumentException::class) { ProtoBufSchemaGenerator.generateSchemaText(descriptors) }
}

@Test
fun testInvalidClassSerialName() {
val descriptors = listOf(InvalidClassName.serializer().descriptor)
assertFailsWith(IllegalArgumentException::class) { generateProto2Schema(descriptors) }
assertFailsWith(IllegalArgumentException::class) { ProtoBufSchemaGenerator.generateSchemaText(descriptors) }
}

@Test
fun testInvalidClassFieldSerialName() {
val descriptors = listOf(InvalidClassFieldName.serializer().descriptor)
assertFailsWith(IllegalArgumentException::class) { generateProto2Schema(descriptors) }
assertFailsWith(IllegalArgumentException::class) { ProtoBufSchemaGenerator.generateSchemaText(descriptors) }
}

@Test
fun testDuplicateSerialNames() {
val descriptors = listOf(InvalidClassFieldName.serializer().descriptor)
assertFailsWith(IllegalArgumentException::class) { generateProto2Schema(descriptors) }
assertFailsWith(IllegalArgumentException::class) { ProtoBufSchemaGenerator.generateSchemaText(descriptors) }
}

@Test
fun testInvalidEnumSerialName() {
val descriptors = listOf(InvalidEnumName.serializer().descriptor)
assertFailsWith(IllegalArgumentException::class) { generateProto2Schema(descriptors) }
assertFailsWith(IllegalArgumentException::class) { ProtoBufSchemaGenerator.generateSchemaText(descriptors) }
}

@Test
fun testDuplicationSerialName() {
val descriptors = listOf(ValidClass.serializer().descriptor, DuplicateClass.serializer().descriptor)
assertFailsWith(IllegalArgumentException::class) { generateProto2Schema(descriptors) }
assertFailsWith(IllegalArgumentException::class) { ProtoBufSchemaGenerator.generateSchemaText(descriptors) }
}

@Test
fun testIllegalPackageNames() {
val descriptors = listOf(ValidClass.serializer().descriptor)
assertFailsWith(IllegalArgumentException::class) { generateProto2Schema(descriptors, "") }
assertFailsWith(IllegalArgumentException::class) { generateProto2Schema(descriptors, ".") }
assertFailsWith(IllegalArgumentException::class) { generateProto2Schema(descriptors, ".first.dot") }
assertFailsWith(IllegalArgumentException::class) { generateProto2Schema(descriptors, "ended.with.dot.") }
assertFailsWith(IllegalArgumentException::class) { generateProto2Schema(descriptors, "first._underscore") }
assertFailsWith(IllegalArgumentException::class) { generateProto2Schema(descriptors, "first.1digit") }
assertFailsWith(IllegalArgumentException::class) { generateProto2Schema(descriptors, "illegal.sym+bol") }
assertFailsWith(IllegalArgumentException::class) { ProtoBufSchemaGenerator.generateSchemaText(descriptors, "") }
assertFailsWith(IllegalArgumentException::class) { ProtoBufSchemaGenerator.generateSchemaText(descriptors, ".") }
assertFailsWith(IllegalArgumentException::class) { ProtoBufSchemaGenerator.generateSchemaText(descriptors, ".first.dot") }
assertFailsWith(IllegalArgumentException::class) { ProtoBufSchemaGenerator.generateSchemaText(descriptors, "ended.with.dot.") }
assertFailsWith(IllegalArgumentException::class) { ProtoBufSchemaGenerator.generateSchemaText(descriptors, "first._underscore") }
assertFailsWith(IllegalArgumentException::class) { ProtoBufSchemaGenerator.generateSchemaText(descriptors, "first.1digit") }
assertFailsWith(IllegalArgumentException::class) { ProtoBufSchemaGenerator.generateSchemaText(descriptors, "illegal.sym+bol") }
}

@Test
fun testValidPackageNames() {
val descriptors = listOf(ValidClass.serializer().descriptor)
generateProto2Schema(descriptors, "singleIdent")
generateProto2Schema(descriptors, "double.ident")
generateProto2Schema(descriptors, "with.digits0123")
generateProto2Schema(descriptors, "with.underscore_")
ProtoBufSchemaGenerator.generateSchemaText(descriptors, "singleIdent")
ProtoBufSchemaGenerator.generateSchemaText(descriptors, "double.ident")
ProtoBufSchemaGenerator.generateSchemaText(descriptors, "with.digits0123")
ProtoBufSchemaGenerator.generateSchemaText(descriptors, "with.underscore_")
}

@Test
fun testFieldNumberDuplicates() {
assertFailsWith(IllegalArgumentException::class) { generateProto2Schema(listOf(FieldNumberDuplicates.serializer().descriptor)) }
assertFailsWith(IllegalArgumentException::class) { generateProto2Schema(listOf(FieldNumberImplicitlyDuplicates.serializer().descriptor)) }
assertFailsWith(IllegalArgumentException::class) { ProtoBufSchemaGenerator.generateSchemaText(listOf(FieldNumberDuplicates.serializer().descriptor)) }
assertFailsWith(IllegalArgumentException::class) { ProtoBufSchemaGenerator.generateSchemaText(listOf(FieldNumberImplicitlyDuplicates.serializer().descriptor)) }
}
}
2 changes: 1 addition & 1 deletion formats/protobuf/jvmTest/resources/AbstractHolder.proto
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ message AbstractHolder {
required KotlinxSerializationPolymorphic abs = 1;
}

// serial name 'KotlinxSerializationPolymorphic'
// This message was generated to support polymorphic types and does not present in Kotlin.
message KotlinxSerializationPolymorphic {
required string type = 1;
required bytes value = 2;
Expand Down
28 changes: 20 additions & 8 deletions formats/protobuf/jvmTest/resources/LegacyMapHolder.proto
Original file line number Diff line number Diff line change
Expand Up @@ -9,51 +9,63 @@ message LegacyMapHolder {
repeated LegacyMapHolder_keyAsBytes keyAsBytes = 3;
repeated LegacyMapHolder_keyAsList keyAsList = 4;
repeated LegacyMapHolder_keyAsDeepList keyAsDeepList = 5;
repeated LegacyMapHolder_nullableKeyAndValue nullableKeyAndValue = 6;
}

// serial name 'LegacyMapHolder_keyAsMessage'
// This message was generated to support legacy map and does not present in Kotlin.
// Containing message 'LegacyMapHolder', field 'keyAsMessage'
message LegacyMapHolder_keyAsMessage {
required OptionsClass key = 1;
required int32 value = 2;
}

// serial name 'LegacyMapHolder_keyAsEnum'
// This message was generated to support legacy map and does not present in Kotlin.
// Containing message 'LegacyMapHolder', field 'keyAsEnum'
message LegacyMapHolder_keyAsEnum {
required OverriddenEnumName key = 1;
required OptionsClass value = 2;
}

// serial name 'LegacyMapHolder_keyAsBytes'
// This message was generated to support legacy map and does not present in Kotlin.
// Containing message 'LegacyMapHolder', field 'keyAsBytes'
message LegacyMapHolder_keyAsBytes {
required bytes key = 1;
required bytes value = 2;
}

// serial name 'LegacyMapHolder_keyAsList'
// This message was generated to support legacy map and does not present in Kotlin.
// Containing message 'LegacyMapHolder', field 'keyAsList'
message LegacyMapHolder_keyAsList {
repeated int32 key = 1;
required bytes value = 2;
}

// serial name 'LegacyMapHolder_keyAsDeepList'
// This message was generated to support legacy map and does not present in Kotlin.
// Containing message 'LegacyMapHolder', field 'keyAsDeepList'
message LegacyMapHolder_keyAsDeepList {
repeated LegacyMapHolder_keyAsDeepList_key key = 1;
required bytes value = 2;
}

// This message was generated to support legacy map and does not present in Kotlin.
// Containing message 'LegacyMapHolder', field 'nullableKeyAndValue'
message LegacyMapHolder_nullableKeyAndValue {
required OptionsClass key = 1;
required OptionsClass value = 2;
}

// serial name 'kotlinx.serialization.protobuf.schema.GenerationTest.OptionsClass'
message OptionsClass {
required int32 i = 1;
}

// serial name 'OverriddenEnumName'
enum OverriddenEnumName {
FIRST = 0;
OverriddenElementName = 1;
}


// serial name 'LegacyMapHolder_keyAsDeepList_key'
// This message was generated to support nested collection in list and does not present in Kotlin.
// Containing message 'LegacyMapHolder', field 'keyAsDeepList'
message LegacyMapHolder_keyAsDeepList_key {
repeated int32 value = 1;
}
3 changes: 1 addition & 2 deletions formats/protobuf/jvmTest/resources/ListClass.proto
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ package kotlinx.serialization.protobuf.schema.generator;
message ListClass {
repeated int32 intList = 1;
repeated int32 intArray = 2;
// WARNING: null value is not supported for list elements
// WARNING: nullable elements of collections can not be represented in protobuf
repeated int32 boxedIntArray = 3;
repeated OptionsClass messageList = 4;
repeated OverriddenEnumName enumList = 5;
Expand All @@ -17,7 +17,6 @@ message OptionsClass {
required int32 i = 1;
}

// serial name 'OverriddenEnumName'
enum OverriddenEnumName {
FIRST = 0;
OverriddenElementName = 1;
Expand Down
1 change: 0 additions & 1 deletion formats/protobuf/jvmTest/resources/MapClass.proto
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ message OptionsClass {
required int32 i = 1;
}

// serial name 'OverriddenEnumName'
enum OverriddenEnumName {
FIRST = 0;
OverriddenElementName = 1;
Expand Down
13 changes: 8 additions & 5 deletions formats/protobuf/jvmTest/resources/NestedCollections.proto
Original file line number Diff line number Diff line change
Expand Up @@ -10,22 +10,26 @@ message NestedCollections {
map<string, NestedCollections_listInMap> listInMap = 4;
}

// serial name 'NestedCollections_intList'
// This message was generated to support nested collection in list and does not present in Kotlin.
// Containing message 'NestedCollections', field 'intList'
message NestedCollections_intList {
repeated int32 value = 1;
}

// serial name 'NestedCollections_messageList'
// This message was generated to support nested collection in list and does not present in Kotlin.
// Containing message 'NestedCollections', field 'messageList'
message NestedCollections_messageList {
repeated OptionsClass value = 1;
}

// serial name 'NestedCollections_mapInList'
// This message was generated to support nested collection in list and does not present in Kotlin.
// Containing message 'NestedCollections', field 'mapInList'
message NestedCollections_mapInList {
map<string, OptionsClass> value = 1;
}

// serial name 'NestedCollections_listInMap'
// This message was generated to support nested collection in map value and does not present in Kotlin.
// Containing message 'NestedCollections', field 'listInMap'
message NestedCollections_listInMap {
repeated int32 value = 1;
}
Expand All @@ -34,4 +38,3 @@ message NestedCollections_listInMap {
message OptionsClass {
required int32 i = 1;
}

26 changes: 13 additions & 13 deletions formats/protobuf/jvmTest/resources/NullableNestedCollections.proto
Original file line number Diff line number Diff line change
Expand Up @@ -4,43 +4,43 @@ package kotlinx.serialization.protobuf.schema.generator;

// serial name 'kotlinx.serialization.protobuf.schema.GenerationTest.NullableNestedCollections'
message NullableNestedCollections {
// WARNING: null value is not supported for nested collections
repeated NullableNestedCollections_nullableIntList nullableIntList = 1;
// WARNING: null value is not supported for nested collections
// WARNING: nullable map values can not be represented in protobuf
map<string, NullableNestedCollections_nullableIntMap> nullableIntMap = 2;
map<string, NullableNestedCollections_intMap> intMap = 3;
repeated NullableNestedCollections_intList intList = 4;
repeated NullableNestedCollections_legacyMap legacyMap = 5;
}

// serial name 'NullableNestedCollections_nullableIntList'
// This message was generated to support nested collection in list and does not present in Kotlin.
// Containing message 'NullableNestedCollections', field 'nullableIntList'
message NullableNestedCollections_nullableIntList {
// WARNING: This field is marked as nullable but it does not support null values
repeated int32 value = 1;
}

// serial name 'NullableNestedCollections_nullableIntMap'
// This message was generated to support nested collection in map value and does not present in Kotlin.
// Containing message 'NullableNestedCollections', field 'nullableIntMap'
message NullableNestedCollections_nullableIntMap {
// WARNING: This field is marked as nullable but it does not support null values
repeated int32 value = 1;
}

// serial name 'NullableNestedCollections_intMap'
// This message was generated to support nested collection in map value and does not present in Kotlin.
// Containing message 'NullableNestedCollections', field 'intMap'
message NullableNestedCollections_intMap {
// WARNING: null value is not supported for list elements
// WARNING: nullable elements of collections can not be represented in protobuf
repeated int32 value = 1;
}

// serial name 'NullableNestedCollections_intList'
// This message was generated to support nested collection in list and does not present in Kotlin.
// Containing message 'NullableNestedCollections', field 'intList'
message NullableNestedCollections_intList {
// WARNING: null value is not supported for list elements
// WARNING: nullable elements of collections can not be represented in protobuf
repeated int32 value = 1;
}

// serial name 'NullableNestedCollections_legacyMap'
// This message was generated to support legacy map and does not present in Kotlin.
// Containing message 'NullableNestedCollections', field 'legacyMap'
message NullableNestedCollections_legacyMap {
// WARNING: This field is marked as nullable but it does not support null values
repeated int32 key = 1;
// WARNING: This field is marked as nullable but it does not support null values
repeated int32 value = 2;
}
5 changes: 2 additions & 3 deletions formats/protobuf/jvmTest/resources/OptionalClass.proto
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,9 @@ package kotlinx.serialization.protobuf.schema.generator;
// serial name 'kotlinx.serialization.protobuf.schema.GenerationTest.OptionalClass'
message OptionalClass {
required int32 requiredInt = 1;
// WARNING: an absence value is decoded as a default value that is not present in the schema
// WARNING: a default value decoded when value is missing
optional int32 optionalInt = 2;
optional int32 nullableInt = 3;
// WARNING: this field is nullable and has default value, it's impossible to unambiguously interpret an absence value.
// For this field null value does not support and absence value denotes as default value. Default value is not present in the schema.
// WARNING: a default value decoded when value is missing
optional int32 nullableOptionalInt = 4;
}
16 changes: 6 additions & 10 deletions formats/protobuf/jvmTest/resources/OptionalCollections.proto
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,17 @@ package kotlinx.serialization.protobuf.schema.generator;
// serial name 'kotlinx.serialization.protobuf.schema.GenerationTest.OptionalCollections'
message OptionalCollections {
repeated int32 requiredList = 1;
// WARNING: This field does not support empty list
// An absence value is decoded as a default value that is not present in the schema
// WARNING: a default value decoded when value is missing
repeated int32 optionalList = 2;
// WARNING: This field is marked as nullable but it does not support null values
// WARNING: an empty collection decoded when a value is missing
repeated int32 nullableList = 3;
// WARNING: This field is marked as nullable and has a default value but it does not support null values and empty list
// An absence value is decoded as a default value that is not present in the schema
// WARNING: a default value decoded when value is missing
repeated int32 nullableOptionalList = 4;
map<int32, int32> requiredMap = 5;
// WARNING: This field does not support empty map
// An absence value is decoded as a default value that is not present in the schema
// WARNING: a default value decoded when value is missing
map<int32, int32> optionalMap = 6;
// WARNING: This field is marked as nullable but it does not support null values
// WARNING: an empty collection decoded when a value is missing
map<int32, int32> nullableMap = 7;
// WARNING: This field is marked as nullable and has a default value but it does not support null values and empty map
// An absence value is decoded as a default value that is not present in the schema
// WARNING: a default value decoded when value is missing
map<int32, int32> nullableOptionalMap = 8;
}
2 changes: 0 additions & 2 deletions formats/protobuf/jvmTest/resources/SerialNameClass.proto
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,11 @@ syntax = "proto2";

package kotlinx.serialization.protobuf.schema.generator;

// serial name 'OverriddenClassName'
message OverriddenClassName {
required int32 original = 1;
required OverriddenEnumName OverriddenFieldName = 2;
}

// serial name 'OverriddenEnumName'
enum OverriddenEnumName {
FIRST = 0;
OverriddenElementName = 1;
Expand Down
Loading

0 comments on commit 7fd492a

Please sign in to comment.