From 663a107ae6ce3b354f53c3a73119698f0093373d Mon Sep 17 00:00:00 2001 From: Alan Chiu Date: Mon, 27 Feb 2023 09:41:43 -0800 Subject: [PATCH 01/21] Add callback signature to opt --- protoc-gen-connect-kotlin/buf.gen.yaml | 1 + .../build/buf/protocgen/connect/Generator.kt | 24 +++++++++++++++- .../protocgen/connect/internal/Parameters.kt | 28 +++++++++++++++++++ 3 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/internal/Parameters.kt diff --git a/protoc-gen-connect-kotlin/buf.gen.yaml b/protoc-gen-connect-kotlin/buf.gen.yaml index fae9725d..490bebb5 100644 --- a/protoc-gen-connect-kotlin/buf.gen.yaml +++ b/protoc-gen-connect-kotlin/buf.gen.yaml @@ -3,5 +3,6 @@ plugins: - name: connect-kotlin out: src/test/java/ path: ./protoc-gen-connect-kotlin/protoc-gen-connect-kotlin + opt: callback_signature=true - name: java out: src/test/java/ diff --git a/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt b/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt index 3c10e6fe..a234c832 100644 --- a/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt +++ b/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt @@ -21,9 +21,11 @@ import build.buf.connect.ProtocolClientInterface import build.buf.connect.ResponseMessage import build.buf.connect.ServerOnlyStreamInterface import build.buf.protocgen.connect.internal.CodeGenerator +import build.buf.protocgen.connect.internal.Configuration import build.buf.protocgen.connect.internal.Plugin import build.buf.protocgen.connect.internal.getClassName import build.buf.protocgen.connect.internal.getFileJavaPackage +import build.buf.protocgen.connect.internal.parse import com.google.protobuf.Descriptors import com.google.protobuf.compiler.PluginProtos import com.squareup.kotlinpoet.ClassName @@ -31,11 +33,14 @@ import com.squareup.kotlinpoet.CodeBlock import com.squareup.kotlinpoet.FileSpec import com.squareup.kotlinpoet.FunSpec import com.squareup.kotlinpoet.KModifier +import com.squareup.kotlinpoet.LambdaTypeName import com.squareup.kotlinpoet.ParameterSpec import com.squareup.kotlinpoet.ParameterizedTypeName.Companion.parameterizedBy import com.squareup.kotlinpoet.PropertySpec +import com.squareup.kotlinpoet.TypeName import com.squareup.kotlinpoet.TypeSpec import com.squareup.kotlinpoet.asClassName +import com.squareup.kotlinpoet.asTypeName /* * These are constants since build.buf.connect.Headers and build.buf.connect.http.Cancelable @@ -48,9 +53,11 @@ import com.squareup.kotlinpoet.asClassName * move off of type aliases, this can be changed without user API breakage. */ private val HEADERS_CLASS_NAME = ClassName("build.buf.connect", "Headers") +private val CANCELLABLE_CLASS_NAME = ClassName("build.buf.connect.http", "Cancelable") class Generator : CodeGenerator { private lateinit var descriptorSource: Plugin.DescriptorSource + private lateinit var configuration: Configuration override fun generate( request: PluginProtos.CodeGeneratorRequest, @@ -58,7 +65,7 @@ class Generator : CodeGenerator { response: Plugin.Response ) { this.descriptorSource = descriptorSource - + configuration = parse(request.parameter) for (fileName in request.fileToGenerateList) { val file = descriptorSource.findFileByName(fileName) ?: throw RuntimeException("no descriptor sources found.") if (file.services.isEmpty()) { @@ -161,6 +168,21 @@ class Generator : CodeGenerator { .returns(ResponseMessage::class.asClassName().parameterizedBy(outputClassName)) .build() functions.add(unarySuspendFunction) + if (configuration.callbackSignature) { + val callbackType = LambdaTypeName.get( + receiver = ResponseMessage::class.asTypeName().parameterizedBy(outputClassName), + returnType = Unit::class.java.asTypeName() + ) + val unaryCallbackFunction = FunSpec.builder(method.name.lowerCamelCase()) + .addModifiers(KModifier.ABSTRACT) + .addModifiers(KModifier.SUSPEND) + .addParameter("request", inputClassName) + .addParameter(headerParameterSpec) + .addParameter("onResult", callbackType) + .returns(CANCELLABLE_CLASS_NAME) + .build() + functions.add(unaryCallbackFunction) + } } } return functions diff --git a/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/internal/Parameters.kt b/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/internal/Parameters.kt new file mode 100644 index 00000000..c6fc64c8 --- /dev/null +++ b/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/internal/Parameters.kt @@ -0,0 +1,28 @@ +// Copyright 2022-2023 Buf Technologies, 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 build.buf.protocgen.connect.internal + +internal const val CALLBACK_SIGNATURE = "callback_signature" + +internal data class Configuration( + val callbackSignature: Boolean +) + +internal fun parse(parameter: String): Configuration { + val parameters = parseGeneratorParameter(parameter) + return Configuration( + callbackSignature = parameters[CALLBACK_SIGNATURE]?.toBoolean() ?: false + ) +} From 975dcc45a1d9c63b702f289cec3857d1c831586f Mon Sep 17 00:00:00 2001 From: Alan Chiu Date: Mon, 27 Feb 2023 10:56:21 -0800 Subject: [PATCH 02/21] add callback signature --- .../build/buf/protocgen/connect/Generator.kt | 44 +++++++++++++++---- 1 file changed, 36 insertions(+), 8 deletions(-) diff --git a/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt b/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt index a234c832..859e1cd7 100644 --- a/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt +++ b/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt @@ -37,7 +37,6 @@ import com.squareup.kotlinpoet.LambdaTypeName import com.squareup.kotlinpoet.ParameterSpec import com.squareup.kotlinpoet.ParameterizedTypeName.Companion.parameterizedBy import com.squareup.kotlinpoet.PropertySpec -import com.squareup.kotlinpoet.TypeName import com.squareup.kotlinpoet.TypeSpec import com.squareup.kotlinpoet.asClassName import com.squareup.kotlinpoet.asTypeName @@ -168,6 +167,7 @@ class Generator : CodeGenerator { .returns(ResponseMessage::class.asClassName().parameterizedBy(outputClassName)) .build() functions.add(unarySuspendFunction) + if (configuration.callbackSignature) { val callbackType = LambdaTypeName.get( receiver = ResponseMessage::class.asTypeName().parameterizedBy(outputClassName), @@ -225,14 +225,14 @@ class Generator : CodeGenerator { for (method in methods) { val inputClassName = classNameFromType(method.inputType) val outputClassName = classNameFromType(method.outputType) - val methodCallBlock = CodeBlock.builder() + val methodSpecCallBlock = CodeBlock.builder() .addStatement("MethodSpec(") .addStatement("\"$packageName.$serviceName/${method.name}\",") .indent() .addStatement("$inputClassName::class,") - .addStatement("$outputClassName::class,") + .addStatement("$outputClassName::class") .unindent() - .addStatement("),") + .addStatement(")") .build() if (method.isClientStreaming && method.isServerStreaming) { val streamingFunction = FunSpec.builder(method.name.lowerCamelCase()) @@ -252,7 +252,7 @@ class Generator : CodeGenerator { .addStatement("client.stream(") .indent() .addStatement("headers,") - .add(methodCallBlock) + .add(methodSpecCallBlock) .unindent() .addStatement(")") .build() @@ -273,7 +273,7 @@ class Generator : CodeGenerator { .addStatement("client.serverStream(") .indent() .addStatement("headers,") - .add(methodCallBlock) + .add(methodSpecCallBlock) .unindent() .addStatement(")") .build() @@ -294,7 +294,7 @@ class Generator : CodeGenerator { .addStatement("client.clientStream(") .indent() .addStatement("headers,") - .add(methodCallBlock) + .add(methodSpecCallBlock) .unindent() .addStatement(")") .build() @@ -315,13 +315,41 @@ class Generator : CodeGenerator { .indent() .addStatement("request,") .addStatement("headers,") - .add(methodCallBlock) + .add(methodSpecCallBlock) .unindent() .addStatement(")") .build() ) .build() functions.add(unarySuspendFunction) + if (configuration.callbackSignature) { + + val callbackType = LambdaTypeName.get( + receiver = ResponseMessage::class.asTypeName().parameterizedBy(outputClassName), + returnType = Unit::class.java.asTypeName() + ) + val unaryCallbackFunction = FunSpec.builder(method.name.lowerCamelCase()) + .addModifiers(KModifier.SUSPEND) + .addParameter("request", inputClassName) + .addParameter("headers", HEADERS_CLASS_NAME) + .addParameter("onResult", callbackType) + .returns(CANCELLABLE_CLASS_NAME) + .addStatement( + "return %L", + CodeBlock.builder() + .addStatement("client.unary(") + .indent() + .addStatement("request,") + .addStatement("headers,") + .add(methodSpecCallBlock) + .addStatement("onResult") + .unindent() + .addStatement(")") + .build() + ) + .build() + functions.add(unaryCallbackFunction) + } } } return functions From 0cb83dd814cecc84994a4d40632a4be9336db8b4 Mon Sep 17 00:00:00 2001 From: Alan Chiu Date: Mon, 27 Feb 2023 10:58:13 -0800 Subject: [PATCH 03/21] more --- .../src/main/kotlin/build/buf/protocgen/connect/Generator.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt b/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt index 859e1cd7..775179dc 100644 --- a/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt +++ b/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt @@ -330,6 +330,7 @@ class Generator : CodeGenerator { ) val unaryCallbackFunction = FunSpec.builder(method.name.lowerCamelCase()) .addModifiers(KModifier.SUSPEND) + .addModifiers(KModifier.OVERRIDE) .addParameter("request", inputClassName) .addParameter("headers", HEADERS_CLASS_NAME) .addParameter("onResult", callbackType) From b58eab8009d9012deb40be5a62c200ce95f0fe55 Mon Sep 17 00:00:00 2001 From: Alan Chiu Date: Mon, 27 Feb 2023 11:01:09 -0800 Subject: [PATCH 04/21] spotless --- .../src/main/kotlin/build/buf/protocgen/connect/Generator.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt b/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt index 775179dc..44a567a9 100644 --- a/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt +++ b/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt @@ -323,7 +323,6 @@ class Generator : CodeGenerator { .build() functions.add(unarySuspendFunction) if (configuration.callbackSignature) { - val callbackType = LambdaTypeName.get( receiver = ResponseMessage::class.asTypeName().parameterizedBy(outputClassName), returnType = Unit::class.java.asTypeName() From c377fab0a528a37b290d362b744846f8f112273a Mon Sep 17 00:00:00 2001 From: Alan Chiu Date: Mon, 27 Feb 2023 11:26:23 -0800 Subject: [PATCH 05/21] add callbackSignature docs --- README.md | 7 +++++++ .../main/kotlin/build/buf/protocgen/connect/Generator.kt | 2 +- .../build/buf/protocgen/connect/internal/Parameters.kt | 2 +- .../build/buf/protocgen/connect/internal/ProtoHelpers.kt | 7 ++++--- 4 files changed, 13 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 7da8bef5..cdd5c756 100644 --- a/README.md +++ b/README.md @@ -104,6 +104,13 @@ Comprehensive documentation for everything, including [interceptors][interceptors], [streaming][streaming], and [error handling][error-handling] is available on the [connect.build website][getting-started]. +## Generation Options + +| **Option** | **Type** | **Default** | **Repeatable** | **Details** | +|---------------------|:--------:|:-----------:|:--------------:|-------------------------------------------------| +| `callbackSignature` | Boolean | `false` | No | Generate callback signatures for unary methods. | + + ## Example Apps Example apps are available in [`/examples`](./examples). First, run `make generate` to generate diff --git a/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt b/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt index 44a567a9..54ab0293 100644 --- a/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt +++ b/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt @@ -232,7 +232,7 @@ class Generator : CodeGenerator { .addStatement("$inputClassName::class,") .addStatement("$outputClassName::class") .unindent() - .addStatement(")") + .addStatement("),") .build() if (method.isClientStreaming && method.isServerStreaming) { val streamingFunction = FunSpec.builder(method.name.lowerCamelCase()) diff --git a/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/internal/Parameters.kt b/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/internal/Parameters.kt index c6fc64c8..b7e4a892 100644 --- a/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/internal/Parameters.kt +++ b/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/internal/Parameters.kt @@ -14,7 +14,7 @@ package build.buf.protocgen.connect.internal -internal const val CALLBACK_SIGNATURE = "callback_signature" +internal const val CALLBACK_SIGNATURE = "callbackSignature" internal data class Configuration( val callbackSignature: Boolean diff --git a/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/internal/ProtoHelpers.kt b/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/internal/ProtoHelpers.kt index 9aadc5ec..8e0ffb2a 100644 --- a/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/internal/ProtoHelpers.kt +++ b/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/internal/ProtoHelpers.kt @@ -47,7 +47,7 @@ internal fun parseGeneratorParameter( if (text.isEmpty()) { return emptyMap() } - val ret: MutableMap = HashMap() + val result: MutableMap = HashMap() val parts = text.split(",".toRegex()).dropLastWhile { it.isEmpty() }.toTypedArray() for (part in parts) { if (part.isEmpty()) { @@ -63,9 +63,10 @@ internal fun parseGeneratorParameter( key = part.substring(0, equalsPos) value = part.substring(equalsPos + 1) } - ret[key] = value + val normalizedKey = underscoresToCamelCaseImpl(key, false) + result[normalizedKey] = value } - return ret + return result } /** From 350627e85b761d78eb2ffc0c9a08d7fba0fe76bb Mon Sep 17 00:00:00 2001 From: Alan Chiu Date: Mon, 27 Feb 2023 11:29:25 -0800 Subject: [PATCH 06/21] cu --- README.md | 1 - .../src/main/kotlin/build/buf/protocgen/connect/Generator.kt | 2 -- 2 files changed, 3 deletions(-) diff --git a/README.md b/README.md index cdd5c756..9be43053 100644 --- a/README.md +++ b/README.md @@ -110,7 +110,6 @@ is available on the [connect.build website][getting-started]. |---------------------|:--------:|:-----------:|:--------------:|-------------------------------------------------| | `callbackSignature` | Boolean | `false` | No | Generate callback signatures for unary methods. | - ## Example Apps Example apps are available in [`/examples`](./examples). First, run `make generate` to generate diff --git a/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt b/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt index 54ab0293..8c72c278 100644 --- a/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt +++ b/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt @@ -175,7 +175,6 @@ class Generator : CodeGenerator { ) val unaryCallbackFunction = FunSpec.builder(method.name.lowerCamelCase()) .addModifiers(KModifier.ABSTRACT) - .addModifiers(KModifier.SUSPEND) .addParameter("request", inputClassName) .addParameter(headerParameterSpec) .addParameter("onResult", callbackType) @@ -328,7 +327,6 @@ class Generator : CodeGenerator { returnType = Unit::class.java.asTypeName() ) val unaryCallbackFunction = FunSpec.builder(method.name.lowerCamelCase()) - .addModifiers(KModifier.SUSPEND) .addModifiers(KModifier.OVERRIDE) .addParameter("request", inputClassName) .addParameter("headers", HEADERS_CLASS_NAME) From 8305a09ee4f2c2aa79a5d4f23b272c9d28d2c9d8 Mon Sep 17 00:00:00 2001 From: Alan Chiu Date: Mon, 27 Feb 2023 11:53:34 -0800 Subject: [PATCH 07/21] add test --- .../build/buf/protocgen/connect/Generator.kt | 8 +++--- .../src/test/kotlin/PluginGenerationTest.kt | 27 +++++++++++++++++++ 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt b/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt index 8c72c278..a8c4ff67 100644 --- a/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt +++ b/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt @@ -170,8 +170,8 @@ class Generator : CodeGenerator { if (configuration.callbackSignature) { val callbackType = LambdaTypeName.get( - receiver = ResponseMessage::class.asTypeName().parameterizedBy(outputClassName), - returnType = Unit::class.java.asTypeName() + parameters = listOf(ParameterSpec("", ResponseMessage::class.asTypeName().parameterizedBy(outputClassName))), + returnType = Unit::class.java.asTypeName(), ) val unaryCallbackFunction = FunSpec.builder(method.name.lowerCamelCase()) .addModifiers(KModifier.ABSTRACT) @@ -323,8 +323,8 @@ class Generator : CodeGenerator { functions.add(unarySuspendFunction) if (configuration.callbackSignature) { val callbackType = LambdaTypeName.get( - receiver = ResponseMessage::class.asTypeName().parameterizedBy(outputClassName), - returnType = Unit::class.java.asTypeName() + parameters = listOf(ParameterSpec("", ResponseMessage::class.asTypeName().parameterizedBy(outputClassName))), + returnType = Unit::class.java.asTypeName(), ) val unaryCallbackFunction = FunSpec.builder(method.name.lowerCamelCase()) .addModifiers(KModifier.OVERRIDE) diff --git a/protoc-gen-connect-kotlin/src/test/kotlin/PluginGenerationTest.kt b/protoc-gen-connect-kotlin/src/test/kotlin/PluginGenerationTest.kt index bd34dd7e..1a475e9d 100644 --- a/protoc-gen-connect-kotlin/src/test/kotlin/PluginGenerationTest.kt +++ b/protoc-gen-connect-kotlin/src/test/kotlin/PluginGenerationTest.kt @@ -17,6 +17,7 @@ import buf.javamultiplefiles.disabled.v1.DisabledEmptyServiceClient import buf.javamultiplefiles.disabled.v1.DisabledInnerMessageServiceClient import buf.javamultiplefiles.disabled.v1.DisabledServiceClient import buf.javamultiplefiles.enabled.v1.EnabledEmptyRPCRequest +import buf.javamultiplefiles.enabled.v1.EnabledEmptyRPCResponse import buf.javamultiplefiles.enabled.v1.EnabledEmptyServiceClient import buf.javamultiplefiles.enabled.v1.EnabledInnerMessageServiceClient import buf.javamultiplefiles.enabled.v1.EnabledServiceClient @@ -81,4 +82,30 @@ class PluginGenerationTest { assertThat(UnspecifiedServiceClient::class.java).isNotNull assertThat(UnspecifiedInnerMessageServiceClient::class.java).isNotNull } + + @Test + fun callbackSignature() { + val unspecifiedEmptyServiceClient = UnspecifiedEmptyServiceClient(mock { }) + val request = UnspecifiedEmptyOuterClass.UnspecifiedEmptyRPCRequest.getDefaultInstance() + unspecifiedEmptyServiceClient.unspecifiedEmptyRPC(request) { response -> + response.success { success -> + assertThat(success.message).isOfAnyClassIn(UnspecifiedEmptyOuterClass.UnspecifiedEmptyRPCResponse::class.java) + + } + } + val disabledEmptyServiceClient = DisabledEmptyServiceClient(mock { }) + disabledEmptyServiceClient.disabledEmptyRPC(DisabledEmptyOuterClass.DisabledEmptyRPCRequest.getDefaultInstance()) { response -> + response.success { success -> + success.message + assertThat(success.message).isOfAnyClassIn(DisabledEmptyOuterClass.DisabledEmptyRPCResponse::class.java) + } + } + val enabledEmptyServiceClient = EnabledEmptyServiceClient(mock { }) + enabledEmptyServiceClient.enabledEmptyRPC(EnabledEmptyRPCRequest.getDefaultInstance()) { response -> + response.success { success -> + success.message + assertThat(success.message).isOfAnyClassIn(EnabledEmptyRPCResponse::class.java) + } + } + } } From 0293129dbce6ce0afdf3c653b2c7a6706c841246 Mon Sep 17 00:00:00 2001 From: Alan Chiu Date: Mon, 27 Feb 2023 11:54:12 -0800 Subject: [PATCH 08/21] camelcase --- protoc-gen-connect-kotlin/buf.gen.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/protoc-gen-connect-kotlin/buf.gen.yaml b/protoc-gen-connect-kotlin/buf.gen.yaml index 490bebb5..3a80fc64 100644 --- a/protoc-gen-connect-kotlin/buf.gen.yaml +++ b/protoc-gen-connect-kotlin/buf.gen.yaml @@ -3,6 +3,6 @@ plugins: - name: connect-kotlin out: src/test/java/ path: ./protoc-gen-connect-kotlin/protoc-gen-connect-kotlin - opt: callback_signature=true + opt: callbackSignature=true - name: java out: src/test/java/ From 1ff85331672637169ab8e64114c460f800a29217 Mon Sep 17 00:00:00 2001 From: Alan Chiu Date: Mon, 27 Feb 2023 11:56:07 -0800 Subject: [PATCH 09/21] comments --- .../buf/protocgen/connect/internal/Parameters.kt | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/internal/Parameters.kt b/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/internal/Parameters.kt index b7e4a892..892dc72c 100644 --- a/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/internal/Parameters.kt +++ b/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/internal/Parameters.kt @@ -16,10 +16,21 @@ package build.buf.protocgen.connect.internal internal const val CALLBACK_SIGNATURE = "callbackSignature" +/** + * The protoc plugin configuration class representation. + */ internal data class Configuration( val callbackSignature: Boolean ) +/** + * Parse options passed as a string. + * + * Key values are parsed with `parseGeneratorParameter()`. + * The key values are expected to be in camel casing but + * will internally translate from snake casing to camel + * casing. + */ internal fun parse(parameter: String): Configuration { val parameters = parseGeneratorParameter(parameter) return Configuration( From 89fbb4b15320fda8ea875146a9cd7233482cbf82 Mon Sep 17 00:00:00 2001 From: Alan Chiu Date: Mon, 27 Feb 2023 11:56:46 -0800 Subject: [PATCH 10/21] more --- .../kotlin/build/buf/protocgen/connect/internal/Parameters.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/internal/Parameters.kt b/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/internal/Parameters.kt index 892dc72c..7e171755 100644 --- a/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/internal/Parameters.kt +++ b/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/internal/Parameters.kt @@ -20,6 +20,7 @@ internal const val CALLBACK_SIGNATURE = "callbackSignature" * The protoc plugin configuration class representation. */ internal data class Configuration( + // Enable or disable callback signature generation. val callbackSignature: Boolean ) From 9af299a4aef8571f4dc6d6106a426440a2677ac5 Mon Sep 17 00:00:00 2001 From: Alan Chiu Date: Mon, 27 Feb 2023 12:00:33 -0800 Subject: [PATCH 11/21] spotless --- .../src/main/kotlin/build/buf/protocgen/connect/Generator.kt | 4 ++-- .../src/test/kotlin/PluginGenerationTest.kt | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt b/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt index a8c4ff67..9703deba 100644 --- a/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt +++ b/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt @@ -171,7 +171,7 @@ class Generator : CodeGenerator { if (configuration.callbackSignature) { val callbackType = LambdaTypeName.get( parameters = listOf(ParameterSpec("", ResponseMessage::class.asTypeName().parameterizedBy(outputClassName))), - returnType = Unit::class.java.asTypeName(), + returnType = Unit::class.java.asTypeName() ) val unaryCallbackFunction = FunSpec.builder(method.name.lowerCamelCase()) .addModifiers(KModifier.ABSTRACT) @@ -324,7 +324,7 @@ class Generator : CodeGenerator { if (configuration.callbackSignature) { val callbackType = LambdaTypeName.get( parameters = listOf(ParameterSpec("", ResponseMessage::class.asTypeName().parameterizedBy(outputClassName))), - returnType = Unit::class.java.asTypeName(), + returnType = Unit::class.java.asTypeName() ) val unaryCallbackFunction = FunSpec.builder(method.name.lowerCamelCase()) .addModifiers(KModifier.OVERRIDE) diff --git a/protoc-gen-connect-kotlin/src/test/kotlin/PluginGenerationTest.kt b/protoc-gen-connect-kotlin/src/test/kotlin/PluginGenerationTest.kt index 1a475e9d..081e2b47 100644 --- a/protoc-gen-connect-kotlin/src/test/kotlin/PluginGenerationTest.kt +++ b/protoc-gen-connect-kotlin/src/test/kotlin/PluginGenerationTest.kt @@ -90,7 +90,6 @@ class PluginGenerationTest { unspecifiedEmptyServiceClient.unspecifiedEmptyRPC(request) { response -> response.success { success -> assertThat(success.message).isOfAnyClassIn(UnspecifiedEmptyOuterClass.UnspecifiedEmptyRPCResponse::class.java) - } } val disabledEmptyServiceClient = DisabledEmptyServiceClient(mock { }) From 6d7c70e3978d086053b4c1cd5cb6752954428a3f Mon Sep 17 00:00:00 2001 From: Alan Chiu Date: Mon, 27 Feb 2023 14:37:42 -0800 Subject: [PATCH 12/21] add pr feedback for testing callbacks --- crosstests/buf.gen.yaml | 3 + .../buf/connect/crosstest/ssl/TestSuite.kt | 12 ++ .../build/buf/connect/crosstest/Main.kt | 22 +- .../TestServiceClientCallbackSuite.kt | 190 ++++++++++++++++++ .../crosstest/TestServiceClientSuite.kt | 2 +- .../build/buf/connect/crosstest/Main.kt | 4 +- .../crosstest/TestServiceClientSuite.kt | 2 +- protoc-gen-connect-kotlin/buf.gen.yaml | 4 +- .../build/buf/protocgen/connect/Generator.kt | 70 ++++--- .../protocgen/connect/internal/Parameters.kt | 14 +- 10 files changed, 278 insertions(+), 45 deletions(-) create mode 100644 crosstests/google-java/src/main/kotlin/build/buf/connect/crosstest/TestServiceClientCallbackSuite.kt diff --git a/crosstests/buf.gen.yaml b/crosstests/buf.gen.yaml index 02063d57..6c2ab266 100644 --- a/crosstests/buf.gen.yaml +++ b/crosstests/buf.gen.yaml @@ -6,6 +6,9 @@ plugins: - name: connect-kotlin out: google-java/src/main/kotlin/generated path: ./protoc-gen-connect-kotlin/protoc-gen-connect-kotlin + opt: + - generateCallbackMethods=true + - generateCoroutineMethods=true - name: java out: google-java/src/main/java/generated - name: kotlin diff --git a/crosstests/common/src/main/kotlin/build/buf/connect/crosstest/ssl/TestSuite.kt b/crosstests/common/src/main/kotlin/build/buf/connect/crosstest/ssl/TestSuite.kt index 9d189aa3..44a59cb7 100644 --- a/crosstests/common/src/main/kotlin/build/buf/connect/crosstest/ssl/TestSuite.kt +++ b/crosstests/common/src/main/kotlin/build/buf/connect/crosstest/ssl/TestSuite.kt @@ -38,3 +38,15 @@ interface TestSuite { suspend fun failUnary() suspend fun failServerStreaming() } + +interface UnaryCallbackTestSuite { + suspend fun test(tag: String) + suspend fun emptyUnary() + suspend fun largeUnary() + suspend fun customMetadata() + suspend fun statusCodeAndMessage() + suspend fun specialStatus() + suspend fun unimplementedMethod() + suspend fun unimplementedService() + suspend fun failUnary() +} \ No newline at end of file diff --git a/crosstests/google-java/src/main/kotlin/build/buf/connect/crosstest/Main.kt b/crosstests/google-java/src/main/kotlin/build/buf/connect/crosstest/Main.kt index 9724e3ee..87cfea4a 100644 --- a/crosstests/google-java/src/main/kotlin/build/buf/connect/crosstest/Main.kt +++ b/crosstests/google-java/src/main/kotlin/build/buf/connect/crosstest/Main.kt @@ -87,10 +87,11 @@ class Main { compressionPools = listOf(GzipCompressionPool) ) ) - tests(tag, connectClient, shortTimeoutClient) + coroutineTests(tag, connectClient, shortTimeoutClient) + callbackTests(tag, connectClient) } - private suspend fun tests( + private suspend fun coroutineTests( tag: String, protocolClient: ProtocolClient, shortTimeoutClient: ProtocolClient @@ -114,5 +115,22 @@ class Main { testServiceClientSuite.test(tag) } + + private suspend fun callbackTests( + tag: String, + protocolClient: ProtocolClient, + ) { + val testServiceClientSuite = TestServiceClientCallbackSuite(protocolClient) + testServiceClientSuite.emptyUnary() + testServiceClientSuite.largeUnary() + testServiceClientSuite.customMetadata() + testServiceClientSuite.statusCodeAndMessage() + testServiceClientSuite.specialStatus() + testServiceClientSuite.unimplementedMethod() + testServiceClientSuite.unimplementedService() + testServiceClientSuite.failUnary() + + testServiceClientSuite.test(tag) + } } } diff --git a/crosstests/google-java/src/main/kotlin/build/buf/connect/crosstest/TestServiceClientCallbackSuite.kt b/crosstests/google-java/src/main/kotlin/build/buf/connect/crosstest/TestServiceClientCallbackSuite.kt new file mode 100644 index 00000000..4970509d --- /dev/null +++ b/crosstests/google-java/src/main/kotlin/build/buf/connect/crosstest/TestServiceClientCallbackSuite.kt @@ -0,0 +1,190 @@ +// Copyright 2022-2023 Buf Technologies, 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 build.buf.connect.crosstest + +import build.buf.connect.Code +import build.buf.connect.crosstest.ssl.UnaryCallbackTestSuite +import build.buf.connect.impl.ProtocolClient +import com.google.protobuf.ByteString +import com.grpc.testing.ErrorDetail +import com.grpc.testing.TestServiceClient +import com.grpc.testing.UnimplementedServiceClient +import com.grpc.testing.echoStatus +import com.grpc.testing.empty +import com.grpc.testing.errorDetail +import com.grpc.testing.payload +import com.grpc.testing.simpleRequest +import org.assertj.core.api.Assertions.assertThat +import org.assertj.core.api.Assertions.fail +import kotlin.system.measureTimeMillis + +class TestServiceClientCallbackSuite( + client: ProtocolClient, +) : UnaryCallbackTestSuite { + private val testServiceConnectClient = TestServiceClient(client) + private val unimplementedServiceClient = UnimplementedServiceClient(client) + + private val tests = mutableListOf Unit>>() + + override suspend fun test(tag: String) { + println() + tests.forEachIndexed { index, (testName, test) -> + print("[$tag] Executing test case ${index + 1}/${tests.size}: $testName") + val millis = measureTimeMillis { + test() + } + println(" [$millis ms]") + } + } + + private fun register(testName: String, test: suspend () -> Unit) { + tests.add(testName to test) + } + + override suspend fun emptyUnary() = register("empty_unary") { + testServiceConnectClient.emptyCall(empty {}) { response -> + response.failure { + fail("expected error to be null") + } + response.success { success -> + assertThat(success.message).isEqualTo(empty {}) + } + } + + } + + override suspend fun largeUnary() = register("large_unary") { + val size = 314159 + val message = simpleRequest { + responseSize = size + payload = payload { + body = ByteString.copyFrom(ByteArray(size)) + } + } + testServiceConnectClient.unaryCall(message) { response -> + response.failure { + fail("expected error to be null") + } + response.success { success -> + assertThat(success.message.payload?.body?.toByteArray()?.size).isEqualTo(size) + } + } + + } + + override suspend fun customMetadata() = register("custom_metadata") { + val size = 314159 + val leadingKey = "x-grpc-test-echo-initial" + val leadingValue = "test_initial_metadata_value" + val trailingKey = "x-grpc-test-echo-trailing-bin" + val trailingValue = byteArrayOf(0xab.toByte(), 0xab.toByte(), 0xab.toByte()) + val headers = + mapOf( + leadingKey to listOf(leadingValue), + trailingKey to listOf(trailingValue.b64Encode()) + ) + val message = simpleRequest { + responseSize = size + payload = payload { body = ByteString.copyFrom(ByteArray(size)) } + } + testServiceConnectClient.unaryCall(message, headers) { response -> + assertThat(response.code).isEqualTo(Code.OK) + assertThat(response.headers[leadingKey]).containsExactly(leadingValue) + assertThat(response.trailers[trailingKey]).containsExactly(trailingValue.b64Encode()) + response.failure { + fail("expected error to be null") + } + response.success { success -> + assertThat(success.message.payload!!.body!!.size()).isEqualTo(size) + } + } + + } + + override suspend fun statusCodeAndMessage() = register("status_code_and_message") { + val message = simpleRequest { + responseStatus = echoStatus { + code = Code.UNKNOWN.value + message = "test status message" + } + } + testServiceConnectClient.unaryCall(message) { response -> + assertThat(response.code).isEqualTo(Code.UNKNOWN) + response.failure { errorResponse -> + assertThat(errorResponse.error).isNotNull() + assertThat(errorResponse.code).isEqualTo(Code.UNKNOWN) + assertThat(errorResponse.error.message).isEqualTo("test status message") + } + response.success { + fail("unexpected success") + } + } + } + + override suspend fun specialStatus() = register("special_status") { + val statusMessage = + "\\t\\ntest with whitespace\\r\\nand Unicode BMP ☺ and non-BMP \uD83D\uDE08\\t\\n" + testServiceConnectClient.unaryCall( + simpleRequest { + responseStatus = echoStatus { + code = 2 + message = statusMessage + } + } + ) { response -> + response.failure { errorResponse -> + val error = errorResponse.error + assertThat(error.code).isEqualTo(Code.UNKNOWN) + assertThat(response.code).isEqualTo(Code.UNKNOWN) + assertThat(error.message).isEqualTo(statusMessage) + } + response.success { + fail("unexpected success") + } + } + } + + override suspend fun unimplementedMethod() = register("unimplemented_method") { + testServiceConnectClient.unimplementedCall(empty {}) { response -> + assertThat(response.code).isEqualTo(Code.UNIMPLEMENTED) + } + } + + override suspend fun unimplementedService() = register("unimplemented_service") { + unimplementedServiceClient.unimplementedCall(empty {}) { response -> + assertThat(response.code).isEqualTo(Code.UNIMPLEMENTED) + } + } + + override suspend fun failUnary() = register("fail_unary") { + val expectedErrorDetail = errorDetail { + reason = "soirée 🎉" + domain = "connect-crosstest" + } + testServiceConnectClient.failUnaryCall(simpleRequest {}) { response -> + response.failure { errorResponse -> + val error = errorResponse.error + assertThat(error.code).isEqualTo(Code.RESOURCE_EXHAUSTED) + assertThat(error.message).isEqualTo("soirée 🎉") + val connectErrorDetails = error.unpackedDetails(ErrorDetail::class) + assertThat(connectErrorDetails).containsExactly(expectedErrorDetail) + } + response.success { + fail("unexpected success") + } + assertThat(response.code).isEqualTo(Code.RESOURCE_EXHAUSTED) + } + } +} \ No newline at end of file diff --git a/crosstests/google-java/src/main/kotlin/build/buf/connect/crosstest/TestServiceClientSuite.kt b/crosstests/google-java/src/main/kotlin/build/buf/connect/crosstest/TestServiceClientSuite.kt index cb976c5b..2bee7ae0 100644 --- a/crosstests/google-java/src/main/kotlin/build/buf/connect/crosstest/TestServiceClientSuite.kt +++ b/crosstests/google-java/src/main/kotlin/build/buf/connect/crosstest/TestServiceClientSuite.kt @@ -428,6 +428,6 @@ class TestServiceClientSuite( } } -private fun ByteArray.b64Encode(): String { +internal fun ByteArray.b64Encode(): String { return this.toByteString().base64() } diff --git a/crosstests/google-javalite/src/main/kotlin/build/buf/connect/crosstest/Main.kt b/crosstests/google-javalite/src/main/kotlin/build/buf/connect/crosstest/Main.kt index 85febc7e..c6fe3ce0 100644 --- a/crosstests/google-javalite/src/main/kotlin/build/buf/connect/crosstest/Main.kt +++ b/crosstests/google-javalite/src/main/kotlin/build/buf/connect/crosstest/Main.kt @@ -87,10 +87,10 @@ class Main { compressionPools = listOf(GzipCompressionPool) ) ) - tests(tag, connectClient, shortTimeoutClient) + suspendTests(tag, connectClient, shortTimeoutClient) } - private suspend fun tests( + private suspend fun suspendTests( tag: String, protocolClient: ProtocolClient, shortTimeoutClient: ProtocolClient diff --git a/crosstests/google-javalite/src/main/kotlin/build/buf/connect/crosstest/TestServiceClientSuite.kt b/crosstests/google-javalite/src/main/kotlin/build/buf/connect/crosstest/TestServiceClientSuite.kt index cb976c5b..2bee7ae0 100644 --- a/crosstests/google-javalite/src/main/kotlin/build/buf/connect/crosstest/TestServiceClientSuite.kt +++ b/crosstests/google-javalite/src/main/kotlin/build/buf/connect/crosstest/TestServiceClientSuite.kt @@ -428,6 +428,6 @@ class TestServiceClientSuite( } } -private fun ByteArray.b64Encode(): String { +internal fun ByteArray.b64Encode(): String { return this.toByteString().base64() } diff --git a/protoc-gen-connect-kotlin/buf.gen.yaml b/protoc-gen-connect-kotlin/buf.gen.yaml index 3a80fc64..75ad0a5a 100644 --- a/protoc-gen-connect-kotlin/buf.gen.yaml +++ b/protoc-gen-connect-kotlin/buf.gen.yaml @@ -3,6 +3,8 @@ plugins: - name: connect-kotlin out: src/test/java/ path: ./protoc-gen-connect-kotlin/protoc-gen-connect-kotlin - opt: callbackSignature=true + opt: + - generateCallbackMethods=true + - generateCoroutineMethods=true - name: java out: src/test/java/ diff --git a/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt b/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt index 9703deba..c04a90fe 100644 --- a/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt +++ b/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt @@ -52,7 +52,7 @@ import com.squareup.kotlinpoet.asTypeName * move off of type aliases, this can be changed without user API breakage. */ private val HEADERS_CLASS_NAME = ClassName("build.buf.connect", "Headers") -private val CANCELLABLE_CLASS_NAME = ClassName("build.buf.connect.http", "Cancelable") +private val CANCELABLE_CLASS_NAME = ClassName("build.buf.connect.http", "Cancelable") class Generator : CodeGenerator { private lateinit var descriptorSource: Plugin.DescriptorSource @@ -159,16 +159,18 @@ class Generator : CodeGenerator { .build() functions.add(clientStreamingFunction) } else { - val unarySuspendFunction = FunSpec.builder(method.name.lowerCamelCase()) - .addModifiers(KModifier.ABSTRACT) - .addModifiers(KModifier.SUSPEND) - .addParameter("request", inputClassName) - .addParameter(headerParameterSpec) - .returns(ResponseMessage::class.asClassName().parameterizedBy(outputClassName)) - .build() - functions.add(unarySuspendFunction) + if (configuration.generateCoroutineMethods) { + val unarySuspendFunction = FunSpec.builder(method.name.lowerCamelCase()) + .addModifiers(KModifier.ABSTRACT) + .addModifiers(KModifier.SUSPEND) + .addParameter("request", inputClassName) + .addParameter(headerParameterSpec) + .returns(ResponseMessage::class.asClassName().parameterizedBy(outputClassName)) + .build() + functions.add(unarySuspendFunction) + } - if (configuration.callbackSignature) { + if (configuration.generateCallbackMethods) { val callbackType = LambdaTypeName.get( parameters = listOf(ParameterSpec("", ResponseMessage::class.asTypeName().parameterizedBy(outputClassName))), returnType = Unit::class.java.asTypeName() @@ -178,7 +180,7 @@ class Generator : CodeGenerator { .addParameter("request", inputClassName) .addParameter(headerParameterSpec) .addParameter("onResult", callbackType) - .returns(CANCELLABLE_CLASS_NAME) + .returns(CANCELABLE_CLASS_NAME) .build() functions.add(unaryCallbackFunction) } @@ -301,27 +303,29 @@ class Generator : CodeGenerator { .build() functions.add(clientStreamingFunction) } else { - val unarySuspendFunction = FunSpec.builder(method.name.lowerCamelCase()) - .addModifiers(KModifier.SUSPEND) - .addModifiers(KModifier.OVERRIDE) - .addParameter("request", inputClassName) - .addParameter("headers", HEADERS_CLASS_NAME) - .returns(ResponseMessage::class.asClassName().parameterizedBy(outputClassName)) - .addStatement( - "return %L", - CodeBlock.builder() - .addStatement("client.unary(") - .indent() - .addStatement("request,") - .addStatement("headers,") - .add(methodSpecCallBlock) - .unindent() - .addStatement(")") - .build() - ) - .build() - functions.add(unarySuspendFunction) - if (configuration.callbackSignature) { + if (configuration.generateCoroutineMethods) { + val unarySuspendFunction = FunSpec.builder(method.name.lowerCamelCase()) + .addModifiers(KModifier.SUSPEND) + .addModifiers(KModifier.OVERRIDE) + .addParameter("request", inputClassName) + .addParameter("headers", HEADERS_CLASS_NAME) + .returns(ResponseMessage::class.asClassName().parameterizedBy(outputClassName)) + .addStatement( + "return %L", + CodeBlock.builder() + .addStatement("client.unary(") + .indent() + .addStatement("request,") + .addStatement("headers,") + .add(methodSpecCallBlock) + .unindent() + .addStatement(")") + .build() + ) + .build() + functions.add(unarySuspendFunction) + } + if (configuration.generateCallbackMethods) { val callbackType = LambdaTypeName.get( parameters = listOf(ParameterSpec("", ResponseMessage::class.asTypeName().parameterizedBy(outputClassName))), returnType = Unit::class.java.asTypeName() @@ -331,7 +335,7 @@ class Generator : CodeGenerator { .addParameter("request", inputClassName) .addParameter("headers", HEADERS_CLASS_NAME) .addParameter("onResult", callbackType) - .returns(CANCELLABLE_CLASS_NAME) + .returns(CANCELABLE_CLASS_NAME) .addStatement( "return %L", CodeBlock.builder() diff --git a/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/internal/Parameters.kt b/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/internal/Parameters.kt index 7e171755..2a16c7d9 100644 --- a/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/internal/Parameters.kt +++ b/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/internal/Parameters.kt @@ -14,14 +14,17 @@ package build.buf.protocgen.connect.internal -internal const val CALLBACK_SIGNATURE = "callbackSignature" +internal const val CALLBACK_SIGNATURE = "generateCallbackMethods" +internal const val COROUTINE_SIGNATURE = "generateCoroutineMethods" /** * The protoc plugin configuration class representation. */ internal data class Configuration( // Enable or disable callback signature generation. - val callbackSignature: Boolean + val generateCallbackMethods: Boolean, + // Enable or disable coroutine signature generation. + val generateCoroutineMethods: Boolean, ) /** @@ -32,9 +35,10 @@ internal data class Configuration( * will internally translate from snake casing to camel * casing. */ -internal fun parse(parameter: String): Configuration { - val parameters = parseGeneratorParameter(parameter) +internal fun parse(input: String): Configuration { + val parameters = parseGeneratorParameter(input) return Configuration( - callbackSignature = parameters[CALLBACK_SIGNATURE]?.toBoolean() ?: false + generateCallbackMethods = parameters[CALLBACK_SIGNATURE]?.toBoolean() ?: false, + generateCoroutineMethods = parameters[COROUTINE_SIGNATURE]?.toBoolean() ?: true, // Defaulted to true. ) } From 217b3ae771730990ab3c3f44db1f499e17cba1b8 Mon Sep 17 00:00:00 2001 From: Alan Chiu Date: Mon, 27 Feb 2023 14:41:06 -0800 Subject: [PATCH 13/21] docs --- README.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 9be43053..c7baff31 100644 --- a/README.md +++ b/README.md @@ -106,9 +106,10 @@ is available on the [connect.build website][getting-started]. ## Generation Options -| **Option** | **Type** | **Default** | **Repeatable** | **Details** | -|---------------------|:--------:|:-----------:|:--------------:|-------------------------------------------------| -| `callbackSignature` | Boolean | `false` | No | Generate callback signatures for unary methods. | +| **Option** | **Type** | **Default** | **Repeatable** | **Details** | +|----------------------------|:--------:|:-----------:|:--------------:|-------------------------------------------------| +| `generateCallbackMethods` | Boolean | `false` | No | Generate callback signatures for unary methods. | +| `generateCoroutineMethods` | Boolean | `true` | No | Generate suspend signatures for unary methods. | ## Example Apps From 1785e6766be53062fcd9b0229a0ad2801f9709f2 Mon Sep 17 00:00:00 2001 From: Alan Chiu Date: Mon, 27 Feb 2023 14:50:29 -0800 Subject: [PATCH 14/21] fix empty package --- protoc-gen-connect-kotlin/buf.gen.yaml | 2 ++ .../main/kotlin/build/buf/protocgen/connect/Generator.kt | 7 ++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/protoc-gen-connect-kotlin/buf.gen.yaml b/protoc-gen-connect-kotlin/buf.gen.yaml index 75ad0a5a..c8704931 100644 --- a/protoc-gen-connect-kotlin/buf.gen.yaml +++ b/protoc-gen-connect-kotlin/buf.gen.yaml @@ -8,3 +8,5 @@ plugins: - generateCoroutineMethods=true - name: java out: src/test/java/ + - plugin: buf.build/grpc/java + out: src/test/java/ diff --git a/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt b/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt index c04a90fe..af588741 100644 --- a/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt +++ b/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt @@ -224,11 +224,16 @@ class Generator : CodeGenerator { ): List { val functions = mutableListOf() for (method in methods) { + val path = if (packageName.isNotEmpty()) { + "$packageName.$serviceName/${method.name}" + } else { + "$serviceName/${method.name}" + } val inputClassName = classNameFromType(method.inputType) val outputClassName = classNameFromType(method.outputType) val methodSpecCallBlock = CodeBlock.builder() .addStatement("MethodSpec(") - .addStatement("\"$packageName.$serviceName/${method.name}\",") + .addStatement("\"$path\",") .indent() .addStatement("$inputClassName::class,") .addStatement("$outputClassName::class") From 9d46c80cd885ee7431aa7bb308c1c2ffc132bbd7 Mon Sep 17 00:00:00 2001 From: Alan Chiu Date: Mon, 27 Feb 2023 14:50:47 -0800 Subject: [PATCH 15/21] fix --- protoc-gen-connect-kotlin/buf.gen.yaml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/protoc-gen-connect-kotlin/buf.gen.yaml b/protoc-gen-connect-kotlin/buf.gen.yaml index c8704931..fd60ff0b 100644 --- a/protoc-gen-connect-kotlin/buf.gen.yaml +++ b/protoc-gen-connect-kotlin/buf.gen.yaml @@ -8,5 +8,4 @@ plugins: - generateCoroutineMethods=true - name: java out: src/test/java/ - - plugin: buf.build/grpc/java - out: src/test/java/ + From 777cb5c51966b1381c376b94edbd46abd1318e57 Mon Sep 17 00:00:00 2001 From: Alan Chiu Date: Mon, 27 Feb 2023 14:52:02 -0800 Subject: [PATCH 16/21] more --- .../buf/connect/crosstest/TestServiceClientCallbackSuite.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crosstests/google-java/src/main/kotlin/build/buf/connect/crosstest/TestServiceClientCallbackSuite.kt b/crosstests/google-java/src/main/kotlin/build/buf/connect/crosstest/TestServiceClientCallbackSuite.kt index 4970509d..05638473 100644 --- a/crosstests/google-java/src/main/kotlin/build/buf/connect/crosstest/TestServiceClientCallbackSuite.kt +++ b/crosstests/google-java/src/main/kotlin/build/buf/connect/crosstest/TestServiceClientCallbackSuite.kt @@ -187,4 +187,4 @@ class TestServiceClientCallbackSuite( assertThat(response.code).isEqualTo(Code.RESOURCE_EXHAUSTED) } } -} \ No newline at end of file +} From 40ecde2b5061413d0d8ae9ec8d2aa1299798d8a2 Mon Sep 17 00:00:00 2001 From: Alan Chiu Date: Mon, 27 Feb 2023 14:52:17 -0800 Subject: [PATCH 17/21] more --- .../main/kotlin/build/buf/connect/crosstest/ssl/TestSuite.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crosstests/common/src/main/kotlin/build/buf/connect/crosstest/ssl/TestSuite.kt b/crosstests/common/src/main/kotlin/build/buf/connect/crosstest/ssl/TestSuite.kt index 44a59cb7..06636683 100644 --- a/crosstests/common/src/main/kotlin/build/buf/connect/crosstest/ssl/TestSuite.kt +++ b/crosstests/common/src/main/kotlin/build/buf/connect/crosstest/ssl/TestSuite.kt @@ -49,4 +49,4 @@ interface UnaryCallbackTestSuite { suspend fun unimplementedMethod() suspend fun unimplementedService() suspend fun failUnary() -} \ No newline at end of file +} From 17e98498eda8c7b65ef86c29053ffc36dccd0888 Mon Sep 17 00:00:00 2001 From: Alan Chiu Date: Mon, 27 Feb 2023 14:55:42 -0800 Subject: [PATCH 18/21] cdl --- .../TestServiceClientCallbackSuite.kt | 40 +++++++++++++++++-- 1 file changed, 36 insertions(+), 4 deletions(-) diff --git a/crosstests/google-java/src/main/kotlin/build/buf/connect/crosstest/TestServiceClientCallbackSuite.kt b/crosstests/google-java/src/main/kotlin/build/buf/connect/crosstest/TestServiceClientCallbackSuite.kt index 05638473..360a06d1 100644 --- a/crosstests/google-java/src/main/kotlin/build/buf/connect/crosstest/TestServiceClientCallbackSuite.kt +++ b/crosstests/google-java/src/main/kotlin/build/buf/connect/crosstest/TestServiceClientCallbackSuite.kt @@ -28,6 +28,8 @@ import com.grpc.testing.payload import com.grpc.testing.simpleRequest import org.assertj.core.api.Assertions.assertThat import org.assertj.core.api.Assertions.fail +import java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit import kotlin.system.measureTimeMillis class TestServiceClientCallbackSuite( @@ -54,15 +56,18 @@ class TestServiceClientCallbackSuite( } override suspend fun emptyUnary() = register("empty_unary") { + val countDownLatch = CountDownLatch(1) testServiceConnectClient.emptyCall(empty {}) { response -> response.failure { fail("expected error to be null") } response.success { success -> assertThat(success.message).isEqualTo(empty {}) + countDownLatch.countDown() } } - + countDownLatch.await(500, TimeUnit.MILLISECONDS) + assertThat(countDownLatch.count).isZero() } override suspend fun largeUnary() = register("large_unary") { @@ -73,15 +78,18 @@ class TestServiceClientCallbackSuite( body = ByteString.copyFrom(ByteArray(size)) } } + val countDownLatch = CountDownLatch(1) testServiceConnectClient.unaryCall(message) { response -> response.failure { fail("expected error to be null") } response.success { success -> assertThat(success.message.payload?.body?.toByteArray()?.size).isEqualTo(size) + countDownLatch.countDown() } } - + countDownLatch.await(500, TimeUnit.MILLISECONDS) + assertThat(countDownLatch.count).isZero() } override suspend fun customMetadata() = register("custom_metadata") { @@ -99,6 +107,7 @@ class TestServiceClientCallbackSuite( responseSize = size payload = payload { body = ByteString.copyFrom(ByteArray(size)) } } + val countDownLatch = CountDownLatch(1) testServiceConnectClient.unaryCall(message, headers) { response -> assertThat(response.code).isEqualTo(Code.OK) assertThat(response.headers[leadingKey]).containsExactly(leadingValue) @@ -108,9 +117,11 @@ class TestServiceClientCallbackSuite( } response.success { success -> assertThat(success.message.payload!!.body!!.size()).isEqualTo(size) + countDownLatch.countDown() } } - + countDownLatch.await(500, TimeUnit.MILLISECONDS) + assertThat(countDownLatch.count).isZero() } override suspend fun statusCodeAndMessage() = register("status_code_and_message") { @@ -120,22 +131,28 @@ class TestServiceClientCallbackSuite( message = "test status message" } } + val countDownLatch = CountDownLatch(1) testServiceConnectClient.unaryCall(message) { response -> assertThat(response.code).isEqualTo(Code.UNKNOWN) response.failure { errorResponse -> assertThat(errorResponse.error).isNotNull() assertThat(errorResponse.code).isEqualTo(Code.UNKNOWN) assertThat(errorResponse.error.message).isEqualTo("test status message") + countDownLatch.countDown() } response.success { fail("unexpected success") } } + + countDownLatch.await(500, TimeUnit.MILLISECONDS) + assertThat(countDownLatch.count).isZero() } override suspend fun specialStatus() = register("special_status") { val statusMessage = "\\t\\ntest with whitespace\\r\\nand Unicode BMP ☺ and non-BMP \uD83D\uDE08\\t\\n" + val countDownLatch = CountDownLatch(1) testServiceConnectClient.unaryCall( simpleRequest { responseStatus = echoStatus { @@ -149,23 +166,34 @@ class TestServiceClientCallbackSuite( assertThat(error.code).isEqualTo(Code.UNKNOWN) assertThat(response.code).isEqualTo(Code.UNKNOWN) assertThat(error.message).isEqualTo(statusMessage) + countDownLatch.countDown() } response.success { fail("unexpected success") } } + countDownLatch.await(500, TimeUnit.MILLISECONDS) + assertThat(countDownLatch.count).isZero() } override suspend fun unimplementedMethod() = register("unimplemented_method") { + val countDownLatch = CountDownLatch(1) testServiceConnectClient.unimplementedCall(empty {}) { response -> assertThat(response.code).isEqualTo(Code.UNIMPLEMENTED) + countDownLatch.countDown() } + countDownLatch.await(500, TimeUnit.MILLISECONDS) + assertThat(countDownLatch.count).isZero() } override suspend fun unimplementedService() = register("unimplemented_service") { + val countDownLatch = CountDownLatch(1) unimplementedServiceClient.unimplementedCall(empty {}) { response -> assertThat(response.code).isEqualTo(Code.UNIMPLEMENTED) + countDownLatch.countDown() } + countDownLatch.await(500, TimeUnit.MILLISECONDS) + assertThat(countDownLatch.count).isZero() } override suspend fun failUnary() = register("fail_unary") { @@ -173,18 +201,22 @@ class TestServiceClientCallbackSuite( reason = "soirée 🎉" domain = "connect-crosstest" } + val countDownLatch = CountDownLatch(1) testServiceConnectClient.failUnaryCall(simpleRequest {}) { response -> + assertThat(response.code).isEqualTo(Code.RESOURCE_EXHAUSTED) response.failure { errorResponse -> val error = errorResponse.error assertThat(error.code).isEqualTo(Code.RESOURCE_EXHAUSTED) assertThat(error.message).isEqualTo("soirée 🎉") val connectErrorDetails = error.unpackedDetails(ErrorDetail::class) assertThat(connectErrorDetails).containsExactly(expectedErrorDetail) + countDownLatch.countDown() } response.success { fail("unexpected success") } - assertThat(response.code).isEqualTo(Code.RESOURCE_EXHAUSTED) } + countDownLatch.await(500, TimeUnit.MILLISECONDS) + assertThat(countDownLatch.count).isZero() } } From 8399f642f1a0d49e45cd4ecf7bb853faa046b13a Mon Sep 17 00:00:00 2001 From: Alan Chiu Date: Mon, 27 Feb 2023 15:00:52 -0800 Subject: [PATCH 19/21] spotless --- .../src/main/kotlin/build/buf/connect/crosstest/Main.kt | 2 +- .../buf/connect/crosstest/TestServiceClientCallbackSuite.kt | 2 +- .../kotlin/build/buf/protocgen/connect/internal/Parameters.kt | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crosstests/google-java/src/main/kotlin/build/buf/connect/crosstest/Main.kt b/crosstests/google-java/src/main/kotlin/build/buf/connect/crosstest/Main.kt index 87cfea4a..40848cc4 100644 --- a/crosstests/google-java/src/main/kotlin/build/buf/connect/crosstest/Main.kt +++ b/crosstests/google-java/src/main/kotlin/build/buf/connect/crosstest/Main.kt @@ -118,7 +118,7 @@ class Main { private suspend fun callbackTests( tag: String, - protocolClient: ProtocolClient, + protocolClient: ProtocolClient ) { val testServiceClientSuite = TestServiceClientCallbackSuite(protocolClient) testServiceClientSuite.emptyUnary() diff --git a/crosstests/google-java/src/main/kotlin/build/buf/connect/crosstest/TestServiceClientCallbackSuite.kt b/crosstests/google-java/src/main/kotlin/build/buf/connect/crosstest/TestServiceClientCallbackSuite.kt index 360a06d1..f55ab9f6 100644 --- a/crosstests/google-java/src/main/kotlin/build/buf/connect/crosstest/TestServiceClientCallbackSuite.kt +++ b/crosstests/google-java/src/main/kotlin/build/buf/connect/crosstest/TestServiceClientCallbackSuite.kt @@ -33,7 +33,7 @@ import java.util.concurrent.TimeUnit import kotlin.system.measureTimeMillis class TestServiceClientCallbackSuite( - client: ProtocolClient, + client: ProtocolClient ) : UnaryCallbackTestSuite { private val testServiceConnectClient = TestServiceClient(client) private val unimplementedServiceClient = UnimplementedServiceClient(client) diff --git a/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/internal/Parameters.kt b/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/internal/Parameters.kt index 2a16c7d9..aabb770a 100644 --- a/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/internal/Parameters.kt +++ b/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/internal/Parameters.kt @@ -24,7 +24,7 @@ internal data class Configuration( // Enable or disable callback signature generation. val generateCallbackMethods: Boolean, // Enable or disable coroutine signature generation. - val generateCoroutineMethods: Boolean, + val generateCoroutineMethods: Boolean ) /** @@ -39,6 +39,6 @@ internal fun parse(input: String): Configuration { val parameters = parseGeneratorParameter(input) return Configuration( generateCallbackMethods = parameters[CALLBACK_SIGNATURE]?.toBoolean() ?: false, - generateCoroutineMethods = parameters[COROUTINE_SIGNATURE]?.toBoolean() ?: true, // Defaulted to true. + generateCoroutineMethods = parameters[COROUTINE_SIGNATURE]?.toBoolean() ?: true // Defaulted to true. ) } From 7d8d3bc854091d85cf1c2c64773a71368281a0bf Mon Sep 17 00:00:00 2001 From: Alan Chiu Date: Mon, 27 Feb 2023 15:06:32 -0800 Subject: [PATCH 20/21] jhmagic --- .../build/buf/protocgen/connect/Generator.kt | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt b/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt index af588741..431e2817 100644 --- a/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt +++ b/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt @@ -98,7 +98,7 @@ class Generator : CodeGenerator { .addFileComment("\n") .addFileComment("Source: ${file.name}\n") // Set the file package for the generated methods. - .addType(serviceClientImplementation(file.`package`, packageName, service)) + .addType(serviceClientImplementation(packageName, service)) .build() fileSpecs.put(serviceClientImplementationClassName(packageName, service), implementationFileSpec) } @@ -190,7 +190,6 @@ class Generator : CodeGenerator { } private fun serviceClientImplementation( - packageName: String, javaPackageName: String, service: Descriptors.ServiceDescriptor ): TypeSpec { @@ -208,8 +207,7 @@ class Generator : CodeGenerator { .build() ) val functionSpecs = implementationMethods( - packageName, - service.name, + service.fullName, service.methods ) return classBuilder @@ -218,22 +216,16 @@ class Generator : CodeGenerator { } private fun implementationMethods( - packageName: String, serviceName: String, methods: List ): List { val functions = mutableListOf() for (method in methods) { - val path = if (packageName.isNotEmpty()) { - "$packageName.$serviceName/${method.name}" - } else { - "$serviceName/${method.name}" - } val inputClassName = classNameFromType(method.inputType) val outputClassName = classNameFromType(method.outputType) val methodSpecCallBlock = CodeBlock.builder() .addStatement("MethodSpec(") - .addStatement("\"$path\",") + .addStatement("\"$serviceName/${method.name}\",") .indent() .addStatement("$inputClassName::class,") .addStatement("$outputClassName::class") From 1e9f5cd13eaec087c63a6384f173704d65189c75 Mon Sep 17 00:00:00 2001 From: Alan Chiu Date: Mon, 27 Feb 2023 15:08:37 -0800 Subject: [PATCH 21/21] fixup --- .../main/kotlin/build/buf/protocgen/connect/Generator.kt | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt b/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt index 431e2817..91f32ab1 100644 --- a/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt +++ b/protoc-gen-connect-kotlin/src/main/kotlin/build/buf/protocgen/connect/Generator.kt @@ -206,17 +206,13 @@ class Generator : CodeGenerator { .initializer("client") .build() ) - val functionSpecs = implementationMethods( - service.fullName, - service.methods - ) + val functionSpecs = implementationMethods(service.methods) return classBuilder .addFunctions(functionSpecs) .build() } private fun implementationMethods( - serviceName: String, methods: List ): List { val functions = mutableListOf() @@ -225,7 +221,7 @@ class Generator : CodeGenerator { val outputClassName = classNameFromType(method.outputType) val methodSpecCallBlock = CodeBlock.builder() .addStatement("MethodSpec(") - .addStatement("\"$serviceName/${method.name}\",") + .addStatement("\"${method.service.fullName}/${method.name}\",") .indent() .addStatement("$inputClassName::class,") .addStatement("$outputClassName::class")