From 6bbd5208f18e0823421dc45dac3a432d942be9f7 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Tue, 25 Jan 2022 09:53:52 -0800 Subject: [PATCH] Removing from API Description parameters when inferred (FromPath) and not in the route (#39607) * Changing from Path to ModelBinding when inferred * Removing whitespace * Removing the inferred paraemter when not in the route * Avoiding extra allocation * Remove extra line * Updating comment * Updating test to use BindingSource.Path * Remove extra line * Update src/Mvc/Mvc.ApiExplorer/src/DefaultApiDescriptionProvider.cs Co-authored-by: Stephen Halter Co-authored-by: Stephen Halter --- .../src/DefaultApiDescriptionProvider.cs | 24 +++++++-- .../test/DefaultApiDescriptionProviderTest.cs | 51 +++++++++++++++++++ 2 files changed, 72 insertions(+), 3 deletions(-) diff --git a/src/Mvc/Mvc.ApiExplorer/src/DefaultApiDescriptionProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/DefaultApiDescriptionProvider.cs index 2096d88decb9..879c425f183e 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/DefaultApiDescriptionProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/DefaultApiDescriptionProvider.cs @@ -2,12 +2,14 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Linq; +using Microsoft.AspNetCore.Http.Metadata; using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.ActionConstraints; using Microsoft.AspNetCore.Mvc.Controllers; using Microsoft.AspNetCore.Mvc.Formatters; using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.AspNetCore.Mvc.ModelBinding; +using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata; using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.Routing.Template; using Microsoft.Extensions.Options; @@ -239,11 +241,13 @@ private void ProcessRouteParameters(ApiParameterContext context) routeParameters.Add(routeParameter.Name!, CreateRouteInfo(routeParameter)); } - foreach (var parameter in context.Results) + for (var i = context.Results.Count - 1; i >= 0; i--) { + var parameter = context.Results[i]; + if (parameter.Source == BindingSource.Path || - parameter.Source == BindingSource.ModelBinding || - parameter.Source == BindingSource.Custom) + parameter.Source == BindingSource.ModelBinding || + parameter.Source == BindingSource.Custom) { if (routeParameters.TryGetValue(parameter.Name, out var routeInfo)) { @@ -258,6 +262,20 @@ private void ProcessRouteParameters(ApiParameterContext context) parameter.Source = BindingSource.Path; } } + else + { + if (parameter.Source == BindingSource.Path && + parameter.ModelMetadata is DefaultModelMetadata defaultModelMetadata && + !defaultModelMetadata.Attributes.Attributes.OfType().Any()) + { + // If we didn't see the parameter in the route and no FromRoute metadata is set, it probably means + // the parameter binding source was inferred (InferParameterBindingInfoConvention) + // probably because another route to this action contains it as route parameter and + // will be removed from the API description + // https://github.com/dotnet/aspnetcore/issues/26234 + context.Results.RemoveAt(i); + } + } } } diff --git a/src/Mvc/Mvc.ApiExplorer/test/DefaultApiDescriptionProviderTest.cs b/src/Mvc/Mvc.ApiExplorer/test/DefaultApiDescriptionProviderTest.cs index 124a66f60074..d080ae410817 100644 --- a/src/Mvc/Mvc.ApiExplorer/test/DefaultApiDescriptionProviderTest.cs +++ b/src/Mvc/Mvc.ApiExplorer/test/DefaultApiDescriptionProviderTest.cs @@ -243,6 +243,57 @@ public void GetApiDescription_PopulatesParametersThatAppearOnRouteTemplate_AndHa } } + [Fact] + public void GetApiDescription_WithInferredBindingSource_ExcludesPathParametersWhenNotPresentInRoute() + { + // Arrange + var action = CreateActionDescriptor(nameof(FromModelBinding)); + action.AttributeRouteInfo = new AttributeRouteInfo { Template = "api/products" }; + + action.Parameters[0].BindingInfo = new BindingInfo() + { + BindingSource = BindingSource.Path + }; + + // Act + var descriptions = GetApiDescriptions(action); + + // Assert + var description = Assert.Single(descriptions); + Assert.Empty(description.ParameterDescriptions); + } + + [Theory] + [InlineData("api/products/{id}")] + [InlineData("api/products/{id?}")] + [InlineData("api/products/{id=5}")] + [InlineData("api/products/{id:int}")] + [InlineData("api/products/{id:int?}")] + [InlineData("api/products/{id:int=5}")] + [InlineData("api/products/{*id}")] + [InlineData("api/products/{*id:int}")] + [InlineData("api/products/{*id:int=5}")] + public void GetApiDescription_WithInferredBindingSource_IncludesPathParametersWhenPresentInRoute(string template) + { + // Arrange + var action = CreateActionDescriptor(nameof(FromModelBinding)); + action.AttributeRouteInfo = new AttributeRouteInfo { Template = template }; + + action.Parameters[0].BindingInfo = new BindingInfo() + { + BindingSource = BindingSource.Path + }; + + // Act + var descriptions = GetApiDescriptions(action); + + // Assert + var description = Assert.Single(descriptions); + var parameter = Assert.Single(description.ParameterDescriptions); + Assert.Equal(BindingSource.Path, parameter.Source); + Assert.Equal("id", parameter.Name); + } + [Fact] public void GetApiDescription_ParameterDescription_IncludesParameterDescriptor() {