Fix the code to ignore parameters from incorrect locations (#42878)

* Fixing issue #42483

* update according to feedback

* update according to feedback 2

* update after receiving feedback

* fix a small error in the tests

src/OpenApi/src/OpenApiGenerator.cs

@@ -380,16 +380,14 @@ internal sealed class OpenApiGenerator
                 throw new InvalidOperationException($"Encountered a parameter of type '{parameter.ParameterType}' without a name. Parameters must have a name.");
             }
 
-            var (isBodyOrFormParameter, parameterLocation) = GetOpenApiParameterLocation(parameter, pattern, disableInferredBody);
+            var (_, parameterLocation) = GetOpenApiParameterLocation(parameter, pattern, disableInferredBody);
 
-            // If the parameter isn't something that would be populated in RequestBody
-            // or doesn't have a valid ParameterLocation, then it must be a service
-            // parameter that we can ignore.
-            if (!isBodyOrFormParameter && parameterLocation is null)
+            // if the parameter doesn't have a valid location
+            // then we should ignore it
+            if (parameterLocation is null)
             {
                 continue;
             }
-
             var nullabilityContext = new NullabilityInfoContext();
             var nullability = nullabilityContext.Create(parameter);
             var isOptional = parameter.HasDefaultValue || nullability.ReadState != NullabilityState.NotNull;

src/OpenApi/test/OpenApiGeneratorTests.cs

@@ -83,9 +83,9 @@ public class OpenApiOperationGeneratorTests
     {
         static void AssertCustomRequestFormat(OpenApiOperation operation)
         {
-            var request = Assert.Single(operation.Parameters);
-            var content = Assert.Single(request.Content);
-            Assert.Equal("application/custom", content.Key);
+            Assert.Empty(operation.Parameters);
+            var content = operation.RequestBody.Content.Keys.FirstOrDefault();
+            Assert.Equal("application/custom", content);
         }
 
         AssertCustomRequestFormat(GetOpenApiOperation(
@@ -101,10 +101,11 @@ public class OpenApiOperationGeneratorTests
         var operation = GetOpenApiOperation(
             [Consumes("application/custom0", "application/custom1")] (InferredJsonClass fromBody) => { });
 
-        var request = Assert.Single(operation.Parameters);
+        Assert.Empty(operation.Parameters);
 
-        Assert.Equal(2, request.Content.Count);
-        Assert.Equal(new[] { "application/custom0", "application/custom1" }, request.Content.Keys);
+        var content = operation.RequestBody.Content;
+        Assert.Equal(2, content.Count);
+        Assert.Equal(new[] { "application/custom0", "application/custom1" }, content.Keys);
     }
 
     [Fact]
@@ -114,8 +115,8 @@ public class OpenApiOperationGeneratorTests
             [Consumes(typeof(InferredJsonClass), "application/custom0", "application/custom1", IsOptional = true)] () => { });
         var request = operation.RequestBody;
         Assert.NotNull(request);
-
         Assert.Equal(2, request.Content.Count);
+        Assert.Empty(operation.Parameters);
 
         Assert.Equal("application/custom0", request.Content.First().Key);
         Assert.Equal("application/custom1", request.Content.Last().Key);
@@ -136,6 +137,7 @@ public class OpenApiOperationGeneratorTests
         Assert.Equal("application/custom0", request.Content.First().Key);
         Assert.Equal("application/custom1", request.Content.Last().Key);
         Assert.True(request.Required);
+        Assert.Empty(operation.Parameters);
     }
 
 #nullable disable
@@ -187,8 +189,8 @@ public class OpenApiOperationGeneratorTests
     {
         var operation = GetOpenApiOperation(
             [ProducesResponseType(typeof(TimeSpan), StatusCodes.Status201Created)]
-            [ProducesResponseType(StatusCodes.Status400BadRequest)]
-            () => new InferredJsonClass());
+        [ProducesResponseType(StatusCodes.Status400BadRequest)]
+        () => new InferredJsonClass());
 
         var responses = operation.Responses;
 
@@ -212,8 +214,8 @@ public class OpenApiOperationGeneratorTests
     {
         var operation = GetOpenApiOperation(
             [ProducesResponseType(typeof(InferredJsonClass), StatusCodes.Status201Created)]
-            [ProducesResponseType(StatusCodes.Status400BadRequest)]
-            () => Results.Ok(new InferredJsonClass()));
+        [ProducesResponseType(StatusCodes.Status400BadRequest)]
+        () => Results.Ok(new InferredJsonClass()));
 
         Assert.Equal(2, operation.Responses.Count);
 
@@ -409,6 +411,7 @@ public class OpenApiOperationGeneratorTests
             var requestBody = operation.RequestBody;
             var content = Assert.Single(requestBody.Content);
             Assert.Equal("application/json", content.Key);
+            Assert.Empty(operation.Parameters);
         }
 
         AssertBodyParameter(GetOpenApiOperation((InferredJsonClass foo) => { }), "foo", "object");
