From ab4b35a25914b4fb9913d8e330d6f9589d12a98a Mon Sep 17 00:00:00 2001 From: BuddhiWathsala Date: Thu, 8 Apr 2021 15:20:44 +0530 Subject: [PATCH 1/2] Improve the compiler plugin to check package aliases --- .../net/grpc/plugin/GrpcConstants.java | 2 +- .../net/grpc/plugin/GrpcServiceValidator.java | 108 ++++++++++-------- 2 files changed, 59 insertions(+), 51 deletions(-) diff --git a/grpc-compiler-plugin/src/main/java/org/ballerinalang/net/grpc/plugin/GrpcConstants.java b/grpc-compiler-plugin/src/main/java/org/ballerinalang/net/grpc/plugin/GrpcConstants.java index a84561376..8e81c8464 100644 --- a/grpc-compiler-plugin/src/main/java/org/ballerinalang/net/grpc/plugin/GrpcConstants.java +++ b/grpc-compiler-plugin/src/main/java/org/ballerinalang/net/grpc/plugin/GrpcConstants.java @@ -23,7 +23,7 @@ */ public class GrpcConstants { - public static final String GRPC_ANNOTATION_NAME = "grpc:ServiceDescriptor"; + public static final String GRPC_ANNOTATION_NAME = "ServiceDescriptor"; public static final String BALLERINA_ORG_NAME = "ballerina"; public static final String GRPC_PACKAGE_NAME = "grpc"; diff --git a/grpc-compiler-plugin/src/main/java/org/ballerinalang/net/grpc/plugin/GrpcServiceValidator.java b/grpc-compiler-plugin/src/main/java/org/ballerinalang/net/grpc/plugin/GrpcServiceValidator.java index 7ebd5e5ac..ad972dce3 100644 --- a/grpc-compiler-plugin/src/main/java/org/ballerinalang/net/grpc/plugin/GrpcServiceValidator.java +++ b/grpc-compiler-plugin/src/main/java/org/ballerinalang/net/grpc/plugin/GrpcServiceValidator.java @@ -18,13 +18,13 @@ package org.ballerinalang.net.grpc.plugin; +import io.ballerina.compiler.api.symbols.AnnotationSymbol; import io.ballerina.compiler.api.symbols.ServiceDeclarationSymbol; import io.ballerina.compiler.api.symbols.Symbol; import io.ballerina.compiler.api.symbols.TypeDescKind; import io.ballerina.compiler.api.symbols.TypeReferenceTypeSymbol; import io.ballerina.compiler.api.symbols.TypeSymbol; import io.ballerina.compiler.api.symbols.UnionTypeSymbol; -import io.ballerina.compiler.syntax.tree.AnnotationNode; import io.ballerina.compiler.syntax.tree.DefaultableParameterNode; import io.ballerina.compiler.syntax.tree.FunctionDefinitionNode; import io.ballerina.compiler.syntax.tree.FunctionSignatureNode; @@ -58,74 +58,82 @@ public class GrpcServiceValidator implements AnalysisTask - child.kind() == SyntaxKind.OBJECT_METHOD_DEFINITION - || child.kind() == SyntaxKind.RESOURCE_ACCESSOR_DEFINITION).forEach(node -> { - - FunctionDefinitionNode functionDefinitionNode = (FunctionDefinitionNode) node; - // Check functions are remote or not - validateServiceFunctions(functionDefinitionNode, syntaxNodeAnalysisContext); - // Check params and return types - validateFunctionSignature(functionDefinitionNode, syntaxNodeAnalysisContext, serviceName); - - }); + Optional optionalServiceDeclarationSymbol = syntaxNodeAnalysisContext.semanticModel() + .symbol(serviceDeclarationNode); + if (optionalServiceDeclarationSymbol.isPresent() && + (optionalServiceDeclarationSymbol.get() instanceof ServiceDeclarationSymbol)) { + + ServiceDeclarationSymbol serviceDeclarationSymbol = (ServiceDeclarationSymbol) + optionalServiceDeclarationSymbol.get(); + + if (isBallerinaGrpcService(serviceDeclarationSymbol)) { + String serviceName = serviceNameFromServiceDeclarationNode(serviceDeclarationNode); + validateServiceAnnotation(serviceDeclarationNode, syntaxNodeAnalysisContext, serviceDeclarationSymbol); + serviceDeclarationNode.members().stream().filter(child -> + child.kind() == SyntaxKind.OBJECT_METHOD_DEFINITION + || child.kind() == SyntaxKind.RESOURCE_ACCESSOR_DEFINITION).forEach(node -> { + + FunctionDefinitionNode functionDefinitionNode = (FunctionDefinitionNode) node; + // Check functions are remote or not + validateServiceFunctions(functionDefinitionNode, syntaxNodeAnalysisContext); + // Check params and return types + validateFunctionSignature(functionDefinitionNode, syntaxNodeAnalysisContext, serviceName); + + }); + } + } + } - public boolean isBallerinaGrpcService(SyntaxNodeAnalysisContext syntaxNodeAnalysisContext) { + public boolean isBallerinaGrpcService(ServiceDeclarationSymbol serviceDeclarationSymbol) { - ServiceDeclarationNode serviceDeclarationNode = (ServiceDeclarationNode) syntaxNodeAnalysisContext.node(); - Optional serviceDeclarationSymbol = syntaxNodeAnalysisContext.semanticModel() - .symbol(serviceDeclarationNode); - if (serviceDeclarationSymbol.isPresent()) { - List listenerTypes = ((ServiceDeclarationSymbol) serviceDeclarationSymbol.get()) - .listenerTypes(); - for (TypeSymbol listenerType : listenerTypes) { - if (listenerType.typeKind() == TypeDescKind.UNION) { - List memberDescriptors = ((UnionTypeSymbol) listenerType).memberTypeDescriptors(); - for (TypeSymbol typeSymbol : memberDescriptors) { - if (typeSymbol.getModule().isPresent() && typeSymbol.getModule().get().id().orgName() - .equals(GrpcConstants.BALLERINA_ORG_NAME) && typeSymbol.getModule() - .flatMap(Symbol::getName).orElse("").equals(GrpcConstants.GRPC_PACKAGE_NAME)) { - - return true; - } - } - } else if (listenerType.typeKind() == TypeDescKind.TYPE_REFERENCE - && listenerType.getModule().isPresent() - && listenerType.getModule().get().id().orgName().equals(GrpcConstants.BALLERINA_ORG_NAME) - && ((TypeReferenceTypeSymbol) listenerType).typeDescriptor().getModule() - .flatMap(Symbol::getName).orElse("").equals(GrpcConstants.GRPC_PACKAGE_NAME)) { + List listenerTypes = serviceDeclarationSymbol.listenerTypes(); + for (TypeSymbol listenerType : listenerTypes) { + if (listenerType.typeKind() == TypeDescKind.UNION) { + List memberDescriptors = ((UnionTypeSymbol) listenerType).memberTypeDescriptors(); + for (TypeSymbol typeSymbol : memberDescriptors) { + if (typeSymbol.getModule().isPresent() && typeSymbol.getModule().get().id().orgName() + .equals(GrpcConstants.BALLERINA_ORG_NAME) && typeSymbol.getModule() + .flatMap(Symbol::getName).orElse("").equals(GrpcConstants.GRPC_PACKAGE_NAME)) { - return true; + return true; + } } + } else if (listenerType.typeKind() == TypeDescKind.TYPE_REFERENCE + && listenerType.getModule().isPresent() + && listenerType.getModule().get().id().orgName().equals(GrpcConstants.BALLERINA_ORG_NAME) + && ((TypeReferenceTypeSymbol) listenerType).typeDescriptor().getModule() + .flatMap(Symbol::getName).orElse("").equals(GrpcConstants.GRPC_PACKAGE_NAME)) { + + return true; } } return false; } public void validateServiceAnnotation(ServiceDeclarationNode serviceDeclarationNode, - SyntaxNodeAnalysisContext syntaxNodeAnalysisContext) { + SyntaxNodeAnalysisContext syntaxNodeAnalysisContext, + ServiceDeclarationSymbol serviceDeclarationSymbol) { boolean isServiceDescAnnotationPresents = false; - if (serviceDeclarationNode.metadata().isPresent()) { - NodeList nodeList = serviceDeclarationNode.metadata().get().annotations(); - for (AnnotationNode annotationNode : nodeList) { - if (GrpcConstants.GRPC_ANNOTATION_NAME.equals(annotationNode.annotReference().toString().strip())) { - isServiceDescAnnotationPresents = true; - break; - } + List annotationSymbols = serviceDeclarationSymbol.annotations(); + for (AnnotationSymbol annotationSymbol : annotationSymbols) { + if (annotationSymbol.getModule().isPresent() + && GrpcConstants.GRPC_PACKAGE_NAME.equals(annotationSymbol.getModule().get().id().moduleName()) + && annotationSymbol.getName().isPresent() + && GrpcConstants.GRPC_ANNOTATION_NAME.equals(annotationSymbol.getName().get())) { + + isServiceDescAnnotationPresents = true; + break; } } if (!isServiceDescAnnotationPresents) { reportErrorDiagnostic(serviceDeclarationNode, syntaxNodeAnalysisContext, - (GrpcConstants.UNDEFINED_ANNOTATION_MSG + GrpcConstants.GRPC_ANNOTATION_NAME), - GrpcConstants.UNDEFINED_ANNOTATION_ID); + (GrpcConstants.UNDEFINED_ANNOTATION_MSG + GrpcConstants.GRPC_PACKAGE_NAME + ":" + + GrpcConstants.GRPC_ANNOTATION_NAME), GrpcConstants.UNDEFINED_ANNOTATION_ID); } } From 49ebfef5317c0c49f3123d89eba8c4406254f6b2 Mon Sep 17 00:00:00 2001 From: BuddhiWathsala Date: Thu, 8 Apr 2021 15:21:20 +0530 Subject: [PATCH 2/2] Add tests to cover compiler plugin with package aliases --- .../net/grpc/plugin/CompilerPluginTest.java | 19 ++- .../test-src/package_08/Ballerina.toml | 7 ++ .../package_08/grpc_server_streaming_pb.bal | 119 ++++++++++++++++++ .../grpc_server_streaming_service.bal | 38 ++++++ 4 files changed, 178 insertions(+), 5 deletions(-) create mode 100644 grpc-compiler-plugin-tests/src/test/resources/test-src/package_08/Ballerina.toml create mode 100644 grpc-compiler-plugin-tests/src/test/resources/test-src/package_08/grpc_server_streaming_pb.bal create mode 100644 grpc-compiler-plugin-tests/src/test/resources/test-src/package_08/grpc_server_streaming_service.bal diff --git a/grpc-compiler-plugin-tests/src/test/java/org/ballerinalang/net/grpc/plugin/CompilerPluginTest.java b/grpc-compiler-plugin-tests/src/test/java/org/ballerinalang/net/grpc/plugin/CompilerPluginTest.java index 279676f32..a1ee88646 100644 --- a/grpc-compiler-plugin-tests/src/test/java/org/ballerinalang/net/grpc/plugin/CompilerPluginTest.java +++ b/grpc-compiler-plugin-tests/src/test/java/org/ballerinalang/net/grpc/plugin/CompilerPluginTest.java @@ -74,7 +74,7 @@ public void testCompilerPluginServerStreamingWithCaller() { Package currentPackage = loadPackage("package_03"); PackageCompilation compilation = currentPackage.getCompilation(); String errMsg = "ERROR [grpc_server_streaming_service.bal:(27:4,38:5)] return types are not allowed with " + - "the caller"; + "the caller"; DiagnosticResult diagnosticResult = compilation.diagnosticResult(); Assert.assertEquals(diagnosticResult.diagnostics().size(), 1); Assert.assertTrue(diagnosticResult.diagnostics().stream().anyMatch( @@ -87,7 +87,7 @@ public void testCompilerPluginServerStreamingWithExtraParams() { Package currentPackage = loadPackage("package_04"); PackageCompilation compilation = currentPackage.getCompilation(); String errMsg = "ERROR [grpc_server_streaming_service.bal:(27:4,38:5)] when there are two parameters to " + - "a remote function, the first one must be a caller type"; + "a remote function, the first one must be a caller type"; DiagnosticResult diagnosticResult = compilation.diagnosticResult(); Assert.assertEquals(diagnosticResult.diagnostics().size(), 1); Assert.assertTrue(diagnosticResult.diagnostics().stream().anyMatch( @@ -100,7 +100,7 @@ public void testCompilerPluginServerStreamingWithoutAnnotations() { Package currentPackage = loadPackage("package_05"); PackageCompilation compilation = currentPackage.getCompilation(); String errMsg = "ERROR [grpc_server_streaming_service.bal:(29:0,42:1)] undefined annotation: " + - "grpc:ServiceDescriptor"; + "grpc:ServiceDescriptor"; DiagnosticResult diagnosticResult = compilation.diagnosticResult(); Assert.assertEquals(diagnosticResult.diagnostics().size(), 1); Assert.assertTrue(diagnosticResult.diagnostics().stream().anyMatch( @@ -113,7 +113,7 @@ public void testCompilerPluginServerStreamingWithResourceFunction() { Package currentPackage = loadPackage("package_06"); PackageCompilation compilation = currentPackage.getCompilation(); String errMsg = "ERROR [grpc_server_streaming_service.bal:(40:4,42:5)] only remote functions are " + - "allowed inside gRPC services"; + "allowed inside gRPC services"; DiagnosticResult diagnosticResult = compilation.diagnosticResult(); Assert.assertEquals(diagnosticResult.diagnostics().size(), 1); Assert.assertTrue(diagnosticResult.diagnostics().stream().anyMatch( @@ -126,13 +126,22 @@ public void testCompilerPluginServerStreamingWithInvalidCaller() { Package currentPackage = loadPackage("package_07"); PackageCompilation compilation = currentPackage.getCompilation(); String errMsg = "ERROR [grpc_server_streaming_service.bal:(27:4,39:5)] expected caller type " + - "\"HelloWorldStringCaller\" but found \"CustomCaller\""; + "\"HelloWorldStringCaller\" but found \"CustomCaller\""; DiagnosticResult diagnosticResult = compilation.diagnosticResult(); Assert.assertEquals(diagnosticResult.diagnostics().size(), 1); Assert.assertTrue(diagnosticResult.diagnostics().stream().anyMatch( diagnostic -> errMsg.equals(diagnostic.toString()))); } + @Test + public void testCompilerPluginServerStreamingWithAlias() { + + Package currentPackage = loadPackage("package_08"); + PackageCompilation compilation = currentPackage.getCompilation(); + DiagnosticResult diagnosticResult = compilation.diagnosticResult(); + Assert.assertEquals(diagnosticResult.diagnostics().size(), 0); + } + private Package loadPackage(String path) { Path projectDirPath = RESOURCE_DIRECTORY.resolve(path); diff --git a/grpc-compiler-plugin-tests/src/test/resources/test-src/package_08/Ballerina.toml b/grpc-compiler-plugin-tests/src/test/resources/test-src/package_08/Ballerina.toml new file mode 100644 index 000000000..8d53c43d0 --- /dev/null +++ b/grpc-compiler-plugin-tests/src/test/resources/test-src/package_08/Ballerina.toml @@ -0,0 +1,7 @@ +[package] +org = "grpc_test" +name = "package_02" +version = "0.1.0" + +[build-options] +observabilityIncluded = true diff --git a/grpc-compiler-plugin-tests/src/test/resources/test-src/package_08/grpc_server_streaming_pb.bal b/grpc-compiler-plugin-tests/src/test/resources/test-src/package_08/grpc_server_streaming_pb.bal new file mode 100644 index 000000000..363b172fb --- /dev/null +++ b/grpc-compiler-plugin-tests/src/test/resources/test-src/package_08/grpc_server_streaming_pb.bal @@ -0,0 +1,119 @@ +// Copyright (c) 2021 WSO2 Inc. (http://www.wso2.org) All Rights Reserved. +// +// WSO2 Inc. licenses this file to you 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. +import ballerina/grpc; + +public client class HelloWorldClient { + + *grpc:AbstractClientEndpoint; + + private grpc:Client grpcClient; + + public isolated function init(string url, *grpc:ClientConfiguration config) returns grpc:Error? { + // initialize client endpoint. + self.grpcClient = check new (url, config); + check self.grpcClient.initStub(self, ROOT_DESCRIPTOR, getDescriptorMap()); + } + + isolated remote function lotsOfReplies(string req) returns stream|grpc:Error { + + var payload = check self.grpcClient->executeServerStreaming("HelloWorld/lotsOfReplies", req); + [stream, map] [result, _] = payload; + StringStream outputStream = new StringStream(result); + return new stream(outputStream); + } + + isolated remote function lotsOfRepliesContext(string req) returns ContextStringStream|grpc:Error { + + var payload = check self.grpcClient->executeServerStreaming("HelloWorld/lotsOfReplies", req); + [stream, map] [result, headers] = payload; + StringStream outputStream = new StringStream(result); + return { + content: new stream(outputStream), + headers: headers + }; + } +} + +public class StringStream { + private stream anydataStream; + + public isolated function init(stream anydataStream) { + self.anydataStream = anydataStream; + } + + public isolated function next() returns record {| string value; |}|grpc:Error? { + var streamValue = self.anydataStream.next(); + if (streamValue is ()) { + return streamValue; + } else if (streamValue is grpc:Error) { + return streamValue; + } else { + record {| string value; |} nextRecord = {value: streamValue.value}; + return nextRecord; + } + } + + public isolated function close() returns grpc:Error? { + return self.anydataStream.close(); + } +} + +public client class HelloWorldStringCaller { + private grpc:Caller caller; + + public isolated function init(grpc:Caller caller) { + self.caller = caller; + } + + public isolated function getId() returns int { + return self.caller.getId(); + } + + isolated remote function sendString(string response) returns grpc:Error? { + return self.caller->send(response); + } + isolated remote function sendContextString(ContextString response) returns grpc:Error? { + return self.caller->send(response); + } + + isolated remote function sendError(grpc:Error response) returns grpc:Error? { + return self.caller->sendError(response); + } + + isolated remote function complete() returns grpc:Error? { + return self.caller->complete(); + } +} + +public type ContextStringStream record {| + stream content; + map headers; +|}; + +public type ContextString record {| + string content; + map headers; +|}; + +const string ROOT_DESCRIPTOR = "0A1B677270635F7365727665725F73747265616D696E672E70726F746F1A1E676F6F676C652F70726F746F6275662F77726170706572732E70726F746F325B0A0A48656C6C6F576F726C64124D0A0D6C6F74734F665265706C696573121C2E676F6F676C652E70726F746F6275662E537472696E6756616C75651A1C2E676F6F676C652E70726F746F6275662E537472696E6756616C75653001620670726F746F33"; + +isolated function getDescriptorMap() returns map { + return + { + "grpc_server_streaming.proto": "0A1B677270635F7365727665725F73747265616D696E672E70726F746F1A1E676F6F676C652F70726F746F6275662F77726170706572732E70726F746F325B0A0A48656C6C6F576F726C64124D0A0D6C6F74734F665265706C696573121C2E676F6F676C652E70726F746F6275662E537472696E6756616C75651A1C2E676F6F676C652E70726F746F6275662E537472696E6756616C75653001620670726F746F33", + "google/protobuf/wrappers.proto": "0A1E676F6F676C652F70726F746F6275662F77726170706572732E70726F746F120F676F6F676C652E70726F746F62756622230A0B446F75626C6556616C756512140A0576616C7565180120012801520576616C756522220A0A466C6F617456616C756512140A0576616C7565180120012802520576616C756522220A0A496E74363456616C756512140A0576616C7565180120012803520576616C756522230A0B55496E74363456616C756512140A0576616C7565180120012804520576616C756522220A0A496E74333256616C756512140A0576616C7565180120012805520576616C756522230A0B55496E74333256616C756512140A0576616C756518012001280D520576616C756522210A09426F6F6C56616C756512140A0576616C7565180120012808520576616C756522230A0B537472696E6756616C756512140A0576616C7565180120012809520576616C756522220A0A427974657356616C756512140A0576616C756518012001280C520576616C756542570A13636F6D2E676F6F676C652E70726F746F627566420D577261707065727350726F746F50015A057479706573F80101A20203475042AA021E476F6F676C652E50726F746F6275662E57656C6C4B6E6F776E5479706573620670726F746F33" + }; +} diff --git a/grpc-compiler-plugin-tests/src/test/resources/test-src/package_08/grpc_server_streaming_service.bal b/grpc-compiler-plugin-tests/src/test/resources/test-src/package_08/grpc_server_streaming_service.bal new file mode 100644 index 000000000..6570df444 --- /dev/null +++ b/grpc-compiler-plugin-tests/src/test/resources/test-src/package_08/grpc_server_streaming_service.bal @@ -0,0 +1,38 @@ +// Copyright (c) 2021 WSO2 Inc. (http://www.wso2.org) All Rights Reserved. +// +// WSO2 Inc. licenses this file to you 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. +// This is the server implementation of the server streaming scenario. +import ballerina/grpc as gc; +import ballerina/log; + + +@gc:ServiceDescriptor { + descriptor: ROOT_DESCRIPTOR, + descMap: getDescriptorMap() +} +service "HelloWorld" on new gc:Listener(9090) { + remote function lotsOfReplies(string name) returns stream|error { + log:printInfo("Server received hello from " + name); + string[] greets = ["Hi", "Hey", "GM"]; + // Create the array of responses by appending the received name. + int i = 0; + foreach string greet in greets { + greets[i] = greet + " " + name; + i += 1; + } + // Return the stream of strings back to the client. + return greets.toStream(); + } +}