[Design] Set the endpoint feature only on success

src/Microsoft.AspNetCore.Routing/EndpointMiddleware.cs

@@ -32,26 +32,18 @@ namespace Microsoft.AspNetCore.Routing
 
         public async Task Invoke(HttpContext httpContext)
         {
-            var feature = httpContext.Features.Get<IEndpointFeature>();
+            var endpoint = httpContext.Features.Get<IEndpointFeature>()?.Endpoint;
-            if (feature == null)
+            if (endpoint?.RequestDelegate != null)
             {
-                var message = $"Unable to execute an endpoint because the {nameof(EndpointRoutingMiddleware)} was not run for this request. " +
+                Log.ExecutingEndpoint(_logger, endpoint);
-                    $"Ensure {nameof(EndpointRoutingMiddleware)} is added to the request execution pipeline before {nameof(EndpointMiddleware)} in application startup code.";
-
-                throw new InvalidOperationException(message);
-            }
-
-            if (feature.Endpoint?.RequestDelegate != null)
-            {
-                Log.ExecutingEndpoint(_logger, feature.Endpoint);
 
                 try
                 {
-                    await feature.Endpoint.RequestDelegate(httpContext);
+                    await endpoint.RequestDelegate(httpContext);
                 }
                 finally
                 {
-                    Log.ExecutedEndpoint(_logger, feature.Endpoint);
+                    Log.ExecutedEndpoint(_logger, endpoint);
                 }
 
                 return;

src/Microsoft.AspNetCore.Routing/EndpointRoutingMiddleware.cs

@@ -55,11 +55,8 @@ namespace Microsoft.AspNetCore.Routing
 
         public async Task Invoke(HttpContext httpContext)
         {
-            // For back-compat EndpointRouteValuesFeature implements IEndpointFeature, IRouteValuesFeature and IRoutingFeature
             var feature = new EndpointFeature();
 
-            SetEndpointFeature(httpContext, feature);
-
             // 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();
@@ -67,6 +64,10 @@ namespace Microsoft.AspNetCore.Routing
             await matcher.MatchAsync(httpContext, feature);
             if (feature.Endpoint != null)
             {
+                // Set the endpoint feature only on success. This means we won't overwrite any
+                // existing state for related features unless we did something.
+                SetEndpointFeature(httpContext, feature);
+
                 Log.MatchSuccess(_logger, feature);
             }
             else
@@ -79,11 +80,8 @@ namespace Microsoft.AspNetCore.Routing
 
         private static void SetEndpointFeature(HttpContext httpContext, EndpointFeature feature)
         {
-            // An IRouteValuesFeature might have already been set
+            // For back-compat EndpointRouteValuesFeature implements IEndpointFeature,
-            // Copy its RouteValues collection if present
+            // IRouteValuesFeature and IRoutingFeature
-            var currentRouteValuesFeature = httpContext.Features.Get<IRouteValuesFeature>();
-            feature.RouteValues = currentRouteValuesFeature?.RouteValues;
-
             httpContext.Features.Set<IRoutingFeature>(feature);
             httpContext.Features.Set<IRouteValuesFeature>(feature);
             httpContext.Features.Set<IEndpointFeature>(feature);

test/Microsoft.AspNetCore.Routing.Tests/Builder/EndpointRoutingBuilderExtensionsTest.cs

@@ -8,6 +8,7 @@ using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Http.Features;
 using Microsoft.AspNetCore.Internal;
 using Microsoft.AspNetCore.Routing;
+using Microsoft.AspNetCore.Routing.Patterns;
 using Microsoft.Extensions.DependencyInjection;
 using Moq;
 using Xunit;
@@ -51,7 +52,7 @@ namespace Microsoft.AspNetCore.Builder
         }
 
         [Fact]
-        public async Task UseEndpointRouting_ServicesRegistered_SetsFeature()
+        public async Task UseEndpointRouting_ServicesRegistered_NoMatch_DoesNotSetFeature()
         {
             // Arrange
             var services = CreateServices();
@@ -67,7 +68,36 @@ namespace Microsoft.AspNetCore.Builder
             await appFunc(httpContext);
 
             // Assert
-            Assert.NotNull(httpContext.Features.Get<IEndpointFeature>());
+            Assert.Null(httpContext.Features.Get<IEndpointFeature>());
+        }
+
+        [Fact]
+        public async Task UseEndpointRouting_ServicesRegistered_Match_DoesNotSetsFeature()
+        {
+            // Arrange
+            var endpoint = new RouteEndpoint(
+                TestConstants.EmptyRequestDelegate,
+                RoutePatternFactory.Parse("{*p}"),
+                0,
+                EndpointMetadataCollection.Empty,
+                "Test");
+
+            var services = CreateServices(endpoint);
+
+            var app = new ApplicationBuilder(services);
+
+            app.UseEndpointRouting();
+
+            var appFunc = app.Build();
+            var httpContext = new DefaultHttpContext();
+
+            // Act
+            await appFunc(httpContext);
+
+            // Assert
+            var feature = httpContext.Features.Get<IEndpointFeature>();
+            Assert.NotNull(feature);
+            Assert.Same(endpoint, feature.Endpoint);
         }
 
         [Fact]
@@ -90,7 +120,7 @@ namespace Microsoft.AspNetCore.Builder
         }
 
         [Fact]
-        public async Task UseEndpoint_ServicesRegisteredAndEndpointRoutingRegistered_SetsFeature()
+        public async Task UseEndpoint_ServicesRegisteredAndEndpointRoutingRegistered_NoMatch_DoesNotSetFeature()
         {
             // Arrange
             var services = CreateServices();
@@ -107,10 +137,10 @@ namespace Microsoft.AspNetCore.Builder
             await appFunc(httpContext);
 
             // Assert
-            Assert.NotNull(httpContext.Features.Get<IEndpointFeature>());
+            Assert.Null(httpContext.Features.Get<IEndpointFeature>());
         }
 
-        private IServiceProvider CreateServices()
+        private IServiceProvider CreateServices(params Endpoint[] endpoints)
         {
             var services = new ServiceCollection();
 
@@ -118,6 +148,8 @@ namespace Microsoft.AspNetCore.Builder
             services.AddOptions();
             services.AddRouting();
 
+            services.AddSingleton<EndpointDataSource>(new DefaultEndpointDataSource(endpoints));
+
             return services.BuildServiceProvider();
         }
     }

test/Microsoft.AspNetCore.Routing.Tests/EndpointMiddlewareTest.cs

@@ -3,11 +3,9 @@
 
 using System;
 using System.Threading.Tasks;
-using Microsoft.AspNetCore.Builder;
 using Microsoft.AspNetCore.Http;
-using Microsoft.Extensions.DependencyInjection;
+using Microsoft.AspNetCore.Http.Features;
 using Microsoft.Extensions.Logging.Abstractions;
-using Microsoft.Extensions.Logging.Testing;
 using Xunit;
 
 namespace Microsoft.AspNetCore.Routing
@@ -15,7 +13,7 @@ namespace Microsoft.AspNetCore.Routing
     public class EndpointMiddlewareTest
     {
         [Fact]
-        public async Task Invoke_NoFeature_ThrowFriendlyErrorMessage()
+        public async Task Invoke_NoFeature_NoOps()
         {
             // Arrange
             var httpContext = new DefaultHttpContext();
@@ -23,22 +21,73 @@ namespace Microsoft.AspNetCore.Routing
 
             RequestDelegate next = (c) =>
             {
-                return Task.FromResult<object>(null);
+                return Task.CompletedTask;
             };
 
             var middleware = new EndpointMiddleware(NullLogger<EndpointMiddleware>.Instance, next);
 
             // Act
-            var invokeTask = middleware.Invoke(httpContext);
+            await middleware.Invoke(httpContext);
+
+            // Assert - does not throw
+        }
+
+        [Fact]
+        public async Task Invoke_NoEndpoint_NoOps()
+        {
+            // Arrange
+            var httpContext = new DefaultHttpContext();
+            httpContext.RequestServices = new ServiceProvider();
+
+            httpContext.Features.Set<IEndpointFeature>(new EndpointFeature()
+            {
+                Endpoint = null,
+            });
+
+            RequestDelegate next = (c) =>
+            {
+                return Task.CompletedTask;
+            };
+
+            var middleware = new EndpointMiddleware(NullLogger<EndpointMiddleware>.Instance, next);
+
+            // Act
+            await middleware.Invoke(httpContext);
+
+            // Assert - does not throw
+        }
+
+        [Fact]
+        public async Task Invoke_WithEndpoint_InvokesDelegate()
+        {
+            // Arrange
+            var httpContext = new DefaultHttpContext();
+            httpContext.RequestServices = new ServiceProvider();
+
+            var invoked = false;
+            RequestDelegate endpointFunc = (c) =>
+            {
+                invoked = true;
+                return Task.CompletedTask;
+            };
+
+            httpContext.Features.Set<IEndpointFeature>(new EndpointFeature()
+            {
+                Endpoint = new Endpoint(endpointFunc, EndpointMetadataCollection.Empty, "Test"),
+            });
+
+            RequestDelegate next = (c) =>
+            {
+                return Task.CompletedTask;
+            };
+
+            var middleware = new EndpointMiddleware(NullLogger<EndpointMiddleware>.Instance, next);
+
+            // Act
+            await middleware.Invoke(httpContext);
 
             // Assert
-            var ex = await Assert.ThrowsAsync<InvalidOperationException>(async () => await invokeTask);
+            Assert.True(invoked);
-
-            Assert.Equal(
-                "Unable to execute an endpoint because the EndpointRoutingMiddleware was not run for this request. " +
-                "Ensure EndpointRoutingMiddleware is added to the request execution pipeline before EndpointMiddleware " +
-                "in application startup code.",
-                ex.Message);
         }
 
         private class ServiceProvider : IServiceProvider