Skip to content

Commit 8ab6265

Browse files
captainsafiacaptainsafia
authored
Set EndpointName and DisplayName given method name in endpoints (#35439)
1 parent 00a3439 commit 8ab6265

File tree

7 files changed

+200
-16
lines changed

7 files changed

+200
-16
lines changed

‎src/Http/Routing/src/Builder/MinimalActionEndpointRouteBuilderExtensions.cs‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,11 @@
55
using System.Collections.Generic;
66
using System.Linq;
77
using System.Reflection;
8+
using System.Runtime.CompilerServices;
89
using Microsoft.AspNetCore.Http;
910
using Microsoft.AspNetCore.Routing;
1011
using Microsoft.AspNetCore.Routing.Patterns;
12+
using Microsoft.CodeAnalysis.CSharp.Symbols;
1113

1214
namespace Microsoft.AspNetCore.Builder
1315
{
@@ -107,7 +109,8 @@ public static MinimalActionEndpointConventionBuilder MapMethods(
107109
}
108110

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

190+
// Methods defined in a top-level program are generated as statics so the delegate
191+
// target will be null. Inline lambdas are compiler generated method so they can
192+
// be filtered that way.
193+
if (GeneratedNameParser.TryParseLocalFunctionName(action.Method.Name, out var endpointName)
194+
|| !TypeHelper.IsCompilerGeneratedMethod(action.Method))
195+
{
196+
endpointName ??= action.Method.Name;
197+
198+
builder.Metadata.Add(new EndpointNameMetadata(endpointName));
199+
builder.Metadata.Add(new RouteNameMetadata(endpointName));
200+
builder.DisplayName = $"{builder.DisplayName} => {endpointName}";
201+
}
202+
187203
// Add delegate attributes as metadata
188204
var attributes = action.Method.GetCustomAttributes();
189205

‎src/Http/Routing/src/Microsoft.AspNetCore.Routing.csproj‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ Microsoft.AspNetCore.Routing.RouteCollection</Description>
2424

2525
<ItemGroup>
2626
<Compile Include="$(SharedSourceRoot)PropertyHelper\*.cs" />
27+
<Compile Include="$(SharedSourceRoot)RoslynUtils\TypeHelper.cs" />
28+
<Compile Include="$(SharedSourceRoot)RoslynUtils\GeneratedNameParser.cs" />
2729
</ItemGroup>
2830

2931
<ItemGroup>

‎src/Http/Routing/test/UnitTests/Builder/MinimalActionEndpointRouteBuilderExtensionsTest.cs‎

Lines changed: 108 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ public void MapGet_BuildsEndpointWithCorrectMethod()
9999
Assert.Equal("GET", method);
100100

101101
var routeEndpointBuilder = GetRouteEndpointBuilder(builder);
102-
Assert.Equal("/ HTTP: GET", routeEndpointBuilder.DisplayName);
102+
Assert.Equal("HTTP: GET /", routeEndpointBuilder.DisplayName);
103103
Assert.Equal("/", routeEndpointBuilder.RoutePattern.RawText);
104104
}
105105

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

127127
var routeEndpointBuilder = GetRouteEndpointBuilder(builder);
128-
Assert.Equal("/{id} HTTP: GET", routeEndpointBuilder.DisplayName);
128+
Assert.Equal("HTTP: GET /{id}", routeEndpointBuilder.DisplayName);
129129
Assert.Equal("/{id}", routeEndpointBuilder.RoutePattern.RawText);
130130

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

165165
var routeEndpointBuilder = GetRouteEndpointBuilder(builder);
166-
Assert.Equal("/ HTTP: GET", routeEndpointBuilder.DisplayName);
166+
Assert.Equal("HTTP: GET /", routeEndpointBuilder.DisplayName);
167167
Assert.Equal("/", routeEndpointBuilder.RoutePattern.RawText);
168168

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

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

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

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

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

283283
var routeEndpointBuilder = GetRouteEndpointBuilder(builder);
284-
Assert.Equal("/ HTTP: POST", routeEndpointBuilder.DisplayName);
284+
Assert.Equal("HTTP: POST /", routeEndpointBuilder.DisplayName);
285285
Assert.Equal("/", routeEndpointBuilder.RoutePattern.RawText);
286286
}
287287

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

303303
var routeEndpointBuilder = GetRouteEndpointBuilder(builder);
304-
Assert.Equal("/ HTTP: PUT", routeEndpointBuilder.DisplayName);
304+
Assert.Equal("HTTP: PUT /", routeEndpointBuilder.DisplayName);
305305
Assert.Equal("/", routeEndpointBuilder.RoutePattern.RawText);
306306
}
307307

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

323323
var routeEndpointBuilder = GetRouteEndpointBuilder(builder);
324-
Assert.Equal("/ HTTP: DELETE", routeEndpointBuilder.DisplayName);
324+
Assert.Equal("HTTP: DELETE /", routeEndpointBuilder.DisplayName);
325325
Assert.Equal("/", routeEndpointBuilder.RoutePattern.RawText);
326326
}
327327

