From 5b851c49a337e49001ed2351253ab0f52b696040 Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Fri, 15 May 2015 13:46:58 -0700 Subject: [PATCH] Throw when ConfigureServices has wrong signature --- .../Internal/HostingUtilities.cs | 2 +- .../Internal/RequestServicesContainer.cs | 1 - src/Microsoft.AspNet.Hosting/Program.cs | 2 ++ .../Server/ServerLoader.cs | 4 +-- .../Startup/ConfigureDelegate.cs | 5 +-- .../Startup/ConfigureServicesDelegate.cs | 33 ++++++++++--------- .../Fakes/Startup.cs | 2 +- .../HostingEngineTests.cs | 17 ++++++++++ 8 files changed, 42 insertions(+), 24 deletions(-) diff --git a/src/Microsoft.AspNet.Hosting/Internal/HostingUtilities.cs b/src/Microsoft.AspNet.Hosting/Internal/HostingUtilities.cs index cec8d4d762..9d868fe532 100644 --- a/src/Microsoft.AspNet.Hosting/Internal/HostingUtilities.cs +++ b/src/Microsoft.AspNet.Hosting/Internal/HostingUtilities.cs @@ -13,7 +13,7 @@ namespace Microsoft.AspNet.Hosting.Internal internal static Tuple SplitTypeName(string identifier) { string typeName = null; - string assemblyName = identifier.Trim(); + var assemblyName = identifier.Trim(); var parts = identifier.Split(new[] { ',' }, 2); if (parts.Length == 2) { diff --git a/src/Microsoft.AspNet.Hosting/Internal/RequestServicesContainer.cs b/src/Microsoft.AspNet.Hosting/Internal/RequestServicesContainer.cs index 74e6df0fe2..a4ab07d546 100644 --- a/src/Microsoft.AspNet.Hosting/Internal/RequestServicesContainer.cs +++ b/src/Microsoft.AspNet.Hosting/Internal/RequestServicesContainer.cs @@ -39,7 +39,6 @@ namespace Microsoft.AspNet.Hosting.Internal private IServiceProvider PriorRequestServices { get; set; } private IServiceScope Scope { get; set; } - // CONSIDER: this could be an extension method on HttpContext instead public static RequestServicesContainer EnsureRequestServices(HttpContext httpContext, IServiceProvider services) { diff --git a/src/Microsoft.AspNet.Hosting/Program.cs b/src/Microsoft.AspNet.Hosting/Program.cs index 495d1380b0..8e40a30f0e 100644 --- a/src/Microsoft.AspNet.Hosting/Program.cs +++ b/src/Microsoft.AspNet.Hosting/Program.cs @@ -3,6 +3,8 @@ using System; using System.IO; +using System.Threading; +using System.Threading.Tasks; using Microsoft.AspNet.Hosting.Internal; using Microsoft.Framework.ConfigurationModel; using Microsoft.Framework.DependencyInjection; diff --git a/src/Microsoft.AspNet.Hosting/Server/ServerLoader.cs b/src/Microsoft.AspNet.Hosting/Server/ServerLoader.cs index 0327ae6a31..51cdecfe36 100644 --- a/src/Microsoft.AspNet.Hosting/Server/ServerLoader.cs +++ b/src/Microsoft.AspNet.Hosting/Server/ServerLoader.cs @@ -26,8 +26,8 @@ namespace Microsoft.AspNet.Hosting.Server } var nameParts = HostingUtilities.SplitTypeName(serverFactoryIdentifier); - string typeName = nameParts.Item1; - string assemblyName = nameParts.Item2; + var typeName = nameParts.Item1; + var assemblyName = nameParts.Item2; var assembly = Assembly.Load(new AssemblyName(assemblyName)); if (assembly == null) diff --git a/src/Microsoft.AspNet.Hosting/Startup/ConfigureDelegate.cs b/src/Microsoft.AspNet.Hosting/Startup/ConfigureDelegate.cs index 3275f056d9..50f2b22005 100644 --- a/src/Microsoft.AspNet.Hosting/Startup/ConfigureDelegate.cs +++ b/src/Microsoft.AspNet.Hosting/Startup/ConfigureDelegate.cs @@ -20,10 +20,7 @@ namespace Microsoft.AspNet.Hosting.Startup public MethodInfo MethodInfo { get; } - public Action Build(object instance) - { - return builder => Invoke(instance, builder); - } + public Action Build(object instance) => builder => Invoke(instance, builder); private void Invoke(object instance, IApplicationBuilder builder) { diff --git a/src/Microsoft.AspNet.Hosting/Startup/ConfigureServicesDelegate.cs b/src/Microsoft.AspNet.Hosting/Startup/ConfigureServicesDelegate.cs index d500ccea06..0935c7a69d 100644 --- a/src/Microsoft.AspNet.Hosting/Startup/ConfigureServicesDelegate.cs +++ b/src/Microsoft.AspNet.Hosting/Startup/ConfigureServicesDelegate.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.Linq; using System.Reflection; using Microsoft.Framework.DependencyInjection; +using Microsoft.Framework.Internal; namespace Microsoft.AspNet.Hosting.Startup { @@ -11,32 +13,33 @@ namespace Microsoft.AspNet.Hosting.Startup public class ConfigureServicesBuilder { - public ConfigureServicesBuilder(MethodInfo configureServices) + public ConfigureServicesBuilder([NotNull] MethodInfo configureServices) { + // Only support IServiceCollection parameters + var parameters = configureServices.GetParameters(); + if (parameters.Length > 1 || + parameters.Any(p => p.ParameterType != typeof(IServiceCollection))) + { + throw new InvalidOperationException("ConfigureServices can take at most a single IServiceCollection parameter."); + } + MethodInfo = configureServices; } public MethodInfo MethodInfo { get; } - public ConfigureServicesDelegate Build(object instance) - { - return services => Invoke(instance, services); - } + public ConfigureServicesDelegate Build(object instance) => services => Invoke(instance, services); - private IServiceProvider Invoke(object instance, IServiceCollection exportServices) + private IServiceProvider Invoke(object instance, [NotNull] IServiceCollection exportServices) { - var parameterInfos = MethodInfo.GetParameters(); - var parameters = new object[parameterInfos.Length]; - for (var index = 0; index != parameterInfos.Length; ++index) + var parameters = new object[MethodInfo.GetParameters().Length]; + + // Ctor ensures we have at most one IServiceCollection parameter + if (parameters.Length > 0) { - var parameterInfo = parameterInfos[index]; - if (exportServices != null && parameterInfo.ParameterType == typeof(IServiceCollection)) - { - parameters[index] = exportServices; - } + parameters[0] = exportServices; } - // REVIEW: We null ref if exportServices is null, cuz it should not be null return MethodInfo.Invoke(instance, parameters) as IServiceProvider ?? exportServices.BuildServiceProvider(); } } diff --git a/test/Microsoft.AspNet.Hosting.Tests/Fakes/Startup.cs b/test/Microsoft.AspNet.Hosting.Tests/Fakes/Startup.cs index 09a4a3360f..87950f61dc 100644 --- a/test/Microsoft.AspNet.Hosting.Tests/Fakes/Startup.cs +++ b/test/Microsoft.AspNet.Hosting.Tests/Fakes/Startup.cs @@ -82,7 +82,7 @@ namespace Microsoft.AspNet.Hosting.Fakes return services.BuildServiceProvider(); } - public IServiceProvider ConfigureProviderArgsServices(IApplicationBuilder me) + public IServiceProvider ConfigureProviderArgsServices() { var services = new ServiceCollection().AddOptions(); services.Configure(o => diff --git a/test/Microsoft.AspNet.Hosting.Tests/HostingEngineTests.cs b/test/Microsoft.AspNet.Hosting.Tests/HostingEngineTests.cs index 6ad8ab0b87..2234fcaa5d 100644 --- a/test/Microsoft.AspNet.Hosting.Tests/HostingEngineTests.cs +++ b/test/Microsoft.AspNet.Hosting.Tests/HostingEngineTests.cs @@ -323,6 +323,23 @@ namespace Microsoft.AspNet.Hosting } } + [Fact] + public void HostingEngine_ThrowsForBadConfigureServiceSignature() + { + var engine = CreateBuilder() + .UseServer(this) + .UseStartup() + .Build(); + var ex = Assert.Throws(() => engine.Start()); + Assert.True(ex.Message.Contains("ConfigureServices")); + } + + public class BadConfigureServicesStartup + { + public void ConfigureServices(IServiceCollection services, int gunk) { } + public void Configure(IApplicationBuilder app) { } + } + private IHostingEngine CreateHostingEngine(RequestDelegate requestDelegate) { var applicationBuilder = new Mock();