Skip to content

Set EndpointName and DisplayName given method name in endpoints (#35439) #35527

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.

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
merged 1 commit into from
Aug 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using System.Runtime.CompilerServices;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Routing;
using Microsoft.AspNetCore.Routing.Patterns;
using Microsoft.CodeAnalysis.CSharp.Symbols;

namespace Microsoft.AspNetCore.Builder
{
Expand Down Expand Up @@ -107,7 +109,8 @@ public static MinimalActionEndpointConventionBuilder MapMethods(
}

var builder = endpoints.Map(RoutePatternFactory.Parse(pattern), action);
builder.WithDisplayName($"{pattern} HTTP: {string.Join(", ", httpMethods)}");
// Prepends the HTTP method to the DisplayName produced with pattern + method name
builder.Add(b => b.DisplayName = $"HTTP: {string.Join(", ", httpMethods)} {b.DisplayName}");
builder.WithMetadata(new HttpMethodMetadata(httpMethods));
return builder;
}
Expand Down Expand Up @@ -184,6 +187,19 @@ public static MinimalActionEndpointConventionBuilder Map(
// Add MethodInfo as metadata to assist with OpenAPI generation for the endpoint.
builder.Metadata.Add(action.Method);

// Methods defined in a top-level program are generated as statics so the delegate
// target will be null. Inline lambdas are compiler generated method so they can
// be filtered that way.
if (GeneratedNameParser.TryParseLocalFunctionName(action.Method.Name, out var endpointName)
|| !TypeHelper.IsCompilerGeneratedMethod(action.Method))
{
endpointName ??= action.Method.Name;

builder.Metadata.Add(new EndpointNameMetadata(endpointName));
builder.Metadata.Add(new RouteNameMetadata(endpointName));
builder.DisplayName = $"{builder.DisplayName} => {endpointName}";
}

// Add delegate attributes as metadata
var attributes = action.Method.GetCustomAttributes();

Expand Down
2 changes: 2 additions & 0 deletions src/Http/Routing/src/Microsoft.AspNetCore.Routing.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ Microsoft.AspNetCore.Routing.RouteCollection</Description>

<ItemGroup>
<Compile Include="$(SharedSourceRoot)PropertyHelper\*.cs" />
<Compile Include="$(SharedSourceRoot)RoslynUtils\TypeHelper.cs" />
<Compile Include="$(SharedSourceRoot)RoslynUtils\GeneratedNameParser.cs" />
</ItemGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ public void MapGet_BuildsEndpointWithCorrectMethod()
Assert.Equal("GET", method);

var routeEndpointBuilder = GetRouteEndpointBuilder(builder);
Assert.Equal("/ HTTP: GET", routeEndpointBuilder.DisplayName);
Assert.Equal("HTTP: GET /", routeEndpointBuilder.DisplayName);
Assert.Equal("/", routeEndpointBuilder.RoutePattern.RawText);
}

Expand All @@ -125,7 +125,7 @@ public async Task MapGetWithRouteParameter_BuildsEndpointWithRouteSpecificBindin
Assert.Equal("GET", method);

var routeEndpointBuilder = GetRouteEndpointBuilder(builder);
Assert.Equal("/{id} HTTP: GET", routeEndpointBuilder.DisplayName);
Assert.Equal("HTTP: GET /{id}", routeEndpointBuilder.DisplayName);
Assert.Equal("/{id}", routeEndpointBuilder.RoutePattern.RawText);

// Assert that we don't fallback to the query string
Expand Down Expand Up @@ -163,7 +163,7 @@ public async Task MapGetWithoutRouteParameter_BuildsEndpointWithQuerySpecificBin
Assert.Equal("GET", method);

var routeEndpointBuilder = GetRouteEndpointBuilder(builder);
Assert.Equal("/ HTTP: GET", routeEndpointBuilder.DisplayName);
Assert.Equal("HTTP: GET /", routeEndpointBuilder.DisplayName);
Assert.Equal("/", routeEndpointBuilder.RoutePattern.RawText);

// Assert that we don't fallback to the route values
Expand Down Expand Up @@ -205,7 +205,7 @@ public async Task MapVerbWithExplicitRouteParameterIsCaseInsensitive(Action<IEnd
Assert.Equal(expectedMethod, method);

var routeEndpointBuilder = GetRouteEndpointBuilder(builder);
Assert.Equal($"/{{ID}} HTTP: {expectedMethod}", routeEndpointBuilder.DisplayName);
Assert.Equal($"HTTP: {expectedMethod} /{{ID}}", routeEndpointBuilder.DisplayName);
Assert.Equal($"/{{ID}}", routeEndpointBuilder.RoutePattern.RawText);

var httpContext = new DefaultHttpContext();
Expand Down Expand Up @@ -241,7 +241,7 @@ public async Task MapVerbWithRouteParameterDoesNotFallbackToQuery(Action<IEndpoi
Assert.Equal(expectedMethod, method);

var routeEndpointBuilder = GetRouteEndpointBuilder(builder);
Assert.Equal($"/{{ID}} HTTP: {expectedMethod}", routeEndpointBuilder.DisplayName);
Assert.Equal($"HTTP: {expectedMethod} /{{ID}}", routeEndpointBuilder.DisplayName);
Assert.Equal($"/{{ID}}", routeEndpointBuilder.RoutePattern.RawText);

// Assert that we don't fallback to the query string
Expand Down Expand Up @@ -281,7 +281,7 @@ public void MapPost_BuildsEndpointWithCorrectMethod()
Assert.Equal("POST", method);

var routeEndpointBuilder = GetRouteEndpointBuilder(builder);
Assert.Equal("/ HTTP: POST", routeEndpointBuilder.DisplayName);
Assert.Equal("HTTP: POST /", routeEndpointBuilder.DisplayName);
Assert.Equal("/", routeEndpointBuilder.RoutePattern.RawText);
}

Expand All @@ -301,7 +301,7 @@ public void MapPut_BuildsEndpointWithCorrectMethod()
Assert.Equal("PUT", method);

var routeEndpointBuilder = GetRouteEndpointBuilder(builder);
Assert.Equal("/ HTTP: PUT", routeEndpointBuilder.DisplayName);
Assert.Equal("HTTP: PUT /", routeEndpointBuilder.DisplayName);
Assert.Equal("/", routeEndpointBuilder.RoutePattern.RawText);
}

Expand All @@ -321,7 +321,7 @@ public void MapDelete_BuildsEndpointWithCorrectMethod()
Assert.Equal("DELETE", method);

var routeEndpointBuilder = GetRouteEndpointBuilder(builder);
Assert.Equal("/ HTTP: DELETE", routeEndpointBuilder.DisplayName);
Assert.Equal("HTTP: DELETE /", routeEndpointBuilder.DisplayName);
Assert.Equal("/", routeEndpointBuilder.RoutePattern.RawText);
}

