Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 5 commits into
base: contrib
Choose a base branch
from

Conversation

kasparboelkjeldsen
Copy link
Contributor

Prerequisites

  • [ x] I have added steps to test this contribution in the description below

Description

This is based off this discussion 17047

The aim of this pull request is to enable further customization of the TextBuilder for Models generation, generating csharp classes by splitting up the existing implementation into multiple classes and interfaces that are registered for dependency injection.

I have done my best to document the new interfaces. Actual implementation yields indentical models to existing implementation. But by allowing it to run with dependency injection, package developers and others are now able to replace and decorate functionality closer to what they are trying to solve with the TextBuilder.

The new interfaces are:

IBuilderBase which is thought of as a dependency for basically anything related to generate models. (Previously Builder.cs)

ITextBuilder - extracted from the TextBuilder implementation, exposes 2 Generate methods.

ITextBuilderActions - first level down in complexity, exposing methods for writing header, usings, namespaces, class definition, properties etc.

ITextBuilderSubActions - nitty gritty level where you affect individual items of a class, such as attributes written on top of methods and properties, as well as actual property implementation.

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; }
}

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>
    void Generate(StringBuilder sb, TypeModel typeModel);

    /// <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="addAssemblyMarker"></param>
    void Generate(StringBuilder sb, IEnumerable<TypeModel> typeModels, bool addAssemblyMarker);
}

public interface ITextBuilderActions
{
    /// <summary>
    ///     Writes a header to top of the generated code.
    ///     Before usings and namespace.
    /// </summary>
    /// <param name="sb"></param>
    void WriteHeader(StringBuilder sb);

    /// <summary>
    ///    Writes a marker for assembly attributes.
    ///    Used by in-memory models builder.
    /// </summary>
    /// <param name="sb"></param>
    void WriteAssemblyAttributesMarker(StringBuilder sb);

    /// <summary>
    ///    Writes the using directives to the generated code.
    /// </summary>
    /// <param name="sb"></param>
    /// <param name="typeUsing"></param>
    void WriteUsing(StringBuilder sb, IEnumerable<string> typeUsing);

    /// <summary>
    ///     Writes the namespace to the generated code.
    /// </summary>
    /// <param name="sb"></param>
    /// <param name="modelNamespace"></param>
    void WriteNamespace(StringBuilder sb, string modelNamespace);

    /// <summary>
    ///    Writes the content type to the generated code.
    ///    IE. the class declaration.
    /// </summary>
    /// <param name="sb"></param>
    /// <param name="type"></param>
    /// <param name="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"></param>
    /// <param name="type"></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"></param>
    /// <param name="typeMode"></param>
    void WriteCustomCodeBeforeClassClose(StringBuilder sb, TypeModel typeMode);
}

public interface ITextBuilderSubActions
{
    /// <summary>
    ///     Transforms input into valid XML comment string.
    /// </summary>
    /// <param name="input"></param>
    /// <returns></returns>
    string XmlCommentString(string input);

    /// <summary>
    ///    Writes a property inherited by interface to the generated code.
    /// </summary>
    /// <param name="sb"></param>
    /// <param name="property"></param>
    void WriteInterfaceProperty(StringBuilder sb, PropertyModel property);


    /// <summary>
    ///     Writes attributes to the generated code.
    ///     IE. [global::System.CodeDom.Compiler.GeneratedCodeAttribute("Umbraco.ModelsBuilder.Embedded", "14.xxxxx")].
    /// </summary>
    /// <param name="sb"></param>
    /// <param name="tabs"></param>        
    void WriteGeneratedCodeAttribute(StringBuilder sb, string tabs);

    /// <summary>
    ///     Writes MaybeNull attributes to the generated code.
    ///     IE. [return: global::System.Diagnostics.CodeAnalysis.MaybeNull].
    /// </summary>
    /// <param name="sb"></param>
    /// <param name="tabs"></param>
    /// <param name="isReturn"></param>
    void WriteMaybeNullAttribute(StringBuilder sb, string tabs, bool isReturn = false);

    /// <summary>
    ///    Writes Mixin property to the generated code.
    /// </summary>
    /// <param name="sb"></param>
    /// <param name="property"></param>
    /// <param name="mixinClrName"></param>
    void WriteMixinProperty(StringBuilder sb, PropertyModel property, string mixinClrName);

    /// <summary>
    ///   Writes a property to the generated code.
    /// </summary>
    /// <param name="sb"></param>
    /// <param name="type"></param>
    /// <param name="property"></param>
    /// <param name="mixinClrName"></param>
    void WriteProperty(StringBuilder sb, TypeModel type, PropertyModel property, string? mixinClrName = null);


    void WriteClrType(StringBuilder sb, Type type);

    void WriteNonGenericClrType(StringBuilder sb, string s);

    void WriteClrType(StringBuilder sb, string type);
}
´´´

I've also added a couple of constants to Core.Constants.ModelsBuilder for cleaner code.

Splitting Builder and TextBuilder out into 4 interfaces that are registered as services so they now are injectable
Bit of rewrite to pass all tests.
Copy link

Hi there @kasparboelkjeldsen, thank you for this contribution! 👍

While we wait for one of the Core Collaborators team to have a look at your work, we wanted to let you know about that we have a checklist for some of the things we will consider during review:

  • It's clear what problem this is solving, there's a connected issue or a description of what the changes do and how to test them
  • The automated tests all pass (see "Checks" tab on this PR)
  • The level of security for this contribution is the same or improved
  • The level of performance for this contribution is the same or improved
  • Avoids creating breaking changes; note that behavioral changes might also be perceived as breaking
  • If this is a new feature, Umbraco HQ provided guidance on the implementation beforehand
  • 💡 The contribution looks original and the contributor is presumably allowed to share it

Don't worry if you got something wrong. We like to think of a pull request as the start of a conversation, we're happy to provide guidance on improving your contribution.

If you realize that you might want to make some changes then you can do that by adding new commits to the branch you created for this work and pushing new commits. They should then automatically show up as updates to this pull request.

Thanks, from your friendly Umbraco GitHub bot 🤖 🙂

@kasparboelkjeldsen
Copy link
Contributor Author

Hold off on reviewing, I'll fix the tests first :)

Added "availableTypes" as parameter to avoid having to use both builderbase and textbuilder when generating.
@kasparboelkjeldsen
Copy link
Contributor Author

Locally the PR now passes all integration and unit tests. It looks like I'm timing out in your pipeline here right now, but I'm ready for feedback. I know it's a doozy, but I also think it unlocks all kinds of cool things for developers.

@JasonElkin
Copy link
Contributor

Hey @kasparboelkjeldsen, thanks for looking at this. On a personal note I'm really excited to see modelsbuilder getting some love.

We'll get this reviewed and get back to you soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants