Merge pull request #9734 from umbraco/netcore/task/MB-cleanup-9973

Cleanup some of the MB changes for netcore

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
     /// </summary>
     public static class ServiceProviderExtensions
     {
-
         /// <summary>
         /// Creates an instance with arguments.
         /// </summary>
@@ -27,7 +31,7 @@ namespace Umbraco.Core.DependencyInjection
         /// <summary>
         /// Creates an instance of a service, with arguments.
         /// </summary>
-        /// <param name="serviceProvider"></param>
+        /// <param name="serviceProvider">The <see cref="IServiceProvider"/></param>
         /// <param name="type">The type of the instance.</param>
         /// <param name="args">Named arguments.</param>
         /// <returns>An instance of the specified type.</returns>
@@ -37,46 +41,17 @@ namespace Umbraco.Core.DependencyInjection
         /// are retrieved from the factory.</para>
         /// </remarks>
         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
+            TypeLoader typeLoader = factory.GetRequiredService<TypeLoader>();
-            // including the most annoying one, which is that it does not work on singletons (hard to fix)
+            IPublishedValueFallback publishedValueFallback = factory.GetRequiredService<IPublishedValueFallback>();
-            //return factory.GetInstance(type, args);
+            IEnumerable<Type> types = typeLoader
-
+                .GetTypes<PublishedElementModel>() // element models
-            // this method is essentially used to build singleton instances, so it is assumed that it would be
+                .Concat(typeLoader.GetTypes<PublishedContentModel>()); // content models
-            // more expensive to build and cache a dynamic method ctor than to simply invoke the ctor, as we do
+            return new PublishedModelFactory(types, publishedValueFallback);
-            // 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<object>(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);
         }
     }
 }

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
 {
+
     /// <summary>
     /// Contains extensions methods for <see cref="IUmbracoBuilder"/> used for registering event handlers.
     /// </summary>
@@ -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<UniqueServiceDescriptor>
-        {
-            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<Type>.Default.Equals(ServiceType, other.ServiceType) && EqualityComparer<Type>.Default.Equals(ImplementationType, other.ImplementationType) && EqualityComparer<object>.Default.Equals(ImplementationInstance, other.ImplementationInstance) && EqualityComparer<Func<IServiceProvider, object>>.Default.Equals(ImplementationFactory, other.ImplementationFactory);
-
-            public override int GetHashCode()
-            {
-                int hashCode = 493849952;
-                hashCode = hashCode * -1521134295 + Lifetime.GetHashCode();
-                hashCode = hashCode * -1521134295 + EqualityComparer<Type>.Default.GetHashCode(ServiceType);
-                hashCode = hashCode * -1521134295 + EqualityComparer<Type>.Default.GetHashCode(ImplementationType);
-                hashCode = hashCode * -1521134295 + EqualityComparer<object>.Default.GetHashCode(ImplementationInstance);
-                hashCode = hashCode * -1521134295 + EqualityComparer<Func<IServiceProvider, object>>.Default.GetHashCode(ImplementationFactory);
-                return hashCode;
-            }
-        }
-
     }
 }

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<IServerRegistrationService>());
             });
