TypedPageData composition

Oct 20, 2010 at 5:31 PM

Hi Joel,

One of the cases I'm encountering more and more when using PageTypeBuilder is the limitation caused by c# singular inheritance, meaning that a lot of my TypedPageData classes contain repeated property definitions.

Scenario:

I have a bunch of common elements that are used across multiple page templates. These elements each require multiple PageTypeProperties.

Page1 contains Element1, Element2
Page2 contains Element2, Element3
Page3 contains Element1, Element3

In simple scenarios this could be achieved using a singular TypedPageData base class, however in the above situation, we can't do that as there isn't a common set of elements between all three pages. This means that I end up with three TypedPageData classes all of which have multiple definitions for the same property. Extending this scenario further to a more realistic number of templates and it starts to become a bit of a pain.

I actually wonder whether this need is the same as mentioned ion the existing request in the issue tracker

'Read PageTypeProperty attributes from interfaces' - http://pagetypebuilder.codeplex.com/workitem/5708

Reading the discussion on that thread, I think your solution about using interfaces to ensure the necessary properties are implemented by each PageType is sensible. Where you  want to change implementations per class this is very flexible, but it still means some additional work to implement each PageTypeProperty on every TypedPageData class. If you actually want the same implementation and attribute meta-data, you've no option but to repeat your code.

What I was wondering is whether there was a better way, as I'm pretty sure this is a common scenario. My initial thoughts were as to whether we could compose TypedPageData classes from other TypedPageData classes rather than relying on singular inheritance.

 [PageType]
    public class PageType : TypedPageData
    {
        [PageTypeProperty]
        public string StringProperty { get; set; }

	public EmbeddedPageType AnEmbeddedPageType { get; set; }
    }


[PageType]
    public class EmbeddedPageType : TypedPageData
    {
        [PageTypeProperty]
        public string AnotherStringProperty { get; set; }

    }

and PageTypeBuilder discovery could flatten these out to a single definition list containing StringProperty and AnotherStringProperty. Initially I can see a few validation cases that must be handled (property name conflicts for example, circular compositions).

Not too sure about this approach so just wanted to get your opinions before I cracked on with a test implementation.

Thanks

Mark

 

Coordinator
Oct 20, 2010 at 8:44 PM

Hi Mark!

It sounds like a good idea from a technical perspective, although I think it might be necessary to create a separate interface/base class for "property groups" that can be embedded into other page types rather than embedding other page types or it might otherwise be tricky to access the property values.

However, from an API/product perspective, I think such a solution might be pushing the complexity/benefit ratio a bit.

I hope/plan to make PTB far more extensible in the future allowing features such as this to be added easily without modifying PTB's source code. That would also give me a good way to measure what potential features people actually use and adopt those that solve the most common problems into the core library. I think a feature such as this would be perfect to develop and try out as an extension.

With that said, you are of course most welcome to fork the source code right away and start doing a test implementation.

As for reading attributes from interfaces, make sure to check out the 1.3 RC version that does that.

Coordinator
Oct 22, 2010 at 10:14 AM

The more I think about this, and also seeing Patrik's comment on my recent blog post, I see more and more benefit. Add to that my mind can't seem to stop trying to come up with the least complex and most flexible way to implement this and I'm pretty excited about adding a feature similar to what you describe.

I have a few ideas about how I'd like to implement it but I'd love to hear more about yours! Especially about how you would access properties of an embedded page type.

Some of the things I've thought about so far is:

  • EPiServer properties could be named using a combination of the two code properties names separated with a character that is invalid for code property names. For instance CurrentPage.Image.AltText could be equal to CurrentPage["Image-AltText"]. This would limit the risk for clashing properties.
  • A key issue is how the property value's are accessed. I'm currently thinking that the Image property in the above example could be of a special type, let's call it PropertyGroup. PropertyGroup could require a PageData object as a ctor parameter. In other words I'm thinking about not embedding page types but rather groups of properties. With that said I'd love to hear ideas on how to embed entire page types.
  • The actual Image property on the page type class would somehow need to create a PropertyGroup and allow for two things: 1. It needs to create the PropertyGroup using DynamicProxy to enable automatic getters and setters inside PropertyGroups. 2. It needs to be able scenarios where the PropertyGroup has more than one ctor parameter to enable dependency injection into PropertyGroups the same way it's enabled for page types. Some sort of factory class could probably be used for this.
Editor
Oct 23, 2010 at 8:41 PM

Hi Joel

I have had a look at implementing this in a similar way as you have suggested above so far I have done the following:

1.  Created a new PropertyGroup abstract class which has two protected properties:

  •  TypedPageData of type TypedPagedData
  •  Heirarchy of type string

