dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.22k stars 9.95k forks source link

Generic pipeline #3173

Closed ObsidianMinor closed 5 years ago

ObsidianMinor commented 6 years ago

Edit: This proposal has been heavily reduced, and removed the abstractions surrounding the creation of data, focusing more on the pipeline itself.

Generic pipeline

We've decoupled the HTTP pipeline from the host with the generic host, but now we could consider decoupling the "HTTP" from the pipeline itself.

Introducing Microsoft.Extensions.ApplicationPipeline

The generic pipeline would allow consumers to easily create pipelines with custom contexts for non-HTTP workloads. For example, clients and non-HTTP servers such as TCP servers with custom messages or language servers reading and writing to standard IO. This interface along with a default implementation would allow developers to easily build lightweight, modular pipelines for processing data.

In my experience, many clients / servers are based on a handler / module model like that of old ASP.NET. The benefits the HTTP pipeline has provided to ASP.NET can be applied to these other clients and servers. Things like pipeline branching, the simplification that everything is middleware, along with the ability to short-circuit processing are useful in many places besides HTTP. And with a generic pipeline it all happens with one library that's easy to use, understand, and extend, especially if you've ever used ASP.NET Core already

One interface: IApplicationBuilder

This interface would exist in the abstractions lib. A default implementation would be provided in the implementation lib.

public delegate Task ApplicationDelegate<TContext>(TContext context);

public interface IApplicationBuilder<TContext>
{
    IServiceProvider ApplicationServices { get; set; }

    IDictionary<string, object> Properties { get; }

    ApplicationDelegate<TContext> Build();

    IApplicationBuilder<TContext> New();

    IApplicationBuilder<TContext> Use(Func<ApplicationDelegate<TContext>, ApplicationDelegate<TContext>> middleware);
}

IApplicationBuilder is the IApplicationBuilder from ASP.NET Core, but generic as to allow custom contexts. To extend it, extension methods should use the proper context being used such as HttpContext.

For example, UseExceptionHandler would be modified to look like:

public static IApplicationBuilder<HttpContext> UseExceptionHandler(this IApplicationBuilder<HttpContext> app);
davidfowl commented 6 years ago

The generic pipeline would allow consumers to easily create pipelines with custom contexts for non-HTTP workloads. For example, clients and non-HTTP servers such as TCP servers with custom messages or language servers reading and writing to standard IO.

Just FYI Bedrock is our story for non http things https://github.com/aspnet/KestrelHttpServer/issues/1980.

As for this specific idea, I experimented with it ~2 years ago https://github.com/davidfowl/ServerStack (sample here https://github.com/davidfowl/ServerStack/blob/e25feadf59eed899f79459aa8b6fcc771783b214/samples/EchoServerSample/EchoServer.cs) and after playing with it for a bit I concluded it was a pointless abstraction. I don't think the pipeline abstraction applies generally to all things, at least not at this level. For example, with TCP what does the context look like? It's a connection and a Stream (or IDuplexPipe) at the core of it and middleware basically wraps the stream for the lifetime of the connection.

I do think the builder pattern is useful for both clients and servers, I'm just not sure that this specific pattern is a good one. Happy to be proven wrong though!

tuscen commented 6 years ago

This would be a very useful addition for me. Right now I have a custom made framework for Telegram bots that is built using almost the same abstractions as in ASP.NET Core and middleware pipeline is a very useful abstraction for my use case. If that proposal were to be implemented I could remove a ton of my code and just use generic pipeline so I could concentrate on actual application code and not maintain my own pipeline.

davidfowl commented 6 years ago

If that proposal were to be implemented I could remove a ton of my code and just use generic pipeline so I could concentrate on actual application code and not maintain my own pipeline.

A ton of code? The logic to build the pipeline fits on a single page of code. Did you mean something else?

Can you show me what your pipeline looks like? What is your TContext? Seeing more concrete examples would make this more real for me.

tuscen commented 6 years ago

@davidfowl sorry, it was a bit of a hyperbole on my side. Not a ton, but I have to maintain my own analog of Microsoft.AspNetCore.Hosting package so I could have Startup class for pipeline building without depending Microsoft.AspNetCore.Hosting which itself depends on a lot of HTTP related packages so I can use WebHostBuilder due to generic host lacking Startup class feature.

Pipeline building process looks almost exactly like in ASP.NET Core

That's a subset of my Startup class:

public class Startup
{
    public Startup(IConfiguration config)
    {
        Configuration = config;
    }

    private IConfiguration Configuration { get; }

