Fix aspnet/Routing#722

Exposes a separate change token that will be triggered after action
descriptors have been updated.

benchmarks/Microsoft.AspNetCore.Mvc.Performance/MvcEndpointDatasourceBenchmark.cs

@@ -111,7 +111,6 @@ namespace Microsoft.AspNetCore.Mvc.Performance
             var dataSource = new MvcEndpointDataSource(
                 actionDescriptorCollectionProvider,
                 new MvcEndpointInvokerFactory(new ActionInvokerFactory(Array.Empty<IActionInvokerProvider>())),
-                Array.Empty<IActionDescriptorChangeProvider>(),
                 new MockServiceProvider());
 
             return dataSource;

src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs

@@ -165,7 +165,7 @@ namespace Microsoft.Extensions.DependencyInjection
             services.TryAddEnumerable(
                 ServiceDescriptor.Transient<IActionDescriptorProvider, ControllerActionDescriptorProvider>());
 
-            services.TryAddSingleton<IActionDescriptorCollectionProvider, ActionDescriptorCollectionProvider>();
+            services.TryAddSingleton<IActionDescriptorCollectionProvider, DefaultActionDescriptorCollectionProvider>();
 
             //
             // Action Selection

src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ActionDescriptorCollectionProvider.cs

@@ -0,0 +1,33 @@
+// 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.Mvc.Abstractions;
+using Microsoft.Extensions.Primitives;
+
+namespace Microsoft.AspNetCore.Mvc.Infrastructure
+{
+    /// <summary>
+    /// A base class for <see cref="IActionDescriptorCollectionProvider"/> which also provides an <see cref="IChangeToken"/>
+    /// for reactive notifications of <see cref="ActionDescriptor"/> changes.
+    /// </summary>
+    /// <remarks>
+    /// <see cref="ActionDescriptorCollectionProvider"/> is used as a base class by the default implementation of
+    /// <see cref="IActionDescriptorCollectionProvider"/>. To retrieve an instance of <see cref="ActionDescriptorCollectionProvider"/>,
+    /// obtain the <see cref="IActionDescriptorCollectionProvider"/> from the dependency injection provider and
+    /// downcast to <see cref="ActionDescriptorCollectionProvider"/>.
+    /// </remarks>
+    public abstract class ActionDescriptorCollectionProvider : IActionDescriptorCollectionProvider
+    {
+        /// <summary>
+        /// Returns the current cached <see cref="ActionDescriptorCollection"/>
+        /// </summary>
+        public abstract ActionDescriptorCollection ActionDescriptors { get; }
+
+        /// <summary>
+        /// Gets an <see cref="IChangeToken"/> that will be signaled after the <see cref="ActionDescriptors"/>
+        /// collection has changed.
+        /// </summary>
+        /// <returns>The <see cref="IChangeToken"/>.</returns>
+        public abstract IChangeToken GetChangeToken();
+    }
+}

src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/DefaultActionDescriptorCollectionProvider.cs

@@ -0,0 +1,160 @@
+// 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.Collections.ObjectModel;
+using System.Diagnostics;
+using System.Linq;
+using System.Threading;
+using Microsoft.AspNetCore.Mvc.Abstractions;
+using Microsoft.Extensions.Primitives;
+
+namespace Microsoft.AspNetCore.Mvc.Infrastructure
+{
+    internal class DefaultActionDescriptorCollectionProvider : ActionDescriptorCollectionProvider
+    {
+        private readonly IActionDescriptorProvider[] _actionDescriptorProviders;
+        private readonly IActionDescriptorChangeProvider[] _actionDescriptorChangeProviders;
+
+        // The lock is used to protect WRITES to the following (do not need to protect reads once initialized).
+        private readonly object _lock;
+        private ActionDescriptorCollection _collection;
+        private IChangeToken _changeToken;
+        private CancellationTokenSource _cancellationTokenSource;
+        private int _version = 0;
+
+        public DefaultActionDescriptorCollectionProvider(
+            IEnumerable<IActionDescriptorProvider> actionDescriptorProviders,
+            IEnumerable<IActionDescriptorChangeProvider> actionDescriptorChangeProviders)
+        {
+            _actionDescriptorProviders = actionDescriptorProviders
+                .OrderBy(p => p.Order)
+                .ToArray();
+
+            _actionDescriptorChangeProviders = actionDescriptorChangeProviders.ToArray();
+
+            ChangeToken.OnChange(
+                GetCompositeChangeToken,
+                UpdateCollection);
+
+            _lock = new object();
+        }
+
+        /// <summary>
+        /// Returns a cached collection of <see cref="ActionDescriptor" />.
+        /// </summary>
+        public override ActionDescriptorCollection ActionDescriptors
+        {
+            get
+            {
+                Initialize();
+                Debug.Assert(_collection != null);
+                Debug.Assert(_changeToken != null);
+
+                return _collection;
+            }
+        }
+
+        /// <summary>
+        /// Gets an <see cref="IChangeToken"/> that will be signaled after the <see cref="ActionDescriptors"/>
+        /// collection has changed.
+        /// </summary>
+        /// <returns>The <see cref="IChangeToken"/>.</returns>
+        public override IChangeToken GetChangeToken()
+        {
+            Initialize();
+            Debug.Assert(_collection != null);
+            Debug.Assert(_changeToken != null);
+
+            return _changeToken;
+        }
+
+        private IChangeToken GetCompositeChangeToken()
+        {
+            if (_actionDescriptorChangeProviders.Length == 1)
+            {
+                return _actionDescriptorChangeProviders[0].GetChangeToken();
+            }
+
+            var changeTokens = new IChangeToken[_actionDescriptorChangeProviders.Length];
+            for (var i = 0; i < _actionDescriptorChangeProviders.Length; i++)
+            {
+                changeTokens[i] = _actionDescriptorChangeProviders[i].GetChangeToken();
+            }
+
+            return new CompositeChangeToken(changeTokens);
+        }
+        
+        private void Initialize()
+        {
+            // Using double-checked locking on initialization because we fire change token callbacks
+            // when the collection changes. We don't want to do that repeatedly for redundant changes.
+            //
+            // The main call path of this code on the first call is async initialization from Endpoint Routing
+            // which is done in a non-blocking way so in practice no caller will ever block here.
+            if (_collection == null)
+            {
+                lock (_lock)
+                {
+                    if (_collection == null)
+                    {
+                        UpdateCollection();
+                    }
+                }
+            }
+        }
+
+        private void UpdateCollection()
+        {
+            // Using the lock to initialize writes means that we serialize changes. This eliminates
+            // the potential for changes to be processed out of order - the risk is that newer data
+            // could be overwritten by older data.
+            lock (_lock)
+            {
+                var context = new ActionDescriptorProviderContext();
+
+                for (var i = 0; i < _actionDescriptorProviders.Length; i++)
+                {
+                    _actionDescriptorProviders[i].OnProvidersExecuting(context);
+                }
+
+                for (var i = _actionDescriptorProviders.Length - 1; i >= 0; i--)
+                {
+                    _actionDescriptorProviders[i].OnProvidersExecuted(context);
+                }
+
+                // The sequence for an update is important because we don't want anyone to obtain
+                // the new change token but the old action descriptor collection.
+                // 1. Obtain the old cancellation token source (don't trigger it yet)
+                // 2. Set the new action descriptor collection
+                // 3. Set the new change token
+                // 4. Trigger the old cancellation token source
+                //
+                // Consumers who poll will observe a new action descriptor collection at step 2 - they will see
+                // the new collection and ignore the change token.
+                //
+                // Consumers who listen to the change token will requery at step 4 - they will see the new collection
+                // and new change token.
+                //
+                // Anyone who acquires the collection and change token between steps 2 and 3 will be notified of
+                // a no-op change at step 4.
+
+                // Step 1.
+                var oldCancellationTokenSource = _cancellationTokenSource;
+
+                // Step 2.
+                _collection = new ActionDescriptorCollection(
+                    new ReadOnlyCollection<ActionDescriptor>(context.Results),
+                    _version++);
+
+                // Step 3.
+                _cancellationTokenSource = new CancellationTokenSource();
+                _changeToken = new CancellationChangeToken(_cancellationTokenSource.Token);
+
+                // Step 4 - might be null if it's the first time.
+                oldCancellationTokenSource?.Cancel();
+            }
+        }
+    }
+}

