[TT-10291]: support gql-go-tools verison 2 by kofoworola · Pull Request #6240 · TykTechnologies/tyk

kofoworola · 2024-04-23T06:50:28Z

User description

Description

This PR adds the suppport of the new graphql-go-tools v2 with the use of the version 3-preview in the graphql version of the api definition

TT-10291

Related Issue

Motivation and Context

How This Has Been Tested

Screenshots (if appropriate)

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Refactoring or add test (improvements in base code or adds test coverage to functionality)

Checklist

I ensured that the documentation is up to date
I explained why this PR updates go.mod in detail with reasoning why it's required
I would like a code coverage CI quality gate exception and have explained why

Type

Enhancement

Description

Added a series of unit tests across multiple files to ensure the robustness and reliability of the new GraphQL engine configurations and their related functionalities.
Tests cover the new EngineV3 processing, UniversalDataGraph configurations, proxy-only setups, and utility functions in the GraphQL Go Tools V2.
Ensures that new functionalities integrate correctly with existing systems and behave as expected under various scenarios.

Changes walkthrough

Relevant files

Tests

adapter_udg_test.go `Add Unit Tests for UniversalDataGraph Engine Configuration` apidef/adapter/gqlengineadapter/v2/adapter_udg_test.go Added comprehensive tests for the `UniversalDataGraph` struct's methods related to engine configuration. Tests cover scenarios for creating engine configurations, handling field configurations, and data source configurations. Utilizes mock data and assertions to ensure correct behavior of the configuration generation.	+775/-0
adapter_proxy_only_test.go `Implement Tests for ProxyOnly Engine Configuration` apidef/adapter/gqlengineadapter/v2/adapter_proxy_only_test.go Introduced tests for the `ProxyOnly` struct focusing on the engine configuration. Checks the correct handling of proxy-only mode configurations including headers and subscription setups. Ensures that the engine configuration is correctly set up with the expected properties from the API definition.	+272/-0
mw_graphql_test.go `Extend GraphQL Middleware Tests for Engine V3` gateway/mw_graphql_test.go Extended the existing tests for GraphQL middleware to include scenarios for the new GraphQL engine version 3. Added test cases to verify the correct initialization and request processing by the GraphQL middleware using the new engine. Ensures middleware handles different configurations and request types correctly.	+167/-13
engine_v3_test.go `Add Tests for EngineV3 Request Processing` internal/graphengine/engine_v3_test.go Created tests for the new `EngineV3` struct to validate its request processing logic. Tests cover error handling, schema parsing, and request execution scenarios. Validates the integration of the new engine with expected GraphQL operations and error management.	+100/-0
graphql_go_tools_v2_test.go `Unit Tests for GraphQL Go Tools V2 Utilities` internal/graphengine/graphql_go_tools_v2_test.go Introduced unit tests for the utility functions in `graphql_go_tools_v2` which are used across various GraphQL engine configurations. Focuses on testing the parsing of schemas, handling of introspection, and header modifications. Ensures utility functions behave as expected when used in engine configurations.	+176/-0

✨ PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

github-actions · 2024-04-23T06:51:44Z

💥 CI tests failed 🙈

git-state

all ok

Please look at the run or in the Checks tab.

github-actions · 2024-04-23T06:51:51Z

PR Description updated to latest commit (9d4c650)

github-actions · 2024-04-23T06:52:33Z

PR Review

⏱️ Estimated effort to review [1-5]	4, due to the extensive changes across multiple files, the introduction of new features, and the complexity of integrating GraphQL v2 and v3 configurations. The PR involves updates to core components like schema parsing, data source configuration, and engine setup which require careful review to ensure compatibility and correctness.
🧪 Relevant tests	No
🔍 Possible issues	Possible Bug: The PR introduces changes to the GraphQL engine configuration and handling that could potentially disrupt existing functionalities or introduce new bugs if not thoroughly tested. For example, changes in schema parsing and error handling might affect how queries are processed.
🔍 Possible issues	Performance Concern: The new configurations and additional layers of processing might introduce performance overhead, especially in high-load environments. It's crucial to benchmark these changes against current performance metrics.
🔒 Security concerns	No

