From 551485a2eb8be4f44aaeb02566a2cae9c884172f Mon Sep 17 00:00:00 2001 From: Ruben Guerrero Samaniego Date: Tue, 9 May 2023 17:40:53 -0700 Subject: [PATCH 1/3] Allow $WinGetConfigRoot variable expansion --- .../Public/AppInstallerErrors.h | 1 + .../Exceptions/ErrorCodes.cs | 5 ++ .../UnitSettingConfigRootException.cs | 39 +++++++++++++ .../Helpers/ConfigurationUnitAndResource.cs | 8 +-- .../Helpers/ConfigurationUnitInternal.cs | 55 ++++++++++++++++++ .../Set/ConfigurationSetProcessor.cs | 4 +- .../Unit/ConfigurationUnitProcessor.cs | 6 +- .../Tests/ConfigurationDetailsTests.cs | 2 +- .../Tests/ConfigurationSetProcessorTests.cs | 2 +- .../Tests/ConfigurationUnitInternalTests.cs | 56 ++++++++++++++++++- .../Tests/ConfigurationUnitProcessorTests.cs | 1 + 11 files changed, 165 insertions(+), 14 deletions(-) create mode 100644 src/Microsoft.Management.Configuration.Processor/Exceptions/UnitSettingConfigRootException.cs diff --git a/src/AppInstallerSharedLib/Public/AppInstallerErrors.h b/src/AppInstallerSharedLib/Public/AppInstallerErrors.h index 8e6fa30524..fdd0183654 100644 --- a/src/AppInstallerSharedLib/Public/AppInstallerErrors.h +++ b/src/AppInstallerSharedLib/Public/AppInstallerErrors.h @@ -178,6 +178,7 @@ #define WINGET_CONFIG_ERROR_UNIT_MODULE_CONFLICT ((HRESULT)0x8A15C107) #define WINGET_CONFIG_ERROR_UNIT_IMPORT_MODULE ((HRESULT)0x8A15C108) #define WINGET_CONFIG_ERROR_UNIT_INVOKE_INVALID_RESULT ((HRESULT)0x8A15C109) +#define WINGET_CONFIG_ERROR_UNIT_SETTING_CONFIG_ROOT ((HRESULT)0x8A15C110) namespace AppInstaller { diff --git a/src/Microsoft.Management.Configuration.Processor/Exceptions/ErrorCodes.cs b/src/Microsoft.Management.Configuration.Processor/Exceptions/ErrorCodes.cs index 5431a296bf..89b32996c8 100644 --- a/src/Microsoft.Management.Configuration.Processor/Exceptions/ErrorCodes.cs +++ b/src/Microsoft.Management.Configuration.Processor/Exceptions/ErrorCodes.cs @@ -60,5 +60,10 @@ internal static class ErrorCodes /// The unit returned an invalid result. /// internal const int WinGetConfigUnitInvokeInvalidResult = unchecked((int)0x8A15C109); + + /// + /// The unit contains a setting that requires config root. + /// + internal const int WinGetConfigUnitSettingConfigRoot = unchecked((int)0x8A15C110); } } diff --git a/src/Microsoft.Management.Configuration.Processor/Exceptions/UnitSettingConfigRootException.cs b/src/Microsoft.Management.Configuration.Processor/Exceptions/UnitSettingConfigRootException.cs new file mode 100644 index 0000000000..90c0ef5b6b --- /dev/null +++ b/src/Microsoft.Management.Configuration.Processor/Exceptions/UnitSettingConfigRootException.cs @@ -0,0 +1,39 @@ +// ----------------------------------------------------------------------------- +// +// Copyright (c) Microsoft Corporation. Licensed under the MIT License. +// +// ----------------------------------------------------------------------------- + +namespace Microsoft.Management.Configuration.Processor.Exceptions +{ + using System; + + /// + /// A setting uses the config root variable and the Origin was not set in the ConfigurationSet. + /// + internal class UnitSettingConfigRootException : Exception + { + /// + /// Initializes a new instance of the class. + /// + /// Unit name. + /// Setting. + public UnitSettingConfigRootException(string unitName, string setting) + : base($"Unit: {unitName} Setting {setting} requires the ConfigurationSet Origin") + { + this.HResult = ErrorCodes.WinGetConfigUnitSettingConfigRoot; + this.UnitName = unitName; + this.Setting = setting; + } + + /// + /// Gets the resource name. + /// + public string UnitName { get; } + + /// + /// Gets the setting that reference the config root variable. + /// + public string Setting { get; } + } +} diff --git a/src/Microsoft.Management.Configuration.Processor/Helpers/ConfigurationUnitAndResource.cs b/src/Microsoft.Management.Configuration.Processor/Helpers/ConfigurationUnitAndResource.cs index 5afa2d4fcf..645d298499 100644 --- a/src/Microsoft.Management.Configuration.Processor/Helpers/ConfigurationUnitAndResource.cs +++ b/src/Microsoft.Management.Configuration.Processor/Helpers/ConfigurationUnitAndResource.cs @@ -99,14 +99,12 @@ public ModuleSpecification? Module } /// - /// TODO: Implement. - /// I am so sad because rs.SessionStateProxy.InvokeCommand.ExpandString doesn't work as I wanted. - /// PowerShell assumes all code passed to ExpandString is trusted and we cannot assume that. + /// Gets the settings of the unit. /// /// ValueSet with settings. - public ValueSet GetExpandedSettings() + public ValueSet GetSettings() { - return this.Unit.Settings; + return this.UnitInternal.GetExpandedSettings(); } } } diff --git a/src/Microsoft.Management.Configuration.Processor/Helpers/ConfigurationUnitInternal.cs b/src/Microsoft.Management.Configuration.Processor/Helpers/ConfigurationUnitInternal.cs index 6a8b1dc654..f6d151fef5 100644 --- a/src/Microsoft.Management.Configuration.Processor/Helpers/ConfigurationUnitInternal.cs +++ b/src/Microsoft.Management.Configuration.Processor/Helpers/ConfigurationUnitInternal.cs @@ -6,9 +6,12 @@ namespace Microsoft.Management.Configuration.Processor.Helpers { + using System; using System.Collections.Generic; using Microsoft.Management.Configuration.Processor.Constants; + using Microsoft.Management.Configuration.Processor.Exceptions; using Microsoft.PowerShell.Commands; + using Windows.Foundation.Collections; /// /// Wrapper around Configuration units and its directives. Creates a normalized directives map @@ -16,18 +19,24 @@ namespace Microsoft.Management.Configuration.Processor.Helpers /// internal class ConfigurationUnitInternal { + private static string configRoot = "$WinGetConfigRoot"; + + private readonly string origin; private readonly Dictionary normalizedDirectives = new (); /// /// Initializes a new instance of the class. /// /// Configuration unit. + /// The origin of the unit. /// Directives overlay. public ConfigurationUnitInternal( ConfigurationUnit unit, + string origin, IReadOnlyDictionary? directivesOverlay = null) { this.Unit = unit; + this.origin = origin; this.DirectivesOverlay = directivesOverlay; this.InitializeDirectives(); @@ -150,6 +159,52 @@ public string ToIdentifyingString() return null; } + /// + /// TODO: Implement for more variables. + /// I am so sad because rs.SessionStateProxy.InvokeCommand.ExpandString doesn't work as I wanted. + /// PowerShell assumes all code passed to ExpandString is trusted and we cannot assume that. + /// + /// ValueSet with settings. + public ValueSet GetExpandedSettings() + { + var valueSet = new ValueSet(); + foreach (var value in this.Unit.Settings) + { + if (value.Value is string) + { + // For now, we just expand origin. + valueSet.Add(value.Key, this.ExpandOrigin(value.Value as string, value.Key)); + } + else + { + valueSet.Add(value); + } + } + + return valueSet; + } + + private string? ExpandOrigin(string? value, string settingName) + { + if (!string.IsNullOrEmpty(value)) + { + // TODO: since we only support one variable, this only finds and replace + // $WingetConfigRoot if found in the string when the work of expanding + // string is done it should take into account other operators like the subexpression operator $() + if (value.Contains(configRoot, StringComparison.OrdinalIgnoreCase)) + { + if (string.IsNullOrEmpty(this.origin)) + { + throw new UnitSettingConfigRootException(this.Unit.UnitName, settingName); + } + + return value.Replace(configRoot, this.origin, StringComparison.OrdinalIgnoreCase); + } + } + + return value; + } + private void InitializeDirectives() { // Overlay directives have precedence. diff --git a/src/Microsoft.Management.Configuration.Processor/Set/ConfigurationSetProcessor.cs b/src/Microsoft.Management.Configuration.Processor/Set/ConfigurationSetProcessor.cs index 4a0cdc3a51..868438599c 100644 --- a/src/Microsoft.Management.Configuration.Processor/Set/ConfigurationSetProcessor.cs +++ b/src/Microsoft.Management.Configuration.Processor/Set/ConfigurationSetProcessor.cs @@ -57,7 +57,7 @@ public IConfigurationUnitProcessor CreateUnitProcessor( { try { - var configurationUnitInternal = new ConfigurationUnitInternal(unit, directivesOverlay); + var configurationUnitInternal = new ConfigurationUnitInternal(unit, this.configurationSet.Origin, directivesOverlay); this.OnDiagnostics(DiagnosticLevel.Verbose, $"Creating unit processor for: {configurationUnitInternal.ToIdentifyingString()}..."); var dscResourceInfo = this.PrepareUnitForProcessing(configurationUnitInternal); @@ -87,7 +87,7 @@ public IConfigurationUnitProcessor CreateUnitProcessor( { try { - var unitInternal = new ConfigurationUnitInternal(unit); + var unitInternal = new ConfigurationUnitInternal(unit, this.configurationSet.Origin); this.OnDiagnostics(DiagnosticLevel.Verbose, $"Getting unit details [{detailLevel}] for: {unitInternal.ToIdentifyingString()}"); var dscResourceInfo = this.ProcessorEnvironment.GetDscResource(unitInternal); diff --git a/src/Microsoft.Management.Configuration.Processor/Unit/ConfigurationUnitProcessor.cs b/src/Microsoft.Management.Configuration.Processor/Unit/ConfigurationUnitProcessor.cs index 3fba8c04d8..ddc77a04f4 100644 --- a/src/Microsoft.Management.Configuration.Processor/Unit/ConfigurationUnitProcessor.cs +++ b/src/Microsoft.Management.Configuration.Processor/Unit/ConfigurationUnitProcessor.cs @@ -64,7 +64,7 @@ public GetSettingsResult GetSettings() try { result.Settings = this.processorEnvironment.InvokeGetResource( - this.unitResource.GetExpandedSettings(), + this.unitResource.GetSettings(), this.unitResource.ResourceName, this.unitResource.Module); } @@ -97,7 +97,7 @@ public TestSettingsResult TestSettings() try { bool testResult = this.processorEnvironment.InvokeTestResource( - this.unitResource.GetExpandedSettings(), + this.unitResource.GetSettings(), this.unitResource.ResourceName, this.unitResource.Module); @@ -132,7 +132,7 @@ public ApplySettingsResult ApplySettings() try { result.RebootRequired = this.processorEnvironment.InvokeSetResource( - this.unitResource.GetExpandedSettings(), + this.unitResource.GetSettings(), this.unitResource.ResourceName, this.unitResource.Module); } diff --git a/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationDetailsTests.cs b/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationDetailsTests.cs index 25b674e2ca..7e8573352f 100644 --- a/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationDetailsTests.cs +++ b/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationDetailsTests.cs @@ -189,7 +189,7 @@ private ConfigurationUnit CreteConfigurationUnit() // This is easier than trying to mock sealed class from external code... var testEnv = this.fixture.PrepareTestProcessorEnvironment(true); - var dscResourceInfo = testEnv.GetDscResource(new ConfigurationUnitInternal(unit, null)); + var dscResourceInfo = testEnv.GetDscResource(new ConfigurationUnitInternal(unit, string.Empty, null)); var psModuleInfo = testEnv.GetAvailableModule(PowerShellHelpers.CreateModuleSpecification("xSimpleTestResource", "0.0.0.1")); if (dscResourceInfo is null || psModuleInfo is null) diff --git a/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationSetProcessorTests.cs b/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationSetProcessorTests.cs index d1d18b7f71..2214f8c59e 100644 --- a/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationSetProcessorTests.cs +++ b/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationSetProcessorTests.cs @@ -694,7 +694,7 @@ private ConfigurationUnit CreteConfigurationUnit() { // This is easier than trying to mock sealed class from external code... var testEnv = this.fixture.PrepareTestProcessorEnvironment(true); - var dscResourceInfo = testEnv.GetDscResource(new ConfigurationUnitInternal(unit, null)); + var dscResourceInfo = testEnv.GetDscResource(new ConfigurationUnitInternal(unit, string.Empty, null)); var psModuleInfo = testEnv.GetAvailableModule(PowerShellHelpers.CreateModuleSpecification("xSimpleTestResource", "0.0.0.1")); if (dscResourceInfo is null || psModuleInfo is null) diff --git a/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationUnitInternalTests.cs b/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationUnitInternalTests.cs index 7cf05f480f..f23844759b 100644 --- a/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationUnitInternalTests.cs +++ b/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationUnitInternalTests.cs @@ -10,6 +10,7 @@ namespace Microsoft.Management.Configuration.UnitTests.Tests using System.Collections.Generic; using System.Management.Automation; using Microsoft.Management.Configuration; + using Microsoft.Management.Configuration.Processor.Exceptions; using Microsoft.Management.Configuration.Processor.Helpers; using Microsoft.Management.Configuration.UnitTests.Fixtures; using Xunit; @@ -73,7 +74,7 @@ public void GetDirectivesTest() { anotherDirective, overlayAnother }, }; - var unitInternal = new ConfigurationUnitInternal(unit, overlays); + var unitInternal = new ConfigurationUnitInternal(unit, string.Empty, overlays); var description = unitInternal.GetDirective(descriptionDirective); Assert.Equal(description, overlayDescription); @@ -107,7 +108,58 @@ public void GetVersion_BadVersion() unit.Directives.Add("version", "not a version"); Assert.Throws( - () => new ConfigurationUnitInternal(unit, null)); + () => new ConfigurationUnitInternal(unit, string.Empty, null)); + } + + /// + /// Verifies expansion of origin. + /// + [Fact] + public void GetExpandedSettings_Origin() + { + var unit = new ConfigurationUnit(); + unit.Settings.Add("var1", @"WinGetConfigRoot\this\is\a\path.txt"); + unit.Settings.Add("var2", @"$WinGetConfigRoot\this\is\a\path.txt"); + unit.Settings.Add("var3", @"this\is\a\WINGETCONFIGROOT\path.txt"); + unit.Settings.Add("var4", @"this\is\a\$WINGETCONFIGROOT\path.txt"); + unit.Settings.Add("var5", @"this\is\a\path\wingetconfigroot"); + unit.Settings.Add("var6", @"this\is\a\path\$wingetconfigroot"); + + string configPath = @"one\root"; + var unitInternal = new ConfigurationUnitInternal(unit, configPath); + + var expandedSettings = unitInternal.GetExpandedSettings(); + + var var1 = expandedSettings["var1"]; + Assert.Equal(@"WinGetConfigRoot\this\is\a\path.txt", var1 as string); + + var var2 = expandedSettings["var2"]; + Assert.Equal(@"one\root\this\is\a\path.txt", var2 as string); + + var var3 = expandedSettings["var3"]; + Assert.Equal(@"this\is\a\WINGETCONFIGROOT\path.txt", var3 as string); + + var var4 = expandedSettings["var4"]; + Assert.Equal(@"this\is\a\one\root\path.txt", var4 as string); + + var var5 = expandedSettings["var5"]; + Assert.Equal(@"this\is\a\path\wingetconfigroot", var5 as string); + + var var6 = expandedSettings["var6"]; + Assert.Equal(@"this\is\a\path\one\root", var6 as string); + } + + /// + /// Verifies throws when origin is not set. + /// + [Fact] + public void GetExpandedSetting_Origin_Throw() + { + var unit = new ConfigurationUnit(); + unit.Settings.Add("var2", @"$WinGetConfigRoot\this\is\a\path.txt"); + + var unitInternal = new ConfigurationUnitInternal(unit, null!); + Assert.Throws(() => unitInternal.GetExpandedSettings()); } } } diff --git a/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationUnitProcessorTests.cs b/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationUnitProcessorTests.cs index 0cdf6d9b87..467fa486a6 100644 --- a/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationUnitProcessorTests.cs +++ b/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationUnitProcessorTests.cs @@ -362,6 +362,7 @@ private ConfigurationUnitAndResource CreateUnitResource(ConfigurationUnitIntent UnitName = resourceName, Intent = intent, }, + string.Empty, new Dictionary()), new DscResourceInfoInternal(resourceName, null, null)); } From 8344bd0542d93f16c9a5c41fdae64d46cd8470ed Mon Sep 17 00:00:00 2001 From: Ruben Guerrero Samaniego Date: Tue, 9 May 2023 19:54:33 -0700 Subject: [PATCH 2/3] Path origin story --- .github/actions/spelling/expect.txt | 1 + .../UnitSettingConfigRootException.cs | 4 +-- .../Helpers/ConfigurationUnitInternal.cs | 34 +++++++++++++------ .../Set/ConfigurationSetProcessor.cs | 4 +-- .../Tests/ConfigurationUnitInternalTests.cs | 21 +++++++----- 5 files changed, 41 insertions(+), 23 deletions(-) diff --git a/.github/actions/spelling/expect.txt b/.github/actions/spelling/expect.txt index ce8e06d60e..930e9f6146 100644 --- a/.github/actions/spelling/expect.txt +++ b/.github/actions/spelling/expect.txt @@ -431,6 +431,7 @@ winapifamily windir windowsdeveloper winerror +wingetconfigroot wingetcreate wingetdev wingetutil diff --git a/src/Microsoft.Management.Configuration.Processor/Exceptions/UnitSettingConfigRootException.cs b/src/Microsoft.Management.Configuration.Processor/Exceptions/UnitSettingConfigRootException.cs index 90c0ef5b6b..54aa4c051f 100644 --- a/src/Microsoft.Management.Configuration.Processor/Exceptions/UnitSettingConfigRootException.cs +++ b/src/Microsoft.Management.Configuration.Processor/Exceptions/UnitSettingConfigRootException.cs @@ -9,7 +9,7 @@ namespace Microsoft.Management.Configuration.Processor.Exceptions using System; /// - /// A setting uses the config root variable and the Origin was not set in the ConfigurationSet. + /// A setting uses the config root variable and the Path was not set in the ConfigurationSet. /// internal class UnitSettingConfigRootException : Exception { @@ -19,7 +19,7 @@ internal class UnitSettingConfigRootException : Exception /// Unit name. /// Setting. public UnitSettingConfigRootException(string unitName, string setting) - : base($"Unit: {unitName} Setting {setting} requires the ConfigurationSet Origin") + : base($"Unit: {unitName} Setting {setting} requires the ConfigurationSet Path") { this.HResult = ErrorCodes.WinGetConfigUnitSettingConfigRoot; this.UnitName = unitName; diff --git a/src/Microsoft.Management.Configuration.Processor/Helpers/ConfigurationUnitInternal.cs b/src/Microsoft.Management.Configuration.Processor/Helpers/ConfigurationUnitInternal.cs index f6d151fef5..c15059d53a 100644 --- a/src/Microsoft.Management.Configuration.Processor/Helpers/ConfigurationUnitInternal.cs +++ b/src/Microsoft.Management.Configuration.Processor/Helpers/ConfigurationUnitInternal.cs @@ -8,6 +8,7 @@ namespace Microsoft.Management.Configuration.Processor.Helpers { using System; using System.Collections.Generic; + using System.IO; using Microsoft.Management.Configuration.Processor.Constants; using Microsoft.Management.Configuration.Processor.Exceptions; using Microsoft.PowerShell.Commands; @@ -19,24 +20,24 @@ namespace Microsoft.Management.Configuration.Processor.Helpers /// internal class ConfigurationUnitInternal { - private static string configRoot = "$WinGetConfigRoot"; + private static string configRootVar = "$WinGetConfigRoot"; - private readonly string origin; + private readonly string configurationFilePath; private readonly Dictionary normalizedDirectives = new (); /// /// Initializes a new instance of the class. /// /// Configuration unit. - /// The origin of the unit. + /// The configuration file path. /// Directives overlay. public ConfigurationUnitInternal( ConfigurationUnit unit, - string origin, + string configurationFilePath, IReadOnlyDictionary? directivesOverlay = null) { this.Unit = unit; - this.origin = origin; + this.configurationFilePath = configurationFilePath; this.DirectivesOverlay = directivesOverlay; this.InitializeDirectives(); @@ -172,8 +173,8 @@ public ValueSet GetExpandedSettings() { if (value.Value is string) { - // For now, we just expand origin. - valueSet.Add(value.Key, this.ExpandOrigin(value.Value as string, value.Key)); + // For now, we just expand config root. + valueSet.Add(value.Key, this.ExpandConfigRoot(value.Value as string, value.Key)); } else { @@ -184,21 +185,32 @@ public ValueSet GetExpandedSettings() return valueSet; } - private string? ExpandOrigin(string? value, string settingName) + private string? ExpandConfigRoot(string? value, string settingName) { if (!string.IsNullOrEmpty(value)) { // TODO: since we only support one variable, this only finds and replace // $WingetConfigRoot if found in the string when the work of expanding // string is done it should take into account other operators like the subexpression operator $() - if (value.Contains(configRoot, StringComparison.OrdinalIgnoreCase)) + if (value.Contains(configRootVar, StringComparison.OrdinalIgnoreCase)) { - if (string.IsNullOrEmpty(this.origin)) + if (string.IsNullOrEmpty(this.configurationFilePath)) { throw new UnitSettingConfigRootException(this.Unit.UnitName, settingName); } - return value.Replace(configRoot, this.origin, StringComparison.OrdinalIgnoreCase); + if (!File.Exists(this.configurationFilePath)) + { + throw new FileNotFoundException(this.configurationFilePath); + } + + var configRoot = Path.GetDirectoryName(this.configurationFilePath); + if (configRoot == null) + { + throw new ArgumentException(); + } + + return value.Replace(configRootVar, configRoot, StringComparison.OrdinalIgnoreCase); } } diff --git a/src/Microsoft.Management.Configuration.Processor/Set/ConfigurationSetProcessor.cs b/src/Microsoft.Management.Configuration.Processor/Set/ConfigurationSetProcessor.cs index 868438599c..e11e0a3824 100644 --- a/src/Microsoft.Management.Configuration.Processor/Set/ConfigurationSetProcessor.cs +++ b/src/Microsoft.Management.Configuration.Processor/Set/ConfigurationSetProcessor.cs @@ -57,7 +57,7 @@ public IConfigurationUnitProcessor CreateUnitProcessor( { try { - var configurationUnitInternal = new ConfigurationUnitInternal(unit, this.configurationSet.Origin, directivesOverlay); + var configurationUnitInternal = new ConfigurationUnitInternal(unit, this.configurationSet.Path, directivesOverlay); this.OnDiagnostics(DiagnosticLevel.Verbose, $"Creating unit processor for: {configurationUnitInternal.ToIdentifyingString()}..."); var dscResourceInfo = this.PrepareUnitForProcessing(configurationUnitInternal); @@ -87,7 +87,7 @@ public IConfigurationUnitProcessor CreateUnitProcessor( { try { - var unitInternal = new ConfigurationUnitInternal(unit, this.configurationSet.Origin); + var unitInternal = new ConfigurationUnitInternal(unit, this.configurationSet.Path); this.OnDiagnostics(DiagnosticLevel.Verbose, $"Getting unit details [{detailLevel}] for: {unitInternal.ToIdentifyingString()}"); var dscResourceInfo = this.ProcessorEnvironment.GetDscResource(unitInternal); diff --git a/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationUnitInternalTests.cs b/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationUnitInternalTests.cs index f23844759b..a9c0759294 100644 --- a/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationUnitInternalTests.cs +++ b/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationUnitInternalTests.cs @@ -8,11 +8,13 @@ namespace Microsoft.Management.Configuration.UnitTests.Tests { using System; using System.Collections.Generic; + using System.IO; using System.Management.Automation; using Microsoft.Management.Configuration; using Microsoft.Management.Configuration.Processor.Exceptions; using Microsoft.Management.Configuration.Processor.Helpers; using Microsoft.Management.Configuration.UnitTests.Fixtures; + using Microsoft.Management.Configuration.UnitTests.Helpers; using Xunit; using Xunit.Abstractions; @@ -112,11 +114,13 @@ public void GetVersion_BadVersion() } /// - /// Verifies expansion of origin. + /// Verifies expansion of ConfigRoot. /// [Fact] - public void GetExpandedSettings_Origin() + public void GetExpandedSettings_ConfigRoot() { + using var tmpFile = new TempFile("fakeConfigFile.yml", content: "content"); + var unit = new ConfigurationUnit(); unit.Settings.Add("var1", @"WinGetConfigRoot\this\is\a\path.txt"); unit.Settings.Add("var2", @"$WinGetConfigRoot\this\is\a\path.txt"); @@ -125,7 +129,8 @@ public void GetExpandedSettings_Origin() unit.Settings.Add("var5", @"this\is\a\path\wingetconfigroot"); unit.Settings.Add("var6", @"this\is\a\path\$wingetconfigroot"); - string configPath = @"one\root"; + string configPath = tmpFile.FullFileName; + string? expectedPath = Path.GetDirectoryName(configPath); var unitInternal = new ConfigurationUnitInternal(unit, configPath); var expandedSettings = unitInternal.GetExpandedSettings(); @@ -134,26 +139,26 @@ public void GetExpandedSettings_Origin() Assert.Equal(@"WinGetConfigRoot\this\is\a\path.txt", var1 as string); var var2 = expandedSettings["var2"]; - Assert.Equal(@"one\root\this\is\a\path.txt", var2 as string); + Assert.Equal($@"{expectedPath}\this\is\a\path.txt", var2 as string); var var3 = expandedSettings["var3"]; Assert.Equal(@"this\is\a\WINGETCONFIGROOT\path.txt", var3 as string); var var4 = expandedSettings["var4"]; - Assert.Equal(@"this\is\a\one\root\path.txt", var4 as string); + Assert.Equal($@"this\is\a\{expectedPath}\path.txt", var4 as string); var var5 = expandedSettings["var5"]; Assert.Equal(@"this\is\a\path\wingetconfigroot", var5 as string); var var6 = expandedSettings["var6"]; - Assert.Equal(@"this\is\a\path\one\root", var6 as string); + Assert.Equal($@"this\is\a\path\{expectedPath}", var6 as string); } /// - /// Verifies throws when origin is not set. + /// Verifies throws when config root is not set. /// [Fact] - public void GetExpandedSetting_Origin_Throw() + public void GetExpandedSetting_ConfigRoot_Throw() { var unit = new ConfigurationUnit(); unit.Settings.Add("var2", @"$WinGetConfigRoot\this\is\a\path.txt"); From 7a24c7e9df7cadd39a7b8043e7a78546d3b7cd3e Mon Sep 17 00:00:00 2001 From: Ruben Guerrero Samaniego Date: Wed, 10 May 2023 10:21:48 -0700 Subject: [PATCH 3/3] curly variables --- .../Helpers/ConfigurationUnitInternal.cs | 31 ++++++++++--------- .../Tests/ConfigurationUnitInternalTests.cs | 20 ++++++------ 2 files changed, 27 insertions(+), 24 deletions(-) diff --git a/src/Microsoft.Management.Configuration.Processor/Helpers/ConfigurationUnitInternal.cs b/src/Microsoft.Management.Configuration.Processor/Helpers/ConfigurationUnitInternal.cs index c15059d53a..1d91a4e0b2 100644 --- a/src/Microsoft.Management.Configuration.Processor/Helpers/ConfigurationUnitInternal.cs +++ b/src/Microsoft.Management.Configuration.Processor/Helpers/ConfigurationUnitInternal.cs @@ -20,9 +20,9 @@ namespace Microsoft.Management.Configuration.Processor.Helpers /// internal class ConfigurationUnitInternal { - private static string configRootVar = "$WinGetConfigRoot"; + private const string ConfigRootVar = "${WinGetConfigRoot}"; - private readonly string configurationFilePath; + private readonly string? configurationFileRootPath = null; private readonly Dictionary normalizedDirectives = new (); /// @@ -37,7 +37,6 @@ public ConfigurationUnitInternal( IReadOnlyDictionary? directivesOverlay = null) { this.Unit = unit; - this.configurationFilePath = configurationFilePath; this.DirectivesOverlay = directivesOverlay; this.InitializeDirectives(); @@ -55,6 +54,16 @@ public ConfigurationUnitInternal( this.GetDirective(DirectiveConstants.MaxVersion), this.GetDirective(DirectiveConstants.ModuleGuid)); } + + if (!string.IsNullOrEmpty(configurationFilePath)) + { + if (!File.Exists(configurationFilePath)) + { + throw new FileNotFoundException(configurationFilePath); + } + + this.configurationFileRootPath = Path.GetDirectoryName(configurationFilePath); + } } /// @@ -190,27 +199,21 @@ public ValueSet GetExpandedSettings() if (!string.IsNullOrEmpty(value)) { // TODO: since we only support one variable, this only finds and replace - // $WingetConfigRoot if found in the string when the work of expanding + // ${WingetConfigRoot} if found in the string when the work of expanding // string is done it should take into account other operators like the subexpression operator $() - if (value.Contains(configRootVar, StringComparison.OrdinalIgnoreCase)) + if (value.Contains(ConfigRootVar, StringComparison.OrdinalIgnoreCase)) { - if (string.IsNullOrEmpty(this.configurationFilePath)) + if (string.IsNullOrEmpty(this.configurationFileRootPath)) { throw new UnitSettingConfigRootException(this.Unit.UnitName, settingName); } - if (!File.Exists(this.configurationFilePath)) - { - throw new FileNotFoundException(this.configurationFilePath); - } - - var configRoot = Path.GetDirectoryName(this.configurationFilePath); - if (configRoot == null) + if (this.configurationFileRootPath == null) { throw new ArgumentException(); } - return value.Replace(configRootVar, configRoot, StringComparison.OrdinalIgnoreCase); + return value.Replace(ConfigRootVar, this.configurationFileRootPath, StringComparison.OrdinalIgnoreCase); } } diff --git a/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationUnitInternalTests.cs b/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationUnitInternalTests.cs index a9c0759294..e48f80b76b 100644 --- a/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationUnitInternalTests.cs +++ b/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationUnitInternalTests.cs @@ -122,12 +122,12 @@ public void GetExpandedSettings_ConfigRoot() using var tmpFile = new TempFile("fakeConfigFile.yml", content: "content"); var unit = new ConfigurationUnit(); - unit.Settings.Add("var1", @"WinGetConfigRoot\this\is\a\path.txt"); - unit.Settings.Add("var2", @"$WinGetConfigRoot\this\is\a\path.txt"); - unit.Settings.Add("var3", @"this\is\a\WINGETCONFIGROOT\path.txt"); - unit.Settings.Add("var4", @"this\is\a\$WINGETCONFIGROOT\path.txt"); - unit.Settings.Add("var5", @"this\is\a\path\wingetconfigroot"); - unit.Settings.Add("var6", @"this\is\a\path\$wingetconfigroot"); + unit.Settings.Add("var1", @"$WinGetConfigRoot\this\is\a\path.txt"); + unit.Settings.Add("var2", @"${WinGetConfigRoot}\this\is\a\path.txt"); + unit.Settings.Add("var3", @"this\is\a\$WINGETCONFIGROOT\path.txt"); + unit.Settings.Add("var4", @"this\is\a\${WINGETCONFIGROOT}\path.txt"); + unit.Settings.Add("var5", @"this\is\a\path\$wingetconfigroot"); + unit.Settings.Add("var6", @"this\is\a\path\${wingetconfigroot}"); string configPath = tmpFile.FullFileName; string? expectedPath = Path.GetDirectoryName(configPath); @@ -136,19 +136,19 @@ public void GetExpandedSettings_ConfigRoot() var expandedSettings = unitInternal.GetExpandedSettings(); var var1 = expandedSettings["var1"]; - Assert.Equal(@"WinGetConfigRoot\this\is\a\path.txt", var1 as string); + Assert.Equal(@"$WinGetConfigRoot\this\is\a\path.txt", var1 as string); var var2 = expandedSettings["var2"]; Assert.Equal($@"{expectedPath}\this\is\a\path.txt", var2 as string); var var3 = expandedSettings["var3"]; - Assert.Equal(@"this\is\a\WINGETCONFIGROOT\path.txt", var3 as string); + Assert.Equal(@"this\is\a\$WINGETCONFIGROOT\path.txt", var3 as string); var var4 = expandedSettings["var4"]; Assert.Equal($@"this\is\a\{expectedPath}\path.txt", var4 as string); var var5 = expandedSettings["var5"]; - Assert.Equal(@"this\is\a\path\wingetconfigroot", var5 as string); + Assert.Equal(@"this\is\a\path\$wingetconfigroot", var5 as string); var var6 = expandedSettings["var6"]; Assert.Equal($@"this\is\a\path\{expectedPath}", var6 as string); @@ -161,7 +161,7 @@ public void GetExpandedSettings_ConfigRoot() public void GetExpandedSetting_ConfigRoot_Throw() { var unit = new ConfigurationUnit(); - unit.Settings.Add("var2", @"$WinGetConfigRoot\this\is\a\path.txt"); + unit.Settings.Add("var2", @"${WinGetConfigRoot}\this\is\a\path.txt"); var unitInternal = new ConfigurationUnitInternal(unit, null!); Assert.Throws(() => unitInternal.GetExpandedSettings());