From 4cd30128862d1cf7c5fab535cb7d5d62e45182c1 Mon Sep 17 00:00:00 2001 From: Kiran Challa Date: Tue, 8 Mar 2016 09:13:04 -0800 Subject: [PATCH] Fix mixed route for action error message --- .../ControllerActionDescriptorBuilder.cs | 10 ++- ...ControllerActionDescriptorProviderTests.cs | 76 ++++++++++++++++++- 2 files changed, 82 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionDescriptorBuilder.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionDescriptorBuilder.cs index dd72658f3a..313bd1506e 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionDescriptorBuilder.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionDescriptorBuilder.cs @@ -721,8 +721,14 @@ namespace Microsoft.AspNetCore.Mvc.Internal { var routeTemplate = action.AttributeRouteInfo?.Template ?? nullTemplate; - var verbs = action.ActionConstraints.OfType().FirstOrDefault()?.HttpMethods; - var formattedVerbs = string.Join(", ", verbs.OrderBy(v => v, StringComparer.Ordinal)); + var verbs = action.ActionConstraints?.OfType() + .FirstOrDefault()?.HttpMethods; + + var formattedVerbs = string.Empty; + if (verbs != null) + { + formattedVerbs = string.Join(", ", verbs.OrderBy(v => v, StringComparer.OrdinalIgnoreCase)); + } var description = Resources.FormatAttributeRoute_MixedAttributeAndConventionallyRoutedActions_ForMethod_Item( diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionDescriptorProviderTests.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionDescriptorProviderTests.cs index d953c8fed6..6330aea5eb 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionDescriptorProviderTests.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionDescriptorProviderTests.cs @@ -12,8 +12,6 @@ using Microsoft.AspNetCore.Mvc.ApplicationModels; using Microsoft.AspNetCore.Mvc.ApplicationParts; using Microsoft.AspNetCore.Mvc.Controllers; using Microsoft.AspNetCore.Mvc.Filters; -using Microsoft.AspNetCore.Mvc.Infrastructure; -using Microsoft.AspNetCore.Mvc.Internal; using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.AspNetCore.Mvc.Routing; using Microsoft.AspNetCore.Routing.Tree; @@ -775,6 +773,49 @@ namespace Microsoft.AspNetCore.Mvc.Internal VerifyMultiLineError(expectedMessage, exception.Message, unorderedStart: 1, unorderedLineCount: 2); } + // Verify that the expected exception and error message is thrown even when the user builds the model + // incorrectly. + [Fact] + public void AttributeRouting_ThrowsIfAttributeRoutedAndNonAttributedActions_OnTheSameMethod_UsingCustomConvention() + { + // Arrange + var controllerTypeInfo = typeof(UserController).GetTypeInfo(); + var controllerTypeProvider = new StaticControllerTypeProvider(new[] { controllerTypeInfo }); + var options = new TestOptionsManager(); + options.Value.Conventions.Add(new TestRoutingConvention()); + var modelProvider = new DefaultApplicationModelProvider(options); + var provider = new ControllerActionDescriptorProvider( + controllerTypeProvider, + new[] { modelProvider }, + options); + var assemblyName = controllerTypeInfo.Assembly.GetName().Name; + var expectedMessage = + "The following errors occurred with attribute routing information:" + + Environment.NewLine + + Environment.NewLine + + "Error 1:" + + Environment.NewLine + + $"A method '{controllerTypeInfo.FullName}.GetUser ({assemblyName})'" + + " must not define attribute routed actions and non attribute routed actions at the same time:" + + Environment.NewLine + + $"Action: '{controllerTypeInfo.FullName}.GetUser ({assemblyName})' " + + "- Route Template: '(none)' - " + + "HTTP Verbs: ''" + + Environment.NewLine + + $"Action: '{controllerTypeInfo.FullName}.GetUser ({assemblyName})' " + + "- Route Template: 'Microsoft/AspNetCore/Mvc/Internal/User/GetUser/{id?}' - " + + "HTTP Verbs: ''" + Environment.NewLine + + Environment.NewLine + + "Use 'AcceptVerbsAttribute' to create a single route that allows multiple HTTP verbs and defines a " + + "route, or set a route template in all attributes that constrain HTTP verbs."; + + // Act + var exception = Assert.Throws(() => provider.GetDescriptors()); + + // Assert + VerifyMultiLineError(expectedMessage, exception.Message, unorderedStart: 1, unorderedLineCount: 2); + } + [Fact] public void AttributeRouting_RouteOnControllerAndAction_CreatesActionDescriptorWithoutHttpConstraints() { @@ -2063,5 +2104,36 @@ namespace Microsoft.AspNetCore.Mvc.Internal application.ApiExplorer.IsVisible = _isVisible; } } + + private class TestRoutingConvention : IApplicationModelConvention + { + public void Apply(ApplicationModel application) + { + foreach (var controller in application.Controllers) + { + var hasAttributeRouteModels = controller.Selectors + .Any(selector => selector.AttributeRouteModel != null); + if (!hasAttributeRouteModels) + { + var template = controller.ControllerType.Namespace.Replace('.', '/') + + "/[controller]/[action]/{id?}"; + var attributeRouteModel = new AttributeRouteModel() + { + Template = template + }; + + controller.Selectors.Add(new SelectorModel { AttributeRouteModel = attributeRouteModel }); + } + } + } + } + + private class UserController : Controller + { + public string GetUser(int id) + { + return string.Format("User {0} retrieved successfully", id); + } + } } }