PR feedback

samples/DispatcherSample.Web/Startup.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 System;
 using System.Text;
 using Microsoft.AspNetCore.Builder;
 using Microsoft.AspNetCore.Routing;
@@ -11,6 +12,7 @@ namespace DispatcherSample.Web
 {
     public class Startup
     {
+        private static readonly byte[] _homePayload = Encoding.UTF8.GetBytes("Dispatcher sample endpoints:" + Environment.NewLine + "/plaintext");
         private static readonly byte[] _helloWorldPayload = Encoding.UTF8.GetBytes("Hello, World!");
 
         public void ConfigureServices(IServiceCollection services)
@@ -21,15 +23,25 @@ namespace DispatcherSample.Web
                 options.DataSources.Add(new DefaultEndpointDataSource(new[]
                 {
                     new MatcherEndpoint((next) => (httpContext) =>
-                    {
+                        {
-                        var response = httpContext.Response;
+                            var response = httpContext.Response;
-                        var payloadLength = _helloWorldPayload.Length;
+                            var payloadLength = _homePayload.Length;
-                        response.StatusCode = 200;
+                            response.StatusCode = 200;
-                        response.ContentType = "text/plain";
+                            response.ContentType = "text/plain";
-                        response.ContentLength = payloadLength;
+                            response.ContentLength = payloadLength;
-                        return response.Body.WriteAsync(_helloWorldPayload, 0, payloadLength);
+                            return response.Body.WriteAsync(_homePayload, 0, payloadLength);
-                    },
+                        },
-                    "/plaintext", new { }, 0, EndpointMetadataCollection.Empty, "Plaintext"),
+                        "/", new { }, 0, EndpointMetadataCollection.Empty, "Home"),
+                    new MatcherEndpoint((next) => (httpContext) =>
+                        {
+                            var response = httpContext.Response;
+                            var payloadLength = _helloWorldPayload.Length;
+                            response.StatusCode = 200;
+                            response.ContentType = "text/plain";
+                            response.ContentLength = payloadLength;
+                            return response.Body.WriteAsync(_helloWorldPayload, 0, payloadLength);
+                        },
+                        "/plaintext", new { }, 0, EndpointMetadataCollection.Empty, "Plaintext"),
                 }));
             });
         }

src/Microsoft.AspNetCore.Routing.Abstractions/MetadataCollection.cs

@@ -30,10 +30,10 @@ namespace Microsoft.AspNetCore.Routing
 
         public T GetMetadata<T>() where T : class
         {
-            for (var i = _items.Length -1; i >= 0; i--)
+            for (var i = _items.Length - 1; i >= 0; i--)
             {
                 var item = _items[i] as T;
-                if (item !=null)
+                if (item != null)
                 {
                     return item;
                 }
@@ -62,6 +62,7 @@ namespace Microsoft.AspNetCore.Routing
 
         public struct Enumerator : IEnumerator<object>
         {
+            // Intentionally not readonly to prevent defensive struct copies
             private object[] _items;
             private int _index;
             private object _current;

src/Microsoft.AspNetCore.Routing/DispatcherMiddleware.cs

@@ -57,7 +57,7 @@ namespace Microsoft.AspNetCore.Routing
             var feature = new EndpointFeature();
             httpContext.Features.Set<IEndpointFeature>(feature);
 
-            // There's an inherit race condition between waiting for init and accessing the matcher
+            // There's an inherent race condition between waiting for init and accessing the matcher
             // this is OK because once `_matcher` is initialized, it will not be set to null again.
             var matcher = await InitializeAsync();
 

src/Microsoft.AspNetCore.Routing/EndpointMiddleware.cs

@@ -70,7 +70,7 @@ namespace Microsoft.AspNetCore.Routing
 
             public static void ExecutedEndpoint(ILogger logger, Endpoint endpoint)
             {
-                _executingEndpoint(logger, endpoint.DisplayName, null);
+                _executedEndpoint(logger, endpoint.DisplayName, null);
             }
         }
     }

src/Microsoft.AspNetCore.Routing/Internal/DataSourceDependantCache.cs

@@ -33,6 +33,9 @@ namespace Microsoft.AspNetCore.Routing.Internal
             _lock = new object();
         }
 
