Inner recipe overwrite parent variables (#6731)

src/OrchardCore.Modules/OrchardCore.Recipes/Controllers/AdminController.cs

@@ -19,6 +19,7 @@ namespace OrchardCore.Recipes.Controllers
 {
     public class AdminController : Controller
     {
+        private readonly IShellHost _shellHost;
         private readonly ShellSettings _shellSettings;
         private readonly IExtensionManager _extensionManager;
         private readonly IAuthorizationService _authorizationService;
@@ -29,6 +30,7 @@ public class AdminController : Controller
         private readonly IHtmlLocalizer H;
 
         public AdminController(
+            IShellHost shellHost,
             ShellSettings shellSettings,
             ISiteService siteService,
             IExtensionManager extensionManager,
@@ -38,6 +40,7 @@ public AdminController(
             IRecipeExecutor recipeExecutor,
             INotifier notifier)
         {
+            _shellHost = shellHost;
             _shellSettings = shellSettings;
             _siteService = siteService;
             _recipeExecutor = recipeExecutor;
@@ -117,6 +120,8 @@ public async Task<ActionResult> Execute(string basePath, string fileName)
                 _shellSettings.State = TenantState.Running;
             }
 
+            await _shellHost.ReleaseShellContextAsync(_shellSettings);
+
             _notifier.Success(H["The recipe '{0}' has been run successfully", recipe.Name]);
             return RedirectToAction("Index");
         }

src/OrchardCore.Modules/OrchardCore.Recipes/Services/RecipeDeploymentTargetHandler.cs

@@ -3,16 +3,21 @@
 using System.Threading.Tasks;
 using Microsoft.Extensions.FileProviders;
 using OrchardCore.Deployment;
+using OrchardCore.Environment.Shell;
 using OrchardCore.Recipes.Models;
 
 namespace OrchardCore.Recipes.Services
 {
     public class RecipeDeploymentTargetHandler : IDeploymentTargetHandler
     {
+        private readonly IShellHost _shellHost;
+        private readonly ShellSettings _shellSettings;
         private readonly IRecipeExecutor _recipeExecutor;
 
-        public RecipeDeploymentTargetHandler(IRecipeExecutor recipeExecutor)
+        public RecipeDeploymentTargetHandler(IShellHost shellHost, ShellSettings shellSettings, IRecipeExecutor recipeExecutor)
         {
+            _shellHost = shellHost;
+            _shellSettings = shellSettings;
             _recipeExecutor = recipeExecutor;
         }
 
@@ -27,6 +32,8 @@ public async Task ImportFromFileAsync(IFileProvider fileProvider)
             };
 
             await _recipeExecutor.ExecuteAsync(executionId, recipeDescriptor, new object(), CancellationToken.None);
+
+            await _shellHost.ReleaseShellContextAsync(_shellSettings);
         }
     }
 }

src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Scripting/IScriptingManager.cs

@@ -29,6 +29,6 @@ public interface IScriptingManager
         /// The list of available method providers for this <see cref="IScriptingManager"/>
         /// instance.
         /// </summary>
-        IList<IGlobalMethodProvider> GlobalMethodProviders { get; }
+        IReadOnlyList<IGlobalMethodProvider> GlobalMethodProviders { get; }
     }
 }

src/OrchardCore/OrchardCore.Infrastructure/Scripting/DefaultScriptingManager.cs

@@ -14,10 +14,10 @@ public DefaultScriptingManager(
             IEnumerable<IGlobalMethodProvider> globalMethodProviders)
         {
             _engines = engines;
-            GlobalMethodProviders = new List<IGlobalMethodProvider>(globalMethodProviders);
+            GlobalMethodProviders = new List<IGlobalMethodProvider>(globalMethodProviders).AsReadOnly();
         }
 
