Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fusion] Do not send omitted arguments with default values to subgraph #6767

Merged
merged 6 commits into from
Dec 14, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,14 @@ public FetchRewriterContext(
FragmentSpreadNode? placeholder,
IReadOnlyDictionary<string, IValueNode> variables,
SelectionSetNode? selectionSet,
string? responseName)
string? responseName,
IReadOnlyList<string>? unspecifiedArguments)
{
Placeholder = placeholder;
Variables = variables;
SelectionSet = selectionSet;
ResponseName = responseName;
UnspecifiedArguments = unspecifiedArguments;
}

public string? ResponseName { get; }
Expand All @@ -29,6 +31,11 @@ public FetchRewriterContext(

public IReadOnlyDictionary<string, IValueNode> Variables { get; }

/// <summary>
/// An optional list of arguments that weren't explicitly specified in the original query.
/// </summary>
public IReadOnlyList<string>? UnspecifiedArguments { get; }

public SelectionSetNode? SelectionSet { get; }

public IReadOnlyList<string> SelectionPath { get; set; } = Array.Empty<string>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,21 @@ private class ResolverRewriter : SyntaxRewriter<FetchRewriterContext>
{
var result = base.RewriteField(node, context);

if (result is not null && context.PlaceholderFound)
if (result is null)
{
return null;
}

if (context.UnspecifiedArguments?.Count > 0)
{
var explicitlyDefinedArguments = result.Arguments
.ExceptBy(context.UnspecifiedArguments, a => a.Name.Value)
.ToList();

result = result.WithArguments(explicitlyDefinedArguments);
}

if (context.PlaceholderFound)
{
context.PlaceholderFound = false;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,10 @@ public ResolverDefinition(
public (ISelectionNode selectionNode, IReadOnlyList<string> Path) CreateSelection(
IReadOnlyDictionary<string, IValueNode> variables,
SelectionSetNode? selectionSet,
string? responseName)
string? responseName,
IReadOnlyList<string>? unspecifiedArguments)
{
var context = new FetchRewriterContext(Placeholder, variables, selectionSet, responseName);
var context = new FetchRewriterContext(Placeholder, variables, selectionSet, responseName, unspecifiedArguments);
var selection = _rewriter.Rewrite(_field ?? (ISyntaxNode)Select, context);

if (Placeholder is null && selectionSet is not null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ internal RequestDocument CreateRequestDocument(
OperationType operationType = OperationType.Query)
{
var rootSelectionSetNode =
CreateRooSelectionSetNode(
CreateRootSelectionSetNode(
context,
executionStep,
entityTypeName);
Expand All @@ -47,7 +47,7 @@ internal RequestDocument CreateRequestDocument(
path);
}

private SelectionSetNode CreateRooSelectionSetNode(
private SelectionSetNode CreateRootSelectionSetNode(
QueryPlanContext context,
SelectionExecutionStep executionStep,
string entityTypeName)
Expand Down Expand Up @@ -90,7 +90,8 @@ private SelectionSetNode CreateRooSelectionSetNode(
var (selectionNode, _) = nodeSelection.Resolver.CreateSelection(
context.VariableValues,
selectionSetNode,
nodeSelection.Selection.ResponseName);
nodeSelection.Selection.ResponseName,
null);

if (selectionNode is FieldNode fieldNode &&
!nodeSelection.Selection.ResponseName.EqualsOrdinal(fieldNode.Name.Value))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,14 @@ internal RequestDocument CreateRequestDocument(
executionStep.Resolver,
executionStep.Variables);

var unspecifiedArguments = GetUnspecifiedArguments(executionStep.ParentSelection);

var (rootResolver, p) =
executionStep.Resolver.CreateSelection(
context.VariableValues,
rootSelectionSetNode,
null);
null,
unspecifiedArguments);

rootSelectionSetNode = new SelectionSetNode(new[] { rootResolver });
path = p;
Expand Down Expand Up @@ -85,19 +88,16 @@ internal RequestDocument CreateRequestDocument(
}

private SelectionSetNode CreateRootLevelQuery(
SelectionPath path,
SelectionPath path,
SelectionSetNode selectionSet)
{
var current = path;

while (current is not null)
{
selectionSet = new SelectionSetNode(
new[]
{
current.Selection.SyntaxNode.WithSelectionSet(selectionSet)
});

new[] { current.Selection.SyntaxNode.WithSelectionSet(selectionSet) });

current = current.Parent;
}

Expand Down Expand Up @@ -165,10 +165,13 @@ protected virtual SelectionSetNode CreateRootSelectionSetNode(
rootSelection.Resolver,
executionStep.Variables);

var unspecifiedArguments = GetUnspecifiedArguments(rootSelection.Selection);

var (s, _) = rootSelection.Resolver.CreateSelection(
context.VariableValues,
selectionSetNode,
rootSelection.Selection.ResponseName);
rootSelection.Selection.ResponseName,
unspecifiedArguments);
selectionNode = s;
}

Expand Down Expand Up @@ -278,6 +281,7 @@ protected virtual SelectionSetNode CreateSelectionSetNode(
selectionNodes.Add(TypeNameField);
needsTypeNameField = false;
}

AddInlineFragment(possibleType);
}
}
Expand Down Expand Up @@ -314,7 +318,7 @@ protected virtual bool CreateSelectionNodes(
ref var selection = ref selectionSet.GetSelectionsReference();
ref var end = ref Unsafe.Add(ref selection, selectionSet.Selections.Count);

while(Unsafe.IsAddressLessThan(ref selection, ref end))
while (Unsafe.IsAddressLessThan(ref selection, ref end))
{
if (!executionStep.AllSelections.Contains(selection) &&
!selection.Field.Name.EqualsOrdinal(IntrospectionFields.TypeName))
Expand Down Expand Up @@ -428,6 +432,14 @@ protected void ResolveRequirements(
}

var argumentValue = selection.Arguments[argumentVariable.ArgumentName];

if (argumentValue.IsDefaultValue)
{
// We don't want to register and pass a value to an argument
// that wasn't explicitly specified in the original operation.
continue;
}

context.VariableValues.Add(variable.Name, argumentValue.ValueLiteral!);
TryForwardVariable(
context,
Expand Down Expand Up @@ -469,9 +481,9 @@ protected void ResolveRequirements(

foreach (var requirement in resolver.Requires)
{
if (!context.VariableValues.ContainsKey(requirement))
if (!context.VariableValues.ContainsKey(requirement) &&
variableStateLookup.TryGetValue(requirement, out var stateKey))
{
var stateKey = variableStateLookup[requirement];
context.VariableValues.Add(requirement, new VariableNode(stateKey));
}
}
Expand Down Expand Up @@ -529,6 +541,22 @@ protected void TryForwardVariable(
}
}

private static IReadOnlyList<string>? GetUnspecifiedArguments(ISelection selection)
{
List<string>? unspecifiedArguments = null;

foreach (var argument in selection.Arguments)
{
if (argument.IsDefaultValue)
{
unspecifiedArguments ??= new List<string>();
unspecifiedArguments.Add(argument.Name);
}
}

return unspecifiedArguments;
}

private sealed class VariableVisitor : SyntaxWalker<VariableVisitorContext>
{
protected override ISyntaxVisitorAction Enter(
Expand Down
Loading
Loading