Omitting GroupNameFormat causes Service Provider Exceptions

[sander1095]

Is there an existing issue for this?

• I have searched the existing issues

Describe the bug

In our recent email conversations, you said:

- The statement about options.GroupNameFormat = "'v'VVV" isn’t entirely accurate
    - Setting this is a not a hard requirement for OpenAPI

I believe this isn't true. Perhaps it is on a technical level, but exceptions occur when it is missing.

You can find the runnable sample here. Just comment out the GroupNameFormat and navigate to /openapi/v1.json, and you'll see an exception.

using Asp.Versioning;

var builder = WebApplication.CreateBuilder(args);

builder.Services.AddControllers();

builder.Services.AddApiVersioning(options =>
{
    options.ReportApiVersions = true;

    options.DefaultApiVersion = new ApiVersion(1, 0);

    options.AssumeDefaultVersionWhenUnspecified = false;

    options.ApiVersionReader = new QueryStringApiVersionReader("api-version");

})
.AddApiExplorer(options =>
{
    // We are NOT setting GroupNameFormat. This causes:
    // InvalidOperationException: This service provider doesn't support keyed services.
    // Microsoft.Extensions.DependencyInjection.ServiceProviderKeyedServiceExtensions.GetKeyedService(IServiceProvider provider, Type serviceType, object serviceKey) 
    // options.GroupNameFormat = "'v'VVV";
})
.AddMvc()
.AddOpenApi();

var app = builder.Build();

app.MapOpenApi().WithDocumentPerVersion();

app.MapControllers();

app.Run();

The exception:

fail: Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware[1]
      An unhandled exception has occurred while executing the request.
      System.InvalidOperationException: This service provider doesn't support keyed services.
         at Microsoft.Extensions.DependencyInjection.ServiceProviderKeyedServiceExtensions.GetKeyedService(IServiceProvider provider, Type serviceType, Object serviceKey)
         at Microsoft.Extensions.DependencyInjection.KeyedServiceContainer.GetKeyedService(Type serviceType, Object serviceKey) 
         at Microsoft.Extensions.DependencyInjection.ServiceProviderKeyedServiceExtensions.GetKeyedService[T](IServiceProvider provider, Object serviceKey)
         at Microsoft.AspNetCore.Builder.OpenApiEndpointRouteBuilderExtensions.<>c__DisplayClass0_0.<<MapOpenApi>b__0>d.MoveNext()
      --- End of stack trace from previous location ---
         at Microsoft.AspNetCore.Http.Generated.<GeneratedRouteBuilderExtensions_g>F0399B8B15BE443123F4BECEE84B112C1912EE3F44D9DDD7BFA38E371FB917343__GeneratedRouteBuilderExtensionsCore.<>c__DisplayClass2_0.<<MapGet0>g__RequestHandler|5>d.MoveNext()       
      --- End of stack trace from previous location ---
         at Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.Invoke(HttpContext context)
         at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
         at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddlewareImpl.Invoke(HttpContext context)

Expected Behavior

Either:

• A clear exception (and documentation/explanation) that GroupNameFormat has to be set
• A Roslyn analyzer that warns me that GroupNameFormat isnt set and will cause issues
• A different library design that doesn't allow me to configure API Versioning + OpenAPI in an incompatible way

Steps To Reproduce

Go to openapi/v1.json

using Asp.Versioning;

var builder = WebApplication.CreateBuilder(args);

builder.Services.AddControllers();

builder.Services.AddApiVersioning(options =>
{
    options.ReportApiVersions = true;

    options.DefaultApiVersion = new ApiVersion(1, 0);

    options.AssumeDefaultVersionWhenUnspecified = false;

    options.ApiVersionReader = new QueryStringApiVersionReader("api-version");

})
.AddApiExplorer(options =>
{
    // We are NOT setting GroupNameFormat. This causes:
    // InvalidOperationException: This service provider doesn't support keyed services.
    // Microsoft.Extensions.DependencyInjection.ServiceProviderKeyedServiceExtensions.GetKeyedService(IServiceProvider provider, Type serviceType, object serviceKey) 
    // options.GroupNameFormat = "'v'VVV";
})
.AddMvc()
.AddOpenApi();

var app = builder.Build();

app.MapOpenApi().WithDocumentPerVersion();

app.MapControllers();

app.Run();

Exceptions (if any)

fail: Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware[1]
An unhandled exception has occurred while executing the request.
System.InvalidOperationException: This service provider doesn't support keyed services.
at Microsoft.Extensions.DependencyInjection.ServiceProviderKeyedServiceExtensions.GetKeyedService(IServiceProvider provider, Type serviceType, Object serviceKey)
at Microsoft.Extensions.DependencyInjection.KeyedServiceContainer.GetKeyedService(Type serviceType, Object serviceKey)
at Microsoft.Extensions.DependencyInjection.ServiceProviderKeyedServiceExtensions.GetKeyedService[T](IServiceProvider provider, Object serviceKey)
at Microsoft.AspNetCore.Builder.OpenApiEndpointRouteBuilderExtensions.<>c__DisplayClass0_0.<b__0>d.MoveNext()
--- End of stack trace from previous location ---
at Microsoft.AspNetCore.Http.Generated.<GeneratedRouteBuilderExtensions_g>F0399B8B15BE443123F4BECEE84B112C1912EE3F44D9DDD7BFA38E371FB917343__GeneratedRouteBuilderExtensionsCore.<>c__DisplayClass2_0.<g__RequestHandler|5>d.MoveNext()
--- End of stack trace from previous location ---
at Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.Invoke(HttpContext context)
at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddlewareImpl.Invoke(HttpContext context)

