https://discord.umbraco.com logo
Join Discord
Powered by
What are the implications of removing the `Generat...
# help-with-umbraco
j
hey out there, we've been experimenting with the idea of removing the 
GeneratedCodeAttribute
from the models builder and we're trying to work out what kind of impact this would have? it came up after reviewing pull requests of umbraco upgrades. as every single generated model is regenerated, not only are our pull requests more bloated than they need to be but it also makes it very hard to spot if the upgrade has changed a property type when it was generated... when we found the 
IModelsGenerator
interface in the core https://docs.umbraco.com/umbraco-cms/reference/templating/modelsbuilder/understand-and-extend#imodelsgenerator we threw some code together that strips the attribute out which results in much cleaner models. based on our experiments, this doesn't impact the running of umbraco but we're wondering if this is a safe thing to do?! according to msdn:
Copy code
The GeneratedCodeAttribute class can be used by code analysis tools to identify computer-generated code, and to provide an analysis based on the tool and the version of the tool that generated the code.
we're not running the code through an analysis tool and we know that the umbraco core has generated the code.... so we're happy to live without it if its not strictly needed? any feedback would be great!
btw, this is the code we've been experimenting with:
Copy code
public class SiteModelsGeneratorComposer : IComposer
{
    public void Compose(IUmbracoBuilder builder)
    {
        builder.Services.AddSingleton<IModelsGenerator, SiteModelsGenerator>();
    }
}

public class SiteModelsGenerator : IModelsGenerator
{
    private readonly IHostingEnvironment _hostingEnvironment;

    private ModelsBuilderSettings _config;

    public SiteModelsGenerator(IOptionsMonitor<ModelsBuilderSettings> config, IHostingEnvironment hostingEnvironment)
    {
        _config = config.CurrentValue;
        _hostingEnvironment = hostingEnvironment;

        config.OnChange(x => _config = x);
    }

    public void GenerateModels()
    {
        var modelsDirectory = _config.ModelsDirectoryAbsolute(_hostingEnvironment);

        var files = Directory.GetFiles(modelsDirectory);

        var generatedByPattern = @"//\s+Umbraco\.ModelsBuilder\.Embedded v[^\s]+";

        var generatedCodeAttributePattern = @"\t\t\[global::System\.CodeDom\.Compiler\.GeneratedCodeAttribute\(""Umbraco\.ModelsBuilder\.Embedded"", "".*?""\)\]\r\n";

        foreach (var file in files)
        {
            var content = File.ReadAllText(file);

            var contentWithoutGeneratedBy = Regex.Replace(content, generatedByPattern, "//    Umbraco.ModelsBuilder.Embedded");

            var contentWithoutGeneratedCodeAttribute = Regex.Replace(contentWithoutGeneratedBy, generatedCodeAttributePattern, string.Empty);

            File.WriteAllText(file, contentWithoutGeneratedCodeAttribute);
        }
    }
}
s
In you code, it only works on already generated models. How will you generate new models then?
k
On-topic: It would probably make things like JetBrains ReSharper less useful, since it would complain about all generated models. But I think that folder can be excluded manually from that analysis tool. Not sure about the built-in analysis stuff in e.g., VS. Off-topic: Isn't there a different way to make it not clutter pull requests? Why does it even do that? I don't recall us having that problem?
s
I guess the cluttering is from the version number of Umbraco.ModelsBuilder.Embedded being written into to .cs files? You can probably get away with just removing that, and keeping the attributes.
But I still think your code prevents you from generating any new models, as I read it, it only changes already existing files in the models directory.
j
thank you for the replies peoples, really appreciate it 👍 so the go with the above code @skttl is that the composer adds the 
IModelsGenerator
implimentation after the core one meaning the code runs after the core generates the models so any new models are then also manipulated. good call on the resharper side of things @kdx-perbol that makes sense, we'll give it a try 😉 re the pr clutter, i'm not sure if there's a way to make the compare tool ignore the attribute? i think you are on to something @skttl - it'd be better to strip out the version rather than the attribute? that way any tools like resharper etc are happy and our prs stay the same? i'll have a play with this over the weekend and post the updated code 👍 but the impression i'm getting is that there wouldn't be any harm in removing the attribute and/or version number?!
@skttl you were 100% correct - the initial code example wasn't outputting new models 🤦‍♂️ i think i'd misread the docos, the key being You can then overwrite the Umbraco implementation with dependency injection. so rather than adding an implementation the site code replaces the core code...
after a bit more experimentation, this is the updated code we're trying now:
Copy code
/// <summary>
/// Removes the version number from the generated models making file comparison after Umbraco upgrades easier
/// </summary>
public class SiteModelsGenerator : ModelsGenerator, IModelsGenerator
{
    private readonly IHostingEnvironment _hostingEnvironment;

    private ModelsBuilderSettings _config;

    public SiteModelsGenerator(UmbracoServices umbracoService, IOptionsMonitor<ModelsBuilderSettings> config, OutOfDateModelsStatus outOfDateModels, IHostingEnvironment hostingEnvironment) : base(umbracoService, config, outOfDateModels, hostingEnvironment)
    {
        _config = config.CurrentValue;
        _hostingEnvironment = hostingEnvironment;

        config.OnChange(x => _config = x);
    }

    public new void GenerateModels()
    {
        base.GenerateModels();

        var modelsDirectory = _config.ModelsDirectoryAbsolute(_hostingEnvironment);

        var files = Directory.GetFiles(modelsDirectory);

        const string generatedByPattern = @"//\s+Umbraco\.ModelsBuilder\.Embedded v[^\s]+";

        const string generatedCodeAttributePattern = @"GeneratedCodeAttribute\(""Umbraco\.ModelsBuilder\.Embedded"", "".*?""\)\]";

        foreach (var file in files)
        {
            var content = File.ReadAllText(file);

            var contentWithoutGeneratedBy = Regex.Replace(content, generatedByPattern, "//    Umbraco.ModelsBuilder.Embedded");

            var contentWithoutGeneratedCodeAttribute = Regex.Replace(contentWithoutGeneratedBy, generatedCodeAttributePattern, @"GeneratedCodeAttribute(""Umbraco.ModelsBuilder.Embedded"", """")]");

            File.WriteAllText(file, contentWithoutGeneratedCodeAttribute);
        }
    }
}
so far so good! it means the code keeps up-to-date with the core and makes our pull request comparisons after upgrades far cleaner 👍
k
@jake williamson I'm -so- stealing this. Having a diff on every generated model just due to version numbers get old real quick.
a
If it helps anything, this can be disabled from 
appsettings.json
if using our custom Models Builder: https://packages.limbo.works/limbo.umbraco.modelsbuilder/docs/v13/configuration/#include-version-in-file-headers
38 Views
PreviousNext
Powered by