Support for transient lifetimes and dependency injection in the Dialogs API

[drub0y]

Is your feature request related to a problem? Please describe.
The design of the Dialogs API is such that the developer constructs a DialogSet by adding instances of Dialog subclasses to it when the DialogSet is created which means the developer has to create those Dialog instances themselves. While these instances could be resolve from a DI container, thus causing their dependencies to be resolved from the container as well automatically, none of the samples do this. Additionally this essentially leads to realizing an entire, potentially deeply nested dependency graph during DialogSet creation time. Any non-trivial conversation structure is going to contain several dialogs and those dialogs each may require several dependencies which themselves may require dependencies.

.NET developers are more dependent on DI these days than ever with the advent of .NET Core and, more specifically, ASP.NET intrinsically being built around DI patterns. They expect to be able to use these patterns in other modern .NET frameworks... especially ones that integration with ASP.NET.

Describe the solution you'd like
The Dialogs API should be reworked to describe the types of Dialogs rather than instances and then use a factory abstraction to resolve Dialog instances in a just-in-time, transient manner when they are needed. A basic implementation might just creates instances via a default constructor, but then a richer implementation could be created to resolve the instances from a DI container (e.g. IServiceProvider) which would enable transient instances whose entire dependency chain would be automatically resolved for a given turn.

Additional context
I intend to submit a full DCR around this and have begun work on prototyping the implementation in the drmarsh/dcr/better-dialogs-integration branch with great success including full backwards compatibility with the existing static APIs (e.g. DialogSet).

[johnataylor]

Perhaps @Stevenic can comment further.

In many ways what you have here with the Dialog stack is a cross language (but very simple) reflection system. Specifically, the Dialog stack is really just naming pieces of implementation code. Whether you take these names as the class name or a user provided string doesn't change the architecture. But have these explicit strings does make the design language independent. A nice pattern in C# is to use the nameof(MyClass) operator for the Dialog name as you add it to the DialogSet.

Regarding all the construction at runtime, the pattern that works more effectively is to use ComponentDialogs. For example, write a RootDialog class derived from ComponentDialog and then inside that class build your dialog. The point is each ComponentDialog brings its own DialogSet.

Having done that you should be able to create a singleton and then dependency inject that, if that's the model you like to use.

Of course, in your IBot or Controller instance you will need to instantiate a minimal stub DialogSet and do the boilerplate Continue/Begin dance but that isn't much runtime from a construction point of view because you only have a single item in the top level DialogSet. I'm sure this could be optimized further but it's much less of a problem when you factor your code like this.

There is certainly a real problem here in that this boilerplate code is repeated everywhere but the solution that has been discussed there is a "Run" method that can encapsulate all this. However, the fisrt step towards this was simply to make everything consistent and then iterate from there.

[drub0y]

In many ways what you have here with the Dialog stack is a cross language (but very simple) reflection system. Specifically, the Dialog stack is really just naming pieces of implementation code. Whether you take these names as the class name or a user provided string doesn't change the architecture. But have these explicit strings does make the design language independent. A nice pattern in C# is to use the nameof(MyClass) operator for the Dialog name as you add it to the DialogSet.

This seems slightly off topic. Yes, the indirection of using ids is great... I shouldn't (necessarily) know the implementation of the "airport picker" dialog. Though, technically, there's a small catch there too because there is a minimal contract around the result type of a dialog (e.g. wanting an airport code as a string vs an Airport instance), but I digress.

Regarding all the construction at runtime, the pattern that works more effectively is to use ComponentDialogs. For example, write a RootDialog class derived from ComponentDialog and then inside that class build your dialog. The point is each ComponentDialog brings its own DialogSet.

Sure, I'm aware of ComponentDialog (frankly I love the concept), but I don't think it addresses the details I'm raising in this issue. In fact, it introduces its own set of unique issues. More on this below.

Having done that you should be able to create a singleton and then dependency inject that, if that's the model you like to use.

The issue is partly about having transient lifetimes for Dialog instances, just like controllers in ASP.NET. I know I can have singletons, but that causes the realization of an entire dependency graph which, by virtue of the root service (be it DialogSet or ComponentDialog) being a singleton, themselves become singletons. Singletons are not the answer to this issue.

Of course, in your IBot or Controller instance you will need to instantiate a minimal stub DialogSet and do the boilerplate Continue/Begin dance but that isn't much runtime from a construction point of view because you only have a single item in the top level DialogSet. I'm sure this could be optimized further but it's much less of a problem when you factor your code like this.

First, I don't want to "instantiate" anything. My dependencies should be passed in via the constructor just like an ASP.NET controller. All of the samples seem to be "instantiating" DialogSets in their constructors and filling them out by instantiating specific Dialogs and adding them with Add[Dialog]. This should be considered an anti-pattern. This will be a performance nightmare for any non-trivial dialog tree.