@@ -359,6 +359,106 @@ public void MapFallbackWithoutPath_BuildsEndpointWithLowestRouteOrder()
359359
Assert.Equal(int.MaxValue, routeEndpointBuilder.Order);
360360
}
361361

362+
[Fact]
363+
// This test scenario simulates methods defined in a top-level program
364+
// which are compiler generated. We currently do some manually parsing leveraging
365+
// code in Roslyn to support this scenario. More info at https://github.com/dotnet/roslyn/issues/55651.
366+
public void MapMethod_DoesNotEndpointNameForInnerMethod()
367+
{
368+
var name = "InnerGetString";
369+
var builder = new DefaultEndpointRouteBuilder(new ApplicationBuilder(new EmptyServiceProvdier()));
370+
string InnerGetString() => "TestString";
371+
_ = builder.MapDelete("/", InnerGetString);
372+
373+
var dataSource = GetBuilderEndpointDataSource(builder);
374+
// Trigger Endpoint build by calling getter.
375+
var endpoint = Assert.Single(dataSource.Endpoints);
376+
377+
var endpointName = endpoint.Metadata.GetMetadata<IEndpointNameMetadata>();
378+
var routeName = endpoint.Metadata.GetMetadata<IRouteNameMetadata>();
379+
var routeEndpointBuilder = GetRouteEndpointBuilder(builder);
380+
Assert.Equal(name, endpointName?.EndpointName);
381+
Assert.Equal(name, routeName?.RouteName);
382+
Assert.Equal("HTTP: DELETE / => InnerGetString", routeEndpointBuilder.DisplayName);
383+
}
384+
385+
[Fact]
386+
public void MapMethod_DoesNotEndpointNameForInnerMethodWithTarget()
387+
{
388+
var name = "InnerGetString";
389+
var builder = new DefaultEndpointRouteBuilder(new ApplicationBuilder(new EmptyServiceProvdier()));
390+
var testString = "TestString";
391+
string InnerGetString() => testString;
392+
_ = builder.MapDelete("/", InnerGetString);
393+
394+
var dataSource = GetBuilderEndpointDataSource(builder);
395+
// Trigger Endpoint build by calling getter.
396+
var endpoint = Assert.Single(dataSource.Endpoints);
397+
398+
var endpointName = endpoint.Metadata.GetMetadata<IEndpointNameMetadata>();
399+
var routeName = endpoint.Metadata.GetMetadata<IRouteNameMetadata>();
400+
var routeEndpointBuilder = GetRouteEndpointBuilder(builder);
401+
Assert.Equal(name, endpointName?.EndpointName);
402+
Assert.Equal(name, routeName?.RouteName);
403+
Assert.Equal("HTTP: DELETE / => InnerGetString", routeEndpointBuilder.DisplayName);
404+
}
405+
406+
407+
[Fact]
408+
public void MapMethod_SetsEndpointNameForMethodGroup()
409+
{
410+
var name = "GetString";
411+
var builder = new DefaultEndpointRouteBuilder(new ApplicationBuilder(new EmptyServiceProvdier()));
412+
_ = builder.MapDelete("/", GetString);
413+
414+
var dataSource = GetBuilderEndpointDataSource(builder);
415+
// Trigger Endpoint build by calling getter.
416+
var endpoint = Assert.Single(dataSource.Endpoints);
417+
418+
var endpointName = endpoint.Metadata.GetMetadata<IEndpointNameMetadata>();
419+
var routeName = endpoint.Metadata.GetMetadata<IRouteNameMetadata>();
420+
var routeEndpointBuilder = GetRouteEndpointBuilder(builder);
421+
Assert.Equal(name, endpointName?.EndpointName);
422+
Assert.Equal(name, routeName?.RouteName);
423+
Assert.Equal("HTTP: DELETE / => GetString", routeEndpointBuilder.DisplayName);
424+
}
425+
426+
[Fact]
427+
public void WithNameOverridesDefaultEndpointName()
428+
{
429+
var name = "SomeCustomName";
430+
var builder = new DefaultEndpointRouteBuilder(new ApplicationBuilder(new EmptyServiceProvdier()));
431+
_ = builder.MapDelete("/", GetString).WithName(name);
432+
433+
var dataSource = GetBuilderEndpointDataSource(builder);
434+
// Trigger Endpoint build by calling getter.
435+
var endpoint = Assert.Single(dataSource.Endpoints);
436+
437+
var endpointName = endpoint.Metadata.GetMetadata<IEndpointNameMetadata>();
438+
var routeName = endpoint.Metadata.GetMetadata<IRouteNameMetadata>();
439+
var routeEndpointBuilder = GetRouteEndpointBuilder(builder);
440+
Assert.Equal(name, endpointName?.EndpointName);
441+
Assert.Equal(name, routeName?.RouteName);
442+
// Will still use the original method name, not the custom endpoint name
443+
Assert.Equal("HTTP: DELETE / => GetString", routeEndpointBuilder.DisplayName);
444+
}
445+
446+
private string GetString() => "TestString";
447+
448+
[Fact]
449+
public void MapMethod_DoesNotSetEndpointNameForLambda()
450+
{
451+
var builder = new DefaultEndpointRouteBuilder(new ApplicationBuilder(new EmptyServiceProvdier()));
452+
_ = builder.MapDelete("/", () => { });
453+
454+
var dataSource = GetBuilderEndpointDataSource(builder);
455+
// Trigger Endpoint build by calling getter.
456+
var endpoint = Assert.Single(dataSource.Endpoints);
457+
458+
var endpointName = endpoint.Metadata.GetMetadata<IEndpointNameMetadata>();
459+
Assert.Null(endpointName);
460+
}
461+
362462
class FromRoute : Attribute, IFromRouteMetadata
363463
{
364464
public string? Name { get; set; }

‎src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ private ApiDescription CreateApiDescription(RouteEndpoint routeEndpoint, string
7878
// For now, put all methods defined the same declaring type together.
7979
string controllerName;
8080

81-
if (methodInfo.DeclaringType is not null && !IsCompilerGenerated(methodInfo.DeclaringType))
81+
if (methodInfo.DeclaringType is not null && !TypeHelper.IsCompilerGeneratedType(methodInfo.DeclaringType))
8282
{
8383
controllerName = methodInfo.DeclaringType.Name;
8484
}
@@ -363,11 +363,5 @@ private static void AddActionDescriptorEndpointMetadata(
363363
actionDescriptor.EndpointMetadata = new List<object>(endpointMetadata);
364364
}
365365
}
366-
367-
// The CompilerGeneratedAttribute doesn't always get added so we also check if the type name starts with "<"
368-
// For example, "<>c" is a "declaring" type the C# compiler will generate without the attribute for a top-level lambda
369-
// REVIEW: Is there a better way to do this?
370-
private static bool IsCompilerGenerated(Type type) =>
371-
Attribute.IsDefined(type, typeof(CompilerGeneratedAttribute)) || type.Name.StartsWith('<');
372366
}
373367
}