.NET Version

10

Anything else?

No response

[sander1095]

Just to add another "thing": When I use "'v'VVVV" (4 Vs instead of 3), exception occur as well.

This is kinda difficult to debug. Perhaps something like a StringSyntaxAttribute, analyzer, or compile-time safety would be helpful. This should probably become another issue if this something actionable for the library

[commonsensesoftware]

GroupNameFormat is definitely not required; however, I think you need to understand the ramifications of not doing so. The name of JSON file is based of the group name, which for API Versioning is the version. Remember that out-of-the-box the OpenAPI support does not support anything about versioning and defines a /openapi/v1.json as an assumed default. This also implicitly assumes that you will be versioning using the scheme v# (which is also not required). Once you enable API Versioning, this is no longer true. The GroupNameFormat controls how the API version will be formatted for the API Explorer and OpenAPI. Many people expect or want to use a format such as /openapi/v1.json; however, without GroupNameFormat, the formatting will simply be ApiVersion.ToString(). The exact value depends on how it ApiVersion was constructed. If a minor version isn't provided, then it will be omitted by ToString(). This is by design to ensure the value roundtrips. The result is that the correct URL should be /openapi/1.json ([ApiVersion(1)]) or /openapi/1.0.json ([ApiVersion(1.0)]). This is why the example shows VVV as that format will not show the minor version if it wasn't specified or it's 0.

The "'v'VVVV" format should be valid. It doesn't look like you're getting FormatException. The "This service provider doesn't support keyed services." is likely because you are not requesting the URL in the format that the was used. Assuming you used [ApiVersion(1.0)], I would still expect the URL to be /openapi/1.0.json. If you had [ApiVersion("1.1-rc.1")], then the URL would be /openapi/1.1-rc.1.json. AFAIK, the version value is emitted verbatim.

[commonsensesoftware]

The StringSyntaxAttribute, unfortunately, is not an option. There is a loose coupling on an IDE. In order to show the syntax, the IDE (e.g. Visual Studio) has to understand the format you've provided. Thus far, it does not support custom formats or a syntax dialect to do so. Perhaps one day. I'm all for supporting it, if it's possible. I seem to recall exploring this idea once and I was surprised to find out there is a requirement for the IDE side to do work as well. 🤷🏽

[Kumima]

@commonsensesoftware , I'm meeting this as well. But I dont think it's related to GroupNameFormat.

builder.Services.AddApiVersioning(
        options =>
        {
            options.ReportApiVersions = true;
            options.ApiVersionReader = new UrlSegmentApiVersionReader();
        })
    .AddApiExplorer(
        options =>
        {
            options.GroupNameFormat = "'v'VV";
            options.SubstitutionFormat = "VV";
            options.SubstituteApiVersionInUrl = true;
        })
    .AddOpenApi();

var app = builder.Build();

app.MapOpenApi().WithDocumentPerVersion();

IVersionedEndpointRouteBuilder versionedApi = app.NewVersionedApi();
RouteGroupBuilder v1Group = versionedApi.MapGroup("v{version:apiVersion}").HasApiVersion(1.0);
// This one throws
v1Group.MapGet(
    "test",
    async Task<ContentHttpResult> (
        [FromKeyedServices("service name")] SomeService someService) =>
    {
         //...
    });

// While this one not
app.MapGet("test2",
  async Task<ContentHttpResult> (
        [FromKeyedServices("service name")] SomeService someService) =>
    {
         //...
    });

Am I doing some anti pattern that I should not use the group way? I saw you did some trick on KeyedService for WithDocumentPerVersion. Is it related?

I tried similiar code on 8.1.1, it does not throw. I do think this is a critical issue. Also tried Asp.Versioning.Http 10.0.0-preview.2 only without OpenAPI stuffs, and it throws.

[commonsensesoftware]

@Kumima does the exception occur at setup time, requesting the endpoint at runtime, or just trying to navigate to the OpenAPI document? If it's the latter, then it's almost certainly related to the name you are requesting in the URL doesn't match the formatted API version that is used at the group name and service key. I see why it was done, but using the API version as the service key was a bad design decision IMHO. Keyed services should have a discrete set of keys, but API versions do not (e.g. they are variable). Unfortunately, that's out of my hands. I had to do some sorcery by creating my own KeyedServicesContainer to get in front of the default behavior. While I can look for a matching key, I can't reason about not matching. Requests that do not match forward to the default IKeyedServiceProvider.

