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 <halter73@gmail.com> Co-authored-by: Stephen Halter <halter73@gmail.com>
dotnet · Jan 25, 2022 · 6bbd520 · 6bbd520
1 parent 209ff13
commit 6bbd520
Show file tree

Hide file tree

Showing 2 changed files with 72 additions and 3 deletions.
diff --git 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<IFromRouteMetadata>().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
@@ -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()
     {