src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/IActionDescriptorChangeProvider.cs

@@ -1,6 +1,7 @@
 // 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.Mvc.Abstractions;
 using Microsoft.Extensions.Primitives;
 
 namespace Microsoft.AspNetCore.Mvc.Infrastructure
@@ -9,6 +10,11 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure
     /// Provides a way to signal invalidation of the cached collection of <see cref="Abstractions.ActionDescriptor" /> from an
     /// <see cref="IActionDescriptorCollectionProvider"/>.
     /// </summary>
+    /// <remarks>
+    /// The change token returned from <see cref="GetChangeToken"/> is only for use inside the MVC infrastructure. 
+    /// Use <see cref="ActionDescriptorCollectionProvider.GetChangeToken"/> to be notified of <see cref="ActionDescriptor"/>
+    /// changes.
+    /// </remarks>
     public interface IActionDescriptorChangeProvider
     {
         /// <summary>
@@ -16,6 +22,11 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure
         /// instances.
         /// </summary>
         /// <returns>The <see cref="IChangeToken"/>.</returns>
+        /// <remarks>
+        /// The change token returned from <see cref="GetChangeToken"/> is only for use inside the MVC infrastructure. 
+        /// Use <see cref="ActionDescriptorCollectionProvider.GetChangeToken"/> to be notified of <see cref="ActionDescriptor"/>
+        /// changes.
+        /// </remarks>
         IChangeToken GetChangeToken();
     }
 }

src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/IActionDescriptorCollectionProvider.cs

@@ -1,18 +1,28 @@
 // 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.Extensions.Primitives;