‎src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
<ItemGroup>
1313
<Compile Include="$(SharedSourceRoot)TryParseMethodCache.cs" />
14+
<Compile Include="$(SharedSourceRoot)RoslynUtils\TypeHelper.cs" />
1415
</ItemGroup>
1516

1617
<ItemGroup>

‎src/Shared/RoslynUtils/GeneratedNameParser.cs‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System.Diagnostics.CodeAnalysis;
5+
6+
// This code is a stop-gap and exists to address the issues with extracting
7+
// original method names from generated local functions. See https://github.com/dotnet/roslyn/issues/55651
8+
// for more info.
9+
namespace Microsoft.CodeAnalysis.CSharp.Symbols
10+
{
11+
internal static class GeneratedNameParser
12+
{
13+
/// <summary>
14+
/// Parses generated local function name out of a generated method name.
15+
/// </summary>
16+
internal static bool TryParseLocalFunctionName(string generatedName, [NotNullWhen(true)] out string? originalName)
17+
{
18+
originalName = null;
19+
20+
var startIndex = generatedName.LastIndexOf(">g__", StringComparison.Ordinal);
21+
var endIndex = generatedName.LastIndexOf("|", StringComparison.Ordinal);
22+
if (startIndex >= 0 && endIndex >= 0 && endIndex - startIndex > 4)
23+
{
24+
originalName = generatedName.Substring(startIndex + 4, endIndex - startIndex - 4);
25+
return true;
26+
}
27+
28+
return false;
29+
}
30+
}
31+
}

‎src/Shared/RoslynUtils/TypeHelper.cs‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System.Reflection;
5+
6+
namespace System.Runtime.CompilerServices
7+
{
8+
internal static class TypeHelper
9+
{
10+
/// <summary>
11+
/// Checks to see if a given type is compiler generated.
12+
/// <remarks>
13+
/// The compiler will annotate either the target type or the declaring type
14+
/// with the CompilerGenerated attribute. We walk up the declaring types until
15+
/// we find a CompilerGenerated attribute or declare the type as not compiler
16+
/// generated otherwise.
17+
/// </remarks>
18+
/// </summary>
19+
/// <param name="type">The type to evaluate.</param>
20+
/// <returns><see langword="true" /> if <paramref name="type"/> is compiler generated.</returns>
21+
internal static bool IsCompilerGeneratedType(Type? type = null)
22+
{
23+
if (type is not null)
24+
{
25+
return Attribute.IsDefined(type, typeof(CompilerGeneratedAttribute)) || IsCompilerGeneratedType(type.DeclaringType);
26+
}
27+
return false;
28+
}
29+
30+
/// <summary>
31+
/// Checks to see if a given method is compiler generated.
32+
/// </summary>
33+
/// <param name="method">The method to evaluate.</param>
34+
/// <returns><see langword="true" /> if <paramref name="method"/> is compiler generated.</returns>
35+
internal static bool IsCompilerGeneratedMethod(MethodInfo method)
36+
{
37+
return Attribute.IsDefined(method, typeof(CompilerGeneratedAttribute)) || IsCompilerGeneratedType(method.DeclaringType);
38+
}
39+
}
40+
}

0 commit comments

Comments
 (0)