2. Modified the GetPageTypePropertyDefinitions method within the PageTypePropertyDefinitionLocator class to recursively look for properties within a PageType class or PropertyGroup class which have properties which are of type PropertyGroup.

3. Modified the CreateAndPopulateTypedInstance method within the TypedPageActivator class to recursively create instances of the PropertyGroups and set the TypedPageData property and heirarchy e.g. Image-AltText.

4. Modified the Intercept method within the PageTypePropertyInterceptor class to check whether the PageTypeProperty being intercepted is within a PropertyGroup class.  If it is I use it's TypedPageData and heirarchy properties, otherwise I use the existing intercepting code.

This all seems to work fine, I have yet to create any unit tests to verify the discovery and activation.  I am a bit unsure at the moment as to what I would need to do regarding the dependency injection, this stuff is a bit new to me :)

Lee

Oct 24, 2010 at 10:07 PM

Hey Joel,

I'm glad you're as excited about this feature as Lee and I am.

1) I definitely agree about creating a new PropertyGroup type. Directly embedding entire TypedPageData objects leads to a messy API, i.e. CurrentPage.PageName == CurrentPage.Image.PageName

2) Lee's design above seems to remove the need to explicitly create a DynamicProxy of the PropertyGroup as the interception still occurs from the TypedPageData class - though this does come at the expense of added complexity in the Intercept method. It would be pretty simple to enable Dependency injection using a similar pattern to that used for used for TypedPageData - ie - moving the ctor parameters into a separate PopulateInstance method on the PropertyGroup abstract class and using the container to resolve any dependencies.

Cool stuff

Mark

 

 

 

Editor
Oct 25, 2010 at 8:41 AM
Edited Oct 25, 2010 at 8:47 AM

Hi Mark/Joel

I am still using dynamic proxies for the class properties that will inherit from the new PropertyGroup class, so dependency injection and interception would work in the same way, I think.

I have also added a class called PropertyGroupAttribute which can be used to decorate the property that will appear on the PageType, for example we could have a PageType like the following

 

    [PageType]
    public class TestPageTypeWithPropertyGroups : TypedPageData
    {

        [PageTypeProperty(EditCaption = "Property one", SortOrder = 100)]
        public virtual string LongStringProperty { get; set; }

        [PropertyGroup(EditCaptionPrefix = "Image one - ", StartSortOrderFrom = 400)]
        public virtual Image ImageOne { get; set; }

        [PropertyGroup(EditCaptionPrefix = "Image two - ", StartSortOrderFrom = 500)]
        public virtual Image ImageTwo { get; set; }

        [PropertyGroup(EditCaptionPrefix = "Image three - ", StartSortOrderFrom = 600)]
        public virtual Image ImageThree { get; set; }
    }

    public class Image : PropertyGroup
    {

        [PageTypeProperty(EditCaption = "Image Url", SortOrder = 200)]
        public virtual string ImageUrl { get; set; }

        [PageTypeProperty(EditCaption = "Alt text", SortOrder = 210)]
        public virtual string AltText { get; set; }

    }

The PropertyGroupAttribute allows you to set an EditCaptionPrefix so in edit mode the ImageUrl of ImageOne will appear as "Image one - Image Url".  You can also specify the StartSortOrderFrom which allows you to reset the sort ordering.

This is all working OK, but still no unit tests.  I am going to commit the work I have done so far to our local repository so Mark can have a look at what has been extended so far :)

Lee

 

Coordinator
Oct 25, 2010 at 11:15 AM

Sounds really good! The example you give above is pretty much exactly how I had imagined it. I would love to take a look at your implementation.

Editor
Oct 26, 2010 at 8:21 AM

Hi Joel

I am fairly happy with what I have implemented so far and have also implemented discovery and activation unit tests.

The changes I have made so far have been against v1.2. 

I suppose the next step if for me to download v1.3 make the changes in a local copy of that then somehow send the new and updated files to yourself, what is the best way of doing this?

Lee

Oct 26, 2010 at 5:50 PM

Hi Joel,

I've posted our contribution here, as Lee mentioned - we forked at v1.2

http://pagetypebuilder.codeplex.com/workitem/7435

(I couldn't see how to add a file to a discussion - so I opened it as an issue)

Cheers

Mark

 

Coordinator
Nov 22, 2010 at 10:20 AM

Sorry for not replying earlier.

Thanks for the code. This functionality will definitely be included in the next version. For legal and other reasons (I'm redoing some other stuff) I might not use the exact same code but using it for inspiration will be great!

Editor
Nov 22, 2010 at 12:02 PM

That's great! Thanks Joel