Skip to content

Commit

Permalink
Fixed Variable Coercion of Nullable Enum Values (ChilliCream#4267)
Browse files Browse the repository at this point in the history 
Co-authored-by: Michael Staib <michael@chillicream.com>
  • Loading branch information
@PascalSenn @michaelstaib
PascalSenn and michaelstaib authored Sep 26, 2021
1 parent 445b874 commit d42a42c
Show file tree
Hide file tree
Showing 14 changed files with 601 additions and 26 deletions.
47 changes: 37 additions & 10 deletions src/HotChocolate/Core/src/Execution/Processing/VariableCoercionHelper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,10 +142,11 @@ private static IValueNode Rewrite(
case ListValueNode lv:
return Rewrite(inputType, lv);

case StringValueNode sv:
return inputType.Kind is TypeKind.Enum
? new EnumValueNode(sv.Location, sv.Value)
: node;
case StringValueNode sv when inputType.IsEnumType():
return new EnumValueNode(sv.Location, sv.Value);

case StringValueNode:
return node;

default:
return node;
Expand All @@ -156,8 +157,10 @@ private static ObjectValueNode Rewrite(
IType inputType,
ObjectValueNode node)
{
if (!(inputType.NamedType() is InputObjectType inputObjectType))
if (inputType.NamedType() is not InputObjectType inputObjectType)
{
// if the node type is not an input object, we will just return the node
// as if and the deserialization will produce a proper error.
return node;
}

Expand All @@ -169,25 +172,38 @@ private static ObjectValueNode Rewrite(

if (!inputObjectType.Fields.TryGetField(current.Name.Value, out IInputField? field))
{
// if we do not find a field on the type we also skip this error and let
// the deserialization produce a proper error on this.
continue;
}

IValueNode value = Rewrite(field.Type, current.Value);
IValueNode rewritten = Rewrite(field.Type, current.Value);

// we try initially just to traverse the input graph, only if we detect a change
// will we create a new input object. In this case if the fields list is initialized
// we know that we have already collected at least one change. In this case
// all further field nodes have to be added as well even if they do not have
// a changed value since we need to produce a complete new input object value node.
if (fields is not null)
{
fields.Add(current.WithValue(value));
fields.Add(current.WithValue(rewritten));
}
else if (!ReferenceEquals(current.Value, value))

// if we did not so far detect any rewritten field value we will compare if the
// field value node changed. Since, all syntax nodes are immutable we can just
// check if the reference is not the same.
else if (!ReferenceEquals(current.Value, rewritten))
{
// if we detect a reference change we will create the fields list
// that contains all previous field values plus the changed field value.
fields = new List<ObjectFieldNode>();

for (var j = 0; j < i; j++)
{
fields.Add(node.Fields[j]);
}

fields.Add(current.WithValue(value));
fields.Add(current.WithValue(rewritten));
}
}

Expand All @@ -209,12 +225,23 @@ private static ListValueNode Rewrite(IType inputType, ListValueNode node)
IValueNode current = node.Items[i];
IValueNode value = Rewrite(elementType, current);

// we try initially just to traverse the list graph, only if we detect a change
// will we create a new list object. In this case if values list is initialized
// we know that we have already collected at least one change. In this case
// all further value nodes have to be added as well even if they do not have
// a changed value since we need to produce a complete new list value node.
if (values is not null)
{
values.Add(value);
}
else if (!ReferenceEquals(current.Value, value))

// if we did not so far detect any rewritten value we will compare if the
// value node changed. Since, all syntax nodes are immutable we can just
// check if the reference is not the same.
else if (!ReferenceEquals(current, value))
{
// if we detect a reference change we will create the values list
// that contains all previous list values plus the changed list value.
values = new List<IValueNode>();

for (var j = 0; j < i; j++)
Expand Down
255 changes: 253 additions & 2 deletions src/HotChocolate/Core/test/Execution.Tests/Processing/VariableCoercionHelperTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -604,7 +604,7 @@ void Action() => helper.CoerceVariableValues(
}

[Fact]
public void CoerceVariableValues_Should_CoerceEnumList_AsEnumValues()
public void StringValues_Representing_EnumValues_In_Lists_ShouldBe_Rewritten()
{
// arrange
ISchema schema = SchemaBuilder.New()
Expand Down Expand Up @@ -665,7 +665,68 @@ enum TestEnum {
}

[Fact]
public void CoerceVariableValues_Should_CoerceEnumObject_AsEnumValues()
public void StringValues_Representing_NonNullEnumValues_In_Lists_ShouldBe_Rewritten()
{
// arrange
ISchema schema = SchemaBuilder.New()
.AddDocumentFromString(
@"
type Query {
test(list: [FooInput]): String
}
input FooInput {
enum: TestEnum!
}
enum TestEnum {
Foo
Bar
}")
.Use(_ => _ => default)
.Create();

var variableDefinitions = new List<VariableDefinitionNode>
{
new(null,
new VariableNode("abc"),
new ListTypeNode(new NamedTypeNode("FooInput")),
null,
Array.Empty<DirectiveNode>())
};

var variableValues = new Dictionary<string, object>
{
{
"abc",
new ListValueNode(
new ObjectValueNode(
new ObjectFieldNode("enum", "Foo")),
new ObjectValueNode(
new ObjectFieldNode("enum", "Bar")))
}
};

var coercedValues = new Dictionary<string, VariableValueOrLiteral>();
var helper = new VariableCoercionHelper(new(), new(new DefaultTypeConverter()));

// act
helper.CoerceVariableValues(
schema, variableDefinitions, variableValues, coercedValues);

// assert
Assert.Collection(coercedValues,
t =>
{
Assert.Equal("abc", t.Key);
Assert.Equal(
"[ { enum: Foo }, { enum: Bar } ]",
t.Value.ValueLiteral!.ToString());
});
}

[Fact]
public void StringValues_Representing_EnumValues_In_Objects_ShouldBe_Rewritten()
{
// arrange
ISchema schema = SchemaBuilder.New()
Expand Down Expand Up @@ -722,6 +783,196 @@ enum TestEnum {
});
}

[Fact]
public void StringValues_Representing_NonNullEnumValues_In_Objects_ShouldBe_Rewritten()
{
// arrange
ISchema schema = SchemaBuilder.New()
.AddDocumentFromString(
@"
type Query {
test(list: FooInput): String
}
input FooInput {
enum: TestEnum!
enum2: TestEnum!
}
enum TestEnum {
Foo
Bar
}")
.Use(_ => _ => default)
.Create();

var variableDefinitions = new List<VariableDefinitionNode>
{
new(null,
new VariableNode("abc"),
new NamedTypeNode("FooInput"),
null,
Array.Empty<DirectiveNode>())
};

var variableValues = new Dictionary<string, object>
{
{
"abc",
new ObjectValueNode(
new ObjectFieldNode("enum", "Foo"),
new ObjectFieldNode("enum2", "Bar"))
}
};

var coercedValues = new Dictionary<string, VariableValueOrLiteral>();
var helper = new VariableCoercionHelper(new(), new(new DefaultTypeConverter()));

// act
helper.CoerceVariableValues(
schema, variableDefinitions, variableValues, coercedValues);

// assert
Assert.Collection(coercedValues,
t =>
{
Assert.Equal("abc", t.Key);
Assert.Equal("{ enum: Foo, enum2: Bar }", t.Value.ValueLiteral!.ToString());
});
}

[Fact]
public void If_Second_Item_In_Object_Is_Rewritten_The_Previous_Values_Are_Correctly_Copied()
{
// arrange
ISchema schema = SchemaBuilder.New()
.AddDocumentFromString(
@"
type Query {
test(list: FooInput): String
}
input FooInput {
value_a: String
value_b: TestEnum
}
enum TestEnum {
Foo
Bar
}")
.Use(_ => _ => default)
.Create();

var variableDefinitions = new List<VariableDefinitionNode>
{
new(null,
new VariableNode("abc"),
new NamedTypeNode("FooInput"),
null,
Array.Empty<DirectiveNode>())
};

var expectToBeUnchanged = new ObjectFieldNode("value_a", "Foo");
var expectToBeRewritten = new ObjectFieldNode("value_b", "Bar");

var variableValues = new Dictionary<string, object>
{
{
"abc",
new ObjectValueNode(
expectToBeUnchanged,
expectToBeRewritten)
}
};

var coercedValues = new Dictionary<string, VariableValueOrLiteral>();
var helper = new VariableCoercionHelper(new(), new(new DefaultTypeConverter()));

// act
helper.CoerceVariableValues(
schema, variableDefinitions, variableValues, coercedValues);

// assert
Assert.Collection(coercedValues,
t =>
{
Assert.Equal("abc", t.Key);
Assert.Equal(
@"{ value_a: ""Foo"", value_b: Bar }",
t.Value.ValueLiteral.ToString());

ObjectValueNode obj = Assert.IsType<ObjectValueNode>(t.Value.ValueLiteral);
Assert.Same(expectToBeUnchanged, obj.Fields[0]);
Assert.NotSame(expectToBeRewritten, obj.Fields[1]);
});
}

[Fact]
public void If_Second_Item_In_List_Is_Rewritten_The_Previous_Values_Are_Correctly_Copied()
{
// arrange
ISchema schema = SchemaBuilder.New()
.AddDocumentFromString(
@"
type Query {
test(list: [FooInput]): String
}
input FooInput {
value_a: String
value_b: TestEnum
}
enum TestEnum {
Foo
Bar
}")
.Use(_ => _ => default)
.Create();

var variableDefinitions = new List<VariableDefinitionNode>
{
new(null,
new VariableNode("abc"),
new ListTypeNode(new NamedTypeNode("FooInput")),
null,
Array.Empty<DirectiveNode>())
};

var expectToBeUnchanged = new ObjectValueNode(new ObjectFieldNode("value_a", "Foo"));
var expectToBeRewritten = new ObjectValueNode(new ObjectFieldNode("value_b", "Bar"));

var variableValues = new Dictionary<string, object>
{
{
"abc",
new ListValueNode(expectToBeUnchanged, expectToBeRewritten)
}
};

var coercedValues = new Dictionary<string, VariableValueOrLiteral>();
var helper = new VariableCoercionHelper(new(), new(new DefaultTypeConverter()));

// act
helper.CoerceVariableValues(
schema, variableDefinitions, variableValues, coercedValues);

// assert
Assert.Collection(coercedValues,
t =>
{
Assert.Equal("abc", t.Key);
Assert.Equal(
@"[ { value_a: ""Foo"" }, { value_b: Bar } ]",
t.Value.ValueLiteral.ToString());

ListValueNode list = Assert.IsType<ListValueNode>(t.Value.ValueLiteral);
Assert.Same(expectToBeUnchanged, list.Items[0]);
Assert.NotSame(expectToBeRewritten, list.Items[1]);
});
}

[Fact]
public void Variable_Is_Nullable_And_Not_Set()
{
Expand Down
Loading

0 comments on commit d42a42c

Please sign in to comment.