Expand Down Expand Up @@ -359,6 +359,106 @@ public void MapFallbackWithoutPath_BuildsEndpointWithLowestRouteOrder()
Assert.Equal(int.MaxValue, routeEndpointBuilder.Order);
}

[Fact]
// This test scenario simulates methods defined in a top-level program
// which are compiler generated. We currently do some manually parsing leveraging
// code in Roslyn to support this scenario. More info at https://github.com/dotnet/roslyn/issues/55651.
public void MapMethod_DoesNotEndpointNameForInnerMethod()
{
var name = "InnerGetString";
var builder = new DefaultEndpointRouteBuilder(new ApplicationBuilder(new EmptyServiceProvdier()));
string InnerGetString() => "TestString";
_ = builder.MapDelete("/", InnerGetString);

var dataSource = GetBuilderEndpointDataSource(builder);
// Trigger Endpoint build by calling getter.
var endpoint = Assert.Single(dataSource.Endpoints);

var endpointName = endpoint.Metadata.GetMetadata<IEndpointNameMetadata>();
var routeName = endpoint.Metadata.GetMetadata<IRouteNameMetadata>();
var routeEndpointBuilder = GetRouteEndpointBuilder(builder);
Assert.Equal(name, endpointName?.EndpointName);
Assert.Equal(name, routeName?.RouteName);
Assert.Equal("HTTP: DELETE / => InnerGetString", routeEndpointBuilder.DisplayName);
}

