Skip to content

Commit

Permalink
Fixed Fusion Non-Null propagation (#7011)
Browse files Browse the repository at this point in the history
  • Loading branch information
@tobias-tengler
tobias-tengler authored Mar 25, 2024
1 parent d47aa2c commit 6270df4
Show file tree
Hide file tree
Showing 8 changed files with 261 additions and 44 deletions.
20 changes: 10 additions & 10 deletions src/HotChocolate/Core/src/Execution/Processing/ValueCompletion.List.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ internal static partial class ValueCompletion
{
return null;
}

var elementType = type.InnerType();
var isLeafType = elementType.IsLeafType();
var operationContext = context.OperationContext;
Expand Down Expand Up @@ -103,7 +103,7 @@ internal static partial class ValueCompletion
{
resultList.Grow();
}

if (!TryCompleteElement(context, selection, elementType, isLeafType, resultList, i++, element))
{
operationContext.Result.AddRemovedResult(resultList);
Expand Down Expand Up @@ -148,12 +148,12 @@ private static bool TryCompleteElement(
{
return list.IsNullable;
}

list.SetUnsafe(index, resultData);
}
return true;
}

return list.IsNullable;
}

Expand All @@ -163,14 +163,14 @@ internal static void PropagateNullValues(ResultData result)
{
return;
}

result.IsInvalidated = true;

while (result.Parent is not null)
{
var index = result.ParentIndex;
var parent = result.Parent;

if(parent.IsInvalidated)
{
return;
Expand All @@ -186,7 +186,7 @@ internal static void PropagateNullValues(ResultData result)
}
objectResult.IsInvalidated = true;
break;

case ListResult listResult:
if (listResult.TrySetNull(index))
{
Expand All @@ -195,8 +195,8 @@ internal static void PropagateNullValues(ResultData result)
listResult.IsInvalidated = true;
break;
}

result = parent;
}
}
}
}
32 changes: 20 additions & 12 deletions src/HotChocolate/Fusion/src/Core/Execution/ExecutorUtils.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
using HotChocolate.Language;
using HotChocolate.Types;
using static HotChocolate.Execution.Processing.Selection;
using static HotChocolate.Execution.Processing.ValueCompletion;
using IType = HotChocolate.Types.IType;
using ObjectType = HotChocolate.Types.ObjectType;