@@ -421,7 +424,7 @@ public class OpenApiOperationGeneratorTests
     public void AddsMultipleParameters()
     {
         var operation = GetOpenApiOperation(([FromRoute] int foo, int bar, InferredJsonClass fromBody) => { });
-        Assert.Equal(3, operation.Parameters.Count);
+        Assert.Equal(2, operation.Parameters.Count);
 
         var fooParam = operation.Parameters[0];
         Assert.Equal("foo", fooParam.Name);
@@ -458,14 +461,6 @@ public class OpenApiOperationGeneratorTests
                     Assert.Equal("Bar", param.Name);
                     Assert.Equal(ParameterLocation.Query, param.In);
                     Assert.True(param.Required);
-                },
-                param =>
-                {
-                    Assert.Equal("FromBody", param.Name);
-                    var fromBodyParam = operation.RequestBody;
-                    var fromBodyContent = Assert.Single(fromBodyParam.Content);
-                    Assert.Equal("application/json", fromBodyContent.Key);
-                    Assert.False(fromBodyParam.Required);
                 }
             );
         }
@@ -645,6 +640,7 @@ public class OpenApiOperationGeneratorTests
         var requestBody = operation.RequestBody;
 
         // Assert
+        Assert.Empty(operation.Parameters);
         Assert.Collection(
             requestBody.Content,
             parameter =>
@@ -671,6 +667,7 @@ public class OpenApiOperationGeneratorTests
         var requestBody = operation.RequestBody;
         var content = Assert.Single(requestBody.Content);
         Assert.False(requestBody.Required);
+        Assert.Empty(operation.Parameters);
     }
 
 #nullable enable
@@ -686,6 +683,7 @@ public class OpenApiOperationGeneratorTests
         var content = Assert.Single(requestBody.Content);
         Assert.Equal("application/json", content.Key);
         Assert.True(requestBody.Required);
+        Assert.Empty(operation.Parameters);
     }
 
     [Fact]
@@ -699,6 +697,7 @@ public class OpenApiOperationGeneratorTests
         var content = Assert.Single(requestBody.Content);
         Assert.Equal("application/json", content.Key);
         Assert.False(requestBody.Required);
+        Assert.Empty(operation.Parameters);
     }
 
     [Fact]
@@ -712,6 +711,7 @@ public class OpenApiOperationGeneratorTests
         var content = Assert.Single(requestBody.Content);
         Assert.Equal("application/xml", content.Key);
         Assert.False(requestBody.Required);
+        Assert.Empty(operation.Parameters);
     }
 
     [Fact]
@@ -725,6 +725,7 @@ public class OpenApiOperationGeneratorTests
         var content = Assert.Single(requestBody.Content);
         Assert.Equal("multipart/form-data", content.Key);
         Assert.True(requestBody.Required);
+        Assert.Empty(operation.Parameters);
     }
 
     [Fact]
@@ -738,6 +739,7 @@ public class OpenApiOperationGeneratorTests
         var content = Assert.Single(requestBody.Content);
         Assert.Equal("multipart/form-data", content.Key);
         Assert.False(requestBody.Required);
+        Assert.Empty(operation.Parameters);
     }
 
     [Fact]
@@ -751,6 +753,7 @@ public class OpenApiOperationGeneratorTests
         var content = Assert.Single(requestBody.Content);
         Assert.Equal("multipart/form-data", content.Key);
         Assert.True(requestBody.Required);
+        Assert.Empty(operation.Parameters);
     }
 
     [Fact]
@@ -769,6 +772,8 @@ public class OpenApiOperationGeneratorTests
 
         var requestFormat1 = content["application/custom1"];
         Assert.NotNull(requestFormat1);
+
+        Assert.Empty(operation.Parameters);
     }
 
     [Fact]
@@ -798,6 +803,7 @@ public class OpenApiOperationGeneratorTests
             var requestBody = operation.RequestBody;
             var content = Assert.Single(requestBody.Content);
             Assert.Equal("multipart/form-data", content.Key);
+            Assert.Empty(operation.Parameters);
         }
 
         AssertFormFileParameter(GetOpenApiOperation((IFormFile file) => { }), "object", "file");
@@ -820,6 +826,7 @@ public class OpenApiOperationGeneratorTests
             var content = Assert.Single(requestBody.Content);
             Assert.Equal("multipart/form-data", content.Key);
             Assert.True(requestBody.Required);
+            Assert.Empty(operation.Parameters);
         }
     }
 
@@ -856,6 +863,14 @@ public class OpenApiOperationGeneratorTests
 
     }
 
+    [Fact]
+    public void OnlyAddParametersWithCorrectLocations()
+    {
+        var operation = GetOpenApiOperation(([FromBody] int fromBody, [FromRoute] int fromRoute, [FromServices] int fromServices) => { });
+
+        Assert.Single(operation.Parameters);
+    }
+
     [Theory]
     [InlineData("/todos/{id}", "id")]
     [InlineData("/todos/{Id}", "Id")]