Skip to content

Commit 7156ab0

Browse files
authoredApr 28, 2021
Fixed variable rewriter (#3611)
1 parent 6617ec6 commit 7156ab0

9 files changed

+366
-24
lines changed
 

‎src/HotChocolate/Core/src/Execution/Processing/VariableCoercionHelper.cs

+4-4
Original file line numberDiff line numberDiff line change
@@ -52,13 +52,13 @@ public void CoerceVariableValues(
5252
{
5353
throw ThrowHelper.NonNullVariableIsNull(variableDefinition);
5454
}
55-
coercedValues[variableName] = new VariableValueOrLiteral(
56-
variableType, null, NullValueNode.Default);
55+
coercedValues[variableName] =
56+
new VariableValueOrLiteral(variableType, null, NullValueNode.Default);
5757
}
5858
else
5959
{
60-
coercedValues[variableName] = CoerceVariableValue(
61-
variableDefinition, variableType, value);
60+
coercedValues[variableName] =
61+
CoerceVariableValue(variableDefinition, variableType, value);
6262
}
6363
}
6464
}

‎src/HotChocolate/Core/src/Execution/Processing/VariableRewriter.cs

+8
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,10 @@ private static ObjectValueNode Rewrite(
7878
}
7979
rewrittenItems[i] = rewritten;
8080
}
81+
else if (rewrittenItems is { })
82+
{
83+
rewrittenItems[i] = node.Fields[i];
84+
}
8185
}
8286

8387
if (rewrittenItems is { })
@@ -138,6 +142,10 @@ private static ListValueNode Rewrite(
138142
}
139143
rewrittenItems[i] = rewritten;
140144
}
145+
else if (rewrittenItems is { })
146+
{
147+
rewrittenItems[i] = node.Items[i];
148+
}
141149
}
142150

143151
if (rewrittenItems is { })

‎src/HotChocolate/Data/src/Data/Filters/Expressions/QueryableFilterProvider.cs

+8-12
Original file line numberDiff line numberDiff line change
@@ -55,12 +55,11 @@ context.LocalContextData[ContextValueNodeKey] is IValueNode node
5555
: context.ArgumentLiteral<IValueNode>(argumentName);
5656

