From 24bb391aa73dd3f1e3c06776cccbb94191290aac Mon Sep 17 00:00:00 2001 From: AmirMS <104940545+AmelBawa-msft@users.noreply.github.com> Date: Thu, 8 May 2025 10:49:19 -0700 Subject: [PATCH 1/6] Make partial the default --- .../Commands/DscUserSettingsFileResource.cpp | 4 +- .../DSCv3UserSettingsFileResourceCommand.cs | 61 +++++++++++++------ 2 files changed, 45 insertions(+), 20 deletions(-) diff --git a/src/AppInstallerCLICore/Commands/DscUserSettingsFileResource.cpp b/src/AppInstallerCLICore/Commands/DscUserSettingsFileResource.cpp index 27a44e4ad7..e03b30f0f1 100644 --- a/src/AppInstallerCLICore/Commands/DscUserSettingsFileResource.cpp +++ b/src/AppInstallerCLICore/Commands/DscUserSettingsFileResource.cpp @@ -16,7 +16,7 @@ namespace AppInstaller::CLI namespace { WINGET_DSC_DEFINE_COMPOSABLE_PROPERTY_FLAGS(SettingsProperty, Json::Value, Settings, "settings", DscComposablePropertyFlag::Required | DscComposablePropertyFlag::CopyToOutput, Resource::String::DscResourcePropertyDescriptionUserSettingsFileSettings); - WINGET_DSC_DEFINE_COMPOSABLE_PROPERTY_ENUM(ActionProperty, std::string, Action, "action", Resource::String::DscResourcePropertyDescriptionUserSettingsFileAction, ({ ACTION_PARTIAL, ACTION_FULL }), ACTION_FULL); + WINGET_DSC_DEFINE_COMPOSABLE_PROPERTY_ENUM(ActionProperty, std::string, Action, "action", Resource::String::DscResourcePropertyDescriptionUserSettingsFileAction, ({ ACTION_PARTIAL, ACTION_FULL }), ACTION_PARTIAL); using UserSettingsFileResourceObject = DscComposableObject; @@ -38,7 +38,7 @@ namespace AppInstaller::CLI void Get() { - Output.Action(ACTION_FULL); + Output.Action(ACTION_PARTIAL); Output.Settings(GetUserSettings()); } diff --git a/src/AppInstallerCLIE2ETests/DSCv3UserSettingsFileResourceCommand.cs b/src/AppInstallerCLIE2ETests/DSCv3UserSettingsFileResourceCommand.cs index b798243759..f3c28bd8d1 100644 --- a/src/AppInstallerCLIE2ETests/DSCv3UserSettingsFileResourceCommand.cs +++ b/src/AppInstallerCLIE2ETests/DSCv3UserSettingsFileResourceCommand.cs @@ -61,7 +61,7 @@ public void Setup() } /// - /// Calls `get` on the `user-settings` resource. + /// Calls `get` on the `user-settings-file` resource. /// [Test] public void UserSettingsFile_Get() @@ -70,12 +70,12 @@ public void UserSettingsFile_Get() var getOutput = Get(new ()); Assert.IsNotNull(getOutput); - Assert.AreEqual(ActionPropertyValueFull, getOutput.Action); + Assert.AreEqual(ActionPropertyValuePartial, getOutput.Action); AssertSettingsAreEqual(expected, getOutput.Settings); } /// - /// Calls `set` on the `user-settings` resource with no diff. + /// Calls `set` on the `user-settings-file` resource with no diff. /// /// The action value. [Test] @@ -90,13 +90,13 @@ public void UserSettingsFile_Set_NoDiff(string action) var expected = GetCurrentUserSettings(); Assert.IsNotNull(setOutput); - Assert.AreEqual(ActionPropertyValueFull, setOutput.Action); + Assert.AreEqual(ActionPropertyValuePartial, setOutput.Action); AssertSettingsAreEqual(expected, setOutput.Settings); AssertDiffState(setDiff, []); } /// - /// Calls `set` on the `user-settings` resource to add fields. + /// Calls `set` on the `user-settings-file` resource to add fields. /// /// The action value. [Test] @@ -114,14 +114,38 @@ public void UserSettingsFile_Set_AddFields(string action) // Assert that the settings are added Assert.IsNotNull(setOutput); - Assert.AreEqual(ActionPropertyValueFull, setOutput.Action); + Assert.AreEqual(ActionPropertyValuePartial, setOutput.Action); AssertMockProperties(setOutput.Settings, "mock"); AssertSettingsAreEqual(expected, setOutput.Settings); AssertDiffState(setDiff, [ SettingsPropertyName ]); } /// - /// Calls `set` on the `user-settings` resource to update fields. + /// Calls `set` on the `user-settings-file` resource to ensure action is partial by default. + /// + [Test] + public void UserSettingsFile_Set_ActionIsPartialByDefault() + { + // Call `set` to add mock properties to the settings + var setSettings = GetSettingsArg(ActionPropertyValuePartial); + AddOrModifyMockProperties(setSettings, "mock"); + + var expected = GetCurrentUserSettings(); + AddOrModifyMockProperties(expected, "mock"); + + (var setOutput, var setDiff) = Set(new () { Settings = setSettings }); + + + // Assert that the settings are added + Assert.IsNotNull(setOutput); + Assert.AreEqual(ActionPropertyValuePartial, setOutput.Action); + AssertMockProperties(setOutput.Settings, "mock"); + AssertSettingsAreEqual(expected, setOutput.Settings); + AssertDiffState(setDiff, [ SettingsPropertyName ]); + } + + /// + /// Calls `set` on the `user-settings-file` resource to update fields. /// /// The action value. [Test] @@ -144,14 +168,14 @@ public void UserSettingsFile_Set_UpdateFields(string action) // Assert that the settings are updated Assert.IsNotNull(setOutput); - Assert.AreEqual(ActionPropertyValueFull, setOutput.Action); + Assert.AreEqual(ActionPropertyValuePartial, setOutput.Action); AssertMockProperties(setOutput.Settings, "mock_new"); AssertSettingsAreEqual(expected, setOutput.Settings); AssertDiffState(setDiff, [ SettingsPropertyName ]); } /// - /// Calls `test` on the `user-settings` resource to check if the settings are in desired state. + /// Calls `test` on the `user-settings-file` resource to check if the settings are in desired state. /// /// The action value. [Test] @@ -174,7 +198,7 @@ public void UserSettingsFile_Test_InDesiredState(string action) // Assert that the settings are in desired state Assert.IsNotNull(testOutput); - Assert.AreEqual(ActionPropertyValueFull, testOutput.Action); + Assert.AreEqual(ActionPropertyValuePartial, testOutput.Action); AssertMockProperties(testOutput.Settings, "mock"); AssertSettingsAreEqual(expected, testOutput.Settings); Assert.IsTrue(testOutput.InDesiredState); @@ -182,7 +206,7 @@ public void UserSettingsFile_Test_InDesiredState(string action) } /// - /// Calls `test` on the `user-settings` resource to check if the settings are not in desired state. + /// Calls `test` on the `user-settings-file` resource to check if the settings are not in desired state. /// /// The action value. [Test] @@ -205,7 +229,7 @@ public void UserSettingsFile_Test_NotInDesiredState(string action) // Assert that the settings are not in desired state Assert.IsNotNull(testOutput); - Assert.AreEqual(ActionPropertyValueFull, testOutput.Action); + Assert.AreEqual(ActionPropertyValuePartial, testOutput.Action); AssertMockProperties(testOutput.Settings, "mock_set"); AssertSettingsAreEqual(expected, testOutput.Settings); Assert.IsFalse(testOutput.InDesiredState); @@ -213,7 +237,7 @@ public void UserSettingsFile_Test_NotInDesiredState(string action) } /// - /// Calls `export` on the `user-settings` resource to export the settings. + /// Calls `export` on the `user-settings-file` resource to export the settings. /// [Test] public void UserSettingsFile_Export() @@ -222,12 +246,12 @@ public void UserSettingsFile_Export() var exportOutput = Export(new ()); Assert.IsNotNull(exportOutput); - Assert.AreEqual(ActionPropertyValueFull, exportOutput.Action); + Assert.AreEqual(ActionPropertyValuePartial, exportOutput.Action); AssertSettingsAreEqual(expected, exportOutput.Settings); } /// - /// Calls `get` on the `user-settings` resource. + /// Calls `get` on the `user-settings-file` resource. /// /// The input resource data. /// The output resource data. @@ -239,7 +263,7 @@ private static UserSettingsFileResourceData Get(UserSettingsFileResourceData res } /// - /// Calls `set` on the `user-settings` resource. + /// Calls `set` on the `user-settings-file` resource. /// /// The input resource data. /// The output resource data and the diff. @@ -251,7 +275,7 @@ private static (UserSettingsFileResourceData, List) Set(UserSettingsFile } /// - /// Calls `test` on the `user-settings` resource. + /// Calls `test` on the `user-settings-file` resource. /// /// The input resource data. /// The output resource data and the diff. @@ -263,7 +287,7 @@ private static (UserSettingsFileResourceData, List) Test(UserSettingsFil } /// - /// Calls `export` on the `user-settings` resource. + /// Calls `export` on the `user-settings-file` resource. /// /// The input resource data. /// The output resource data. @@ -333,6 +357,7 @@ private class UserSettingsFileResourceData [JsonPropertyName(InDesiredStatePropertyName)] public bool? InDesiredState { get; set; } + [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingDefault)] public string Action { get; set; } public JsonObject Settings { get; set; } From 86fca70ccc1df47f17528f466cdad7f452ec08b0 Mon Sep 17 00:00:00 2001 From: AmirMS <104940545+AmelBawa-msft@users.noreply.github.com> Date: Thu, 8 May 2025 15:50:01 -0700 Subject: [PATCH 2/6] Case in --- .../Commands/DscUserSettingsFileResource.cpp | 9 +++++++-- .../DSCv3UserSettingsFileResourceCommand.cs | 16 ++++++++-------- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/AppInstallerCLICore/Commands/DscUserSettingsFileResource.cpp b/src/AppInstallerCLICore/Commands/DscUserSettingsFileResource.cpp index e03b30f0f1..ebaa5fbb4b 100644 --- a/src/AppInstallerCLICore/Commands/DscUserSettingsFileResource.cpp +++ b/src/AppInstallerCLICore/Commands/DscUserSettingsFileResource.cpp @@ -4,6 +4,7 @@ #include "DscUserSettingsFileResource.h" #include "DscComposableObject.h" #include "Resources.h" +#include "AppInstallerStrings.h" using namespace AppInstaller::Utility::literals; using namespace AppInstaller::Settings; @@ -31,6 +32,11 @@ namespace AppInstaller::CLI Input(json, ignoreFieldRequirements), _userSettingsPath(UserSettings::SettingsFilePath()) { + const auto& action = Input.Action(); + if (action.has_value() && (Utility::CaseInsensitiveEquals(action.value(), ACTION_FULL) || Utility::CaseInsensitiveEquals(action.value(), ACTION_PARTIAL))) + { + Output.Action(Input.Action()); + } } const UserSettingsFileResourceObject Input; @@ -38,7 +44,6 @@ namespace AppInstaller::CLI void Get() { - Output.Action(ACTION_PARTIAL); Output.Settings(GetUserSettings()); } @@ -64,7 +69,7 @@ namespace AppInstaller::CLI THROW_HR_IF(E_UNEXPECTED, !Input.Settings().has_value()); if (!_resolvedInputUserSettings) { - if(Input.Action() == ACTION_FULL) + if(Input.Action().has_value() && Utility::CaseInsensitiveEquals(Input.Action().value(), ACTION_FULL)) { _resolvedInputUserSettings = Input.Settings(); } diff --git a/src/AppInstallerCLIE2ETests/DSCv3UserSettingsFileResourceCommand.cs b/src/AppInstallerCLIE2ETests/DSCv3UserSettingsFileResourceCommand.cs index f3c28bd8d1..b190469fc6 100644 --- a/src/AppInstallerCLIE2ETests/DSCv3UserSettingsFileResourceCommand.cs +++ b/src/AppInstallerCLIE2ETests/DSCv3UserSettingsFileResourceCommand.cs @@ -70,7 +70,7 @@ public void UserSettingsFile_Get() var getOutput = Get(new ()); Assert.IsNotNull(getOutput); - Assert.AreEqual(ActionPropertyValuePartial, getOutput.Action); + Assert.IsNull(getOutput.Action); AssertSettingsAreEqual(expected, getOutput.Settings); } @@ -90,7 +90,7 @@ public void UserSettingsFile_Set_NoDiff(string action) var expected = GetCurrentUserSettings(); Assert.IsNotNull(setOutput); - Assert.AreEqual(ActionPropertyValuePartial, setOutput.Action); + Assert.AreEqual(action, setOutput.Action); AssertSettingsAreEqual(expected, setOutput.Settings); AssertDiffState(setDiff, []); } @@ -114,7 +114,7 @@ public void UserSettingsFile_Set_AddFields(string action) // Assert that the settings are added Assert.IsNotNull(setOutput); - Assert.AreEqual(ActionPropertyValuePartial, setOutput.Action); + Assert.AreEqual(action, setOutput.Action); AssertMockProperties(setOutput.Settings, "mock"); AssertSettingsAreEqual(expected, setOutput.Settings); AssertDiffState(setDiff, [ SettingsPropertyName ]); @@ -138,7 +138,7 @@ public void UserSettingsFile_Set_ActionIsPartialByDefault() // Assert that the settings are added Assert.IsNotNull(setOutput); - Assert.AreEqual(ActionPropertyValuePartial, setOutput.Action); + Assert.IsNull(setOutput.Action); AssertMockProperties(setOutput.Settings, "mock"); AssertSettingsAreEqual(expected, setOutput.Settings); AssertDiffState(setDiff, [ SettingsPropertyName ]); @@ -168,7 +168,7 @@ public void UserSettingsFile_Set_UpdateFields(string action) // Assert that the settings are updated Assert.IsNotNull(setOutput); - Assert.AreEqual(ActionPropertyValuePartial, setOutput.Action); + Assert.AreEqual(action, setOutput.Action); AssertMockProperties(setOutput.Settings, "mock_new"); AssertSettingsAreEqual(expected, setOutput.Settings); AssertDiffState(setDiff, [ SettingsPropertyName ]); @@ -198,7 +198,7 @@ public void UserSettingsFile_Test_InDesiredState(string action) // Assert that the settings are in desired state Assert.IsNotNull(testOutput); - Assert.AreEqual(ActionPropertyValuePartial, testOutput.Action); + Assert.AreEqual(action, testOutput.Action); AssertMockProperties(testOutput.Settings, "mock"); AssertSettingsAreEqual(expected, testOutput.Settings); Assert.IsTrue(testOutput.InDesiredState); @@ -229,7 +229,7 @@ public void UserSettingsFile_Test_NotInDesiredState(string action) // Assert that the settings are not in desired state Assert.IsNotNull(testOutput); - Assert.AreEqual(ActionPropertyValuePartial, testOutput.Action); + Assert.AreEqual(action, testOutput.Action); AssertMockProperties(testOutput.Settings, "mock_set"); AssertSettingsAreEqual(expected, testOutput.Settings); Assert.IsFalse(testOutput.InDesiredState); @@ -246,7 +246,7 @@ public void UserSettingsFile_Export() var exportOutput = Export(new ()); Assert.IsNotNull(exportOutput); - Assert.AreEqual(ActionPropertyValuePartial, exportOutput.Action); + Assert.IsNull(exportOutput.Action); AssertSettingsAreEqual(expected, exportOutput.Settings); } From 7ba413c8f7046cb820d91d989ae279be265d175e Mon Sep 17 00:00:00 2001 From: AmirMS <104940545+AmelBawa-msft@users.noreply.github.com> Date: Thu, 8 May 2025 16:05:59 -0700 Subject: [PATCH 3/6] Addressing comments --- .../Commands/DscUserSettingsFileResource.cpp | 8 ++++++-- .../DSCv3UserSettingsFileResourceCommand.cs | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/AppInstallerCLICore/Commands/DscUserSettingsFileResource.cpp b/src/AppInstallerCLICore/Commands/DscUserSettingsFileResource.cpp index ebaa5fbb4b..4947fb265a 100644 --- a/src/AppInstallerCLICore/Commands/DscUserSettingsFileResource.cpp +++ b/src/AppInstallerCLICore/Commands/DscUserSettingsFileResource.cpp @@ -33,9 +33,13 @@ namespace AppInstaller::CLI _userSettingsPath(UserSettings::SettingsFilePath()) { const auto& action = Input.Action(); - if (action.has_value() && (Utility::CaseInsensitiveEquals(action.value(), ACTION_FULL) || Utility::CaseInsensitiveEquals(action.value(), ACTION_PARTIAL))) + if (action.has_value() && Utility::CaseInsensitiveEquals(action.value(), ACTION_FULL)) { - Output.Action(Input.Action()); + Output.Action(ACTION_FULL); + } + else + { + Output.Action(ACTION_PARTIAL); } } diff --git a/src/AppInstallerCLIE2ETests/DSCv3UserSettingsFileResourceCommand.cs b/src/AppInstallerCLIE2ETests/DSCv3UserSettingsFileResourceCommand.cs index b190469fc6..e34106d60a 100644 --- a/src/AppInstallerCLIE2ETests/DSCv3UserSettingsFileResourceCommand.cs +++ b/src/AppInstallerCLIE2ETests/DSCv3UserSettingsFileResourceCommand.cs @@ -138,7 +138,7 @@ public void UserSettingsFile_Set_ActionIsPartialByDefault() // Assert that the settings are added Assert.IsNotNull(setOutput); - Assert.IsNull(setOutput.Action); + Assert.AreEqual(setOutput.Action, ActionPropertyValuePartial); AssertMockProperties(setOutput.Settings, "mock"); AssertSettingsAreEqual(expected, setOutput.Settings); AssertDiffState(setDiff, [ SettingsPropertyName ]); From 4721404faa70514f48f6920068f177fdbc303654 Mon Sep 17 00:00:00 2001 From: AmirMS <104940545+AmelBawa-msft@users.noreply.github.com> Date: Thu, 8 May 2025 16:07:41 -0700 Subject: [PATCH 4/6] Addressing comments --- .../Commands/DscUserSettingsFileResource.cpp | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/AppInstallerCLICore/Commands/DscUserSettingsFileResource.cpp b/src/AppInstallerCLICore/Commands/DscUserSettingsFileResource.cpp index 4947fb265a..59e427e0fe 100644 --- a/src/AppInstallerCLICore/Commands/DscUserSettingsFileResource.cpp +++ b/src/AppInstallerCLICore/Commands/DscUserSettingsFileResource.cpp @@ -33,13 +33,16 @@ namespace AppInstaller::CLI _userSettingsPath(UserSettings::SettingsFilePath()) { const auto& action = Input.Action(); - if (action.has_value() && Utility::CaseInsensitiveEquals(action.value(), ACTION_FULL)) + if (action.has_value()) { - Output.Action(ACTION_FULL); - } - else - { - Output.Action(ACTION_PARTIAL); + if (Utility::CaseInsensitiveEquals(action.value(), ACTION_FULL)) + { + Output.Action(ACTION_FULL); + } + else + { + Output.Action(ACTION_PARTIAL); + } } } From 62b6699fa0cc0f2e5d03b230718a0a85edfb1994 Mon Sep 17 00:00:00 2001 From: AmirMS <104940545+AmelBawa-msft@users.noreply.github.com> Date: Thu, 8 May 2025 16:14:04 -0700 Subject: [PATCH 5/6] More changes --- .../Commands/DscUserSettingsFileResource.cpp | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/src/AppInstallerCLICore/Commands/DscUserSettingsFileResource.cpp b/src/AppInstallerCLICore/Commands/DscUserSettingsFileResource.cpp index 59e427e0fe..1dfa846070 100644 --- a/src/AppInstallerCLICore/Commands/DscUserSettingsFileResource.cpp +++ b/src/AppInstallerCLICore/Commands/DscUserSettingsFileResource.cpp @@ -32,18 +32,6 @@ namespace AppInstaller::CLI Input(json, ignoreFieldRequirements), _userSettingsPath(UserSettings::SettingsFilePath()) { - const auto& action = Input.Action(); - if (action.has_value()) - { - if (Utility::CaseInsensitiveEquals(action.value(), ACTION_FULL)) - { - Output.Action(ACTION_FULL); - } - else - { - Output.Action(ACTION_PARTIAL); - } - } } const UserSettingsFileResourceObject Input; @@ -78,10 +66,12 @@ namespace AppInstaller::CLI { if(Input.Action().has_value() && Utility::CaseInsensitiveEquals(Input.Action().value(), ACTION_FULL)) { + Output.Action(ACTION_FULL); _resolvedInputUserSettings = Input.Settings(); } else { + Output.Action(ACTION_PARTIAL); _resolvedInputUserSettings = MergeUserSettingsFiles(*Input.Settings()); } } From 543c8dfd454c46abeff8486db5f6fd0edda9e25b Mon Sep 17 00:00:00 2001 From: AmirMS <104940545+AmelBawa-msft@users.noreply.github.com> Date: Thu, 8 May 2025 16:15:04 -0700 Subject: [PATCH 6/6] Typo --- .../DSCv3UserSettingsFileResourceCommand.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/AppInstallerCLIE2ETests/DSCv3UserSettingsFileResourceCommand.cs b/src/AppInstallerCLIE2ETests/DSCv3UserSettingsFileResourceCommand.cs index e34106d60a..c3f86c2692 100644 --- a/src/AppInstallerCLIE2ETests/DSCv3UserSettingsFileResourceCommand.cs +++ b/src/AppInstallerCLIE2ETests/DSCv3UserSettingsFileResourceCommand.cs @@ -135,7 +135,6 @@ public void UserSettingsFile_Set_ActionIsPartialByDefault() (var setOutput, var setDiff) = Set(new () { Settings = setSettings }); - // Assert that the settings are added Assert.IsNotNull(setOutput); Assert.AreEqual(setOutput.Action, ActionPropertyValuePartial);