[Fixes #5170] Fixed KeyNotFoundException in UrlHelperFactory.GetUrlHe…

…lper()
aspnet · Aug 26, 2016 · 9ed7532 · 9ed7532
1 parent 3aa6d73
commit 9ed7532
Show file tree

Hide file tree

Showing 2 changed files with 68 additions and 4 deletions.
diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Routing/UrlHelperFactory.cs b/src/Microsoft.AspNetCore.Mvc.Core/Routing/UrlHelperFactory.cs
@@ -32,13 +32,15 @@ public IUrlHelper GetUrlHelper(ActionContext context)
             }
 
             // Perf: Create only one UrlHelper per context
-            var urlHelper = httpContext.Items[typeof(IUrlHelper)] as IUrlHelper;
-            if (urlHelper == null)
+            object value;
+            if (httpContext.Items.TryGetValue(typeof(IUrlHelper), out value) && value is IUrlHelper)
             {
-                urlHelper = new UrlHelper(context);
-                httpContext.Items[typeof(IUrlHelper)] = urlHelper;
+                return (IUrlHelper)value;
             }
 
+            var urlHelper = new UrlHelper(context);
+            httpContext.Items[typeof(IUrlHelper)] = urlHelper;
+
             return urlHelper;
         }
     }

diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/UrlHelperTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/UrlHelperTest.cs
@@ -971,6 +971,68 @@ public void GenerateUrl_FastAndSlowPathsReturnsExpected(
             Assert.Equal(expected, url);
         }
 
+        [Fact]
+        public void GetUrlHelper_ReturnsSameInstance_IfAlreadyPresent()
+        {
+            // Arrange
+            var expectedUrlHelper = CreateUrlHelper();
+            var httpContext = new Mock<HttpContext>();
+            var mockItems = new Dictionary<object, object>
+            {
+                { typeof(IUrlHelper), expectedUrlHelper }
+            };
+            httpContext.Setup(h => h.Items).Returns(mockItems);
+
+            var actionContext = CreateActionContext(httpContext.Object, Mock.Of<IRouter>());
+            var urlHelperFactory = new UrlHelperFactory();
+
+            // Act
+            var urlHelper = urlHelperFactory.GetUrlHelper(actionContext);
+
+            // Assert
+            Assert.Same(expectedUrlHelper, urlHelper);
+        }
+
+        [Fact]
+        public void GetUrlHelper_CreatesNewInstance_IfNotAlreadyPresent()
+        {
+            // Arrange
+            var httpContext = new Mock<HttpContext>();
+            httpContext.Setup(h => h.Items).Returns(new Dictionary<object, object>());
+
+            var actionContext = CreateActionContext(httpContext.Object, Mock.Of<IRouter>());
+            var urlHelperFactory = new UrlHelperFactory();
+
+            // Act
+            var urlHelper = urlHelperFactory.GetUrlHelper(actionContext);
+
+            // Assert
+            Assert.NotNull(urlHelper);
+            Assert.Same(urlHelper, actionContext.HttpContext.Items[typeof(IUrlHelper)] as IUrlHelper);
+        }
+
+        [Fact]
+        public void GetUrlHelper_CreatesNewInstance_IfExpectedTypeIsNotPresent()
+        {
+            // Arrange
+            var httpContext = new Mock<HttpContext>();
+            var mockItems = new Dictionary<object, object>
+            {
+                { typeof(IUrlHelper), null }
+            };
+            httpContext.Setup(h => h.Items).Returns(mockItems);
+
+            var actionContext = CreateActionContext(httpContext.Object, Mock.Of<IRouter>());
+            var urlHelperFactory = new UrlHelperFactory();
+
+            // Act
+            var urlHelper = urlHelperFactory.GetUrlHelper(actionContext);
+
+            // Assert
+            Assert.NotNull(urlHelper);
+            Assert.Same(urlHelper, actionContext.HttpContext.Items[typeof(IUrlHelper)] as IUrlHelper);
+        }
+
         private static HttpContext CreateHttpContext(
             IServiceProvider services,
             string appRoot)