Models Builder for “simpler” sites? 🤔

[enkelmedia]

Hi Denis (and other contributors)!

Great work on this one! Really awesome and thanks for sharing it! I would definitely contribute in the future! I really think that this project has the potential of being a great source of inspiration and best practice for Umbraco websites going forward. Like a documentation that actually shows how to solve the “real” problems implementors are facing.

I’m wondering about ditching Models Builder. Is there a really good reason not to use it in a best practice example?

I do get that you don’t want to expose the whole cms-model (IPublishedContent) in the view but wouldn’t the mapping in the controllers be cleaner/safer if done from a models builder model? The models created by Models Builder is basically what you are doing manually in the controllers but without all the constants for property names, or actually more “automatic constants” with the strongly typed names. They also keep information about any Compositions so one can go ie:

if(model is IPageWithHero hero) {
…
}

It’s also safer for any changes in the document type - MB would give a compiler warning. The current solution will throw at runtime in best case or silently fail/behave strange if I forget to change the constant for the property alias.

Did you actively decide not to go with Models Builder? It would be nice to hear more about why and maybe document that decision.

And also, I think it sounds bad to indicate that models builder is only suitable for “simpler projects” (in the readme) - that might put a lot of people in the wrong direction, I would say that it might be a matter of taste but from a technical standpoint running without Model Builder is a less safe and sustainable way of building a complex and maintainable solution with Umbraco - more manual work and more prone to errors.

Would be nice to hear your thoughts?

Cheers man!

[Adolfi]

Hey Markus, thank you so much for being involved in this.
And thanks for your kind words regarding this project in terms of being a good source of inspiration and documentation.

1. Regarding the ModelsBuilder being used in "simpler" sites/projects.
I think you are paraphrasing me here, I'm actually never saying that ModelsBuilder is good for simpler sites or projects. Im saying its a simpler way of building sites, which is a huge difference. That being said, I do agree that using the phrase "simpler" is in poor taste and should be avoided. Thank you for this feedback!

2. Is there a really good reason not to use it in a best practice example?
No, there isn't. I started this project on the very first beta and at that point there where some issues with ModelsBuilder generating .cs files that where not compiling. Therefor I went in a direction of not using ModelsBuilder. At that point I had no intention of doing a public demo site, it was just used for personal project. But now as it is being viewed as a "best practice" it makes a lot of sense adding ModelsBuilder models. As you mention, what I'm doing at the moment is basically what models builder is doing anyway, except automagically. I would love to see an update / PR from you (or someone else) where models builder is introduced. As long as all the logic is still through controllers (i.e not just throwing ModelsBuilder models straight in a view without going through a controller). Possibly also with partial modelsbuilder classes where necassary logic is needed (modifying properties etc). And that it's still testable of course. Then I have nothing agains ModelsBuilder being used in this demo site.

I will make sure to update the readme based on your feedback.

Cheers mate! ❤️

[enkelmedia]

Hi man!

I see where you're coming from makes sense! Hard to use something that does not work =D

Also, it might be that my interpretation of your intent was wrong but it's always good to be super-clear (reg. the readme).

Thanks for the positive leaning towards my suggestions I'd be happy to help but probably need some time before I can get my hands dirty. I'll update here when I start, meanwhile - if someone want to grab it before just drop a comment here and we could work together.

// m

[Adolfi]

Awesome, thanks man! ❤️
I totally get that this kind of update would require some work and you might not have that time at the moment.
I'll leave this issue open for anyone to pick up if they feel inspired to update it.

Cheers!

[mistyn8]

Interesting topic of discussion. We're still using DTGEGridEditor in v9 and started with viewcomponents.. so I've seemed to end up have modelsbuilder, as a view model for dtge, then a ViewModel simplifying the modelsbuilder model to pass into the viewcomponent, which then has a ViewModel to pass back to the viewcomponent for rendering it's view.. :-) Seem to be creating a lot of overhead there. (and none of this can be fully accomplished in the backoffice anymore)

