menes-dotnet / Menes

OpenAPI and dotnet service abstractions. Sponsored by endjin.
Apache License 2.0
34 stars 2 forks source link

Add support for isolated Azure Functions #318

Open idg10 opened 1 year ago

idg10 commented 1 year ago

The isolated worked process model available for non-LTS versions of .NET (and currently slated to be the main model even for LTS versions from .NET 8.0 on) introduces some changes to the programming model. Most notably, Http Triggers no longer get access to ASP.NET Core representations of requests or responses. Instead, the Microsoft.Azure.Functions.Worker libraries supply HttpRequestData and HttpResponseData wrapper types, and a functions-specific pipeline model.

This means that neither the original IOpenApiHost<HttpRequest, IActionResult> implementation available when you called services.AddOpenApiActionResultHosting<TContext>()> nor the IOpenApiHost<HttpRequest, IHttpResponseResult> version available with services.AddOpenApiAspNetPipelineHosting<TContext>() is applicable.

We need IOpenApiHost<HttpRequestData, Something>, because the HttpRequest (common to both of the existing implementations) is no longer available. The Something can't be HttpResponseData, even though that is ultimately the response type, because you can't construct an HttpResponseData directly. So we will need to follow a similar pattern to the one we introduced for the direct pipeline mode, so most likely an IHttpResponseDataResult.

We want the usage model to look something like this:

private readonly IOpenApiHost<HttpRequestData, IHttpResponseDataResult> host;

...

[Function("TenancyHost-OpenApiHostRoot")]
public Task<HttpResponseData> RunAsync(
    [HttpTrigger(AuthorizationLevel.Anonymous, "get", "patch", "post", "put", "delete", Route = "{*path}")]
    HttpRequestData req,
    ExecutionContext executionContext)
{
    return this.host.HandleRequestAsync(req, new { ExecutionContext = executionContext });
}

and HandleRequestAsync here would be an extension method that used the underlying host interface and dealt with getting the HttpRequestResponse from the IHttpResponseDataResult:

// This would be built into Menes
public static async Task<HttpResponseData> HandleRequestAsync(
    this IOpenApiHost<HttpRequestData, IHttpResponseDataResult> host,
    HttpRequestData request,
    object parameters)
{
    IHttpResponseDataResult result = await host.HandleRequestAsync(
        request.Url.AbsolutePath,
        request.Method,
        request,
        parameters);
    return await result.ExecuteResultAsync(request);
}

Our implementation of IOpenApiHost<TRequest, TResponse> is already generic, so to enable this we just need to supply implementations of the various types that is depends on.

We would need an implementation of IOpenApiParameterBuilder<HttpRequestData>. the existing IOpenApiParameterBuilder<HttpRequest> implementation, Menes.Internal.HttpRequestParameterBuilder in the Menes.Hosting.AspNetCore project currently contains a mixture of logic that is specific to HttpRequest and which would be equally applicable to HttpRequestData. We should refactor this so that the common behaviour is in a base class in Menes.Hosting, with the current Menes.Hosting.AspNetCore and a new Menes.Hosting.FunctionsWorker containing just the request-type-specific behaviour.

We would also need an IOpenApiResultBuilder<IHttpResponseDataResult> implementation. This should require slightly less rework, because we already have IOpenApiResultBuilder<IActionResult> and IOpenApiResultBuilder<IHttpResponseResult> implementations (OpenApiActionResultBuilder and OpenApiHttpResponseResultBuilder respectively) that both derive from OpenApiResultBuilder<TResult>. These already split generic behaviour out from response-specific behaviour. (That's because whereas we only had a single input type before, we already had two different result types.)

We would also need two IResponseOutputBuilder<IHttpResponseDataResult> implementations, one for POCOs, and one for OpenApiResults. Again, because this concerns response types, the library is already reasonably well factored—there are equivalent pairs of implementations of this for IActionResult and IHttpResponseResult already, and these pairs derive from a pair of base classes: PocoOutputBuilder<TResult> and OpenApiResultOutputBuilder<TResult>. We should be able to derive two more types from these to define ones that work with IHttpResponseDataResult.

We would also need to ensure that the OpenApiWebHostManager type that provides the ability to host services in-process works with the new approach.

So in summary:

APIWT commented 1 year ago

Would it make more sense to modify the abstractions to use HttpRequestMessage from System.Net.Http, then create adapters per host to go to/from HttpRequest/HttpRequestData?

idg10 commented 1 year ago

Since none of the existing frameworks we support (in-process Azure Functions, isolated Azure Functions, plain old ASP.NET Core) represent incoming requests and and outgoing responses using these types, forcing everything to use this would ensure that we would invariably end up allocating additional objects. (We'd need to build an HttpRequestMessage in addition to whatever the host environment's representation is. Likewise an HttpResponseMessage.)

One of our medium term goals is to get to a much lower allocation implementation of Menes. (We are gradually updating all our OSS IP to use System.Text.Json in place of Newtonsoft.Json as part of this work. In fact, that very transformation has, indirectly, ended up pushing us into supporting the isolated functions model. We'd have done it eventually, but that was made us look at this right now.)

So although today's Menes is very allocation-heavy, and adding one more per-request allocation isn't going to change much, it would directly conflict with where we're trying to get to.

There's also a possible "lowest common denominator" problem. An advantage of writing per-host adapters is that we can make it work in a way that is as close to optimal for that framework as possible. If instead everything goes via some intermediate representation, that can become harder.

Also, these types are mainly used client side. Trying to dredge up information from the depths of my memory I think there may once have been a point where the ASP.NET (not Core) WebAPI libraries did use these types to represent requests on the server (giving symmetry between the client and server representations of requests and responses) that's not how ASP.NET Core (or either of the Functions host models) works today. So an ASP.NET Core app might use an HttpRequestMessage to represent an outbound request to some other service, but never an inbound request. And with good reason - ASP.NET Core has gradually adapted to make use of the IMemory<T> and Span<T> representations to improve performance, and they couldn't have done this if they were stuck with the old System.Net.Http types.

Also, I'm not sure this would save us very much. We'd still end up writing per-host adapters. And also a separate adaptation layer between these types and what Menes needs. We currently have a prototype on a branch for enabling this in which we've moved the common code up into a core library, enabling everything that can be shared to be shared, with framework-specific adapter types needing to contain only the fairly small amount of code that is specific to the framework.