Let's walk through this idea that you'd instantiate a "minimal stub" DialogSet that may contain a single ComponentDialog subclass (e.g. the RootDialog you mentioned).

Here's what the bot might look like:

public class MyBot : IBot
{
    private readonly DialogSet _myDialogs;

    public MyBot(IStatePropertyAccessor<DialogState> dialogStatePropertyAccessor)
    {
        _myDialogs = new DialogSet(dialogStatePropertyAccessor)
            .Add(new RootDialog("root"));
    }

    // OnTurn impl elided for brevity
}

Looks "minimal", seems lightweight... 2 objects being new'd up, no biggy. Except that's not exactly the case. When you instantiate RootDialog, which is a ComponentDialog, its constructor is going to do something like this:

public class RootDialog : ComponentDialog
{
    public RootDialog(string dialogId) : base(dialogId)
    {
        AddDialog(new WaterfallDialog("mainMenu", CreateMainMenuWaterfall()));
        AddDialog(new AirportPickerDialog("airport"));
        // probably a bunch more here
    }

    // CreateMainMenuWaterfall elided for brevity, but obviously would create the steps
}

Hmm, ok so now we've got 2 more Dialog instantiations here, as well as that whole waterfall creation, but... wait, what's that AirportPickerDialog? Oh that's another ComponentDialog subclass that I brought in from a NuGet package and happens to have 10 more dialogs inside of it and the pattern repeats itself until the dialog tree is fully realized.

If my breakdown of the scenario above is accurate, there is no scenario where instantiating a DialogSet does not result in the entire dialog tree and all of its dependencies being fully realized.

Thus far, this has only addressed Dialogs themselves. What if my AirportPickerDialog contains a sub-dialog that needs an IAirportListingService dependency? Is the proposal that someone who writes a ComponentDialog would need their constructor to take all dependencies for all sub-dialogs so that I can plumb them through? In this case I'd have to change my RootDialog to take the IAirportListingService and pass it to the new AirportPickerDialog(...) which would then have to pass it to the subdialog that needed it. Imagine if the AirportPickerDialog itself used another ComponentDialog that needed another N services? Extrapolate this out even just a little bit and I don't see how anyone could argue that this won't become completely unmanageable very quickly.

As mentioned, I have already taken the steps towards providing a solution to the problem in this working branch and have had great success, including keeping 100% backwards compatibility with the existing API. But, before we go over the details of that solution, let’s get agreement on the problem.

[johnataylor]

I was suggesting creating a singleton RootDialog object and injecting that in the constructor of the IBot instance? So that's slightly different than the code you outlined.

Also something to keep in mind is that there is a requirement to have exactly the same core model (and that would include dialogs) in every implementation language.

[hansmbakker]

I believe the issue of not having DI in place does not only concern Dialogs, but also the access to service clients.

Imagine I have a service client class that sends a webhook to Teams. I don't want to instantiate a new object of it every time. It would be much more convenient and clean if I could register that once and then use it afterwards.

[hansmbakker]

Besides, Singleton-style objects have three risks:

• they increase the risk of (memory) leaks because they will never be garbage collected
• one should be careful with stateful vs stateless
• one should be careful with multithreading

Of course a programmer is still responsible for writing good code without leaks, but having no proper dependency injection as a fallback is a lack.

This is another reason why having proper dependency injection with proper lifetime scoping is a best practice and a request for bot framework.

References:

• https://medium.com/volosoft/asp-net-core-dependency-injection-best-practices-tips-tricks-c6e9c67f9d96
• https://docs.microsoft.com/en-us/aspnet/core/fundamentals/dependency-injection?view=aspnetcore-2.2#design-services-for-dependency-injection

[johnataylor]

Creating a root component dialog effectively addresses this issue, for example this pattern is used effectively in sample 42.

The other point that sample illustrates is that the dialog stack can effectively be contained and encapsulated in a single and relatively simple function call. Basically along the lines of:

        (newState, activities) Process(oldState, activity)

This clean functional approach is a big step towards making the whole framework more friendly to DI in particular and more modular in general. The current plan is to work towards this incrementally.

Agreed this is not specifically an issue about dialogs.

[hansmbakker]

The current plan is to work towards this incrementally.

@johnataylor Could you be a bit more concrete on the steps that are taken towards this?

[hansmbakker]

People are asking for it in https://github.com/Microsoft/AI/issues/579, too.

[hansmbakker]

@johnataylor @cleemullins is there any update on this?

In my opinion this issue is closed too early as it is not solved and a clear path towards a solution is not given.

[mitkodi]

IMHO, I think that the main problem is that the dialogs are stuck with the current implementation of DialogSet and would be better if Dialogs expect not a DialogSet but just an interface with single method (#1409):

Task<Dialog> FindDialogAsync(string id)

and leave us to decide how to implement it (current DialogSet could be just one possible implementation, but not the only one).