I think this feeds into another topic, that of is the umbraco backoffice (template/partial/model generation) all seen as legacy and not to be used in anger.. as we should be deploying source controlled templates/partials and compiled models as part of a deploy process.
Having used umbraco since v4 the integrated backoffice approach, as a cms for a developer abstracted out so much of the learning curve of underlying web forms and then MVC whilst still giving so much control (not to mention being able to hand over to a client and still allow them to do things that would normally require a developer) was part of the joy of working with umbraco.
Once setup it was completely plausible to just use the backoffice to do cms things.

Recent forum posts also talk about js/css also shouldn't be edited via the backoffice again as source control should rule.
But again that's the beatury of the umbraco backoffice.. no need for ftp/ssh etc just a web browser and a login!

I am a little downhearted that this USP is being eroded (that USP was why we went for umbraco in the first place over other CMS's at the time that enforced needing a season developer to even simply extend a template), but understand why the direction.

(for simple sites where we leave the client able to use the backoffice for the should be source controlled items in order to have the best of both worlds we have taken to leaving the production site backed by a local git repo on azure app_services, so when coming back to the site as a developer we can fetch down the backup giving a git repo with all backoffice instigated changes ready to be staged. and committed)

There was also ModelsBuilder.Embedded in v8 that seemed to go the other way. No more intellisence in VS so hampering the developer. So this is a two way street ;-)

However, on the flip side I wouldn't be without source control, MVC separation of concern and testable code (though I don't do enough of that)
.
I wonder if the backoffice might ever be extended to allow editing of controllers? maybe in app_code for JIT compilation.. if that is even still a thing in .netcore? to make the backoffice holistic again? and pie in the sky could the backoffice not have some of the features of deploy ported (umbcloud) so that saving templates commits to your repo ?? to keep the USP?

Please know that I'm loving seeing your demo site evolve, and am indebted to you for simplifying the viewcomponents learning curve. Just the topic here resonated with some of the thoughts I'd been having lately, around the diminishing of the backoffice. to just a content,media.member and user manager.

[enkelmedia]

This is an interesting point, we often find what we have to reference some packages in our ".Core" (we actually call them .Web)-projects to be able to pass these "package specific models" to the views for rendering.

It's not really related to Models Builder as the underlaying model from something like Content.GetProperty("intro") would be what ever the ValueConverter returns so the models builder models only wraps a call like this:

public string Intro => _content.GetProperty<string>("into")

But the problem is the same, the .Core-project might need to reference some of the package dlls to be able to pass the models to the view model. I still think it's worth it and that passing the whole Models Builder-model to the view should be avoided.

This is something that would be good to address as well, how to include a package with a PropertyValueConverter and use these models in the views. A good fit might be Contentment that is widely used and freaking awesome.

[idseefeld]

@Adolfi @enkelmedia Indeed I am a great fan of ModelsBuilder and was actually planning to start my own Demo project. But as Dennis agrees that this project should evolve into this direction, I like to start a branch for this here.
Currently I have some time which I can devote for this😁

[idseefeld]

I just startet ModelsBuilder in fork https://github.com/idseefeld/UmbracoNineDemoSite (select branch: modelsbuilder)

[Adolfi]

That’s awesome Dirk, thank you for this! ModelsBuilder will be great for a demo site, as long as it’s still Testable and logic is still going trough controllers (ie not just throwing ModelsBuilder models straight in to views).

also would prefer it if the generated models would be generated to its own separate project, for example *.Models, for nice separation. What do you think?

[idseefeld]

@Adolfi Indeed I think it is not only nice to separate the generate model into its own project, but actually necessary to avoid circular dependencies. That was at least my understanding for the previous approach from Stephan with the VisualStudio extension.
Now the setup looks different for v9 and I still have to figure this all out.
I also agree that we need extra mapping for viewModels and of course these changes should be testable!

