Ensure previous task awaited before starting next one

Author: captainsafia

src/Components/Components/src/Properties/AssemblyInfo.cs

@@ -8,3 +8,5 @@
 [assembly: InternalsVisibleTo("Microsoft.AspNetCore.Components.Tests, PublicKey=0024000004800000940000000602000000240000525341310004000001000100f33a29044fa9d740c9b3213a93e57c84b472c84e0b8a0e1ae48e67a9f8f6de9d5f7f3d52ac23e48ac51801f1dc950abe901da34d2a9e3baadb141a17c77ef3c565dd5ee5054b91cf63bb3c6ab83f72ab3aafe93d0fc3c2348b764fafb0b1c0733de51459aeab46580384bf9d74c4e28164b7cde247f891ba07891c9d872ad2bb")]
 [assembly: InternalsVisibleTo("Microsoft.AspNetCore.Components.Web.Tests, PublicKey=0024000004800000940000000602000000240000525341310004000001000100f33a29044fa9d740c9b3213a93e57c84b472c84e0b8a0e1ae48e67a9f8f6de9d5f7f3d52ac23e48ac51801f1dc950abe901da34d2a9e3baadb141a17c77ef3c565dd5ee5054b91cf63bb3c6ab83f72ab3aafe93d0fc3c2348b764fafb0b1c0733de51459aeab46580384bf9d74c4e28164b7cde247f891ba07891c9d872ad2bb")]
 [assembly: InternalsVisibleTo("Microsoft.AspNetCore.Components.Web.Extensions.Tests, PublicKey=0024000004800000940000000602000000240000525341310004000001000100f33a29044fa9d740c9b3213a93e57c84b472c84e0b8a0e1ae48e67a9f8f6de9d5f7f3d52ac23e48ac51801f1dc950abe901da34d2a9e3baadb141a17c77ef3c565dd5ee5054b91cf63bb3c6ab83f72ab3aafe93d0fc3c2348b764fafb0b1c0733de51459aeab46580384bf9d74c4e28164b7cde247f891ba07891c9d872ad2bb")]
+
+[assembly: InternalsVisibleTo("DynamicProxyGenAssembly2, PublicKey=0024000004800000940000000602000000240000525341310004000001000100c547cac37abd99c8db225ef2f6c8a3602f3b3606cc9891605d02baa56104f4cfc0734aa39b93bf7852f7d9266654753cc297e7d2edfe0bac1cdcf9f717241550e0a7b191195b7667bb4f64bcb8e2121380fd1d9d46ad2d92d2d15605093924cceaf74c4861eff62abf69b9291ed0a340e113be11e6a7d3113e92484cf7045cc7")]

src/Components/Components/src/Routing/Router.cs

@@ -23,19 +23,21 @@ public class Router : IComponent, IHandleAfterRender, IDisposable
         static readonly ReadOnlyDictionary<string, object> _emptyParametersDictionary
             = new ReadOnlyDictionary<string, object>(new Dictionary<string, object>());
 
-        internal RenderHandle _renderHandle { get; set; }
+        RenderHandle _renderHandle;
         string _baseUri;
         string _locationAbsolute;
         bool _navigationInterceptionEnabled;
         ILogger<Router> _logger;
 
         private CancellationTokenSource _onNavigateCts;
 
+        private Task _previousOnNavigateTask = Task.CompletedTask;
+
         private readonly HashSet<Assembly> _assemblies = new HashSet<Assembly>();
 
         private bool _onNavigateCalled = false;
 
-        [Inject] internal NavigationManager NavigationManager { get; set; }
+        [Inject] private NavigationManager NavigationManager { get; set; }
 
         [Inject] private INavigationInterception NavigationInterception { get; set; }
 
@@ -72,7 +74,7 @@ static readonly ReadOnlyDictionary<string, object> _emptyParametersDictionary
         /// </summary>
         [Parameter] public EventCallback<NavigationContext> OnNavigateAsync { get; set; }
 
-        internal RouteTable Routes { get; set; }
+        private RouteTable Routes { get; set; }
 
         /// <inheritdoc />
         public void Attach(RenderHandle renderHandle)