[Fact]
public void MapMethod_DoesNotEndpointNameForInnerMethodWithTarget()
{
var name = "InnerGetString";
var builder = new DefaultEndpointRouteBuilder(new ApplicationBuilder(new EmptyServiceProvdier()));
var testString = "TestString";
string InnerGetString() => testString;
_ = builder.MapDelete("/", InnerGetString);

var dataSource = GetBuilderEndpointDataSource(builder);
// Trigger Endpoint build by calling getter.
var endpoint = Assert.Single(dataSource.Endpoints);

var endpointName = endpoint.Metadata.GetMetadata<IEndpointNameMetadata>();
var routeName = endpoint.Metadata.GetMetadata<IRouteNameMetadata>();
var routeEndpointBuilder = GetRouteEndpointBuilder(builder);
Assert.Equal(name, endpointName?.EndpointName);
Assert.Equal(name, routeName?.RouteName);
Assert.Equal("HTTP: DELETE / => InnerGetString", routeEndpointBuilder.DisplayName);
}


[Fact]
public void MapMethod_SetsEndpointNameForMethodGroup()
{
var name = "GetString";
var builder = new DefaultEndpointRouteBuilder(new ApplicationBuilder(new EmptyServiceProvdier()));
_ = builder.MapDelete("/", GetString);

var dataSource = GetBuilderEndpointDataSource(builder);
// Trigger Endpoint build by calling getter.
var endpoint = Assert.Single(dataSource.Endpoints);

var endpointName = endpoint.Metadata.GetMetadata<IEndpointNameMetadata>();
var routeName = endpoint.Metadata.GetMetadata<IRouteNameMetadata>();
var routeEndpointBuilder = GetRouteEndpointBuilder(builder);
Assert.Equal(name, endpointName?.EndpointName);
Assert.Equal(name, routeName?.RouteName);
Assert.Equal("HTTP: DELETE / => GetString", routeEndpointBuilder.DisplayName);
}

[Fact]
public void WithNameOverridesDefaultEndpointName()
{
var name = "SomeCustomName";
var builder = new DefaultEndpointRouteBuilder(new ApplicationBuilder(new EmptyServiceProvdier()));
_ = builder.MapDelete("/", GetString).WithName(name);

var dataSource = GetBuilderEndpointDataSource(builder);
// Trigger Endpoint build by calling getter.
var endpoint = Assert.Single(dataSource.Endpoints);

var endpointName = endpoint.Metadata.GetMetadata<IEndpointNameMetadata>();
var routeName = endpoint.Metadata.GetMetadata<IRouteNameMetadata>();
var routeEndpointBuilder = GetRouteEndpointBuilder(builder);
Assert.Equal(name, endpointName?.EndpointName);
Assert.Equal(name, routeName?.RouteName);
// Will still use the original method name, not the custom endpoint name
Assert.Equal("HTTP: DELETE / => GetString", routeEndpointBuilder.DisplayName);
}

private string GetString() => "TestString";

[Fact]
public void MapMethod_DoesNotSetEndpointNameForLambda()
{
var builder = new DefaultEndpointRouteBuilder(new ApplicationBuilder(new EmptyServiceProvdier()));
_ = builder.MapDelete("/", () => { });

var dataSource = GetBuilderEndpointDataSource(builder);
// Trigger Endpoint build by calling getter.
var endpoint = Assert.Single(dataSource.Endpoints);

var endpointName = endpoint.Metadata.GetMetadata<IEndpointNameMetadata>();
Assert.Null(endpointName);
}

