Come up with a way of chaining Resolvers

[mattbrailsford]

I think I have finally figured out why the use of both Resolvers and Type Converters seems confusing, and that's because we are using them for much the same things, it just depends in what phase of the converter cycle we need them to run at. Essentially what I think we need as a single concept that can be chained together in a given order, so, for example, you could say use UmbracoProperty to grab a specific property, but assuming that property returns IEnumberable but you only want the first item, you could then pass it through another resovler that can convert IEnumerable to a single item, but then maybe you want to convert that single item into something else so you'd want to chain another converter together.

I think if we could come up with a way of chaining attributes together (right now you can't really because the order isn't maintained) that this could become a single concept that can handle everything, cutting down on confusion.

Maybe if you just introduced an order property to all attributes, you could have something like

[UmbracoProperty(0, "Alias", Recursive = true)]
[DittoValueResolver(1, typeof(FirstItemResolver)]
[DittoValueResolver(2, typeof(MyValueConverter)]
public MyType Property {get; set;}

There is the potential issue of inheritance, but I say we could make attribute none inheritable so that if someone overrides "Property", they have to redefine how to get the value.

I'm not sure on naming convention here, ie, is a Resolver the right word now, but it's more about the idea of having a single strategy for doing multiple things during the conversion.

Matt

[mattbrailsford]

The concept is very much like Flow-based programming, so you use attributes to define single chunks of chain-able functionality

https://github.com/jpaulm/csharpfbp

[mattbrailsford]

Maybe these become "Processors", so each attribute runs a process that can return a value. The first is in the chain is always passed the current IPublishedContent, but the next is passed the result from the first 1 and so on until there are no processors left.

[jamiepollock]

@mattbrailsford I like the idea, initially I was sceptical but I think if we can find a way to resolve (ha!) this confusion between TypeConverters & ValueResolvers. I understand the difference between the two but it seems ValueResolvers are far more mailable in how they can be used.

Not sure how I feel about an index value in constructor though. It should be fairly reasonable to assume first come first serve?

Not sure we'll avoid glossing over the usual TypeConverters vs ValueResolvers conversation though. ;)

[mattbrailsford]

If this worked, then it would essentially do away with resolvers/converters and we'd just have chainable processors as a single concept, thus no more confusion.

I agree I'd rather not have the index value, but the problem is the CLR won't guarantee the order in which it retrieves attributes from a type so there would need to be an explicit declaration of the order.

[jamiepollock]

Agreed this seems like the way to go. As I said on Twitter, ValueResolvers are far more practical as TypeConverters feel too ridged.

[micklaw]

@mattbrailsford @jamiepollock I had a similar thought previously about the need for both and the confusion it brings.

It seems to me like a IDittoBuider and Middleware approach for value converting could be good, much like IAppBuilder. So create a DittoBuilder (Processor?) where you could register (builder.Use || builder.Run) a bunch of AppFunc = Func<DittoContext, Task> (or processor as Matt says), this could then be chained in a similar way that IAppBuider does to then call the next.Invoke(context). Meaning you can manage them async etc too.

Just thinking out loud here as this would obviously be a big change, just think this could a million times more flexible if chaining is a thing that is important to the future of the project. Would also mean all you have a single DittoBuider(or Processor?) attribute which consumes DittoMiddleware which can be chained?

[micklaw]

Func Above got stripped for some reason, on iOS here.

Func<DittoContext, Task>

[micklaw]

http://benfoster.io/blog/how-to-write-owin-middleware-in-5-different-steps

Some good reading on the pattern as to implementation.

[mattbrailsford]

@micklaw interesting approach. I assume that's more a fluent api style of configuration?

I've just done a PR which implemented processors as attributes like I mentioned previously, which seems to work. I've got all the existing tests passing, and changed one that was to do with type converters to prove that chaining would work.

[mattbrailsford]

@micklaw the middleware approach does look cool, I'm just not quite sure how we'd make it work. One of the nice things about the attribute based stuff is that it's right next to the property they effect, so it's very explicit and easy to follow what is going on, where as if the mapping happened elsewhere (possibly just within a map function on the model) it feels a bit more disjointed, and I'm not quite sure how we'd create reusable "processors"?

[micklaw]

Yeah totally, so you could have a DittoProcessorAttribute which by default takes a Type[] 'middleware' (bad naming convention here) types and iterates them in sequence to create the pipeline for conversion. Or, you could create you own class inheriting from it where you can fluently create it, this could potentially be injectable by consumers of it (e.g. InsertType(int Index))?

That pull requests is huuuge, I get the gist of where you're going with it though, nice work dude, and think I'm def thinking along the same sort of lines. One concern for me would be the amount of annotations decorated on my POCOs, might end up a little messy if you have a few on each property, but that's probably more a cosmetic thing though would be interested to a see a performance benchmark against just doing this stuff in in Value resolver.

I'll give knocking up a demo of the DittoBuilder I was talking about with a mocked data set and put up a gist or something.

[mattbrailsford]

The only problem with passing types in attributes is you can't pass configuration values such as those used on the Umbraco property attribute.

[mattbrailsford]

Look forward to seeing a poc though

[micklaw]

Yeah totally, I think there is a trade off here between usability and implementation. The attribute approach doesn't seem very DRY to me, for example if use the same attribute setup a few times then I'm copying and pasting the same code, be a shame if used something 20+ times on a site and had t change code in 20+ places. Where if we create a it fluently with a BuilderAttribute then it can be passed params, plus if this needs to be extened, then you could use methods on the Builder for InsertUseAt(int index, AppFunc), InsertRunAt(int index, AppFunc), Remove(int index) .

Yeah, Ill crack on with a POC and send something on.

[mattbrailsford]

You can still register attributes globally if there is a common conversion that should occur all the time for a given type. But otherwise I can't see how either is less or more dry than the other, but as I say, happy to see the alternative.

[mattbrailsford]

Ooh, wonder if I could use this to infer the order https://msdn.microsoft.com/en-us/library/system.runtime.compilerservices.callerlinenumberattribute(v=vs.110).aspx

[mattbrailsford]

So that works. If I set the order param of the base DittoProcessorAttribute to optional, and use the [CallerLineNumber], it automatically injects the line number of the calling code, which is effectively the order in which you declare them, so in theory, you can do away with an explicit order param.

I've tested it with attributes on the same line, and it does give them different line numbers, so I'm guessing the number is based on the CLR code, not your actual .CS. I guess it's just a question of whether we are happy to rely on this?

[mattbrailsford]

Scratch that, if you use the same Attribute type multiple times on the same line, you get the same line number, so I guess that's not so good.

Good

 [UmbracoPropertyProcessor]
 [DittoProcessor(typeof(MyProcessor))]
 [DittoProcessor(typeof(MyProcessor))]
 public string MyProp { get; set; }

Bad

[UmbracoPropertyProcessor, DittoProcessor(typeof(MyProcessor))], DittoProcessor(typeof(MyProcessor))]
public string MyProp { get; set; }

[mattbrailsford]

Made a small tweak to my PR, you still need to set the order, but I've made it explicit, rather than an attribute constructor param, so you end up doing something like:

[UmbracoPropertyProcessor("Name", Order = 0), 
    DittoProcessor(typeof(MyCustomProcessor2), Order = 1), 
    DittoProcessor(typeof(MyCustomProcessor3), Order = 2)]
public MyCustomModel MyProperty { get; set; }

This prevents people who create their own attributes from having to support an order constructor param in their implementations, and I kinda like how explicit it is, compared to just a number which is unclear as to what it's purpose is if it was a constructor param.

[mattbrailsford]

I think personally I'm pretty happy with this approach. The only other major difference (other than the order property), is just that anything that was a type converter before, would likely require you to use 2 processor attributes, ie the first to process the UmbracoProperty, then the second to do the conversion, but again, it's nice that it's explicit imo:

[UmbracoPropertyProcessor(Order = 0), 
    DittoProcessor(typeof(MyCustomProcessor2), Order = 1)]
public MyCustomModel MyProperty { get; set; }

[mattbrailsford]

Cool example alert :) Imagine needing to have multiple fallbacks and a default value if no value is processed:

