Utilizing custom code for DocType Compositions? about modelsbuilder.original

[zpqrtbnk]

This is a situation I have faced too, and I don't have a definite answer yet, so happy to discuss it. At the moment, when you create composition "Composition" and type "ContentType", we generate interface IComposition, and classes Composition (implementing IComposition) and ContentType (implementing IComposition).

Each class implements the interface locally, ie redefines the corresponding properties. Just because it's easy to generate and it made sense. But it means that if you want to add or change a property of the composition, you need to declare that change both in Composition and in ContentType.

I never thought of simplifying the implementation of ContentType properties by creating an inner Composition object... That's a clever idea. Ie, doing

public Whatever PropertyFromComposition
{
  get { return new Composition(this).PropertyFromComposition; }
}

But you seem to say that it does not work? Let me have a look at my tests...

[zpqrtbnk]

Mmm... works by my tests... so you say mntpSelected is a property that is defined on Composition and returns a CSV string, right? So I guess we generate something along...

public string MntpSelect { get { return this.GetPropertyValue<string>("mntpSelected"); } }

both in the Composition and ChildDocument, right again? Which allows you to write

@Model.Content.MntpSelect

to display the CSV string. Now what you want is not a CSV string but actual nodes. So you want to implement the following property:

public IEnumerable<IPublishedContent> SelectedNodes
{
  get
  {
    return this.MntpSelect.Split(',')
      .Select(x => int.Parse(x))
      .Select(x => UmbracoContext.Current.ContentCache.GetById(x));
  }
}

Still correct? Does that property work if you implement it at ChildDocument level? And then, you want to implement that property for anything that implements the composition, so you want to move that property to Composition and then in ChildDocument have:

public IEnumerable<IPublishedContent> SelectedNodes
{
  get { return new Composition(this).SelectedNodes; }
}

Still correct? That way, you don't duplicate the actual implementation of SelectedNodes in every content type implementing the composition, which makes sense. But you are facing an issue, because when the property moves to Composition then it does not work?

[hfloyd]

You understand it all perfectly, I think.

It's possible that there is something wrong with my code implementation of "SelectedNodes"... and it's not a problem with the use of the Composition object... I am investigating that right now, and will get back to you.

I am glad to get your opinion on the whole concept, though... Do you see any potential problems with using the Composition object?

get { return new Composition(this).PropertyFromComposition; }

[hfloyd]

Yes, there was something wrong with my code to get the nodes... since I had found that I could implicitly convert between IPublishedContent and the custom model objects like:

var foo = umbracoHelper.GetTypedContent(x) as MyCustomModelObject

I thought this would work also:

var selectedNodes = [...code to get nodes from "mntpSelected"... ] as IEnumerable<MyCustomModelObject>

but that didn't actually work without an explicit conversion, but it wasn't throwing an exception or anything, just returning null.

We've got that fixed now, and using "Composition(this).MyProp" works now :-)

I'm still curious to hear any additional thoughts about it that you might have.

[zpqrtbnk]

Been thinking about it... the issue with

return new Composition(this).PropertyFromComposition;

is that you create a new instance of the Composition class anytime you get the property value, which is not optimum. Better do something like:

public class Composition : IComposition
{
  internal static Something SomePropertyInternal(IComposition that)
  {
    return that.GetPropertyValue<Something>("something");
  }

  public Something SomeProperty { get { return SomePropertyInternal(this); } }
}

public class ContentType : IComposition
{
  public Something SomeProperty { get { return Composition.SomePropertyInternal(this); } }
}

In fact, thinking about it, maybe that's how the code should be generated in the very first place. What do you think?

[hfloyd]

That is easy to do, and I had just added a new Composition to my project, so when I added the custom property, I did it that way, and it works fine...

If you feel that it's more efficient, then perhaps that should be the recommended pattern.

Which portion would be auto-generated in your view?

[zpqrtbnk]

I am currently experimenting with generating both the static method and the properties. Looks like it works fine. Still work-in-progress during CodeGarden as I am a bit busy now.

[zpqrtbnk]

Adding this to 2.1.3 milestone, but it will be an option that will not be activated by default.

[hfloyd]

Sounds great 👍 See you next week at CG!

[zpqrtbnk]

So... we introduce new options. StaticMixinGetters when true (is false by default) will cause all getters in compositions to use the static methods pattern described above. By default the name of the method is built from the name of the property, eg property Foo will use static method GetFoo but the StaticMixinGetterPattern option allows to specify a new (string format) pattern eg "StaticGetterForThe{0}Property" in case someone thinks that's better.

We automatically detect whether the static method already exists in the existing partial code, and only generate the method if needed. So if one wants to implement ones own definition of the static getter... just do it and we'll detect and use it.

Status: implemented, currently testing.

[hfloyd]

Lovely!

[highstone]

Just ran into this. See that it has been added to the 2.1.3 milestone, but is this the milestone that has been released to NuGet (2.1.3-beta001)? Can't get the option to work when I add it to the appsettings :(

