From 51fdd9dc5927ef72f4f43efa12aa917f1856dbf5 Mon Sep 17 00:00:00 2001 From: Thevakumar-Luheerathan Date: Thu, 13 Oct 2022 15:08:21 +0530 Subject: [PATCH] Modify based on review --- ballerina-tests/tests/main.bal | 32 ++++++------ .../tests/resources/httpHeaderTest.json | 4 +- ballerina-tests/tests/test.bal | 10 ++-- .../test/ProjectValidationTests.java | 13 +++++ .../http-service-config/Ballerina.toml | 13 +++++ .../validations/http-service-config/main.bal | 27 ++++++++++ .../azurefunctions/AzureDiagnosticCodes.java | 5 +- .../AzureFunctionServiceVisitor.java | 1 - .../ballerinax/azurefunctions/Constants.java | 5 ++ .../validators/HttpListenerValidator.java | 50 +++++++++++++++++-- .../stdlib/azure/functions/HttpResource.java | 1 - 11 files changed, 131 insertions(+), 30 deletions(-) create mode 100644 compiler-plugin-tests/src/test/resources/validations/http-service-config/Ballerina.toml create mode 100644 compiler-plugin-tests/src/test/resources/validations/http-service-config/main.bal diff --git a/ballerina-tests/tests/main.bal b/ballerina-tests/tests/main.bal index 33ee6b15..4a6809ca 100644 --- a/ballerina-tests/tests/main.bal +++ b/ballerina-tests/tests/main.bal @@ -36,22 +36,22 @@ listener af:HttpListener ep2 = new (); treatNilableAsOptional: false } service /httpHeader on ep2 { - resource function post nonTreatNilAsOpt\-Nil\-noHeaderTest(@http:Header string? Hoste) returns string? { - return Hoste; + resource function post nonTreatNilAsOpt\-Nil\-noHeaderTest(@http:Header string? hoste) returns string? { + return hoste; } - resource function post nonTreatNilAsOpt\-nonNil\-noHeaderTest(@http:Header string Hoste) returns string { - return Hoste; + resource function post nonTreatNilAsOpt\-nonNil\-noHeaderTest(@http:Header string hoste) returns string { + return hoste; } - resource function post nonTreatNilAsOpt\-nonNil\-HeaderTest(@http:Header string Hos) returns string { - return Hos; + resource function post nonTreatNilAsOpt\-nonNil\-HeaderTest(@http:Header string hos) returns string { + return hos; } - resource function post nonTreatNilAsOpt\-Nil\-HeaderTest(@http:Header string? Hos) returns string? { - return Hos; + resource function post nonTreatNilAsOpt\-Nil\-HeaderTest(@http:Header string? hos) returns string? { + return hos; } } @@ -97,13 +97,13 @@ service /httpHeader on ep3 { } - resource function post treatNilAsOpt\-nonNil\-noHeaderTest(@http:Header string Hoste) returns string { - return Hoste; + resource function post treatNilAsOpt\-nonNil\-noHeaderTest(@http:Header string hoste) returns string { + return hoste; } - resource function post treatNilAsOpt\-nonNil\-HeaderTest(@http:Header string Hos) returns string { - return Hos; + resource function post treatNilAsOpt\-nonNil\-HeaderTest(@http:Header string hos) returns string { + return hos; } @@ -112,13 +112,13 @@ service /httpHeader on ep3 { } - resource function post treatNilAsOpt\-Nil\-noHeaderTest(@http:Header string? Hoste) returns string? { - return Hoste; + resource function post treatNilAsOpt\-Nil\-noHeaderTest(@http:Header string? hoste) returns string? { + return hoste; } - resource function post treatNilAsOpt\-Nil\-HeaderTest(@http:Header string? Hos) returns string? { - return Hos; + resource function post treatNilAsOpt\-Nil\-HeaderTest(@http:Header string? hos) returns string? { + return hos; } } diff --git a/ballerina-tests/tests/resources/httpHeaderTest.json b/ballerina-tests/tests/resources/httpHeaderTest.json index 2e6ae99b..7992b0b8 100644 --- a/ballerina-tests/tests/resources/httpHeaderTest.json +++ b/ballerina-tests/tests/resources/httpHeaderTest.json @@ -25,7 +25,7 @@ "test":[ "12, 13, 14" ], - "Hos":[ + "hos":[ "" ], "X-ARR-LOG-ID":[ @@ -97,7 +97,7 @@ "Max-Forwards":"10", "User-Agent":"ballerina", "test":"12, 13, 14", - "Hos": "", + "hos": "", "X-ARR-LOG-ID":"77ef8e37-520e-417b-a972-30746b719b60", "CLIENT-IP":"45.121.88.92:53880", "DISGUISED-HOST":"az-func-http-test.azurewebsites.net", diff --git a/ballerina-tests/tests/test.bal b/ballerina-tests/tests/test.bal index 50f4982d..108918a8 100644 --- a/ballerina-tests/tests/test.bal +++ b/ballerina-tests/tests/test.bal @@ -96,7 +96,7 @@ function nnonTreatNilAsOpt\-Nil\-noHeaderTest() returns error? { string replacedString = regex:replaceAll(readString, "(FUNC_NAME)", "nonTreatNilAsOpt-Nil-noHeaderTest"); json readJson = check value:fromJsonString(replacedString); json resp = check clientEndpoint->post("/post-httpHeader-nonTreatNilAsOpt-Nil-noHeaderTest", readJson); - json expectedResp = {"Outputs":{"resp":{"statusCode":400, "body":"no header value found for 'Hoste'", "headers":{"Content-Type":"text/plain"}}}, "Logs":[], "ReturnValue":null}; + json expectedResp = {"Outputs":{"resp":{"statusCode":400, "body":"no header value found for 'hoste'", "headers":{"Content-Type":"text/plain"}}}, "Logs":[], "ReturnValue":null}; test:assertEquals(resp, expectedResp); } @@ -108,7 +108,7 @@ function treatNilAsOpt\-nonNil\-noHeaderTest() returns error? { string replacedString = regex:replaceAll(readString, "(FUNC_NAME)", "treatNilAsOpt-nonNil-noHeaderTest"); json readJson = check value:fromJsonString(replacedString); json resp = check clientEndpoint->post("/post-httpHeader-treatNilAsOpt-nonNil-noHeaderTest", readJson); - json expectedResp = {"Outputs":{"resp":{"statusCode":400, "body":"no header value found for 'Hoste'", "headers":{"Content-Type":"text/plain"}}}, "Logs":[], "ReturnValue":null}; + json expectedResp = {"Outputs":{"resp":{"statusCode":400, "body":"no header value found for 'hoste'", "headers":{"Content-Type":"text/plain"}}}, "Logs":[], "ReturnValue":null}; test:assertEquals(resp, expectedResp); } @@ -120,7 +120,7 @@ function treatNilAsOpt\-nonNil\-HeaderTest() returns error? { string replacedString = regex:replaceAll(readString, "(FUNC_NAME)", "treatNilAsOpt-nonNil-HeaderTest"); json readJson = check value:fromJsonString(replacedString); json resp = check clientEndpoint->post("/post-httpHeader-treatNilAsOpt-nonNil-HeaderTest", readJson); - json expectedResp = {"Outputs":{"resp":{"statusCode":400, "body":"no header value found for 'Hos'", "headers":{"Content-Type":"text/plain"}}}, "Logs":[], "ReturnValue":null}; + json expectedResp = {"Outputs":{"resp":{"statusCode":400, "body":"no header value found for 'hos'", "headers":{"Content-Type":"text/plain"}}}, "Logs":[], "ReturnValue":null}; test:assertEquals(resp, expectedResp); } @@ -168,7 +168,7 @@ function nonTreatNilAsOpt\-nonNil\-noHeaderTest() returns error? { string replacedString = regex:replaceAll(readString, "(FUNC_NAME)", "nonTreatNilAsOpt-nonNil-noHeaderTest"); json readJson = check value:fromJsonString(replacedString); json resp = check clientEndpoint->post("/post-httpHeader-nonTreatNilAsOpt-nonNil-noHeaderTest", readJson); - json expectedResp = {"Outputs":{"resp":{"statusCode":400, "body":"no header value found for 'Hoste'", "headers":{"Content-Type":"text/plain"}}}, "Logs":[], "ReturnValue":null}; + json expectedResp = {"Outputs":{"resp":{"statusCode":400, "body":"no header value found for 'hoste'", "headers":{"Content-Type":"text/plain"}}}, "Logs":[], "ReturnValue":null}; test:assertEquals(resp, expectedResp); } @@ -180,7 +180,7 @@ function nonTreatNilAsOpt\-nonNil\-HeaderTest() returns error? { string replacedString = regex:replaceAll(readString, "(FUNC_NAME)", "nonTreatNilAsOpt-nonNil-HeaderTest"); json readJson = check value:fromJsonString(replacedString); json resp = check clientEndpoint->post("/post-httpHeader-nonTreatNilAsOpt-nonNil-HeaderTest", readJson); - json expectedResp = {"Outputs":{"resp":{"statusCode":400, "body":"no header value found for 'Hos'", "headers":{"Content-Type":"text/plain"}}}, "Logs":[], "ReturnValue":null}; + json expectedResp = {"Outputs":{"resp":{"statusCode":400, "body":"no header value found for 'hos'", "headers":{"Content-Type":"text/plain"}}}, "Logs":[], "ReturnValue":null}; test:assertEquals(resp, expectedResp); } diff --git a/compiler-plugin-tests/src/test/java/org/ballerinax/azurefunctions/test/ProjectValidationTests.java b/compiler-plugin-tests/src/test/java/org/ballerinax/azurefunctions/test/ProjectValidationTests.java index 1a764cdd..ba855b4d 100644 --- a/compiler-plugin-tests/src/test/java/org/ballerinax/azurefunctions/test/ProjectValidationTests.java +++ b/compiler-plugin-tests/src/test/java/org/ballerinax/azurefunctions/test/ProjectValidationTests.java @@ -91,4 +91,17 @@ public void headerAnnotationTest() { Assert.assertEquals(((Diagnostic) diagnostics[11]).diagnosticInfo().messageFormat(), diagnosticMessage11); Assert.assertEquals(((Diagnostic) diagnostics[12]).diagnosticInfo().messageFormat(), diagnosticMessage12); } + + @Test + public void httpServiceConfigTest() { + BuildProject project = BuildProject.load(RESOURCE_DIRECTORY.resolve("http-service-config")); + PackageCompilation compilation = project.currentPackage().getCompilation(); + DiagnosticResult diagnosticResult = compilation.diagnosticResult(); + Object[] diagnostics = diagnosticResult.warnings().toArray(); + Assert.assertEquals(diagnosticResult.warningCount(), 2); + String diagnosticMessage = "'treatNilableAsOptional' is the only @http:serviceConfig " + + "field supported by Azure Function at the moment"; + Assert.assertEquals(((Diagnostic) diagnostics[0]).diagnosticInfo().messageFormat(), diagnosticMessage); + Assert.assertEquals(((Diagnostic) diagnostics[1]).diagnosticInfo().messageFormat(), diagnosticMessage); + } } diff --git a/compiler-plugin-tests/src/test/resources/validations/http-service-config/Ballerina.toml b/compiler-plugin-tests/src/test/resources/validations/http-service-config/Ballerina.toml new file mode 100644 index 00000000..e9f5cff9 --- /dev/null +++ b/compiler-plugin-tests/src/test/resources/validations/http-service-config/Ballerina.toml @@ -0,0 +1,13 @@ +[package] +org = "luheerathan" +name = "http_header_annotation" +version = "0.1.0" +distribution = "2201.1.0" + +[build-options] +observabilityIncluded = true + +[[dependency]] +org = "ballerinax" +name = "azure_functions" +version = "3.0.0-alpha.1" diff --git a/compiler-plugin-tests/src/test/resources/validations/http-service-config/main.bal b/compiler-plugin-tests/src/test/resources/validations/http-service-config/main.bal new file mode 100644 index 00000000..bd96d751 --- /dev/null +++ b/compiler-plugin-tests/src/test/resources/validations/http-service-config/main.bal @@ -0,0 +1,27 @@ +import ballerinax/azure_functions as af; +import ballerina/http; + +listener af:HttpListener ep1 = new (); +@http:ServiceConfig{treatNilableAsOptional: true, host : "b7a.default"} +service "hello" on ep1 { + resource function get test1(@http:Header string? hoste) returns string? { + return hoste; + } +} + +listener af:HttpListener ep2 = new (); +@http:ServiceConfig{host : "b7a.default"} +service "hello" on ep2 { + resource function get test1(@http:Header string? hoste) returns string? { + return hoste; + } +} + +listener af:HttpListener ep3 = new (); +@http:ServiceConfig{treatNilableAsOptional: true} +service "hello" on ep3 { + resource function get test1(@http:Header string? hoste) returns string? { + return hoste; + } +} + diff --git a/compiler-plugin/src/main/java/org/ballerinax/azurefunctions/AzureDiagnosticCodes.java b/compiler-plugin/src/main/java/org/ballerinax/azurefunctions/AzureDiagnosticCodes.java index 270b18f2..290a5540 100644 --- a/compiler-plugin/src/main/java/org/ballerinax/azurefunctions/AzureDiagnosticCodes.java +++ b/compiler-plugin/src/main/java/org/ballerinax/azurefunctions/AzureDiagnosticCodes.java @@ -21,6 +21,7 @@ import io.ballerina.tools.diagnostics.DiagnosticSeverity; import static io.ballerina.tools.diagnostics.DiagnosticSeverity.ERROR; +import static io.ballerina.tools.diagnostics.DiagnosticSeverity.WARNING; /** * {@code DiagnosticCodes} is used to hold diagnostic codes. @@ -37,7 +38,9 @@ public enum AzureDiagnosticCodes { AF_005("AF_005", "invalid intersection type : '%s'. Only readonly type is allowed", ERROR), AF_006("AF_006", "rest fields are not allowed for header binding records. Use 'http:Headers' type to access " + "all headers", ERROR), - AF_007("AF_007", "invalid multiple resource parameter annotations for '%s'", ERROR); + AF_007("AF_007", "invalid multiple resource parameter annotations for '%s'", ERROR), + AF_008("AF_008", "'treatNilableAsOptional' is the only @http:serviceConfig field supported by Azure " + + "Function at the moment", WARNING); private final String code; private final String message; diff --git a/compiler-plugin/src/main/java/org/ballerinax/azurefunctions/AzureFunctionServiceVisitor.java b/compiler-plugin/src/main/java/org/ballerinax/azurefunctions/AzureFunctionServiceVisitor.java index 99f87fed..691b5a4a 100644 --- a/compiler-plugin/src/main/java/org/ballerinax/azurefunctions/AzureFunctionServiceVisitor.java +++ b/compiler-plugin/src/main/java/org/ballerinax/azurefunctions/AzureFunctionServiceVisitor.java @@ -18,7 +18,6 @@ package org.ballerinax.azurefunctions; import io.ballerina.compiler.api.SemanticModel; -import io.ballerina.compiler.syntax.tree.ModulePartNode; import io.ballerina.compiler.syntax.tree.NodeVisitor; import io.ballerina.compiler.syntax.tree.ServiceDeclarationNode; import org.ballerinax.azurefunctions.service.ServiceHandler; diff --git a/compiler-plugin/src/main/java/org/ballerinax/azurefunctions/Constants.java b/compiler-plugin/src/main/java/org/ballerinax/azurefunctions/Constants.java index abb771d4..6093b57e 100644 --- a/compiler-plugin/src/main/java/org/ballerinax/azurefunctions/Constants.java +++ b/compiler-plugin/src/main/java/org/ballerinax/azurefunctions/Constants.java @@ -23,6 +23,8 @@ public class Constants { public static final String AZURE_FUNCTIONS_MODULE_NAME = "azure_functions"; + public static final String COLON = ":"; + public static final String HTTP = "http"; public static final String HEADER_ANNOTATION_TYPE = "HttpHeader"; public static final String AZURE_FUNCTIONS_PACKAGE_ORG = "ballerinax"; @@ -64,4 +66,7 @@ public class Constants { public static final String TASKS_FILE_NAME = "tasks.json"; public static final String FUNCTION_ANNOTATION = "Function"; + public static final String SERVICE_CONFIG_ANNOTATION = "ServiceConfig"; + public static final String TREAT_NILABLE_AS_OPTIONAL = "treatNilableAsOptional"; + } diff --git a/compiler-plugin/src/main/java/org/ballerinax/azurefunctions/validators/HttpListenerValidator.java b/compiler-plugin/src/main/java/org/ballerinax/azurefunctions/validators/HttpListenerValidator.java index f86ae0b4..183998b5 100644 --- a/compiler-plugin/src/main/java/org/ballerinax/azurefunctions/validators/HttpListenerValidator.java +++ b/compiler-plugin/src/main/java/org/ballerinax/azurefunctions/validators/HttpListenerValidator.java @@ -31,12 +31,19 @@ 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.FunctionDefinitionNode; +import io.ballerina.compiler.syntax.tree.IdentifierToken; +import io.ballerina.compiler.syntax.tree.MappingConstructorExpressionNode; +import io.ballerina.compiler.syntax.tree.MappingFieldNode; +import io.ballerina.compiler.syntax.tree.MetadataNode; import io.ballerina.compiler.syntax.tree.Node; import io.ballerina.compiler.syntax.tree.NodeList; import io.ballerina.compiler.syntax.tree.ServiceDeclarationNode; import io.ballerina.compiler.syntax.tree.SyntaxKind; import io.ballerina.projects.plugins.SyntaxNodeAnalysisContext; +import io.ballerina.tools.diagnostics.DiagnosticFactory; +import io.ballerina.tools.diagnostics.DiagnosticInfo; import io.ballerina.tools.diagnostics.Location; import org.ballerinax.azurefunctions.AzureDiagnosticCodes; import org.wso2.ballerinalang.compiler.diagnostic.properties.BSymbolicProperty; @@ -47,9 +54,13 @@ import java.util.Optional; import java.util.stream.Collectors; +import static org.ballerinax.azurefunctions.AzureDiagnosticCodes.AF_008; import static org.ballerinax.azurefunctions.Constants.AZURE_FUNCTIONS_MODULE_NAME; +import static org.ballerinax.azurefunctions.Constants.COLON; import static org.ballerinax.azurefunctions.Constants.HEADER_ANNOTATION_TYPE; import static org.ballerinax.azurefunctions.Constants.HTTP; +import static org.ballerinax.azurefunctions.Constants.SERVICE_CONFIG_ANNOTATION; +import static org.ballerinax.azurefunctions.Constants.TREAT_NILABLE_AS_OPTIONAL; import static org.ballerinax.azurefunctions.Util.updateDiagnostic; /** @@ -76,7 +87,36 @@ private static void validateResourceFunction(SyntaxNodeAnalysisContext ctx, Func private static void extractServiceAnnotationAndValidate(SyntaxNodeAnalysisContext syntaxNodeAnalysisContext, ServiceDeclarationNode serviceDeclarationNode) { - //TODO : Validate service annotation fields + //HTTP serviceconfig validation currently supports only for treatNilableAsTrue field + Optional metadataNodeOptional = serviceDeclarationNode.metadata(); + if (metadataNodeOptional.isEmpty()) { + return; + } + NodeList annotations = metadataNodeOptional.get().annotations(); + for (AnnotationNode annotation : annotations) { + Node annotReference = annotation.annotReference(); + String annotName = annotReference.toString(); + Optional annotValue = annotation.annotValue(); + if (annotReference.kind() != SyntaxKind.QUALIFIED_NAME_REFERENCE) { + continue; + } + String[] annotStrings = annotName.split(COLON); + if (SERVICE_CONFIG_ANNOTATION.equals(annotStrings[annotStrings.length - 1].trim()) + && (annotValue.isPresent())) { + MappingConstructorExpressionNode mapping = annotValue.get(); + NodeList fields = mapping.fields(); + if (fields.size() == 1) { + MappingFieldNode field = (MappingFieldNode) fields.get(0); + String fieldName = ((IdentifierToken) (field.children()).get(0)).text(); + if (!TREAT_NILABLE_AS_OPTIONAL.equals(fieldName)) { + warnInvalidServiceConfig(syntaxNodeAnalysisContext, field); + } + } else { + warnInvalidServiceConfig(syntaxNodeAnalysisContext, mapping); + } + } + } + } private static void validateInputParameters(SyntaxNodeAnalysisContext ctx, FunctionDefinitionNode member) { @@ -90,9 +130,6 @@ private static void validateInputParameters(SyntaxNodeAnalysisContext ctx, Funct if (parametersOptional.isEmpty()) { return; } - if (parametersOptional.get().size() == 0) { - return; - } for (ParameterSymbol param : parametersOptional.get()) { Optional paramLocationOptional = param.getLocation(); @@ -315,6 +352,11 @@ private static void reportInvalidMultipleAnnotation(SyntaxNodeAnalysisContext ct String paramName) { updateDiagnostic(ctx, location, AzureDiagnosticCodes.AF_007, paramName); } + + private static void warnInvalidServiceConfig(SyntaxNodeAnalysisContext ctx, Node node) { + DiagnosticInfo diagInfo = new DiagnosticInfo(AF_008.getCode(), AF_008.getMessage(), AF_008.getSeverity()); + ctx.reportDiagnostic(DiagnosticFactory.createDiagnostic(diagInfo, node.location())); + } } diff --git a/native/src/main/java/io/ballerina/stdlib/azure/functions/HttpResource.java b/native/src/main/java/io/ballerina/stdlib/azure/functions/HttpResource.java index a92fc88f..05e9eeb2 100644 --- a/native/src/main/java/io/ballerina/stdlib/azure/functions/HttpResource.java +++ b/native/src/main/java/io/ballerina/stdlib/azure/functions/HttpResource.java @@ -212,7 +212,6 @@ private Optional processHeaderParam(ResourceMethodType resource return Optional.of(new HeaderParameter(i, parameter, headerParam)); } else { throw new RuntimeException("Header annotation can have only one name field."); - //TODO :- add proper exception } } return Optional.empty();