+
 namespace Microsoft.AspNetCore.Mvc.Infrastructure
 {
     /// <summary>
     /// Provides the currently cached collection of <see cref="Abstractions.ActionDescriptor"/>.
     /// </summary>
     /// <remarks>
+    /// <para>
     /// The default implementation internally caches the collection and uses
     /// <see cref="IActionDescriptorChangeProvider"/> to invalidate this cache, incrementing
     /// <see cref="ActionDescriptorCollection.Version"/> the collection is reconstructed.
-    ///
+    ///</para>
+    ///<para>
+    /// To be reactively notified of changes, downcast to <see cref="ActionDescriptorCollectionProvider"/> and
+    /// subcribe to the change token returned from <see cref="ActionDescriptorCollectionProvider.GetChangeToken"/>
+    /// using <see cref="ChangeToken.OnChange(System.Func{IChangeToken}, System.Action)"/>.
+    /// </para>
+    /// <para>
     /// Default consumers of this service, are aware of the version and will recache
     /// data as appropriate, but rely on the version being unique.
+    /// </para>
     /// </remarks>
     public interface IActionDescriptorCollectionProvider
     {

src/Microsoft.AspNetCore.Mvc.Core/Internal/ActionDescriptorCollectionProvider.cs

@@ -1,95 +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 System.Collections.ObjectModel;
-using System.Linq;
-using System.Threading;
-using Microsoft.AspNetCore.Mvc.Abstractions;
-using Microsoft.AspNetCore.Mvc.Infrastructure;
-using Microsoft.Extensions.Primitives;
-
-namespace Microsoft.AspNetCore.Mvc.Internal
-{
-    /// <summary>
-    /// Default implementation of <see cref="IActionDescriptorCollectionProvider"/>.
-    /// </summary>
-    public class ActionDescriptorCollectionProvider : IActionDescriptorCollectionProvider
-    {
-        private readonly IActionDescriptorProvider[] _actionDescriptorProviders;
-        private readonly IActionDescriptorChangeProvider[] _actionDescriptorChangeProviders;
-        private ActionDescriptorCollection _collection;
-        private int _version = -1;
-
-        /// <summary>
-        /// Initializes a new instance of the <see cref="ActionDescriptorCollectionProvider" /> class.
-        /// </summary>
-        /// <param name="actionDescriptorProviders">The sequence of <see cref="IActionDescriptorProvider"/>.</param>
-        /// <param name="actionDescriptorChangeProviders">The sequence of <see cref="IActionDescriptorChangeProvider"/>.</param>
-        public ActionDescriptorCollectionProvider(
-            IEnumerable<IActionDescriptorProvider> actionDescriptorProviders,
-            IEnumerable<IActionDescriptorChangeProvider> actionDescriptorChangeProviders)
-        {
-            _actionDescriptorProviders = actionDescriptorProviders
-                .OrderBy(p => p.Order)
-                .ToArray();
-
-            _actionDescriptorChangeProviders = actionDescriptorChangeProviders.ToArray();
-
-            ChangeToken.OnChange(
-                GetCompositeChangeToken,
-                UpdateCollection);
-        }
-
-        private IChangeToken GetCompositeChangeToken()
-        {
-            if (_actionDescriptorChangeProviders.Length == 1)
-            {
-                return _actionDescriptorChangeProviders[0].GetChangeToken();
-            }
-
-            var changeTokens = new IChangeToken[_actionDescriptorChangeProviders.Length];
-            for (var i = 0; i < _actionDescriptorChangeProviders.Length; i++)
-            {
-                changeTokens[i] = _actionDescriptorChangeProviders[i].GetChangeToken();
-            }
-
-            return new CompositeChangeToken(changeTokens);
-        }
-
-        /// <summary>
-        /// Returns a cached collection of <see cref="ActionDescriptor" />.
-        /// </summary>
-        public ActionDescriptorCollection ActionDescriptors
-        {
-            get
-            {
-                if (_collection == null)
-                {
-                    UpdateCollection();
-                }
-
-                return _collection;
-            }
-        }
-
-        private void UpdateCollection()
-        {
-            var context = new ActionDescriptorProviderContext();
-
-            for (var i = 0; i < _actionDescriptorProviders.Length; i++)
-            {
-                _actionDescriptorProviders[i].OnProvidersExecuting(context);
-            }
-
-            for (var i = _actionDescriptorProviders.Length - 1; i >= 0; i--)
-            {
-                _actionDescriptorProviders[i].OnProvidersExecuted(context);
-            }
-
-            _collection = new ActionDescriptorCollection(
-                new ReadOnlyCollection<ActionDescriptor>(context.Results),
-                Interlocked.Increment(ref _version));
-        }
-    }
-}

src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcEndpointDataSource.cs

@@ -3,8 +3,10 @@
 
 using System;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.Linq;
 using System.Text;
+using System.Threading;
 using Microsoft.AspNetCore.Builder;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Http.Features;
@@ -13,25 +15,27 @@ using Microsoft.AspNetCore.Mvc.Infrastructure;
 using Microsoft.AspNetCore.Mvc.Routing;
 using Microsoft.AspNetCore.Routing;
 using Microsoft.AspNetCore.Routing.Patterns;
-using Microsoft.Extensions.FileProviders;
 using Microsoft.Extensions.Primitives;
 
 namespace Microsoft.AspNetCore.Mvc.Internal
 {
     internal class MvcEndpointDataSource : EndpointDataSource
     {
-        private readonly object _lock = new object();
         private readonly IActionDescriptorCollectionProvider _actions;
         private readonly MvcEndpointInvokerFactory _invokerFactory;
         private readonly DefaultHttpContext _httpContextInstance;
-        private readonly IActionDescriptorChangeProvider[] _actionDescriptorChangeProviders;
 
+        // The following are protected by this lock for WRITES only. This pattern is similar
+        // to DefaultActionDescriptorChangeProvider - see comments there for details on 
+        // all of the threading behaviors.
+        private readonly object _lock = new object();
         private List<Endpoint> _endpoints;
+        private CancellationTokenSource _cancellationTokenSource;
+        private IChangeToken _changeToken;
 
         public MvcEndpointDataSource(
             IActionDescriptorCollectionProvider actions,
             MvcEndpointInvokerFactory invokerFactory,
-            IEnumerable<IActionDescriptorChangeProvider> actionDescriptorChangeProviders,
             IServiceProvider serviceProvider)
         {
             if (actions == null)
@@ -44,11 +48,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal
                 throw new ArgumentNullException(nameof(invokerFactory));
             }
 
-            if (actionDescriptorChangeProviders == null)
-            {
-                throw new ArgumentNullException(nameof(actionDescriptorChangeProviders));
-            }
-
             if (serviceProvider == null)
             {
                 throw new ArgumentNullException(nameof(serviceProvider));
@@ -56,139 +55,199 @@ namespace Microsoft.AspNetCore.Mvc.Internal
 
             _actions = actions;
             _invokerFactory = invokerFactory;
-            _actionDescriptorChangeProviders = actionDescriptorChangeProviders.ToArray();
             _httpContextInstance = new DefaultHttpContext() { RequestServices = serviceProvider };
 
             ConventionalEndpointInfos = new List<MvcEndpointInfo>();
+
+            // It's possible for someone to override the collection provider without providing
+            // change notifications. If that's the case we won't process changes.
+            if (actions is ActionDescriptorCollectionProvider collectionProviderWithChangeToken)
+            {
+                ChangeToken.OnChange(
+                    () => collectionProviderWithChangeToken.GetChangeToken(),
+                    UpdateEndpoints);
+            }
         }
 
-        private List<Endpoint> CreateEndpoints()
+        public List<MvcEndpointInfo> ConventionalEndpointInfos { get; }
+
+        public override IReadOnlyList<Endpoint> Endpoints
         {
-            var endpoints = new List<Endpoint>();
+            get
-            StringBuilder patternStringBuilder = null;
-
-            foreach (var action in _actions.ActionDescriptors.Items)
             {
-                if (action.AttributeRouteInfo == null)
+                Initialize();
+                Debug.Assert(_changeToken != null);
+                Debug.Assert(_endpoints != null);
+                return _endpoints;
+            }
+        }
+
+        public override IChangeToken GetChangeToken()
+        {
+            Initialize();
+            Debug.Assert(_changeToken != null);
+            Debug.Assert(_endpoints != null);
+            return _changeToken;
+        }
+
+        private void Initialize()
+        {
+            if (_endpoints == null)
+            {
+                lock (_lock)
                 {
-                    // In traditional conventional routing setup, the routes defined by a user have a static order
+                    if (_endpoints == null)
-                    // defined by how they are added into the list. We would like to maintain the same order when building
-                    // up the endpoints too.
-                    //
-                    // Start with an order of '1' for conventional routes as attribute routes have a default order of '0'.
-                    // This is for scenarios dealing with migrating existing Router based code to Endpoint Routing world.
-                    var conventionalRouteOrder = 0;
-
-                    // Check each of the conventional patterns to see if the action would be reachable
-                    // If the action and pattern are compatible then create an endpoint with the
-                    // area/controller/action parameter parts replaced with literals
-                    //
-                    // e.g. {controller}/{action} with HomeController.Index and HomeController.Login
-                    // would result in endpoints:
-                    // - Home/Index
-                    // - Home/Login
-                    foreach (var endpointInfo in ConventionalEndpointInfos)
                     {
-                        // An 'endpointInfo' is applicable if:
+                        UpdateEndpoints();
-                        // 1. it has a parameter (or default value) for 'required' non-null route value
+                    }
-                        // 2. it does not have a parameter (or default value) for 'required' null route value
+                }
-                        var isApplicable = true;
+            }
-                        foreach (var routeKey in action.RouteValues.Keys)
+        }
+
+        private void UpdateEndpoints()
+        {
+            lock (_lock)
+            {
+                var endpoints = new List<Endpoint>();
+                StringBuilder patternStringBuilder = null;
+
+                foreach (var action in _actions.ActionDescriptors.Items)
+                {
+                    if (action.AttributeRouteInfo == null)
+                    {
+                        // In traditional conventional routing setup, the routes defined by a user have a static order
+                        // defined by how they are added into the list. We would like to maintain the same order when building
+                        // up the endpoints too.
+                        //
+                        // Start with an order of '1' for conventional routes as attribute routes have a default order of '0'.
+                        // This is for scenarios dealing with migrating existing Router based code to Endpoint Routing world.
+                        var conventionalRouteOrder = 0;
+
+                        // Check each of the conventional patterns to see if the action would be reachable
+                        // If the action and pattern are compatible then create an endpoint with the
+                        // area/controller/action parameter parts replaced with literals
+                        //
+                        // e.g. {controller}/{action} with HomeController.Index and HomeController.Login
+                        // would result in endpoints:
+                        // - Home/Index
+                        // - Home/Login
+                        foreach (var endpointInfo in ConventionalEndpointInfos)
                         {
-                            if (!MatchRouteValue(action, endpointInfo, routeKey))
+                            // An 'endpointInfo' is applicable if:
+                            // 1. it has a parameter (or default value) for 'required' non-null route value
+                            // 2. it does not have a parameter (or default value) for 'required' null route value
+                            var isApplicable = true;
+                            foreach (var routeKey in action.RouteValues.Keys)
                             {
-                                isApplicable = false;
+                                if (!MatchRouteValue(action, endpointInfo, routeKey))
-                                break;
-                            }
-                        }
-
-                        if (!isApplicable)
-                        {
-                            continue;
-                        }
-
-                        var newPathSegments = endpointInfo.ParsedPattern.PathSegments.ToList();
-
-                        for (var i = 0; i < newPathSegments.Count; i++)
-                        {
-                            // Check if the pattern can be shortened because the remaining parameters are optional
-                            //
-                            // e.g. Matching pattern {controller=Home}/{action=Index}/{id?} against HomeController.Index
-                            // can resolve to the following endpoints:
-                            // - /Home/Index/{id?}
-                            // - /Home
-                            // - /
-                            if (UseDefaultValuePlusRemainingSegementsOptional(i, action, endpointInfo, newPathSegments))
-                            {
-                                var subPathSegments = newPathSegments.Take(i);
-
-                                var subEndpoint = CreateEndpoint(
-                                    action,
-                                    endpointInfo.Name,
-                                    GetPattern(ref patternStringBuilder, subPathSegments),
-                                    subPathSegments,
-                                    endpointInfo.Defaults,
-                                    ++conventionalRouteOrder,
-                                    endpointInfo,
-                                    suppressLinkGeneration: false);
-                                endpoints.Add(subEndpoint);
-                            }
-
-                            List<RoutePatternPart> segmentParts = null; // Initialize only as needed
-                            var segment = newPathSegments[i];
-                            for (var j = 0; j < segment.Parts.Count; j++)
-                            {
-                                var part = segment.Parts[j];
-
-                                if (part.IsParameter &&
-                                    part is RoutePatternParameterPart parameterPart &&
-                                    action.RouteValues.ContainsKey(parameterPart.Name))
                                 {
-                                    if (segmentParts == null)
+                                    isApplicable = false;
-                                    {
+                                    break;
-                                        segmentParts = segment.Parts.ToList();
-                                    }
-
-                                    // Replace parameter with literal value
-                                    segmentParts[j] = RoutePatternFactory.LiteralPart(action.RouteValues[parameterPart.Name]);
                                 }
                             }
 
-                            // A parameter part was replaced so replace segment with updated parts
+                            if (!isApplicable)
-                            if (segmentParts != null)
                             {
-                                newPathSegments[i] = RoutePatternFactory.Segment(segmentParts);
+                                continue;
                             }
-                        }
 
+                            var newPathSegments = endpointInfo.ParsedPattern.PathSegments.ToList();
+
+                            for (var i = 0; i < newPathSegments.Count; i++)
+                            {
+                                // Check if the pattern can be shortened because the remaining parameters are optional
+                                //
+                                // e.g. Matching pattern {controller=Home}/{action=Index}/{id?} against HomeController.Index
+                                // can resolve to the following endpoints:
+                                // - /Home/Index/{id?}
+                                // - /Home
+                                // - /
+                                if (UseDefaultValuePlusRemainingSegementsOptional(i, action, endpointInfo, newPathSegments))
+                                {
+                                    var subPathSegments = newPathSegments.Take(i);
+
+                                    var subEndpoint = CreateEndpoint(
+                                        action,
+                                        endpointInfo.Name,
+                                        GetPattern(ref patternStringBuilder, subPathSegments),
+                                        subPathSegments,
+                                        endpointInfo.Defaults,
+                                        ++conventionalRouteOrder,
+                                        endpointInfo,
+                                        suppressLinkGeneration: false);
+                                    endpoints.Add(subEndpoint);
+                                }
+
+                                List<RoutePatternPart> segmentParts = null; // Initialize only as needed
+                                var segment = newPathSegments[i];
+                                for (var j = 0; j < segment.Parts.Count; j++)
+                                {
+                                    var part = segment.Parts[j];
+
+                                    if (part.IsParameter &&
+                                        part is RoutePatternParameterPart parameterPart &&
+                                        action.RouteValues.ContainsKey(parameterPart.Name))
+                                    {
+                                        if (segmentParts == null)
+                                        {
+                                            segmentParts = segment.Parts.ToList();
+                                        }
+
+                                        // Replace parameter with literal value
+                                        segmentParts[j] = RoutePatternFactory.LiteralPart(action.RouteValues[parameterPart.Name]);
+                                    }
+                                }
+
+                                // A parameter part was replaced so replace segment with updated parts
+                                if (segmentParts != null)
+                                {
+                                    newPathSegments[i] = RoutePatternFactory.Segment(segmentParts);
+                                }
+                            }
+
+                            var endpoint = CreateEndpoint(
+                                action,
+                                endpointInfo.Name,
+                                GetPattern(ref patternStringBuilder, newPathSegments),
+                                newPathSegments,
+                                endpointInfo.Defaults,
+                                ++conventionalRouteOrder,
+                                endpointInfo,
+                                suppressLinkGeneration: false);
+                            endpoints.Add(endpoint);
+                        }
+                    }
+                    else
+                    {
                         var endpoint = CreateEndpoint(
                             action,
-                            endpointInfo.Name,
+                            action.AttributeRouteInfo.Name,
-                            GetPattern(ref patternStringBuilder, newPathSegments),
+                            action.AttributeRouteInfo.Template,
-                            newPathSegments,
+                            RoutePatternFactory.Parse(action.AttributeRouteInfo.Template).PathSegments,
-                            endpointInfo.Defaults,
+                            nonInlineDefaults: null,
-                            ++conventionalRouteOrder,
+                            action.AttributeRouteInfo.Order,
-                            endpointInfo,
+                            action.AttributeRouteInfo,
-                            suppressLinkGeneration: false);
+                            suppressLinkGeneration: action.AttributeRouteInfo.SuppressLinkGeneration);
                         endpoints.Add(endpoint);
                     }
                 }
-                else
-                {
-                    var endpoint = CreateEndpoint(
-                        action,
-                        action.AttributeRouteInfo.Name,
-                        action.AttributeRouteInfo.Template,
-                        RoutePatternFactory.Parse(action.AttributeRouteInfo.Template).PathSegments,
-                        nonInlineDefaults: null,
-                        action.AttributeRouteInfo.Order,
-                        action.AttributeRouteInfo,
-                        suppressLinkGeneration: action.AttributeRouteInfo.SuppressLinkGeneration);
-                    endpoints.Add(endpoint);
-                }
-            }
 
-            return endpoints;
+                // See comments in DefaultActionDescriptorCollectionProvider. These steps are done
+                // in a specific order to ensure callers always see a consistent state.
+
+                // Step 1 - capture old token
+                var oldCancellationTokenSource = _cancellationTokenSource;
+
+                // Step 2 - update endpoints
+                _endpoints = endpoints;
+
+                // Step 3 - create new change token
+                _cancellationTokenSource = new CancellationTokenSource();
+                _changeToken = new CancellationChangeToken(_cancellationTokenSource.Token);
+
+                // Step 4 - trigger old token
+                oldCancellationTokenSource?.Cancel();
+            }
 
             string GetPattern(ref StringBuilder sb, IEnumerable<RoutePatternPathSegment> segments)
             {
@@ -442,49 +501,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal
             }
         }
 
-        private IChangeToken GetCompositeChangeToken()
-        {
-            if (_actionDescriptorChangeProviders.Length == 1)
-            {
-                return _actionDescriptorChangeProviders[0].GetChangeToken();
-            }
-
-            var changeTokens = new IChangeToken[_actionDescriptorChangeProviders.Length];
-            for (var i = 0; i < _actionDescriptorChangeProviders.Length; i++)
-            {
-                changeTokens[i] = _actionDescriptorChangeProviders[i].GetChangeToken();
-            }
-
-            return new CompositeChangeToken(changeTokens);
-        }
-
-        public override IChangeToken GetChangeToken() => NullChangeToken.Singleton;
-
-        public override IReadOnlyList<Endpoint> Endpoints
-        {
-            get
-            {
-                // Want to initialize endpoints once and then cache while ensuring a null collection is never returned
-                // Local copy for thread safety + double check locking
-                var localEndpoints = _endpoints;
-                if (localEndpoints == null)
-                {
-                    lock (_lock)
-                    {
-                        localEndpoints = _endpoints;
-                        if (localEndpoints == null)
-                        {
-                            _endpoints = localEndpoints = CreateEndpoints();
-                        }
-                    }
-                }
-
-                return localEndpoints;
-            }
-        }
-
-        public List<MvcEndpointInfo> ConventionalEndpointInfos { get; }
-
         private class SuppressLinkGenerationMetadata : ISuppressLinkGenerationMetadata { }
     }
 }

