Skip to content

Commit de86920

Browse files
captainsafiacaptainsafia
committed
Address feedback
1 parent 5d844ff commit de86920

File tree

7 files changed

+103
-35
lines changed

7 files changed

+103
-35
lines changed

‎eng/SharedFramework.External.props‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242
<ExternalAspNetCoreAppReference Include="Microsoft.Extensions.Options.DataAnnotations" Version="$(MicrosoftExtensionsOptionsDataAnnotationsVersion)" />
4343
<ExternalAspNetCoreAppReference Include="Microsoft.Extensions.Options" Version="$(MicrosoftExtensionsOptionsVersion)" />
4444
<ExternalAspNetCoreAppReference Include="Microsoft.Extensions.Primitives" Version="$(MicrosoftExtensionsPrimitivesVersion)" />
45-
<ExternalAspNetCoreAppReference Include="System.Net.ServerSentEvents" Version="$(SystemNextServerSentEvents)" />
45+
<ExternalAspNetCoreAppReference Include="System.Net.ServerSentEvents" Version="$(SystemNetServerSentEvents)" />
4646
<ExternalAspNetCoreAppReference Include="System.Security.Cryptography.Xml" Version="$(SystemSecurityCryptographyXmlVersion)" />
4747
<ExternalAspNetCoreAppReference Include="System.Threading.RateLimiting" Version="$(SystemThreadingRateLimitingVersion)" />
4848

‎src/Framework/test/TestData.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@ static TestData()
155155
"Microsoft.Net.Http.Headers",
156156
"System.Diagnostics.EventLog",
157157
"System.Diagnostics.EventLog.Messages",
158+
"System.Net.ServerSentEvents",
158159
"System.Security.Cryptography.Pkcs",
159160
"System.Security.Cryptography.Xml",
160161
"System.Threading.RateLimiting",
@@ -305,6 +306,7 @@ static TestData()
305306
{ "Microsoft.JSInterop" },
306307
{ "Microsoft.Net.Http.Headers" },
307308
{ "System.Diagnostics.EventLog" },
309+
{ "System.Net.ServerSentEvents" },
308310
{ "System.Security.Cryptography.Xml" },
309311
{ "System.Threading.RateLimiting" },
310312
};

‎src/Http/Http.Results/src/PublicAPI.Unshipped.txt‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@ Microsoft.AspNetCore.Http.HttpResults.ServerSentEventsResult<T>
22
Microsoft.AspNetCore.Http.HttpResults.ServerSentEventsResult<T>.ExecuteAsync(Microsoft.AspNetCore.Http.HttpContext! httpContext) -> System.Threading.Tasks.Task!
33
Microsoft.AspNetCore.Http.HttpResults.ServerSentEventsResult<T>.StatusCode.get -> int?
44
static Microsoft.AspNetCore.Http.HttpResults.RedirectHttpResult.IsLocalUrl(string? url) -> bool
5-
static Microsoft.AspNetCore.Http.Results.ServerSentEvents(System.Collections.Generic.IAsyncEnumerable<string!>! value, string? eventType = null) -> Microsoft.AspNetCore.Http.IResult!
6-
static Microsoft.AspNetCore.Http.Results.ServerSentEvents<T>(System.Collections.Generic.IAsyncEnumerable<System.Net.ServerSentEvents.SseItem<T>>! value) -> Microsoft.AspNetCore.Http.IResult!
7-
static Microsoft.AspNetCore.Http.Results.ServerSentEvents<T>(System.Collections.Generic.IAsyncEnumerable<T>! value, string? eventType = null) -> Microsoft.AspNetCore.Http.IResult!
5+
static Microsoft.AspNetCore.Http.Results.ServerSentEvents(System.Collections.Generic.IAsyncEnumerable<string!>! values, string? eventType = null) -> Microsoft.AspNetCore.Http.IResult!
6+
static Microsoft.AspNetCore.Http.Results.ServerSentEvents<T>(System.Collections.Generic.IAsyncEnumerable<System.Net.ServerSentEvents.SseItem<T>>! values) -> Microsoft.AspNetCore.Http.IResult!
7+
static Microsoft.AspNetCore.Http.Results.ServerSentEvents<T>(System.Collections.Generic.IAsyncEnumerable<T>! values, string? eventType = null) -> Microsoft.AspNetCore.Http.IResult!
88
static Microsoft.AspNetCore.Http.TypedResults.ServerSentEvents(System.Collections.Generic.IAsyncEnumerable<string!>! values, string? eventType = null) -> Microsoft.AspNetCore.Http.HttpResults.ServerSentEventsResult<string!>!
99
static Microsoft.AspNetCore.Http.TypedResults.ServerSentEvents<T>(System.Collections.Generic.IAsyncEnumerable<System.Net.ServerSentEvents.SseItem<T>>! values) -> Microsoft.AspNetCore.Http.HttpResults.ServerSentEventsResult<T>!
1010
static Microsoft.AspNetCore.Http.TypedResults.ServerSentEvents<T>(System.Collections.Generic.IAsyncEnumerable<T>! values, string? eventType = null) -> Microsoft.AspNetCore.Http.HttpResults.ServerSentEventsResult<T>!

