Inner recipe overwrite parent variables

[jtkech]

Fixes #6717 and refactoring

• Fixes the related issue by holding in a dictionary additional method providers per recipe executionId. @deanmarcussen i made the recipe executor transient, not to fix the issue but to be thread safe even if we use here a simple dictionary.

• Here the singularity is that the recipe executor creates a new scope on each step that may be based on a new shell / container, e.g. after updating the features, so resolving a service in a given step scope, even it is a tenant singleton, may return another instance (including the recipe executor itself if you try it).

  That's why we need to resolve the script manager in each step scope, and we need that the recipe executor only injects e.g. app level transients / singletons as the shell host, that's okay for the shell settings that stays the same as a recipe is not intended to reload it, only externally after its execution e.g. at the end of a setup.

• But i'm not sure that the injection and the usage of the IRecipeEventHandler are good, they would need to be app level transients / singletons. Maybe at least tenant level transients / singletons, and then okay for the event at the beginning but they would need to be part of the setup features. But at the end of a recipe, maybe better to be resolved in a new scope that may be based on a new shell, as we do at the end of a setup for the ISetupEventHandler.

  They also have step events called for each step, here maybe better to resolve them in each step scope. Notice that here they were called each time we call a scoped IRecipeStepHandler, fixed here by calling them outside the related loop. So i'm tempted to remove them, we have no concrete implementations and i think there are not used or very rarely.

• In the recipes admin controller, we now release the tenant after a recipe execution, e.g if they have changed some site settings that needs to release the tenant so that it will be rebuilt. If features have changed it is automatically done through the shell descriptor manager, at the end of a setup it is done somewhere when updating shell settings.

Need more thinkings about the need / usage of IRecipeEventHandler (not IRecipeStepHandler), but @deanmarcussen as it is already done, when you will have time can you try it to check if it fixes the usage of your inner recipe.

Update: That said there is still an unit test that is failing, will see tomorrow

[deanmarcussen]

@jtkech tried, and it fixes the inner recipe issue.

Was thinking about thread safety as well when I suggested a transient here, but I see the dilemma with the IRecipeEventHandler

Need more thinking :)

[jtkech]

@jtkech tried, and it fixes the inner recipe issue.

Okay cool

I fixed an unit test that was expecting an exception that was no more thrown because i was using our InvokeAsync() extension in place of a foreach loop.

I kept the IRecipeEventHandler as is to not break anything, but knowing that, as they are used, they would need to be e.g. app level singletons, not tenant level singletons, otherwise they may be used in a shell container they don't belong.

[deanmarcussen]

I kept the IRecipeEventHandler as is to not break anything, but knowing that, as they are used, they would need to be e.g. app level singletons, not tenant level singletons, otherwise they may be used in a shell container they don't belong.

Cool, probably for the best, someone will be using them.

How about a comment on their interface that they must be registered as app level singletons?

(if not for someone else, just for us, to remember / know next time)