Request Dispatch Pipeline

[bernardnormier]

This is a proposal to convert ObjectAdapter / Server into a request dispatch pipeline, similar to ASP.NET's pipeline.

A request dispatch pipeline consists of a chain of Dispatcher (RequestDelegate in ASP.NET). A dispatch interceptor is a Func<Dispatcher, Dispatcher> that inserts a new dispatcher in this chain. See also #85.

The API is very simple and consists of a (thread-unsafe) DispatchBuilder, a Server class plus various extension classes. The extension classes use only public APIs exposed by DispatchBuilder / Server.

See https://docs.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.builder.iapplicationbuilder?view=aspnetcore-5.0

public sealed class DispatchBuilder
{
    public DispatchBuilder() { ... }
    public DispatchBuilder Use(Func<Dispatcher, Dispatcher> dispatchInterceptor) { ... }
    public DispatchBuilder New() { ... } // creates a new branch

    public Dispatcher Build() { ... } // the new request dispatch pipeline
}

public sealed class Server : IAsyncDisposable
{
    public Server(Communicator communicator, Dispatcher dispatchPipeline, ServerOptions options) { ... }

    public Task ActivateAsync() { ... }
    public ValueTask DisposeAsync() => new(ShutdownAsync());
    public Task ShutdownAsync(CancellationToken cancel = default) { ... }
}

The user constructs the request dispatch pipeline by adding dispatch interceptors, usually through easy-to-use DispatchBuilder extensions similar to ASP.NET's extensions:
https://docs.microsoft.com/en-us/aspnet/core/fundamentals/middleware/?view=aspnetcore-5.0

In particular, the ASM is replaced by a Use extension:

public static class DispatchBuilderExtensions
{
    public static DispatchBuilder UseASM(this DispatchBuilder builder, IReadOnlyDictionary<Identity, IService> asm)
    {
          ... if current.Identity is in ASM, run associated service ...
    }

    // Faceted version
    public static DispatchBuilder UseASM(this DispatchBuilder builder, IReadOnlyDictionary<(Identity, string), IService> asm)
    {
          ... if current.Identity is in ASM, run associated service ...
    }
}

This way, the user can insert her own ASM dictionary (preferably a concurrent dictionary) anywhere in the pipeline. And if there is no hit in the ASM, the pipeline continues. Naturally the leaf dispatcher throws ONE.

The old default servant is replaced by a terminal Run:

public static class DispatchBuilderExtensions
{
    public static DispatchBuilder Run(this DispatchBuilder builder, Dispatcher dispatcher) { ... regular terminal run ... }
}

Note that unlike default servants today, this default servant cannot be added or removed once the server is built. It's not a big drawback, and if the user needs runtime addition/removal, she can always add her own interceptor.

A default servant for a given category could be replaced by a Map:

public static class DispatchBuilderExtensions
{
    public static DispatchBuilder Map(this DispatchBuilder builder, string category, Action<DispatchBuilder> configuration)
    {
           ....
    }

    public static DispatchBuilder MapWhen(this DispatchBuilder builder, Func<Current, bool> predicate, Action<DispatchBuilder> configuration)
    {
          ...
    }
}

Note that you can very easily convert a servant into a Dispatcher in C#, for example this works fine:

builder.Run(defaultServant.DispatchAsync); // defaultServant.DispatchAsync is a Dispatcher

For ease of use, we would provide additional extensions for servants, e.g.

public static class DispatchBuilderExtensions
{
    public static DispatchBuilder Run(this DispatchBuilder builder, IService servant) => builder.Run(servant.DispatchAsync);
}

With this proposal, adding a servant / default servant to the "server" no longer returns a proxy. Proxies would be manufactured separately using an extension class for Server:

// the proxy extension methods for a server
public static class ServerProxyExtensions
{
    public static T CreateProxy(this Server server, Identity identity, ProxyFactory<T> factory) { ... }
    public static T CreateProxy(this Server server, string identity, ProxyFactory<T> factory) { ... }
}

