diff --git a/execution/engine/execution_engine.go b/execution/engine/execution_engine.go index e920e5e00f..5927f3eb70 100644 --- a/execution/engine/execution_engine.go +++ b/execution/engine/execution_engine.go @@ -136,6 +136,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 3b15c6dce4..d73cfaeb1a 100644 --- a/execution/engine/execution_engine_test.go +++ b/execution/engine/execution_engine_test.go @@ -224,6 +224,7 @@ type _executionTestOptions struct { validateRequiredExternalFields bool computeStaticCost bool relaxFieldSelectionMergingNullability bool + relaxFieldSelectionMergingTypeMismatch bool } type executionTestOptions func(*_executionTestOptions) @@ -260,6 +261,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() @@ -297,6 +304,7 @@ func TestExecutionEngine_Execute(t *testing.T) { engineConf.plannerConfig.ComputeStaticCost = opts.computeStaticCost engineConf.plannerConfig.StaticCostDefaultListSize = 10 engineConf.plannerConfig.RelaxSubgraphOperationFieldSelectionMergingNullability = opts.relaxFieldSelectionMergingNullability + engineConf.plannerConfig.RelaxSubgraphOperationFieldSelectionMergingTypeMismatch = opts.relaxFieldSelectionMergingTypeMismatch resolveOpts := resolve.ResolverOptions{ MaxConcurrency: 1024, ResolvableOptions: opts.resolvableOptions, @@ -6916,7 +6924,7 @@ func TestExecutionEngine_Execute(t *testing.T) { relaxFieldSelectionMergingNullability(), )) - t.Run("null email from Organization type", runWithoutError( + t.Run("null email from Organization type with nullability relaxation", runWithoutError( ExecutionEngineTestCase{ schema: schema, operation: func(t *testing.T) graphql.Request { @@ -6948,6 +6956,134 @@ func TestExecutionEngine_Execute(t *testing.T) { relaxFieldSelectionMergingNullability(), )) }) + + t.Run("field merging with different types on non-overlapping union types", func(t *testing.T) { + unionSchema := ` + union Entity = User | Organization + type Query { entity: Entity } + type User { id: ID!, score: Int } + type Organization { id: ID!, score: String } + ` + schema, err := graphql.NewSchemaFromString(unionSchema) + require.NoError(t, err) + + rootNodes := []plan.TypeField{ + {TypeName: "Query", FieldNames: []string{"entity"}}, + {TypeName: "User", FieldNames: []string{"id", "score"}}, + {TypeName: "Organization", FieldNames: []string{"id", "score"}}, + } + + 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: "entity", + Path: []string{"entity"}, + }, + } + + t.Run("without relaxation flag, validation rejects differing types", runWithAndCompareError( + ExecutionEngineTestCase{ + schema: schema, + operation: func(t *testing.T) graphql.Request { + return graphql.Request{ + OperationName: "O", + Query: `query O { entity { ... on User { score } ... on Organization { score } } }`, + } + }, + dataSources: []plan.DataSource{ + mustGraphqlDataSourceConfiguration(t, "ds-id", + mustFactory(t, + testNetHttpClient(t, roundTripperTestCase{ + expectedHost: "example.com", + expectedPath: "/", + expectedBody: "", + sendResponseBody: `{"data":{"entity":{"__typename":"User","score":42}}}`, + sendStatusCode: 200, + }), + ), + &plan.DataSourceMetadata{ + RootNodes: rootNodes, + }, + customConfig, + ), + }, + fields: fieldConfig, + }, + `fields 'score' conflict because they return conflicting types 'Int' and 'String', locations: [], path: [query,entity,$1Organization]`, + )) + + t.Run("integer score from User type with type mismatch relaxation", runWithoutError( + ExecutionEngineTestCase{ + schema: schema, + operation: func(t *testing.T) graphql.Request { + return graphql.Request{ + OperationName: "O", + Query: `query O { entity { ... on User { score } ... on Organization { score } } }`, + } + }, + dataSources: []plan.DataSource{ + mustGraphqlDataSourceConfiguration(t, "ds-id", + mustFactory(t, + testNetHttpClient(t, roundTripperTestCase{ + expectedHost: "example.com", + expectedPath: "/", + expectedBody: "", + sendResponseBody: `{"data":{"entity":{"__typename":"User","score":42}}}`, + sendStatusCode: 200, + }), + ), + &plan.DataSourceMetadata{ + RootNodes: rootNodes, + }, + customConfig, + ), + }, + fields: fieldConfig, + expectedResponse: `{"data":{"entity":{"score":42}}}`, + }, + relaxFieldSelectionMergingTypeMismatch(), + )) + + t.Run("string score from Organization type with type mismatch relaxation", runWithoutError( + ExecutionEngineTestCase{ + schema: schema, + operation: func(t *testing.T) graphql.Request { + return graphql.Request{ + OperationName: "O", + Query: `query O { entity { ... on User { score } ... on Organization { score } } }`, + } + }, + dataSources: []plan.DataSource{ + mustGraphqlDataSourceConfiguration(t, "ds-id", + mustFactory(t, + testNetHttpClient(t, roundTripperTestCase{ + expectedHost: "example.com", + expectedPath: "/", + expectedBody: "", + sendResponseBody: `{"data":{"entity":{"__typename":"Organization","score":"high"}}}`, + sendStatusCode: 200, + }), + ), + &plan.DataSourceMetadata{ + RootNodes: rootNodes, + }, + customConfig, + ), + }, + fields: fieldConfig, + expectedResponse: `{"data":{"entity":{"score":"high"}}}`, + }, + relaxFieldSelectionMergingTypeMismatch(), + )) + }) } func testNetHttpClient(t *testing.T, testCase roundTripperTestCase) *http.Client { diff --git a/v2/pkg/astvalidation/operation_rule_field_selection_merging.go b/v2/pkg/astvalidation/operation_rule_field_selection_merging.go index 04dad67ecc..d6db6185f7 100644 --- a/v2/pkg/astvalidation/operation_rule_field_selection_merging.go +++ b/v2/pkg/astvalidation/operation_rule_field_selection_merging.go @@ -9,11 +9,31 @@ import ( "github.com/wundergraph/graphql-go-tools/v2/pkg/operationreport" ) +// FieldSelectionMergingOptions configures relaxation flags for the FieldSelectionMerging rule. +// +// RelaxNullabilityCheck allows fields with differing nullability (e.g. String! vs String) +// on non-overlapping concrete object types within inline fragments. +// +// RelaxTypeMismatchCheck is a strict superset: it allows completely differing field types +// (e.g. Int vs String) on non-overlapping concrete types. When set, RelaxNullabilityCheck +// has no additional effect. +type FieldSelectionMergingOptions struct { + RelaxNullabilityCheck bool + RelaxTypeMismatchCheck bool +} + // FieldSelectionMerging validates if field selections can be merged -func FieldSelectionMerging(relaxNullabilityCheck ...bool) Rule { - relax := len(relaxNullabilityCheck) > 0 && relaxNullabilityCheck[0] +func FieldSelectionMerging(options *FieldSelectionMergingOptions) Rule { + var opts FieldSelectionMergingOptions + if options != nil { + opts = *options + } return func(walker *astvisitor.Walker) { - visitor := fieldSelectionMergingVisitor{Walker: walker, relaxNullabilityCheck: relax} + visitor := fieldSelectionMergingVisitor{ + Walker: walker, + relaxNullabilityCheck: opts.RelaxNullabilityCheck, + relaxTypeMismatchCheck: opts.RelaxTypeMismatchCheck, + } walker.RegisterEnterDocumentVisitor(&visitor) walker.RegisterEnterFieldVisitor(&visitor) walker.RegisterEnterOperationVisitor(&visitor) @@ -24,11 +44,12 @@ func FieldSelectionMerging(relaxNullabilityCheck ...bool) Rule { type fieldSelectionMergingVisitor struct { *astvisitor.Walker - definition, operation *ast.Document - scalarRequirements scalarRequirements - nonScalarRequirements nonScalarRequirements - refs []int - relaxNullabilityCheck bool + definition, operation *ast.Document + scalarRequirements scalarRequirements + nonScalarRequirements nonScalarRequirements + refs []int + relaxNullabilityCheck bool + relaxTypeMismatchCheck bool } type nonScalarRequirement struct { path ast.Path @@ -119,13 +140,7 @@ 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) { + if !f.canRelaxTypeMismatch(f.nonScalarRequirements[i].enclosingTypeDefinition, f.nonScalarRequirements[i].fieldTypeRef, fieldType) { left, err := f.definition.PrintTypeBytes(f.nonScalarRequirements[i].fieldTypeRef, nil) if err != nil { f.StopWithInternalErr(err) @@ -172,13 +187,7 @@ 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) { + if !f.canRelaxTypeMismatch(f.scalarRequirements[i].enclosingTypeDefinition, f.scalarRequirements[i].fieldType, fieldType) { left, err := f.definition.PrintTypeBytes(f.scalarRequirements[i].fieldType, nil) if err != nil { f.StopWithInternalErr(err) @@ -213,6 +222,26 @@ func (f *fieldSelectionMergingVisitor) EnterField(ref int) { }) } +// canRelaxTypeMismatch reports whether a type incompatibility between two fields can +// be relaxed. This is a deliberate deviation from SameResponseShape (spec sec 5.3.2) +// for fields on non-overlapping concrete object types within inline fragments. +// Relaxation has two levels: +// - relaxTypeMismatchCheck (superset): skip the type compatibility check entirely, +// allowing completely different types (e.g. Int vs String). Covers nullability +// differences as a subset. +// - relaxNullabilityCheck: only relax when the underlying named types match but +// differ in nullability wrappers (e.g. String! vs String). +// +// Both require that the enclosing types are distinct concrete objects that cannot +// overlap at runtime. +func (f *fieldSelectionMergingVisitor) canRelaxTypeMismatch(enclosingType ast.Node, leftTypeRef, rightTypeRef int) bool { + if f.potentiallySameObject(enclosingType, f.EnclosingTypeDefinition) { + return false + } + return f.relaxTypeMismatchCheck || + (f.relaxNullabilityCheck && f.definition.TypesAreCompatibleIgnoringNullability(leftTypeRef, rightTypeRef)) +} + // 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. diff --git a/v2/pkg/astvalidation/operation_validation.go b/v2/pkg/astvalidation/operation_validation.go index e24a72a5be..44b50b1912 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 differing field types (e.g. IssueState vs PullRequestReviewState, or String vs Int) +// on fields in non-overlapping concrete object types within inline fragments. +// This is a superset of WithRelaxFieldSelectionMergingNullability. +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,10 @@ func DefaultOperationValidator(options ...Option) *OperationValidator { validator.RegisterRule(LoneAnonymousOperation()) validator.RegisterRule(SubscriptionSingleRootField()) validator.RegisterRule(FieldSelections()) - validator.RegisterRule(FieldSelectionMerging(opts.RelaxFieldSelectionMergingNullabilityCheck)) + validator.RegisterRule(FieldSelectionMerging(&FieldSelectionMergingOptions{ + RelaxNullabilityCheck: opts.RelaxFieldSelectionMergingNullabilityCheck, + RelaxTypeMismatchCheck: opts.RelaxFieldSelectionMergingTypeMismatchCheck, + })) validator.RegisterRule(KnownArguments()) validator.RegisterRule(Values()) validator.RegisterRule(ArgumentUniqueness()) diff --git a/v2/pkg/astvalidation/operation_validation_test.go b/v2/pkg/astvalidation/operation_validation_test.go index 74783bae26..b65e10e7b8 100644 --- a/v2/pkg/astvalidation/operation_validation_test.go +++ b/v2/pkg/astvalidation/operation_validation_test.go @@ -660,7 +660,7 @@ func TestExecutionValidation(t *testing.T) { } } } - }`, FieldSelectionMerging(), Valid) + }`, FieldSelectionMerging(nil), Valid) }) t.Run("reference implementation tests", func(t *testing.T) { t.Run("Same aliases allowed on non-overlapping fields", func(t *testing.T) { @@ -672,7 +672,7 @@ func TestExecutionValidation(t *testing.T) { ... on Cat { name: nickname } - }`, FieldSelectionMerging(), Valid) + }`, FieldSelectionMerging(nil), Valid) }) t.Run("allows different args where no conflict is possible", func(t *testing.T) { run(t, ` @@ -683,7 +683,7 @@ func TestExecutionValidation(t *testing.T) { ... on Cat { name } - }`, FieldSelectionMerging(), Valid) + }`, FieldSelectionMerging(nil), Valid) }) t.Run("encounters conflict in fragments", func(t *testing.T) { run(t, ` @@ -696,7 +696,7 @@ func TestExecutionValidation(t *testing.T) { } fragment B on Query { x: b - }`, FieldSelectionMerging(), Invalid, + }`, FieldSelectionMerging(nil), Invalid, withValidationErrors(`differing fields for objectName 'x' on (potentially) same type`)) }) t.Run("reports each conflict once", func(t *testing.T) { @@ -721,7 +721,7 @@ func TestExecutionValidation(t *testing.T) { } fragment B on Field { x: b - }`, FieldSelectionMerging(), Invalid, + }`, FieldSelectionMerging(nil), Invalid, withValidationErrors(`differing fields for objectName 'x' on (potentially) same type`)) }) t.Run("deep conflict", func(t *testing.T) { @@ -733,7 +733,7 @@ func TestExecutionValidation(t *testing.T) { field { x: b } - }`, FieldSelectionMerging(), Invalid, + }`, FieldSelectionMerging(nil), Invalid, withValidationErrors(`differing fields for objectName 'x' on (potentially) same type`)) }) t.Run("deep conflict with multiple issues", func(t *testing.T) { @@ -747,7 +747,7 @@ func TestExecutionValidation(t *testing.T) { x: b y: d } - }`, FieldSelectionMerging(), Invalid, + }`, FieldSelectionMerging(nil), Invalid, withValidationErrors(`differing fields for objectName 'x' on (potentially) same type`)) }) t.Run("very deep conflict", func(t *testing.T) { @@ -763,7 +763,7 @@ func TestExecutionValidation(t *testing.T) { x: b } } - }`, FieldSelectionMerging(), Invalid, + }`, FieldSelectionMerging(nil), Invalid, withValidationErrors(`differing fields for objectName 'x' on (potentially) same type`)) }) t.Run("very deep conflict validated", func(t *testing.T) { @@ -779,7 +779,7 @@ func TestExecutionValidation(t *testing.T) { x: a } } - }`, FieldSelectionMerging(), Valid) + }`, FieldSelectionMerging(nil), Valid) }) t.Run("reports deep conflict to nearest common ancestor", func(t *testing.T) { run(t, ` @@ -797,7 +797,7 @@ func TestExecutionValidation(t *testing.T) { y } } - }`, FieldSelectionMerging(), Invalid, + }`, FieldSelectionMerging(nil), Invalid, withValidationErrors(`differing fields for objectName 'x' on (potentially) same type`)) }) t.Run("reports deep conflict to nearest common ancestor in fragments", func(t *testing.T) { @@ -824,7 +824,7 @@ func TestExecutionValidation(t *testing.T) { y } } - }`, FieldSelectionMerging(), Invalid, + }`, FieldSelectionMerging(nil), Invalid, withValidationErrors(`differing fields for objectName 'x' on (potentially) same type`)) }) t.Run("reports deep conflict in nested fragments", func(t *testing.T) { @@ -850,7 +850,7 @@ func TestExecutionValidation(t *testing.T) { } fragment J on Field { x: b - }`, FieldSelectionMerging(), Invalid, + }`, FieldSelectionMerging(nil), Invalid, withValidationErrors(`differing fields for objectName 'y' on (potentially) same type`)) }) t.Run("ignores unknown fragments", func(t *testing.T) { @@ -863,7 +863,7 @@ func TestExecutionValidation(t *testing.T) { fragment Known on T { field ...OtherUnknown - }`, FieldSelectionMerging(), Invalid, withExpectNormalizationError()) + }`, FieldSelectionMerging(nil), Invalid, withExpectNormalizationError()) }) t.Run("return types must be unambiguous", func(t *testing.T) { t.Run("conflicting return types which potentially overlap", func(t *testing.T) { @@ -881,7 +881,7 @@ func TestExecutionValidation(t *testing.T) { scalar } } - }`, FieldSelectionMerging(), Invalid, + }`, FieldSelectionMerging(nil), Invalid, withValidationErrors(`fields 'scalar' conflict because they return conflicting types 'Int' and 'String!'`)) }) t.Run("compatible return shapes on different return types", func(t *testing.T) { @@ -899,7 +899,7 @@ func TestExecutionValidation(t *testing.T) { } } } - }`, FieldSelectionMerging(), Valid) + }`, FieldSelectionMerging(nil), Valid) }) t.Run("disallows differing return types despite no overlap", func(t *testing.T) { runWithDefinition(t, boxDefinition, ` @@ -912,7 +912,7 @@ func TestExecutionValidation(t *testing.T) { scalar } } - }`, FieldSelectionMerging(), Invalid, + }`, FieldSelectionMerging(nil), Invalid, withValidationErrors(`fields 'scalar' conflict because they return conflicting types 'Int' and 'String'`)) }) t.Run("disallows differing return types despite no overlap", func(t *testing.T) { @@ -929,7 +929,7 @@ func TestExecutionValidation(t *testing.T) { b: scalar } } - }`, FieldSelectionMerging(), Invalid, + }`, FieldSelectionMerging(nil), Invalid, withValidationErrors(`differing fields for objectName 'b' on (potentially) same type`)) }) t.Run("deeply nested", func(t *testing.T) { @@ -976,7 +976,7 @@ func TestExecutionValidation(t *testing.T) { } fragment Y on SomeBox { scalar: unrelatedField - }`, FieldSelectionMerging(), Invalid, + }`, FieldSelectionMerging(nil), Invalid, withValidationErrors(`fields 'scalar' conflict because they return conflicting types 'Int' and 'String'`)) }) t.Run("disallows differing return type nullability despite no overlap", func(t *testing.T) { @@ -990,7 +990,7 @@ func TestExecutionValidation(t *testing.T) { scalar } } - }`, FieldSelectionMerging(), Invalid, + }`, FieldSelectionMerging(nil), Invalid, withValidationErrors(`fields 'scalar' conflict because they return conflicting types 'String!' and 'String'`)) }) t.Run("disallows differing return type list despite no overlap", func(t *testing.T) { @@ -1008,7 +1008,7 @@ func TestExecutionValidation(t *testing.T) { } } } - }`, FieldSelectionMerging(), Invalid, + }`, FieldSelectionMerging(nil), Invalid, withValidationErrors(`differing types '[StringBox]' and 'StringBox' for objectName 'box'`)) runWithDefinition(t, boxDefinition, ` { @@ -1024,7 +1024,7 @@ func TestExecutionValidation(t *testing.T) { } } } - }`, FieldSelectionMerging(), Invalid, + }`, FieldSelectionMerging(nil), Invalid, withValidationErrors(`differing types 'StringBox' and '[StringBox]' for objectName 'box'`)) }) t.Run("disallows differing subfields", func(t *testing.T) { @@ -1043,7 +1043,7 @@ func TestExecutionValidation(t *testing.T) { } } } - }`, FieldSelectionMerging(), Invalid, + }`, FieldSelectionMerging(nil), Invalid, withValidationErrors(`differing fields for objectName 'val' on (potentially) same type`)) }) t.Run("disallows differing deep return types despite no overlap", func(t *testing.T) { @@ -1061,7 +1061,7 @@ func TestExecutionValidation(t *testing.T) { } } } - }`, FieldSelectionMerging(), Invalid, + }`, FieldSelectionMerging(nil), Invalid, withValidationErrors(`fields 'scalar' conflict because they return conflicting types 'String' and 'Int'`)) }) t.Run("allows non-conflicting overlapping types", func(t *testing.T) { @@ -1075,7 +1075,7 @@ func TestExecutionValidation(t *testing.T) { scalar } } - }`, FieldSelectionMerging(), Valid) + }`, FieldSelectionMerging(nil), Valid) }) t.Run("allows differing scalar nullability on non-overlapping object types", func(t *testing.T) { runWithDefinition(t, entityDefinition, ` @@ -1084,7 +1084,7 @@ func TestExecutionValidation(t *testing.T) { ... on User { email } ... on Organization { email } } - }`, FieldSelectionMerging(true), Valid) + }`, FieldSelectionMerging(&FieldSelectionMergingOptions{RelaxNullabilityCheck: true}), Valid) }) t.Run("rejects differing scalar nullability on non-overlapping object types without relaxation flag", func(t *testing.T) { runWithDefinition(t, entityDefinition, ` @@ -1093,7 +1093,7 @@ func TestExecutionValidation(t *testing.T) { ... on User { email } ... on Organization { email } } - }`, FieldSelectionMerging(), Invalid, + }`, FieldSelectionMerging(nil), Invalid, withValidationErrors(`fields 'email' conflict because they return conflicting types 'String!' and 'String'`)) }) t.Run("allows differing non-scalar nullability on non-overlapping object types", func(t *testing.T) { @@ -1103,7 +1103,7 @@ func TestExecutionValidation(t *testing.T) { ... on User { profile { name } } ... on Organization { profile { name } } } - }`, FieldSelectionMerging(true), Valid) + }`, FieldSelectionMerging(&FieldSelectionMergingOptions{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, ` @@ -1112,9 +1112,107 @@ func TestExecutionValidation(t *testing.T) { ... on User { profile { name } } ... on Organization { profile { name } } } - }`, FieldSelectionMerging(), Invalid, + }`, FieldSelectionMerging(nil), Invalid, withValidationErrors(`differing types 'Profile!' and 'Profile' for objectName 'profile'`)) }) + // Type mismatch relaxation tests + t.Run("allows differing scalar types on non-overlapping object types with type mismatch flag", func(t *testing.T) { + runWithDefinition(t, entityDefinition, ` + { + entity { + ... on User { score } + ... on Organization { score } + } + }`, FieldSelectionMerging(&FieldSelectionMergingOptions{RelaxTypeMismatchCheck: true}), Valid) + }) + t.Run("rejects differing scalar types on non-overlapping object types without flag", func(t *testing.T) { + runWithDefinition(t, entityDefinition, ` + { + entity { + ... on User { score } + ... on Organization { score } + } + }`, FieldSelectionMerging(nil), Invalid, + withValidationErrors(`fields 'score' conflict because they return conflicting types 'Int' and 'String'`)) + }) + t.Run("rejects differing scalar types with only nullability flag", func(t *testing.T) { + runWithDefinition(t, entityDefinition, ` + { + entity { + ... on User { score } + ... on Organization { score } + } + }`, FieldSelectionMerging(&FieldSelectionMergingOptions{RelaxNullabilityCheck: true}), Invalid, + withValidationErrors(`fields 'score' conflict because they return conflicting types 'Int' and 'String'`)) + }) + t.Run("allows three-way type mismatch on non-overlapping types", func(t *testing.T) { + runWithDefinition(t, entityDefinition, ` + { + entity { + ... on User { score } + ... on Organization { score } + ... on Bot { score } + } + }`, FieldSelectionMerging(&FieldSelectionMergingOptions{RelaxTypeMismatchCheck: true}), Valid) + }) + t.Run("allows differing enum types on non-overlapping object types", func(t *testing.T) { + // Enum types go through the non-scalar path where different enum type + // definition nodes hit potentiallySameObject=false on the field types, + // so no error is produced even without relaxation flags. + runWithDefinition(t, entityDefinition, ` + { + entity { + ... on User { role } + ... on Organization { role } + } + }`, FieldSelectionMerging(nil), Valid) + }) + t.Run("allows differing enum types on non-overlapping object types with type mismatch flag", func(t *testing.T) { + runWithDefinition(t, entityDefinition, ` + { + entity { + ... on User { role } + ... on Organization { role } + } + }`, FieldSelectionMerging(&FieldSelectionMergingOptions{RelaxTypeMismatchCheck: true}), Valid) + }) + t.Run("rejects nullability mismatch when interface could overlap even with type mismatch flag", func(t *testing.T) { + // NonNullStringBox1 is an interface, so potentiallySameObject returns true + // for the enclosing types — relaxation must not apply. + runWithDefinition(t, boxDefinition, ` + { + someBox { + ... on NonNullStringBox1 { + scalar + } + ... on StringBox { + scalar + } + } + }`, FieldSelectionMerging(&FieldSelectionMergingOptions{RelaxTypeMismatchCheck: true}), Invalid, + withValidationErrors(`fields 'scalar' conflict because they return conflicting types 'String!' and 'String'`)) + }) + t.Run("allows nullability differences with type mismatch flag", func(t *testing.T) { + runWithDefinition(t, entityDefinition, ` + { + entity { + ... on User { email } + ... on Organization { email } + } + }`, FieldSelectionMerging(&FieldSelectionMergingOptions{RelaxTypeMismatchCheck: true}), Valid) + }) + t.Run("allows differing scalar types with both flags set", func(t *testing.T) { + runWithDefinition(t, entityDefinition, ` + { + entity { + ... on User { score } + ... on Organization { score } + } + }`, FieldSelectionMerging(&FieldSelectionMergingOptions{ + RelaxNullabilityCheck: true, + RelaxTypeMismatchCheck: true, + }), Valid) + }) t.Run("disallows differing return type nullability when interface could overlap", func(t *testing.T) { runWithDefinition(t, boxDefinition, ` { @@ -1126,7 +1224,7 @@ func TestExecutionValidation(t *testing.T) { scalar } } - }`, FieldSelectionMerging(), Invalid, + }`, FieldSelectionMerging(nil), Invalid, withValidationErrors(`fields 'scalar' conflict because they return conflicting types 'String!' and 'String'`)) }) t.Run("same wrapped scalar return types", func(t *testing.T) { @@ -1140,7 +1238,7 @@ func TestExecutionValidation(t *testing.T) { scalar } } - }`, FieldSelectionMerging(), Valid) + }`, FieldSelectionMerging(nil), Valid) }) t.Run("allows inline typeless fragments", func(t *testing.T) { run(t, ` @@ -1149,7 +1247,7 @@ func TestExecutionValidation(t *testing.T) { ... { a } - }`, FieldSelectionMerging(), Valid) + }`, FieldSelectionMerging(nil), Valid) }) t.Run("compares deep types including list", func(t *testing.T) { runWithDefinition(t, boxDefinition, ` @@ -1169,7 +1267,7 @@ func TestExecutionValidation(t *testing.T) { id } } - }`, FieldSelectionMerging(), Invalid, + }`, FieldSelectionMerging(nil), Invalid, withValidationErrors(`differing fields for objectName 'id' on (potentially) same type`)) }) t.Run("ignores unknown types", func(t *testing.T) { @@ -1183,7 +1281,7 @@ func TestExecutionValidation(t *testing.T) { scalar } } - }`, FieldSelectionMerging(), Invalid, withExpectNormalizationError()) + }`, FieldSelectionMerging(nil), Invalid, withExpectNormalizationError()) }) }) }) @@ -1197,7 +1295,7 @@ func TestExecutionValidation(t *testing.T) { otherName: name otherName: name }`, - FieldSelectionMerging(), Valid) + FieldSelectionMerging(nil), Valid) }) t.Run("107 variant", func(t *testing.T) { run(t, ` @@ -1213,7 +1311,7 @@ func TestExecutionValidation(t *testing.T) { otherName: name } }`, - FieldSelectionMerging(), Valid) + FieldSelectionMerging(nil), Valid) }) t.Run("108", func(t *testing.T) { run(t, ` @@ -1221,7 +1319,7 @@ func TestExecutionValidation(t *testing.T) { name: nickname name }`, - FieldSelectionMerging(), Invalid, + FieldSelectionMerging(nil), Invalid, withValidationErrors(`differing fields for objectName 'name' on (potentially) same type`)) }) t.Run("108 variant", func(t *testing.T) { @@ -1232,7 +1330,7 @@ func TestExecutionValidation(t *testing.T) { name } }`, - FieldSelectionMerging(), Invalid, + FieldSelectionMerging(nil), Invalid, withValidationErrors(`differing fields for objectName 'name' on (potentially) same type`)) }) t.Run("108 variant", func(t *testing.T) { @@ -1242,7 +1340,7 @@ func TestExecutionValidation(t *testing.T) { extra { string } } }`, - FieldSelectionMerging(), Valid) + FieldSelectionMerging(nil), Valid) }) t.Run("108 variant", func(t *testing.T) { run(t, ` @@ -1252,7 +1350,7 @@ func TestExecutionValidation(t *testing.T) { extra { string } } }`, - FieldSelectionMerging(), Valid) + FieldSelectionMerging(nil), Valid) }) t.Run("108 variant", func(t *testing.T) { run(t, ` @@ -1262,7 +1360,7 @@ func TestExecutionValidation(t *testing.T) { extra { string } } }`, - FieldSelectionMerging(), Valid) + FieldSelectionMerging(nil), Valid) }) t.Run("108 variant", func(t *testing.T) { run(t, ` @@ -1272,7 +1370,7 @@ func TestExecutionValidation(t *testing.T) { extra { noString: string } } }`, - FieldSelectionMerging(), Valid) + FieldSelectionMerging(nil), Valid) }) t.Run("108 variant", func(t *testing.T) { run(t, ` @@ -1282,7 +1380,7 @@ func TestExecutionValidation(t *testing.T) { extra { string: noString } } }`, - FieldSelectionMerging(), Invalid, + FieldSelectionMerging(nil), Invalid, withValidationErrors(`differing fields for objectName 'string' on (potentially) same type`)) }) t.Run("108 variant", func(t *testing.T) { @@ -1292,7 +1390,7 @@ func TestExecutionValidation(t *testing.T) { extra: extras { string } } }`, - FieldSelectionMerging(), Invalid, + FieldSelectionMerging(nil), Invalid, withValidationErrors(`differing types 'DogExtra' and '[DogExtra]' for objectName 'extra'`)) }) t.Run("108 variant", func(t *testing.T) { @@ -1302,7 +1400,7 @@ func TestExecutionValidation(t *testing.T) { extras: mustExtras { string } } }`, - FieldSelectionMerging(), Invalid, + FieldSelectionMerging(nil), Invalid, withValidationErrors(`differing types '[DogExtra]' and '[DogExtra]!' for objectName 'extras'`)) }) t.Run("108 variant", func(t *testing.T) { @@ -1313,7 +1411,7 @@ func TestExecutionValidation(t *testing.T) { x: mustExtras { string } } }`, - FieldSelectionMerging(), Invalid, + FieldSelectionMerging(nil), Invalid, withValidationErrors(`differing types '[DogExtra]' and '[DogExtra]!' for objectName 'x'`)) }) t.Run("108 variant", func(t *testing.T) { @@ -1324,7 +1422,7 @@ func TestExecutionValidation(t *testing.T) { extras { string,string3: string } } }`, - FieldSelectionMerging(), Valid) + FieldSelectionMerging(nil), Valid) }) t.Run("108 variant", func(t *testing.T) { run(t, ` @@ -1334,7 +1432,7 @@ func TestExecutionValidation(t *testing.T) { extras { string,string2: string } } }`, - FieldSelectionMerging(), Valid) + FieldSelectionMerging(nil), Valid) }) t.Run("108 variant", func(t *testing.T) { run(t, ` @@ -1344,7 +1442,7 @@ func TestExecutionValidation(t *testing.T) { extras { string,string2: string,string3: string } } }`, - FieldSelectionMerging(), Invalid, + FieldSelectionMerging(nil), Invalid, withValidationErrors(`differing fields for objectName 'string2' on (potentially) same type`)) }) t.Run("108 variant", func(t *testing.T) { @@ -1355,7 +1453,7 @@ func TestExecutionValidation(t *testing.T) { extras { ... { string },... { string },string2: string } } }`, - FieldSelectionMerging(), Valid) + FieldSelectionMerging(nil), Valid) }) t.Run("108 variant", func(t *testing.T) { run(t, ` query conflictingBecauseAlias { @@ -1364,7 +1462,7 @@ func TestExecutionValidation(t *testing.T) { extras { ... { string1: string },string2: string } } }`, - FieldSelectionMerging(), Invalid, + FieldSelectionMerging(nil), Invalid, withValidationErrors(`differing fields for objectName 'string' on (potentially) same type`)) }) t.Run("108 variant", func(t *testing.T) { @@ -1376,7 +1474,7 @@ func TestExecutionValidation(t *testing.T) { } } fragment frag on DogExtra { string }`, - FieldSelectionMerging(), Valid) + FieldSelectionMerging(nil), Valid) }) t.Run("108 variant", func(t *testing.T) { run(t, ` query conflictingBecauseAlias { @@ -1393,7 +1491,7 @@ func TestExecutionValidation(t *testing.T) { } } }`, - FieldSelectionMerging(), Invalid, + FieldSelectionMerging(nil), Invalid, withValidationErrors(`differing fields for objectName 'string1' on (potentially) same type`)) }) t.Run("108 variant", func(t *testing.T) { @@ -1406,7 +1504,7 @@ func TestExecutionValidation(t *testing.T) { } fragment frag on DogExtra { string1 } fragment frag2 on DogExtra { string1: string }`, - FieldSelectionMerging(), Invalid, + FieldSelectionMerging(nil), Invalid, withValidationErrors(`differing fields for objectName 'string1' on (potentially) same type`)) }) t.Run("108 variant", func(t *testing.T) { @@ -1416,7 +1514,7 @@ func TestExecutionValidation(t *testing.T) { extra { looksLikeString: bool } } }`, - FieldSelectionMerging(), Invalid, + FieldSelectionMerging(nil), Invalid, withValidationErrors(`differing fields for objectName 'looksLikeString' on (potentially) same type`)) }) t.Run("108 variant", func(t *testing.T) { @@ -1429,7 +1527,7 @@ func TestExecutionValidation(t *testing.T) { fragment nameFrag on Dog { name }`, - FieldSelectionMerging(), Invalid, + FieldSelectionMerging(nil), Invalid, withValidationErrors(`differing fields for objectName 'name' on (potentially) same type`)) }) t.Run("108 variant", func(t *testing.T) { @@ -1445,7 +1543,7 @@ func TestExecutionValidation(t *testing.T) { fragment nameFrag2 on Dog { name }`, - FieldSelectionMerging(), Invalid, + FieldSelectionMerging(nil), Invalid, withValidationErrors(`differing fields for objectName 'name' on (potentially) same type`)) }) t.Run("108 variant", func(t *testing.T) { @@ -1460,7 +1558,7 @@ func TestExecutionValidation(t *testing.T) { fragment nameFrag on Dog { name }`, - FieldSelectionMerging(), Invalid, + FieldSelectionMerging(nil), Invalid, withValidationErrors(`differing fields for objectName 'name' on (potentially) same type`)) }) t.Run("108 variant", func(t *testing.T) { @@ -1472,7 +1570,7 @@ func TestExecutionValidation(t *testing.T) { } } }`, - FieldSelectionMerging(), Invalid, + FieldSelectionMerging(nil), Invalid, withValidationErrors(`differing fields for objectName 'name' on (potentially) same type`)) }) t.Run("108 variant", func(t *testing.T) { @@ -1484,7 +1582,7 @@ func TestExecutionValidation(t *testing.T) { } } }`, - FieldSelectionMerging(), Invalid, + FieldSelectionMerging(nil), Invalid, withValidationErrors(`differing fields for objectName 'name' on (potentially) same type`)) }) t.Run("108 variant", func(t *testing.T) { @@ -1496,7 +1594,7 @@ func TestExecutionValidation(t *testing.T) { } } }`, - FieldSelectionMerging(), Valid) + FieldSelectionMerging(nil), Valid) }) t.Run("109", func(t *testing.T) { run(t, ` @@ -1508,14 +1606,14 @@ func TestExecutionValidation(t *testing.T) { doesKnowCommand(dogCommand: $dogCommand) doesKnowCommand(dogCommand: $dogCommand) }`, - FieldSelectionMerging(), Valid) + FieldSelectionMerging(nil), Valid) }) t.Run("109 variant", func(t *testing.T) { run(t, ` fragment mergeIdenticalFieldsWithIdenticalValues on Dog { doesKnowCommand(dogCommand: 1) doesKnowCommand(dogCommand: 1) }`, - FieldSelectionMerging(), Valid) + FieldSelectionMerging(nil), Valid) }) t.Run("109 variant", func(t *testing.T) { run(t, ` @@ -1523,7 +1621,7 @@ func TestExecutionValidation(t *testing.T) { doesKnowCommand(dogCommand: 1) doesKnowCommand(dogCommand: 0) }`, - FieldSelectionMerging(), Invalid, + FieldSelectionMerging(nil), Invalid, withValidationErrors(`differing fields for objectName 'doesKnowCommand' on (potentially) same type`)) }) t.Run("109 variant", func(t *testing.T) { @@ -1531,14 +1629,14 @@ func TestExecutionValidation(t *testing.T) { doesKnowCommand(dogCommand: 1.1) doesKnowCommand(dogCommand: 1.1) }`, - FieldSelectionMerging(), Valid) + FieldSelectionMerging(nil), Valid) }) t.Run("109 variant", func(t *testing.T) { run(t, ` fragment mergeIdenticalFieldsWithIdenticalValues on Dog { doesKnowCommand(dogCommand: 1.1) doesKnowCommand(dogCommand: 0.1) }`, - FieldSelectionMerging(), Invalid, + FieldSelectionMerging(nil), Invalid, withValidationErrors(`differing fields for objectName 'doesKnowCommand' on (potentially) same type`)) }) t.Run("109 variant", func(t *testing.T) { @@ -1546,14 +1644,14 @@ func TestExecutionValidation(t *testing.T) { doesKnowCommand(dogCommand: "foo") doesKnowCommand(dogCommand: "foo") }`, - FieldSelectionMerging(), Valid) + FieldSelectionMerging(nil), Valid) }) t.Run("109 variant", func(t *testing.T) { run(t, ` fragment mergeIdenticalFieldsWithIdenticalValues on Dog { doesKnowCommand(dogCommand: "foo") doesKnowCommand(dogCommand: "bar") }`, - FieldSelectionMerging(), Invalid, + FieldSelectionMerging(nil), Invalid, withValidationErrors(`differing fields for objectName 'doesKnowCommand' on (potentially) same type`)) }) t.Run("109 variant", func(t *testing.T) { @@ -1561,14 +1659,14 @@ func TestExecutionValidation(t *testing.T) { doesKnowCommand(dogCommand: null) doesKnowCommand(dogCommand: null) }`, - FieldSelectionMerging(), Valid) + FieldSelectionMerging(nil), Valid) }) t.Run("109 variant", func(t *testing.T) { run(t, ` fragment mergeIdenticalFieldsWithIdenticalValues on Dog { doesKnowCommand(dogCommand: null) doesKnowCommand(dogCommand: 0) }`, - FieldSelectionMerging(), Invalid, + FieldSelectionMerging(nil), Invalid, withValidationErrors(`differing fields for objectName 'doesKnowCommand' on (potentially) same type`)) }) t.Run("109 variant", func(t *testing.T) { @@ -1576,14 +1674,14 @@ func TestExecutionValidation(t *testing.T) { doesKnowCommand(dogCommand: [1.1]) doesKnowCommand(dogCommand: [1.1]) }`, - FieldSelectionMerging(), Valid) + FieldSelectionMerging(nil), Valid) }) t.Run("109 variant", func(t *testing.T) { run(t, ` fragment mergeIdenticalFieldsWithIdenticalValues on Dog { doesKnowCommand(dogCommand: [1.1]) doesKnowCommand(dogCommand: [0.1]) }`, - FieldSelectionMerging(), Invalid, + FieldSelectionMerging(nil), Invalid, withValidationErrors(`differing fields for objectName 'doesKnowCommand' on (potentially) same type`)) }) t.Run("109 variant", func(t *testing.T) { @@ -1591,7 +1689,7 @@ func TestExecutionValidation(t *testing.T) { doesKnowCommand(dogCommand: [1.1]) doesKnowCommand(dogCommand: [1.1,1.1]) }`, - FieldSelectionMerging(), Invalid, + FieldSelectionMerging(nil), Invalid, withValidationErrors(`differing fields for objectName 'doesKnowCommand' on (potentially) same type`)) }) t.Run("109 variant", func(t *testing.T) { @@ -1600,7 +1698,7 @@ func TestExecutionValidation(t *testing.T) { doesKnowCommand(dogCommand: {foo: "bar"}) doesKnowCommand(dogCommand: {foo: "bar"}) }`, - FieldSelectionMerging(), Valid) + FieldSelectionMerging(nil), Valid) }) t.Run("109 variant", func(t *testing.T) { run(t, ` @@ -1608,7 +1706,7 @@ func TestExecutionValidation(t *testing.T) { doesKnowCommand(dogCommand: {foo: "bar"}) doesKnowCommand(dogCommand: {bar: "bar"}) }`, - FieldSelectionMerging(), Invalid, + FieldSelectionMerging(nil), Invalid, withValidationErrors(`differing fields for objectName 'doesKnowCommand' on (potentially) same type`)) }) t.Run("109 variant", func(t *testing.T) { @@ -1617,7 +1715,7 @@ func TestExecutionValidation(t *testing.T) { doesKnowCommand(dogCommand: {foo: "baz"}) doesKnowCommand(dogCommand: {bar: "baz"}) }`, - FieldSelectionMerging(), Invalid, + FieldSelectionMerging(nil), Invalid, withValidationErrors(`differing fields for objectName 'doesKnowCommand' on (potentially) same type`)) }) t.Run("109 variant", func(t *testing.T) { @@ -1625,7 +1723,7 @@ func TestExecutionValidation(t *testing.T) { doesKnowCommand(dogCommand: {foo: "bar"}) doesKnowCommand(dogCommand: {foo: "baz",bar: "bat"}) }`, - FieldSelectionMerging(), Invalid, + FieldSelectionMerging(nil), Invalid, withValidationErrors(`differing fields for objectName 'doesKnowCommand' on (potentially) same type`)) }) t.Run("110", func(t *testing.T) { @@ -1633,31 +1731,31 @@ func TestExecutionValidation(t *testing.T) { doesKnowCommand(dogCommand: SIT) doesKnowCommand(dogCommand: HEEL) }`, - FieldSelectionMerging(), Invalid, + FieldSelectionMerging(nil), Invalid, withValidationErrors(`differing fields for objectName 'doesKnowCommand' on (potentially) same type`)) run(t, ` fragment conflictingArgsOnValues on Dog { doesKnowCommand(dogCommand: SIT) doesKnowCommand(dogCommand1: HEEL) }`, - FieldSelectionMerging(), Invalid, + FieldSelectionMerging(nil), Invalid, withValidationErrors(`differing fields for objectName 'doesKnowCommand' on (potentially) same type`)) run(t, ` fragment conflictingArgsValueAndVar on Dog { doesKnowCommand(dogCommand: SIT) doesKnowCommand(dogCommand: $dogCommand) }`, - FieldSelectionMerging(), Invalid, + FieldSelectionMerging(nil), Invalid, withValidationErrors(`differing fields for objectName 'doesKnowCommand' on (potentially) same type`)) run(t, ` fragment conflictingArgsWithVars on Dog { doesKnowCommand(dogCommand: $varOne) doesKnowCommand(dogCommand: $varTwo) }`, - FieldSelectionMerging(), Invalid, + FieldSelectionMerging(nil), Invalid, withValidationErrors(`differing fields for objectName 'doesKnowCommand' on (potentially) same type`)) run(t, ` fragment differingArgs on Dog { doesKnowCommand(dogCommand: SIT) doesKnowCommand }`, - FieldSelectionMerging(), Invalid, + FieldSelectionMerging(nil), Invalid, withValidationErrors(`differing fields for objectName 'doesKnowCommand' on (potentially) same type`)) }) t.Run("111", func(t *testing.T) { @@ -1678,7 +1776,7 @@ func TestExecutionValidation(t *testing.T) { doesKnowCommand(catCommand: JUMP) } }`, - FieldSelectionMerging(), Valid) + FieldSelectionMerging(nil), Valid) }) t.Run("112", func(t *testing.T) { run(t, ` @@ -1690,7 +1788,7 @@ func TestExecutionValidation(t *testing.T) { someValue: meowVolume } }`, - FieldSelectionMerging(), Invalid, + FieldSelectionMerging(nil), Invalid, withValidationErrors(`fields 'someValue' conflict because they return conflicting types 'String!' and 'Int'`)) }) t.Run("112 variant", func(t *testing.T) { @@ -1707,7 +1805,7 @@ func TestExecutionValidation(t *testing.T) { } } }`, - FieldSelectionMerging(), Valid) + FieldSelectionMerging(nil), Valid) }) t.Run("112 variant", func(t *testing.T) { run(t, ` @@ -1723,7 +1821,7 @@ func TestExecutionValidation(t *testing.T) { } } }`, - FieldSelectionMerging(), Valid) + FieldSelectionMerging(nil), Valid) }) t.Run("112 variant", func(t *testing.T) { run(t, ` fragment conflictingDifferingResponses on Pet { @@ -1738,7 +1836,7 @@ func TestExecutionValidation(t *testing.T) { } } }`, - FieldSelectionMerging(), Invalid, + FieldSelectionMerging(nil), Invalid, withValidationErrors(`fields 'string' conflict because they return conflicting types 'String' and '[String]'`)) }) t.Run("112 variant", func(t *testing.T) { @@ -1754,7 +1852,7 @@ func TestExecutionValidation(t *testing.T) { } } }`, - FieldSelectionMerging(), Invalid, + FieldSelectionMerging(nil), Invalid, withValidationErrors(`fields 'string' conflict because they return conflicting types 'String' and '[String]!'`)) }) t.Run("112 variant", func(t *testing.T) { @@ -1770,7 +1868,7 @@ func TestExecutionValidation(t *testing.T) { } } }`, - FieldSelectionMerging(), Valid) + FieldSelectionMerging(nil), Valid) }) t.Run("112 variant", func(t *testing.T) { run(t, ` @@ -1786,7 +1884,7 @@ func TestExecutionValidation(t *testing.T) { } } }`, - FieldSelectionMerging(), Valid) + FieldSelectionMerging(nil), Valid) }) t.Run("112 variant", func(t *testing.T) { run(t, ` fragment conflictingDifferingResponses on Pet { @@ -1801,7 +1899,7 @@ func TestExecutionValidation(t *testing.T) { } } }`, - FieldSelectionMerging(), Invalid, + FieldSelectionMerging(nil), Invalid, withValidationErrors(`fields 'string' conflict because they return conflicting types 'String' and 'Boolean'`)) }) t.Run("112 variant", func(t *testing.T) { @@ -1819,7 +1917,7 @@ func TestExecutionValidation(t *testing.T) { } } }`, - FieldSelectionMerging(), Invalid, + FieldSelectionMerging(nil), Invalid, withValidationErrors(`fields 'string' conflict because they return conflicting types 'String' and 'Boolean'`)) }) t.Run("112 variant", func(t *testing.T) { @@ -1837,7 +1935,7 @@ func TestExecutionValidation(t *testing.T) { } } }`, - FieldSelectionMerging(), Valid) + FieldSelectionMerging(nil), Valid) }) t.Run("112 variant", func(t *testing.T) { run(t, ` fragment conflictingDifferingResponses on Pet { @@ -1854,7 +1952,7 @@ func TestExecutionValidation(t *testing.T) { } } }`, - FieldSelectionMerging(), Valid) + FieldSelectionMerging(nil), Valid) }) t.Run("112 variant", func(t *testing.T) { run(t, `query conflictingDifferingResponses { @@ -1873,7 +1971,7 @@ func TestExecutionValidation(t *testing.T) { } } }`, - FieldSelectionMerging(), Invalid, withExpectNormalizationError()) + FieldSelectionMerging(nil), Invalid, withExpectNormalizationError()) }) t.Run("112 variant", func(t *testing.T) { run(t, ` @@ -1891,7 +1989,7 @@ func TestExecutionValidation(t *testing.T) { } } }`, - FieldSelectionMerging(), Valid) + FieldSelectionMerging(nil), Valid) }) t.Run("112 variant", func(t *testing.T) { run(t, ` @@ -1907,7 +2005,7 @@ func TestExecutionValidation(t *testing.T) { fragment catFrag on Cat { someValue: meowVolume }`, - FieldSelectionMerging(), Invalid, withExpectNormalizationError()) + FieldSelectionMerging(nil), Invalid, withExpectNormalizationError()) }) t.Run("112 variant", func(t *testing.T) { run(t, ` query conflictingDifferingResponses { @@ -1922,7 +2020,7 @@ func TestExecutionValidation(t *testing.T) { fragment catFrag on Cat { someValue: meowVolume }`, - FieldSelectionMerging(), Invalid, + FieldSelectionMerging(nil), Invalid, withValidationErrors(`fields 'someValue' conflict because they return conflicting types 'String!' and 'Int'`)) }) t.Run("112 variant", func(t *testing.T) { @@ -1941,7 +2039,7 @@ func TestExecutionValidation(t *testing.T) { fragment dogFrag on Dog { someValue: barkVolume }`, - FieldSelectionMerging(), Valid) + FieldSelectionMerging(nil), Valid) }) t.Run("112 variant", func(t *testing.T) { run(t, ` query conflictingDifferingResponses { @@ -1956,7 +2054,7 @@ func TestExecutionValidation(t *testing.T) { fragment pet2 on Pet { name }`, - FieldSelectionMerging(), Valid) + FieldSelectionMerging(nil), Valid) }) t.Run("112 variant", func(t *testing.T) { run(t, ` query conflictingDifferingResponses { @@ -1971,7 +2069,7 @@ func TestExecutionValidation(t *testing.T) { fragment pet2 on Pet { name1: nickname }`, - FieldSelectionMerging(), Invalid, withExpectNormalizationError()) + FieldSelectionMerging(nil), Invalid, withExpectNormalizationError()) }) t.Run("112 variant", func(t *testing.T) { run(t, ` @@ -1990,7 +2088,7 @@ func TestExecutionValidation(t *testing.T) { fragment dogFrag on Dog { someValue: name }`, - FieldSelectionMerging(), Invalid, + FieldSelectionMerging(nil), Invalid, withValidationErrors(`fields 'someValue' conflict because they return conflicting types 'Int' and 'String!'`)) }) t.Run("112 variant", func(t *testing.T) { @@ -2004,7 +2102,7 @@ func TestExecutionValidation(t *testing.T) { fragment catFrag on Cat { someValue: meowVolume }`, - FieldSelectionMerging(), Valid) + FieldSelectionMerging(nil), Valid) }) t.Run("112 variant", func(t *testing.T) { run(t, `query conflictingDifferingResponses { @@ -2019,7 +2117,7 @@ func TestExecutionValidation(t *testing.T) { fragment catFrag on Cat { someValue: name }`, - FieldSelectionMerging(), Invalid, withExpectNormalizationError()) + FieldSelectionMerging(nil), Invalid, withExpectNormalizationError()) }) t.Run("112 variant", func(t *testing.T) { run(t, ` @@ -2033,7 +2131,7 @@ func TestExecutionValidation(t *testing.T) { fragment catFrag on Cat { someValue: meowVolume }`, - FieldSelectionMerging(), Valid) + FieldSelectionMerging(nil), Valid) }) t.Run("112 variant", func(t *testing.T) { run(t, ` @@ -2046,7 +2144,7 @@ func TestExecutionValidation(t *testing.T) { fragment dogFrag on Dog { someValue: barkVolume }`, - FieldSelectionMerging(), Valid) + FieldSelectionMerging(nil), Valid) }) t.Run("112 variant", func(t *testing.T) { run(t, ` query conflictingDifferingResponses { @@ -2061,7 +2159,7 @@ func TestExecutionValidation(t *testing.T) { fragment catFrag on Cat { someValue: meowVolume }`, - FieldSelectionMerging(), Valid) + FieldSelectionMerging(nil), Valid) }) t.Run("112 variant", func(t *testing.T) { run(t, ` query conflictingDifferingResponses { @@ -2075,7 +2173,7 @@ func TestExecutionValidation(t *testing.T) { fragment dogFrag on Dog { someValue: barkVolume }`, - FieldSelectionMerging(), Valid) + FieldSelectionMerging(nil), Valid) }) t.Run("112 variant", func(t *testing.T) { run(t, ` @@ -2090,7 +2188,7 @@ func TestExecutionValidation(t *testing.T) { fragment dogFrag on Dog { someValue: barkVolume }`, - FieldSelectionMerging(), Invalid, withExpectNormalizationError()) + FieldSelectionMerging(nil), Invalid, withExpectNormalizationError()) }) t.Run("112 variant", func(t *testing.T) { run(t, ` query conflictingDifferingResponses { @@ -2099,7 +2197,7 @@ func TestExecutionValidation(t *testing.T) { ... on DogExtra { value: bool } } }`, - FieldSelectionMerging(), Invalid, + FieldSelectionMerging(nil), Invalid, withValidationErrors(`fields 'value' conflict because they return conflicting types 'Boolean' and 'Int'`)) }) t.Run("skip/include do not creates conflicts", func(t *testing.T) { @@ -2111,7 +2209,7 @@ func TestExecutionValidation(t *testing.T) { nickname @skip(if: false) } }`, - FieldSelectionMerging(), Valid, withDisableNormalization()) + FieldSelectionMerging(nil), Valid, withDisableNormalization()) }) t.Run("execution directives do not creates conflicts", func(t *testing.T) { // it is questionable if this is correct, but spec do not check directives at all @@ -2121,7 +2219,7 @@ func TestExecutionValidation(t *testing.T) { nickname @tag(name: "b") } }`, - FieldSelectionMerging(), Valid, withDisableNormalization()) + FieldSelectionMerging(nil), Valid, withDisableNormalization()) }) t.Run("different stream directives creates conflicts", func(t *testing.T) { run(t, `query noConflictOnDifferentFieldIncludeSkip { @@ -2130,7 +2228,7 @@ func TestExecutionValidation(t *testing.T) { nickname @stream(label: "b") } }`, - FieldSelectionMerging(), Invalid, withDisableNormalization()) + FieldSelectionMerging(nil), Invalid, withDisableNormalization()) }) }) t.Run("5.3.3 Leaf Field Selections", func(t *testing.T) { @@ -5701,8 +5799,13 @@ schema { const entityDefinition = ` scalar String scalar Int +scalar Boolean scalar ID +enum UserRole { ADMIN MEMBER } +enum OrgRole { OWNER CONTRIBUTOR } +enum BotKind { CI CD } + interface Node { id: ID! } @@ -5711,19 +5814,28 @@ type User implements Node { id: ID! email: String! profile: Profile! + role: UserRole + score: Int } type Organization implements Node { id: ID! email: String profile: Profile + role: OrgRole + score: String +} + +type Bot implements Node { + id: ID! + score: Boolean } type Profile { name: String } -union Entity = User | Organization +union Entity = User | Organization | Bot type Query { entity: Entity diff --git a/v2/pkg/astvalidation/reference/testsgo/harness_test.go b/v2/pkg/astvalidation/reference/testsgo/harness_test.go index fc4a46431d..5f3a3db7f0 100644 --- a/v2/pkg/astvalidation/reference/testsgo/harness_test.go +++ b/v2/pkg/astvalidation/reference/testsgo/harness_test.go @@ -71,7 +71,7 @@ var rulesMap = map[string][]astvalidation.Rule{ LoneAnonymousOperationRule: {astvalidation.LoneAnonymousOperation()}, NoUndefinedVariablesRule: {astvalidation.AllVariableUsesDefined()}, NoUnusedVariablesRule: {astvalidation.AllVariablesUsed()}, - OverlappingFieldsCanBeMergedRule: {astvalidation.FieldSelectionMerging()}, + OverlappingFieldsCanBeMergedRule: {astvalidation.FieldSelectionMerging(nil)}, ProvidedRequiredArgumentsRule: {astvalidation.RequiredArguments()}, ProvidedRequiredArgumentsOnDirectivesRule: {}, SingleFieldSubscriptionsRule: {astvalidation.SubscriptionSingleRootField()}, diff --git a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go index f4268d1f6a..d17b7ea691 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go +++ b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go @@ -1715,9 +1715,11 @@ func newPrintKitPool(validationOptions ...astvalidation.Option) *sync.Pool { } var ( - defaultPrintKitPool = newPrintKitPool() - relaxedPrintKitPool *sync.Pool - relaxedPrintKitPoolOnce sync.Once + defaultPrintKitPool = newPrintKitPool() + relaxedPrintKitPool *sync.Pool + relaxedPrintKitPoolOnce sync.Once + typeMismatchRelaxedPrintKitPool *sync.Pool + typeMismatchRelaxedPrintKitPoolOnce sync.Once ) func getRelaxedPrintKitPool() *sync.Pool { @@ -1727,6 +1729,13 @@ func getRelaxedPrintKitPool() *sync.Pool { return relaxedPrintKitPool } +func getTypeMismatchRelaxedPrintKitPool() *sync.Pool { + typeMismatchRelaxedPrintKitPoolOnce.Do(func() { + typeMismatchRelaxedPrintKitPool = newPrintKitPool(astvalidation.WithRelaxFieldSelectionMergingTypeMismatch()) + }) + return typeMismatchRelaxedPrintKitPool +} + type Factory[T Configuration] struct { executionContext context.Context httpClient *http.Client @@ -1821,6 +1830,17 @@ func (f *Factory[T]) EnableSubgraphFieldSelectionMergingNullabilityRelaxation() f.printKitPool = getRelaxedPrintKitPool() } +// EnableSubgraphFieldSelectionMergingTypeMismatchRelaxation implements +// plan.SubgraphFieldSelectionMergingTypeMismatchRelaxer. It configures the +// factory to use a shared process-global pool (created via sync.Once) whose +// validator allows completely differing field types on non-overlapping concrete +// types. This is a superset of the nullability relaxation — callers should not +// also call EnableSubgraphFieldSelectionMergingNullabilityRelaxation, as the +// type mismatch pool already covers nullability differences. +func (f *Factory[T]) EnableSubgraphFieldSelectionMergingTypeMismatchRelaxation() { + f.printKitPool = getTypeMismatchRelaxedPrintKitPool() +} + func (f *Factory[T]) getPrintKitPool() *sync.Pool { if f.printKitPool != nil { return f.printKitPool 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..e5f7dba5ff 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_test.go +++ b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_test.go @@ -8233,6 +8233,125 @@ func TestGraphQLDataSource(t *testing.T) { DisableResolveFieldPositions: true, }, WithDefaultPostProcessor())) + t.Run("inline fragments on non-overlapping types with different field types", func(t *testing.T) { + definition := ` + scalar String + scalar Int + scalar ID + + type User { + id: ID! + score: Int + } + + type Organization { + id: ID! + score: String + } + + union Entity = User | Organization + + type Query { + entity: Entity + } + + schema { + query: Query + } + ` + + t.Run("run", RunTest(definition, ` + query MyQuery { + entity { + ... on User { score } + ... on Organization { score } + } + }`, + "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 {score} ... on Organization {score}}}"}}`, + 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("score"), + Value: &resolve.Integer{ + Path: []string{"score"}, + Nullable: true, + }, + OnTypeNames: [][]byte{[]byte("User")}, + }, + { + Name: []byte("score"), + Value: &resolve.String{ + Path: []string{"score"}, + Nullable: true, + }, + OnTypeNames: [][]byte{[]byte("Organization")}, + }, + }, + }, + }, + }, + }, + }, + }, 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", "score"}, + }, + { + TypeName: "Organization", + FieldNames: []string{"id", "score"}, + }, + }, + }, + mustCustomConfiguration(t, ConfigurationInput{ + Fetch: &FetchConfiguration{ + URL: "https://example.com/graphql", + }, + SchemaConfiguration: mustSchema(t, nil, definition), + }), + ), + }, + DisableResolveFieldPositions: true, + RelaxSubgraphOperationFieldSelectionMergingTypeMismatch: true, + }, WithDefaultPostProcessor(), + WithValidationOptions(astvalidation.WithRelaxFieldSelectionMergingTypeMismatch()), + )) + }) + t.Run("inline fragments on non-overlapping types with different field nullability", func(t *testing.T) { definition := ` scalar String diff --git a/v2/pkg/engine/plan/configuration.go b/v2/pkg/engine/plan/configuration.go index 5194c8e976..f5db63ecc5 100644 --- a/v2/pkg/engine/plan/configuration.go +++ b/v2/pkg/engine/plan/configuration.go @@ -59,6 +59,14 @@ 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 differing field types + // (e.g. IssueState vs PullRequestReviewState) on fields in non-overlapping inline fragments. + // This is a superset of RelaxSubgraphOperationFieldSelectionMergingNullability. + 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..e0cd6fffb9 100644 --- a/v2/pkg/engine/plan/datasource_configuration.go +++ b/v2/pkg/engine/plan/datasource_configuration.go @@ -300,7 +300,13 @@ func (d *dataSourceConfiguration[T]) CustomConfiguration() T { } func (d *dataSourceConfiguration[T]) CreatePlannerConfiguration(logger abstractlogger.Logger, fetchConfig *objectFetchConfiguration, pathConfig *plannerPathsConfiguration, configuration *Configuration) PlannerConfiguration { - if configuration.RelaxSubgraphOperationFieldSelectionMergingNullability { + // Type mismatch relaxation is a superset of nullability relaxation, so only + // one pool should be applied. Prefer the broader type mismatch pool when set. + if configuration.RelaxSubgraphOperationFieldSelectionMergingTypeMismatch { + if relaxer, ok := d.factory.(SubgraphFieldSelectionMergingTypeMismatchRelaxer); ok { + relaxer.EnableSubgraphFieldSelectionMergingTypeMismatchRelaxation() + } + } else if configuration.RelaxSubgraphOperationFieldSelectionMergingNullability { if relaxer, ok := d.factory.(SubgraphFieldSelectionMergingNullabilityRelaxer); ok { relaxer.EnableSubgraphFieldSelectionMergingNullabilityRelaxation() } @@ -489,6 +495,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 differing +// field types on non-overlapping concrete types. +type SubgraphFieldSelectionMergingTypeMismatchRelaxer interface { + EnableSubgraphFieldSelectionMergingTypeMismatchRelaxation() +} + type DataSourcePlanner[T any] interface { DataSourceFetchPlanner DataSourceBehavior