Expand Down Expand Up @@ -68,7 +67,7 @@ private static void ComposeResult(
{
if (!nullable)
{
PropagateNullValues(selectionSetResult);
PropagateNullValues(context.Result, selection, selectionSetResult, responseIndex);
break;
}

Expand All @@ -79,10 +78,9 @@ private static void ComposeResult(
{
var value = data.Single.Element;

if (value.ValueKind is JsonValueKind.Null or JsonValueKind.Undefined &&
!nullable)
if (value.ValueKind is JsonValueKind.Null or JsonValueKind.Undefined && !nullable)
{
PropagateNullValues(selectionSetResult);
PropagateNullValues(context.Result, selection, selectionSetResult, responseIndex);
break;
}

Expand All @@ -104,7 +102,7 @@ private static void ComposeResult(
if (value.ValueKind is JsonValueKind.Null or JsonValueKind.Undefined &&
!nullable)
{
PropagateNullValues(selectionSetResult);
PropagateNullValues(context.Result, selection, selectionSetResult, responseIndex);
break;
}

Expand All @@ -123,7 +121,7 @@ private static void ComposeResult(

if (value is null && !nullable)
{
PropagateNullValues(selectionSetResult);
PropagateNullValues(context.Result, selection, selectionSetResult, responseIndex);
break;
}

Expand All @@ -144,7 +142,7 @@ private static void ComposeResult(

if (value is null && !nullable)
{
PropagateNullValues(selectionSetResult);
PropagateNullValues(context.Result, selection, selectionSetResult, responseIndex);
break;
}

Expand Down Expand Up @@ -216,7 +214,7 @@ private static void ComposeResult(

if (!nullable && element is null)
{
PropagateNullValues(result);
PropagateNullValues(context.Result, selection, result, index);
return null;
}

Expand Down Expand Up @@ -495,14 +493,13 @@ public static void ExtractErrors(
var path = PathHelper.CreatePathFromContext(selectionSetResult);
foreach (var error in errors.EnumerateArray())
{
ExtractError(resultBuilder, error, selectionSetResult, path, pathDepth, addDebugInfo);
ExtractError(resultBuilder, error, path, pathDepth, addDebugInfo);
}
}

private static void ExtractError(
ResultBuilder resultBuilder,
JsonElement error,
ObjectResult selectionSetResult,
Path parentPath,
int pathDepth,
bool addDebugInfo)
Expand Down Expand Up @@ -538,7 +535,7 @@ private static void ExtractError(
{
var path = PathHelper.CombinePath(parentPath, remotePath, pathDepth);
errorBuilder.SetPath(path);

if (addDebugInfo)
{
errorBuilder.SetExtension("remotePath", remotePath);
Expand Down Expand Up @@ -642,4 +639,15 @@ public static void ExtractVariables(
}
}
}

private static void PropagateNullValues(
ResultBuilder resultBuilder,
Selection selection,
ResultData selectionSetResult,
int responseIndex)
{
var path = PathHelper.CreatePathFromContext(selection, selectionSetResult, responseIndex);
resultBuilder.AddNonNullViolation(selection, path);
ValueCompletion.PropagateNullValues(selectionSetResult);
}
}
17 changes: 0 additions & 17 deletions src/HotChocolate/Fusion/src/Core/Execution/Nodes/QueryPlan.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,23 +188,6 @@ public async Task<IQueryResult> ExecuteAsync(
{
await RootNode.ExecuteAsync(context, cancellationToken).ConfigureAwait(false);
}
catch (NonNullPropagateException ex)
{
context.Result.SetData(null);

// TODO : REMOVE after non-null prop is good.
if (context.Result.Errors.Count == 0)
{
var error =
context.OperationContext.ErrorHandler.Handle(
ErrorBuilder.New()
.SetMessage("NON NULL PROPAGATION")
.SetException(ex)
.Build());

context.Result.AddError(error);
}
}
catch (Exception ex)
{
if (context.Result.Errors.Count == 0)
Expand Down
5 changes: 0 additions & 5 deletions src/HotChocolate/Fusion/src/Core/Execution/NonNullPropagateException.cs
View file

This file was deleted.

72 changes: 72 additions & 0 deletions ...ion/test/Core.Tests/__snapshots__/ErrorTests.Accounts_Offline_Author_NonNull.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,78 @@ query ReformatIds {
```json
{
"errors": [
{
"message": "Cannot return null for non-nullable field.",
"locations": [
{
"line": 5,
"column": 13
}
],
"path": [
"reviews",
3,
"author",
"birthdate"
],
"extensions": {
"code": "HC0018"
}
},
{
"message": "Cannot return null for non-nullable field.",
"locations": [
{
"line": 5,
"column": 13
}
],
"path": [
"reviews",
2,
"author",
"birthdate"
],
"extensions": {
"code": "HC0018"
}
},
{
"message": "Cannot return null for non-nullable field.",
"locations": [
{
"line": 5,
"column": 13
}
],
"path": [
"reviews",
1,
"author",
"birthdate"
],
"extensions": {
"code": "HC0018"
}
},
{
"message": "Cannot return null for non-nullable field.",
"locations": [
{
"line": 5,
"column": 13
}
],
"path": [
"reviews",
0,
"author",
"birthdate"
],
"extensions": {
"code": "HC0018"
}
},
{
"message": "Internal Execution Error"
}
Expand Down
72 changes: 72 additions & 0 deletions ...on/test/Core.Tests/__snapshots__/ErrorTests.Accounts_Offline_Author_Nullable.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,78 @@ query ReformatIds {
```json
{
"errors": [
{
"message": "Cannot return null for non-nullable field.",
"locations": [
{
"line": 5,
"column": 13
}
],
"path": [
"reviews",
3,
"author",
"birthdate"
],
"extensions": {
"code": "HC0018"
}
},
{
"message": "Cannot return null for non-nullable field.",
"locations": [
{
"line": 5,
"column": 13
}
],
"path": [
"reviews",
2,
"author",
"birthdate"
],
"extensions": {
"code": "HC0018"
}
},
{
"message": "Cannot return null for non-nullable field.",
"locations": [
{
"line": 5,
"column": 13
}
],
"path": [
"reviews",
1,
"author",
"birthdate"
],
"extensions": {
"code": "HC0018"
}
},
{
"message": "Cannot return null for non-nullable field.",
"locations": [
{
"line": 5,
"column": 13
}
],
"path": [
"reviews",
0,
"author",
"birthdate"
],
"extensions": {
"code": "HC0018"
}
},
{
"message": "Internal Execution Error"
}
Expand Down
Loading

0 comments on commit 6270df4

Please sign in to comment.