Skip to content

Commit b54cfbe

Browse files
KoditkarVedantjoemcbride
KoditkarVedant
authored and
joemcbride
committed
424 When an object's non-null property resolver returns null (or exception), the 'null' isn't bubbled up to the parent (graphql-dotnet#980)
* Bubble up the null to the parent node * bubble null for non nullable array node * remove NonNullGraphType from queryType and updated the test cases - revert the deleted IntrospectionResult - fix the breaking test cases * Set queryType as NonNullableGraphType Added TestCoreSchema and TestQuery Fixed test cases * Remove redundant `TestCoreSchema` class. * remove space change * Add test cases for BubbleUpTheNullToNextNullable - fix the logic to bubble up the null - code cleanup * Intial Commit * Add test cases for ListGraphType to bubble up the null for non null type * Bubble up the null to the parent node * bubble null for non nullable array node * remove NonNullGraphType from queryType and updated the test cases - revert the deleted IntrospectionResult - fix the breaking test cases * Set queryType as NonNullableGraphType Added TestCoreSchema and TestQuery Fixed test cases * Remove redundant `TestCoreSchema` class. * remove space change * Add test cases for BubbleUpTheNullToNextNullable - fix the logic to bubble up the null - code cleanup * Add test cases for ListGraphType to bubble up the null for non null type * update test cases * override the ToString() method to get the GraphQL Type * ListGraphType: add error when NonNull item resolve to null * if NonNull list returns null or Any element in List of NonNull items returns null * refactor the code * add additional test case to test the case if there are not nullable field in schema * * fix GraphQL Type value retuned for [Type!]!. * fix the breaking test cases * make test cases more readable * refactor the test cases for readability * rename ListOfNonNullable to ListOfStrings to be more generic
1 parent cf296ac commit b54cfbe

File tree

10 files changed

+360
-33
lines changed

10 files changed

+360
-33
lines changed

‎src/GraphQL.Tests/Bugs/BubbleUpTheNullToNextNullable.cs

Lines changed: 285 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,285 @@
1+
namespace GraphQL.Tests.Bugs
2+
{
3+
using GraphQL.Types;
4+
using Shouldly;
5+
using System.Collections.Generic;
6+
using Xunit;
7+
8+
public class BubbleUpTheNullToNextNullable : QueryTestBase<BubbleNullSchema>
9+
{
10+
[Fact]
11+
public void Nullable_field_resolve_to_null_should_not_bubble_up_the_null()
12+
{
13+
const string QUERY = "{ nullableDataGraph { nullable } }";
14+
const string EXPECTED = "{ nullableDataGraph: { nullable: null } }";
15+
var data = new Data {Nullable = null};
16+
var errors = new ExecutionError[] { };
17+
18+
AssertResult(QUERY, EXPECTED, data, errors);
19+
}
20+
21+
[Fact]
22+
public void NonNull_field_resolve_with_non_null_value_should_not_throw_error()
23+
{
24+
const string QUERY = "{ nullableDataGraph { nonNullable } }";
25+
const string EXPECTED = "{ nullableDataGraph: { nonNullable: 'data' } }";
26+
var data = new Data { NonNullable = "data" };
27+
var errors = new ExecutionError[] { };
28+
29+
AssertResult(QUERY, EXPECTED, data, errors);
30+
}
31+
32+
[Fact]
33+
public void NonNull_field_resolve_to_null_should_bubble_up_null_to_parent()
34+
{
35+
const string QUERY = "{ nullableDataGraph { nonNullable } }";
36+
const string EXPECTED = "{ nullableDataGraph: null }";
37+
var data = new Data {NonNullable = null};
38+
var errors = new[]
39+
{
40+
new ExecutionError("Cannot return null for non-null type. Field: nonNullable, Type: String!.")
41+
{
42+
Path = new[] {"nullableDataGraph", "nonNullable"}
43+
}
44+
};
45+
46+
AssertResult(QUERY,EXPECTED, data, errors);
47+
}
48+
49+
[Fact]
50+
public void NonNull_field_resolve_to_null_should_bubble_up_the_null_to_first_nullable_parent_in_chain_of_nullable()
51+
{
52+
const string QUERY = "{ nullableDataGraph { nullableNest { nonNullable } } }";
53+
const string EXPECTED = "{ nullableDataGraph: { nullableNest: null } }";
54+
var data = new Data {NullableNest = new Data {NonNullable = null}};
55+
var errors = new[]
56+
{
57+
new ExecutionError("Cannot return null for non-null type. Field: nonNullable, Type: String!.")
58+
{
59+
Path = new[] {"nullableDataGraph", "nullableNest", "nonNullable"}
60+
}
61+
};
62+
63+
AssertResult(QUERY, EXPECTED, data, errors);
64+
}
65+
66+
[Fact]
67+
public void NonNull_field_resolve_to_null_should_bubble_up_the_null_to_first_nullable_parent_in_chain_of_non_null_fields()
68+
{
69+
const string QUERY = "{ nullableDataGraph { nonNullableNest { nonNullable } } }";
70+
const string EXPECTED = "{ nullableDataGraph: null }";
71+
var data = new Data {NonNullableNest = new Data {NonNullable = null}};
72+
var errors = new[]
73+
{
74+
new ExecutionError("Cannot return null for non-null type. Field: nonNullable, Type: String!.")
75+
{
76+
Path = new[] {"nullableDataGraph", "nonNullableNest", "nonNullable"}
77+
}
78+
};
79+
80+
AssertResult(QUERY, EXPECTED, data, errors);
81+
}
82+
83+
[Fact]
84+
public void NonNull_field_resolve_to_null_should_null_the_top_level_if_no_nullable_parent_present()
85+
{
86+
const string QUERY = "{ nonNullableDataGraph { nonNullableNest { nonNullable } } }";
87+
const string EXPECTED = null;
88+
var data = new Data {NonNullableNest = new Data {NonNullable = null}};
89+
var errors = new[]
90+
{
91+
new ExecutionError("Cannot return null for non-null type. Field: nonNullable, Type: String!.")
92+
{
93+
Path = new[] {"nonNullableDataGraph", "nonNullableNest", "nonNullable"}
94+
}
95+
};
96+
97+
AssertResult(QUERY, EXPECTED, data, errors);
98+
}
99+
100+
[Fact]
101+
public void ListOfNonNull_containing_null_should_bubble_up_the_null()
102+
{
103+
const string QUERY = "{ nonNullableDataGraph { listOfNonNullable } }";
104+
const string EXPECTED = "{ nonNullableDataGraph: { listOfNonNullable: null } }";
105+
var data = new Data {ListOfStrings = new List<string> {"text", null, null}};
106+
var errors = new[]
107+
{
108+
new ExecutionError("Cannot return null for non-null type. Field: listOfNonNullable, Type: [String!].")
109+
{
110+
Path = new[] {"nonNullableDataGraph", "listOfNonNullable", "1"}
111+
}
112+
};
113+
114+
AssertResult(QUERY, EXPECTED, data, errors);
115+
}
116+
117+
[Fact]
118+
public void NonNullList_resolve_to_null_should_bubble_up_the_null()
119+
{
120+
const string QUERY = "{ nullableDataGraph { nonNullableList } }";
121+
const string EXPECTED = "{ nullableDataGraph: null }";
122+
var data = new Data {ListOfStrings = null};
123+
var errors = new[]
124+
{
125+
new ExecutionError("Cannot return null for non-null type. Field: nonNullableList, Type: [String]!.")
126+
{
127+
Path = new[] {"nullableDataGraph", "nonNullableList"}
128+
}
129+
};
130+
131+
AssertResult(QUERY, EXPECTED, data, errors);
132+
}
133+
134+
[Fact]
135+
public void NonNullList_resolve_to_null_should_null_top_level_if_no_nullable_parent_found()
136+
{
137+
const string QUERY = "{ nonNullableDataGraph { nonNullableList } }";
138+
const string EXPECTED = null;
139+
var data = new Data {ListOfStrings = null};
140+
var errors = new[]
141+
{
142+
new ExecutionError("Cannot return null for non-null type. Field: nonNullableList, Type: [String]!.")
143+
{
144+
Path = new[] {"nonNullableDataGraph", "nonNullableList"}
145+
}
146+
};
147+
148+
AssertResult(QUERY, EXPECTED, data, errors);
149+
}
150+
151+
[Fact]
152+
public void NoNullListOfNonNull_contains_null_should_bubble_up_the_null()
153+
{
154+
const string QUERY = "{ nullableDataGraph { nonNullableListOfNonNullable } }";
155+
const string EXPECTED = "{ nullableDataGraph: null }";
156+
var data = new Data {ListOfStrings = new List<string> {"text", null, null}};
157+
var errors = new[]
158+
{
159+
new ExecutionError(
160+
"Cannot return null for non-null type. Field: nonNullableListOfNonNullable, Type: [String!]!.")
161+
{
162+
Path = new[] {"nullableDataGraph", "nonNullableListOfNonNullable", "1"}
163+
}
164+
};
165+
166+
AssertResult(QUERY, EXPECTED, data, errors);
167+
}
168+
169+
[Fact]
170+
public void NonNullListOfNonNull_resolve_to_null_should_bubble_up_the_null()
171+
{
172+
const string QUERY = "{ nullableDataGraph { nonNullableListOfNonNullable } }";
173+
const string EXPECTED = "{ nullableDataGraph: null }";
174+
var data = new Data {ListOfStrings = null};
175+
var errors = new[]
176+
{
177+
new ExecutionError(
178+
"Cannot return null for non-null type. Field: nonNullableListOfNonNullable, Type: [String!]!.")
179+
{
180+
Path = new[] {"nullableDataGraph", "nonNullableListOfNonNullable"}
181+
}
182+
};
183+
184+
AssertResult(QUERY, EXPECTED, data, errors);
185+
}
186+
187+
private void AssertResult(string query, string expected, Data data, IReadOnlyList<ExecutionError> errors)
188+
{
189+
ExecutionResult result =
190+
AssertQueryWithErrors(
191+
query,
192+
expected,
193+
root: data,
194+
expectedErrorCount: errors.Count);
195+
196+
ExecutionErrors actualErrors = result.Errors;
197+
198+
if (errors.Count == 0)
199+
{
200+
actualErrors.ShouldBeNull();
201+
}
202+
else
203+
{
204+
actualErrors.Count.ShouldBe(errors.Count);
205+
206+
for (var i = 0; i < errors.Count; i++)
207+
{
208+
ExecutionError actualError = actualErrors[i];
209+
ExecutionError expectedError = errors[i];
210+
211+
actualError.Message.ShouldBe(expectedError.Message);
212+
actualError.Path.ShouldBe(expectedError.Path);
213+
}
214+
}
215+
}
216+
217+
}
218+
219+
public class BubbleNullSchema : Schema
220+
{
221+
public BubbleNullSchema()
222+
{
223+
var query = new ObjectGraphType();
224+
225+
query.Field<NonNullGraphType<DataGraphType>>(
226+
"nonNullableDataGraph",
227+
resolve: c => new DataGraphType() { Data = c.Source as Data }
228+
);
229+
230+
query.Field<DataGraphType>(
231+
"nullableDataGraph",
232+
resolve: c => new DataGraphType() { Data = c.Source as Data }
233+
);
234+
235+
Query = query;
236+
}
237+
}
238+
239+
public class DataGraphType : ObjectGraphType<DataGraphType>
240+
{
241+
public DataGraphType()
242+
{
243+
Name = "dataType";
244+
245+
Field<StringGraphType>(
246+
"nullable",
247+
resolve: c => c.Source.Data.Nullable);
248+
249+
Field<NonNullGraphType<StringGraphType>>(
250+
"nonNullable",
251+
resolve: c => c.Source.Data.NonNullable);
252+
253+
Field<ListGraphType<NonNullGraphType<StringGraphType>>>(
254+
"listOfNonNullable",
255+
resolve: c => c.Source.Data.ListOfStrings);
256+
257+
Field<NonNullGraphType<ListGraphType<StringGraphType>>>(
258+
"nonNullableList",
259+
resolve: c => c.Source.Data.ListOfStrings);
260+
261+
Field<NonNullGraphType<ListGraphType<NonNullGraphType<StringGraphType>>>>(
262+
"nonNullableListOfNonNullable",
263+
resolve: c => c.Source.Data.ListOfStrings);
264+
265+
Field<NonNullGraphType<DataGraphType>>(
266+
"nonNullableNest",
267+
resolve: c => new DataGraphType() { Data = c.Source.Data.NonNullableNest });
268+
269+
Field<DataGraphType>(
270+
"nullableNest",
271+
resolve: c => new DataGraphType() { Data = c.Source.Data.NullableNest });
272+
}
273+
274+
public Data Data { get; set; }
275+
}
276+
277+
public class Data
278+
{
279+
public string Nullable { get; set; }
280+
public Data NullableNest { get; set; }
281+
public string NonNullable { get; set; }
282+
public Data NonNullableNest { get; set; }
283+
public List<string> ListOfStrings { get; set; }
284+
}
285+
}

‎src/GraphQL.Tests/Introspection/IntrospectionResult.cs

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@ public class IntrospectionResult
66
@"{
77
""data"": {
88
""__schema"": {
9-
""queryType"": null,
9+
""queryType"": {
10+
""name"": ""TestQuery""
11+
},
1012
""mutationType"": null,
1113
""subscriptionType"": null,
1214
""types"": [
@@ -967,6 +969,16 @@ public class IntrospectionResult
967969
}
968970
],
969971
""possibleTypes"": null
972+
},
973+
{
974+
""kind"": ""OBJECT"",
975+
""name"": ""TestQuery"",
976+
""description"": null,
977+
""fields"": [],
978+
""inputFields"": null,
979+
""interfaces"": [],
980+
""enumValues"": null,
981+
""possibleTypes"": null
970982
}
971983
],
972984
""directives"": [
@@ -1042,22 +1054,7 @@ public class IntrospectionResult
10421054
}
10431055
]
10441056
}
1045-
},
1046-
""errors"": [
1047-
{
1048-
""message"": ""Cannot return null for non-null type. Field: queryType, Type: __Type!."",
1049-
""locations"": [
1050-
{
1051-
""line"": 4,
1052-
""column"": 7
1053-
}
1054-
],
1055-
""path"": [
1056-
""__schema"",
1057-
""queryType""
1058-
]
1059-
}
1060-
]
1057+
}
10611058
}";
10621059
}
10631060
}

