Skip to content

Commit e4f056b

Browse files
captainsafiacaptainsafia
authored
Add tests for null response scenarios to RDG (#49970)
1 parent 3546c16 commit e4f056b

6 files changed

+114
-24
lines changed

‎src/Http/Http.Extensions/gen/StaticRouteHandlerModel/StaticRouteHandlerModel.Emitter.cs

Lines changed: 33 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -84,21 +84,28 @@ public static void EmitRequestHandler(this Endpoint endpoint, CodeWriter codeWri
8484
{
8585
return;
8686
}
87-
if (!endpoint.Response.HasNoResponse)
87+
if (endpoint.Response.IsAwaitable)
8888
{
89-
codeWriter.Write("var result = ");
89+
codeWriter.WriteLine($"var task = handler({endpoint.EmitArgumentList()});");
9090
}
91-
if (endpoint.Response.IsAwaitable)
91+
if (endpoint.Response.IsAwaitable && endpoint.Response.ResponseType?.NullableAnnotation == NullableAnnotation.Annotated)
92+
{
93+
codeWriter.WriteLine("if (task == null)");
94+
codeWriter.StartBlock();
95+
codeWriter.WriteLine("""throw new InvalidOperationException("The Task returned by the Delegate must not be null.");""");
96+
codeWriter.EndBlock();
97+
}
98+
if (!endpoint.Response.HasNoResponse)
9299
{
93-
codeWriter.Write("await ");
100+
codeWriter.Write("var result = ");
94101
}
95-
codeWriter.WriteLine($"handler({endpoint.EmitArgumentList()});");
102+
codeWriter.WriteLine(endpoint.Response.IsAwaitable ? "await task;" : $"handler({endpoint.EmitArgumentList()});");
96103

97104
endpoint.Response.EmitHttpResponseContentType(codeWriter);
98105

99106
if (!endpoint.Response.HasNoResponse)
100107
{
101-
codeWriter.WriteLine(endpoint.Response.EmitResponseWritingCall(endpoint.IsAwaitable));
108+
endpoint.Response.EmitResponseWritingCall(codeWriter, endpoint.IsAwaitable);
102109
}
103110
else if (!endpoint.IsAwaitable)
104111
{
@@ -125,33 +132,37 @@ private static void EmitHttpResponseContentType(this EndpointResponse endpointRe
125132
}
126133
}
127134

128-
private static string EmitResponseWritingCall(this EndpointResponse endpointResponse, bool isAwaitable)
135+
private static void EmitResponseWritingCall(this EndpointResponse endpointResponse, CodeWriter codeWriter, bool isAwaitable)
129136
{
130137
var returnOrAwait = isAwaitable ? "await" : "return";
131138

132139
if (endpointResponse.IsIResult)
133140
{
134-
return $"{returnOrAwait} GeneratedRouteBuilderExtensionsCore.ExecuteAsyncExplicit(result, httpContext);";
141+
codeWriter.WriteLine("if (result == null)");
142+
codeWriter.StartBlock();
143+
codeWriter.WriteLine("""throw new InvalidOperationException("The IResult returned by the Delegate must not be null.");""");
144+
codeWriter.EndBlock();
145+
codeWriter.WriteLine($"{returnOrAwait} GeneratedRouteBuilderExtensionsCore.ExecuteAsyncExplicit(result, httpContext);");
135146
}
136147
else if (endpointResponse.ResponseType?.SpecialType == SpecialType.System_String)
137148
{
138-
return $"{returnOrAwait} httpContext.Response.WriteAsync(result);";
149+
codeWriter.WriteLine($"{returnOrAwait} httpContext.Response.WriteAsync(result);");
139150
}
140151
else if (endpointResponse.ResponseType?.SpecialType == SpecialType.System_Object)
141152
{
142-
return $"{returnOrAwait} GeneratedRouteBuilderExtensionsCore.ExecuteReturnAsync(result, httpContext, objectJsonTypeInfo);";
153+
codeWriter.WriteLine($"{returnOrAwait} GeneratedRouteBuilderExtensionsCore.ExecuteReturnAsync(result, httpContext, objectJsonTypeInfo);");
143154
}
144155
else if (!endpointResponse.HasNoResponse)
145156
{
146-
return $"{returnOrAwait} {endpointResponse.EmitJsonResponse()}";
157+
codeWriter.WriteLine($"{returnOrAwait} {endpointResponse.EmitJsonResponse()}");
147158
}
148159
else if (!endpointResponse.IsAwaitable && endpointResponse.HasNoResponse)
149160
{
150-
return $"{returnOrAwait} Task.CompletedTask;";
161+
codeWriter.WriteLine($"{returnOrAwait} Task.CompletedTask;");
151162
}
152163
else
153164
{
154-
return $"{returnOrAwait} httpContext.Response.WriteAsync(result);";
165+
codeWriter.WriteLine($"{returnOrAwait} httpContext.Response.WriteAsync(result);");
155166
}
156167
}
157168

@@ -366,7 +377,15 @@ public static void EmitFilteredInvocation(this Endpoint endpoint, CodeWriter cod
366377
}
367378
else if (endpoint.Response?.IsAwaitable == true)
368379
{
369-
codeWriter.WriteLine($"var result = await handler({endpoint.EmitFilteredArgumentList()});");
380+
codeWriter.WriteLine($"var task = handler({endpoint.EmitFilteredArgumentList()});");
381+
if (endpoint.Response?.ResponseType?.NullableAnnotation == NullableAnnotation.Annotated)
382+
{
383+
codeWriter.WriteLine("if (task == null)");
384+
codeWriter.StartBlock();
385+
codeWriter.WriteLine("return (object?)Results.Empty;");
386+
codeWriter.EndBlock();
387+
}
388+
codeWriter.WriteLine($"var result = await task;");
370389
codeWriter.WriteLine("return (object?)result;");
371390
}
372391
else

‎src/Http/Http.Extensions/test/RequestDelegateGenerator/Baselines/MapAction_ExplicitBodyParam_ComplexReturn_Snapshot.generated.txt

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,10 @@ namespace Microsoft.AspNetCore.Http.Generated
120120
return;
121121
}
122122
var result = handler(todo_local!);
123+
if (result == null)
124+
{
125+
throw new InvalidOperationException("The IResult returned by the Delegate must not be null.");
126+
}
123127
await GeneratedRouteBuilderExtensionsCore.ExecuteAsyncExplicit(result, httpContext);
124128
}
125129