[Adolfi]

Sounds amazing. I have completed faith in you seeing I know how much you value clean code/architecture! ❤️

[enkelmedia]

I was just about to add that I also think that the models can go in the same project - a separate project only for models would just make the it worse with all the dependencies.

I also think that it would be good to show how to use a "Property Editor Package" that ships with custom types for the models, ie:

https://our.umbraco.com/packages/backoffice-extensions/contentment/

https://our.umbraco.com/packages/collaboration/seo-visualizer/

[idseefeld]

I have startet with Models Builder in fork https://github.com/idseefeld/UmbracoNineDemoSite (select branch: modelsbuilder)

After checking out branch modelsbuilder and building, go into back office settings > uSync section and import Settings and Content.

Then goto settings : Models Builder and generate models (which should be outdated).

What I have done so far:

1. Added a new project for generated models and added the necessary configuration in /appSettings.Development.json
2. Updated HomeViewModel using the generated strong typed model.
3. Updated Tests for Home controller

What do you think about this approach?

@enkelmedia Maybe I am not up to date with compiler capabilities regarding circular dependencies, but I think you need the generated models in a dedicated project to use them e.g. in the core project.
Here is a nice article which explains my concern in detail: https://stackoverflow.com/questions/38042130/what-is-a-circular-dependency-and-how-can-i-solve-it/38042916

[Adolfi]

Hi Dirk!
I like what I see. I personally prefer the models in a separate project like you have done, and I don’t see how that would ever cause a circular dependency problem since the .GeneratedModels project doesn’t reference any other project. Right?

regard the HomeController and ViewModel, it’s not exactly what I pictured it but close. Instead of parsing the ContentModel as a gm.Home, you should be able to just take in a Home model in the ActinResult and then just set each property of the view model in the constructor. What do you think?

Here how I mean with the ActionResult method:

public IActionResult HomePage(Home model) //credit to https://github.com/bakersbakebread for finding this one.
    {    
      ... 
    }

see this gist for more info https://t.co/ep6CnqGHee

Looking forward hearing what you think..

[Adolfi]

Also, would it be possible to name the project .Models instead of GeneratedModels and place all the classes in the root of the project, since this is the only thing this project will ever contain Models its unnecessary with a folder called Models IMHO

[enkelmedia]

Hi guys!

Nice work on this one! I have never had an issue with circular dependencies, I was referring to dependencies on ie. Contentment etc. since a strongly typed model from a 3rd party property editor might be needed in the view model (as someone talked about earlier) a dedicated project for the models adds yet another project that needs to depend on this external property editor. Not an issue, more a matter of taste.

I would like to challenge @Adolfi on the approach of injecting the IPublishedContent into the view model, this ties the view model it self to the IPublishedContent-interface and Umbraco - I would argue that a mapper would be the cleanest way. Also sometimes the view model might consist of more than just properties on the current IPublishedContent, some of them might be cached or assembled from somewhere else.

[idseefeld]

Ok, circular dependency is not an issue when we use an extra project just for the generated models (btw. I have renamed it 😉).
My concern was with generating the models in the web-project. Because than the core-project needs to reference the web-project.

I have to reconsider your suggestion for public IActionResult HomePage(Home model). At the first glance it sounds interesting. Let's see how this will work for testing...

[Adolfi]

@enkelmedia I love it! ❤️ A mapper would be really great here and it would make the view model a lot cleaner with no external dependencies and no need for the constructor at all! Personally I prefer AutoMapper, do we know if Umbraco ships with AutoMapper (or other) out of the box?

[Adolfi]

Regarding the 3rd party packages I would prefer have as few (or none) 3rd party packages in this project unless it is really necessary (like uSync). Don’t want to force any packages on any1.

[enkelmedia]

@Adolfi =D Thanks!