    public void ConfigureServices(IServiceCollection services)
    {
        var storageOptions = new PostgreSqlStorageOptions();

        services.AddHangfire(config =>
        {
            config.UsePostgreSqlStorage(Configuration["DefaultConnection"], storageOptions);
        });

        services.Configure<BotOptions>(Configuration.GetSection("BotOptions"));

        services.AddDbContext<MyContext>(options =>
        {
            var connectionString = Configuration["DefaultConnection"];
            options.UseNpgsql(connectionString);
        });

        // From Microsoft.Extensions.Caching.Redis
        services.AddDistributedRedisCache(options =>
        {
            options.Configuration = Configuration["REDIS_SERVER"];
        });

        // My own session middleware based on IDistributedCache
        services.AddSession();

        services.AddCommands()
            /* ... */
            .Configure();

        services.AddCallbackCommands()
            /* ... */
            .Configure();

        services.AddSingleton<AssetsProvider>();

        services.AddMenu();

        services.AddTransient<IBroadcastJob, BroadcastJob>();
        services.AddAutoMapper();

        services.Configure<BroadcastJobOptions>(Configuration.GetSection("BroadcastJobOptions"));
        services.AddScoped<BroadcastManager>();
    }

    public void Configure(IPipelineBuilder pipeline)
    {
        pipeline
            .UseHangfireServer()
            .UseExceptionCatcher()
            .UseSession()
            .UseMiddleware<BannedFilterMiddleware>()
            .UseCommands()
            .UseCallbackCommands()
            .UseMenu();
    }
}

My context is also looks similar but without any HTTP stuff. The thing is that I have to depend on Microsoft.AspNetCore.Http.Features to have FeatureCollection so I could have the same context extensibility mechanisms as in ASP.NET Core.

davidfowl commented 6 years ago

@davidfowl sorry, it was a bit of a hyperbole on my side. Not a ton, but I have to maintain my own analog of Microsoft.AspNetCore.Hosting package so I could have Startup class for pipeline building without depending Microsoft.AspNetCore.Hosting which itself depends on a lot of HTTP related packages so I can use WebHostBuilder due to generic host lacking Startup class feature.

We released the HostBuilder in 2.1. A generic host that isn't tied to the ASP.NET Hosting abstractions. It doesn't come with any support for Startup so it can be trivially added for your scenario.

FWIW though I'm still not convinced as yet. What does the context look like and what does your middleware look like. What is it doing, how is it used? How often is the delegate invoked? What is the pipeline about?

ObsidianMinor commented 6 years ago

I don't think the pipeline abstraction applies generally to all things, at least not at this level. For example, with TCP what does the context look like? It's a connection and a Stream (or IDuplexPipe) at the core of it and middleware basically wraps the stream for the lifetime of the connection.

It doesn't make much sense at the base TCP level, you're right. It's a bit too simple. It makes more sense the higher you go like in ASP.NET Core, with things like sessions, requests and responses, WebSockets, etc.

For my Steam client framework, a client context has connection info, session info, the message itself, the interface for outgoing messages, along with the items dictionary and service provider.

For middleware, it contains standard exception handlers, a "multi" processor for unwrapping a specific message type containing multiple messages, a job handler that processes jobs (async network operations), a session processor, a service method processor, and finally on the end is an MVC type system just simply called modules that reflectively routes messages to handlers for the type of the message. This along with whatever other middleware needs to be added in the future depending on what Valve adds to Steam.

However, this is an abstract framework and works with many other types of Steam clients like game coordinator clients which can borrow some of this middleware as well as add some of its own.

davidfowl commented 6 years ago

Sounds like WCF 😉.

However, this is an abstract framework and works with many other types of Steam clients like game coordinator clients which can borrow some of this middleware as well as add some of its own.

I don't think there's a framework here, this is mostly a pattern thing and I agree that pattern totally makes sense if you do things exactly like how the http request response pipeline works (invoking the built up delegate on a context per "message").

We wouldn't make a change like this in ASP.NET Core unless there was a tangible benefit and I'm not sure we would create the abstraction unless there was a reason we wanted to use it in our stack.

For example, UseExceptionHandler would be modified to look like:

Except, UseExceptionHandler doesn't just catch exceptions, https://github.com/aspnet/Diagnostics/blob/3a868d87a2458fd025ca71e2538b3a72f67c7c50/src/Microsoft.AspNetCore.Diagnostics/ExceptionHandler/ExceptionHandlerMiddleware.cs#L51-L93.

The hypothetical case where you share middleware across problem domains works well if the middleware doesn't care about TContext. Other than that, it's more about pattern similarity than anything else.

ObsidianMinor commented 6 years ago

I don't think there's a framework here