@@ -220,6 +224,10 @@ namespace Microsoft.AspNetCore.Http.Generated
220224
return;
221225
}
222226
var result = handler(todo_local);
227+
if (result == null)
228+
{
229+
throw new InvalidOperationException("The IResult returned by the Delegate must not be null.");
230+
}
223231
await GeneratedRouteBuilderExtensionsCore.ExecuteAsyncExplicit(result, httpContext);
224232
}
225233

‎src/Http/Http.Extensions/test/RequestDelegateGenerator/Baselines/MapAction_ReturnsValidationProblemResult_Has_Metadata.generated.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,10 @@ namespace Microsoft.AspNetCore.Http.Generated
109109
return Task.CompletedTask;
110110
}
111111
var result = handler();
112+
if (result == null)
113+
{
114+
throw new InvalidOperationException("The IResult returned by the Delegate must not be null.");
115+
}
112116
return GeneratedRouteBuilderExtensionsCore.ExecuteAsyncExplicit(result, httpContext);
113117
}
114118

‎src/Http/Http.Extensions/test/RequestDelegateGenerator/Baselines/Multiple_MapAction_NoParam_StringReturn.generated.txt

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,8 @@ namespace Microsoft.AspNetCore.Http.Generated
184184
{
185185
return (object?)Results.Empty;
186186
}
187-
var result = await handler();
187+
var task = handler();
188+
var result = await task;
188189
return (object?)result;
189190
},
190191
options.EndpointBuilder,
@@ -199,7 +200,8 @@ namespace Microsoft.AspNetCore.Http.Generated
199200
httpContext.Response.StatusCode = 400;
200201
return;
201202
}
202-
var result = await handler();
203+
var task = handler();
204+
var result = await task;
203205
if (result is string)
204206
{
205207
httpContext.Response.ContentType ??= "text/plain; charset=utf-8";
@@ -274,7 +276,8 @@ namespace Microsoft.AspNetCore.Http.Generated
274276
{
275277
return (object?)Results.Empty;
276278
}
277-
var result = await handler();
279+
var task = handler();
280+
var result = await task;
278281
return (object?)result;
279282
},
280283
options.EndpointBuilder,
@@ -289,7 +292,8 @@ namespace Microsoft.AspNetCore.Http.Generated
289292
httpContext.Response.StatusCode = 400;
290293
return;
291294
}
292-
var result = await handler();
295+
var task = handler();
296+
var result = await task;
293297
if (result is string)
294298
{
295299
httpContext.Response.ContentType ??= "text/plain; charset=utf-8";

‎src/Http/Http.Extensions/test/RequestDelegateGenerator/CompileTimeCreationTests.cs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -374,4 +374,35 @@ public async Task MapAction_NoJsonTypeInfoResolver_ThrowsException()
374374
var exception = Assert.Throws<InvalidOperationException>(() => GetEndpointFromCompilation(compilation, serviceProvider: serviceProvider));
375375
Assert.Equal("JsonSerializerOptions instance must specify a TypeInfoResolver setting before being marked as read-only.", exception.Message);
376376
}
377+
378+
public static IEnumerable<object[]> NullResult
379+
{
380+
get
381+
{
382+
return new List<object[]>
383+
{
384+
new object[] { "IResult? () => null", "The IResult returned by the Delegate must not be null." },
385+
new object[] { "Task<IResult?>? () => null", "The Task returned by the Delegate must not be null." },
386+
new object[] { "Task<bool?>? () => null", "The Task returned by the Delegate must not be null." },
387+
new object[] { "Task<IResult?> () => Task.FromResult<IResult?>(null)", "The IResult returned by the Delegate must not be null." },
388+
new object[] { "ValueTask<IResult?> () => ValueTask.FromResult<IResult?>(null)", "The IResult returned by the Delegate must not be null." },
389+
};
390+
}
391+
}
392+
393+
[Theory]
394+
[MemberData(nameof(NullResult))]
395+
public async Task RequestDelegateThrowsInvalidOperationExceptionOnNullDelegate(string innerSource, string message)
396+
{
397+
var source = $"""
398+
app.MapGet("/", {innerSource});
399+
""";
400+
var (_, compilation) = await RunGeneratorAsync(source);
401+
var endpoint = GetEndpointFromCompilation(compilation);
402+
403+
var httpContext = CreateHttpContext();
404+
var exception = await Assert.ThrowsAsync<InvalidOperationException>(async () => await endpoint.RequestDelegate(httpContext));
405+
406+
Assert.Equal(message, exception.Message);
407+
}
377408
}