test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/DefaultActionDescriptorCollectionProviderTest.cs

@@ -4,14 +4,13 @@
 using System.Linq;
 using System.Threading;
 using Microsoft.AspNetCore.Mvc.Abstractions;
-using Microsoft.AspNetCore.Mvc.Infrastructure;
 using Microsoft.Extensions.Primitives;
 using Moq;
 using Xunit;
 
-namespace Microsoft.AspNetCore.Mvc.Internal
+namespace Microsoft.AspNetCore.Mvc.Infrastructure
 {
-    public class ActionDescriptorCollectionProviderTest
+    public class DefaultActionDescriptorCollectionProviderTest
     {
         [Fact]
         public void ActionDescriptors_ReadsDescriptorsFromActionDescriptorProviders()
@@ -24,7 +23,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal
             var expected3 = new ActionDescriptor();
             var actionDescriptorProvider2 = GetActionDescriptorProvider(expected2, expected3);
 
-            var actionDescriptorCollectionProvider = new ActionDescriptorCollectionProvider(
+            var actionDescriptorCollectionProvider = new DefaultActionDescriptorCollectionProvider(
                 new[] { actionDescriptorProvider1, actionDescriptorProvider2 },
                 Enumerable.Empty<IActionDescriptorChangeProvider>());
 
@@ -46,7 +45,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal
             // Arrange
             var actionDescriptorProvider = GetActionDescriptorProvider(new ActionDescriptor());
 
-            var actionDescriptorCollectionProvider = new ActionDescriptorCollectionProvider(
+            var actionDescriptorCollectionProvider = new DefaultActionDescriptorCollectionProvider(
                 new[] { actionDescriptorProvider },
                 Enumerable.Empty<IActionDescriptorChangeProvider>());
 
@@ -66,55 +65,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal
         }
 
         [Fact]
-        public void ActionDescriptors_UpdateWhenChangeTokenProviderChanges()
+        public void ActionDescriptors_UpdatesAndResubscripes_WhenChangeTokenTriggers()
-        {
-            // Arrange
-            var actionDescriptorProvider = new Mock<IActionDescriptorProvider>();
-            var expected1 = new ActionDescriptor();
-            var expected2 = new ActionDescriptor();
-
-            var invocations = 0;
-            actionDescriptorProvider
-                .Setup(p => p.OnProvidersExecuting(It.IsAny<ActionDescriptorProviderContext>()))
-                .Callback((ActionDescriptorProviderContext context) =>
-                {
-                    if (invocations == 0)
-                    {
-                        context.Results.Add(expected1);
-                    }
-                    else
-                    {
-                        context.Results.Add(expected2);
-                    }
-
-                    invocations++;
-                });
-            var changeProvider = new TestChangeProvider();
-            var actionDescriptorCollectionProvider = new ActionDescriptorCollectionProvider(
-                new[] { actionDescriptorProvider.Object },
-                new[] { changeProvider });
-
-            // Act - 1
-            var collection1 = actionDescriptorCollectionProvider.ActionDescriptors;
-
-            // Assert - 1
-            Assert.Equal(0, collection1.Version);
-            Assert.Collection(collection1.Items,
-                item => Assert.Same(expected1, item));
-
-            // Act - 2
-            changeProvider.TokenSource.Cancel();
-            var collection2 = actionDescriptorCollectionProvider.ActionDescriptors;
-
-            // Assert - 2
-            Assert.NotSame(collection1, collection2);
-            Assert.Equal(1, collection2.Version);
-            Assert.Collection(collection2.Items,
-                item => Assert.Same(expected2, item));
-        }
-
-        [Fact]
-        public void ActionDescriptors_SubscribesToNewChangeNotificationsAfterInvalidating()
         {
             // Arrange
             var actionDescriptorProvider = new Mock<IActionDescriptorProvider>();
@@ -143,34 +94,61 @@ namespace Microsoft.AspNetCore.Mvc.Internal
                     invocations++;
                 });
             var changeProvider = new TestChangeProvider();
-            var actionDescriptorCollectionProvider = new ActionDescriptorCollectionProvider(
+            var actionDescriptorCollectionProvider = new DefaultActionDescriptorCollectionProvider(
                 new[] { actionDescriptorProvider.Object },
                 new[] { changeProvider });
 
             // Act - 1
+            var changeToken1 = actionDescriptorCollectionProvider.GetChangeToken();
             var collection1 = actionDescriptorCollectionProvider.ActionDescriptors;
 
+            ActionDescriptorCollection captured = null;
+            changeToken1.RegisterChangeCallback((_) =>
+            {
+                captured = actionDescriptorCollectionProvider.ActionDescriptors;
+            }, null);
+
             // Assert - 1
+            Assert.False(changeToken1.HasChanged);
             Assert.Equal(0, collection1.Version);
             Assert.Collection(collection1.Items,
                 item => Assert.Same(expected1, item));
 
             // Act - 2
             changeProvider.TokenSource.Cancel();
+            var changeToken2 = actionDescriptorCollectionProvider.GetChangeToken();
             var collection2 = actionDescriptorCollectionProvider.ActionDescriptors;
 
+            changeToken2.RegisterChangeCallback((_) =>
+            {
+                captured = actionDescriptorCollectionProvider.ActionDescriptors;
+            }, null);
+
             // Assert - 2
+            Assert.NotSame(changeToken1, changeToken2);
+            Assert.True(changeToken1.HasChanged);
+            Assert.False(changeToken2.HasChanged);
+
             Assert.NotSame(collection1, collection2);
+            Assert.NotNull(captured);
+            Assert.Same(captured, collection2);
             Assert.Equal(1, collection2.Version);
             Assert.Collection(collection2.Items,
                 item => Assert.Same(expected2, item));
 
             // Act - 3
             changeProvider.TokenSource.Cancel();
+            var changeToken3 = actionDescriptorCollectionProvider.GetChangeToken();
             var collection3 = actionDescriptorCollectionProvider.ActionDescriptors;
 
             // Assert - 3
+            Assert.NotSame(changeToken2, changeToken3);
+            Assert.True(changeToken2.HasChanged);
+            Assert.False(changeToken3.HasChanged);
+
             Assert.NotSame(collection2, collection3);
+            Assert.NotNull(captured);
+            Assert.Same(captured, collection3);
             Assert.Equal(2, collection3.Version);
             Assert.Collection(collection3.Items,
                 item => Assert.Same(expected3, item));

test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ActionConstraintCacheTest.cs

@@ -159,7 +159,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal
 
         private static ActionConstraintCache CreateCache(params IActionConstraintProvider[] providers)
         {
-            var descriptorProvider = new ActionDescriptorCollectionProvider(
+            var descriptorProvider = new DefaultActionDescriptorCollectionProvider(
                 Enumerable.Empty<IActionDescriptorProvider>(),
                 Enumerable.Empty<IActionDescriptorChangeProvider>());
             return new ActionConstraintCache(descriptorProvider, providers);

test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ActionSelectorTest.cs

@@ -932,7 +932,7 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure
         private ControllerActionDescriptor InvokeActionSelector(RouteContext context)
         {
             var actionDescriptorProvider = GetActionDescriptorProvider();
-            var actionDescriptorCollectionProvider = new ActionDescriptorCollectionProvider(
+            var actionDescriptorCollectionProvider = new DefaultActionDescriptorCollectionProvider(
                 new[] { actionDescriptorProvider },
                 Enumerable.Empty<IActionDescriptorChangeProvider>());
 
@@ -1092,7 +1092,7 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure
 
         private static ActionConstraintCache GetActionConstraintCache(IActionConstraintProvider[] actionConstraintProviders = null)
         {
-            var descriptorProvider = new ActionDescriptorCollectionProvider(
+            var descriptorProvider = new DefaultActionDescriptorCollectionProvider(
                 Enumerable.Empty<IActionDescriptorProvider>(),
                 Enumerable.Empty<IActionDescriptorChangeProvider>());
             return new ActionConstraintCache(descriptorProvider, actionConstraintProviders.AsEnumerable() ?? new List<IActionConstraintProvider>());

test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/MvcEndpointDataSourceTests.cs

@@ -133,47 +133,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal
             Assert.True(actionInvokerCalled);
         }
 
-        [Fact(Skip = "https://github.com/aspnet/Routing/issues/722")]
-        public void GetChangeToken_MultipleChangeTokenProviders_ComposedResult()
-        {
-            // Arrange
-            var featureCollection = new FeatureCollection();
-            featureCollection.Set<IEndpointFeature>(new EndpointFeature
-            {
-                RouteValues = new RouteValueDictionary()
-            });
-
-            var httpContextMock = new Mock<HttpContext>();
-            httpContextMock.Setup(m => m.Features).Returns(featureCollection);
-
-            var descriptorProviderMock = new Mock<IActionDescriptorCollectionProvider>();
-            descriptorProviderMock.Setup(m => m.ActionDescriptors).Returns(new ActionDescriptorCollection(new List<ActionDescriptor>(), 0));
-
-            var actionInvokerMock = new Mock<IActionInvoker>();
-
-            var actionInvokerProviderMock = new Mock<IActionInvokerFactory>();
-            actionInvokerProviderMock.Setup(m => m.CreateInvoker(It.IsAny<ActionContext>())).Returns(actionInvokerMock.Object);
-
-            var changeTokenMock = new Mock<IChangeToken>();
-
-            var changeProvider1Mock = new Mock<IActionDescriptorChangeProvider>();
-            changeProvider1Mock.Setup(m => m.GetChangeToken()).Returns(changeTokenMock.Object);
-            var changeProvider2Mock = new Mock<IActionDescriptorChangeProvider>();
-            changeProvider2Mock.Setup(m => m.GetChangeToken()).Returns(changeTokenMock.Object);
-
-            var dataSource = CreateMvcEndpointDataSource(
-                descriptorProviderMock.Object,
-                new MvcEndpointInvokerFactory(actionInvokerProviderMock.Object),
-                new[] { changeProvider1Mock.Object, changeProvider2Mock.Object });
-
-            // Act
-            var changeToken = dataSource.GetChangeToken();
-
-            // Assert
-            var compositeChangeToken = Assert.IsType<CompositeChangeToken>(changeToken);
-            Assert.Equal(2, compositeChangeToken.ChangeTokens.Count);
-        }
-
         [Theory]
         [InlineData("{controller}/{action}/{id?}", new[] { "TestController/TestAction/{id?}" })]
         [InlineData("{controller}/{id?}", new string[] { })]
@@ -287,11 +246,11 @@ namespace Microsoft.AspNetCore.Mvc.Internal
             actionDescriptorCollectionProviderMock.VerifyGet(m => m.ActionDescriptors, Times.Once);
         }
 
-        [Fact(Skip = "https://github.com/aspnet/Routing/issues/722")]
+        [Fact]
         public void Endpoints_ChangeTokenTriggered_EndpointsRecreated()
         {
             // Arrange
-            var actionDescriptorCollectionProviderMock = new Mock<IActionDescriptorCollectionProvider>();
+            var actionDescriptorCollectionProviderMock = new Mock<ActionDescriptorCollectionProvider>();
             actionDescriptorCollectionProviderMock
                 .Setup(m => m.ActionDescriptors)
                 .Returns(new ActionDescriptorCollection(new[]
@@ -300,19 +259,18 @@ namespace Microsoft.AspNetCore.Mvc.Internal
                 }, version: 0));
 
             CancellationTokenSource cts = null;
+            actionDescriptorCollectionProviderMock
+                .Setup(m => m.GetChangeToken())
+                .Returns(() =>
+                {
+                    cts = new CancellationTokenSource();
+                    var changeToken = new CancellationChangeToken(cts.Token);
 
-            var changeProviderMock = new Mock<IActionDescriptorChangeProvider>();
+                    return changeToken;
-            changeProviderMock.Setup(m => m.GetChangeToken()).Returns(() =>
+                });
-            {
-                cts = new CancellationTokenSource();
-                var changeToken = new CancellationChangeToken(cts.Token);
 
-                return changeToken;
-            });
 
-            var dataSource = CreateMvcEndpointDataSource(
+            var dataSource = CreateMvcEndpointDataSource(actionDescriptorCollectionProviderMock.Object);
-                actionDescriptorCollectionProviderMock.Object,
-                actionDescriptorChangeProviders: new[] { changeProviderMock.Object });
             dataSource.ConventionalEndpointInfos.Add(CreateEndpointInfo(
                 string.Empty,
                 "{controller}/{action}",
@@ -752,15 +710,13 @@ namespace Microsoft.AspNetCore.Mvc.Internal
 
         private MvcEndpointDataSource CreateMvcEndpointDataSource(
             IActionDescriptorCollectionProvider actionDescriptorCollectionProvider = null,
-            MvcEndpointInvokerFactory mvcEndpointInvokerFactory = null,
+            MvcEndpointInvokerFactory mvcEndpointInvokerFactory = null)
-            IEnumerable<IActionDescriptorChangeProvider> actionDescriptorChangeProviders = null)
         {
             if (actionDescriptorCollectionProvider == null)
             {
-                var mockDescriptorProvider = new Mock<IActionDescriptorCollectionProvider>();
+                actionDescriptorCollectionProvider = new DefaultActionDescriptorCollectionProvider(
-                mockDescriptorProvider.Setup(m => m.ActionDescriptors).Returns(new ActionDescriptorCollection(new List<ActionDescriptor>(), 0));
+                    Array.Empty<IActionDescriptorProvider>(),
-
+                    Array.Empty<IActionDescriptorChangeProvider>());
-                actionDescriptorCollectionProvider = mockDescriptorProvider.Object;
             }
 
             var serviceProviderMock = new Mock<IServiceProvider>();
@@ -769,7 +725,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal
             var dataSource = new MvcEndpointDataSource(
                 actionDescriptorCollectionProvider,
                 mvcEndpointInvokerFactory ?? new MvcEndpointInvokerFactory(new ActionInvokerFactory(Array.Empty<IActionInvokerProvider>())),
-                actionDescriptorChangeProviders ?? Array.Empty<IActionDescriptorChangeProvider>(),
                 serviceProviderMock.Object);
 
             return dataSource;

test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/ActionConstraintMatcherPolicyTest.cs

@@ -351,7 +351,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing
                     }
                 });
 
-            var actionDescriptorCollectionProvider = new ActionDescriptorCollectionProvider(
+            var actionDescriptorCollectionProvider = new DefaultActionDescriptorCollectionProvider(
                 new IActionDescriptorProvider[] { actionDescriptorProvider.Object, },
                 Enumerable.Empty<IActionDescriptorChangeProvider>());
 
@@ -398,7 +398,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing
 
         private static ActionConstraintCache GetActionConstraintCache(IActionConstraintProvider[] actionConstraintProviders = null)
         {
-            var descriptorProvider = new ActionDescriptorCollectionProvider(
+            var descriptorProvider = new DefaultActionDescriptorCollectionProvider(
                 Enumerable.Empty<IActionDescriptorProvider>(),
                 Enumerable.Empty<IActionDescriptorChangeProvider>());
             return new ActionConstraintCache(descriptorProvider, actionConstraintProviders.AsEnumerable() ?? new List<IActionConstraintProvider>());

test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/KnownRouteValueConstraintTests.cs

@@ -173,7 +173,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing
                 .Setup(p => p.OnProvidersExecuted(It.IsAny<ActionDescriptorProviderContext>()))
                 .Verifiable();
 
-            var descriptorCollectionProvider = new ActionDescriptorCollectionProvider(
+            var descriptorCollectionProvider = new DefaultActionDescriptorCollectionProvider(
                 new[] { actionProvider.Object },
                 Enumerable.Empty<IActionDescriptorChangeProvider>());
 

test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiExplorerTest.cs

@@ -1087,7 +1087,7 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests
                 });
         }
 
-        [Fact(Skip = "https://github.com/aspnet/Routing/issues/722")]
+        [Fact]
         public async Task ApiExplorer_Updates_WhenActionDescriptorCollectionIsUpdated()
         {
             // Act - 1