I was talking about my Steam client framework, sorry about the poor choice of words.

Except, UseExceptionHandler doesn't just catch exceptions

Right, I meant if this were to be used in ASP.NET Core, existing extensions would just change the type of context used in the extension method definition.

It doesn't have to be used, of course. It could not be used and instead just be provided like how the generic host is right now.

The hypothetical case where you share middleware across problem domains works well if the middleware doesn't care about TContext.

Not too hypothetical actually... I can think of three extensions that could be shared:

Use<T>(IApplicationBuilder<T> app, Func<T, Func<Task> Task> middleware);
Run<T>(IApplicationBuilder<T> app, ApplicationDelegate<T> middleware);
MapWhen<T>(IApplicationBuilder<T> app, Func<T, bool> predicate, Action<IApplicationBuilder<T>> configuration);

First two are both defined for IConnectionBuilder and IApplicationBuilder. They're also pretty simple extensions so they're easy to implement.

davidfowl commented 6 years ago

I just don’t see he point other than it would have Microsoft in the package name

johnbouma commented 6 years ago

Personally, while i don't disagree with your last point, @davidfowl, modern web apps are essentially data processing pipelines. I gave my coworker a print copy of the middleware section of the docs yesterday explaining how I wanted to abstract the pipeline concept since every non-crud operation that our app does is essentially a context driven pipeline. I was in the beginning stages of pulling digesting the Hosting assembly today, but it's honestly a daunting task. Like I said, i don't disagree with your last point, but at the end of the day an abstraction of this sort will help developers write better software that is easier to test, easier to maintain and would be a mainstay of my personal stack for many years to come.

davidfowl commented 6 years ago

What I can’t judge if this is one of those “build it and they will come” things or if it’s a thing where everyone is doing this already and we just need to add the abstraction.

@johnbouma do you have a concrete idea on what your pipeline and context object would look like?

khionu commented 6 years ago

I'd like to make a point of the ASP.NET pipeline being familiar to many C# devs. Whether it's popular or not right now (and based on the initial response to this issue, it seems to be so), the pattern make codebases easier to pick up for those who teethed on ASP.NET. I would argue that, by being something a good portion of C# devs get a significant amount of exposure to, even if only in the early days of them learning C#, it's symbolic enough to be/become a standard pattern.

While a standard doesn't need to become part of ASP.NET's codebase, it helps to proliferate it if it is.

davidfowl commented 6 years ago

Sure I just need more concrete examples to be convinced.

johnbouma commented 6 years ago

@davidfowl Sure: I have a set collection of objects that represent trades that are in the market and orders that have been proposed for execution, I have roughly 70 validations that may or may not apply to the collection depending on whether or not they are toggled on or off. Currently, I check whether each validator is enabled and properly configured and add them to a queue. I then construct a validation context object containing the collection of objects to be validated, and some additional information required to perform the validations. I iterate thru the queue, passing the context into each validator to perform the validation, and write the results into the context. I would prefer to construct the pipeline of validations and have each validation determine whether or not is applies to the operation, then just call the next validator in the line.

Some other examples: Most of the operations my app perform rely on having an input value denominated in one currency converted and available as a value in another currency (tied to the input value and currency that was converted). One idea I had was to have an IRawContext with the unconverted values and an IConvertedContext containing both the converted and unconverted values that would be returned by a ConvertValues 'middleware'. This would let me implement the conversion operation using a single, standard methodology. Any 'middleware' requiring the converted values in addition to the unconverted values would take in a context that is constrained to implement the IConvertedContext.

davidfowl commented 6 years ago

The first scenario seems like overkill for the russian doll/decorator model that the middleware pipeline offers. Lets not forget here that there are different ways to do a loop and run a function over it. Why benefits would you get with this validation pipeline mimicking middleware? Can you change it dynamically? Is the order fixed?

One example we have in the today in ASP.NET Core of a pipeline that doesn't use the exact same pattern is the MVC filter pipeline because it has additional features that don't map 1:1 (like order).

If you're convinced the the IApplicationBuilder<T> model is the best way to implement this, do it and then show that it absolutely makes sense. Once you have it running in your application, give an update on this issue with how it's been working.

The good news is that this the logic for this builder can be copied into a single class in your project and if people come up with good use that they have actually implemented and used it will help drive the point home here.

As of right now, we're not going to be adding this abstraction pre-maturely, not switching ASP.NET Core over to something more generic until further notice 😄 . I await more of your awesome implementation examples.

Eilon commented 5 years ago

@davidfowl any further action planned on this issue? Or close it?

davidfowl commented 5 years ago

Close it 😄