diff --git a/README.md b/README.md index 88b3a841..2347bdda 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,4 @@ -# protokt +# protokt [![Github Actions](https://github.com/open-toast/protokt/actions/workflows/ci.yml/badge.svg)](https://github.com/open-toast/protokt/actions/workflows/ci.yml) [![Maven Central](https://img.shields.io/maven-central/v/com.toasttab.protokt/protokt-runtime)](https://search.maven.org/artifact/com.toasttab.protokt/protokt-runtime) @@ -23,21 +23,21 @@ is represented as `String?`, etc. CodedOutputStream for best performance - (JS) Built on protobufJS for best performance - gRPC [method descriptor and service descriptor generation](#grpc-code-generation) -for use with [grpc-java](#integrating-with-grpcs-java-api), +for use with [grpc-java](#integrating-with-grpcs-java-api), [grpc-kotlin](#integrating-with-grpcs-kotlin-api), and [grpc-node](#integrating-with-grpcs-nodejs-api) (experimental) (see examples in [examples](examples)) ### Not yet implemented - Kotlin/Native support -- Protobuf JSON support +- Protobuf JSON support ### Compatibility The Gradle plugin requires Java 8+ and Gradle 5.6+. It runs on recent versions of MacOS, Linux, and Windows. -The runtime and generated code are compatible with Kotlin 1.8+, Java 8+, and Android 4.4+. +The runtime and generated code are compatible with Kotlin 1.8+, Java 8+, and Android 4.4+. ## Usage @@ -354,7 +354,7 @@ while using the lite runtime: protokt { generate { descriptors = false - } + } } ``` @@ -639,8 +639,8 @@ Wrapper types that wrap protobuf messages are nullable. For example, can be made non-nullable by using the non-null option described below. Wrapper types that wrap protobuf primitives, for example `java.util.UUID` -which wraps `bytes`, are nullable when they cannot wrap their wrapped type's -default value. Converters must override `acceptsDefaultValue` to be `false` in +which wraps `bytes`, are nullable when they cannot wrap their wrapped type's +default value. Converters must override `acceptsDefaultValue` to be `false` in these cases. For example, a UUID cannot wrap an empty byte array and each of the following declarations will produce a nullable property: @@ -774,15 +774,17 @@ message ImplementsWithDelegate { ``` Note that the `by` clause references the field by its lower camel case name. +Properties on delegate interfaces must be nullable since fields themselves +may not be present on the wire. #### Oneof Fields -Oneof fields can declare that they implement an interface with the +Oneof fields can declare that they implement an interface with the `(protokt.v1.oneof).implements` option. Each possible field type of the oneof must also implement the interface. This allows access of common properties without a `when` statement that always ultimately extracts the same property. -Suppose you have a domain object MyObjectWithConfig that has a non-null configuration +Suppose you have a domain object MyObjectWithConfig that has a configuration that specifies a third-party server for communication. For flexibility, this configuration will be modifiable by the server and versioned by a simple integer. To hasten subsequent loading of the configuration, a client may save a resolved @@ -816,7 +818,6 @@ message MyObjectWithConfig { ]; oneof Config { - option (protokt.v1.oneof).non_null = true; option (protokt.v1.oneof).implements = "com.toasttab.example.Config"; ServerSpecified server_specified = 2; @@ -836,9 +837,7 @@ message ServerSpecified { message ClientResolved { option (protokt.v1.class).implements = "com.toasttab.example.Config by config"; - ServerSpecified config = 1 [ - (protokt.v1.property).non_null = true - ]; + ServerSpecified config = 1; bytes last_known_address = 2 [ (protokt.v1.property).wrap = "java.net.InetAddress" @@ -853,7 +852,7 @@ Protokt will generate: public class MyObjectWithConfig private constructor( @GeneratedProperty(1) public val id: UUID?, - public val config: Config, + public val config: Config?, public val unknownFields: UnknownFieldSet = UnknownFieldSet.empty() ) : AbstractMessage() { @@ -906,18 +905,10 @@ accessing the property via a `when` expression: ```kotlin fun printVersion(config: MyObjectWithConfig.Config) { - println(config.version) + println(config?.version) } ``` -This pattern is dangerous: since the oneof must be marked non-nullable, you -cannot compatibly add new implementing fields to a producer before a consumer -is updated with the new generated code. The old consumer will attempt to -deserialize the new field as an unknown field and the non-null assertion on the -oneof field during the constructor call will fail. - -This functionality will likely be removed. - ### BytesSlice When reading messages that contain other serialized messages as `bytes` fields, @@ -946,7 +937,7 @@ protokt { generate { grpcDescriptors = true grpcKotlinStubs = true - } + } } ``` @@ -1061,7 +1052,7 @@ Protokt generates complete server and client stub implementations for use with N The generated implementations are nearly the same as those generated by grpc-kotlin and are supported by an analogous runtime library in ServerCalls and ClientCalls objects. -These implementations are alpha-quality and for demonstration only. External contributions +These implementations are alpha-quality and for demonstration only. External contributions to harden the implementation are welcome. They use the same `grpcDescriptors` and `grpcKotlinStubs` plugin options to control code generation. @@ -1104,7 +1095,7 @@ protokt % protoc \ ## Contribution -Community contributions are welcome. See the +Community contributions are welcome. See the [contribution guidelines](CONTRIBUTING.md) and the project [code of conduct](CODE-OF-CONDUCT.md). diff --git a/extensions/protokt-extensions-lite/api/protokt-extensions-lite.api b/extensions/protokt-extensions-lite/api/protokt-extensions-lite.api index e13ac3b1..5009ea61 100644 --- a/extensions/protokt-extensions-lite/api/protokt-extensions-lite.api +++ b/extensions/protokt-extensions-lite/api/protokt-extensions-lite.api @@ -593,13 +593,12 @@ public final class protokt/v1/MethodOptions$Deserializer : protokt/v1/AbstractDe public final class protokt/v1/OneofOptions : protokt/v1/AbstractMessage { public static final field Deserializer Lprotokt/v1/OneofOptions$Deserializer; - public synthetic fun (ZLjava/lang/String;Ljava/lang/String;Lprotokt/v1/UnknownFieldSet;Lkotlin/jvm/internal/DefaultConstructorMarker;)V + public synthetic fun (Ljava/lang/String;Ljava/lang/String;Lprotokt/v1/UnknownFieldSet;Lkotlin/jvm/internal/DefaultConstructorMarker;)V public final fun copy (Lkotlin/jvm/functions/Function1;)Lprotokt/v1/OneofOptions; public static fun deserialize (Lprotokt/v1/Reader;)Lprotokt/v1/OneofOptions; public fun equals (Ljava/lang/Object;)Z public final fun getDeprecationMessage ()Ljava/lang/String; public final fun getImplements ()Ljava/lang/String; - public final fun getNonNull ()Z public final fun getUnknownFields ()Lprotokt/v1/UnknownFieldSet; public fun hashCode ()I public static final fun invoke (Lkotlin/jvm/functions/Function1;)Lprotokt/v1/OneofOptions; @@ -613,11 +612,9 @@ public final class protokt/v1/OneofOptions$Builder { public final fun build ()Lprotokt/v1/OneofOptions; public final fun getDeprecationMessage ()Ljava/lang/String; public final fun getImplements ()Ljava/lang/String; - public final fun getNonNull ()Z public final fun getUnknownFields ()Lprotokt/v1/UnknownFieldSet; public final fun setDeprecationMessage (Ljava/lang/String;)V public final fun setImplements (Ljava/lang/String;)V - public final fun setNonNull (Z)V public final fun setUnknownFields (Lprotokt/v1/UnknownFieldSet;)V } diff --git a/extensions/protokt-extensions-lite/src/extensions-proto/protokt/v1/protokt.proto b/extensions/protokt-extensions-lite/src/extensions-proto/protokt/v1/protokt.proto index 42169385..e3fde759 100644 --- a/extensions/protokt-extensions-lite/src/extensions-proto/protokt/v1/protokt.proto +++ b/extensions/protokt-extensions-lite/src/extensions-proto/protokt/v1/protokt.proto @@ -125,22 +125,7 @@ extend google.protobuf.FieldOptions { } message OneofOptions { - // Makes a oneof field non-nullable in generated Kotlin code. Beware that - // deserialization will NPE if the field is missing from the protobuf payload. - // Adding a non-null field to an existing message is a backwards-incompatible - // change. - // - // For example: - // - // message Message { - // oneof some_field_name { - // option (protokt.v1.oneof).non_null = true; - // - // string id = 1; - // } - // } - // - bool non_null = 1; + reserved 1; // Make the sealed class implement an interface, enforcing the presence of a // property in each possible variant. Scoping rules are the same as those diff --git a/protokt-codegen/src/main/kotlin/protokt/v1/codegen/generate/Implements.kt b/protokt-codegen/src/main/kotlin/protokt/v1/codegen/generate/Implements.kt index 3ba0b3ef..4d16e9c1 100644 --- a/protokt-codegen/src/main/kotlin/protokt/v1/codegen/generate/Implements.kt +++ b/protokt-codegen/src/main/kotlin/protokt/v1/codegen/generate/Implements.kt @@ -16,10 +16,16 @@ package protokt.v1.codegen.generate import com.squareup.kotlinpoet.ClassName +import com.squareup.kotlinpoet.FunSpec +import com.squareup.kotlinpoet.KModifier +import com.squareup.kotlinpoet.PropertySpec import com.squareup.kotlinpoet.TypeSpec +import com.squareup.kotlinpoet.asTypeName import protokt.v1.codegen.generate.CodeGenerator.Context +import protokt.v1.codegen.generate.Nullability.nullable import protokt.v1.codegen.util.Message import protokt.v1.codegen.util.StandardField +import kotlin.reflect.KClass internal object Implements { fun StandardField.overrides( @@ -27,7 +33,7 @@ internal object Implements { msg: Message ) = msg.superInterface(ctx) - ?.let { fieldName in ctx.info.context.classLookup.properties(it.canonicalName) } + ?.let { fieldName in ctx.info.context.classLookup.properties(it.canonicalName).map { p -> p.name } } ?: false fun TypeSpec.Builder.handleSuperInterface(implements: ClassName?, v: OneofGeneratorInfo? = null) = @@ -43,27 +49,78 @@ internal object Implements { fun TypeSpec.Builder.handleSuperInterface(msg: Message, ctx: Context) = apply { - if (msg.options.protokt.implements.isNotEmpty()) { - if (msg.options.protokt.implements.delegates()) { - addSuperinterface( - ClassName.bestGuess(msg.options.protokt.implements.substringBefore(" by ")), - msg.options.protokt.implements.substringAfter(" by ") - ) - } else { - addSuperinterface(msg.superInterface(ctx)!!) + val superInterface = msg.superInterface(ctx) + if (superInterface != null) { + addSuperinterface(superInterface.`interface`) + if (superInterface.delegate != null) { + // don't delegate because message types may be nullable + delegateProperties(msg, ctx, superInterface.canonicalName, superInterface.delegate) } } } - private fun String.delegates() = - contains(" by ") + private fun TypeSpec.Builder.delegateProperties(msg: Message, ctx: Context, canonicalName: String, fieldName: String) { + val fieldsByName = msg.fields.filterIsInstance().associateBy { it.fieldName } - private fun Message.superInterface(ctx: Context) = - options.protokt.implements.let { - if (it.isNotEmpty() && !it.delegates()) { - inferClassName(it, ctx) - } else { - null - } + val interfaceFields = + ctx.info.context.classLookup + .properties(canonicalName) + .associateBy { it.name } + + val implementFields = interfaceFields.values.filter { it.name !in fieldsByName.keys } + + implementFields.forEach { field -> + val standardFieldNullable = fieldsByName.getValue(fieldName).nullable + addProperty( + PropertySpec.builder( + field.name, + (field.returnType.classifier as KClass<*>).asTypeName() + .let { + if (standardFieldNullable) { + it.copy(nullable = true) + } else { + it + } + } + ) + .addModifiers(KModifier.OVERRIDE) + .getter( + FunSpec.getterBuilder() + .addCode( + "return %L" + + if (standardFieldNullable) { + "?" + } else { + "" + } + + ".%L", + fieldName, + field.name + ) + .build() + ) + .build() + ) + } + } + + private class SuperInterface( + val `interface`: ClassName, + val delegate: String? + ) { + val canonicalName = `interface`.canonicalName + } + + private fun Message.superInterface(ctx: Context): SuperInterface? { + val implements = options.protokt.implements + return when { + implements.isEmpty() -> null + implements.contains(" by ") -> + SuperInterface( + inferClassName(implements.substringBefore(" by "), ctx), + implements.substringAfter(" by ") + ) + else -> SuperInterface(inferClassName(implements, ctx), null) } + } } diff --git a/protokt-codegen/src/main/kotlin/protokt/v1/codegen/generate/Nullability.kt b/protokt-codegen/src/main/kotlin/protokt/v1/codegen/generate/Nullability.kt index 48110e48..b1739272 100644 --- a/protokt-codegen/src/main/kotlin/protokt/v1/codegen/generate/Nullability.kt +++ b/protokt-codegen/src/main/kotlin/protokt/v1/codegen/generate/Nullability.kt @@ -24,15 +24,10 @@ import protokt.v1.reflect.FieldType internal object Nullability { val Field.hasNonNullOption - get() = - when (this) { - is StandardField -> options.protokt.nonNull - is Oneof -> options.protokt.nonNull - } + get() = this is StandardField && options.protokt.nonNull val Field.nullable - get() = - isKotlinRepresentationNullable && !hasNonNullOption + get() = isKotlinRepresentationNullable && !hasNonNullOption private val Field.isKotlinRepresentationNullable get() = diff --git a/protokt-codegen/src/test/kotlin/protokt/v1/codegen/ImplementDelegationTest.kt b/protokt-codegen/src/test/kotlin/protokt/v1/codegen/ImplementDelegationTest.kt new file mode 100644 index 00000000..17c815de --- /dev/null +++ b/protokt-codegen/src/test/kotlin/protokt/v1/codegen/ImplementDelegationTest.kt @@ -0,0 +1,30 @@ +/* + * Copyright (c) 2023 Toast, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package protokt.v1.codegen + +import org.junit.jupiter.api.Test + +class ImplementDelegationTest : AbstractProtoktCodegenTest() { + @Test + fun `delegate to interface with non-null property`() { + runPlugin("implement_by_delegate_with_non_null_property.proto").orFail() + } +} + +@Suppress("UNUSED") +interface Model { + val id: String +} diff --git a/protokt-codegen/src/test/resources/implement_by_delegate_with_non_null_property.proto b/protokt-codegen/src/test/resources/implement_by_delegate_with_non_null_property.proto new file mode 100644 index 00000000..df524290 --- /dev/null +++ b/protokt-codegen/src/test/resources/implement_by_delegate_with_non_null_property.proto @@ -0,0 +1,32 @@ +/* + * Copyright (c) 2023 Toast, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +syntax = "proto3"; + +package toasttab.protokt.v1.codegen.testing; + +import "protokt/v1/protokt.proto"; + +message ImplementsModel { + option (.protokt.v1.class).implements = "protokt.v1.codegen.Model"; + + string id = 1; +} + +message ImplementsWithDelegate { + option (.protokt.v1.class).implements = "protokt.v1.codegen.Model by model"; + + ImplementsModel model = 1; +} diff --git a/protokt-reflect/api/protokt-reflect.api b/protokt-reflect/api/protokt-reflect.api index d951bdb0..74a4b61a 100644 --- a/protokt-reflect/api/protokt-reflect.api +++ b/protokt-reflect/api/protokt-reflect.api @@ -733,7 +733,6 @@ public abstract interface class com/toasttab/protokt/v1/ProtoktProtos$MethodOpti public final class com/toasttab/protokt/v1/ProtoktProtos$OneofOptions : com/google/protobuf/GeneratedMessageV3, com/toasttab/protokt/v1/ProtoktProtos$OneofOptionsOrBuilder { public static final field DEPRECATION_MESSAGE_FIELD_NUMBER I public static final field IMPLEMENTS_FIELD_NUMBER I - public static final field NON_NULL_FIELD_NUMBER I public fun equals (Ljava/lang/Object;)Z public static fun getDefaultInstance ()Lcom/toasttab/protokt/v1/ProtoktProtos$OneofOptions; public synthetic fun getDefaultInstanceForType ()Lcom/google/protobuf/Message; @@ -744,7 +743,6 @@ public final class com/toasttab/protokt/v1/ProtoktProtos$OneofOptions : com/goog public static final fun getDescriptor ()Lcom/google/protobuf/Descriptors$Descriptor; public fun getImplements ()Ljava/lang/String; public fun getImplementsBytes ()Lcom/google/protobuf/ByteString; - public fun getNonNull ()Z public fun getParserForType ()Lcom/google/protobuf/Parser; public fun getSerializedSize ()I public final fun getUnknownFields ()Lcom/google/protobuf/UnknownFieldSet; @@ -794,7 +792,6 @@ public final class com/toasttab/protokt/v1/ProtoktProtos$OneofOptions$Builder : public synthetic fun clearField (Lcom/google/protobuf/Descriptors$FieldDescriptor;)Lcom/google/protobuf/Message$Builder; public fun clearField (Lcom/google/protobuf/Descriptors$FieldDescriptor;)Lcom/toasttab/protokt/v1/ProtoktProtos$OneofOptions$Builder; public fun clearImplements ()Lcom/toasttab/protokt/v1/ProtoktProtos$OneofOptions$Builder; - public fun clearNonNull ()Lcom/toasttab/protokt/v1/ProtoktProtos$OneofOptions$Builder; public synthetic fun clearOneof (Lcom/google/protobuf/Descriptors$OneofDescriptor;)Lcom/google/protobuf/AbstractMessage$Builder; public synthetic fun clearOneof (Lcom/google/protobuf/Descriptors$OneofDescriptor;)Lcom/google/protobuf/GeneratedMessageV3$Builder; public synthetic fun clearOneof (Lcom/google/protobuf/Descriptors$OneofDescriptor;)Lcom/google/protobuf/Message$Builder; @@ -815,7 +812,6 @@ public final class com/toasttab/protokt/v1/ProtoktProtos$OneofOptions$Builder : public fun getDescriptorForType ()Lcom/google/protobuf/Descriptors$Descriptor; public fun getImplements ()Ljava/lang/String; public fun getImplementsBytes ()Lcom/google/protobuf/ByteString; - public fun getNonNull ()Z public final fun isInitialized ()Z public synthetic fun mergeFrom (Lcom/google/protobuf/CodedInputStream;Lcom/google/protobuf/ExtensionRegistryLite;)Lcom/google/protobuf/AbstractMessage$Builder; public synthetic fun mergeFrom (Lcom/google/protobuf/CodedInputStream;Lcom/google/protobuf/ExtensionRegistryLite;)Lcom/google/protobuf/AbstractMessageLite$Builder; @@ -837,7 +833,6 @@ public final class com/toasttab/protokt/v1/ProtoktProtos$OneofOptions$Builder : public fun setField (Lcom/google/protobuf/Descriptors$FieldDescriptor;Ljava/lang/Object;)Lcom/toasttab/protokt/v1/ProtoktProtos$OneofOptions$Builder; public fun setImplements (Ljava/lang/String;)Lcom/toasttab/protokt/v1/ProtoktProtos$OneofOptions$Builder; public fun setImplementsBytes (Lcom/google/protobuf/ByteString;)Lcom/toasttab/protokt/v1/ProtoktProtos$OneofOptions$Builder; - public fun setNonNull (Z)Lcom/toasttab/protokt/v1/ProtoktProtos$OneofOptions$Builder; public synthetic fun setRepeatedField (Lcom/google/protobuf/Descriptors$FieldDescriptor;ILjava/lang/Object;)Lcom/google/protobuf/GeneratedMessageV3$Builder; public synthetic fun setRepeatedField (Lcom/google/protobuf/Descriptors$FieldDescriptor;ILjava/lang/Object;)Lcom/google/protobuf/Message$Builder; public fun setRepeatedField (Lcom/google/protobuf/Descriptors$FieldDescriptor;ILjava/lang/Object;)Lcom/toasttab/protokt/v1/ProtoktProtos$OneofOptions$Builder; @@ -851,7 +846,6 @@ public abstract interface class com/toasttab/protokt/v1/ProtoktProtos$OneofOptio public abstract fun getDeprecationMessageBytes ()Lcom/google/protobuf/ByteString; public abstract fun getImplements ()Ljava/lang/String; public abstract fun getImplementsBytes ()Lcom/google/protobuf/ByteString; - public abstract fun getNonNull ()Z } public final class com/toasttab/protokt/v1/ProtoktProtos$ServiceOptions : com/google/protobuf/GeneratedMessageV3, com/toasttab/protokt/v1/ProtoktProtos$ServiceOptionsOrBuilder { @@ -1155,13 +1149,12 @@ public final class protokt/v1/MethodOptions$Deserializer : protokt/v1/AbstractDe public final class protokt/v1/OneofOptions : protokt/v1/AbstractMessage { public static final field Deserializer Lprotokt/v1/OneofOptions$Deserializer; - public synthetic fun (ZLjava/lang/String;Ljava/lang/String;Lprotokt/v1/UnknownFieldSet;Lkotlin/jvm/internal/DefaultConstructorMarker;)V + public synthetic fun (Ljava/lang/String;Ljava/lang/String;Lprotokt/v1/UnknownFieldSet;Lkotlin/jvm/internal/DefaultConstructorMarker;)V public final fun copy (Lkotlin/jvm/functions/Function1;)Lprotokt/v1/OneofOptions; public static fun deserialize (Lprotokt/v1/Reader;)Lprotokt/v1/OneofOptions; public fun equals (Ljava/lang/Object;)Z public final fun getDeprecationMessage ()Ljava/lang/String; public final fun getImplements ()Ljava/lang/String; - public final fun getNonNull ()Z public final fun getUnknownFields ()Lprotokt/v1/UnknownFieldSet; public fun hashCode ()I public static final fun invoke (Lkotlin/jvm/functions/Function1;)Lprotokt/v1/OneofOptions; @@ -1175,11 +1168,9 @@ public final class protokt/v1/OneofOptions$Builder { public final fun build ()Lprotokt/v1/OneofOptions; public final fun getDeprecationMessage ()Ljava/lang/String; public final fun getImplements ()Ljava/lang/String; - public final fun getNonNull ()Z public final fun getUnknownFields ()Lprotokt/v1/UnknownFieldSet; public final fun setDeprecationMessage (Ljava/lang/String;)V public final fun setImplements (Ljava/lang/String;)V - public final fun setNonNull (Z)V public final fun setUnknownFields (Lprotokt/v1/UnknownFieldSet;)V } diff --git a/shared-src/reflect/protokt/v1/reflect/ClassLookup.kt b/shared-src/reflect/protokt/v1/reflect/ClassLookup.kt index 9a6d766a..f77fb7d5 100644 --- a/shared-src/reflect/protokt/v1/reflect/ClassLookup.kt +++ b/shared-src/reflect/protokt/v1/reflect/ClassLookup.kt @@ -21,6 +21,7 @@ import protokt.v1.OptimizedSizeOfConverter import java.io.File import java.net.URLClassLoader import kotlin.reflect.KClass +import kotlin.reflect.KProperty import kotlin.reflect.full.hasAnnotation import kotlin.reflect.full.memberProperties @@ -63,11 +64,11 @@ internal class ClassLookup(classpath: List) { private val classLookup = mutableMapOf>() - fun properties(canonicalClassName: String): Collection = + fun properties(canonicalClassName: String): Collection> = try { classLookup.getOrPut(canonicalClassName) { classLoader.loadClass(canonicalClassName).kotlin - }.memberProperties.map { it.name } + }.memberProperties } catch (t: Throwable) { throw Exception("Class not found: $canonicalClassName") } diff --git a/testing/interop/src/main/proto/protokt/v1/testing/wrappers_dynamic.proto b/testing/interop/src/main/proto/protokt/v1/testing/wrappers_dynamic.proto index 278ea49f..13bb58fa 100644 --- a/testing/interop/src/main/proto/protokt/v1/testing/wrappers_dynamic.proto +++ b/testing/interop/src/main/proto/protokt/v1/testing/wrappers_dynamic.proto @@ -75,8 +75,6 @@ message Wrappers { message OneofWrappers { oneof wrapped_oneof { - option (.protokt.v1.oneof).non_null = true; - bytes uuid_oneof = 2 [ (.protokt.v1.property).wrap = "java.util.UUID" ]; diff --git a/testing/options-api/src/main/kotlin/protokt/v1/testing/IModel2.kt b/testing/options-api/src/main/kotlin/protokt/v1/testing/IModel2.kt index 256c6cae..bd10869b 100644 --- a/testing/options-api/src/main/kotlin/protokt/v1/testing/IModel2.kt +++ b/testing/options-api/src/main/kotlin/protokt/v1/testing/IModel2.kt @@ -16,5 +16,5 @@ package protokt.v1.testing interface IModel2 { - val id: String + val id: String? } diff --git a/testing/options/src/main/proto/protokt/v1/testing/message_implements.proto b/testing/options/src/main/proto/protokt/v1/testing/message_implements.proto index ff568d25..5180983b 100644 --- a/testing/options/src/main/proto/protokt/v1/testing/message_implements.proto +++ b/testing/options/src/main/proto/protokt/v1/testing/message_implements.proto @@ -42,3 +42,9 @@ message ImplementsWithDelegate { (.protokt.v1.property).non_null = true ]; } + +message ImplementsWithNullableDelegate { + option (.protokt.v1.class).implements = "IModel2 by modelTwo"; + + ImplementsModel2 model_two = 1; +} diff --git a/testing/options/src/main/proto/protokt/v1/testing/non_null_property_and_oneof.proto b/testing/options/src/main/proto/protokt/v1/testing/non_null_property_and_oneof.proto index d348af90..4b905674 100644 --- a/testing/options/src/main/proto/protokt/v1/testing/non_null_property_and_oneof.proto +++ b/testing/options/src/main/proto/protokt/v1/testing/non_null_property_and_oneof.proto @@ -24,12 +24,6 @@ message NonNullModel { google.protobuf.StringValue non_null_string_value = 1 [ (.protokt.v1.property).non_null = true ]; - - oneof non_null_oneof { - option (.protokt.v1.oneof).non_null = true; - - string message = 2; - } } message NonNullModelMirror { diff --git a/testing/options/src/main/proto/protokt/v1/testing/oneof_implements.proto b/testing/options/src/main/proto/protokt/v1/testing/oneof_implements.proto index 96b2a21a..6c7f48c5 100644 --- a/testing/options/src/main/proto/protokt/v1/testing/oneof_implements.proto +++ b/testing/options/src/main/proto/protokt/v1/testing/oneof_implements.proto @@ -39,7 +39,6 @@ message ImplementsOneof2 { message ContainsOneofThatImplements { oneof implementing_oneof { - option (.protokt.v1.oneof).non_null = true; option (.protokt.v1.oneof).implements = "OneofModel"; ImplementsOneof1 implements_one = 1; diff --git a/testing/options/src/main/proto/protokt/v1/testing/wrapper_types.proto b/testing/options/src/main/proto/protokt/v1/testing/wrapper_types.proto index 6f45d39a..08c8027f 100644 --- a/testing/options/src/main/proto/protokt/v1/testing/wrapper_types.proto +++ b/testing/options/src/main/proto/protokt/v1/testing/wrapper_types.proto @@ -105,8 +105,6 @@ message Wrappers { message OneofWrappers { oneof wrapped_oneof { - option (.protokt.v1.oneof).non_null = true; - bytes id_oneof = 1 [ (.protokt.v1.property).wrap = "protokt.v1.testing.Id" ]; diff --git a/testing/options/src/test/kotlin/protokt/v1/testing/MessageImplementsTest.kt b/testing/options/src/test/kotlin/protokt/v1/testing/MessageImplementsTest.kt index 0ed56b7e..e8137809 100644 --- a/testing/options/src/test/kotlin/protokt/v1/testing/MessageImplementsTest.kt +++ b/testing/options/src/test/kotlin/protokt/v1/testing/MessageImplementsTest.kt @@ -42,4 +42,11 @@ class MessageImplementsTest { assertThat(byDelegate.id).isEqualTo(model2.id) } + + @Test + fun `message implementing by a nullable delegate can be assigned to its interface`() { + val byDelegate: IModel2 = ImplementsWithNullableDelegate { modelTwo = model2 } + + assertThat(byDelegate.id).isEqualTo(model2.id) + } } diff --git a/testing/options/src/test/kotlin/protokt/v1/testing/NonNullableTest.kt b/testing/options/src/test/kotlin/protokt/v1/testing/NonNullableTest.kt index 29b0dc2b..60fd71d5 100644 --- a/testing/options/src/test/kotlin/protokt/v1/testing/NonNullableTest.kt +++ b/testing/options/src/test/kotlin/protokt/v1/testing/NonNullableTest.kt @@ -25,10 +25,6 @@ class NonNullableTest { assertThat( NonNullModel::class.propertyIsMarkedNullable("nonNullStringValue") ).isFalse() - - assertThat( - NonNullModel::class.propertyIsMarkedNullable("nonNullOneof") - ).isFalse() } @Test @@ -48,22 +44,4 @@ class NonNullableTest { contains("(protokt.property).non_null") } } - - @Test - fun `detailed error when attempting to deserialize null oneof`() { - val thrown = assertThrows { - NonNullModel.deserialize( - NonNullModelMirror { - nonNullStringValue = "asdf" - nonNullOneof = null - }.serialize() - ) - } - - assertThat(thrown).hasMessageThat().apply { - contains("nonNullOneof") - contains("was null") - contains("(protokt.oneof).non_null") - } - } } diff --git a/testing/options/src/test/kotlin/protokt/v1/testing/OneofImplementsTest.kt b/testing/options/src/test/kotlin/protokt/v1/testing/OneofImplementsTest.kt index 7a524533..3c86904b 100644 --- a/testing/options/src/test/kotlin/protokt/v1/testing/OneofImplementsTest.kt +++ b/testing/options/src/test/kotlin/protokt/v1/testing/OneofImplementsTest.kt @@ -28,8 +28,8 @@ class OneofImplementsTest { @Test fun `property shared between oneof types can be assigned and accessed without switching`() { - val assigned: OneofModel = obj.implementingOneof + val assigned: OneofModel? = obj.implementingOneof - assertThat(assigned.id).isEqualTo(Id("val")) + assertThat(assigned?.id).isEqualTo(Id("val")) } }