Not included in this proposal (to keep it focused):

• how services with default identities fit in all this. See https://docs.microsoft.com/en-us/aspnet/core/grpc/services?view=aspnetcore-5.0 and https://docs.microsoft.com/en-us/aspnet/core/grpc/aspnetcore?view=aspnetcore-5.0&tabs=visual-studio

[bernardnormier]

Map implementation:
https://github.com/dotnet/aspnetcore/blob/main/src/Http/Http.Abstractions/src/Extensions/MapExtensions.cs#L55

[bentoi]

I like this even though it's quite complicated!

We'll definitely need some easy to use extensions for registering servants for services and possibly a SessionDispatcher ;-).

Do we really need ServerProxyExtensions? What's the advantage over keeping the CreateProxy methods on the Server class?

Also if we allow registering servants with the following Map method:

public static DispatchBuilder Map(this DispatchBuilder builder, string category, IService servant) => ...

Isn't this potentially expensive if the user registers a bunch of services with this method? If I understand it correctly, each Map dispatcher will be chained.

It might be clearer to provide the following instead:

public static DispatchBuilder ServiceMap(this DispatchBuilder builder, IList<IService servant>) => ...
public static DispatchBuilder ObjectMap(this DispatchBuilder builder, IDictionary<Ice.Identity, IService servant>) => ...
public static DispatchBuilder DefaultServantMap(this DispatchBuilder builder, IDictionary<string, IService servant>) => ...
public static DispatchBuilder DefaultServant(this DispatchBuilder builder, IService servant) => ...

Or ServiceWithIdentityMap instead of ObjectMap.

[pepone]

I like this especially the splitting of the servant map and the server, for servers hosting many servants MapWithASM seems can provide behavior equivalent to what we have today, there is the extra step of creating the Dictionary but it also gives you more control, so I think it is a good trade-off.

[bernardnormier]

I read more about it and switched the ASM to a UseASM extension method, which is more appropriate than MapXxx. There are basically 3 types of methods:

• UseXxx: insert an interceptor. This can be terminal or not.
• MapXxx: create a new branch in the dispatch tree, with its own Dispatcher built using a shallow clone of this DispatchBuilder
• Run: terminal that executes the provided dispatcher no matter what - there is no next

Now, ASP.NET IApplicationBuilder also provides a very powerful mechanism called "routing", that relies on two Use methods: UseRouting and UseEndpoints. See https://docs.microsoft.com/en-us/aspnet/core/fundamentals/routing?view=aspnetcore-5.0.

Note that endpoint here does not mean transport endpoint: it's more like a "terminal" in a dispatch pipeline. And the endpoints are actually defined within the UseEndpoints(...) call.

Endpoints represent units of the app's functionality that are distinct from each other in terms of routing, authorization, and any number of ASP.NET Core's systems.

In grpc dotnet, it's in UseEndpoints that services are registered, see: https://docs.microsoft.com/en-us/aspnet/core/grpc/aspnetcore?view=aspnetcore-5.0&tabs=visual-studio

I believe we could do the same, with UseRouting() and UseEndpoints(), and a Current.Endpoint set during UseRouting(). Current.Endpoint would be a DispatchEndpoint, similar to an ASP.NET Endpoint/RouteEndpoint.

