Refactoring the ModelsBuilder TextWriter to use Interfaces and DI for more flexibility in how we build our models

src/Umbraco.Cms.Api.Management/Controllers/ModelsBuilder/BuildModelsBuilderController.cs

@@ -8,6 +8,7 @@
 using Umbraco.Cms.Core.Configuration.Models;
 using Umbraco.Cms.Infrastructure.ModelsBuilder;
 using Umbraco.Cms.Infrastructure.ModelsBuilder.Building;
+using Umbraco.Cms.Infrastructure.ModelsBuilder.Building.Interfaces;
 using Umbraco.Extensions;
 
 namespace Umbraco.Cms.Api.Management.Controllers.ModelsBuilder;
@@ -73,7 +74,7 @@ public async Task<IActionResult> BuildModels(CancellationToken cancellationToken
                 return new ObjectResult(problemDetailsModel) { StatusCode = StatusCodes.Status428PreconditionRequired };
             }
 
-            _modelGenerator.GenerateModels();
+            _modelGenerator.GenerateModels(Core.Constants.ModelsBuilder.DefaultOutputFileExtension);
             _mbErrors.Clear();
         }
         catch (Exception e)

src/Umbraco.Core/Constants-ModelsBuilder.cs

@@ -11,5 +11,7 @@ public static partial class Constants
     public static class ModelsBuilder
     {
         public const string DefaultModelsNamespace = "Umbraco.Cms.Web.Common.PublishedModels";
+        public const string DefaultOutputFileExtension = ".generated.cs";
+        public const string DefaultAssemblyMarker = "//ASSATTR";
     }
 }

src/Umbraco.Infrastructure/ModelsBuilder/AutoModelsNotificationHandler.cs

@@ -5,7 +5,7 @@
 using Umbraco.Cms.Core.Events;
 using Umbraco.Cms.Core.Notifications;
 using Umbraco.Cms.Core.Runtime;
-using Umbraco.Cms.Infrastructure.ModelsBuilder.Building;
+using Umbraco.Cms.Infrastructure.ModelsBuilder.Building.Interfaces;
 using Umbraco.Extensions;
 
 namespace Umbraco.Cms.Infrastructure.ModelsBuilder;
@@ -112,7 +112,7 @@ private void GenerateModelsIfRequested()
                     _logger.LogDebug("Generate models...");
                 }
                 _logger.LogInformation("Generate models now.");
-                _modelGenerator.GenerateModels();
+                _modelGenerator.GenerateModels(Core.Constants.ModelsBuilder.DefaultModelsNamespace);
                 _mbErrors.Clear();
                 _logger.LogInformation("Generated.");
             }

src/Umbraco.Infrastructure/ModelsBuilder/Building/Builder.cs → src/Umbraco.Infrastructure/ModelsBuilder/Building/BuilderBase.cs

@@ -1,45 +1,29 @@
 using Umbraco.Cms.Core;
 using Umbraco.Cms.Core.Configuration;
 using Umbraco.Cms.Core.Configuration.Models;
+using Umbraco.Cms.Core.Models;
+using Umbraco.Cms.Core.Models.PublishedContent;
+using Umbraco.Cms.Core.Services;
+using Umbraco.Cms.Core.Strings;
+using Umbraco.Cms.Infrastructure.ModelsBuilder.Building.Interfaces;
 
 namespace Umbraco.Cms.Infrastructure.ModelsBuilder.Building;
 
-// NOTE
-// The idea was to have different types of builder, because I wanted to experiment with
-// building code with CodeDom. Turns out more complicated than I thought and maybe not
-// worth it at the moment, to we're using TextBuilder and its Generate method is specific.
-//
-// Keeping the code as-is for the time being...
-
 /// <summary>
 ///     Provides a base class for all builders.
 /// </summary>