‎src/Http/Http.Extensions/test/RequestDelegateGenerator/RequestDelegateCreationTests.Responses.cs

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,9 @@ public async Task MapAction_NoParam_StringReturn_WithFilter()
6767
[InlineData(@"app.MapGet(""/"", () => 123456);", "123456")]
6868
[InlineData(@"app.MapGet(""/"", () => true);", "true")]
6969
[InlineData(@"app.MapGet(""/"", () => new DateTime(2023, 1, 1));", @"""2023-01-01T00:00:00""")]
70+
[InlineData(@"app.MapGet(""/"", int () => 123456);", "123456")]
71+
[InlineData(@"app.MapGet(""/"", bool () => true);", "true")]
72+
[InlineData(@"app.MapGet(""/"", DateTime () => new DateTime(2023, 1, 1));", @"""2023-01-01T00:00:00""")]
7073
public async Task MapAction_NoParam_AnyReturn(string source, string expectedBody)
7174
{
7275
var (result, compilation) = await RunGeneratorAsync(source);
@@ -85,11 +88,17 @@ public async Task MapAction_NoParam_AnyReturn(string source, string expectedBody
8588
public static IEnumerable<object[]> MapAction_NoParam_ComplexReturn_Data => new List<object[]>()
8689
{
8790
new object[] { """app.MapGet("/", () => new Todo() { Name = "Test Item"});""" },
91+
new object[] { """app.MapGet("/", Todo () => new Todo() { Name = "Test Item"});""" },
92+
new object[] { """app.MapGet("/", Todo? () => new Todo() { Name = "Test Item"});""" },
8893
new object[] { """
8994
object GetTodo() => new Todo() { Name = "Test Item"};
9095
app.MapGet("/", GetTodo);
9196
"""},
92-
new object[] { """app.MapGet("/", () => TypedResults.Ok(new Todo() { Name = "Test Item"}));""" }
97+
new object[] { """
98+
object? GetTodo() => new Todo() { Name = "Test Item"};
99+
app.MapGet("/", GetTodo);
100+
"""},
101+
new object[] { """app.MapGet("/", IResult () => TypedResults.Ok(new Todo() { Name = "Test Item"}));""" }
93102
};
94103

95104
[Theory]
@@ -138,7 +147,10 @@ public async Task MapAction_NoParam_ExtensionResult(string source)
138147
{
139148
new object[] { @"app.MapGet(""/"", () => Task.FromResult(""Hello world!""));", "Hello world!" },
140149
new object[] { @"app.MapGet(""/"", () => Task.FromResult(new Todo() { Name = ""Test Item"" }));", """{"id":0,"name":"Test Item","isComplete":false}""" },
141-
new object[] { @"app.MapGet(""/"", () => Task.FromResult(TypedResults.Ok(new Todo() { Name = ""Test Item"" })));", """{"id":0,"name":"Test Item","isComplete":false}""" }
150+
new object[] { @"app.MapGet(""/"", () => Task.FromResult(TypedResults.Ok(new Todo() { Name = ""Test Item"" })));", """{"id":0,"name":"Test Item","isComplete":false}""" },
151+
new object[] { @"app.MapGet(""/"", Task<string> () => Task.FromResult(""Hello world!""));", "Hello world!" },
152+
new object[] { @"app.MapGet(""/"", Task<Todo> () => Task.FromResult(new Todo() { Name = ""Test Item"" }));", """{"id":0,"name":"Test Item","isComplete":false}""" },
153+
new object[] { @"app.MapGet(""/"", Task<Microsoft.AspNetCore.Http.HttpResults.Ok<Todo>> () => Task.FromResult(TypedResults.Ok(new Todo() { Name = ""Test Item"" })));", """{"id":0,"name":"Test Item","isComplete":false}""" }
142154
};
143155

144156
[Theory]
@@ -890,10 +902,12 @@ public static IEnumerable<object[]> NullContentResult
890902

891903
var testTaskBoolAction = """
892904
app.MapPost("/", () => Task.FromResult<bool?>(null));
905+
app.MapPost("/", Task<bool?> () => Task.FromResult<bool?>(null));
893906
""";
894907

895908
var testValueTaskBoolAction = """
896909
app.MapPost("/", () => ValueTask.FromResult<bool?>(null));
910+
app.MapPost("/", ValueTask<bool?> () => ValueTask.FromResult<bool?>(null));
897911
""";
898912

899913
var testIntAction = """
@@ -902,22 +916,29 @@ public static IEnumerable<object[]> NullContentResult
902916

903917
var testTaskIntAction = """
904918
app.MapPost("/", () => Task.FromResult<int?>(null));
919+
app.MapPost("/", Task<int?> () => Task.FromResult<int?>(null));
905920
""";
906921

907922
var testValueTaskIntAction = """
908923
app.MapPost("/", () => ValueTask.FromResult<int?>(null));
924+
app.MapPost("/", ValueTask<int?> () => ValueTask.FromResult<int?>(null));
909925
""";
910926

911927
var testTodoAction = """
928+
int id = 0;
929+
Todo? GetMaybeTodo() => id == 0 ? null : new Todo();
912930
app.MapPost("/", Todo? () => null);
931+
app.MapGet("/", GetMaybeTodo);
913932
""";
914933

915934
var testTaskTodoAction = """
916935
app.MapPost("/", () => Task.FromResult<Todo?>(null));
936+
app.MapPost("/", Task<Todo?>? () => Task.FromResult<Todo?>(null));
917937
""";
918938

919939
var testValueTaskTodoAction = """
920940
app.MapPost("/", () => ValueTask.FromResult<Todo?>(null));
941+
app.MapPost("/", ValueTask<Todo?> () => ValueTask.FromResult<Todo?>(null));
921942
""";
922943

923944
var testTodoStructAction = """
@@ -945,12 +966,15 @@ public static IEnumerable<object[]> NullContentResult
945966
public async Task RequestDelegateWritesNullReturnNullValue(string source)
946967
{
947968
var (_, compilation) = await RunGeneratorAsync(source);
948-
var endpoint = GetEndpointFromCompilation(compilation);
969+
var endpoints = GetEndpointsFromCompilation(compilation);
949970

950-
var httpContext = CreateHttpContext();
951-
await endpoint.RequestDelegate(httpContext);
971+
foreach (var endpoint in endpoints)
972+
{
973+
var httpContext = CreateHttpContext();
974+
await endpoint.RequestDelegate(httpContext);
952975

953-
await VerifyResponseBodyAsync(httpContext, "null");
976+
await VerifyResponseBodyAsync(httpContext, "null");
977+
}
954978
}
955979

956980
[Theory]

0 commit comments

Comments
 (0)