+        // Note that we don't lock here, and think about that in the context of a 'push'. So when data gets 'pushed'
+        // we start computing a new state, but we're still able to perform operations on the old state until we've
+        // processed the update.
         public T Value => _value;
 
         public T EnsureInitialized()

src/Microsoft.AspNetCore.Routing/Matchers/MatcherEndpoint.cs

@@ -7,7 +7,7 @@ using Microsoft.AspNetCore.Http;
 
 namespace Microsoft.AspNetCore.Routing.Matchers
 {
-    internal sealed class MatcherEndpoint : Endpoint
+    public sealed class MatcherEndpoint : Endpoint
     {
         public MatcherEndpoint(
             Func<RequestDelegate, RequestDelegate> invoker,
@@ -31,6 +31,7 @@ namespace Microsoft.AspNetCore.Routing.Matchers
             Invoker = invoker;
             Template = template;
             Values = new RouteValueDictionary(values);
+            Order = order;
         }
 
         public int Order { get; }

src/Microsoft.AspNetCore.Routing/Matchers/TreeMatcher.cs

@@ -137,6 +137,7 @@ namespace Microsoft.AspNetCore.Routing.Matchers
 
         private Task SelectEndpointAsync(HttpContext httpContext, IEndpointFeature feature, IReadOnlyList<MatcherEndpoint> endpoints)
         {
+            // REVIEW: Note that this code doesn't do anything significant now. This will eventually incorporate something like IActionConstraint
             switch (endpoints.Count)
             {
                 case 0:
@@ -262,7 +263,7 @@ namespace Microsoft.AspNetCore.Routing.Matchers
             return entry;
         }
 
-        private struct Key : IEquatable<Key>
+        private readonly struct Key : IEquatable<Key>
         {
             public readonly int Order;
             public readonly string Template;

src/Microsoft.AspNetCore.Routing/Properties/AssemblyInfo.cs

@@ -4,4 +4,3 @@
 using System.Runtime.CompilerServices;
 
 [assembly: InternalsVisibleTo("Microsoft.AspNetCore.Routing.Tests, PublicKey=0024000004800000940000000602000000240000525341310004000001000100f33a29044fa9d740c9b3213a93e57c84b472c84e0b8a0e1ae48e67a9f8f6de9d5f7f3d52ac23e48ac51801f1dc950abe901da34d2a9e3baadb141a17c77ef3c565dd5ee5054b91cf63bb3c6ab83f72ab3aafe93d0fc3c2348b764fafb0b1c0733de51459aeab46580384bf9d74c4e28164b7cde247f891ba07891c9d872ad2bb")]
-[assembly: InternalsVisibleTo("DispatcherSample.Web, PublicKey=0024000004800000940000000602000000240000525341310004000001000100f33a29044fa9d740c9b3213a93e57c84b472c84e0b8a0e1ae48e67a9f8f6de9d5f7f3d52ac23e48ac51801f1dc950abe901da34d2a9e3baadb141a17c77ef3c565dd5ee5054b91cf63bb3c6ab83f72ab3aafe93d0fc3c2348b764fafb0b1c0733de51459aeab46580384bf9d74c4e28164b7cde247f891ba07891c9d872ad2bb")]

src/Microsoft.AspNetCore.Routing/Tree/InboundRouteEntry.cs

@@ -53,6 +53,7 @@ namespace Microsoft.AspNetCore.Routing.Tree
         /// </summary>
         public RouteTemplate RouteTemplate { get; set; }
 
+        // REVIEW: temporary change to enable reusing the tree router
         public object Tag { get; set; }
     }
 }