Code feedback:

relevant file	apidef/adapter/gqlengineadapter/v2/adapter_udg.go
suggestion	Consider handling errors from `parseSchema` directly within the `EngineConfigV2` method to simplify error handling and avoid repeating the check for `nil` schema across different parts of the code. This change ensures that any issues with schema parsing are caught early and managed in a consistent manner. [important]
relevant line	p.Schema, err = parseSchema(p.ApiDefinition.GraphQL.Schema)

relevant file	apidef/adapter/gqlengineadapter/v2/adapter_proxy_only.go
suggestion	It's recommended to validate the `url` after it's modified by replacing "tyk://" with "http://". This validation can prevent configuration errors that might lead to runtime issues. Adding a simple URL validation check can enhance the robustness of the proxy configuration. [important]
relevant line	url = strings.ReplaceAll(url, "tyk://", "http://")

relevant file	gateway/mw_graphql.go
suggestion	Ensure that the new GraphQL engine (V3) initialization includes error handling similar to other engine versions. This would involve logging the error and potentially stopping further initialization if the engine setup fails, which is critical to prevent runtime panics or malfunctions. [important]
relevant line	log.Errorf("Error creating enginev3: %v", err)

relevant file	internal/graphengine/engine_v3.go
suggestion	For the `ProcessRequest` method, consider adding detailed logging for each type of error returned. This would help in debugging and maintaining the system by providing clear insights into the flow and types of issues encountered during request processing. [medium]
relevant line	return err, http.StatusBadRequest

✨ Review tool usage guide:

Overview:
The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:

/review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...

With a configuration file, use the following template:

[pr_reviewer]
some_config1=...
some_config2=...

See the review usage page for a comprehensive guide on using this tool.

github-actions · 2024-04-23T06:53:45Z

PR Code Suggestions