-        public IList<IGlobalMethodProvider> GlobalMethodProviders { get; }
+        public IReadOnlyList<IGlobalMethodProvider> GlobalMethodProviders { get; }
 
         public object Evaluate(string directive,
             IFileProvider fileProvider,

src/OrchardCore/OrchardCore.Recipes.Abstractions/Events/IRecipeEventHandler.cs

@@ -3,6 +3,11 @@
 
 namespace OrchardCore.Recipes.Events
 {
+    /// <summary>
+    /// Note: The recipe executor creates for each step a scope that may be based on a new shell if the features have changed,
+    /// so an 'IRecipeEventHandler', that is also used in each step scope, can't be a tenant level service, otherwise it may
+    /// be used in a shell container it doesn't belong, so it should be an application level transient or singleton service.
+    /// </summary>
     public interface IRecipeEventHandler
     {
         Task RecipeExecutingAsync(string executionId, RecipeDescriptor descriptor);

src/OrchardCore/OrchardCore.Recipes.Core/ServiceCollectionExtensions.cs

@@ -9,7 +9,7 @@ public static IServiceCollection AddRecipes(this IServiceCollection services)
         {
             services.AddScoped<IRecipeHarvester, ApplicationRecipeHarvester>();
             services.AddScoped<IRecipeHarvester, RecipeHarvester>();
-            services.AddSingleton<IRecipeExecutor, RecipeExecutor>();
+            services.AddTransient<IRecipeExecutor, RecipeExecutor>();
             services.AddScoped<IRecipeMigrator, RecipeMigrator>();
             services.AddScoped<IRecipeReader, RecipeReader>();
 

src/OrchardCore/OrchardCore.Recipes.Core/Services/RecipeExecutor.cs

@@ -19,19 +19,18 @@ namespace OrchardCore.Recipes.Services
 {
     public class RecipeExecutor : IRecipeExecutor
     {
-        private readonly ShellSettings _shellSettings;
         private readonly IShellHost _shellHost;
+        private readonly ShellSettings _shellSettings;
         private readonly IEnumerable<IRecipeEventHandler> _recipeEventHandlers;
         private readonly ILogger _logger;
 
-        private VariablesMethodProvider _variablesMethodProvider;
-        private ConfigurationMethodProvider _configurationMethodProvider;
-        private ParametersMethodProvider _environmentMethodProvider;
+        private readonly Dictionary<string, List<IGlobalMethodProvider>> _methodProviders = new Dictionary<string, List<IGlobalMethodProvider>>();
 
-        public RecipeExecutor(IEnumerable<IRecipeEventHandler> recipeEventHandlers,
-                              ShellSettings shellSettings,
-                              IShellHost shellHost,
-                              ILogger<RecipeExecutor> logger)
+        public RecipeExecutor(
+            IShellHost shellHost,
+            ShellSettings shellSettings,
+            IEnumerable<IRecipeEventHandler> recipeEventHandlers,
+            ILogger<RecipeExecutor> logger)
         {
             _shellHost = shellHost;
             _shellSettings = shellSettings;
@@ -45,86 +44,87 @@ public async Task<string> ExecuteAsync(string executionId, RecipeDescriptor reci
 
             try
             {
-                _environmentMethodProvider = new ParametersMethodProvider(environment);
-                _configurationMethodProvider = new ConfigurationMethodProvider(_shellSettings.ShellConfiguration);
+                var methodProviders = new List<IGlobalMethodProvider>();
+                _methodProviders.Add(executionId, methodProviders);
+
+                methodProviders.Add(new ParametersMethodProvider(environment));
+                methodProviders.Add(new ConfigurationMethodProvider(_shellSettings.ShellConfiguration));
 
                 var result = new RecipeResult { ExecutionId = executionId };
 
                 using (var stream = recipeDescriptor.RecipeFileInfo.CreateReadStream())
                 {
-                    using (var file = new StreamReader(stream))
+                    using var file = new StreamReader(stream);
+                    using var reader = new JsonTextReader(file);
+
+                    // Go to Steps, then iterate.
+                    while (await reader.ReadAsync())
                     {
-                        using (var reader = new JsonTextReader(file))
+                        if (reader.Path == "variables")
+                        {
+                            await reader.ReadAsync();
+
+                            var variables = await JObject.LoadAsync(reader);
+
+                            methodProviders.Add(new VariablesMethodProvider(variables));
+                        }
+
+                        if (reader.Path == "steps" && reader.TokenType == JsonToken.StartArray)
                         {
-                            // Go to Steps, then iterate.
-                            while (await reader.ReadAsync())
+                            while (await reader.ReadAsync() && reader.Depth > 1)
                             {
-                                if (reader.Path == "variables")
+                                if (reader.Depth == 2)
                                 {
-                                    await reader.ReadAsync();
+                                    var child = await JObject.LoadAsync(reader);
 
-                                    var variables = await JObject.LoadAsync(reader);
-                                    _variablesMethodProvider = new VariablesMethodProvider(variables);
-                                }
+                                    var recipeStep = new RecipeExecutionContext
+                                    {
+                                        Name = child.Value<string>("name"),
+                                        Step = child,
+                                        ExecutionId = executionId,
+                                        Environment = environment,
+                                        RecipeDescriptor = recipeDescriptor
+                                    };
+
+                                    if (cancellationToken.IsCancellationRequested)
+                                    {
+                                        _logger.LogError("Recipe interrupted by cancellation token.");
+                                        return null;
+                                    }
 
-                                if (reader.Path == "steps" && reader.TokenType == JsonToken.StartArray)
-                                {
-                                    while (await reader.ReadAsync() && reader.Depth > 1)
+                                    var stepResult = new RecipeStepResult { StepName = recipeStep.Name };
+                                    result.Steps.Add(stepResult);
+
+                                    ExceptionDispatchInfo capturedException = null;
+                                    try
+                                    {
+                                        await ExecuteStepAsync(recipeStep);
+                                        stepResult.IsSuccessful = true;
+                                    }
+                                    catch (Exception e)
+                                    {
+                                        stepResult.IsSuccessful = false;
+                                        stepResult.ErrorMessage = e.ToString();
+
+                                        // Because we can't do some async processing the in catch or finally
+                                        // blocks, we store the exception to throw it later.
+
+                                        capturedException = ExceptionDispatchInfo.Capture(e);
+                                    }
+
+                                    stepResult.IsCompleted = true;
+
+                                    if (stepResult.IsSuccessful == false)
+                                    {
+                                        capturedException.Throw();
+                                    }
+
+                                    if (recipeStep.InnerRecipes != null)
                                     {
-                                        if (reader.Depth == 2)
+                                        foreach (var descriptor in recipeStep.InnerRecipes)
                                         {
-                                            var child = await JObject.LoadAsync(reader);
-
-                                            var recipeStep = new RecipeExecutionContext
-                                            {
-                                                Name = child.Value<string>("name"),
-                                                Step = child,
-                                                ExecutionId = executionId,
-                                                Environment = environment,
-                                                RecipeDescriptor = recipeDescriptor
-                                            };
-
-                                            if (cancellationToken.IsCancellationRequested)
-                                            {
-                                                _logger.LogError("Recipe interrupted by cancellation token.");
-                                                return null;
-                                            }
-
-                                            var stepResult = new RecipeStepResult { StepName = recipeStep.Name };
-                                            result.Steps.Add(stepResult);
-
-                                            ExceptionDispatchInfo capturedException = null;
-                                            try
-                                            {
-                                                await ExecuteStepAsync(recipeStep);
-                                                stepResult.IsSuccessful = true;
-                                            }
-                                            catch (Exception e)
-                                            {
-                                                stepResult.IsSuccessful = false;
-                                                stepResult.ErrorMessage = e.ToString();
-
-                                                // Because we can't do some async processing the in catch or finally
-                                                // blocks, we store the exception to throw it later.
-
-                                                capturedException = ExceptionDispatchInfo.Capture(e);
-                                            }
-
-                                            stepResult.IsCompleted = true;
-
-                                            if (stepResult.IsSuccessful == false)
-                                            {
-                                                capturedException.Throw();
-                                            }
-
-                                            if (recipeStep.InnerRecipes != null)
-                                            {
-                                                foreach (var descriptor in recipeStep.InnerRecipes)
-                                                {
-                                                    var innerExecutionId = Guid.NewGuid().ToString();
-                                                    await ExecuteAsync(innerExecutionId, descriptor, environment, cancellationToken);
-                                                }
-                                            }
+                                            var innerExecutionId = Guid.NewGuid().ToString();
+                                            await ExecuteAsync(innerExecutionId, descriptor, environment, cancellationToken);
                                         }
                                     }
                                 }
@@ -143,6 +143,10 @@ public async Task<string> ExecuteAsync(string executionId, RecipeDescriptor reci
 
                 throw;
             }
+            finally
+            {
+                _methodProviders.Remove(executionId);
+            }
         }
 
         private async Task ExecuteStepAsync(RecipeExecutionContext recipeStep)
@@ -155,47 +159,31 @@ await shellScope.UsingAsync(async scope =>
             {
                 var recipeStepHandlers = scope.ServiceProvider.GetServices<IRecipeStepHandler>();
                 var scriptingManager = scope.ServiceProvider.GetRequiredService<IScriptingManager>();
-                scriptingManager.GlobalMethodProviders.Add(_environmentMethodProvider);
-                scriptingManager.GlobalMethodProviders.Add(_configurationMethodProvider);
 
                 // Substitutes the script elements by their actual values
-                EvaluateScriptNodes(recipeStep, scriptingManager);
+                EvaluateJsonTree(scriptingManager, recipeStep, recipeStep.Step);
 
-                foreach (var recipeStepHandler in recipeStepHandlers)
+                if (_logger.IsEnabled(LogLevel.Information))
                 {
-                    if (_logger.IsEnabled(LogLevel.Information))
-                    {
-                        _logger.LogInformation("Executing recipe step '{RecipeName}'.", recipeStep.Name);
-                    }
+                    _logger.LogInformation("Executing recipe step '{RecipeName}'.", recipeStep.Name);
+                }
 
-                    await _recipeEventHandlers.InvokeAsync((handler, recipeStep) => handler.RecipeStepExecutingAsync(recipeStep), recipeStep, _logger);
+                await _recipeEventHandlers.InvokeAsync((handler, recipeStep) => handler.RecipeStepExecutingAsync(recipeStep), recipeStep, _logger);
 
+                foreach (var recipeStepHandler in recipeStepHandlers)
+                {
                     await recipeStepHandler.ExecuteAsync(recipeStep);
+                }
 
-                    await _recipeEventHandlers.InvokeAsync((handler, recipeStep) => handler.RecipeStepExecutedAsync(recipeStep), recipeStep, _logger);
+                await _recipeEventHandlers.InvokeAsync((handler, recipeStep) => handler.RecipeStepExecutedAsync(recipeStep), recipeStep, _logger);
 
-                    if (_logger.IsEnabled(LogLevel.Information))
-                    {
-                        _logger.LogInformation("Finished executing recipe step '{RecipeName}'.", recipeStep.Name);
-                    }
+                if (_logger.IsEnabled(LogLevel.Information))
+                {
+                    _logger.LogInformation("Finished executing recipe step '{RecipeName}'.", recipeStep.Name);
                 }
             });
         }
 
-        /// <summary>
-        /// Traverse all the nodes of the recipe steps and replaces their value if they are scripted.
-        /// </summary>
-        private void EvaluateScriptNodes(RecipeExecutionContext context, IScriptingManager scriptingManager)
-        {
-            if (_variablesMethodProvider != null)
-            {
-                _variablesMethodProvider.ScriptingManager = scriptingManager;
-                scriptingManager.GlobalMethodProviders.Add(_variablesMethodProvider);
-            }
-
-            EvaluateJsonTree(scriptingManager, context, context.Step);
-        }
-
         /// <summary>
         /// Traverse all the nodes of the json document and replaces their value if they are scripted.
         /// </summary>
@@ -225,7 +213,14 @@ private void EvaluateJsonTree(IScriptingManager scriptingManager, RecipeExecutio
                     while (value.StartsWith('[') && value.EndsWith(']'))
                     {
                         value = value.Trim('[', ']');
-                        value = (scriptingManager.Evaluate(value, context.RecipeDescriptor.FileProvider, context.RecipeDescriptor.BasePath, null) ?? "").ToString();
+
+                        value = (scriptingManager.Evaluate(
+                            value,
+                            context.RecipeDescriptor.FileProvider,
+                            context.RecipeDescriptor.BasePath,
+                            _methodProviders[context.ExecutionId])
+                            ?? "").ToString();
+
                         ((JValue)node).Value = value;
                     }
                     break;

src/OrchardCore/OrchardCore.Recipes.Core/VariablesMethodProvider.cs

@@ -1,6 +1,8 @@
 using System;
 using System.Collections.Generic;
+using Microsoft.Extensions.DependencyInjection;
 using Newtonsoft.Json.Linq;
+using OrchardCore.Environment.Shell.Scope;
 using OrchardCore.Scripting;
 
 namespace OrchardCore.Recipes
@@ -31,7 +33,7 @@ public VariablesMethodProvider(JObject variables)
             };
         }
 
-        public IScriptingManager ScriptingManager { get; set; }
+        public IScriptingManager ScriptingManager => ShellScope.Services.GetRequiredService<IScriptingManager>();
 
         public IEnumerable<GlobalMethod> GetMethods()
         {