From 3c9445ff3c15909410395ed9309b67f8aceff8cf Mon Sep 17 00:00:00 2001 From: DanielBMann9000 Date: Thu, 28 Apr 2016 19:28:50 -0400 Subject: [PATCH] Fix for errors in generated PowerShell scripts (#21) * Fixing issue where calls to powershell scripts aren't being translated properly. Fixes issue #16 * Script generation when the action to run isn't an extracted tool has been changed to only include the command and the arguments. * Corrected unit test that was passing when it should be failing; added a new minimal repro test case for the scenario. Issue #20 * Variables should be correctly remapped when variable name + value occurs across multiple actions. Fixes issue #20 --- .../Model/ScriptAction.cs | 4 +- .../ScriptGenerator.cs | 8 +- .../Templates/IndividualActionTemplate.cs | 99 ++++++++++++++++--- .../Templates/IndividualActionTemplate.tt | 18 +++- .../UniquePropertyResolver.cs | 52 ++++++---- ...When_Resolving_Unique_Script_Parameters.cs | 60 ++++++++++- 6 files changed, 205 insertions(+), 36 deletions(-) diff --git a/src/RMWorkflowMigrator.Generator.PowerShell/Model/ScriptAction.cs b/src/RMWorkflowMigrator.Generator.PowerShell/Model/ScriptAction.cs index 9b630a1..b287f22 100644 --- a/src/RMWorkflowMigrator.Generator.PowerShell/Model/ScriptAction.cs +++ b/src/RMWorkflowMigrator.Generator.PowerShell/Model/ScriptAction.cs @@ -31,6 +31,8 @@ public class ScriptAction public int Sequence { get; set; } - public Dictionary> RollbackScripts { get; set; } = new Dictionary>(); + public Dictionary> RollbackScripts { get; set; } = new Dictionary>(); + + public bool CommandIsExtractedTool { get; set; } } } diff --git a/src/RMWorkflowMigrator.Generator.PowerShell/ScriptGenerator.cs b/src/RMWorkflowMigrator.Generator.PowerShell/ScriptGenerator.cs index f227e1a..cb0ce53 100644 --- a/src/RMWorkflowMigrator.Generator.PowerShell/ScriptGenerator.cs +++ b/src/RMWorkflowMigrator.Generator.PowerShell/ScriptGenerator.cs @@ -35,6 +35,7 @@ public class ScriptGenerator private readonly string meaninglessDisplayNameFolderFormat = "{0}_{1}"; private readonly string releaseScriptName = "ReleaseScript.ps1"; + private readonly string[] possibleToolExtensions = { ".bat", ".exe", ".com", ".cmd", string.Empty }; private readonly IRMComponentRepository componentRepo; @@ -327,7 +328,11 @@ stuff extracted from the RM XAML workflow and stuff we pull out of the RM databa { var component = await this.componentRepo.GetComponentByIdAsync(action.WorkflowActivityId, stageId); await this.deployerToolRepo.WriteToolToDiskAsync(component.DeployerToolId, this.deployerToolsPath); - scriptActionElements.Add(CreateScriptAction(component, action)); + var scriptAction = CreateScriptAction(component, action); + + scriptAction.CommandIsExtractedTool = possibleToolExtensions.Select(extension => this.fs.Exists(Path.Combine(this.deployerToolsPath, scriptAction.Command + extension))).Any(t=>t==true); + + scriptActionElements.Add(scriptAction); } this.ScriptGenerationNotification?.Invoke(this, GetActionGenerationArgs(action, ActionEnd)); @@ -458,6 +463,7 @@ private async Task> ResolveRo await this.deployerToolRepo.WriteToolToDiskAsync(component.DeployerToolId, this.deployerToolsPath); var action = CreateScriptAction(component, rollbackAction); action.Sequence += rollbackGroup.Item.Sequence; + action.CommandIsExtractedTool = possibleToolExtensions.Select(extension => this.fs.Exists(Path.Combine(this.deployerToolsPath, action.Command + extension))).Any(t => t == true); rollbackScriptActions.Add(action); } else diff --git a/src/RMWorkflowMigrator.Generator.PowerShell/Templates/IndividualActionTemplate.cs b/src/RMWorkflowMigrator.Generator.PowerShell/Templates/IndividualActionTemplate.cs index 71c47f0..8e3c89a 100644 --- a/src/RMWorkflowMigrator.Generator.PowerShell/Templates/IndividualActionTemplate.cs +++ b/src/RMWorkflowMigrator.Generator.PowerShell/Templates/IndividualActionTemplate.cs @@ -265,91 +265,162 @@ public virtual string TransformText() #line default #line hidden - this.Write(" &\"$(join-path $DeployerToolsPath \""); #line 53 "C:\Users\Administrator\Documents\GitHubVisualStudio\Migrate-assets-from-RM-server-to-VSTS\src\RMWorkflowMigrator.Generator.PowerShell\Templates\IndividualActionTemplate.tt" +if (action.Command.StartsWith("powershell")) { + if (action.Arguments.StartsWith(@"""&") || action.Arguments.StartsWith(@"-command ""&") || action.Arguments.StartsWith(@"-Command ""&")) { + + #line default + #line hidden + this.Write("\tpowershell "); + + #line 55 "C:\Users\Administrator\Documents\GitHubVisualStudio\Migrate-assets-from-RM-server-to-VSTS\src\RMWorkflowMigrator.Generator.PowerShell\Templates\IndividualActionTemplate.tt" + this.Write(this.ToStringHelper.ToStringWithCulture(action.Arguments)); + + #line default + #line hidden + this.Write("\r\n"); + + #line 56 "C:\Users\Administrator\Documents\GitHubVisualStudio\Migrate-assets-from-RM-server-to-VSTS\src\RMWorkflowMigrator.Generator.PowerShell\Templates\IndividualActionTemplate.tt" +} + else { + var ps1Index = action.Arguments.IndexOf(".ps1")+4; + var actualCommand = action.Arguments.Substring(0,ps1Index); + var strippedArguments = action.Arguments.Replace(actualCommand, string.Empty); + actualCommand = actualCommand.Replace("-command", string.Empty).Replace("-Command", string.Empty).Trim(); + + #line default + #line hidden + this.Write("\tInvoke-Expression \"& `\"$(join-path $DeployerToolsPath \'"); + + #line 62 "C:\Users\Administrator\Documents\GitHubVisualStudio\Migrate-assets-from-RM-server-to-VSTS\src\RMWorkflowMigrator.Generator.PowerShell\Templates\IndividualActionTemplate.tt" + this.Write(this.ToStringHelper.ToStringWithCulture(actualCommand)); + + #line default + #line hidden + this.Write("\')`\" "); + + #line 62 "C:\Users\Administrator\Documents\GitHubVisualStudio\Migrate-assets-from-RM-server-to-VSTS\src\RMWorkflowMigrator.Generator.PowerShell\Templates\IndividualActionTemplate.tt" + this.Write(this.ToStringHelper.ToStringWithCulture(strippedArguments)); + + #line default + #line hidden + this.Write("\"\r\n"); + + #line 63 "C:\Users\Administrator\Documents\GitHubVisualStudio\Migrate-assets-from-RM-server-to-VSTS\src\RMWorkflowMigrator.Generator.PowerShell\Templates\IndividualActionTemplate.tt" +} +} +else if (!action.CommandIsExtractedTool) { + + #line default + #line hidden + this.Write(" "); + + #line 66 "C:\Users\Administrator\Documents\GitHubVisualStudio\Migrate-assets-from-RM-server-to-VSTS\src\RMWorkflowMigrator.Generator.PowerShell\Templates\IndividualActionTemplate.tt" this.Write(this.ToStringHelper.ToStringWithCulture(action.Command)); #line default #line hidden - this.Write("\")\" "); + this.Write(" "); - #line 53 "C:\Users\Administrator\Documents\GitHubVisualStudio\Migrate-assets-from-RM-server-to-VSTS\src\RMWorkflowMigrator.Generator.PowerShell\Templates\IndividualActionTemplate.tt" + #line 66 "C:\Users\Administrator\Documents\GitHubVisualStudio\Migrate-assets-from-RM-server-to-VSTS\src\RMWorkflowMigrator.Generator.PowerShell\Templates\IndividualActionTemplate.tt" this.Write(this.ToStringHelper.ToStringWithCulture(action.Arguments)); #line default #line hidden this.Write("\r\n"); - #line 54 "C:\Users\Administrator\Documents\GitHubVisualStudio\Migrate-assets-from-RM-server-to-VSTS\src\RMWorkflowMigrator.Generator.PowerShell\Templates\IndividualActionTemplate.tt" + #line 67 "C:\Users\Administrator\Documents\GitHubVisualStudio\Migrate-assets-from-RM-server-to-VSTS\src\RMWorkflowMigrator.Generator.PowerShell\Templates\IndividualActionTemplate.tt" } +else { + + #line default + #line hidden + this.Write(" &\"$(join-path $DeployerToolsPath \""); + + #line 69 "C:\Users\Administrator\Documents\GitHubVisualStudio\Migrate-assets-from-RM-server-to-VSTS\src\RMWorkflowMigrator.Generator.PowerShell\Templates\IndividualActionTemplate.tt" + this.Write(this.ToStringHelper.ToStringWithCulture(action.Command)); + + #line default + #line hidden + this.Write("\")\" "); + + #line 69 "C:\Users\Administrator\Documents\GitHubVisualStudio\Migrate-assets-from-RM-server-to-VSTS\src\RMWorkflowMigrator.Generator.PowerShell\Templates\IndividualActionTemplate.tt" + this.Write(this.ToStringHelper.ToStringWithCulture(action.Arguments)); + + #line default + #line hidden + this.Write("\r\n"); + + #line 70 "C:\Users\Administrator\Documents\GitHubVisualStudio\Migrate-assets-from-RM-server-to-VSTS\src\RMWorkflowMigrator.Generator.PowerShell\Templates\IndividualActionTemplate.tt" +}} #line default #line hidden this.Write("}\r\ncatch {\r\n Write-output \""); - #line 57 "C:\Users\Administrator\Documents\GitHubVisualStudio\Migrate-assets-from-RM-server-to-VSTS\src\RMWorkflowMigrator.Generator.PowerShell\Templates\IndividualActionTemplate.tt" + #line 73 "C:\Users\Administrator\Documents\GitHubVisualStudio\Migrate-assets-from-RM-server-to-VSTS\src\RMWorkflowMigrator.Generator.PowerShell\Templates\IndividualActionTemplate.tt" this.Write(this.ToStringHelper.ToStringWithCulture(action.DisplayName)); #line default #line hidden this.Write(" failed. Error:\"\r\n Write-output $_\r\n"); - #line 59 "C:\Users\Administrator\Documents\GitHubVisualStudio\Migrate-assets-from-RM-server-to-VSTS\src\RMWorkflowMigrator.Generator.PowerShell\Templates\IndividualActionTemplate.tt" + #line 75 "C:\Users\Administrator\Documents\GitHubVisualStudio\Migrate-assets-from-RM-server-to-VSTS\src\RMWorkflowMigrator.Generator.PowerShell\Templates\IndividualActionTemplate.tt" if (action.RollbackScripts.Any()) { #line default #line hidden this.Write(" Write-output \"Executing rollback script(s)\"\r\n"); - #line 61 "C:\Users\Administrator\Documents\GitHubVisualStudio\Migrate-assets-from-RM-server-to-VSTS\src\RMWorkflowMigrator.Generator.PowerShell\Templates\IndividualActionTemplate.tt" + #line 77 "C:\Users\Administrator\Documents\GitHubVisualStudio\Migrate-assets-from-RM-server-to-VSTS\src\RMWorkflowMigrator.Generator.PowerShell\Templates\IndividualActionTemplate.tt" if (action.IsComponent) { #line default #line hidden this.Write(" cd $basePath\r\n"); - #line 63 "C:\Users\Administrator\Documents\GitHubVisualStudio\Migrate-assets-from-RM-server-to-VSTS\src\RMWorkflowMigrator.Generator.PowerShell\Templates\IndividualActionTemplate.tt" + #line 79 "C:\Users\Administrator\Documents\GitHubVisualStudio\Migrate-assets-from-RM-server-to-VSTS\src\RMWorkflowMigrator.Generator.PowerShell\Templates\IndividualActionTemplate.tt" } #line default #line hidden - #line 64 "C:\Users\Administrator\Documents\GitHubVisualStudio\Migrate-assets-from-RM-server-to-VSTS\src\RMWorkflowMigrator.Generator.PowerShell\Templates\IndividualActionTemplate.tt" + #line 80 "C:\Users\Administrator\Documents\GitHubVisualStudio\Migrate-assets-from-RM-server-to-VSTS\src\RMWorkflowMigrator.Generator.PowerShell\Templates\IndividualActionTemplate.tt" foreach (var script in action.RollbackScripts) { #line default #line hidden this.Write(" .\\"); - #line 65 "C:\Users\Administrator\Documents\GitHubVisualStudio\Migrate-assets-from-RM-server-to-VSTS\src\RMWorkflowMigrator.Generator.PowerShell\Templates\IndividualActionTemplate.tt" + #line 81 "C:\Users\Administrator\Documents\GitHubVisualStudio\Migrate-assets-from-RM-server-to-VSTS\src\RMWorkflowMigrator.Generator.PowerShell\Templates\IndividualActionTemplate.tt" this.Write(this.ToStringHelper.ToStringWithCulture(script.Key)); #line default #line hidden this.Write(".ps1 "); - #line 65 "C:\Users\Administrator\Documents\GitHubVisualStudio\Migrate-assets-from-RM-server-to-VSTS\src\RMWorkflowMigrator.Generator.PowerShell\Templates\IndividualActionTemplate.tt" + #line 81 "C:\Users\Administrator\Documents\GitHubVisualStudio\Migrate-assets-from-RM-server-to-VSTS\src\RMWorkflowMigrator.Generator.PowerShell\Templates\IndividualActionTemplate.tt" this.Write(this.ToStringHelper.ToStringWithCulture(string.Join(" ", script.Value.OrderBy(rsa => rsa.Sequence).SelectMany(rsa => rsa.ConfigurationVariables).Where(s => !string.IsNullOrWhiteSpace(s.Value) && s.IsParameter).Distinct(new ConfigurationVariableEqualityComparer()).Select(s => "-" + s.RemappedName + " \"$" + s.RemappedName + "\"")))); #line default #line hidden this.Write(" "); - #line 65 "C:\Users\Administrator\Documents\GitHubVisualStudio\Migrate-assets-from-RM-server-to-VSTS\src\RMWorkflowMigrator.Generator.PowerShell\Templates\IndividualActionTemplate.tt" + #line 81 "C:\Users\Administrator\Documents\GitHubVisualStudio\Migrate-assets-from-RM-server-to-VSTS\src\RMWorkflowMigrator.Generator.PowerShell\Templates\IndividualActionTemplate.tt" this.Write(this.ToStringHelper.ToStringWithCulture(string.Join(" ", script.Value.Where(s => s.IsComponent).Select(c => $"-ComponentPath{c.Sequence} $ComponentPath{c.Sequence}")))); #line default #line hidden this.Write(" -DeployerToolsPath $DeployerToolsPath\r\n"); - #line 66 "C:\Users\Administrator\Documents\GitHubVisualStudio\Migrate-assets-from-RM-server-to-VSTS\src\RMWorkflowMigrator.Generator.PowerShell\Templates\IndividualActionTemplate.tt" + #line 82 "C:\Users\Administrator\Documents\GitHubVisualStudio\Migrate-assets-from-RM-server-to-VSTS\src\RMWorkflowMigrator.Generator.PowerShell\Templates\IndividualActionTemplate.tt" } #line default #line hidden - #line 67 "C:\Users\Administrator\Documents\GitHubVisualStudio\Migrate-assets-from-RM-server-to-VSTS\src\RMWorkflowMigrator.Generator.PowerShell\Templates\IndividualActionTemplate.tt" + #line 83 "C:\Users\Administrator\Documents\GitHubVisualStudio\Migrate-assets-from-RM-server-to-VSTS\src\RMWorkflowMigrator.Generator.PowerShell\Templates\IndividualActionTemplate.tt" } #line default diff --git a/src/RMWorkflowMigrator.Generator.PowerShell/Templates/IndividualActionTemplate.tt b/src/RMWorkflowMigrator.Generator.PowerShell/Templates/IndividualActionTemplate.tt index ab5b929..079f34f 100644 --- a/src/RMWorkflowMigrator.Generator.PowerShell/Templates/IndividualActionTemplate.tt +++ b/src/RMWorkflowMigrator.Generator.PowerShell/Templates/IndividualActionTemplate.tt @@ -50,8 +50,24 @@ var targetPath = action.ConfigurationVariables.First(cv => cv.OriginalName == "I &"$(join-path $DeployerToolsPath "TokenizationScript.ps1")" -FilePath $<#=targetPath#> -FileSpec "<#=action.FileExtensionFilter#>" -Values $configVariables <#}#> <#} else {#> +<#if (action.Command.StartsWith("powershell")) { + if (action.Arguments.StartsWith(@"""&") || action.Arguments.StartsWith(@"-command ""&") || action.Arguments.StartsWith(@"-Command ""&")) {#> + powershell <#=action.Arguments#> +<#} + else { + var ps1Index = action.Arguments.IndexOf(".ps1")+4; + var actualCommand = action.Arguments.Substring(0,ps1Index); + var strippedArguments = action.Arguments.Replace(actualCommand, string.Empty); + actualCommand = actualCommand.Replace("-command", string.Empty).Replace("-Command", string.Empty).Trim();#> + Invoke-Expression "& `"$(join-path $DeployerToolsPath '<#=actualCommand#>')`" <#=strippedArguments#>" +<#} +} +else if (!action.CommandIsExtractedTool) {#> + <#=action.Command#> <#=action.Arguments#> +<#} +else {#> &"$(join-path $DeployerToolsPath "<#=action.Command#>")" <#=action.Arguments#> -<#}#> +<#}}#> } catch { Write-output "<#=action.DisplayName#> failed. Error:" diff --git a/src/RMWorkflowMigrator.Generator.PowerShell/UniquePropertyResolver.cs b/src/RMWorkflowMigrator.Generator.PowerShell/UniquePropertyResolver.cs index 9e1bf89..5426329 100644 --- a/src/RMWorkflowMigrator.Generator.PowerShell/UniquePropertyResolver.cs +++ b/src/RMWorkflowMigrator.Generator.PowerShell/UniquePropertyResolver.cs @@ -19,7 +19,7 @@ public static class UniquePropertyResolver public static IEnumerable ResolveProperties(IEnumerable actions) { var sequence = 2; - var properties = new HashSet>(); + var properties = new HashSet>(); foreach (var action in actions) { action.Arguments = CleanActionParameters(action.Arguments); @@ -31,7 +31,7 @@ public static IEnumerable ResolveProperties(IEnumerable> properties, int sequence) + private static int MakeParametersUnique(ScriptAction action, ISet> properties, int sequence) { if (action.ConfigurationVariables == null) { @@ -40,29 +40,45 @@ private static int MakeParametersUnique(ScriptAction action, ISet p.Item1 == tuple.Item1 && p.Item2 == tuple.Item2); + if (match != null) { - if (properties.Any(p => p.Item1 == tuple.Item1 && p.Item2 != tuple.Item2)) - { - var newVariableName = configVar.RemappedName + sequence; - sequence++; - action.Arguments = action.Arguments?.Replace($"${configVar.RemappedName}", $"${newVariableName}"); - var commandHasParameter = action.Command?.Contains(configVar.RemappedName); - if (commandHasParameter.HasValue && commandHasParameter.Value) - { - action.Command = action.Command?.Replace($"${configVar.RemappedName}", $"${newVariableName}"); - } - configVar.RemappedName = newVariableName; - } + UpdateRemappedVariable(action, configVar, match.Item3); } - - properties.Add(tuple); + // If we have a matching variable name with a different value, remap it with a sequence number + else if (properties.Any(p => p.Item1 == tuple.Item1 && p.Item2 != tuple.Item2)) + { + var newVariableName = configVar.RemappedName + sequence; + sequence++; + UpdateRemappedVariable(action, configVar, newVariableName); + tuple = Tuple.Create(CleanInvalidCharacters(configVar.OriginalName), configVar.Value, newVariableName); + properties.Add(tuple); + } + // Otherwise, we've never seen this variable + value combo before so we can just track that we've seen it + else + { + properties.Add(tuple); + } + } return sequence; } + private static void UpdateRemappedVariable(ScriptAction action, ConfigurationVariable configVar, string newVariableName) + { + action.Arguments = action.Arguments?.Replace($"${configVar.RemappedName}", $"${newVariableName}"); + var commandHasParameter = action.Command?.Contains(configVar.RemappedName); + if (commandHasParameter.HasValue && commandHasParameter.Value) + { + action.Command = action.Command?.Replace($"${configVar.RemappedName}", $"${newVariableName}"); + } + configVar.RemappedName = newVariableName; + } + private static void CleanConfigurationValues(IEnumerable configurationVariables) { if (configurationVariables == null) diff --git a/src/RMWorkflowMigrator.Tests.Unit/When_Resolving_Unique_Script_Parameters.cs b/src/RMWorkflowMigrator.Tests.Unit/When_Resolving_Unique_Script_Parameters.cs index ecd3878..78e4ad8 100644 --- a/src/RMWorkflowMigrator.Tests.Unit/When_Resolving_Unique_Script_Parameters.cs +++ b/src/RMWorkflowMigrator.Tests.Unit/When_Resolving_Unique_Script_Parameters.cs @@ -245,7 +245,7 @@ public void Many_Identical_And_Nonidentical_Parameters_And_Config_Values_Are_Mad Assert.IsTrue(action2.ConfigurationVariables.Any(cv => cv.RemappedName == "HelloWorld2")); Assert.IsTrue(action2.ConfigurationVariables.Any(cv => cv.RemappedName == "AnotherConfigToken3")); - Assert.IsTrue(action3.ConfigurationVariables.Any(cv => cv.RemappedName == "HelloWorld")); + Assert.IsTrue(action3.ConfigurationVariables.Any(cv => cv.RemappedName == "HelloWorld2")); Assert.IsTrue(action3.ConfigurationVariables.Any(cv => cv.RemappedName == "ConfigToken4")); Assert.IsTrue(action3.ConfigurationVariables.Any(cv => cv.RemappedName == "AnotherConfigToken5")); } @@ -302,6 +302,64 @@ public void Two_Identical_Parameters_That_Take_The_Different_Values_Are_Made_Uni Assert.IsTrue(results[1].ConfigurationVariables.All(cv => cv.RemappedName == "HelloWorld2")); } + [TestMethod] + public void A_Set_Of_NonSequential_Identical_Parameters_That_Take_A_Combination_Of_Different_And_Same_Values_Are_Made_Unique() + { + // Arrange + var actions = new List + { + new ScriptAction + { + Arguments = @"-Param __Hello World__", + ConfigurationVariables = + new List + { + new ConfigurationVariable { OriginalName = "Hello World", Value = "placeholder" } + } + }, + new ScriptAction + { + Arguments = @"-Param __Hello World__", + ConfigurationVariables = + new List + { + new ConfigurationVariable { OriginalName = "Hello World", Value = "Bar" } + } + }, + new ScriptAction + { + Arguments = @"-Param __Hello World__", + ConfigurationVariables = + new List + { + new ConfigurationVariable { OriginalName = "Hello World", Value = "placeholder" } + } + }, + new ScriptAction + { + Arguments = @"-Param __Hello World__", + ConfigurationVariables = + new List + { + new ConfigurationVariable { OriginalName = "Hello World", Value = "Bar" } + } + } + }; + + // Act + var results = UniquePropertyResolver.ResolveProperties(actions).ToList(); + + // Assert + Assert.AreEqual("-Param $HelloWorld", results[0].Arguments); + Assert.IsTrue(results[0].ConfigurationVariables.All(cv => cv.RemappedName == "HelloWorld")); + Assert.AreEqual("-Param $HelloWorld2", results[1].Arguments); + Assert.IsTrue(results[1].ConfigurationVariables.All(cv => cv.RemappedName == "HelloWorld2")); + Assert.AreEqual("-Param $HelloWorld", results[2].Arguments); + Assert.IsTrue(results[2].ConfigurationVariables.All(cv => cv.RemappedName == "HelloWorld")); + Assert.AreEqual("-Param $HelloWorld2", results[3].Arguments); + Assert.IsTrue(results[3].ConfigurationVariables.All(cv => cv.RemappedName == "HelloWorld2")); + } + [TestMethod] public void Two_Identical_Parameters_That_Take_The_Different_Values_Are_Made_Unique_When_One_Is_A_Command() {