Skip to content

Commit ed2079a

Browse files
authored
Fix issue where non-conditional fields become conditional. (#9706)
1 parent 86c6ede commit ed2079a

7 files changed

Lines changed: 429 additions & 21 deletions

File tree

src/HotChocolate/Core/src/Types/Execution/Processing/OperationCompiler.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -291,13 +291,12 @@ private SelectionSet BuildSelectionSet(
291291
var includeFlags = new List<ulong>();
292292
var deferUsages = new List<DeferUsage>();
293293
var selectionSetId = ++lastId;
294-
var alwaysIncluded = false;
295-
296294
foreach (var (responseName, nodes) in fieldMap)
297295
{
298296
includeFlags.Clear();
299297
deferUsages.Clear();
300298

299+
var alwaysIncluded = false;
301300
var first = nodes[0];
302301
var isInternal = IsInternal(first.Node);
303302
var hasNonDeferredNode = first.DeferUsage is null;

src/HotChocolate/Core/test/Execution.Tests/Processing/SelectionIncludeConditionTests.cs

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -758,12 +758,134 @@ fragment B on Person {
758758
""");
759759
}
760760

761+
[Fact]
762+
public async Task Complementary_Fragment_Spreads_Should_Use_Full_Fragment_When_Minimal_Is_False()
763+
{
764+
// arrange
765+
var result = await ExecuteComplementaryFragmentQueryAsync(minimal: false);
766+
767+
// assert
768+
result.MatchInlineSnapshot(
769+
"""
770+
{
771+
"data": {
772+
"series": [
773+
{
774+
"streams": [
775+
{
776+
"__typename": "Stream",
777+
"id": "stream-1",
778+
"hasDrm": true,
779+
"title": "Stream One",
780+
"publishedAt": "2026-01-01"
781+
}
782+
]
783+
}
784+
]
785+
}
786+
}
787+
""");
788+
}
789+
790+
[Fact]
791+
public async Task Complementary_Fragment_Spreads_Should_Use_Minimal_Fragment_When_Minimal_Is_True()
792+
{
793+
// arrange
794+
var result = await ExecuteComplementaryFragmentQueryAsync(minimal: true);
795+
796+
// assert
797+
result.MatchInlineSnapshot(
798+
"""
799+
{
800+
"data": {
801+
"series": [
802+
{
803+
"streams": [
804+
{
805+
"__typename": "Stream",
806+
"id": "stream-1",
807+
"hasDrm": true
808+
}
809+
]
810+
}
811+
]
812+
}
813+
}
814+
""");
815+
}
816+
817+
private static async Task<IExecutionResult> ExecuteComplementaryFragmentQueryAsync(bool minimal)
818+
{
819+
var schema = SchemaBuilder.New()
820+
.AddDocumentFromString(
821+
"""
822+
type Query {
823+
series: [Series!]!
824+
}
825+
826+
type Series {
827+
id: ID!
828+
streams: [Stream!]!
829+
}
830+
831+
type Stream {
832+
id: ID!
833+
title: String
834+
hasDrm: Boolean
835+
publishedAt: String
836+
}
837+
""")
838+
.AddResolver<Query>()
839+
.Create();
840+
841+
return await schema.MakeExecutable().ExecuteAsync(
842+
OperationRequestBuilder.New()
843+
.SetDocument(
844+
"""
845+
query TestQuery($minimal: Boolean = false) {
846+
series {
847+
streams {
848+
__typename
849+
...streamFields
850+
}
851+
}
852+
}
853+
854+
fragment streamFields on Stream {
855+
__typename
856+
...MinimalStream @include(if: $minimal)
857+
...FullStream @skip(if: $minimal)
858+
}
859+
860+
fragment MinimalStream on Stream {
861+
id
862+
hasDrm
863+
}
864+
865+
fragment FullStream on Stream {
866+
id
867+
title
868+
hasDrm
869+
publishedAt
870+
}
871+
""")
872+
.SetVariableValues(
873+
$$"""
874+
{
875+
"minimal": {{minimal.ToString().ToLowerInvariant()}}
876+
}
877+
""")
878+
.Build());
879+
}
880+
761881
public sealed class Query
762882
{
763883
public Person Person() => new Person();
764884

765885
[UsePaging]
766886
public Person[] Persons() => [new Person()];
887+
888+
public Series[] Series() => [new Series()];
767889
}
768890

769891
public sealed class Person
@@ -772,4 +894,22 @@ public sealed class Person
772894

773895
public string Address { get; } = "world";
774896
}
897+
898+
public sealed class Series
899+
{
900+
public string Id { get; } = "series-1";
901+
902+
public Stream[] Streams() => [new Stream()];
903+
}
904+
905+
public sealed class Stream
906+
{
907+
public string Id { get; } = "stream-1";
908+
909+
public string Title { get; } = "Stream One";
910+
911+
public bool HasDrm { get; } = true;
912+
913+
public string PublishedAt { get; } = "2026-01-01";
914+
}
775915
}

src/HotChocolate/Fusion/src/Fusion.Execution.Types/FusionSchemaDefinition.cs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -424,8 +424,10 @@ private ImmutableArray<Lookup> GetPossibleLookupsInternal(ITypeDefinition type,
424424
}
425425

426426
/// <summary>
427-
/// Tries to get the best direct lookup to transition from one schema to another without intermediary transitions.
428-
/// The best lookup algorithm will try to find the smallest possible key that does not require any intermediary transitions.
427+
/// Tries to get the best direct lookup to transition from one schema
428+
/// to another without intermediary transitions.
429+
/// The best lookup algorithm will try to find the smallest possible key that does not
430+
/// require any intermediary transitions.
429431
/// </summary>
430432
/// <param name="type">The type to get the best direct lookup for.</param>
431433
/// <param name="fromSchemas">The schemas to get the best direct lookup from.</param>
@@ -442,7 +444,7 @@ public bool TryGetBestDirectLookup(
442444
ArgumentNullException.ThrowIfNull(fromSchemas);
443445
ArgumentException.ThrowIfNullOrEmpty(toSchema);
444446

445-
foreach (var fromSchema in fromSchemas)
447+
foreach (var fromSchema in fromSchemas.OrderBy(static t => t, StringComparer.Ordinal))
446448
{
447449
if (TryGetBestDirectLookup(type, fromSchema, toSchema, out lookup))
448450
{

src/HotChocolate/Fusion/src/Fusion.Execution/Execution/Nodes/Operation.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,8 +242,10 @@ public ulong CreateIncludeFlags(IVariableValueCollection variables)
242242
{
243243
if (includeCondition.IsIncluded(variables))
244244
{
245-
includeFlags |= 1ul << index++;
245+
includeFlags |= 1ul << index;
246246
}
247+
248+
index++;
247249
}
248250

249251
return includeFlags;

src/HotChocolate/Fusion/src/Fusion.Execution/Execution/Nodes/OperationCompiler.cs

Lines changed: 42 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -245,14 +245,12 @@ private SelectionSet BuildSelectionSet(
245245
includeFlags.Clear();
246246
deliveryGroups.Clear();
247247

248+
var alwaysIncluded = false;
248249
var first = nodes[0];
249-
var isInternal = IsInternal(first.Node);
250+
var isInternal = IsInternal(nodes);
250251
var hasImmediateNode = first.DeliveryGroup is null;
251252

252-
if (first.PathIncludeFlags > 0)
253-
{
254-
includeFlags.Add(first.PathIncludeFlags);
255-
}
253+
AddIncludeFlags(first, isInternal, includeFlags, ref alwaysIncluded);
256254

257255
if (first.DeliveryGroup is not null)
258256
{
@@ -271,10 +269,7 @@ private SelectionSet BuildSelectionSet(
271269
$"The syntax nodes for the response name {responseName} are not all the same.");
272270
}
273271

274-
if (next.PathIncludeFlags > 0)
275-
{
276-
includeFlags.Add(next.PathIncludeFlags);
277-
}
272+
AddIncludeFlags(next, isInternal, includeFlags, ref alwaysIncluded);
278273

279274
if (next.DeliveryGroup is null)
280275
{
@@ -284,11 +279,6 @@ private SelectionSet BuildSelectionSet(
284279
{
285280
deliveryGroups.Add(next.DeliveryGroup);
286281
}
287-
288-
if (isInternal)
289-
{
290-
isInternal = IsInternal(next.Node);
291-
}
292282
}
293283
}
294284

@@ -413,6 +403,44 @@ private static void CollapseIncludeFlags(List<ulong> includeFlags)
413403
}
414404
}
415405

406+
private static void AddIncludeFlags(
407+
FieldSelectionNode node,
408+
bool isInternalSelection,
409+
List<ulong> includeFlags,
410+
ref bool alwaysIncluded)
411+
{
412+
if (!isInternalSelection && IsInternal(node.Node))
413+
{
414+
return;
415+
}
416+
417+
if (node.PathIncludeFlags == 0)
418+
{
419+
alwaysIncluded = true;
420+
if (includeFlags.Count > 0)
421+
{
422+
includeFlags.Clear();
423+
}
424+
}
425+
else if (!alwaysIncluded)
426+
{
427+
includeFlags.Add(node.PathIncludeFlags);
428+
}
429+
}
430+
431+
private static bool IsInternal(List<FieldSelectionNode> nodes)
432+
{
433+
for (var i = 0; i < nodes.Count; i++)
434+
{
435+
if (!IsInternal(nodes[i].Node))
436+
{
437+
return false;
438+
}
439+
}
440+
441+
return true;
442+
}
443+
416444
private bool DoesTypeApply(NamedTypeNode? typeCondition, IObjectTypeDefinition typeContext)
417445
{
418446
if (typeCondition is null)

src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/__snapshots__/ConditionalTests.NodeField_Skip_On_Interface_Selection_Type_Refinement_With_Same_Unskipped_Selection.yaml

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,11 @@ response:
2323
body: |
2424
{
2525
"data": {
26-
"node": {}
26+
"node": {
27+
"author": {
28+
"rating": 123
29+
}
30+
}
2731
}
2832
}
2933
sourceSchemas:

0 commit comments

Comments
 (0)