Make HttpContext available to OpenAPI transformers

[martincostello]

Background and Motivation

I was looking at how to wire up the servers property based on the current HttpContext like NSwag does after opening #56188, and noticed that the HttpContext isn't immediately available to any OpenAPI transformers.

It can be easily be made available via IHttpContextAccessor, but that's often frowned upon from a performance perspective, so I figured it would be worth raising the possibility of making it available from the request pipeline to avoid the need to do that.

The HttpContext could be passed through into OpenApiDocumentService.GetOpenApiDocumentAsync() here:

https://github.com/dotnet/aspnetcore/blob/2b5d2b36a04f3a4a9bb20bbca38617d1cd6a3a1a/src/OpenApi/src/Extensions/OpenApiEndpointRouteBuilderExtensions.cs#L46

Then it can be directly assigned into the various context objects passed to any transformers, as well as being available to the document service itself.

Proposed API

namespace Microsoft.AspNetCore.OpenApi;

public sealed partial class OpenApiDocumentTransformerContext
{
+   /// <summary>
+   /// Gets the HTTP context associated with the current HTTP request.
+   /// </summary>
+   public required HttpContext { get; init }
}

public sealed partial class OpenApiOperationTransformerContext
{
+   /// <summary>
+   /// Gets the HTTP context associated with the current HTTP request.
+   /// </summary>
+   public required HttpContext { get; init }
}

public sealed partial class OpenApiSchemaTransformerContext
{
+   /// <summary>
+   /// Gets the HTTP context associated with the current HTTP request.
+   /// </summary>
+   public required HttpContext { get; init }
}

Usage Examples

internal sealed class AddServersTransformer : IOpenApiDocumentTransformer
{
    /// <inheritdoc/>
    public Task TransformAsync(
        OpenApiDocument document,
        OpenApiDocumentTransformerContext context,
        CancellationToken cancellationToken)
    {
        document.Servers = [new() { Url = GetServerUrl(context) }];
        return Task.CompletedTask;
    }

    private static string GetServerUrl(OpenApiDocumentTransformerContext context)
    {
        var request = context.HttpContext.Request;

        var scheme = TryGetFirstHeader("X-Forwarded-Proto") ?? request.Scheme;
        var host = TryGetFirstHeader("X-Forwarded-Host") ?? request.Host.ToString();

        return new Uri($"{scheme}://{host}").ToString().TrimEnd('/');

        string? TryGetFirstHeader(string name)
            => request.Headers.TryGetValue(name, out var values) ? values.FirstOrDefault() : null;
    }
}

Alternative Designs

None.

Risks

None?

[captainsafia]

I intentionally avoided introducing the HttpContext into the context object provided to transformers because the OpenAPI document isn't always guaranteed to be constructed in the context of an HTTP request. For example, when we plug into the document service via the IDocumentProvider interface at build-time, there's no HttpContext for the pipeline to plug into.

In this way, I like that having to access IHttpContextAccessor from DI (perf-ramifications aside) encourages users to be more cognizant of what they are doing when they take a dependency on the HTTP context in a transformer.

As far as usefulness, the servers scenario that you mentioned is definitely a big one. One of the reasons that I was motivated to use an IServer-based approach for resolving the addresses locally is because it avoids having to take a dependency on the request pipeline for document construction.

It's a bit of a puritan take to not want to intermingle the two concepts (request pipeline and document construction) so closely and I'm open to having my mind changed if more compelling scenarios arise.

Thoughts?

[martincostello]

I think that's a fair point to have on it.

At first, I did what you'd alluded to in the epic, and used IServer, but then I got the HTTP and HTTPS addresses locally and thought to myself whether that would be correct when deployed (localhost vs. knowing what domain it's hosted on), so looked into what NSwag does which lead me to needing the HttpContext and then opening this.

OpenAPI isn't as perf-critical as general request serving, and I didn't consider the tooling aspect, so it's not the end of the world to go down the IHttpContextAccessor route (which is what I've done for now anyway) if this isn't something that you want to expose on the accessors.

I guess a semi-middleground could be to adjust the API suggestion to make the properties nullable and not required, but I imagine that would proliferate a lot of !s over deep thought over the context where the generator runs.

[captainsafia]

At first, I did what you'd alluded to in the epic, and used IServer, but then I got the HTTP and HTTPS addresses locally and thought to myself whether that would be correct when deployed (localhost vs. knowing what domain it's hosted on)

Yep, this is the gotcha I ran into as well (I'll post more notes related to this in the issue you linked).

For now, I'd be curious to see what other scenarios exist that would merit making HttpContext more of a first-class element in the transformer context. 🤔