[UmbracoPropertyProcessor("Title", Order = 0),
    AltUmbracoPropertyProcessor("Heading", Order = 1),
    AltUmbracoPropertyProcessor("Name", Order = 2),
    DefaultValueProcessor("Default value", Order = 3)]
public string Title { get; set; }

Tada :)

[mattbrailsford]

To help keep things a bit more DRY, if you do end up running a bunch of processors all the time. You could just merge them into a single processor, but I've just created the concept of a DittoMultiProcessorAttribute that lets you define a single ProcessorAttribute which will run a bunch of processors all at once. This way you can just add your MultiProcessorAtribute on your property, and all the attributes declared in it will run, and in the order defined (if you delcare an attribute before / after your multi processor, the multi attributes get injected between the ones the multi processor sits between, so the overall order is maintained / honored).

public class MyMultiProcessorAttribute : DittoMultiProcessorAttribute
{
    public MyMultiProcessorAttribute()
        : base(new[] {
                new DittoProcessorAttribute(typeof(MyCustomProcessor1)), 
                new DittoProcessorAttribute(typeof(MyCustomProcessor2)), 
                new DittoProcessorAttribute(typeof(MyCustomProcessor3))         
        })
    { }
}

public class MyModel
{
    [MyMultiProcessor]
    public string MyProperty { get; set; }
}