-public abstract class Builder
+public class BuilderBase : IBuilderBase
 {
     /// <summary>
-    ///     Initializes a new instance of the <see cref="Builder" /> class with a list of models to generate,
+    ///     Initializes a new instance of the <see cref="BuilderBase" /> class with a list of models to generate,
     ///     the result of code parsing, and a models namespace.
     /// </summary>
-    /// <param name="typeModels">The list of models to generate.</param>
-    /// <param name="config">Configuration for modelsbuilder settings</param>
-    protected Builder(ModelsBuilderSettings config, IList<TypeModel> typeModels)
+    /// <param name="umbracoService"></param>
+    public BuilderBase()
     {
-        TypeModels = typeModels ?? throw new ArgumentNullException(nameof(typeModels));
-
-        Config = config ?? throw new ArgumentNullException(nameof(config));
-
-        // can be null or empty, we'll manage
+        Config = new ModelsBuilderSettings();
         ModelsNamespace = Config.ModelsNamespace;
 
-        // but we want it to prepare
-        Prepare();
-    }
-
-    // for unit tests only
-#pragma warning disable CS8618
-    protected Builder()
-#pragma warning restore CS8618
-    {
     }
 
     /// <summary>
@@ -48,41 +32,37 @@ protected Builder()
     /// <remarks>May be overriden by code attributes.</remarks>
     public string ModelsNamespace { get; set; }
 
-    protected Dictionary<string, string> ModelsMap { get; } = new();
-
-    // the list of assemblies that will be 'using' by default
-    protected IList<string> TypesUsing { get; } = new List<string>
-    {
+    /// <inheritdoc/>
+    public IList<string> Using { get; set; } =
+    [
         "System",
         "System.Linq.Expressions",
         "Umbraco.Cms.Core.Models.PublishedContent",
         "Umbraco.Cms.Core.PublishedCache",
         "Umbraco.Cms.Infrastructure.ModelsBuilder",
         "Umbraco.Cms.Core",
         "Umbraco.Extensions",
-    };
+    ];
 
     /// <summary>
-    ///     Gets the list of assemblies to add to the set of 'using' assemblies in each model file.
+    ///     Gets or sets the list of all models.
     /// </summary>
-    public IList<string> Using => TypesUsing;
+    /// <remarks>Includes those that are ignored.</remarks>
+    private IList<TypeModel>? TypeModels { get; set; }
 
     /// <summary>
-    ///     Gets the list of all models.
+    ///     For testing purposes only.
     /// </summary>
-    /// <remarks>Includes those that are ignored.</remarks>
-    public IList<TypeModel> TypeModels { get; }
-
     public string? ModelsNamespaceForTests { get; set; }
 
     protected ModelsBuilderSettings Config { get; }
 
-    /// <summary>
-    ///     Gets the list of models to generate.
-    /// </summary>
-    /// <returns>The models to generate</returns>
-    public IEnumerable<TypeModel> GetModelsToGenerate() => TypeModels;
 
+    /// <inheritdoc/>
+    public IEnumerable<TypeModel> GetModelsToGenerate() => TypeModels ?? new List<TypeModel>();
+    private void SetModelsToGenerate(IEnumerable<TypeModel>  types) => TypeModels = types.ToList();
+
+    /// <inheritdoc/>
     public string GetModelsNamespace()
     {
         if (ModelsNamespaceForTests != null)
@@ -102,25 +82,11 @@ public string GetModelsNamespace()
             : Config.ModelsNamespace;
     }
 
-    // looking for a simple symbol eg 'Umbraco' or 'String'
-    // expecting to match eg 'Umbraco' or 'System.String'
-    // returns true if either
-    // - more than 1 symbol is found (explicitely ambiguous)
-    // - 1 symbol is found BUT not matching (implicitely ambiguous)
-    protected bool IsAmbiguousSymbol(string symbol, string match) =>
-
-        // cannot figure out is a symbol is ambiguous without Roslyn
-        // so... let's say everything is ambiguous - code won't be
-        // pretty but it'll work
-        // Essentially this means that a `global::` syntax will be output for the generated models
-        true;
-
-    /// <summary>
-    ///     Prepares generation by processing the result of code parsing.
-    /// </summary>
-    private void Prepare()
+   /// <inheritdoc/>
+    public void Prepare(IEnumerable<TypeModel> types)
     {
-        TypeModel.MapModelTypes(TypeModels, ModelsNamespace);
+        SetModelsToGenerate(types);
+        TypeModel.MapModelTypes(GetModelsToGenerate().ToList(), ModelsNamespace);
 
         var isInMemoryMode = Config.ModelsMode == ModelsMode.InMemoryAuto;
 
@@ -130,15 +96,15 @@ private void Prepare()
         // for the last one, don't throw in InMemory mode, see comment
 
         // ensure we have no duplicates type names
-        foreach (IGrouping<string, TypeModel> xx in TypeModels.GroupBy(x => x.ClrName).Where(x => x.Count() > 1))
+        foreach (IGrouping<string, TypeModel> xx in GetModelsToGenerate().GroupBy(x => x.ClrName).Where(x => x.Count() > 1))
         {
             throw new InvalidOperationException($"Type name \"{xx.Key}\" is used"
                                                 + $" for types with alias {string.Join(", ", xx.Select(x => x.ItemType + ":\"" + x.Alias + "\""))}. Names have to be unique."
                                                 + " Consider using an attribute to assign different names to conflicting types.");
         }
 
         // ensure we have no duplicates property names