Category	Suggestions
Best practice	Add error handling after calling `RemoveDuplicateApiDefinitionHeaders`. Consider checking for errors after calling `RemoveDuplicateApiDefinitionHeaders` to ensure that the headers are processed correctly without issues. apidef/adapter/gqlengineadapter/adapter_supergraph.go [69] -hdr := RemoveDuplicateApiDefinitionHeaders(apiDefSubgraphConf.Headers, s.ApiDefinition.GraphQL.Supergraph.GlobalHeaders) +hdr, err := RemoveDuplicateApiDefinitionHeaders(apiDefSubgraphConf.Headers, s.ApiDefinition.GraphQL.Supergraph.GlobalHeaders) +if err != nil { + // handle error +}
	Add error handling for header conversion to ensure robustness. Ensure that the `ConvertApiDefinitionHeadersToHttpHeaders` function handles potential errors or unexpected input, as it directly affects HTTP header configuration. apidef/adapter/gqlengineadapter/adapter_udg.go [106] -Header: ConvertApiDefinitionHeadersToHttpHeaders(restConfig.Headers), +Header, err := ConvertApiDefinitionHeadersToHttpHeaders(restConfig.Headers) +if err != nil { + // handle error +}
	Add validation for the URL after modifying its scheme to ensure it's valid. Validate the URL after replacing the "tyk://" scheme to ensure it results in a valid HTTP URL. apidef/adapter/gqlengineadapter/v2/adapter_proxy_only.go [36] url = strings.ReplaceAll(url, "tyk://", "http://") +if _, err := url.ParseRequestURI(url); err != nil { + // handle invalid URL +}
	Normalize HTTP header keys using `http.CanonicalHeaderKey`. In the function `ConvertApiDefinitionHeadersToHttpHeaders`, consider using `http.CanonicalHeaderKey` to normalize the header keys. This ensures that the headers are correctly recognized regardless of their case. apidef/adapter/gqlengineadapter/v2/utils.go [78] -engineV2Headers.Add(apiDefHeaderKey, apiDefHeaderValue) +engineV2Headers.Add(http.CanonicalHeaderKey(apiDefHeaderKey), apiDefHeaderValue)
	Add explicit handling for unexpected `GraphQLConfigVersion` values. In the `needsGraphQLExecutionEngine` function, ensure that the case for `GraphQLExecutionModeProxyOnly` includes a default return of `false` to handle unexpected `GraphQLConfigVersion` values explicitly. gateway/mw_graphql.go [324-326] if apiSpec.GraphQL.Version == apidef.GraphQLConfigVersion2 \|\| apiSpec.GraphQL.Version == apidef.GraphQLConfigVersion3Preview { return true } +return false
	Replace `panic` with error handling in test cases. Consider handling errors from `json.Unmarshal` instead of using `panic`. This will allow for more graceful error handling and better control over the application flow. gateway/mw_graphql_test.go [467-469] if err := json.Unmarshal([]byte(testRESTDataSourceConfigurationV3), &ds); err != nil { - panic(err) + t.Fatalf("Failed to unmarshal data source configuration: %v", err) }
	Add error handling for test execution methods. Consider adding error checks for the `g.Run` method calls to handle potential test case failures more explicitly. gateway/mw_graphql_test.go [517] -_, _ = g.Run(t, test.TestCase{Data: request, BodyMatch: `{"kind":"OBJECT","name":"Country"`, Code: http.StatusOK}) +_, err = g.Run(t, test.TestCase{Data: request, BodyMatch: `{"kind":"OBJECT","name":"Country"`, Code: http.StatusOK}) +if err != nil { + t.Errorf("Failed to run test case: %v", err) +}
Enhancement	Modify `RemoveDuplicateApiDefinitionHeaders` to return an error for better error handling. Refactor `RemoveDuplicateApiDefinitionHeaders` to return an error along with the headers map to handle potential issues during header processing. apidef/adapter/gqlengineadapter/utils.go [71] -func RemoveDuplicateApiDefinitionHeaders(headers ...map[string]string) map[string]string { +func RemoveDuplicateApiDefinitionHeaders(headers ...map[string]string) (map[string]string, error) {
	Improve URL scheme replacement by using URL parsing. Replace the direct string replacement for URL schemes with a more robust URL parsing and modification approach to avoid potential bugs with incorrect URLs. apidef/adapter/gqlengineadapter/v2/adapter_supergraph.go [78-81] if strings.HasPrefix(url, "tyk://") { - url = strings.ReplaceAll(url, "tyk://", "http://") + parsedUrl, err := url.Parse(url) + if err != nil { + return nil, err + } + parsedUrl.Scheme = "http" + url = parsedUrl.String() dataSourceHeaders[apidef.TykInternalApiHeader] = "true" }
	Handle URL scheme changes dynamically for multiple schemes. Use a loop or a map to handle multiple URL schemes dynamically instead of hardcoding only one scheme change. apidef/adapter/gqlengineadapter/v2/adapter_supergraph.go [78-81] -if strings.HasPrefix(url, "tyk://") { - url = strings.ReplaceAll(url, "tyk://", "http://") - dataSourceHeaders[apidef.TykInternalApiHeader] = "true" +schemes := map[string]string{ + "tyk": "http", +} +for oldScheme, newScheme := range schemes { + if strings.HasPrefix(url, oldScheme+"://") { + url = strings.Replace(url, oldScheme+"://", newScheme+"://", 1) + dataSourceHeaders[apidef.TykInternalApiHeader] = "true" + break + } }
	Add logging for errors in setting the merged schema. Add error handling for the `SetMergedSchemaFromString` method to ensure that the configuration process can gracefully handle failures. apidef/adapter/gqlengineadapter/v2/adapter_supergraph.go [30-32] err := federationConfigV2Factory.SetMergedSchemaFromString(s.ApiDefinition.GraphQL.Supergraph.MergedSDL) if err != nil { + log.Printf("Error setting merged schema: %v", err) return nil, err }
	Add error handling for data source configuration creation. Implement error handling for the `graphqlDataSourceConfiguration` function to manage potential failures in configuration setup. apidef/adapter/gqlengineadapter/v2/adapter_supergraph.go [56-60] -conf := graphqlDataSourceConfiguration( +conf, err := graphqlDataSourceConfiguration( apiDefSubgraphConf.URL, http.MethodPost, hdr, apiDefSubgraphConf.SubscriptionType) +if err != nil { + return nil, err +}
	Improve URL parsing to handle cases with multiple '?' characters. The function `extractURLQueryParamsForEngineV2` should handle cases where the URL contains more than one '?' character. Currently, it only checks for exactly one '?' which might not be sufficient for all URLs. A more robust approach would involve using `strings.Index` to find the first occurrence of '?' and then splitting the string based on that index. apidef/adapter/gqlengineadapter/v2/utils.go [19-25] -urlParts := strings.Split(url, "?") -urlWithoutParams = urlParts[0] +if idx := strings.Index(url, "?"); idx != -1 { + urlWithoutParams = url[:idx] + queryPart = url[idx+1:] +} else { + urlWithoutParams = url +}
	Add a check to prevent appending empty query values to the query configurations. The function `appendURLQueryParamsToEngineV2Queries` should check for empty query values before appending them to the `engineV2Queries`. This will prevent unnecessary query parameters from being sent in requests. apidef/adapter/gqlengineadapter/v2/utils.go [45-47] -engineV2Queries = append(engineV2Queries, restDataSource.QueryConfiguration{ - Name: queryKey, - Value: strings.Join(queryValue, ","), -}) +if len(queryValue) > 0 { + engineV2Queries = append(engineV2Queries, restDataSource.QueryConfiguration{ + Name: queryKey, + Value: strings.Join(queryValue, ","), + }) +}
	Ensure the HTTP method is normalized to uppercase to avoid method mismatch issues. The function `generateRestDataSourceFromGraphql` should handle the case where `config.Method` is not a valid HTTP method by either defaulting to a safe method like GET or by returning an error. apidef/adapter/gqlengineadapter/v2/utils.go [100] -Method: config.Method, +Method: strings.ToUpper(config.Method),
Performance	Add a check for empty headers before conversion to optimize processing. Consider adding a check for empty headers before converting them in `ConvertApiDefinitionHeadersToHttpHeaders` to avoid unnecessary processing. apidef/adapter/gqlengineadapter/utils_engine_v2.go [39] -Header: ConvertApiDefinitionHeadersToHttpHeaders(dataSourceHeaders), +var header http.Header +if len(dataSourceHeaders) > 0 { + header = ConvertApiDefinitionHeadersToHttpHeaders(dataSourceHeaders) +}
Maintainability	Refactor data source configuration creation into a separate function. Refactor the `subgraphDataSourceConfigs` function to separate concerns, improving readability and maintainability. apidef/adapter/gqlengineadapter/v2/adapter_supergraph.go [51-66] for _, apiDefSubgraphConf := range s.ApiDefinition.GraphQL.Supergraph.Subgraphs { - if len(apiDefSubgraphConf.SDL) == 0 { - continue - } - hdr := gqlengineadapter.RemoveDuplicateApiDefinitionHeaders(apiDefSubgraphConf.Headers, s.ApiDefinition.GraphQL.Supergraph.GlobalHeaders) - conf := graphqlDataSourceConfiguration( - apiDefSubgraphConf.URL, - http.MethodPost, - hdr, - apiDefSubgraphConf.SubscriptionType) - conf.Federation = graphqlDataSource.FederationConfiguration{ - Enabled: true, - ServiceSDL: apiDefSubgraphConf.SDL, + conf, err := createDataSourceConfig(apiDefSubgraphConf) + if err != nil { + return nil, err } confs = append(confs, conf) }
	Use constants for HTTP status codes. Use a constant for the HTTP status code instead of a hard-coded value to improve code readability and maintainability. gateway/mw_graphql_test.go [501-506] -Code: http.StatusOK +Code: http.StatusOK // Use http constant for status code
	Refactor repeated setup code into a helper function. Refactor repeated code blocks into a helper function to reduce code duplication and improve test maintainability. gateway/mw_graphql_test.go [413-419] -spec.UseKeylessAccess = true -spec.GraphQL.Enabled = true -spec.GraphQL.ExecutionMode = apidef.GraphQLExecutionModeProxyOnly -spec.GraphQL.Version = apidef.GraphQLConfigVersion3Preview -spec.GraphQL.Schema = gqlProxyUpstreamSchema +configureGraphQLProxy(spec)
Bug	Correctly escape regex patterns in `BodyMatch`. Ensure that the `BodyMatch` regex patterns are correctly escaped and tested to avoid runtime errors and ensure they match the intended patterns. gateway/mw_graphql_test.go [501-502] -BodyMatch: `"countries":.{"name":"Turkey"},{"name":"Russia"}.` +BodyMatch: `"countries":.\{"name":"Turkey"\},\{"name":"Russia"\}.`

✨ Improve tool usage guide:

Overview:
The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.

When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:

/improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...

With a configuration file, use the following template:

[pr_code_suggestions]
some_config1=...
some_config2=...

See the improve usage page for a comprehensive guide on using this tool.

github-actions · 2024-04-23T06:54:41Z

API Changes

--- prev.txt	2024-05-08 07:21:55.521930831 +0000
+++ current.txt	2024-05-08 07:21:52.269909386 +0000
@@ -1262,9 +1262,10 @@
 type GraphQLConfigVersion string
 
 const (
-	GraphQLConfigVersionNone GraphQLConfigVersion = ""
-	GraphQLConfigVersion1    GraphQLConfigVersion = "1"
-	GraphQLConfigVersion2    GraphQLConfigVersion = "2"
+	GraphQLConfigVersionNone     GraphQLConfigVersion = ""
+	GraphQLConfigVersion1        GraphQLConfigVersion = "1"
+	GraphQLConfigVersion2        GraphQLConfigVersion = "2"
+	GraphQLConfigVersion3Preview GraphQLConfigVersion = "3-preview"
 )
 type GraphQLEngineConfig struct {
 	FieldConfigs  []GraphQLFieldConfig      `bson:"field_configs" json:"field_configs"`
@@ -1291,15 +1292,15 @@
 }
 
 type GraphQLEngineDataSourceConfigKafka struct {
-	BrokerAddresses      []string              `bson:"broker_addresses" json:"broker_addresses"`
-	Topics               []string              `bson:"topics" json:"topics"`
-	GroupID              string                `bson:"group_id" json:"group_id"`
-	ClientID             string                `bson:"client_id" json:"client_id"`
-	KafkaVersion         string                `bson:"kafka_version" json:"kafka_version"`
-	StartConsumingLatest bool                  `json:"start_consuming_latest"`
-	BalanceStrategy      string                `json:"balance_strategy"`
-	IsolationLevel       string                `json:"isolation_level"`
-	SASL                 kafka_datasource.SASL `json:"sasl"`
+	BrokerAddresses      []string               `bson:"broker_addresses" json:"broker_addresses"`
+	Topics               []string               `bson:"topics" json:"topics"`
+	GroupID              string                 `bson:"group_id" json:"group_id"`
+	ClientID             string                 `bson:"client_id" json:"client_id"`
+	KafkaVersion         string                 `bson:"kafka_version" json:"kafka_version"`
+	StartConsumingLatest bool                   `json:"start_consuming_latest"`
+	BalanceStrategy      string                 `json:"balance_strategy"`
+	IsolationLevel       string                 `json:"isolation_level"`
+	SASL                 GraphQLEngineKafkaSASL `json:"sasl"`
 }
 
 type GraphQLEngineDataSourceConfigREST struct {
@@ -1312,6 +1313,12 @@
 
 type GraphQLEngineDataSourceKind string
 
+type GraphQLEngineKafkaSASL struct {
+	Enable   bool   `json:"enable"`
+	User     string `json:"user"`
+	Password string `json:"password"`
+}
+
 type GraphQLExecutionMode string
     GraphQLExecutionMode is the mode in which the GraphQL Middleware should
     operate.
@@ -2009,6 +2016,8 @@
 
 func (g *GraphQLConfigAdapter) EngineConfigV2() (*graphql.EngineV2Configuration, error)
 
+func (g *GraphQLConfigAdapter) EngineConfigV3() (*gqlv2.EngineV2Configuration, error)
+
 type GraphQLConfigAdapterOption func(adapter *GraphQLConfigAdapter)
 
 func WithHttpClient(httpClient *http.Client) GraphQLConfigAdapterOption
@@ -2017,12 +2026,18 @@
 
 func WithStreamingClient(streamingClient *http.Client) GraphQLConfigAdapterOption
 
+func WithV2Schema(schema *gqlv2.Schema) GraphQLConfigAdapterOption
+
 type GraphQLEngineAdapter interface {
 	EngineConfig() (*graphql.EngineV2Configuration, error)
 }
 
 type GraphQLEngineAdapterType int
 
+type GraphQLEngineAdapterV3 interface {
+	EngineConfigV3() (*gqlv2.EngineV2Configuration, error)
+}
+
 type ImportAdapter interface {
 	Import() (*apidef.APIDefinition, error)
 }
@@ -2042,6 +2057,11 @@
 	ErrGraphQLConfigIsMissingOperation = errors.New("graphql data source config is missing an operation")
 )
 
+FUNCTIONS
+
+func ConvertApiDefinitionHeadersToHttpHeaders(apiDefHeaders map[string]string) http.Header
+func RemoveDuplicateApiDefinitionHeaders(headers ...map[string]string) map[string]string
+
 TYPES
 
 type ProxyOnly struct {
@@ -2076,6 +2096,49 @@
 
 func (u *UniversalDataGraph) EngineConfig() (*graphql.EngineV2Configuration, error)
 
+# Package: ./apidef/adapter/gqlengineadapter/enginev3
+
+package enginev3 // import "github.com/TykTechnologies/tyk/apidef/adapter/gqlengineadapter/enginev3"
+
+
+FUNCTIONS
+
+func ConvertApiDefinitionHeadersToHttpHeaders(apiDefHeaders map[string]string) http.Header
+
+TYPES
+
+type ProxyOnly struct {
+	ApiDefinition   *apidef.APIDefinition
+	HttpClient      *http.Client
+	StreamingClient *http.Client
+	Schema          *graphql.Schema
+
+	// Has unexported fields.
+}
+
+func (p *ProxyOnly) EngineConfigV3() (*graphql.EngineV2Configuration, error)
+
+type Supergraph struct {
+	ApiDefinition   *apidef.APIDefinition
+	HttpClient      *http.Client
+	StreamingClient *http.Client
+
+	// Has unexported fields.
+}
+
+func (s *Supergraph) EngineConfigV3() (*graphql.EngineV2Configuration, error)
+
+type UniversalDataGraph struct {
+	ApiDefinition   *apidef.APIDefinition
+	HttpClient      *http.Client
+	StreamingClient *http.Client
+	Schema          *graphql.Schema
+
+	// Has unexported fields.
+}
+
+func (u *UniversalDataGraph) EngineConfigV3() (*graphql.EngineV2Configuration, error)
+
 # Package: ./apidef/importer
 
 package importer // import "github.com/TykTechnologies/tyk/apidef/importer"

github-actions · 2024-04-23T06:55:07Z

💥 CI tests failed 🙈

git-state

all ok

Please look at the run or in the Checks tab.

pvormste · 2024-05-02T09:37:34Z

+
+	switch {
+	case params.IsCORSPreflight:
+		if params.NeedsEngine {


This needs to be changed, as there was a bug regarding OPTIONS pass-through and proxy-only.

See: https://github.com/TykTechnologies/tyk/pull/6221/files

github-actions · 2024-05-06T14:07:29Z

💥 CI tests failed 🙈

git-state

all ok

Please look at the run or in the Checks tab.

github-actions · 2024-05-06T14:07:33Z

💥 CI tests failed 🙈

git-state

all ok

Please look at the run or in the Checks tab.

github-actions · 2024-05-06T14:08:02Z

💥 CI tests failed 🙈

git-state

all ok

Please look at the run or in the Checks tab.

github-actions · 2024-05-08T06:41:24Z

💥 CI tests failed 🙈

git-state

all ok

Please look at the run or in the Checks tab.

sonarqubecloud · 2024-05-08T07:34:17Z

Quality Gate failed

Failed conditions
58.6% Coverage on New Code (required ≥ 80%)
9.6% Duplication on New Code (required ≤ 3%)
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

## **User description**  ## Description This PR adds the suppport of the new graphql-go-tools v2 with the use of the version 3-preview in the graphql version of the api definition [TT-10291](https://tyktech.atlassian.net/browse/TT-10291)  ## Related Issue     ## Motivation and Context  ## How This Has Been Tested     ## Screenshots (if appropriate) ## Types of changes  - [ ] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Refactoring or add test (improvements in base code or adds test coverage to functionality) ## Checklist    - [ ] I ensured that the documentation is up to date - [ ] I explained why this PR updates go.mod in detail with reasoning why it's required - [ ] I would like a code coverage CI quality gate exception and have explained why [TT-10291]: https://tyktech.atlassian.net/browse/TT-10291?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ ___ ## **Type** Enhancement ___ ## **Description** - Added a series of unit tests across multiple files to ensure the robustness and reliability of the new GraphQL engine configurations and their related functionalities. - Tests cover the new `EngineV3` processing, `UniversalDataGraph` configurations, proxy-only setups, and utility functions in the GraphQL Go Tools V2. - Ensures that new functionalities integrate correctly with existing systems and behave as expected under various scenarios. ___ ## **Changes walkthrough** <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td>Tests </td><td><table> <tr> <td> <details> <summary>adapter_udg_test.go<dd><code>Add Unit Tests for UniversalDataGraph Engine Configuration</code></dd></summary> <hr> apidef/adapter/gqlengineadapter/v2/adapter_udg_test.go <li>Added comprehensive tests for the <code>UniversalDataGraph</code> struct's methods related to engine configuration. <li> Tests cover scenarios for creating engine configurations, handling field configurations, and data source configurations. <li> Utilizes mock data and assertions to ensure correct behavior of the configuration generation. </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6240/files#diff-c297559ca728c2e8a5a000579affbf009b37cec49553b2fd80d1b124d41cba66">+775/-0</a>  </td> </tr> <tr> <td> <details> <summary>adapter_proxy_only_test.go<dd><code>Implement Tests for ProxyOnly Engine Configuration</code>              </dd></summary> <hr> apidef/adapter/gqlengineadapter/v2/adapter_proxy_only_test.go <li>Introduced tests for the <code>ProxyOnly</code> struct focusing on the engine configuration. <li> Checks the correct handling of proxy-only mode configurations including headers and subscription setups. <li> Ensures that the engine configuration is correctly set up with the expected properties from the API definition. </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6240/files#diff-6a77a4bec63cc72b90f18cc53a41b329bd799f9f3f4687bfa39f0b4343847f81">+272/-0</a>  </td> </tr> <tr> <td> <details> <summary>mw_graphql_test.go<dd><code>Extend GraphQL Middleware Tests for Engine V3</code>                        </dd></summary> <hr> gateway/mw_graphql_test.go <li>Extended the existing tests for GraphQL middleware to include scenarios for the new GraphQL engine version 3. <li> Added test cases to verify the correct initialization and request processing by the GraphQL middleware using the new engine. <li> Ensures middleware handles different configurations and request types correctly. </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6240/files#diff-72c11b8efcaceccfbbd87e75565353706443bf56420d3fdba6b5bf5f632f4b33">+167/-13</a></td> </tr> <tr> <td> <details> <summary>engine_v3_test.go<dd><code>Add Tests for EngineV3 Request Processing</code>                                </dd></summary> <hr> internal/graphengine/engine_v3_test.go <li>Created tests for the new <code>EngineV3</code> struct to validate its request processing logic. <li> Tests cover error handling, schema parsing, and request execution scenarios. <li> Validates the integration of the new engine with expected GraphQL operations and error management. </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6240/files#diff-4948a6d04106feed51c084f3e15a3aac17a098791678c577355dcd7ba5a839d4">+100/-0</a>  </td> </tr> <tr> <td> <details> <summary>graphql_go_tools_v2_test.go<dd><code>Unit Tests for GraphQL Go Tools V2 Utilities</code>                          </dd></summary> <hr> internal/graphengine/graphql_go_tools_v2_test.go <li>Introduced unit tests for the utility functions in <code>graphql_go_tools_v2</code> which are used across various GraphQL engine configurations. <li> Focuses on testing the parsing of schemas, handling of introspection, and header modifications. <li> Ensures utility functions behave as expected when used in engine configurations. </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6240/files#diff-743b61933cadae116d62b0bca24afeaaee49eee10d18d4e34cf63a6e234a7f7e">+176/-0</a>  </td> </tr> </table></td></tr></tr></tbody></table> ___ > ✨ **PR-Agent usage**: >Comment `/help` on the PR to get a list of all available PR-Agent tools and their descriptions

kofoworola requested a review from a team April 23, 2024 06:51

github-actions Bot added the enhancement label Apr 23, 2024

github-actions Bot added the Review effort [1-5]: 4 label Apr 23, 2024

kofoworola force-pushed the feat/gql-version-3 branch from 9d4c650 to 916b986 Compare April 23, 2024 06:53

pvormste requested changes May 2, 2024

View reviewed changes

kofoworola force-pushed the feat/gql-version-3 branch 2 times, most recently from 91f7cf6 to ceb08b1 Compare May 6, 2024 13:57

kofoworola requested a review from pvormste May 6, 2024 13:58

kofoworola added 12 commits May 8, 2024 08:21

added proxy only adapter and test

f1dc4a8

supergraph tests work fine

175df1d

fixing rest datasource tests

c82532f

correct rest ds imports after adding rest ds

c49389e

used appropriate schema for mw_gql

6f1a901

added first part of mw_graphql logic for v2

3fca90e

fixed test by removing location data on validation inpu

1e407a2

moved Process request to engine

86b66e8

fixed engine process and store request

62de586

fixed reverse proxy

1c68f79

proxy only test works

e0b4c65

fixed issue with federation spec being used

4b8aeac

kofoworola added 9 commits May 8, 2024 08:21

fixing rest datasource

78bf621

modified test case for udg rest data

28b4a39

remove subgraph

0d88913

fixed the adapter test

4113681

make lint fix

676ca9d

rename packages

e2e8dd0

added reverse proxy fix

3216a4a

fixed cors option and disable complexity checker

8020e39

fx test

13c9529

kofoworola force-pushed the feat/gql-version-3 branch from 0fc2904 to 13c9529 Compare May 8, 2024 07:21

kofoworola enabled auto-merge (squash) May 8, 2024 07:21

pvormste approved these changes May 8, 2024

View reviewed changes

kofoworola merged commit b36c95d into master May 8, 2024

kofoworola deleted the feat/gql-version-3 branch May 8, 2024 10:03

Uh oh!

Conversation

kofoworola commented Apr 23, 2024 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

User description

Description

Related Issue

Motivation and Context

How This Has Been Tested

Screenshots (if appropriate)

Types of changes

Checklist

Type

Description

Changes walkthrough

Uh oh!

github-actions Bot commented Apr 23, 2024

💥 CI tests failed 🙈

git-state

Uh oh!

github-actions Bot commented Apr 23, 2024

Uh oh!

github-actions Bot commented Apr 23, 2024

PR Review

Uh oh!

github-actions Bot commented Apr 23, 2024

PR Code Suggestions

Uh oh!

github-actions Bot commented Apr 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Apr 23, 2024

💥 CI tests failed 🙈

git-state

Uh oh!

Uh oh!

Uh oh!

pvormste May 2, 2024

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented May 6, 2024

💥 CI tests failed 🙈

git-state

Uh oh!

github-actions Bot commented May 6, 2024

💥 CI tests failed 🙈

git-state

Uh oh!

github-actions Bot commented May 6, 2024

💥 CI tests failed 🙈

git-state

Uh oh!

github-actions Bot commented May 8, 2024

💥 CI tests failed 🙈

git-state

Uh oh!

sonarqubecloud Bot commented May 8, 2024

Quality Gate failed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kofoworola commented Apr 23, 2024 •

edited by github-actions Bot

Loading

github-actions Bot commented Apr 23, 2024 •

edited

Loading