Skip to content

Commit 4ecb40d

Browse files
glen-84glen-84
authored
[Fusion] Fixed the FieldSelectionMapValidator (#8615)
1 parent 84ae022 commit 4ecb40d

File tree

10 files changed

+336
-38
lines changed

10 files changed

+336
-38
lines changed

‎src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PostMergeValidationRules/IsInvalidFieldRule.cs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,12 @@ public void Handle(SchemaEvent @event, CompositionContext context)
4242
var fieldArgumentValue = (string)isDirective.Arguments[Field].Value!;
4343
var fieldSelectionMapParser = new FieldSelectionMapParser(fieldArgumentValue);
4444
var fieldSelectionMap = fieldSelectionMapParser.Parse();
45-
var inputType = schema.Types[sourceArgument.Type.AsTypeDefinition().Name];
46-
var outputType = schema.Types[sourceField.Type.AsTypeDefinition().Name];
45+
var inputTypeNode = sourceArgument.Type.ToTypeNode();
46+
var inputTypeDefinition = schema.Types[sourceArgument.Type.AsTypeDefinition().Name];
47+
var inputType = inputTypeNode.RewriteToType(inputTypeDefinition);
48+
var outputTypeNode = sourceField.Type.ToTypeNode();
49+
var outputTypeDefinition = schema.Types[sourceField.Type.AsTypeDefinition().Name];
50+
var outputType = outputTypeNode.RewriteToType(outputTypeDefinition);
4751

4852
var errors = validator.Validate(fieldSelectionMap, inputType, outputType);
4953

‎src/HotChocolate/Fusion-vnext/src/Fusion.Utilities/Extensions/TypeExtensions.cs

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
using HotChocolate.Types;
2+
3+
namespace HotChocolate.Fusion;
4+
5+
public static class TypeExtensions
6+
{
7+
/// <summary>
8+
/// Determines whether <paramref name="type"/> is compatible with <paramref name="other"/>
9+
/// under the rule:
10+
/// <para>
11+
/// <b>At any depth</b> (including nested list elements), <paramref name="type"/>
12+
/// may only be as nullable as (or stricter than) <paramref name="other"/>.
13+
/// </para>
14+
/// In other words: wherever <paramref name="other"/> is wrapped in <see cref="NonNullType"/>,
15+
/// <paramref name="type"/> must also be <see cref="NonNullType"/> at the same position.
16+
/// The list structure and underlying named type must match.
17+
/// </summary>
18+
/// <remarks>
19+
/// Examples:
20+
/// <list type="bullet">
21+
/// <item>
22+
/// <description><c>other: ID</c> → <c>type: ID</c> ✅ or <c>ID!</c> ✅</description>
23+
/// </item>
24+
/// <item>
25+
/// <description><c>other: ID!</c> → <c>type: ID!</c> ✅ (<c>ID</c> ❌)</description>
26+
/// </item>
27+
/// <item>
28+
/// <description><c>other: [ID]</c> → <c>type: [ID]</c> ✅, <c>[ID]!</c> ✅, <c>[ID!]</c> ✅, <c>[ID!]!</c> ✅</description>
29+
/// </item>
30+
/// <item>
31+
/// <description><c>other: [ID!]!</c> → <c>type: [ID!]!</c> ✅ (anything more nullable at any level ❌)</description>
32+
/// </item>
33+
/// </list>
34+
/// </remarks>
35+
/// <param name="type">The candidate type (the one you are checking).</param>
36+
/// <param name="other">The reference type that defines the maximum allowed nullability.</param>
37+
/// <returns>
38+
/// <c>true</c> if <paramref name="type"/> is compatible with <paramref name="other"/> per the rule above; otherwise <c>false</c>.
39+
/// </returns>
40+
public static bool IsCompatibleWith(this IType type, IType other)
41+
{
42+
ArgumentNullException.ThrowIfNull(type);
43+
ArgumentNullException.ThrowIfNull(other);
44+
45+
// Work on local variables we can unwrap as we traverse down the shape.
46+
var t = type;
47+
var o = other;
48+
49+
// We iteratively walk both types in lockstep:
50+
// 1) Enforce the NonNull rule at the current level.
51+
// 2) Strip NonNull from both sides.
52+
// 3) If we're at lists, both must be lists; step into element types.
53+
// 4) If we're at named types, names must match.
54+
while (true)
55+
{
56+
// Rule enforcement: if "other" is NonNull here, "type" must be NonNull too.
57+
var oIsNonNull = o is NonNullType;
58+
var tIsNonNull = t is NonNullType;
59+
60+
if (oIsNonNull && !tIsNonNull)
61+
{
62+
// "type" is looser (more nullable) than "other" at this level → incompatible.
63+
return false;
64+
}
65+
66+
// Unwrap any NonNull wrappers so we can compare the inner shape.
67+
if (oIsNonNull)
68+
{
69+
o = ((NonNullType)o).NullableType;
70+
}
71+
72+
if (tIsNonNull)
73+
{
74+
t = ((NonNullType)t).NullableType;
75+
}
76+
77+
// If "other" is a list at this level, "type" must also be a list.
78+
if (o is ListType oList)
79+
{
80+
if (t is not ListType tList)
81+
{
82+
// Shape mismatch: one is a list, the other is not.
83+
return false;
84+
}
85+
86+
// Dive into element types and continue the loop.
87+
o = oList.ElementType;
88+
t = tList.ElementType;
89+
continue;
90+
}
91+
92+
// At this point, both should be named types
93+
// (e.g., ID, String, Foo, etc.). Names must match.
94+
if (o is INameProvider oNamed && t is INameProvider tNamed)
95+
{
96+
return string.Equals(oNamed.Name, tNamed.Name, StringComparison.Ordinal);
97+
}
98+
99+
// Any other combination is a shape mismatch.
100+
return false;
101+
}
102+
}
103+
}

‎src/HotChocolate/Fusion-vnext/src/Fusion.Utilities/Properties/FusionUtilitiesResources.Designer.cs

Lines changed: 9 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/HotChocolate/Fusion-vnext/src/Fusion.Utilities/Properties/FusionUtilitiesResources.resx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@
1818
<resheader name="writer">
1919
<value>System.Resources.ResXResourceWriter, System.Windows.Forms, Version=2.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
2020
</resheader>
21+
<data name="FieldSelectionMapValidator_ExpectedInputObjectType" xml:space="preserve">
22+
<value>Expected an input object type but found '{0}'.</value>
23+
</data>
2124
<data name="FieldSelectionMapValidator_FieldDoesNotExistOnInputType" xml:space="preserve">
2225
<value>The field '{0}' does not exist on the input type '{1}'.</value>
2326
</data>

‎src/HotChocolate/Fusion-vnext/src/Fusion.Utilities/Validators/FieldSelectionMapValidator.cs

Lines changed: 99 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,22 @@ public ImmutableArray<string> Validate(
3737
return [.. context.Errors];
3838
}
3939

40+
protected override ISyntaxVisitorAction Enter(
41+
IFieldSelectionMapSyntaxNode node,
42+
FieldSelectionMapValidatorContext context)
43+
{
44+
context.Nodes.Push(node);
45+
return base.Enter(node, context);
46+
}
47+
48+
protected override ISyntaxVisitorAction Leave(
49+
IFieldSelectionMapSyntaxNode node,
50+
FieldSelectionMapValidatorContext context)
51+
{
52+
context.Nodes.Pop();
53+
return base.Leave(node, context);
54+
}
55+
4056
protected override ISyntaxVisitorAction Enter(
4157
PathNode node,
4258
FieldSelectionMapValidatorContext context)
@@ -84,6 +100,21 @@ protected override ISyntaxVisitorAction Leave(
84100
context.OutputTypes.Pop();
85101
}
86102

103+
var terminalType = context.TerminalTypes.Pop();
104+
105+
if (context.Nodes.TryPeek(out var contextNode)
106+
&& contextNode is PathObjectValueSelectionNode or PathListValueSelectionNode)
107+
{
108+
context.OutputTypes.Push(terminalType);
109+
}
110+
else if (terminalType.IsComplexType())
111+
{
112+
context.Errors.Add(
113+
string.Format(
114+
FieldSelectionMapValidator_FieldMissingSubselections,
115+
node.ToString(indented: false)));
116+
}
117+
87118
return Continue;
88119
}
89120

