Skip to content

Commit e59159a

Browse files
twsouthwicktwsouthwick
authored
Remove extra HttpApplication events registration option parameter
This reverts a recent change to add a RegisterEvent option and overrides the IHttpApplicationFeature for the integrated OWIN pipeline.
1 parent bf4f929 commit e59159a

File tree

5 files changed

+80
-50
lines changed

5 files changed

+80
-50
lines changed

‎src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/Features/HttpApplicationFeature.cs‎

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -23,18 +23,16 @@ internal sealed class HttpApplicationFeature : IHttpApplicationFeature, IHttpRes
2323
];
2424

2525
private readonly IHttpResponseEndFeature _previous;
26-
private readonly ImmutableDictionary<ApplicationEvent, ImmutableList<RequestDelegate>> _events;
2726
private readonly ObjectPool<HttpApplication> _pool;
2827

2928
private object? _contextOrApplication;
3029
private List<Exception>? _exceptions;
3130

32-
public HttpApplicationFeature(HttpContextCore context, IHttpResponseEndFeature previousEnd, ObjectPool<HttpApplication> pool, HttpApplicationOptions options)
31+
public HttpApplicationFeature(HttpContextCore context, IHttpResponseEndFeature previousEnd, ObjectPool<HttpApplication> pool)
3332
{
3433
_contextOrApplication = context;
3534
_pool = pool;
3635
_previous = previousEnd;
37-
_events = options.EventHandlers;
3836
}
3937

4038
public RequestNotification CurrentNotification { get; set; }
@@ -59,19 +57,10 @@ private HttpApplication InitializeApplication(HttpContextCore context)
5957
return app;
6058
}
6159

62-
async ValueTask IHttpApplicationFeature.RaiseEventAsync(ApplicationEvent @event)
60+
ValueTask IHttpApplicationFeature.RaiseEventAsync(ApplicationEvent @event)
6361
{
64-
if (_events.TryGetValue(@event, out var handlers))
65-
{
66-
var context = Application.Context.AsAspNetCore();
67-
68-
foreach (var handler in handlers)
69-
{
70-
await handler(context);
71-
}
72-
}
73-
7462
RaiseEvent(@event);
63+
return ValueTask.CompletedTask;
7564
}
7665

7766
[System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "Must handle all exceptions here")]

