Allow defining a typed context to be used throughout a Cake script.

[patriksvensson]

Allow defining a typed context data class to be used throughout a Cake script.

Example of usage:

Declare a context in your Cake script like this:

public class MyData
{
    public string MyProperty { get; set; }
}

And now you can use it like this:

#load "./MyData.cake"

/////////////////////////////////////////////////////
// SETUP / TEARDOWN
/////////////////////////////////////////////////////

Setup<MyData>(context, data => 
{
    return new MyData {
        MyProperty = "Foo"
    }
});

/////////////////////////////////////////////////////
// TASKS
/////////////////////////////////////////////////////

Task("First")
    .Does<MyData>((context, data) => 
{
    Information("MyProperty = {0}", data.MyProperty);
});

Task("Second")
    .IsDependentOn("First")
    .Does(context => 
{
    // We can add an alias to make it easier to get data.
    Information("MyProperty = {0}", 
        context.Data.Get<MyData>().MyProperty);
});

/////////////////////////////////////////////////////
// EXECUTE
/////////////////////////////////////////////////////

RunTarget("Second");

This would remove dependencies on global variables and shared mutable states that make larger Cake script difficult to maintain in some scenarios. Declaring a Setup<TContextData> should be required to use the context data in the script. If no Setup<TContextData> has been declared, an error should be thrown when the script compiles.

With this approach we could even allow multiple setup/teardown methods for different types of context data, but that is something for later.

I'm going to write a quick proof-of-concept for this, and we can discuss this more after support for .NET Standard 2.0 has been released.

[kcamp]

I like this idea. I may be missing context, but just reading over the code real quick, I have one comment.

For this, it appears that the delegate type is Action<ISomeCakeContextType<TModel>> where ISomeCakeContextType<TModel> has TModel Data { get; [set;]} , which implies that TModel : new()

Setup<MyData>(context => 
{
    context.Data.MyProperty = "lol";
});

Assuming that the definition of ISomeCakeContextType<TModel> supports a mutable Data property, then the implication of TModel : new() may not be a valid conclusion or concern, depending on implementation(s).

At any rate, there are several categories of types that I would find useful to manage in this fashion, but they have parameterized construction, so it would be useful, if even by providing overloads, to have a method like this:

Setup<TData>(Func<TData> builder);

To facilitate something like this:

Setup<MyData>(() => new MyData(arg1, arg2, arg3); );

[jnm2]

While you're looking into making changes, can you see about making the PerformSetup change I asked about in #1772?

[patriksvensson]

@kcamp I've taken your suggestions into consideration and updated the description of this issue and the PR.

@jnm2 Sure, it sounds like a good idea.

[kcamp]

@patriksvensson great, thanks.

If I am reading the code right (pretty sure I am), the intention is:

1. CakeEngine has 1 and only 1 ICakeDataContextRepository instance
2. The ICakeDataContextRepository instance accepts 1 and only 1 <TData> assignment, and that TData : ICakeDataContext.

So, effectively, the marker interface is the derived requirement of what is intended to unify 1..N values or references under a single context.

So what you have intended for an overall real-world use-case is more or less this, I suspect. (Let me know if this is way off!)

class MyBuildDataContext : ICakeDataContext
{
    public BuildParameters Parameters { get; set; }
    public PackageAnalyzer PackageAnalyzer { get; set; } 
    public MySomeOtherThing Thing {get; set; }
    ...
}

Setup<MyBuildDataContext>(context, data => 
{
    return new MyBuildDataContext 
    {
        Parameters = new BuildParameters(context);
        PackageAnalyzer = new PackageAnalyzer(context, new Dependency( ... ),
        .....
     }
});

The only thought I have is that I didn't fully grasp that this was going to hinge on the same RegisterSetupAction implementation; i had a mental model of a bit more granularity in terms of setting up contexts - something like:

Setup<BuildParameters>( context => new BuildParameters(context); )
Setup<PackageAnalyzer>( context => new PackageAnalyzer(context, ....); }

I still like that as an idea because it would eliminate the need to create additional aggregating types (to implement ICakeDataContext) across different build configurations (we have many).
That said, I don't think that that notion (of multiple Setup invocations) plays nicely with things like the CakeEngine.PerformSetup method, where it assumes a 1..1 cardinality of setup tasks, without creating an aggregate Action<ICakeContext>, which may or may not be desirable.

PublishEvent(Setup, new SetupEventArgs(context));
if (_setupAction != null)
{
    strategy.PerformSetup(_setupAction, context);
}

In summary -

1. Your changes address my concern nicely. Thanks 👍
2. If there was a way to support this with a bit more granularity, it might be useful, but certainly isn't a must-have in any way.

[patriksvensson]

@kcamp

What you're describing will be possible (multiple setup/teardown for different typed contexts), it's internals have just not been implemented yet. This is just a proof of concept and I wanted to have a discussion about it before writing more code.

My thought about this is that this should be supported:

#load "./MyData.cake"

/////////////////////////////////////////////////////
// SETUP / TEARDOWN
/////////////////////////////////////////////////////

Setup<MyData>(context, data => 
{
    return new MyData {
        MyProperty = "Foo"
    }
});

Setup<MyOtherData>(context, data => 
{
    return new MyOtherData {
        MyOtherProperty = "Foo"
    }
});

Teardown<MyOtherData>(context, data => 
{
    // Do some teardown
});

/////////////////////////////////////////////////////
// TASKS
/////////////////////////////////////////////////////

Task("First", 
    .Does<MyData>((context, data) => 
{
    Information("MyProperty = {0}", data.MyProperty);
});

Task("Second", 
    .IsDependentOn("First")
    .Does<MyOtherData>((context, data) => 
{
    Information("MyOtherProperty = {0}", data.MyProperty);
});

Task("Third")
    .IsDependentOn("Second")
    .Does(context => 
{
    // We can add an alias to make it easier to get data.
    Information("MyProperty = {0}", context.Data.Get<MyData>().MyProperty);
    Information("MyOtherProperty = {0}", context.Data.Get<MyOtherData>().MyOtherProperty);
});

/////////////////////////////////////////////////////
// EXECUTE
/////////////////////////////////////////////////////

RunTarget("Second");

I'm also thinking about adding automatic disposal of contexts implementing IDisposable (if no teardown method have been implemented for the context type).

[kcamp]

@patriksvensson Got it, thanks for the explanation. Agree with you on all points as far as your use-case as described above. I don't think it would hurt anything to handle IDisposable types automatically.

As a final point of clarification, your types MyData and MyOtherData implement ICakeDataContext -- That gives me pause, especially since it's just an empty marker interface. Since I'd have to backtrack through *.cake source and re-publish nugets, the data-context would have limited utility (for me) given that constraint. Could that constraint be dropped?

[asbjornu]

I was just asking for this feature on Gitter. I would love to have something like this. 👍

[patriksvensson]

@asbjornu I've created a PR for it here: #2016