[zpqrtbnk]

Not release - I'm late ;-(

[highstone]

That explains, no rush ;-)

[zpqrtbnk]

Closing the issue as it's been implemented (though not released yet).

[zpqrtbnk]

(released)

[hfloyd]

#H5YR

[hfloyd]

Hi Stephan,
I have noticed something with the way this is working....

If I have a Composition with customized properties, the automatic generation seems to only be working for properties which I haven't customized....

For instance,
I have a Composition name "CompPageSettings"...

These props are in the "CompPageSettings.generated.cs" file:

public partial interface ICompPageSettings : IPublishedContent
{
    ...
    /// <summary>Exclude Page from Navigation Menus?</summary>
    bool UmbracoNaviHide { get; }

    /// <summary>Exclude Page from Sitemap?</summary>
    bool UmbracoSitemapHide { get; }

    /// <summary>Page URL Aliases</summary>
    string UmbracoUrlAlias { get; }

    /// <summary>Change Page URL to:</summary>
    string UmbracoUrlName { get; }
}

public partial class CompPageSettings : PublishedContentModel, ICompPageSettings
{
    ...
    /// <summary>Exclude Page from Navigation Menus?</summary>
    [ImplementPropertyType("umbracoNaviHide")]
    public bool UmbracoNaviHide
    {
        get { return GetUmbracoNaviHide(this); }
    }

    /// <summary>Static getter for Exclude Page from Navigation Menus?</summary>
    public static bool GetUmbracoNaviHide(ICompPageSettings that) { return that.GetPropertyValue<bool>("umbracoNaviHide"); }

    /// <summary>Exclude Page from Sitemap?</summary>
    [ImplementPropertyType("umbracoSitemapHide")]
    public bool UmbracoSitemapHide
    {
        get { return GetUmbracoSitemapHide(this); }
    }

    /// <summary>Static getter for Exclude Page from Sitemap?</summary>
    public static bool GetUmbracoSitemapHide(ICompPageSettings that) { return that.GetPropertyValue<bool>("umbracoSitemapHide"); }

    /// <summary>Page URL Aliases</summary>
    [ImplementPropertyType("umbracoUrlAlias")]
    public string UmbracoUrlAlias
    {
        get { return GetUmbracoUrlAlias(this); }
    }

    /// <summary>Static getter for Page URL Aliases</summary>
    public static string GetUmbracoUrlAlias(ICompPageSettings that) { return that.GetPropertyValue<string>("umbracoUrlAlias"); }

    /// <summary>Change Page URL to:</summary>
    [ImplementPropertyType("umbracoUrlName")]
    public string UmbracoUrlName
    {
        get { return GetUmbracoUrlName(this); }
    }

    /// <summary>Static getter for Change Page URL to:</summary>
    public static string GetUmbracoUrlName(ICompPageSettings that) { return that.GetPropertyValue<string>("umbracoUrlName"); }
}

And these props are customized in the "CompPageSettings.cs" file:

public partial interface ICompPageSettings : IPublishedContent
{
    /// <summary>Meta Description</summary>
    string MetaDescription { get; }

    /// <summary>Meta Keywords</summary>
    string MetaKeywords { get; }

    /// <summary>Meta Title</summary>
    string MetaTitle { get; }
}

public partial class CompPageSettings
{
    /// <summary>Meta Description</summary>
    [ImplementPropertyType("MetaDescription")]
    public string MetaDescription
    {
        get { return GetMetaDescription(this); }
    }

    /// <summary>Static getter for Meta Description</summary>
    public static string GetMetaDescription(ICompPageSettings that)
    {
        var meta = that.GetPropertyValue("MetaDescription").ToString();
        return meta;
    }

    /// <summary>Meta Keywords</summary>
    [ImplementPropertyType("MetaKeywords")]
    public string MetaKeywords
    {
        get { return GetMetaKeywords(this); }
    }

    /// <summary>Static getter for Meta Keywords</summary>
    public static string GetMetaKeywords(ICompPageSettings that)
    {
        var meta = that.GetPropertyValue("MetaKeywords").ToString();
        return meta;
    }

    /// <summary>Meta Title</summary>
    [ImplementPropertyType("MetaTitle")]
    public string MetaTitle
    {
        get { return GetMetaTitle(this); }
    }

    /// <summary>Static getter for Meta Title</summary>
    public static string GetMetaTitle(ICompPageSettings that)
    {
        var meta = that.GetPropertyValue("MetaTitle").ToString();
        return meta != "" ? meta : that.Name;
    }

}

If I then "Run Custom Tool" to generate the DocType classes which utilize the Composition... It causes compile errors in the generated files.

For example, in Homepage.generated.cs", this line is showing an error:

public partial class Homepage : BasePage, ICompPageContent, ICompPageSettings

with the messages:

'MySite.Web.Models.Homepage' does not implement interface member 'MySite.Web.Models.ICompPageSettings.MetaTitle'