Another potential gotcha is that IKeyedServiceProvider uses object for the key. This means that the keys will be case-sensitive. 😞 Make sure the casing matches. @sander1095 mentioned something similar. I'll be looking to see if there's a way to make that less brittle. This behavior should be a non-issue of things that use the reported names verbatim (which is why I don't see it), but manual attempts to do it by hand have to match up.

If you want to be sure what the formatted group names will be, you can use:

foreach (var desription in app.DescribeApiVersions())
{
    // TODO: log or print description.GroupName
}

any after you've setup all your endpoints, but before you run the application. This is how the pieces are connected internally, which you can see here:

aspnet-api-versioning/src/AspNetCore/WebApi/src/Asp.Versioning.OpenApi/DependencyInjection/IApiVersioningBuilderExtensions.cs

Line 109 in 3bdca4a

private static KeyedServiceContainer NewRequestServices( IServiceProvider services )

[Kumima]

@commonsensesoftware It happens at runtime when requesting that. I'm not using the api version as key, it's just an another string.

You should try the code above on 8.1.1 and 10 preview 2. The behaviors are different. Also for key not found problem, the exception should be unable to resolve, but not above. It's not a key not found problem.

To reproduce

WebApplicationBuilder builder = WebApplication.CreateBuilder(args);

builder.Services.AddProblemDetails();

builder.Services.AddApiVersioning(
    options =>
    {
        options.ReportApiVersions = true;

        options.DefaultApiVersion = new ApiVersion(1, 0);

        options.AssumeDefaultVersionWhenUnspecified = true;
    });

builder.Services.AddKeyedSingleton<SomeService>(nameof(SomeService));

WebApplication app = builder.Build();

app.UseHttpsRedirection();

app.UseRouting();

app.MapGet(
    "root",
    ([FromKeyedServices(nameof(SomeService))] SomeService someService) => TypedResults.Text(someService.ServiceName));

// throws
IVersionedEndpointRouteBuilder versionedApi = app.NewVersionedApi();
versionedApi.MapGet(
        "versioned-api",
        ([FromKeyedServices(nameof(SomeService))] SomeService someService)
            => TypedResults.Text(someService.ServiceName))
    .HasApiVersion(1.0);


RouteGroupBuilder versionedGroup = versionedApi.MapGroup("versioned-group").HasApiVersion(1.0);

// throws
versionedGroup.MapGet(
    string.Empty,
    ([FromKeyedServices(nameof(SomeService))] SomeService someService) => TypedResults.Text(someService.ServiceName));

app.Run();

[Kumima]

@commonsensesoftware InjectApiVersion needs to implement IKeyedServiceProvider

aspnet-api-versioning/src/AspNetCore/WebApi/src/Asp.Versioning.Http/Builder/EndpointBuilderFinalizer.cs

Line 79 in 3bdca4a

context.RequestServices = new InjectApiVersion( context );

aspnet-api-versioning/src/AspNetCore/WebApi/src/Asp.Versioning.Http/Builder/EndpointBuilderFinalizer.cs

Lines 285 to 308 in 3bdca4a

	private sealed class InjectApiVersion : IServiceProvider
	{
	private static readonly Type ApiVersionType = typeof( ApiVersion );
	private readonly IServiceProvider provider;
	private readonly HttpContext context;
	public InjectApiVersion( HttpContext context )
	{
	this.context = context;
	provider = context.RequestServices;
	context.RequestServices = this;
	}
	public object? GetService( Type serviceType )
	{
	if ( serviceType.IsAssignableFrom( ApiVersionType ) )
	{
	return context.RequestedApiVersion;
	}
	return provider.GetService( serviceType );
	}
	}
	}

https://github.com/dotnet/dotnet/blob/d70206844a95b337601237466bfc6cbb7d52d6d4/src/runtime/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ServiceProviderKeyedServiceExtensions.cs#L62-L73

That ApiVerson seems accessable using Features. I'd suggest do not override build-in services things like that. What if some crazy one has logic based on httpContext.RequestServices.GetType().Name, then it differs from not using ASP.NET API Versioning

[commonsensesoftware]

Ah ... I see now. This is not related to OpenAPI at all. It's probably worth opening a new, separate issue so as not to conflate the two. I used a similar technique, but it's a different scenario.

TL;DR, ASP.NET still doesn't have any way to provide surrogate parameter binding. All of the currently supported methods won't work for API Versioning without IHttpContextAccessor, which is gross with a bunch of other problems. The use case is simple:

var api = app.NewVersionedApi("Example");
var v1 = app.MapGroup("/example");

v1.MapGet("/", (ApiVersion version) => TypedResults.Text($"Hello world from v{version}");

ApiVersion is special just like CancellationToken, but there's no exposed way to hook it in except by DI.

This is certainly a 🐞 and a good find. The underlying implementation must be doing a cast, which I'm a bit surprised by. The classic approach for this type of behavior would be Services.GetService(typeof(IKeyedServiceProvider)), even if it just returns itself. That's clearly not what it does. This should be a pretty simple fix. With respect to someone using httpContext.RequestServices.GetType().Name directly - caveat emptor. If someone does that and it breaks them, they're doing it wrong (IMHO). The right way is the with is or as operator against the type and the only good reason I can think of that anyone would need to do that is if you're decorating the current instance and you don't want to do it more than once. I've even written such defensive cases in the past.