Fix endpoint constraint cache (#555)

src/Microsoft.AspNetCore.Routing/EndpointConstraints/EndpointConstraintCache.cs

@@ -2,6 +2,7 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using Microsoft.AspNetCore.Http;
+using System;
 using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Diagnostics;
@@ -50,38 +51,48 @@ namespace Microsoft.AspNetCore.Routing.EndpointConstraints
                 return GetEndpointConstraintsFromEntry(entry, httpContext, endpoint);
             }
 
-            if (endpoint.Metadata == null || endpoint.Metadata.Count == 0)
+            List<EndpointConstraintItem> items = null;
+
+            if (endpoint.Metadata != null && endpoint.Metadata.Count > 0)
             {
-                return null;
+                items = endpoint.Metadata
+                    .OfType<IEndpointConstraintMetadata>()
+                    .Select(m => new EndpointConstraintItem(m))
+                    .ToList();
             }
 
-            var items = endpoint.Metadata
+            IReadOnlyList<IEndpointConstraint> endpointConstraints = null;
-                .OfType<IEndpointConstraintMetadata>()
-                .Select(m => new EndpointConstraintItem(m))
-                .ToList();
 
-            ExecuteProviders(httpContext, endpoint, items);
+            if (items != null && items.Count > 0)
-
-            var endpointConstraints = ExtractEndpointConstraints(items);
-
-            var allEndpointConstraintsCached = true;
-            for (var i = 0; i < items.Count; i++)
             {
-                var item = items[i];
+                ExecuteProviders(httpContext, endpoint, items);
-                if (!item.IsReusable)
+
+                endpointConstraints = ExtractEndpointConstraints(items);
+
+                var allEndpointConstraintsCached = true;
+                for (var i = 0; i < items.Count; i++)
                 {
-                    item.Constraint = null;
+                    var item = items[i];
-                    allEndpointConstraintsCached = false;
+                    if (!item.IsReusable)
+                    {
+                        item.Constraint = null;
+                        allEndpointConstraintsCached = false;
+                    }
                 }
-            }
 
-            if (allEndpointConstraintsCached)
+                if (allEndpointConstraintsCached)
-            {
+                {
-                entry = new CacheEntry(endpointConstraints);
+                    entry = new CacheEntry(endpointConstraints);
+                }
+                else
+                {
+                    entry = new CacheEntry(items);
+                }
             }
             else
             {
-                entry = new CacheEntry(items);
+                // No constraints
+                entry = new CacheEntry();
             }
 
             cache.Entries.TryAdd(endpoint, entry);
@@ -90,13 +101,17 @@ namespace Microsoft.AspNetCore.Routing.EndpointConstraints
 
         private IReadOnlyList<IEndpointConstraint> GetEndpointConstraintsFromEntry(CacheEntry entry, HttpContext httpContext, Endpoint endpoint)
         {
-            Debug.Assert(entry.EndpointConstraints != null || entry.Items != null);
-
             if (entry.EndpointConstraints != null)
             {
                 return entry.EndpointConstraints;
             }
 
+            if (entry.Items == null)
+            {
+                // Endpoint has no constraints
+                return null;
+            }
+
             var items = new List<EndpointConstraintItem>(entry.Items.Count);
             for (var i = 0; i < entry.Items.Count; i++)
             {

test/Microsoft.AspNetCore.Routing.Tests/EndpointConstraints/EndpointSelectorTests.cs

@@ -210,6 +210,49 @@ namespace Microsoft.AspNetCore.Routing.EndpointConstraints
             Assert.Same(action, actionWithConstraints);
         }
 
+        [Fact]
+        public void SelectBestCandidate_MultipleCallsNoConstraint_ReturnsEndpoint()
+        {
+            // Arrange
+            var noConstraint = new TestEndpoint(EndpointMetadataCollection.Empty, "noConstraint");
+
+            var actions = new Endpoint[] { noConstraint };
+
+            var selector = CreateSelector(actions);
+            var context = CreateHttpContext("POST");
+
+            // Act
+            var action1 = selector.SelectBestCandidate(context, actions);
+            var action2 = selector.SelectBestCandidate(context, actions);
+
+            // Assert
+            Assert.Same(action1, noConstraint);
+            Assert.Same(action2, noConstraint);
+        }
+
+        [Fact]
+        public void SelectBestCandidate_MultipleCallsNonConstraintMetadata_ReturnsEndpoint()
+        {
+            // Arrange
+            var noConstraint = new TestEndpoint(new EndpointMetadataCollection(new[]
+            {
+                new object(),
+            }), "noConstraint");
+
+            var actions = new Endpoint[] { noConstraint };
+
+            var selector = CreateSelector(actions);
+            var context = CreateHttpContext("POST");
+
+            // Act
+            var action1 = selector.SelectBestCandidate(context, actions);
+            var action2 = selector.SelectBestCandidate(context, actions);
+
+            // Assert
+            Assert.Same(action1, noConstraint);
+            Assert.Same(action2, noConstraint);
+        }
+
         [Fact]
         public void SelectBestCandidate_EndpointConstraintFactory_ReturnsNull()
         {
@@ -225,10 +268,12 @@ namespace Microsoft.AspNetCore.Routing.EndpointConstraints
             var context = CreateHttpContext("POST");
 
             // Act
-            var action = selector.SelectBestCandidate(context, actions);
+            var action1 = selector.SelectBestCandidate(context, actions);
+            var action2 = selector.SelectBestCandidate(context, actions);
 
             // Assert
-            Assert.Same(action, nullConstraint);
+            Assert.Same(action1, nullConstraint);
+            Assert.Same(action2, nullConstraint);
         }
 
         // There's a custom constraint provider registered that only understands BooleanConstraintMarker