-        foreach (TypeModel typeModel in TypeModels)
+        foreach (TypeModel typeModel in GetModelsToGenerate())
         {
             foreach (IGrouping<string, PropertyModel> xx in typeModel.Properties.GroupBy(x => x.ClrName)
                          .Where(x => x.Count() > 1))
@@ -151,7 +117,7 @@ private void Prepare()
         }
 
         // ensure content & property type don't have identical name (csharp hates it)
-        foreach (TypeModel typeModel in TypeModels)
+        foreach (TypeModel typeModel in GetModelsToGenerate())
         {
             foreach (PropertyModel xx in typeModel.Properties.Where(x => x.ClrName == typeModel.ClrName))
             {
@@ -182,7 +148,7 @@ private void Prepare()
         //        xx.Alias));
 
         // discover interfaces that need to be declared / implemented
-        foreach (TypeModel typeModel in TypeModels)
+        foreach (TypeModel typeModel in GetModelsToGenerate())
         {
             // collect all the (non-removed) types implemented at parent level
             // ie the parent content types and the mixins content types, recursively
@@ -215,7 +181,7 @@ private void Prepare()
         }
 
         // ensure elements don't inherit from non-elements
-        foreach (TypeModel typeModel in TypeModels.Where(x => x.IsElement))
+        foreach (TypeModel typeModel in GetModelsToGenerate().Where(x => x.IsElement))
         {
             if (typeModel.BaseType != null && !typeModel.BaseType.IsElement)
             {
@@ -232,8 +198,11 @@ private void Prepare()
         }
     }
 
-    protected string GetModelsBaseClassName(TypeModel type) =>
+    /// <inheritdoc/>
+    public string GetModelsBaseClassName(TypeModel type) =>
 
         // default
         type.IsElement ? "PublishedElementModel" : "PublishedContentModel";
+
+
 }

src/Umbraco.Infrastructure/ModelsBuilder/Building/Interfaces/IBuilderBase.cs

@@ -0,0 +1,29 @@
+namespace Umbraco.Cms.Infrastructure.ModelsBuilder.Building.Interfaces
+{
+    public interface IBuilderBase
+    {
+        /// <summary>
+        ///     Gets the list of models to generate.
+        /// </summary>
+        /// <returns>The models to generate</returns>
+        IEnumerable<TypeModel> GetModelsToGenerate();
+        string GetModelsNamespace();
+
+        /// <summary>
+        ///     Returns PublishedElementModel or PublishedContentModel dependant on whether given type is an element.
+        /// </summary>
+        /// <param name="type"></param>
+        /// <returns></returns>
+        string GetModelsBaseClassName(TypeModel type);
+
+        /// <summary>
+        ///    Gets or sets and sets the list of using directives added to the generated model
+        /// </summary>
+        IList<string> Using { get; set; }
+
+        /// <summary>
+        ///     Prepares generation by processing the result of code parsing.
+        /// </summary>
+        void Prepare(IEnumerable<TypeModel> types);
+    }
+}

src/Umbraco.Infrastructure/ModelsBuilder/Building/Interfaces/IModelsGenerator.cs

@@ -0,0 +1,11 @@
+namespace Umbraco.Cms.Infrastructure.ModelsBuilder.Building.Interfaces
+{
+    public interface IModelsGenerator
+    {
+        /// <summary>
+        ///    Generates the models and writes them to disk.
+        /// </summary>
+        /// <param name="outputFileExtension"></param>
+        void GenerateModels(string outputFileExtension);
+    }
+}

src/Umbraco.Infrastructure/ModelsBuilder/Building/Interfaces/ITextBuilder.cs

@@ -0,0 +1,24 @@
+using System.Text;
+
+namespace Umbraco.Cms.Infrastructure.ModelsBuilder.Building.Interfaces
+{
+    public interface ITextBuilder
+    {
+        /// <summary>
+        ///     Outputs a generated model to a string builder.
+        /// </summary>
+        /// <param name="sb">The string builder.</param>
+        /// <param name="typeModel">The model to generate.</param>
+        /// <param name="availableTypes">All types available to the modelsbuilder (ie. things a model can be composed of)</param>
+        void Generate(StringBuilder sb, TypeModel typeModel, IEnumerable<TypeModel> availableTypes);
+
+        /// <summary>
+        ///     Outputs generated models to a string builder.
+        /// </summary>
+        /// <param name="sb">The string builder.</param>
+        /// <param name="typeModels">The models to generate.</param>
+        /// <param name="availableTypes">All types available to the modelsbuilder (ie. things a model can be composed of)</param>
+        /// <param name="addAssemblyMarker">Whether the modelsbuilder should add an assembly marker. Used by in-memory modelsbuilder</param>
+        void Generate(StringBuilder sb, IEnumerable<TypeModel> typeModels, IEnumerable<TypeModel> availableTypes, bool addAssemblyMarker);
+    }
+}

src/Umbraco.Infrastructure/ModelsBuilder/Building/Interfaces/ITextBuilderActions.cs

@@ -0,0 +1,59 @@
+using System.Text;
+
+namespace Umbraco.Cms.Infrastructure.ModelsBuilder.Building.Interfaces
+{
+    public interface ITextBuilderActions
+    {
+        /// <summary>
+        ///     Writes a header to top of the generated code.
+        ///     Before usings and namespace.
+        /// </summary>
+        /// <param name="sb">The string builder.</param>
+        void WriteHeader(StringBuilder sb);
+
+        /// <summary>
+        ///    Writes a marker for assembly attributes.
+        ///    Used by in-memory models builder.
+        /// </summary>
+        /// <param name="sb">The string builder.</param>
+        void WriteAssemblyAttributesMarker(StringBuilder sb);
+
+        /// <summary>
+        ///    Writes the using directives to the generated code.
+        /// </summary>
+        /// <param name="sb">The string builder.</param>
+        /// <param name="typeUsing">List of using statements</param>
+        void WriteUsing(StringBuilder sb, IEnumerable<string> typeUsing);
+
+        /// <summary>
+        ///     Writes the namespace to the generated code.
+        /// </summary>
+        /// <param name="sb">The string builder.</param>
+        /// <param name="modelNamespace">Namespace of the model</param>
+        void WriteNamespace(StringBuilder sb, string modelNamespace);
+
+        /// <summary>
+        ///    Writes the content type to the generated code.
+        ///    IE. the class declaration.
+        /// </summary>
+        /// <param name="sb">The string builder.</param>
+        /// <param name="type">The type of the model</param>
+        /// <param name="lineBreak">whether to add linebreak</param>
+        void WriteContentType(StringBuilder sb, TypeModel type, bool lineBreak);
+
+        /// <summary>
+        ///     Writes the properties of the content type to the generated code.
+        /// </summary>
+        /// <param name="sb">The string builder.</param>
+        /// <param name="type">The type of the model</param>
+        void WriteContentTypeProperties(StringBuilder sb, TypeModel type);
+
+        /// <summary>
+        ///    Entry point for extensions that wants to add custom code to the generated class.
+        ///    Does nothing by default.
+        /// </summary>
+        /// <param name="sb">The string builder.</param>
+        /// <param name="typeMode">The type of the model</param>
+        void WriteCustomCodeBeforeClassClose(StringBuilder sb, TypeModel typeMode);
+    }
+}