+
+            // 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<IPublishedModelFactory>(factory => factory.CreateDefaultPublishedModelFactory());
         }
     }
 }

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
+{
+    /// <summary>
+    /// A custom <see cref="ServiceDescriptor"/> that supports unique checking
+    /// </summary>
+    /// <remarks>
+    /// This is required because the default implementation doesn't implement Equals or GetHashCode.
+    /// see: https://github.com/dotnet/runtime/issues/47262
+    /// </remarks>
+    public sealed class UniqueServiceDescriptor : ServiceDescriptor, IEquatable<UniqueServiceDescriptor>
+    {
+        /// <summary>
+        /// Initializes a new instance of the <see cref="UniqueServiceDescriptor"/> class.
+        /// </summary>
+        public UniqueServiceDescriptor(Type serviceType, Type implementationType, ServiceLifetime lifetime)
+            : base(serviceType, implementationType, lifetime)
+        {
+        }
+
+        /// <inheritdoc/>
+        public override bool Equals(object obj) => Equals(obj as UniqueServiceDescriptor);
+
+        /// <inheritdoc/>
+        public bool Equals(UniqueServiceDescriptor other) => other != null && Lifetime == other.Lifetime && EqualityComparer<Type>.Default.Equals(ServiceType, other.ServiceType) && EqualityComparer<Type>.Default.Equals(ImplementationType, other.ImplementationType) && EqualityComparer<object>.Default.Equals(ImplementationInstance, other.ImplementationInstance) && EqualityComparer<Func<IServiceProvider, object>>.Default.Equals(ImplementationFactory, other.ImplementationFactory);
+
+        /// <inheritdoc/>
+        public override int GetHashCode()
+        {
+            int hashCode = 493849952;
+            hashCode = (hashCode * -1521134295) + Lifetime.GetHashCode();
+            hashCode = (hashCode * -1521134295) + EqualityComparer<Type>.Default.GetHashCode(ServiceType);
+            hashCode = (hashCode * -1521134295) + EqualityComparer<Type>.Default.GetHashCode(ImplementationType);
+            hashCode = (hashCode * -1521134295) + EqualityComparer<object>.Default.GetHashCode(ImplementationInstance);
+            hashCode = (hashCode * -1521134295) + EqualityComparer<Func<IServiceProvider, object>>.Default.GetHashCode(ImplementationFactory);
+            return hashCode;
+        }
+    }
+}

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<object, IPublishedValueFallback, object> Ctor { get; set; }
+
             public Type ModelType { get; set; }
+
             public Func<IList> ListCtor { get; set; }
         }
 
@@ -36,8 +39,7 @@ namespace Umbraco.Core.Models.PublishedContent
         /// PublishedContentModelFactoryResolver.Current.SetFactory(factory);
         /// </code>
         /// </remarks>
-        public PublishedModelFactory(IEnumerable<Type> types,
+        public PublishedModelFactory(IEnumerable<Type> types, IPublishedValueFallback publishedValueFallback)
-            IPublishedValueFallback publishedValueFallback)
         {
             var modelInfos = new Dictionary<string, ModelInfo>(StringComparer.InvariantCultureIgnoreCase);
             var modelTypeMap = new Dictionary<string, Type>(StringComparer.InvariantCultureIgnoreCase);

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
         /// <summary>
         /// Adds distributed cache support
         /// </summary>
+        /// <remarks>
+        /// 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.
+        /// </remarks>
         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<BatchedDatabaseServerMessenger>();
             builder.AddNotificationHandler<UmbracoApplicationStarting, DatabaseServerMessengerNotificationHandler>();
-
-            builder.Services.AddUnique<IDistributedCacheBinder, DistributedCacheBinder>();
             return builder;
         }
 

src/Umbraco.Infrastructure/Umbraco.Infrastructure.csproj

@@ -10,7 +10,7 @@
     </PropertyGroup>
 
     <ItemGroup>
-      <PackageReference Include="HtmlAgilityPack" Version="1.11.29" />
+      <PackageReference Include="HtmlAgilityPack" Version="1.11.30" />
       <PackageReference Include="MailKit" Version="2.10.1" />
       <PackageReference Include="Markdown" Version="2.2.1" />
       <PackageReference Include="Microsoft.CSharp" Version="4.7.0" />
@@ -102,8 +102,4 @@
       <ProjectReference Include="..\Umbraco.Core\Umbraco.Core.csproj" />
     </ItemGroup>
 
