From 3add4746feb95640fb1f677b2bd2f4e7046e77e3 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 12 May 2020 12:29:03 +1000 Subject: [PATCH] Gets body model binders working per controller and figures out how the application model works --- ...racoApiBehaviorApplicationModelProvider.cs | 22 ++--- .../UmbracoJsonModelBinderConvention.cs | 24 ++++++ .../UmbracoWebServiceCollectionExtensions.cs | 22 ++--- .../Install/InstallApiController.cs | 4 +- .../UmbracoJsonModelBinderProvider.cs | 85 +------------------ 5 files changed, 43 insertions(+), 114 deletions(-) create mode 100644 src/Umbraco.Web.Common/ApplicationModels/UmbracoJsonModelBinderConvention.cs diff --git a/src/Umbraco.Web.Common/ApplicationModels/UmbracoApiBehaviorApplicationModelProvider.cs b/src/Umbraco.Web.Common/ApplicationModels/UmbracoApiBehaviorApplicationModelProvider.cs index 54cb650ce0..d5268a884f 100644 --- a/src/Umbraco.Web.Common/ApplicationModels/UmbracoApiBehaviorApplicationModelProvider.cs +++ b/src/Umbraco.Web.Common/ApplicationModels/UmbracoApiBehaviorApplicationModelProvider.cs @@ -5,23 +5,9 @@ using System.Collections.Generic; using System.Linq; using Umbraco.Core; using Umbraco.Web.Common.Attributes; -using Umbraco.Web.Common.ModelBinding; namespace Umbraco.Web.Common.ApplicationModels { - public class NewtonsoftJsonModelBinderConvention : IActionModelConvention - { - public void Apply(ActionModel action) - { - foreach(var p in action.Parameters) - { - if (p.BindingInfo?.BindingSource == BindingSource.Body) - { - p.BindingInfo.BinderType = typeof(UmbracoJsonModelBinder); - } - } - } - } /// /// A custom application model provider for Umbraco controllers @@ -34,6 +20,10 @@ namespace Umbraco.Web.Common.ApplicationModels /// This is nearly a copy of aspnetcore's ApiBehaviorApplicationModelProvider which supplies a convention for the /// [ApiController] attribute, however that convention is too strict for our purposes so we will have our own. /// + /// + /// See https://shazwazza.com/post/custom-body-model-binding-per-controller-in-asp-net-core/ + /// and https://github.com/dotnet/aspnetcore/issues/21724 + /// /// public class UmbracoApiBehaviorApplicationModelProvider : IApplicationModelProvider { @@ -50,7 +40,9 @@ namespace Umbraco.Web.Common.ApplicationModels new ConsumesConstraintForFormFileParameterConvention(), // If an controller accepts files, it must accept multipart/form-data. new InferParameterBindingInfoConvention(modelMetadataProvider), // no need for [FromBody] everywhere, A complex type parameter is assigned to FromBody - new NewtonsoftJsonModelBinderConvention() + // This ensures that all parameters of type BindingSource.Body (based on the above InferParameterBindingInfoConvention) are bound + // using our own UmbracoJsonModelBinder + new UmbracoJsonModelBinderConvention() }; // TODO: Need to determine exactly how this affects errors diff --git a/src/Umbraco.Web.Common/ApplicationModels/UmbracoJsonModelBinderConvention.cs b/src/Umbraco.Web.Common/ApplicationModels/UmbracoJsonModelBinderConvention.cs new file mode 100644 index 0000000000..96c60398f0 --- /dev/null +++ b/src/Umbraco.Web.Common/ApplicationModels/UmbracoJsonModelBinderConvention.cs @@ -0,0 +1,24 @@ +using Microsoft.AspNetCore.Mvc.ApplicationModels; +using Microsoft.AspNetCore.Mvc.ModelBinding; +using Umbraco.Web.Common.ModelBinding; +using System.Linq; + +namespace Umbraco.Web.Common.ApplicationModels +{ + /// + /// Applies the body model binder to any parameter binding source of type + /// + /// + /// For this to work Microsoft's own convention must be executed before this one + /// + public class UmbracoJsonModelBinderConvention : IActionModelConvention + { + public void Apply(ActionModel action) + { + foreach (var p in action.Parameters.Where(p => p.BindingInfo?.BindingSource == BindingSource.Body)) + { + p.BindingInfo.BinderType = typeof(UmbracoJsonModelBinder); + } + } + } +} diff --git a/src/Umbraco.Web.Common/Extensions/UmbracoWebServiceCollectionExtensions.cs b/src/Umbraco.Web.Common/Extensions/UmbracoWebServiceCollectionExtensions.cs index 0278630be8..fb7d379ae5 100644 --- a/src/Umbraco.Web.Common/Extensions/UmbracoWebServiceCollectionExtensions.cs +++ b/src/Umbraco.Web.Common/Extensions/UmbracoWebServiceCollectionExtensions.cs @@ -34,9 +34,7 @@ namespace Umbraco.Extensions public static IServiceCollection AddUmbracoWebComponents(this IServiceCollection services) { services.TryAddSingleton(); - services.TryAddSingleton(); - services.TryAddSingleton(); - //services.ConfigureOptions(); + services.ConfigureOptions(); services.TryAddEnumerable(ServiceDescriptor.Transient()); // TODO: We need to avoid this, surely there's a way? See ContainerTests.BuildServiceProvider_Before_Host_Is_Configured @@ -118,7 +116,7 @@ namespace Umbraco.Extensions } /// - /// Options for configuring MVC + /// Options for globally configuring MVC for Umbraco /// /// /// We generally don't want to change the global MVC settings since we want to be unobtrusive as possible but some @@ -126,22 +124,16 @@ namespace Umbraco.Extensions /// private class UmbracoMvcConfigureOptions : IConfigureOptions { - private readonly IHttpRequestStreamReaderFactory _readerFactory; - private readonly ILoggerFactory _logger; - private readonly ArrayPool _arrayPool; - private readonly ObjectPoolProvider _objectPoolProvider; - public UmbracoMvcConfigureOptions(IHttpRequestStreamReaderFactory readerFactory, ILoggerFactory logger, ArrayPool arrayPool, ObjectPoolProvider objectPoolProvider) - { - _readerFactory = readerFactory; - _logger = logger; - _arrayPool = arrayPool; - _objectPoolProvider = objectPoolProvider; + // TODO: we can inject params with DI here + public UmbracoMvcConfigureOptions() + { } + // TODO: we can configure global mvc options here if we need to public void Configure(MvcOptions options) { - options.ModelBinderProviders.Insert(0, new UmbracoJsonModelBinderProvider(_readerFactory, _logger, _arrayPool, _objectPoolProvider)); + } } diff --git a/src/Umbraco.Web.Common/Install/InstallApiController.cs b/src/Umbraco.Web.Common/Install/InstallApiController.cs index c0c714a838..ca3596e93c 100644 --- a/src/Umbraco.Web.Common/Install/InstallApiController.cs +++ b/src/Umbraco.Web.Common/Install/InstallApiController.cs @@ -34,12 +34,10 @@ namespace Umbraco.Web.Common.Install private readonly ILogger _logger; private readonly IProfilingLogger _proflog; - public InstallApiController(UmbracoJsonModelBinderFactory modelBinderFactory, DatabaseBuilder databaseBuilder, IProfilingLogger proflog, + public InstallApiController(DatabaseBuilder databaseBuilder, IProfilingLogger proflog, InstallHelper installHelper, InstallStepCollection installSteps, InstallStatusTracker installStatusTracker, IUmbracoApplicationLifetime umbracoApplicationLifetime) { - ModelBinderFactory = modelBinderFactory; - _databaseBuilder = databaseBuilder ?? throw new ArgumentNullException(nameof(databaseBuilder)); _proflog = proflog ?? throw new ArgumentNullException(nameof(proflog)); _installSteps = installSteps; diff --git a/src/Umbraco.Web.Common/ModelBinding/UmbracoJsonModelBinderProvider.cs b/src/Umbraco.Web.Common/ModelBinding/UmbracoJsonModelBinderProvider.cs index 120a6cf4f5..7ce781ba45 100644 --- a/src/Umbraco.Web.Common/ModelBinding/UmbracoJsonModelBinderProvider.cs +++ b/src/Umbraco.Web.Common/ModelBinding/UmbracoJsonModelBinderProvider.cs @@ -5,53 +5,13 @@ using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.AspNetCore.Mvc.ModelBinding.Binders; using Microsoft.Extensions.Logging; using Microsoft.Extensions.ObjectPool; -using Microsoft.Extensions.Options; -using System; using System.Buffers; -using System.Collections.Generic; -using System.Linq; -using System.Threading.Tasks; -using Umbraco.Core; -using Umbraco.Web.Common.Controllers; namespace Umbraco.Web.Common.ModelBinding { - - public class UmbracoJsonModelBinderFactory : ModelBinderFactory - { - public UmbracoJsonModelBinderFactory( - UmbracoJsonModelBinderProvider umbracoJsonModelBinderProvider, - IModelMetadataProvider metadataProvider, - IOptions options, - IServiceProvider serviceProvider) - : base(metadataProvider, GetOptions(options, umbracoJsonModelBinderProvider), serviceProvider) - { - } - - private static IOptions GetOptions(IOptions options, UmbracoJsonModelBinderProvider umbracoJsonModelBinderProvider) - { - // copy to new collection - var providers = options.Value.ModelBinderProviders.ToList(); - // remove the default - providers.RemoveType(); - // prepend our own - providers.Insert(0, umbracoJsonModelBinderProvider); - var newOptions = new MvcOptions(); - foreach (var p in providers) - newOptions.ModelBinderProviders.Add(p); - return new CustomOptions(newOptions); - } - - private class CustomOptions : IOptions - { - public CustomOptions(MvcOptions options) - { - Value = options; - } - public MvcOptions Value { get; } - } - } - + /// + /// A custom body model binder that only uses a to bind body action parameters + /// public class UmbracoJsonModelBinder : BodyModelBinder, IModelBinder { public UmbracoJsonModelBinder(ArrayPool arrayPool, ObjectPoolProvider objectPoolProvider, IHttpRequestStreamReaderFactory readerFactory, ILoggerFactory loggerFactory) @@ -59,7 +19,7 @@ namespace Umbraco.Web.Common.ModelBinding { } - internal static IInputFormatter[] GetNewtonsoftJsonFormatter(ILoggerFactory logger, ArrayPool arrayPool, ObjectPoolProvider objectPoolProvider) + private static IInputFormatter[] GetNewtonsoftJsonFormatter(ILoggerFactory logger, ArrayPool arrayPool, ObjectPoolProvider objectPoolProvider) { var jsonOptions = new MvcNewtonsoftJsonOptions { @@ -76,42 +36,5 @@ namespace Umbraco.Web.Common.ModelBinding jsonOptions) }; } - - Task IModelBinder.BindModelAsync(ModelBindingContext bindingContext) - { - return BindModelAsync(bindingContext); - } - } - - /// - /// A model binder for Umbraco models that require Newtonsoft Json serializers - /// - public class UmbracoJsonModelBinderProvider : BodyModelBinderProvider, IModelBinderProvider - { - public UmbracoJsonModelBinderProvider(IHttpRequestStreamReaderFactory readerFactory, ILoggerFactory logger, ArrayPool arrayPool, ObjectPoolProvider objectPoolProvider) - : base(UmbracoJsonModelBinder.GetNewtonsoftJsonFormatter(logger, arrayPool, objectPoolProvider), readerFactory) - { - } - - /// - /// Returns the model binder if it's for Umbraco models - /// - /// - /// - IModelBinder IModelBinderProvider.GetBinder(ModelBinderProviderContext context) - { - // Must be 'Body' (json) binding - if (context.BindingInfo.BindingSource != BindingSource.Body) - return null; - - if (context.Metadata?.UnderlyingOrModelType.Assembly == typeof(UmbracoJsonModelBinderProvider).Assembly // Umbraco.Web.Common - || context.Metadata?.UnderlyingOrModelType.Assembly == typeof(IRuntimeState).Assembly // Umbraco.Core - || context.Metadata?.UnderlyingOrModelType.Assembly == typeof(RuntimeState).Assembly) // Umbraco.Infrastructure - { - return GetBinder(context); - } - - return null; - } } }