class FromRoute : Attribute, IFromRouteMetadata
{
public string? Name { get; set; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ private ApiDescription CreateApiDescription(RouteEndpoint routeEndpoint, string
// For now, put all methods defined the same declaring type together.
string controllerName;

if (methodInfo.DeclaringType is not null && !IsCompilerGenerated(methodInfo.DeclaringType))
if (methodInfo.DeclaringType is not null && !TypeHelper.IsCompilerGeneratedType(methodInfo.DeclaringType))
{
controllerName = methodInfo.DeclaringType.Name;
}
Expand Down Expand Up @@ -364,11 +364,5 @@ private static void AddActionDescriptorEndpointMetadata(
actionDescriptor.EndpointMetadata = new List<object>(endpointMetadata);
}
}

// The CompilerGeneratedAttribute doesn't always get added so we also check if the type name starts with "<"
// For example, "<>c" is a "declaring" type the C# compiler will generate without the attribute for a top-level lambda
// REVIEW: Is there a better way to do this?
private static bool IsCompilerGenerated(Type type) =>
Attribute.IsDefined(type, typeof(CompilerGeneratedAttribute)) || type.Name.StartsWith('<');
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

<ItemGroup>
<Compile Include="$(SharedSourceRoot)TryParseMethodCache.cs" />
<Compile Include="$(SharedSourceRoot)RoslynUtils\TypeHelper.cs" />
</ItemGroup>

<ItemGroup>
Expand Down
31 changes: 31 additions & 0 deletions src/Shared/RoslynUtils/GeneratedNameParser.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// 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.CodeAnalysis;

// This code is a stop-gap and exists to address the issues with extracting
// original method names from generated local functions. See https://github.com/dotnet/roslyn/issues/55651
// for more info.
namespace Microsoft.CodeAnalysis.CSharp.Symbols
{
internal static class GeneratedNameParser
{
/// <summary>
/// Parses generated local function name out of a generated method name.
/// </summary>
internal static bool TryParseLocalFunctionName(string generatedName, [NotNullWhen(true)] out string? originalName)
{
originalName = null;

var startIndex = generatedName.LastIndexOf(">g__", StringComparison.Ordinal);
var endIndex = generatedName.LastIndexOf("|", StringComparison.Ordinal);
if (startIndex >= 0 && endIndex >= 0 && endIndex - startIndex > 4)
{
originalName = generatedName.Substring(startIndex + 4, endIndex - startIndex - 4);
return true;
}

return false;
}
}
}
40 changes: 40 additions & 0 deletions src/Shared/RoslynUtils/TypeHelper.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Reflection;

namespace System.Runtime.CompilerServices
{
internal static class TypeHelper
{
/// <summary>
/// Checks to see if a given type is compiler generated.
/// <remarks>
/// The compiler will annotate either the target type or the declaring type
/// with the CompilerGenerated attribute. We walk up the declaring types until
/// we find a CompilerGenerated attribute or declare the type as not compiler
/// generated otherwise.
/// </remarks>
/// </summary>
/// <param name="type">The type to evaluate.</param>
/// <returns><see langword="true" /> if <paramref name="type"/> is compiler generated.</returns>
internal static bool IsCompilerGeneratedType(Type? type = null)
{
if (type is not null)
{
return Attribute.IsDefined(type, typeof(CompilerGeneratedAttribute)) || IsCompilerGeneratedType(type.DeclaringType);
}
return false;
}

/// <summary>
/// Checks to see if a given method is compiler generated.
/// </summary>
/// <param name="method">The method to evaluate.</param>
/// <returns><see langword="true" /> if <paramref name="method"/> is compiler generated.</returns>
internal static bool IsCompilerGeneratedMethod(MethodInfo method)
{
return Attribute.IsDefined(method, typeof(CompilerGeneratedAttribute)) || IsCompilerGeneratedType(method.DeclaringType);
}
}
}