From a5658a8c95c4f8868e7e6db54303f278d34efe87 Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Sun, 13 Jan 2019 21:03:26 -0800 Subject: [PATCH] Fix #6102 - Intense CPU utilization on page change (#6542) * Fix #6102 - Intense CPU utilization on page change The issue here was that every time a Razor Page changed, we would subscribe an additional time to the endpoint change notifications. This means that if you tweaked a page 30 times, we would update the address table 31 times when you save the file. If you were doing a lot of editing then this would grow to a really large amount of computation. The fix is to use DataSourceDependentCache, which is an existing utility type we developed for this purpose. I'm not sure why it wasn't being used for this already. We're already using DataSourceDependentCache in a bunch of other places, and it's well tested. I also tweaked the stucture of this code to be more similar to EndpointNameAddressScheme. This involved some test changes that all seemed like good cleanup. The way this was being tested was a little wonky. --- .../Routing/src/DataSourceDependentCache.cs | 37 ++++- src/Http/Routing/src/DefaultLinkGenerator.cs | 7 +- .../RoutingServiceCollectionExtensions.cs | 3 +- .../Routing/src/EndpointNameAddressScheme.cs | 9 +- .../Matching/DataSourceDependentMatcher.cs | 44 +++++- .../Routing/src/Matching/DfaMatcherFactory.cs | 8 +- .../Routing/src/RouteValuesAddressScheme.cs | 127 +++++++++--------- .../UnitTests/DataSourceDependentCacheTest.cs | 47 ++++++- .../DataSourceDependentMatcherTest.cs | 68 +++++----- .../UnitTests/RouteValuesAddressSchemeTest.cs | 94 ++++++------- 10 files changed, 289 insertions(+), 155 deletions(-) diff --git a/src/Http/Routing/src/DataSourceDependentCache.cs b/src/Http/Routing/src/DataSourceDependentCache.cs index 4daf256327..2a1068c0fb 100644 --- a/src/Http/Routing/src/DataSourceDependentCache.cs +++ b/src/Http/Routing/src/DataSourceDependentCache.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// 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; @@ -8,7 +8,7 @@ using Microsoft.AspNetCore.Http; namespace Microsoft.AspNetCore.Routing { - internal class DataSourceDependentCache where T : class + internal class DataSourceDependentCache : IDisposable where T : class { private readonly EndpointDataSource _dataSource; private readonly Func, T> _initializeCore; @@ -18,6 +18,9 @@ namespace Microsoft.AspNetCore.Routing private object _lock; private bool _initialized; private T _value; + + private IDisposable _disposable; + private bool _disposed; public DataSourceDependentCache(EndpointDataSource dataSource, Func, T> initialize) { @@ -26,6 +29,11 @@ namespace Microsoft.AspNetCore.Routing throw new ArgumentNullException(nameof(dataSource)); } + if (initialize == null) + { + throw new ArgumentNullException(nameof(initialize)); + } + _dataSource = dataSource; _initializeCore = initialize; @@ -51,9 +59,32 @@ namespace Microsoft.AspNetCore.Routing var changeToken = _dataSource.GetChangeToken(); _value = _initializeCore(_dataSource.Endpoints); - changeToken.RegisterChangeCallback(_initializerWithState, null); + // Don't resubscribe if we're already disposed. + if (_disposed) + { + return _value; + } + + _disposable = changeToken.RegisterChangeCallback(_initializerWithState, null); return _value; } } + + public void Dispose() + { + lock (_lock) + { + if (!_disposed) + { + _disposable?.Dispose(); + _disposable = null; + + // Tracking whether we're disposed or not prevents a race-condition + // between disposal and Initialize(). If a change callback fires after + // we dispose, then we don't want to reregister. + _disposed = true; + } + } + } } } diff --git a/src/Http/Routing/src/DefaultLinkGenerator.cs b/src/Http/Routing/src/DefaultLinkGenerator.cs index 7670b7c6b4..21f36cac63 100644 --- a/src/Http/Routing/src/DefaultLinkGenerator.cs +++ b/src/Http/Routing/src/DefaultLinkGenerator.cs @@ -19,7 +19,7 @@ using Microsoft.Extensions.Options; namespace Microsoft.AspNetCore.Routing { - internal sealed class DefaultLinkGenerator : LinkGenerator + internal sealed class DefaultLinkGenerator : LinkGenerator, IDisposable { private readonly ParameterPolicyFactory _parameterPolicyFactory; private readonly ObjectPool _uriBuildingContextPool; @@ -364,6 +364,11 @@ namespace Microsoft.AspNetCore.Routing return httpContext?.Features.Get()?.RouteValues; } + public void Dispose() + { + _cache.Dispose(); + } + private static class Log { public static class EventIds diff --git a/src/Http/Routing/src/DependencyInjection/RoutingServiceCollectionExtensions.cs b/src/Http/Routing/src/DependencyInjection/RoutingServiceCollectionExtensions.cs index e18850d0df..1a11d5ebec 100644 --- a/src/Http/Routing/src/DependencyInjection/RoutingServiceCollectionExtensions.cs +++ b/src/Http/Routing/src/DependencyInjection/RoutingServiceCollectionExtensions.cs @@ -76,6 +76,7 @@ namespace Microsoft.Extensions.DependencyInjection services.TryAddSingleton(); services.TryAddTransient(); services.TryAddSingleton(); + services.TryAddTransient(); // Link generation related services services.TryAddSingleton(); @@ -93,7 +94,7 @@ namespace Microsoft.Extensions.DependencyInjection // Misc infrastructure // services.TryAddSingleton(); - + return services; } diff --git a/src/Http/Routing/src/EndpointNameAddressScheme.cs b/src/Http/Routing/src/EndpointNameAddressScheme.cs index 5fd81b40c2..e45adf4eb6 100644 --- a/src/Http/Routing/src/EndpointNameAddressScheme.cs +++ b/src/Http/Routing/src/EndpointNameAddressScheme.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// 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; @@ -9,7 +9,7 @@ using Microsoft.AspNetCore.Http; namespace Microsoft.AspNetCore.Routing { - internal class EndpointNameAddressScheme : IEndpointAddressScheme + internal class EndpointNameAddressScheme : IEndpointAddressScheme, IDisposable { private readonly DataSourceDependentCache> _cache; @@ -103,5 +103,10 @@ namespace Microsoft.AspNetCore.Routing return endpoint.Metadata.GetMetadata()?.EndpointName; } } + + public void Dispose() + { + _cache.Dispose(); + } } } diff --git a/src/Http/Routing/src/Matching/DataSourceDependentMatcher.cs b/src/Http/Routing/src/Matching/DataSourceDependentMatcher.cs index 89c11090ba..8716da5752 100644 --- a/src/Http/Routing/src/Matching/DataSourceDependentMatcher.cs +++ b/src/Http/Routing/src/Matching/DataSourceDependentMatcher.cs @@ -1,11 +1,10 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// 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; using System.Collections.Generic; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; -using Microsoft.AspNetCore.Http.Features; namespace Microsoft.AspNetCore.Routing.Matching { @@ -16,12 +15,17 @@ namespace Microsoft.AspNetCore.Routing.Matching public DataSourceDependentMatcher( EndpointDataSource dataSource, + Lifetime lifetime, Func matcherBuilderFactory) { _matcherBuilderFactory = matcherBuilderFactory; _cache = new DataSourceDependentCache(dataSource, CreateMatcher); _cache.EnsureInitialized(); + + // This will Dispose the cache when the lifetime is disposed, this allows + // the service provider to manage the lifetime of the cache. + lifetime.Cache = _cache; } // Used in tests @@ -48,5 +52,41 @@ namespace Microsoft.AspNetCore.Routing.Matching return builder.Build(); } + + // Used to tie the lifetime of a DataSourceDependentCache to the service provider + public class Lifetime : IDisposable + { + private readonly object _lock = new object(); + private DataSourceDependentCache _cache; + private bool _disposed; + + public DataSourceDependentCache Cache + { + get => _cache; + set + { + lock (_lock) + { + if (_disposed) + { + value?.Dispose(); + } + + _cache = value; + } + } + } + + public void Dispose() + { + lock (_lock) + { + _cache?.Dispose(); + _cache = null; + + _disposed = true; + } + } + } } } diff --git a/src/Http/Routing/src/Matching/DfaMatcherFactory.cs b/src/Http/Routing/src/Matching/DfaMatcherFactory.cs index c4b932d3df..9fa684d944 100644 --- a/src/Http/Routing/src/Matching/DfaMatcherFactory.cs +++ b/src/Http/Routing/src/Matching/DfaMatcherFactory.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// 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; @@ -29,7 +29,11 @@ namespace Microsoft.AspNetCore.Routing.Matching throw new ArgumentNullException(nameof(dataSource)); } - return new DataSourceDependentMatcher(dataSource, () => + // Creates a tracking entry in DI to stop listening for change events + // when the services are disposed. + var lifetime = _services.GetRequiredService(); + + return new DataSourceDependentMatcher(dataSource, lifetime, () => { return _services.GetRequiredService(); }); diff --git a/src/Http/Routing/src/RouteValuesAddressScheme.cs b/src/Http/Routing/src/RouteValuesAddressScheme.cs index 6d77645dd1..3d173872f0 100644 --- a/src/Http/Routing/src/RouteValuesAddressScheme.cs +++ b/src/Http/Routing/src/RouteValuesAddressScheme.cs @@ -1,46 +1,44 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// 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; using System.Collections.Generic; -using System.Linq; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Routing.Internal; using Microsoft.AspNetCore.Routing.Template; using Microsoft.AspNetCore.Routing.Tree; -using Microsoft.Extensions.Primitives; namespace Microsoft.AspNetCore.Routing { - internal class RouteValuesAddressScheme : IEndpointAddressScheme + internal class RouteValuesAddressScheme : IEndpointAddressScheme, IDisposable { - private readonly EndpointDataSource _dataSource; - private LinkGenerationDecisionTree _allMatchesLinkGenerationTree; - private Dictionary> _namedMatchResults; - + private readonly DataSourceDependentCache _cache; + public RouteValuesAddressScheme(EndpointDataSource dataSource) { - _dataSource = dataSource; - - // Build initial matches - BuildOutboundMatches(); - - // Register for changes in endpoints - ChangeToken.OnChange( - _dataSource.GetChangeToken, - HandleChange); + _cache = new DataSourceDependentCache(dataSource, Initialize); } + // Internal for tests + internal StateEntry State => _cache.EnsureInitialized(); + public IEnumerable FindEndpoints(RouteValuesAddress address) { + if (address == null) + { + throw new ArgumentNullException(nameof(address)); + } + + var state = State; + IList matchResults = null; if (string.IsNullOrEmpty(address.RouteName)) { - matchResults = _allMatchesLinkGenerationTree.GetMatches( + matchResults = state.AllMatchesLinkGenerationTree.GetMatches( address.ExplicitValues, address.AmbientValues); } - else if (_namedMatchResults.TryGetValue(address.RouteName, out var namedMatchResults)) + else if (state.NamedMatches.TryGetValue(address.RouteName, out var namedMatchResults)) { matchResults = namedMatchResults; } @@ -74,52 +72,31 @@ namespace Microsoft.AspNetCore.Routing } } - private void HandleChange() - { - // rebuild the matches - BuildOutboundMatches(); - - // re-register the callback as the change token is one time use only and a new change token - // is produced every time - ChangeToken.OnChange( - _dataSource.GetChangeToken, - HandleChange); - } - - private void BuildOutboundMatches() - { - // Refresh the matches in the case where a datasource's endpoints changes. The following is OK to do - // as refresh of new endpoints happens within a lock and also these fields are not publicly accessible. - var (allMatches, namedMatchResults) = GetOutboundMatches(); - _namedMatchResults = namedMatchResults; - _allMatchesLinkGenerationTree = new LinkGenerationDecisionTree(allMatches); - } - - /// Decision tree is built using the 'required values' of actions. - /// - When generating a url using route values, decision tree checks the explicitly supplied route values + - /// ambient values to see if they have a match for the required-values-based-tree. - /// - When generating a url using route name, route values for controller, action etc.might not be provided - /// (this is expected because as a user I want to avoid writing all those and instead chose to use a - /// routename which is quick). So since these values are not provided and might not be even in ambient - /// values, decision tree would fail to find a match. So for this reason decision tree is not used for named - /// matches. Instead all named matches are returned as is and the LinkGenerator uses a TemplateBinder to - /// decide which of the matches can generate a url. - /// For example, for a route defined like below with current ambient values like new { controller = "Home", - /// action = "Index" } - /// "api/orders/{id}", - /// routeName: "OrdersApi", - /// defaults: new { controller = "Orders", action = "GetById" }, - /// requiredValues: new { controller = "Orders", action = "GetById" }, - /// A call to GetLink("OrdersApi", new { id = "10" }) cannot generate url as neither the supplied values or - /// current ambient values do not satisfy the decision tree that is built based on the required values. - protected virtual (List, Dictionary>) GetOutboundMatches() + private StateEntry Initialize(IReadOnlyList endpoints) { var allOutboundMatches = new List(); - var namedOutboundMatchResults = new Dictionary>( - StringComparer.OrdinalIgnoreCase); + var namedOutboundMatchResults = new Dictionary>(StringComparer.OrdinalIgnoreCase); - foreach (var endpoint in _dataSource.Endpoints) + // Decision tree is built using the 'required values' of actions. + // - When generating a url using route values, decision tree checks the explicitly supplied route values + + // ambient values to see if they have a match for the required-values-based-tree. + // - When generating a url using route name, route values for controller, action etc.might not be provided + // (this is expected because as a user I want to avoid writing all those and instead chose to use a + // routename which is quick). So since these values are not provided and might not be even in ambient + // values, decision tree would fail to find a match. So for this reason decision tree is not used for named + // matches. Instead all named matches are returned as is and the LinkGenerator uses a TemplateBinder to + // decide which of the matches can generate a url. + // For example, for a route defined like below with current ambient values like new { controller = "Home", + // action = "Index" } + // "api/orders/{id}", + // routeName: "OrdersApi", + // defaults: new { controller = "Orders", action = "GetById" }, + // requiredValues: new { controller = "Orders", action = "GetById" }, + // A call to GetLink("OrdersApi", new { id = "10" }) cannot generate url as neither the supplied values or + // current ambient values do not satisfy the decision tree that is built based on the required values. + for (var i = 0; i < endpoints.Count; i++) { + var endpoint = endpoints[i]; if (!(endpoint is RouteEndpoint routeEndpoint)) { continue; @@ -157,7 +134,10 @@ namespace Microsoft.AspNetCore.Routing matchResults.Add(new OutboundMatchResult(outboundMatch, isFallbackMatch: false)); } - return (allOutboundMatches, namedOutboundMatchResults); + return new StateEntry( + allOutboundMatches, + new LinkGenerationDecisionTree(allOutboundMatches), + namedOutboundMatchResults); } private OutboundRouteEntry CreateOutboundRouteEntry( @@ -178,5 +158,28 @@ namespace Microsoft.AspNetCore.Routing entry.Defaults = new RouteValueDictionary(endpoint.RoutePattern.Defaults); return entry; } + + public void Dispose() + { + _cache.Dispose(); + } + + internal class StateEntry + { + // For testing + public readonly List AllMatches; + public readonly LinkGenerationDecisionTree AllMatchesLinkGenerationTree; + public readonly Dictionary> NamedMatches; + + public StateEntry( + List allMatches, + LinkGenerationDecisionTree allMatchesLinkGenerationTree, + Dictionary> namedMatches) + { + AllMatches = allMatches; + AllMatchesLinkGenerationTree = allMatchesLinkGenerationTree; + NamedMatches = namedMatches; + } + } } } diff --git a/src/Http/Routing/test/UnitTests/DataSourceDependentCacheTest.cs b/src/Http/Routing/test/UnitTests/DataSourceDependentCacheTest.cs index 33da589661..3fd4c75191 100644 --- a/src/Http/Routing/test/UnitTests/DataSourceDependentCacheTest.cs +++ b/src/Http/Routing/test/UnitTests/DataSourceDependentCacheTest.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// 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; @@ -76,5 +76,50 @@ namespace Microsoft.AspNetCore.Routing Assert.Equal(2, count); Assert.Equal("hello, 2!", cache.Value); } + + [Fact] + public void Cache_CanDispose_WhenUninitialized() + { + // Arrange + var count = 0; + + var dataSource = new DynamicEndpointDataSource(); + var cache = new DataSourceDependentCache(dataSource, (endpoints) => + { + count++; + return $"hello, {count}!"; + }); + + // Act + cache.Dispose(); + + // Assert + dataSource.AddEndpoint(null); + Assert.Null(cache.Value); + } + + [Fact] + public void Cache_CanDispose_WhenInitialized() + { + // Arrange + var count = 0; + + var dataSource = new DynamicEndpointDataSource(); + var cache = new DataSourceDependentCache(dataSource, (endpoints) => + { + count++; + return $"hello, {count}!"; + }); + + cache.EnsureInitialized(); + Assert.Equal("hello, 1!", cache.Value); + + // Act + cache.Dispose(); + + // Assert + dataSource.AddEndpoint(null); + Assert.Equal("hello, 1!", cache.Value); // Ignores update + } } } diff --git a/src/Http/Routing/test/UnitTests/Matching/DataSourceDependentMatcherTest.cs b/src/Http/Routing/test/UnitTests/Matching/DataSourceDependentMatcherTest.cs index 3559dd529b..da46b300ef 100644 --- a/src/Http/Routing/test/UnitTests/Matching/DataSourceDependentMatcherTest.cs +++ b/src/Http/Routing/test/UnitTests/Matching/DataSourceDependentMatcherTest.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// 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; @@ -18,13 +18,16 @@ namespace Microsoft.AspNetCore.Routing.Matching { // Arrange var dataSource = new DynamicEndpointDataSource(); + var lifetime = new DataSourceDependentMatcher.Lifetime(); // Act - var matcher = new DataSourceDependentMatcher(dataSource, TestMatcherBuilder.Create); + var matcher = new DataSourceDependentMatcher(dataSource, lifetime, TestMatcherBuilder.Create); // Assert var inner = Assert.IsType(matcher.CurrentMatcher); Assert.Empty(inner.Endpoints); + + Assert.NotNull(lifetime.Cache); } [Fact] @@ -32,7 +35,8 @@ namespace Microsoft.AspNetCore.Routing.Matching { // Arrange var dataSource = new DynamicEndpointDataSource(); - var matcher = new DataSourceDependentMatcher(dataSource, TestMatcherBuilder.Create); + var lifetime = new DataSourceDependentMatcher.Lifetime(); + var matcher = new DataSourceDependentMatcher(dataSource, lifetime, TestMatcherBuilder.Create); var endpoint = new RouteEndpoint( TestConstants.EmptyRequestDelegate, @@ -51,16 +55,42 @@ namespace Microsoft.AspNetCore.Routing.Matching e => Assert.Same(endpoint, e)); } + [Fact] + public void Matcher_IgnoresUpdate_WhenDisposed() + { + // Arrange + var dataSource = new DynamicEndpointDataSource(); + var lifetime = new DataSourceDependentMatcher.Lifetime(); + var matcher = new DataSourceDependentMatcher(dataSource, lifetime, TestMatcherBuilder.Create); + + var endpoint = new RouteEndpoint( + TestConstants.EmptyRequestDelegate, + RoutePatternFactory.Parse("a/b/c"), + 0, + EndpointMetadataCollection.Empty, + "test"); + + lifetime.Dispose(); + + // Act + dataSource.AddEndpoint(endpoint); + + // Assert + var inner = Assert.IsType(matcher.CurrentMatcher); + Assert.Empty(inner.Endpoints); + } + [Fact] public void Matcher_Ignores_NonRouteEndpoint() { // Arrange var dataSource = new DynamicEndpointDataSource(); + var lifetime = new DataSourceDependentMatcher.Lifetime(); var endpoint = new Endpoint(TestConstants.EmptyRequestDelegate, EndpointMetadataCollection.Empty, "test"); dataSource.AddEndpoint(endpoint); // Act - var matcher = new DataSourceDependentMatcher(dataSource, TestMatcherBuilder.Create); + var matcher = new DataSourceDependentMatcher(dataSource, lifetime, TestMatcherBuilder.Create); // Assert var inner = Assert.IsType(matcher.CurrentMatcher); @@ -72,6 +102,7 @@ namespace Microsoft.AspNetCore.Routing.Matching { // Arrange var dataSource = new DynamicEndpointDataSource(); + var lifetime = new DataSourceDependentMatcher.Lifetime(); var endpoint = new RouteEndpoint( TestConstants.EmptyRequestDelegate, RoutePatternFactory.Parse("/"), @@ -81,7 +112,7 @@ namespace Microsoft.AspNetCore.Routing.Matching dataSource.AddEndpoint(endpoint); // Act - var matcher = new DataSourceDependentMatcher(dataSource, TestMatcherBuilder.Create); + var matcher = new DataSourceDependentMatcher(dataSource, lifetime, TestMatcherBuilder.Create); // Assert var inner = Assert.IsType(matcher.CurrentMatcher); @@ -93,6 +124,7 @@ namespace Microsoft.AspNetCore.Routing.Matching { // Arrange var dataSource = new DynamicEndpointDataSource(); + var lifetime = new DataSourceDependentMatcher.Lifetime(); var endpoint = new RouteEndpoint( TestConstants.EmptyRequestDelegate, RoutePatternFactory.Parse("/"), @@ -102,37 +134,13 @@ namespace Microsoft.AspNetCore.Routing.Matching dataSource.AddEndpoint(endpoint); // Act - var matcher = new DataSourceDependentMatcher(dataSource, TestMatcherBuilder.Create); + var matcher = new DataSourceDependentMatcher(dataSource, lifetime, TestMatcherBuilder.Create); // Assert var inner = Assert.IsType(matcher.CurrentMatcher); Assert.Same(endpoint, Assert.Single(inner.Endpoints)); } - [Fact] - public void Cache_Reinitializes_WhenDataSourceChanges() - { - // Arrange - var count = 0; - - var dataSource = new DynamicEndpointDataSource(); - var cache = new DataSourceDependentCache(dataSource, (endpoints) => - { - count++; - return $"hello, {count}!"; - }); - - cache.EnsureInitialized(); - Assert.Equal("hello, 1!", cache.Value); - - // Act - dataSource.AddEndpoint(null); - - // Assert - Assert.Equal(2, count); - Assert.Equal("hello, 2!", cache.Value); - } - private class TestMatcherBuilder : MatcherBuilder { public static Func Create = () => new TestMatcherBuilder(); diff --git a/src/Http/Routing/test/UnitTests/RouteValuesAddressSchemeTest.cs b/src/Http/Routing/test/UnitTests/RouteValuesAddressSchemeTest.cs index 4b15a1aca3..023b594465 100644 --- a/src/Http/Routing/test/UnitTests/RouteValuesAddressSchemeTest.cs +++ b/src/Http/Routing/test/UnitTests/RouteValuesAddressSchemeTest.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// 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; @@ -25,10 +25,10 @@ namespace Microsoft.AspNetCore.Routing var addressScheme = CreateAddressScheme(endpoint1, endpoint2); // Assert - Assert.NotNull(addressScheme.AllMatches); - Assert.Equal(2, addressScheme.AllMatches.Count()); - Assert.NotNull(addressScheme.NamedMatches); - Assert.True(addressScheme.NamedMatches.TryGetValue("named", out var namedMatches)); + Assert.NotNull(addressScheme.State.AllMatches); + Assert.Equal(2, addressScheme.State.AllMatches.Count()); + Assert.NotNull(addressScheme.State.NamedMatches); + Assert.True(addressScheme.State.NamedMatches.TryGetValue("named", out var namedMatches)); var namedMatch = Assert.Single(namedMatches); var actual = Assert.IsType(namedMatch.Match.Entry.Data); Assert.Same(endpoint2, actual); @@ -46,10 +46,10 @@ namespace Microsoft.AspNetCore.Routing var addressScheme = CreateAddressScheme(endpoint1, endpoint2, endpoint3); // Assert - Assert.NotNull(addressScheme.AllMatches); - Assert.Equal(3, addressScheme.AllMatches.Count()); - Assert.NotNull(addressScheme.NamedMatches); - Assert.True(addressScheme.NamedMatches.TryGetValue("named", out var namedMatches)); + Assert.NotNull(addressScheme.State.AllMatches); + Assert.Equal(3, addressScheme.State.AllMatches.Count()); + Assert.NotNull(addressScheme.State.NamedMatches); + Assert.True(addressScheme.State.NamedMatches.TryGetValue("named", out var namedMatches)); Assert.Equal(2, namedMatches.Count); Assert.Same(endpoint2, Assert.IsType(namedMatches[0].Match.Entry.Data)); Assert.Same(endpoint3, Assert.IsType(namedMatches[1].Match.Entry.Data)); @@ -67,10 +67,10 @@ namespace Microsoft.AspNetCore.Routing var addressScheme = CreateAddressScheme(endpoint1, endpoint2, endpoint3); // Assert - Assert.NotNull(addressScheme.AllMatches); - Assert.Equal(3, addressScheme.AllMatches.Count()); - Assert.NotNull(addressScheme.NamedMatches); - Assert.True(addressScheme.NamedMatches.TryGetValue("named", out var namedMatches)); + Assert.NotNull(addressScheme.State.AllMatches); + Assert.Equal(3, addressScheme.State.AllMatches.Count()); + Assert.NotNull(addressScheme.State.NamedMatches); + Assert.True(addressScheme.State.NamedMatches.TryGetValue("named", out var namedMatches)); Assert.Equal(2, namedMatches.Count); Assert.Same(endpoint2, Assert.IsType(namedMatches[0].Match.Entry.Data)); Assert.Same(endpoint3, Assert.IsType(namedMatches[1].Match.Entry.Data)); @@ -84,11 +84,12 @@ namespace Microsoft.AspNetCore.Routing var dynamicDataSource = new DynamicEndpointDataSource(new[] { endpoint1 }); // Act 1 - var addressScheme = new CustomRouteValuesBasedAddressScheme(new CompositeEndpointDataSource(new[] { dynamicDataSource })); + var addressScheme = new RouteValuesAddressScheme(new CompositeEndpointDataSource(new[] { dynamicDataSource })); // Assert 1 - Assert.NotNull(addressScheme.AllMatches); - var match = Assert.Single(addressScheme.AllMatches); + var state = addressScheme.State; + Assert.NotNull(state.AllMatches); + var match = Assert.Single(state.AllMatches); var actual = Assert.IsType(match.Entry.Data); Assert.Same(endpoint1, actual); @@ -99,24 +100,35 @@ namespace Microsoft.AspNetCore.Routing // Trigger change dynamicDataSource.AddEndpoint(endpoint2); - // Arrange 2 - var endpoint3 = CreateEndpoint("/c", routeName: "c"); - - // Act 2 - // Trigger change - dynamicDataSource.AddEndpoint(endpoint3); + // Assert 2 + Assert.NotSame(state, addressScheme.State); + state = addressScheme.State; // Arrange 3 - var endpoint4 = CreateEndpoint("/d", routeName: "d"); + var endpoint3 = CreateEndpoint("/c", routeName: "c"); // Act 3 // Trigger change - dynamicDataSource.AddEndpoint(endpoint4); + dynamicDataSource.AddEndpoint(endpoint3); // Assert 3 - Assert.NotNull(addressScheme.AllMatches); + Assert.NotSame(state, addressScheme.State); + state = addressScheme.State; + + // Arrange 4 + var endpoint4 = CreateEndpoint("/d", routeName: "d"); + + // Act 4 + // Trigger change + dynamicDataSource.AddEndpoint(endpoint4); + + // Assert 4 + Assert.NotSame(state, addressScheme.State); + state = addressScheme.State; + + Assert.NotNull(state.AllMatches); Assert.Collection( - addressScheme.AllMatches, + state.AllMatches, (m) => { actual = Assert.IsType(m.Entry.Data); @@ -337,7 +349,7 @@ namespace Microsoft.AspNetCore.Routing var addressScheme = CreateAddressScheme(endpoint); // Assert - Assert.Empty(addressScheme.AllMatches); + Assert.Empty(addressScheme.State.AllMatches); } [Fact] @@ -352,17 +364,17 @@ namespace Microsoft.AspNetCore.Routing var addressScheme = CreateAddressScheme(endpoint); // Assert - Assert.Same(endpoint, Assert.Single(addressScheme.AllMatches).Entry.Data); + Assert.Same(endpoint, Assert.Single(addressScheme.State.AllMatches).Entry.Data); } - private CustomRouteValuesBasedAddressScheme CreateAddressScheme(params Endpoint[] endpoints) + private RouteValuesAddressScheme CreateAddressScheme(params Endpoint[] endpoints) { return CreateAddressScheme(new DefaultEndpointDataSource(endpoints)); } - private CustomRouteValuesBasedAddressScheme CreateAddressScheme(params EndpointDataSource[] dataSources) + private RouteValuesAddressScheme CreateAddressScheme(params EndpointDataSource[] dataSources) { - return new CustomRouteValuesBasedAddressScheme(new CompositeEndpointDataSource(dataSources)); + return new RouteValuesAddressScheme(new CompositeEndpointDataSource(dataSources)); } private RouteEndpoint CreateEndpoint( @@ -391,26 +403,6 @@ namespace Microsoft.AspNetCore.Routing null); } - private class CustomRouteValuesBasedAddressScheme : RouteValuesAddressScheme - { - public CustomRouteValuesBasedAddressScheme(CompositeEndpointDataSource dataSource) - : base(dataSource) - { - } - - public IEnumerable AllMatches { get; private set; } - - public IDictionary> NamedMatches { get; private set; } - - protected override (List, Dictionary>) GetOutboundMatches() - { - var matches = base.GetOutboundMatches(); - AllMatches = matches.Item1; - NamedMatches = matches.Item2; - return matches; - } - } - private class EncourageLinkGenerationMetadata : ISuppressLinkGenerationMetadata { public bool SuppressLinkGeneration => false;