From 45adb7c74983ac74ec21e2eecd499c6828fc3113 Mon Sep 17 00:00:00 2001 From: lnash94 Date: Thu, 19 Sep 2024 21:51:36 +0530 Subject: [PATCH 1/5] Add query name overwrite support for the http service --- ...e_dispatching_query_param_binding_test.bal | 57 +++++++++++++++++++ .../api/service/signature/ParamHandler.java | 23 +++++++- 2 files changed, 78 insertions(+), 2 deletions(-) diff --git a/ballerina-tests/http-dispatching-tests/tests/service_dispatching_query_param_binding_test.bal b/ballerina-tests/http-dispatching-tests/tests/service_dispatching_query_param_binding_test.bal index 1ce2f6b41f..98b23e2343 100644 --- a/ballerina-tests/http-dispatching-tests/tests/service_dispatching_query_param_binding_test.bal +++ b/ballerina-tests/http-dispatching-tests/tests/service_dispatching_query_param_binding_test.bal @@ -18,6 +18,7 @@ import ballerina/http; import ballerina/test; import ballerina/url; import ballerina/http_test_common as common; +import ballerina/constraint; listener http:Listener QueryBindingEP = new (queryParamBindingTestPort, httpVersion = http:HTTP_1_1); final http:Client queryBindingClient = check new ("http://localhost:" + queryParamBindingTestPort.toString(), httpVersion = http:HTTP_1_1); @@ -167,8 +168,39 @@ service /queryparamservice on QueryBindingEP { resource function get studentRest(StudentRest studentRest) returns StudentRest { return studentRest; } + + resource function get queryAnnotation(@http:Query{ name: "first"} string firstName, @http:Query{ name: "last-name"} string lastName) returns string { + return string `Hello, ${firstName} ${lastName}`; + } + + resource function get queryAnnotation/negative/q1(@http:Query{ name: ""} string firstName) returns string { + return string `Hello, ${firstName}`; + } + + resource function get queryAnnotation/negative/q2(@http:Query{ name: ()} string lastName) returns string { + return string `Hello, ${lastName}`; + } + + resource function get queryAnnotation/mapQueries(Mq mq) returns string { + return string `Hello, ${mq.name} ${mq.personAge}`; + } } +public type Mq record { + string name; + // @http:Query { + // name: "age" + // } + @constraint:Int { + maxValue: 100 + } + int personAge; + // @http:Query { + // name: "table" + // } + int tableNo; +}; + service /default on QueryBindingEP { resource function get checkstring(string foo = "hello") returns json { json responseJson = {value1: foo}; @@ -736,3 +768,28 @@ function testMapOfJsonTypedQueryParamBinding3() returns error? { response = check queryBindingClient->get("/queryparamservice/q13?obj=" + mapOfJsonsEncoded); test:assertEquals(response.statusCode, 400, msg = "Found unexpected output"); } + +@test:Config{} +function testforQueryParamterNameOverwrite() returns error? { + string result = check queryBindingClient->get("/queryparamservice/queryAnnotation?first=Harry&last-name=Potter"); + test:assertEquals(result, "Hello, Harry Potter", msg = string `Found ${result}, expected Harry`); + + // map mapOfJsons = { + // name: "Ron", + // age: 10, + // 'table: 5 + // }; + // string mapOfQueries = check url:encode(mapOfJsons.toJsonString(), "UTF-8"); + + // result = check queryBindingClient->get("/queryparamservice/queryAnnotation/mapQueries?mq=" + mapOfQueries); + // test:assertEquals(result, "Hello, Ron 10", msg = string `Found ${result}, expected Harry`); +} + +@test:Config{} +function testforNegativeQueryParamterNameOverwrite() returns error? { + string result = check queryBindingClient->get("/queryparamservice/queryAnnotation/negative/q1?firstName=Harry"); + test:assertEquals(result, "Hello, Harry"); + + result = check queryBindingClient->get("/queryparamservice/queryAnnotation/negative/q2?lastName=Anne"); + test:assertEquals(result, "Hello, Anne"); +} \ No newline at end of file diff --git a/native/src/main/java/io/ballerina/stdlib/http/api/service/signature/ParamHandler.java b/native/src/main/java/io/ballerina/stdlib/http/api/service/signature/ParamHandler.java index 2bca7fdd5d..514ee11c6d 100644 --- a/native/src/main/java/io/ballerina/stdlib/http/api/service/signature/ParamHandler.java +++ b/native/src/main/java/io/ballerina/stdlib/http/api/service/signature/ParamHandler.java @@ -47,6 +47,7 @@ import static io.ballerina.stdlib.http.api.HttpConstants.ANN_NAME_CALLER_INFO; import static io.ballerina.stdlib.http.api.HttpConstants.ANN_NAME_HEADER; import static io.ballerina.stdlib.http.api.HttpConstants.ANN_NAME_PAYLOAD; +import static io.ballerina.stdlib.http.api.HttpConstants.ANN_NAME_QUERY; import static io.ballerina.stdlib.http.api.HttpConstants.COLON; import static io.ballerina.stdlib.http.api.HttpConstants.PROTOCOL_HTTP; import static io.ballerina.stdlib.http.api.HttpUtil.getParameterTypes; @@ -89,6 +90,7 @@ public class ParamHandler { + ANN_NAME_CALLER_INFO; public static final String CACHE_ANNOTATION = ModuleUtils.getHttpPackageIdentifier() + COLON + ANN_NAME_CACHE; + public static final String QUERY_ANNOTATION = ModuleUtils.getHttpPackageIdentifier() + COLON + ANN_NAME_QUERY; public ParamHandler(ResourceMethodType resource, int pathParamCount, boolean constraintValidation) { this.resource = resource; @@ -272,11 +274,28 @@ private void createHeaderParam(String paramName, BMap annotations) { private void createQueryParam(int index, ResourceMethodType balResource, Type originalType) { io.ballerina.runtime.api.types.Parameter parameter = balResource.getParameters()[index]; - QueryParam queryParam = new QueryParam(originalType, HttpUtil.unescapeAndEncodeValue(parameter.name), index, - parameter.isDefault, constraintValidation); + String paramName = parameter.name; + BMap annotations = (BMap) balResource.getAnnotation( + StringUtils.fromString(PARAM_ANNOT_PREFIX + IdentifierUtils.escapeSpecialCharacters(paramName))); + if (annotations != null) { + String queryParamName = getQueryParamName(paramName, annotations); + paramName = queryParamName.isBlank() ? paramName : queryParamName; + } + paramName = HttpUtil.unescapeAndEncodeValue(paramName); + QueryParam queryParam = new QueryParam(originalType, paramName, index, parameter.isDefault, + constraintValidation); this.queryParams.add(queryParam); } + private static String getQueryParamName(String paramName, BMap annotations) { + BMap mapValue = annotations.getMapValue(StringUtils.fromString(QUERY_ANNOTATION)); + Object queryName = mapValue.get(HttpConstants.ANN_FIELD_NAME); + if (queryName instanceof BString query) { + paramName = query.getValue(); + } + return paramName; + } + public boolean isPayloadBindingRequired() { return payloadParam != null; } From 0dd605dbf175c14701bac6177298ab9fa69bb6fe Mon Sep 17 00:00:00 2001 From: lnash94 Date: Thu, 19 Sep 2024 21:56:04 +0530 Subject: [PATCH 2/5] Update change log --- changelog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/changelog.md b/changelog.md index 83dd8125fb..e86f171661 100644 --- a/changelog.md +++ b/changelog.md @@ -11,6 +11,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - [Add `anydata` support for `setPayload` methods in the request and response objects](https://github.com/ballerina-platform/ballerina-library/issues/6954) - [Improve `@http:Query` annotation to overwrite the query parameter name in client] (https://github.com/ballerina-platform/ballerina-library/issues/6983) +- [Improve `@http:Query` annotation to overwrite the query parameter name in service] (https://github.com/ballerina-platform/ballerina-library/issues/7006) ## [2.12.0] - 2024-08-20 From 8f73f3e945d7a688e47ace311855b2dafe2cbc10 Mon Sep 17 00:00:00 2001 From: lnash94 Date: Fri, 20 Sep 2024 10:04:02 +0530 Subject: [PATCH 3/5] Update the tests --- ...e_dispatching_query_param_binding_test.bal | 34 ++++++------------- 1 file changed, 11 insertions(+), 23 deletions(-) diff --git a/ballerina-tests/http-dispatching-tests/tests/service_dispatching_query_param_binding_test.bal b/ballerina-tests/http-dispatching-tests/tests/service_dispatching_query_param_binding_test.bal index 98b23e2343..894d71ca80 100644 --- a/ballerina-tests/http-dispatching-tests/tests/service_dispatching_query_param_binding_test.bal +++ b/ballerina-tests/http-dispatching-tests/tests/service_dispatching_query_param_binding_test.bal @@ -18,7 +18,6 @@ import ballerina/http; import ballerina/test; import ballerina/url; import ballerina/http_test_common as common; -import ballerina/constraint; listener http:Listener QueryBindingEP = new (queryParamBindingTestPort, httpVersion = http:HTTP_1_1); final http:Client queryBindingClient = check new ("http://localhost:" + queryParamBindingTestPort.toString(), httpVersion = http:HTTP_1_1); @@ -181,24 +180,14 @@ service /queryparamservice on QueryBindingEP { return string `Hello, ${lastName}`; } - resource function get queryAnnotation/mapQueries(Mq mq) returns string { - return string `Hello, ${mq.name} ${mq.personAge}`; + resource function get queryAnnotation/mapQueries(@http:Query{ name: "rMq"} Mq mq) returns string { + return string `Hello, ${mq.name} ${mq.age}`; } } public type Mq record { string name; - // @http:Query { - // name: "age" - // } - @constraint:Int { - maxValue: 100 - } - int personAge; - // @http:Query { - // name: "table" - // } - int tableNo; + int age; }; service /default on QueryBindingEP { @@ -774,15 +763,14 @@ function testforQueryParamterNameOverwrite() returns error? { string result = check queryBindingClient->get("/queryparamservice/queryAnnotation?first=Harry&last-name=Potter"); test:assertEquals(result, "Hello, Harry Potter", msg = string `Found ${result}, expected Harry`); - // map mapOfJsons = { - // name: "Ron", - // age: 10, - // 'table: 5 - // }; - // string mapOfQueries = check url:encode(mapOfJsons.toJsonString(), "UTF-8"); + map mapOfJsons = { + name: "Ron", + age: 10 + }; + string mapOfQueries = check url:encode(mapOfJsons.toJsonString(), "UTF-8"); - // result = check queryBindingClient->get("/queryparamservice/queryAnnotation/mapQueries?mq=" + mapOfQueries); - // test:assertEquals(result, "Hello, Ron 10", msg = string `Found ${result}, expected Harry`); + result = check queryBindingClient->get("/queryparamservice/queryAnnotation/mapQueries?rMq=" + mapOfQueries); + test:assertEquals(result, "Hello, Ron 10", msg = string `Found ${result}, expected Harry`); } @test:Config{} @@ -792,4 +780,4 @@ function testforNegativeQueryParamterNameOverwrite() returns error? { result = check queryBindingClient->get("/queryparamservice/queryAnnotation/negative/q2?lastName=Anne"); test:assertEquals(result, "Hello, Anne"); -} \ No newline at end of file +} From f40ab3bdd0f4795f044f9ed9eadc60254c33b16b Mon Sep 17 00:00:00 2001 From: lnash94 Date: Fri, 20 Sep 2024 10:47:40 +0530 Subject: [PATCH 4/5] Fix format issue --- ...e_dispatching_query_param_binding_test.bal | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/ballerina-tests/http-dispatching-tests/tests/service_dispatching_query_param_binding_test.bal b/ballerina-tests/http-dispatching-tests/tests/service_dispatching_query_param_binding_test.bal index 894d71ca80..f3b97520d6 100644 --- a/ballerina-tests/http-dispatching-tests/tests/service_dispatching_query_param_binding_test.bal +++ b/ballerina-tests/http-dispatching-tests/tests/service_dispatching_query_param_binding_test.bal @@ -15,9 +15,9 @@ // under the License. import ballerina/http; +import ballerina/http_test_common as common; import ballerina/test; import ballerina/url; -import ballerina/http_test_common as common; listener http:Listener QueryBindingEP = new (queryParamBindingTestPort, httpVersion = http:HTTP_1_1); final http:Client queryBindingClient = check new ("http://localhost:" + queryParamBindingTestPort.toString(), httpVersion = http:HTTP_1_1); @@ -168,19 +168,19 @@ service /queryparamservice on QueryBindingEP { return studentRest; } - resource function get queryAnnotation(@http:Query{ name: "first"} string firstName, @http:Query{ name: "last-name"} string lastName) returns string { + resource function get queryAnnotation(@http:Query {name: "first"} string firstName, @http:Query {name: "last-name"} string lastName) returns string { return string `Hello, ${firstName} ${lastName}`; } - resource function get queryAnnotation/negative/q1(@http:Query{ name: ""} string firstName) returns string { + resource function get queryAnnotation/negative/q1(@http:Query {name: ""} string firstName) returns string { return string `Hello, ${firstName}`; } - resource function get queryAnnotation/negative/q2(@http:Query{ name: ()} string lastName) returns string { + resource function get queryAnnotation/negative/q2(@http:Query {name: ()} string lastName) returns string { return string `Hello, ${lastName}`; } - resource function get queryAnnotation/mapQueries(@http:Query{ name: "rMq"} Mq mq) returns string { + resource function get queryAnnotation/mapQueries(@http:Query {name: "rMq"} Mq mq) returns string { return string `Hello, ${mq.name} ${mq.age}`; } } @@ -343,11 +343,11 @@ function testMapJsonOfDefaultableQueryBinding() returns error? { function testMapJsonArrOfDefaultableQueryBinding() returns error? { json response = check queryBindingClient->get("/default/q4"); common:assertJsonPayloadtoJsonString(response, { - objects: [ - {name: "test1", value: "json1"}, - {name: "test2", value: "json2"} - ] - }); + objects: [ + {name: "test1", value: "json1"}, + {name: "test2", value: "json2"} + ] + }); } @test:Config {} @@ -374,7 +374,7 @@ function testNegativeStringQueryBindingCaseSensitivity() returns error? { if response is http:Response { test:assertEquals(response.statusCode, 400); check common:assertJsonErrorPayload(check response.getJsonPayload(), "no query param value found for 'foo'", - "Bad Request", 400, "/queryparamservice/?FOO=WSO2&bar=go", "GET"); + "Bad Request", 400, "/queryparamservice/?FOO=WSO2&bar=go", "GET"); } else { test:assertFail(msg = "Found unexpected output type: " + response.message()); } @@ -386,7 +386,7 @@ function testNegativeIntQueryBindingCastingError() returns error? { if response is http:Response { test:assertEquals(response.statusCode, 400); check common:assertJsonErrorPayload(check response.getJsonPayload(), "error in casting query param : 'bar'", - "Bad Request", 400, "/queryparamservice/?foo=WSO2&bar=go", "GET"); + "Bad Request", 400, "/queryparamservice/?foo=WSO2&bar=go", "GET"); } else { test:assertFail(msg = "Found unexpected output type: " + response.message()); } @@ -395,7 +395,7 @@ function testNegativeIntQueryBindingCastingError() returns error? { if response is http:Response { test:assertEquals(response.statusCode, 400); check common:assertJsonErrorPayload(check response.getJsonPayload(), "error in casting query param : 'bar'", - "Bad Request", 400, "/queryparamservice/?foo=WSO2&bar=", "GET"); + "Bad Request", 400, "/queryparamservice/?foo=WSO2&bar=", "GET"); } else { test:assertFail(msg = "Found unexpected output type: " + response.message()); } @@ -570,7 +570,7 @@ function testEmptyQueryParamBinding() returns error? { if response is http:Response { test:assertEquals(response.statusCode, 400); check common:assertJsonErrorPayload(check response.getJsonPayload(), "no query param value found for 'x-Type'", - "Bad Request", 400, "/queryparamservice/q9?x-Type", "GET"); + "Bad Request", 400, "/queryparamservice/q9?x-Type", "GET"); } else { test:assertFail(msg = "Found unexpected output type: " + response.message()); } @@ -758,7 +758,7 @@ function testMapOfJsonTypedQueryParamBinding3() returns error? { test:assertEquals(response.statusCode, 400, msg = "Found unexpected output"); } -@test:Config{} +@test:Config {} function testforQueryParamterNameOverwrite() returns error? { string result = check queryBindingClient->get("/queryparamservice/queryAnnotation?first=Harry&last-name=Potter"); test:assertEquals(result, "Hello, Harry Potter", msg = string `Found ${result}, expected Harry`); @@ -768,12 +768,12 @@ function testforQueryParamterNameOverwrite() returns error? { age: 10 }; string mapOfQueries = check url:encode(mapOfJsons.toJsonString(), "UTF-8"); - + result = check queryBindingClient->get("/queryparamservice/queryAnnotation/mapQueries?rMq=" + mapOfQueries); test:assertEquals(result, "Hello, Ron 10", msg = string `Found ${result}, expected Harry`); } -@test:Config{} +@test:Config {} function testforNegativeQueryParamterNameOverwrite() returns error? { string result = check queryBindingClient->get("/queryparamservice/queryAnnotation/negative/q1?firstName=Harry"); test:assertEquals(result, "Hello, Harry"); From e3855c53a29907ccda8f2ae93758e028c99b6922 Mon Sep 17 00:00:00 2001 From: lnash94 Date: Fri, 20 Sep 2024 18:02:14 +0530 Subject: [PATCH 5/5] Fix review suggestions --- .../service_dispatching_query_param_binding_test.bal | 10 +++++----- .../http/api/service/signature/ParamHandler.java | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/ballerina-tests/http-dispatching-tests/tests/service_dispatching_query_param_binding_test.bal b/ballerina-tests/http-dispatching-tests/tests/service_dispatching_query_param_binding_test.bal index f3b97520d6..c442d6ec0d 100644 --- a/ballerina-tests/http-dispatching-tests/tests/service_dispatching_query_param_binding_test.bal +++ b/ballerina-tests/http-dispatching-tests/tests/service_dispatching_query_param_binding_test.bal @@ -343,11 +343,11 @@ function testMapJsonOfDefaultableQueryBinding() returns error? { function testMapJsonArrOfDefaultableQueryBinding() returns error? { json response = check queryBindingClient->get("/default/q4"); common:assertJsonPayloadtoJsonString(response, { - objects: [ - {name: "test1", value: "json1"}, - {name: "test2", value: "json2"} - ] - }); + objects: [ + {name: "test1", value: "json1"}, + {name: "test2", value: "json2"} + ] + }); } @test:Config {} diff --git a/native/src/main/java/io/ballerina/stdlib/http/api/service/signature/ParamHandler.java b/native/src/main/java/io/ballerina/stdlib/http/api/service/signature/ParamHandler.java index 514ee11c6d..213d80318d 100644 --- a/native/src/main/java/io/ballerina/stdlib/http/api/service/signature/ParamHandler.java +++ b/native/src/main/java/io/ballerina/stdlib/http/api/service/signature/ParamHandler.java @@ -291,7 +291,7 @@ private static String getQueryParamName(String paramName, BMap annotations) { BMap mapValue = annotations.getMapValue(StringUtils.fromString(QUERY_ANNOTATION)); Object queryName = mapValue.get(HttpConstants.ANN_FIELD_NAME); if (queryName instanceof BString query) { - paramName = query.getValue(); + return query.getValue(); } return paramName; }