From 791518d44d14e4e86ea7b467e53b0e98168279ff Mon Sep 17 00:00:00 2001 From: sornaks Date: Wed, 20 Aug 2014 15:24:48 -0700 Subject: [PATCH] Issue #347: Throw a better exception when the user didn't add the MVC services. Introducing MarkerService for identifying if MvcServices were added. --- Mvc.sln | 13 ++++++ .../Internal/MvcMarkerService.cs | 13 ++++++ .../Internal/MvcServicesHelper.cs | 30 +++++++++++++ .../MvcRouteHandler.cs | 6 +++ .../Properties/Resources.Designer.cs | 16 +++++++ src/Microsoft.AspNet.Mvc.Core/Resources.resx | 3 ++ src/Microsoft.AspNet.Mvc/BuilderExtensions.cs | 5 +++ src/Microsoft.AspNet.Mvc/MvcServices.cs | 3 ++ .../Properties/Resources.Designer.cs | 45 +++++++++++++++++++ .../Internal/MvcServicesHelperTests.cs | 44 ++++++++++++++++++ .../MvcRouteHandlerTests.cs | 4 ++ .../MvcStartupTests.cs | 29 ++++++++++++ .../project.json | 1 + .../AddServicesWebSite.kproj | 31 +++++++++++++ test/WebSites/AddServicesWebSite/Startup.cs | 32 +++++++++++++ test/WebSites/AddServicesWebSite/project.json | 10 +++++ 16 files changed, 285 insertions(+) create mode 100644 src/Microsoft.AspNet.Mvc.Core/Internal/MvcMarkerService.cs create mode 100644 src/Microsoft.AspNet.Mvc.Core/Internal/MvcServicesHelper.cs create mode 100644 src/Microsoft.AspNet.Mvc/Properties/Resources.Designer.cs create mode 100644 test/Microsoft.AspNet.Mvc.Core.Test/Internal/MvcServicesHelperTests.cs create mode 100644 test/Microsoft.AspNet.Mvc.FunctionalTests/MvcStartupTests.cs create mode 100644 test/WebSites/AddServicesWebSite/AddServicesWebSite.kproj create mode 100644 test/WebSites/AddServicesWebSite/Startup.cs create mode 100644 test/WebSites/AddServicesWebSite/project.json diff --git a/Mvc.sln b/Mvc.sln index 0a7055d74d..7160ae03c8 100644 --- a/Mvc.sln +++ b/Mvc.sln @@ -72,6 +72,8 @@ Project("{8BB2217D-0F2D-49D1-97BC-3654ED321F3B}") = "ConnegWebsite", "test\WebSi EndProject Project("{8BB2217D-0F2D-49D1-97BC-3654ED321F3B}") = "AntiForgeryWebSite", "test\WebSites\AntiForgeryWebSite\AntiForgeryWebSite.kproj", "{A353B17E-A940-4CE8-8BF9-179E24A9041F}" EndProject +Project("{8BB2217D-0F2D-49D1-97BC-3654ED321F3B}") = "AddServicesWebSite", "test\WebSites\AddServicesWebSite\AddServicesWebSite.kproj", "{6A0B65CE-6B01-40D0-840D-EFF3680D1547}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -362,6 +364,16 @@ Global {A353B17E-A940-4CE8-8BF9-179E24A9041F}.Release|Mixed Platforms.ActiveCfg = Release|Any CPU {A353B17E-A940-4CE8-8BF9-179E24A9041F}.Release|Mixed Platforms.Build.0 = Release|Any CPU {A353B17E-A940-4CE8-8BF9-179E24A9041F}.Release|x86.ActiveCfg = Release|Any CPU + {6A0B65CE-6B01-40D0-840D-EFF3680D1547}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {6A0B65CE-6B01-40D0-840D-EFF3680D1547}.Debug|Any CPU.Build.0 = Debug|Any CPU + {6A0B65CE-6B01-40D0-840D-EFF3680D1547}.Debug|Mixed Platforms.ActiveCfg = Debug|Any CPU + {6A0B65CE-6B01-40D0-840D-EFF3680D1547}.Debug|Mixed Platforms.Build.0 = Debug|Any CPU + {6A0B65CE-6B01-40D0-840D-EFF3680D1547}.Debug|x86.ActiveCfg = Debug|Any CPU + {6A0B65CE-6B01-40D0-840D-EFF3680D1547}.Release|Any CPU.ActiveCfg = Release|Any CPU + {6A0B65CE-6B01-40D0-840D-EFF3680D1547}.Release|Any CPU.Build.0 = Release|Any CPU + {6A0B65CE-6B01-40D0-840D-EFF3680D1547}.Release|Mixed Platforms.ActiveCfg = Release|Any CPU + {6A0B65CE-6B01-40D0-840D-EFF3680D1547}.Release|Mixed Platforms.Build.0 = Release|Any CPU + {6A0B65CE-6B01-40D0-840D-EFF3680D1547}.Release|x86.ActiveCfg = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -396,5 +408,6 @@ Global {EE1AB716-F102-4CA3-AD2C-214A44B459A0} = {16703B76-C9F7-4C75-AE6C-53D92E308E3C} {C6E5AFFA-890A-448F-8DE3-878B1D3C9FC7} = {16703B76-C9F7-4C75-AE6C-53D92E308E3C} {A353B17E-A940-4CE8-8BF9-179E24A9041F} = {16703B76-C9F7-4C75-AE6C-53D92E308E3C} + {6A0B65CE-6B01-40D0-840D-EFF3680D1547} = {16703B76-C9F7-4C75-AE6C-53D92E308E3C} EndGlobalSection EndGlobal diff --git a/src/Microsoft.AspNet.Mvc.Core/Internal/MvcMarkerService.cs b/src/Microsoft.AspNet.Mvc.Core/Internal/MvcMarkerService.cs new file mode 100644 index 0000000000..ec3573b1f6 --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.Core/Internal/MvcMarkerService.cs @@ -0,0 +1,13 @@ +// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +namespace Microsoft.AspNet.Mvc.Internal +{ + /// + /// This is a Marker class which is used to determine if all the services were added + /// to when Mvc is loaded. + /// + public class MvcMarkerService + { + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Core/Internal/MvcServicesHelper.cs b/src/Microsoft.AspNet.Mvc.Core/Internal/MvcServicesHelper.cs new file mode 100644 index 0000000000..c018a8c114 --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.Core/Internal/MvcServicesHelper.cs @@ -0,0 +1,30 @@ +// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using Microsoft.AspNet.Mvc.Core; +using Microsoft.Framework.DependencyInjection; + +namespace Microsoft.AspNet.Mvc.Internal +{ + /// + /// Helper class which contains MvcServices related helpers. + /// + public static class MvcServicesHelper + { + /// + /// Throws InvalidOperationException when MvcMarkerService is not present + /// in the list of services. + /// + /// The list of services. + /// The type of service which needs to be searched for. + public static void ThrowIfMvcNotRegistered(IServiceProvider services) + { + if (services.GetServiceOrNull(typeof(MvcMarkerService)) == null) + { + throw new InvalidOperationException(Resources.FormatUnableToFindServices( + "IServiceCollection.AddMvc()", "IBuilder.UseServices(...)", "IBuilder.UseMvc(...)")); + } + } + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Core/MvcRouteHandler.cs b/src/Microsoft.AspNet.Mvc.Core/MvcRouteHandler.cs index 3a35dbfa69..58e2a02e94 100644 --- a/src/Microsoft.AspNet.Mvc.Core/MvcRouteHandler.cs +++ b/src/Microsoft.AspNet.Mvc.Core/MvcRouteHandler.cs @@ -6,6 +6,7 @@ using System.Diagnostics.Contracts; using System.Threading.Tasks; using Microsoft.AspNet.Http; using Microsoft.AspNet.Mvc.Core; +using Microsoft.AspNet.Mvc.Internal; using Microsoft.AspNet.Mvc.Logging; using Microsoft.AspNet.Routing; using Microsoft.Framework.DependencyInjection; @@ -31,6 +32,11 @@ namespace Microsoft.AspNet.Mvc public async Task RouteAsync([NotNull] RouteContext context) { var services = context.HttpContext.RequestServices; + + // Verify if AddMvc was done before calling UseMvc + // We use the MvcMarkerService to make sure if all the services were added. + MvcServicesHelper.ThrowIfMvcNotRegistered(services); + Contract.Assert(services != null); // TODO: Throw an error here that's descriptive enough so that diff --git a/src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs b/src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs index 99db9b5234..6128205e7e 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs @@ -1258,6 +1258,22 @@ namespace Microsoft.AspNet.Mvc.Core return string.Format(CultureInfo.CurrentCulture, GetString("RouteConstraintAttribute_InvalidKeyHandlingValue"), p0, p1); } + /// + /// Unable to find the required services. Please add all the required services by calling '{0}' inside the call to '{1}' or before calling '{2}' in the application startup code. + /// + internal static string UnableToFindServices + { + get { return GetString("UnableToFindServices"); } + } + + /// + /// Unable to find the required services. Please add all the required services by calling '{0}' inside the call to '{1}' or before calling '{2}' in the application startup code. + /// + internal static string FormatUnableToFindServices(object p0, object p1, object p2) + { + return string.Format(CultureInfo.CurrentCulture, GetString("UnableToFindServices"), p0, p1, p2); + } + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/src/Microsoft.AspNet.Mvc.Core/Resources.resx b/src/Microsoft.AspNet.Mvc.Core/Resources.resx index 8225d5379f..cb350b487e 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Resources.resx +++ b/src/Microsoft.AspNet.Mvc.Core/Resources.resx @@ -354,4 +354,7 @@ The value must be either '{0}' or '{1}'. + + Unable to find the required services. Please add all the required services by calling '{0}' inside the call to '{1}' or '{2}' in the application startup code. + \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc/BuilderExtensions.cs b/src/Microsoft.AspNet.Mvc/BuilderExtensions.cs index d274b40e01..dbb580d9e7 100644 --- a/src/Microsoft.AspNet.Mvc/BuilderExtensions.cs +++ b/src/Microsoft.AspNet.Mvc/BuilderExtensions.cs @@ -3,6 +3,7 @@ using System; using Microsoft.AspNet.Mvc; +using Microsoft.AspNet.Mvc.Internal; using Microsoft.AspNet.Mvc.Routing; using Microsoft.AspNet.Routing; @@ -24,6 +25,10 @@ namespace Microsoft.AspNet.Builder public static IBuilder UseMvc([NotNull] this IBuilder app, [NotNull] Action configureRoutes) { + // Verify if AddMvc was done before calling UseMvc + // We use the MvcMarkerService to make sure if all the services were added. + MvcServicesHelper.ThrowIfMvcNotRegistered(app.ApplicationServices); + var routes = new RouteBuilder { DefaultHandler = new MvcRouteHandler(), diff --git a/src/Microsoft.AspNet.Mvc/MvcServices.cs b/src/Microsoft.AspNet.Mvc/MvcServices.cs index e5d4e993cf..195f331a5e 100644 --- a/src/Microsoft.AspNet.Mvc/MvcServices.cs +++ b/src/Microsoft.AspNet.Mvc/MvcServices.cs @@ -4,6 +4,7 @@ using System.Collections.Generic; using Microsoft.AspNet.Mvc.Core; using Microsoft.AspNet.Mvc.Filters; +using Microsoft.AspNet.Mvc.Internal; using Microsoft.AspNet.Mvc.ModelBinding; using Microsoft.AspNet.Mvc.OptionDescriptors; using Microsoft.AspNet.Mvc.Razor; @@ -117,6 +118,8 @@ namespace Microsoft.AspNet.Mvc typeof(HtmlHelper<>), implementationInstance: null, lifecycle: LifecycleKind.Transient); + + yield return describe.Transient(); } } } diff --git a/src/Microsoft.AspNet.Mvc/Properties/Resources.Designer.cs b/src/Microsoft.AspNet.Mvc/Properties/Resources.Designer.cs new file mode 100644 index 0000000000..3ee6068e7c --- /dev/null +++ b/src/Microsoft.AspNet.Mvc/Properties/Resources.Designer.cs @@ -0,0 +1,45 @@ +// +namespace Microsoft.AspNet.Mvc +{ + using System.Reflection; + using System.Resources; + + internal static class Resources + { + private static readonly ResourceManager _resourceManager + = new ResourceManager("Microsoft.AspNet.Mvc.Resources", typeof(Resources).GetTypeInfo().Assembly); + + /// + /// Unable to find the required services. Please add all the required services by calling AddMvc() before calling UseMvc()/UseServices() in the Application Startup. + /// + internal static string UnableToFindServices + { + get { return GetString("UnableToFindServices"); } + } + + /// + /// Unable to find the required services. Please add all the required services by calling AddMvc() before calling UseMvc()/UseServices() in the Application Startup. + /// + internal static string FormatUnableToFindServices() + { + return GetString("UnableToFindServices"); + } + + private static string GetString(string name, params string[] formatterNames) + { + var value = _resourceManager.GetString(name); + + System.Diagnostics.Debug.Assert(value != null); + + if (formatterNames != null) + { + for (var i = 0; i < formatterNames.Length; i++) + { + value = value.Replace("{" + formatterNames[i] + "}", "{" + i + "}"); + } + } + + return value; + } + } +} diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/Internal/MvcServicesHelperTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/Internal/MvcServicesHelperTests.cs new file mode 100644 index 0000000000..04510a91c4 --- /dev/null +++ b/test/Microsoft.AspNet.Mvc.Core.Test/Internal/MvcServicesHelperTests.cs @@ -0,0 +1,44 @@ +// Copyright (c) Microsoft Open Technologies, Inc. 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.Collections.Generic; +using Microsoft.AspNet.Mvc.Internal; +using Moq; +using Xunit; + +namespace Microsoft.AspNet.Mvc +{ + public class MvcServicesHelperTests + { + [Fact] + public void MvcServicesHelperThrowsIfServiceIsAbsent() + { + // Arrange + var services = new Mock(); + services.Setup(o => o.GetService(typeof(IEnumerable))) + .Returns(new List()); + var expectedMessage = "Unable to find the required services. Please add all the required " + + "services by calling 'IServiceCollection.AddMvc()' inside the call to 'IBuilder.UseServices(...)' " + + "or 'IBuilder.UseMvc(...)' in the application startup code."; + + // Act & Assert + var ex = Assert.Throws( + () => MvcServicesHelper.ThrowIfMvcNotRegistered(services.Object)); + Assert.Equal(expectedMessage, ex.Message); + } + + [Fact] + public void MvcServicesHelperDoesNotThrowIfServiceExists() + { + // Arrange + var services = new Mock(); + var expectedOutput = new MvcMarkerService(); + services.Setup(o => o.GetService(typeof(IEnumerable))) + .Returns(new List { expectedOutput }); + + // Act & Assert + Assert.DoesNotThrow(() => MvcServicesHelper.ThrowIfMvcNotRegistered(services.Object)); + } + } +} \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/MvcRouteHandlerTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/MvcRouteHandlerTests.cs index 2ef9808e75..0faf10ca08 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/MvcRouteHandlerTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/MvcRouteHandlerTests.cs @@ -2,8 +2,10 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections.Generic; using System.Threading.Tasks; using Microsoft.AspNet.Http; +using Microsoft.AspNet.Mvc.Internal; using Microsoft.AspNet.Mvc.Logging; using Microsoft.AspNet.Routing; using Microsoft.Framework.DependencyInjection; @@ -192,6 +194,8 @@ namespace Microsoft.AspNet.Mvc .Returns(invokerFactory); httpContext.Setup(h => h.RequestServices.GetService(typeof(ILoggerFactory))) .Returns(loggerFactory); + httpContext.Setup(h => h.RequestServices.GetService(typeof(IEnumerable))) + .Returns(new List { new MvcMarkerService() }); return new RouteContext(httpContext.Object); } diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/MvcStartupTests.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/MvcStartupTests.cs new file mode 100644 index 0000000000..8dd98a966d --- /dev/null +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/MvcStartupTests.cs @@ -0,0 +1,29 @@ +// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using Microsoft.AspNet.Builder; +using Microsoft.AspNet.TestHost; +using Xunit; + +namespace Microsoft.AspNet.Mvc.FunctionalTests +{ + public class MvcStartupTests + { + private readonly IServiceProvider _provider = TestHelper.CreateServices("AddServicesWebSite"); + private readonly Action _app = new AddServicesWebSite.Startup().Configure; + + [Fact] + public void MvcThrowsWhenRequiredServicesAreNotAdded() + { + // Arrange + var expectedMessage = "Unable to find the required services. Please add all the required " + + "services by calling 'IServiceCollection.AddMvc()' inside the call to 'IBuilder.UseServices(...)' " + + "or 'IBuilder.UseMvc(...)' in the application startup code."; + + // Act & Assert + var ex = Assert.Throws(() => TestServer.Create(_provider, _app)); + Assert.Equal(expectedMessage, ex.Message); + } + } +} \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/project.json b/test/Microsoft.AspNet.Mvc.FunctionalTests/project.json index a56aec347d..52c6c4f7b2 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/project.json +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/project.json @@ -4,6 +4,7 @@ }, "dependencies": { "ActivatorWebSite": "", + "AddServicesWebSite": "", "AntiForgeryWebSite": "", "BasicWebSite": "", "CompositeViewEngine": "", diff --git a/test/WebSites/AddServicesWebSite/AddServicesWebSite.kproj b/test/WebSites/AddServicesWebSite/AddServicesWebSite.kproj new file mode 100644 index 0000000000..9bdc9670d6 --- /dev/null +++ b/test/WebSites/AddServicesWebSite/AddServicesWebSite.kproj @@ -0,0 +1,31 @@ + + + + 12.0 + $(MSBuildExtensionsPath32)\Microsoft\VisualStudio\v$(VisualStudioVersion) + + + Debug + AnyCPU + + + + 6a0b65ce-6b01-40d0-840d-eff3680d1547 + Web + + + ConsoleDebugger + + + WebDebugger + + + + + 2.0 + + + 38820 + + + \ No newline at end of file diff --git a/test/WebSites/AddServicesWebSite/Startup.cs b/test/WebSites/AddServicesWebSite/Startup.cs new file mode 100644 index 0000000000..9397f5956c --- /dev/null +++ b/test/WebSites/AddServicesWebSite/Startup.cs @@ -0,0 +1,32 @@ +// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.AspNet.Builder; +using Microsoft.AspNet.Routing; + +namespace AddServicesWebSite +{ + public class Startup + { + public void Configure(IBuilder app) + { + var configuration = app.GetTestConfiguration(); + + // Not calling AddMvc() here. + // The purpose of the Website is to demonstrate that it throws + // when AddMvc() is not called. + + // Add MVC to the request pipeline + app.UseMvc(routes => + { + routes.MapRoute("areaRoute", + "{area:exists}/{controller}/{action}", + new { controller = "Home", action = "Index" }); + + routes.MapRoute("ActionAsMethod", "{controller}/{action}", + defaults: new { controller = "Home", action = "Index" }); + + }); + } + } +} diff --git a/test/WebSites/AddServicesWebSite/project.json b/test/WebSites/AddServicesWebSite/project.json new file mode 100644 index 0000000000..81cb7a4e8b --- /dev/null +++ b/test/WebSites/AddServicesWebSite/project.json @@ -0,0 +1,10 @@ +{ + "dependencies": { + "Microsoft.AspNet.Mvc": "", + "Microsoft.AspNet.Mvc.TestConfiguration": "" + }, + "frameworks": { + "net45": { }, + "k10": { } + } +}