@@ -109,18 +111,14 @@ public async Task SetParametersAsync(ParameterView parameters)
                 throw new InvalidOperationException($"The {nameof(Router)} component requires a value for the parameter {nameof(NotFound)}.");
             }
 
-            var shouldRefresh = true;
             if (!_onNavigateCalled)
             {
                 _onNavigateCalled = true;
-                shouldRefresh = await RunOnNavigateAsync(NavigationManager.ToBaseRelativePath(_locationAbsolute));
-            }
-
-            if (shouldRefresh)
-            {
-                Refresh(isNavigationIntercepted: false);
+                await RunOnNavigateWithRefreshAsync(NavigationManager.ToBaseRelativePath(_locationAbsolute), isNavigationIntercepted: false);
+                return;
             }
 
+            Refresh(isNavigationIntercepted: false);
         }
 
         /// <inheritdoc />
@@ -151,7 +149,7 @@ private void RefreshRouteTable()
 
         }
 
-        private void Refresh(bool isNavigationIntercepted)
+        internal virtual void Refresh(bool isNavigationIntercepted)
         {
             RefreshRouteTable();
 
@@ -194,7 +192,7 @@ private void Refresh(bool isNavigationIntercepted)
             }
         }
 
-        internal async Task<bool> RunOnNavigateAsync(string path)
+        private async ValueTask<bool> RunOnNavigateAsync(string path, Task prevOnNavigate)
         {
             // If this router instance does not provide an OnNavigateAsync parameter
             // then we render the component associated with the route as per usual.
@@ -204,8 +202,11 @@ internal async Task<bool> RunOnNavigateAsync(string path)
             }
 
             // If we've already invoked a task and stored its CTS, then
-            // cancel the existing task.
+            // cancel that existing task.
             _onNavigateCts?.Cancel();
+            // Then make sure that the task has been completed cancelled or
+            // completed before continuing with the execution of this current task.
+            await prevOnNavigate;
 
             // Create a new cancellation token source for this instance
             _onNavigateCts = new CancellationTokenSource();
@@ -229,16 +230,31 @@ internal async Task<bool> RunOnNavigateAsync(string path)
             return task == completedTask;
         }
 
-        private async Task RunOnNavigateWithRefreshAsync(string path, bool isNavigationIntercepted)
+        internal async Task RunOnNavigateWithRefreshAsync(string path, bool isNavigationIntercepted)
         {
-            var shouldRefresh = await RunOnNavigateAsync(path);
-            if (shouldRefresh)
+            // We cache the Task representing the previously invoked RunOnNavigateWithRefreshAsync
+            // that is stored
+            var previousTask = _previousOnNavigateTask;
+            // Then we create a new one that represents our current invocation and store it
+            // globally for the next invocation.
+            var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
+            _previousOnNavigateTask = tcs.Task;
+            try
+            {
+                // And pass an indicator for the previous task to the currently running one.
+                var shouldRefresh = await RunOnNavigateAsync(path, previousTask);
+                if (shouldRefresh)
+                {
+                    Refresh(isNavigationIntercepted);
+                }
+            }
+            finally
             {
-                Refresh(isNavigationIntercepted);
+                tcs.SetResult();
             }
         }
 
