Skip to content

Commit f5a5b61

Browse files
twsouthwicktwsouthwick
committed
respond to feedback
1 parent 03046e9 commit f5a5b61

File tree

7 files changed

+91
-37
lines changed

7 files changed

+91
-37
lines changed

‎src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/SessionState/RemoteSession/DoubleConnectionRemoteAppSessionManager.cs

+2
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ public DoubleConnectionRemoteAppSessionManager(ISessionSerializer serializer, IO
3232
{
3333
}
3434

35+
public Task<ISessionState> GetReadOnlySessionStateAsync(HttpContextCore context) => CreateAsync(context, isReadOnly: true);
36+
3537
[System.Diagnostics.CodeAnalysis.SuppressMessage("Reliability", "CA2000:Dispose objects before losing scope", Justification = "They are either passed into another object or are manually disposed")]
3638
protected override async Task<ISessionState> GetSessionDataAsync(string? sessionId, bool readOnly, HttpContextCore callingContext, CancellationToken token)
3739
{

‎src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/SessionState/RemoteSession/RemoteAppSessionDispatcher.cs

+30-5
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,48 @@
1-
// Licensed to the .NET Foundation under one or more agreements.
1+
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
using System.Net;
5+
using System.Net.Http;
6+
using System.Threading;
47
using System.Threading.Tasks;
8+
using Microsoft.Extensions.Options;
59

610
namespace Microsoft.AspNetCore.SystemWebAdapters.SessionState.RemoteSession;
711

812
/// <summary>
913
/// This is used to dispatch to either <see cref="DoubleConnectionRemoteAppSessionManager"/> if in read-only mode or <see cref="SingleConnectionWriteableRemoteAppSessionStateManager"/> if not.
1014
/// </summary>
11-
internal sealed class RemoteAppSessionDispatcher(SingleConnectionWriteableRemoteAppSessionStateManager singleConnection, DoubleConnectionRemoteAppSessionManager doubleConnection) : ISessionManager
15+
internal sealed class RemoteAppSessionDispatcher(
16+
IOptions<RemoteAppSessionStateClientOptions> options,
17+
SingleConnectionWriteableRemoteAppSessionStateManager singleConnection,
18+
DoubleConnectionRemoteAppSessionManager doubleConnection
19+
) : ISessionManager
1220
{
13-
public Task<ISessionState> CreateAsync(HttpContextCore context, SessionAttribute metadata)
21+
private bool _singleConnectionSupported = true;
22+
23+
public async Task<ISessionState> CreateAsync(HttpContextCore context, SessionAttribute metadata)
1424
{
1525
if (metadata.IsReadOnly)
1626
{
1727
// In readonly mode it's a simple GET request
18-
return doubleConnection.CreateAsync(context, metadata);
28+
return await doubleConnection.GetReadOnlySessionStateAsync(context);
1929
}
2030

21-
return singleConnection.CreateAsync(context, metadata);
31+
if (!_singleConnectionSupported || options.Value.UseSingleConnection)
32+
{
33+
return await doubleConnection.CreateAsync(context, metadata);
34+
}
35+
36+
try
37+
{
38+
return await singleConnection.CreateAsync(context, metadata);
39+
}
40+
41+
// If
42+
catch (HttpRequestException ex) when (ex.StatusCode == HttpStatusCode.MethodNotAllowed)
43+
{
44+
_singleConnectionSupported = false;
45+
return await doubleConnection.CreateAsync(context, metadata);
46+
}
2247
}
2348
}

‎src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/SessionState/RemoteSession/RemoteAppSessionStateExtensions.cs

+9-9
Original file line numberDiff line numberDiff line change
@@ -16,18 +16,18 @@ public static ISystemWebAdapterRemoteClientAppBuilder AddSessionClient(this ISys
1616

1717
builder.Services.AddTransient<DoubleConnectionRemoteAppSessionManager>();
1818
builder.Services.AddTransient<SingleConnectionWriteableRemoteAppSessionStateManager>();
19-
builder.Services.AddTransient<RemoteAppSessionDispatcher>();
20-
builder.Services.AddSingleton<ISessionManager>(ctx =>
21-
{
22-
var options = ctx.GetRequiredService<IOptions<RemoteAppSessionStateClientOptions>>();
23-
24-
return options.Value.UseSingleConnection
25-
? ctx.GetRequiredService<RemoteAppSessionDispatcher>()
26-
: ctx.GetRequiredService<DoubleConnectionRemoteAppSessionManager>();
27-
});
19+
builder.Services.AddTransient<ISessionManager, RemoteAppSessionDispatcher>();
2820

2921
builder.Services.AddOptions<RemoteAppSessionStateClientOptions>()
3022
.Configure(configure ?? (_ => { }))
23+
.PostConfigure<RemoteAppClientOptions>((options, remote) =>
24+
{
25+
// The single connection remote app session client requires https to work so if that's not the case, we'll disable it
26+
if (string.Equals(remote.RemoteAppUrl.Scheme, "https", StringComparison.OrdinalIgnoreCase))
27+
{
28+
options.UseSingleConnection = false;
29+
}
30+
})
3131
.ValidateDataAnnotations();
3232

3333
return builder;

‎src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/SessionState/RemoteSession/RemoteAppSessionStateManager.cs

+5-2
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,10 @@ public RemoteAppSessionStateManager(
5353
[LoggerMessage(EventId = 3, Level = LogLevel.Trace, Message = "Received {StatusCode} response committing remote session state")]
5454
protected partial void LogCommitResponse(HttpStatusCode statusCode);
5555

56-
public async Task<ISessionState> CreateAsync(HttpContextCore context, SessionAttribute metadata)
56+
public Task<ISessionState> CreateAsync(HttpContextCore context, SessionAttribute metadata)
57+
=> CreateAsync(context, metadata.IsReadOnly);
58+
59+
protected async Task<ISessionState> CreateAsync(HttpContextCore context, bool isReadOnly)
5760
{
5861
// If an existing remote session ID is present in the request, use its session ID.
5962
// Otherwise, leave session ID null for now; it will be provided by the remote service
@@ -63,7 +66,7 @@ public async Task<ISessionState> CreateAsync(HttpContextCore context, SessionAtt
6366
try
6467
{
6568
// Get or create session data
66-
var response = await GetSessionDataAsync(sessionId, metadata.IsReadOnly, context, context.RequestAborted);
69+
var response = await GetSessionDataAsync(sessionId, isReadOnly, context, context.RequestAborted);
6770

6871
LogSessionLoad(response.Count, response.SessionID);
6972

‎src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/SessionState/RemoteSession/SingleConnectionWriteableRemoteAppSessionStateManager.cs

+2-9
Original file line numberDiff line numberDiff line change
@@ -78,15 +78,8 @@ protected override async Task<ISessionState> GetSessionDataAsync(string? session
7878
return new RemoteSessionState(remoteSessionState, request, response, content, responseStream);
7979
}
8080

81-
private sealed class SessionPostResult
82-
{
83-
public bool Success { get; set; }
84-
85-
public string? Message { get; set; }
86-
}
87-
8881
[JsonSerializable(typeof(SessionPostResult))]
89-
private sealed partial class ResultContext : JsonSerializerContext
82+
private sealed partial class SessionPostResultContext : JsonSerializerContext
9083
{
9184
}
9285

@@ -142,7 +135,7 @@ public override async Task CommitAsync(CancellationToken token)
142135
{
143136
content.Commit(State);
144137

145-
var result = await JsonSerializer.DeserializeAsync(stream, ResultContext.Default.SessionPostResult, token);
138+
var result = await JsonSerializer.DeserializeAsync(stream, SessionPostResultContext.Default.SessionPostResult, token);
146139

147140
if (result is not { Success: true })
148141
{

‎src/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices/SessionState/RemoteSession/ReadWriteSessionHandler.cs

+28-12
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,23 @@ public override async Task ProcessRequestAsync(HttpContext context)
2323
using var timeoutCts = new CancellationTokenSource(TimeSpan.FromMinutes(context.Session.Timeout));
2424
using var cts = CancellationTokenSource.CreateLinkedTokenSource(timeoutCts.Token, context.Response.ClientDisconnectedToken);
2525

26-
await SendSessionAsync(new HttpContextWrapper(context), cts.Token).ConfigureAwait(false);
26+
var contextWrapper = new HttpContextWrapper(context);
27+
28+
await SendSessionAsync(contextWrapper, cts.Token).ConfigureAwait(false);
29+
30+
if (await RetrieveUpdatedSessionAsync(contextWrapper, cts.Token))
31+
{
32+
await SendSessionWriteResultAsync(contextWrapper.Response, Results.Succeeded, cts.Token);
33+
}
34+
else
35+
{
36+
await SendSessionWriteResultAsync(contextWrapper.Response, Results.NoSessionData, cts.Token);
37+
}
2738

2839
context.ApplicationInstance.CompleteRequest();
2940
}
3041

31-
public async Task SendSessionAsync(HttpContextBase context, CancellationToken token)
42+
private async Task SendSessionAsync(HttpContextBase context, CancellationToken token)
3243
{
3344
// Send the initial snapshot of session data
3445
context.Response.ContentType = "text/event-stream";
@@ -40,7 +51,10 @@ public async Task SendSessionAsync(HttpContextBase context, CancellationToken to
4051

4152
// Ensure to call HttpResponse.FlushAsync to flush the request itself, and not context.Response.OutputStream.FlushAsync()
4253
await context.Response.FlushAsync();
54+
}
4355

56+
private async Task<bool> RetrieveUpdatedSessionAsync(HttpContextBase context, CancellationToken token)
57+
{
4458
// This will wait for data to be pushed for the session info to be committed
4559
using var stream = context.Request.GetBufferlessInputStream();
4660

@@ -49,25 +63,27 @@ public async Task SendSessionAsync(HttpContextBase context, CancellationToken to
4963
if (deserialized is { })
5064
{
5165
deserialized.CopyTo(context.Session);
52-
53-
await JsonSerializer.SerializeAsync(context.Response.OutputStream, new SessionPostResult() { Success = true }, ResultContext.Default.SessionPostResult, token);
66+
return true;
5467
}
5568
else
5669
{
57-
await JsonSerializer.SerializeAsync(context.Response.OutputStream, new SessionPostResult() { Success = false, Message = "No session data was supplied for commit" }, ResultContext.Default.SessionPostResult, token);
70+
return false;
5871
}
5972
}
6073

61-
private class SessionPostResult
62-
{
63-
public bool Success { get; set; }
74+
private static Task SendSessionWriteResultAsync(HttpResponseBase response, SessionPostResult result, CancellationToken token)
75+
=> JsonSerializer.SerializeAsync(response.OutputStream, result, SessionPostResultContext.Default.SessionPostResult, token);
6476

65-
public string? Message { get; set; }
77+
[JsonSerializable(typeof(SessionPostResult))]
78+
[JsonSourceGenerationOptions(DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingDefault)]
79+
private partial class SessionPostResultContext : JsonSerializerContext
80+
{
6681
}
6782

68-
[JsonSerializable(typeof(SessionPostResult))]
69-
private partial class ResultContext : JsonSerializerContext
83+
private static class Results
7084
{
85+
public static SessionPostResult Succeeded { get; } = new() { Success = true };
86+
87+
public static SessionPostResult NoSessionData { get; } = new() { Success = false, Message = "No session data was supplied for commit" };
7188
}
7289
}
73-

‎src/Services/SessionState/SessionPostResult.cs

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System.Text.Json.Serialization;
5+
6+
namespace Microsoft.AspNetCore.SystemWebAdapters.SessionState.RemoteSession;
7+
8+
internal class SessionPostResult
9+
{
10+
[JsonPropertyName("s")]
11+
public bool Success { get; set; }
12+
13+
[JsonPropertyName("m")]
14+
public string? Message { get; set; }
15+
}

0 commit comments

Comments
 (0)