@@ -106,21 +137,11 @@ protected override ISyntaxVisitorAction Enter(
106137

107138
context.SelectedFields.Add(field);
108139

109-
var fieldType = field.Type.NullableType();
140+
var fieldNullableType = field.Type.NullableType();
110141

111-
if (fieldType is IComplexTypeDefinition or IUnionTypeDefinition)
142+
if (fieldNullableType is IComplexTypeDefinition or IUnionTypeDefinition or ListType)
112143
{
113-
if (node.PathSegment is null)
114-
{
115-
context.Errors.Add(
116-
string.Format(
117-
FieldSelectionMapValidator_FieldMissingSubselections,
118-
field.Name));
119-
120-
return Break;
121-
}
122-
123-
if (fieldType is IUnionTypeDefinition && node.TypeName is null)
144+
if (fieldNullableType is IUnionTypeDefinition && node.TypeName is null)
124145
{
125146
context.Errors.Add(
126147
string.Format(
@@ -136,9 +157,18 @@ protected override ISyntaxVisitorAction Enter(
136157
{
137158
if (node.PathSegment is null)
138159
{
139-
var inputType = context.InputTypes.Peek().NullableType();
160+
var fieldType = field.Type;
161+
var inputType = context.InputTypes.Peek();
162+
163+
// Fields of a OneOf input object are always non-nullable.
164+
if (inputType.IsNullableType()
165+
&& context.InputTypes.ElementAtOrDefault(1) is IInputObjectTypeDefinition inputObjectType
166+
&& inputObjectType.Directives.ContainsName(OneOf))
167+
{
168+
inputType = new NonNullType(inputType);
169+
}
140170

141-
if (!fieldType.Equals(inputType, TypeComparison.Structural))
171+
if (!fieldType.IsCompatibleWith(inputType))
142172
{
143173
var printedFieldType = fieldType.ToTypeNode().Print(indented: false);
144174
var printedInputType = inputType.ToTypeNode().Print(indented: false);
@@ -165,6 +195,11 @@ protected override ISyntaxVisitorAction Enter(
165195
return Break;
166196
}
167197
}
198+
199+
if (node.PathSegment is null)
200+
{
201+
context.TerminalTypes.Push(field.Type);
202+
}
168203
}
169204

170205
if (node.TypeName is { } typeName)
@@ -277,12 +312,17 @@ protected override ISyntaxVisitorAction Enter(
277312
node.Name,
278313
inputType.Name));
279314

280-
return Break;
315+
return Skip;
281316
}
282317
}
283318
else
284319
{
285-
context.InputTypes.Push(currentInputType);
320+
context.Errors.Add(
321+
string.Format(
322+
FieldSelectionMapValidator_ExpectedInputObjectType,
323+
currentInputType.ToTypeNode().ToString(indented: false)));
324+
325+
return Break;
286326
}
287327

288328
if (node.ValueSelection is null
@@ -313,6 +353,44 @@ protected override ISyntaxVisitorAction Leave(
313353

314354
return Continue;
315355
}
356+
357+
protected override ISyntaxVisitorAction Leave(
358+
PathObjectValueSelectionNode node,
359+
FieldSelectionMapValidatorContext context)
360+
{
361+
context.OutputTypes.Pop();
362+
363+
return Continue;
364+
}
365+
366+
protected override ISyntaxVisitorAction Leave(
367+
PathListValueSelectionNode node,
368+
FieldSelectionMapValidatorContext context)
369+
{
370+
context.OutputTypes.Pop();
371+
372+
return Continue;
373+
}
374+
375+
protected override ISyntaxVisitorAction Enter(
376+
ListValueSelectionNode selectionNode,
377+
FieldSelectionMapValidatorContext context)
378+
{
379+
context.InputTypes.Push(context.InputTypes.Peek().ElementType());
380+
context.OutputTypes.Push(context.OutputTypes.Peek().ElementType());
381+
382+
return Continue;
383+
}
384+
385+
protected override ISyntaxVisitorAction Leave(
386+
ListValueSelectionNode selectionNode,
387+
FieldSelectionMapValidatorContext context)
388+
{
389+
context.InputTypes.Pop();
390+
context.OutputTypes.Pop();
391+
392+
return Continue;
393+
}
316394
}
317395

318396
public sealed class FieldSelectionMapValidatorContext
@@ -323,10 +401,14 @@ public FieldSelectionMapValidatorContext(IType inputType, IType outputType)
323401
OutputTypes.Push(outputType);
324402
}
325403

404+
public Stack<IFieldSelectionMapSyntaxNode> Nodes { get; } = [];
405+
326406
public Stack<IType> InputTypes { get; } = [];
327407

328408
public Stack<IType> OutputTypes { get; } = [];
329409

410+
public Stack<IType> TerminalTypes { get; } = [];
411+
330412
public HashSet<IOutputFieldDefinition> SelectedFields { get; } = [];
331413

332414
public List<string> Errors { get; } = [];

‎src/HotChocolate/Fusion-vnext/test/Fusion.Composition.Tests/PostMergeValidationRules/RequireInvalidFieldsRuleTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ public static TheoryData<string[]> ValidExamplesData()
6565
# Schema A
6666
type User @key(fields: "id") {
6767
id: ID!
68-
profile(name: String! @require(field: "name")): Profile
68+
profile(name: String @require(field: "name")): Profile
6969
}
7070
7171
type Profile {

‎src/HotChocolate/Fusion-vnext/test/Fusion.Composition.Tests/SourceSchemaValidationRules/RequireInvalidFieldTypeRuleTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ public static TheoryData<string[]> ValidExamplesData()
5555
"""
5656
type User @key(fields: "id") {
5757
id: ID!
58-
profile(name: String! @require(field: "name")): Profile
58+
profile(name: String @require(field: "name")): Profile
5959
}
6060
6161
type Profile {

‎src/HotChocolate/Fusion-vnext/test/Fusion.Composition.Tests/SourceSchemaValidationRules/RequireInvalidSyntaxRuleTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ public static TheoryData<string[]> ValidExamplesData()
5555
"""
5656
type User @key(fields: "id") {
5757
id: ID!
58-
profile(name: String! @require(field: "name")): Profile
58+
profile(name: String @require(field: "name")): Profile
5959
}
6060
6161
type Profile {

0 commit comments

Comments
 (0)