From ee419e24424bff2c46cf60f72c977c33d8ce8c47 Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Fri, 23 Jan 2015 15:13:59 -0800 Subject: [PATCH] Add ApiExplorer details to ApplicationModel This change allows you to set global defaults for ApiExplorer on the ApplicationModel. Additionally, we're more lenient about configuring ApiExplorer = on with conventional routing. If you turn on ApiExplorer at the application level, we'll just skip over all conventionally routed controllers instead of throwing. --- .../ApplicationModels/ActionModel.cs | 7 +- .../ApplicationModels/ApplicationModel.cs | 14 ++++ .../ApplicationModels/ControllerModel.cs | 7 ++ .../ControllerActionDescriptorBuilder.cs | 45 +++++++++--- ...ControllerActionDescriptorProviderTests.cs | 69 ++++++++++++++++++- 5 files changed, 128 insertions(+), 14 deletions(-) diff --git a/src/Microsoft.AspNet.Mvc.Core/ApplicationModels/ActionModel.cs b/src/Microsoft.AspNet.Mvc.Core/ApplicationModels/ActionModel.cs index 0829b27abc..fbb06b8dfd 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ApplicationModels/ActionModel.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ApplicationModels/ActionModel.cs @@ -58,8 +58,11 @@ namespace Microsoft.AspNet.Mvc.ApplicationModels /// Gets or sets the for this action. /// /// - /// Setting the value of any properties on will override any - /// values set on the associated . + /// allows configuration of settings for ApiExplorer + /// which apply to the action. + /// + /// Settings applied by override settings from + /// and . /// public ApiExplorerModel ApiExplorer { get; set; } diff --git a/src/Microsoft.AspNet.Mvc.Core/ApplicationModels/ApplicationModel.cs b/src/Microsoft.AspNet.Mvc.Core/ApplicationModels/ApplicationModel.cs index cbb87b98d8..aa93de8246 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ApplicationModels/ApplicationModel.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ApplicationModels/ApplicationModel.cs @@ -9,10 +9,24 @@ namespace Microsoft.AspNet.Mvc.ApplicationModels { public ApplicationModel() { + ApiExplorer = new ApiExplorerModel(); Controllers = new List(); Filters = new List(); } + /// + /// Gets or sets the for the application. + /// + /// + /// allows configuration of default settings + /// for ApiExplorer that apply to all actions unless overridden by + /// or . + /// + /// If using to set to + /// true, this setting will only be honored for actions which use attribute routing. + /// + public ApiExplorerModel ApiExplorer { get; set; } + public IList Controllers { get; private set; } public IList Filters { get; private set; } diff --git a/src/Microsoft.AspNet.Mvc.Core/ApplicationModels/ControllerModel.cs b/src/Microsoft.AspNet.Mvc.Core/ApplicationModels/ControllerModel.cs index ef22386b47..5377fadfed 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ApplicationModels/ControllerModel.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ApplicationModels/ControllerModel.cs @@ -51,6 +51,13 @@ namespace Microsoft.AspNet.Mvc.ApplicationModels /// /// Gets or sets the for this controller. /// + /// + /// allows configuration of settings for ApiExplorer + /// which apply to all actions in the controller unless overridden by . + /// + /// Settings applied by override settings from + /// . + /// public ApiExplorerModel ApiExplorer { get; set; } public ApplicationModel Application { get; set; } diff --git a/src/Microsoft.AspNet.Mvc.Core/ControllerActionDescriptorBuilder.cs b/src/Microsoft.AspNet.Mvc.Core/ControllerActionDescriptorBuilder.cs index d45aeb2ad0..e4998dbd65 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ControllerActionDescriptorBuilder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ControllerActionDescriptorBuilder.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Globalization; using System.Linq; using System.Reflection; @@ -55,7 +56,7 @@ namespace Microsoft.AspNet.Mvc actionDescriptor.ControllerName = controller.ControllerName; actionDescriptor.ControllerTypeInfo = controller.ControllerType; - AddApiExplorerInfo(actionDescriptor, action, controller); + AddApiExplorerInfo(actionDescriptor, application, controller, action); AddRouteConstraints(removalConstraints, actionDescriptor, controller, action); if (IsAttributeRoutedAction(actionDescriptor)) @@ -284,18 +285,40 @@ namespace Microsoft.AspNet.Mvc private static void AddApiExplorerInfo( ControllerActionDescriptor actionDescriptor, - ActionModel action, - ControllerModel controller) + ApplicationModel application, + ControllerModel controller, + ActionModel action) { - var apiExplorerIsVisible = action.ApiExplorer?.IsVisible ?? controller.ApiExplorer?.IsVisible ?? false; - if (apiExplorerIsVisible) + + var isVisible = + action.ApiExplorer?.IsVisible ?? + controller.ApiExplorer?.IsVisible ?? + application.ApiExplorer?.IsVisible ?? + false; + + var isVisibleSetOnActionOrController = + action.ApiExplorer?.IsVisible ?? + controller.ApiExplorer?.IsVisible ?? + false; + + // ApiExplorer isn't supported on conventional-routed actions, but we still allow you to configure + // it at the application level when you have a mix of controller types. We'll just skip over enabling + // ApiExplorer for conventional-routed controllers when this happens. + var isVisibleSetOnApplication = application.ApiExplorer?.IsVisible ?? false; + + if (isVisibleSetOnActionOrController && !IsAttributeRoutedAction(actionDescriptor)) { - if (!IsAttributeRoutedAction(actionDescriptor)) - { - // ApiExplorer is only supported on attribute routed actions. - throw new InvalidOperationException(Resources.FormatApiExplorer_UnsupportedAction( - actionDescriptor.DisplayName)); - } + // ApiExplorer is only supported on attribute routed actions. + throw new InvalidOperationException(Resources.FormatApiExplorer_UnsupportedAction( + actionDescriptor.DisplayName)); + } + else if (isVisibleSetOnApplication && !IsAttributeRoutedAction(actionDescriptor)) + { + // This is the case where we're going to be lenient, just ignore it. + } + else if (isVisible) + { + Debug.Assert(IsAttributeRoutedAction(actionDescriptor)); var apiExplorerActionData = new ApiDescriptionActionData() { diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ControllerActionDescriptorProviderTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ControllerActionDescriptorProviderTests.cs index ecf66d8348..b6be78d158 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ControllerActionDescriptorProviderTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ControllerActionDescriptorProviderTests.cs @@ -1092,6 +1092,38 @@ namespace Microsoft.AspNet.Mvc.Test Assert.Equal("Store", action.GetProperty().GroupName); } + [Fact] + public void ApiExplorer_IsVisibleOnApplication_CanOverrideOnController() + { + // Arrange + var convention = new ApiExplorerIsVisibleConvention(isVisible: true); + var provider = GetProvider(typeof(ApiExplorerExplicitlyNotVisibleController).GetTypeInfo(), convention); + + // Act + var actions = provider.GetDescriptors(); + + // Assert + var action = Assert.Single(actions); + Assert.Null(action.GetProperty()); + } + + [Fact] + public void ApiExplorer_IsVisibleOnApplication_CanOverrideOnAction() + { + // Arrange + var convention = new ApiExplorerIsVisibleConvention(isVisible: true); + var provider = GetProvider( + typeof(ApiExplorerExplicitlyNotVisibleOnActionController).GetTypeInfo(), + convention); + + // Act + var actions = provider.GetDescriptors(); + + // Assert + var action = Assert.Single(actions); + Assert.Null(action.GetProperty()); + } + [Theory] [InlineData("A", typeof(ApiExplorerEnabledConventionalRoutedController))] [InlineData("A", typeof(ApiExplorerEnabledActionConventionalRoutedController))] @@ -1110,6 +1142,23 @@ namespace Microsoft.AspNet.Mvc.Test Assert.Equal(expected, ex.Message); } + [Fact] + public void ApiExplorer_SkipsConventionalRoutedController_WhenConfiguredOnApplication() + { + // Arrange + var convention = new ApiExplorerIsVisibleConvention(isVisible: true); + var provider = GetProvider( + typeof(ConventionallyRoutedController).GetTypeInfo(), + convention); + + // Act + var actions = provider.GetDescriptors(); + + // Assert + var action = Assert.Single(actions); + Assert.Null(action.GetProperty()); + } + // Verifies the sequence of conventions running [Fact] public void ApplyConventions_RunsInOrderOfDecreasingScope() @@ -1352,7 +1401,7 @@ namespace Microsoft.AspNet.Mvc.Test private ControllerActionDescriptorProvider GetProvider( TypeInfo type, - IOptions options) + IApplicationModelConvention convention) { var modelBuilder = new StaticControllerModelBuilder(type); @@ -1361,6 +1410,9 @@ namespace Microsoft.AspNet.Mvc.Test .SetupGet(ap => ap.CandidateAssemblies) .Returns(new Assembly[] { type.Assembly }); + var options = new MockMvcOptionsAccessor(); + options.Options.ApplicationModelConventions.Add(convention); + return new ControllerActionDescriptorProvider( assemblyProvider.Object, modelBuilder, @@ -1894,5 +1946,20 @@ namespace Microsoft.AspNet.Mvc.Test { } } + + private class ApiExplorerIsVisibleConvention : IApplicationModelConvention + { + private bool _isVisible; + + public ApiExplorerIsVisibleConvention(bool isVisible) + { + _isVisible = isVisible; + } + + public void Apply([NotNull] ApplicationModel application) + { + application.ApiExplorer.IsVisible = _isVisible; + } + } } }