‎src/Http/Http.Results/src/Results.cs‎

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -982,37 +982,45 @@ public static IResult AcceptedAtRoute<TValue>(string? routeName, RouteValueDicti
982982
/// <summary>
983983
/// Produces a <see cref="ServerSentEventsResult{TValue}"/> response.
984984
/// </summary>
985-
/// <param name="value">The value to be included in the HTTP response body.</param>
985+
/// <param name="values">The values to be included in the HTTP response body.</param>
986986
/// <param name="eventType">The event type to be included in the HTTP response body.</param>
987987
/// <returns>The created <see cref="ServerSentEventsResult{TValue}"/> for the response.</returns>
988988
/// <remarks>
989989
/// Strings serialized by this result type are serialized as raw strings without any additional formatting.
990990
/// </remarks>
991991
#pragma warning disable RS0026 // Do not add multiple public overloads with optional parameters
992-
public static IResult ServerSentEvents(IAsyncEnumerable<string> value, string? eventType = null)
992+
public static IResult ServerSentEvents(IAsyncEnumerable<string> values, string? eventType = null)
993993
#pragma warning restore RS0026 // Do not add multiple public overloads with optional parameters
994-
=> new ServerSentEventsResult<string>(value, eventType);
994+
=> new ServerSentEventsResult<string>(values, eventType);
995995

996996
/// <summary>
997997
/// Produces a <see cref="ServerSentEventsResult{T}"/> response.
998998
/// </summary>
999999
/// <typeparam name="T">The type of object that will be serialized to the response body.</typeparam>
1000-
/// <param name="value">The value to be included in the HTTP response body.</param>
1000+
/// <param name="values">The values to be included in the HTTP response body.</param>
10011001
/// <param name="eventType">The event type to be included in the HTTP response body.</param>
10021002
/// <returns>The created <see cref="ServerSentEventsResult{T}"/> for the response.</returns>
1003+
/// <remarks>
1004+
/// Strings serialized by this result type are serialized as raw strings without any additional formatting.
1005+
/// Other types are serialized using the configured JSON serializer options.
1006+
/// </remarks>
10031007
#pragma warning disable RS0026 // Do not add multiple public overloads with optional parameters
1004-
public static IResult ServerSentEvents<T>(IAsyncEnumerable<T> value, string? eventType = null)
1008+
public static IResult ServerSentEvents<T>(IAsyncEnumerable<T> values, string? eventType = null)
10051009
#pragma warning restore RS0026 // Do not add multiple public overloads with optional parameters
1006-
=> new ServerSentEventsResult<T>(value, eventType);
1010+
=> new ServerSentEventsResult<T>(values, eventType);
10071011

10081012
/// <summary>
10091013
/// Produces a <see cref="ServerSentEventsResult{T}"/> response.
10101014
/// </summary>
10111015
/// <typeparam name="T">The type of object that will be serialized to the response body.</typeparam>
1012-
/// <param name="value">The value to be included in the HTTP response body.</param>
1016+
/// <param name="values">The values to be included in the HTTP response body.</param>
10131017
/// <returns>The created <see cref="ServerSentEventsResult{T}"/> for the response.</returns>
1014-
public static IResult ServerSentEvents<T>(IAsyncEnumerable<SseItem<T>> value)
1015-
=> new ServerSentEventsResult<T>(value);
1018+
/// <remarks>
1019+
/// Strings serialized by this result type are serialized as raw strings without any additional formatting.
1020+
/// Other types are serialized using the configured JSON serializer options.
1021+
/// </remarks>
1022+
public static IResult ServerSentEvents<T>(IAsyncEnumerable<SseItem<T>> values)
1023+
=> new ServerSentEventsResult<T>(values);
10161024

10171025
/// <summary>
10181026
/// Produces an empty result response, that when executed will do nothing.

‎src/Http/Http.Results/src/ServerSentEventsResult.cs‎

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33

44
using System.Buffers;
55
using System.Net.ServerSentEvents;
6-
using System.Text;
76
using Microsoft.AspNetCore.Http.Metadata;
87
using System.Reflection;
98
using Microsoft.AspNetCore.Builder;
@@ -41,29 +40,33 @@ public async Task ExecuteAsync(HttpContext httpContext)
4140
ArgumentNullException.ThrowIfNull(httpContext);
4241

4342
httpContext.Response.ContentType = "text/event-stream";
43+
httpContext.Response.Headers.CacheControl = "no-cache,no-store";
44+
httpContext.Response.Headers.Pragma = "no-cache";
4445

45-
await SseFormatter.WriteAsync(_events, httpContext.Response.Body,
46-
(item, writer) => FormatSseItem(item, writer, httpContext),
47-
httpContext.RequestAborted);
48-
}
46+
var jsonOptions = httpContext.RequestServices.GetService<IOptions<JsonOptions>>()?.Value ?? new JsonOptions();
4947