Umbraco remove the dependency on AutoMapper in V8 (I think) they have a built in concept called "UmbracoMapper" https://our.umbraco.com/documentation/reference/Mapping/ that they use internally and that could be used.

Personally I'm not a fan of AutoMapper I used to use it a lot but found that it often ended up in strange runtime exceptions and specially hard to follow code with static analysis (like CodeLens ie). You would loose the "references" since they are implicit. And I found my self debugging AutoMapper-mapping very often.

Any way.. I would vote for a simple class that just mapps, or using the UmbracoMapper (but that is just my opinion =D ). This way the mapper could be registered with DI and take in dependencies (on a cache ie) if needed.

On 3rd party packages - I agree! If this was a "starting template" (which it might be) I would avoid it. But if the purpose is to show how to build a maintainable site the idea behind having a 3rd party Property Editor is to show how to use it in the code - the generated models, mapping it to a view model etc.

[enkelmedia]

@idseefeld I was more leaning towards putting the models in the .Core-project and have the .Web-project depend on that. I don't really see a value in having a separate project just for the generated models - but its a matter of taste.

In our projects we mostly setup the structure like this:

• Project.Core = No dependency on Umbraco at all.
• Project.Web = Dependency only on Umbraco.Core and Umbraco.Web (we put generated models here)
• Project.Web.UI = The Umbraco-project with dependency on UmbracoCms

[idseefeld]

@enkelmedia Ok, than we are on the same track. Good idea to put the generated models into .core project as we have only this extra project. Your mostly used projects structure makes sense to me, but for now let's stick with one extra project in this Demo.

@Adolfi What do you think about the consolidation of .Models and .Core project?
Will it make sense you create a modelsbuilder branch and I make PRs into that branch (make collaboration probably simpler)?

[Adolfi]

I guess most of the things we are discussing here in terms of project structure all boils down to taste and personal preferences. I don’t really have a super strong opinion on where the models should be placed. Both is fine by me, every one use different approach and I don’t think we will regret the decision regardless of which one we choose.

Otherwise I’m afraid we may be stuck discussing these things until the end of time. 😊

@enkelmedia agreed, we can add our own mapping logic instead of installing a mapping project.

@idseefeld sounds good, however I won’t find time to start that branch before sometime next week unfortunately.

[Adolfi]

@enkelmedia agree, i wouldn’t mind a nice example of using Contentment in a strongly typed example seeing how this is a demo site and not a starter/template. Maybe we could add this as a feature request for someone to pick up and not mix it in with this ticket which is primarily focused on ModelsBuilder (even if it relates to 3rd party property editor).

[enkelmedia]

@Adolfi True! Lets keep the scope of this one simple =D

[idseefeld]

I have changed the page view models into simple POCOs and manual mappings to the page controllers.
Next I'll give the UmbracoMapper a try😁

[idseefeld]

Today I have looked into the UmbracoMapper and these are my findings:

1. In contrast to the AutoMapper package UmbracoMapper provides no automatic mapping of properties with same name and type.
2. It has quite an overhead of definitions and composition
3. Test need more setup and have not even finished the implementation

My conclusion for this Demo-Site: UmbracoMapper does not make sense. The UmbracoMapper overhead might useful in cases, where you have a lot of repeated model mappings. Than it might be an option for cleaner code.

3rd party package Anaximapper from Andy Butland might be an interesting alternative to ModelsBuilder + UmbarcoMapper/manual mapping, but again not for this project which should rely on pure Umbraco, shouldn't it?

Nevertheless, if you like to review my attempt, you can check out branch modelsbuilder-with-UmbarcoMapper of my fork https://github.com/idseefeld/UmbracoNineDemoSite.

[enkelmedia]

I would vote for a simple "Mapping-class" that is just vanilla C# OR to do the mapping in the controller. No need for UmbracoMapper or 3rd party magic =D

[Adolfi]

@enkelmedia agreed! Simple mapper-class is 100% enough!

[idseefeld]

@Adolfi just added a PR #15