All the endpoint-definition methods (https://docs.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.builder.endpointroutebuilderextensions?view=aspnetcore-5.0) start with Map.

In addition to UseASM (for DispatchBuilder), we could have a MapASM (for our DispatchEndpointRouteBuilder):

IReadOnlyDictionary<Identity, IService> asm = ...

builder.UseEndpoints(endpoints =>
    endpoints.MapASM(asm); // the whole ASM "match" is a single endpoint
    ...);

And just like UseASM, it could be called several times with different little ASMs, e.g.

builder.UseEndpoints(endpoints =>
    endpoints.MapASM(adminASM).RequireAuthorization(); // all services in adminASM require authorization
  
    endpoints.MapASM(widgetASM); // the services in the widget ASM don't
    ...);

[bernardnormier]

After thinking more about this topic, I think we should only implement a basic pipeline and leave routing/endpoints for a later version.

routing/endpoints are really about communications between interceptors, particularly with built-in interceptors like authentication (and many others), and obviously so far we don't have any interceptors.

Side note:
Interestingly, the low-level property that routing/endpoints use is the feature collection on HttpContext: https://docs.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.http.httpcontext.features?view=aspnetcore-5.0#Microsoft_AspNetCore_Http_HttpContext_Features
(SetEndpoint and GetEndpoint are extension methods that use Features, as you would expect).

So to restate the proposal:

• we provide a DispatchBuilder similar to IApplicationBuilder (TODO: should we use an interface? or a generic PipelineBuilder<T...> that we can reuse for the invocation pipeline?)
• we provide an extension class with UseXxx, MapXxx and Run:

public static class DispatchBuilderExtensions
{
     public static DispatchBuilder Use(this DispatchBuilder builder, Func<...> dispatchInterceptor) { ... }

     public static DispatchBuilder UseASM(this DispatchBuilder builder, IReadOnlyDictionary<Identity, IService> asm)
     {
          ... if current.Identity is in ASM, run associated service ...
     }
     // for simplicity, no faceted version (assuming we drop facets from ice2)

     public static DispatchBuilder UseForCategory(this DispatchBuilder builder, string category, Dispatcher dispatcher)
     {
        ...
     }

     // replacement for category-specific default servant
     public static DispatchBuilder UseForCategory(this DispatchBuilder builder, string category, IService servant)
     {
        ...
     }

      public static DispatchBuilder MapWhen(this DispatchBuilder builder, Func<Current, bool> predicate, Action<DispatchBuilder> configuration)
      {
            ...
      }

      public static DispatchBuilder Run(this DispatchBuilder builder, Dispatcher dispatcher) { ... regular terminal run ... }

      // replacement for "catch all" default servant
      public static DispatchBuilder Run(this DispatchBuilder builder, IService servant) { ... regular terminal run ... }
}

I'll create a separate proposal for the proxy factories.

[bentoi]

I would drop "ASM". What's an active servant? Are there inactive servants if there are active ones :)?

It's a bit strange to mix Use and UseXxx methods. Shouldn't it be one or the other (and possibly rename Use to UseDispatchInterceptor if we want to be more explicit on the kind of registered object).

We could consider the following names otherwise:

• UseServiceMap, UseServiceList or just UseServices (services registered with default identity optionally set in the Slice).
• UseServiceMapWithIdentity, UseIdentityServiceMap, UseServicesWithIdentity (services registered with identities)

Why is there two UseForCategory, one that takes a service and the other that takes a dispatcher? Could we just drop the one that takes a dispatcher? The user can rely on MapWhen for this one, right?

[bernardnormier]

It's a bit strange to mix Use and UseXxx

It's just like ASP.NET middlewares, see: https://docs.microsoft.com/en-us/aspnet/core/fundamentals/middleware/?view=aspnetcore-5.0 (toward the end).

Basically, plain Use is for a plain interceptor/middleware that you provide an argument (possibly a "simple" one registered through an extension), while UseXxx is to register a specific interceptor not-supplied as an argument.

I think the bright-line here is if an argument calls next (for the next interceptor) => plain Use. Otherwise => UseXxx.

I would drop "ASM". What's an active servant? Are there inactive servants if there are active ones :)?

I picked ASM as an abstract name :). It's really an identity to servant dictionary, and UseIdentityToServantDictionary does not roll off the tongue :).

UseServiceMap, UseServiceList or just UseServices (services registered with default identity optionally set in the Slice).

Are you in favor of dropping the distinction servant/service, and using service for both?
There is some logic to it, since a servant is an IService. However, it makes it harder to talk about "services with multiple identities" implemented/incarnated by the same default service.

In practice, I suspect this ASM support (regardless of the name) is not going to be used all that much. And we should provide a way to register a single identity/servant pair, e.g.:

public DispatchBuilder UseService(this DispatchBuilder builder, Identity identity, IService service);

and perhaps even:

// a dependency-injection servant with an identity derived from T's Slice Type ID
public DispatchBuilder UseService<T>(this DispatchBuilder builder) where T : IService { ... }

Why is there two UseForCategory, one that takes a service and the other that takes a dispatcher? Could we just drop the one that takes a dispatcher? The user can rely on MapWhen for this one, right?

There are all helpers built using IDispatchBuilder.Use (and other methods), so we can drop any of them. The UseForCategory with a servant is just a tiny wrapper around the dispatcher version (since it's trivial to convert a servant into a dispatcher).

Using Map/MapXxx is not appropriate here. Map creates a new branch in the pipeline and is therefore pretty heavy. Look at its configuration parameter.

This also highlights a missing item with this proposal: how/when is the DispatchBuilder called? In ASP.NET, all this setup occurs in Startup.Configure, and the builder (app) is the first parameter of Startup.Configure. Creating a new branch (with MapXxx) basically "runs" a new Configure-liked method with a clone of the builder ("clone" here meaning that shares properties like the logger).

[bentoi]

Now that you're mentioning it, dropping "servant" might indeed be worth considering :) A service implementation can be registered with an identity, a default identity or no identity (in which case it will be called for any identity). I don't think it's too difficult to explain without "servant".

If we add UseService(), will this result in a pipeline where services will be checked one after the other? (which wouldn't be very efficient!).

I'm not a specialist of ASP.NET and it's quite possible there are things I didn't understand with the proposal, but I'm wondering if we really need a builder for building the pipeline. What is the advantage here? It seems to me that the builder is useful when using dependency injection to not have to instantiate directly the concrete classes. Unless we decide to do dependency injection, couldn't we simply provide factory methods to create the dispatchers?

[pepone]

I think would be nice to have UseServices (use a list of services with default identity) and UseServiceMap (a dictionary of identity service)

Using Use is fine for servers with a single service, but it is nice to have the other extensions to better handling servers with many services.

I might be wrong, but I think the builder is unrelated to DI, it just holds the pipeline state while it is being built, calling Build just materialize the pipeline as a delegate and returns it.

[bentoi]

Right the builder design pattern isn't specific to DI but is often used with DI. I'm fine for using a builder if it really makes things simpler.

[bernardnormier]

If we add UseService(), will this result in a pipeline where services will be checked one after the other? (which wouldn't be very efficient!).

Indeed, that's not good because a user could naively add a bunch of services with UseService.

I think would be nice to have UseServices (use a list of services with default identity) and UseServiceMap (a dictionary of identity service)

I am wondering if UseServices should be more like UseEndpoints in ASP.NET, i.e. it takes a builder that is used to build a static "service map". Something like:

dispatch.UseServices(services =>
{
    services.Map(new Identity("foo", bar"), new Widget()); // "registers" a Widget service with identity foo/bar
  
    services.Map("foo/bar2"), new Widget()); // same with different syntax

    services.Map(new Widget()); // this one is used Widget's Slice type-id to compute the identity 
}

This way, it can be efficient even if you add a bunch of services: we're just building a dictionary at configuration-time.

One difficulty is this dictionary would be static, and some applications want to create more services at runtime. So we still need an IReadOnlyDictionary<Identity, IService> somewhere.

I also suspect creating services with a default identity post-startup is not needed.

[bernardnormier]

Now that you're mentioning it, dropping "servant" might indeed be worth considering :) A service implementation can be registered with an identity, a default identity or no identity (in which case it will be called for any identity). I don't think it's too difficult to explain without "servant".

Ok, let's drop servant!

[bernardnormier]

Using Use is fine for servers with a single service, but it is nice to have the other extensions to better handling servers with many services.

If you have a single catch-all service, you should use Run. Use without suffix is to register an interceptor.

[bernardnormier]

Right the builder design pattern isn't specific to DI but is often used with DI. I'm fine for using a builder if it really makes things simpler.

How we plug [I]DispatchBuilder into Server is still TBD.

I think we definitely want to build the dispatch pipeline during startup in a thread-unsafe way, and then somehow provide a "frozen" dispatch pipeline to the server. While the pipeline itself is frozen, some elements (interceptors) can use a IReadOnlyDictionary<Identity, IService> (or similar) that actually changes at runtime.

[pepone]

I am wondering if UseServices should be more like UseEndpoints in ASP.NET, i.e. it takes a builder that is used to build a static "service map". Something like:

Seems good to me, here you can add services with the default identity or custom identities and still end up on the same map

So we still need an IReadOnlyDictionary<Identity, IService> somewhere.

I was thinking UseServiceMap can do that, it can hold an IReadOnlyDictionary<Identity, IService> and dispatch to it

[bernardnormier]

I think it would make sense to integrate ASM and ForCategory services in UseServices, with more MapXxx overloads. See also: https://docs.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.routing.iendpointroutebuilder?view=aspnetcore-5.0

For example:

dispatch.UseServices(services =>
{
    services.Map(new Identity("foo", bar"), new Widget()); // "registers" a Widget service with identity foo/bar
  
    services.Map("foo/bar2"), new Widget()); // same with different syntax

    services.Map(new Widget()); // this one is used Widget's Slice type-id to compute the identity 

    services.MapFallback("bar", new WidgetForBar()); // category-specific fallback. Does not matter the order where it appears here

   services.MapDynamic(myDict); // myDict is an IReadOnlyDictionary<Identity, IService>   
}

And the logic for this interceptor could be:

1. first lookup in static dictionary built with services.Map(...)
2. second lookup in the dynamic map, if any (and only one dynamic map is allowed?)
3. finally lookup in the category-specific Fallback static map

TBD: what about duplicate registrations?
My impression is ASP.NET just ignores them, e.g. https://docs.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.builder.endpointroutebuilderextensions.mapget?view=aspnetcore-5.0#Microsoft_AspNetCore_Builder_EndpointRouteBuilderExtensions_MapGet_Microsoft_AspNetCore_Routing_IEndpointRouteBuilder_System_String_Microsoft_AspNetCore_Http_RequestDelegate_ does not throw any exception.

It's also possible the first registration (not the last) wins with ASP.NET.

[bernardnormier]

The UseServices above is a simplified version of ASP.NET UseEndpoints, and hence we could complete the job, i.e.

ASP.NET UseRouting => IceRPC: implicit service lookup at the beginning of the pipeline (if the pipeline has a UseServices), and set Current.Service if found

ASP.NET UseEndpoints => IceRPC UseServices

ASP.NET endpoint => IceRPC service

ASP.NET endpoint properties => IceRPC service properties (doesn't exist currently)

ASP.NET HttpContext Features (including Endpoint) => IceRPC Current.Service

[pepone]

TBD: what about duplicate registrations?
My impression is ASP.NET just ignores them,

Not clear what you mean, do you mean that only the first call to UseServices has any effect?

[bernardnormier]

TBD: what about duplicate registrations?
My impression is ASP.NET just ignores them,

Not clear what you mean, do you mean that only the first call to UseServices has any effect?

I think UseEndpoints() builds a filter where order matters. So if a Map matches first, further Map that match as well are ignored. I could be wrong of course.

[externl]

TBD: what about duplicate registrations?
My impression is ASP.NET just ignores them,

Not clear what you mean, do you mean that only the first call to UseServices has any effect?

I think UseEndpoints() builds a filter where order matters. So if a Map matches first, further Map that match as well are ignored. I could be wrong of course.

I suspect you're correct here. That's typically how it works with many web frameworks, order added is important and first match wins.

[bernardnormier]

One issue with my proposal is ASP.NET endpoint does not map well to IceRPC service, because UseEndpoints actually builds the endpoints, including the endpoints metadata. With the UseServices suggested above, it's the user who actually builds the services.

Keeping them separate sounds better. So the API could be:

dispatch.UseEndpoints(endpoints =>
{
    endpoints.Map(new Identity("foo", bar"), new Widget()); // creates an endpoint that wraps the Widget service
  
    endpoints.Map("foo/bar2"), new Widget()); // same with different syntax

    endpoints.Map(new Widget()); // this one is using Widget's Slice type-id to compute the identity 

    endpoints.MapFallback("bar", new WidgetForBar()); // category-specific fallback. Also creates a single endpoint.

    endpoints.MapDynamic(myDict); // creates again a single endpoint that uses myDict to do the job

    var facet = new Facet("facet");
    endpoints.MapDynamic(myDict).WiithMetadata(facet);  // creates a single endpoint with Metadata T=Facet and value facet
}

with
public sealed record Facet(string Name);

See: https://docs.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.builder.routingendpointconventionbuilderextensions.withmetadata?view=aspnetcore-5.0#Microsoft_AspNetCore_Builder_RoutingEndpointConventionBuilderExtensions_WithMetadata__1___0_System_Object___

[bentoi]

endpoints.MapDynamic sounds a bit strange, can't it just be Map?

For endpoints.Map(new Widget()); // this one is using Widget's Slice type-id to compute the identity, I insist but I really think it should be a Slice defined identity if we want to pave the way for REST accessible IceRPC services (which would expose the default service identity in the HTTP URI... better have a nice identity rather than a strange typed ID based generated identity).

[pepone]

, I insist but I really think it should be a Slice defined identity

I guess we can use type-id as the default, and have metadata to control it, or what do you have in mind?

[bentoi]

I guess we can use type-id as the default, and have metadata to control it, or what do you have in mind?

Something like the following: #65 (comment) and yes, either it's required or we provide a default derived from the type-id.

[bernardnormier]

A popular alternative to the ASP.NET pipeline (and terminology) is https://github.com/go-chi/chi and maybe using the Chi terminology for IceRPC would be better?

• Interceptor vs Middleware
  Even though middleware sounds strange to me, it's the standard term for this concept so I think we should switch to middleware

• Pipeline
  With ASP.NET, you build the/a "request pipeline" with an IApplicationBuilder named app. Wit go-chi, you build the/a "Router" named r. This router can have sub-routers created with Route. This router is itself an http.handler so there is no need for a Build() call to get the actual pipeline (dispatcher with IceRPC, ). And in go-chi, Router is implemented by Mux: https://github.com/go-chi/chi/blob/master/mux.go#L21

I find go-chi more elegant and a better match for IceRPC. The IceRPC verbs would be Use, Mount, Route, Handle (but not Get, Put, Delete, Post, Method)

If we follow the go-chi example, we would need to convert Dispatcher into an interface, with a Router than extends Dispatcher.

[bernardnormier]

Restated go-chi -like proposal:

1. Convert Dispatcher into an interface (IDispatcher), with a single DispatchAsync method. Make IService derive from IDispatcher.

2. Simplify Server: remove all services (Add, Remove, Find etc.) from Server. Instead, a server is constructed with an IDispatcher. This dispatcher can be a service, or more likely, an IRouter (see below).

3. A router (IRouter) is a dispatcher that can be used to compose middlewares, create sub-routes, and associate a dispatcher to a path. The verbs are:

• Use: add a middleware
• Dispatch(path, dispatcher): associate a full path (aka endpoint) to a dispatcher
• Mount(prefix, dispatcher): associate a prefix ("/foo") to a dispatcher

4. We provide a Mount(prefix, ...) overload where the second parameter is a IReadOnlyDictionary<string, IDispatcher>, for example:

r.Mount("/foo", fooDict); // "/foo" consumed before looking up in fooDict.

r.Mount("/", catchAllDict); // prefix = "/" meaning catch-all, and "/" not consumed when looking up in dict

The dictionary is naturally not copied and the application can update it after server creation.

5. Later on, consider adding more methods such as Route(prefix, Action)

[bernardnormier]

Fixed by #178 and subsequent PRs.