commit b8a99f5845cc431a49f0052a3466c48a592fd836
parent 3887bc8188f9dc6ebddecc685a29273edf4962b7
Author: Ryan Fu <69221034+ryfu-msft@users.noreply.github.com>
Date: Tue, 7 Feb 2023 08:28:00 -0800
Fix behavior for user settings scope preference/requirement for portable install (#2918)
Diffstat:
9 files changed, 159 insertions(+), 29 deletions(-)
diff --git a/src/AppInstallerCLICore/Workflows/ManifestComparator.cpp b/src/AppInstallerCLICore/Workflows/ManifestComparator.cpp
@@ -364,7 +364,7 @@ namespace AppInstaller::CLI::Workflow
static std::unique_ptr<ScopeComparator> Create(const Execution::Context& context)
{
// Preference will always come from settings
- Manifest::ScopeEnum preference = ConvertScope(Settings::User().Get<Settings::Setting::InstallScopePreference>());
+ Manifest::ScopeEnum preference = Settings::User().Get<Settings::Setting::InstallScopePreference>();
// Requirement may come from args or settings; args overrides settings.
Manifest::ScopeEnum requirement = Manifest::ScopeEnum::Unknown;
@@ -376,7 +376,7 @@ namespace AppInstaller::CLI::Workflow
}
else
{
- requirement = ConvertScope(Settings::User().Get<Settings::Setting::InstallScopeRequirement>());
+ requirement = Settings::User().Get<Settings::Setting::InstallScopeRequirement>();
}
bool allowUnknownInAdditionToRequired = false;
@@ -434,18 +434,6 @@ namespace AppInstaller::CLI::Workflow
}
private:
- static Manifest::ScopeEnum ConvertScope(Settings::ScopePreference scope)
- {
- switch (scope)
- {
- case Settings::ScopePreference::None: return Manifest::ScopeEnum::Unknown;
- case Settings::ScopePreference::User: return Manifest::ScopeEnum::User;
- case Settings::ScopePreference::Machine: return Manifest::ScopeEnum::Machine;
- }
-
- return Manifest::ScopeEnum::Unknown;
- }
-
Manifest::ScopeEnum m_preference;
Manifest::ScopeEnum m_requirement;
bool m_allowUnknownInAdditionToRequired;
diff --git a/src/AppInstallerCLICore/Workflows/PortableFlow.cpp b/src/AppInstallerCLICore/Workflows/PortableFlow.cpp
@@ -122,7 +122,17 @@ namespace AppInstaller::CLI::Workflow
}
else
{
- scope = Manifest::ConvertToScopeEnum(context.Args.GetArg(Execution::Args::Type::InstallScope));
+ if (context.Args.Contains(Execution::Args::Type::InstallScope))
+ {
+ scope = Manifest::ConvertToScopeEnum(context.Args.GetArg(Execution::Args::Type::InstallScope));
+ }
+ else
+ {
+ Manifest::ScopeEnum requiredScope = Settings::User().Get<Settings::Setting::InstallScopeRequirement>();
+ Manifest::ScopeEnum preferredScope = Settings::User().Get<Settings::Setting::InstallScopePreference>();
+
+ scope = requiredScope != Manifest::ScopeEnum::Unknown ? requiredScope : preferredScope;
+ }
}
Utility::Architecture arch = context.Get<Execution::Data::Installer>()->Arch;
diff --git a/src/AppInstallerCLIE2ETests/Constants.cs b/src/AppInstallerCLIE2ETests/Constants.cs
@@ -111,6 +111,7 @@ namespace AppInstallerCLIE2ETests
// User settings
public const string PortablePackageUserRoot = "portablePackageUserRoot";
public const string PortablePackageMachineRoot = "portablePackageMachineRoot";
+ public const string InstallBehaviorScope = "scope";
/// <summary>
/// Error codes.
diff --git a/src/AppInstallerCLIE2ETests/InstallCommand.cs b/src/AppInstallerCLIE2ETests/InstallCommand.cs
@@ -439,6 +439,50 @@ namespace AppInstallerCLIE2ETests
}
/// <summary>
+ /// Test install portable package with settings set to user install scope.
+ /// </summary>
+ [Test]
+ public void InstallPortable_InstallScopePreference_User()
+ {
+ string installDir = TestCommon.GetRandomTestDir();
+ WinGetSettingsHelper.ConfigureInstallBehavior(Constants.PortablePackageUserRoot, installDir);
+ WinGetSettingsHelper.ConfigureInstallBehaviorPreferences(Constants.InstallBehaviorScope, "user");
+
+ string packageId, commandAlias, fileName, packageDirName, productCode;
+ packageId = "AppInstallerTest.TestPortableExe";
+ packageDirName = productCode = packageId + "_" + Constants.TestSourceIdentifier;
+ commandAlias = fileName = "AppInstallerTestExeInstaller.exe";
+
+ var result = TestCommon.RunAICLICommand("install", $"{packageId}");
+ WinGetSettingsHelper.ConfigureInstallBehavior(Constants.PortablePackageUserRoot, string.Empty);
+ Assert.AreEqual(Constants.ErrorCode.S_OK, result.ExitCode);
+ Assert.True(result.StdOut.Contains("Successfully installed"));
+ TestCommon.VerifyPortablePackage(Path.Combine(installDir, packageDirName), commandAlias, fileName, productCode, true);
+ }
+
+ /// <summary>
+ /// Test install portable package with settings set to machine install scope.
+ /// </summary>
+ [Test]
+ public void InstallPortable_InstallScopePreference_Machine()
+ {
+ string installDir = TestCommon.GetRandomTestDir();
+ WinGetSettingsHelper.ConfigureInstallBehavior(Constants.PortablePackageMachineRoot, installDir);
+ WinGetSettingsHelper.ConfigureInstallBehaviorPreferences(Constants.InstallBehaviorScope, "machine");
+
+ string packageId, commandAlias, fileName, packageDirName, productCode;
+ packageId = "AppInstallerTest.TestPortableExe";
+ packageDirName = productCode = packageId + "_" + Constants.TestSourceIdentifier;
+ commandAlias = fileName = "AppInstallerTestExeInstaller.exe";
+
+ var result = TestCommon.RunAICLICommand("install", $"{packageId}");
+ WinGetSettingsHelper.ConfigureInstallBehavior(Constants.PortablePackageMachineRoot, string.Empty);
+ Assert.AreEqual(Constants.ErrorCode.S_OK, result.ExitCode);
+ Assert.True(result.StdOut.Contains("Successfully installed"));
+ TestCommon.VerifyPortablePackage(Path.Combine(installDir, packageDirName), commandAlias, fileName, productCode, true, TestCommon.Scope.Machine);
+ }
+
+ /// <summary>
/// Test install zip exe.
/// </summary>
[Test]
diff --git a/src/AppInstallerCLIE2ETests/WinGetSettingsHelper.cs b/src/AppInstallerCLIE2ETests/WinGetSettingsHelper.cs
@@ -126,6 +126,60 @@ namespace AppInstallerCLIE2ETests
}
/// <summary>
+ /// Configure the install behavior preferences.
+ /// </summary>
+ /// <param name="settingName">Setting name.</param>
+ /// <param name="value">Setting value.</param>
+ public static void ConfigureInstallBehaviorPreferences(string settingName, string value)
+ {
+ JObject settingsJson = JObject.Parse(File.ReadAllText(TestCommon.SettingsJsonFilePath));
+
+ if (!settingsJson.ContainsKey("installBehavior"))
+ {
+ settingsJson["installBehavior"] = new JObject();
+ }
+
+ var installBehavior = settingsJson["installBehavior"];
+
+ if (installBehavior["preferences"] == null)
+ {
+ installBehavior["preferences"] = new JObject();
+ }
+
+ var preferences = installBehavior["preferences"];
+ preferences[settingName] = value;
+
+ File.WriteAllText(TestCommon.SettingsJsonFilePath, settingsJson.ToString());
+ }
+
+ /// <summary>
+ /// Configure the install behavior requirements.
+ /// </summary>
+ /// <param name="settingName">Setting name.</param>
+ /// <param name="value">Setting value.</param>
+ public static void ConfigureInstallBehaviorRequirements(string settingName, string value)
+ {
+ JObject settingsJson = JObject.Parse(File.ReadAllText(TestCommon.SettingsJsonFilePath));
+
+ if (!settingsJson.ContainsKey("installBehavior"))
+ {
+ settingsJson["installBehavior"] = new JObject();
+ }
+
+ var installBehavior = settingsJson["installBehavior"];
+
+ if (installBehavior["requirements"] == null)
+ {
+ installBehavior["requirements"] = new JObject();
+ }
+
+ var requirements = installBehavior["requirements"];
+ requirements[settingName] = value;
+
+ File.WriteAllText(TestCommon.SettingsJsonFilePath, settingsJson.ToString());
+ }
+
+ /// <summary>
/// Initialize all features.
/// </summary>
/// <param name="status">Initialized feature value.</param>
diff --git a/src/AppInstallerCLITests/ManifestComparator.cpp b/src/AppInstallerCLITests/ManifestComparator.cpp
@@ -313,7 +313,7 @@ TEST_CASE("ManifestComparator_ScopeCompare", "[manifest_comparator]")
SECTION("User Preference")
{
TestUserSettings settings;
- settings.Set<Setting::InstallScopePreference>(ScopePreference::User);
+ settings.Set<Setting::InstallScopePreference>(ScopeEnum::User);
ManifestComparator mc(ManifestComparatorTestContext{}, {});
auto [result, inapplicabilities] = mc.GetPreferredInstaller(manifest);
@@ -324,7 +324,7 @@ TEST_CASE("ManifestComparator_ScopeCompare", "[manifest_comparator]")
SECTION("Machine Preference")
{
TestUserSettings settings;
- settings.Set<Setting::InstallScopePreference>(ScopePreference::Machine);
+ settings.Set<Setting::InstallScopePreference>(ScopeEnum::Machine);
ManifestComparator mc(ManifestComparatorTestContext{}, {});
auto [result, inapplicabilities] = mc.GetPreferredInstaller(manifest);
diff --git a/src/AppInstallerCLITests/UserSettings.cpp b/src/AppInstallerCLITests/UserSettings.cpp
@@ -469,3 +469,43 @@ TEST_CASE("SettingsPortablePackageMachineRoot", "[settings]")
REQUIRE(userSettingTest.GetWarnings().size() == 0);
}
}
+
+TEST_CASE("SettingsInstallScope", "[settings]")
+{
+ SECTION("User scope preference")
+ {
+ DeleteUserSettingsFiles();
+ std::string_view json = R"({ "installBehavior": { "preferences": { "scope": "user" } } })";
+ SetSetting(Stream::PrimaryUserSettings, json);
+ UserSettingsTest userSettingTest;
+
+ REQUIRE(userSettingTest.Get<Setting::InstallScopePreference>() == AppInstaller::Manifest::ScopeEnum::User);
+ }
+ SECTION("Machine scope preference")
+ {
+ DeleteUserSettingsFiles();
+ std::string_view json = R"({ "installBehavior": { "preferences": { "scope": "machine" } } })";
+ SetSetting(Stream::PrimaryUserSettings, json);
+ UserSettingsTest userSettingTest;
+
+ REQUIRE(userSettingTest.Get<Setting::InstallScopePreference>() == AppInstaller::Manifest::ScopeEnum::Machine);
+ }
+ SECTION("User scope requirement")
+ {
+ DeleteUserSettingsFiles();
+ std::string_view json = R"({ "installBehavior": { "requirements": { "scope": "user" } } })";
+ SetSetting(Stream::PrimaryUserSettings, json);
+ UserSettingsTest userSettingTest;
+
+ REQUIRE(userSettingTest.Get<Setting::InstallScopeRequirement>() == AppInstaller::Manifest::ScopeEnum::User);
+ }
+ SECTION("Machine scope requirement")
+ {
+ DeleteUserSettingsFiles();
+ std::string_view json = R"({ "installBehavior": { "requirements": { "scope": "machine" } } })";
+ SetSetting(Stream::PrimaryUserSettings, json);
+ UserSettingsTest userSettingTest;
+
+ REQUIRE(userSettingTest.Get<Setting::InstallScopeRequirement>() == AppInstaller::Manifest::ScopeEnum::Machine);
+ }
+}
diff --git a/src/AppInstallerCommonCore/Public/winget/UserSettings.h b/src/AppInstallerCommonCore/Public/winget/UserSettings.h
@@ -5,6 +5,7 @@
#include "AppInstallerLogging.h"
#include "winget/GroupPolicy.h"
#include "winget/Resources.h"
+#include "winget/ManifestCommon.h"
#include <filesystem>
#include <map>
@@ -43,14 +44,6 @@ namespace AppInstaller::Settings
Rainbow,
};
- // The preferred scope for installs.
- enum class ScopePreference
- {
- None,
- User,
- Machine,
- };
-
// The download code to use for *installers*.
enum class InstallerDownloader
{
@@ -153,8 +146,8 @@ namespace AppInstaller::Settings
// Install behavior
SETTINGMAPPING_SPECIALIZATION(Setting::InstallArchitecturePreference, std::vector<std::string>, std::vector<Utility::Architecture>, {}, ".installBehavior.preferences.architectures"sv);
SETTINGMAPPING_SPECIALIZATION(Setting::InstallArchitectureRequirement, std::vector<std::string>, std::vector<Utility::Architecture>, {}, ".installBehavior.requirements.architectures"sv);
- SETTINGMAPPING_SPECIALIZATION(Setting::InstallScopePreference, std::string, ScopePreference, ScopePreference::User, ".installBehavior.preferences.scope"sv);
- SETTINGMAPPING_SPECIALIZATION(Setting::InstallScopeRequirement, std::string, ScopePreference, ScopePreference::None, ".installBehavior.requirements.scope"sv);
+ SETTINGMAPPING_SPECIALIZATION(Setting::InstallScopePreference, std::string, Manifest::ScopeEnum, Manifest::ScopeEnum::User, ".installBehavior.preferences.scope"sv);
+ SETTINGMAPPING_SPECIALIZATION(Setting::InstallScopeRequirement, std::string, Manifest::ScopeEnum, Manifest::ScopeEnum::Unknown, ".installBehavior.requirements.scope"sv);
SETTINGMAPPING_SPECIALIZATION(Setting::InstallLocalePreference, std::vector<std::string>, std::vector<std::string>, {}, ".installBehavior.preferences.locale"sv);
SETTINGMAPPING_SPECIALIZATION(Setting::InstallLocaleRequirement, std::vector<std::string>, std::vector<std::string>, {}, ".installBehavior.requirements.locale"sv);
SETTINGMAPPING_SPECIALIZATION(Setting::InstallIgnoreWarnings, bool, bool, false, ".installBehavior.ignoreWarnings"sv);
diff --git a/src/AppInstallerCommonCore/UserSettings.cpp b/src/AppInstallerCommonCore/UserSettings.cpp
@@ -305,11 +305,11 @@ namespace AppInstaller::Settings
if (Utility::CaseInsensitiveEquals(value, s_scope_user))
{
- return ScopePreference::User;
+ return Manifest::ScopeEnum::User;
}
else if (Utility::CaseInsensitiveEquals(value, s_scope_machine))
{
- return ScopePreference::Machine;
+ return Manifest::ScopeEnum::Machine;
}
return {};
