Comments (28)
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...
from modelsbuilder.original.
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?
from modelsbuilder.original.
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; }
from modelsbuilder.original.
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.
from modelsbuilder.original.
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?
from modelsbuilder.original.
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?
from modelsbuilder.original.
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.
from modelsbuilder.original.
Adding this to 2.1.3 milestone, but it will be an option that will not be activated by default.
from modelsbuilder.original.
Sounds great
from modelsbuilder.original.
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.
from modelsbuilder.original.
Lovely!
from modelsbuilder.original.
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 :(
from modelsbuilder.original.
Not release - I'm late ;-(
from modelsbuilder.original.
That explains, no rush ;-)
from modelsbuilder.original.
Closing the issue as it's been implemented (though not released yet).
from modelsbuilder.original.
(released)
from modelsbuilder.original.
#H5YR
from modelsbuilder.original.
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!
from modelsbuilder.original.
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...
from modelsbuilder.original.
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?
from modelsbuilder.original.
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.)
from modelsbuilder.original.
But what Umbraco document type carries the "meta title" document property?
from modelsbuilder.original.
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.
from modelsbuilder.original.
The CompPageSettings DocType:
The Homepage DocType:
from modelsbuilder.original.
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...
from modelsbuilder.original.
Yes, that is correct.... :-)
from modelsbuilder.original.
See #72 - suggesting we further discuss there as this issue is closed.
from modelsbuilder.original.
So what ends up happening, is that those properties basically "dissapear" from the DocTypes that should have them.
from modelsbuilder.original.
Related Issues (20)
- Generating models using the VS extension can cause namespace issues HOT 3
- Cleaner csproj include generated files HOT 1
- Can't Rebuild Models After Deleting Document Type HOT 6
- Interface definition only generated when document type is in use HOT 1
- Request error: The URL returned a 404 (not found): /App_Plugins/ModelsBuilder/modelsbuilder.html HOT 1
- Provide option for generated code to use Language Fallback HOT 7
- ModelsBuilder API Generates first IPublishedElement then never finishes HOT 10
- VS Extension: Method not found: 'System.String Umbraco.ModelsBuilder.Building.Compiler.CreateValidIdentifier(System.String)' HOT 2
- v4: Repository shouldnt link to local nuget server
- v4: If Document type with Template is created Umbraco fails with save of Document Type
- Implemented property keeps being generated HOT 6
- ModelsBuilder on Umbraco v8.8 HOT 10
- v4: Generator should be swappable / extendable
- Generated Models missing PublishedElementModel on generated model.
- Stack Overflow when implementing same interface in generated and partial class
- VS Extension install issue
- Execute ModelsBuilder Programmatically
- When generating models for IPublishedElement it adds a controller no matter what
- UmbracoModelsBuilder: UmbracoModelsBuilder failed to generate code: NullReferenceException HOT 1
- VS2022 HOT 5
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from modelsbuilder.original.