-        internal void OnLocationChanged(object sender, LocationChangedEventArgs args)
+        private void OnLocationChanged(object sender, LocationChangedEventArgs args)
         {
             _locationAbsolute = args.Location;
             if (_renderHandle.IsInitialized && Routes != null)

src/Components/Components/test/Routing/RouterTest.cs

@@ -4,19 +4,22 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using System.Reflection;
+using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Components;
 using Microsoft.AspNetCore.Components.Routing;
 using Microsoft.AspNetCore.Components.Test.Helpers;
 using Microsoft.Extensions.DependencyModel;
+using Moq;
 using Xunit;
 
 namespace Microsoft.AspNetCore.Components.Test.Routing
 {
     public class RouterTest
     {
         [Fact]
-        public void CanRunOnNavigateViaLocationChangeAsync()
+        public void CanRunOnNavigateAsync()
         {
             // Arrange
             var router = CreateMockRouter();
@@ -26,10 +29,10 @@ async Task OnNavigateAsync(NavigationContext args)
                 await Task.CompletedTask;
                 called = true;
             }
-            router.OnNavigateAsync = new EventCallbackFactory().Create<NavigationContext>(router, OnNavigateAsync);
+            router.Object.OnNavigateAsync = new EventCallbackFactory().Create<NavigationContext>(router, OnNavigateAsync);
 
             // Act
-            router.OnLocationChanged(null, new LocationChangedEventArgs("http://example.com/jan", false));
+            router.Object.RunOnNavigateWithRefreshAsync("http://example.com/jan", false);
 
             // Assert
             Assert.True(called);
@@ -46,54 +49,43 @@ async Task OnNavigateAsync(NavigationContext args)
                 await Task.CompletedTask;
                 args.CancellationToken.Register(() => cancelled = args.Path);
             };
-            router.OnNavigateAsync = new EventCallbackFactory().Create<NavigationContext>(router, OnNavigateAsync);
+            router.Object.OnNavigateAsync = new EventCallbackFactory().Create<NavigationContext>(router, OnNavigateAsync);
 
             // Act
-            router.OnLocationChanged(null, new LocationChangedEventArgs("http://example.com/jan", false));
-            router.OnLocationChanged(null, new LocationChangedEventArgs("http://example.com/feb", false));
+            router.Object.RunOnNavigateWithRefreshAsync("jan", false);
+            router.Object.RunOnNavigateWithRefreshAsync("feb", false);
 
             // Assert
             var expected = "jan";
             Assert.Equal(cancelled, expected);
         }
 
         [Fact]
-        public async Task RefreshesOnceOnCancelledOnNavigateAsync()
+        public void RefreshesOnceOnCancelledOnNavigateAsync()
         {
             // Arrange
             var router = CreateMockRouter();
             async Task OnNavigateAsync(NavigationContext args)
             {
                 if (args.Path.EndsWith("jan"))
                 {
-                    await Task.Delay(5000);
-                }
-                if (args.Path.EndsWith("feb"))
-                {
-                    await Task.CompletedTask;
+                    await Task.Delay(Timeout.Infinite);
                 }
             };
-            router.OnNavigateAsync = new EventCallbackFactory().Create<NavigationContext>(router, OnNavigateAsync);
+            router.Object.OnNavigateAsync = new EventCallbackFactory().Create<NavigationContext>(router, OnNavigateAsync);
 
             // Act
-            var janTask = router.RunOnNavigateAsync("jan");
-            var febTask = router.RunOnNavigateAsync("feb");
+            router.Object.RunOnNavigateWithRefreshAsync("jan", false);
+            router.Object.RunOnNavigateWithRefreshAsync("feb", false);
 
-            await janTask;
-            await febTask;
-
-            // Assert
-            Assert.False(janTask.Result);
-            Assert.True(febTask.Result);
+            // Assert refresh should've only been called once for the second route
+            router.Verify(x => x.Refresh(It.IsAny<bool>()), Times.Once());
         }
 
-        private Router CreateMockRouter()
+        private Mock<Router> CreateMockRouter()
         {
-            var router = new Router();
-            var renderer = new TestRenderer();
-            router._renderHandle = new RenderHandle(renderer, 0);
-            router.Routes = RouteTableFactory.Create(new[] { typeof(JanComponent), typeof(FebComponent) });
-            router.NavigationManager = new TestNavigationManager();
+            var router = new Mock<Router>() { CallBase = true };
+            router.Setup(x => x.Refresh(It.IsAny<bool>())).Verifiable();
             return router;
         }
 
@@ -102,23 +94,5 @@ private class JanComponent : ComponentBase { }
 
         [Route("feb")]
         private class FebComponent : ComponentBase { }
-
-        private class TestNavigationManager : NavigationManager
-        {
-            public TestNavigationManager()
-            {
-                Initialize("http://example.com/", "http://example.com/months");
-            }
-
-            public new void Initialize(string baseUri, string uri)
-            {
-                base.Initialize(baseUri, uri);
-            }
-
-            protected override void NavigateToCore(string uri, bool forceLoad)
-            {
-                throw new System.NotImplementedException();
-            }
-        }
     }
 }