Skip to content

Commit 61306cf

Browse files
rynowakrynowak
committed
PR feedback
1 parent 9c69ad3 commit 61306cf

File tree

4 files changed

+39
-17
lines changed

4 files changed

+39
-17
lines changed

‎src/Http/Routing/src/DefaultLinkParser.cs‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public override RouteValueDictionary ParsePathByAddress<TAddress>(TAddress addre
5959
var endpoint = endpoints[i];
6060
if (TryParse(endpoint, path, out var values))
6161
{
62-
Log.PathParsingSucceeded(_logger, path, endpoints);
62+
Log.PathParsingSucceeded(_logger, path, endpoint);
6363
return values;
6464
}
6565
}
@@ -85,7 +85,6 @@ private List<RouteEndpoint> GetEndpoints<TAddress>(TAddress address)
8585
return endpoints;
8686
}
8787

88-
8988
private MatcherState CreateRoutePatternMatcher(RouteEndpoint endpoint)
9089
{
9190
var constraints = new Dictionary<string, List<IRouteConstraint>>(StringComparer.OrdinalIgnoreCase);
@@ -190,10 +189,10 @@ public static class EventIds
190189
EventIds.EndpointsNotFound,
191190
"No endpoints found for address {Address}");
192191

193-
private static readonly Action<ILogger, IEnumerable<string>, string, Exception> _pathParsingSucceeded = LoggerMessage.Define<IEnumerable<string>, string>(
192+
private static readonly Action<ILogger, string, string, Exception> _pathParsingSucceeded = LoggerMessage.Define<string, string>(
194193
LogLevel.Debug,
195194
EventIds.PathParsingSucceeded,
196-
"Path parsing succeeded for endpoints {Endpoints} and URI path {URI}");
195+
"Path parsing succeeded for endpoint {Endpoint} and URI path {URI}");
197196

198197
private static readonly Action<ILogger, IEnumerable<string>, string, Exception> _pathParsingFailed = LoggerMessage.Define<IEnumerable<string>, string>(
199198
LogLevel.Debug,
@@ -214,12 +213,12 @@ public static void EndpointsNotFound(ILogger logger, object address)
214213
_endpointsNotFound(logger, address, null);
215214
}
216215

217-
public static void PathParsingSucceeded(ILogger logger, PathString path, IEnumerable<Endpoint> endpoints)
216+
public static void PathParsingSucceeded(ILogger logger, PathString path, Endpoint endpoint)
218217
{
219218
// Checking level again to avoid allocation on the common path
220219
if (logger.IsEnabled(LogLevel.Debug))
221220
{
222-
_pathParsingSucceeded(logger, endpoints.Select(e => e.DisplayName), path.Value, null);
221+
_pathParsingSucceeded(logger, endpoint.DisplayName, path.Value, null);
223222
}
224223
}
225224

‎src/Http/Routing/src/LinkParserEndpointNameAddressExtensions.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ public static class LinkParserEndpointNameAddressExtensions
1515
/// Attempts to parse the provided <paramref name="path"/> using the route pattern
1616
/// specified by the <see cref="Endpoint"/> matching <paramref name="endpointName"/>.
1717
/// </summary>
18-
/// <param name="parser">The <see cref="LinkGenerator"/>.</param>
18+
/// <param name="parser">The <see cref="LinkParser"/>.</param>
1919
/// <param name="endpointName">The endpoint name. Used to resolve endpoints.</param>
2020
/// <param name="path">The URI path to parse.</param>
2121
/// <returns>

‎src/Http/Routing/test/UnitTests/DefaultLinkParserTest.cs‎

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
using Microsoft.AspNetCore.Routing.Matching;
1010
using Microsoft.AspNetCore.Routing.TestObjects;
1111
using Microsoft.Extensions.DependencyInjection;
12+
using Microsoft.Extensions.Logging;
13+
using Microsoft.Extensions.Logging.Testing;
1214
using Xunit;
1315

1416
namespace Microsoft.AspNetCore.Routing
@@ -23,48 +25,68 @@ public class DefaultLinkParserTest : LinkParserTestBase
2325
public void ParsePathByAddresss_NoMatchingEndpoint_ReturnsNull()
2426
{
2527
// Arrange
26-
var endpoint = EndpointFactory.CreateRouteEndpoint("{controller}/{action}/{id?}", metadata: new object[] { new IntMetadata(1), });
28+
var endpoint = EndpointFactory.CreateRouteEndpoint("{controller}/{action}/{id?}", displayName: "Test1", metadata: new object[] { new IntMetadata(1), });
2729

28-
var parser = CreateLinkParser(endpoint);
30+
var sink = new TestSink();
31+
var loggerFactory = new TestLoggerFactory(sink, enabled: true);
32+
var parser = CreateLinkParser(services => { services.AddSingleton<ILoggerFactory>(loggerFactory); }, endpoint);
2933

3034
// Act
3135
var values = parser.ParsePathByAddress(0, "/Home/Index/17");
3236

3337
// Assert
3438
Assert.Null(values);
39+
40+
Assert.Collection(
41+
sink.Writes,
42+
w => Assert.Equal("No endpoints found for address 0", w.Message));
3543
}
3644

3745
[Fact]
3846
public void ParsePathByAddresss_HasMatches_ReturnsNullWhenParsingFails()
3947
{
4048
// Arrange
41-
var endpoint1 = EndpointFactory.CreateRouteEndpoint("{controller}/{action}/{id}", metadata: new object[] { new IntMetadata(1), });
42-
var endpoint2 = EndpointFactory.CreateRouteEndpoint("{controller}/{action}/{id2}", metadata: new object[] { new IntMetadata(0), });
49+
var endpoint1 = EndpointFactory.CreateRouteEndpoint("{controller}/{action}/{id}", displayName: "Test1", metadata: new object[] { new IntMetadata(1), });
50+
var endpoint2 = EndpointFactory.CreateRouteEndpoint("{controller}/{action}/{id2}", displayName: "Test2", metadata: new object[] { new IntMetadata(0), });
4351

44-
var parser = CreateLinkParser(endpoint1, endpoint2);
52+
var sink = new TestSink();
53+
var loggerFactory = new TestLoggerFactory(sink, enabled: true);
54+
var parser = CreateLinkParser(services => { services.AddSingleton<ILoggerFactory>(loggerFactory); }, endpoint1, endpoint2);
4555

4656
// Act
4757
var values = parser.ParsePathByAddress(0, "/");
4858

4959
// Assert
5060
Assert.Null(values);
61+
62+
Assert.Collection(
63+
sink.Writes,
64+
w => Assert.Equal("Found the endpoints Test2 for address 0", w.Message),
65+
w => Assert.Equal("Path parsing failed for endpoints Test2 and URI path /", w.Message));
5166
}
5267

5368
[Fact]
5469
public void ParsePathByAddresss_HasMatches_ReturnsFirstSuccessfulParse()
5570
{
5671
// Arrange
57-
var endpoint0 = EndpointFactory.CreateRouteEndpoint("{controller}/{action}", metadata: new object[] { new IntMetadata(0), });
58-
var endpoint1 = EndpointFactory.CreateRouteEndpoint("{controller}/{action}/{id}", metadata: new object[] { new IntMetadata(0), });
59-
var endpoint2 = EndpointFactory.CreateRouteEndpoint("{controller}/{action}/{id2}", metadata: new object[] { new IntMetadata(0), });
72+
var endpoint0 = EndpointFactory.CreateRouteEndpoint("{controller}/{action}", displayName: "Test1",metadata: new object[] { new IntMetadata(0), });
73+
var endpoint1 = EndpointFactory.CreateRouteEndpoint("{controller}/{action}/{id}", displayName: "Test2", metadata: new object[] { new IntMetadata(0), });
74+
var endpoint2 = EndpointFactory.CreateRouteEndpoint("{controller}/{action}/{id2}", displayName: "Test3", metadata: new object[] { new IntMetadata(0), });
6075

61-
var parser = CreateLinkParser(endpoint0, endpoint1, endpoint2);
76+
var sink = new TestSink();
77+
var loggerFactory = new TestLoggerFactory(sink, enabled: true);
78+
var parser = CreateLinkParser(services => { services.AddSingleton<ILoggerFactory>(loggerFactory); }, endpoint0, endpoint1, endpoint2);
6279

6380
// Act
6481
var values = parser.ParsePathByAddress(0, "/Home/Index/17");
6582

6683
// Assert
6784
MatcherAssert.AssertRouteValuesEqual(new { controller= "Home", action = "Index", id = "17" }, values);
85+
86+
Assert.Collection(
87+
sink.Writes,
88+
w => Assert.Equal("Found the endpoints Test1, Test2, Test3 for address 0", w.Message),
89+
w => Assert.Equal("Path parsing succeeded for endpoint Test2 and URI path /Home/Index/17", w.Message));
6890
}
6991

7092
[Fact]

‎src/Http/Routing/test/UnitTests/LinkParserTestBase.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using System;
55
using Microsoft.AspNetCore.Http;
66
using Microsoft.Extensions.DependencyInjection;
7+
using Microsoft.Extensions.Logging;
78
using Microsoft.Extensions.Logging.Abstractions;
89
using Microsoft.Extensions.Options;
910

@@ -66,7 +67,7 @@ private protected DefaultLinkParser CreateLinkParser(
6667
return new DefaultLinkParser(
6768
new DefaultParameterPolicyFactory(routeOptions, serviceProvider),
6869
new CompositeEndpointDataSource(routeOptions.Value.EndpointDataSources),
69-
NullLogger<DefaultLinkParser>.Instance,
70+
serviceProvider.GetRequiredService<ILoggerFactory>().CreateLogger<DefaultLinkParser>(),
7071
serviceProvider);
7172
}
7273
}

0 commit comments

Comments
 (0)