‎src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/HttpApplication/HttpApplicationOptions.cs‎

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -17,23 +17,17 @@ public class HttpApplicationOptions
1717
{
1818
private ModuleCollection? _modules;
1919
private Type _applicationType = typeof(HttpApplication);
20-
private ImmutableDictionary<ApplicationEvent, ImmutableList<RequestDelegate>> _eventHandlers = ImmutableDictionary<ApplicationEvent, ImmutableList<RequestDelegate>>.Empty;
2120

2221
internal ModuleCollection ModuleCollection
2322
{
2423
get => _modules ?? throw new InvalidOperationException("HttpApplicationOptions must be initialized with a module collection");
2524
set => _modules = value;
2625
}
2726

28-
internal ImmutableDictionary<ApplicationEvent, ImmutableList<RequestDelegate>> EventHandlers => _eventHandlers;
29-
3027
/// <summary>
3128
/// Used to track if the middleware for HttpApplication infrastructure should be added
3229
/// </summary>
33-
internal bool ShouldBeRegistered =>
34-
_modules is { Count: > 0 } ||
35-
_applicationType != typeof(HttpApplication) ||
36-
EventHandlers is { Count: > 0 };
30+
internal bool ShouldBeRegistered => _modules is not null;
3731

3832
public Type ApplicationType
3933
{
@@ -55,20 +49,6 @@ public Type ApplicationType
5549

5650
public IDictionary<string, Type> Modules => ModuleCollection;
5751

58-
public void RegisterEvent(ApplicationEvent @event, RequestDelegate func)
59-
{
60-
ImmutableInterlocked.Update(ref _eventHandlers, static (dict, state) =>
61-
{
62-
var (@event, func) = state;
63-
if (!dict.TryGetValue(@event, out var handlers))
64-
{
65-
handlers = ImmutableList<RequestDelegate>.Empty;
66-
}
67-
handlers = handlers.Add(func);
68-
return dict.SetItem(@event, handlers);
69-
}, (@event, func));
70-
}
71-
7252
/// <summary>
7353
/// Gets or sets whether <see cref="HttpApplication.PreSendRequestHeaders"/> and <see cref="HttpApplication.PreSendRequestContent"/> is supported
7454
/// </summary>

‎src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/HttpApplication/RegisterHttpApplicationFeatureMiddleware.cs‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,19 +15,17 @@ internal sealed class RegisterHttpApplicationFeatureMiddleware
1515
{
1616
private readonly RequestDelegate _next;
1717
private readonly ObjectPool<HttpApplication> _pool;
18-
private readonly IOptions<HttpApplicationOptions> _options;
1918

20-
public RegisterHttpApplicationFeatureMiddleware(RequestDelegate next, ObjectPool<HttpApplication> pool, IOptions<HttpApplicationOptions> options)
19+
public RegisterHttpApplicationFeatureMiddleware(RequestDelegate next, ObjectPool<HttpApplication> pool)
2120
{
2221
_next = next;
2322
_pool = pool;
24-
_options = options;
2523
}
2624

2725
public async Task InvokeAsync(HttpContextCore context)
2826
{
2927
var endFeature = context.Features.GetRequiredFeature<IHttpResponseEndFeature>();
30-
using var httpApplicationFeature = new HttpApplicationFeature(context, endFeature, _pool, _options.Value);
28+
using var httpApplicationFeature = new HttpApplicationFeature(context, endFeature, _pool);
3129

3230
context.Features.Set<IHttpApplicationFeature>(httpApplicationFeature);
3331
context.Features.Set<IHttpResponseEndFeature>(httpApplicationFeature);

‎src/Microsoft.AspNetCore.SystemWebAdapters.Owin/OwinAspNetCoreExtension.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// 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 Microsoft.AspNetCore.Hosting;
45
using Microsoft.AspNetCore.SystemWebAdapters;
56
using Microsoft.Extensions.DependencyInjection.Extensions;
67
using Microsoft.Extensions.Options;
@@ -26,7 +27,7 @@ public static ISystemWebAdapterBuilder AddOwinApp(this ISystemWebAdapterBuilder
2627

2728
builder.AddHttpApplication();
2829
builder.Services.Configure<OwinAppOptions>(options => options.Configure += configure);
29-
builder.Services.TryAddEnumerable(ServiceDescriptor.Transient<IConfigureOptions<HttpApplicationOptions>, IntegratedPipelineConfigureOptions>());
30+
builder.Services.TryAddEnumerable(ServiceDescriptor.Transient<IStartupFilter, OwinHttpApplicationIntegrationStartup>());
3031

3132
return builder;
3233
}

‎src/Microsoft.AspNetCore.SystemWebAdapters.Owin/IntegratedPipelineConfigureOptions.cs‎ renamed to ‎src/Microsoft.AspNetCore.SystemWebAdapters.Owin/OwinHttpApplicationIntegrationStartup.cs‎

Lines changed: 72 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
// 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.Collections.Frozen;
45
using Microsoft.AspNetCore.Builder;
6+
using Microsoft.AspNetCore.Hosting;
57
using Microsoft.AspNetCore.Http;
8+
using Microsoft.AspNetCore.Http.Features;
69
using Microsoft.AspNetCore.SystemWebAdapters.Features;
710
using Microsoft.Extensions.DependencyInjection;
811
using Microsoft.Extensions.Logging;
@@ -12,9 +15,66 @@
1215

1316
namespace Microsoft.AspNetCore.SystemWebAdapters;
1417

15-
internal sealed partial class IntegratedPipelineConfigureOptions(IOptions<OwinAppOptions> owinOptions, IServiceProvider sp) : IConfigureOptions<HttpApplicationOptions>
18+
internal sealed partial class OwinHttpApplicationIntegrationStartup(
19+
IOptions<OwinAppOptions> owinOptions,
20+
ILogger<OwinHttpApplicationIntegrationStartup> logger,
21+
IServiceProvider sp)
22+
: IStartupFilter
1623
{
17-
public void Configure(HttpApplicationOptions options)
24+
[LoggerMessage(EventId = 0, Level = LogLevel.Error, Message = "Integrated OWIN pipeline stage '{StageName}' added out of order before stage '{CurrentStageName}'. This stage will be ignored and middleware may run earlier than expected.")]
25+
private static partial void LogOutOfOrder(ILogger logger, string stageName, string currentStageName);
26+
27+
[LoggerMessage(EventId = 1, Level = LogLevel.Error, Message = "Integrated OWIN pipeline stage '{StageName}' could not be mapped to an ApplicationEvent")]
28+
private static partial void LogSkippedStage(ILogger logger, string stageName);
29+
30+
public Action<IApplicationBuilder> Configure(Action<IApplicationBuilder> next)
31+
=> builder =>
32+
{
33+
var stages = BuildStages().ToFrozenDictionary(kv => kv.Item1, kv => kv.Item2);
34+
35+
builder.Use(async (ctx, next) =>
36+
{
37+
var existing = ctx.Features.GetRequiredFeature<IHttpApplicationFeature>();
38+
39+
ctx.Features.Set<IHttpApplicationFeature>(new OwinPipelineApplicationEventsFeatures(existing, stages, ctx));
40+
41+
try
42+
{
43+
await next();
44+
}
45+
finally
46+
{
47+
ctx.Features.Set<IHttpApplicationFeature>(existing);
48+
}
49+
});
50+
51+
next(builder);
52+
};
53+
54+
private sealed class OwinPipelineApplicationEventsFeatures(
55+
IHttpApplicationFeature other,
56+
FrozenDictionary<ApplicationEvent, RequestDelegate> stages,
57+
HttpContext context)
58+
: IHttpApplicationFeature
59+
{
60+
System.Web.HttpApplication IHttpApplicationFeature.Application => other.Application;
61+
62+
System.Web.RequestNotification IHttpApplicationFeature.CurrentNotification => other.CurrentNotification;
63+
64+
bool IHttpApplicationFeature.IsPostNotification => other.IsPostNotification;
65+
66+
async ValueTask IHttpApplicationFeature.RaiseEventAsync(ApplicationEvent appEvent)
67+
{
68+
await other.RaiseEventAsync(appEvent);
69+
70+
if (stages.TryGetValue(appEvent, out var stage))
71+
{
72+
await stage(context);
73+
}
74+
}
75+
}
76+
77+
public IEnumerable<(ApplicationEvent, RequestDelegate)> BuildStages()
1878
{
1979
var stages = CreateStages(owinOptions.Value.Configure, sp);
2080

@@ -38,14 +98,18 @@ public void Configure(HttpApplicationOptions options)
3898

3999
if (appEvent is { } value)
40100
{
41-
options.RegisterEvent(value, stage.Next);
101+
yield return (value, stage.Next);
102+
}
103+
else
104+
{
105+
LogSkippedStage(logger, stage.Name);
42106
}
43107
}
44108
}
45109

46110
private sealed record OwinStage(string Name, RequestDelegate Next);
47111

48-
private static IEnumerable<OwinStage> CreateStages(Action<IAppBuilder, IServiceProvider>? configure, IServiceProvider services)
112+
private IEnumerable<OwinStage> CreateStages(Action<IAppBuilder, IServiceProvider>? configure, IServiceProvider services)
49113
{
50114
if (configure is null)
51115
{
@@ -54,7 +118,7 @@ private static IEnumerable<OwinStage> CreateStages(Action<IAppBuilder, IServiceP
54118

55119
StageBuilder? firstStage = null;
56120

57-
Task DefaultApp(IDictionary<string, object> env)
121+
static Task DefaultApp(IDictionary<string, object> env)
58122
{
59123
if (!env.TryGetValue(OwinConstants.IntegratedPipelineCurrentStage, out var currentStage))
60124
{
@@ -71,7 +135,7 @@ Task DefaultApp(IDictionary<string, object> env)
71135

72136
var appFunc = OwinBuilder.Build(DefaultApp, (builder, sp) =>
73137
{
74-
EnableIntegratedPipeline(builder, stage => firstStage = stage, DefaultApp, sp.GetRequiredService<ILogger<OwinStage>>());
138+
EnableIntegratedPipeline(builder, stage => firstStage = stage, DefaultApp, logger);
75139
configure(builder, services);
76140
}, services);
77141

@@ -80,7 +144,7 @@ Task DefaultApp(IDictionary<string, object> env)
80144
throw new InvalidOperationException("Did not have a stage");
81145
}
82146

83-
return [.. GetStages(firstStage, appFunc, services)];
147+
return GetStages(firstStage, appFunc, services);
84148
}
85149

86150
private static IEnumerable<OwinStage> GetStages(StageBuilder? stage, AppFunc appFunc, IServiceProvider services)
@@ -118,9 +182,6 @@ private sealed class StageBuilder
118182
public AppFunc? EntryPoint { get; set; }
119183
}
120184

121-
[LoggerMessage(EventId = 1, Level = LogLevel.Error, Message = "Integrated Pipeline stage '{StageName}' added out of order before stage '{CurrentStageName}'. This stage will be ignored and middleware may run earlier than expected.")]
122-
private static partial void LogOutOfOrder(ILogger logger, string stageName, string currentStageName);
123-
124185
private static void EnableIntegratedPipeline(IAppBuilder app, Action<StageBuilder> onStageCreated, AppFunc exitPoint, ILogger logger)
125186
{
126187
var stage = new StageBuilder { Name = "PreHandlerExecute" };
@@ -181,6 +242,7 @@ internal static bool VerifyStageOrder(string stage1, string stage2)
181242
{
182243
return false;
183244
}
245+
184246
return stage1Index < stage2Index;
185247
}
186248
}

0 commit comments

Comments
 (0)