diff --git a/src/HttpExtensions/EndpointDataSourceServiceCollectionExtensions.cs b/src/HttpExtensions/EndpointDataSourceServiceCollectionExtensions.cs new file mode 100644 index 000000000000..aedd83df4d22 --- /dev/null +++ b/src/HttpExtensions/EndpointDataSourceServiceCollectionExtensions.cs @@ -0,0 +1,52 @@ +using Microsoft.AspNetCore.Routing; +using Microsoft.Extensions.DependencyInjection; + +namespace Bit.HttpExtensions; + +public static class EndpointDataSourceServiceCollectionExtensions +{ + /// + /// Registers so the offline OpenAPI generator (dotnet swagger tofile) can + /// discover Minimal API endpoints it would otherwise miss — it never runs the Configure pipeline where + /// the endpoints are mapped. Multiple features may call this; a single + /// composes all of their mappings, because ApiExplorer injects only one and a + /// source-per-feature would let the last-registered hide the rest. + /// + /// Intentionally a no-op outside of swagger generation: at runtime the endpoints are served by the host's + /// UseEndpoints mapping, and registering this source then would replace the default composite + /// that runtime routing and link generation depend on. + /// + public static IServiceCollection AddOpenApiEndpointDataSource( + this IServiceCollection services, Action configure) + { + // Set by dev/generate_openapi_files.ps1 (and the CI spec-generation job) while running the CLI generator. + var generatingOpenApi = string.Equals( + Environment.GetEnvironmentVariable("swaggerGen"), "true", StringComparison.OrdinalIgnoreCase); + if (!generatingOpenApi) + { + return services; + } + + // Register the composing data source once, on the first feature. Its factory runs only when ApiExplorer + // resolves EndpointDataSource — by then every feature has registered its delegate, so the single instance + // sees them all. The plain AddSingleton appends, so it wins over any framework-default EndpointDataSource. + var firstFeature = services.All(d => d.ServiceType != typeof(OpenApiEndpointRouteConfiguration)); + services.AddSingleton(new OpenApiEndpointRouteConfiguration(configure)); + if (firstFeature) + { + services.AddSingleton(sp => new StandaloneEndpointDataSource( + sp, sp.GetServices().Select(c => c.Configure))); + } + + return services; + } + + /// + /// Wraps a feature's endpoint-mapping delegate so the set of features can be enumerated from DI without + /// registering a bare , which would risk colliding with unrelated delegate registrations. + /// + private sealed class OpenApiEndpointRouteConfiguration(Action configure) + { + public Action Configure { get; } = configure; + } +} diff --git a/src/HttpExtensions/StandaloneEndpointDataSource.cs b/src/HttpExtensions/StandaloneEndpointDataSource.cs new file mode 100644 index 000000000000..e0f37e6706cb --- /dev/null +++ b/src/HttpExtensions/StandaloneEndpointDataSource.cs @@ -0,0 +1,52 @@ +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Routing; +using Microsoft.Extensions.Primitives; + +namespace Bit.HttpExtensions; + +/// +/// An that materializes Minimal API endpoints from one or more route-mapping +/// delegates outside the request pipeline. +/// +/// Hosts that use the Startup/Configure pattern map their Minimal API endpoints with UseEndpoints. +/// The offline OpenAPI generator (dotnet swagger tofile) never executes Configure, so those +/// endpoints are invisible to ApiExplorer/Swashbuckle and the generated spec omits them. Registering this source +/// in DI makes the same endpoints discoverable without the request pipeline, while UseEndpoints still picks +/// them up at runtime. +/// +/// A single instance composes every feature's mapping delegate: ApiExplorer injects one +/// , so a source-per-feature would let the last-registered hide the rest. See +/// . +/// +public sealed class StandaloneEndpointDataSource : EndpointDataSource +{ + private readonly EndpointDataSource _source; + + public StandaloneEndpointDataSource( + IServiceProvider serviceProvider, IEnumerable> configure) + { + var routeBuilder = new StandaloneEndpointRouteBuilder(serviceProvider); + foreach (var map in configure) + { + map(routeBuilder); + } + + _source = new CompositeEndpointDataSource(routeBuilder.DataSources); + } + + public override IReadOnlyList Endpoints => _source.Endpoints; + + public override IChangeToken GetChangeToken() => _source.GetChangeToken(); + + /// + /// Minimal used only to materialize route groups into endpoints outside + /// of the request pipeline. + /// + private sealed class StandaloneEndpointRouteBuilder(IServiceProvider serviceProvider) : IEndpointRouteBuilder + { + public IServiceProvider ServiceProvider { get; } = serviceProvider; + public ICollection DataSources { get; } = new List(); + public IApplicationBuilder CreateApplicationBuilder() => new ApplicationBuilder(ServiceProvider); + } +} diff --git a/src/SharedWeb/Swagger/ActionNameOperationFilter.cs b/src/SharedWeb/Swagger/ActionNameOperationFilter.cs index 23602ca49582..1cedcc01bb83 100644 --- a/src/SharedWeb/Swagger/ActionNameOperationFilter.cs +++ b/src/SharedWeb/Swagger/ActionNameOperationFilter.cs @@ -1,4 +1,6 @@ using System.Text.Json; +using Microsoft.AspNetCore.Mvc.ApiExplorer; +using Microsoft.AspNetCore.Routing; using Microsoft.OpenApi; using Swashbuckle.AspNetCore.SwaggerGen; @@ -14,7 +16,7 @@ public class ActionNameOperationFilter : IOperationFilter { public void Apply(OpenApiOperation operation, OperationFilterContext context) { - if (!context.ApiDescription.ActionDescriptor.RouteValues.TryGetValue("action", out var action)) return; + var action = GetActionName(context.ApiDescription); if (string.IsNullOrEmpty(action)) return; operation.Extensions ??= new Dictionary(); @@ -22,4 +24,29 @@ public void Apply(OpenApiOperation operation, OperationFilterContext context) // We can't do case changes in the codegen templates, so we also add the snake_case version of the action name operation.Extensions.Add("x-action-name-snake-case", new JsonNodeExtension(JsonNamingPolicy.SnakeCaseLower.ConvertName(action))); } + + /// + /// Resolves the action name for an operation. MVC controllers expose it as the "action" route value. + /// Minimal API endpoints have none, so we derive it from the endpoint name set via .WithName(...) + /// (e.g. "Pam_AccessRequests_GetInbox" → "GetInbox"). + /// + private static string? GetActionName(ApiDescription apiDescription) + { + if (apiDescription.ActionDescriptor.RouteValues.TryGetValue("action", out var action) + && !string.IsNullOrEmpty(action)) + { + return action; + } + + var endpointName = apiDescription.ActionDescriptor.EndpointMetadata + .OfType() + .LastOrDefault()?.EndpointName; + if (string.IsNullOrEmpty(endpointName)) + { + return null; + } + + var lastSeparator = endpointName.LastIndexOf('_'); + return lastSeparator >= 0 ? endpointName[(lastSeparator + 1)..] : endpointName; + } } diff --git a/src/SharedWeb/Swagger/SwaggerGenOptionsExt.cs b/src/SharedWeb/Swagger/SwaggerGenOptionsExt.cs index 71dc52c28106..930d48b80f14 100644 --- a/src/SharedWeb/Swagger/SwaggerGenOptionsExt.cs +++ b/src/SharedWeb/Swagger/SwaggerGenOptionsExt.cs @@ -1,4 +1,6 @@ using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Mvc.ApiExplorer; +using Microsoft.AspNetCore.Routing; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; using Swashbuckle.AspNetCore.SwaggerGen; @@ -22,7 +24,7 @@ public static void InitializeSwaggerFilters( // Set the operation ID to the name of the controller followed by the name of the function. // Note that the "Controller" suffix for the controllers, and the "Async" suffix for the actions // are removed already, so we don't need to do that ourselves. - config.CustomOperationIds(e => $"{e.ActionDescriptor.RouteValues["controller"]}_{e.ActionDescriptor.RouteValues["action"]}"); + config.CustomOperationIds(BuildOperationId); // Because we're setting custom operation IDs, we need to ensure that we don't accidentally // introduce duplicate IDs, which is against the OpenAPI specification and could lead to issues. config.DocumentFilter(); @@ -34,4 +36,42 @@ public static void InitializeSwaggerFilters( config.OperationFilter(); } } + + /// + /// Builds the operation ID for an endpoint. MVC controllers produce "{controller}_{action}". + /// Minimal API endpoints carry no controller/action route values, so we fall back to the endpoint + /// name assigned via .WithName(...), and finally to a deterministic id derived from the route. + /// + /// + /// Never returns null or empty. Swashbuckle writes the selector's return value straight onto + /// operation.OperationId — it does not substitute a default when a custom selector is set — so a + /// null/empty id would collapse distinct endpoints onto the same id, which + /// rejects, aborting offline spec generation. + /// + public static string BuildOperationId(ApiDescription apiDescription) + { + apiDescription.ActionDescriptor.RouteValues.TryGetValue("controller", out var controller); + apiDescription.ActionDescriptor.RouteValues.TryGetValue("action", out var action); + if (!string.IsNullOrEmpty(controller) && !string.IsNullOrEmpty(action)) + { + return $"{controller}_{action}"; + } + + var endpointName = apiDescription.ActionDescriptor.EndpointMetadata + .OfType() + .LastOrDefault()?.EndpointName; + if (!string.IsNullOrEmpty(endpointName)) + { + return endpointName; + } + + // Last resort for a Minimal API endpoint mapped without .WithName(): derive a stable id from the + // HTTP method and route template. {method}+{path} is OpenAPI's uniqueness key, so this stays unique + // per operation and keeps the generated spec valid. HttpMethod defaults to "ANY", so the result is + // never empty after sanitizing non-identifier characters. + var method = apiDescription.HttpMethod ?? "ANY"; + var path = apiDescription.RelativePath ?? string.Empty; + var sanitized = new string($"{method}_{path}".Select(c => char.IsLetterOrDigit(c) ? c : '_').ToArray()); + return sanitized.Trim('_'); + } } diff --git a/test/HttpExtensions.Test/EndpointDataSourceServiceCollectionExtensionsTests.cs b/test/HttpExtensions.Test/EndpointDataSourceServiceCollectionExtensionsTests.cs new file mode 100644 index 000000000000..aeb612c47f32 --- /dev/null +++ b/test/HttpExtensions.Test/EndpointDataSourceServiceCollectionExtensionsTests.cs @@ -0,0 +1,59 @@ +using Microsoft.AspNetCore.Routing; +using Microsoft.Extensions.DependencyInjection; +using Xunit; + +namespace Bit.HttpExtensions.Test; + +public class EndpointDataSourceServiceCollectionExtensionsTests +{ + private const string SwaggerGenEnvVar = "swaggerGen"; + + [Fact] + public void AddOpenApiEndpointDataSource_WhenNotGenerating_IsNoOp() + { + using var _ = WithSwaggerGen(null); + var services = new ServiceCollection(); + + services.AddOpenApiEndpointDataSource(b => b.DataSources.Add(new StubEndpointDataSource(EndpointTestData.Make("A")))); + + Assert.DoesNotContain(services, d => d.ServiceType == typeof(EndpointDataSource)); + } + + [Fact] + public void AddOpenApiEndpointDataSource_WhenGenerating_RegistersSingleSourceComposingAllFeatures() + { + using var _ = WithSwaggerGen("true"); + var services = new ServiceCollection(); + + services.AddOpenApiEndpointDataSource(b => b.DataSources.Add(new StubEndpointDataSource(EndpointTestData.Make("A")))); + services.AddOpenApiEndpointDataSource(b => b.DataSources.Add(new StubEndpointDataSource(EndpointTestData.Make("B")))); + + // A single EndpointDataSource is registered regardless of how many features register, otherwise + // ApiExplorer (which injects only one) would surface just the last feature's endpoints. + Assert.Single(services, d => d.ServiceType == typeof(EndpointDataSource)); + + var provider = services.BuildServiceProvider(); + var source = provider.GetRequiredService(); + + Assert.Equal(2, source.Endpoints.Count); + Assert.Contains(source.Endpoints, e => e.DisplayName == "A"); + Assert.Contains(source.Endpoints, e => e.DisplayName == "B"); + } + + /// + /// Temporarily overrides the swaggerGen environment variable the extension gates on, restoring the + /// previous value on dispose. Tests in this class run sequentially (single xUnit collection), so the + /// process-wide variable does not race between them. + /// + private static IDisposable WithSwaggerGen(string? value) + { + var previous = Environment.GetEnvironmentVariable(SwaggerGenEnvVar); + Environment.SetEnvironmentVariable(SwaggerGenEnvVar, value); + return new EnvironmentVariableScope(SwaggerGenEnvVar, previous); + } + + private sealed class EnvironmentVariableScope(string name, string? previousValue) : IDisposable + { + public void Dispose() => Environment.SetEnvironmentVariable(name, previousValue); + } +} diff --git a/test/HttpExtensions.Test/StandaloneEndpointDataSourceTests.cs b/test/HttpExtensions.Test/StandaloneEndpointDataSourceTests.cs new file mode 100644 index 000000000000..7f4ebfe6dc92 --- /dev/null +++ b/test/HttpExtensions.Test/StandaloneEndpointDataSourceTests.cs @@ -0,0 +1,80 @@ +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Routing; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Primitives; +using Xunit; + +namespace Bit.HttpExtensions.Test; + +public class StandaloneEndpointDataSourceTests +{ + [Fact] + public void Endpoints_ComposesEveryMappingDelegate() + { + var serviceProvider = new ServiceCollection().BuildServiceProvider(); + Action mapA = b => b.DataSources.Add(new StubEndpointDataSource(EndpointTestData.Make("A"))); + Action mapB = b => b.DataSources.Add(new StubEndpointDataSource(EndpointTestData.Make("B"))); + + var source = new StandaloneEndpointDataSource(serviceProvider, [mapA, mapB]); + + Assert.Equal(2, source.Endpoints.Count); + Assert.Contains(source.Endpoints, e => e.DisplayName == "A"); + Assert.Contains(source.Endpoints, e => e.DisplayName == "B"); + } + + [Fact] + public void Endpoints_NoDelegates_IsEmpty() + { + var serviceProvider = new ServiceCollection().BuildServiceProvider(); + + var source = new StandaloneEndpointDataSource(serviceProvider, []); + + Assert.Empty(source.Endpoints); + } + + [Fact] + public void GetChangeToken_ReturnsNonNullToken() + { + var serviceProvider = new ServiceCollection().BuildServiceProvider(); + + var source = new StandaloneEndpointDataSource(serviceProvider, []); + + Assert.NotNull(source.GetChangeToken()); + } + + [Fact] + public void Endpoints_MaterializesMappedMinimalApiEndpoints() + { + // End-to-end against real Minimal API mapping (not just stub data sources): the delegate maps a + // route, and StandaloneEndpointDataSource must surface it as a RouteEndpoint outside the request pipeline. + var serviceProvider = new ServiceCollection() + .AddLogging() + .AddRouting() + .BuildServiceProvider(); + Action map = b => b.MapGet("widgets/{id}", (string id) => id).WithName("Widgets_Get"); + + var source = new StandaloneEndpointDataSource(serviceProvider, [map]); + + var endpoint = Assert.Single(source.Endpoints); + var routeEndpoint = Assert.IsType(endpoint); + Assert.Equal("widgets/{id}", routeEndpoint.RoutePattern.RawText); + } +} + +/// +/// Minimal exposing a fixed set of endpoints, used to drive the composition +/// logic of without the Minimal API request-delegate machinery. +/// +internal sealed class StubEndpointDataSource(params Endpoint[] endpoints) : EndpointDataSource +{ + public override IReadOnlyList Endpoints { get; } = endpoints; + + public override IChangeToken GetChangeToken() => new CancellationChangeToken(CancellationToken.None); +} + +internal static class EndpointTestData +{ + public static Endpoint Make(string displayName) => + new(_ => Task.CompletedTask, new EndpointMetadataCollection(), displayName); +} diff --git a/test/SharedWeb.Test/ActionNameOperationFilterTest.cs b/test/SharedWeb.Test/ActionNameOperationFilterTest.cs index 89ab7e327aa0..03515c75495f 100644 --- a/test/SharedWeb.Test/ActionNameOperationFilterTest.cs +++ b/test/SharedWeb.Test/ActionNameOperationFilterTest.cs @@ -1,6 +1,7 @@ using Bit.SharedWeb.Swagger; using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.ApiExplorer; +using Microsoft.AspNetCore.Routing; using Microsoft.OpenApi; using Swashbuckle.AspNetCore.SwaggerGen; @@ -69,4 +70,65 @@ public void WithMissingActionRouteValueDoesNotAddExtensions() Assert.False(operation.Extensions.ContainsKey("x-action-name")); Assert.False(operation.Extensions.ContainsKey("x-action-name-snake-case")); } + + [Fact] + public void WithMinimalApiEndpointNameUsesSegmentAfterLastUnderscore() + { + // Minimal API endpoints have no "action" route value; the action is derived from the endpoint name + // set via .WithName(...), taking the segment after the last underscore. + var actionDescriptor = new ActionDescriptor + { + EndpointMetadata = new List { new EndpointNameMetadata("Pam_AccessRequests_GetInbox") } + }; + + var operation = ApplyFilter(actionDescriptor); + + Assert.Equal("GetInbox", (operation.Extensions["x-action-name"] as JsonNodeExtension)!.Node.ToString()); + Assert.Equal("get_inbox", (operation.Extensions["x-action-name-snake-case"] as JsonNodeExtension)!.Node.ToString()); + } + + [Fact] + public void WithMinimalApiEndpointNameWithoutUnderscoreUsesWholeName() + { + var actionDescriptor = new ActionDescriptor + { + EndpointMetadata = new List { new EndpointNameMetadata("GetInbox") } + }; + + var operation = ApplyFilter(actionDescriptor); + + Assert.Equal("GetInbox", (operation.Extensions["x-action-name"] as JsonNodeExtension)!.Node.ToString()); + } + + [Fact] + public void WithBothActionRouteValueAndEndpointNamePrefersRouteValue() + { + // A controller-style "action" route value takes precedence over the Minimal API endpoint name. + var actionDescriptor = new ActionDescriptor + { + EndpointMetadata = new List { new EndpointNameMetadata("Pam_AccessRequests_GetInbox") } + }; + actionDescriptor.RouteValues["action"] = "GetUsers"; + + var operation = ApplyFilter(actionDescriptor); + + Assert.Equal("GetUsers", (operation.Extensions["x-action-name"] as JsonNodeExtension)!.Node.ToString()); + } + + private static OpenApiOperation ApplyFilter(ActionDescriptor actionDescriptor) + { + var operation = new OpenApiOperation + { + Extensions = new Dictionary() + }; + var apiDescription = new ApiDescription + { + ActionDescriptor = actionDescriptor + }; + var context = new OperationFilterContext(apiDescription, null, null, null, null); + + new ActionNameOperationFilter().Apply(operation, context); + + return operation; + } } diff --git a/test/SharedWeb.Test/SwaggerDocUtil.cs b/test/SharedWeb.Test/SwaggerDocUtil.cs index 67779ac44764..dbfa49b5adde 100644 --- a/test/SharedWeb.Test/SwaggerDocUtil.cs +++ b/test/SharedWeb.Test/SwaggerDocUtil.cs @@ -1,4 +1,5 @@ using System.Reflection; +using Bit.SharedWeb.Swagger; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.ApiExplorer; @@ -47,7 +48,7 @@ public static (OpenApiDocument, DocumentFilterContext) CreateDocFromControllers( services.AddSwaggerGen(config => { config.SwaggerDoc("v1", new OpenApiInfo { Title = "Test API", Version = "v1" }); - config.CustomOperationIds(e => $"{e.ActionDescriptor.RouteValues["controller"]}_{e.ActionDescriptor.RouteValues["action"]}"); + config.CustomOperationIds(SwaggerGenOptionsExt.BuildOperationId); }); var serviceProvider = services.BuildServiceProvider(); diff --git a/test/SharedWeb.Test/SwaggerGenOptionsExtTest.cs b/test/SharedWeb.Test/SwaggerGenOptionsExtTest.cs new file mode 100644 index 000000000000..48a0536fb0c1 --- /dev/null +++ b/test/SharedWeb.Test/SwaggerGenOptionsExtTest.cs @@ -0,0 +1,87 @@ +using Bit.SharedWeb.Swagger; +using Microsoft.AspNetCore.Mvc.Abstractions; +using Microsoft.AspNetCore.Mvc.ApiExplorer; +using Microsoft.AspNetCore.Routing; + +namespace SharedWeb.Test; + +public class SwaggerGenOptionsExtTest +{ + [Fact] + public void BuildOperationId_Controller_UsesControllerAndAction() + { + var apiDescription = new ApiDescription + { + ActionDescriptor = new ActionDescriptor + { + RouteValues = new Dictionary + { + ["controller"] = "AccessRequests", + ["action"] = "GetInbox", + }, + EndpointMetadata = new List(), + }, + }; + + Assert.Equal("AccessRequests_GetInbox", SwaggerGenOptionsExt.BuildOperationId(apiDescription)); + } + + [Fact] + public void BuildOperationId_MinimalApi_FallsBackToEndpointName() + { + // Minimal API endpoints carry no controller/action route values, only the name set via .WithName(...). + var apiDescription = new ApiDescription + { + ActionDescriptor = new ActionDescriptor + { + RouteValues = new Dictionary(), + EndpointMetadata = new List { new EndpointNameMetadata("Pam_Leases_GetActive") }, + }, + }; + + Assert.Equal("Pam_Leases_GetActive", SwaggerGenOptionsExt.BuildOperationId(apiDescription)); + } + + [Fact] + public void BuildOperationId_NoRouteValuesOrEndpointName_FallsBackToRouteAndMethod() + { + // A Minimal API endpoint mapped without .WithName() has neither route values nor an endpoint name. + // BuildOperationId must still return a stable, non-empty id: Swashbuckle writes it straight onto + // operation.OperationId, and a null/empty id collapses distinct endpoints together, which + // CheckDuplicateOperationIdsDocumentFilter rejects, aborting spec generation. + var apiDescription = new ApiDescription + { + HttpMethod = "GET", + RelativePath = "api/leases/active", + ActionDescriptor = new ActionDescriptor + { + RouteValues = new Dictionary(), + EndpointMetadata = new List(), + }, + }; + + Assert.Equal("GET_api_leases_active", SwaggerGenOptionsExt.BuildOperationId(apiDescription)); + } + + [Fact] + public void BuildOperationId_DistinctUnnamedEndpoints_ProduceDistinctNonEmptyIds() + { + static ApiDescription Unnamed(string method, string path) => new() + { + HttpMethod = method, + RelativePath = path, + ActionDescriptor = new ActionDescriptor + { + RouteValues = new Dictionary(), + EndpointMetadata = new List(), + }, + }; + + var first = SwaggerGenOptionsExt.BuildOperationId(Unnamed("GET", "api/widgets")); + var second = SwaggerGenOptionsExt.BuildOperationId(Unnamed("POST", "api/widgets")); + + Assert.False(string.IsNullOrEmpty(first)); + Assert.False(string.IsNullOrEmpty(second)); + Assert.NotEqual(first, second); + } +}