From 4943bc489604927ac88c297b8b145bbdc6c69a2e Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Fri, 19 Oct 2018 17:31:54 -0700 Subject: [PATCH 1/4] Remove LinkGenerationTemplate This doesn't really accomplish our goals for 2.2 - I don't have a clear scenario where I would tell a developer to use this VS something else. Will reevaluate in 3.0 --- .../LinkGenerationTemplate.cs | 126 ----------- .../LinkGenerationTemplateOptions.cs | 17 -- .../LinkGenerator.cs | 11 - .../DefaultLinkGenerationTemplate.cs | 120 ---------- .../DefaultLinkGenerator.cs | 12 - ...kGeneratorEndpointNameAddressExtensions.cs | 28 --- ...nkGeneratorRouteValuesAddressExtensions.cs | 28 --- .../DefaultLinkGenerationTemplateTest.cs | 207 ------------------ .../DefaultLinkGeneratorTest.cs | 36 --- ...LinkGeneratorEndpointNameExtensionsTest.cs | 19 -- ...neratorRouteValuesAddressExtensionsTest.cs | 26 --- 11 files changed, 630 deletions(-) delete mode 100644 src/Microsoft.AspNetCore.Routing.Abstractions/LinkGenerationTemplate.cs delete mode 100644 src/Microsoft.AspNetCore.Routing.Abstractions/LinkGenerationTemplateOptions.cs delete mode 100644 src/Microsoft.AspNetCore.Routing/DefaultLinkGenerationTemplate.cs delete mode 100644 test/Microsoft.AspNetCore.Routing.Tests/DefaultLinkGenerationTemplateTest.cs diff --git a/src/Microsoft.AspNetCore.Routing.Abstractions/LinkGenerationTemplate.cs b/src/Microsoft.AspNetCore.Routing.Abstractions/LinkGenerationTemplate.cs deleted file mode 100644 index b54692bd15..0000000000 --- a/src/Microsoft.AspNetCore.Routing.Abstractions/LinkGenerationTemplate.cs +++ /dev/null @@ -1,126 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using Microsoft.AspNetCore.Http; - -namespace Microsoft.AspNetCore.Routing -{ - /// - /// Defines a contract to generate a URL from a template. - /// - /// - /// A can be created from - /// by supplying an address value which has matching endpoints. The returned - /// will be bound to the endpoints matching the address that was originally provided. - /// - public abstract class LinkGenerationTemplate - { - /// - /// Generates a URI with an absolute path based on the provided values. - /// - /// The associated with the current request. - /// The route values. Used to expand parameters in the route template. Optional. - /// - /// An optional URI path base. Prepended to the path in the resulting URI. If not provided, the value of will be used. - /// - /// An optional URI fragment. Appended to the resulting URI. - /// - /// An optional . Settings on provided object override the settings with matching - /// names from RouteOptions. - /// - /// A URI with an absolute path, or null. - public abstract string GetPath( - HttpContext httpContext, - object values, - PathString? pathBase = default, - FragmentString fragment = default, - LinkOptions options = default); - - /// - /// Generates a URI with an absolute path based on the provided values. - /// - /// The route values. Used to expand parameters in the route template. Optional. - /// An optional URI path base. Prepended to the path in the resulting URI. - /// An optional URI fragment. Appended to the resulting URI. - /// - /// An optional . Settings on provided object override the settings with matching - /// names from RouteOptions. - /// - /// A URI with an absolute path, or null. - public abstract string GetPath( - object values, - PathString pathBase = default, - FragmentString fragment = default, - LinkOptions options = default); - - /// - /// Generates an absolute URI based on the provided values. - /// - /// The associated with the current request. - /// The route values. Used to expand parameters in the route template. Optional. - /// - /// The URI scheme, applied to the resulting URI. Optional. If not provided, the value of will be used. - /// - /// - /// The URI host/authority, applied to the resulting URI. Optional. If not provided, the value will be used. - /// See the remarks section for details about the security implications of the . - /// - /// - /// An optional URI path base. Prepended to the path in the resulting URI. If not provided, the value of will be used. - /// - /// An optional URI fragment. Appended to the resulting URI. - /// - /// An optional . Settings on provided object override the settings with matching - /// names from RouteOptions. - /// - /// A URI with an absolute path, or null. - /// - /// - /// The value of should be a trusted value. Relying on the value of the current request - /// can allow untrusted input to influence the resulting URI unless the Host header has been validated. - /// See the deployment documentation for instructions on how to properly validate the Host header in - /// your deployment environment. - /// - /// - public abstract string GetUri( - HttpContext httpContext, - object values, - string scheme = default, - HostString? host = default, - PathString? pathBase = default, - FragmentString fragment = default, - LinkOptions options = default); - - /// - /// Generates an absolute URI based on the provided values. - /// - /// The route values. Used to expand parameters in the route template. Optional. - /// The URI scheme, applied to the resulting URI. - /// - /// The URI host/authority, applied to the resulting URI. - /// See the remarks section for details about the security implications of the . - /// - /// An optional URI path base. Prepended to the path in the resulting URI. - /// An optional URI fragment. Appended to the resulting URI. - /// - /// An optional . Settings on provided object override the settings with matching - /// names from RouteOptions. - /// - /// An absolute URI, or null. - /// - /// - /// The value of should be a trusted value. Relying on the value of the current request - /// can allow untrusted input to influence the resulting URI unless the Host header has been validated. - /// See the deployment documentation for instructions on how to properly validate the Host header in - /// your deployment environment. - /// - /// - public abstract string GetUri( - object values, - string scheme, - HostString host, - PathString pathBase = default, - FragmentString fragment = default, - LinkOptions options = default); - } -} \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Routing.Abstractions/LinkGenerationTemplateOptions.cs b/src/Microsoft.AspNetCore.Routing.Abstractions/LinkGenerationTemplateOptions.cs deleted file mode 100644 index 18eb0f8c7c..0000000000 --- a/src/Microsoft.AspNetCore.Routing.Abstractions/LinkGenerationTemplateOptions.cs +++ /dev/null @@ -1,17 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -namespace Microsoft.AspNetCore.Routing -{ - /// - /// Contains options for creating a . - /// - public class LinkGenerationTemplateOptions - { - /// - /// Gets or sets a value indicating whether the template will use route values from the current request - /// when generating a URI. - /// - public bool UseAmbientValues { get; set; } - } -} diff --git a/src/Microsoft.AspNetCore.Routing.Abstractions/LinkGenerator.cs b/src/Microsoft.AspNetCore.Routing.Abstractions/LinkGenerator.cs index 1f91a4446d..add5eaf119 100644 --- a/src/Microsoft.AspNetCore.Routing.Abstractions/LinkGenerator.cs +++ b/src/Microsoft.AspNetCore.Routing.Abstractions/LinkGenerator.cs @@ -147,16 +147,5 @@ namespace Microsoft.AspNetCore.Routing PathString pathBase = default, FragmentString fragment = default, LinkOptions options = default); - - /// - /// Gets a based on the provided . - /// - /// The address type. - /// The address value. Used to resolve endpoints. - /// Options for the created . - /// - /// A if one or more endpoints matching the address can be found, otherwise null. - /// - public abstract LinkGenerationTemplate GetTemplateByAddress(TAddress address, LinkGenerationTemplateOptions options = null); } } diff --git a/src/Microsoft.AspNetCore.Routing/DefaultLinkGenerationTemplate.cs b/src/Microsoft.AspNetCore.Routing/DefaultLinkGenerationTemplate.cs deleted file mode 100644 index d1f83ca6e0..0000000000 --- a/src/Microsoft.AspNetCore.Routing/DefaultLinkGenerationTemplate.cs +++ /dev/null @@ -1,120 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using Microsoft.AspNetCore.Http; -using System; -using System.Collections.Generic; - -namespace Microsoft.AspNetCore.Routing -{ - internal sealed class DefaultLinkGenerationTemplate : LinkGenerationTemplate - { - public DefaultLinkGenerationTemplate(DefaultLinkGenerator linkGenerator, List endpoints, LinkGenerationTemplateOptions options) - { - LinkGenerator = linkGenerator; - Endpoints = endpoints; - Options = options; - } - - public DefaultLinkGenerator LinkGenerator { get; } - - public List Endpoints { get; } - - public LinkGenerationTemplateOptions Options { get; } - - public override string GetPath( - HttpContext httpContext, - object values, - PathString? pathBase = default, - FragmentString fragment = default, - LinkOptions options = default) - { - if (httpContext == null) - { - throw new ArgumentNullException(nameof(httpContext)); - } - - return LinkGenerator.GetPathByEndpoints( - Endpoints, - new RouteValueDictionary(values), - GetAmbientValues(httpContext), - pathBase ?? httpContext.Request.PathBase, - fragment, - options); - } - - public override string GetPath( - object values, - PathString pathBase = default, - FragmentString fragment = default, - LinkOptions options = default) - { - return LinkGenerator.GetPathByEndpoints( - Endpoints, - new RouteValueDictionary(values), - ambientValues: null, - pathBase: pathBase, - fragment: fragment, - options: options); - } - - public override string GetUri( - HttpContext httpContext, - object values, - string scheme = default, - HostString? host = default, - PathString? pathBase = default, - FragmentString fragment = default, - LinkOptions options = default) - { - if (httpContext == null) - { - throw new ArgumentNullException(nameof(httpContext)); - } - - return LinkGenerator.GetUriByEndpoints( - Endpoints, - new RouteValueDictionary(values), - GetAmbientValues(httpContext), - scheme ?? httpContext.Request.Scheme, - host ?? httpContext.Request.Host, - pathBase ?? httpContext.Request.PathBase, - fragment, - options); - } - - public override string GetUri( - object values, - string scheme, - HostString host, - PathString pathBase = default, - FragmentString fragment = default, - LinkOptions options = default) - { - if (string.IsNullOrEmpty(scheme)) - { - throw new ArgumentException("A scheme must be provided.", nameof(scheme)); - } - - if (!host.HasValue) - { - throw new ArgumentException("A host must be provided.", nameof(host)); - } - - return LinkGenerator.GetUriByEndpoints( - Endpoints, - new RouteValueDictionary(values), - ambientValues: null, - scheme: scheme, - host: host, - pathBase: pathBase, - fragment: fragment, - options: options); - } - - private RouteValueDictionary GetAmbientValues(HttpContext httpContext) - { - return (Options?.UseAmbientValues ?? false) ? DefaultLinkGenerator.GetAmbientValues(httpContext) : null; - } - } -} diff --git a/src/Microsoft.AspNetCore.Routing/DefaultLinkGenerator.cs b/src/Microsoft.AspNetCore.Routing/DefaultLinkGenerator.cs index bc4b3f85b5..086d9bf026 100644 --- a/src/Microsoft.AspNetCore.Routing/DefaultLinkGenerator.cs +++ b/src/Microsoft.AspNetCore.Routing/DefaultLinkGenerator.cs @@ -189,17 +189,6 @@ namespace Microsoft.AspNetCore.Routing options: options); } - public override LinkGenerationTemplate GetTemplateByAddress(TAddress address, LinkGenerationTemplateOptions options = default) - { - var endpoints = GetEndpoints(address); - if (endpoints.Count == 0) - { - return null; - } - - return new DefaultLinkGenerationTemplate(this, endpoints, options); - } - private List GetEndpoints(TAddress address) { var addressingScheme = _serviceProvider.GetRequiredService>(); @@ -217,7 +206,6 @@ namespace Microsoft.AspNetCore.Routing return endpoints; } - // Also called from DefaultLinkGenerationTemplate public string GetPathByEndpoints( List endpoints, RouteValueDictionary values, diff --git a/src/Microsoft.AspNetCore.Routing/LinkGeneratorEndpointNameAddressExtensions.cs b/src/Microsoft.AspNetCore.Routing/LinkGeneratorEndpointNameAddressExtensions.cs index d3e50328cf..dd0e462c20 100644 --- a/src/Microsoft.AspNetCore.Routing/LinkGeneratorEndpointNameAddressExtensions.cs +++ b/src/Microsoft.AspNetCore.Routing/LinkGeneratorEndpointNameAddressExtensions.cs @@ -11,11 +11,6 @@ namespace Microsoft.AspNetCore.Routing /// public static class LinkGeneratorEndpointNameAddressExtensions { - private static readonly LinkGenerationTemplateOptions _templateOptions = new LinkGenerationTemplateOptions() - { - UseAmbientValues = false, - }; - /// /// Generates a URI with an absolute path based on the provided values. /// @@ -227,28 +222,5 @@ namespace Microsoft.AspNetCore.Routing return generator.GetUriByAddress(endpointName, new RouteValueDictionary(values), scheme, host, pathBase, fragment, options); } - - /// - /// Gets a based on the provided . - /// - /// The . - /// The endpoint name. Used to resolve endpoints. Optional. - /// - /// A if one or more endpoints matching the address can be found, otherwise null. - /// - public static LinkGenerationTemplate GetTemplateByName(this LinkGenerator generator, string endpointName) - { - if (generator == null) - { - throw new ArgumentNullException(nameof(generator)); - } - - if (endpointName == null) - { - throw new ArgumentNullException(nameof(endpointName)); - } - - return generator.GetTemplateByAddress(endpointName, _templateOptions); - } } } diff --git a/src/Microsoft.AspNetCore.Routing/LinkGeneratorRouteValuesAddressExtensions.cs b/src/Microsoft.AspNetCore.Routing/LinkGeneratorRouteValuesAddressExtensions.cs index d41f43d644..f683b4d3ca 100644 --- a/src/Microsoft.AspNetCore.Routing/LinkGeneratorRouteValuesAddressExtensions.cs +++ b/src/Microsoft.AspNetCore.Routing/LinkGeneratorRouteValuesAddressExtensions.cs @@ -11,11 +11,6 @@ namespace Microsoft.AspNetCore.Routing /// public static class LinkGeneratorRouteValuesAddressExtensions { - private static readonly LinkGenerationTemplateOptions _templateOptions = new LinkGenerationTemplateOptions() - { - UseAmbientValues = true, - }; - /// /// Generates a URI with an absolute path based on the provided values. /// @@ -202,29 +197,6 @@ namespace Microsoft.AspNetCore.Routing return generator.GetUriByAddress(address, address.ExplicitValues, scheme, host, pathBase, fragment, options); } - /// - /// Gets a based on the provided and . - /// - /// The . - /// The route name. Used to resolve endpoints. Optional. - /// The route values. Used to resolve endpoints and expand parameters in the route template. Optional. - /// - /// A if one or more endpoints matching the address can be found, otherwise null. - /// - public static LinkGenerationTemplate GetTemplateByRouteValues( - this LinkGenerator generator, - string routeName, - object values) - { - if (generator == null) - { - throw new ArgumentNullException(nameof(generator)); - } - - var address = CreateAddress(httpContext: null, routeName, values); - return generator.GetTemplateByAddress(address, _templateOptions); - } - private static RouteValuesAddress CreateAddress(HttpContext httpContext, string routeName, object values) { return new RouteValuesAddress() diff --git a/test/Microsoft.AspNetCore.Routing.Tests/DefaultLinkGenerationTemplateTest.cs b/test/Microsoft.AspNetCore.Routing.Tests/DefaultLinkGenerationTemplateTest.cs deleted file mode 100644 index 12f797fdea..0000000000 --- a/test/Microsoft.AspNetCore.Routing.Tests/DefaultLinkGenerationTemplateTest.cs +++ /dev/null @@ -1,207 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System.Collections.Generic; -using Microsoft.AspNetCore.Http; -using Xunit; - -namespace Microsoft.AspNetCore.Routing -{ - // Tests DefaultLinkGenerationTemplate functionality - these are pretty light since most of the functionality - // is a direct subset of DefaultLinkGenerator - // - // Does not cover template processing in detail, those scenarios are validated by TemplateBinderTests - // and DefaultLinkGeneratorProcessTemplateTest - public class DefaultLinkGenerationTemplateTest : LinkGeneratorTestBase - { - [Fact] - public void GetPath_WithoutHttpContext_WithPathBaseAndFragment() - { - // Arrange - var endpoint1 = EndpointFactory.CreateRouteEndpoint("{controller}/{action}/{id}"); - var endpoint2 = EndpointFactory.CreateRouteEndpoint("{controller}/{action}/{id?}"); - - var linkGenerator = CreateLinkGenerator(); - var template = new DefaultLinkGenerationTemplate(linkGenerator, new List() { endpoint1, endpoint2, }, options: null); - - // Act - var path = template.GetPath( - values: new RouteValueDictionary(new { controller = "Home", action = "In?dex", query = "some?query" }), - new PathString("/Foo/Bar?encodeme?"), - new FragmentString("#Fragment?"), - new LinkOptions() { AppendTrailingSlash = true, }); - - // Assert - Assert.Equal("/Foo/Bar%3Fencodeme%3F/Home/In%3Fdex/?query=some%3Fquery#Fragment?", path); - } - - [Fact] - public void GetPath_WithHttpContext_WithPathBaseAndFragment() - { - // Arrange - var endpoint1 = EndpointFactory.CreateRouteEndpoint("{controller}/{action}/{id}"); - var endpoint2 = EndpointFactory.CreateRouteEndpoint("{controller}/{action}/{id?}"); - - var linkGenerator = CreateLinkGenerator(); - var template = new DefaultLinkGenerationTemplate(linkGenerator, new List() { endpoint1, endpoint2, }, options: null); - - var httpContext = CreateHttpContext(); - httpContext.Request.PathBase = new PathString("/Foo/Bar?encodeme?"); - - // Act - var path = template.GetPath( - httpContext, - values: new RouteValueDictionary(new { controller = "Home", action = "In?dex", query = "some?query" }), - fragment: new FragmentString("#Fragment?"), - options: new LinkOptions() { AppendTrailingSlash = true, }); - - // Assert - Assert.Equal("/Foo/Bar%3Fencodeme%3F/Home/In%3Fdex/?query=some%3Fquery#Fragment?", path); - } - - [Fact] - public void GetUri_WithoutHttpContext_WithPathBaseAndFragment() - { - // Arrange - var endpoint1 = EndpointFactory.CreateRouteEndpoint("{controller}/{action}/{id}"); - var endpoint2 = EndpointFactory.CreateRouteEndpoint("{controller}/{action}/{id?}"); - - var linkGenerator = CreateLinkGenerator(); - var template = new DefaultLinkGenerationTemplate(linkGenerator, new List() { endpoint1, endpoint2, }, options: null); - - // Act - var path = template.GetUri( - values: new RouteValueDictionary(new { controller = "Home", action = "In?dex", query = "some?query" }), - "http", - new HostString("example.com"), - new PathString("/Foo/Bar?encodeme?"), - new FragmentString("#Fragment?"), - new LinkOptions() { AppendTrailingSlash = true, }); - - // Assert - Assert.Equal("http://example.com/Foo/Bar%3Fencodeme%3F/Home/In%3Fdex/?query=some%3Fquery#Fragment?", path); - } - - [Fact] - public void GetUri_WithHttpContext_WithPathBaseAndFragment() - { - // Arrange - var endpoint1 = EndpointFactory.CreateRouteEndpoint("{controller}/{action}/{id}"); - var endpoint2 = EndpointFactory.CreateRouteEndpoint("{controller}/{action}/{id?}"); - - var linkGenerator = CreateLinkGenerator(); - var template = new DefaultLinkGenerationTemplate(linkGenerator, new List() { endpoint1, endpoint2, }, options: null); - - var httpContext = CreateHttpContext(); - httpContext.Request.Scheme = "http"; - httpContext.Request.Host = new HostString("example.com"); - httpContext.Request.PathBase = new PathString("/Foo/Bar?encodeme?"); - - // Act - var uri = template.GetUri( - httpContext, - values: new RouteValueDictionary(new { controller = "Home", action = "In?dex", query = "some?query" }), - fragment: new FragmentString("#Fragment?"), - options: new LinkOptions() { AppendTrailingSlash = true, }); - - // Assert - Assert.Equal("http://example.com/Foo/Bar%3Fencodeme%3F/Home/In%3Fdex/?query=some%3Fquery#Fragment?", uri); - } - - [Fact] - public void GetPath_WithHttpContext_IncludesAmbientValues_WhenUseAmbientValuesIsTrue() - { - // Arrange - var endpoint1 = EndpointFactory.CreateRouteEndpoint("{controller}/{action}/{id}"); - var endpoint2 = EndpointFactory.CreateRouteEndpoint("{controller}/{action}/{id?}"); - - var linkGenerator = CreateLinkGenerator(); - var template = new DefaultLinkGenerationTemplate(linkGenerator, new List() { endpoint1, endpoint2, }, options: new LinkGenerationTemplateOptions() - { - UseAmbientValues = true, - }); - - var httpContext = CreateHttpContext(new { controller = "Home", }); - httpContext.Request.Scheme = "http"; - httpContext.Request.Host = new HostString("example.com"); - - // Act - var uri = template.GetPath(httpContext, values: new RouteValueDictionary(new { action = "Index", })); - - // Assert - Assert.Equal("/Home/Index", uri); - } - - [Fact] - public void GetPath_WithHttpContext_ExcludesAmbientValues_WhenUseAmbientValuesIsFalse() - { - // Arrange - var endpoint1 = EndpointFactory.CreateRouteEndpoint("{controller}/{action}/{id}"); - var endpoint2 = EndpointFactory.CreateRouteEndpoint("{controller}/{action}/{id?}"); - - var linkGenerator = CreateLinkGenerator(); - var template = new DefaultLinkGenerationTemplate(linkGenerator, new List() { endpoint1, endpoint2, }, options: new LinkGenerationTemplateOptions() - { - UseAmbientValues = false, - }); - - var httpContext = CreateHttpContext(new { controller = "Home", }); - httpContext.Request.Scheme = "http"; - httpContext.Request.Host = new HostString("example.com"); - - // Act - var uri = template.GetPath(httpContext, values: new RouteValueDictionary(new { action = "Index", })); - - // Assert - Assert.Null(uri); - } - - [Fact] - public void GetUri_WithHttpContext_IncludesAmbientValues_WhenUseAmbientValuesIsTrue() - { - // Arrange - var endpoint1 = EndpointFactory.CreateRouteEndpoint("{controller}/{action}/{id}"); - var endpoint2 = EndpointFactory.CreateRouteEndpoint("{controller}/{action}/{id?}"); - - var linkGenerator = CreateLinkGenerator(); - var template = new DefaultLinkGenerationTemplate(linkGenerator, new List() { endpoint1, endpoint2, }, options: new LinkGenerationTemplateOptions() - { - UseAmbientValues = true, - }); - - var httpContext = CreateHttpContext(new { controller = "Home", }); - httpContext.Request.Scheme = "http"; - httpContext.Request.Host = new HostString("example.com"); - - // Act - var uri = template.GetUri(httpContext, values: new { action = "Index", }); - - // Assert - Assert.Equal("http://example.com/Home/Index", uri); - } - - [Fact] - public void GetUri_WithHttpContext_ExcludesAmbientValues_WhenUseAmbientValuesIsFalse() - { - // Arrange - var endpoint1 = EndpointFactory.CreateRouteEndpoint("{controller}/{action}/{id}"); - var endpoint2 = EndpointFactory.CreateRouteEndpoint("{controller}/{action}/{id?}"); - - var linkGenerator = CreateLinkGenerator(); - var template = new DefaultLinkGenerationTemplate(linkGenerator, new List() { endpoint1, endpoint2, }, options: new LinkGenerationTemplateOptions() - { - UseAmbientValues = false, - }); - - var httpContext = CreateHttpContext(new { controller = "Home", }); - httpContext.Request.Scheme = "http"; - httpContext.Request.Host = new HostString("example.com"); - - // Act - var uri = template.GetUri(httpContext, values: new { action = "Index", }); - - // Assert - Assert.Null(uri); - } - } -} diff --git a/test/Microsoft.AspNetCore.Routing.Tests/DefaultLinkGeneratorTest.cs b/test/Microsoft.AspNetCore.Routing.Tests/DefaultLinkGeneratorTest.cs index 5a2d8d465c..3c20c704a0 100644 --- a/test/Microsoft.AspNetCore.Routing.Tests/DefaultLinkGeneratorTest.cs +++ b/test/Microsoft.AspNetCore.Routing.Tests/DefaultLinkGeneratorTest.cs @@ -524,42 +524,6 @@ namespace Microsoft.AspNetCore.Routing Assert.Equal("ftp://example.com:5000/Home/Index", uri); } - [Fact] - public void GetTemplateByAddress_WithNoMatch_ReturnsNull() - { - // Arrange - var endpoint1 = EndpointFactory.CreateRouteEndpoint("{controller}/{action}/{id}", metadata: new object[] { new IntMetadata(1), }); - var endpoint2 = EndpointFactory.CreateRouteEndpoint("{controller}/{action}/{id?}", metadata: new object[] { new IntMetadata(1), }); - - var linkGenerator = CreateLinkGenerator(endpoint1, endpoint2); - - // Act - var template = linkGenerator.GetTemplateByAddress(address: 0); - - // Assert - Assert.Null(template); - } - - [Fact] - public void GetTemplateByAddress_WithMatch_ReturnsTemplate() - { - // Arrange - var endpoint1 = EndpointFactory.CreateRouteEndpoint("{controller}/{action}/{id}", metadata: new object[] { new IntMetadata(1), }); - var endpoint2 = EndpointFactory.CreateRouteEndpoint("{controller}/{action}/{id?}", metadata: new object[] { new IntMetadata(1), }); - - var linkGenerator = CreateLinkGenerator(endpoint1, endpoint2); - - // Act - var template = linkGenerator.GetTemplateByAddress(address: 1); - - // Assert - Assert.NotNull(template); - Assert.Collection( - Assert.IsType(template).Endpoints, - e => Assert.Same(endpoint1, e), - e => Assert.Same(endpoint2, e)); - } - [Fact] public void GetTemplateBinder_CanCache() { diff --git a/test/Microsoft.AspNetCore.Routing.Tests/LinkGeneratorEndpointNameExtensionsTest.cs b/test/Microsoft.AspNetCore.Routing.Tests/LinkGeneratorEndpointNameExtensionsTest.cs index c81d134ce6..e2a9382281 100644 --- a/test/Microsoft.AspNetCore.Routing.Tests/LinkGeneratorEndpointNameExtensionsTest.cs +++ b/test/Microsoft.AspNetCore.Routing.Tests/LinkGeneratorEndpointNameExtensionsTest.cs @@ -149,24 +149,5 @@ namespace Microsoft.AspNetCore.Routing // Assert Assert.Equal("http://example.com/Foo/Bar%3Fencodeme%3F/some%23-other-endpoint/In%3Fdex/?query=some%3Fquery#Fragment?", uri); } - - [Fact] - public void GetTemplateByName_CreatesTemplate() - { - // Arrange - var endpoint1 = EndpointFactory.CreateRouteEndpoint("some-endpoint/{p}", metadata: new[] { new EndpointNameMetadata("name1"), }); - var endpoint2 = EndpointFactory.CreateRouteEndpoint("some#-other-endpoint/{p}", metadata: new[] { new EndpointNameMetadata("name2"), }); - - var linkGenerator = CreateLinkGenerator(endpoint1, endpoint2); - - // Act - var template = linkGenerator.GetTemplateByName(endpointName: "name2"); - - // Assert - Assert.NotNull(template); - Assert.Collection( - Assert.IsType(template).Endpoints.Cast().OrderBy(e => e.RoutePattern.RawText), - e => Assert.Same(endpoint2, e)); - } } } diff --git a/test/Microsoft.AspNetCore.Routing.Tests/LinkGeneratorRouteValuesAddressExtensionsTest.cs b/test/Microsoft.AspNetCore.Routing.Tests/LinkGeneratorRouteValuesAddressExtensionsTest.cs index 548cdadcad..b317929355 100644 --- a/test/Microsoft.AspNetCore.Routing.Tests/LinkGeneratorRouteValuesAddressExtensionsTest.cs +++ b/test/Microsoft.AspNetCore.Routing.Tests/LinkGeneratorRouteValuesAddressExtensionsTest.cs @@ -201,31 +201,5 @@ namespace Microsoft.AspNetCore.Routing // Assert Assert.Equal("http://example.com/Foo/Bar%3Fencodeme%3F/Home/Index/?query=some%3Fquery#Fragment?", uri); } - - [Fact] - public void GetTemplateByRouteValues_CreatesTemplate() - { - // Arrange - var endpoint1 = EndpointFactory.CreateRouteEndpoint( - "{controller}/{action}/{id}", - metadata: new[] { new RouteValuesAddressMetadata(new RouteValueDictionary(new { controller = "Home", action = "In?dex", })) }); - var endpoint2 = EndpointFactory.CreateRouteEndpoint( - "{controller}/{action}/{id?}", - metadata: new[] { new RouteValuesAddressMetadata(new RouteValueDictionary(new { controller = "Home", action = "In?dex", })) }); - - var linkGenerator = CreateLinkGenerator(endpoint1, endpoint2); - - // Act - var template = linkGenerator.GetTemplateByRouteValues( - routeName: null, - values: new RouteValueDictionary(new { controller = "Home", action = "In?dex", query = "some?query" })); - - // Assert - Assert.NotNull(template); - Assert.Collection( - Assert.IsType(template).Endpoints.Cast().OrderBy(e => e.RoutePattern.RawText), - e => Assert.Same(endpoint2, e), - e => Assert.Same(endpoint1, e)); - } } } From 26e5ea32741f1782037ce06d90aa5c63a1f54996 Mon Sep 17 00:00:00 2001 From: Gert Driesen Date: Mon, 22 Oct 2018 23:20:15 +0200 Subject: [PATCH 2/4] Improves performance and reduce allocations in DefaultEndpointDataSource (#882) --- .../DefaultEndpointDataSource.cs | 12 ++-- .../DefaultEndpointDataSourceTests.cs | 57 ++++++++++++++++++- 2 files changed, 64 insertions(+), 5 deletions(-) diff --git a/src/Microsoft.AspNetCore.Routing/DefaultEndpointDataSource.cs b/src/Microsoft.AspNetCore.Routing/DefaultEndpointDataSource.cs index 94728feb8c..7586ee4a2e 100644 --- a/src/Microsoft.AspNetCore.Routing/DefaultEndpointDataSource.cs +++ b/src/Microsoft.AspNetCore.Routing/DefaultEndpointDataSource.cs @@ -14,15 +14,20 @@ namespace Microsoft.AspNetCore.Routing /// public sealed class DefaultEndpointDataSource : EndpointDataSource { - private readonly List _endpoints; + private readonly IReadOnlyList _endpoints; /// /// Initializes a new instance of the class. /// /// The instances that the data source will return. public DefaultEndpointDataSource(params Endpoint[] endpoints) - : this((IEnumerable) endpoints) { + if (endpoints == null) + { + throw new ArgumentNullException(nameof(endpoints)); + } + + _endpoints = (Endpoint[])endpoints.Clone(); } /// @@ -36,8 +41,7 @@ namespace Microsoft.AspNetCore.Routing throw new ArgumentNullException(nameof(endpoints)); } - _endpoints = new List(); - _endpoints.AddRange(endpoints); + _endpoints = new List(endpoints); } /// diff --git a/test/Microsoft.AspNetCore.Routing.Tests/DefaultEndpointDataSourceTests.cs b/test/Microsoft.AspNetCore.Routing.Tests/DefaultEndpointDataSourceTests.cs index 09278332df..033f32ab55 100644 --- a/test/Microsoft.AspNetCore.Routing.Tests/DefaultEndpointDataSourceTests.cs +++ b/test/Microsoft.AspNetCore.Routing.Tests/DefaultEndpointDataSourceTests.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; -using System.Text; using Microsoft.AspNetCore.Http; using Xunit; @@ -26,6 +25,34 @@ namespace Microsoft.AspNetCore.Routing endpoint => Assert.Equal("2", endpoint.DisplayName)); } + [Fact] + public void Constructor_Params_ShouldMakeCopyOfEndpoints() + { + // Arrange + var endpoint1 = new Endpoint(TestConstants.EmptyRequestDelegate, EndpointMetadataCollection.Empty, "1"); + var endpoint2 = new Endpoint(TestConstants.EmptyRequestDelegate, EndpointMetadataCollection.Empty, "2"); + var endpoints = new[] { endpoint1, endpoint2 }; + + // Act + var dataSource = new DefaultEndpointDataSource(endpoints); + Array.Resize(ref endpoints, 1); + endpoints[0] = null; + + // Assert + Assert.Equal(2, dataSource.Endpoints.Count); + Assert.Contains(endpoint1, dataSource.Endpoints); + Assert.Contains(endpoint2, dataSource.Endpoints); + } + + [Fact] + public void Constructor_Params_ShouldThrowArgumentNullExceptionWhenEndpointsIsNull() + { + Endpoint[] endpoints = null; + + var actual = Assert.Throws(() => new DefaultEndpointDataSource(endpoints)); + Assert.Equal("endpoints", actual.ParamName); + } + [Fact] public void Constructor_Enumerable_EndpointsInitialized() { @@ -41,5 +68,33 @@ namespace Microsoft.AspNetCore.Routing endpoint => Assert.Equal("1", endpoint.DisplayName), endpoint => Assert.Equal("2", endpoint.DisplayName)); } + + [Fact] + public void Constructor_Enumerable_ShouldMakeCopyOfEndpoints() + { + // Arrange + var endpoint1 = new Endpoint(TestConstants.EmptyRequestDelegate, EndpointMetadataCollection.Empty, "1"); + var endpoint2 = new Endpoint(TestConstants.EmptyRequestDelegate, EndpointMetadataCollection.Empty, "2"); + var endpoints = new List { endpoint1, endpoint2 }; + + // Act + var dataSource = new DefaultEndpointDataSource((IEnumerable)endpoints); + endpoints.RemoveAt(0); + endpoints[0] = null; + + // Assert + Assert.Equal(2, dataSource.Endpoints.Count); + Assert.Contains(endpoint1, dataSource.Endpoints); + Assert.Contains(endpoint2, dataSource.Endpoints); + } + + [Fact] + public void Constructor_Enumerable_ShouldThrowArgumentNullExceptionWhenEndpointsIsNull() + { + IEnumerable endpoints = null; + + var actual = Assert.Throws(() => new DefaultEndpointDataSource(endpoints)); + Assert.Equal("endpoints", actual.ParamName); + } } } \ No newline at end of file From c93e3a76ff1529c24c1691d1ff82e605a6f95ecc Mon Sep 17 00:00:00 2001 From: Gert Driesen Date: Mon, 22 Oct 2018 23:20:30 +0200 Subject: [PATCH 3/4] Eliminate redundant isValid check from DefaultEndpointSelector.ProcessFinalCandidates(...). (#881) --- .../Matching/DefaultEndpointSelector.cs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.AspNetCore.Routing/Matching/DefaultEndpointSelector.cs b/src/Microsoft.AspNetCore.Routing/Matching/DefaultEndpointSelector.cs index f354e507a3..30882cc5f2 100644 --- a/src/Microsoft.AspNetCore.Routing/Matching/DefaultEndpointSelector.cs +++ b/src/Microsoft.AspNetCore.Routing/Matching/DefaultEndpointSelector.cs @@ -75,16 +75,20 @@ namespace Microsoft.AspNetCore.Routing.Matching int? foundScore = null; for (var i = 0; i < candidateSet.Count; i++) { + if (!candidateSet.IsValidCandidate(i)) + { + continue; + } + ref var state = ref candidateSet[i]; - var isValid = candidateSet.IsValidCandidate(i); - if (isValid && foundScore == null) + if (foundScore == null) { // This is the first match we've seen - speculatively assign it. endpoint = state.Endpoint; values = state.Values; foundScore = state.Score; } - else if (isValid && foundScore < state.Score) + else if (foundScore < state.Score) { // This candidate is lower priority than the one we've seen // so far, we can stop. @@ -92,7 +96,7 @@ namespace Microsoft.AspNetCore.Routing.Matching // Don't worry about the 'null < state.Score' case, it returns false. break; } - else if (isValid && foundScore == state.Score) + else if (foundScore == state.Score) { // This is the second match we've found of the same score, so there // must be an ambiguity. From bcbf2a1a688faef6c0b8b72ff78c4902d5d5c69e Mon Sep 17 00:00:00 2001 From: Gert Driesen Date: Mon, 22 Oct 2018 23:20:52 +0200 Subject: [PATCH 4/4] Minor performance improvement for UriBuildingContext.ToPathString() with zero-length path. (#880) --- .../Internal/UriBuildingContext.cs | 29 +++++++++++------ .../Internal/UriBuildingContextTest.cs | 31 +++++++++++++++++++ 2 files changed, 51 insertions(+), 9 deletions(-) diff --git a/src/Microsoft.AspNetCore.Routing/Internal/UriBuildingContext.cs b/src/Microsoft.AspNetCore.Routing/Internal/UriBuildingContext.cs index 1f61db30b2..82e0b4137b 100644 --- a/src/Microsoft.AspNetCore.Routing/Internal/UriBuildingContext.cs +++ b/src/Microsoft.AspNetCore.Routing/Internal/UriBuildingContext.cs @@ -242,18 +242,29 @@ namespace Microsoft.AspNetCore.Routing.Internal // Used by TemplateBinder.TryBindValues - the new code path of LinkGenerator public PathString ToPathString() { - if (_path.Length > 0 && _path[0] != '/') + PathString pathString; + + if (_path.Length > 0) { - // Normalize generated paths so that they always contain a leading slash. - _path.Insert(0, '/'); + if (_path[0] != '/') + { + // Normalize generated paths so that they always contain a leading slash. + _path.Insert(0, '/'); + } + + if (AppendTrailingSlash && _path[_path.Length - 1] != '/') + { + _path.Append('/'); + } + + pathString = new PathString(_path.ToString()); + } + else + { + pathString = PathString.Empty; } - if (AppendTrailingSlash && _path.Length > 0 && _path[_path.Length - 1] != '/') - { - _path.Append('/'); - } - - return new PathString(_path.ToString()); + return pathString; } // Used by TemplateBinder.TryBindValues - the new code path of LinkGenerator diff --git a/test/Microsoft.AspNetCore.Routing.Tests/Internal/UriBuildingContextTest.cs b/test/Microsoft.AspNetCore.Routing.Tests/Internal/UriBuildingContextTest.cs index 137520051c..cfd1531a5d 100644 --- a/test/Microsoft.AspNetCore.Routing.Tests/Internal/UriBuildingContextTest.cs +++ b/test/Microsoft.AspNetCore.Routing.Tests/Internal/UriBuildingContextTest.cs @@ -65,5 +65,36 @@ namespace Microsoft.AspNetCore.Routing.Internal // Assert Assert.Equal(expected, uriBuilldingContext.ToString()); } + + [Theory] + [InlineData("/Author", false, false, "/UrlEncode[[Author]]")] + [InlineData("/Author", false, true, "/UrlEncode[[Author]]")] + [InlineData("/Author", true, false, "/UrlEncode[[Author]]/")] + [InlineData("/Author", true, true, "/UrlEncode[[Author]]/")] + [InlineData("/Author/", false, false, "/UrlEncode[[Author]]/")] + [InlineData("/Author/", false, true, "/UrlEncode[[Author/]]")] + [InlineData("/Author/", true, false, "/UrlEncode[[Author]]/")] + [InlineData("/Author/", true, true, "/UrlEncode[[Author/]]/")] + [InlineData("Author", false, false, "/UrlEncode[[Author]]")] + [InlineData("Author", false, true, "/UrlEncode[[Author]]")] + [InlineData("Author", true, false, "/UrlEncode[[Author]]/")] + [InlineData("Author", true, true, "/UrlEncode[[Author]]/")] + [InlineData("", false, false, "")] + [InlineData("", false, true, "")] + [InlineData("", true, false, "")] + [InlineData("", true, true, "")] + public void ToPathString(string url, bool appendTrailingSlash, bool encodeSlashes, string expected) + { + // Arrange + var urlTestEncoder = new UrlTestEncoder(); + var uriBuilldingContext = new UriBuildingContext(urlTestEncoder); + uriBuilldingContext.AppendTrailingSlash = appendTrailingSlash; + + // Act + uriBuilldingContext.Accept(url, encodeSlashes); + + // Assert + Assert.Equal(expected, uriBuilldingContext.ToPathString().Value); + } } }