Skip to content

Fix ApiExplorer to include FromQuery(Name) prefix in parameter names #68344

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.

Sign up for GitHub

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

Merged
Youssef1313 merged 3 commits into from
Aug 19, 2026
Merged

Fix ApiExplorer to include FromQuery(Name) prefix in parameter names #68344

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9223b65
Fix ApiExplorer to include FromQuery(Name) prefix in parameter names
snemeckayova Aug 11, 2026
60598ff
Add test coverage
snemeckayova Aug 12, 2026
0e53fbd
Fix header binding
snemeckayova Aug 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 11 additions & 6 deletions src/Mvc/Mvc.ApiExplorer/src/DefaultApiDescriptionProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using Microsoft.AspNetCore.Http.Metadata;
Expand Down Expand Up @@ -633,11 +634,11 @@ private void Visit(
// Order - source: Body
//

// We don't want to append the **parameter** name when building a model name.
// Append the property name when building a model name, or the parameter name when it was explicitly provided via IModelNameProvider.
var newContainerName = containerName;
if (modelMetadata.ContainerType != null)
if (modelMetadata.ContainerType != null || !string.IsNullOrEmpty(bindingContext.BinderModelName))
{
newContainerName = GetName(containerName, bindingContext);
newContainerName = GetName(containerName, bindingContext, source);
}

var metadataProperties = modelMetadata.Properties;
Expand Down Expand Up @@ -674,7 +675,7 @@ private ApiParameterDescription CreateResult(
return new ApiParameterDescription()
{
ModelMetadata = bindingContext.ModelMetadata,
Name = GetName(containerName, bindingContext),
Name = GetName(containerName, bindingContext, source),
Source = source,
Type = GetModelType(bindingContext.ModelMetadata),
ParameterDescriptor = Parameter,
Expand All @@ -693,10 +694,14 @@ private static Type GetModelType(ModelMetadata metadata)
return metadata.ModelType;
}

private static string GetName(string containerName, ApiParameterDescriptionContext metadata)
private static string GetName(string containerName, ApiParameterDescriptionContext metadata, BindingSource? source)
{
var propertyName = !string.IsNullOrEmpty(metadata.BinderModelName) ? metadata.BinderModelName : metadata.PropertyName;
return ModelNames.CreatePropertyModelName(containerName, propertyName);
Debug.Assert(propertyName is not null);

return source == BindingSource.Header
? propertyName
: ModelNames.CreatePropertyModelName(containerName, propertyName);
}

private readonly struct PropertyKey
Expand Down
37 changes: 36 additions & 1 deletion src/Mvc/Mvc.ApiExplorer/test/DefaultApiDescriptionProviderTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1624,11 +1624,33 @@ public void GetApiDescription_ParameterDescription_FromQueryEmployee()
var description = Assert.Single(descriptions);
Assert.Single(description.ParameterDescriptions);

var id = Assert.Single(description.ParameterDescriptions, p => p.Name == "Name");
var id = Assert.Single(description.ParameterDescriptions, p => p.Name == "employee.Name");

@Youssef1313 Youssef1313 Aug 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if Employee class had a property that had [FromHeader("X-MyCustomHeader")]?

Are we prefixing this as well? And is it expected to be prefixed? Same question applies to FromRoute

In addition, let's make sure we have equivalent tests for both Mvc and minimal API, including tests directed for OpenAPI

@Youssef1313 Youssef1313 Aug 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If no scenario here is applicable for minimal API (e.g, an analyzer warning is produced), feel free to skip the minimal API part here.

Assert.Same(BindingSource.Query, id.Source);
Assert.Equal(typeof(string), id.Type);
}

[Fact]
public void GetApiDescription_ParameterDescription_FromQueryEmployee_WithCustomPropertyBindingNames()
{
// Arrange
var action = CreateActionDescriptor(nameof(AcceptsEmployeeWithCustomPropertyNames));

// Act
var descriptions = GetApiDescriptions(action);

// Assert
var description = Assert.Single(descriptions);
Assert.Equal(2, description.ParameterDescriptions.Count);

var header = Assert.Single(description.ParameterDescriptions, p => p.Name == "X-MyCustomHeader");
Assert.Same(BindingSource.Header, header.Source);
Assert.Equal(typeof(string), header.Type);

var route = Assert.Single(description.ParameterDescriptions, p => p.Name == "employee.employeeid");
Assert.Same(BindingSource.Path, route.Source);
Assert.Equal(typeof(string), route.Type);
Comment on lines +1649 to +1651

@Youssef1313 Youssef1313 Aug 13, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question here, but Path is a bit special as it relates to the placeholder in the template.

Does this parameter bind correctly for MapGet(/api/{employeeid}, ...)? Or for {employee.employee.id}? Or none of them actually binds?

@snemeckayova snemeckayova Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{employee.employeeid} binds; {employeeid} and {employee.employee.id} don't

@Youssef1313 Youssef1313 Aug 18, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@snemeckayova Thanks for confirming. So only headers are special here?

@snemeckayova snemeckayova Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's correct

}

[Fact]
public void GetApiDescription_ParameterDescription_ParsablePrimitiveType()
{
Expand Down Expand Up @@ -2542,6 +2564,10 @@ private void AcceptsEmployee([FromQuery(Name = "employee")] Employee dto)
{
}

private void AcceptsEmployeeWithCustomPropertyNames([FromQuery(Name = "employee")] EmployeeWithCustomPropertyNames dto)
{
}

private void AcceptsTryParsablePrimitiveType([FromQuery] Guid id)
{
}
Expand Down Expand Up @@ -2695,6 +2721,15 @@ private class Employee
public string Name { get; set; }
}

private class EmployeeWithCustomPropertyNames
{
[FromHeader(Name = "X-MyCustomHeader")]
public string HeaderName { get; set; }

[FromRoute(Name = "employeeid")]
public string EmployeeId { get; set; }
}

[TypeConverter(typeof(EmployeeConverter))]
private class ConvertibleEmployee
{
Expand Down
Loading