From dd90193365170bbc725322d64f4e586dc25168aa Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 1 Feb 2021 15:37:41 +1100 Subject: [PATCH 1/5] makes MB event an INotification --- .../Umbraco.Infrastructure.csproj | 4 -- .../UmbracoBuilderExtensions.cs | 2 + .../ModelsBuilderNotificationHandler.cs | 42 ++++++++-------- .../ModelBinders/ContentModelBinderTests.cs | 3 +- .../Views/UmbracoViewPageTests.cs | 4 +- .../ModelBinders/ContentModelBinder.cs | 49 +++---------------- .../ModelBinders/ModelBindingError.cs | 43 ++++++++++++++++ 7 files changed, 78 insertions(+), 69 deletions(-) create mode 100644 src/Umbraco.Web.Common/ModelBinders/ModelBindingError.cs diff --git a/src/Umbraco.Infrastructure/Umbraco.Infrastructure.csproj b/src/Umbraco.Infrastructure/Umbraco.Infrastructure.csproj index f3b6e0973a..5361d6ae4b 100644 --- a/src/Umbraco.Infrastructure/Umbraco.Infrastructure.csproj +++ b/src/Umbraco.Infrastructure/Umbraco.Infrastructure.csproj @@ -102,8 +102,4 @@ - - - - diff --git a/src/Umbraco.ModelsBuilder.Embedded/DependencyInjection/UmbracoBuilderExtensions.cs b/src/Umbraco.ModelsBuilder.Embedded/DependencyInjection/UmbracoBuilderExtensions.cs index f67a9e22c9..764ae9ab60 100644 --- a/src/Umbraco.ModelsBuilder.Embedded/DependencyInjection/UmbracoBuilderExtensions.cs +++ b/src/Umbraco.ModelsBuilder.Embedded/DependencyInjection/UmbracoBuilderExtensions.cs @@ -12,6 +12,7 @@ using Umbraco.Core.DependencyInjection; using Umbraco.Core.Events; using Umbraco.Core.Models.PublishedContent; using Umbraco.ModelsBuilder.Embedded.Building; +using Umbraco.Web.Common.ModelBinders; using Umbraco.Web.WebAssets; /* @@ -91,6 +92,7 @@ namespace Umbraco.ModelsBuilder.Embedded.DependencyInjection // would automatically just register for all implemented INotificationHandler{T}? builder.AddNotificationHandler(); builder.AddNotificationHandler(); + builder.AddNotificationHandler(); builder.AddNotificationHandler(); builder.AddNotificationHandler(); builder.AddNotificationHandler(); diff --git a/src/Umbraco.ModelsBuilder.Embedded/ModelsBuilderNotificationHandler.cs b/src/Umbraco.ModelsBuilder.Embedded/ModelsBuilderNotificationHandler.cs index 9e11a9808d..db2a76abcb 100644 --- a/src/Umbraco.ModelsBuilder.Embedded/ModelsBuilderNotificationHandler.cs +++ b/src/Umbraco.ModelsBuilder.Embedded/ModelsBuilderNotificationHandler.cs @@ -22,24 +22,21 @@ namespace Umbraco.ModelsBuilder.Embedded /// /// Handles and notifications to initialize MB /// - internal class ModelsBuilderNotificationHandler : INotificationHandler, INotificationHandler + internal class ModelsBuilderNotificationHandler : INotificationHandler, INotificationHandler, INotificationHandler { private readonly ModelsBuilderSettings _config; private readonly IShortStringHelper _shortStringHelper; private readonly LinkGenerator _linkGenerator; - private readonly ContentModelBinder _modelBinder; public ModelsBuilderNotificationHandler( IOptions config, IShortStringHelper shortStringHelper, - LinkGenerator linkGenerator, - ContentModelBinder modelBinder) + LinkGenerator linkGenerator) { _config = config.Value; _shortStringHelper = shortStringHelper; _shortStringHelper = shortStringHelper; _linkGenerator = linkGenerator; - _modelBinder = modelBinder; } /// @@ -49,8 +46,6 @@ namespace Umbraco.ModelsBuilder.Embedded { // always setup the dashboard // note: UmbracoApiController instances are automatically registered - _modelBinder.ModelBindingException += ContentModelBinder_ModelBindingException; - if (_config.ModelsMode != ModelsMode.Nothing) { FileService.SavingTemplate += FileService_SavingTemplate; @@ -149,10 +144,13 @@ namespace Umbraco.ModelsBuilder.Embedded } } - private void ContentModelBinder_ModelBindingException(object sender, ContentModelBinder.ModelBindingArgs args) + /// + /// Handles when a model binding error occurs + /// + public void Handle(ModelBindingError notification) { - ModelsBuilderAssemblyAttribute sourceAttr = args.SourceType.Assembly.GetCustomAttribute(); - ModelsBuilderAssemblyAttribute modelAttr = args.ModelType.Assembly.GetCustomAttribute(); + ModelsBuilderAssemblyAttribute sourceAttr = notification.SourceType.Assembly.GetCustomAttribute(); + ModelsBuilderAssemblyAttribute modelAttr = notification.ModelType.Assembly.GetCustomAttribute(); // if source or model is not a ModelsBuider type... if (sourceAttr == null || modelAttr == null) @@ -164,11 +162,11 @@ namespace Umbraco.ModelsBuilder.Embedded } // else report, but better not restart (loops?) - args.Message.Append(" The "); - args.Message.Append(sourceAttr == null ? "view model" : "source"); - args.Message.Append(" is a ModelsBuilder type, but the "); - args.Message.Append(sourceAttr != null ? "view model" : "source"); - args.Message.Append(" is not. The application is in an unstable state and should be restarted."); + notification.Message.Append(" The "); + notification.Message.Append(sourceAttr == null ? "view model" : "source"); + notification.Message.Append(" is a ModelsBuilder type, but the "); + notification.Message.Append(sourceAttr != null ? "view model" : "source"); + notification.Message.Append(" is not. The application is in an unstable state and should be restarted."); return; } @@ -181,22 +179,22 @@ namespace Umbraco.ModelsBuilder.Embedded if (pureSource == false || pureModel == false) { // only one is pure - report, but better not restart (loops?) - args.Message.Append(pureSource + notification.Message.Append(pureSource ? " The content model is PureLive, but the view model is not." : " The view model is PureLive, but the content model is not."); - args.Message.Append(" The application is in an unstable state and should be restarted."); + notification.Message.Append(" The application is in an unstable state and should be restarted."); } else { // both are pure - report, and if different versions, restart // if same version... makes no sense... and better not restart (loops?) - var sourceVersion = args.SourceType.Assembly.GetName().Version; - var modelVersion = args.ModelType.Assembly.GetName().Version; - args.Message.Append(" Both view and content models are PureLive, with "); - args.Message.Append(sourceVersion == modelVersion + var sourceVersion = notification.SourceType.Assembly.GetName().Version; + var modelVersion = notification.ModelType.Assembly.GetName().Version; + notification.Message.Append(" Both view and content models are PureLive, with "); + notification.Message.Append(sourceVersion == modelVersion ? "same version. The application is in an unstable state and should be restarted." : "different versions. The application is in an unstable state and is going to be restarted."); - args.Restart = sourceVersion != modelVersion; + notification.Restart = sourceVersion != modelVersion; } } } diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/ModelBinders/ContentModelBinderTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/ModelBinders/ContentModelBinderTests.cs index d62497b173..01b6032c36 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/ModelBinders/ContentModelBinderTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/ModelBinders/ContentModelBinderTests.cs @@ -11,6 +11,7 @@ using Microsoft.AspNetCore.Routing; using Moq; using NUnit.Framework; using Umbraco.Core; +using Umbraco.Core.Events; using Umbraco.Core.Models.PublishedContent; using Umbraco.Core.Services; using Umbraco.Web.Common.ModelBinders; @@ -26,7 +27,7 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.Common.ModelBinders private ContentModelBinder _contentModelBinder; [SetUp] - public void SetUp() => _contentModelBinder = new ContentModelBinder(); + public void SetUp() => _contentModelBinder = new ContentModelBinder(Mock.Of()); [Test] [TestCase(typeof(IPublishedContent), false)] diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Views/UmbracoViewPageTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Views/UmbracoViewPageTests.cs index f6f7eaa2d5..b227d8a903 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Views/UmbracoViewPageTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Views/UmbracoViewPageTests.cs @@ -6,7 +6,9 @@ using System.Threading.Tasks; using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.AspNetCore.Mvc.Rendering; using Microsoft.AspNetCore.Mvc.ViewFeatures; +using Moq; using NUnit.Framework; +using Umbraco.Core.Events; using Umbraco.Core.Models.PublishedContent; using Umbraco.Web.Common.AspNetCore; using Umbraco.Web.Common.ModelBinders; @@ -311,7 +313,7 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.Common.Views public class TestPage : UmbracoViewPage { - private readonly ContentModelBinder _modelBinder = new ContentModelBinder(); + private readonly ContentModelBinder _modelBinder = new ContentModelBinder(Mock.Of()); public override Task ExecuteAsync() => throw new NotImplementedException(); diff --git a/src/Umbraco.Web.Common/ModelBinders/ContentModelBinder.cs b/src/Umbraco.Web.Common/ModelBinders/ContentModelBinder.cs index 47ca49f014..cc6678abfc 100644 --- a/src/Umbraco.Web.Common/ModelBinders/ContentModelBinder.cs +++ b/src/Umbraco.Web.Common/ModelBinders/ContentModelBinder.cs @@ -3,21 +3,25 @@ using System.Text; using System.Threading.Tasks; using Microsoft.AspNetCore.Mvc.ModelBinding; using Umbraco.Core; +using Umbraco.Core.Events; using Umbraco.Core.Models.PublishedContent; using Umbraco.Web.Common.Routing; using Umbraco.Web.Models; namespace Umbraco.Web.Common.ModelBinders { + /// /// Maps view models, supporting mapping to and from any or . /// public class ContentModelBinder : IModelBinder { + private readonly IEventAggregator _eventAggregator; + /// - /// Occurs on model binding exceptions. + /// Initializes a new instance of the class. /// - public event EventHandler ModelBindingException; // TODO: This cannot use IEventAggregator currently because it cannot be async + public ContentModelBinder(IEventAggregator eventAggregator) => _eventAggregator = eventAggregator; /// public Task BindModelAsync(ModelBindingContext bindingContext) @@ -156,47 +160,10 @@ namespace Umbraco.Web.Common.ModelBinders // raise event, to give model factories a chance at reporting // the error with more details, and optionally request that // the application restarts. - var args = new ModelBindingArgs(sourceType, modelType, msg); - ModelBindingException?.Invoke(this, args); + var args = new ModelBindingError(sourceType, modelType, msg); + _eventAggregator.Publish(args); throw new ModelBindingException(msg.ToString()); } - - /// - /// Contains event data for the event. - /// - public class ModelBindingArgs : EventArgs - { - /// - /// Initializes a new instance of the class. - /// - public ModelBindingArgs(Type sourceType, Type modelType, StringBuilder message) - { - SourceType = sourceType; - ModelType = modelType; - Message = message; - } - - /// - /// Gets the type of the source object. - /// - public Type SourceType { get; set; } - - /// - /// Gets the type of the view model. - /// - public Type ModelType { get; set; } - - /// - /// Gets the message string builder. - /// - /// Handlers of the event can append text to the message. - public StringBuilder Message { get; } - - /// - /// Gets or sets a value indicating whether the application should restart. - /// - public bool Restart { get; set; } - } } } diff --git a/src/Umbraco.Web.Common/ModelBinders/ModelBindingError.cs b/src/Umbraco.Web.Common/ModelBinders/ModelBindingError.cs new file mode 100644 index 0000000000..3da698df5f --- /dev/null +++ b/src/Umbraco.Web.Common/ModelBinders/ModelBindingError.cs @@ -0,0 +1,43 @@ +using System; +using System.Text; +using Umbraco.Core.Events; + +namespace Umbraco.Web.Common.ModelBinders +{ + /// + /// Contains event data for the event. + /// + public class ModelBindingError : INotification + { + /// + /// Initializes a new instance of the class. + /// + public ModelBindingError(Type sourceType, Type modelType, StringBuilder message) + { + SourceType = sourceType; + ModelType = modelType; + Message = message; + } + + /// + /// Gets the type of the source object. + /// + public Type SourceType { get; set; } + + /// + /// Gets the type of the view model. + /// + public Type ModelType { get; set; } + + /// + /// Gets the message string builder. + /// + /// Handlers of the event can append text to the message. + public StringBuilder Message { get; } + + /// + /// Gets or sets a value indicating whether the application should restart. + /// + public bool Restart { get; set; } + } +} From 624739e697fb35bf9a871e5e4f17581c48a4880f Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 1 Feb 2021 15:39:28 +1100 Subject: [PATCH 2/5] Streamlines HtmlAgilityPack dependency --- src/Umbraco.Infrastructure/Umbraco.Infrastructure.csproj | 2 +- src/Umbraco.Tests/Umbraco.Tests.csproj | 4 ++-- src/Umbraco.Web/Umbraco.Web.csproj | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Umbraco.Infrastructure/Umbraco.Infrastructure.csproj b/src/Umbraco.Infrastructure/Umbraco.Infrastructure.csproj index 5361d6ae4b..8115c95c50 100644 --- a/src/Umbraco.Infrastructure/Umbraco.Infrastructure.csproj +++ b/src/Umbraco.Infrastructure/Umbraco.Infrastructure.csproj @@ -10,7 +10,7 @@ - + diff --git a/src/Umbraco.Tests/Umbraco.Tests.csproj b/src/Umbraco.Tests/Umbraco.Tests.csproj index 82aa9f612b..b09787dd2c 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -86,7 +86,7 @@ 2.0.0-alpha.20200128.15 - 1.11.24 + 1.11.30 @@ -317,7 +317,7 @@ - + diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index 2446653e9d..f5445a4bc9 100644 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -65,7 +65,7 @@ 2.0.0-alpha.20200128.15 - + 5.0.376 From 2a41cec2634ee1ddae8083ed312c83fd358017da Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 1 Feb 2021 16:53:24 +1100 Subject: [PATCH 3/5] Ensures the default PublishedModelFactory is always registered. Ensures AddModelsBuilder is done when adding the website (not required for the back office). --- .../ServiceProviderExtensions.cs | 57 ++++++------------- .../UmbracoBuilder.Events.cs | 28 +-------- .../DependencyInjection/UmbracoBuilder.cs | 6 ++ .../UniqueServiceDescriptor.cs | 45 +++++++++++++++ .../PublishedContent/PublishedModelFactory.cs | 8 ++- .../UmbracoBuilder.DistributedCache.cs | 16 +++++- .../Building/ModelsGenerator.cs | 2 +- .../UmbracoBuilderExtensions.cs | 32 ++++++----- .../LiveModelsProvider.cs | 5 +- .../ModelsBuilderNotificationHandler.cs | 11 ++-- .../ModelsGenerationError.cs | 27 +++++++-- .../OutOfDateModelsStatus.cs | 17 ++++-- .../UmbracoServices.cs | 3 + .../Events/UmbracoRequestEnd.cs | 11 ++-- .../ModelBinders/ModelBindingError.cs | 11 +--- src/Umbraco.Web.UI.NetCore/Startup.cs | 8 --- .../UmbracoBuilderExtensions.cs | 5 +- .../Umbraco.Web.Website.csproj | 1 + 18 files changed, 165 insertions(+), 128 deletions(-) create mode 100644 src/Umbraco.Core/DependencyInjection/UniqueServiceDescriptor.cs diff --git a/src/Umbraco.Core/DependencyInjection/ServiceProviderExtensions.cs b/src/Umbraco.Core/DependencyInjection/ServiceProviderExtensions.cs index a1cc779500..2c17709b33 100644 --- a/src/Umbraco.Core/DependencyInjection/ServiceProviderExtensions.cs +++ b/src/Umbraco.Core/DependencyInjection/ServiceProviderExtensions.cs @@ -1,5 +1,10 @@ using System; +using System.Collections.Generic; +using System.ComponentModel; +using System.Linq; using Microsoft.Extensions.DependencyInjection; +using Umbraco.Core.Composing; +using Umbraco.Core.Models.PublishedContent; namespace Umbraco.Core.DependencyInjection { @@ -8,7 +13,6 @@ namespace Umbraco.Core.DependencyInjection /// public static class ServiceProviderExtensions { - /// /// Creates an instance with arguments. /// @@ -27,7 +31,7 @@ namespace Umbraco.Core.DependencyInjection /// /// Creates an instance of a service, with arguments. /// - /// + /// The /// The type of the instance. /// Named arguments. /// An instance of the specified type. @@ -37,46 +41,17 @@ namespace Umbraco.Core.DependencyInjection /// are retrieved from the factory. /// public static object CreateInstance(this IServiceProvider serviceProvider, Type type, params object[] args) + => ActivatorUtilities.CreateInstance(serviceProvider, type, args); + + [EditorBrowsable(EditorBrowsableState.Never)] + public static PublishedModelFactory CreateDefaultPublishedModelFactory(this IServiceProvider factory) { - // LightInject has this, but then it requires RegisterConstructorDependency etc and has various oddities - // including the most annoying one, which is that it does not work on singletons (hard to fix) - //return factory.GetInstance(type, args); - - // this method is essentially used to build singleton instances, so it is assumed that it would be - // more expensive to build and cache a dynamic method ctor than to simply invoke the ctor, as we do - // here - this can be discussed - - // TODO: we currently try the ctor with most parameters, but we could want to fall back to others - - //var ctor = type.GetConstructors(BindingFlags.Instance | BindingFlags.Public).OrderByDescending(x => x.GetParameters().Length).FirstOrDefault(); - //if (ctor == null) throw new InvalidOperationException($"Could not find a public constructor for type {type.FullName}."); - - //var ctorParameters = ctor.GetParameters(); - //var ctorArgs = new object[ctorParameters.Length]; - //var availableArgs = new List(args); - //var i = 0; - //foreach (var parameter in ctorParameters) - //{ - // // no! IsInstanceOfType is not ok here - // // ReSharper disable once UseMethodIsInstanceOfType - // var idx = availableArgs.FindIndex(a => parameter.ParameterType.IsAssignableFrom(a.GetType())); - // if (idx >= 0) - // { - // // Found a suitable supplied argument - // ctorArgs[i++] = availableArgs[idx]; - - // // A supplied argument can be used at most once - // availableArgs.RemoveAt(idx); - // } - // else - // { - // // None of the provided arguments is suitable: get an instance from the factory - // ctorArgs[i++] = serviceProvider.GetRequiredService(parameter.ParameterType); - // } - //} - //return ctor.Invoke(ctorArgs); - - return ActivatorUtilities.CreateInstance(serviceProvider, type, args); + TypeLoader typeLoader = factory.GetRequiredService(); + IPublishedValueFallback publishedValueFallback = factory.GetRequiredService(); + IEnumerable types = typeLoader + .GetTypes() // element models + .Concat(typeLoader.GetTypes()); // content models + return new PublishedModelFactory(types, publishedValueFallback); } } } diff --git a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Events.cs b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Events.cs index 3fc8952dfa..c5bdb20b40 100644 --- a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Events.cs +++ b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Events.cs @@ -1,13 +1,12 @@ // Copyright (c) Umbraco. // See LICENSE for more details. -using System; -using System.Collections.Generic; using Microsoft.Extensions.DependencyInjection; using Umbraco.Core.Events; namespace Umbraco.Core.DependencyInjection { + /// /// Contains extensions methods for used for registering event handlers. /// @@ -56,30 +55,5 @@ namespace Umbraco.Core.DependencyInjection return builder; } - - // This is required because the default implementation doesn't implement Equals or GetHashCode. - // see: https://github.com/dotnet/runtime/issues/47262 - private class UniqueServiceDescriptor : ServiceDescriptor, IEquatable - { - public UniqueServiceDescriptor(Type serviceType, Type implementationType, ServiceLifetime lifetime) - : base(serviceType, implementationType, lifetime) - { - } - - public override bool Equals(object obj) => Equals(obj as UniqueServiceDescriptor); - public bool Equals(UniqueServiceDescriptor other) => other != null && Lifetime == other.Lifetime && EqualityComparer.Default.Equals(ServiceType, other.ServiceType) && EqualityComparer.Default.Equals(ImplementationType, other.ImplementationType) && EqualityComparer.Default.Equals(ImplementationInstance, other.ImplementationInstance) && EqualityComparer>.Default.Equals(ImplementationFactory, other.ImplementationFactory); - - public override int GetHashCode() - { - int hashCode = 493849952; - hashCode = hashCode * -1521134295 + Lifetime.GetHashCode(); - hashCode = hashCode * -1521134295 + EqualityComparer.Default.GetHashCode(ServiceType); - hashCode = hashCode * -1521134295 + EqualityComparer.Default.GetHashCode(ImplementationType); - hashCode = hashCode * -1521134295 + EqualityComparer.Default.GetHashCode(ImplementationInstance); - hashCode = hashCode * -1521134295 + EqualityComparer>.Default.GetHashCode(ImplementationFactory); - return hashCode; - } - } - } } diff --git a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs index b69a2c1677..a31e36db69 100644 --- a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs +++ b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Linq; using System.Runtime.InteropServices; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; @@ -216,6 +217,11 @@ namespace Umbraco.Core.DependencyInjection ? (IServerRoleAccessor)new SingleServerRoleAccessor() : new ElectedServerRoleAccessor(f.GetRequiredService()); }); + + // For Umbraco to work it must have the default IPublishedModelFactory + // which may be replaced by models builder but the default is required to make plain old IPublishedContent + // instances. + Services.AddSingleton(factory => factory.CreateDefaultPublishedModelFactory()); } } } diff --git a/src/Umbraco.Core/DependencyInjection/UniqueServiceDescriptor.cs b/src/Umbraco.Core/DependencyInjection/UniqueServiceDescriptor.cs new file mode 100644 index 0000000000..1d01d632a6 --- /dev/null +++ b/src/Umbraco.Core/DependencyInjection/UniqueServiceDescriptor.cs @@ -0,0 +1,45 @@ +// Copyright (c) Umbraco. +// See LICENSE for more details. + +using System; +using System.Collections.Generic; +using Microsoft.Extensions.DependencyInjection; + +namespace Umbraco.Core.DependencyInjection +{ + /// + /// A custom that supports unique checking + /// + /// + /// This is required because the default implementation doesn't implement Equals or GetHashCode. + /// see: https://github.com/dotnet/runtime/issues/47262 + /// + public sealed class UniqueServiceDescriptor : ServiceDescriptor, IEquatable + { + /// + /// Initializes a new instance of the class. + /// + public UniqueServiceDescriptor(Type serviceType, Type implementationType, ServiceLifetime lifetime) + : base(serviceType, implementationType, lifetime) + { + } + + /// + public override bool Equals(object obj) => Equals(obj as UniqueServiceDescriptor); + + /// + public bool Equals(UniqueServiceDescriptor other) => other != null && Lifetime == other.Lifetime && EqualityComparer.Default.Equals(ServiceType, other.ServiceType) && EqualityComparer.Default.Equals(ImplementationType, other.ImplementationType) && EqualityComparer.Default.Equals(ImplementationInstance, other.ImplementationInstance) && EqualityComparer>.Default.Equals(ImplementationFactory, other.ImplementationFactory); + + /// + public override int GetHashCode() + { + int hashCode = 493849952; + hashCode = (hashCode * -1521134295) + Lifetime.GetHashCode(); + hashCode = (hashCode * -1521134295) + EqualityComparer.Default.GetHashCode(ServiceType); + hashCode = (hashCode * -1521134295) + EqualityComparer.Default.GetHashCode(ImplementationType); + hashCode = (hashCode * -1521134295) + EqualityComparer.Default.GetHashCode(ImplementationInstance); + hashCode = (hashCode * -1521134295) + EqualityComparer>.Default.GetHashCode(ImplementationFactory); + return hashCode; + } + } +} diff --git a/src/Umbraco.Core/Models/PublishedContent/PublishedModelFactory.cs b/src/Umbraco.Core/Models/PublishedContent/PublishedModelFactory.cs index 3ea7653a7f..0c6a21ddb3 100644 --- a/src/Umbraco.Core/Models/PublishedContent/PublishedModelFactory.cs +++ b/src/Umbraco.Core/Models/PublishedContent/PublishedModelFactory.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections; using System.Collections.Generic; using System.Reflection; @@ -17,8 +17,11 @@ namespace Umbraco.Core.Models.PublishedContent private class ModelInfo { public Type ParameterType { get; set; } + public Func Ctor { get; set; } + public Type ModelType { get; set; } + public Func ListCtor { get; set; } } @@ -36,8 +39,7 @@ namespace Umbraco.Core.Models.PublishedContent /// PublishedContentModelFactoryResolver.Current.SetFactory(factory); /// /// - public PublishedModelFactory(IEnumerable types, - IPublishedValueFallback publishedValueFallback) + public PublishedModelFactory(IEnumerable types, IPublishedValueFallback publishedValueFallback) { var modelInfos = new Dictionary(StringComparer.InvariantCultureIgnoreCase); var modelTypeMap = new Dictionary(StringComparer.InvariantCultureIgnoreCase); diff --git a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.DistributedCache.cs b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.DistributedCache.cs index bff55c12e0..b88c2346a7 100644 --- a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.DistributedCache.cs +++ b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.DistributedCache.cs @@ -1,5 +1,6 @@ using System; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection.Extensions; using Umbraco.Core.DependencyInjection; using Umbraco.Core.Events; using Umbraco.Core.Services.Changes; @@ -19,13 +20,24 @@ namespace Umbraco.Infrastructure.DependencyInjection /// /// Adds distributed cache support /// + /// + /// This is still required for websites that are not load balancing because this ensures that sites hosted + /// with managed hosts like IIS/etc... work correctly when AppDomains are running in parallel. + /// public static IUmbracoBuilder AddDistributedCache(this IUmbracoBuilder builder) { + var distCacheBinder = new UniqueServiceDescriptor(typeof(IDistributedCacheBinder), typeof(DistributedCacheBinder), ServiceLifetime.Singleton); + if (builder.Services.Contains(distCacheBinder)) + { + // if this is called more than once just exit + return builder; + } + + builder.Services.Add(distCacheBinder); + builder.SetDatabaseServerMessengerCallbacks(GetCallbacks); builder.SetServerMessenger(); builder.AddNotificationHandler(); - - builder.Services.AddUnique(); return builder; } diff --git a/src/Umbraco.ModelsBuilder.Embedded/Building/ModelsGenerator.cs b/src/Umbraco.ModelsBuilder.Embedded/Building/ModelsGenerator.cs index 1c5160df18..9431b0141a 100644 --- a/src/Umbraco.ModelsBuilder.Embedded/Building/ModelsGenerator.cs +++ b/src/Umbraco.ModelsBuilder.Embedded/Building/ModelsGenerator.cs @@ -3,8 +3,8 @@ using System.Text; using Microsoft.Extensions.Options; using Umbraco.Core.Configuration; using Umbraco.Core.Configuration.Models; -using Umbraco.Core.IO; using Umbraco.Core.Hosting; +using Umbraco.Core.IO; namespace Umbraco.ModelsBuilder.Embedded.Building { diff --git a/src/Umbraco.ModelsBuilder.Embedded/DependencyInjection/UmbracoBuilderExtensions.cs b/src/Umbraco.ModelsBuilder.Embedded/DependencyInjection/UmbracoBuilderExtensions.cs index 764ae9ab60..852cde55fc 100644 --- a/src/Umbraco.ModelsBuilder.Embedded/DependencyInjection/UmbracoBuilderExtensions.cs +++ b/src/Umbraco.ModelsBuilder.Embedded/DependencyInjection/UmbracoBuilderExtensions.cs @@ -85,8 +85,16 @@ namespace Umbraco.ModelsBuilder.Embedded.DependencyInjection /// public static IUmbracoBuilder AddModelsBuilder(this IUmbracoBuilder builder) { + var umbServices = new UniqueServiceDescriptor(typeof(UmbracoServices), typeof(UmbracoServices), ServiceLifetime.Singleton); + if (builder.Services.Contains(umbServices)) + { + // if this ext method is called more than once just exit + return builder; + } + + builder.Services.Add(umbServices); + builder.AddPureLiveRazorEngine(); - builder.Services.AddSingleton(); // TODO: I feel like we could just do builder.AddNotificationHandler() and it // would automatically just register for all implemented INotificationHandler{T}? @@ -96,13 +104,16 @@ namespace Umbraco.ModelsBuilder.Embedded.DependencyInjection builder.AddNotificationHandler(); builder.AddNotificationHandler(); builder.AddNotificationHandler(); - builder.Services.AddUnique(); - builder.Services.AddUnique(); - builder.Services.AddUnique(); - builder.Services.AddUnique(); + builder.Services.AddSingleton(); + builder.Services.AddSingleton(); + builder.Services.AddSingleton(); + builder.Services.AddSingleton(); - builder.Services.AddUnique(); - builder.Services.AddUnique(factory => + builder.Services.AddSingleton(); + + // This is what the community MB would replace, all of the above services are fine to be registered + // even if the community MB is in place. + builder.Services.AddSingleton(factory => { ModelsBuilderSettings config = factory.GetRequiredService>().Value; if (config.ModelsMode == ModelsMode.PureLive) @@ -111,12 +122,7 @@ namespace Umbraco.ModelsBuilder.Embedded.DependencyInjection } else { - TypeLoader typeLoader = factory.GetRequiredService(); - IPublishedValueFallback publishedValueFallback = factory.GetRequiredService(); - IEnumerable types = typeLoader - .GetTypes() // element models - .Concat(typeLoader.GetTypes()); // content models - return new PublishedModelFactory(types, publishedValueFallback); + return factory.CreateDefaultPublishedModelFactory(); } }); diff --git a/src/Umbraco.ModelsBuilder.Embedded/LiveModelsProvider.cs b/src/Umbraco.ModelsBuilder.Embedded/LiveModelsProvider.cs index a9ac5c7f77..cf5235387d 100644 --- a/src/Umbraco.ModelsBuilder.Embedded/LiveModelsProvider.cs +++ b/src/Umbraco.ModelsBuilder.Embedded/LiveModelsProvider.cs @@ -45,10 +45,7 @@ namespace Umbraco.ModelsBuilder.Embedded /// /// Handles the notification /// - public void Handle(UmbracoApplicationStarting notification) - { - Install(); - } + public void Handle(UmbracoApplicationStarting notification) => Install(); private void Install() { diff --git a/src/Umbraco.ModelsBuilder.Embedded/ModelsBuilderNotificationHandler.cs b/src/Umbraco.ModelsBuilder.Embedded/ModelsBuilderNotificationHandler.cs index db2a76abcb..0d6d1cc668 100644 --- a/src/Umbraco.ModelsBuilder.Embedded/ModelsBuilderNotificationHandler.cs +++ b/src/Umbraco.ModelsBuilder.Embedded/ModelsBuilderNotificationHandler.cs @@ -53,7 +53,7 @@ namespace Umbraco.ModelsBuilder.Embedded } /// - /// Handles the notification + /// Handles the notification to add custom urls and MB mode /// public void Handle(ServerVariablesParsing notification) { @@ -93,7 +93,7 @@ namespace Umbraco.ModelsBuilder.Embedded { var settings = new Dictionary { - {"mode", _config.ModelsMode.ToString()} + {"mode", _config.ModelsMode.ToString() } }; return settings; @@ -188,13 +188,12 @@ namespace Umbraco.ModelsBuilder.Embedded { // both are pure - report, and if different versions, restart // if same version... makes no sense... and better not restart (loops?) - var sourceVersion = notification.SourceType.Assembly.GetName().Version; - var modelVersion = notification.ModelType.Assembly.GetName().Version; + Version sourceVersion = notification.SourceType.Assembly.GetName().Version; + Version modelVersion = notification.ModelType.Assembly.GetName().Version; notification.Message.Append(" Both view and content models are PureLive, with "); notification.Message.Append(sourceVersion == modelVersion ? "same version. The application is in an unstable state and should be restarted." - : "different versions. The application is in an unstable state and is going to be restarted."); - notification.Restart = sourceVersion != modelVersion; + : "different versions. The application is in an unstable state and should be restarted."); } } } diff --git a/src/Umbraco.ModelsBuilder.Embedded/ModelsGenerationError.cs b/src/Umbraco.ModelsBuilder.Embedded/ModelsGenerationError.cs index dca2fda0dc..25f48a19cc 100644 --- a/src/Umbraco.ModelsBuilder.Embedded/ModelsGenerationError.cs +++ b/src/Umbraco.ModelsBuilder.Embedded/ModelsGenerationError.cs @@ -1,10 +1,10 @@ -using System; +using System; using System.IO; using System.Text; using Microsoft.Extensions.Options; using Umbraco.Core.Configuration; -using Umbraco.Core.Hosting; using Umbraco.Core.Configuration.Models; +using Umbraco.Core.Hosting; namespace Umbraco.ModelsBuilder.Embedded { @@ -13,6 +13,9 @@ namespace Umbraco.ModelsBuilder.Embedded private readonly ModelsBuilderSettings _config; private readonly IHostingEnvironment _hostingEnvironment; + /// + /// Initializes a new instance of the class. + /// public ModelsGenerationError(IOptions config, IHostingEnvironment hostingEnvironment) { _config = config.Value; @@ -22,7 +25,10 @@ namespace Umbraco.ModelsBuilder.Embedded public void Clear() { var errFile = GetErrFile(); - if (errFile == null) return; + if (errFile == null) + { + return; + } // "If the file to be deleted does not exist, no exception is thrown." File.Delete(errFile); @@ -31,7 +37,10 @@ namespace Umbraco.ModelsBuilder.Embedded public void Report(string message, Exception e) { var errFile = GetErrFile(); - if (errFile == null) return; + if (errFile == null) + { + return; + } var sb = new StringBuilder(); sb.Append(message); @@ -47,14 +56,18 @@ namespace Umbraco.ModelsBuilder.Embedded public string GetLastError() { var errFile = GetErrFile(); - if (errFile == null) return null; + if (errFile == null) + { + return null; + } try { return File.ReadAllText(errFile); } - catch // accepted + catch { + // accepted return null; } } @@ -63,7 +76,9 @@ namespace Umbraco.ModelsBuilder.Embedded { var modelsDirectory = _config.ModelsDirectoryAbsolute(_hostingEnvironment); if (!Directory.Exists(modelsDirectory)) + { return null; + } return Path.Combine(modelsDirectory, "models.err"); } diff --git a/src/Umbraco.ModelsBuilder.Embedded/OutOfDateModelsStatus.cs b/src/Umbraco.ModelsBuilder.Embedded/OutOfDateModelsStatus.cs index ad757cbf7e..83f105a486 100644 --- a/src/Umbraco.ModelsBuilder.Embedded/OutOfDateModelsStatus.cs +++ b/src/Umbraco.ModelsBuilder.Embedded/OutOfDateModelsStatus.cs @@ -8,19 +8,31 @@ using Umbraco.Web.Cache; namespace Umbraco.ModelsBuilder.Embedded { + /// + /// Used to track if ModelsBuilder models are out of date/stale + /// public sealed class OutOfDateModelsStatus : INotificationHandler { private readonly ModelsBuilderSettings _config; private readonly IHostingEnvironment _hostingEnvironment; + /// + /// Initializes a new instance of the class. + /// public OutOfDateModelsStatus(IOptions config, IHostingEnvironment hostingEnvironment) { _config = config.Value; _hostingEnvironment = hostingEnvironment; } + /// + /// Gets a value indicating whether flagging out of date models is enabled + /// public bool IsEnabled => _config.FlagOutOfDateModels; + /// + /// Gets a value indicating whether models are out of date + /// public bool IsOutOfDate { get @@ -38,10 +50,7 @@ namespace Umbraco.ModelsBuilder.Embedded /// /// Handles the notification /// - public void Handle(UmbracoApplicationStarting notification) - { - Install(); - } + public void Handle(UmbracoApplicationStarting notification) => Install(); private void Install() { diff --git a/src/Umbraco.ModelsBuilder.Embedded/UmbracoServices.cs b/src/Umbraco.ModelsBuilder.Embedded/UmbracoServices.cs index 76803abe1f..86954a8a85 100644 --- a/src/Umbraco.ModelsBuilder.Embedded/UmbracoServices.cs +++ b/src/Umbraco.ModelsBuilder.Embedded/UmbracoServices.cs @@ -20,6 +20,9 @@ namespace Umbraco.ModelsBuilder.Embedded private readonly IPublishedContentTypeFactory _publishedContentTypeFactory; private readonly IShortStringHelper _shortStringHelper; + /// + /// Initializes a new instance of the class. + /// public UmbracoServices( IContentTypeService contentTypeService, IMediaTypeService mediaTypeService, diff --git a/src/Umbraco.Web.Common/Events/UmbracoRequestEnd.cs b/src/Umbraco.Web.Common/Events/UmbracoRequestEnd.cs index 61138cf02b..459eca2bba 100644 --- a/src/Umbraco.Web.Common/Events/UmbracoRequestEnd.cs +++ b/src/Umbraco.Web.Common/Events/UmbracoRequestEnd.cs @@ -10,11 +10,14 @@ namespace Umbraco.Core.Events /// public class UmbracoRequestEnd : INotification { - public UmbracoRequestEnd(HttpContext httpContext) - { - HttpContext = httpContext; - } + /// + /// Initializes a new instance of the class. + /// + public UmbracoRequestEnd(HttpContext httpContext) => HttpContext = httpContext; + /// + /// Gets the + /// public HttpContext HttpContext { get; } } } diff --git a/src/Umbraco.Web.Common/ModelBinders/ModelBindingError.cs b/src/Umbraco.Web.Common/ModelBinders/ModelBindingError.cs index 3da698df5f..352ca842a5 100644 --- a/src/Umbraco.Web.Common/ModelBinders/ModelBindingError.cs +++ b/src/Umbraco.Web.Common/ModelBinders/ModelBindingError.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Text; using Umbraco.Core.Events; @@ -22,22 +22,17 @@ namespace Umbraco.Web.Common.ModelBinders /// /// Gets the type of the source object. /// - public Type SourceType { get; set; } + public Type SourceType { get; } /// /// Gets the type of the view model. /// - public Type ModelType { get; set; } + public Type ModelType { get; } /// /// Gets the message string builder. /// /// Handlers of the event can append text to the message. public StringBuilder Message { get; } - - /// - /// Gets or sets a value indicating whether the application should restart. - /// - public bool Restart { get; set; } } } diff --git a/src/Umbraco.Web.UI.NetCore/Startup.cs b/src/Umbraco.Web.UI.NetCore/Startup.cs index 3029423bf5..7f2ede1845 100644 --- a/src/Umbraco.Web.UI.NetCore/Startup.cs +++ b/src/Umbraco.Web.UI.NetCore/Startup.cs @@ -49,14 +49,6 @@ namespace Umbraco.Web.UI.NetCore .AddBackOffice() .AddWebsite() .AddComposers() - // TODO: This call and AddDistributedCache are interesting ones. They are both required for back office and front-end to render - // but we don't want to force people to call so many of these ext by default and want to keep all of this relatively simple. - // but we still need to allow the flexibility for people to use their own ModelsBuilder. In that case people can call a different - // AddModelsBuilderCommunity (or whatever) after our normal calls to replace our services. - // So either we call AddModelsBuilder within AddBackOffice AND AddWebsite just like we do with AddDistributedCache or we - // have a top level method to add common things required for backoffice/frontend like .AddCommon() - // or we allow passing in options to these methods to configure what happens within them. - .AddModelsBuilder() .Build(); #pragma warning restore IDE0022 // Use expression body for methods diff --git a/src/Umbraco.Web.Website/DependencyInjection/UmbracoBuilderExtensions.cs b/src/Umbraco.Web.Website/DependencyInjection/UmbracoBuilderExtensions.cs index ca6c2da6e8..3d0c482a30 100644 --- a/src/Umbraco.Web.Website/DependencyInjection/UmbracoBuilderExtensions.cs +++ b/src/Umbraco.Web.Website/DependencyInjection/UmbracoBuilderExtensions.cs @@ -6,6 +6,7 @@ using Umbraco.Core.DependencyInjection; using Umbraco.Extensions; using Umbraco.Infrastructure.DependencyInjection; using Umbraco.Infrastructure.PublishedCache.DependencyInjection; +using Umbraco.ModelsBuilder.Embedded.DependencyInjection; using Umbraco.Web.Common.Routing; using Umbraco.Web.Website.Collections; using Umbraco.Web.Website.Controllers; @@ -43,7 +44,9 @@ namespace Umbraco.Web.Website.DependencyInjection builder.Services.AddSingleton(); builder.Services.AddSingleton(); - builder.AddDistributedCache(); + builder + .AddDistributedCache() + .AddModelsBuilder(); return builder; } diff --git a/src/Umbraco.Web.Website/Umbraco.Web.Website.csproj b/src/Umbraco.Web.Website/Umbraco.Web.Website.csproj index 970b39411f..2ff13dec89 100644 --- a/src/Umbraco.Web.Website/Umbraco.Web.Website.csproj +++ b/src/Umbraco.Web.Website/Umbraco.Web.Website.csproj @@ -21,6 +21,7 @@ + From a974ba71fd348abf765a556a996a5383bb4f42a4 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 1 Feb 2021 17:50:20 +1100 Subject: [PATCH 4/5] linting --- .../Extensions/HtmlHelperRenderExtensions.cs | 535 +++++++----------- 1 file changed, 208 insertions(+), 327 deletions(-) diff --git a/src/Umbraco.Web.Website/Extensions/HtmlHelperRenderExtensions.cs b/src/Umbraco.Web.Website/Extensions/HtmlHelperRenderExtensions.cs index e56b136b9c..449797a8a1 100644 --- a/src/Umbraco.Web.Website/Extensions/HtmlHelperRenderExtensions.cs +++ b/src/Umbraco.Web.Website/Extensions/HtmlHelperRenderExtensions.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Linq; using System.Net; @@ -32,15 +32,15 @@ namespace Umbraco.Extensions /// public static class HtmlHelperRenderExtensions { - private static T GetRequiredService(IHtmlHelper htmlHelper) - { - return GetRequiredService(htmlHelper.ViewContext); - } + private static T GetRequiredService(IHtmlHelper htmlHelper) + { + return GetRequiredService(htmlHelper.ViewContext); + } private static T GetRequiredService(ViewContext viewContext) - { - return viewContext.HttpContext.RequestServices.GetRequiredService(); - } + { + return viewContext.HttpContext.RequestServices.GetRequiredService(); + } /// /// Renders the markup for the profiler @@ -85,13 +85,14 @@ namespace Umbraco.Extensions if (umbrcoContext.InPreviewMode) { var htmlBadge = - String.Format(contentSettings.PreviewBadge, - ioHelper.ResolveUrl(globalSettings.UmbracoPath), - WebUtility.UrlEncode(httpContextAccessor.GetRequiredHttpContext().Request.Path), - umbrcoContext.PublishedRequest.PublishedContent.Id); + string.Format( + contentSettings.PreviewBadge, + ioHelper.ResolveUrl(globalSettings.UmbracoPath), + WebUtility.UrlEncode(httpContextAccessor.GetRequiredHttpContext().Request.Path), + umbrcoContext.PublishedRequest.PublishedContent.Id); return new HtmlString(htmlBadge); } - return new HtmlString(""); + return new HtmlString(string.Empty); } @@ -106,7 +107,7 @@ namespace Umbraco.Extensions Func contextualKeyBuilder = null) { var cacheKey = new StringBuilder(partialViewName); - //let's always cache by the current culture to allow variants to have different cache results + //let's always cache by the current culture to allow variants to have different cache results var cultureName = System.Threading.Thread.CurrentThread.CurrentUICulture.Name; if (!String.IsNullOrEmpty(cultureName)) { @@ -126,7 +127,7 @@ namespace Umbraco.Extensions { //TODO reintroduce when members are migrated throw new NotImplementedException("Reintroduce when members are migrated"); - // var helper = Current.MembershipHelper; + // var helper = Current.MembershipHelper; // var currentMember = helper.GetCurrentMember(); // cacheKey.AppendFormat("m{0}-", currentMember?.Id ?? 0); } @@ -163,34 +164,29 @@ namespace Umbraco.Extensions /// A validation summary that lets you pass in a prefix so that the summary only displays for elements /// containing the prefix. This allows you to have more than on validation summary on a page. /// - /// - /// - /// - /// - /// - /// - public static IHtmlContent ValidationSummary(this IHtmlHelper htmlHelper, - string prefix = "", - bool excludePropertyErrors = false, - string message = "", - object htmlAttributes = null) + public static IHtmlContent ValidationSummary( + this IHtmlHelper htmlHelper, + string prefix = "", + bool excludePropertyErrors = false, + string message = "", + object htmlAttributes = null) { if (prefix.IsNullOrWhiteSpace()) { return htmlHelper.ValidationSummary(excludePropertyErrors, message, htmlAttributes); } - - - var htmlGenerator = GetRequiredService(htmlHelper); var viewContext = htmlHelper.ViewContext.Clone(); - foreach (var key in viewContext.ViewData.Keys.ToArray()){ - if(!key.StartsWith(prefix)){ + foreach (var key in viewContext.ViewData.Keys.ToArray()) + { + if (!key.StartsWith(prefix)) + { viewContext.ViewData.Remove(key); - } - } + } + } + var tagBuilder = htmlGenerator.GenerateValidationSummary( viewContext, excludePropertyErrors, @@ -205,7 +201,7 @@ namespace Umbraco.Extensions return tagBuilder; } -// TODO what to do here? This will be view components right? + // TODO what to do here? This will be view components right? // /// // /// Returns the result of a child action of a strongly typed SurfaceController // /// @@ -220,7 +216,7 @@ namespace Umbraco.Extensions // } // -// TODO what to do here? This will be view components right? + // TODO what to do here? This will be view components right? // /// // /// Returns the result of a child action of a SurfaceController // /// @@ -265,15 +261,14 @@ namespace Umbraco.Extensions /// internal class UmbracoForm : MvcForm { + private readonly ViewContext _viewContext; + private bool _disposed; + private readonly string _encryptedString; + private readonly string _controllerName; + /// - /// Creates an UmbracoForm + /// Initializes a new instance of the class. /// - /// - /// - /// - /// - /// - /// public UmbracoForm( ViewContext viewContext, HtmlEncoder htmlEncoder, @@ -288,28 +283,27 @@ namespace Umbraco.Extensions _encryptedString = EncryptionHelper.CreateEncryptedRouteString(GetRequiredService(viewContext), controllerName, controllerAction, area, additionalRouteVals); } - private readonly ViewContext _viewContext; - private bool _disposed; - private readonly string _encryptedString; - private readonly string _controllerName; - protected new void Dispose() { - if (this._disposed) + if (_disposed) + { return; - this._disposed = true; - //Detect if the call is targeting UmbRegisterController/UmbProfileController/UmbLoginStatusController/UmbLoginController and if it is we automatically output a AntiForgeryToken() + } + + _disposed = true; + + // Detect if the call is targeting UmbRegisterController/UmbProfileController/UmbLoginStatusController/UmbLoginController and if it is we automatically output a AntiForgeryToken() // We have a controllerName and area so we can match if (_controllerName == "UmbRegister" || _controllerName == "UmbProfile" || _controllerName == "UmbLoginStatus" || _controllerName == "UmbLogin") { - var antiforgery = _viewContext.HttpContext.RequestServices.GetRequiredService(); + IAntiforgery antiforgery = _viewContext.HttpContext.RequestServices.GetRequiredService(); _viewContext.Writer.Write(antiforgery.GetHtml(_viewContext.HttpContext).ToString()); } - //write out the hidden surface form routes + // write out the hidden surface form routes _viewContext.Writer.Write(""); base.Dispose(); @@ -323,104 +317,79 @@ namespace Umbraco.Extensions /// Name of the action. /// Name of the controller. /// The method. - /// + /// the public static MvcForm BeginUmbracoForm(this IHtmlHelper html, string action, string controllerName, FormMethod method) - { - return html.BeginUmbracoForm(action, controllerName, null, new Dictionary(), method); - } + => html.BeginUmbracoForm(action, controllerName, null, new Dictionary(), method); /// /// Helper method to create a new form to execute in the Umbraco request pipeline against a locally declared controller /// - /// - /// - /// - /// public static MvcForm BeginUmbracoForm(this IHtmlHelper html, string action, string controllerName) - { - return html.BeginUmbracoForm(action, controllerName, null, new Dictionary()); - } + => html.BeginUmbracoForm(action, controllerName, null, new Dictionary()); /// /// Helper method to create a new form to execute in the Umbraco request pipeline against a locally declared controller /// - /// - /// - /// - /// - /// - /// public static MvcForm BeginUmbracoForm(this IHtmlHelper html, string action, string controllerName, object additionalRouteVals, FormMethod method) - { - return html.BeginUmbracoForm(action, controllerName, additionalRouteVals, new Dictionary(), method); - } + => html.BeginUmbracoForm(action, controllerName, additionalRouteVals, new Dictionary(), method); /// /// Helper method to create a new form to execute in the Umbraco request pipeline against a locally declared controller /// - /// - /// - /// - /// - /// public static MvcForm BeginUmbracoForm(this IHtmlHelper html, string action, string controllerName, object additionalRouteVals) - { - return html.BeginUmbracoForm(action, controllerName, additionalRouteVals, new Dictionary()); - } + => html.BeginUmbracoForm(action, controllerName, additionalRouteVals, new Dictionary()); /// /// Helper method to create a new form to execute in the Umbraco request pipeline against a locally declared controller /// - /// - /// - /// - /// - /// - /// - /// - public static MvcForm BeginUmbracoForm(this IHtmlHelper html, string action, string controllerName, + public static MvcForm BeginUmbracoForm( + this IHtmlHelper html, + string action, + string controllerName, object additionalRouteVals, object htmlAttributes, - FormMethod method) - { - return html.BeginUmbracoForm(action, controllerName, additionalRouteVals, HtmlHelper.AnonymousObjectToHtmlAttributes(htmlAttributes), method); - } + FormMethod method) => html.BeginUmbracoForm(action, controllerName, additionalRouteVals, HtmlHelper.AnonymousObjectToHtmlAttributes(htmlAttributes), method); /// /// Helper method to create a new form to execute in the Umbraco request pipeline against a locally declared controller /// - /// - /// - /// - /// - /// - /// - public static MvcForm BeginUmbracoForm(this IHtmlHelper html, string action, string controllerName, + public static MvcForm BeginUmbracoForm( + this IHtmlHelper html, + string action, + string controllerName, object additionalRouteVals, - object htmlAttributes) - { - return html.BeginUmbracoForm(action, controllerName, additionalRouteVals, HtmlHelper.AnonymousObjectToHtmlAttributes(htmlAttributes)); - } + object htmlAttributes) => html.BeginUmbracoForm(action, controllerName, additionalRouteVals, HtmlHelper.AnonymousObjectToHtmlAttributes(htmlAttributes)); /// /// Helper method to create a new form to execute in the Umbraco request pipeline against a locally declared controller /// - /// - /// - /// - /// - /// - /// - /// - public static MvcForm BeginUmbracoForm(this IHtmlHelper html, string action, string controllerName, + public static MvcForm BeginUmbracoForm( + this IHtmlHelper html, + string action, + string controllerName, object additionalRouteVals, IDictionary htmlAttributes, FormMethod method) { - if (action == null) throw new ArgumentNullException(nameof(action)); - if (string.IsNullOrWhiteSpace(action)) throw new ArgumentException("Value can't be empty or consist only of white-space characters.", nameof(action)); - if (controllerName == null) throw new ArgumentNullException(nameof(controllerName)); - if (string.IsNullOrWhiteSpace(controllerName)) throw new ArgumentException("Value can't be empty or consist only of white-space characters.", nameof(controllerName)); + if (action == null) + { + throw new ArgumentNullException(nameof(action)); + } + + if (string.IsNullOrWhiteSpace(action)) + { + throw new ArgumentException("Value can't be empty or consist only of white-space characters.", nameof(action)); + } + + if (controllerName == null) + { + throw new ArgumentNullException(nameof(controllerName)); + } + + if (string.IsNullOrWhiteSpace(controllerName)) + { + throw new ArgumentException("Value can't be empty or consist only of white-space characters.", nameof(controllerName)); + } return html.BeginUmbracoForm(action, controllerName, "", additionalRouteVals, htmlAttributes, method); } @@ -428,20 +397,32 @@ namespace Umbraco.Extensions /// /// Helper method to create a new form to execute in the Umbraco request pipeline against a locally declared controller /// - /// - /// - /// - /// - /// - /// - public static MvcForm BeginUmbracoForm(this IHtmlHelper html, string action, string controllerName, + public static MvcForm BeginUmbracoForm( + this IHtmlHelper html, + string action, + string controllerName, object additionalRouteVals, IDictionary htmlAttributes) { - if (action == null) throw new ArgumentNullException(nameof(action)); - if (string.IsNullOrWhiteSpace(action)) throw new ArgumentException("Value can't be empty or consist only of white-space characters.", nameof(action)); - if (controllerName == null) throw new ArgumentNullException(nameof(controllerName)); - if (string.IsNullOrWhiteSpace(controllerName)) throw new ArgumentException("Value can't be empty or consist only of white-space characters.", nameof(controllerName)); + if (action == null) + { + throw new ArgumentNullException(nameof(action)); + } + + if (string.IsNullOrWhiteSpace(action)) + { + throw new ArgumentException("Value can't be empty or consist only of white-space characters.", nameof(action)); + } + + if (controllerName == null) + { + throw new ArgumentNullException(nameof(controllerName)); + } + + if (string.IsNullOrWhiteSpace(controllerName)) + { + throw new ArgumentException("Value can't be empty or consist only of white-space characters.", nameof(controllerName)); + } return html.BeginUmbracoForm(action, controllerName, "", additionalRouteVals, htmlAttributes); } @@ -449,207 +430,139 @@ namespace Umbraco.Extensions /// /// Helper method to create a new form to execute in the Umbraco request pipeline to a surface controller plugin /// - /// - /// - /// The surface controller to route to - /// - /// public static MvcForm BeginUmbracoForm(this IHtmlHelper html, string action, Type surfaceType, FormMethod method) - { - return html.BeginUmbracoForm(action, surfaceType, null, new Dictionary(), method); - } + => html.BeginUmbracoForm(action, surfaceType, null, new Dictionary(), method); /// /// Helper method to create a new form to execute in the Umbraco request pipeline to a surface controller plugin /// - /// - /// - /// The surface controller to route to - /// public static MvcForm BeginUmbracoForm(this IHtmlHelper html, string action, Type surfaceType) - { - return html.BeginUmbracoForm(action, surfaceType, null, new Dictionary()); - } + => html.BeginUmbracoForm(action, surfaceType, null, new Dictionary()); /// /// Helper method to create a new form to execute in the Umbraco request pipeline to a surface controller plugin /// - /// - /// - /// - /// - /// + /// The type public static MvcForm BeginUmbracoForm(this IHtmlHelper html, string action, FormMethod method) - where T : SurfaceController - { - return html.BeginUmbracoForm(action, typeof(T), method); - } + where T : SurfaceController => html.BeginUmbracoForm(action, typeof(T), method); /// /// Helper method to create a new form to execute in the Umbraco request pipeline to a surface controller plugin /// - /// - /// - /// - /// + /// The type public static MvcForm BeginUmbracoForm(this IHtmlHelper html, string action) - where T : SurfaceController - { - return html.BeginUmbracoForm(action, typeof(T)); - } + where T : SurfaceController => html.BeginUmbracoForm(action, typeof(T)); /// /// Helper method to create a new form to execute in the Umbraco request pipeline to a surface controller plugin /// - /// - /// - /// The surface controller to route to - /// - /// - /// - public static MvcForm BeginUmbracoForm(this IHtmlHelper html, string action, Type surfaceType, - object additionalRouteVals, FormMethod method) - { - return html.BeginUmbracoForm(action, surfaceType, additionalRouteVals, new Dictionary(), method); - } + public static MvcForm BeginUmbracoForm( + this IHtmlHelper html, + string action, + Type surfaceType, + object additionalRouteVals, + FormMethod method) => html.BeginUmbracoForm(action, surfaceType, additionalRouteVals, new Dictionary(), method); /// /// Helper method to create a new form to execute in the Umbraco request pipeline to a surface controller plugin /// - /// - /// - /// The surface controller to route to - /// - /// - public static MvcForm BeginUmbracoForm(this IHtmlHelper html, string action, Type surfaceType, - object additionalRouteVals) - { - return html.BeginUmbracoForm(action, surfaceType, additionalRouteVals, new Dictionary()); - } + public static MvcForm BeginUmbracoForm( + this IHtmlHelper html, + string action, + Type surfaceType, + object additionalRouteVals) => html.BeginUmbracoForm(action, surfaceType, additionalRouteVals, new Dictionary()); /// /// Helper method to create a new form to execute in the Umbraco request pipeline to a surface controller plugin /// - /// - /// - /// - /// - /// - /// + /// The type public static MvcForm BeginUmbracoForm(this IHtmlHelper html, string action, object additionalRouteVals, FormMethod method) - where T : SurfaceController - { - return html.BeginUmbracoForm(action, typeof(T), additionalRouteVals, method); - } + where T : SurfaceController => html.BeginUmbracoForm(action, typeof(T), additionalRouteVals, method); /// /// Helper method to create a new form to execute in the Umbraco request pipeline to a surface controller plugin /// - /// - /// - /// - /// - /// + /// The type public static MvcForm BeginUmbracoForm(this IHtmlHelper html, string action, object additionalRouteVals) - where T : SurfaceController - { - return html.BeginUmbracoForm(action, typeof(T), additionalRouteVals); - } + where T : SurfaceController => html.BeginUmbracoForm(action, typeof(T), additionalRouteVals); /// /// Helper method to create a new form to execute in the Umbraco request pipeline to a surface controller plugin /// - /// - /// - /// The surface controller to route to - /// - /// - /// - /// - public static MvcForm BeginUmbracoForm(this IHtmlHelper html, string action, Type surfaceType, - object additionalRouteVals, - object htmlAttributes, - FormMethod method) - { - return html.BeginUmbracoForm(action, surfaceType, additionalRouteVals, HtmlHelper.AnonymousObjectToHtmlAttributes(htmlAttributes), method); - } + public static MvcForm BeginUmbracoForm( + this IHtmlHelper html, + string action, + Type surfaceType, + object additionalRouteVals, + object htmlAttributes, + FormMethod method) => html.BeginUmbracoForm(action, surfaceType, additionalRouteVals, HtmlHelper.AnonymousObjectToHtmlAttributes(htmlAttributes), method); /// /// Helper method to create a new form to execute in the Umbraco request pipeline to a surface controller plugin /// - /// - /// - /// The surface controller to route to - /// - /// - /// - public static MvcForm BeginUmbracoForm(this IHtmlHelper html, string action, Type surfaceType, - object additionalRouteVals, - object htmlAttributes) - { - return html.BeginUmbracoForm(action, surfaceType, additionalRouteVals, HtmlHelper.AnonymousObjectToHtmlAttributes(htmlAttributes)); - } + public static MvcForm BeginUmbracoForm( + this IHtmlHelper html, + string action, + Type surfaceType, + object additionalRouteVals, + object htmlAttributes) => html.BeginUmbracoForm(action, surfaceType, additionalRouteVals, HtmlHelper.AnonymousObjectToHtmlAttributes(htmlAttributes)); /// /// Helper method to create a new form to execute in the Umbraco request pipeline to a surface controller plugin /// - /// - /// - /// - /// - /// - /// - /// - public static MvcForm BeginUmbracoForm(this IHtmlHelper html, string action, - object additionalRouteVals, - object htmlAttributes, - FormMethod method) - where T : SurfaceController - { - return html.BeginUmbracoForm(action, typeof(T), additionalRouteVals, htmlAttributes, method); - } + /// The type + public static MvcForm BeginUmbracoForm( + this IHtmlHelper html, + string action, + object additionalRouteVals, + object htmlAttributes, + FormMethod method) + where T : SurfaceController => html.BeginUmbracoForm(action, typeof(T), additionalRouteVals, htmlAttributes, method); /// /// Helper method to create a new form to execute in the Umbraco request pipeline to a surface controller plugin /// - /// - /// - /// - /// - /// - /// - public static MvcForm BeginUmbracoForm(this IHtmlHelper html, string action, - object additionalRouteVals, - object htmlAttributes) - where T : SurfaceController - { - return html.BeginUmbracoForm(action, typeof(T), additionalRouteVals, htmlAttributes); - } + public static MvcForm BeginUmbracoForm( + this IHtmlHelper html, + string action, + object additionalRouteVals, + object htmlAttributes) + where T : SurfaceController => html.BeginUmbracoForm(action, typeof(T), additionalRouteVals, htmlAttributes); /// /// Helper method to create a new form to execute in the Umbraco request pipeline to a surface controller plugin /// - /// - /// - /// The surface controller to route to - /// - /// - /// - /// - public static MvcForm BeginUmbracoForm(this IHtmlHelper html, string action, Type surfaceType, - object additionalRouteVals, - IDictionary htmlAttributes, - FormMethod method) + public static MvcForm BeginUmbracoForm( + this IHtmlHelper html, + string action, + Type surfaceType, + object additionalRouteVals, + IDictionary htmlAttributes, + FormMethod method) { - if (action == null) throw new ArgumentNullException(nameof(action)); - if (string.IsNullOrWhiteSpace(action)) throw new ArgumentException("Value can't be empty or consist only of white-space characters.", nameof(action)); - if (surfaceType == null) throw new ArgumentNullException(nameof(surfaceType)); + if (action == null) + { + throw new ArgumentNullException(nameof(action)); + } + + if (string.IsNullOrWhiteSpace(action)) + { + throw new ArgumentException("Value can't be empty or consist only of white-space characters.", nameof(action)); + } + + if (surfaceType == null) + { + throw new ArgumentNullException(nameof(surfaceType)); + } var surfaceControllerTypeCollection = GetRequiredService(html); var surfaceController = surfaceControllerTypeCollection.SingleOrDefault(x => x == surfaceType); if (surfaceController == null) + { throw new InvalidOperationException("Could not find the surface controller of type " + surfaceType.FullName); + } + var metaData = PluginController.GetMetadata(surfaceController); var area = string.Empty; @@ -681,7 +594,7 @@ namespace Umbraco.Extensions /// /// Helper method to create a new form to execute in the Umbraco request pipeline to a surface controller plugin /// - /// + /// The type /// /// /// @@ -700,7 +613,7 @@ namespace Umbraco.Extensions /// /// Helper method to create a new form to execute in the Umbraco request pipeline to a surface controller plugin /// - /// + /// The type /// /// /// @@ -757,10 +670,14 @@ namespace Umbraco.Extensions IDictionary htmlAttributes, FormMethod method) { - if (action == null) throw new ArgumentNullException(nameof(action)); - if (string.IsNullOrEmpty(action)) throw new ArgumentException("Value can't be empty.", nameof(action)); - if (controllerName == null) throw new ArgumentNullException(nameof(controllerName)); - if (string.IsNullOrWhiteSpace(controllerName)) throw new ArgumentException("Value can't be empty or consist only of white-space characters.", nameof(controllerName)); + if (action == null) + throw new ArgumentNullException(nameof(action)); + if (string.IsNullOrEmpty(action)) + throw new ArgumentException("Value can't be empty.", nameof(action)); + if (controllerName == null) + throw new ArgumentNullException(nameof(controllerName)); + if (string.IsNullOrWhiteSpace(controllerName)) + throw new ArgumentException("Value can't be empty or consist only of white-space characters.", nameof(controllerName)); var umbracoContextAccessor = GetRequiredService(html); var formAction = umbracoContextAccessor.UmbracoContext.OriginalRequestUrl.PathAndQuery; @@ -809,7 +726,7 @@ namespace Umbraco.Extensions object additionalRouteVals = null) { - //ensure that the multipart/form-data is added to the HTML attributes + // ensure that the multipart/form-data is added to the HTML attributes if (htmlAttributes.ContainsKey("enctype") == false) { htmlAttributes.Add("enctype", "multipart/form-data"); @@ -825,13 +742,13 @@ namespace Umbraco.Extensions if (traditionalJavascriptEnabled) { // forms must have an ID for client validation - tagBuilder.GenerateId("form" + Guid.NewGuid().ToString("N"), string.Empty); + tagBuilder.GenerateId("form" + Guid.NewGuid().ToString("N"), string.Empty); } htmlHelper.ViewContext.Writer.Write(tagBuilder.RenderStartTag()); var htmlEncoder = GetRequiredService(htmlHelper); - //new UmbracoForm: + // new UmbracoForm: var theForm = new UmbracoForm(htmlHelper.ViewContext, htmlEncoder, surfaceController, surfaceAction, area, additionalRouteVals); if (traditionalJavascriptEnabled) @@ -856,9 +773,7 @@ namespace Umbraco.Extensions /// The HTML encoded value. /// public static IHtmlContent If(this IHtmlHelper html, bool test, string valueIfTrue) - { - return If(html, test, valueIfTrue, string.Empty); - } + => If(html, test, valueIfTrue, string.Empty); /// /// If is true, the HTML encoded will be returned; otherwise, . @@ -871,9 +786,7 @@ namespace Umbraco.Extensions /// The HTML encoded value. /// public static IHtmlContent If(this IHtmlHelper html, bool test, string valueIfTrue, string valueIfFalse) - { - return new HtmlString(HttpUtility.HtmlEncode(test ? valueIfTrue : valueIfFalse)); - } + => new HtmlString(HttpUtility.HtmlEncode(test ? valueIfTrue : valueIfFalse)); #endregion @@ -890,113 +803,82 @@ namespace Umbraco.Extensions /// The HTML encoded text with text line breaks replaced with HTML line breaks (<br />). /// public static IHtmlContent ReplaceLineBreaks(this IHtmlHelper helper, string text) - { - return StringUtilities.ReplaceLineBreaks(text); - } + => StringUtilities.ReplaceLineBreaks(text); /// /// Generates a hash based on the text string passed in. This method will detect the /// security requirements (is FIPS enabled) and return an appropriate hash. /// - /// + /// The /// The text to create a hash from /// Hash of the text string - public static string CreateHash(this IHtmlHelper helper, string text) - { - return text.GenerateHash(); - } + public static string CreateHash(this IHtmlHelper helper, string text) => text.GenerateHash(); /// /// Strips all HTML tags from a given string, all contents of the tags will remain. /// public static IHtmlContent StripHtml(this HtmlHelper helper, IHtmlContent html, params string[] tags) - { - return helper.StripHtml(html.ToHtmlString(), tags); - } - - + => helper.StripHtml(html.ToHtmlString(), tags); /// /// Strips all HTML tags from a given string, all contents of the tags will remain. /// public static IHtmlContent StripHtml(this HtmlHelper helper, string html, params string[] tags) - { - return StringUtilities.StripHtmlTags(html, tags); - } + => StringUtilities.StripHtmlTags(html, tags); /// /// Will take the first non-null value in the collection and return the value of it. /// public static string Coalesce(this HtmlHelper helper, params object[] args) - { - return StringUtilities.Coalesce(args); - } + => StringUtilities.Coalesce(args); /// /// Joins any number of int/string/objects into one string /// public static string Concatenate(this HtmlHelper helper, params object[] args) - { - return StringUtilities.Concatenate(args); - } + => StringUtilities.Concatenate(args); /// /// Joins any number of int/string/objects into one string and separates them with the string separator parameter. /// public static string Join(this HtmlHelper helper, string separator, params object[] args) - { - return StringUtilities.Join(separator, args); - } + => StringUtilities.Join(separator, args); /// /// Truncates a string to a given length, can add a ellipsis at the end (...). Method checks for open HTML tags, and makes sure to close them /// public static IHtmlContent Truncate(this HtmlHelper helper, IHtmlContent html, int length) - { - return helper.Truncate(html.ToHtmlString(), length, true, false); - } + => helper.Truncate(html.ToHtmlString(), length, true, false); /// /// Truncates a string to a given length, can add a ellipsis at the end (...). Method checks for open HTML tags, and makes sure to close them /// public static IHtmlContent Truncate(this HtmlHelper helper, IHtmlContent html, int length, bool addElipsis) - { - return helper.Truncate(html.ToHtmlString(), length, addElipsis, false); - } + => helper.Truncate(html.ToHtmlString(), length, addElipsis, false); /// /// Truncates a string to a given length, can add a ellipsis at the end (...). Method checks for open HTML tags, and makes sure to close them /// public static IHtmlContent Truncate(this HtmlHelper helper, IHtmlContent html, int length, bool addElipsis, bool treatTagsAsContent) - { - return helper.Truncate(html.ToHtmlString(), length, addElipsis, treatTagsAsContent); - } + => helper.Truncate(html.ToHtmlString(), length, addElipsis, treatTagsAsContent); /// /// Truncates a string to a given length, can add a ellipsis at the end (...). Method checks for open HTML tags, and makes sure to close them /// public static IHtmlContent Truncate(this HtmlHelper helper, string html, int length) - { - return helper.Truncate(html, length, true, false); - } + => helper.Truncate(html, length, true, false); /// /// Truncates a string to a given length, can add a ellipsis at the end (...). Method checks for open HTML tags, and makes sure to close them /// public static IHtmlContent Truncate(this HtmlHelper helper, string html, int length, bool addElipsis) - { - return helper.Truncate(html, length, addElipsis, false); - } + => helper.Truncate(html, length, addElipsis, false); /// /// Truncates a string to a given length, can add a ellipsis at the end (...). Method checks for open HTML tags, and makes sure to close them /// public static IHtmlContent Truncate(this HtmlHelper helper, string html, int length, bool addElipsis, bool treatTagsAsContent) - { - return StringUtilities.Truncate(html, length, addElipsis, treatTagsAsContent); - } - - #region Truncate by Words + => StringUtilities.Truncate(html, length, addElipsis, treatTagsAsContent); /// /// Truncates a string to a given amount of words, can add a ellipsis at the end (...). Method checks for open HTML tags, and makes sure to close them @@ -1038,7 +920,6 @@ namespace Umbraco.Extensions return helper.Truncate(html, length, addElipsis, false); } - #endregion #endregion } From a0cbff2868af8506feb2fdbc3e2f690b007ffe90 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 1 Feb 2021 17:53:40 +1100 Subject: [PATCH 5/5] linting --- .../Extensions/HtmlHelperRenderExtensions.cs | 172 +++++++----------- 1 file changed, 67 insertions(+), 105 deletions(-) diff --git a/src/Umbraco.Web.Website/Extensions/HtmlHelperRenderExtensions.cs b/src/Umbraco.Web.Website/Extensions/HtmlHelperRenderExtensions.cs index 449797a8a1..48367c8e44 100644 --- a/src/Umbraco.Web.Website/Extensions/HtmlHelperRenderExtensions.cs +++ b/src/Umbraco.Web.Website/Extensions/HtmlHelperRenderExtensions.cs @@ -578,108 +578,82 @@ namespace Umbraco.Extensions /// /// Helper method to create a new form to execute in the Umbraco request pipeline to a surface controller plugin /// - /// - /// - /// The surface controller to route to - /// - /// - /// - public static MvcForm BeginUmbracoForm(this IHtmlHelper html, string action, Type surfaceType, - object additionalRouteVals, - IDictionary htmlAttributes) - { - return html.BeginUmbracoForm(action, surfaceType, additionalRouteVals, htmlAttributes, FormMethod.Post); - } + public static MvcForm BeginUmbracoForm( + this IHtmlHelper html, + string action, + Type surfaceType, + object additionalRouteVals, + IDictionary htmlAttributes) + => html.BeginUmbracoForm(action, surfaceType, additionalRouteVals, htmlAttributes, FormMethod.Post); /// /// Helper method to create a new form to execute in the Umbraco request pipeline to a surface controller plugin /// /// The type - /// - /// - /// - /// - /// - /// - public static MvcForm BeginUmbracoForm(this IHtmlHelper html, string action, - object additionalRouteVals, - IDictionary htmlAttributes, - FormMethod method) - where T : SurfaceController - { - return html.BeginUmbracoForm(action, typeof(T), additionalRouteVals, htmlAttributes, method); - } + public static MvcForm BeginUmbracoForm( + this IHtmlHelper html, + string action, + object additionalRouteVals, + IDictionary htmlAttributes, + FormMethod method) + where T : SurfaceController => html.BeginUmbracoForm(action, typeof(T), additionalRouteVals, htmlAttributes, method); /// /// Helper method to create a new form to execute in the Umbraco request pipeline to a surface controller plugin /// /// The type - /// - /// - /// - /// - /// - public static MvcForm BeginUmbracoForm(this IHtmlHelper html, string action, - object additionalRouteVals, - IDictionary htmlAttributes) - where T : SurfaceController - { - return html.BeginUmbracoForm(action, typeof(T), additionalRouteVals, htmlAttributes); - } + public static MvcForm BeginUmbracoForm( + this IHtmlHelper html, + string action, + object additionalRouteVals, + IDictionary htmlAttributes) + where T : SurfaceController => html.BeginUmbracoForm(action, typeof(T), additionalRouteVals, htmlAttributes); /// /// Helper method to create a new form to execute in the Umbraco request pipeline to a surface controller plugin /// - /// - /// - /// - /// - /// - /// public static MvcForm BeginUmbracoForm(this IHtmlHelper html, string action, string controllerName, string area, FormMethod method) - { - return html.BeginUmbracoForm(action, controllerName, area, null, new Dictionary(), method); - } + => html.BeginUmbracoForm(action, controllerName, area, null, new Dictionary(), method); /// /// Helper method to create a new form to execute in the Umbraco request pipeline to a surface controller plugin /// - /// - /// - /// - /// - /// public static MvcForm BeginUmbracoForm(this IHtmlHelper html, string action, string controllerName, string area) - { - return html.BeginUmbracoForm(action, controllerName, area, null, new Dictionary()); - } + => html.BeginUmbracoForm(action, controllerName, area, null, new Dictionary()); /// /// Helper method to create a new form to execute in the Umbraco request pipeline to a surface controller plugin /// - /// - /// - /// - /// - /// - /// - /// - /// - public static MvcForm BeginUmbracoForm(this IHtmlHelper html, string action, string controllerName, string area, - object additionalRouteVals, - IDictionary htmlAttributes, - FormMethod method) + public static MvcForm BeginUmbracoForm( + this IHtmlHelper html, + string action, + string controllerName, + string area, + object additionalRouteVals, + IDictionary htmlAttributes, + FormMethod method) { if (action == null) + { throw new ArgumentNullException(nameof(action)); - if (string.IsNullOrEmpty(action)) - throw new ArgumentException("Value can't be empty.", nameof(action)); - if (controllerName == null) - throw new ArgumentNullException(nameof(controllerName)); - if (string.IsNullOrWhiteSpace(controllerName)) - throw new ArgumentException("Value can't be empty or consist only of white-space characters.", nameof(controllerName)); + } - var umbracoContextAccessor = GetRequiredService(html); + if (string.IsNullOrEmpty(action)) + { + throw new ArgumentException("Value can't be empty.", nameof(action)); + } + + if (controllerName == null) + { + throw new ArgumentNullException(nameof(controllerName)); + } + + if (string.IsNullOrWhiteSpace(controllerName)) + { + throw new ArgumentException("Value can't be empty or consist only of white-space characters.", nameof(controllerName)); + } + + IUmbracoContextAccessor umbracoContextAccessor = GetRequiredService(html); var formAction = umbracoContextAccessor.UmbracoContext.OriginalRequestUrl.PathAndQuery; return html.RenderForm(formAction, method, htmlAttributes, controllerName, action, area, additionalRouteVals); } @@ -687,45 +661,30 @@ namespace Umbraco.Extensions /// /// Helper method to create a new form to execute in the Umbraco request pipeline to a surface controller plugin /// - /// - /// - /// - /// - /// - /// - /// - public static MvcForm BeginUmbracoForm(this IHtmlHelper html, string action, string controllerName, string area, - object additionalRouteVals, - IDictionary htmlAttributes) - { - return html.BeginUmbracoForm(action, controllerName, area, additionalRouteVals, htmlAttributes, FormMethod.Post); - } + public static MvcForm BeginUmbracoForm( + this IHtmlHelper html, + string action, + string controllerName, + string area, + object additionalRouteVals, + IDictionary htmlAttributes) => html.BeginUmbracoForm(action, controllerName, area, additionalRouteVals, htmlAttributes, FormMethod.Post); /// /// This renders out the form for us /// - /// - /// - /// - /// - /// - /// - /// - /// - /// /// /// This code is pretty much the same as the underlying MVC code that writes out the form /// - private static MvcForm RenderForm(this IHtmlHelper htmlHelper, - string formAction, - FormMethod method, - IDictionary htmlAttributes, - string surfaceController, - string surfaceAction, - string area, - object additionalRouteVals = null) + private static MvcForm RenderForm( + this IHtmlHelper htmlHelper, + string formAction, + FormMethod method, + IDictionary htmlAttributes, + string surfaceController, + string surfaceAction, + string area, + object additionalRouteVals = null) { - // ensure that the multipart/form-data is added to the HTML attributes if (htmlAttributes.ContainsKey("enctype") == false) { @@ -734,8 +693,10 @@ namespace Umbraco.Extensions var tagBuilder = new TagBuilder("form"); tagBuilder.MergeAttributes(htmlAttributes); + // action is implicitly generated, so htmlAttributes take precedence. tagBuilder.MergeAttribute("action", formAction); + // method is an explicit parameter, so it takes precedence over the htmlAttributes. tagBuilder.MergeAttribute("method", HtmlHelper.GetFormMethodString(method), true); var traditionalJavascriptEnabled = htmlHelper.ViewContext.ClientValidationEnabled; @@ -748,6 +709,7 @@ namespace Umbraco.Extensions htmlHelper.ViewContext.Writer.Write(tagBuilder.RenderStartTag()); var htmlEncoder = GetRequiredService(htmlHelper); + // new UmbracoForm: var theForm = new UmbracoForm(htmlHelper.ViewContext, htmlEncoder, surfaceController, surfaceAction, area, additionalRouteVals);