From 8fbcfacf46c76a2b6ff70b365469e2a7c689c926 Mon Sep 17 00:00:00 2001 From: Thisaru Guruge Date: Wed, 29 Jan 2025 13:06:39 +0530 Subject: [PATCH] Fix listener error when passing configurables as init parameters --- .../tests/listeners.bal | 5 +- .../tests/listeners.bal | 1 - .../tests/30_graphiql_client.bal | 2 +- .../graphql-subgraph-test-suite/subgraphs.bal | 2 +- .../tests/01_subgraph_tests.bal | 6 +- .../tests/listeners.bal | 19 -- .../five/tests/01_subscription_five_tests.bal | 6 +- .../modules/five/tests/listeners.bal | 4 - .../modules/six/tests/listeners.bal | 1 - ballerina/annotation_processor.bal | 30 --- ballerina/build.gradle | 1 + ballerina/listener.bal | 186 +++++++++++------- ballerina/listener_utils.bal | 9 + changelog.md | 1 + examples/starwars/tests/tests.bal | 9 +- 15 files changed, 147 insertions(+), 135 deletions(-) delete mode 100644 ballerina-tests/graphql-subgraph-test-suite/tests/listeners.bal diff --git a/ballerina-tests/graphql-advanced-test-suite/tests/listeners.bal b/ballerina-tests/graphql-advanced-test-suite/tests/listeners.bal index f094cfe5b..770b60b5f 100644 --- a/ballerina-tests/graphql-advanced-test-suite/tests/listeners.bal +++ b/ballerina-tests/graphql-advanced-test-suite/tests/listeners.bal @@ -16,4 +16,7 @@ import ballerina/graphql; -listener graphql:Listener graphqlListener = new (9090); +configurable int port = 9090; +configurable graphql:ListenerConfiguration graphqlListenerConfigs = {}; + +listener graphql:Listener graphqlListener = new (port, graphqlListenerConfigs); diff --git a/ballerina-tests/graphql-interceptor-test-suite/tests/listeners.bal b/ballerina-tests/graphql-interceptor-test-suite/tests/listeners.bal index f9e09624a..4d208a2e5 100644 --- a/ballerina-tests/graphql-interceptor-test-suite/tests/listeners.bal +++ b/ballerina-tests/graphql-interceptor-test-suite/tests/listeners.bal @@ -17,4 +17,3 @@ import ballerina/graphql; listener graphql:Listener basicListener = new (9090); -listener graphql:Listener subscriptionListener = new (9091); diff --git a/ballerina-tests/graphql-service-test-suite/tests/30_graphiql_client.bal b/ballerina-tests/graphql-service-test-suite/tests/30_graphiql_client.bal index 62fae15db..ccb94ba0d 100644 --- a/ballerina-tests/graphql-service-test-suite/tests/30_graphiql_client.bal +++ b/ballerina-tests/graphql-service-test-suite/tests/30_graphiql_client.bal @@ -26,7 +26,7 @@ function testGraphiqlWithSamePathAsGraphQLService() returns error? { graphql:Error? result = basicListener.attach(graphiqlConfigService, "ballerina/graphiql"); test:assertTrue(result is graphql:Error); graphql:Error err = result; - test:assertEquals(err.message(), "Error occurred while attaching the GraphiQL endpoint"); + test:assertEquals(err.message(), "Error occurred while attaching the HTTP service"); } @test:Config { diff --git a/ballerina-tests/graphql-subgraph-test-suite/subgraphs.bal b/ballerina-tests/graphql-subgraph-test-suite/subgraphs.bal index 59412a9a2..45959e329 100644 --- a/ballerina-tests/graphql-subgraph-test-suite/subgraphs.bal +++ b/ballerina-tests/graphql-subgraph-test-suite/subgraphs.bal @@ -21,7 +21,7 @@ service /subgraph on new graphql:Listener(9088) { resource function get greet() returns string => "welcome"; } -public graphql:Service subgraphServivce = @subgraph:Subgraph service object { +public graphql:Service subgraphService = @subgraph:Subgraph service object { resource function get greeting() returns string => "welcome"; }; diff --git a/ballerina-tests/graphql-subgraph-test-suite/tests/01_subgraph_tests.bal b/ballerina-tests/graphql-subgraph-test-suite/tests/01_subgraph_tests.bal index 9e7ab46c1..433339a9c 100644 --- a/ballerina-tests/graphql-subgraph-test-suite/tests/01_subgraph_tests.bal +++ b/ballerina-tests/graphql-subgraph-test-suite/tests/01_subgraph_tests.bal @@ -109,13 +109,15 @@ isolated function testQueringSdlOnSubgraph() returns error? { groups: ["federation", "subgraph"] } function testAttachingSubgraphServiceToDynamicListener() returns error? { - check subgraphListener.attach(subgraphServivce, "subgraph"); + graphql:Listener subgraphListener = check new (9095); + check subgraphListener.attach(subgraphService, "subgraph"); + check subgraphListener.start(); string url = "http://localhost:9095/subgraph"; graphql:Client graphqlClient = check new (url); string document = check common:getGraphqlDocumentFromFile("querying_entities_field_on_subgraph"); json response = check graphqlClient->execute(document); json expectedPayload = check common:getJsonContentFromFile("querying_entities_field_on_subgraph"); - check subgraphListener.detach(subgraphServivce); + check subgraphListener.detach(subgraphService); common:assertJsonValuesWithOrder(response, expectedPayload); } diff --git a/ballerina-tests/graphql-subgraph-test-suite/tests/listeners.bal b/ballerina-tests/graphql-subgraph-test-suite/tests/listeners.bal deleted file mode 100644 index 3ccf7b58f..000000000 --- a/ballerina-tests/graphql-subgraph-test-suite/tests/listeners.bal +++ /dev/null @@ -1,19 +0,0 @@ -// Copyright (c) 2024 WSO2 LLC. (http://www.wso2.com). -// -// WSO2 LLC. 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/graphql; - -listener graphql:Listener subgraphListener = new (9095); diff --git a/ballerina-tests/graphql-subscription-test-suite/modules/five/tests/01_subscription_five_tests.bal b/ballerina-tests/graphql-subscription-test-suite/modules/five/tests/01_subscription_five_tests.bal index d00a126bf..50829a8cb 100644 --- a/ballerina-tests/graphql-subscription-test-suite/modules/five/tests/01_subscription_five_tests.bal +++ b/ballerina-tests/graphql-subscription-test-suite/modules/five/tests/01_subscription_five_tests.bal @@ -16,6 +16,7 @@ import ballerina/graphql; import ballerina/graphql_test_common as common; +import ballerina/http; import ballerina/test; import ballerina/websocket; @@ -23,11 +24,12 @@ import ballerina/websocket; groups: ["listener", "subscriptions"] } function testAttachServiceWithSubscriptionToHttp2BasedListener() returns error? { + http:Listener http2Listener = check new http:Listener(9090); + graphql:Listener http2BasedListener = check new (http2Listener); graphql:Error? result = http2BasedListener.attach(subscriptionService); test:assertTrue(result is graphql:Error); graphql:Error err = result; - string expectedMessage = string `Websocket listener initialization failed due to the incompatibility of ` + - string `provided HTTP(version 2.0) listener`; + string expectedMessage = "GraphQL subscriptions are only supported over HTTP/1.1 or HTTP/1.0. Found 2.0"; test:assertEquals(err.message(), expectedMessage); } diff --git a/ballerina-tests/graphql-subscription-test-suite/modules/five/tests/listeners.bal b/ballerina-tests/graphql-subscription-test-suite/modules/five/tests/listeners.bal index bcd752de9..046624e3c 100644 --- a/ballerina-tests/graphql-subscription-test-suite/modules/five/tests/listeners.bal +++ b/ballerina-tests/graphql-subscription-test-suite/modules/five/tests/listeners.bal @@ -15,9 +15,5 @@ // under the License. import ballerina/graphql; -import ballerina/http; - -listener http:Listener http2Listener = new http:Listener(9090); -listener graphql:Listener http2BasedListener = new (http2Listener); listener graphql:Listener subscriptionListener = new (9091); diff --git a/ballerina-tests/graphql-subscription-test-suite/modules/six/tests/listeners.bal b/ballerina-tests/graphql-subscription-test-suite/modules/six/tests/listeners.bal index f9e09624a..046624e3c 100644 --- a/ballerina-tests/graphql-subscription-test-suite/modules/six/tests/listeners.bal +++ b/ballerina-tests/graphql-subscription-test-suite/modules/six/tests/listeners.bal @@ -16,5 +16,4 @@ import ballerina/graphql; -listener graphql:Listener basicListener = new (9090); listener graphql:Listener subscriptionListener = new (9091); diff --git a/ballerina/annotation_processor.bal b/ballerina/annotation_processor.bal index 69da67b48..139f64021 100644 --- a/ballerina/annotation_processor.bal +++ b/ballerina/annotation_processor.bal @@ -59,20 +59,6 @@ isolated function getServiceInterceptors(GraphqlServiceConfig? serviceConfig) re return []; } -isolated function getIntrospection(GraphqlServiceConfig? serviceConfig) returns boolean { - if serviceConfig is GraphqlServiceConfig { - return serviceConfig.introspection; - } - return true; -} - -isolated function getValidation(GraphqlServiceConfig? serviceConfig) returns boolean { - if serviceConfig is GraphqlServiceConfig { - return serviceConfig.validation; - } - return true; -} - isolated function getFieldInterceptors(service object {} serviceObj, parser:RootOperationType operationType, string fieldName, string[] resourcePath) returns readonly & (readonly & Interceptor)[] { GraphqlResourceConfig? resourceConfig = getResourceAnnotation(serviceObj, operationType, resourcePath, fieldName); @@ -109,22 +95,6 @@ isolated function getInterceptorConfig(readonly & Interceptor interceptor) retur return classType.@InterceptorConfig; } -isolated function getCacheConfig(GraphqlServiceConfig? serviceConfig) returns ServerCacheConfig? { - if serviceConfig is GraphqlServiceConfig { - if serviceConfig.cacheConfig is ServerCacheConfig { - return serviceConfig.cacheConfig; - } - } - return; -} - -isolated function getFieldCacheConfigFromServiceConfig(GraphqlServiceConfig? serviceConfig) returns ServerCacheConfig? { - if serviceConfig is GraphqlServiceConfig { - return serviceConfig.fieldCacheConfig; - } - return; -} - isolated function getFieldCacheConfig(service object {} serviceObj, parser:RootOperationType operationType, string fieldName, string[] resourcePath) returns ServerCacheConfig? { GraphqlResourceConfig? resourceConfig = getResourceAnnotation(serviceObj, operationType, resourcePath, fieldName); diff --git a/ballerina/build.gradle b/ballerina/build.gradle index 0bd973a2d..feac67dc8 100644 --- a/ballerina/build.gradle +++ b/ballerina/build.gradle @@ -50,6 +50,7 @@ ballerina { module = packageName langVersion = ballerinaLangVersion testCoverageParam = "--code-coverage --coverage-format=xml --includes=io.ballerina.stdlib.graphql.*:ballerina.graphql*" + platform = 'java21' } task updateTomlFiles { diff --git a/ballerina/listener.bal b/ballerina/listener.bal index 608847ebd..e213b1b2b 100644 --- a/ballerina/listener.bal +++ b/ballerina/listener.bal @@ -15,14 +15,16 @@ // under the License. import ballerina/http; -import ballerina/websocket; import ballerina/io; +import ballerina/websocket; # Represents a Graphql listener endpoint. public class Listener { - private http:Listener httpListener; - private websocket:Listener? wsListener; - private Graphiql graphiql; + private http:Listener? httpListener = (); + private int|http:Listener listenTo; + private final ListenerConfiguration configuration; + private websocket:Listener? wsListener = (); + private Graphiql graphiql = {}; private string httpEndpoint; private string websocketEndpoint; // The service attach method does not contain the annotation values properly assigned. Therefore, the services are @@ -38,16 +40,8 @@ public class Listener { # + return - A `graphql:Error` if the listener initialization is failed or else `()` public isolated function init(int|http:Listener listenTo, *ListenerConfiguration configuration) returns Error? { - configuration.httpVersion = http:HTTP_1_1; - if listenTo is int { - http:Listener|error httpListener = new (listenTo, configuration); - if httpListener is error { - return error Error("Listener initialization failed", httpListener); - } - self.httpListener = httpListener; - } else { - self.httpListener = listenTo; - } + self.listenTo = listenTo; + self.configuration = configuration; self.wsListener = (); self.graphiql = {}; [string, string][httpEndpoint, websocketEndpoint] = getEndpoints(listenTo, configuration); @@ -67,36 +61,34 @@ public class Listener { string schemaString = serviceConfig.schemaString; int? maxQueryDepth = serviceConfig.maxQueryDepth; readonly & Interceptor[] interceptors = getServiceInterceptors(serviceConfig); - boolean introspection = getIntrospection(serviceConfig); - boolean validation = getValidation(serviceConfig); - ServerCacheConfig? operationCacheConfig = getCacheConfig(serviceConfig); - ServerCacheConfig? fieldCacheConfig = getFieldCacheConfigFromServiceConfig(serviceConfig); + boolean introspection = serviceConfig.introspection; + boolean validation = serviceConfig.validation; + ServerCacheConfig? operationCacheConfig = serviceConfig.cacheConfig; + ServerCacheConfig? fieldCacheConfig = serviceConfig.fieldCacheConfig; QueryComplexityConfig? queryComplexityConfig = serviceConfig.queryComplexityConfig; Engine engine = check new (schemaString, maxQueryDepth, s, interceptors, introspection, validation, operationCacheConfig, fieldCacheConfig, queryComplexityConfig); - if self.graphiql.enabled { - check validateGraphiqlPath(self.graphiql.path); - check self.initGraphiqlService(s, engine, name, serviceConfig); - } HttpService httpService = getHttpService(engine, serviceConfig); attachHttpServiceToGraphqlService(s, httpService); - error? result = self.httpListener.attach(httpService, name); + __Schema & readonly schema = engine.getSchema(); + boolean isSubscriptionService = schema.subscriptionType is __Type; + check self.initHttpListener(isSubscriptionService); + check self.initWebsocketListener(isSubscriptionService); + + http:Listener|Error httpListener = self.getHttpListener(); + if httpListener is Error { + return httpListener; + } + error? result = httpListener.attach(httpService, name); if result is error { - return error Error("Error occurred while attaching the service", result); + return error Error("Error occurred while attaching the HTTP service", result); } - __Schema & readonly schema = engine.getSchema(); - __Type? subscriptionType = schema.subscriptionType; - if subscriptionType is __Type && self.wsListener is () { - check self.initWebsocketListener(); + if isSubscriptionService { + check self.attachWebSocketService(s, engine, schema, serviceConfig, name); } - websocket:Listener? wsListener = self.wsListener; - if wsListener is websocket:Listener { - UpgradeService wsService = getWebsocketService(engine, schema, serviceConfig); - attachWebsocketServiceToGraphqlService(s, wsService); - result = wsListener.attach(wsService, name); - if result is error { - return error Error("Error occurred while attaching the websocket service", result); - } + if self.graphiql.enabled { + check validateGraphiqlPath(self.graphiql.path); + check self.initGraphiqlService(s, engine, name, serviceConfig); } self.services.push(s); } @@ -107,12 +99,7 @@ public class Listener { # + return - A `graphql:Error` if an error occurred during the service detaching process or else `()` public isolated function detach(Service s) returns Error? { HttpService? httpService = getHttpServiceFromGraphqlService(s); - if httpService is HttpService { - error? result = self.httpListener.detach(httpService); - if result is error { - return error Error("Error occurred while detaching the service", result); - } - } + check self.detachHttpService(httpService); websocket:Listener? wsListener = self.wsListener; if wsListener is websocket:Listener { @@ -126,12 +113,7 @@ public class Listener { } HttpService? graphiqlService = getGraphiqlServiceFromGraphqlService(s); - if graphiqlService is HttpService { - error? result = self.httpListener.detach(graphiqlService); - if result is error { - return error Error("Error occurred while detaching the GraphiQL endpoint", result); - } - } + check self.detachHttpService(graphiqlService); } # Starts the attached service. @@ -139,7 +121,11 @@ public class Listener { # + return - A `graphql:Error`, if an error occurred during the service starting process, otherwise nil public isolated function 'start() returns Error? { analyzeServices(self.services); - error? result = self.httpListener.'start(); + http:Listener|Error httpListener = self.getHttpListener(); + if httpListener is Error { + return httpListener; + } + error? result = httpListener.'start(); if result is error { return error Error("Error occurred while starting the service", result); } @@ -161,15 +147,18 @@ public class Listener { # # + return - A `graphql:Error`, if an error occurred during the service stopping process, otherwise nil public isolated function gracefulStop() returns Error? { - error? result = self.httpListener.gracefulStop(); - if result is error { - return error Error("Error occurred while stopping the service", result); + http:Listener|Error httpListener = self.getHttpListener(); + if httpListener is http:Listener { + error? result = httpListener.gracefulStop(); + if result is error { + return error Error("Error occurred while stopping the HTTP listener", result); + } } websocket:Listener? wsListener = self.wsListener; if wsListener is websocket:Listener { - result = wsListener.gracefulStop(); + error? result = wsListener.gracefulStop(); if result is error { - return error Error("Error occurred while stopping the websocket service", result); + return error Error("Error occurred while stopping the WebSocket listener", result); } } } @@ -178,33 +167,90 @@ public class Listener { # # + return - A `graphql:Error` if an error occurred during the service stopping process or else `()` public isolated function immediateStop() returns Error? { - error? result = self.httpListener.immediateStop(); - if result is error { - return error Error("Error occurred while stopping the service", result); + http:Listener|Error httpListener = self.getHttpListener(); + if httpListener is http:Listener { + error? result = httpListener.immediateStop(); + if result is error { + return error Error("Error occurred while stopping the service", result); + } } websocket:Listener? wsListener = self.wsListener; if wsListener is websocket:Listener { - result = wsListener.immediateStop(); + error? result = wsListener.immediateStop(); if result is error { return error Error("Error occurred while stopping the websocket service", result); } } } - private isolated function initWebsocketListener() returns Error? { - string httpVersion = self.httpListener.getConfig().httpVersion; - if httpVersion !is http:HTTP_1_1|http:HTTP_1_0 { - string message = string `Websocket listener initialization failed due to the incompatibility of ` + - string `provided HTTP(version ${httpVersion}) listener`; - return error Error(message); + private isolated function initHttpListener(boolean isSubscriptionService) returns Error? { + if self.httpListener is http:Listener { + return; + } + int|http:Listener listenTo = self.listenTo; + if listenTo is http:Listener { + if isSubscriptionService { + check validateHttpVersion(listenTo); + } + self.httpListener = listenTo; + return; } - websocket:Listener|error wsListener = new(self.httpListener); + ListenerConfiguration configuration = self.configuration; + if isSubscriptionService { + configuration.httpVersion = http:HTTP_1_1; + } + http:Listener|error httpListener = new (listenTo, configuration); + if httpListener is error { + return error Error("HTTP listener initialization failed", httpListener); + } + self.httpListener = httpListener; + } + + private isolated function initWebsocketListener(boolean isSubscriptionService) returns Error? { + if self.wsListener is websocket:Listener ||!isSubscriptionService { + return; + } + http:Listener|Error httpListener = self.getHttpListener(); + if httpListener is Error { + return httpListener; + } + check validateHttpVersion(httpListener); + + websocket:Listener|error wsListener = new(httpListener); if wsListener is error { return error Error("Websocket listener initialization failed", wsListener); } self.wsListener = wsListener; } + private isolated function attachWebSocketService(Service s, Engine engine, __Schema & readonly schema, + GraphqlServiceConfig serviceConfig, string[]|string? name) returns Error? { + websocket:Listener? wsListener = self.wsListener; + if wsListener is () { + return error Error("Websocket listener is not initialized"); + } + UpgradeService wsService = getWebsocketService(engine, schema, serviceConfig); + attachWebsocketServiceToGraphqlService(s, wsService); + error? result = wsListener.attach(wsService, name); + if result is error { + return error Error("Error occurred while attaching the WebSocket service", result); + } + } + + private isolated function detachHttpService(HttpService? httpService) returns Error? { + if httpService is () { + return; + } + http:Listener|Error httpListener = self.getHttpListener(); + if httpListener is Error { + return httpListener; + } + error? result = httpListener.detach(httpService); + if result is error { + return error Error("Error occurred while detaching the service", result); + } + } + private isolated function initGraphiqlService(Service s, Engine engine, string[]|string? name, GraphqlServiceConfig serviceConfig) returns Error? { string gqlServiceBasePath = name is () ? "" : getBasePath(name); @@ -216,9 +262,17 @@ public class Listener { ? getGraphiqlService(serviceConfig, graphqlUrl, subscriptionUrl) : getGraphiqlService(serviceConfig, graphqlUrl); attachGraphiqlServiceToGraphqlService(s, graphiqlService); - error? result = self.httpListener.attach(graphiqlService, self.graphiql.path); + error? result = (check self.getHttpListener()).attach(graphiqlService, self.graphiql.path); if result is error { return error Error("Error occurred while attaching the GraphiQL endpoint", result); } } + + private isolated function getHttpListener() returns http:Listener|Error { + http:Listener? httpListener = self.httpListener; + if httpListener is http:Listener { + return httpListener; + } + return error Error("HTTP listener is not initialized"); + } } diff --git a/ballerina/listener_utils.bal b/ballerina/listener_utils.bal index 6fbe04831..38e1f8e02 100644 --- a/ballerina/listener_utils.bal +++ b/ballerina/listener_utils.bal @@ -450,6 +450,15 @@ returns [string, string] { return [string `https://${LOCALHOST}:${port}`, string `wss://${LOCALHOST}:${port}`]; } +isolated function validateHttpVersion(http:Listener httpListener) returns Error? { + string httpVersion = httpListener.getConfig().httpVersion; + if httpVersion !is http:HTTP_1_1|http:HTTP_1_0 { + string message = + string `GraphQL subscriptions are only supported over HTTP/1.1 or HTTP/1.0. Found ${httpVersion}`; + return error Error(message); + } +} + isolated function attachHttpServiceToGraphqlService(Service s, HttpService httpService) = @java:Method { 'class: "io.ballerina.stdlib.graphql.runtime.engine.ListenerUtils" } external; diff --git a/changelog.md b/changelog.md index 3d768c640..b16fe2c38 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/), ### Fixed - [[#7317] Fix Service Crashing when Input Object Type Variable Value Includes an Additional Field](https://github.com/ballerina-platform/ballerina-library/issues/7317) +- [[#7502] Fix not Allowing to Pass `ListenerConfigurations` using Configurable Variables](https://github.com/ballerina-platform/ballerina-library/issues/7502) ## [1.14.0] - 2024-08-20 diff --git a/examples/starwars/tests/tests.bal b/examples/starwars/tests/tests.bal index a505afafb..2dd67c030 100644 --- a/examples/starwars/tests/tests.bal +++ b/examples/starwars/tests/tests.bal @@ -265,7 +265,7 @@ function testFriends() returns error? { } @test:Config { - enable: false // Disabled the test becasue the service is not closing when this test is done. + enable: false // Disabled the test because the service is not closing when this test is done. } function testReviewAdded() returns error? { string document = check getGraphqlDocumentFromFile("reviewAdded"); @@ -300,12 +300,7 @@ function addReview(string document) returns error? { function subscribeReviewAdded(string document) returns error? { json variables = { - episode: "EMPIRE", - // TODO: This field is not required. We can remove the `review` field from the `variables` map after fixing #4206 - review: { - stars: 5, - commentary: "Nice!" - } + episode: "EMPIRE" }; websocket:ClientConfiguration config = {subProtocols: ["graphql-transport-ws"]}; websocket:Client wsClient = check new ("ws://localhost:9090/graphql", config);