5757
// if no filter is defined we can stop here and yield back control.
58-
if (filter.IsNull() ||
59-
(context.LocalContextData.TryGetValue(
60-
SkipFilteringKey,
61-
out object? skipObject) &&
62-
skipObject is bool skip &&
63-
skip))
58+
var skipFiltering =
59+
context.LocalContextData.TryGetValue(SkipFilteringKey, out object? skip) &&
60+
skip is true;
61+
62+
if (filter.IsNull() || skipFiltering)
6463
{
6564
return;
6665
}
@@ -72,15 +71,12 @@ skipObject is bool skip &&
7271
executorObj is VisitFilterArgument executor)
7372
{
7473
var inMemory =
75-
context.Result is QueryableExecutable<TEntityType> executable &&
76-
executable.InMemory ||
74+
context.Result is QueryableExecutable<TEntityType> { InMemory: true } ||
7775
context.Result is not IQueryable ||
7876
context.Result is EnumerableQuery;
7977

80-
QueryableFilterContext visitorContext = executor(
81-
filter,
82-
filterInput,
83-
inMemory);
78+
QueryableFilterContext visitorContext =
79+
executor(filter, filterInput, inMemory);
8480

8581
// compile expression tree
8682
if (visitorContext.TryCreateLambda(

‎src/HotChocolate/Data/src/Data/Sorting/Expressions/QueryableSortProvider.cs

+6-8
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,11 @@ async ValueTask ExecuteAsync(
4444
IValueNode sort = context.ArgumentLiteral<IValueNode>(argumentName);
4545

4646
// if no sort is defined we can stop here and yield back control.
47-
if (sort.IsNull() ||
48-
(context.LocalContextData.TryGetValue(
49-
SkipSortingKey,
50-
out object? skipObject) &&
51-
skipObject is bool skip &&
52-
skip))
47+
var skipSorting =
48+
context.LocalContextData.TryGetValue(SkipSortingKey, out object? skip) &&
49+
skip is true;
50+
51+
if (sort.IsNull() || skipSorting)
5352
{
5453
return;
5554
}
@@ -63,8 +62,7 @@ lt.ElementType is NonNullType nn &&
6362
executorObj is VisitSortArgument executor)
6463
{
6564
var inMemory =
66-
context.Result is QueryableExecutable<TEntityType> executable &&
67-
executable.InMemory ||
65+
context.Result is QueryableExecutable<TEntityType> { InMemory: true } ||
6866
context.Result is not IQueryable ||
6967
context.Result is EnumerableQuery;
7068

‎src/HotChocolate/Data/test/Data.Tests/IntegrationTests.cs

+64
Original file line numberDiff line numberDiff line change
@@ -577,6 +577,70 @@ public async Task CreateSchema_OnDifferentScope()
577577
result.ToJson().MatchSnapshot(new SnapshotNameExtension("Result"));
578578
}
579579

580+
[Fact]
581+
public async Task Execute_And_OnRoot()
582+
{
583+
// arrange
584+
IRequestExecutor executor = await new ServiceCollection()
585+
.AddGraphQL()
586+
.AddFiltering("Foo")
587+
.AddSorting("Foo")
588+
.AddProjections("Foo")
589+
.AddQueryType<DifferentScope>()
590+
.BuildRequestExecutorAsync();
591+
592+
// act
593+
IExecutionResult result = await executor.ExecuteAsync(
594+
@"
595+
query GetBooks($title: String) {
596+
books(where: {
597+
and: [
598+
{ title: { startsWith: $title } },
599+
{ title: { eq: ""BookTitle"" } },
600+
]
601+
}) {
602+
nodes { title }
603+
}
604+
}",
605+
new Dictionary<string, object?> { ["title"] = "BookTitle" });
606+
607+
// assert
608+
executor.Schema.Print().MatchSnapshot(new SnapshotNameExtension("Schema"));
609+
result.ToJson().MatchSnapshot(new SnapshotNameExtension("Result"));
610+
}
611+
612+
[Fact]
613+
public async Task Execute_And_OnRoot_Reverse()
614+
{
615+
// arrange
616+
IRequestExecutor executor = await new ServiceCollection()
617+
.AddGraphQL()
618+
.AddFiltering("Foo")
619+
.AddSorting("Foo")
620+
.AddProjections("Foo")
621+
.AddQueryType<DifferentScope>()
622+
.BuildRequestExecutorAsync();
623+
624+
// act
625+
IExecutionResult result = await executor.ExecuteAsync(
626+
@"
627+
query GetBooks($title: String) {
628+
books(where: {
629+
and: [
630+
{ title: { eq: ""BookTitle"" } },
631+
{ title: { startsWith: $title } },
632+
]
633+
}) {
634+
nodes { title }
635+
}
636+
}",
637+
new Dictionary<string, object?> { ["title"] = "BookTitle" });
638+
639+
// assert
640+
executor.Schema.Print().MatchSnapshot(new SnapshotNameExtension("Schema"));
641+
result.ToJson().MatchSnapshot(new SnapshotNameExtension("Result"));
642+
}
643+
580644
public class FooType : ObjectType
581645
{
582646
protected override void Configure(IObjectTypeDescriptor descriptor)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
{
2+
"data": {
3+
"books": {
4+
"nodes": [
5+
{
6+
"title": "BookTitle"
7+
}
8+
]
9+
}
10+
}
11+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
{
2+
"data": {
3+
"books": {
4+
"nodes": [
5+
{
6+
"title": "BookTitle"
7+
}
8+
]
9+
}
10+
}
11+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
schema {
2+
query: DifferentScope
3+
}
4+
5+
type Author {
6+
id: Int!
7+
name: String
8+
books: [Book!]!
9+
}
10+
11+
type Book {
12+
id: Int!
13+
authorId: Int!
14+
title: String
15+
author: Author
16+
}
17+
18+
"A connection to a list of items."
19+
type BookConnection {
20+
"Information to aid in pagination."
21+
pageInfo: PageInfo!
22+
"A list of edges."
23+
edges: [BookEdge!]
24+
"A flattened list of the nodes."
25+
nodes: [Book!]
26+
}
27+
28+
"An edge in a connection."
29+
type BookEdge {
30+
"A cursor for use in pagination."
31+
cursor: String!
32+
"The item at the end of the edge."
33+
node: Book!
34+
}
35+
36+
type DifferentScope {
37+
books(first: Int after: String last: Int before: String where: Foo_BookFilterInput order: [Foo_BookSortInput!]): BookConnection
38+
}
39+
40+
"Information about pagination in a connection."
41+
type PageInfo {
42+
"Indicates whether more edges exist following the set defined by the clients arguments."
43+
hasNextPage: Boolean!
44+
"Indicates whether more edges exist prior the set defined by the clients arguments."
45+
hasPreviousPage: Boolean!
46+
"When paginating backwards, the cursor to continue."
47+
startCursor: String
48+
"When paginating forwards, the cursor to continue."
49+
endCursor: String
50+
}
51+
52+
input Foo_AuthorFilterInput {
53+
and: [Foo_AuthorFilterInput!]
54+
or: [Foo_AuthorFilterInput!]
55+
id: Foo_ComparableInt32OperationFilterInput
56+
name: Foo_StringOperationFilterInput
57+
books: Foo_ListFilterInputTypeOfBookFilterInput
58+
}
59+
60+
input Foo_AuthorSortInput {
61+
id: Foo_SortEnumType
62+
name: Foo_SortEnumType
63+
}
64+
65+
input Foo_BookFilterInput {
66+
and: [Foo_BookFilterInput!]
67+
or: [Foo_BookFilterInput!]
68+
id: Foo_ComparableInt32OperationFilterInput
69+
authorId: Foo_ComparableInt32OperationFilterInput
70+
title: Foo_StringOperationFilterInput
71+
author: Foo_AuthorFilterInput
72+
}
73+
74+
input Foo_BookSortInput {
75+
id: Foo_SortEnumType
76+
authorId: Foo_SortEnumType
77+
title: Foo_SortEnumType
78+
author: Foo_AuthorSortInput
79+
}
80+
81+
input Foo_ComparableInt32OperationFilterInput {
82+
eq: Int
83+
neq: Int
84+
in: [Int!]
85+
nin: [Int!]
86+
gt: Int
87+
ngt: Int
88+
gte: Int
89+
ngte: Int
90+
lt: Int
91+
nlt: Int
92+
lte: Int
93+
nlte: Int
94+
}
95+
96+
input Foo_ListFilterInputTypeOfBookFilterInput {
97+
all: Foo_BookFilterInput
98+
none: Foo_BookFilterInput
99+
some: Foo_BookFilterInput
100+
any: Boolean
101+
}
102+
103+
input Foo_StringOperationFilterInput {
104+
and: [Foo_StringOperationFilterInput!]
105+
or: [Foo_StringOperationFilterInput!]
106+
eq: String
107+
neq: String
108+
contains: String
109+
ncontains: String
110+
in: [String]
111+
nin: [String]
112+
startsWith: String
113+
nstartsWith: String
114+
endsWith: String
115+
nendsWith: String
116+
}
117+
118+
enum Foo_SortEnumType {
119+
ASC
120+
DESC
121+
}
122+
123+
"The `@defer` directive may be provided for fragment spreads and inline fragments to inform the executor to delay the execution of the current fragment to indicate deprioritization of the current fragment. A query with `@defer` directive will cause the request to potentially return multiple responses, where non-deferred data is delivered in the initial response and data deferred is delivered in a subsequent response. `@include` and `@skip` take precedence over `@defer`."
124+
directive @defer("If this argument label has a value other than null, it will be passed on to the result of this defer directive. This label is intended to give client applications a way to identify to which fragment a deferred result belongs to." label: String "Deferred when true." if: Boolean) on FRAGMENT_SPREAD | INLINE_FRAGMENT
125+
126+
"The `@stream` directive may be provided for a field of `List` type so that the backend can leverage technology such as asynchronous iterators to provide a partial list in the initial response, and additional list items in subsequent responses. `@include` and `@skip` take precedence over `@stream`."
127+
directive @stream("If this argument label has a value other than null, it will be passed on to the result of this stream directive. This label is intended to give client applications a way to identify to which fragment a streamed result belongs to." label: String "The initial elements that shall be send down to the consumer." initialCount: Int! "Streamed when true." if: Boolean!) on FIELD
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
schema {
2+
query: DifferentScope
3+
}
4+
5+
type Author {
6+
id: Int!
7+
name: String
8+
books: [Book!]!
9+
}
10+
11+
type Book {
12+
id: Int!
13+
authorId: Int!
14+
title: String
15+
author: Author
16+
}
17+
18+
"A connection to a list of items."
19+
type BookConnection {
20+
"Information to aid in pagination."
21+
pageInfo: PageInfo!
22+
"A list of edges."
23+
edges: [BookEdge!]
24+
"A flattened list of the nodes."
25+
nodes: [Book!]
26+
}
27+
28+
"An edge in a connection."
29+
type BookEdge {
30+
"A cursor for use in pagination."
31+
cursor: String!
32+
"The item at the end of the edge."
33+
node: Book!
34+
}
35+
36+
type DifferentScope {
37+
books(first: Int after: String last: Int before: String where: Foo_BookFilterInput order: [Foo_BookSortInput!]): BookConnection
38+
}
39+
40+
"Information about pagination in a connection."
41+
type PageInfo {
42+
"Indicates whether more edges exist following the set defined by the clients arguments."
43+
hasNextPage: Boolean!
44+
"Indicates whether more edges exist prior the set defined by the clients arguments."
45+
hasPreviousPage: Boolean!
46+
"When paginating backwards, the cursor to continue."
47+
startCursor: String
48+
"When paginating forwards, the cursor to continue."
49+
endCursor: String
50+
}
51+
52+
input Foo_AuthorFilterInput {
53+
and: [Foo_AuthorFilterInput!]
54+
or: [Foo_AuthorFilterInput!]
55+
id: Foo_ComparableInt32OperationFilterInput
56+
name: Foo_StringOperationFilterInput
57+
books: Foo_ListFilterInputTypeOfBookFilterInput
58+
}
59+
60+
input Foo_AuthorSortInput {
61+
id: Foo_SortEnumType
62+
name: Foo_SortEnumType
63+
}
64+
65+
input Foo_BookFilterInput {
66+
and: [Foo_BookFilterInput!]
67+
or: [Foo_BookFilterInput!]
68+
id: Foo_ComparableInt32OperationFilterInput
69+
authorId: Foo_ComparableInt32OperationFilterInput
70+
title: Foo_StringOperationFilterInput
71+
author: Foo_AuthorFilterInput
72+
}
73+
74+
input Foo_BookSortInput {
75+
id: Foo_SortEnumType
76+
authorId: Foo_SortEnumType
77+
title: Foo_SortEnumType
78+
author: Foo_AuthorSortInput
79+
}
80+
81+
input Foo_ComparableInt32OperationFilterInput {
82+
eq: Int
83+
neq: Int
84+
in: [Int!]
85+
nin: [Int!]
86+
gt: Int
87+
ngt: Int
88+
gte: Int
89+
ngte: Int
90+
lt: Int
91+
nlt: Int
92+
lte: Int
93+
nlte: Int
94+
}
95+
96+
input Foo_ListFilterInputTypeOfBookFilterInput {
97+
all: Foo_BookFilterInput
98+
none: Foo_BookFilterInput
99+
some: Foo_BookFilterInput
100+
any: Boolean
101+
}
102+
103+
input Foo_StringOperationFilterInput {
104+
and: [Foo_StringOperationFilterInput!]
105+
or: [Foo_StringOperationFilterInput!]
106+
eq: String
107+
neq: String
108+
contains: String
109+
ncontains: String
110+
in: [String]
111+
nin: [String]
112+
startsWith: String
113+
nstartsWith: String
114+
endsWith: String
115+
nendsWith: String
116+
}
117+
118+
enum Foo_SortEnumType {
119+
ASC
120+
DESC
121+
}
122+
123+
"The `@defer` directive may be provided for fragment spreads and inline fragments to inform the executor to delay the execution of the current fragment to indicate deprioritization of the current fragment. A query with `@defer` directive will cause the request to potentially return multiple responses, where non-deferred data is delivered in the initial response and data deferred is delivered in a subsequent response. `@include` and `@skip` take precedence over `@defer`."
124+
directive @defer("If this argument label has a value other than null, it will be passed on to the result of this defer directive. This label is intended to give client applications a way to identify to which fragment a deferred result belongs to." label: String "Deferred when true." if: Boolean) on FRAGMENT_SPREAD | INLINE_FRAGMENT
125+
126+
"The `@stream` directive may be provided for a field of `List` type so that the backend can leverage technology such as asynchronous iterators to provide a partial list in the initial response, and additional list items in subsequent responses. `@include` and `@skip` take precedence over `@stream`."
127+
directive @stream("If this argument label has a value other than null, it will be passed on to the result of this stream directive. This label is intended to give client applications a way to identify to which fragment a streamed result belongs to." label: String "The initial elements that shall be send down to the consumer." initialCount: Int! "Streamed when true." if: Boolean!) on FIELD

0 commit comments

Comments
 (0)
Please sign in to comment.