-    <ItemGroup>
-      <Folder Include="Scheduling" />
-    </ItemGroup>
-
 </Project>

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
 {

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;
 
 /*
@@ -84,23 +85,35 @@ namespace Umbraco.ModelsBuilder.Embedded.DependencyInjection
         /// </summary>
         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<UmbracoServices>();
 
             // TODO: I feel like we could just do builder.AddNotificationHandler<ModelsBuilderNotificationHandler>() and it
             // would automatically just register for all implemented INotificationHandler{T}?
             builder.AddNotificationHandler<UmbracoApplicationStarting, ModelsBuilderNotificationHandler>();
             builder.AddNotificationHandler<ServerVariablesParsing, ModelsBuilderNotificationHandler>();
+            builder.AddNotificationHandler<ModelBindingError, ModelsBuilderNotificationHandler>();
             builder.AddNotificationHandler<UmbracoApplicationStarting, LiveModelsProvider>();
             builder.AddNotificationHandler<UmbracoRequestEnd, LiveModelsProvider>();
             builder.AddNotificationHandler<UmbracoApplicationStarting, OutOfDateModelsStatus>();
-            builder.Services.AddUnique<ModelsGenerator>();
+            builder.Services.AddSingleton<ModelsGenerator>();
-            builder.Services.AddUnique<LiveModelsProvider>();
+            builder.Services.AddSingleton<LiveModelsProvider>();
-            builder.Services.AddUnique<OutOfDateModelsStatus>();
+            builder.Services.AddSingleton<OutOfDateModelsStatus>();
-            builder.Services.AddUnique<ModelsGenerationError>();
+            builder.Services.AddSingleton<ModelsGenerationError>();
 
-            builder.Services.AddUnique<PureLiveModelFactory>();
+            builder.Services.AddSingleton<PureLiveModelFactory>();
-            builder.Services.AddUnique<IPublishedModelFactory>(factory =>
+
+            // 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<IPublishedModelFactory>(factory =>
             {
                 ModelsBuilderSettings config = factory.GetRequiredService<IOptions<ModelsBuilderSettings>>().Value;
                 if (config.ModelsMode == ModelsMode.PureLive)
@@ -109,12 +122,7 @@ namespace Umbraco.ModelsBuilder.Embedded.DependencyInjection
                 }
                 else
                 {
-                    TypeLoader typeLoader = factory.GetRequiredService<TypeLoader>();
+                    return factory.CreateDefaultPublishedModelFactory();
-                    IPublishedValueFallback publishedValueFallback = factory.GetRequiredService<IPublishedValueFallback>();
-                    IEnumerable<Type> types = typeLoader
-                        .GetTypes<PublishedElementModel>() // element models
-                        .Concat(typeLoader.GetTypes<PublishedContentModel>()); // content models
-                    return new PublishedModelFactory(types, publishedValueFallback);
                 }
             });
 

src/Umbraco.ModelsBuilder.Embedded/LiveModelsProvider.cs

@@ -45,10 +45,7 @@ namespace Umbraco.ModelsBuilder.Embedded
         /// <summary>
         /// Handles the <see cref="UmbracoApplicationStarting"/> notification
         /// </summary>
-        public void Handle(UmbracoApplicationStarting notification)
+        public void Handle(UmbracoApplicationStarting notification) => Install();
-        {
-            Install();
-        }
 
         private void Install()
         {

src/Umbraco.ModelsBuilder.Embedded/ModelsBuilderNotificationHandler.cs

@@ -22,24 +22,21 @@ namespace Umbraco.ModelsBuilder.Embedded
     /// <summary>
     /// Handles <see cref="UmbracoApplicationStarting"/> and <see cref="ServerVariablesParsing"/> notifications to initialize MB
     /// </summary>
-    internal class ModelsBuilderNotificationHandler : INotificationHandler<UmbracoApplicationStarting>, INotificationHandler<ServerVariablesParsing>
+    internal class ModelsBuilderNotificationHandler : INotificationHandler<UmbracoApplicationStarting>, INotificationHandler<ServerVariablesParsing>, INotificationHandler<ModelBindingError>
     {
         private readonly ModelsBuilderSettings _config;
         private readonly IShortStringHelper _shortStringHelper;
         private readonly LinkGenerator _linkGenerator;
-        private readonly ContentModelBinder _modelBinder;
 
         public ModelsBuilderNotificationHandler(
             IOptions<ModelsBuilderSettings> config,
             IShortStringHelper shortStringHelper,
-            LinkGenerator linkGenerator,
+            LinkGenerator linkGenerator)
-            ContentModelBinder modelBinder)
         {
             _config = config.Value;
             _shortStringHelper = shortStringHelper;
             _shortStringHelper = shortStringHelper;
             _linkGenerator = linkGenerator;
-            _modelBinder = modelBinder;
         }
 
         /// <summary>
@@ -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;
@@ -58,7 +53,7 @@ namespace Umbraco.ModelsBuilder.Embedded
         }
 
         /// <summary>
-        /// Handles the <see cref="ServerVariablesParsing"/> notification
+        /// Handles the <see cref="ServerVariablesParsing"/> notification to add custom urls and MB mode
         /// </summary>
         public void Handle(ServerVariablesParsing notification)
         {
@@ -98,7 +93,7 @@ namespace Umbraco.ModelsBuilder.Embedded
         {
             var settings = new Dictionary<string, object>
             {
-                {"mode", _config.ModelsMode.ToString()}
+                {"mode", _config.ModelsMode.ToString() }
             };
 
             return settings;
@@ -149,10 +144,13 @@ namespace Umbraco.ModelsBuilder.Embedded
             }
         }
 
-        private void ContentModelBinder_ModelBindingException(object sender, ContentModelBinder.ModelBindingArgs args)
+        /// <summary>
+        /// Handles when a model binding error occurs
+        /// </summary>
+        public void Handle(ModelBindingError notification)
         {
-            ModelsBuilderAssemblyAttribute sourceAttr = args.SourceType.Assembly.GetCustomAttribute<ModelsBuilderAssemblyAttribute>();
+            ModelsBuilderAssemblyAttribute sourceAttr = notification.SourceType.Assembly.GetCustomAttribute<ModelsBuilderAssemblyAttribute>();
-            ModelsBuilderAssemblyAttribute modelAttr = args.ModelType.Assembly.GetCustomAttribute<ModelsBuilderAssemblyAttribute>();
+            ModelsBuilderAssemblyAttribute modelAttr = notification.ModelType.Assembly.GetCustomAttribute<ModelsBuilderAssemblyAttribute>();
 
             // 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 ");
+                notification.Message.Append(" The ");
-                args.Message.Append(sourceAttr == null ? "view model" : "source");
+                notification.Message.Append(sourceAttr == null ? "view model" : "source");
-                args.Message.Append(" is a ModelsBuilder type, but the ");
+                notification.Message.Append(" is a ModelsBuilder type, but the ");
-                args.Message.Append(sourceAttr != null ? "view model" : "source");
+                notification.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(" is not. The application is in an unstable state and should be restarted.");
                 return;
             }
 
@@ -181,22 +179,21 @@ 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;
+                    Version sourceVersion = notification.SourceType.Assembly.GetName().Version;
-                    var modelVersion = args.ModelType.Assembly.GetName().Version;
+                    Version modelVersion = notification.ModelType.Assembly.GetName().Version;
-                    args.Message.Append(" Both view and content models are PureLive, with ");
+                    notification.Message.Append(" Both view and content models are PureLive, with ");
-                    args.Message.Append(sourceVersion == modelVersion
+                    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.");
+                        : "different versions. The application is in an unstable state and should be restarted.");
-                    args.Restart = sourceVersion != modelVersion;
                 }
             }
         }

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;
 
+        /// <summary>
+        /// Initializes a new instance of the <see cref="ModelsGenerationError"/> class.
+        /// </summary>
         public ModelsGenerationError(IOptions<ModelsBuilderSettings> 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");
         }

src/Umbraco.ModelsBuilder.Embedded/OutOfDateModelsStatus.cs

@@ -8,19 +8,31 @@ using Umbraco.Web.Cache;
 
 namespace Umbraco.ModelsBuilder.Embedded
 {
+    /// <summary>
+    /// Used to track if ModelsBuilder models are out of date/stale
+    /// </summary>
     public sealed class OutOfDateModelsStatus : INotificationHandler<UmbracoApplicationStarting>
     {
         private readonly ModelsBuilderSettings _config;
         private readonly IHostingEnvironment _hostingEnvironment;
 
+        /// <summary>
+        /// Initializes a new instance of the <see cref="OutOfDateModelsStatus"/> class.
+        /// </summary>
         public OutOfDateModelsStatus(IOptions<ModelsBuilderSettings> config, IHostingEnvironment hostingEnvironment)
         {
             _config = config.Value;
             _hostingEnvironment = hostingEnvironment;
         }
 
+        /// <summary>
+        /// Gets a value indicating whether flagging out of date models is enabled
+        /// </summary>
         public bool IsEnabled => _config.FlagOutOfDateModels;
 
+        /// <summary>
+        /// Gets a value indicating whether models are out of date
+        /// </summary>
         public bool IsOutOfDate
         {
             get
@@ -38,10 +50,7 @@ namespace Umbraco.ModelsBuilder.Embedded
         /// <summary>
         /// Handles the <see cref="UmbracoApplicationStarting"/> notification
         /// </summary>
-        public void Handle(UmbracoApplicationStarting notification)
+        public void Handle(UmbracoApplicationStarting notification) => Install();
-        {
-            Install();
-        }
 
         private void Install()
         {

src/Umbraco.ModelsBuilder.Embedded/UmbracoServices.cs

@@ -20,6 +20,9 @@ namespace Umbraco.ModelsBuilder.Embedded
         private readonly IPublishedContentTypeFactory _publishedContentTypeFactory;
         private readonly IShortStringHelper _shortStringHelper;
 
+        /// <summary>
+        /// Initializes a new instance of the <see cref="UmbracoServices"/> class.
+        /// </summary>
         public UmbracoServices(
             IContentTypeService contentTypeService,
             IMediaTypeService mediaTypeService,

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<IEventAggregator>());
 
         [Test]
         [TestCase(typeof(IPublishedContent), false)]

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<TModel> : UmbracoViewPage<TModel>
         {
-            private readonly ContentModelBinder _modelBinder = new ContentModelBinder();
+            private readonly ContentModelBinder _modelBinder = new ContentModelBinder(Mock.Of<IEventAggregator>());
 
             public override Task ExecuteAsync() => throw new NotImplementedException();
 

src/Umbraco.Tests/Umbraco.Tests.csproj

@@ -86,7 +86,7 @@
       <Version>2.0.0-alpha.20200128.15</Version>
     </PackageReference>
     <PackageReference Include="HtmlAgilityPack">
-      <Version>1.11.24</Version>
+      <Version>1.11.30</Version>
     </PackageReference>
     <PackageReference Include="Lucene.Net.Contrib" Version="3.0.3" />
     <PackageReference Include="Microsoft.AspNet.Identity.Core" Version="2.2.3" />
@@ -317,7 +317,7 @@
     <Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
   </ItemGroup>
   <ItemGroup>
-    <Folder Include="Persistence\SyntaxProvider" />
+    <Folder Include="Persistence\SyntaxProvider\" />
   </ItemGroup>
   <!-- get NuGet packages directory -->
   <PropertyGroup>

src/Umbraco.Web.Common/Events/UmbracoRequestEnd.cs

@@ -10,11 +10,14 @@ namespace Umbraco.Core.Events
     /// </summary>
     public class UmbracoRequestEnd : INotification
     {
-        public UmbracoRequestEnd(HttpContext httpContext)
+        /// <summary>
-        {
+        /// Initializes a new instance of the <see cref="UmbracoRequestEnd"/> class.
-            HttpContext = httpContext;
+        /// </summary>
-        }
+        public UmbracoRequestEnd(HttpContext httpContext) => HttpContext = httpContext;
 
+        /// <summary>
+        /// Gets the <see cref="HttpContext"/>
+        /// </summary>
         public HttpContext HttpContext { get; }
     }
 }

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
 {
+
     /// <summary>
     /// Maps view models, supporting mapping to and from any <see cref="IPublishedContent"/> or <see cref="IContentModel"/>.
     /// </summary>
     public class ContentModelBinder : IModelBinder
     {
+        private readonly IEventAggregator _eventAggregator;
+
         /// <summary>
-        /// Occurs on model binding exceptions.
+        /// Initializes a new instance of the <see cref="ContentModelBinder"/> class.
         /// </summary>
-        public event EventHandler<ModelBindingArgs> ModelBindingException; // TODO: This cannot use IEventAggregator currently because it cannot be async
+        public ContentModelBinder(IEventAggregator eventAggregator) => _eventAggregator = eventAggregator;
 
         /// <inheritdoc/>
         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);
+            var args = new ModelBindingError(sourceType, modelType, msg);
-            ModelBindingException?.Invoke(this, args);
+            _eventAggregator.Publish(args);
 
             throw new ModelBindingException(msg.ToString());
         }
-
-        /// <summary>
-        /// Contains event data for the <see cref="ModelBindingException"/> event.
-        /// </summary>
-        public class ModelBindingArgs : EventArgs
-        {
-            /// <summary>
-            /// Initializes a new instance of the <see cref="ModelBindingArgs"/> class.
-            /// </summary>
-            public ModelBindingArgs(Type sourceType, Type modelType, StringBuilder message)
-            {
-                SourceType = sourceType;
-                ModelType = modelType;
-                Message = message;
-            }
-
-            /// <summary>
-            /// Gets the type of the source object.
-            /// </summary>
-            public Type SourceType { get; set; }
-
-            /// <summary>
-            /// Gets the type of the view model.
-            /// </summary>
-            public Type ModelType { get; set; }
-
-            /// <summary>
-            /// Gets the message string builder.
-            /// </summary>
-            /// <remarks>Handlers of the event can append text to the message.</remarks>
-            public StringBuilder Message { get; }
-
-            /// <summary>
-            /// Gets or sets a value indicating whether the application should restart.
-            /// </summary>
-            public bool Restart { get; set; }
-        }
     }
 }

src/Umbraco.Web.Common/ModelBinders/ModelBindingError.cs

@@ -0,0 +1,38 @@
+using System;
+using System.Text;
+using Umbraco.Core.Events;
+
+namespace Umbraco.Web.Common.ModelBinders
+{
+    /// <summary>
+    /// Contains event data for the <see cref="ModelBindingException"/> event.
+    /// </summary>
+    public class ModelBindingError : INotification
+    {
+        /// <summary>
+        /// Initializes a new instance of the <see cref="ModelBindingError"/> class.
+        /// </summary>
+        public ModelBindingError(Type sourceType, Type modelType, StringBuilder message)
+        {
+            SourceType = sourceType;
+            ModelType = modelType;
+            Message = message;
+        }
+
+        /// <summary>
+        /// Gets the type of the source object.
+        /// </summary>
+        public Type SourceType { get; }
+
+        /// <summary>
+        /// Gets the type of the view model.
+        /// </summary>
+        public Type ModelType { get; }
+
+        /// <summary>
+        /// Gets the message string builder.
+        /// </summary>
+        /// <remarks>Handlers of the event can append text to the message.</remarks>
+        public StringBuilder Message { get; }
+    }
+}

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
 

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<IUmbracoRenderingDefaults, UmbracoRenderingDefaults>();
             builder.Services.AddSingleton<IRoutableDocumentFilter, RoutableDocumentFilter>();
 
-            builder.AddDistributedCache();
+            builder
+                .AddDistributedCache()
+                .AddModelsBuilder();
 
             return builder;
         }

src/Umbraco.Web.Website/Umbraco.Web.Website.csproj

@@ -21,6 +21,7 @@
     <ItemGroup>
         <ProjectReference Include="..\Umbraco.Core\Umbraco.Core.csproj" />
         <ProjectReference Include="..\Umbraco.Infrastructure\Umbraco.Infrastructure.csproj" />
+        <ProjectReference Include="..\Umbraco.ModelsBuilder.Embedded\Umbraco.ModelsBuilder.Embedded.csproj" />
         <ProjectReference Include="..\Umbraco.Web.Common\Umbraco.Web.Common.csproj" />
     </ItemGroup>
 

src/Umbraco.Web/Umbraco.Web.csproj

@@ -65,7 +65,7 @@
     <PackageReference Include="Examine.Core">
       <Version>2.0.0-alpha.20200128.15</Version>
     </PackageReference>
-    <PackageReference Include="HtmlAgilityPack" Version="1.11.24" />
+    <PackageReference Include="HtmlAgilityPack" Version="1.11.30" />
     <PackageReference Include="HtmlSanitizer">
       <Version>5.0.376</Version>
     </PackageReference>