The Wayback Machine - https://web.archive.org/web/20220118175332/https://github.com/dotnet/aspnetcore/issues/39604
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Design: IEndpointConventionBuilder Extensions Unusable By Other Extensions #39604

Open
commonsensesoftware opened this issue Jan 18, 2022 · 0 comments

Comments

@commonsensesoftware
Copy link

@commonsensesoftware commonsensesoftware commented Jan 18, 2022

Background and Motivation

API Versioning has been investigating support for Minimal APIs per dotnet/aspnet-api-versioning#751. In doing so, it has come to light that the extension methods for IEndpointConventionBuilder are inconsistently implemented and many of them have little-to-no usably by other extensions such as API Versioning.

The primary issues relate to OpenApiRouteHandlerBuilderExtensions.cs. These extensions are very likely to be used by customers in conjunction with API Versioning, but cannot be for the following reasons:

  1. RouteHandlerBuilder is sealed
  2. The extensions methods accept and return the concrete RouteHandlerBuilder type

For completeness, a similar problem exists for FallbackEndpointRouteBuilderExtensions.cs. Fallback endpoints are not expected to be used with API Versioning, but it could affect other extensions. These extension methods accept and return IEndpointConventionBuilder, which makes them more usable than OpenApiRouteHandlerBuilderExtensions; however, the lack of passing through a more specific type means that the order setup by developers matters.

The design and implementation of each set of extension methods appears to have been done by different people, at different times, and with different design review considerations.

Proposed API

There doesn't appear to be a clear reason why these decisions were made. There seems to be no reason to not implement all of the extension methods using the same approach that @JamesNK used in RoutingEndpointConventionBuilderExtensions.cs. This would mean that all extension methods have the form of:

public static TBuilder SomeExtension<TBuilder>(this TBuilder builder) where TBuilder : IEndpointConventionBuilder { } 

This approach appears to have been lightly discussed in #8902 previously, which might explain why future extension methods did not follow suite.

The proposed change would benefit not just API Versioning, but any other extension that needs to add/change significant parts of the default Minimal API implementation.

Risks

Changing the signature of the existing APIs are a breaking change, but I believe that adding the intended generic implementations can live side-by-side with the existing non-generic variants.

If the proposal were accepted, when would that happen? As it stands, this issue cascades across APIs. API Versioning would be required to reimplement all of the applicable, existing extension methods to retain feature parity for non-versioned Minimal APIs. Furthermore, the unnecessary non-generic extensions methods have to be retained just as they do in ASP.NET Core - likely forever more. If API Versioning doesn't reimplement the extensions methods, then there is a feature gap that must be filled by customers.

API Versioning is looking for guidance to achieve the right level of synergy in both the short and long terms.

Example Usage

The tentative design for Minimal APIs for API Versioning will look something like:

app.DefineApi()
   .HasVersion(1.0)
   .HasVersion(2.0)
   .ReportApiVersions()
   .HasMapping(api => {
       // OpenAPI extensions cannot be used here without reimplementing and maintaining them
   	api.MapGet("/speak", () => "Hello world!");
   	api.MapPost("/speak", (string text) => text).MapToApiVersion(2.0);
   });

cc: @davidfowl @JamesNK

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant