dotnet / systemweb-adapters

MIT License
338 stars 60 forks source link

Consider supporting handler/module infrastructure #146

Closed twsouthwick closed 1 year ago

twsouthwick commented 2 years ago

Summary

Two of the building blocks in System.Web were IHttpModule andIHttpHandler. These allowed hooking into the framework in different ways.

Motivation and goals

Users may have projects that want to use the IHttpModule and IHttpHandler types as they are migrating to ASP.NET Core. With the current infrastructure, much of this could be supported. The main issue is that there is no equivalent eventing system, but that can be mocked in a way with some custom middleware that would end up calling the modules as (mostly) expected.

In scope

Out of scope

Risks / unknowns

Examples

Common usage may be:

public class MyModule : IHttpModule
{
  public void Init(HttpApplication app)
  {
    app.BeginRequest += OnEvent;
    app.MapRequestHandler += OnEvent;
  }

  private void OnEvent(object sender, EventArgs e)
  {
      var context = ((HttpApplication)s).Context;

      if (context.CurrentNotification == Notification.MapRequestHandler)
      {
         context.Handler = new MyCustomHandler();
      }
  }

  public void Dispose()
  {
  }
}

public class MyCustomHandler : IHttpHandler
{
  public void ProcessRequest(HttpContext context)
  {
    ...
  ]
}
CZEMacLeod commented 2 years ago

@twsouthwick I've done an initial implementation of IHttpHandler (not IHttpModule) here https://github.com/CZEMacLeod/systemweb-adapters/tree/czemacleod/ihttphandler

I've mentioned this in #149 as it impacts and is related to a couple of the properties mentioned there.

There are a few things that I don't like in this implementation and could probably use some input - I currently use HttpContext.Items to store some of the data moved between the Core Framework and the System.Web implementation - I think there should be a better way of doing this but am not entirely sure what it would be. The mapping of handlers as endpoints looks a little messy. You can map them directly under the main IApplicationBuilder, but if you exit the handler doing nothing you get an error, which is not what happens on net4x. Hence the code in the sample

app.UseEndpoints(endpoints =>
{
    app.MapDefaultControllerRoute();
    // This method can be used to enable session (or read-only session) on all controllers
    //.RequireSystemWebAdapterSession();

    var epApp = endpoints.CreateApplicationBuilder();
    epApp.UseHttpHandler<ClassLibrary.InfoHttpHandler>("*.info");
    epApp.UseHttpHandler<HandlerLibrary.HelloWorldHandler>("*.sample");
    epApp.UseHttpHandler<HandlerLibrary.HelloWorldAsyncHandler>("*.sampleasync");
    epApp.UseHttpHandlerFactory<HandlerLibrary.MyHandlerFactory>("abc.test");
    epApp.UseHttpHandlerFactory<HandlerLibrary.MyHandlerFactory>("def.test");
    epApp.UseHttpHandlerFactory<HandlerLibrary.MyHandlerFactory>("xyz.test");

    app.MapReverseProxy();
});

This includes

Also for Core:

Also fixes for HttpRequest.AppRelativeCurrentExecutionFilePath and HttpServer.MapPath due to the difference between Path and FilePath when PathInfo is not empty.

twsouthwick commented 2 years ago

@CZEMacLeod thanks for exploring this! I also have a branch that does this for handlers and another one that builds on it for modules.

The fundamental difference in our approach appears to be that you're using middleware to execute the handlers while I set mine up to replace the IEndpointFeature since the concepts are similar (see here).

We're starting planning for .NET 8 and the investments in this project will be part of that, so once we get v1 out, we can hopefully explore this.

To help with prioritization, how much would enabling this help you with your migration efforts? Do you have a lot of investments in custom handlers? modules? That will help identify what features we add in-box.

CZEMacLeod commented 2 years ago

I have a couple of async handlers (mainly generating custom favicons with caching) but I think they could probably be replaced fairly easily with an endpoint or controller; especially with output caching.

I have a couple of modules, one which could probably be replaced with startup.cs/program.cs which does a lot of assembly scanning to load components and register compiled mvc5 razor libraries and such. The other handles di for mvc5/owin/webapi which again could probably be migrated to scrutor and/or custom startup code.

The biggest migration headaches for me just now is my dynamically built bundles from aspnet.optimization and converting a lot of vbhtml razor files.

I've been trying to backport or implement similar patterns to aspnetcore in the components of my application stacks for a while to make the upgrade process easier and I really hope this library will make that process a lot easier.

Unfortunately, this will be the 4th major migration of the system which was originally asp classic, then webforms, now mvc5/owin/webapi2/ef6 and there is a reasonable amount of technical debt built up, but the cost to rewrite the full stack would be unfeasible, so this piecemeal approach is absolutely the best way forward allowing me to migrate the 180+ projects over as and when they can be.

CZEMacLeod commented 2 years ago

@twsouthwick I am less familiar with endpoint features in aspnetcore 6, but this looks good. I'm not sure how the handling of registering wildcard paths and handling pathinfo would work in your version. Perhaps you could include versions of that in the examples? It would be nice to get the upgrade wizard to translate web.config handlers to endpoint registration down the road.

I think that adding FilePath and PathInfo to IHttpHandlerFeature rather than using HttpContext.Items would be a much cleaner implementation, but don't forget to include the adjustments to MapPath and the related changes to HttpRequest

danroth27 commented 1 year ago

HTTP module support is in 1.2. We can treat any remaining ideas for helping with HTTP handlers separately as needed.