'MySite.Web.Models.Homepage' does not implement interface member 'MySite.Web.Models.ICompPageSettings.MetaKeywords' 

'MySite.Web.Models.Homepage' does not implement interface member 'MySite.Web.Models.ICompPageSettings.MetaDescription'

The "Homepage.generated.cs" files DOES contain this code:

...
/// <summary>Exclude Page from Navigation Menus?</summary>
[ImplementPropertyType("umbracoNaviHide")]
public bool UmbracoNaviHide
{
    get { return CompPageSettings.GetUmbracoNaviHide(this); }
}

/// <summary>Exclude Page from Sitemap?</summary>
[ImplementPropertyType("umbracoSitemapHide")]
public bool UmbracoSitemapHide
{
    get { return CompPageSettings.GetUmbracoSitemapHide(this); }
}

/// <summary>Page URL Aliases</summary>
[ImplementPropertyType("umbracoUrlAlias")]
public string UmbracoUrlAlias
{
    get { return CompPageSettings.GetUmbracoUrlAlias(this); }
}

/// <summary>Change Page URL to:</summary>
[ImplementPropertyType("umbracoUrlName")]
public string UmbracoUrlName
{
    get { return CompPageSettings.GetUmbracoUrlName(this); }
}
...

I wonder if it is possible to have ModelsBuilder add the same implementations for customized properties on compositions as it does for non-customized ones?

Or do you have a suggestion to otherwise handle this?

Thanks!

[hfloyd]

Ps. The only way I have seen to deal with this is to add the customized properties explicitly to every Doctype class's custom file like this:

    #region CompPageSettings Overrides

    /// <summary>Meta Description</summary>
    [ImplementPropertyType("MetaDescription")]
    public string MetaDescription
    {
        get { return CompPageSettings.GetMetaDescription(this); }
    }

    /// <summary>Meta Keywords</summary>
    [ImplementPropertyType("MetaKeywords")]
    public string MetaKeywords
    {
        get { return CompPageSettings.GetMetaKeywords(this); }
    }

    /// <summary>Meta Title</summary>
    [ImplementPropertyType("MetaTitle")]
    public string MetaTitle
    {
        get { return CompPageSettings.GetMetaTitle(this); }
    }

    #endregion

But that seems to defeat the point of having it auto-managed...

[zpqrtbnk]

Not sure I understand what you are trying to do here. Why would you need to define the "meta" properties explicitely in your CompPageSettings.cs file? Aren't they normal, existing Umbraco properties--and then on what document type are they defined?

[hfloyd]

Hi Stephan.
Yes, they are "normal" properties... however, I have put them together into the Composition so they can be added to various DocTypes easily in a group. So, for something like the "Meta Description", it makes sense to use a "textbox multiple" property type, but by default, ModelsBuilder generates that as an "object" rather than a "string". Because of this, I tend to re-define this property in the model as a string. In addition, for the Meta Title property, if no Meta title is explicitly provided, I want to fall-back to the Node Name, which also requires customized logic.

Ideally, this customized logic would be defined in a single place, and all the DocTypes utilizing the Composition would use it automatically. However, I end up with interface type-mistmatch for MetaDescription - since ModelsBuilder thinks it should be an object, so I have to remove it from the "generated" interface and put it into a "custom" interface (as a string).

But then when I compile, I get errors about the inheriting Doctypes "not implementing all members of the interface" - since ModelsbUilder doesn't pick up on the "custom" interface bits and auo-generate the stubs for those properties. It would be great if ModelsBuilder would add this to the Doctypes using the Composition:

/// <summary>Meta Title</summary>
[ImplementPropertyType("MetaTitle")]
public string MetaTitle
{
    get { return CompPageSettings.GetMetaTitle(this); }
}

/// <summary>Meta Description</summary>
[ImplementPropertyType("MetaDescription")]
public string MetaDescription
{
    get { return CompPageSettings.GetMetaDescription(this); }
}

(Notice that the "GetMetaTitle" & "GetMetaDescription" methods are defined in the "CompPageSettings.cs" file.)

[zpqrtbnk]

But what Umbraco document type carries the "meta title" document property?

[hfloyd]

CompPageSettings has those properties defined, then Homepage, TextPage, Product... etc. All reference CompPageSettings as a composition.

So, technically, when generated, those properties do appear on the Homepage, TextPage, Product... etc. models... but I need to "fix" them.

[hfloyd]

The CompPageSettings DocType:

The Homepage DocType:

[zpqrtbnk]

Got it. And once you override them in CompPageSettings.cs, we decide that you are on your own and don't implement them anymore in types that implement ICompPageSettings. Correct?
Creating a new issue for that one, hold on...

[hfloyd]

Yes, that is correct.... :-)

[zpqrtbnk]

See #72 - suggesting we further discuss there as this issue is closed.

[hfloyd]

So what ends up happening, is that those properties basically "dissapear" from the DocTypes that should have them.