See unit tests here: https://github.com/leekelleher/umbraco-ditto/pull/143/files#diff-55

[micklaw]

Thumbs up dude, that's a good solution to me. Does this mean that TypeConverters and ValueResolvers are no more? It would also be good to see some benchmarking on this in comparison to using a single value resolver doing the same task. Nice work man.

[mattbrailsford]

Correct yes. It would just be processors and that's it. Processors can cover both tasks. Their job is just to process the passed in value with the first in the chain being passed the content (though all processors can access the content from the processor context object as well) I'd imagine it's ever so slightly slower, but it should be negligible, but yea, if we are happy with this as an approach, we can do some proper benchmarking.

[mattbrailsford]

As we are in agreement about the need for this, I'm going to close this issue. The discussion of the actual implementation can continue on PR #143

[Nicholas-Westby]

@mattbrailsford Thanks for the multi-processor concept! Works nicely for some extra consolidation:

namespace Wiski.App.Mapping.Processors
{

    // Namespaces.
    using Our.Umbraco.Ditto;
    using System.Collections.Generic;

    /// <summary>
    /// This multi-processor combines the Archetype and DocTypeFactory processors to allow for
    /// both of those processors to be run with this single unified processor rather than
    /// having to specify the other two.
    /// </summary>
    /// <remarks>
    /// This can be used when you have an Umbraco property that is an Archetype that allows for
    /// multiple items and multiple fieldset types (e.g., a widget collection).
    /// </remarks>
    public class DittoMixedArchetypeAttribute : DittoMultiProcessorAttribute
    {

        #region Constructors

        /// <summary>
        /// Default constructor.
        /// </summary>
        public DittoMixedArchetypeAttribute() : base(GetProcessors())
        {
        }

        #endregion

        #region Methods

        /// <summary>
        /// Returns the processors that should be run whenever this multi-processor is used.
        /// </summary>
        /// <returns>
        /// The collection of processors to run.
        /// </returns>
        private static IEnumerable<DittoProcessorAttribute> GetProcessors()
        {
            return new DittoProcessorAttribute[]
            {
                // First, the Archetype processor converts the ArchetypeModel
                // to an IPublishedContent collection.
                new DittoArchetypeAttribute(),
                // Next, the DocType processor converts each item in the previously
                // created collection to a type that is indicated by the alias of the
                // item (since they were created from Archetypes, this alias corresponds
                // to the fieldset alias for each item).
                new DittoDocTypeFactoryAttribute()
            };
        }

        #endregion

    }

}

And here's how I'm using it:

namespace Wiski.App.Mapping.Models.Pages
{

    // Namespaces.
    using Interfaces;
    using Processors;
    using System.Collections.Generic;

    /// <summary>
    /// The typical page.
    /// </summary>
    public class Typical
    {
        [DittoMixedArchetype]
        public IEnumerable<IWidget> MainContent { get; set; }
    }

}

And for anybody who stumbles across this, here's where the DittoArchetypeAttribute is implemented: https://github.com/leekelleher/umbraco-ditto-labs/blob/7469acfbf9fab5f63cda1847aba9761fd978ffa3/src/Our.Umbraco.Ditto.Archetype/ComponentModel/Processors/DittoArchetypeAttribute.cs