diff --git a/execution/engine/execution_engine.go b/execution/engine/execution_engine.go index 27c7a46df1..5e95d8d2fb 100644 --- a/execution/engine/execution_engine.go +++ b/execution/engine/execution_engine.go @@ -132,6 +132,9 @@ func NewExecutionEngine(ctx context.Context, logger abstractlogger.Logger, engin if engineConfig.plannerConfig.RelaxSubgraphOperationFieldSelectionMergingNullability { validationOpts = append(validationOpts, astvalidation.WithRelaxFieldSelectionMergingNullability()) } + if engineConfig.plannerConfig.RelaxSubgraphOperationFieldSelectionMergingTypeMismatch { + validationOpts = append(validationOpts, astvalidation.WithRelaxFieldSelectionMergingTypeMismatch()) + } return &ExecutionEngine{ logger: logger, diff --git a/execution/engine/execution_engine_test.go b/execution/engine/execution_engine_test.go index 9c3370efe0..be3df48ae2 100644 --- a/execution/engine/execution_engine_test.go +++ b/execution/engine/execution_engine_test.go @@ -225,6 +225,7 @@ type _executionTestOptions struct { validateRequiredExternalFields bool computeCosts bool relaxFieldSelectionMergingNullability bool + relaxFieldSelectionMergingTypeMismatch bool } type executionTestOptions func(*_executionTestOptions) @@ -261,6 +262,12 @@ func relaxFieldSelectionMergingNullability() executionTestOptions { } } +func relaxFieldSelectionMergingTypeMismatch() executionTestOptions { + return func(options *_executionTestOptions) { + options.relaxFieldSelectionMergingTypeMismatch = true + } +} + func TestExecutionEngine_Execute(t *testing.T) { run := func(testCase ExecutionEngineTestCase, withError bool, expectedErrorMessage string, options ...executionTestOptions) func(t *testing.T) { t.Helper() @@ -298,6 +305,7 @@ func TestExecutionEngine_Execute(t *testing.T) { engineConf.plannerConfig.ComputeCosts = opts.computeCosts engineConf.plannerConfig.StaticCostDefaultListSize = 10 engineConf.plannerConfig.RelaxSubgraphOperationFieldSelectionMergingNullability = opts.relaxFieldSelectionMergingNullability + engineConf.plannerConfig.RelaxSubgraphOperationFieldSelectionMergingTypeMismatch = opts.relaxFieldSelectionMergingTypeMismatch resolveOpts := resolve.ResolverOptions{ MaxConcurrency: 1024, ResolvableOptions: opts.resolvableOptions, @@ -7715,6 +7723,201 @@ func TestExecutionEngine_Execute(t *testing.T) { relaxFieldSelectionMergingNullability(), )) }) + + t.Run("field merging with different types on non-overlapping union types", func(t *testing.T) { + unionSchema := ` + enum IssueState { OPEN CLOSED } + enum PullRequestReviewState { PENDING APPROVED } + union Updatable = Issue | PullRequestReview + type Query { updatable: Updatable } + type Issue { id: ID!, state: IssueState, title: String!, priority: Int } + type PullRequestReview { id: ID!, state: PullRequestReviewState, title: String, priority: String } + ` + schema, err := graphql.NewSchemaFromString(unionSchema) + require.NoError(t, err) + + rootNodes := []plan.TypeField{ + {TypeName: "Query", FieldNames: []string{"updatable"}}, + {TypeName: "Issue", FieldNames: []string{"id", "state", "title", "priority"}}, + {TypeName: "PullRequestReview", FieldNames: []string{"id", "state", "title", "priority"}}, + } + + customConfig := mustConfiguration(t, graphql_datasource.ConfigurationInput{ + Fetch: &graphql_datasource.FetchConfiguration{ + URL: "https://example.com/", + Method: "POST", + }, + SchemaConfiguration: mustSchemaConfig(t, nil, unionSchema), + }) + + fieldConfig := []plan.FieldConfiguration{ + { + TypeName: "Query", + FieldName: "updatable", + Path: []string{"updatable"}, + }, + } + + // Strict-mode failure test: use scalar type mismatch (Int vs String) to + // verify that validation rejects different types without the relaxation flag. + scalarMismatchSchema := ` + type User { id: ID!, priority: Int } + type Organization { id: ID!, priority: String } + union Entity = User | Organization + type Query { entity: Entity } + ` + scalarMismatchParsedSchema, err2 := graphql.NewSchemaFromString(scalarMismatchSchema) + require.NoError(t, err2) + + scalarMismatchRootNodes := []plan.TypeField{ + {TypeName: "Query", FieldNames: []string{"entity"}}, + {TypeName: "User", FieldNames: []string{"id", "priority"}}, + {TypeName: "Organization", FieldNames: []string{"id", "priority"}}, + } + + scalarMismatchConfig := mustConfiguration(t, graphql_datasource.ConfigurationInput{ + Fetch: &graphql_datasource.FetchConfiguration{ + URL: "https://example.com/", + Method: "POST", + }, + SchemaConfiguration: mustSchemaConfig(t, nil, scalarMismatchSchema), + }) + + scalarMismatchFieldConfig := []plan.FieldConfiguration{ + { + TypeName: "Query", + FieldName: "entity", + Path: []string{"entity"}, + }, + } + + t.Run("without relaxation flag, validation rejects different scalar types", runWithAndCompareError( + ExecutionEngineTestCase{ + schema: scalarMismatchParsedSchema, + operation: func(t *testing.T) graphql.Request { + return graphql.Request{ + OperationName: "O", + Query: `query O { entity { ... on User { priority } ... on Organization { priority } } }`, + } + }, + dataSources: []plan.DataSource{ + mustGraphqlDataSourceConfiguration(t, "ds-id", + mustFactory(t, + testNetHttpClient(t, roundTripperTestCase{ + expectedHost: "example.com", + expectedPath: "/", + expectedBody: "", + sendResponseBody: `{"data":{"entity":{"__typename":"User","priority":1}}}`, + sendStatusCode: 200, + }), + ), + &plan.DataSourceMetadata{ + RootNodes: scalarMismatchRootNodes, + }, + scalarMismatchConfig, + ), + }, + fields: scalarMismatchFieldConfig, + }, + `fields 'priority' conflict because they return conflicting types 'Int' and 'String', locations: [], path: [query,entity,$1Organization]`, + )) + + t.Run("without relaxation flag, validation rejects different scalar types on union members", runWithAndCompareError( + ExecutionEngineTestCase{ + schema: schema, + operation: func(t *testing.T) graphql.Request { + return graphql.Request{ + OperationName: "O", + Query: `query O { updatable { ... on Issue { priority } ... on PullRequestReview { priority } } }`, + } + }, + dataSources: []plan.DataSource{ + mustGraphqlDataSourceConfiguration(t, "ds-id", + mustFactory(t, + testNetHttpClient(t, roundTripperTestCase{ + expectedHost: "example.com", + expectedPath: "/", + expectedBody: "", + sendResponseBody: `{"data":{"updatable":{"__typename":"Issue","priority":1}}}`, + sendStatusCode: 200, + }), + ), + &plan.DataSourceMetadata{ + RootNodes: rootNodes, + }, + customConfig, + ), + }, + fields: fieldConfig, + }, + `fields 'priority' conflict because they return conflicting types 'Int' and 'String', locations: [], path: [query,updatable,$1PullRequestReview]`, + )) + + t.Run("with relaxation flag, different enum types work", runWithoutError( + ExecutionEngineTestCase{ + schema: schema, + operation: func(t *testing.T) graphql.Request { + return graphql.Request{ + OperationName: "O", + Query: `query O { updatable { ... on Issue { state } ... on PullRequestReview { state } } }`, + } + }, + dataSources: []plan.DataSource{ + mustGraphqlDataSourceConfiguration(t, "ds-id", + mustFactory(t, + testNetHttpClient(t, roundTripperTestCase{ + expectedHost: "example.com", + expectedPath: "/", + expectedBody: "", + sendResponseBody: `{"data":{"updatable":{"__typename":"Issue","state":"OPEN"}}}`, + sendStatusCode: 200, + }), + ), + &plan.DataSourceMetadata{ + RootNodes: rootNodes, + }, + customConfig, + ), + }, + fields: fieldConfig, + expectedResponse: `{"data":{"updatable":{"state":"OPEN"}}}`, + }, + relaxFieldSelectionMergingTypeMismatch(), + )) + + t.Run("with both relaxation flags, different enum types and nullability work", runWithoutError( + ExecutionEngineTestCase{ + schema: schema, + operation: func(t *testing.T) graphql.Request { + return graphql.Request{ + OperationName: "O", + Query: `query O { updatable { ... on Issue { state title } ... on PullRequestReview { state title } } }`, + } + }, + dataSources: []plan.DataSource{ + mustGraphqlDataSourceConfiguration(t, "ds-id", + mustFactory(t, + testNetHttpClient(t, roundTripperTestCase{ + expectedHost: "example.com", + expectedPath: "/", + expectedBody: "", + sendResponseBody: `{"data":{"updatable":{"__typename":"Issue","state":"OPEN","title":"Bug report"}}}`, + sendStatusCode: 200, + }), + ), + &plan.DataSourceMetadata{ + RootNodes: rootNodes, + }, + customConfig, + ), + }, + fields: fieldConfig, + expectedResponse: `{"data":{"updatable":{"state":"OPEN","title":"Bug report"}}}`, + }, + relaxFieldSelectionMergingNullability(), + relaxFieldSelectionMergingTypeMismatch(), + )) + }) } func testNetHttpClient(t *testing.T, testCase roundTripperTestCase) *http.Client { diff --git a/v2/pkg/astvalidation/nullability_relaxation_justification.md b/v2/pkg/astvalidation/field_selection_merging_relaxation_justification.md similarity index 69% rename from v2/pkg/astvalidation/nullability_relaxation_justification.md rename to v2/pkg/astvalidation/field_selection_merging_relaxation_justification.md index 8233b60856..8c60aa16b3 100644 --- a/v2/pkg/astvalidation/nullability_relaxation_justification.md +++ b/v2/pkg/astvalidation/field_selection_merging_relaxation_justification.md @@ -1,4 +1,4 @@ -# Justification: Relaxed Nullability Validation on Non-Overlapping Concrete Types +# Justification: Relaxed Field Selection Merging on Non-Overlapping Concrete Types ## Problem Statement @@ -130,9 +130,12 @@ fails (e.g. `String!` vs `String`), we now check: - Same object type: YES (strict check) - Different object types: NO (relaxed check) -2. **If they can't overlap**: use `TypesAreCompatibleIgnoringNullability` which - strips `NonNull` wrappers at every nesting level but still requires the same - list structure and base named type. +2. **If they can't overlap** (nullability relaxation only): use + `TypesAreCompatibleIgnoringNullability` which strips `NonNull` wrappers at + every nesting level but still requires the same list structure and base named + type. Note: this "base types must match" guarantee applies only to + `RelaxNullabilityCheck`. The broader `RelaxTypeMismatchCheck` permits + entirely different base named types on non-overlapping concrete types. Existing test cases are unaffected: @@ -142,10 +145,47 @@ Existing test cases are unaffected: | `IntBox.scalar: Int` + `StringBox.scalar: String` | Different base types | **Still Invalid** | | All "112" tests with `String` vs `Int` on Dog/Cat | Different base types | **Still Invalid** | +## Type Mismatch Relaxation (RelaxTypeMismatchCheck) + +In addition to the nullability relaxation above, a broader relaxation is +available via `RelaxTypeMismatchCheck`. This allows **completely different types** +(e.g. `IssueState` vs `PullRequestReviewState`) on fields in non-overlapping +concrete object types. + +### Problem + +Schemas like GitHub's define interfaces (e.g. `Updatable`) whose implementing +types have same-named fields returning different leaf types: + +```graphql +interface Updatable { id: ID! } +type Issue implements Updatable { id: ID!, state: IssueState } +type PullRequestReview implements Updatable { id: ID!, state: PullRequestReviewState } +``` + +The spec's `SameResponseShape` algorithm rejects `state` across inline fragments +even though only one branch executes at runtime. + +### Safety + +The same `potentiallySameObject` guard applies: the relaxation only fires when +both enclosing types are different concrete Object types (never for interfaces). +At runtime, the response object can only match one concrete type, so different +field types cannot both appear for the same object. + +### Relationship to nullability relaxation + +`RelaxTypeMismatchCheck` is strictly broader than `RelaxNullabilityCheck`. When +the type mismatch flag is enabled on non-overlapping types, **all** type +differences (including nullability) are skipped because the code `continue`s +before reaching the nullability check. Both flags exist because callers may want +the narrower nullability-only relaxation without the broader type mismatch +relaxation. + ## Conclusion -This is a **deliberate, targeted deviation** from the GraphQL specification. It -addresses a real-world pain point that has been independently reported across -multiple GraphQL implementations. The relaxation is narrowly scoped (only -different concrete object types, only nullability differences) and preserves all -existing rejection behavior for genuinely conflicting types. +These are **deliberate, targeted deviations** from the GraphQL specification. +They address real-world pain points that have been independently reported across +multiple GraphQL implementations. Both relaxations are narrowly scoped (only +different concrete object types) and preserve all existing rejection behavior +for genuinely conflicting types on overlapping or same types. diff --git a/v2/pkg/astvalidation/operation_rule_field_selection_merging.go b/v2/pkg/astvalidation/operation_rule_field_selection_merging.go index 04dad67ecc..e49885e820 100644 --- a/v2/pkg/astvalidation/operation_rule_field_selection_merging.go +++ b/v2/pkg/astvalidation/operation_rule_field_selection_merging.go @@ -9,11 +9,40 @@ import ( "github.com/wundergraph/graphql-go-tools/v2/pkg/operationreport" ) +// FieldSelectionMergingConfig controls which spec-deviation relaxations are active. +// +// Note: RelaxTypeMismatchCheck is strictly broader than RelaxNullabilityCheck — +// when RelaxTypeMismatchCheck is true on non-overlapping types, all type differences +// (including nullability) are allowed, making RelaxNullabilityCheck redundant for +// those cases. Both flags exist because callers may want the narrower nullability-only +// relaxation without the broader type mismatch relaxation. +type FieldSelectionMergingConfig struct { + // RelaxNullabilityCheck allows differing nullability (e.g. String! vs String) + // on fields in non-overlapping concrete object types. + RelaxNullabilityCheck bool + // RelaxTypeMismatchCheck allows completely different types (e.g. IssueState vs + // PullRequestReviewState) on fields in non-overlapping concrete object types. + RelaxTypeMismatchCheck bool +} + // FieldSelectionMerging validates if field selections can be merged -func FieldSelectionMerging(relaxNullabilityCheck ...bool) Rule { - relax := len(relaxNullabilityCheck) > 0 && relaxNullabilityCheck[0] +func FieldSelectionMerging(config ...FieldSelectionMergingConfig) Rule { + var cfg FieldSelectionMergingConfig + if len(config) > 0 { + cfg = config[0] + } + return fieldSelectionMergingRule(&cfg) +} + +// fieldSelectionMergingRule creates the rule with a shared config pointer. +// The visitor reads from the pointer on each walk, so the caller can update +// the config between Validate calls without recreating the validator. +func fieldSelectionMergingRule(cfg *FieldSelectionMergingConfig) Rule { return func(walker *astvisitor.Walker) { - visitor := fieldSelectionMergingVisitor{Walker: walker, relaxNullabilityCheck: relax} + visitor := fieldSelectionMergingVisitor{ + Walker: walker, + config: cfg, + } walker.RegisterEnterDocumentVisitor(&visitor) walker.RegisterEnterFieldVisitor(&visitor) walker.RegisterEnterOperationVisitor(&visitor) @@ -28,7 +57,7 @@ type fieldSelectionMergingVisitor struct { scalarRequirements scalarRequirements nonScalarRequirements nonScalarRequirements refs []int - relaxNullabilityCheck bool + config *FieldSelectionMergingConfig } type nonScalarRequirement struct { path ast.Path @@ -119,13 +148,12 @@ func (f *fieldSelectionMergingVisitor) EnterField(ref int) { return } } else if !f.definition.TypesAreCompatibleDeep(f.nonScalarRequirements[i].fieldTypeRef, fieldType) { - // Deliberate deviation from SameResponseShape (spec sec 5.3.2): when enclosing - // types cannot overlap at runtime (two distinct concrete object types), - // we allow nullability differences because only one branch will ever - // contribute to the response. This is gated behind relaxNullabilityCheck. - if !f.relaxNullabilityCheck || - f.potentiallySameObject(f.nonScalarRequirements[i].enclosingTypeDefinition, f.EnclosingTypeDefinition) || - !f.definition.TypesAreCompatibleIgnoringNullability(f.nonScalarRequirements[i].fieldTypeRef, fieldType) { + switch f.checkTypeMismatch(f.nonScalarRequirements[i].enclosingTypeDefinition, f.nonScalarRequirements[i].fieldTypeRef, fieldType) { + case typeMismatchAccept: + // Types differ only in nullability; allowed by relaxation config. + case typeMismatchSkip: + continue + case typeMismatchReject: left, err := f.definition.PrintTypeBytes(f.nonScalarRequirements[i].fieldTypeRef, nil) if err != nil { f.StopWithInternalErr(err) @@ -172,13 +200,12 @@ func (f *fieldSelectionMergingVisitor) EnterField(ref int) { } } if !f.definition.TypesAreCompatibleDeep(f.scalarRequirements[i].fieldType, fieldType) { - // Per SameResponseShape (spec sec 5.3.2), when enclosing types cannot overlap - // at runtime (two distinct concrete object types), nullability differences are - // acceptable because only one branch will ever contribute to the response. - // This relaxation is gated behind the relaxNullabilityCheck flag. - if !f.relaxNullabilityCheck || - f.potentiallySameObject(f.scalarRequirements[i].enclosingTypeDefinition, f.EnclosingTypeDefinition) || - !f.definition.TypesAreCompatibleIgnoringNullability(f.scalarRequirements[i].fieldType, fieldType) { + switch f.checkTypeMismatch(f.scalarRequirements[i].enclosingTypeDefinition, f.scalarRequirements[i].fieldType, fieldType) { + case typeMismatchAccept: + // Types differ only in nullability; allowed by relaxation config. + case typeMismatchSkip: + continue + case typeMismatchReject: left, err := f.definition.PrintTypeBytes(f.scalarRequirements[i].fieldType, nil) if err != nil { f.StopWithInternalErr(err) @@ -213,6 +240,46 @@ func (f *fieldSelectionMergingVisitor) EnterField(ref int) { }) } +type typeMismatchResult int + +const ( + // typeMismatchAccept means the types differ only in nullability and the + // relaxation flag allowed it — no error, but keep processing this requirement. + typeMismatchAccept typeMismatchResult = iota + // typeMismatchSkip means the enclosing types are provably disjoint and + // the relaxation flag allowed the full type mismatch — skip this requirement. + typeMismatchSkip + // typeMismatchReject means the type difference must be reported as an error. + typeMismatchReject +) + +// checkTypeMismatch decides how to handle a type incompatibility between an +// existing requirement's field type and the current field's type. It applies +// type mismatch relaxation first (broader), then nullability relaxation (narrower). +// +// Examples: +// - typeMismatchAccept: User.email is String!, Organization.email is String +// → types differ only in nullability, RelaxNullabilityCheck allows it +// - typeMismatchSkip: Issue.state is IssueState, PullRequestReview.state is PullRequestReviewState +// → enclosing types are disjoint concrete objects, RelaxTypeMismatchCheck skips the check entirely +// - typeMismatchReject: NonNullStringBox1.scalar is String!, IntBox.scalar is Int +// → one enclosing type is an interface (could overlap), difference must be reported as error +func (f *fieldSelectionMergingVisitor) checkTypeMismatch(existingEnclosing ast.Node, existingFieldType, currentFieldType int) typeMismatchResult { + sameObject := f.potentiallySameObject(existingEnclosing, f.EnclosingTypeDefinition) + // Type mismatch relaxation (spec sec 5.3.2, SameResponseShape): when enclosing + // types are provably non-overlapping, any type difference is safe. + if f.config.RelaxTypeMismatchCheck && !sameObject { + return typeMismatchSkip + } + // Nullability relaxation (spec sec 5.3.2, SameResponseShape): when enclosing + // types cannot overlap at runtime, we allow nullability differences. + if f.config.RelaxNullabilityCheck && !sameObject && + f.definition.TypesAreCompatibleIgnoringNullability(existingFieldType, currentFieldType) { + return typeMismatchAccept + } + return typeMismatchReject +} + // potentiallySameObject reports whether two enclosing type definitions could apply // to the same runtime object. This determines whether field merging must enforce // strict type equality (including nullability) or may relax it. @@ -221,6 +288,11 @@ func (f *fieldSelectionMergingVisitor) EnterField(ref int) { // type might implement that interface). // - Two object types overlap only when they share the same name. // - All other combinations return false. +// +// Union types are not handled explicitly because the AST walker resolves inline +// fragment type conditions to their concrete member types before visiting fields; +// the enclosing type definition seen here is always the concrete type from the +// inline fragment, never the union itself. func (f *fieldSelectionMergingVisitor) potentiallySameObject(left, right ast.Node) bool { switch { case left.Kind == ast.NodeKindInterfaceTypeDefinition || right.Kind == ast.NodeKindInterfaceTypeDefinition: diff --git a/v2/pkg/astvalidation/operation_validation.go b/v2/pkg/astvalidation/operation_validation.go index e24a72a5be..7dc14969da 100644 --- a/v2/pkg/astvalidation/operation_validation.go +++ b/v2/pkg/astvalidation/operation_validation.go @@ -12,8 +12,9 @@ import ( ) type OperationValidatorOptions struct { - ApolloCompatibilityFlags apollocompatibility.Flags - RelaxFieldSelectionMergingNullabilityCheck bool + ApolloCompatibilityFlags apollocompatibility.Flags + RelaxFieldSelectionMergingNullabilityCheck bool + RelaxFieldSelectionMergingTypeMismatchCheck bool } func WithApolloCompatibilityFlags(flags apollocompatibility.Flags) Option { @@ -32,6 +33,16 @@ func WithRelaxFieldSelectionMergingNullability() Option { } } +// WithRelaxFieldSelectionMergingTypeMismatch enables a deliberate spec deviation that allows +// completely different types (e.g. IssueState vs PullRequestReviewState) on fields in +// non-overlapping concrete object types within inline fragments. Without this option, the +// validator enforces the strict GraphQL spec behavior where leaf types must be identical. +func WithRelaxFieldSelectionMergingTypeMismatch() Option { + return func(options *OperationValidatorOptions) { + options.RelaxFieldSelectionMergingTypeMismatchCheck = true + } +} + type Option func(options *OperationValidatorOptions) // DefaultOperationValidator returns a fully initialized OperationValidator with all default rules registered @@ -58,7 +69,11 @@ func DefaultOperationValidator(options ...Option) *OperationValidator { validator.RegisterRule(LoneAnonymousOperation()) validator.RegisterRule(SubscriptionSingleRootField()) validator.RegisterRule(FieldSelections()) - validator.RegisterRule(FieldSelectionMerging(opts.RelaxFieldSelectionMergingNullabilityCheck)) + validator.fieldSelectionMergingConfig = FieldSelectionMergingConfig{ + RelaxNullabilityCheck: opts.RelaxFieldSelectionMergingNullabilityCheck, + RelaxTypeMismatchCheck: opts.RelaxFieldSelectionMergingTypeMismatchCheck, + } + validator.RegisterRule(fieldSelectionMergingRule(&validator.fieldSelectionMergingConfig)) validator.RegisterRule(KnownArguments()) validator.RegisterRule(Values()) validator.RegisterRule(ArgumentUniqueness()) @@ -87,7 +102,8 @@ func NewOperationValidator(rules []Rule) *OperationValidator { // OperationValidator orchestrates the validation process of Operations type OperationValidator struct { - walker astvisitor.Walker + walker astvisitor.Walker + fieldSelectionMergingConfig FieldSelectionMergingConfig } // RegisterRule registers a rule to the OperationValidator @@ -95,6 +111,18 @@ func (o *OperationValidator) RegisterRule(rule Rule) { rule(&o.walker) } +// SetFieldSelectionMergingConfig updates the field selection merging relaxation flags. +// This allows reusing a single validator (e.g. from a sync.Pool) with different +// relaxation settings per call, instead of maintaining separate validator instances. +// +// Note: This only works when the validator was created via DefaultOperationValidator, +// which binds the FieldSelectionMerging rule to the validator's config field. +// Validators created via NewOperationValidator with FieldSelectionMerging() will +// have an independent config copy and will not be affected by this method. +func (o *OperationValidator) SetFieldSelectionMergingConfig(config FieldSelectionMergingConfig) { + o.fieldSelectionMergingConfig = config +} + // Validate validates the operation against the definition using the registered ruleset. func (o *OperationValidator) Validate(operation, definition *ast.Document, report *operationreport.Report) ValidationState { diff --git a/v2/pkg/astvalidation/operation_validation_test.go b/v2/pkg/astvalidation/operation_validation_test.go index 74783bae26..10e2480dc2 100644 --- a/v2/pkg/astvalidation/operation_validation_test.go +++ b/v2/pkg/astvalidation/operation_validation_test.go @@ -1084,7 +1084,7 @@ func TestExecutionValidation(t *testing.T) { ... on User { email } ... on Organization { email } } - }`, FieldSelectionMerging(true), Valid) + }`, FieldSelectionMerging(FieldSelectionMergingConfig{RelaxNullabilityCheck: true}), Valid) }) t.Run("rejects differing scalar nullability on non-overlapping object types without relaxation flag", func(t *testing.T) { runWithDefinition(t, entityDefinition, ` @@ -1103,7 +1103,7 @@ func TestExecutionValidation(t *testing.T) { ... on User { profile { name } } ... on Organization { profile { name } } } - }`, FieldSelectionMerging(true), Valid) + }`, FieldSelectionMerging(FieldSelectionMergingConfig{RelaxNullabilityCheck: true}), Valid) }) t.Run("rejects differing non-scalar nullability on non-overlapping object types without relaxation flag", func(t *testing.T) { runWithDefinition(t, entityDefinition, ` @@ -1129,6 +1129,146 @@ func TestExecutionValidation(t *testing.T) { }`, FieldSelectionMerging(), Invalid, withValidationErrors(`fields 'scalar' conflict because they return conflicting types 'String!' and 'String'`)) }) + // Type mismatch relaxation tests + t.Run("rejects differing enum types on union members without type mismatch flag", func(t *testing.T) { + runWithDefinition(t, typeMismatchDefinition, ` + { + entity { + ... on User { priority } + ... on Organization { priority } + } + }`, FieldSelectionMerging(), Invalid, + withValidationErrors(`fields 'priority' conflict because they return conflicting types 'Int' and 'String'`)) + }) + t.Run("allows differing enum types on union members with type mismatch flag", func(t *testing.T) { + runWithDefinition(t, typeMismatchDefinition, ` + { + searchable { + ... on Issue { state } + ... on PullRequestReview { state } + } + }`, FieldSelectionMerging(FieldSelectionMergingConfig{RelaxTypeMismatchCheck: true}), Valid) + }) + t.Run("allows differing base types on union members with type mismatch flag", func(t *testing.T) { + runWithDefinition(t, typeMismatchDefinition, ` + { + entity { + ... on User { priority } + ... on Organization { priority } + } + }`, FieldSelectionMerging(FieldSelectionMergingConfig{RelaxTypeMismatchCheck: true}), Valid) + }) + t.Run("rejects differing types when enclosing type is interface (could overlap)", func(t *testing.T) { + // NonNullStringBox1 is an interface (scalar: String!), IntBox is a + // concrete type (scalar: Int). Because one enclosing type is an + // interface, potentiallySameObject returns true (conservative) and + // the relaxation must NOT apply. + runWithDefinition(t, boxDefinition, ` + { + someBox { + ... on NonNullStringBox1 { + scalar + } + ... on IntBox { + scalar + } + } + }`, FieldSelectionMerging(FieldSelectionMergingConfig{RelaxTypeMismatchCheck: true}), Invalid, + withValidationErrors(`fields 'scalar' conflict because they return conflicting types 'String!' and 'Int'`)) + }) + t.Run("allows differing enum types on interface implementors with type mismatch flag", func(t *testing.T) { + // This is the exact GitHub use case: Updatable is an interface, + // Issue and PullRequestReview implement it with different enum types + // for the state field. The inline fragments narrow to concrete types. + runWithDefinition(t, typeMismatchDefinition, ` + { + updatable { + ... on Issue { state } + ... on PullRequestReview { state } + } + }`, FieldSelectionMerging(FieldSelectionMergingConfig{RelaxTypeMismatchCheck: true}), Valid) + }) + t.Run("type mismatch flag also covers nullability differences on disjoint types", func(t *testing.T) { + runWithDefinition(t, entityDefinition, ` + { + entity { + ... on User { email } + ... on Organization { email } + } + }`, FieldSelectionMerging(FieldSelectionMergingConfig{RelaxTypeMismatchCheck: true}), Valid) + }) + t.Run("only nullability flag does not cover type mismatches", func(t *testing.T) { + runWithDefinition(t, typeMismatchDefinition, ` + { + entity { + ... on User { priority } + ... on Organization { priority } + } + }`, FieldSelectionMerging(FieldSelectionMergingConfig{RelaxNullabilityCheck: true}), Invalid, + withValidationErrors(`fields 'priority' conflict because they return conflicting types 'Int' and 'String'`)) + }) + t.Run("both flags enabled together", func(t *testing.T) { + runWithDefinition(t, typeMismatchDefinition, ` + { + entity { + ... on User { priority } + ... on Organization { priority } + } + }`, FieldSelectionMerging(FieldSelectionMergingConfig{ + RelaxNullabilityCheck: true, + RelaxTypeMismatchCheck: true, + }), Valid) + }) + // Narrow-scope tests: verify that the type mismatch relaxation is limited to + // disjoint concrete object types. These checks must still fail with the flag enabled. + t.Run("with type mismatch flag, nullability on overlapping interface types still rejects", func(t *testing.T) { + // Even a pure nullability difference (String! vs String) is rejected + // when one enclosing type is an interface (could overlap at runtime). + runWithDefinition(t, boxDefinition, ` + { + someBox { + ... on NonNullStringBox1 { scalar } + ... on StringBox { scalar } + } + }`, FieldSelectionMerging(FieldSelectionMergingConfig{RelaxTypeMismatchCheck: true}), Invalid, + withValidationErrors(`fields 'scalar' conflict because they return conflicting types 'String!' and 'String'`)) + }) + t.Run("with both flags, field selected at interface level conflicts with concrete fragment field", func(t *testing.T) { + // SomeBox.scalar is String (interface), IntBox.scalar is Int (concrete fragment). + // The interface-level selection causes potentiallySameObject to return true, + // so relaxation does not apply even with both flags enabled. + runWithDefinition(t, boxDefinition, ` + { + someBox { + scalar + ... on IntBox { scalar } + } + }`, FieldSelectionMerging(FieldSelectionMergingConfig{ + RelaxNullabilityCheck: true, + RelaxTypeMismatchCheck: true, + }), Invalid, + withValidationErrors(`fields 'scalar' conflict because they return conflicting types 'String' and 'Int'`)) + }) + t.Run("with both flags, type mismatch on same concrete type still rejects", func(t *testing.T) { + // IntBox.scalar is Int, StringBox.scalar is String. Both concrete types + // implement SomeBox, but the type condition in inline fragments narrows + // to concrete types. Without the inline fragments the scalar is accessed + // at the SomeBox interface level where it's String. + // Test that the field selected at the interface level (String) conflicts + // with the concrete fragment field (Int) even with both flags. + runWithDefinition(t, boxDefinition, ` + { + someBox { + scalar + ... on StringBox { scalar } + ... on IntBox { scalar } + } + }`, FieldSelectionMerging(FieldSelectionMergingConfig{ + RelaxNullabilityCheck: true, + RelaxTypeMismatchCheck: true, + }), Invalid, + withValidationErrors(`fields 'scalar' conflict because they return conflicting types 'String' and 'Int'`)) + }) t.Run("same wrapped scalar return types", func(t *testing.T) { runWithDefinition(t, boxDefinition, ` { @@ -5734,6 +5874,52 @@ schema { query: Query }` +const typeMismatchDefinition = ` +scalar String +scalar Int +scalar ID + +enum IssueState { OPEN CLOSED } +enum PullRequestReviewState { PENDING APPROVED } + +interface Updatable { + id: ID! +} + +type Issue implements Updatable { + id: ID! + state: IssueState +} + +type PullRequestReview implements Updatable { + id: ID! + state: PullRequestReviewState +} + +type User { + id: ID! + priority: Int +} + +type Organization { + id: ID! + priority: String +} + +union Entity = User | Organization + +union Searchable = Issue | PullRequestReview + +type Query { + updatable: Updatable + entity: Entity + searchable: Searchable +} + +schema { + query: Query +}` + const countriesDefinition = `directive @cacheControl(maxAge: Int, scope: CacheControlScope) on FIELD_DEFINITION | OBJECT | INTERFACE scalar String @@ -12401,3 +12587,84 @@ input UpdateRegionGameInput { input UserPreferencesInput { notifications: PreNotificationsInput! }` + +// TestSetFieldSelectionMergingConfig proves that a single DefaultOperationValidator +// can be reconfigured between Validate calls. This is the foundation for using a +// single sync.Pool of validators instead of one pool per relaxation-flag combination. +func TestSetFieldSelectionMergingConfig(t *testing.T) { + // Helper: parse and normalize, then validate with the given validator. + validate := func(t *testing.T, validator *OperationValidator, definitionStr, operationStr string) ValidationState { + t.Helper() + definition, report := astparser.ParseGraphqlDocumentString(definitionStr) + require.False(t, report.HasErrors(), report.Error()) + operation, report2 := astparser.ParseGraphqlDocumentString(operationStr) + require.False(t, report2.HasErrors(), report2.Error()) + var validationReport operationreport.Report + normalizer := astnormalization.NewWithOpts(astnormalization.WithInlineFragmentSpreads()) + normalizer.NormalizeOperation(&operation, &definition, &validationReport) + return validator.Validate(&operation, &definition, &validationReport) + } + + // nullabilityOp: User.email is String!, Organization.email is String → differs in nullability + const nullabilityOp = `{ entity { ... on User { email } ... on Organization { email } } }` + // typeMismatchOp: User.priority is Int, Organization.priority is String → completely different types + const typeMismatchOp = `{ entity { ... on User { priority } ... on Organization { priority } } }` + + t.Run("nullability relaxation cycle", func(t *testing.T) { + validator := DefaultOperationValidator() + + // Strict (default) → reject + require.Equal(t, Invalid, validate(t, validator, entityDefinition, nullabilityOp)) + + // Enable nullability relaxation → accept + validator.SetFieldSelectionMergingConfig(FieldSelectionMergingConfig{RelaxNullabilityCheck: true}) + require.Equal(t, Valid, validate(t, validator, entityDefinition, nullabilityOp)) + + // Reset to strict → reject again + validator.SetFieldSelectionMergingConfig(FieldSelectionMergingConfig{}) + require.Equal(t, Invalid, validate(t, validator, entityDefinition, nullabilityOp)) + }) + + t.Run("type mismatch relaxation cycle", func(t *testing.T) { + validator := DefaultOperationValidator() + + // Strict → reject + require.Equal(t, Invalid, validate(t, validator, typeMismatchDefinition, typeMismatchOp)) + + // Enable type mismatch relaxation → accept + validator.SetFieldSelectionMergingConfig(FieldSelectionMergingConfig{RelaxTypeMismatchCheck: true}) + require.Equal(t, Valid, validate(t, validator, typeMismatchDefinition, typeMismatchOp)) + + // Reset → reject + validator.SetFieldSelectionMergingConfig(FieldSelectionMergingConfig{}) + require.Equal(t, Invalid, validate(t, validator, typeMismatchDefinition, typeMismatchOp)) + }) + + t.Run("both flags together", func(t *testing.T) { + validator := DefaultOperationValidator() + + // Both operations fail under strict + require.Equal(t, Invalid, validate(t, validator, entityDefinition, nullabilityOp)) + require.Equal(t, Invalid, validate(t, validator, typeMismatchDefinition, typeMismatchOp)) + + // Enable both → both pass + validator.SetFieldSelectionMergingConfig(FieldSelectionMergingConfig{ + RelaxNullabilityCheck: true, + RelaxTypeMismatchCheck: true, + }) + require.Equal(t, Valid, validate(t, validator, entityDefinition, nullabilityOp)) + require.Equal(t, Valid, validate(t, validator, typeMismatchDefinition, typeMismatchOp)) + }) + + t.Run("nullability flag alone does not cover type mismatches", func(t *testing.T) { + validator := DefaultOperationValidator() + validator.SetFieldSelectionMergingConfig(FieldSelectionMergingConfig{RelaxNullabilityCheck: true}) + require.Equal(t, Invalid, validate(t, validator, typeMismatchDefinition, typeMismatchOp)) + }) + + t.Run("type mismatch flag covers nullability differences", func(t *testing.T) { + validator := DefaultOperationValidator() + validator.SetFieldSelectionMergingConfig(FieldSelectionMergingConfig{RelaxTypeMismatchCheck: true}) + require.Equal(t, Valid, validate(t, validator, entityDefinition, nullabilityOp)) + }) +} diff --git a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go index f4268d1f6a..7a93f42f4f 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go +++ b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go @@ -93,6 +93,9 @@ type Planner[T Configuration] struct { // gRPC grpcClient grpc.ClientConnInterface + relaxNullabilityCheck bool + relaxTypeMismatchCheck bool + printKitPool *sync.Pool } @@ -1468,6 +1471,10 @@ func (p *Planner[T]) printOperation() (operationBytes []byte, variablesBytes []b copy(variablesBytes, p.upstreamOperation.Input.Variables) } + kit.validator.SetFieldSelectionMergingConfig(astvalidation.FieldSelectionMergingConfig{ + RelaxNullabilityCheck: p.relaxNullabilityCheck, + RelaxTypeMismatchCheck: p.relaxTypeMismatchCheck, + }) kit.validator.Validate(p.upstreamOperation, definition, kit.report) if kit.report.HasErrors() { p.stopWithError(errors.WithStack(fmt.Errorf("printOperation planner id: %d: validation failed: %w", p.id, kit.report))) @@ -1687,10 +1694,10 @@ type printKit struct { report *operationreport.Report } -func newPrintKitPool(validationOptions ...astvalidation.Option) *sync.Pool { +func newPrintKitPool() *sync.Pool { return &sync.Pool{ New: func() any { - validator := astvalidation.DefaultOperationValidator(validationOptions...) + validator := astvalidation.DefaultOperationValidator() // as we are creating operation programmatically in the graphql datasource planner, // we need to catch incorrect behavior of the planner // as graphql datasource planner should visit only selection sets which has fields, @@ -1714,26 +1721,25 @@ func newPrintKitPool(validationOptions ...astvalidation.Option) *sync.Pool { } } -var ( - defaultPrintKitPool = newPrintKitPool() - relaxedPrintKitPool *sync.Pool - relaxedPrintKitPoolOnce sync.Once -) - -func getRelaxedPrintKitPool() *sync.Pool { - relaxedPrintKitPoolOnce.Do(func() { - relaxedPrintKitPool = newPrintKitPool(astvalidation.WithRelaxFieldSelectionMergingNullability()) - }) - return relaxedPrintKitPool -} - type Factory[T Configuration] struct { executionContext context.Context httpClient *http.Client grpcClient grpc.ClientConnInterface grpcClientProvider func() grpc.ClientConnInterface subscriptionClient GraphQLSubscriptionClient - printKitPool *sync.Pool + + relaxNullabilityCheck bool + relaxTypeMismatchCheck bool + + printKitPool *sync.Pool + poolOnce sync.Once +} + +func (f *Factory[T]) getPrintKitPool() *sync.Pool { + f.poolOnce.Do(func() { + f.printKitPool = newPrintKitPool() + }) + return f.printKitPool } // NewFactory (HTTP) creates a new factory for the GraphQL datasource planner @@ -1796,36 +1802,25 @@ func NewFactoryGRPCClientProvider(executionContext context.Context, clientProvid } func (p *Planner[T]) getKit() *printKit { - pool := p.printKitPool - if pool == nil { - pool = defaultPrintKitPool - } - return pool.Get().(*printKit) + return p.printKitPool.Get().(*printKit) } func (p *Planner[T]) releaseKit(kit *printKit) { kit.buf.Reset() kit.report.Reset() - pool := p.printKitPool - if pool == nil { - pool = defaultPrintKitPool - } - pool.Put(kit) + p.printKitPool.Put(kit) } // EnableSubgraphFieldSelectionMergingNullabilityRelaxation implements -// plan.SubgraphFieldSelectionMergingNullabilityRelaxer. It configures the -// factory to use a shared pool whose validator allows differing nullability -// on fields in non-overlapping concrete types. +// plan.SubgraphFieldSelectionMergingNullabilityRelaxer. func (f *Factory[T]) EnableSubgraphFieldSelectionMergingNullabilityRelaxation() { - f.printKitPool = getRelaxedPrintKitPool() + f.relaxNullabilityCheck = true } -func (f *Factory[T]) getPrintKitPool() *sync.Pool { - if f.printKitPool != nil { - return f.printKitPool - } - return defaultPrintKitPool +// EnableSubgraphFieldSelectionMergingTypeMismatchRelaxation implements +// plan.SubgraphFieldSelectionMergingTypeMismatchRelaxer. +func (f *Factory[T]) EnableSubgraphFieldSelectionMergingTypeMismatchRelaxation() { + f.relaxTypeMismatchCheck = true } func (f *Factory[T]) Planner(logger abstractlogger.Logger) plan.DataSourcePlanner[T] { @@ -1835,10 +1830,12 @@ func (f *Factory[T]) Planner(logger abstractlogger.Logger) plan.DataSourcePlanne } return &Planner[T]{ - fetchClient: f.httpClient, - grpcClient: grpcClient, - subscriptionClient: f.subscriptionClient, - printKitPool: f.getPrintKitPool(), + fetchClient: f.httpClient, + grpcClient: grpcClient, + subscriptionClient: f.subscriptionClient, + relaxNullabilityCheck: f.relaxNullabilityCheck, + relaxTypeMismatchCheck: f.relaxTypeMismatchCheck, + printKitPool: f.getPrintKitPool(), } } diff --git a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_test.go b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_test.go index 05b07df9e1..5ab19a37f7 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_test.go +++ b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_test.go @@ -8366,6 +8366,283 @@ func TestGraphQLDataSource(t *testing.T) { }) } +func TestFactorySinglePrintKitPool(t *testing.T) { + factory := &Factory[Configuration]{} + + p1 := factory.Planner(nil).(*Planner[Configuration]) + p2 := factory.Planner(nil).(*Planner[Configuration]) + p3 := factory.Planner(nil).(*Planner[Configuration]) + + // All planners must share the exact same pool instance. + require.Same(t, p1.printKitPool, p2.printKitPool) + require.Same(t, p2.printKitPool, p3.printKitPool) + + // Get and return a kit to verify the pool works. + kit := p1.getKit() + require.NotNil(t, kit) + p2.releaseKit(kit) + + // Getting from another planner may return the same kit (same pool). + kit2 := p3.getKit() + require.NotNil(t, kit2) + p3.releaseKit(kit2) +} + +func TestFieldSelectionMergingRelaxationCombinations(t *testing.T) { + // Schema where User.priority is Int and Organization.priority is String (type mismatch) + // and User.email is String! vs Organization.email is String (nullability mismatch). + definition := ` + type User { + id: ID! + email: String! + priority: Int + } + + type Organization { + id: ID! + email: String + priority: String + } + + union Entity = User | Organization + + type Query { + entity: Entity + } + ` + + makePlanConfig := func(t *testing.T, relaxNullability, relaxTypeMismatch bool) plan.Configuration { + t.Helper() + return plan.Configuration{ + DataSources: []plan.DataSource{ + mustDataSourceConfiguration( + t, + "ds-id", + &plan.DataSourceMetadata{ + RootNodes: []plan.TypeField{ + {TypeName: "Query", FieldNames: []string{"entity"}}, + }, + ChildNodes: []plan.TypeField{ + {TypeName: "User", FieldNames: []string{"id", "email", "priority"}}, + {TypeName: "Organization", FieldNames: []string{"id", "email", "priority"}}, + }, + }, + mustCustomConfiguration(t, ConfigurationInput{ + Fetch: &FetchConfiguration{URL: "https://example.com/graphql"}, + SchemaConfiguration: mustSchema(t, nil, definition), + }), + ), + }, + DisableResolveFieldPositions: true, + RelaxSubgraphOperationFieldSelectionMergingNullability: relaxNullability, + RelaxSubgraphOperationFieldSelectionMergingTypeMismatch: relaxTypeMismatch, + } + } + + t.Run("nullability relaxation only", func(t *testing.T) { + t.Run("run", RunTest(definition, ` + query MyQuery { + entity { + ... on User { email } + ... on Organization { email } + } + }`, + "MyQuery", + &plan.SynchronousResponsePlan{ + Response: &resolve.GraphQLResponse{ + Fetches: resolve.Sequence( + resolve.Single(&resolve.SingleFetch{ + FetchConfiguration: resolve.FetchConfiguration{ + DataSource: &Source{}, + Input: `{"method":"POST","url":"https://example.com/graphql","body":{"query":"{entity {__typename ... on User {email} ... on Organization {email}}}"}}`, + PostProcessing: DefaultPostProcessingConfiguration, + }, + DataSourceIdentifier: []byte("graphql_datasource.Source"), + })), + Data: &resolve.Object{ + Fields: []*resolve.Field{ + { + Name: []byte("entity"), + Value: &resolve.Object{ + Path: []string{"entity"}, + Nullable: true, + PossibleTypes: map[string]struct{}{ + "User": {}, + "Organization": {}, + }, + TypeName: "Entity", + Fields: []*resolve.Field{ + { + Name: []byte("email"), + Value: &resolve.String{ + Path: []string{"email"}, + }, + OnTypeNames: [][]byte{[]byte("User")}, + }, + { + Name: []byte("email"), + Value: &resolve.String{ + Path: []string{"email"}, + Nullable: true, + }, + OnTypeNames: [][]byte{[]byte("Organization")}, + }, + }, + }, + }, + }, + }, + }, + }, + makePlanConfig(t, true, false), + WithDefaultPostProcessor(), + WithValidationOptions(astvalidation.WithRelaxFieldSelectionMergingNullability()), + )) + }) + + t.Run("type mismatch relaxation only", func(t *testing.T) { + t.Run("run", RunTest(definition, ` + query MyQuery { + entity { + ... on User { priority } + ... on Organization { priority } + } + }`, + "MyQuery", + &plan.SynchronousResponsePlan{ + Response: &resolve.GraphQLResponse{ + Fetches: resolve.Sequence( + resolve.Single(&resolve.SingleFetch{ + FetchConfiguration: resolve.FetchConfiguration{ + DataSource: &Source{}, + Input: `{"method":"POST","url":"https://example.com/graphql","body":{"query":"{entity {__typename ... on User {priority} ... on Organization {priority}}}"}}`, + PostProcessing: DefaultPostProcessingConfiguration, + }, + DataSourceIdentifier: []byte("graphql_datasource.Source"), + })), + Data: &resolve.Object{ + Fields: []*resolve.Field{ + { + Name: []byte("entity"), + Value: &resolve.Object{ + Path: []string{"entity"}, + Nullable: true, + PossibleTypes: map[string]struct{}{ + "User": {}, + "Organization": {}, + }, + TypeName: "Entity", + Fields: []*resolve.Field{ + { + Name: []byte("priority"), + Value: &resolve.Integer{ + Path: []string{"priority"}, + Nullable: true, + }, + OnTypeNames: [][]byte{[]byte("User")}, + }, + { + Name: []byte("priority"), + Value: &resolve.String{ + Path: []string{"priority"}, + Nullable: true, + }, + OnTypeNames: [][]byte{[]byte("Organization")}, + }, + }, + }, + }, + }, + }, + }, + }, + makePlanConfig(t, false, true), + WithDefaultPostProcessor(), + WithValidationOptions(astvalidation.WithRelaxFieldSelectionMergingTypeMismatch()), + )) + }) + + t.Run("both relaxations", func(t *testing.T) { + t.Run("run", RunTest(definition, ` + query MyQuery { + entity { + ... on User { email priority } + ... on Organization { email priority } + } + }`, + "MyQuery", + &plan.SynchronousResponsePlan{ + Response: &resolve.GraphQLResponse{ + Fetches: resolve.Sequence( + resolve.Single(&resolve.SingleFetch{ + FetchConfiguration: resolve.FetchConfiguration{ + DataSource: &Source{}, + Input: `{"method":"POST","url":"https://example.com/graphql","body":{"query":"{entity {__typename ... on User {email priority} ... on Organization {email priority}}}"}}`, + PostProcessing: DefaultPostProcessingConfiguration, + }, + DataSourceIdentifier: []byte("graphql_datasource.Source"), + })), + Data: &resolve.Object{ + Fields: []*resolve.Field{ + { + Name: []byte("entity"), + Value: &resolve.Object{ + Path: []string{"entity"}, + Nullable: true, + PossibleTypes: map[string]struct{}{ + "User": {}, + "Organization": {}, + }, + TypeName: "Entity", + Fields: []*resolve.Field{ + { + Name: []byte("email"), + Value: &resolve.String{ + Path: []string{"email"}, + }, + OnTypeNames: [][]byte{[]byte("User")}, + }, + { + Name: []byte("priority"), + Value: &resolve.Integer{ + Path: []string{"priority"}, + Nullable: true, + }, + OnTypeNames: [][]byte{[]byte("User")}, + }, + { + Name: []byte("email"), + Value: &resolve.String{ + Path: []string{"email"}, + Nullable: true, + }, + OnTypeNames: [][]byte{[]byte("Organization")}, + }, + { + Name: []byte("priority"), + Value: &resolve.String{ + Path: []string{"priority"}, + Nullable: true, + }, + OnTypeNames: [][]byte{[]byte("Organization")}, + }, + }, + }, + }, + }, + }, + }, + }, + makePlanConfig(t, true, true), + WithDefaultPostProcessor(), + WithValidationOptions( + astvalidation.WithRelaxFieldSelectionMergingNullability(), + astvalidation.WithRelaxFieldSelectionMergingTypeMismatch(), + ), + )) + }) +} + var errSubscriptionClientFail = errors.New("subscription client fail error") type FailingSubscriptionClient struct{} diff --git a/v2/pkg/engine/plan/configuration.go b/v2/pkg/engine/plan/configuration.go index eebd9df352..aa8e63957b 100644 --- a/v2/pkg/engine/plan/configuration.go +++ b/v2/pkg/engine/plan/configuration.go @@ -59,6 +59,13 @@ type Configuration struct { // When true, factories that support it will allow differing nullability // (e.g. String! vs String) on fields in non-overlapping inline fragments. RelaxSubgraphOperationFieldSelectionMergingNullability bool + + // RelaxSubgraphOperationFieldSelectionMergingTypeMismatch relaxes the type mismatch + // validation for field selection merging in upstream (subgraph) operations when enclosing + // types are non-overlapping concrete object types. This is a deliberate spec deviation. + // When true, factories that support it will allow completely different types + // (e.g. IssueState vs PullRequestReviewState) on fields in non-overlapping inline fragments. + RelaxSubgraphOperationFieldSelectionMergingTypeMismatch bool } type DebugConfiguration struct { diff --git a/v2/pkg/engine/plan/datasource_configuration.go b/v2/pkg/engine/plan/datasource_configuration.go index 0601e3abfb..49ee30c67a 100644 --- a/v2/pkg/engine/plan/datasource_configuration.go +++ b/v2/pkg/engine/plan/datasource_configuration.go @@ -305,6 +305,11 @@ func (d *dataSourceConfiguration[T]) CreatePlannerConfiguration(logger abstractl relaxer.EnableSubgraphFieldSelectionMergingNullabilityRelaxation() } } + if configuration.RelaxSubgraphOperationFieldSelectionMergingTypeMismatch { + if relaxer, ok := d.factory.(SubgraphFieldSelectionMergingTypeMismatchRelaxer); ok { + relaxer.EnableSubgraphFieldSelectionMergingTypeMismatchRelaxation() + } + } planner := d.factory.Planner(logger) @@ -489,6 +494,14 @@ type SubgraphFieldSelectionMergingNullabilityRelaxer interface { EnableSubgraphFieldSelectionMergingNullabilityRelaxation() } +// SubgraphFieldSelectionMergingTypeMismatchRelaxer is an optional interface that a PlannerFactory +// can implement to support relaxed type mismatch checks when validating upstream operations. +// When called, the factory should configure its internal validator to allow completely different +// types on fields in non-overlapping concrete types. +type SubgraphFieldSelectionMergingTypeMismatchRelaxer interface { + EnableSubgraphFieldSelectionMergingTypeMismatchRelaxation() +} + type DataSourcePlanner[T any] interface { DataSourceFetchPlanner DataSourceBehavior