‎src/GraphQL.Tests/Introspection/SchemaIntrospectionTests.cs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
using GraphQL.Http;
1+
using GraphQL.Http;
22
using GraphQL.Introspection;
33
using GraphQL.Types;
44
using Shouldly;
@@ -14,7 +14,10 @@ public void validate_core_schema()
1414
var documentExecuter = new DocumentExecuter();
1515
var executionResult = documentExecuter.ExecuteAsync(_ =>
1616
{
17-
_.Schema = new Schema();
17+
_.Schema = new Schema()
18+
{
19+
Query = new TestQuery()
20+
};
1821
_.Query = SchemaIntrospection.IntrospectionQuery;
1922
}).GetAwaiter().GetResult();
2023

@@ -23,6 +26,11 @@ public void validate_core_schema()
2326
ShouldBe(json, IntrospectionResult.Data);
2427
}
2528

29+
public class TestQuery : ObjectGraphType
30+
{
31+
public TestQuery() => Name = "TestQuery";
32+
}
33+
2634
[Fact]
2735
public void validate_non_null_schema()
2836
{

‎src/GraphQL.Tests/Types/NonNullGraphTypeTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public void nonnullable_fields_without_values_do_complain()
3939
{
4040
var result = AssertQueryWithErrors(
4141
"{ nonNullable { a b c } }",
42-
"{ nonNullable: { a: null, b: null, c: null } }",
42+
"{ nonNullable: null }",
4343
root: new ExampleContext(null, null, null),
4444
expectedErrorCount: 3);
4545

0 commit comments

Comments
 (0)