50-
private static void FormatSseItem(SseItem<T> item, IBufferWriter<byte> writer, HttpContext httpContext)
51-
{
52-
// Emit string and null values as-is
53-
if (item.Data is string stringData)
48+
// If the event type is string, we can skip JSON serialization
49+
// and directly use the SseFormatter's WriteAsync overload for strings.
50+
if (_events is IAsyncEnumerable<SseItem<string>> stringEvents)
5451
{
55-
writer.Write(Encoding.UTF8.GetBytes(stringData));
52+
await SseFormatter.WriteAsync(stringEvents, httpContext.Response.Body, httpContext.RequestAborted);
5653
return;
5754
}
5855

56+
await SseFormatter.WriteAsync(_events, httpContext.Response.Body,
57+
(item, writer) => FormatSseItem(item, writer, jsonOptions),
58+
httpContext.RequestAborted);
59+
}
60+
61+
private static void FormatSseItem(SseItem<T> item, IBufferWriter<byte> writer, JsonOptions jsonOptions)
62+
{
5963
if (item.Data is null)
6064
{
6165
writer.Write([]);
6266
return;
6367
}
6468

6569
// For non-string types, use JSON serialization with options from DI
66-
var jsonOptions = httpContext.RequestServices.GetService<IOptions<JsonOptions>>()?.Value ?? new JsonOptions();
6770
var runtimeType = item.Data.GetType();
6871
var jsonTypeInfo = jsonOptions.SerializerOptions.GetTypeInfo(typeof(T));
6972

@@ -72,8 +75,8 @@ private static void FormatSseItem(SseItem<T> item, IBufferWriter<byte> writer, H
7275
? jsonTypeInfo
7376
: jsonOptions.SerializerOptions.GetTypeInfo(typeof(object));
7477

75-
var json = JsonSerializer.Serialize(item.Data, typeInfo);
76-
writer.Write(Encoding.UTF8.GetBytes(json));
78+
var json = JsonSerializer.SerializeToUtf8Bytes(item.Data, typeInfo);
79+
writer.Write(json);
7780
}
7881

7982
private static async IAsyncEnumerable<SseItem<T>> WrapEvents(IAsyncEnumerable<T> events, string? eventType = null)

‎src/Http/Http.Results/src/TypedResults.cs‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1072,7 +1072,7 @@ public static AcceptedAtRoute<TValue> AcceptedAtRoute<TValue>(TValue? value, str
10721072
/// <summary>
10731073
/// Produces a <see cref="ServerSentEventsResult{TValue}"/> response.
10741074
/// </summary>
1075-
/// <param name="values">The value to be included in the HTTP response body.</param>
1075+
/// <param name="values">The values to be included in the HTTP response body.</param>
10761076
/// <param name="eventType">The event type to be included in the HTTP response body.</param>
10771077
/// <returns>The created <see cref="ServerSentEventsResult{TValue}"/> for the response.</returns>
10781078
/// <remarks>
@@ -1087,9 +1087,13 @@ public static ServerSentEventsResult<string> ServerSentEvents(IAsyncEnumerable<s
10871087
/// Produces a <see cref="ServerSentEventsResult{T}"/> response.
10881088
/// </summary>
10891089
/// <typeparam name="T">The type of object that will be serialized to the response body.</typeparam>
1090-
/// <param name="values">The value to be included in the HTTP response body.</param>
1090+
/// <param name="values">The values to be included in the HTTP response body.</param>
10911091
/// <param name="eventType">The event type to be included in the HTTP response body.</param>
10921092
/// <returns>The created <see cref="ServerSentEventsResult{T}"/> for the response.</returns>
1093+
/// <remarks>
1094+
/// Strings serialized by this result type are serialized as raw strings without any additional formatting.
1095+
/// Other types are serialized using the configured JSON serializer options.
1096+
/// </remarks>
10931097
#pragma warning disable RS0026 // Do not add multiple public overloads with optional parameters
10941098
public static ServerSentEventsResult<T> ServerSentEvents<T>(IAsyncEnumerable<T> values, string? eventType = null)
10951099
#pragma warning restore RS0026 // Do not add multiple public overloads with optional parameters
@@ -1099,8 +1103,12 @@ public static ServerSentEventsResult<T> ServerSentEvents<T>(IAsyncEnumerable<T>
10991103
/// Produces a <see cref="ServerSentEventsResult{T}"/> response.
11001104
/// </summary>
11011105
/// <typeparam name="T">The type of object that will be serialized to the response body.</typeparam>
1102-
/// <param name="values">The value to be included in the HTTP response body.</param>
1106+
/// <param name="values">The values to be included in the HTTP response body.</param>
11031107
/// <returns>The created <see cref="ServerSentEventsResult{T}"/> for the response.</returns>
1108+
/// <remarks>
1109+
/// Strings serialized by this result type are serialized as raw strings without any additional formatting.
1110+
/// Other types are serialized using the configured JSON serializer options.
1111+
/// </remarks>
11041112
public static ServerSentEventsResult<T> ServerSentEvents<T>(IAsyncEnumerable<SseItem<T>> values)
11051113
=> new(values);
11061114

‎src/Http/Http.Results/test/ServerSentEventsResultTests.cs‎

Lines changed: 53 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System.Net.ServerSentEvents;
55
using System.Reflection;
6+
using System.Runtime.CompilerServices;
67
using System.Text;
78
using Microsoft.AspNetCore.Builder;
89
using Microsoft.AspNetCore.Http.Json;
@@ -16,7 +17,7 @@ namespace Microsoft.AspNetCore.Http.HttpResults;
1617
public class ServerSentEventsResultTests
1718
{
1819
[Fact]
19-
public async Task ExecuteAsync_SetsContentType()
20+
public async Task ExecuteAsync_SetsContentTypeAndHeaders()
2021
{
2122
// Arrange
2223
var httpContext = GetHttpContext();
@@ -28,6 +29,8 @@ public async Task ExecuteAsync_SetsContentType()
2829

2930
// Assert
3031
Assert.Equal("text/event-stream", httpContext.Response.ContentType);
32+
Assert.Equal("no-cache,no-store", httpContext.Response.Headers.CacheControl);
33+
Assert.Equal("no-cache", httpContext.Response.Headers.Pragma);
3134
}
3235

3336
[Fact]
@@ -52,20 +55,20 @@ public async Task ExecuteAsync_WritesStringEventsToResponse()
5255
}
5356

5457
[Fact]
55-
public async Task ExecuteAsync_WritesStringsEventsWithType()
58+
public async Task ExecuteAsync_WritesStringsEventsWithEventType()
5659
{
5760
// Arrange
5861
var httpContext = GetHttpContext();
59-
var events = new[] { "event1" }.ToAsyncEnumerable();
62+
var events = new[] { "event1", "event2" }.ToAsyncEnumerable();
6063
var result = TypedResults.ServerSentEvents(events, "test-event");
6164

6265
// Act
6366
await result.ExecuteAsync(httpContext);
6467

6568
// Assert
6669
var responseBody = Encoding.UTF8.GetString(((MemoryStream)httpContext.Response.Body).ToArray());
67-
Assert.Contains("event: test-event\n", responseBody);
68-
Assert.Contains("data: event1\n\n", responseBody);
70+
Assert.Contains("event: test-event\ndata: event1\n\n", responseBody);
71+
Assert.Contains("event: test-event\ndata: event2\n\n", responseBody);
6972
}
7073

7174
[Fact]
@@ -207,7 +210,51 @@ public async Task ExecuteAsync_WithPolymorphicType_SerializesCorrectly()
207210

208211
// Assert
209212
var responseBody = Encoding.UTF8.GetString(((MemoryStream)httpContext.Response.Body).ToArray());
210-
Assert.Contains(@"""extra"":""Additional""", responseBody);
213+
Assert.Contains(@"data: {""extra"":""Additional"",""name"":""Test"",""value"":42}", responseBody);
214+
}
215+
216+
[Fact]
217+
public async Task ExecuteAsync_ObservesCancellationViaRequestAborted()
218+
{
219+
// Arrange
220+
var cts = new CancellationTokenSource();
221+
var httpContext = GetHttpContext();
222+
httpContext.RequestAborted = cts.Token;
223+
var firstEventReceived = new TaskCompletionSource();
224+
var secondEventAttempted = new TaskCompletionSource();
225+
226+
var events = GetEvents(cts.Token);
227+
var result = TypedResults.ServerSentEvents(events);
228+
229+
// Act & Assert
230+
var executeTask = result.ExecuteAsync(httpContext);
231+
232+
// Wait for first event to be processed then cancel the request and wait
233+
// to observe the cancellation
234+
await firstEventReceived.Task;
235+
cts.Cancel();
236+
await secondEventAttempted.Task;
237+
238+
// Verify the execution was cancelled and only the first event was written
239+
await Assert.ThrowsAsync<TaskCanceledException>(() => executeTask);
240+
var responseBody = Encoding.UTF8.GetString(((MemoryStream)httpContext.Response.Body).ToArray());
241+
Assert.Contains("data: event1\n\n", responseBody);
242+
Assert.DoesNotContain("data: event2\n\n", responseBody);
243+
244+
async IAsyncEnumerable<string> GetEvents([EnumeratorCancellation] CancellationToken cancellationToken = default)
245+
{
246+
try
247+
{
248+
yield return "event1";
249+
firstEventReceived.SetResult();
250+
await Task.Delay(1, cancellationToken);
251+
yield return "event2";
252+
}
253+
finally
254+
{
255+
secondEventAttempted.SetResult();
256+
}
257+
}
211258
}
212259

213260
private static void PopulateMetadata<TResult>(MethodInfo method, EndpointBuilder builder)

0 commit comments

Comments
 (0)